UART: Remove blocking version of read_bytes and rename drain_fifo to read_bytes instead (#2895)

* Remove `read_bytes` function, rename `drain_fifo` to `read_bytes`

* Update migration guide

* Update `CHANGELOG.md`

* Use `crate::interrupt::free` in `read_byte`

* Fix HIL test

* Only use `crate::interrupt:free` for ESP32

esp-hal/CHANGELOG.md

@@ -92,6 +92,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - UART: Make `AtCmdConfig` and `ConfigError` non-exhaustive (#2851)
 - UART: Make `AtCmdConfig` use builder-lite pattern (#2851)
 - UART: Fix naming violations for `DataBits`, `Parity`, and `StopBits` enum variants (#2893)
+- UART: Remove blocking version of `read_bytes` and rename `drain_fifo` to `read_bytes` instead (#2895)
 
 ### Fixed
 

esp-hal/MIGRATING-0.22.md

@@ -382,3 +382,7 @@ e.g.)
 - StopBits::Stop1
 + StopBits::_1
 ```
+
+The previous blocking implementation of `read_bytes` has been removed, and the non-blocking `drain_fifo` has instead been renamed to `read_bytes` in its place.
+
+Any code which was previously using `read_bytes` to fill a buffer in a blocking manner will now need to implement the necessary logic to block until the buffer is filled in their application instead.

esp-hal/src/uart.rs

@@ -848,40 +848,6 @@ where
         self.uart.info().apply_config(config)
     }
 
-    /// Fill a buffer with received bytes
-    pub fn read_bytes(&mut self, buf: &mut [u8]) -> Result<(), Error> {
-        cfg_if::cfg_if! {
-            if #[cfg(esp32s2)] {
-                // On the ESP32-S2 we need to use PeriBus2 to read the FIFO:
-                let fifo = unsafe {
-                    &*((self.register_block().fifo().as_ptr() as *mut u8).add(0x20C00000)
-                        as *mut crate::peripherals::uart0::FIFO)
-                };
-            } else {
-                let fifo = self.register_block().fifo();
-            }
-        }
-
-        for byte in buf.iter_mut() {
-            while self.rx_fifo_count() == 0 {
-                // Block until we received at least one byte
-            }
-
-            cfg_if::cfg_if! {
-                if #[cfg(esp32)] {
-                    // https://docs.espressif.com/projects/esp-chip-errata/en/latest/esp32/03-errata-description/esp32/cpu-subsequent-access-halted-when-get-interrupted.html
-                    crate::interrupt::free(|| {
-                        *byte = fifo.read().rxfifo_rd_byte().bits();
-                    });
-                } else {
-                    *byte = fifo.read().rxfifo_rd_byte().bits();
-                }
-            }
-        }
-
-        Ok(())
-    }
-
     /// Read a byte from the UART
     pub fn read_byte(&mut self) -> nb::Result<u8, Error> {
         cfg_if::cfg_if! {
@@ -897,15 +863,24 @@ where
         }
 
         if self.rx_fifo_count() > 0 {
-            Ok(fifo.read().rxfifo_rd_byte().bits())
+            // https://docs.espressif.com/projects/esp-chip-errata/en/latest/esp32/03-errata-description/esp32/cpu-subsequent-access-halted-when-get-interrupted.html
+            cfg_if::cfg_if! {
+                if #[cfg(esp32)] {
+                    let byte = crate::interrupt::free(|| fifo.read().rxfifo_rd_byte().bits());
+                } else {
+                    let byte = fifo.read().rxfifo_rd_byte().bits();
+                }
+            }
+
+            Ok(byte)
         } else {
             Err(nb::Error::WouldBlock)
         }
     }
 
     /// Read all available bytes from the RX FIFO into the provided buffer and
-    /// returns the number of read bytes. Never blocks
+    /// returns the number of read bytes without blocking.
-    pub fn drain_fifo(&mut self, buf: &mut [u8]) -> usize {
+    pub fn read_bytes(&mut self, buf: &mut [u8]) -> usize {
         let mut count = 0;
         while count < buf.len() {
             if let Ok(byte) = self.read_byte() {
@@ -1143,8 +1118,9 @@ where
         self.tx.write_bytes(data)
     }
 
-    /// Fill a buffer with received bytes
+    /// Read all available bytes from the RX FIFO into the provided buffer and
-    pub fn read_bytes(&mut self, buf: &mut [u8]) -> Result<(), Error> {
+    /// returns the number of read bytes without blocking.
+    pub fn read_bytes(&mut self, buf: &mut [u8]) -> usize {
         self.rx.read_bytes(buf)
     }
 
@@ -1481,7 +1457,7 @@ where
             // Block until we received at least one byte
         }
 
-        Ok(self.drain_fifo(buf))
+        Ok(self.read_bytes(buf))
     }
 }
 
@@ -1858,7 +1834,7 @@ where
 
             let events_happened = UartRxFuture::new(self.uart.reborrow(), events).await;
             // always drain the fifo, if an error has occurred the data is lost
-            let read_bytes = self.drain_fifo(buf);
+            let read_bytes = self.read_bytes(buf);
             // check error events
             for event_happened in events_happened {
                 match event_happened {

hil-test/tests/uart_tx_rx.rs

@@ -53,8 +53,15 @@ mod tests {
         ctx.tx.flush().unwrap();
         ctx.tx.write_bytes(&bytes).unwrap();
 
-        ctx.rx.read_bytes(&mut buf).unwrap();
+        let mut bytes_read = 0;
+        loop {
+            bytes_read += ctx.rx.read_bytes(&mut buf[bytes_read..]);
+            if bytes_read == 3 {
+                break;
+            }
+        }
 
+        assert_eq!(bytes_read, 3);
         assert_eq!(buf, bytes);
     }
 }