diff --git a/esp-hal/CHANGELOG.md b/esp-hal/CHANGELOG.md index 45c7048ffb..9db1397563 100644 --- a/esp-hal/CHANGELOG.md +++ b/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 diff --git a/esp-hal/src/uart/mod.rs b/esp-hal/src/uart/mod.rs index add9b58bd8..b8600cdaf7 100644 --- a/esp-hal/src/uart/mod.rs +++ b/esp-hal/src/uart/mod.rs @@ -1072,27 +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 = self.uart.info().rx_fifo_full_threshold(); - let _guard = if current > amount { - // 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)); - Some(OnDrop::new(|| { - unwrap!(info.set_rx_fifo_full_threshold(current)); - })) - } else { - None - }; + let current_threshold = self.uart.info().rx_fifo_full_threshold(); + + // 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); + + if self.uart.info().rx_fifo_count() < minimum { + // 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 @@ -1173,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 @@ -1300,7 +1309,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 +1808,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 +1893,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 +1928,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 @@ -3666,6 +3722,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) } }