Skip to content

Conversation

@embediver
Copy link
Contributor

Puts async functionality and dependencies behind async feature gate.

The serial transport binding was extended by blocking functionality using embedded-io.

embediver and others added 9 commits October 27, 2025 10:37
Signed-off-by: Marvin Gudel <[email protected]>
Signed-off-by: leongross <[email protected]>
Signed-off-by: Marvin Gudel <[email protected]>
Signed-off-by: leongross <[email protected]>
Signed-off-by: Marvin Gudel <[email protected]>
Signed-off-by: leongross <[email protected]>
Signed-off-by: Marvin Gudel <[email protected]>
Signed-off-by: leongross <[email protected]>
Signed-off-by: Marvin Gudel <[email protected]>
Signed-off-by: Marvin Gudel <[email protected]>
@embediver
Copy link
Contributor Author

Should the async feature be activated by default?

Copy link
Member

@mkj mkj left a comment

Choose a reason for hiding this comment

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

I think async makes sense as a default feature. If Router isn't being used then I don't think it would have any binary overhead, just some extra built dependencies for embassy-sync's dep tree.

Comment on lines +6 to +7
A `async` Router for embassy based applications is available
through the `embassy` feature.
Copy link
Member

Choose a reason for hiding this comment

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

"through the async feature"

Comment on lines +23 to +24
- `embassy`: async `Router` for [Embassy](https://embassy.dev/)
- `async`: [embedded-io-async](https://docs.rs/embedded-io-async/0.6.1/embedded_io_async/) serial transport binding (enabled by `embassy` feature)
Copy link
Member

Choose a reason for hiding this comment

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

Combined now

Comment on lines +104 to +107
/// Read a frame synchronously.
/// This function blocks until at least one byte is available
#[cfg(not(feature = "async"))]
pub fn recv(&mut self, input: &mut impl Read) -> Result<&[u8]> {
Copy link
Member

Choose a reason for hiding this comment

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

Name it recv_sync() for consistency

Comment on lines +15 to +20
#[cfg(feature = "async")]
use embedded_io_async::{Read, Write};

#[cfg(not(feature = "async"))]
use embedded_io::{Read, Write};

Copy link
Member

Choose a reason for hiding this comment

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

For this serial module I'd be inclined to get rid of all the cfg and always include both async and blocking functionality. embedded_io_async is a small dependency.

cfg(not(feature = "async")) shouldn't be used since that prevents a program linking crates that use both modes. (More generally features should always be additive, where possible)

Comment on lines +244 to 248
/// Frame a MCTP packet into a serial frame, writing to `output`.
#[cfg(feature = "async")]
async fn frame_to_serial<W>(
p: &[u8],
output: &mut W,
Copy link
Member

Choose a reason for hiding this comment

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

Will need renaming to frame_to_serial_async or similar

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.

3 participants