From 6494354a91c3b8e04b23c8c87da073a867bc0263 Mon Sep 17 00:00:00 2001 From: Dominic Fischer <14130965+Dominaezzz@users.noreply.github.com> Date: Mon, 6 Jan 2025 07:14:47 +0000 Subject: [PATCH 1/6] Fix small typo in migration guide (#2888) Co-authored-by: Dominic Fischer --- esp-hal/MIGRATING-0.22.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/esp-hal/MIGRATING-0.22.md b/esp-hal/MIGRATING-0.22.md index 3a50f105d..b88c6e302 100644 --- a/esp-hal/MIGRATING-0.22.md +++ b/esp-hal/MIGRATING-0.22.md @@ -262,7 +262,7 @@ is not compatible with the hardware. +.unwrap(); ``` -### LCD_CAM configuration changes +## LCD_CAM configuration changes - `cam` now has a `Config` strurct that contains frequency, bit/byte order, VSync filter options. - DPI, I8080: `frequency` has been moved into `Config`. From 05b5bac027a53b44243cba9ff2b1d737ac145471 Mon Sep 17 00:00:00 2001 From: Jesse Braham Date: Mon, 6 Jan 2025 00:03:53 -0800 Subject: [PATCH 2/6] Add package metadata to `esp-storage` to make documentation build (#2891) --- esp-storage/Cargo.toml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/esp-storage/Cargo.toml b/esp-storage/Cargo.toml index 3cf282d30..56947d9b8 100644 --- a/esp-storage/Cargo.toml +++ b/esp-storage/Cargo.toml @@ -9,6 +9,9 @@ categories = ["embedded", "hardware-support", "no-std"] repository = "https://github.com/esp-rs/esp-storage" license = "MIT OR Apache-2.0" +[package.metadata.docs.rs] +default-target = "riscv32imac-unknown-none-elf" +features = ["esp32c6"] [dependencies] embedded-storage = "0.3.1" From 7319458dd2f3a700cedbf039b9ad12052dc8b958 Mon Sep 17 00:00:00 2001 From: Dominic Fischer <14130965+Dominaezzz@users.noreply.github.com> Date: Mon, 6 Jan 2025 09:16:44 +0000 Subject: [PATCH 3/6] Properly reset descriptors in `DmaRxStreamBuf::prepare` (#2890) Co-authored-by: Dominic Fischer --- esp-hal/CHANGELOG.md | 1 + esp-hal/src/dma/buffers.rs | 14 ++++++-------- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/esp-hal/CHANGELOG.md b/esp-hal/CHANGELOG.md index ab9f122d9..22abde76b 100644 --- a/esp-hal/CHANGELOG.md +++ b/esp-hal/CHANGELOG.md @@ -99,6 +99,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `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) +- `DmaRxStreamBuf` now correctly resets the descriptors the next time it's used (#2890) ### Removed diff --git a/esp-hal/src/dma/buffers.rs b/esp-hal/src/dma/buffers.rs index 78a6a68bf..2bf12111d 100644 --- a/esp-hal/src/dma/buffers.rs +++ b/esp-hal/src/dma/buffers.rs @@ -1137,13 +1137,6 @@ impl DmaRxStreamBuf { return Err(DmaBufError::InsufficientDescriptors); } - // Link up all the descriptors (but not in a circle). - let mut next = null_mut(); - for desc in descriptors.iter_mut().rev() { - desc.next = next; - next = desc; - } - let mut chunks = buffer.chunks_exact_mut(chunk_size); for (desc, chunk) in descriptors.iter_mut().zip(chunks.by_ref()) { desc.buffer = chunk.as_mut_ptr(); @@ -1176,7 +1169,12 @@ unsafe impl DmaRxBuffer for DmaRxStreamBuf { type View = DmaRxStreamBufView; fn prepare(&mut self) -> Preparation { - for desc in self.descriptors.iter_mut() { + // Link up all the descriptors (but not in a circle). + let mut next = null_mut(); + for desc in self.descriptors.iter_mut().rev() { + desc.next = next; + next = desc; + desc.reset_for_rx(); } Preparation { From 39da5337acf03ce18b7ba7d19236b66bd3f9cf02 Mon Sep 17 00:00:00 2001 From: Jesse Braham Date: Mon, 6 Jan 2025 04:11:06 -0800 Subject: [PATCH 4/6] Fix naming violations and clean up some doc comments in UART driver (#2893) * Resolve naming violation for `Parity` enum * Remove unnecessary/redundant information from doc comments * Resolve naming violations for `DataBits` and `StopBits` enums * Update migration guide * Update `CHANGELOG.md` --- esp-hal/CHANGELOG.md | 1 + esp-hal/MIGRATING-0.22.md | 36 ++++++++++++------- esp-hal/src/uart.rs | 74 +++++++++++++++++---------------------- 3 files changed, 58 insertions(+), 53 deletions(-) diff --git a/esp-hal/CHANGELOG.md b/esp-hal/CHANGELOG.md index 22abde76b..36a6e86ae 100644 --- a/esp-hal/CHANGELOG.md +++ b/esp-hal/CHANGELOG.md @@ -91,6 +91,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - I2C: Replaced potential panics with errors. (#2831) - 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) ### Fixed diff --git a/esp-hal/MIGRATING-0.22.md b/esp-hal/MIGRATING-0.22.md index b88c6e302..6e2d8a256 100644 --- a/esp-hal/MIGRATING-0.22.md +++ b/esp-hal/MIGRATING-0.22.md @@ -199,7 +199,7 @@ is enabled. To retrieve the address and size of the initialized external memory, The usage of `esp_alloc::psram_allocator!` remains unchanged. -## embedded-hal 0.2.* is not supported anymore. +## embedded-hal 0.2.\* is not supported anymore. As per https://github.com/rust-embedded/embedded-hal/pull/640, our driver no longer implements traits from `embedded-hal 0.2.x`. Analogs of all traits from the above mentioned version are available in `embedded-hal 1.x.x` @@ -325,15 +325,15 @@ The reexports that were previously part of the prelude are available through oth - `ExtU64` and `RateExtU32` have been moved to `esp_hal::time` - `Clock` and `CpuClock`: `esp_hal::clock::{Clock, CpuClock}` - The following traits need to be individually imported when needed: - - `esp_hal::analog::dac::Instance` - - `esp_hal::gpio::Pin` - - `esp_hal::ledc::channel::ChannelHW` - - `esp_hal::ledc::channel::ChannelIFace` - - `esp_hal::ledc::timer::TimerHW` - - `esp_hal::ledc::timer::TimerIFace` - - `esp_hal::timer::timg::TimerGroupInstance` - - `esp_hal::timer::Timer` - - `esp_hal::interrupt::InterruptConfigurable` + - `esp_hal::analog::dac::Instance` + - `esp_hal::gpio::Pin` + - `esp_hal::ledc::channel::ChannelHW` + - `esp_hal::ledc::channel::ChannelIFace` + - `esp_hal::ledc::timer::TimerHW` + - `esp_hal::ledc::timer::TimerIFace` + - `esp_hal::timer::timg::TimerGroupInstance` + - `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 @@ -343,11 +343,10 @@ The reexports that were previously part of the prelude are available through oth + uart0.set_at_cmd(AtCmdConfig::default().with_cmd_char(b'#')); ``` - ## Crate configuration changes To prevent ambiguity between configurations, we had to change the naming format of configuration -keys. Before, we used `{prefix}_{key}`, which meant that esp-hal and esp-hal-* configuration keys +keys. Before, we used `{prefix}_{key}`, which meant that esp-hal and esp-hal-\* configuration keys were impossible to tell apart. To fix this issue, we are changing the separator from one underscore character to `_CONFIG_`. This also means that users will have to change their `config.toml` configurations to match the new format. @@ -370,3 +369,16 @@ The `Config` struct's setters are now prefixed with `with_`. `parity_none`, `par - .parity_even(); + .with_parity(Parity::Even); ``` + +The `DataBits`, `Parity`, and `StopBits` enum variants are no longer prefixed with the name of the enum. + +e.g.) + +```diff +- DataBits::DataBits8 ++ DataBits::_8 +- Parity::ParityNone ++ Parity::None +- StopBits::Stop1 ++ StopBits::_1 +``` diff --git a/esp-hal/src/uart.rs b/esp-hal/src/uart.rs index 36808e668..108fbc608 100644 --- a/esp-hal/src/uart.rs +++ b/esp-hal/src/uart.rs @@ -322,15 +322,13 @@ impl embedded_io::Error for Error { #[derive(Debug, Default, Clone, Copy, PartialEq, Eq)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] pub enum ClockSource { - /// APB_CLK clock source (default for UART on all the chips except of - /// esp32c6 and esp32h2) + /// APB_CLK clock source #[cfg_attr(not(any(esp32c6, esp32h2, lp_uart)), default)] Apb, /// RC_FAST_CLK clock source (17.5 MHz) #[cfg(not(any(esp32, esp32s2)))] RcFast, - /// XTAL_CLK clock source (default for UART on esp32c6 and esp32h2 and - /// LP_UART) + /// XTAL_CLK clock source #[cfg(not(any(esp32, esp32s2)))] #[cfg_attr(any(esp32c6, esp32h2, lp_uart), default)] Xtal, @@ -353,14 +351,14 @@ const UART_TOUT_THRESH_DEFAULT: u8 = 10; #[cfg_attr(feature = "defmt", derive(defmt::Format))] pub enum DataBits { /// 5 data bits per frame. - DataBits5 = 0, + _5 = 0, /// 6 data bits per frame. - DataBits6 = 1, + _6 = 1, /// 7 data bits per frame. - DataBits7 = 2, - /// 8 data bits per frame (most common). + _7 = 2, + /// 8 data bits per frame. #[default] - DataBits8 = 3, + _8 = 3, } /// Parity check @@ -371,17 +369,16 @@ pub enum DataBits { /// either even or odd. #[derive(Debug, Default, Clone, Copy, PartialEq, Eq)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] -#[allow(clippy::enum_variant_names)] // FIXME: resolve this pub enum Parity { - /// No parity bit is used (most common). + /// No parity bit is used. #[default] - ParityNone, + None, /// Even parity: the parity bit is set to make the total number of /// 1-bits even. - ParityEven, + Even, /// Odd parity: the parity bit is set to make the total number of 1-bits /// odd. - ParityOdd, + Odd, } /// Number of stop bits @@ -391,15 +388,14 @@ pub enum Parity { /// bits. #[derive(Debug, Default, Clone, Copy, PartialEq, Eq)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] -#[allow(clippy::enum_variant_names)] // FIXME: resolve this pub enum StopBits { /// 1 stop bit. #[default] - Stop1 = 1, + _1 = 1, /// 1.5 stop bits. - Stop1P5 = 2, + _1p5 = 2, /// 2 stop bits. - Stop2 = 3, + _2 = 3, } /// UART Configuration @@ -430,17 +426,17 @@ impl Config { fn symbol_length(&self) -> u8 { let mut length: u8 = 1; // start bit length += match self.data_bits { - DataBits::DataBits5 => 5, - DataBits::DataBits6 => 6, - DataBits::DataBits7 => 7, - DataBits::DataBits8 => 8, + DataBits::_5 => 5, + DataBits::_6 => 6, + DataBits::_7 => 7, + DataBits::_8 => 8, }; length += match self.parity { - Parity::ParityNone => 0, + Parity::None => 0, _ => 1, }; length += match self.stop_bits { - StopBits::Stop1 => 1, + StopBits::_1 => 1, _ => 2, // esp-idf also counts 2 bits for settings 1.5 and 2 stop bits }; length @@ -461,10 +457,10 @@ impl Default for Config { } } +/// Configuration for the AT-CMD detection functionality #[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 /// cycles. @@ -2125,16 +2121,16 @@ pub mod lp_uart { } fn change_parity(&mut self, parity: Parity) -> &mut Self { - if parity != Parity::ParityNone { + if parity != Parity::None { self.uart .conf0() .modify(|_, w| w.parity().bit((parity as u8 & 0x1) != 0)); } self.uart.conf0().modify(|_, w| match parity { - Parity::ParityNone => w.parity_en().clear_bit(), - Parity::ParityEven => w.parity_en().set_bit().parity().clear_bit(), - Parity::ParityOdd => w.parity_en().set_bit().parity().set_bit(), + Parity::None => w.parity_en().clear_bit(), + Parity::Even => w.parity_en().set_bit().parity().clear_bit(), + Parity::Odd => w.parity_en().set_bit().parity().set_bit(), }); self @@ -2550,9 +2546,9 @@ impl Info { fn change_parity(&self, parity: Parity) { self.register_block().conf0().modify(|_, w| match parity { - Parity::ParityNone => w.parity_en().clear_bit(), - Parity::ParityEven => w.parity_en().set_bit().parity().clear_bit(), - Parity::ParityOdd => w.parity_en().set_bit().parity().set_bit(), + Parity::None => w.parity_en().clear_bit(), + Parity::Even => w.parity_en().set_bit().parity().clear_bit(), + Parity::Odd => w.parity_en().set_bit().parity().set_bit(), }); } @@ -2560,18 +2556,14 @@ impl Info { #[cfg(esp32)] { // workaround for hardware issue, when UART stop bit set as 2-bit mode. - if stop_bits == StopBits::Stop2 { + if stop_bits == StopBits::_2 { self.register_block() .rs485_conf() - .modify(|_, w| w.dl1_en().bit(stop_bits == StopBits::Stop2)); + .modify(|_, w| w.dl1_en().bit(stop_bits == StopBits::_2)); - self.register_block().conf0().modify(|_, w| { - if stop_bits == StopBits::Stop2 { - unsafe { w.stop_bit_num().bits(1) } - } else { - unsafe { w.stop_bit_num().bits(stop_bits as u8) } - } - }); + self.register_block() + .conf0() + .modify(|_, w| unsafe { w.stop_bit_num().bits(1) }); } } From 5cd0d6f6bf59ab09f27d7a2f743b3414de4419f4 Mon Sep 17 00:00:00 2001 From: Juraj Sadel Date: Tue, 7 Jan 2025 10:03:30 +0100 Subject: [PATCH 5/6] Alter stability note in README and briefly explain unstable feature (#2894) * Alter stability note in README and briefly explain unstable feature * Review comments --- README.md | 2 +- esp-hal/README.md | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index aa4c7c319..7293ee20b 100644 --- a/README.md +++ b/README.md @@ -20,7 +20,7 @@ If you have any questions, comments, or concerns, please [open an issue], [start > [!NOTE] > -> This project is still in the relatively early stages of development, and as such there should be no expectation of API stability. A significant number of peripherals currently have drivers implemented but have varying levels of functionality. For most tasks, this should be usable already, however some more advanced or uncommon features may not yet be implemented. +> This repository includes crates that are at various stages of maturity and stability. While many functionalities have already been implemented and are usable for most tasks, certain advanced or less common features may still be under development. Each crate may offer different levels of functionality and guarantees. [esp-lp-hal]: https://github.com/esp-rs/esp-hal/tree/main/esp-lp-hal [esp-idf-svc]: https://github.com/esp-rs/esp-idf-svc diff --git a/esp-hal/README.md b/esp-hal/README.md index 5a5eaf844..09c54ced0 100644 --- a/esp-hal/README.md +++ b/esp-hal/README.md @@ -46,6 +46,10 @@ For help getting started with this HAL, please refer to [The Rust on ESP Book] a [s2-trm]: https://www.espressif.com/sites/default/files/documentation/esp32-s2_technical_reference_manual_en.pdf [s3-trm]: https://www.espressif.com/sites/default/files/documentation/esp32-s3_technical_reference_manual_en.pdf +## `unstable` feature + +The stable feature set is designed to remain consistent and reliable. Other parts guarded by the `unstable` feature, however, are still under active development and may undergo breaking changes and are disabled by default. + ## Minimum Supported Rust Version (MSRV) This crate is guaranteed to compile on stable Rust 1.83 and up. It _might_ From 2a4e58a23009882b438de558d4f393bef9e90501 Mon Sep 17 00:00:00 2001 From: Jesse Braham Date: Tue, 7 Jan 2025 02:02:29 -0800 Subject: [PATCH 6/6] 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 | 1 + esp-hal/MIGRATING-0.22.md | 4 +++ esp-hal/src/uart.rs | 58 +++++++++++------------------------- hil-test/tests/uart_tx_rx.rs | 9 +++++- 4 files changed, 30 insertions(+), 42 deletions(-) diff --git a/esp-hal/CHANGELOG.md b/esp-hal/CHANGELOG.md index 36a6e86ae..ed92d709a 100644 --- a/esp-hal/CHANGELOG.md +++ b/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 diff --git a/esp-hal/MIGRATING-0.22.md b/esp-hal/MIGRATING-0.22.md index 6e2d8a256..cf084c4a1 100644 --- a/esp-hal/MIGRATING-0.22.md +++ b/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. diff --git a/esp-hal/src/uart.rs b/esp-hal/src/uart.rs index 108fbc608..7ca3ed6cd 100644 --- a/esp-hal/src/uart.rs +++ b/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 { 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 - pub fn drain_fifo(&mut self, buf: &mut [u8]) -> usize { + /// returns the number of read bytes without blocking. + 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 - pub fn read_bytes(&mut self, buf: &mut [u8]) -> Result<(), Error> { + /// Read all available bytes from the RX FIFO into the provided buffer and + /// 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 { diff --git a/hil-test/tests/uart_tx_rx.rs b/hil-test/tests/uart_tx_rx.rs index 10840df1c..fb20e0b26 100644 --- a/hil-test/tests/uart_tx_rx.rs +++ b/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); } }