Skip to content

drivers: modem: use zsock_ variants of socket API #26061

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

Merged
merged 1 commit into from
Jun 10, 2020
Merged

drivers: modem: use zsock_ variants of socket API #26061

merged 1 commit into from
Jun 10, 2020

Conversation

aport
Copy link
Contributor

@aport aport commented Jun 8, 2020

By using the Zephyr-native zsock_ family of types and functions, these
drivers will be decoupled from NET_SOCKETS_POSIX_NAMES.

This should help resolve part of #26033

Signed-off-by: Adam Porter [email protected]

@aport aport requested a review from mike-scott as a code owner June 8, 2020 20:47
@zephyrbot
Copy link
Collaborator

zephyrbot commented Jun 8, 2020

All checks passed.

Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages.

@jukkar jukkar requested review from jukkar and pfalcon June 9, 2020 12:30
@jukkar
Copy link
Member

jukkar commented Jun 9, 2020

Using zsock_... API is ok.
What is the exact issue why we need to do this, is it because the socket API gets defined twice or what?

Copy link
Collaborator

@pfalcon pfalcon left a comment

Choose a reason for hiding this comment

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

This change looks right, the idea/convention is that internally within Zephyr codebase, we use zsock_* namespaced API.

@pfalcon
Copy link
Collaborator

pfalcon commented Jun 9, 2020

@aport:

 -:34: WARNING:LONG_LINE: line over 80 characters
#34: FILE: drivers/modem/modem_socket.c:299:
 +		if (fds[i].events & ZSOCK_POLLIN && sock->packet_sizes[0] > 0U) {

Please deal with this line, it requires extra parens and wrapping.

By using the Zephyr-native zsock_ family of types and functions, these
drivers will be decoupled from NET_SOCKETS_POSIX_NAMES.

Signed-off-by: Adam Porter <[email protected]>
@aport
Copy link
Contributor Author

aport commented Jun 9, 2020

@jukkar There were two issues, first was that this driver used both zsock_ types and POSIX types interchangeably, so this converts the latter into the former for consistency.

Second issue was that the use of POSIX types forced the dependency on NET_SOCKETS_POSIX_NAMES which really should be a user option and not a driver dependency.

Currently NET_SOCKETS_OFFLOAD depends on NET_SOCKETS_POSIX_NAMES, presumably to get these drivers to compile. This change is one of several needed to cut that dependency.

At least with this change, I can compile my application without NET_SOCKETS_POSIX_NAMES.

Copy link
Contributor

@mike-scott mike-scott left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you for submitting this change.

@jukkar jukkar merged commit 9da0f2a into zephyrproject-rtos:master Jun 10, 2020
@aport aport deleted the modem-use-zsock-api branch June 10, 2020 19:02
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.

5 participants