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

doc: add notes that UDP receive functions must be called from same thread #4995

Merged
merged 2 commits into from
Mar 8, 2016

Conversation

OlegHahm
Copy link
Member

@OlegHahm OlegHahm commented Mar 8, 2016

fixes #4993.

@OlegHahm OlegHahm added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: network Area: Networking Area: doc Area: Documentation labels Mar 8, 2016
@OlegHahm OlegHahm added this to the Release 2016.04 milestone Mar 8, 2016
@@ -51,6 +51,9 @@ typedef struct conn_udp conn_udp_t;
* @param[in] family The family of @p addr (see @ref net_af).
* @param[in] port The local UDP port for @p conn.
*
* @note @ref conn_udp_recvfrom needs to be called from the same thread as
* for this function.
*
Copy link
Member

Choose a reason for hiding this comment

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

Again: only true for GNRC. #3551 and #4713 don't have this problem.

Copy link
Member

Choose a reason for hiding this comment

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

I would rather add these in form of @todo notes since these are faulty behavior of gnrc_conn_udp/gnrc_conn_ip

Copy link
Member Author

Choose a reason for hiding this comment

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

This documentation describes the current state, #3551 and #4713 are not yet merged and so gnrc is the only available implementation of conn. Futhermore, I couldn't find documentation explicitly for gnrc_conn_udp.

Copy link
Member

Choose a reason for hiding this comment

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

Just mention GNRC in the text. conn is conceptualized as general connection API. So regardless if other implementations exist or not: this is the current state in GNRC, and not an issue of this API.

Copy link
Member

Choose a reason for hiding this comment

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

And again: I want to strongly advice to use @todo here.

Copy link
Member

Choose a reason for hiding this comment

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

Futhermore, I couldn't find documentation explicitly for gnrc_conn_udp.

there is none. It's just the implementation of this API for GNRC and it should not have different behavior than documented here ;-)

@OlegHahm
Copy link
Member Author

OlegHahm commented Mar 8, 2016

addressed comments

@OlegHahm OlegHahm added the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Mar 8, 2016
@miri64
Copy link
Member

miri64 commented Mar 8, 2016

Maybe mention that this is faulty behavior and needs to be fixed... Otherwise ACK, please squash.

@OlegHahm OlegHahm force-pushed the conn_udp_doc_same_thread branch from c1f82e2 to b61b8e9 Compare March 8, 2016 18:49
@OlegHahm OlegHahm added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable labels Mar 8, 2016
@OlegHahm
Copy link
Member Author

OlegHahm commented Mar 8, 2016

addressed comments and squashed

OlegHahm added a commit that referenced this pull request Mar 8, 2016
doc: add notes that UDP receive functions must be called from same thread
@OlegHahm OlegHahm merged commit 2c33e69 into RIOT-OS:master Mar 8, 2016
@OlegHahm OlegHahm deleted the conn_udp_doc_same_thread branch March 8, 2016 23:17
@immesys
Copy link
Contributor

immesys commented Mar 9, 2016

Oh! I did not know this. What happens if you use different threads? I'm pretty sure I have a different thread for sending and receiving. Maybe that explains some of my bugs.

EDIT: sorry, this is just for connection oriented udp. Apologies for the noise

@OlegHahm
Copy link
Member Author

OlegHahm commented Mar 9, 2016

Well, the problem with gnrc_conn_udp() or the corresponding socket layer adaption is that the create function (conn_udp) respectively the sending function (POSIX) registers the thread at netreg. This means that only this thread will receive IPC messages, when a UDP datagram for the matching address and port (socket) is received. Otherwise these messages will get dropped.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: doc Area: Documentation Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

doc: conn_udp: document that send and receives needs to be called from the same thread
3 participants