Add IRQ-driven buffered USART implementation for STM32 v2 usart (#356)

* Add IRQ-driven buffered USART implementation for STM32 v2 usart

* Implementation based on nRF UARTE, but simplified to not use DMA to
  avoid complex interaction between DMA and USART.
* Implementation of AsyncBufRead and AsyncWrite traits
* Some unit tests to ring buffer
* Update polyfill version
* Update sub module to get usart IRQ fix

embassy-hal-common/src/ring_buffer.rs

@@ -67,6 +67,14 @@ impl<'a> RingBuffer<'a> {
         self.empty = self.start == self.end;
     }
 
+    pub fn is_full(&self) -> bool {
+        self.start == self.end && !self.empty
+    }
+
+    pub fn is_empty(&self) -> bool {
+        self.empty
+    }
+
     pub fn clear(&mut self) {
         self.start = 0;
         self.end = 0;
@@ -82,3 +90,40 @@ impl<'a> RingBuffer<'a> {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn push_pop() {
+        let mut b = [0; 4];
+        let mut rb = RingBuffer::new(&mut b);
+        let buf = rb.push_buf();
+        assert_eq!(4, buf.len());
+        buf[0] = 1;
+        buf[1] = 2;
+        buf[2] = 3;
+        buf[3] = 4;
+        rb.push(4);
+
+        let buf = rb.pop_buf();
+        assert_eq!(4, buf.len());
+        assert_eq!(1, buf[0]);
+        rb.pop(1);
+
+        let buf = rb.pop_buf();
+        assert_eq!(3, buf.len());
+        assert_eq!(2, buf[0]);
+        rb.pop(1);
+
+        let buf = rb.pop_buf();
+        assert_eq!(2, buf.len());
+        assert_eq!(3, buf[0]);
+        rb.pop(1);
+
+        let buf = rb.pop_buf();
+        assert_eq!(1, buf.len());
+        assert_eq!(4, buf[0]);
+    }
+}

embassy-stm32/Cargo.toml

@@ -23,7 +23,7 @@ sdio-host = { version = "0.5.0" }
 embedded-sdmmc = { git = "https://github.com/thalesfragoso/embedded-sdmmc-rs", branch = "async", optional = true }
 critical-section = "0.2.1"
 bare-metal = "1.0.0"
-atomic-polyfill = "0.1.2"
+atomic-polyfill = "0.1.3"
 stm32-metapac = { version = "0.1.0", path = "../stm32-metapac", features = ["rt"] }
 vcell = { version = "0.1.3", optional = true }
 

embassy-stm32/src/usart/mod.rs

@@ -8,6 +8,7 @@ pub use _version::*;
 
 use crate::gpio::Pin;
 use crate::rcc::RccPeripheral;
+use embassy::interrupt::Interrupt;
 
 #[derive(Clone, Copy, PartialEq, Eq, Debug)]
 pub enum DataBits {
@@ -100,7 +101,9 @@ pub(crate) mod sealed {
     }
 }
 
-pub trait Instance: sealed::Instance + RccPeripheral {}
+pub trait Instance: sealed::Instance + RccPeripheral {
+    type Interrupt: Interrupt;
+}
 pub trait RxPin<T: Instance>: sealed::RxPin<T> {}
 pub trait TxPin<T: Instance>: sealed::TxPin<T> {}
 pub trait CtsPin<T: Instance>: sealed::CtsPin<T> {}
@@ -109,15 +112,18 @@ pub trait CkPin<T: Instance>: sealed::CkPin<T> {}
 pub trait RxDma<T: Instance>: sealed::RxDma<T> + dma::Channel {}
 pub trait TxDma<T: Instance>: sealed::TxDma<T> + dma::Channel {}
 
-crate::pac::peripherals!(
+crate::pac::interrupts!(
-    (usart, $inst:ident) => {
+    ($inst:ident, usart, $block:ident, $signal_name:ident, $irq:ident) => {
         impl sealed::Instance for peripherals::$inst {
             fn regs(&self) -> crate::pac::usart::Usart {
                 crate::pac::$inst
             }
         }
 
-        impl Instance for peripherals::$inst {}
+        impl Instance for peripherals::$inst {
+            type Interrupt = crate::interrupt::$irq;
+        }
+
     };
 );
 

embassy-stm32/src/usart/v2.rs

@@ -1,6 +1,12 @@
+use atomic_polyfill::{compiler_fence, Ordering};
 use core::future::Future;
 use core::marker::PhantomData;
-use embassy::util::Unborrow;
+use core::pin::Pin;
+use core::task::Context;
+use core::task::Poll;
+use embassy::util::{Unborrow, WakerRegistration};
+use embassy_hal_common::peripheral::{PeripheralMutex, PeripheralState, StateStorage};
+use embassy_hal_common::ring_buffer::RingBuffer;
 use embassy_hal_common::unborrow;
 use futures::TryFutureExt;
 
@@ -12,7 +18,6 @@ pub struct Uart<'d, T: Instance, TxDma = NoDma, RxDma = NoDma> {
     inner: T,
     phantom: PhantomData<&'d mut T>,
     tx_dma: TxDma,
-    #[allow(dead_code)]
     rx_dma: RxDma,
 }
 
@@ -153,25 +158,226 @@ impl<'d, T: Instance, RxDma> embedded_hal::blocking::serial::Write<u8>
 }
 
 // rustfmt::skip because intellij removes the 'where' claus on the associated type.
-#[rustfmt::skip]
 impl<'d, T: Instance, TxDma, RxDma> embassy_traits::uart::Write for Uart<'d, T, TxDma, RxDma>
-    where TxDma: crate::usart::TxDma<T>
+where
+    TxDma: crate::usart::TxDma<T>,
 {
-    type WriteFuture<'a> where Self: 'a = impl Future<Output = Result<(), embassy_traits::uart::Error>> + 'a;
+    // rustfmt::skip because rustfmt removes the 'where' claus on the associated type.
+    #[rustfmt::skip]
+    type WriteFuture<'a> where Self: 'a = impl Future<Output = Result<(), embassy_traits::uart::Error>> +'a;
 
     fn write<'a>(&'a mut self, buf: &'a [u8]) -> Self::WriteFuture<'a> {
-        self.write_dma(buf).map_err(|_| embassy_traits::uart::Error::Other)
+        self.write_dma(buf)
+            .map_err(|_| embassy_traits::uart::Error::Other)
     }
 }
 
-// rustfmt::skip because intellij removes the 'where' claus on the associated type.
-#[rustfmt::skip]
 impl<'d, T: Instance, TxDma, RxDma> embassy_traits::uart::Read for Uart<'d, T, TxDma, RxDma>
-    where RxDma: crate::usart::RxDma<T>
+where
+    RxDma: crate::usart::RxDma<T>,
 {
+    // rustfmt::skip because rustfmt removes the 'where' claus on the associated type.
+    #[rustfmt::skip]
     type ReadFuture<'a> where Self: 'a = impl Future<Output = Result<(), embassy_traits::uart::Error>> + 'a;
 
     fn read<'a>(&'a mut self, buf: &'a mut [u8]) -> Self::ReadFuture<'a> {
-        self.read_dma(buf).map_err(|_| embassy_traits::uart::Error::Other)
+        self.read_dma(buf)
+            .map_err(|_| embassy_traits::uart::Error::Other)
+    }
+}
+
+pub struct State<'d, T: Instance>(StateStorage<StateInner<'d, T>>);
+impl<'d, T: Instance> State<'d, T> {
+    pub fn new() -> Self {
+        Self(StateStorage::new())
+    }
+}
+
+pub struct StateInner<'d, T: Instance> {
+    uart: Uart<'d, T, NoDma, NoDma>,
+    phantom: PhantomData<&'d mut T>,
+
+    rx_waker: WakerRegistration,
+    rx: RingBuffer<'d>,
+
+    tx_waker: WakerRegistration,
+    tx: RingBuffer<'d>,
+}
+
+unsafe impl<'d, T: Instance> Send for StateInner<'d, T> {}
+unsafe impl<'d, T: Instance> Sync for StateInner<'d, T> {}
+
+pub struct BufferedUart<'d, T: Instance> {
+    inner: PeripheralMutex<'d, StateInner<'d, T>>,
+}
+
+impl<'d, T: Instance> Unpin for BufferedUart<'d, T> {}
+
+impl<'d, T: Instance> BufferedUart<'d, T> {
+    pub unsafe fn new(
+        state: &'d mut State<'d, T>,
+        uart: Uart<'d, T, NoDma, NoDma>,
+        irq: impl Unborrow<Target = T::Interrupt> + 'd,
+        tx_buffer: &'d mut [u8],
+        rx_buffer: &'d mut [u8],
+    ) -> BufferedUart<'d, T> {
+        unborrow!(irq);
+
+        let r = uart.inner.regs();
+        r.cr1().modify(|w| {
+            w.set_rxneie(true);
+            w.set_idleie(true);
+        });
+
+        Self {
+            inner: PeripheralMutex::new_unchecked(irq, &mut state.0, move || StateInner {
+                uart,
+                phantom: PhantomData,
+                tx: RingBuffer::new(tx_buffer),
+                tx_waker: WakerRegistration::new(),
+
+                rx: RingBuffer::new(rx_buffer),
+                rx_waker: WakerRegistration::new(),
+            }),
+        }
+    }
+}
+
+impl<'d, T: Instance> StateInner<'d, T>
+where
+    Self: 'd,
+{
+    fn on_rx(&mut self) {
+        let r = self.uart.inner.regs();
+        unsafe {
+            let sr = r.isr().read();
+            if sr.pe() {
+                r.icr().write(|w| {
+                    w.set_pe(true);
+                });
+                trace!("Parity error");
+            } else if sr.fe() {
+                r.icr().write(|w| {
+                    w.set_fe(true);
+                });
+                trace!("Framing error");
+            } else if sr.nf() {
+                r.icr().write(|w| {
+                    w.set_nf(true);
+                });
+                trace!("Noise error");
+            } else if sr.ore() {
+                r.icr().write(|w| {
+                    w.set_ore(true);
+                });
+                trace!("Overrun error");
+            } else if sr.rxne() {
+                let buf = self.rx.push_buf();
+                if buf.is_empty() {
+                    self.rx_waker.wake();
+                } else {
+                    buf[0] = r.rdr().read().0 as u8;
+                    self.rx.push(1);
+                }
+            } else if sr.idle() {
+                r.icr().write(|w| {
+                    w.set_idle(true);
+                });
+                self.rx_waker.wake();
+            };
+        }
+    }
+
+    fn on_tx(&mut self) {
+        let r = self.uart.inner.regs();
+        unsafe {
+            if r.isr().read().txe() {
+                let buf = self.tx.pop_buf();
+                if !buf.is_empty() {
+                    r.cr1().modify(|w| {
+                        w.set_txeie(true);
+                    });
+                    r.tdr().write_value(regs::Dr(buf[0].into()));
+                    self.tx.pop(1);
+                    self.tx_waker.wake();
+                } else {
+                    // Disable interrupt until we have something to transmit again
+                    r.cr1().modify(|w| {
+                        w.set_txeie(false);
+                    });
+                }
+            }
+        }
+    }
+}
+
+impl<'d, T: Instance> PeripheralState for StateInner<'d, T>
+where
+    Self: 'd,
+{
+    type Interrupt = T::Interrupt;
+    fn on_interrupt(&mut self) {
+        self.on_rx();
+        self.on_tx();
+    }
+}
+
+impl<'d, T: Instance> embassy::io::AsyncBufRead for BufferedUart<'d, T> {
+    fn poll_fill_buf(
+        mut self: Pin<&mut Self>,
+        cx: &mut Context<'_>,
+    ) -> Poll<Result<&[u8], embassy::io::Error>> {
+        self.inner.with(|state| {
+            compiler_fence(Ordering::SeqCst);
+
+            // We have data ready in buffer? Return it.
+            let buf = state.rx.pop_buf();
+            if !buf.is_empty() {
+                let buf: &[u8] = buf;
+                // Safety: buffer lives as long as uart
+                let buf: &[u8] = unsafe { core::mem::transmute(buf) };
+                return Poll::Ready(Ok(buf));
+            }
+
+            state.rx_waker.register(cx.waker());
+            Poll::<Result<&[u8], embassy::io::Error>>::Pending
+        })
+    }
+    fn consume(mut self: Pin<&mut Self>, amt: usize) {
+        let signal = self.inner.with(|state| {
+            let full = state.rx.is_full();
+            state.rx.pop(amt);
+            full
+        });
+        if signal {
+            self.inner.pend();
+        }
+    }
+}
+
+impl<'d, T: Instance> embassy::io::AsyncWrite for BufferedUart<'d, T> {
+    fn poll_write(
+        mut self: Pin<&mut Self>,
+        cx: &mut Context<'_>,
+        buf: &[u8],
+    ) -> Poll<Result<usize, embassy::io::Error>> {
+        let (poll, empty) = self.inner.with(|state| {
+            let empty = state.tx.is_empty();
+            let tx_buf = state.tx.push_buf();
+            if tx_buf.is_empty() {
+                state.tx_waker.register(cx.waker());
+                return (Poll::Pending, empty);
+            }
+
+            let n = core::cmp::min(tx_buf.len(), buf.len());
+            tx_buf[..n].copy_from_slice(&buf[..n]);
+            state.tx.push(n);
+
+            (Poll::Ready(Ok(n)), empty)
+        });
+        if empty {
+            self.inner.pend();
+        }
+        poll
     }
 }

examples/stm32l0/Cargo.toml

@@ -21,6 +21,7 @@ embassy = { version = "0.1.0", path = "../../embassy", features = ["defmt", "def
 embassy-traits = { version = "0.1.0", path = "../../embassy-traits", features = ["defmt"] }
 embassy-stm32 = { version = "0.1.0", path = "../../embassy-stm32", features = ["defmt", "defmt-trace", "stm32l072cz", "time-driver-tim3"]  }
 embassy-hal-common = {version = "0.1.0", path = "../../embassy-hal-common" }
+embassy-macros = { path = "../../embassy-macros" }
 
 defmt = "0.2.0"
 defmt-rtt = "0.2.0"

examples/stm32l0/src/bin/usart_irq.rs

@@ -0,0 +1,51 @@
+#![no_std]
+#![no_main]
+#![feature(trait_alias)]
+#![feature(type_alias_impl_trait)]
+#![allow(incomplete_features)]
+
+#[path = "../example_common.rs"]
+mod example_common;
+
+use example_common::*;
+
+use defmt::panic;
+use embassy::executor::Spawner;
+use embassy::io::{AsyncBufReadExt, AsyncWriteExt};
+use embassy_stm32::dma::NoDma;
+use embassy_stm32::interrupt;
+use embassy_stm32::usart::{BufferedUart, Config, State, Uart};
+use embassy_stm32::{rcc, Peripherals};
+
+#[embassy::main]
+async fn main(_spawner: Spawner, mut p: Peripherals) {
+    let mut rcc = rcc::Rcc::new(p.RCC);
+    rcc.enable_debug_wfe(&mut p.DBGMCU, true);
+
+    static mut TX_BUFFER: [u8; 8] = [0; 8];
+    static mut RX_BUFFER: [u8; 256] = [0; 256];
+
+    let mut config = Config::default();
+    config.baudrate = 9600;
+
+    let usart = Uart::new(p.USART1, p.PA10, p.PA9, NoDma, NoDma, config);
+    let mut state = State::new();
+    let mut usart = unsafe {
+        BufferedUart::new(
+            &mut state,
+            usart,
+            interrupt::take!(USART1),
+            &mut TX_BUFFER,
+            &mut RX_BUFFER,
+        )
+    };
+
+    usart.write_all(b"Hello Embassy World!\r\n").await.unwrap();
+    info!("wrote Hello, starting echo");
+
+    let mut buf = [0; 4];
+    loop {
+        usart.read_exact(&mut buf[..]).await.unwrap();
+        usart.write_all(&buf[..]).await.unwrap();
+    }
+}

stm32-data

@@ -1 +1 @@
-Subproject commit af0611ff9b29f2c0ab675a4f0e51d8241a4097f7
+Subproject commit 79ab92a38d1cbb47dda2f6f8556fe1abc9a14f3a