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

@@ -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
 

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.

esp-hal/src/uart.rs

@@ -26,10 +26,10 @@
 //!
 //! let mut uart1 = Uart::new(
 //!     peripherals.UART1,
-//!     Config::default(),
+//!     Config::default())
-//!     peripherals.GPIO1,
+//!     .unwrap()
-//!     peripherals.GPIO2,
+//!     .with_rx(peripherals.GPIO1)
-//! ).unwrap();
+//!     .with_tx(peripherals.GPIO2);
 //! # }
 //! ```
 //! 
@@ -57,9 +57,9 @@
 //! # let mut uart1 = Uart::new(
 //! #     peripherals.UART1,
 //! #     Config::default(),
-//! #     peripherals.GPIO1,
+//! # ).unwrap()
-//! #     peripherals.GPIO2,
+//! # .with_rx(peripherals.GPIO1)
-//! # ).unwrap();
+//! # .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,
+//! # ).unwrap()
-//! #     peripherals.GPIO2,
+//! # .with_rx(peripherals.GPIO1)
-//! # ).unwrap();
+//! # .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(),
+//!     Config::default())
-//!     rx.inverted(),
+//!     .unwrap()
-//!     tx.inverted(),
+//!     .with_rx(rx.inverted())
-//! ).unwrap();
+//!     .with_tx(tx.inverted());
 //! # }
 //! ```
 //! 
@@ -107,14 +107,14 @@
 //!
 //! let tx = UartTx::new(
 //!     peripherals.UART0,
-//!     Config::default(),
+//!     Config::default())
-//!     peripherals.GPIO1,
+//!     .unwrap()
-//! ).unwrap();
+//!     .with_tx(peripherals.GPIO1);
 //! let rx = UartRx::new(
 //!     peripherals.UART1,
-//!     Config::default(),
+//!     Config::default())
-//!     peripherals.GPIO2,
+//!     .unwrap()
-//! ).unwrap();
+//!     .with_rx(peripherals.GPIO2);
 //! # }
 //! ```
 //! 
@@ -156,10 +156,10 @@
 //!
 //! let mut uart0 = Uart::new(
 //!     peripherals.UART0,
-//!     config,
+//!     config)
-//!     tx_pin,
+//!     .unwrap()
-//!     rx_pin
+//!     .with_rx(rx_pin)
-//! ).unwrap();
+//!     .with_tx(tx_pin);
 //!
 //! uart0.set_interrupt_handler(interrupt_handler);
 //!
@@ -499,24 +499,6 @@ where
         }
     }
 
-    fn with_rx(self, rx: impl Peripheral<P = impl PeripheralInput> + '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<P = impl PeripheralOutput> + '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<Uart<'d, Dm, T>, 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<P = impl PeripheralOutput> + '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<P = impl Instance> + 'd,
         config: Config,
-        tx: impl Peripheral<P = impl PeripheralOutput> + 'd,
     ) -> Result<Self, ConfigError> {
-        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<P = T> + 'd,
         config: Config,
-        tx: impl Peripheral<P = impl PeripheralOutput> + 'd,
     ) -> Result<Self, ConfigError> {
         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<P = impl PeripheralInput> + '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<P = impl Instance> + 'd,
         config: Config,
-        rx: impl Peripheral<P = impl PeripheralInput> + 'd,
     ) -> Result<Self, ConfigError> {
-        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<P = T> + 'd,
         config: Config,
-        rx: impl Peripheral<P = impl PeripheralInput> + 'd,
     ) -> Result<Self, ConfigError> {
-        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<P = impl Instance> + 'd,
         config: Config,
-        rx: impl Peripheral<P = impl PeripheralInput> + 'd,
-        tx: impl Peripheral<P = impl PeripheralOutput> + 'd,
     ) -> Result<Self, ConfigError> {
-        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<P = T> + 'd,
         config: Config,
-        rx: impl Peripheral<P = impl PeripheralInput> + 'd,
-        tx: impl Peripheral<P = impl PeripheralOutput> + 'd,
     ) -> Result<Self, ConfigError> {
-        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<P = impl PeripheralInput> + '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<P = impl PeripheralOutput> + '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>

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));
 

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(
+    let mut uart0 = Uart::new(peripherals.UART0, uart::Config::default())
-        peripherals.UART0,
+        .unwrap()
-        uart::Config::default(),
+        .with_tx(tx_pin)
-        &mut rx_pin,
+        .with_rx(rx_pin);
-        &mut tx_pin,
-    )
-    .unwrap();
 
     // read two characters which get parsed as the channel
     let mut cnt = 0;

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 }
     }

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 }

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();

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 tx = UartTx::new(peripherals.UART0, uart::Config::default())
-        let rx = UartRx::new(peripherals.UART1, uart::Config::default(), rx).unwrap();
+            .unwrap()
+            .with_tx(tx);
+        let rx = UartRx::new(peripherals.UART1, uart::Config::default())
+            .unwrap()
+            .with_rx(rx);
 
         Context { rx, tx }
     }

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 }