improve UART impl

va416xx-hal/src/uart.rs

@@ -9,6 +9,7 @@
 //! - [UART simple example](https://egit.irs.uni-stuttgart.de/rust/va416xx-rs/src/branch/main/examples/simple/examples/uart.rs)
 //! - [UART echo with IRQ and Embassy](https://egit.irs.uni-stuttgart.de/rust/va416xx-rs/src/branch/main/examples/embassy/src/bin/uart-echo-with-irq.rs)
 //! - [Flashloader app using UART with IRQs](https://egit.irs.uni-stuttgart.de/rust/va416xx-rs/src/branch/main/flashloader)
+use core::convert::Infallible;
 use core::ops::Deref;
 
 use embedded_hal_nb::serial::Read;
@@ -69,17 +70,28 @@ impl RxPin<Uart2> for Pin<PF9, AltFunc1> {}
 // Regular Definitions
 //==================================================================================================
 
-#[derive(Debug)]
+#[derive(Debug, PartialEq, Eq)]
+#[cfg_attr(feature = "defmt", derive(defmt::Format))]
+pub enum RxError {
+    Overrun,
+    Framing,
+    Parity,
+}
+#[derive(Debug, PartialEq, Eq)]
 #[cfg_attr(feature = "defmt", derive(defmt::Format))]
 pub enum Error {
-    Overrun,
+    Rx(RxError),
-    FramingError,
-    ParityError,
     BreakCondition,
     TransferPending,
     BufferTooShort,
 }
 
+impl From<RxError> for Error {
+    fn from(value: RxError) -> Self {
+        Self::Rx(value)
+    }
+}
+
 #[derive(Debug, PartialEq, Eq, Copy, Clone)]
 #[cfg_attr(feature = "defmt", derive(defmt::Format))]
 pub enum Event {
@@ -295,43 +307,9 @@ enum IrqReceptionMode {
 }
 
 //==================================================================================================
-// UART implementation
+// UART peripheral wrapper
 //==================================================================================================
 
-/// Type erased variant of a UART. Can be created with the [Uart::downgrade] function.
-pub struct UartBase<Uart> {
-    uart: Uart,
-    tx: Tx<Uart>,
-    rx: Rx<Uart>,
-}
-/// Serial abstraction. Entry point to create a new UART
-pub struct Uart<UartInstance, Pins> {
-    inner: UartBase<UartInstance>,
-    pins: Pins,
-}
-
-/// Serial receiver.
-///
-/// Can be created by using the [Uart::split] or [UartBase::split] API.
-pub struct Rx<Uart>(Uart);
-
-/// Serial transmitter
-///
-/// Can be created by using the [Uart::split] or [UartBase::split] API.
-pub struct Tx<Uart>(Uart);
-
-impl<Uart: Instance> Rx<Uart> {
-    fn new(uart: Uart) -> Self {
-        Self(uart)
-    }
-}
-
-impl<Uart> Tx<Uart> {
-    fn new(uart: Uart) -> Self {
-        Self(uart)
-    }
-}
-
 pub trait Instance: Deref<Target = uart_base::RegisterBlock> {
     const IDX: u8;
     const PERIPH_SEL: PeripheralSelect;
@@ -389,6 +367,17 @@ impl Instance for Uart2 {
     }
 }
 
+//==================================================================================================
+// UART implementation
+//==================================================================================================
+
+/// Type erased variant of a UART. Can be created with the [Uart::downgrade] function.
+pub struct UartBase<Uart> {
+    uart: Uart,
+    tx: Tx<Uart>,
+    rx: Rx<Uart>,
+}
+
 impl<Uart: Instance> UartBase<Uart> {
     fn init(self, config: Config, clocks: &Clocks) -> Self {
         if Uart::IDX == 2 {
@@ -522,6 +511,12 @@ impl<Uart: Instance> UartBase<Uart> {
     }
 }
 
+/// Serial abstraction. Entry point to create a new UART
+pub struct Uart<UartInstance, Pins> {
+    inner: UartBase<UartInstance>,
+    pins: Pins,
+}
+
 impl<TxPinInst: TxPin<UartInstance>, RxPinInst: RxPin<UartInstance>, UartInstance: Instance>
     Uart<UartInstance, (TxPinInst, RxPinInst)>
 {
@@ -617,6 +612,17 @@ impl<TxPinInst: TxPin<UartInstance>, RxPinInst: RxPin<UartInstance>, UartInstanc
     }
 }
 
+/// Serial receiver.
+///
+/// Can be created by using the [Uart::split] or [UartBase::split] API.
+pub struct Rx<Uart>(Uart);
+
+impl<Uart: Instance> Rx<Uart> {
+    fn new(uart: Uart) -> Self {
+        Self(uart)
+    }
+}
+
 impl<Uart: Instance> Rx<Uart> {
     /// Direct access to the peripheral structure.
     ///
@@ -642,6 +648,33 @@ impl<Uart: Instance> Rx<Uart> {
         self.0.enable().modify(|_, w| w.rxenable().clear_bit());
     }
 
+    /// Low level function to read a word from the UART FIFO.
+    ///
+    /// Uses the [nb] API to allow usage in blocking and non-blocking contexts.
+    ///
+    /// Please note that you might have to mask the returned value with 0xff to retrieve the actual
+    /// value if you use the manual parity mode. See chapter 11.4.1 for more information.
+    #[inline(always)]
+    pub fn read_fifo(&self) -> nb::Result<u32, Infallible> {
+        if self.0.rxstatus().read().rdavl().bit_is_clear() {
+            return Err(nb::Error::WouldBlock);
+        }
+        Ok(self.read_fifo_unchecked())
+    }
+
+    /// Low level function to read a word from from the UART FIFO.
+    ///
+    /// This does not necesarily mean there is a word in the FIFO available.
+    /// Use the [Self::read_fifo] function to read a word from the FIFO reliably using the [nb]
+    /// API.
+    ///
+    /// Please note that you might have to mask the returned value with 0xff to retrieve the actual
+    /// value if you use the manual parity mode. See chapter 11.4.1 for more information.
+    #[inline(always)]
+    pub fn read_fifo_unchecked(&self) -> u32 {
+        self.0.data().read().bits()
+    }
+
     pub fn to_rx_with_irq(self) -> RxWithIrq<Uart> {
         RxWithIrq(self)
     }
@@ -651,6 +684,17 @@ impl<Uart: Instance> Rx<Uart> {
     }
 }
 
+/// Serial transmitter
+///
+/// Can be created by using the [Uart::split] or [UartBase::split] API.
+pub struct Tx<Uart>(Uart);
+
+impl<Uart> Tx<Uart> {
+    fn new(uart: Uart) -> Self {
+        Self(uart)
+    }
+}
+
 impl<Uart: Instance> Tx<Uart> {
     /// Direct access to the peripheral structure.
     ///
@@ -675,6 +719,32 @@ impl<Uart: Instance> Tx<Uart> {
     pub fn disable(&mut self) {
         self.0.enable().modify(|_, w| w.txenable().clear_bit());
     }
+
+    /// Low level function to write a word to the UART FIFO.
+    ///
+    /// Uses the [nb] API to allow usage in blocking and non-blocking contexts.
+    ///
+    /// Please note that you might have to mask the returned value with 0xff to retrieve the actual
+    /// value if you use the manual parity mode. See chapter 11.4.1 for more information.
+    #[inline(always)]
+    pub fn write_fifo(&self, data: u32) -> nb::Result<(), Infallible> {
+        if self.0.txstatus().read().wrrdy().bit_is_clear() {
+            return Err(nb::Error::WouldBlock);
+        }
+        self.write_fifo_unchecked(data);
+        Ok(())
+    }
+
+    /// Low level function to write a word to the UART FIFO.
+    ///
+    /// This does not necesarily mean that the FIFO can process another word because it might be
+    /// full.
+    /// Use the [Self::read_fifo] function to write a word to the FIFO reliably using the [nb]
+    /// API.
+    #[inline(always)]
+    pub fn write_fifo_unchecked(&self, data: u32) {
+        self.0.data().write(|w| unsafe { w.bits(data) });
+    }
 }
 
 #[derive(Default, Debug)]
@@ -937,25 +1007,22 @@ impl<Uart: Instance> RxWithIrq<Uart> {
     fn read_handler(
         &self,
         errors: &mut IrqUartError,
-        read_res: &nb::Result<u8, Error>,
+        read_res: &nb::Result<u8, RxError>,
     ) -> Option<u8> {
         match read_res {
             Ok(byte) => Some(*byte),
             Err(nb::Error::WouldBlock) => None,
             Err(nb::Error::Other(e)) => {
                 match e {
-                    Error::Overrun => {
+                    RxError::Overrun => {
                         errors.overflow = true;
                     }
-                    Error::FramingError => {
+                    RxError::Framing => {
                         errors.framing = true;
                     }
-                    Error::ParityError => {
+                    RxError::Parity => {
                         errors.parity = true;
                     }
-                    _ => {
-                        errors.other = true;
-                    }
                 }
                 None
             }
@@ -1000,18 +1067,34 @@ impl embedded_io::Error for Error {
     }
 }
 
+impl embedded_io::Error for RxError {
+    fn kind(&self) -> embedded_io::ErrorKind {
+        embedded_io::ErrorKind::Other
+    }
+}
+
 impl embedded_hal_nb::serial::Error for Error {
     fn kind(&self) -> embedded_hal_nb::serial::ErrorKind {
         embedded_hal_nb::serial::ErrorKind::Other
     }
 }
 
+impl embedded_hal_nb::serial::Error for RxError {
+    fn kind(&self) -> embedded_hal_nb::serial::ErrorKind {
+        match self {
+            RxError::Overrun => embedded_hal_nb::serial::ErrorKind::Overrun,
+            RxError::Framing => embedded_hal_nb::serial::ErrorKind::FrameFormat,
+            RxError::Parity => embedded_hal_nb::serial::ErrorKind::Parity,
+        }
+    }
+}
+
 impl<Uart> embedded_io::ErrorType for Rx<Uart> {
-    type Error = Error;
+    type Error = RxError;
 }
 
 impl<Uart> embedded_hal_nb::serial::ErrorType for Rx<Uart> {
-    type Error = Error;
+    type Error = RxError;
 }
 
 impl<Uart: Instance> embedded_hal_nb::serial::Read<u8> for Rx<Uart> {
@@ -1019,11 +1102,11 @@ impl<Uart: Instance> embedded_hal_nb::serial::Read<u8> for Rx<Uart> {
         let uart = unsafe { &(*Uart::ptr()) };
         let status_reader = uart.rxstatus().read();
         let err = if status_reader.rxovr().bit_is_set() {
-            Some(Error::Overrun)
+            Some(RxError::Overrun)
         } else if status_reader.rxfrm().bit_is_set() {
-            Some(Error::FramingError)
+            Some(RxError::Framing)
         } else if status_reader.rxpar().bit_is_set() {
-            Some(Error::ParityError)
+            Some(RxError::Parity)
         } else {
             None
         };
@@ -1032,14 +1115,15 @@ impl<Uart: Instance> embedded_hal_nb::serial::Read<u8> for Rx<Uart> {
             // and parity status bits. We have to read the DATA register
             // so that the next status reflects the next DATA word
             // For overrun error, we read as well to clear the peripheral
-            uart.data().read().bits();
+            self.read_fifo_unchecked();
-            Err(err.into())
+            return Err(err.into());
-        } else if status_reader.rdavl().bit_is_set() {
-            let data = uart.data().read().bits();
-            Ok((data & 0xff) as u8)
-        } else {
-            Err(nb::Error::WouldBlock)
         }
+        self.read_fifo().map(|val| (val & 0xff) as u8).map_err(|e| {
+            if let nb::Error::Other(_) = e {
+                unreachable!()
+            }
+            nb::Error::WouldBlock
+        })
     }
 }
 
@@ -1059,28 +1143,16 @@ impl<Uart: Instance> embedded_io::Read for Rx<Uart> {
 }
 
 impl<Uart> embedded_io::ErrorType for Tx<Uart> {
-    type Error = Error;
+    type Error = Infallible;
 }
 
 impl<Uart> embedded_hal_nb::serial::ErrorType for Tx<Uart> {
-    type Error = Error;
+    type Error = Infallible;
 }
 
 impl<Uart: Instance> embedded_hal_nb::serial::Write<u8> for Tx<Uart> {
     fn write(&mut self, word: u8) -> nb::Result<(), Self::Error> {
-        let reader = unsafe { &(*Uart::ptr()) }.txstatus().read();
+        self.write_fifo(word as u32)
-        if reader.wrrdy().bit_is_clear() {
-            return Err(nb::Error::WouldBlock);
-        } else {
-            // DPARITY bit not supported yet
-            unsafe {
-                // NOTE(unsafe) atomic write to data register
-                // NOTE(write_volatile) 8-bit write that's not
-                // possible through the svd2rust API
-                (*Uart::ptr()).data().write(|w| w.bits(word as u32));
-            }
-        }
-        Ok(())
     }
 
     fn flush(&mut self) -> nb::Result<(), Self::Error> {
@@ -1123,7 +1195,7 @@ impl<UartInstance> embedded_hal_nb::serial::ErrorType for UartBase<UartInstance>
 
 impl<Uart: Instance> embedded_hal_nb::serial::Read<u8> for UartBase<Uart> {
     fn read(&mut self) -> nb::Result<u8, Self::Error> {
-        self.rx.read()
+        self.rx.read().map_err(|e| e.map(Error::Rx))
     }
 }