rust-embedded / nb

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

[DISCUSSION] Value over core::future #14

Open boomshroom opened 5 years ago

boomshroom commented 5 years ago

With futures becoming standardized and integrated into the core library, I feel like this crate has less reason to exist. The biggest part of this crate is type nb::Result<T, E> = Result<T, nb::Error<E>>, but in the current nightly, this can be replicated with core::task::Poll<Result<T, E>>. The biggest reason I can see to use this library is the fact that it doesn't require nightly. It may be a while before core::{future, task} becomes stabalized, but even so, if I'm doing no_std stuff, I tend to be using nightly anyways (for things like asm).

That being said, where do you see this library going in the near or distant future given futures being adopted into the standard library? I'm not saying to shut this down and switch everyone to futures, I just want to get an idea of where things are headed. Thank you for any discussion. (I also apologize if the issue tracker isn't the best place for discussion.)

hannobraun commented 5 years ago

@boomshroom

It may be a while before core::{future, task} becomes stabalized, but even so, if I'm doing no_std stuff, I tend to be using nightly anyways (for things like asm).

There has been a push to get embedded development on stable for the upcoming Rust 2018 release. I don't think we should deprecate nb before we have a stable replacement.

That being said, where do you see this library going in the near or distant future given futures being adopted into the standard library? I'm not saying to shut this down and switch everyone to futures, I just want to get an idea of where things are headed. Thank you for any discussion. (I also apologize if the issue tracker isn't the best place for discussion.)

I've had this thought for a while now, that nb could be replaced one day, and we'd standardize on Future and async/await instead. I haven't followed the latest developments though, and I'm not up to date on current plans. So I don't know if/when this might become viable.

I think it would be very valuable, if you or anyone else wanted to look into this, and report back what Rust's async future means for nb and embedded-hal.

boomshroom commented 5 years ago

Thank you. Your input exactly what I asked for. So it seems as though there's a chance of deprecation once futures hit stable, but it's still not set in stone.

boomshroom commented 5 years ago

Std futures have now been stabilized in nightly and will be arriving in a few months in 1.36. The merge can be found here: https://github.com/rust-lang/rust/pull/59739

XOSplicer commented 4 years ago

Rust 1.36 has been releases with a stabilized Future trait. Can this fact lead this conversation any further?

hannobraun commented 4 years ago

@XOSplicer I'm not speaking for the project, only for myself, but I think what's required here is someone to figure out what using Future actually means in practice. If I had the time to do this, I'd fork embedded-hal, create Future-based versions of the traits and figure out what's required to make that work in a real program.

Maybe there are others that know more than me, but until one of them shows up here and tells us what's up, I assume someone will have to do the actual work :-)

XOSplicer commented 4 years ago

@hannobraun

what's required here is someone to figure out what using Future actually means in practice.

This is very true.

To this time I do also not have a lot of experience in async rust, perhaps the async book is a good place for me to start in order to read up on the current developments of the async story in rust core.

However in the meantime I might suggest a code snippet to be compatible with core::task::Poll:

/// Future adapter
///
/// This operations transforms a `nb::Result` to a `core::task::Poll`
///
#[cfg(feature = "unstable")]
#[macro_export]
macro_rules! poll {
    ($e:expr) => {
        match $e {
            Err($crate::Error::Other(e)) => core::task::Poll::Ready(core::result::Result::Err(e)),
            Err($crate::Error::WouldBlock) => core::task::Poll::Pending,
            Ok(x) => core::task::Poll::Ready(core::result::Result::Ok(x)),
        }
    }
}
XOSplicer commented 4 years ago

This code could also be implemented as a plain function. There is no need for it to be defined as a macro.

Maybe something like this is more usable:

pub fn into_poll<T, E>(r: crate::Result<T, E>)
    -> ::core::task::Poll<::core::result::Result<T, E>>
{
    match r {
        Err(crate::Error::Other(e)) => ::core::task::Poll::Ready(core::result::Result::Err(e)),
        Err(crate::Error::WouldBlock) => ::core::task::Poll::Pending,
        Ok(x) => ::core::task::Poll::Ready(core::result::Result::Ok(x)),
    }
}

pub fn as_poll<T, E>(r: &crate::Result<T, E>)
    -> ::core::task::Poll<::core::result::Result<&T, &E>>
{
    match r {
        Err(crate::Error::Other(ref e)) => ::core::task::Poll::Ready(core::result::Result::Err(e)),
        Err(crate::Error::WouldBlock) => ::core::task::Poll::Pending,
        Ok(ref x) => ::core::task::Poll::Ready(core::result::Result::Ok(x)),
    }
}

pub fn as_mut_poll<T, E>(r: &mut crate::Result<T, E>)
    -> ::core::task::Poll<::core::result::Result<&mut T, &mut E>>
{
    match r {
        Err(crate::Error::Other(ref mut e)) => ::core::task::Poll::Ready(core::result::Result::Err(e)),
        Err(crate::Error::WouldBlock) => ::core::task::Poll::Pending,
        Ok(ref mut x) => ::core::task::Poll::Ready(core::result::Result::Ok(x)),
    }
}
piegamesde commented 3 years ago

Thanks to the comments above sharing their async solutions. In that spirit, maybe someone will find my block_async! macro useful:

macro_rules! block_async {
    ($e:expr) => {
        loop {
            #[allow(unreachable_patterns)]
            match $e {
                Err(nb::Error::Other(e)) => {
                    #[allow(unreachable_code)]
                    break Err(e)
                }
                Err(nb::Error::WouldBlock) => {
                    async_std::task::yield_now().await;
                }
                Ok(x) => break Ok(x),
            }
        }
    };
}

It currently depends on async_std but if you look at the code behind yield_now that really isn't a lot.