GetLitFam/esp-hal
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Fix naming violations and clean up some doc comments in UART driver (#2893)

Browse Source 
* 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`
This commit is contained in:
Jesse Braham 2025-01-06 04:11:06 -08:00 committed by GitHub
parent 7319458dd2
commit 39da5337ac
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 58 additions and 53 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
esp-hal/CHANGELOG.md
View File

@ -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) - I2C: Replaced potential panics with errors. (#2831)
- UART: Make `AtCmdConfig` and `ConfigError` non-exhaustive (#2851) - UART: Make `AtCmdConfig` and `ConfigError` non-exhaustive (#2851)
- UART: Make `AtCmdConfig` use builder-lite pattern (#2851) - UART: Make `AtCmdConfig` use builder-lite pattern (#2851)
- UART: Fix naming violations for `DataBits`, `Parity`, and `StopBits` enum variants (#2893)
### Fixed ### Fixed

36
esp-hal/MIGRATING-0.22.md
View File

@ -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. 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`. 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` 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` - `ExtU64` and `RateExtU32` have been moved to `esp_hal::time`
- `Clock` and `CpuClock`: `esp_hal::clock::{Clock, CpuClock}` - `Clock` and `CpuClock`: `esp_hal::clock::{Clock, CpuClock}`
- The following traits need to be individually imported when needed: - The following traits need to be individually imported when needed:
- `esp_hal::analog::dac::Instance` - `esp_hal::analog::dac::Instance`
- `esp_hal::gpio::Pin` - `esp_hal::gpio::Pin`
- `esp_hal::ledc::channel::ChannelHW` - `esp_hal::ledc::channel::ChannelHW`
- `esp_hal::ledc::channel::ChannelIFace` - `esp_hal::ledc::channel::ChannelIFace`
- `esp_hal::ledc::timer::TimerHW` - `esp_hal::ledc::timer::TimerHW`
- `esp_hal::ledc::timer::TimerIFace` - `esp_hal::ledc::timer::TimerIFace`
- `esp_hal::timer::timg::TimerGroupInstance` - `esp_hal::timer::timg::TimerGroupInstance`
- `esp_hal::timer::Timer` - `esp_hal::timer::Timer`
- `esp_hal::interrupt::InterruptConfigurable` - `esp_hal::interrupt::InterruptConfigurable`
- The `entry` macro can be imported as `esp_hal::entry`, while other macros are found under `esp_hal::macros` - 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 ## `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'#')); + uart0.set_at_cmd(AtCmdConfig::default().with_cmd_char(b'#'));
``` ```
## Crate configuration changes ## Crate configuration changes
To prevent ambiguity between configurations, we had to change the naming format of configuration 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 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` character to `_CONFIG_`. This also means that users will have to change their `config.toml`
configurations to match the new format. 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(); - .parity_even();
+ .with_parity(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
```

74
esp-hal/src/uart.rs
View File

@ -322,15 +322,13 @@ impl embedded_io::Error for Error {
#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)] #[derive(Debug, Default, Clone, Copy, PartialEq, Eq)]
#[cfg_attr(feature = "defmt", derive(defmt::Format))] #[cfg_attr(feature = "defmt", derive(defmt::Format))]
pub enum ClockSource { pub enum ClockSource {
/// APB_CLK clock source (default for UART on all the chips except of /// APB_CLK clock source
/// esp32c6 and esp32h2)
#[cfg_attr(not(any(esp32c6, esp32h2, lp_uart)), default)] #[cfg_attr(not(any(esp32c6, esp32h2, lp_uart)), default)]
Apb, Apb,
/// RC_FAST_CLK clock source (17.5 MHz) /// RC_FAST_CLK clock source (17.5 MHz)
#[cfg(not(any(esp32, esp32s2)))] #[cfg(not(any(esp32, esp32s2)))]
RcFast, RcFast,
/// XTAL_CLK clock source (default for UART on esp32c6 and esp32h2 and /// XTAL_CLK clock source
/// LP_UART)
#[cfg(not(any(esp32, esp32s2)))] #[cfg(not(any(esp32, esp32s2)))]
#[cfg_attr(any(esp32c6, esp32h2, lp_uart), default)] #[cfg_attr(any(esp32c6, esp32h2, lp_uart), default)]
Xtal, Xtal,
@ -353,14 +351,14 @@ const UART_TOUT_THRESH_DEFAULT: u8 = 10;
#[cfg_attr(feature = "defmt", derive(defmt::Format))] #[cfg_attr(feature = "defmt", derive(defmt::Format))]
pub enum DataBits { pub enum DataBits {
/// 5 data bits per frame. /// 5 data bits per frame.
DataBits5 = 0, _5 = 0,
/// 6 data bits per frame. /// 6 data bits per frame.
DataBits6 = 1, _6 = 1,
/// 7 data bits per frame. /// 7 data bits per frame.
DataBits7 = 2, _7 = 2,
/// 8 data bits per frame (most common). /// 8 data bits per frame.
#[default] #[default]
DataBits8 = 3, _8 = 3,
} }
/// Parity check /// Parity check
@ -371,17 +369,16 @@ pub enum DataBits {
/// either even or odd. /// either even or odd.
#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)] #[derive(Debug, Default, Clone, Copy, PartialEq, Eq)]
#[cfg_attr(feature = "defmt", derive(defmt::Format))] #[cfg_attr(feature = "defmt", derive(defmt::Format))]
#[allow(clippy::enum_variant_names)] // FIXME: resolve this
pub enum Parity { pub enum Parity {
/// No parity bit is used (most common). /// No parity bit is used.
#[default] #[default]
ParityNone, None,
/// Even parity: the parity bit is set to make the total number of /// Even parity: the parity bit is set to make the total number of
/// 1-bits even. /// 1-bits even.
ParityEven, Even,
/// Odd parity: the parity bit is set to make the total number of 1-bits /// Odd parity: the parity bit is set to make the total number of 1-bits
/// odd. /// odd.
ParityOdd, Odd,
} }
/// Number of stop bits /// Number of stop bits
@ -391,15 +388,14 @@ pub enum Parity {
/// bits. /// bits.
#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)] #[derive(Debug, Default, Clone, Copy, PartialEq, Eq)]
#[cfg_attr(feature = "defmt", derive(defmt::Format))] #[cfg_attr(feature = "defmt", derive(defmt::Format))]
#[allow(clippy::enum_variant_names)] // FIXME: resolve this
pub enum StopBits { pub enum StopBits {
/// 1 stop bit. /// 1 stop bit.
#[default] #[default]
Stop1 = 1, _1 = 1,
/// 1.5 stop bits. /// 1.5 stop bits.
Stop1P5 = 2, _1p5 = 2,
/// 2 stop bits. /// 2 stop bits.
Stop2 = 3, _2 = 3,
} }
/// UART Configuration /// UART Configuration
@ -430,17 +426,17 @@ impl Config {
fn symbol_length(&self) -> u8 { fn symbol_length(&self) -> u8 {
let mut length: u8 = 1; // start bit let mut length: u8 = 1; // start bit
length += match self.data_bits { length += match self.data_bits {
DataBits::DataBits5 => 5, DataBits::_5 => 5,
DataBits::DataBits6 => 6, DataBits::_6 => 6,
DataBits::DataBits7 => 7, DataBits::_7 => 7,
DataBits::DataBits8 => 8, DataBits::_8 => 8,
}; };
length += match self.parity { length += match self.parity {
Parity::ParityNone => 0, Parity::None => 0,
_ => 1, _ => 1,
}; };
length += match self.stop_bits { 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 _ => 2, // esp-idf also counts 2 bits for settings 1.5 and 2 stop bits
}; };
length 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)] #[derive(Clone, Copy, PartialEq, Eq, Hash, Debug, procmacros::BuilderLite)]
#[cfg_attr(feature = "defmt", derive(defmt::Format))] #[cfg_attr(feature = "defmt", derive(defmt::Format))]
#[non_exhaustive] #[non_exhaustive]
/// Configuration for the AT-CMD detection functionality
pub struct AtCmdConfig { pub struct AtCmdConfig {
/// Optional idle time before the AT command detection begins, in clock /// Optional idle time before the AT command detection begins, in clock
/// cycles. /// cycles.
@ -2125,16 +2121,16 @@ pub mod lp_uart {
} }
fn change_parity(&mut self, parity: Parity) -> &mut Self { fn change_parity(&mut self, parity: Parity) -> &mut Self {
if parity != Parity::ParityNone { if parity != Parity::None {
self.uart self.uart
.conf0() .conf0()
.modify(|_, w| w.parity().bit((parity as u8 & 0x1) != 0)); .modify(|_, w| w.parity().bit((parity as u8 & 0x1) != 0));
} }
self.uart.conf0().modify(|_, w| match parity { self.uart.conf0().modify(|_, w| match parity {
Parity::ParityNone => w.parity_en().clear_bit(), Parity::None => w.parity_en().clear_bit(),
Parity::ParityEven => w.parity_en().set_bit().parity().clear_bit(), Parity::Even => w.parity_en().set_bit().parity().clear_bit(),
Parity::ParityOdd => w.parity_en().set_bit().parity().set_bit(), Parity::Odd => w.parity_en().set_bit().parity().set_bit(),
}); });
self self
@ -2550,9 +2546,9 @@ impl Info {
fn change_parity(&self, parity: Parity) { fn change_parity(&self, parity: Parity) {
self.register_block().conf0().modify(|_, w| match parity { self.register_block().conf0().modify(|_, w| match parity {
Parity::ParityNone => w.parity_en().clear_bit(), Parity::None => w.parity_en().clear_bit(),
Parity::ParityEven => w.parity_en().set_bit().parity().clear_bit(), Parity::Even => w.parity_en().set_bit().parity().clear_bit(),
Parity::ParityOdd => w.parity_en().set_bit().parity().set_bit(), Parity::Odd => w.parity_en().set_bit().parity().set_bit(),
}); });
} }
@ -2560,18 +2556,14 @@ impl Info {
#[cfg(esp32)] #[cfg(esp32)]
{ {
// workaround for hardware issue, when UART stop bit set as 2-bit mode. // 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() self.register_block()
.rs485_conf() .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| { self.register_block()
if stop_bits == StopBits::Stop2 { .conf0()
unsafe { w.stop_bit_num().bits(1) } .modify(|_, w| unsafe { w.stop_bit_num().bits(1) });
} else {
unsafe { w.stop_bit_num().bits(stop_bits as u8) }
}
});
} }
} }