From 3c03e3bce7f7e167902e7ca5be60c12b08e9f384 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Mon, 1 Dec 2025 16:57:16 +0100 Subject: [PATCH 1/9] Correct read documentation --- esp-hal/CHANGELOG.md | 1 + esp-hal/src/uart/mod.rs | 6 ++++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/esp-hal/CHANGELOG.md b/esp-hal/CHANGELOG.md index 45c7048ffb..9099bebb9b 100644 --- a/esp-hal/CHANGELOG.md +++ b/esp-hal/CHANGELOG.md @@ -32,6 +32,7 @@ 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 (#?) ### Removed diff --git a/esp-hal/src/uart/mod.rs b/esp-hal/src/uart/mod.rs index add9b58bd8..a18984428b 100644 --- a/esp-hal/src/uart/mod.rs +++ b/esp-hal/src/uart/mod.rs @@ -1300,7 +1300,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 @@ -1874,7 +1875,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 From 52aaaf97dcf574c842ddeb1052a10dbdcf96354a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Mon, 1 Dec 2025 17:42:14 +0100 Subject: [PATCH 2/9] Fix loop condition --- esp-hal/CHANGELOG.md | 1 + esp-hal/src/uart/mod.rs | 23 ++++++++++++++--------- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/esp-hal/CHANGELOG.md b/esp-hal/CHANGELOG.md index 9099bebb9b..088054c67a 100644 --- a/esp-hal/CHANGELOG.md +++ b/esp-hal/CHANGELOG.md @@ -22,6 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - RMT: `ChannelCreator::configure_tx` and `ChannelCreator::configure_rx` don't take a pin anymore, instead `Channel::with_pin` has been added. (#4302) - RMT: Configuration errors have been split out of `rmt::Error` into the new `rmt::ConfigError` enum. (#4494) - RMT: `Rmt::new()` now returns `Error::UnreachableTargetFrequency` instead of panicking when requesting 0 Hz. (#4509) +- UART: changed the FIFO threshold in `read_exact_async` to reduce the likelyhood of `FifoOverflowed` errors. (#?) - Internal clock configuration rework (#4501) diff --git a/esp-hal/src/uart/mod.rs b/esp-hal/src/uart/mod.rs index a18984428b..5d0543b0bb 100644 --- a/esp-hal/src/uart/mod.rs +++ b/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 From fc5fb06ef44b8c1f93de8bd78f0adf485e63c27f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Mon, 1 Dec 2025 17:47:59 +0100 Subject: [PATCH 3/9] Changelog PR numbers --- esp-hal/CHANGELOG.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/esp-hal/CHANGELOG.md b/esp-hal/CHANGELOG.md index 088054c67a..f205769bf9 100644 --- a/esp-hal/CHANGELOG.md +++ b/esp-hal/CHANGELOG.md @@ -17,12 +17,13 @@ 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 + +- UART: `fn read_ready` is now stable (#4586) - 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) - RMT: Configuration errors have been split out of `rmt::Error` into the new `rmt::ConfigError` enum. (#4494) - RMT: `Rmt::new()` now returns `Error::UnreachableTargetFrequency` instead of panicking when requesting 0 Hz. (#4509) -- UART: changed the FIFO threshold in `read_exact_async` to reduce the likelyhood of `FifoOverflowed` errors. (#?) - Internal clock configuration rework (#4501) @@ -33,7 +34,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 (#?) +- 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 From 146fc27392e2e77e8dfc40812c7dad3b9bae0552 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Tue, 2 Dec 2025 16:35:13 +0100 Subject: [PATCH 4/9] Expand read_ready documentation --- esp-hal/CHANGELOG.md | 1 - esp-hal/src/uart/mod.rs | 29 ++++++++++++++++++++++++++--- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/esp-hal/CHANGELOG.md b/esp-hal/CHANGELOG.md index f205769bf9..9db1397563 100644 --- a/esp-hal/CHANGELOG.md +++ b/esp-hal/CHANGELOG.md @@ -18,7 +18,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed -- UART: `fn read_ready` is now stable (#4586) - 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) diff --git a/esp-hal/src/uart/mod.rs b/esp-hal/src/uart/mod.rs index 5d0543b0bb..07bacd186a 100644 --- a/esp-hal/src/uart/mod.rs +++ b/esp-hal/src/uart/mod.rs @@ -1867,9 +1867,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() @@ -1881,7 +1903,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. If the hardware buffer is empty, this - /// function will block until data is available. + /// 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 From 1335a995d1952cf465d45a61805f903e3794a528 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Tue, 2 Dec 2025 17:13:13 +0100 Subject: [PATCH 5/9] Expand write_ready documentation --- esp-hal/src/uart/mod.rs | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/esp-hal/src/uart/mod.rs b/esp-hal/src/uart/mod.rs index 07bacd186a..87cd5b9579 100644 --- a/esp-hal/src/uart/mod.rs +++ b/esp-hal/src/uart/mod.rs @@ -1804,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() From 564a18b7b47a19083bb0fb3f7c78eaed08f21646 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Wed, 3 Dec 2025 09:24:42 +0100 Subject: [PATCH 6/9] Remove loop, clear fifo full when reading data --- esp-hal/src/uart/mod.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/esp-hal/src/uart/mod.rs b/esp-hal/src/uart/mod.rs index 87cd5b9579..90dbc2f460 100644 --- a/esp-hal/src/uart/mod.rs +++ b/esp-hal/src/uart/mod.rs @@ -1085,7 +1085,7 @@ impl<'d> UartRx<'d, Async> { // of returnable bytes. let minimum = minimum.min(max_threshold); - while self.uart.info().rx_fifo_count() < minimum { + if 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 @@ -3718,6 +3718,9 @@ impl Info { *byte_into = self.read_next_from_fifo(); } + // This bit is not cleared until the FIFO actually drops below the threshold. + self.clear_rx_events(RxEvent::FifoFull); + Ok(to_read) } } From a3f1df5978818af104be75f981a6388aec07cdc0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Wed, 3 Dec 2025 09:41:11 +0100 Subject: [PATCH 7/9] Unconditionally set threshold --- esp-hal/src/uart/mod.rs | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/esp-hal/src/uart/mod.rs b/esp-hal/src/uart/mod.rs index 90dbc2f460..6e04cd1da5 100644 --- a/esp-hal/src/uart/mod.rs +++ b/esp-hal/src/uart/mod.rs @@ -1086,18 +1086,14 @@ impl<'d> UartRx<'d, Async> { let minimum = minimum.min(max_threshold); if 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(max_threshold)); - Some(OnDrop::new(|| { - unwrap!(info.set_rx_fifo_full_threshold(current_threshold)); - })) - } else { - None - }; + // 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(max_threshold)); + let _guard = OnDrop::new(|| { + unwrap!(info.set_rx_fifo_full_threshold(current_threshold)); + }); // Wait for space or event let mut events = RxEvent::FifoFull From 175cfb65b391c13bae98067db513994bce69e662 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Wed, 3 Dec 2025 09:44:10 +0100 Subject: [PATCH 8/9] Drain bytes before waiting for fifo --- esp-hal/src/uart/mod.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/esp-hal/src/uart/mod.rs b/esp-hal/src/uart/mod.rs index 6e04cd1da5..b8600cdaf7 100644 --- a/esp-hal/src/uart/mod.rs +++ b/esp-hal/src/uart/mod.rs @@ -1174,6 +1174,14 @@ impl<'d> UartRx<'d, Async> { /// before it resolves, or if an error occurs during the read operation, /// previously read data may be lost. pub async fn read_exact_async(&mut self, mut buf: &mut [u8]) -> Result<(), RxError> { + if buf.is_empty() { + return Ok(()); + } + + // Drain the buffer first, there's no point in waiting for data we've already received. + let read = self.uart.info().read_buffered(buf)?; + buf = &mut buf[read..]; + while !buf.is_empty() { // No point in listening for timeouts, as we're waiting for an exact amount of // data. On ESP32 and S2, the timeout interrupt can't be cleared unless the FIFO From 76cab5b912eb08ce94152c10aef47035fcf2c471 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Wed, 3 Dec 2025 13:36:00 +0100 Subject: [PATCH 9/9] Remove obsolete changelog entry --- esp-hal/CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/esp-hal/CHANGELOG.md b/esp-hal/CHANGELOG.md index 9db1397563..b6f77ccad5 100644 --- a/esp-hal/CHANGELOG.md +++ b/esp-hal/CHANGELOG.md @@ -34,7 +34,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - 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