rust-embedded / nb

Minimal and reusable non-blocking I/O layer
Apache License 2.0
86 stars 15 forks source link

Use futures instead of most of this lib #2

Closed vitiral closed 7 years ago

vitiral commented 7 years ago

I posit that WouldBlock should be an enum which contains the Result. So the type would be:

enum Async<R> {
    Done<R>,  // Done with Result
    WouldBlock,
}

This also removes the funny buisness of using ! as the type of the Other error. If it is an operation that cannot fail, then R is just the Value (as it should be).

Note that this is exactly the same API as the futures Async data type.

pub enum Async<T> {
    Ready(T),
    NotReady,
}

I argue that we should just use futures under the hood for ALL asynchronous core types, and that can easily be converted to the other APIs. This is, in fact, the entire point of futures. I think the nb library serves no purpose then.

The futures api is pretty dang minimal with default-features = false selected. There may be space for us to implement something on top of that minimal interface for use in embedded. That could be the usefulness of this library.

japaric commented 7 years ago

I think you are missing the whole point of the nb crate which is there to not couple the lowest level API to the futures crate (or any other asynchronous model). That doesn't mean we should not have a futures based HAL; I think we should, but it should be built on top of the embedded-hal crate which uses the nb crate.

What I don't think we should do is have implementations of the HAL traits, specifically the register manipulation code, directly return futures. Reason being that generators (+async/await) are a much more ergonomic alternative to futures (this is not just my opinion) so I don't want us to get locked into a "local maxima".

Specifically I want to prevent this scenario: (a) You write your HAL that returns futures but directly on top of a svd2rust crate. (b) generators land in language. (c) Now you have to rewrite your register manipulation code to use generators which is a lot of work and error prone.

Instead I want this to happen: (a) You implement the embedded-hal traits for your device. (b) You write your applications today using the hal-futures crate which builds on top of the embedded-hal crate and provides a futures based API. (c) generators land in the language. (d) You or someone else writes a hal-async crate that builds on top of embedded-hal but exposes a generator-based API. (e) You move your code from hal-futures to hal-async. No register manipulation code had to be rewritten.

As for your original question / title: Callback-style code does not want a future as the return value. WouldBlock as an error is the correct return type in that case. For example, the Timer API which uses the bottom type (!): if your callback function got called but the timeout had not occurred then wait, which clears the update/interrupt flag, should return an error, which is what it does today: it returns Error::WouldBlock.

vitiral commented 7 years ago

thanks for the response. I was misunderstanding severl important points. Closing in favor of #3