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

net/udp: Relay full UdpMetadata instead of only remote endpoint in poll_ functions #2790

Merged

Conversation

chrysn
Copy link
Contributor

@chrysn chrysn commented Apr 8, 2024

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]

…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).
@chrysn chrysn force-pushed the prep-embedded-nal-async-udp branch from 55d4378 to 7f1bedc Compare April 8, 2024 09:59
chrysn added a commit to chrysn-pull-requests/embassy that referenced this pull request Apr 8, 2024
…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
chrysn added a commit to chrysn-pull-requests/embassy that referenced this pull request Apr 12, 2024
…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>> {
Copy link
Member

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)

Copy link
Contributor Author

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?

@chrysn
Copy link
Contributor Author

chrysn commented Apr 19, 2024

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.

Copy link
Member

@Dirbaio Dirbaio left a comment

Choose a reason for hiding this comment

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

lgtm, thakns!

@Dirbaio Dirbaio added this pull request to the merge queue Apr 26, 2024
Merged via the queue into embassy-rs:main with commit 026445d Apr 26, 2024
6 checks passed
@chrysn chrysn deleted the prep-embedded-nal-async-udp branch April 28, 2024 05:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants