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

Add asynchronous versions of most HAL traits #206

Closed
wants to merge 1 commit into from

Conversation

dflemstr
Copy link

This attempts to take a stab at addressing #172 by implementing asynchronous versions of most HAL traits, using the language-native async/await support. The thinking here is that it's better to have some suggestion for what an async HAL could look like, as a starting point for discussion.

The code was mostly taken from https://github.com/dflemstr/embedded-platform where I've had a chance to battle test it for a while, but the code should not be considered polished by any means.

Any device should be able to support async/await now that Rust no longer relies on TLS for scheduling. Even if a device lacks fancy capabilities like a heap, you can always use an interrupt-driven scheduling model using my https://github.com/dflemstr/direct-executor executor crate for example.

@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ryankurte (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@Disasm
Copy link
Member

Disasm commented Apr 23, 2020

Alternative: https://github.com/Disasm/async-trait-poc/tree/master/embedded-async-sandbox (requires unstable generic_associated_types feature)

Edit: I published my experiments as async-embedded-traits. Feel free to contribute and/or provide a feedback!

@dflemstr
Copy link
Author

Any updates or feedback on this? @ryankurte @thejpster @ilya-epifanov :)

@ryankurte
Copy link
Contributor

ryankurte commented May 20, 2020

hi @dflemstr, thanks for the PR (and the reminder)!

it seems like an interesting approach, however, i think there's both too much ongoing discussion around how we're going to achieve it (#172, #149 as you've seen) and that this is too large of a changeset to land without more discussion / documentation.

i'm not entirely sure what the path forward from here is, will try and arrange a chat with the hal team about how we plan to proceed, but, if you're interested in continuing with this (and per our updated hal guidelines) i would prefer to see a single trait (maybe just serial?) implemented in an async manner with both example implementations (i believe the linux-embedded-hal crate has async methods now) and consumers.

@Disasm also interesting! looks like the generic_associated_types feature fixes the cursed lifetime issues my attempt at this for my radio hal had (cc. @jamesmunns), and that this would also benefit from the switch from nb to poll as per #172?

also cc. @therealprof

@therealprof
Copy link
Contributor

@dflemstr Nice work! I have to agree with @ryankurte about having some example implementations to look and also the way forward is very much under discussion.

Since it would be nice to have a way forward and there's obviously some initiative, how about we turn this into a "community" project where interested people can collaborate and experiment; we might call this async-embedded-hal.

@jamesmunns Do we have pointers to our community efforts? Can we put this on the next meeting agenda?

@dflemstr
Copy link
Author

@ryankurte @therealprof thanks for getting back to me! I think that to get the ball rolling, we need to break out of this sort of catch-22, that I guess lots of "standards bodies" struggle with quite commonly :)

Ideally I think it would be nice to have a shared "playground" as part of embedded-hal (e.g. a feature flag that is considered outside of the public API for semver purposes, or a separate crate that is re-exported) so that different device implementations could try out the shared APIs easily. I do think it makes sense to start with the entire API as a whole rather than start with serial and continue with SPI etc, since there's value in having them share characteristics (e.g. a common Write trait).

Given your suggestions above: We could start from the implementation end, and convince one of the device HAL maintainer to invest in this. However, since there would be no embedded-hal "blessed" direction I fear there will be a risk that the implementation ends up too device-specific, or that there won't be any buy-in for creating async versions of the API to begin with. See for example my work here where I think we got quite far but I felt like I am unsure about what the next steps should be; there are a bunch of discussions I'd like to have with the entire embedded-hal community that don't really belong/can't be answered in that specific PR.

@dflemstr
Copy link
Author

dflemstr commented Jun 5, 2020

@ryankurte @therealprof @jamesmunns I think I will need your help in creating a shared location for this work, either in this repo or else in some other repo (e.g. under the rust-embedded org). What would be your preferences?

@Dirbaio
Copy link
Member

Dirbaio commented May 4, 2022

This can probably be closed, now that we have embedded-hal-async.

@eldruin
Copy link
Member

eldruin commented May 4, 2022

Thank you everybody for the discussion!

@eldruin eldruin closed this May 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Review is incomplete T-hal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants