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

Plan for core::future #149

Closed
tustvold opened this issue Aug 3, 2019 · 14 comments
Closed

Plan for core::future #149

tustvold opened this issue Aug 3, 2019 · 14 comments

Comments

@tustvold
Copy link

tustvold commented Aug 3, 2019

Currently the traits within embedded-hal make use of the nb crate's flavour of async IO, however, core::future and core::task have recently been standardised and I believe are slated for inclusion in Rust 1.38. As such I was wondering whether migration towards using them would be something the maintainers of this project would be open to?

In particular use of core::task::Waker opens up the possibility for HAL crates to use interrupts to signal when they are ready to make progress, avoiding the busy waiting that appears to be largely unavoidable with the current interface.

I'm aware that I'm not the first person to wonder this, see here, but I couldn't find an issue discussing it. The closest I could find was on the nb crate itself here.

If there is interest in this course of action I'm happy to look into writing a PoC, but I'm acutely aware there is, understandably, some caution around making breaking changes to the traits - see here.

@tustvold
Copy link
Author

So I'll caveat all the below with the fact I'm relatively new to rust and so what I say may be incorrect, but following further experimentation I do not believe it is yet possible for embedded_hal to make use of core::future and related types.

This is not to say that these types can't be used in embedded, far from it - it would be clunky but it is perfectly possible to use them in an embedded context, however, the specific constraints imposed by the design of embedded_hal, namely its use of traits and non static lifetimes, make this at least beyond my capabilities.

The easiest way to justify this, is to explain what I did, the problems I faced and how I bodged around them.

My first step was to build an "executor" that can run futures in an embedded environment. I don't believe it is possible to implement something like the executors in the standard library as the spawn interface seems to require dynamic memory allocation. However, it is relatively straightforward to write methods that can drive a core::future or an array of &mut core::future to completion, using a combination of the stateless wakers I describe in the linked issue, and core::future::LocalFutureObj. Implementations of the familiar block!, etc... macros can then be written using these methods.

Next I extended the traits within embedded_hal to use associated types to represent the future return types. I then tried to implement these traits within stm32f4xx_hal, however, immediately ran into an issue when the future needs to borrow a reference to the underlying peripheral for the lifetime of the future. Currently generic associated types are not close to stable, and so I bodged around this by making each of the embedded_hal traits generic over a lifetime parameter, and using this to parameterise the future associated type. The default blocking implementations required some HRTB magic to work, but I managed to make it work.

At this point I had a working stm32f4xx_hal and thought I was in the home stretch, however, I quickly discovered that trying to compose these futures was immensely fiddly. Not only does the somewhat hacky approximation to a generic associated type work against you, but I ended up having to use unsafe in order to preserve both the returned future and the peripheral it was borrowing across a Poll::Pending return. This is exactly what generators were created for, however, there is currently no way to resume them with an argument - in this case a core::task::Context. The current async implementation temporarily hacks around this using thread local storage, and this is why it isn't supported on embedded.

It was at this point that I threw in the towel, I realised I was trying to bodge together something that was complicated enough to have warranted inclusion in the language itself. Ultimately for embedded_hal to be able to use the new API it at a minimum needs generic associated types and the ability to resume generators with an argument. In practice these two features are likely to coincide with embedded async and async functions in traits, at which point the implementation becomes relatively trivial.

I will leave this issue open, but I won't be working on it further unless something changes.

@hannobraun
Copy link
Member

@tustvold Thank you for looking into this, and for writing this extensive report!

I'd like to add my own experience to this bit:

immediately ran into an issue when the future needs to borrow a reference to the underlying peripheral for the lifetime of the future

If I had done this investigation, I would have stopped right here, unless I'd found an alternative to the borrowing. Experience has shown that this kind of borrowing is problematic in practice.

In the past, I've implemented my own future-like API (really basic, no thought spent on composing etc.) for the DW1000 driver. A call to send/receive on the main DW1000 struct would return a TxFuture/RxFuture, which was just another struct that borrowed the main struct and could be used to set up interrupts, poll the current status etc. This approach worked well enough for my examples, but experience with a non-trivial application showed that it didn't work beyond that.

The problem starts when you want to build an abstraction that requires the DW1000 struct to live as a field in another struct. If that encapsulating struct is supposed to fully manage the driver, that means it needs to store TxFuture/RxFuture too. Thanks to the lifetime, this turns into a self-referential struct, which doesn't work.

Maybe some language wizard will weigh in here and explain how that's all easy to solve using Pin or something, but my solution was to remove the futures, and instead add a type state parameter to DW1000. So DW1000 becomes DW1000<Ready>, TxFuture becomes DW1000<Sending> and RxFuture becomes DW1000<Receiving>. send/receive consume the DW1000<Ready> and return the DW1000<Sending>/DW1000<Receiving> respectively. The Sending/Receiving states provide their own methods to move back to Ready. No borrowing necessary.

For the simple examples this doesn't change much, except that they need an additional variable for sending/receiving, and need to assign the driver back to the original variable after completion. The complex application should be able to encapsulate the driver struct in an enum, with one variant for each of the states. Here's what the API looks like today: https://github.com/braun-embedded/rust-dw1000/blob/ceca46f9657fa09cba32b52b3a872b7a7108613e/src/hl.rs
(note: something there is not quite right yet; most testing was successful, but some of my examples stopped working for yet unknown reasons)

@tustvold Do you think this approach could help with a futures-based API? On first glance, it looks like we could get the original struct back from the future via Self::Output, but I haven't tried that.

@tustvold
Copy link
Author

tustvold commented Aug 13, 2019

If I had done this investigation, I would have stopped right here, unless I'd found an alternative to the borrowing. Experience has shown that this kind of borrowing is problematic in practice.

Yeah... I lack the experience with Rust to make those sorts of judgement calls :p That being said it did work quite well until I tried to wrap it with a trait.

If that encapsulating struct is supposed to fully manage the driver, that means it needs to store TxFuture/RxFuture too. Thanks to the lifetime, this turns into a self-referential struct, which doesn't work.

This is the exact issue that resulted in me throwing in the towel. You can work around it with Pin, as I did, but it is incredibly fiddly and I'm not totally sure what I've written is safe. Generators and by extension async solve this, but at the moment don't support embedded nor traits.

The Sending/Receiving states provide their own methods to move back to Ready. No borrowing necessary.

This is possible but unless I'm missing something doesn't compose well. Say for example you wanted to wrap the DW1000 driver in a TDOA abstraction, or wanted to share its SPI bus with another component, neither of these would be trivially possible.

I think an approach similar to the one described here where the peripheral is moved into the future and returned on completion might work. I'll have a play around this evening after work and report back.

@hannobraun
Copy link
Member

@tustvold

Yeah... I lack the experience with Rust to make those sorts of judgement calls :p That being said it did work quite well until I tried to wrap it with a trait.

That's why I'm here: To share the insights gained from my years-long (and ongoing) series of mistakes :-)

This is possible but unless I'm missing something doesn't compose well. Say for example you wanted to wrap the DW1000 driver in a TDOA abstraction, or wanted to share its SPI bus with another component, neither of these would be trivially possible.

I don't see how wrapping the driver in an abstraction is a problem, as that's the exact use case that this design is supposed to enable. But good point about sharing the SPI bus, I hadn't really considered that.

I guess one could create a representation of the shared bus that can move with the driver, similar to Sender/Receiver in std-land, but that would require fiddling with static's behind the scenes. Might be doable, but is also off-topic for this issue.

Lots of stuff to figure out in this space :-)

I think an approach similar to the one described here where the peripheral is moved into the future and returned on completion might work. I'll have a play around this evening after work and report back.

Yeah, that looks kind of what I had in mind. Looking forward to see what you come up with!

@tustvold
Copy link
Author

So one potential downside of moving the peripheral into the futures is that even the blocking APIs and usages can't borrow the values. The result is

let val = block!(serial.read());

Becomes

let (serial, val) = block!(serial.read());

It's also worth considering that whilst most HAL types should be lightweight to move, this isn't necessarily a guarantee. Nothing immediately springs to mind as an obvious use case for data storage within the peripheral, as opposed to the returned future, but I'm sure there must be a valid use case...

@hannobraun
Copy link
Member

Would it be possible to provide multiple methods, one that borrows, one that moves? Maybe in such a case the Future implementation could be generic over what it takes/returns, or different Future implementations could be generated via a macro.

Not sure if any of that is practical. Just throwing out ideas.

@ryankurte
Copy link
Contributor

@tustvold excellent writeup, thanks for exploring this!

i've been writing a bunch of non-embedded futures code recently and, as interesting as the concept is, i don't think they're a good option for embedded. it's possible that the ergonomics will improve significantly in the future, but, at this point in time there are a bunch of issues and hidden complexities that ime make complex applications with futures in rust vastly more difficult than just writing naturally async systems.

your writeup covers most of the embedded issues, the dynamic allocation and ergonomic pains that can only be solved with gratuitous use of Arc and Box are significant, but the major problem i have is that they're super contagious. once you have any component that uses futures, any code that would consume that (efficiently) is also forced to be asynchronous. maybe this can all be implemented in an opt-in way, but, i would be cautious of imposing this on hal users.

So one potential downside of moving the peripheral into the futures is that even the blocking APIs and usages can't borrow the values. The result is

there are a couple of examples of move-into-future in tokio and it seems the only way to ergonomically use them is to immediately wrap them with mpsc::channels so you can store and move references around. i think this has the same composability issues as type-state based futures.

It's also worth considering that whilst most HAL types should be lightweight to move, this isn't necessarily a guarantee

as above, if the hal is futures-based the consumers will be pushed to be to, and drivers with buffers etc. are often not lightweight. it might be possible to work around this with 'static buffers and things, but, it seems like it would introduce some complexity.

and instead add a type state parameter to DW1000

@hannobraun i have also been exploring the... limits ... of type-state programming with radio devices, for configuration rather than operating state. thanks for sharing your example ^_^

i had a very brief look at futures for the same send -> poll_send -> done pattern, but, couldn't conceptualise how they would interact with external interrupts or polling delays in a useful / simple / predictable way.

@tustvold
Copy link
Author

as above, if the hal is futures-based the consumers will be pushed to be to, and drivers with buffers etc. are often not lightweight. it might be possible to work around this with 'static buffers and things, but, it seems like it would introduce some complexity.

My thinking was that such buffers and other related state would likely be stored within the future object as opposed to the peripheral itself, but I agree that there is almost certainly some use case that would result in a chunky peripheral that is costly to move.

make complex applications with futures in rust vastly more difficult than just writing naturally async systems.

I can't agree with this more, it currently suffers from the same problem as C++ libraries like Boost.Asio - fantastically powerful but mind bending to use in practice. First-class language support to make this less painful is pretty much a necessity.

but the major problem i have is that they're super contagious.

And not only this but they don't really result in an abstraction that is particularly compelling - it is more restrictive and harder to reason about than the current API and the signature feature of interrupts isn't prevented by the current API, it is just out of scope and left to the HAL to handle. It's trying to provide a middle ground between bare-metal and an RTOS and I'm not really sure that's a good idea.

It's also worth considering that changes to embedded_hal are likely to be incredibly complicated and given the fact that native async will eventually make its way to embedded and traits, we'd likely end up suffering this pain twice.

@hannobraun
Copy link
Member

@tustvold @ryankurte
Thanks for the additional info. Sounds like futures aren't a good option then, at least for now.

You talked about "naturally async" and "native async". As someone who's been following the async story from afar, what would that involve? async/await? Generators? Something else?

given the fact that native async will eventually make its way to embedded and traits

Any idea what that would look like? Maybe any articles I could read?

@ryankurte
Copy link
Contributor

You talked about "naturally async" and "native async". As someone who's been following the async story from afar, what would that involve? async/await? Generators? Something else?

by "naturally async" i just mean code that is inherently non-blocking, the standard start_transmit()/check_transmit() or poll() paradigm that we've been writing forever, with internal state and without the addition of futures and generators.

i'm not sure what @tustvold means here by "native async", i guess the finalisation of async support (and custom generators etc. to support other uses of them) in rustc?

@tustvold
Copy link
Author

tustvold commented Oct 9, 2019

@ryankurte I indeed meant the stabilization of async support in rustc, with both embedded support and the ability to use them in traits.

@Nemo157
Copy link

Nemo157 commented Nov 27, 2019

cc new RFC for this: #172

@ryankurte
Copy link
Contributor

I think y'all are ahead of me on this, but, had a bash at implementing futures over natually async traits, was okay except for futures traits not yet being available, TLS not being available on embedded platforms, and generators using std::boxed::Box. I also haven't worked out how to usefully integrate wakers (without just waking every time).

@Dirbaio
Copy link
Member

Dirbaio commented Apr 28, 2023

now that embedded-hal-async exists, I think this can be closed.

@Dirbaio Dirbaio closed this as completed Apr 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants