Stable driver analysis: UART

[MabezDev]

As part of #2491, which has more details on driver analysis.

esp-hal API-GUIDELINE omissions

• RX/TX pin assignment in UART driver is done in the constructor instead of builder functions #2632
• Missing Drop implementation(s) for UART driver #2633
• UART driver's set_rx_fifo_full_threshold should use cfg_if! instead of multiple #[cfg]s #2634
• embassy_embedded_hal::SetConfig needs to be gated with the unstable feature in UART driver #2635
• UART driver should follow naming conventions in order to be consistent across drivers #2636
• ConfigError: should be non-exhaustive #2724
• ClockSource: should this be non-exhaustive? #2735
• Uart constructor should take pins via the builder lite pattern #2734
• AtCmdConfig should be a part of Config #2732
• AtCmdConfig should use the builder-lite pattern instead of that constructor #2726
• AtCmdConfig should be #[non_exaustive] #2733
• UART Config has setters manually implemented, while also derives BuilderLite #2751
• UART: mem_clk should be disabled after all driver have been dropped #2759

---

Rust API guideline ommissions

• UART: Enum variants should use UpperCamelCase. This is not true for StopBits. #2637
  • Also, shall we repeat the enum name in the variants?
    • JB: No, this is considered bad style :)
    • Created: Avoid repeating the enum name in the enum variants #2716
• C-COMMON-TRAITS #2810
• C-QUESTION-MARK: Examples use ?, not try!, not unwrap #2811
• C-FAILURE: Function docs include error, panic, and safety considerations #2812

---

Hardware feature omissions

• Uart: problems after added check of err_wr_mask in config #1580
• Add RS485 support to UART driver #2638
• Add IrDA support to UART driver #2639
• ESP32-S2: UART Stop bit can be 3 bits long #2640
• Allow enabling/disabling cts/rts via Config #2731

---

Mode/features of the driver that are missing/lacking

• UART driver does not support autobaud functionality #2641
• UART driver does not support DMA mode #2642
• Wake-up (e.g. light sleep) #2813
• Not all UART hardware interrupts are supported/handled #2643
• UART driver does not support configuring SW/HW flow control #2644
• Should split halves have individual constructors? #2727
• Should split halves be allowed to be in different Modes? #2728
• Missing read_byte that doesn't use nb #2729
• Should uart use nb types and implementations? #2730
• UART: inverting data using UART_RXD_INV/UART_TXD_INV/UART_DTR_INV/... is missing #2749
• UART: FIFO size configuration is missing #2750
• UART: transmitting BRK is not supported #2752
• UART: detecting Break conditions is not supported #2753
• UART: Loopback test mode is not supported #2761

Parts of the driver we should mark as unstable

• Setting the clock source in the Config, we need to resolve RFC: Provide consistent APIs for configuring clock sources for peripheral drivers #1668 before committing to this.
• lp_uart
• anything AT command related
• split tx/rx halves - context: UART: Unexpected results when applying a config while data is being clocked out #2743 (comment)

Other

• TODO on set_rx_timeout (low severity)
• fill_fifo seems redundant vs read_bytes #2725
• Misc UART module-level documentation issues #2744
• UART: enums have opinionated doc comments ("most common") #2758
• UART: ClockSource unnecessarily documents default variants for various MCUs #2757
• UART: cancellation semantics of async operations is unclear #2762

[jessebraham]

I think the wake-up functionality is orthogonal to the operation of the driver itself, and can be handled at a later date. This should be purely additive as far as I can tell, however would be worth getting another set of eyes on it during a subsequent round of analysis.

[bjoernQ]

This should be purely additive as far as I can tell, however would be worth getting another set of eyes on it during a subsequent round of analysis.

If not, we should change that 👍