Skip to content

Commit

Permalink
Uart tweaks (#3141)
Browse files Browse the repository at this point in the history
* Impose some order on UART config impl blocks

* Only pass the relevant config half

* Remove incorrect statements from documentation

* Move error documentation to enum variants

* Changelog

* Undo config split related changes

* Mark rx fifo threshold config unstable

* Remove explicit numeric values

* Add note
  • Loading branch information
bugadani authored Feb 17, 2025
1 parent 07463bf commit acbb983
Show file tree
Hide file tree
Showing 2 changed files with 96 additions and 99 deletions.
2 changes: 1 addition & 1 deletion esp-hal/MIGRATING-0.23.md
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ added for `embedded-io` errors associated to the unsplit `Uart` driver. On `Uart
or `UartTx`) TX-related trait methods return `IoError::Tx(TxError)`, while RX-related methods return
`IoError::Rx(RxError)`.

### UART halves have their configuration split too
### UART halves have their configuration split, too

`Uart::Config` structure now contains separate `RxConfig` and `TxConfig`:

Expand Down
193 changes: 95 additions & 98 deletions esp-hal/src/uart.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,14 +175,14 @@ const UART_TOUT_THRESH_DEFAULT: u8 = 10;
#[cfg_attr(feature = "defmt", derive(defmt::Format))]
pub enum DataBits {
/// 5 data bits per frame.
_5 = 0,
_5,
/// 6 data bits per frame.
_6 = 1,
_6,
/// 7 data bits per frame.
_7 = 2,
_7,
/// 8 data bits per frame.
#[default]
_8 = 3,
_8,
}

/// Parity check
Expand Down Expand Up @@ -215,11 +215,11 @@ pub enum Parity {
pub enum StopBits {
/// 1 stop bit.
#[default]
_1 = 1,
_1,
/// 1.5 stop bits.
_1p5 = 2,
_1p5,
/// 2 stop bits.
_2 = 3,
_2,
}

/// Defines how strictly the requested baud rate must be met.
Expand Down Expand Up @@ -263,17 +263,6 @@ pub struct Config {
tx: TxConfig,
}

/// UART Receive part configuration.
#[derive(Debug, Clone, Copy, procmacros::BuilderLite)]
#[cfg_attr(feature = "defmt", derive(defmt::Format))]
#[non_exhaustive]
pub struct RxConfig {
/// Threshold level at which the RX FIFO is considered full.
fifo_full_threshold: u16,
/// Optional timeout value for RX operations.
timeout: Option<u8>,
}

impl Default for Config {
fn default() -> Config {
Config {
Expand All @@ -289,43 +278,7 @@ impl Default for Config {
}
}

/// UART Transmit part configuration.
#[derive(Debug, Clone, Copy, Default, procmacros::BuilderLite)]
#[cfg_attr(feature = "defmt", derive(defmt::Format))]
#[non_exhaustive]
pub struct TxConfig {}

impl Default for RxConfig {
fn default() -> RxConfig {
RxConfig {
fifo_full_threshold: UART_FULL_THRESH_DEFAULT,
timeout: Some(UART_TOUT_THRESH_DEFAULT),
}
}
}

impl Config {
/// Calculates the total symbol length in bits based on the configured
/// data bits, parity, and stop bits.
fn symbol_length(&self) -> u8 {
let mut length: u8 = 1; // start bit
length += match self.data_bits {
DataBits::_5 => 5,
DataBits::_6 => 6,
DataBits::_7 => 7,
DataBits::_8 => 8,
};
length += match self.parity {
Parity::None => 0,
_ => 1,
};
length += match self.stop_bits {
StopBits::_1 => 1,
_ => 2, // esp-idf also counts 2 bits for settings 1.5 and 2 stop bits
};
length
}

fn validate(&self) -> Result<(), ConfigError> {
if let BaudrateTolerance::ErrorPercent(percentage) = self.baudrate_tolerance {
assert!(percentage > 0 && percentage <= 100);
Expand All @@ -339,6 +292,33 @@ impl Config {
}
}

/// UART Receive part configuration.
#[derive(Debug, Clone, Copy, procmacros::BuilderLite)]
#[cfg_attr(feature = "defmt", derive(defmt::Format))]
#[non_exhaustive]
pub struct RxConfig {
/// Threshold level at which the RX FIFO is considered full.
#[cfg_attr(not(feature = "unstable"), builder_lite(skip))]
fifo_full_threshold: u16,
/// Optional timeout value for RX operations.
timeout: Option<u8>,
}

impl Default for RxConfig {
fn default() -> RxConfig {
RxConfig {
fifo_full_threshold: UART_FULL_THRESH_DEFAULT,
timeout: Some(UART_TOUT_THRESH_DEFAULT),
}
}
}

/// UART Transmit part configuration.
#[derive(Debug, Clone, Copy, Default, procmacros::BuilderLite)]
#[cfg_attr(feature = "defmt", derive(defmt::Format))]
#[non_exhaustive]
pub struct TxConfig {}

/// Configuration for the AT-CMD detection functionality
#[derive(Clone, Copy, PartialEq, Eq, Hash, Debug, procmacros::BuilderLite)]
#[cfg_attr(feature = "defmt", derive(defmt::Format))]
Expand Down Expand Up @@ -464,11 +444,25 @@ pub enum ConfigError {
#[cfg_attr(docsrs, doc(cfg(feature = "unstable")))]
UnachievableBaudrate,
/// The requested baud rate is not supported.
///
/// This error is returned if:
/// * the baud rate exceeds 5MBaud or is equal to zero.
/// * the user has specified an exact baud rate or with some percentage of
/// deviation to the desired value, and the driver cannot reach this
/// speed.
UnsupportedBaudrate,
/// The requested timeout is not supported.
/// The requested timeout exceeds the maximum value (
#[cfg_attr(esp32, doc = "127")]
#[cfg_attr(not(esp32), doc = "1023")]
/// ).
UnsupportedTimeout,
/// The requested FIFO threshold is not supported.
UnsupportedFifoThreshold,
/// The requested RX FIFO threshold exceeds the maximum value (
#[cfg_attr(esp32, doc = "127")]
#[cfg_attr(any(esp32c6, esp32h2), doc = "255")]
#[cfg_attr(any(esp32c2, esp32c3, esp32s2), doc = "511")]
#[cfg_attr(esp32s3, doc = "1023")]
/// ).
UnsupportedRxFifoThreshold,
}

impl core::error::Error for ConfigError {}
Expand All @@ -484,8 +478,8 @@ impl core::fmt::Display for ConfigError {
write!(f, "The requested baud rate is not supported")
}
ConfigError::UnsupportedTimeout => write!(f, "The requested timeout is not supported"),
ConfigError::UnsupportedFifoThreshold => {
write!(f, "The requested FIFO threshold is not supported")
ConfigError::UnsupportedRxFifoThreshold => {
write!(f, "The requested RX FIFO threshold is not supported")
}
}
}
Expand Down Expand Up @@ -563,7 +557,8 @@ where

/// Change the configuration.
///
/// Note that this also changes the configuration of the RX half.
/// This function returns a [`ConfigError`] if the configuration is not
/// supported by the hardware.
#[instability::unstable]
pub fn apply_config(&mut self, _config: &Config) -> Result<(), ConfigError> {
// Nothing to do so far.
Expand Down Expand Up @@ -766,31 +761,18 @@ where

/// Change the configuration.
///
/// Note that this also changes the configuration of the TX half.
///
/// # Errors
///
/// [`ConfigError::UnsupportedFifoThreshold`] will be returned under the
/// following conditions:
/// * if the RX FIFO threshold passed in `Config` exceeds the maximum
/// value (
#[cfg_attr(esp32, doc = "0x7F")]
#[cfg_attr(any(esp32c6, esp32h2), doc = "0xFF")]
#[cfg_attr(any(esp32c3, esp32c2, esp32s2), doc = "0x1FF")]
#[cfg_attr(esp32h2, doc = "0x3FF")]
/// )
/// * if the passed timeout exceeds the maximum value (
#[cfg_attr(esp32, doc = "0x7F")]
#[cfg_attr(not(esp32), doc = "0x3FF")]
/// ).
/// This function returns a [`ConfigError`] if the configuration is not
/// supported by the hardware.
#[instability::unstable]
pub fn apply_config(&mut self, config: &Config) -> Result<(), ConfigError> {
self.uart
.info()
.set_rx_fifo_full_threshold(config.rx.fifo_full_threshold)?;
self.uart
.info()
.set_rx_timeout(config.rx.timeout, config.symbol_length())?;
.set_rx_timeout(config.rx.timeout, self.uart.info().current_symbol_length())?;

self.uart.info().rxfifo_reset();
Ok(())
Expand Down Expand Up @@ -1217,18 +1199,14 @@ where
///
/// # Errors
///
/// [`ConfigError::UnsupportedFifoThreshold`] will be returned in the cases
/// described in [`UartRx::apply_config`].
/// [`ConfigError::UnsupportedBaudrate`] will be returned under the
/// following conditions:
/// * if baud rate passed in config exceeds 5MBaud or is equal to zero.
/// * if the user has specified in the configuration that they want
/// baudrate to correspond exactly or with some percentage of deviation
/// to the desired value, and the driver cannot reach this speed
/// This function returns a [`ConfigError`] if the configuration is not
/// supported by the hardware.
pub fn apply_config(&mut self, config: &Config) -> Result<(), ConfigError> {
// Must apply the common settings first, as `rx.apply_config` reads back symbol
// size.
self.rx.uart.info().apply_config(config)?;
self.rx.apply_config(config)?;
self.tx.apply_config(config)?;
self.rx.uart.info().apply_config(config)?;
Ok(())
}

Expand All @@ -1255,6 +1233,10 @@ where

self.apply_config(&config)?;

// Reset Tx/Rx FIFOs
self.rx.uart.info().rxfifo_reset();
self.rx.uart.info().txfifo_reset();

// Don't wait after transmissions by default,
// so that bytes written to TX FIFO are always immediately transmitted.
self.regs()
Expand Down Expand Up @@ -2170,15 +2152,14 @@ pub mod lp_uart {
.modify(|_, w| unsafe { w.bit_num().bits(data_bits as u8) });

self.update();

self
}

fn change_stop_bits(&mut self, stop_bits: StopBits) -> &mut Self {
self.uart
.register_block()
.conf0()
.modify(|_, w| unsafe { w.stop_bit_num().bits(stop_bits as u8) });
.modify(|_, w| unsafe { w.stop_bit_num().bits(stop_bits as u8 + 1) });

self.update();
self
Expand Down Expand Up @@ -2340,10 +2321,6 @@ impl Info {
self.change_parity(config.parity);
self.change_stop_bits(config.stop_bits);

// Reset Tx/Rx FIFOs
self.rxfifo_reset();
self.txfifo_reset();

Ok(())
}

Expand Down Expand Up @@ -2432,8 +2409,8 @@ impl Info {
///
/// # Errors
///
/// [`Err(ConfigError::UnsupportedFifoThreshold)`][ConfigError::UnsupportedFifoThreshold] if provided value exceeds maximum value
/// for SOC :
/// [ConfigError::UnsupportedFifoThreshold] if provided value exceeds
/// maximum value for SOC:
/// - `esp32` **0x7F**
/// - `esp32c6`, `esp32h2` **0xFF**
/// - `esp32c3`, `esp32c2`, `esp32s2` **0x1FF**
Expand All @@ -2452,7 +2429,7 @@ impl Info {
}

if threshold > MAX_THRHD {
return Err(ConfigError::UnsupportedFifoThreshold);
return Err(ConfigError::UnsupportedRxFifoThreshold);
}

self.regs()
Expand All @@ -2470,12 +2447,11 @@ impl Info {
///
/// # Errors
///
/// [`Err(ConfigError::UnsupportedTimeout)`][ConfigError::UnsupportedTimeout] if the provided value exceeds
/// the maximum value for SOC :
/// [ConfigError::UnsupportedTimeout] if the provided value exceeds
/// the maximum value for SOC:
/// - `esp32`: Symbol size is fixed to 8, do not pass a value > **0x7F**.
/// - `esp32c2`, `esp32c3`, `esp32c6`, `esp32h2`, esp32s2`, esp32s3`: The
/// value you pass times the symbol size must be <= **0x3FF**
// TODO: the above should be a per-chip doc line.
fn set_rx_timeout(&self, timeout: Option<u8>, _symbol_len: u8) -> Result<(), ConfigError> {
cfg_if::cfg_if! {
if #[cfg(esp32)] {
Expand Down Expand Up @@ -2646,7 +2622,7 @@ impl Info {
#[cfg(not(esp32))]
self.regs()
.conf0()
.modify(|_, w| unsafe { w.stop_bit_num().bits(stop_bits as u8) });
.modify(|_, w| unsafe { w.stop_bit_num().bits(stop_bits as u8 + 1) });
}

fn rxfifo_reset(&self) {
Expand Down Expand Up @@ -2719,6 +2695,27 @@ impl Info {

Ok(())
}

fn current_symbol_length(&self) -> u8 {
let conf0 = self.regs().conf0().read();
let data_bits = conf0.bit_num().bits() + 5; // 5 data bits are encoded as variant 0
let parity = conf0.parity_en().bit() as u8;
let mut stop_bits = conf0.stop_bit_num().bits();

match stop_bits {
1 => {
// workaround for hardware issue, when UART stop bit set as 2-bit mode.
#[cfg(esp32)]
if self.regs().rs485_conf().read().dl1_en().bit_is_set() {
stop_bits = 2;
}
}
// esp-idf also counts 2 bits for settings 1.5 and 2 stop bits
_ => stop_bits = 2,
}

1 + data_bits + parity + stop_bits
}
}

impl PartialEq for Info {
Expand Down

0 comments on commit acbb983

Please sign in to comment.