From 8a7f8361a6bb262fce75ae79963ff38b4003deaa Mon Sep 17 00:00:00 2001 From: Juraj Sadel Date: Wed, 8 Jan 2025 17:35:22 +0100 Subject: [PATCH] UART: Move RX/TX pin assignment from constructor to builder functions (#2904) * Add with_rx() and with_tx() methods to UART drivers * changelog * migration guide * forgotten tests * fmt * fix H2 example * Use the same calling order of calling TX and RX as before --- esp-hal/CHANGELOG.md | 1 + esp-hal/MIGRATING-0.22.md | 17 ++++ esp-hal/src/uart.rs | 135 +++++++++++++++---------- examples/src/bin/embassy_serial.rs | 4 +- examples/src/bin/ieee802154_sniffer.rs | 15 ++- hil-test/tests/uart.rs | 5 +- hil-test/tests/uart_async.rs | 4 +- hil-test/tests/uart_regression.rs | 8 +- hil-test/tests/uart_tx_rx.rs | 8 +- hil-test/tests/uart_tx_rx_async.rs | 6 +- 10 files changed, 129 insertions(+), 74 deletions(-) diff --git a/esp-hal/CHANGELOG.md b/esp-hal/CHANGELOG.md index a4d278490..89f7d23ec 100644 --- a/esp-hal/CHANGELOG.md +++ b/esp-hal/CHANGELOG.md @@ -55,6 +55,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `adc::{AdcCalSource, Attenuation, Resolution}` now implement `Hash` and `defmt::Format` (#2840) - `rtc_cntl::{RtcFastClock, RtcSlowClock, RtcCalSel}` now implement `PartialEq`, `Eq`, `Hash` and `defmt::Format` (#2840) - Added `tsens::TemperatureSensor` peripheral for ESP32C6 and ESP32C3 (#2875) +- Added `with_rx()` and `with_tx()` methods to Uart, UartRx, and UartTx () ### Changed diff --git a/esp-hal/MIGRATING-0.22.md b/esp-hal/MIGRATING-0.22.md index 464a56661..d32f08b14 100644 --- a/esp-hal/MIGRATING-0.22.md +++ b/esp-hal/MIGRATING-0.22.md @@ -396,6 +396,23 @@ e.g. + GlitchOccurred ``` +RX/TX pin assignment moved from constructors to builder functions. + +e.g. + +```diff + let mut uart1 = Uart::new( + peripherals.UART1, +- Config::default(), +- peripherals.GPIO1, +- peripherals.GPIO2, +- ).unwrap(); ++ Config::default()) ++ .unwrap() ++ .with_rx(peripherals.GPIO1) ++ .with_tx(peripherals.GPIO2); +``` + ## Spi `with_miso` has been split Previously, `with_miso` set up the provided pin as an input and output, which was necessary for half duplex. diff --git a/esp-hal/src/uart.rs b/esp-hal/src/uart.rs index bfedac448..e696fb1ef 100644 --- a/esp-hal/src/uart.rs +++ b/esp-hal/src/uart.rs @@ -26,10 +26,10 @@ //! //! let mut uart1 = Uart::new( //! peripherals.UART1, -//! Config::default(), -//! peripherals.GPIO1, -//! peripherals.GPIO2, -//! ).unwrap(); +//! Config::default()) +//! .unwrap() +//! .with_rx(peripherals.GPIO1) +//! .with_tx(peripherals.GPIO2); //! # } //! ``` //! @@ -57,9 +57,9 @@ //! # let mut uart1 = Uart::new( //! # peripherals.UART1, //! # Config::default(), -//! # peripherals.GPIO1, -//! # peripherals.GPIO2, -//! # ).unwrap(); +//! # ).unwrap() +//! # .with_rx(peripherals.GPIO1) +//! # .with_tx(peripherals.GPIO2); //! // Write bytes out over the UART: //! uart1.write_bytes(b"Hello, world!").expect("write error!"); //! # } @@ -72,9 +72,9 @@ //! # let mut uart1 = Uart::new( //! # peripherals.UART1, //! # Config::default(), -//! # peripherals.GPIO1, -//! # peripherals.GPIO2, -//! # ).unwrap(); +//! # ).unwrap() +//! # .with_rx(peripherals.GPIO1) +//! # .with_tx(peripherals.GPIO2); //! // The UART can be split into separate Transmit and Receive components: //! let (mut rx, mut tx) = uart1.split(); //! @@ -93,10 +93,10 @@ //! let (_, tx) = peripherals.GPIO1.split(); //! let mut uart1 = Uart::new( //! peripherals.UART1, -//! Config::default(), -//! rx.inverted(), -//! tx.inverted(), -//! ).unwrap(); +//! Config::default()) +//! .unwrap() +//! .with_rx(rx.inverted()) +//! .with_tx(tx.inverted()); //! # } //! ``` //! @@ -107,14 +107,14 @@ //! //! let tx = UartTx::new( //! peripherals.UART0, -//! Config::default(), -//! peripherals.GPIO1, -//! ).unwrap(); +//! Config::default()) +//! .unwrap() +//! .with_tx(peripherals.GPIO1); //! let rx = UartRx::new( //! peripherals.UART1, -//! Config::default(), -//! peripherals.GPIO2, -//! ).unwrap(); +//! Config::default()) +//! .unwrap() +//! .with_rx(peripherals.GPIO2); //! # } //! ``` //! @@ -156,10 +156,10 @@ //! //! let mut uart0 = Uart::new( //! peripherals.UART0, -//! config, -//! tx_pin, -//! rx_pin -//! ).unwrap(); +//! config) +//! .unwrap() +//! .with_rx(rx_pin) +//! .with_tx(tx_pin); //! //! uart0.set_interrupt_handler(interrupt_handler); //! @@ -499,24 +499,6 @@ where } } - fn with_rx(self, rx: impl Peripheral

+ 'd) -> Self { - crate::into_mapped_ref!(rx); - rx.init_input(Pull::Up, Internal); - self.uart.info().rx_signal.connect_to(rx); - - self - } - - fn with_tx(self, tx: impl Peripheral

+ 'd) -> Self { - crate::into_mapped_ref!(tx); - // Make sure we don't cause an unexpected low pulse on the pin. - tx.set_output_high(true, Internal); - tx.set_to_push_pull_output(Internal); - self.uart.info().tx_signal.connect_to(tx); - - self - } - fn init(self, config: Config) -> Result, ConfigError> { let rx_guard = PeripheralGuard::new(self.uart.parts().0.peripheral); let tx_guard = PeripheralGuard::new(self.uart.parts().0.peripheral); @@ -642,6 +624,20 @@ where self } + /// Assign the TX pin for UART instance. + /// + /// Sets the specified pin to push-pull output and connects it to the UART + /// TX signal. + pub fn with_tx(self, tx: impl Peripheral

+ 'd) -> Self { + crate::into_mapped_ref!(tx); + // Make sure we don't cause an unexpected low pulse on the pin. + tx.set_output_high(true, Internal); + tx.set_to_push_pull_output(Internal); + self.uart.info().tx_signal.connect_to(tx); + + self + } + /// Change the configuration. /// /// Note that this also changes the configuration of the RX half. @@ -736,9 +732,8 @@ impl<'d> UartTx<'d, Blocking> { pub fn new( uart: impl Peripheral

+ 'd, config: Config, - tx: impl Peripheral

+ 'd, ) -> Result { - Self::new_typed(uart.map_into(), config, tx) + Self::new_typed(uart.map_into(), config) } } @@ -750,10 +745,8 @@ where pub fn new_typed( uart: impl Peripheral

+ 'd, config: Config, - tx: impl Peripheral

+ 'd, ) -> Result { let (_, uart_tx) = UartBuilder::<'d, Blocking, T>::new(uart) - .with_tx(tx) .init(config)? .split(); @@ -833,6 +826,17 @@ where self } + /// Assign the RX pin for UART instance. + /// + /// Sets the specified pin to input and connects it to the UART RX signal. + pub fn with_rx(self, rx: impl Peripheral

+ 'd) -> Self { + crate::into_mapped_ref!(rx); + rx.init_input(Pull::Up, Internal); + self.uart.info().rx_signal.connect_to(rx); + + self + } + /// Change the configuration. /// /// Note that this also changes the configuration of the TX half. @@ -950,9 +954,8 @@ impl<'d> UartRx<'d, Blocking> { pub fn new( uart: impl Peripheral

+ 'd, config: Config, - rx: impl Peripheral

+ 'd, ) -> Result { - UartRx::new_typed(uart.map_into(), config, rx) + UartRx::new_typed(uart.map_into(), config) } } @@ -964,9 +967,8 @@ where pub fn new_typed( uart: impl Peripheral

+ 'd, config: Config, - rx: impl Peripheral

+ 'd, ) -> Result { - let (uart_rx, _) = UartBuilder::new(uart).with_rx(rx).init(config)?.split(); + let (uart_rx, _) = UartBuilder::new(uart).init(config)?.split(); Ok(uart_rx) } @@ -1015,10 +1017,8 @@ impl<'d> Uart<'d, Blocking> { pub fn new( uart: impl Peripheral

+ 'd, config: Config, - rx: impl Peripheral

+ 'd, - tx: impl Peripheral

+ 'd, ) -> Result { - Self::new_typed(uart.map_into(), config, rx, tx) + Self::new_typed(uart.map_into(), config) } } @@ -1030,10 +1030,8 @@ where pub fn new_typed( uart: impl Peripheral

+ 'd, config: Config, - rx: impl Peripheral

+ 'd, - tx: impl Peripheral

+ 'd, ) -> Result { - UartBuilder::new(uart).with_tx(tx).with_rx(rx).init(config) + UartBuilder::new(uart).init(config) } /// Reconfigures the driver to operate in [`Async`] mode. @@ -1043,6 +1041,31 @@ where tx: self.tx.into_async(), } } + + /// Assign the RX pin for UART instance. + /// + /// Sets the specified pin to input and connects it to the UART RX signal. + pub fn with_rx(self, rx: impl Peripheral

+ 'd) -> Self { + crate::into_mapped_ref!(rx); + rx.init_input(Pull::Up, Internal); + self.rx.uart.info().rx_signal.connect_to(rx); + + self + } + + /// Assign the TX pin for UART instance. + /// + /// Sets the specified pin to push-pull output and connects it to the UART + /// TX signal. + pub fn with_tx(self, tx: impl Peripheral

+ 'd) -> Self { + crate::into_mapped_ref!(tx); + // Make sure we don't cause an unexpected low pulse on the pin. + tx.set_output_high(true, Internal); + tx.set_to_push_pull_output(Internal); + self.tx.uart.info().tx_signal.connect_to(tx); + + self + } } impl<'d, T> Uart<'d, Async, T> diff --git a/examples/src/bin/embassy_serial.rs b/examples/src/bin/embassy_serial.rs index b5477b552..21678edb0 100644 --- a/examples/src/bin/embassy_serial.rs +++ b/examples/src/bin/embassy_serial.rs @@ -89,8 +89,10 @@ async fn main(spawner: Spawner) { let config = Config::default().with_rx_fifo_full_threshold(READ_BUF_SIZE as u16); - let mut uart0 = Uart::new(peripherals.UART0, config, rx_pin, tx_pin) + let mut uart0 = Uart::new(peripherals.UART0, config) .unwrap() + .with_tx(tx_pin) + .with_rx(rx_pin) .into_async(); uart0.set_at_cmd(AtCmdConfig::default().with_cmd_char(AT_CMD)); diff --git a/examples/src/bin/ieee802154_sniffer.rs b/examples/src/bin/ieee802154_sniffer.rs index d53228ccb..c94bb7fc9 100644 --- a/examples/src/bin/ieee802154_sniffer.rs +++ b/examples/src/bin/ieee802154_sniffer.rs @@ -24,19 +24,16 @@ fn main() -> ! { // Default pins for Uart/Serial communication cfg_if::cfg_if! { if #[cfg(feature = "esp32c6")] { - let (mut tx_pin, mut rx_pin) = (peripherals.GPIO16, peripherals.GPIO17); + let (tx_pin, rx_pin) = (peripherals.GPIO16, peripherals.GPIO17); } else if #[cfg(feature = "esp32h2")] { - let (mut tx_pin, mut rx_pin) = (peripherals.GPIO24, peripherals.GPIO23); + let (tx_pin, rx_pin) = (peripherals.GPIO24, peripherals.GPIO23); } } - let mut uart0 = Uart::new( - peripherals.UART0, - uart::Config::default(), - &mut rx_pin, - &mut tx_pin, - ) - .unwrap(); + let mut uart0 = Uart::new(peripherals.UART0, uart::Config::default()) + .unwrap() + .with_tx(tx_pin) + .with_rx(rx_pin); // read two characters which get parsed as the channel let mut cnt = 0; diff --git a/hil-test/tests/uart.rs b/hil-test/tests/uart.rs index 350d89863..c3722c021 100644 --- a/hil-test/tests/uart.rs +++ b/hil-test/tests/uart.rs @@ -30,7 +30,10 @@ mod tests { let (rx, tx) = pin.split(); - let uart = Uart::new(peripherals.UART1, uart::Config::default(), rx, tx).unwrap(); + let uart = Uart::new(peripherals.UART1, uart::Config::default()) + .unwrap() + .with_tx(tx) + .with_rx(rx); Context { uart } } diff --git a/hil-test/tests/uart_async.rs b/hil-test/tests/uart_async.rs index 7c60ab964..e2225a56f 100644 --- a/hil-test/tests/uart_async.rs +++ b/hil-test/tests/uart_async.rs @@ -27,8 +27,10 @@ mod tests { let (rx, tx) = hil_test::common_test_pins!(peripherals); - let uart = Uart::new(peripherals.UART0, uart::Config::default(), rx, tx) + let uart = Uart::new(peripherals.UART0, uart::Config::default()) .unwrap() + .with_tx(tx) + .with_rx(rx) .into_async(); Context { uart } diff --git a/hil-test/tests/uart_regression.rs b/hil-test/tests/uart_regression.rs index de1d9ad02..e1537dfce 100644 --- a/hil-test/tests/uart_regression.rs +++ b/hil-test/tests/uart_regression.rs @@ -21,7 +21,9 @@ mod tests { let (rx, mut tx) = hil_test::common_test_pins!(peripherals); - let mut rx = UartRx::new(peripherals.UART1, uart::Config::default(), rx).unwrap(); + let mut rx = UartRx::new(peripherals.UART1, uart::Config::default()) + .unwrap() + .with_rx(rx); // start reception _ = rx.read_byte(); // this will just return WouldBlock @@ -29,7 +31,9 @@ mod tests { unsafe { tx.set_output_high(false, esp_hal::Internal::conjure()) }; // set up TX and send a byte - let mut tx = UartTx::new(peripherals.UART0, uart::Config::default(), tx).unwrap(); + let mut tx = UartTx::new(peripherals.UART0, uart::Config::default()) + .unwrap() + .with_tx(tx); tx.flush().unwrap(); tx.write_bytes(&[0x42]).unwrap(); diff --git a/hil-test/tests/uart_tx_rx.rs b/hil-test/tests/uart_tx_rx.rs index fb20e0b26..bc05f042e 100644 --- a/hil-test/tests/uart_tx_rx.rs +++ b/hil-test/tests/uart_tx_rx.rs @@ -28,8 +28,12 @@ mod tests { let (rx, tx) = hil_test::common_test_pins!(peripherals); - let tx = UartTx::new(peripherals.UART0, uart::Config::default(), tx).unwrap(); - let rx = UartRx::new(peripherals.UART1, uart::Config::default(), rx).unwrap(); + let tx = UartTx::new(peripherals.UART0, uart::Config::default()) + .unwrap() + .with_tx(tx); + let rx = UartRx::new(peripherals.UART1, uart::Config::default()) + .unwrap() + .with_rx(rx); Context { rx, tx } } diff --git a/hil-test/tests/uart_tx_rx_async.rs b/hil-test/tests/uart_tx_rx_async.rs index 30b916e75..5e8822a01 100644 --- a/hil-test/tests/uart_tx_rx_async.rs +++ b/hil-test/tests/uart_tx_rx_async.rs @@ -28,11 +28,13 @@ mod tests { let (rx, tx) = hil_test::common_test_pins!(peripherals); - let tx = UartTx::new(peripherals.UART0, uart::Config::default(), tx) + let tx = UartTx::new(peripherals.UART0, uart::Config::default()) .unwrap() + .with_tx(tx) .into_async(); - let rx = UartRx::new(peripherals.UART1, uart::Config::default(), rx) + let rx = UartRx::new(peripherals.UART1, uart::Config::default()) .unwrap() + .with_rx(rx) .into_async(); Context { rx, tx }