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

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

Open
Nemo157 opened this issue Mar 19, 2018 · 5 comments

Comments

@Nemo157
Copy link
Contributor

Nemo157 commented Mar 19, 2018

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)

@therealprof
Copy link
Contributor

@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.

@Nemo157
Copy link
Contributor Author

Nemo157 commented Mar 19, 2018

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.

@therealprof
Copy link
Contributor

@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...

@chrysn
Copy link

chrysn commented Feb 1, 2019

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).

@HarkonenBade
Copy link

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.

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