Stream: wg-async-foundations

Topic: avr async handholding


Jake Goulding (Jun 15 2019 at 12:09, on Zulip):

Here is what I think should be a valid leaf future for sending a single byte on AVR. It doesn't seem to work. While I'm debugging further, I was hoping if someone could skim this and see if it seems to make sense.

https://gist.github.com/shepmaster/25fe1180a01e315040d11cd7412f5452

    struct SerialTx(u8);

    static mut TX_WAKER: Option<Waker> = None;

    #[inline(always)]
    pub fn tx_empty_interrupt_handler() {
        // Safety:
        // We are on a single-threaded CPU, so static mutable shoudn't matter.
        unsafe {
            if let Some(waker) = TX_WAKER.take() {
                // Notify our waker to poll the future again
                waker.wake();

                // We must either read from the buffer or disable the
                // interrupt to prevent re-invoking the interrupt
                // handler immediately.
                disable_serial_tx_empty_interrupt();
            }
        }
    }

    fn enable_serial_tx_empty_interrupt() {
        set_bit_in(UCSR0B, UDRIE0);
    }

    fn disable_serial_tx_empty_interrupt() {
        unset_bit_in(UCSR0B, UDRIE0);
    }

    impl Future for SerialTx {
        type Output = ();

        fn poll(self: Pin<&mut Self>, ctx: &mut Context) -> Poll<Self::Output> {
            match serial::try_transmit(self.0) {
                Ok(()) => {
                    Poll::Ready(())
                },
                Err(()) => {
                    // Safety:
                    // We are on a single-threaded CPU, so static mutable shoudn't matter.
                    unsafe {
                        TX_WAKER = Some(ctx.waker().clone());
                    }
                    enable_serial_tx_empty_interrupt();

                    Poll::Pending
                }
            }
        }
    }

/cc @Nemo157

Jake Goulding (Jun 15 2019 at 15:35, on Zulip):

heh. I got basic panic formatting working, only to be told that the panic is coming from #[embrio_async] :stuck_out_tongue:

Matthias247 (Jun 15 2019 at 20:30, on Zulip):

No idea on the panic. But regarding the implementation itself: The basics seem right, you it might be missing some thread-safety. Even if microcontrollers are single-core, one can still run into thread-safety issues due to interrupts and nested interrupts. Typically that is worked around by disabling all interrupts in critical sections. That might be necessary here inside the interrupt handler and and maybe also inside the try_transmit block (depending on whether multiple components can try to use the serial port at once or not).
Also check whether the tx_empty_interrupt is level triggered, not edge triggered. Otherwise there exists a race condition where the port might get readable and get signaled after try_transmit failed but before the interrupt handler is enabled.

Jake Goulding (Jun 15 2019 at 22:32, on Zulip):

That might be necessary here inside the interrupt handler

Should be good here; there's two types of interrupts: avr-interrupt and avr-non-blocking-interrupt; the latter allows reentrant interrupts.

whether multiple components can try to use the serial port at once or not

An interesting problem; I'd probably try to punt on that for now and just document "don't do that" (to have it bite me in the future, I'm sure)

level triggered, not edge triggered

I'll have to dig in the docs for that. If it is, are there any workarounds?

Jake Goulding (Jun 15 2019 at 22:32, on Zulip):

Thanks!

Matthias247 (Jun 16 2019 at 04:43, on Zulip):

I'll have to dig in the docs for that. If it is, are there any workarounds?
You could try reading again after you enabled the interrupt to cover that time. Maybe you could also disable interrupts before performing the first read attempt in order to not run into the issue.

If interrupts are edge triggered you might also be able to just leave them enabled. You could then also e.g. use futures_intrusive::sync::ManualResetEvent and implement the interrupt handler as event.set() and the write function as:

async fn write(&mut self, data: u8)  {
    while true {
        self.event.wait().await;
        let state = self.mutex.lock();
        if state.try_send(data) {
            return;
        }
        // Send failed, loop around and retry when event got set.
        // This needs to be inside the mutex, and setting the event must also be performed inside the mutex.
        self.event.reset();
    }
}
Matthias247 (Jun 16 2019 at 04:44, on Zulip):

Finding out whether the interrupt is level or edge triggered should be easy. Just leave it on and check whether the interrupt handler gets repeatedly called.

Nemo157 (Jun 16 2019 at 10:03, on Zulip):

heh. I got basic panic formatting working, only to be told that the panic is coming from #[embrio_async] :stuck_out_tongue:

Where exactly is it panicking? As far as I remember it shouldn’t be possible to hit any of the panics there, they should all be in dead code

Nemo157 (Jun 16 2019 at 10:06, on Zulip):

In terms of safety, I’m pretty sure that implementation is unsound for spurious wakeups

Nemo157 (Jun 16 2019 at 10:07, on Zulip):

You could poll the future once, set the waker and enable the interrupt, then poll the future again and have the interrupt trigger during setting the waker again

Nemo157 (Jun 16 2019 at 10:09, on Zulip):

If the interrupt occurs mid-write to TX_WAKER then you will be creating an &mut Option<Waker> for .take() during that concurrent modification

Nemo157 (Jun 16 2019 at 10:12, on Zulip):

I haven’t really investigated how to structure stuff like this for safety and speed yet, I’ve just been beating it into working by wrapping the future implementations in large interrupt-free sections, using static Mutexes that are based on those interrupt-free sections

Jake Goulding (Jun 16 2019 at 14:48, on Zulip):

I'm pretty sure the panic is due to bad device initialization: AVR is Harvard architecture so I have to copy over the .data section from the flash into SRAM myself. I'm pretty sure that I was just copying from the wrong section of SRAM back to SRAM. (ld vs lpm)

Jake Goulding (Jun 16 2019 at 14:54, on Zulip):

While I have you, what's a good way to construct an "infinite future"?

Right now, I have

    pub fn do_futures() -> ! {
        use embrio_executor::Executor;

        static mut EXECUTOR: Executor = Executor::new();
        loop {
            crate::write_strln("loop ->");
            let executor = unsafe { &mut EXECUTOR };
            executor.block_on(example());
            crate::write_strln("<- loop");
        }
    }

but there's probably something cleaner.

Jake Goulding (Jun 16 2019 at 15:04, on Zulip):

and check whether the interrupt handler gets repeatedly called.

The docs seem to indicate level-trigger without saying it explicitly:

When the Data Register Empty Interrupt Enable (UDRIEn) bit in UCSRnB is written to one, the USART Data Register Empty Interrupt will be executed as long as UDREn is set (provided that global interrupts are enabled). UDREn is cleared by writing UDRn. When interrupt-driven data transmission is used, the Data Register Empty interrupt routine must either write new data to UDRn in order to clear UDREn or disable the Data Register Empty interrupt, otherwise a new interrupt will occur once the interrupt routine terminates.

Jake Goulding (Jun 16 2019 at 17:41, on Zulip):

@Nemo157 you mentioned checking to see if generators work at all in AVR land. What kind of code would you think would exercise that?

Jake Goulding (Jun 16 2019 at 17:41, on Zulip):

Simply, so that I can look at assembly and figure out if it's correct :-)

Matthias247 (Jun 16 2019 at 17:42, on Zulip):

While I have you, what's a good way to construct an "infinite future"?

Right now, I have

pub fn do_futures() -&gt; ! {
    use embrio_executor::Executor;

    static mut EXECUTOR: Executor = Executor::new();
    loop {
        crate::write_strln("loop -&gt;");
        let executor = unsafe { &amp;mut EXECUTOR };
        executor.block_on(example());
        crate::write_strln("&lt;- loop");
    }
}
but there's probably something cleaner.

You can move the loop into the future/async fn?

Matthias247 (Jun 16 2019 at 17:43, on Zulip):

The docs seem to indicate level-trigger without saying it explicitly:

Yes I would interpret that the same way

Jake Goulding (Jun 16 2019 at 19:24, on Zulip):

move the loop into the future/async fn

Well, that's embarrassingly simple. I gotta get used to this magic async fn world, instead of Ye Olde Combinators

Jake Goulding (Jun 16 2019 at 19:24, on Zulip):

And it works with -> ! too, nice.

Jake Goulding (Jun 16 2019 at 20:53, on Zulip):

Via debugging it appears that calling waker.wake() causes the chip to crash and restart.

Nemo157 (Jun 17 2019 at 16:50, on Zulip):

here's a pretty simple generator you should be able to do something with: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=2442b38c5290c27a20b26187d93eea33

Jake Goulding (Jun 17 2019 at 20:35, on Zulip):

println!("on");

I wish ;-)

Jake Goulding (Jun 17 2019 at 20:35, on Zulip):

I do have something close enough though, thanks!

Jake Goulding (Jun 17 2019 at 21:14, on Zulip):

@Nemo157 why the mutable reference to on? Just to exercise more?

Jake Goulding (Jun 17 2019 at 21:15, on Zulip):

I also don't know that I'll ever get used to static move || {

Taylor Cramer (Jun 17 2019 at 21:37, on Zulip):

hopefully you won't

Taylor Cramer (Jun 17 2019 at 21:37, on Zulip):

that syntax is nonsense :)

Taylor Cramer (Jun 17 2019 at 21:37, on Zulip):

I don't think anyone intends for that ever to become a real thing

Nemo157 (Jun 17 2019 at 22:28, on Zulip):

@Jake Goulding yeah, I just wanted a simple self borrow to stress that part

Nemo157 (Jun 17 2019 at 22:28, on Zulip):

And I was thinking you could replace the printlns with something that you could see the output of

Jake Goulding (Jun 18 2019 at 01:40, on Zulip):

Well, well, well...

Jake Goulding (Jun 18 2019 at 01:40, on Zulip):
off
off
off
off
off
off
off
off
off
off
Jake Goulding (Jun 18 2019 at 01:41, on Zulip):

and changing it to not do the mutable reference fixes it

Jake Goulding (Jun 18 2019 at 01:44, on Zulip):

Now to dig into the codegen... sigh

Jake Goulding (Jun 18 2019 at 01:45, on Zulip):

It does start as on, so that's good

Jake Goulding (Jun 18 2019 at 01:45, on Zulip):

wait...

Jake Goulding (Jun 18 2019 at 01:46, on Zulip):

oh, yeah, that's good:

if *on {
    blink_off();
Daniel Carosone (Jun 18 2019 at 01:59, on Zulip):

when you tog, but forget to gle?

Last update: Nov 18 2019 at 01:05UTC