nb icon indicating copy to clipboard operation
nb copied to clipboard

Clarify whether non-blocking or stateless is the more important aspect of nb

Open Nemo157 opened this issue 7 years ago • 5 comments

In reference to @japaric's comment here.

In my mind operations that return nb::Result signal that they can't be started and completed right now via WouldBlock but once you get an Ok you know that the operation started and completed in one step, without blocking. This also means that operations that return nb::Result carry no program state -- there may be some state in the hardware registers or in the kernel but it's not on the program stack / heap.

Unfortunately this is not always possible, as an example consider the embedded_hal::serial::Write implementation from the nrf51_hal crate:

pub struct Tx<UART> {
    _uart: PhantomData<UART>,
}

impl hal::serial::Write<u8> for Tx<UART0> {
    type Error = !;

    fn flush(&mut self) -> nb::Result<(), !> {
        Ok(())
    }

    fn write(&mut self, byte: u8) -> nb::Result<(), !> {
        /* Write one 8bit value */
        unsafe { (*UART0::ptr()).txd.write(|w| w.bits(u32::from(byte))) }

        /* Wait until written ... */
        while unsafe { (*UART0::ptr()).events_txdrdy.read().bits() } == 0 {}

        /* ... and clear read bit, there's no other way this will work */
        unsafe { (*UART0::ptr()).events_txdrdy.write(|w| w.bits(0)) };
        Ok(())
    }
}

This is currently a blocking implementation. To convert it to a non-blocking implementation we need to introduce some state of whether we're currently transmitting a byte (as far as I can tell, there is no way to derive this from the microcontroller registers):

pub struct Tx<UART> {
    _uart: PhantomData<UART>,
    busy: bool,
}

impl hal::serial::Write<u8> for Tx<UART0> {
    type Error = !;

    fn flush(&mut self) -> nb::Result<(), !> {
        let uart = unsafe { &*UART0::ptr() };
        if self.busy {
            if uart.events_txdrdy.read().bits() == 1 {
                uart.events_txdrdy.reset();
                self.busy = false;
                Ok(())
            } else {
                Err(nb::Error::WouldBlock)
            }
        } else {
            Ok(())
        }
    }

    fn write(&mut self, byte: u8) -> nb::Result<(), !> {
        let uart = unsafe { &*UART0::ptr() };
        self.flush()?;
        uart.txd.write(|w| unsafe { w.bits(u32::from(byte)) });
        self.busy = true;
        Ok(())
    }
}

In my mind the whole purpose of nb is to support implementing non-blocking IO primitives suitable for integrating into something like a futures event loop (although, I'm still trying to work out how to support interrupt-driven task notifications without having to have event loop specific implementations anyway...). If implementations are going to be forced to block to not have a tiny bit of important state, that undermines the entire utility of this crate.

Once this is clarified I will try and open a PR documenting this along with other stuff mentioned in that comment.

cc @therealprof (in case you have some other ideas about non-blocking uart support on the nrf51)

Nemo157 avatar Mar 19 '18 13:03 Nemo157

@Nemo157 I haven't tried but maybe it's possible to set uart.events_txdrdy to 1 in the code. In that case it should be possible to have the write method like this:

fn write(&mut self, byte: u8) -> nb::Result<(), !> {
    let uart = unsafe { &*UART0::ptr() };
    if uart.events_txdrdy.read().bits() == 1 {
         uart.events_txdrdy.reset();
         uart.txd.write(|w| unsafe { w.bits(u32::from(byte)) });
         Ok(())
    } else {
        Err(nb::Error::WouldBlock)
    }
}

I can give it a whirl later.

therealprof avatar Mar 19 '18 14:03 therealprof

maybe it's possible to set uart.events_txdrdy to 1 in the code.

I just attempted that and it doesn't seem to take, I'm pretty sure the event registers are clear-only.

Nemo157 avatar Mar 19 '18 14:03 Nemo157

@Nemo157 The documentation is not quite clear about that. But another person in the interwebs attempted the same and also failed. ;) I'll try a few more variants later, maybe just send a dummy character into the void before setting up the peripheral pins...

therealprof avatar Mar 19 '18 14:03 therealprof

Leaving aside the implementability of this on the concrete hardware, I'd interpret the stateless aspect of a nb method to pertain to state from the individual calls (like the arguments being stored from one method call to the next), not internal management state of the callee.

A UART implementation can get away with having all its management state in registers, but can just as well be a statefull component (like a bit-banging UART that has its registers in software and clocks out bits inside interrupts), or might be a hybrid of that (eg. this UART peripheral that needs additional management that lives inside its wrapper).

chrysn avatar Feb 01 '19 16:02 chrysn

I suppose one thing i'm influenced by is that i'd often consider a generator implementation for a nb call to be reasonable. At which point you are implicitly storing a bunch of state.

HarkonenBade avatar Feb 02 '19 15:02 HarkonenBade