From 67ddba9c42c3e421439fc95a460e0091aee2eb0e Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 10 Feb 2025 16:57:40 +0100 Subject: [PATCH] update error handling --- va108xx-hal/CHANGELOG.md | 14 +- va108xx-hal/Cargo.toml | 1 + va108xx-hal/src/gpio/dynpin.rs | 29 ++-- va108xx-hal/src/gpio/mod.rs | 3 +- va108xx-hal/src/i2c.rs | 54 ++++--- va108xx-hal/src/spi.rs | 5 +- va108xx-hal/src/uart.rs | 260 ++------------------------------- 7 files changed, 77 insertions(+), 289 deletions(-) diff --git a/va108xx-hal/CHANGELOG.md b/va108xx-hal/CHANGELOG.md index 92038de..c3bf0ac 100644 --- a/va108xx-hal/CHANGELOG.md +++ b/va108xx-hal/CHANGELOG.md @@ -10,13 +10,25 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## [v0.9.0] +## Removed + - Deleted some HAL re-exports in the PWM module + +## Changed + - GPIO API: Interrupt, pulse and filter and `set_datamask` and `clear_datamask` APIs are now methods which mutable modify the pin instead of consuming and returning it. +- Simplified PWM module implementation. +- All error types now implement `core::error::Error` by using the `thiserror::Error` derive. +- `InvalidPinTypeError` now wraps the pin mode. +- I2C `TimingCfg` constructor now returns explicit error instead of generic Error. + Removed the timing configuration error type from the generic I2C error enumeration. + +## Added + - Add `downgrade` method for `Pin` and `upgrade` method for `DynPin` as explicit conversion methods. - Add new `get_tim_raw` unsafe method to retrieve TIM peripheral blocks. -- Simplified PWM module implementation. ## [v0.8.0] 2024-09-30 diff --git a/va108xx-hal/Cargo.toml b/va108xx-hal/Cargo.toml index 3ad52c9..5b591c8 100644 --- a/va108xx-hal/Cargo.toml +++ b/va108xx-hal/Cargo.toml @@ -22,6 +22,7 @@ fugit = "0.3" typenum = "1" critical-section = "1" delegate = ">=0.12, <=0.13" +thiserror = { version = "2", default-features = false } void = { version = "1", default-features = false } once_cell = {version = "1", default-features = false } va108xx = { version = "0.3", default-features = false, features = ["critical-section"]} diff --git a/va108xx-hal/src/gpio/dynpin.rs b/va108xx-hal/src/gpio/dynpin.rs index a76a3f6..c3ec315 100644 --- a/va108xx-hal/src/gpio/dynpin.rs +++ b/va108xx-hal/src/gpio/dynpin.rs @@ -75,7 +75,7 @@ pub enum DynDisabled { } /// Value-level `enum` for input configurations -#[derive(PartialEq, Eq, Clone, Copy)] +#[derive(Debug, PartialEq, Eq, Clone, Copy)] pub enum DynInput { Floating, PullDown, @@ -83,7 +83,7 @@ pub enum DynInput { } /// Value-level `enum` for output configurations -#[derive(PartialEq, Eq, Clone, Copy)] +#[derive(Debug, PartialEq, Eq, Clone, Copy)] pub enum DynOutput { PushPull, OpenDrain, @@ -101,9 +101,10 @@ pub type DynAlternate = FunSel; /// /// [`DynPin`]s are not tracked and verified at compile-time, so run-time /// operations are fallible. This `enum` represents the corresponding errors. -#[derive(Debug, PartialEq, Eq)] +#[derive(Debug, PartialEq, Eq, thiserror::Error)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] -pub struct InvalidPinTypeError; +#[error("Invalid pin type for operation: {0:?}")] +pub struct InvalidPinTypeError(DynPinMode); impl embedded_hal::digital::Error for InvalidPinTypeError { fn kind(&self) -> embedded_hal::digital::ErrorKind { @@ -116,7 +117,7 @@ impl embedded_hal::digital::Error for InvalidPinTypeError { //================================================================================================== /// Value-level `enum` representing pin modes -#[derive(PartialEq, Eq, Clone, Copy)] +#[derive(Debug, PartialEq, Eq, Clone, Copy)] pub enum DynPinMode { Input(DynInput), Output(DynOutput), @@ -382,7 +383,7 @@ impl DynPin { self.regs.delay(delay_1, delay_2); Ok(self) } - _ => Err(InvalidPinTypeError), + _ => Err(InvalidPinTypeError(self.mode)), } } @@ -400,7 +401,7 @@ impl DynPin { self.regs.pulse_mode(enable, default_state); Ok(()) } - _ => Err(InvalidPinTypeError), + _ => Err(InvalidPinTypeError(self.mode)), } } @@ -416,7 +417,7 @@ impl DynPin { self.regs.filter_type(filter, clksel); Ok(()) } - _ => Err(InvalidPinTypeError), + _ => Err(InvalidPinTypeError(self.mode)), } } @@ -434,7 +435,7 @@ impl DynPin { self.irq_enb(irq_cfg, syscfg, irqsel); Ok(()) } - _ => Err(InvalidPinTypeError), + _ => Err(InvalidPinTypeError(self.mode)), } } @@ -452,7 +453,7 @@ impl DynPin { self.irq_enb(irq_cfg, syscfg, irqsel); Ok(()) } - _ => Err(InvalidPinTypeError), + _ => Err(InvalidPinTypeError(self.mode)), } } @@ -463,7 +464,7 @@ impl DynPin { self.regs.toggle(); Ok(()) } - _ => Err(InvalidPinTypeError), + _ => Err(InvalidPinTypeError(self.mode)), } } @@ -473,7 +474,7 @@ impl DynPin { DynPinMode::Input(_) | DYN_RD_OPEN_DRAIN_OUTPUT | DYN_RD_PUSH_PULL_OUTPUT => { Ok(self.regs.read_pin()) } - _ => Err(InvalidPinTypeError), + _ => Err(InvalidPinTypeError(self.mode)), } } #[inline] @@ -483,7 +484,7 @@ impl DynPin { self.regs.write_pin(bit); Ok(()) } - _ => Err(InvalidPinTypeError), + _ => Err(InvalidPinTypeError(self.mode)), } } @@ -516,7 +517,7 @@ impl DynPin { // corresponding `Pin` return Ok(unsafe { Pin::new() }); } - Err(InvalidPinTypeError) + Err(InvalidPinTypeError(self.mode)) } } diff --git a/va108xx-hal/src/gpio/mod.rs b/va108xx-hal/src/gpio/mod.rs index 4bdbe41..736e20b 100644 --- a/va108xx-hal/src/gpio/mod.rs +++ b/va108xx-hal/src/gpio/mod.rs @@ -22,8 +22,9 @@ //! //! - [Blinky example](https://egit.irs.uni-stuttgart.de/rust/va108xx-rs/src/branch/main/examples/simple/examples/blinky.rs) -#[derive(Debug, PartialEq, Eq)] +#[derive(Debug, PartialEq, Eq, thiserror::Error)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] +#[error("The pin is masked")] pub struct IsMaskedError; pub mod dynpin; diff --git a/va108xx-hal/src/i2c.rs b/va108xx-hal/src/i2c.rs index 5aa07d8..3a39bc2 100644 --- a/va108xx-hal/src/i2c.rs +++ b/va108xx-hal/src/i2c.rs @@ -23,37 +23,44 @@ pub enum FifoEmptyMode { EndTransaction = 1, } -#[derive(Debug, PartialEq, Eq)] +#[derive(Debug, PartialEq, Eq, thiserror::Error)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] -pub struct ClockTooSlowForFastI2c; +#[error("clock too slow for fast I2C mode")] +pub struct ClockTooSlowForFastI2cError; -#[derive(Debug, PartialEq, Eq)] +#[derive(Debug, PartialEq, Eq, thiserror::Error)] +#[error("invalid timing parameters")] +pub struct InvalidTimingParamsError; + +#[derive(Debug, PartialEq, Eq, thiserror::Error)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] pub enum Error { - InvalidTimingParams, + //#[error("Invalid timing parameters")] + //InvalidTimingParams, + #[error("arbitration lost")] ArbitrationLost, + #[error("nack address")] NackAddr, /// Data not acknowledged in write operation + #[error("data not acknowledged in write operation")] NackData, /// Not enough data received in read operation + #[error("insufficient data received")] InsufficientDataReceived, /// Number of bytes in transfer too large (larger than 0x7fe) + #[error("data too large (larger than 0x7fe)")] DataTooLarge, } -#[derive(Debug, PartialEq, Eq)] +#[derive(Debug, PartialEq, Eq, thiserror::Error)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] pub enum InitError { /// Wrong address used in constructor + #[error("wrong address mode")] WrongAddrMode, /// APB1 clock is too slow for fast I2C mode. - ClkTooSlow(ClockTooSlowForFastI2c), -} - -impl From for InitError { - fn from(value: ClockTooSlowForFastI2c) -> Self { - Self::ClkTooSlow(value) - } + #[error("clock too slow for fast I2C mode: {0}")] + ClkTooSlow(#[from] ClockTooSlowForFastI2cError), } impl embedded_hal::i2c::Error for Error { @@ -66,7 +73,7 @@ impl embedded_hal::i2c::Error for Error { Error::NackData => { embedded_hal::i2c::ErrorKind::NoAcknowledge(i2c::NoAcknowledgeSource::Data) } - Error::DataTooLarge | Error::InsufficientDataReceived | Error::InvalidTimingParams => { + Error::DataTooLarge | Error::InsufficientDataReceived => { embedded_hal::i2c::ErrorKind::Other } } @@ -160,7 +167,7 @@ impl TimingCfg { pub fn new( first_16_bits: TrTfThighTlow, second_16_bits: TsuStoTsuStaThdStaTBuf, - ) -> Result { + ) -> Result { if first_16_bits.0 > 0xf || first_16_bits.1 > 0xf || first_16_bits.2 > 0xf @@ -170,7 +177,7 @@ impl TimingCfg { || second_16_bits.2 > 0xf || second_16_bits.3 > 0xf { - return Err(Error::InvalidTimingParams); + return Err(InvalidTimingParamsError); } Ok(TimingCfg { tr: first_16_bits.0, @@ -299,7 +306,7 @@ impl I2cBase { speed_mode: I2cSpeed, ms_cfg: Option<&MasterConfig>, sl_cfg: Option<&SlaveConfig>, - ) -> Result { + ) -> Result { enable_peripheral_clock(syscfg, I2c::PERIPH_SEL); let mut i2c_base = I2cBase { @@ -402,19 +409,22 @@ impl I2cBase { }); } - fn calc_clk_div(&self, speed_mode: I2cSpeed) -> Result { + fn calc_clk_div(&self, speed_mode: I2cSpeed) -> Result { if speed_mode == I2cSpeed::Regular100khz { Ok(((self.sys_clk.raw() / CLK_100K.raw() / 20) - 1) as u8) } else { if self.sys_clk.raw() < MIN_CLK_400K.raw() { - return Err(ClockTooSlowForFastI2c); + return Err(ClockTooSlowForFastI2cError); } Ok(((self.sys_clk.raw() / CLK_400K.raw() / 25) - 1) as u8) } } /// Configures the clock scale for a given speed mode setting - pub fn cfg_clk_scale(&mut self, speed_mode: I2cSpeed) -> Result<(), ClockTooSlowForFastI2c> { + pub fn cfg_clk_scale( + &mut self, + speed_mode: I2cSpeed, + ) -> Result<(), ClockTooSlowForFastI2cError> { let clk_div = self.calc_clk_div(speed_mode)?; self.i2c .clkscale() @@ -460,7 +470,7 @@ impl I2cMaster { i2c: I2c, cfg: MasterConfig, speed_mode: I2cSpeed, - ) -> Result { + ) -> Result { Ok(I2cMaster { i2c_base: I2cBase::new(syscfg, sysclk, i2c, speed_mode, Some(&cfg), None)?, addr: PhantomData, @@ -990,7 +1000,7 @@ impl I2cSlave { i2c: I2c, cfg: SlaveConfig, speed_mode: I2cSpeed, - ) -> Result { + ) -> Result { Ok(I2cSlave { i2c_base: I2cBase::new(sys_cfg, sys_clk, i2c, speed_mode, None, Some(&cfg))?, addr: PhantomData, @@ -1152,7 +1162,7 @@ impl I2cSlave { i2c: I2c, cfg: SlaveConfig, speed_mode: I2cSpeed, - ) -> Result { + ) -> Result { Self::new_generic(sys_cfg, sys_clk, i2c, cfg, speed_mode) } } diff --git a/va108xx-hal/src/spi.rs b/va108xx-hal/src/spi.rs index c9e41fd..9d76fd5 100644 --- a/va108xx-hal/src/spi.rs +++ b/va108xx-hal/src/spi.rs @@ -571,10 +571,13 @@ impl SpiClkConfig { } } -#[derive(Debug)] +#[derive(Debug, thiserror::Error)] pub enum SpiClkConfigError { + #[error("division by zero")] DivIsZero, + #[error("divide value is not even")] DivideValueNotEven, + #[error("scrdv value is too large")] ScrdvValueTooLarge, } diff --git a/va108xx-hal/src/uart.rs b/va108xx-hal/src/uart.rs index d960c7a..fa6daa5 100644 --- a/va108xx-hal/src/uart.rs +++ b/va108xx-hal/src/uart.rs @@ -48,30 +48,30 @@ impl Pins for (Pin, Pin) {} // Regular Definitions //================================================================================================== -#[derive(Debug, PartialEq, Eq)] +#[derive(Debug, PartialEq, Eq, thiserror::Error)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] +#[error("transer is pending")] pub struct TransferPendingError; -#[derive(Debug, PartialEq, Eq)] +#[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)] +#[derive(Debug, PartialEq, Eq, thiserror::Error)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] pub enum Error { - Rx(RxError), + #[error("rx error: {0}")] + Rx(#[from] RxError), + #[error("break condition")] BreakCondition, } -impl From for Error { - fn from(value: RxError) -> Self { - Self::Rx(value) - } -} - impl embedded_io::Error for Error { fn kind(&self) -> embedded_io::ErrorKind { embedded_io::ErrorKind::Other @@ -1213,243 +1213,3 @@ impl RxWithIrq { self.rx.release() } } - -/* - -impl UartWithIrq { - /// See [`UartWithIrqBase::read_fixed_len_using_irq`] doc - pub fn read_fixed_len_using_irq( - &mut self, - max_len: usize, - enb_timeout_irq: bool, - ) -> Result<(), Error> { - self.irq_base - .read_fixed_len_using_irq(max_len, enb_timeout_irq) - } - - pub fn cancel_transfer(&mut self) { - self.irq_base.cancel_transfer() - } - - /// See [`UartWithIrqBase::irq_handler`] doc - pub fn irq_handler(&mut self, res: &mut IrqResult, buf: &mut [u8]) -> Result<(), Error> { - self.irq_base.irq_handler(res, buf) - } - - pub fn release(self) -> (UART, PINS) { - (self.irq_base.release(), self.pins) - } - - pub fn downgrade(self) -> (UartWithIrqBase, PINS) { - (self.irq_base, self.pins) - } -} - -impl UartWithIrqBase { - fn init(self, sys_cfg: Option<&mut pac::Sysconfig>, irq_sel: Option<&mut pac::Irqsel>) -> Self { - if let Some(sys_cfg) = sys_cfg { - enable_peripheral_clock(sys_cfg, PeripheralClocks::Irqsel) - } - if let Some(irq_sel) = irq_sel { - if self.irq_info.irq_cfg.route { - irq_sel - .uart0(Uart::IDX as usize) - .write(|w| unsafe { w.bits(self.irq_info.irq_cfg.irq as u32) }); - } - } - self - } - - /// This initializes a non-blocking read transfer using the IRQ capabilities of the UART - /// peripheral. - /// - /// The only required information is the maximum length for variable sized reception - /// or the expected length for fixed length reception. If variable sized packets are expected, - /// the timeout functionality of the IRQ should be enabled as well. After calling this function, - /// the [`irq_handler`](Self::irq_handler) function should be called in the user interrupt - /// handler to read the received packets and reinitiate another transfer if desired. - pub fn read_fixed_len_using_irq( - &mut self, - max_len: usize, - enb_timeout_irq: bool, - ) -> Result<(), Error> { - if self.irq_info.mode != IrqReceptionMode::Idle { - return Err(Error::TransferPending); - } - self.irq_info.mode = IrqReceptionMode::Pending; - self.irq_info.rx_idx = 0; - self.irq_info.rx_len = max_len; - self.uart.enable_rx(); - self.uart.enable_tx(); - self.enable_rx_irq_sources(enb_timeout_irq); - if self.irq_info.irq_cfg.enable { - unsafe { - enable_interrupt(self.irq_info.irq_cfg.irq); - } - } - Ok(()) - } - - #[inline] - fn enable_rx_irq_sources(&mut self, timeout: bool) { - self.uart.uart.irq_enb().modify(|_, w| { - if timeout { - w.irq_rx_to().set_bit(); - } - w.irq_rx_status().set_bit(); - w.irq_rx().set_bit() - }); - } - - #[inline] - fn disable_rx_irq_sources(&mut self) { - self.uart.uart.irq_enb().modify(|_, w| { - w.irq_rx_to().clear_bit(); - w.irq_rx_status().clear_bit(); - w.irq_rx().clear_bit() - }); - } - - #[inline] - pub fn enable_tx(&mut self) { - self.uart.enable_tx() - } - - #[inline] - pub fn disable_tx(&mut self) { - self.uart.disable_tx() - } - - pub fn cancel_transfer(&mut self) { - // Disable IRQ - cortex_m::peripheral::NVIC::mask(self.irq_info.irq_cfg.irq); - self.disable_rx_irq_sources(); - self.uart.clear_tx_fifo(); - self.irq_info.rx_idx = 0; - self.irq_info.rx_len = 0; - } - - /// Default IRQ handler which can be used to read the packets arriving on the UART peripheral. - /// - /// If passed buffer is equal to or larger than the specified maximum length, an - /// [`Error::BufferTooShort`] will be returned - pub fn irq_handler(&mut self, res: &mut IrqResult, buf: &mut [u8]) -> Result<(), Error> { - if buf.len() < self.irq_info.rx_len { - return Err(Error::BufferTooShort); - } - - let irq_end = self.uart.uart.irq_end().read(); - let enb_status = self.uart.uart.enable().read(); - let rx_enabled = enb_status.rxenable().bit_is_set(); - let _tx_enabled = enb_status.txenable().bit_is_set(); - let read_handler = - |res: &mut IrqResult, read_res: nb::Result| -> Result, Error> { - match read_res { - Ok(byte) => Ok(Some(byte)), - Err(nb::Error::WouldBlock) => Ok(None), - Err(nb::Error::Other(e)) => match e { - Error::Overrun => { - res.set_result(IrqResultMask::Overflow); - Err(Error::IrqError) - } - Error::FramingError => { - res.set_result(IrqResultMask::FramingError); - Err(Error::IrqError) - } - Error::ParityError => { - res.set_result(IrqResultMask::ParityError); - Err(Error::IrqError) - } - _ => { - res.set_result(IrqResultMask::Unknown); - Err(Error::IrqError) - } - }, - } - }; - if irq_end.irq_rx().bit_is_set() { - // If this interrupt bit is set, the trigger level is available at the very least. - // Read everything as fast as possible - for _ in 0..core::cmp::min( - self.uart.uart.rxfifoirqtrg().read().bits() as usize, - self.irq_info.rx_len, - ) { - buf[self.irq_info.rx_idx] = (self.uart.uart.data().read().bits() & 0xff) as u8; - self.irq_info.rx_idx += 1; - } - - // While there is data in the FIFO, write it into the reception buffer - loop { - if self.irq_info.rx_idx == self.irq_info.rx_len { - self.irq_completion_handler(res); - return Ok(()); - } - if let Some(byte) = read_handler(res, self.uart.read())? { - buf[self.irq_info.rx_idx] = byte; - self.irq_info.rx_idx += 1; - } else { - break; - } - } - } - - // RX transfer not complete, check for RX errors - if (self.irq_info.rx_idx < self.irq_info.rx_len) && rx_enabled { - // Read status register again, might have changed since reading received data - let rx_status = self.uart.uart.rxstatus().read(); - res.clear_result(); - if rx_status.rxovr().bit_is_set() { - res.set_result(IrqResultMask::Overflow); - } - if rx_status.rxfrm().bit_is_set() { - res.set_result(IrqResultMask::FramingError); - } - if rx_status.rxpar().bit_is_set() { - res.set_result(IrqResultMask::ParityError); - } - if rx_status.rxbrk().bit_is_set() { - res.set_result(IrqResultMask::Break); - } - if rx_status.rxto().bit_is_set() { - // A timeout has occured but there might be some leftover data in the FIFO, - // so read that data as well - while let Some(byte) = read_handler(res, self.uart.read())? { - buf[self.irq_info.rx_idx] = byte; - self.irq_info.rx_idx += 1; - } - self.irq_completion_handler(res); - res.set_result(IrqResultMask::Timeout); - return Ok(()); - } - - // If it is not a timeout, it's an error - if res.raw_res != 0 { - self.disable_rx_irq_sources(); - return Err(Error::IrqError); - } - } - - // Clear the interrupt status bits - self.uart - .uart - .irq_clr() - .write(|w| unsafe { w.bits(irq_end.bits()) }); - Ok(()) - } - - fn irq_completion_handler(&mut self, res: &mut IrqResult) { - self.disable_rx_irq_sources(); - self.uart.disable_rx(); - res.bytes_read = self.irq_info.rx_idx; - res.clear_result(); - res.set_result(IrqResultMask::Complete); - self.irq_info.mode = IrqReceptionMode::Idle; - self.irq_info.rx_idx = 0; - self.irq_info.rx_len = 0; - } - - pub fn release(self) -> Uart { - self.uart.release() - } -} -*/