From b401085f3220fd9b3417b906ee048329a0688b13 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 7 Mar 2025 15:56:59 +0100 Subject: [PATCH] simplfied UART error handling --- va108xx-hal/CHANGELOG.md | 46 +++++--- va108xx-hal/src/i2c.rs | 18 +-- va108xx-hal/src/uart/mod.rs | 184 ++++++++++-------------------- va108xx-hal/src/uart/rx_asynch.rs | 4 +- 4 files changed, 101 insertions(+), 151 deletions(-) diff --git a/va108xx-hal/CHANGELOG.md b/va108xx-hal/CHANGELOG.md index 3b2264b..fd7e926 100644 --- a/va108xx-hal/CHANGELOG.md +++ b/va108xx-hal/CHANGELOG.md @@ -8,6 +8,16 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## [unreleased] +## [v0.11.0] 2025-03-07 + +## Changed + +- Bugfix for I2C `TimingCfg::reg` +- Simplified UART error handling. All APIs are now infallible because writing to a FIFO or + reading from a FIFO never fails. Users can either poll errors using `Rx::poll_errors` or + `Uart::poll_rx_errors` / `UartBase::poll_rx_errors`, or detect errors using the provided + interrupt handlers. + ## [v0.10.0] 2025-02-17 ## Added @@ -104,14 +114,14 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Updated `embedded-hal` to v1 - Added optional `defmt` v0.3 feature and support. -## [v0.5.2] 2024-06-16 +## v0.5.2 2024-06-16 ## Fixed - Replaced usage to `ptr::write_volatile` in UART module which is denied on more recent Rust compilers. -## [v0.5.1] +## v0.5.1 ### Changes @@ -120,7 +130,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - `once_cell` to 1.12.0 - Other dependencies: Only revision has changed -## [v0.5.0] +## v0.5.0 ### Added @@ -133,14 +143,14 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Bugfix in UART code where RX and TX could not be enabled or disabled independently -## [v0.4.3] +## v0.4.3 - Various smaller fixes for READMEs, update of links in documentation - Simplified CI for github, do not use `cross` - New `blinky-pac` example - Use HAL delay in `blinky` example -## [v0.4.2] +## v0.4.2 ### Added @@ -150,24 +160,24 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Clear TX and RX FIFO in SPI transfer function -## [v0.4.1] +## v0.4.1 ### Fixed - Initial blockmode setting was not set in SPI constructor -## [v0.4.0] +## v0.4.0 ### Changed - Replaced `Hertz` by `impl Into` completely and removed `+ Copy` where not necessary -## [v0.3.1] +## v0.3.1 - Updated all links to point to new repository -## [v0.3.0] +## v0.3.0 ### Added @@ -179,7 +189,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Primary repository now hosted on IRS external git: https://egit.irs.uni-stuttgart.de/rust/va108xx-hal - Relicensed as Apache-2.0 -## [0.2.3] +## v0.2.3 ### Added @@ -191,7 +201,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Improved Timer API. It is now possible to simply use `new` on `CountDownTimer` -## [0.2.2] +## v0.2.2 ### Added @@ -203,7 +213,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - API which expects values in Hertz now uses `impl Into` as input parameter -## [0.2.1] +## v0.2.1 ### Added @@ -217,7 +227,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Moved the `FilterClkSel` struct to the `clock` module, re-exporting in `gpio` - Clearing output state at initialization of Output pins -## [0.2.0] +## v0.2.0 ### Changed @@ -232,7 +242,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Some bugfixes for GPIO implementation - Rust edition updated to 2021 -## [0.1.0] +## v0.1.0 ### Added @@ -241,3 +251,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - RTT example application - Added basic test binary in form of an example - README with basic instructions how to set up own binary crate + +[unreleased]: https://egit.irs.uni-stuttgart.de/rust/va108xx-rs/compare/va108xx-hal-v0.11.0...HEAD +[v0.11.0]: https://egit.irs.uni-stuttgart.de/rust/va108xx-rs/compare/va108xx-hal-v0.10.0...va108xx-hal-v0.11.0 +[v0.10.0]: https://egit.irs.uni-stuttgart.de/rust/va108xx-rs/compare/va108xx-hal-v0.9.0...va108xx-hal-v0.10.0 +[v0.9.0]: https://egit.irs.uni-stuttgart.de/rust/va108xx-rs/compare/va108xx-hal-v0.8.0...va108xx-hal-v0.9.0 +[v0.8.0]: https://egit.irs.uni-stuttgart.de/rust/va108xx-rs/compare/va108xx-hal-v0.7.0...va108xx-hal-v0.8.0 +[v0.7.0]: https://egit.irs.uni-stuttgart.de/rust/va108xx-rs/compare/va108xx-hal-v0.6.0...va108xx-hal-v0.7.0 +[v0.6.0]: https://egit.irs.uni-stuttgart.de/rust/va108xx-rs/src/tag/va108xx-hal-v0.6.0 diff --git a/va108xx-hal/src/i2c.rs b/va108xx-hal/src/i2c.rs index 4ed9373..1dd53e8 100644 --- a/va108xx-hal/src/i2c.rs +++ b/va108xx-hal/src/i2c.rs @@ -198,13 +198,13 @@ impl TimingCfg { } pub fn reg(&self) -> u32 { - (self.tbuf as u32) << 28 - | (self.thd_sta as u32) << 24 - | (self.tsu_sta as u32) << 20 - | (self.tsu_sto as u32) << 16 - | (self.tlow as u32) << 12 - | (self.thigh as u32) << 8 - | (self.tf as u32) << 4 + ((self.tbuf as u32) << 28) + | ((self.thd_sta as u32) << 24) + | ((self.tsu_sta as u32) << 20) + | ((self.tsu_sto as u32) << 16) + | ((self.tlow as u32) << 12) + | ((self.thigh as u32) << 8) + | ((self.tf as u32) << 4) | (self.tr as u32) } } @@ -376,7 +376,7 @@ impl I2cBase { if let Some(max_words) = max_words { self.i2c .s0_maxwords() - .write(|w| unsafe { w.bits(1 << 31 | max_words as u32) }); + .write(|w| unsafe { w.bits((1 << 31) | max_words as u32) }); } let (addr, addr_mode_mask) = Self::unwrap_addr(sl_cfg.addr); // The first bit is the read/write value. Normally, both read and write are matched @@ -437,7 +437,7 @@ impl I2cBase { let clk_div = self.calc_clk_div(speed_mode)?; self.i2c .clkscale() - .write(|w| unsafe { w.bits((speed_mode as u32) << 31 | clk_div as u32) }); + .write(|w| unsafe { w.bits(((speed_mode as u32) << 31) | clk_div as u32) }); Ok(()) } diff --git a/va108xx-hal/src/uart/mod.rs b/va108xx-hal/src/uart/mod.rs index 115feb9..64414ce 100644 --- a/va108xx-hal/src/uart/mod.rs +++ b/va108xx-hal/src/uart/mod.rs @@ -64,55 +64,6 @@ pub struct NoInterruptIdWasSet; #[error("transer is pending")] pub struct TransferPendingError; -#[derive(Debug, PartialEq, Eq, thiserror::Error)] -#[cfg_attr(feature = "defmt", derive(defmt::Format))] -pub enum RxError { - #[error("overrun error")] - Overrun, - #[error("framing error")] - Framing, - #[error("parity error")] - Parity, -} -#[derive(Debug, PartialEq, Eq, thiserror::Error)] -#[cfg_attr(feature = "defmt", derive(defmt::Format))] -pub enum Error { - #[error("rx error: {0}")] - Rx(#[from] RxError), - #[error("break condition")] - BreakCondition, -} - -impl embedded_io::Error for Error { - fn kind(&self) -> embedded_io::ErrorKind { - embedded_io::ErrorKind::Other - } -} - -impl embedded_io::Error for RxError { - fn kind(&self) -> embedded_io::ErrorKind { - embedded_io::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 embedded_hal_nb::serial::Error for Error { - fn kind(&self) -> embedded_hal_nb::serial::ErrorKind { - match self { - Error::Rx(rx_error) => embedded_hal_nb::serial::Error::kind(rx_error), - Error::BreakCondition => embedded_hal_nb::serial::ErrorKind::Other, - } - } -} - #[derive(Debug, PartialEq, Eq, Copy, Clone)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] pub enum Event { @@ -363,7 +314,7 @@ impl UartErrors { impl UartErrors { #[inline(always)] pub fn error(&self) -> bool { - self.overflow || self.framing || self.parity + self.overflow || self.framing || self.parity || self.other } } @@ -585,22 +536,27 @@ impl UartBase { self.uart } + /// Poll receiver errors. + pub fn poll_rx_errors(&self) -> Option { + self.rx.poll_errors() + } + pub fn split(self) -> (Tx, Rx) { (self.tx, self.rx) } } impl embedded_io::ErrorType for UartBase { - type Error = Error; + type Error = Infallible; } impl embedded_hal_nb::serial::ErrorType for UartBase { - type Error = Error; + type Error = Infallible; } impl embedded_hal_nb::serial::Read for UartBase { fn read(&mut self) -> nb::Result { - self.rx.read().map_err(|e| e.map(Error::Rx)) + self.rx.read() } } @@ -711,6 +667,11 @@ where self } + /// Poll receiver errors. + pub fn poll_rx_errors(&self) -> Option { + self.inner.poll_rx_errors() + } + #[inline] pub fn enable_rx(&mut self) { self.inner.enable_rx(); @@ -825,6 +786,23 @@ impl Rx { &self.0 } + pub fn poll_errors(&self) -> Option { + let mut errors = UartErrors::default(); + + let uart = unsafe { &(*Uart::ptr()) }; + let status_reader = uart.rxstatus().read(); + if status_reader.rxovr().bit_is_set() { + errors.overflow = true; + } else if status_reader.rxfrm().bit_is_set() { + errors.framing = true; + } else if status_reader.rxpar().bit_is_set() { + errors.parity = true; + } else { + return None; + }; + Some(errors) + } + #[inline] pub fn clear_fifo(&self) { self.0.fifo_clr().write(|w| w.rxfifo().set_bit()); @@ -887,34 +865,15 @@ impl Rx { } impl embedded_io::ErrorType for Rx { - type Error = RxError; + type Error = Infallible; } impl embedded_hal_nb::serial::ErrorType for Rx { - type Error = RxError; + type Error = Infallible; } impl embedded_hal_nb::serial::Read for Rx { fn read(&mut self) -> nb::Result { - let uart = unsafe { &(*Uart::ptr()) }; - let status_reader = uart.rxstatus().read(); - let err = if status_reader.rxovr().bit_is_set() { - Some(RxError::Overrun) - } else if status_reader.rxfrm().bit_is_set() { - Some(RxError::Framing) - } else if status_reader.rxpar().bit_is_set() { - Some(RxError::Parity) - } else { - None - }; - if let Some(err) = err { - // The status code is always related to the next bit for the framing - // 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 - self.read_fifo_unchecked(); - return Err(err.into()); - } self.read_fifo().map(|val| (val & 0xff) as u8).map_err(|e| { if let nb::Error::Other(_) = e { unreachable!() @@ -926,16 +885,18 @@ impl embedded_hal_nb::serial::Read for Rx { impl embedded_io::Read for Rx { fn read(&mut self, buf: &mut [u8]) -> Result { - if buf.is_empty() { - return Ok(0); - } - + let mut read = 0; for byte in buf.iter_mut() { - let w = nb::block!(>::read(self))?; - *byte = w; + match >::read(self) { + Ok(w) => { + *byte = w; + read += 1; + } + Err(nb::Error::WouldBlock) => break, + } } - Ok(buf.len()) + Ok(read) } } @@ -1090,14 +1051,12 @@ impl embedded_hal_nb::serial::Write for Tx { impl embedded_io::Write for Tx { fn write(&mut self, buf: &[u8]) -> Result { - if buf.is_empty() { - return Ok(0); - } - + let mut written = 0; for byte in buf.iter() { - nb::block!(>::write( - self, *byte - ))?; + match >::write(self, *byte) { + Ok(_) => written += 1, + Err(nb::Error::WouldBlock) => return Ok(written), + } } Ok(buf.len()) @@ -1218,15 +1177,10 @@ impl RxWithInterrupt { // Timeout, empty the FIFO completely. if irq_end.irq_rx_to().bit_is_set() { - loop { - // While there is data in the FIFO, write it into the reception buffer - let read_result = self.0.read(); - if let Some(byte) = self.read_handler(&mut result.errors, &read_result) { - buf[result.bytes_read] = byte; - result.bytes_read += 1; - } else { - break; - } + // While there is data in the FIFO, write it into the reception buffer + while let Ok(byte) = self.0.read_fifo() { + buf[result.bytes_read] = byte as u8; + result.bytes_read += 1; } } @@ -1303,12 +1257,13 @@ impl RxWithInterrupt { if context.rx_idx == context.max_len { break; } - let read_result = self.0.read(); - if let Some(byte) = self.read_handler(&mut result.errors, &read_result) { - buf[context.rx_idx] = byte; - context.rx_idx += 1; - } else { - break; + // While there is data in the FIFO, write it into the reception buffer + match self.0.read() { + Ok(byte) => { + buf[result.bytes_read] = byte; + result.bytes_read += 1; + } + Err(_) => break, } } self.irq_completion_handler_max_size_timeout(&mut result, context); @@ -1327,29 +1282,6 @@ impl RxWithInterrupt { Ok(result) } - fn read_handler( - &self, - errors: &mut Option, - read_res: &nb::Result, - ) -> Option { - match read_res { - Ok(byte) => Some(*byte), - Err(nb::Error::WouldBlock) => None, - Err(nb::Error::Other(e)) => { - // Ensure `errors` is Some(IrqUartError), initializing if it's None - let err = errors.get_or_insert(UartErrors::default()); - - // Now we can safely modify fields inside `err` - match e { - RxError::Overrun => err.overflow = true, - RxError::Framing => err.framing = true, - RxError::Parity => err.parity = true, - } - None - } - } - } - fn check_for_errors(&self, errors: &mut Option) { let rx_status = self.uart().rxstatus().read(); diff --git a/va108xx-hal/src/uart/rx_asynch.rs b/va108xx-hal/src/uart/rx_asynch.rs index 95e4b65..215bfae 100644 --- a/va108xx-hal/src/uart/rx_asynch.rs +++ b/va108xx-hal/src/uart/rx_asynch.rs @@ -26,7 +26,7 @@ use embedded_io::ErrorType; use portable_atomic::AtomicBool; use va108xx::uarta as uart_base; -use super::{Bank, Instance, Rx, RxError, UartErrors}; +use super::{Bank, Instance, Rx, UartErrors}; static UART_RX_WAKERS: [AtomicWaker; 2] = [const { AtomicWaker::new() }; 2]; static RX_READ_ACTIVE: [AtomicBool; 2] = [const { AtomicBool::new(false) }; 2]; @@ -46,7 +46,7 @@ impl RxFuture { } impl Future for RxFuture { - type Output = Result<(), RxError>; + type Output = Result<(), Infallible>; fn poll( self: core::pin::Pin<&mut Self>,