Skip to content

Commit

Permalink
Remove the single byte read/write inherent functions (#2915)
Browse files Browse the repository at this point in the history
* SPI: remove read/write_byte functions

* UART: make read/write_byte functions private

* changelog

* migration guide

* fix ieee802154_sniffer example

* review comments and cleanup

* use variable name buf instead of buff

* add pub fn read_bytes

* migration guide update

* another migration guide update

* improve docs of read/write_bytes
  • Loading branch information
JurajSadel authored Jan 10, 2025
1 parent 8d948ba commit 03ea4f6
Show file tree
Hide file tree
Showing 7 changed files with 62 additions and 67 deletions.
2 changes: 2 additions & 0 deletions esp-hal/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- `DmaTxBuf::{compute_chunk_size, compute_descriptor_count, new_with_block_size}` (#2543)

- The `prelude` module has been removed (#2845)
- SPI: Removed `pub fn read_byte` and `pub fn write_byte` (#2915)

- Removed all peripheral instance type parameters and `new_typed` constructors (#2907)

Expand Down Expand Up @@ -200,6 +201,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Many peripherals are now disabled by default and also get disabled when the driver is dropped (#2544)

- Config: Crate prefixes and configuration keys are now separated by `_CONFIG_` (#2848)
- UART: `read_byte` and `write_byte` made private. (#2915)

### Fixed

Expand Down
21 changes: 21 additions & 0 deletions esp-hal/MIGRATING-0.22.md
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,18 @@ e.g.
+ .with_tx(peripherals.GPIO2);
```

`write_byte` and `read_byte` have been removed.

e.g.

```dif
- while let nb::Result::Ok(_c) = serial.read_byte() {
- cnt += 1;
- }
+ let mut buff = [0u8; 64];
+ 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.
Expand Down Expand Up @@ -466,6 +478,15 @@ The Address and Command enums have similarly had their variants changed from e.g
+ Command::_1Bit
```

`write_byte` and `read_byte` were removed and `write_bytes` and `read_bytes` can be used as replacement.

e.g.

```rust
let mut byte = [0u8; 1];
spi.read_bytes(&mut byte);
```

## GPIO Changes

The GPIO drive strength variants are renamed from e.g. `I5mA` to `_5mA`.
Expand Down
56 changes: 14 additions & 42 deletions esp-hal/src/spi/master.rs
Original file line number Diff line number Diff line change
Expand Up @@ -505,29 +505,22 @@ where
}
}

/// Read a byte from SPI.
///
/// Sends out a stuffing byte for every byte to read. This function doesn't
/// perform flushing. If you want to read the response to something you
/// have written before, consider using [`Self::transfer`] instead.
pub fn read_byte(&mut self) -> nb::Result<u8, Error> {
self.driver().read_byte()
}

/// Write a byte to SPI.
pub fn write_byte(&mut self, word: u8) -> nb::Result<(), Error> {
self.driver().write_byte(word)
}

/// Write bytes to SPI.
/// Write bytes to SPI. After writing, flush is called to ensure all data
/// has been transmitted.
pub fn write_bytes(&mut self, words: &[u8]) -> Result<(), Error> {
self.driver().write_bytes(words)?;
self.driver().flush()?;

Ok(())
}

/// Sends `words` to the slave. Returns the `words` received from the slave
/// Read bytes from SPI. The provided slice is filled with data received
/// from the slave.
pub fn read_bytes(&mut self, words: &mut [u8]) -> Result<(), Error> {
self.driver().read_bytes(words)
}

/// Sends `words` to the slave. Returns the `words` received from the slave.
pub fn transfer<'w>(&mut self, words: &'w mut [u8]) -> Result<&'w [u8], Error> {
self.driver().transfer(words)
}
Expand Down Expand Up @@ -2153,11 +2146,14 @@ mod ehal1 {
Dm: DriverMode,
{
fn read(&mut self) -> nb::Result<u8, Self::Error> {
self.driver().read_byte()
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_byte(word)
self.driver().write_bytes(&[word])?;
Ok(())
}
}

Expand Down Expand Up @@ -2926,30 +2922,6 @@ impl Driver {
});
}

fn read_byte(&self) -> nb::Result<u8, Error> {
if self.busy() {
return Err(nb::Error::WouldBlock);
}

let reg_block = self.register_block();
Ok(u32::try_into(reg_block.w(0).read().bits()).unwrap_or_default())
}

fn write_byte(&self, word: u8) -> nb::Result<(), Error> {
if self.busy() {
return Err(nb::Error::WouldBlock);
}

self.configure_datalen(0, 1);

let reg_block = self.register_block();
reg_block.w(0).write(|w| w.buf().set(word.into()));

self.start_operation();

Ok(())
}

#[cfg_attr(place_spi_driver_in_ram, ram)]
fn fill_fifo(&self, chunk: &[u8]) {
// TODO: replace with `array_chunks` and `from_le_bytes`
Expand Down
21 changes: 10 additions & 11 deletions esp-hal/src/uart.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@
//!
//! // Each component can be used individually to interact with the UART:
//! tx.write_bytes(&[42u8]).expect("write error!");
//! let byte = rx.read_byte().expect("read error!");
//! let mut byte = [0u8; 1];
//! rx.read_bytes(&mut byte);
//! # }
//! ```
//!
Expand Down Expand Up @@ -197,10 +198,8 @@
//! let mut serial = SERIAL.borrow_ref_mut(cs);
//! let serial = serial.as_mut().unwrap();
//!
//! let mut cnt = 0;
//! while let nb::Result::Ok(_c) = serial.read_byte() {
//! cnt += 1;
//! }
//! let mut buf = [0u8; 64];
//! let cnt = serial.read_bytes(&mut buf);
//! writeln!(serial, "Read {} bytes", cnt).ok();
//!
//! let pending_interrupts = serial.interrupts();
Expand Down Expand Up @@ -826,8 +825,8 @@ where
self.uart.info().apply_config(config)
}

/// Read a byte from the UART
pub fn read_byte(&mut self) -> nb::Result<u8, Error> {
// Read a byte from the UART
fn read_byte(&mut self) -> nb::Result<u8, Error> {
cfg_if::cfg_if! {
if #[cfg(esp32s2)] {
// On the ESP32-S2 we need to use PeriBus2 to read the FIFO:
Expand Down Expand Up @@ -1134,8 +1133,8 @@ where
sync_regs(register_block);
}

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

Expand All @@ -1144,8 +1143,8 @@ where
self.tx.flush()
}

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

Expand Down
15 changes: 8 additions & 7 deletions examples/src/bin/ieee802154_sniffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,14 @@ fn main() -> ! {
let mut cnt = 0;
let mut read = [0u8; 2];
loop {
let c = nb::block!(uart0.read_byte()).unwrap();
if c == b'r' {
let mut buf = [0u8; 1];
while uart0.read_bytes(&mut buf) == 0 {}

if buf[0] == b'r' {
continue;
}

read[cnt] = c;
read[cnt] = buf[0];
cnt += 1;

if cnt >= 2 {
Expand Down Expand Up @@ -74,10 +76,9 @@ fn main() -> ! {
println!("@RAW {:02x?}", &frame.data);
}

if let nb::Result::Ok(c) = uart0.read_byte() {
if c == b'r' {
software_reset();
}
let mut buf = [0u8; 1];
if uart0.read_bytes(&mut buf) > 0 && buf[0] == b'r' {
software_reset();
}
}
}
8 changes: 4 additions & 4 deletions hil-test/tests/uart_regression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ mod tests {
uart::{self, UartRx, UartTx},
};
use hil_test as _;
use nb::block;

#[test]
fn test_that_creating_tx_does_not_cause_a_pulse() {
Expand All @@ -27,7 +26,8 @@ mod tests {
.with_rx(rx);

// start reception
_ = rx.read_byte(); // this will just return WouldBlock
let mut buf = [0u8; 1];
_ = rx.read_bytes(&mut buf); // this will just return WouldBlock

unsafe { tx.set_output_high(false, esp_hal::Internal::conjure()) };

Expand All @@ -38,8 +38,8 @@ mod tests {

tx.flush().unwrap();
tx.write_bytes(&[0x42]).unwrap();
let read = block!(rx.read_byte());
while rx.read_bytes(&mut buf) == 0 {}

assert_eq!(read, Ok(0x42));
assert_eq!(buf[0], 0x42);
}
}
6 changes: 3 additions & 3 deletions hil-test/tests/uart_tx_rx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ use esp_hal::{
Blocking,
};
use hil_test as _;
use nb::block;

struct Context {
rx: UartRx<'static, Blocking>,
Expand Down Expand Up @@ -45,9 +44,10 @@ mod tests {

ctx.tx.flush().unwrap();
ctx.tx.write_bytes(&byte).unwrap();
let read = block!(ctx.rx.read_byte());
let mut buf = [0u8; 1];
while ctx.rx.read_bytes(&mut buf) == 0 {}

assert_eq!(read, Ok(0x42));
assert_eq!(buf[0], 0x42);
}

#[test]
Expand Down

0 comments on commit 03ea4f6

Please sign in to comment.