Skip to content

Add sync methods to embassy-net#5537

Open
chemicstry wants to merge 17 commits intoembassy-rs:mainfrom
chemicstry:net_try_sync
Open

Add sync methods to embassy-net#5537
chemicstry wants to merge 17 commits intoembassy-rs:mainfrom
chemicstry:net_try_sync

Conversation

@chemicstry
Copy link
Copy Markdown
Contributor

This adds sync alternatives to all embassy-net sockets in the try_xyz flavor. This allows using embassy-net in mixed sync/async environments.

@chemicstry chemicstry marked this pull request as draft February 27, 2026 17:09
@chemicstry chemicstry marked this pull request as ready for review February 27, 2026 17:10
@Dirbaio
Copy link
Copy Markdown
Member

Dirbaio commented Mar 12, 2026

Looks good overall. 2 things:

  • The naming for the "not ready yet" errors is a bit inconsistent. It's named Exhausted, BufferFull or WouldBlock depending on the place.
  • Adding it to the error enums means the non-try methods can still potentially return them, users are forced to handle them uselessly.

Not sure what's the cleanest solution. One is to add enum TryError<T>{ WouldBlock, Other(T) }, and wrap the non-try errors with it everywhere. If you have other ideas i'm all ears.

@chemicstry
Copy link
Copy Markdown
Contributor Author

The naming comes from smoltcp, I thought there is a reason for it, but maybe not. The only alternative I can think of is nb crate, but it seems that ecosystem is moving away from it? If not nb, I think your suggested TryError would be best. Let me know how to proceed.

@Dirbaio
Copy link
Copy Markdown
Member

Dirbaio commented Mar 13, 2026

yes, let's avoid nb.

@chemicstry chemicstry marked this pull request as draft March 16, 2026 12:21
@chemicstry chemicstry marked this pull request as ready for review March 16, 2026 12:43
@chemicstry
Copy link
Copy Markdown
Contributor Author

@Dirbaio this should be ready

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.

2 participants