firmware: UART reception rewrite

* Rewrite UART utility function for receiving bytes with timeouts

* Improve structure of FramedUartRx module

* Convert host interface UART back to DMA with ring-buffer on receive
  path

firmware/src/lib.rs

@@ -57,6 +57,8 @@ struct HostInterfaceResources {
     clr_host_transmit: PC14,
     set_host_transmit: PC15,
     host_active_led: PB7,
+    tx_dma: DMA1_CH1,
+    rx_dma: DMA1_CH2,
 }
 
 #[resource_group(no_aliases)]
@@ -101,7 +103,7 @@ struct UnusedPinsResources {
 }
 
 bind_interrupts!(struct Irqs {
-    USART1 => usart::BufferedInterruptHandler<USART1>;
+    USART1 => usart::InterruptHandler<USART1>;
     USART3_4 => usart::BufferedInterruptHandler<USART3>, usart::BufferedInterruptHandler<USART4>;
 });
 

firmware/src/tasks/host_interface/mod.rs

@@ -1,7 +1,7 @@
 use crate::{crc_engine, println, uart, utils};
 use byteorder::{ByteOrder, LittleEndian};
 use embassy_executor::Spawner;
-use embassy_stm32::{gpio, usart};
+use embassy_stm32::{gpio, mode, usart};
 use embassy_time::Duration;
 use embedded_io_async::Write;
 use static_cell::StaticCell;
@@ -17,17 +17,15 @@ const MESSAGE_CRC_SIZE: usize = 2;
 const MAX_INBOUND_MESSAGE_SIZE: usize = MESSAGE_LENGTH_SIZE + MESSAGE_CRC_SIZE + 2048;
 
 type UartRxBuf = Vec<u8, 128>;
-type UartTxBuf = Vec<u8, 128>;
 
 static UART_RX_BUF: StaticCell<UartRxBuf> = StaticCell::new();
-static UART_TX_BUF: StaticCell<UartTxBuf> = StaticCell::new();
 
 type MessageBuf = Vec<u8, MAX_INBOUND_MESSAGE_SIZE>;
 
 static MESSAGE_BUF: StaticCell<MessageBuf> = StaticCell::new();
 
 struct Tx {
-    uart: usart::BufferedUartTx<'static>,
+    uart: usart::UartTx<'static, mode::Async>,
     channel: crate::TargetMessagePublisher,
     crc: crc_engine::CrcHandle,
 }
@@ -35,7 +33,7 @@ struct Tx {
 #[allow(dead_code)]
 struct Rx {
     message_buf: &'static mut MessageBuf,
-    uart: usart::BufferedUartRx<'static>,
+    uart: usart::RingBufferedUartRx<'static>,
     channel: crate::TargetMessageGenerator,
     crc: crc_engine::CrcHandle,
     clr_host_transmit: gpio::OutputOpenDrain<'static>,
@@ -51,24 +49,25 @@ pub fn start(
     generator: crate::TargetMessageGenerator,
 ) {
     let rx_buf = UART_RX_BUF.init(UartRxBuf::new());
-    let tx_buf = UART_TX_BUF.init(UartTxBuf::new());
     let message_buf = MESSAGE_BUF.init(MessageBuf::new());
 
     let mut config = usart::Config::default();
     config.baudrate = 38400;
 
-    let (uart_tx, uart_rx) = usart::BufferedUart::new(
+    let (uart_tx, uart_rx) = usart::Uart::<mode::Async>::new(
         r.usart,
         r.rx_pin,
         r.tx_pin,
-        tx_buf,
-        rx_buf,
         crate::Irqs,
+        r.tx_dma,
+        r.rx_dma,
         config,
     )
     .unwrap()
     .split();
 
+    let uart_rx = uart_rx.into_ring_buffered(rx_buf);
+
     let clr_host_transmit =
         gpio::OutputOpenDrain::new(r.clr_host_transmit, gpio::Level::High, gpio::Speed::Low);
     let set_host_transmit =
@@ -146,17 +145,12 @@ async fn rx_task(rx: Rx) {
         // with timeouts
         match frx.read_frame_fragment(0, MESSAGE_LENGTH_SIZE).await {
             Ok(()) => {}
-            Err(uart::ReadError::ReadyCheck) => {
+            Err(uart::framed_rx::FramingError::BufferCapacity) => {
-                println!("uart ready-check error reading message length");
-                frx.buf.clear();
-                continue;
-            }
-            Err(uart::ReadError::BufferCapacity) => {
                 println!("insufficient space to read message length");
                 frx.buf.clear();
                 continue;
             }
-            Err(uart::ReadError::Timeout) => {
+            Err(uart::framed_rx::FramingError::Timeout) => {
                 // restart loop, since the sync byte seen wasn't
                 // the beginning of a message, or the sender
                 // stopped sending
@@ -174,17 +168,12 @@ async fn rx_task(rx: Rx) {
         let message_pos = MESSAGE_LENGTH_SIZE;
         match frx.read_frame_fragment(message_pos, message_len).await {
             Ok(()) => {}
-            Err(uart::ReadError::ReadyCheck) => {
+            Err(uart::framed_rx::FramingError::BufferCapacity) => {
-                println!("uart ready-check error reading message body");
-                frx.buf.clear();
-                continue;
-            }
-            Err(uart::ReadError::BufferCapacity) => {
                 println!("insufficient space to read message body");
                 frx.buf.clear();
                 continue;
             }
-            Err(uart::ReadError::Timeout) => {
+            Err(uart::framed_rx::FramingError::Timeout) => {
                 // restart loop, since the sync byte seen wasn't
                 // the beginning of a message, or the sender
                 // stopped sending
@@ -198,17 +187,12 @@ async fn rx_task(rx: Rx) {
         let crc_pos = message_pos + message_len;
         match frx.read_frame_fragment(crc_pos, MESSAGE_CRC_SIZE).await {
             Ok(()) => {}
-            Err(uart::ReadError::ReadyCheck) => {
+            Err(uart::framed_rx::FramingError::BufferCapacity) => {
-                println!("uart ready-check error reading CRC-16");
-                frx.buf.clear();
-                continue;
-            }
-            Err(uart::ReadError::BufferCapacity) => {
                 println!("insufficient space to read CRC-16");
                 frx.buf.clear();
                 continue;
             }
-            Err(uart::ReadError::Timeout) => {
+            Err(uart::framed_rx::FramingError::Timeout) => {
                 // restart loop, since the sync byte seen wasn't
                 // the beginning of a message, or the sender
                 // stopped sending
@@ -218,9 +202,12 @@ async fn rx_task(rx: Rx) {
         }
 
         // get the expected CRC-16 from the buffer
-        let expected_crc = LittleEndian::read_u16(&frx.buf[crc_pos..MESSAGE_CRC_SIZE]);
+        let expected_crc = LittleEndian::read_u16(&frx.buf[crc_pos..crc_pos + MESSAGE_CRC_SIZE]);
         // compute the actual CRC-16
-        let computed_crc = rx.crc.compute(&frx.buf[message_pos..message_len]).await;
+        let computed_crc = rx
+            .crc
+            .compute(&frx.buf[message_pos..message_pos + message_len])
+            .await;
 
         if computed_crc != expected_crc {
             println!(
@@ -235,7 +222,7 @@ async fn rx_task(rx: Rx) {
         // at this point the buffer has been confirmed to contain a
         // valid message frame, so any failures beyond this point can
         // only drop the message, it cannot be re-parsed
-        let mut decoder = PbDecoder::new(&frx.buf[message_pos..message_len]);
+        let mut decoder = PbDecoder::new(&frx.buf[message_pos..message_pos + message_len]);
         let mut msg = HostMessage::default();
         match msg.decode(&mut decoder, message_len) {
             Ok(()) => {

firmware/src/uart/framed_rx.rs

@@ -3,12 +3,20 @@ use crate::uart;
 use crate::utils;
 use embassy_stm32::usart;
 use embassy_time::Duration;
-use embedded_io_async::Read;
 use micropb::heapless::Vec;
+use thiserror::Error;
+
+#[derive(Error, Debug)]
+pub enum FramingError {
+    #[error("insufficent buffer capacity")]
+    BufferCapacity,
+    #[error("timeout expired waiting for data")]
+    Timeout,
+}
 
 pub struct FramedUartRx<const N: usize> {
     pub buf: &'static mut Vec<u8, N>,
-    uart: usart::BufferedUartRx<'static>,
+    uart: usart::RingBufferedUartRx<'static>,
     sync: u8,
     first_byte_timeout: Duration,
     between_bytes_timeout: Duration,
@@ -17,7 +25,7 @@ pub struct FramedUartRx<const N: usize> {
 impl<const N: usize> FramedUartRx<N> {
     pub fn new(
         buf: &'static mut Vec<u8, N>,
-        uart: usart::BufferedUartRx<'static>,
+        uart: usart::RingBufferedUartRx<'static>,
         sync: u8,
         first_byte_timeout: Duration,
         between_bytes_timeout: Duration,
@@ -53,7 +61,7 @@ impl<const N: usize> FramedUartRx<N> {
                         }
                     }
                     Err(_) => {
-                        println!("read_exact error");
+                        println!("read error");
                     }
                 }
             }
@@ -64,7 +72,7 @@ impl<const N: usize> FramedUartRx<N> {
         &mut self,
         fragment_pos: usize,
         fragment_len: usize,
-    ) -> Result<(), uart::ReadError> {
+    ) -> Result<(), FramingError> {
         // compute the number of bytes available in the buffer, and the
         // number of bytes to read
         let bytes_available = self.buf[fragment_pos..].len();
@@ -76,27 +84,36 @@ impl<const N: usize> FramedUartRx<N> {
         // if the fragment will not fit in the buffer's available space,
         // return an error
         if bytes_to_read > (self.buf.capacity() - self.buf.len()) {
-            return Err(uart::ReadError::BufferCapacity);
+            return Err(FramingError::BufferCapacity);
         }
-
         // if there aren't enough bytes in the buffer for the
         // fragment, wait for them to arrive, with a timeout
-        if let Ok(r) = uart::read_with_timeouts(
+        let read_start = fragment_pos + bytes_available;
+        unsafe {
+            self.buf.set_len(read_start + bytes_to_read);
+        }
+        if let Ok(r) = uart::read_exact_with_timeouts(
             &mut self.uart,
-            &mut self.buf[fragment_pos + bytes_available..bytes_to_read],
+            &mut self.buf[read_start..read_start + bytes_to_read],
             self.first_byte_timeout,
             self.between_bytes_timeout,
         )
         .await
         {
             unsafe {
-                self.buf.set_len(fragment_pos + bytes_available + r);
+                self.buf.set_len(read_start + r);
             }
             if r < bytes_to_read {
-                return Err(uart::ReadError::Timeout);
+                unsafe {
+                    self.buf.set_len(read_start);
+                }
+                return Err(FramingError::Timeout);
             }
         } else {
-            return Err(uart::ReadError::Timeout);
+            unsafe {
+                self.buf.set_len(read_start);
+            }
+            return Err(FramingError::Timeout);
         }
 
         Ok(())

firmware/src/uart/mod.rs

@@ -1,7 +1,7 @@
+use embassy_futures::select;
 use embassy_stm32::usart;
-use embassy_time::{Duration, WithTimeout};
+use embassy_time::{Duration, Timer};
 use embedded_io::ReadReady;
-use embedded_io_async::Read;
 use thiserror::Error;
 
 pub mod framed_rx;
@@ -9,14 +9,15 @@ pub mod framed_rx;
 #[derive(Error, Debug)]
 pub enum ReadError {
     #[error("timeout expired waiting for data")]
-    Timeout,
+    Timeout { bytes_read: usize },
-    #[error("ready check on uart failed")]
+    #[error("usart error")]
-    ReadyCheck,
+    Other {
-    #[error("buffer is too small for message fragment")]
+        bytes_read: usize,
-    BufferCapacity,
+        source: usart::Error,
+    },
 }
 
-/// Reads data from a `BufferedUartRx`, and supports two timeouts:
+/// Reads data from a `RingBufferedUartRx`, and supports two timeouts:
 ///
 /// `first_byte`: if no data is available immediately, will wait this
 /// long for the first byte to arrive
@@ -24,42 +25,65 @@ pub enum ReadError {
 /// `between_bytes`: if data is available immediately, or after the
 /// first byte has been read, will wait this long between bytes
 ///
-/// Returns `ReadError::ReadyCheck` if the initial check for data returns an
+/// Returns `ReadError::Timeout` if a timeout expires; the
-/// error
+/// `bytes_read` field of the error will contain the number of bytes
+/// which had been read before the timeout occurred, if any
 ///
-/// Returns `ReadError::Timeout` if the first-byte timeout expires
+/// Returns `Ok` with the number of bytes read if no error or
-///
+/// timeout occurs
-/// Returns `Ok()` with the number of bytes read in all other cases,
+async fn read_exact_with_timeouts(
-/// including a between-bytes timeout; in that case the data read
+    rx: &mut usart::RingBufferedUartRx<'_>,
-/// before the timeout occurred is present in the buffer
-async fn read_with_timeouts(
-    rx: &mut usart::BufferedUartRx<'_>,
     buf: &mut [u8],
     first_byte: Duration,
     between_bytes: Duration,
 ) -> Result<usize, ReadError> {
-    // if the requested read size is zero bytes, return immediately
+    let buf_len = buf.len();
-    if buf.is_empty() {
+    let mut bytes_read = 0;
-        return Ok(0);
-    }
 
-    let mut bytes_read: usize = 0;
+    while bytes_read < buf_len {
-    let mut timeout: Duration;
+        match rx.read_ready() {
-    if let Ok(ready) = rx.read_ready() {
+            Ok(false) => {}
-        timeout = if ready { between_bytes } else { first_byte };
+            Ok(true) => match rx.read(&mut buf[bytes_read..buf_len]).await {
-    } else {
+                Ok(r) => {
-        return Err(ReadError::ReadyCheck);
+                    bytes_read += r;
-    }
+                    continue;
-
+                }
-    while bytes_read < buf.len() {
+                Err(e) => {
-        if let Ok(Ok(r)) = rx.read(&mut buf[bytes_read..]).with_timeout(timeout).await {
+                    rx.start_uart();
-            bytes_read += r;
+                    return Err(ReadError::Other {
-            timeout = between_bytes;
+                        source: e,
-        } else {
+                        bytes_read,
-            if bytes_read > 0 {
+                    });
-                return Ok(bytes_read);
+                }
+            },
+            Err(e) => {
+                rx.start_uart();
+                return Err(ReadError::Other {
+                    source: e,
+                    bytes_read,
+                });
+            }
+        }
+        let timeout = Timer::after(if bytes_read == 0 {
+            first_byte
+        } else {
+            between_bytes
+        });
+        let uart = rx.read(&mut buf[bytes_read..=bytes_read]);
+        match select::select(uart, timeout).await {
+            select::Either::First(Ok(r)) => {
+                bytes_read += r;
+            }
+            select::Either::First(Err(e)) => {
+                rx.start_uart();
+                return Err(ReadError::Other {
+                    source: e,
+                    bytes_read,
+                });
+            }
+            select::Either::Second(()) => {
+                return Err(ReadError::Timeout { bytes_read });
             }
-            return Err(ReadError::Timeout);
         }
     }
 

firmware/src/utils.rs

@@ -1,12 +1,12 @@
 use micropb::heapless::Vec;
 
 pub fn remove_leading_bytes<const N: usize>(buf: &mut Vec<u8, N>, remove: usize) {
-    let post_bytes = buf.len() - remove;
+    let retain_bytes = buf.len() - remove;
     #[cfg(not(feature = "copy-within"))]
-    for i in 0..post_bytes {
+    for i in 0..retain_bytes {
         buf[i] = buf[remove + i];
     }
     #[cfg(feature = "copy-within")]
     buf.copy_within(remove.., 0);
-    buf.truncate(post_bytes);
+    buf.truncate(retain_bytes);
 }