From f9b6b7ed71f0b210c17bf2f6d64afb82bf9f49ef Mon Sep 17 00:00:00 2001 From: Rafael Bachmann Date: Sun, 1 Jan 2023 11:18:31 +0100 Subject: [PATCH] Make peripheral constructors fallible (fixes #276) --- Cargo.toml | 2 +- examples/i2c_scanner.rs | 3 +- examples/serial_dma.rs | 2 +- examples/serial_echo_rtic.rs | 2 +- src/i2c.rs | 24 ++++++++++------ src/serial.rs | 12 +++++--- src/serial/config.rs | 4 +++ testsuite/tests/uart.rs | 56 ++++++++++++++++++++++-------------- 8 files changed, 67 insertions(+), 38 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 05a50cc69..7307177f3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -35,7 +35,7 @@ rustc-args = ["--cfg", "docsrs"] [dependencies] cfg-if = "1.0.0" -cortex-m = "0.7.4" +cortex-m = { version = "0.7.6", features = ["critical-section-single-core"] } cortex-m-rt = "0.7" defmt = { version = ">=0.2.3, <0.4.0", optional = true } embedded-dma = "0.2.0" diff --git a/examples/i2c_scanner.rs b/examples/i2c_scanner.rs index c371095e6..9c20a7f55 100644 --- a/examples/i2c_scanner.rs +++ b/examples/i2c_scanner.rs @@ -44,7 +44,8 @@ fn main() -> ! { 100.kHz().try_into().unwrap(), clocks, &mut rcc.apb1, - ); + ) + .unwrap(); hprintln!("Start i2c scanning...").expect("Error using hprintln."); hprintln!().unwrap(); diff --git a/examples/serial_dma.rs b/examples/serial_dma.rs index bf580dbce..5a044500d 100644 --- a/examples/serial_dma.rs +++ b/examples/serial_dma.rs @@ -40,7 +40,7 @@ fn main() -> ! { .pa10 .into_af_push_pull(&mut gpioa.moder, &mut gpioa.otyper, &mut gpioa.afrh), ); - let serial = Serial::new(dp.USART1, pins, 9600.Bd(), clocks, &mut rcc.apb2); + let serial = Serial::new(dp.USART1, pins, 9600.Bd(), clocks, &mut rcc.apb2).unwrap(); let (tx, rx) = serial.split(); let dma1 = dp.DMA1.split(&mut rcc.ahb); diff --git a/examples/serial_echo_rtic.rs b/examples/serial_echo_rtic.rs index 0e7aac75a..236bf23b1 100644 --- a/examples/serial_echo_rtic.rs +++ b/examples/serial_echo_rtic.rs @@ -80,7 +80,7 @@ mod app { ); pins.1.internal_pull_up(&mut gpioa.pupdr, true); let mut serial: SerialType = - Serial::new(cx.device.USART1, pins, 19200.Bd(), clocks, &mut rcc.apb2); + Serial::new(cx.device.USART1, pins, 19200.Bd(), clocks, &mut rcc.apb2).unwrap(); serial.configure_interrupt(Event::ReceiveDataRegisterNotEmpty, Toggle::On); rprintln!("post init"); diff --git a/src/i2c.rs b/src/i2c.rs index 62c978109..1503d6ecb 100644 --- a/src/i2c.rs +++ b/src/i2c.rs @@ -31,6 +31,8 @@ use cfg_if::cfg_if; #[derive(Debug)] #[non_exhaustive] pub enum Error { + /// Configuration + Config, /// Arbitration loss Arbitration, /// Bus error @@ -117,13 +119,15 @@ impl I2c { freq: Hertz, clocks: Clocks, bus: &mut ::Bus, - ) -> Self + ) -> Result where I2C: Instance, SCL: SclPin, SDA: SdaPin, { - crate::assert!(freq.integer() <= 1_000_000); + if freq.integer() > 1_000_000 { + return Err(Error::Config); + } I2C::enable(bus); I2C::reset(bus); @@ -175,11 +179,15 @@ impl I2c { (presc, scll, sclh, sdadel, scldel) }; - crate::assert!(presc < 16); - crate::assert!(scldel < 16); - crate::assert!(sdadel < 16); - let sclh = crate::unwrap!(u8::try_from(sclh).ok()); - let scll = crate::unwrap!(u8::try_from(scll).ok()); + if presc >= 16 || scldel >= 16 || sdadel >= 16 { + return Err(Error::Config); + } + let Ok(sclh) = u8::try_from(sclh) else { + return Err(Error::Config); + }; + let Ok(scll) = u8::try_from(scll) else { + return Err(Error::Config); + }; // Configure for "fast mode" (400 KHz) // NOTE(write): writes all non-reserved bits. @@ -199,7 +207,7 @@ impl I2c { // Enable the peripheral i2c.cr1.modify(|_, w| w.pe().set_bit()); - Self { i2c, pins } + Ok(Self { i2c, pins }) } /// Get access to the underlying register block. diff --git a/src/serial.rs b/src/serial.rs index d41b2f1ee..9324987cb 100644 --- a/src/serial.rs +++ b/src/serial.rs @@ -453,6 +453,8 @@ mod split { pub use split::{Rx, Tx}; +use self::config::InvalidBaudrate; + impl Serial where Usart: Instance, @@ -464,7 +466,7 @@ where config: Config, clocks: Clocks, apb: &mut ::Bus, - ) -> Self + ) -> Result where Usart: Instance, Tx: TxPin, @@ -483,7 +485,9 @@ where usart.cr1.modify(|_, w| w.ue().disabled()); let brr = Usart::clock(&clocks).integer() / config.baudrate.integer(); - crate::assert!(brr >= 16, "impossible baud rate"); + if brr < 16 { + return Err(InvalidBaudrate); + } usart.brr.write(|w| w.brr().bits(brr as u16)); // We currently support only eight data bits as supporting a full-blown @@ -510,7 +514,7 @@ where // Finally enable the configured UART. usart.cr1.modify(|_, w| w.ue().enabled()); - Self { usart, pins } + Ok(Self { usart, pins }) } /// Get access to the underlying register block. @@ -544,7 +548,7 @@ where /// ``` /// let dp = pac::Peripherals::take().unwrap(); /// - /// (tx, rx) = Serial::new(dp.USART1, ...).split(); + /// (tx, rx) = Serial::new(dp.USART1, ...).unwrap().split(); /// /// // Do something with tx and rx /// diff --git a/src/serial/config.rs b/src/serial/config.rs index 89b9af3c4..b633d3c3c 100644 --- a/src/serial/config.rs +++ b/src/serial/config.rs @@ -105,6 +105,10 @@ impl Config { } } +/// Invalid baud rate. +#[derive(Debug)] +pub struct InvalidBaudrate; + impl Default for Config { /// Creates a new configuration with typically used parameters: 115,200 /// Baud 8N1. diff --git a/testsuite/tests/uart.rs b/testsuite/tests/uart.rs index a48946b35..6d58d7b21 100644 --- a/testsuite/tests/uart.rs +++ b/testsuite/tests/uart.rs @@ -52,7 +52,7 @@ const TEST_MSG: [u8; 8] = [0xa, 0xb, 0xc, 0xd, 0xe, 0xf, 0xaa, 0xbb]; fn test_test_msg_loopback(state: &mut State, config: impl Into) { let (usart, pins) = unwrap!(state.serial1.take()).free(); - let mut serial = Serial::new(usart, pins, config, state.clocks, &mut state.apb2); + let mut serial = Serial::new(usart, pins, config, state.clocks, &mut state.apb2).unwrap(); for i in &TEST_MSG { unwrap!(nb::block!(serial.write(*i))); @@ -75,7 +75,7 @@ fn trigger_event( let mut events = EnumSet::new(); events.insert(event); // Clear events, so that previously triggered events do not fire - // the now configured interupt imediatly. + // the now configured interrupt immediately. serial.clear_events(); serial.configure_interrupts(events); // Check that the interrupt has not been run, since the configuration @@ -153,26 +153,33 @@ mod tests { 9600.Bd(), clocks, &mut rcc.apb2, - ); + ) + .unwrap(); unsafe { cortex_m::peripheral::NVIC::unmask(serial1.interrupt()) } super::State { serial1: Some(serial1), - serial_slow: Some(Serial::new( - dp.USART2, - (cs_pair_1.0, cs_pair_2.1), - BAUD_SLOW, - clocks, - &mut rcc.apb1, - )), - serial_fast: Some(Serial::new( - dp.USART3, - (cs_pair_2.0, cs_pair_1.1), - BAUD_FAST, - clocks, - &mut rcc.apb1, - )), + serial_slow: Some( + Serial::new( + dp.USART2, + (cs_pair_1.0, cs_pair_2.1), + BAUD_SLOW, + clocks, + &mut rcc.apb1, + ) + .unwrap(), + ), + serial_fast: Some( + Serial::new( + dp.USART3, + (cs_pair_2.0, cs_pair_1.1), + BAUD_FAST, + clocks, + &mut rcc.apb1, + ) + .unwrap(), + ), clocks, apb1: rcc.apb1, apb2: rcc.apb2, @@ -195,7 +202,8 @@ mod tests { #[test] fn test_overrun(state: &mut super::State) { let (usart, pins) = unwrap!(state.serial1.take()).free(); - let mut serial = Serial::new(usart, pins, 115200.Bd(), state.clocks, &mut state.apb2); + let mut serial = + Serial::new(usart, pins, 115200.Bd(), state.clocks, &mut state.apb2).unwrap(); // Provoke an overrun unwrap!(serial.bwrite_all(&TEST_MSG)); @@ -296,7 +304,8 @@ mod tests { config_even, state.clocks, &mut state.apb1, - ); + ) + .unwrap(); let (usart_odd, pins_odd) = unwrap!(state.serial_fast.take()).free(); let config_odd = Config::default().parity(Parity::Odd); let mut serial_odd = Serial::new( @@ -305,7 +314,8 @@ mod tests { config_odd, state.clocks, &mut state.apb1, - ); + ) + .unwrap(); // Transmit data with wrong parity in both directions. unwrap!(nb::block!(serial_even.write(b'x'))); @@ -326,7 +336,8 @@ mod tests { BAUD_SLOW, state.clocks, &mut state.apb1, - ); + ) + .unwrap(); let (usart_fast, pins_fast) = serial_odd.free(); let serial_fast = Serial::new( usart_fast, @@ -334,7 +345,8 @@ mod tests { BAUD_FAST, state.clocks, &mut state.apb1, - ); + ) + .unwrap(); state.serial_slow = Some(serial_slow); state.serial_fast = Some(serial_fast); }