[RFC] Use core language async primitives instead external crates

[no111u3]

Summary

This proposal proposes to change the way how async api does implemented. It makes async api more clearly for use and extend.

Motivation

Our current approach to implement trait primitives as depends on nb crate. It was actually on start of this project but after version 1.36 rust already have async traits and methodology for work with it. So in 2018 edition of rust lang we have language words as async, await. The marcosses exported by nb are conflicted with key words, we need to use r# prefix for each macros use.

This RFC attempts to change implementation to core types based implementation. It will be more effective and more conform rust language ideas.

Detailed design

This RFC inspired of PR #13, but after apply external crate I redesign for use core functionality because it will be better than external depends.

For example we have interface for serial:

use core::task::Poll;

/// A serial interface
pub trait Serial {
    /// Error type associated to this serial interface
    type Error;
    /// Reads a single byte
    fn read(&mut self) -> Poll<Result<u8, Self::Error>>;
    /// Writes a single byte
    fn write(&mut self, byte: u8) -> Poll<Result<(), Self::Error>>;
}

It use core::task::Poll as point of async run status. It was easy for implement and it isn't depends on nb or another external crates.

Alternatives

Don't implement this RFC.

Uresolved questions:

• More clear examples of right use async code.
• How we need to update depend crates?
• Do we need to rewrite nb prefer than rewrite async api of embedded-hal?

[Nemo157]

There are semantics attached to Poll::Pending that did not exist on nb::Error::WouldBlock, specifically:

When a function returns Pending, the function must also ensure that the current task is scheduled to be awoken when progress can be made.

Having functions that return core::task::Poll but do not take a core::task::Context and arrange to wake the task when necessary is likely to lead to deadlocks if they are used by systems built around core::future::Future.

[Nemo157]

@Nemo157 I understand that problem we have but in current implementation of api i couldn't change behavior of run without more breakable changes on api. I'm planning to find solution in this situation. Thanks.

Sure, I just want to make sure this isn't missed. I have done a lot of work on async and have experimented with using it on embedded devices in https://github.com/Nemo157/embrio-rs/ including playing around with alternative trait definitions, just want to offer whatever insights I might have (I am lacking in experience with using the existing embedded ecosystem much, I was only really focused on trying it out with async).

If there's a plan for breaking changes to the trait APIs it'd be best to do them all at once, which may involve adding an additional core::task::Context parameter to all the traits, and providing extension traits similar to how futures is designed:

use core::{pin::Pin, task::{Poll, Context}, future::Future};

/// A serial interface
pub trait Serial {
    /// Error type associated to this serial interface
    type Error;
    /// Reads a single byte
    fn poll_read(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Result<u8, Self::Error>>;
    /// Writes a single byte
    fn poll_write(self: Pin<&mut Self>, byte: u8, cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>>;
}

pub trait SerialExt: Serial {
    fn read(&mut self) -> impl Future<Output = Result<u8, Self::Error>>;
    fn write(&mut self, byte: u8) -> impl Future<Output = Result<u8, Self::Error>>;
}

[no111u3]

It looks as real solution for me. I talk about current PR with @Disasm, he says: we can create clear path for migrate from old nb api to new async api through default implementation of async api.
But it may be freeze migration from old to new api.

[tustvold]

So the main problem I ran into when trying to do something similar in #149 was that as futures in rust are driven, the returned Poll<Result<u8, Self::Error> needs to borrow a reference to the underlying peripheral. Otherwise it would be possible to, for example, interleave writes to the same peripheral - which the current API prevents you from doing.

One can work around this by moving the peripheral itself into the future and only returning it on completion, but this makes for an incredibly obtuse API that makes simple constructs such as loops hard.

It is this precise issue with ergonomics that was one of the motivators behind adding native async support to Rust in the first place - see here.

It may be that someone smarter than myself can find solutions to these problems, but thought I'd clarify them here. Depending on the timelines for generalized associated types and generator resume arguments, it might be worth waiting for them to to bring native async support to traits and [no_std] respectively, before making breaking changes that would only need to be followed by more breaking changes.

[Nemo157]

The returned Poll shouldn't need to borrow the peripheral, it should be higher level APIs like fn read(&mut self) -> impl Future<Output = Result<u8, Self::Error>>; which borrow the peripheral in the returned Future in order to avoid interleaved reads/writes. The Poll returning methods are really low-level building blocks which don't need to provide full compile-time guarantees, the majority of users should be accessing them through the Future returning functions.

One of the nice things about the split between low-level traits of Poll functions and high-level utilities returning Future used in futures is that it avoids any need for GATs. And there are a lot of other issues with attempting to create core traits that return futures, see rust-lang/futures-rs#1365 for a lot of relevant discussion.

(Generator resume arguments will not affect any public API related to Future/async, it will only mean that async fn becomes usable on no_std, at least until some very far off day when generators themselves might be stabilised).

[therealprof]

@Nemo157 Do you have an example for a Future executor which can run on embedded? Still trying to figure out our options; it would be kind of nice if we could spawn new futures anywhere and have the rest of the main loop be a poller for all futures. That would probably require a global future queue where interrupts and other functions can add their futures to?

[tustvold]

@Nemo157 You are completely correct, I had a brain fart and conflated Poll<Result<u8, Self::Error>> and impl Future<Output = Result<u8, Self::Error>>. GATs are only needed if trying to provide traits with methods that return futures.

The majority of users should be accessing them through the Future returning functions

So is the idea that driver crates would then provide methods returning futures? How would people compose and run these without the afformentioned async support for no_std? You could provide the block, await, etc... macros that the nb crate provides for its futures, but moving away from these was one of the motivators for the RFC?

Edit: I believe you would still need the macros nb provides to aid the writing of the driver crates, and then would additionally need to provide an executor implementation for people to run the futures and possibly further facilities to aid in composing them.

[Nemo157]

There is https://docs.rs/embedded-executor/0.3.1/embedded_executor/, I haven't used it myself, but it was developed as part of https://gitlab.com/polymer-kb/polymer.

How would people compose and run these without the afformentioned async support for no_std?

I think moving to core::future::Future only makes sense when intended to be used with async. The current implementation issue blocking it being used in no_std has workarounds (core-futures-tls using ELF thread-locals, embrio-async for a re-implemented transform, worst case an unsound copy of core-futures-tls that abandons thread-safety). And if there is consensus and experiments in moving some of the embedded ecosystem to async that might provide the incentive to get someone to work on fixing the compiler to support it.

[tustvold]

@Nemo157 So I'm a little bit confused as to what you are proposing exactly, as far as I can see it these are the following options

1. This RFC gets accepted and things remain similar to how they are currently but we lose the utility block, await, etc... macros that are currently provided by nb
2. This RFC gets extended to provide macros similar to those provided within the nb crate, but named in such a way as to not collide
3. This RFC is blocked until async support comes to no_std allowing writing and interacting with methods returning core::future::Future. A utility macro could be provided to facilitate wrapping the embedded_hal methods returning poll types into core::future::Future types that can then be used with async
4. The macros within nb are renamed to not collide with keywords
5. The RFC is rejected

I think moving to core::future::Future only makes sense when intended to be used with async.

And I would argue moving to core::task::Poll only really makes sense if you're going to move to using core::future::Future.

[baloo]

I got a working proof of concept of async/await on a stm32f429 mcu using #172 (comment) approach.
I used:

• embrio-rs's Executor (slightly modified not to depend on rest of embrio)
• made a simple fork of core-futures-tls to use a static mut to store the Context
• used Switch async code to use futures crate #171 and ported stm32f4xx-hal to it.

Being not too knowledgeable about RFC process, should I share?

[therealprof]

Being not too knowledgeable about RFC process, should I share?

Yes, please!

[baloo]

baloo/pm-firmware@8ccdfd2

Some remarks:
I only implemented support for UART for now, because that's what I used to debug.
I used git submodules to ensure I you get the same versions of the libs I was running.
I reused the example from embrio-rs found here:
https://github.com/Nemo157/embrio-rs/blob/master/examples/apps/hello/src/lib.rs