Fix some UART-related issues

esp-hal/CHANGELOG.md

@@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - ESP32-H2: light sleep and deep sleep support with timer wakeup source (#4587)
 
 ### Changed
+
 - RMT: `SingleShotTxTransaction` has been renamed to `TxTransaction`. (#4302)
 - RMT: `ChannelCreator::configure_tx` and `ChannelCreator::configure_rx` now take the configuration by reference. (#4302)
 - RMT: `ChannelCreator::configure_tx` and `ChannelCreator::configure_rx` don't take a pin anymore, instead `Channel::with_pin` has been added. (#4302)
@@ -32,6 +33,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - RMT: the `place_rmt_driver_in_ram` option now also places the async interrupt handler in RAM. (#4302)
 - RMT: When dropping a Tx channel, the driver now disconnects the output pin from the peripheral. (#4302)
 - I2C: avoid potential infinite loop while checking for command completion (#4519)
+- UART: correct documentation of `read` which incorrectly stated that it would never block (#4586)
+- UART: fixed an issue in `read_exact_async` which may cause `FifoOverflowed` to be returned unexpectedly. (#4586)
 
 ### Removed
 

esp-hal/src/uart/mod.rs

@@ -1072,23 +1072,28 @@ impl<'d> UartRx<'d, Async> {
     async fn wait_for_buffered_data(
         &mut self,
         minimum: usize,
-        preferred: usize,
+        max_threshold: usize,
         listen_for_timeout: bool,
     ) -> Result<(), RxError> {
-        while self.uart.info().rx_fifo_count() < (minimum as u16).min(Info::RX_FIFO_MAX_THRHD) {
-            let amount = u16::try_from(preferred)
-                .unwrap_or(Info::RX_FIFO_MAX_THRHD)
-                .min(Info::RX_FIFO_MAX_THRHD);
+        let current_threshold = self.uart.info().rx_fifo_full_threshold();
 
-            let current = self.uart.info().rx_fifo_full_threshold();
-            let _guard = if current > amount {
+        // User preference takes priority.
+        let max_threshold = max_threshold.min(current_threshold as usize) as u16;
+        let minimum = minimum.min(Info::RX_FIFO_MAX_THRHD as usize) as u16;
+
+        // The effective threshold must be >= minimum. We ensure this by lowering the minimum number
+        // of returnable bytes.
+        let minimum = minimum.min(max_threshold);
+
+        while self.uart.info().rx_fifo_count() < minimum {
+            let _guard = if current_threshold > max_threshold {
                 // We're ignoring the user configuration here to ensure that this is not waiting
                 // for more data than the buffer. We'll restore the original value after the
                 // future resolved.
                 let info = self.uart.info();
-                unwrap!(info.set_rx_fifo_full_threshold(amount));
+                unwrap!(info.set_rx_fifo_full_threshold(max_threshold));
                 Some(OnDrop::new(|| {
-                    unwrap!(info.set_rx_fifo_full_threshold(current));
+                    unwrap!(info.set_rx_fifo_full_threshold(current_threshold));
                 }))
             } else {
                 None
@@ -1300,7 +1305,8 @@ where
     ///
     /// The UART hardware continuously receives bytes and stores them in the RX
     /// FIFO. This function reads the bytes from the RX FIFO and returns
-    /// them in the provided buffer, without blocking.
+    /// them in the provided buffer. If the hardware buffer is empty, this
+    /// function will block until data is available.
     ///
     /// The function returns the number of bytes read into the buffer. This may
     /// be less than the length of the buffer. This function only returns 0
@@ -1798,9 +1804,31 @@ where
         self.tx.uart.info().regs()
     }
 
-    /// Returns whether the UART buffer is ready to accept more data.
+    #[procmacros::doc_replace]
+    /// Returns whether the UART TX buffer is ready to accept more data.
     ///
-    /// If this function returns `true`, [`Self::write`] will not block.
+    /// If this function returns `true`, [`Self::write`] and [`Self::write_async`]
+    /// will not block. Otherwise, the functions will not return until the buffer is
+    /// ready.
+    ///
+    /// ## Example
+    ///
+    /// ```rust, no_run
+    /// # {before_snippet}
+    /// use esp_hal::uart::{Config, Uart};
+    /// let mut uart = Uart::new(peripherals.UART0, Config::default())?;
+    ///
+    /// if uart.write_ready() {
+    ///     // Because write_ready has returned true, the following call will immediately
+    ///     // copy some bytes into the FIFO and return a non-zero value.
+    ///     let written = uart.write(b"Hello")?;
+    ///     // ... handle written bytes
+    /// } else {
+    ///     // Calling write would have blocked, but here we can do something useful
+    ///     // instead of waiting for the buffer to become ready.
+    /// }
+    /// # {after_snippet}
+    /// ```
     #[instability::unstable]
     pub fn write_ready(&mut self) -> bool {
         self.tx.write_ready()
@@ -1861,9 +1889,31 @@ where
         self.tx.send_break(bits)
     }
 
-    /// Returns whether the UART buffer has data.
+    #[procmacros::doc_replace]
+    /// Returns whether the UART receive buffer has at least one byte of data.
     ///
-    /// If this function returns `true`, [`Self::read`] will not block.
+    /// If this function returns `true`, [`Self::read`] and [`Self::read_async`]
+    /// will not block. Otherwise, they will not return until data is available.
+    ///
+    /// Data that does not get stored due to an error will be lost and does not count
+    /// towards the number of bytes in the receive buffer.
+    // TODO: once we add support for UART_ERR_WR_MASK it needs to be documented here.
+    /// ## Example
+    ///
+    /// ```rust, no_run
+    /// # {before_snippet}
+    /// use esp_hal::uart::{Config, Uart};
+    /// let mut uart = Uart::new(peripherals.UART0, Config::default())?;
+    ///
+    /// while !uart.read_ready() {
+    ///     // Do something else while waiting for data to be available.
+    /// }
+    ///
+    /// let mut buf = [0u8; 32];
+    /// uart.read(&mut buf[..])?;
+    ///
+    /// # {after_snippet}
+    /// ```
     #[instability::unstable]
     pub fn read_ready(&mut self) -> bool {
         self.rx.read_ready()
@@ -1874,7 +1924,9 @@ where
     ///
     /// The UART hardware continuously receives bytes and stores them in the RX
     /// FIFO. This function reads the bytes from the RX FIFO and returns
-    /// them in the provided buffer, without blocking.
+    /// them in the provided buffer. If the hardware buffer is empty, this
+    /// function will block until data is available. The [`Self::read_ready`]
+    /// function can be used to check if data is available without blocking.
     ///
     /// The function returns the number of bytes read into the buffer. This may
     /// be less than the length of the buffer. This function only returns 0