-
Notifications
You must be signed in to change notification settings - Fork 934
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
net/udp: Relay full UdpMetadata instead of only remote endpoint in poll_ functions #2790
net/udp: Relay full UdpMetadata instead of only remote endpoint in poll_ functions #2790
Conversation
…ll_ functions This is a breaking change for users of the poll_ functions. (Some might not notice if they already pass in an IpEndpoint into poll_send_to, or discard that item in poll_recv_from).
55d4378
to
7f1bedc
Compare
…ll_ functions This is a breaking change for users of the poll_ functions. (Some might not notice if they already pass in an IpEndpoint into poll_send_to, or discard that item in poll_recv_from). Cherry-picked-from: embassy-rs#2790
…ll_ functions This is a breaking change for users of the poll_ functions. (Some might not notice if they already pass in an IpEndpoint into poll_send_to, or discard that item in poll_recv_from). Cherry-picked-from: embassy-rs#2790
&self, | ||
buf: &mut [u8], | ||
cx: &mut Context<'_>, | ||
) -> Poll<Result<(usize, UdpMetadata), RecvError>> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
recv_from
and poll_recv_from
should return the same. can you change recv_from
too?
(the convention is poll_*
functions are identical to their non-poll counterpart, except manually polled)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in a fixup; this even makes the total diff smaller. Shall I squash and force-push now or when reviews are done?
…t in poll_ functions
The merge of main into the branch (AIU this is the pattern used in this repo) fixed the nightly builds. The builds are fixed by providing a fixup as requested in #2790 (review) also for send_to, so that send_to is now consistent with poll_send_to, and another one that fixes unused import warnings. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, thakns!
This is a breaking change for users of the poll_ functions. (Some might not notice if they already pass in an IpEndpoint into poll_send_to, or discard that item in poll_recv_from).
It enables embassy users to utilize enhancements in smoltcp that are hopefully coming up in smoltcp-rs/smoltcp#904. Note that those two do not need to lockstep: We can change this now (without gaining many benefits, just access to the packet ID), and once smoltcp updates, the local address will be available as well.
This is the minimal change from #2519, and enables that to progress more easily -- once this one is in, the larger PR should even be testable without adding too many
[patches.crates-io]
to already complex setups.[edit: simplified link]