Skip to content

Commit

Permalink
Avoid using nb::Result as a return type (#2882)
Browse files Browse the repository at this point in the history
* feat: Avoid using nb::Result in spi

* feat: Avoid using nb::Result in hmac

* feat: Avoid using nb::Result in sha

* feat: Avoid using nb::Result in usb_serial_jtag

* feat: Avoid using nb::Result in adc::read_oneshot

* feat: Avoid using nb::Result in rsa::ready

* feat: Avoid using nb::Result in timer::wait

* feat: Avoid using nb in uart and twai. Udpate examples and tests to avoid using block!

* feat: Block on sha calls, remove Option<> results

* feat: Block on hmac calls, remove Option<> results

* fix: Clippy lints

* feat: Block on spi calls, remove Option<> results

* feat: Block on timer calls, remove Option<> results

* feat: Block on twai calls, remove Option<> results

* docs: Fix wait docstring

* feat: Remove embedded_hal_nb traits

* feat: Block on uart calls, remove Option<> results

* feat: Remove nb stuff from usb_serial_jtag

* feat: Block on rsa calls, remove Option<> results

* feat: Clippy lints

* feat: Make read_bytes return how many bytes it read from the fifo and fix docs/tests

* fix: run_test_periodic_timer test

* feat: Only remove nb from stabilizing drivers

* feat: Remove unused functions

* feat: Remove emnbedded-hal-nb

* test: Adapt tests

* docs: Update changelog and migration

* test: Adapt tests

* docs: Update migration guide

* docs: Update migration

Co-authored-by: Dániel Buga <[email protected]>

* docs: Update changelog and migration guide

* feat: Make `write_bytes` return an Result

---------

Co-authored-by: Dániel Buga <[email protected]>
SergioGasquez and bugadani authored Jan 17, 2025
1 parent 48b52e0 commit c88dbef
Showing 12 changed files with 68 additions and 226 deletions.
5 changes: 5 additions & 0 deletions esp-hal/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -13,6 +13,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- RMT: `TxChannelConfig` and `RxChannelConfig` now support the builder-lite pattern (#2978)
- RMT: Some fields of `TxChannelConfig` and `RxChannelConfig` are now `gpio::Level`-valued instead of `bool` (#2989)
- RMT: The `PulseCode` trait now uses `gpio::Level` to specify output levels instead of `bool` (#2989)
- Uart `write_bytes` and `read_bytes` are now blocking and return the number of bytes written/read (#2882)
- Uart `read_bytes` is now blocking returns the number of bytes read (#2882)
- Uart `flush` is now blocking (#2882)
- Removed `embedded-hal-nb` traits (#2882)
- `timer::wait` is now blocking (#2882)

### Fixed

1 change: 0 additions & 1 deletion esp-hal/Cargo.toml
Original file line number Diff line number Diff line change
@@ -35,7 +35,6 @@ embassy-usb-synopsys-otg = { version = "0.2.0", optional = true }
embedded-can = { version = "0.4.1", optional = true }
embedded-hal = "1.0.0"
embedded-hal-async = "1.0.0"
embedded-hal-nb = "1.0.0"
embedded-io = { version = "0.6.1", optional = true }
embedded-io-async = { version = "0.6.1", optional = true }
enumset = "1.1.5"
2 changes: 2 additions & 0 deletions esp-hal/MIGRATING-0.22.md
Original file line number Diff line number Diff line change
@@ -462,6 +462,8 @@ e.g.
+ let cnt = serial.read_bytes(&mut buff);
```



## Spi `with_miso` has been split

Previously, `with_miso` set up the provided pin as an input and output, which was necessary for half duplex.
25 changes: 24 additions & 1 deletion esp-hal/MIGRATING-0.23.md
Original file line number Diff line number Diff line change
@@ -66,7 +66,30 @@ The more descriptive `gpio::Level` enum is now used to specify output levels of

```diff
+ use esp_hal::gpio::Level;
+
+
- let code = PulseCode::new(true, 200, false, 50);
+ let code = PulseCode::new(Level::High, 200, Level::Low, 50);
```

## UART changes

Uart `write_bytes` is now blocking and return the number of bytes written. `read_bytes` will block until it fills the provided buffer with received bytes, use `read_buffered_bytes` to read the available bytes without blocking.

e.g.

```diff
- uart.write(0x42).ok();
- let read = block!(ctx.uart.read());
+ let data: [u8; 1] = [0x42];
+ uart.write_bytes(&data).unwrap();
+ let mut byte = [0u8; 1];
+ uart.read_bytes(&mut byte);
```

## `timer::wait` is now blocking

```diff
periodic.start(100.millis()).unwrap();
- nb::block!(periodic.wait()).unwrap();
+ periodic.wait();
```
17 changes: 0 additions & 17 deletions esp-hal/src/spi/master.rs
Original file line number Diff line number Diff line change
@@ -2137,30 +2137,13 @@ mod dma {
mod ehal1 {
use embedded_hal::spi::SpiBus;
use embedded_hal_async::spi::SpiBus as SpiBusAsync;
use embedded_hal_nb::spi::FullDuplex;

use super::*;

impl<Dm> embedded_hal::spi::ErrorType for Spi<'_, Dm> {
type Error = Error;
}

impl<Dm> FullDuplex for Spi<'_, Dm>
where
Dm: DriverMode,
{
fn read(&mut self) -> nb::Result<u8, Self::Error> {
let mut buffer = [0u8; 1];
self.driver().read_bytes(&mut buffer)?;
Ok(buffer[0])
}

fn write(&mut self, word: u8) -> nb::Result<(), Self::Error> {
self.driver().write_bytes(&[word])?;
Ok(())
}
}

impl<Dm> SpiBus for Spi<'_, Dm>
where
Dm: DriverMode,
15 changes: 5 additions & 10 deletions esp-hal/src/timer/mod.rs
Original file line number Diff line number Diff line change
@@ -35,7 +35,7 @@
//!
//! periodic.start(1.secs());
//! loop {
//! nb::block!(periodic.wait());
//! periodic.wait();
//! }
//! # }
//! ```
@@ -310,15 +310,10 @@ where
Ok(())
}

/// "Wait" until the count down finishes without blocking.
pub fn wait(&mut self) -> nb::Result<(), void::Void> {
if self.inner.is_interrupt_set() {
self.inner.clear_interrupt();

Ok(())
} else {
Err(nb::Error::WouldBlock)
}
/// "Wait", by blocking, until the count down finishes.
pub fn wait(&mut self) {
while !self.inner.is_interrupt_set() {}
self.inner.clear_interrupt();
}

/// Tries to cancel the active count down.
116 changes: 13 additions & 103 deletions esp-hal/src/uart.rs
Original file line number Diff line number Diff line change
@@ -294,12 +294,6 @@ impl core::fmt::Display for Error {
}
}

impl embedded_hal_nb::serial::Error for Error {
fn kind(&self) -> embedded_hal_nb::serial::ErrorKind {
embedded_hal_nb::serial::ErrorKind::Other
}
}

#[cfg(any(doc, feature = "unstable"))]
#[cfg_attr(docsrs, doc(cfg(feature = "unstable")))]
impl embedded_io::Error for Error {
@@ -645,22 +639,18 @@ where
pub fn write_bytes(&mut self, data: &[u8]) -> Result<usize, Error> {
let count = data.len();

data.iter()
.try_for_each(|c| nb::block!(self.write_byte(*c)))?;
for &byte in data {
self.write_byte(byte);
}

Ok(count)
}

fn write_byte(&mut self, word: u8) -> nb::Result<(), Error> {
if self.tx_fifo_count() < UART_FIFO_SIZE {
self.register_block()
.fifo()
.write(|w| unsafe { w.rxfifo_rd_byte().bits(word) });

Ok(())
} else {
Err(nb::Error::WouldBlock)
}
fn write_byte(&mut self, word: u8) {
while self.tx_fifo_count() >= UART_FIFO_SIZE {}
self.register_block()
.fifo()
.write(|w| unsafe { w.rxfifo_rd_byte().bits(word) });
}

#[allow(clippy::useless_conversion)]
@@ -676,12 +666,8 @@ where
}

/// Flush the transmit buffer of the UART
pub fn flush(&mut self) -> nb::Result<(), Error> {
if self.is_tx_idle() {
Ok(())
} else {
Err(nb::Error::WouldBlock)
}
pub fn flush(&mut self) {
while !self.is_tx_idle() {}
}

/// Checks if the TX line is idle for this UART instance.
@@ -1202,21 +1188,11 @@ where
sync_regs(register_block);
}

// Write a byte out over the UART
fn write_byte(&mut self, word: u8) -> nb::Result<(), Error> {
self.tx.write_byte(word)
}

/// Flush the transmit buffer of the UART
pub fn flush(&mut self) -> nb::Result<(), Error> {
pub fn flush(&mut self) {
self.tx.flush()
}

// Read a byte from the UART
fn read_byte(&mut self) -> nb::Result<u8, Error> {
embedded_hal_nb::serial::Read::read(&mut self.rx)
}

/// Change the configuration.
pub fn apply_config(&mut self, config: &Config) -> Result<(), ConfigError> {
self.rx.apply_config(config)?;
@@ -1383,65 +1359,6 @@ where
}
}

impl<Dm> embedded_hal_nb::serial::ErrorType for Uart<'_, Dm> {
type Error = Error;
}

impl<Dm> embedded_hal_nb::serial::ErrorType for UartTx<'_, Dm> {
type Error = Error;
}

impl<Dm> embedded_hal_nb::serial::ErrorType for UartRx<'_, Dm> {
type Error = Error;
}

impl<Dm> embedded_hal_nb::serial::Read for Uart<'_, Dm>
where
Dm: DriverMode,
{
fn read(&mut self) -> nb::Result<u8, Self::Error> {
self.read_byte()
}
}

impl<Dm> embedded_hal_nb::serial::Read for UartRx<'_, Dm>
where
Dm: DriverMode,
{
fn read(&mut self) -> nb::Result<u8, Self::Error> {
match self.read_byte() {
Some(b) => Ok(b),
None => Err(nb::Error::WouldBlock),
}
}
}

impl<Dm> embedded_hal_nb::serial::Write for Uart<'_, Dm>
where
Dm: DriverMode,
{
fn write(&mut self, word: u8) -> nb::Result<(), Self::Error> {
self.write_byte(word)
}

fn flush(&mut self) -> nb::Result<(), Self::Error> {
self.flush()
}
}

impl<Dm> embedded_hal_nb::serial::Write for UartTx<'_, Dm>
where
Dm: DriverMode,
{
fn write(&mut self, word: u8) -> nb::Result<(), Self::Error> {
self.write_byte(word)
}

fn flush(&mut self) -> nb::Result<(), Self::Error> {
self.flush()
}
}

#[cfg(any(doc, feature = "unstable"))]
#[cfg_attr(docsrs, doc(cfg(feature = "unstable")))]
impl<Dm> embedded_io::ErrorType for Uart<'_, Dm> {
@@ -1538,14 +1455,7 @@ where
}

fn flush(&mut self) -> Result<(), Self::Error> {
loop {
match self.flush() {
Ok(_) => break,
Err(nb::Error::WouldBlock) => { /* Wait */ }
Err(nb::Error::Other(e)) => return Err(e),
}
}

self.flush();
Ok(())
}
}
@@ -1745,7 +1655,7 @@ impl UartTx<'_, Async> {
}

for byte in &words[offset..next_offset] {
self.write_byte(*byte).unwrap(); // should never fail
self.write_byte(*byte);
count += 1;
}

65 changes: 0 additions & 65 deletions esp-hal/src/usb_serial_jtag.rs
Original file line number Diff line number Diff line change
@@ -543,71 +543,6 @@ where
}
}

impl<Dm> embedded_hal_nb::serial::ErrorType for UsbSerialJtag<'_, Dm>
where
Dm: DriverMode,
{
type Error = Error;
}

impl<Dm> embedded_hal_nb::serial::ErrorType for UsbSerialJtagTx<'_, Dm>
where
Dm: DriverMode,
{
type Error = Error;
}

impl<Dm> embedded_hal_nb::serial::ErrorType for UsbSerialJtagRx<'_, Dm>
where
Dm: DriverMode,
{
type Error = Error;
}

impl<Dm> embedded_hal_nb::serial::Read for UsbSerialJtag<'_, Dm>
where
Dm: DriverMode,
{
fn read(&mut self) -> nb::Result<u8, Self::Error> {
embedded_hal_nb::serial::Read::read(&mut self.rx)
}
}

impl<Dm> embedded_hal_nb::serial::Read for UsbSerialJtagRx<'_, Dm>
where
Dm: DriverMode,
{
fn read(&mut self) -> nb::Result<u8, Self::Error> {
self.read_byte()
}
}

impl<Dm> embedded_hal_nb::serial::Write for UsbSerialJtag<'_, Dm>
where
Dm: DriverMode,
{
fn write(&mut self, word: u8) -> nb::Result<(), Self::Error> {
embedded_hal_nb::serial::Write::write(&mut self.tx, word)
}

fn flush(&mut self) -> nb::Result<(), Self::Error> {
embedded_hal_nb::serial::Write::flush(&mut self.tx)
}
}

impl<Dm> embedded_hal_nb::serial::Write for UsbSerialJtagTx<'_, Dm>
where
Dm: DriverMode,
{
fn write(&mut self, word: u8) -> nb::Result<(), Self::Error> {
self.write_byte_nb(word)
}

fn flush(&mut self) -> nb::Result<(), Self::Error> {
self.flush_tx_nb()
}
}

#[cfg(any(doc, feature = "unstable"))]
#[cfg_attr(docsrs, doc(cfg(feature = "unstable")))]
impl<Dm> embedded_io::ErrorType for UsbSerialJtag<'_, Dm>
3 changes: 2 additions & 1 deletion hil-test/tests/embassy_timers_executors.rs
Original file line number Diff line number Diff line change
@@ -68,7 +68,8 @@ mod test_cases {
let t1 = esp_hal::time::now();
periodic.start(100.millis()).unwrap();

nb::block!(periodic.wait()).unwrap();
periodic.wait();

let t2 = esp_hal::time::now();

assert!(t2 > t1, "t2: {:?}, t1: {:?}", t2, t1);
31 changes: 11 additions & 20 deletions hil-test/tests/uart.rs
Original file line number Diff line number Diff line change
@@ -6,13 +6,11 @@
#![no_std]
#![no_main]

use embedded_hal_nb::serial::{Read, Write};
use esp_hal::{
uart::{self, ClockSource, Uart},
Blocking,
};
use hil_test as _;
use nb::block;

struct Context {
uart: Uart<'static, Blocking>,
@@ -39,9 +37,10 @@ mod tests {

#[test]
fn test_send_receive(mut ctx: Context) {
ctx.uart.write(0x42).ok();
let read = block!(ctx.uart.read());
assert_eq!(read, Ok(0x42));
ctx.uart.write_bytes(&[0x42]).unwrap();
let mut byte = [0u8; 1];
ctx.uart.read_bytes(&mut byte).unwrap();
assert_eq!(byte[0], 0x42);
}

#[test]
@@ -53,18 +52,8 @@ mod tests {
assert_eq!(written, BUF_SIZE);

let mut buffer = [0; BUF_SIZE];
let mut i = 0;

while i < BUF_SIZE {
match ctx.uart.read() {
Ok(byte) => {
buffer[i] = byte;
i += 1;
}
Err(nb::Error::WouldBlock) => continue,
Err(nb::Error::Other(_)) => panic!(),
}
}

ctx.uart.read_bytes(&mut buffer).unwrap();

assert_eq!(data, buffer);
}
@@ -94,9 +83,11 @@ mod tests {
.with_clock_source(clock_source),
)
.unwrap();
ctx.uart.write(byte_to_write).ok();
let read = block!(ctx.uart.read());
assert_eq!(read, Ok(byte_to_write));
ctx.uart.write_bytes(&[byte_to_write]).unwrap();
let mut byte = [0u8; 1];
ctx.uart.read_bytes(&mut byte).unwrap();

assert_eq!(byte[0], byte_to_write);
byte_to_write = !byte_to_write;
}
}
10 changes: 4 additions & 6 deletions hil-test/tests/uart_regression.rs
Original file line number Diff line number Diff line change
@@ -25,9 +25,6 @@ mod tests {
.unwrap()
.with_rx(rx);

// start reception
let mut buf = [0u8; 1];

// Start from a low level to verify that UartTx sets the level high initially,
// but don't enable output otherwise we actually pull down against RX's
// pullup resistor.
@@ -39,10 +36,11 @@ mod tests {
.unwrap()
.with_tx(tx);

tx.flush().unwrap();
tx.flush();
tx.write_bytes(&[0x42]).unwrap();
rx.read_bytes(&mut buf).unwrap();
let mut byte = [0u8; 1];
rx.read_bytes(&mut byte).unwrap();

assert_eq!(buf[0], 0x42);
assert_eq!(byte[0], 0x42);
}
}
4 changes: 2 additions & 2 deletions hil-test/tests/uart_tx_rx.rs
Original file line number Diff line number Diff line change
@@ -42,7 +42,7 @@ mod tests {
fn test_send_receive(mut ctx: Context) {
let byte = [0x42];

ctx.tx.flush().unwrap();
ctx.tx.flush();
ctx.tx.write_bytes(&byte).unwrap();
let mut buf = [0u8; 1];
ctx.rx.read_bytes(&mut buf).unwrap();
@@ -55,7 +55,7 @@ mod tests {
let bytes = [0x42, 0x43, 0x44];
let mut buf = [0u8; 3];

ctx.tx.flush().unwrap();
ctx.tx.flush();
ctx.tx.write_bytes(&bytes).unwrap();

ctx.rx.read_bytes(&mut buf).unwrap();

0 comments on commit c88dbef

Please sign in to comment.