Multiple API fixes in UART driver (#2851)

* Close a number of issues

doctest update

fmt

* changelog entry

* dumb

* addressing reviews

* Add an entry to migration guide

esp-hal/CHANGELOG.md

@@ -87,6 +87,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - `crate::Mode` was renamed to `crate::DriverMode` (#2828)
 - Renamed some I2C error variants (#2844)
 - I2C: Replaced potential panics with errors. (#2831)
+- UART: Make `AtCmdConfig` and `ConfigError` non-exhaustive (#2851)
+- UART: Make `AtCmdConfig` use builder-lite pattern (#2851)
 
 ### Fixed
 
@@ -94,6 +96,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - User-bound GPIO interrupt handlers should no longer interfere with async pins. (#2625)
 - `spi::master::Spi::{into_async, into_blocking}` are now correctly available on the typed driver, to. (#2674)
 - It is no longer possible to safely conjure `GpioPin` instances (#2688)
+- UART: Public API follows `C-WORD_ORDER` Rust API standard (`VerbObject` order) (#2851)
 
 ### Removed
 

esp-hal/MIGRATING-0.22.md

@@ -336,3 +336,10 @@ The reexports that were previously part of the prelude are available through oth
     - `esp_hal::timer::Timer`
     - `esp_hal::interrupt::InterruptConfigurable`
 - The `entry` macro can be imported as `esp_hal::entry`, while other macros are found under `esp_hal::macros`
+
+## `AtCmdConfig` now uses builder-lite pattern
+
+```diff
+- uart0.set_at_cmd(AtCmdConfig::new(None, None, None, b'#', None));
++ uart0.set_at_cmd(AtCmdConfig::default().with_cmd_char(b'#'));
+```

esp-hal/src/uart.rs

@@ -164,7 +164,7 @@
 //! uart0.set_interrupt_handler(interrupt_handler);
 //!
 //! critical_section::with(|cs| {
-//!     uart0.set_at_cmd(AtCmdConfig::new(None, None, None, b'#', None));
+//!     uart0.set_at_cmd(AtCmdConfig::default().with_cmd_char(b'#'));
 //!     uart0.listen(UartInterrupt::AtCmd | UartInterrupt::RxFifoFull);
 //!
 //!     SERIAL.borrow_ref_mut(cs).replace(uart0);
@@ -250,6 +250,7 @@ use crate::{
 };
 
 const UART_FIFO_SIZE: u16 = 128;
+const CMD_CHAR_DEFAULT: u8 = 0x2b;
 
 /// UART Error
 #[derive(Debug, Clone, Copy, PartialEq)]
@@ -510,6 +511,9 @@ impl Default for Config {
     }
 }
 
+#[derive(Clone, Copy, PartialEq, Eq, Hash, Debug, procmacros::BuilderLite)]
+#[cfg_attr(feature = "defmt", derive(defmt::Format))]
+#[non_exhaustive]
 /// Configuration for the AT-CMD detection functionality
 pub struct AtCmdConfig {
     /// Optional idle time before the AT command detection begins, in clock
@@ -524,28 +528,17 @@ pub struct AtCmdConfig {
     /// The character that triggers the AT command detection.
     pub cmd_char: u8,
     /// Optional number of characters to detect as part of the AT command.
-    pub char_num: Option<u8>,
+    pub char_num: u8,
 }
 
-impl AtCmdConfig {
-    /// Creates a new `AtCmdConfig` with the specified configuration.
-    ///
-    /// This function sets up the AT command detection parameters, including
-    /// pre- and post-idle times, a gap timeout, the triggering command
-    /// character, and the number of characters to detect.
-    pub fn new(
-        pre_idle_count: Option<u16>,
-        post_idle_count: Option<u16>,
-        gap_timeout: Option<u16>,
-        cmd_char: u8,
-        char_num: Option<u8>,
-    ) -> AtCmdConfig {
+impl Default for AtCmdConfig {
+    fn default() -> Self {
         Self {
-            pre_idle_count,
-            post_idle_count,
-            gap_timeout,
-            cmd_char,
-            char_num,
+            pre_idle_count: None,
+            post_idle_count: None,
+            gap_timeout: None,
+            cmd_char: CMD_CHAR_DEFAULT,
+            char_num: 1,
         }
     }
 }
@@ -631,6 +624,7 @@ pub struct UartRx<'d, Dm, T = AnyUart> {
 /// A configuration error.
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 #[cfg_attr(feature = "defmt", derive(defmt::Format))]
+#[non_exhaustive]
 pub enum ConfigError {
     /// The requested timeout is not supported.
     UnsupportedTimeout,
@@ -752,7 +746,7 @@ where
     }
 
     /// Flush the transmit buffer of the UART
-    pub fn flush_tx(&mut self) -> nb::Result<(), Error> {
+    pub fn flush(&mut self) -> nb::Result<(), Error> {
         if self.is_tx_idle() {
             Ok(())
         } else {
@@ -1219,7 +1213,7 @@ where
 
         register_block.at_cmd_char().write(|w| unsafe {
             w.at_cmd_char().bits(config.cmd_char);
-            w.char_num().bits(config.char_num.unwrap_or(1))
+            w.char_num().bits(config.char_num)
         });
 
         if let Some(pre_idle_count) = config.pre_idle_count {
@@ -1254,8 +1248,8 @@ where
     }
 
     /// Flush the transmit buffer of the UART
-    pub fn flush_tx(&mut self) -> nb::Result<(), Error> {
-        self.tx.flush_tx()
+    pub fn flush(&mut self) -> nb::Result<(), Error> {
+        self.tx.flush()
     }
 
     /// Read a byte from the UART
@@ -1477,7 +1471,7 @@ where
     }
 
     fn flush(&mut self) -> nb::Result<(), Self::Error> {
-        self.flush_tx()
+        self.flush()
     }
 }
 
@@ -1491,7 +1485,7 @@ where
     }
 
     fn flush(&mut self) -> nb::Result<(), Self::Error> {
-        self.flush_tx()
+        self.flush()
     }
 }
 
@@ -1581,7 +1575,7 @@ where
     }
 
     fn flush(&mut self) -> Result<(), Self::Error> {
-        self.tx.flush()
+        embedded_io::Write::flush(&mut self.tx)
     }
 }
 
@@ -1598,7 +1592,7 @@ where
 
     fn flush(&mut self) -> Result<(), Self::Error> {
         loop {
-            match self.flush_tx() {
+            match self.flush() {
                 Ok(_) => break,
                 Err(nb::Error::WouldBlock) => { /* Wait */ }
                 Err(nb::Error::Other(e)) => return Err(e),

examples/src/bin/embassy_serial.rs

@@ -92,7 +92,7 @@ async fn main(spawner: Spawner) {
     let mut uart0 = Uart::new(peripherals.UART0, config, rx_pin, tx_pin)
         .unwrap()
         .into_async();
-    uart0.set_at_cmd(AtCmdConfig::new(None, None, None, AT_CMD, None));
+    uart0.set_at_cmd(AtCmdConfig::default().with_cmd_char(AT_CMD));
 
     let (rx, tx) = uart0.split();
 

hil-test/tests/uart_regression.rs

@@ -31,7 +31,7 @@ mod tests {
         // set up TX and send a byte
         let mut tx = UartTx::new(peripherals.UART0, uart::Config::default(), tx).unwrap();
 
-        tx.flush_tx().unwrap();
+        tx.flush().unwrap();
         tx.write_bytes(&[0x42]).unwrap();
         let read = block!(rx.read_byte());
 

hil-test/tests/uart_tx_rx.rs

@@ -38,7 +38,7 @@ mod tests {
     fn test_send_receive(mut ctx: Context) {
         let byte = [0x42];
 
-        ctx.tx.flush_tx().unwrap();
+        ctx.tx.flush().unwrap();
         ctx.tx.write_bytes(&byte).unwrap();
         let read = block!(ctx.rx.read_byte());
 
@@ -50,7 +50,7 @@ mod tests {
         let bytes = [0x42, 0x43, 0x44];
         let mut buf = [0u8; 3];
 
-        ctx.tx.flush_tx().unwrap();
+        ctx.tx.flush().unwrap();
         ctx.tx.write_bytes(&bytes).unwrap();
 
         ctx.rx.read_bytes(&mut buf).unwrap();