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

@@ -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
 

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
+```

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,28 +2546,24 @@ 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(),
         });
     }
 
     fn change_stop_bits(&self, stop_bits: StopBits) {
         #[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));
-
-                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) }
-                    }
-                });
+                    .modify(|_, w| w.dl1_en().bit(stop_bits == StopBits::_2));
+
+                self.register_block()
+                    .conf0()
+                    .modify(|_, w| unsafe { w.stop_bit_num().bits(1) });
             }
         }