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

use async fn for async trait #15

Merged
merged 1 commit into from
Dec 29, 2024
Merged

Conversation

Be-ing
Copy link
Contributor

@Be-ing Be-ing commented Dec 28, 2024

for readability

A Send variant may be nice to use with Embassy's SendSpawner. However, embedded-hal-async traits don't have Send variants, so it's unlikely the that the drivers called in the impl's of this trait would have async fn's that return Send Futures. You probably don't want to be writing different frames to one LED device from multiple threads simultaneously anyway; that would require the driver to implement synchronization to avoid interleaving of separate frames, which would add a run time cost.

@david-sawatzke
Copy link
Collaborator

Thank you for the PR. I'm still not 100% sure what this implies, but the reasoning behind the choice in the embedded environment does convince me: rust-embedded/embedded-hal#515

Could you please add your reasoning/this reasoning to explain the choice in a comment please, so people unaware are less confused?

for readability

A Send variant may be nice to use with Embassy's SendSpawner.
However, embedded-hal-async traits don't have Send variants, so
it's unlikely the that the drivers called in the impl's of
this trait would have async fn's that return Send Futures.
You probably don't want to be writing different frames to one LED
device from multiple threads simultaneously anyway; that would
require the driver to implement synchronization to avoid
interleaving of separate frames, which would add a run time cost.
@Be-ing
Copy link
Contributor Author

Be-ing commented Dec 29, 2024

Oh nice find digging up the discussion in rust-embedded/embedded-hal#515, that's the perfect link to put in a comment.

I suggest reading the blog post that announced the stabilization of async fn. The change in this PR is literally just syntax sugar, there's no functional implication; the compiler is smart enough to recognize the syntaxes have identical meaning.

@david-sawatzke david-sawatzke merged commit 7dbc764 into smart-leds-rs:master Dec 29, 2024
@Be-ing Be-ing deleted the async_fn branch December 29, 2024 16:53
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