Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Zephyr] Remove redundant config and add implementation of POSIX API wrappers if necessary #37007

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

maciejbaczmanski
Copy link
Contributor

  • Zephyr now implements all required socket APIs, Remove redundant
    CHIP_SYSTEM_CONFIG_USE_ZEPHYR_SOCKET_EXTENSIONS config.

  • CONFIG_NET_SOCKETS_POSIX_NAMES has been deprecated in Zephyr
    in favor of CONFIG_POSIX_API.
    CONFIG_POSIX_API enables additional functionalities other than
    sockets, resulting in increased footprint.
    This commit reuses ZephyrSocket.h to include Zephyr socket
    header and allow building with both configs disabled, adding
    neccessary wrappers.

Zephyr now implements all required socket APIs, Remove redundant
`CHIP_SYSTEM_CONFIG_USE_ZEPHYR_SOCKET_EXTENSIONS` config.

Signed-off-by: Maciej Baczmanski <maciej.baczmanski@nordicsemi.no>
@maciejbaczmanski maciejbaczmanski requested review from a team as code owners January 9, 2025 09:34
Copy link

Review changes with  SemanticDiff

`CONFIG_NET_SOCKETS_POSIX_NAMES` has been deprecated in Zephyr
in favor of `CONFIG_POSIX_API`.
`CONFIG_POSIX_API` enables additional functionalities other than
sockets, resulting in increased footprint.
This commit reuses `ZephyrSocket.h` to include Zephyr socket
header and allow building with both configs disabled, adding
neccessary wrappers.
Copy link
Contributor

@Damian-Nordic Damian-Nordic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great.

Copy link
Contributor

@andy31415 andy31415 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a ### Testing section describing how things were tested that this still works? What commands did you run (assuming manual testing) and what was observed. Did you test all affected platforms or only one/some and assumed the changes will work everywhere? Are all platform owners of the affected platforms aware of the change?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants