firmware/src/lib.rs

@@ -57,8 +57,6 @@ 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)]
@@ -103,7 +101,7 @@ struct UnusedPinsResources {
 }
 
 bind_interrupts!(struct Irqs {
-    USART1 => usart::InterruptHandler<USART1>;
+    USART1 => usart::BufferedInterruptHandler<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, mode, usart};
+use embassy_stm32::{gpio, usart};
 use embassy_time::Duration;
 use embedded_io_async::Write;
 use static_cell::StaticCell;
@@ -17,15 +17,17 @@ 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::UartTx<'static, mode::Async>,
+    uart: usart::BufferedUartTx<'static>,
     channel: crate::TargetMessagePublisher,
     crc: crc_engine::CrcHandle,
 }
@@ -33,7 +35,7 @@ struct Tx {
 #[allow(dead_code)]
 struct Rx {
     message_buf: &'static mut MessageBuf,
-    uart: usart::RingBufferedUartRx<'static>,
+    uart: usart::BufferedUartRx<'static>,
     channel: crate::TargetMessageGenerator,
     crc: crc_engine::CrcHandle,
     clr_host_transmit: gpio::OutputOpenDrain<'static>,
@@ -49,25 +51,24 @@ 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::Uart::<mode::Async>::new(
+    let (uart_tx, uart_rx) = usart::BufferedUart::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 =
@@ -145,12 +146,17 @@ async fn rx_task(rx: Rx) {
         // with timeouts
         match frx.read_frame_fragment(0, MESSAGE_LENGTH_SIZE).await {
             Ok(()) => {}
-            Err(uart::framed_rx::FramingError::BufferCapacity) => {
+            Err(uart::ReadError::ReadyCheck) => {
+                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::framed_rx::FramingError::Timeout) => {
+            Err(uart::ReadError::Timeout) => {
                 // restart loop, since the sync byte seen wasn't
                 // the beginning of a message, or the sender
                 // stopped sending
@@ -168,12 +174,17 @@ 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::framed_rx::FramingError::BufferCapacity) => {
+            Err(uart::ReadError::ReadyCheck) => {
+                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::framed_rx::FramingError::Timeout) => {
+            Err(uart::ReadError::Timeout) => {
                 // restart loop, since the sync byte seen wasn't
                 // the beginning of a message, or the sender
                 // stopped sending
@@ -187,12 +198,17 @@ 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::framed_rx::FramingError::BufferCapacity) => {
+            Err(uart::ReadError::ReadyCheck) => {
+                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::framed_rx::FramingError::Timeout) => {
+            Err(uart::ReadError::Timeout) => {
                 // restart loop, since the sync byte seen wasn't
                 // the beginning of a message, or the sender
                 // stopped sending
@@ -202,12 +218,9 @@ async fn rx_task(rx: Rx) {
         }
 
         // get the expected CRC-16 from the buffer
-        let expected_crc = LittleEndian::read_u16(&frx.buf[crc_pos..crc_pos + MESSAGE_CRC_SIZE]);
+        let expected_crc = LittleEndian::read_u16(&frx.buf[crc_pos..MESSAGE_CRC_SIZE]);
         // compute the actual CRC-16
-        let computed_crc = rx
+        let computed_crc = rx.crc.compute(&frx.buf[message_pos..message_len]).await;
-            .crc
-            .compute(&frx.buf[message_pos..message_pos + message_len])
-            .await;
 
         if computed_crc != expected_crc {
             println!(
@@ -222,7 +235,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_pos + message_len]);
+        let mut decoder = PbDecoder::new(&frx.buf[message_pos..message_len]);
         let mut msg = HostMessage::default();
         match msg.decode(&mut decoder, message_len) {
             Ok(()) => {

firmware/src/uart/framed_rx.rs

@@ -3,20 +3,12 @@ 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::RingBufferedUartRx<'static>,
+    uart: usart::BufferedUartRx<'static>,
     sync: u8,
     first_byte_timeout: Duration,
     between_bytes_timeout: Duration,
@@ -25,7 +17,7 @@ pub struct FramedUartRx<const N: usize> {
 impl<const N: usize> FramedUartRx<N> {
     pub fn new(
         buf: &'static mut Vec<u8, N>,
-        uart: usart::RingBufferedUartRx<'static>,
+        uart: usart::BufferedUartRx<'static>,
         sync: u8,
         first_byte_timeout: Duration,
         between_bytes_timeout: Duration,
@@ -61,7 +53,7 @@ impl<const N: usize> FramedUartRx<N> {
                         }
                     }
                     Err(_) => {
-                        println!("read error");
+                        println!("read_exact error");
                     }
                 }
             }
@@ -72,7 +64,7 @@ impl<const N: usize> FramedUartRx<N> {
         &mut self,
         fragment_pos: usize,
         fragment_len: usize,
-    ) -> Result<(), FramingError> {
+    ) -> Result<(), uart::ReadError> {
         // compute the number of bytes available in the buffer, and the
         // number of bytes to read
         let bytes_available = self.buf[fragment_pos..].len();
@@ -84,36 +76,27 @@ 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(FramingError::BufferCapacity);
+            return Err(uart::ReadError::BufferCapacity);
         }
+
         // if there aren't enough bytes in the buffer for the
         // fragment, wait for them to arrive, with a timeout
-        let read_start = fragment_pos + bytes_available;
+        if let Ok(r) = uart::read_with_timeouts(
-        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[read_start..read_start + bytes_to_read],
+            &mut self.buf[fragment_pos + bytes_available..bytes_to_read],
             self.first_byte_timeout,
             self.between_bytes_timeout,
         )
         .await
         {
             unsafe {
-                self.buf.set_len(read_start + r);
+                self.buf.set_len(fragment_pos + bytes_available + r);
             }
             if r < bytes_to_read {
-                unsafe {
+                return Err(uart::ReadError::Timeout);
-                    self.buf.set_len(read_start);
-                }
-                return Err(FramingError::Timeout);
             }
         } else {
-            unsafe {
+            return Err(uart::ReadError::Timeout);
-                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, Timer};
+use embassy_time::{Duration, WithTimeout};
 use embedded_io::ReadReady;
+use embedded_io_async::Read;
 use thiserror::Error;
 
 pub mod framed_rx;
@@ -9,15 +9,14 @@ pub mod framed_rx;
 #[derive(Error, Debug)]
 pub enum ReadError {
     #[error("timeout expired waiting for data")]
-    Timeout { bytes_read: usize },
+    Timeout,
-    #[error("usart error")]
+    #[error("ready check on uart failed")]
-    Other {
+    ReadyCheck,
-        bytes_read: usize,
+    #[error("buffer is too small for message fragment")]
-        source: usart::Error,
+    BufferCapacity,
-    },
 }
 
-/// Reads data from a `RingBufferedUartRx`, and supports two timeouts:
+/// Reads data from a `BufferedUartRx`, and supports two timeouts:
 ///
 /// `first_byte`: if no data is available immediately, will wait this
 /// long for the first byte to arrive
@@ -25,65 +24,42 @@ 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::Timeout` if a timeout expires; the
+/// Returns `ReadError::ReadyCheck` if the initial check for data returns an
-/// `bytes_read` field of the error will contain the number of bytes
+/// error
-/// which had been read before the timeout occurred, if any
 ///
-/// Returns `Ok` with the number of bytes read if no error or
+/// Returns `ReadError::Timeout` if the first-byte timeout expires
-/// timeout occurs
+///
-async fn read_exact_with_timeouts(
+/// Returns `Ok()` with the number of bytes read in all other cases,
-    rx: &mut usart::RingBufferedUartRx<'_>,
+/// including a between-bytes timeout; in that case the data read
+/// 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> {
-    let buf_len = buf.len();
+    // if the requested read size is zero bytes, return immediately
-    let mut bytes_read = 0;
+    if buf.is_empty() {
+        return Ok(0);
+    }
 
-    while bytes_read < buf_len {
+    let mut bytes_read: usize = 0;
-        match rx.read_ready() {
+    let mut timeout: Duration;
-            Ok(false) => {}
+    if let Ok(ready) = rx.read_ready() {
-            Ok(true) => match rx.read(&mut buf[bytes_read..buf_len]).await {
+        timeout = if ready { between_bytes } else { first_byte };
-                Ok(r) => {
+    } else {
-                    bytes_read += r;
+        return Err(ReadError::ReadyCheck);
-                    continue;
+    }
-                }
+
-                Err(e) => {
+    while bytes_read < buf.len() {
-                    rx.start_uart();
+        if let Ok(Ok(r)) = rx.read(&mut buf[bytes_read..]).with_timeout(timeout).await {
-                    return Err(ReadError::Other {
+            bytes_read += r;
-                        source: e,
+            timeout = between_bytes;
-                        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
+            if bytes_read > 0 {
-        });
+                return Ok(bytes_read);
-        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 retain_bytes = buf.len() - remove;
+    let post_bytes = buf.len() - remove;
     #[cfg(not(feature = "copy-within"))]
-    for i in 0..retain_bytes {
+    for i in 0..post_bytes {
         buf[i] = buf[remove + i];
     }
     #[cfg(feature = "copy-within")]
     buf.copy_within(remove.., 0);
-    buf.truncate(retain_bytes);
+    buf.truncate(post_bytes);
 }