From 7fa3564f20b3bab83ad0f4a6d9a5cfe04b44097b Mon Sep 17 00:00:00 2001 From: Juraj Sadel Date: Thu, 9 Jan 2025 12:27:01 +0100 Subject: [PATCH 01/11] SPI: remove read/write_byte functions --- esp-hal/src/spi/master.rs | 45 +++++---------------------------------- 1 file changed, 5 insertions(+), 40 deletions(-) diff --git a/esp-hal/src/spi/master.rs b/esp-hal/src/spi/master.rs index 5840107ebb3..9aca27dd0fb 100644 --- a/esp-hal/src/spi/master.rs +++ b/esp-hal/src/spi/master.rs @@ -504,20 +504,6 @@ 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 { - 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. pub fn write_bytes(&mut self, words: &[u8]) -> Result<(), Error> { self.driver().write_bytes(words)?; @@ -2139,11 +2125,14 @@ mod ehal1 { Dm: DriverMode, { fn read(&mut self) -> nb::Result { - 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(()) } } @@ -2912,30 +2901,6 @@ impl Driver { }); } - fn read_byte(&self) -> nb::Result { - 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` From 154c7921c6afeb1108c8eabc32a2ac9d888859c2 Mon Sep 17 00:00:00 2001 From: Juraj Sadel Date: Thu, 9 Jan 2025 12:27:37 +0100 Subject: [PATCH 02/11] UART: make read/write_byte functions private --- esp-hal/src/uart.rs | 13 ++++++------- examples/src/bin/ieee802154_sniffer.rs | 15 ++++++++------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/esp-hal/src/uart.rs b/esp-hal/src/uart.rs index 513fa135152..7074578d6ce 100644 --- a/esp-hal/src/uart.rs +++ b/esp-hal/src/uart.rs @@ -198,9 +198,8 @@ //! let serial = serial.as_mut().unwrap(); //! //! let mut cnt = 0; -//! while let nb::Result::Ok(_c) = serial.read_byte() { -//! cnt += 1; -//! } +//! let mut buff = [0u8; 64]; +//! let cnt = serial.read_bytes(&mut buff); //! writeln!(serial, "Read {} bytes", cnt).ok(); //! //! let pending_interrupts = serial.interrupts(); @@ -1133,8 +1132,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) } @@ -1143,8 +1142,8 @@ where self.tx.flush() } - /// Read a byte from the UART - pub fn read_byte(&mut self) -> nb::Result { + // Read a byte from the UART + fn read_byte(&mut self) -> nb::Result { self.rx.read_byte() } diff --git a/examples/src/bin/ieee802154_sniffer.rs b/examples/src/bin/ieee802154_sniffer.rs index c94bb7fc933..3aa80b57b20 100644 --- a/examples/src/bin/ieee802154_sniffer.rs +++ b/examples/src/bin/ieee802154_sniffer.rs @@ -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 buff = [0u8; 1]; + uart0.read_bytes(&mut buff); + + if buff[0] == b'r' { continue; } - read[cnt] = c; + read[cnt] = buff[0]; cnt += 1; if cnt >= 2 { @@ -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 buff = [0u8; 1]; + if uart0.read_bytes(&mut buff) > 0 && buff[0] == b'r' { + software_reset(); } } } From 3bbb707ccdca36e3b5114030f23ae7f881d04fed Mon Sep 17 00:00:00 2001 From: Juraj Sadel Date: Thu, 9 Jan 2025 12:49:28 +0100 Subject: [PATCH 03/11] changelog --- esp-hal/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/esp-hal/CHANGELOG.md b/esp-hal/CHANGELOG.md index 25354f8872c..17a36ba3d97 100644 --- a/esp-hal/CHANGELOG.md +++ b/esp-hal/CHANGELOG.md @@ -129,6 +129,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) @@ -198,6 +199,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 From 73ab2e37ad9f72bcc0aad6d146d000bf0007d6f0 Mon Sep 17 00:00:00 2001 From: Juraj Sadel Date: Thu, 9 Jan 2025 12:49:38 +0100 Subject: [PATCH 04/11] migration guide --- esp-hal/MIGRATING-0.22.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/esp-hal/MIGRATING-0.22.md b/esp-hal/MIGRATING-0.22.md index 59ca402ccf8..c39f9d9f90c 100644 --- a/esp-hal/MIGRATING-0.22.md +++ b/esp-hal/MIGRATING-0.22.md @@ -429,6 +429,18 @@ e.g. + .with_tx(peripherals.GPIO2); ``` +The `write_byte` and `read_byte` made private. + +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. @@ -466,6 +478,9 @@ The Address and Command enums have similarly had their variants changed from e.g + Command::_1Bit ``` +The `write_byte` and `read_byte` was removed. +`driver.write_bytes(&[data])?` could be used. + ## GPIO Changes The GPIO drive strength variants are renamed from e.g. `I5mA` to `_5mA`. From 0d29d95126765adc550ea1191770f0390a3f6376 Mon Sep 17 00:00:00 2001 From: Juraj Sadel Date: Thu, 9 Jan 2025 14:21:23 +0100 Subject: [PATCH 05/11] fix ieee802154_sniffer example --- examples/src/bin/ieee802154_sniffer.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/src/bin/ieee802154_sniffer.rs b/examples/src/bin/ieee802154_sniffer.rs index 3aa80b57b20..b69d17829e8 100644 --- a/examples/src/bin/ieee802154_sniffer.rs +++ b/examples/src/bin/ieee802154_sniffer.rs @@ -40,7 +40,7 @@ fn main() -> ! { let mut read = [0u8; 2]; loop { let mut buff = [0u8; 1]; - uart0.read_bytes(&mut buff); + while uart0.read_bytes(&mut buff) == 0 {} if buff[0] == b'r' { continue; From e64f3eb632621175aaee9d68c904a9d32f52728b Mon Sep 17 00:00:00 2001 From: Juraj Sadel Date: Thu, 9 Jan 2025 17:50:43 +0100 Subject: [PATCH 06/11] review comments and cleanup --- esp-hal/MIGRATING-0.22.md | 9 ++++++--- esp-hal/src/uart.rs | 8 ++++---- hil-test/tests/uart_regression.rs | 8 ++++---- hil-test/tests/uart_tx_rx.rs | 6 +++--- 4 files changed, 17 insertions(+), 14 deletions(-) diff --git a/esp-hal/MIGRATING-0.22.md b/esp-hal/MIGRATING-0.22.md index c39f9d9f90c..bd339de8ac6 100644 --- a/esp-hal/MIGRATING-0.22.md +++ b/esp-hal/MIGRATING-0.22.md @@ -429,7 +429,7 @@ e.g. + .with_tx(peripherals.GPIO2); ``` -The `write_byte` and `read_byte` made private. +`write_byte` and `read_byte` are now private. e.g. @@ -478,8 +478,11 @@ The Address and Command enums have similarly had their variants changed from e.g + Command::_1Bit ``` -The `write_byte` and `read_byte` was removed. -`driver.write_bytes(&[data])?` could be used. +`write_byte` and `read_byte` were removed and `write_bytes` and `read_bytes` can be used as replacement. + +e.g. + +`driver.write_bytes(&[data])?` and `driver.read_bytes(&mut buffer)?` ## GPIO Changes diff --git a/esp-hal/src/uart.rs b/esp-hal/src/uart.rs index 7074578d6ce..1635c459785 100644 --- a/esp-hal/src/uart.rs +++ b/esp-hal/src/uart.rs @@ -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); //! # } //! ``` //! @@ -197,7 +198,6 @@ //! let mut serial = SERIAL.borrow_ref_mut(cs); //! let serial = serial.as_mut().unwrap(); //! -//! let mut cnt = 0; //! let mut buff = [0u8; 64]; //! let cnt = serial.read_bytes(&mut buff); //! writeln!(serial, "Read {} bytes", cnt).ok(); @@ -825,8 +825,8 @@ where self.uart.info().apply_config(config) } - /// Read a byte from the UART - pub fn read_byte(&mut self) -> nb::Result { + // Read a byte from the UART + fn read_byte(&mut self) -> nb::Result { cfg_if::cfg_if! { if #[cfg(esp32s2)] { // On the ESP32-S2 we need to use PeriBus2 to read the FIFO: diff --git a/hil-test/tests/uart_regression.rs b/hil-test/tests/uart_regression.rs index 3a0f1ddc0c8..6d4d53a05d1 100644 --- a/hil-test/tests/uart_regression.rs +++ b/hil-test/tests/uart_regression.rs @@ -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() { @@ -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()) }; @@ -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); } } diff --git a/hil-test/tests/uart_tx_rx.rs b/hil-test/tests/uart_tx_rx.rs index 8494498fae9..69c4acb0157 100644 --- a/hil-test/tests/uart_tx_rx.rs +++ b/hil-test/tests/uart_tx_rx.rs @@ -11,7 +11,6 @@ use esp_hal::{ Blocking, }; use hil_test as _; -use nb::block; struct Context { rx: UartRx<'static, Blocking>, @@ -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] From 475a7ec4930aa884db52e142c6921534c09a6a8b Mon Sep 17 00:00:00 2001 From: Juraj Sadel Date: Fri, 10 Jan 2025 10:18:25 +0100 Subject: [PATCH 07/11] use variable name buf instead of buff --- esp-hal/src/uart.rs | 4 ++-- examples/src/bin/ieee802154_sniffer.rs | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/esp-hal/src/uart.rs b/esp-hal/src/uart.rs index 1635c459785..9ae8ed3dfd0 100644 --- a/esp-hal/src/uart.rs +++ b/esp-hal/src/uart.rs @@ -198,8 +198,8 @@ //! let mut serial = SERIAL.borrow_ref_mut(cs); //! let serial = serial.as_mut().unwrap(); //! -//! let mut buff = [0u8; 64]; -//! let cnt = serial.read_bytes(&mut buff); +//! let mut buf = [0u8; 64]; +//! let cnt = serial.read_bytes(&mut buf); //! writeln!(serial, "Read {} bytes", cnt).ok(); //! //! let pending_interrupts = serial.interrupts(); diff --git a/examples/src/bin/ieee802154_sniffer.rs b/examples/src/bin/ieee802154_sniffer.rs index b69d17829e8..4998fa0c5d6 100644 --- a/examples/src/bin/ieee802154_sniffer.rs +++ b/examples/src/bin/ieee802154_sniffer.rs @@ -39,14 +39,14 @@ fn main() -> ! { let mut cnt = 0; let mut read = [0u8; 2]; loop { - let mut buff = [0u8; 1]; - while uart0.read_bytes(&mut buff) == 0 {} + let mut buf = [0u8; 1]; + while uart0.read_bytes(&mut buf) == 0 {} - if buff[0] == b'r' { + if buf[0] == b'r' { continue; } - read[cnt] = buff[0]; + read[cnt] = buf[0]; cnt += 1; if cnt >= 2 { @@ -76,8 +76,8 @@ fn main() -> ! { println!("@RAW {:02x?}", &frame.data); } - let mut buff = [0u8; 1]; - if uart0.read_bytes(&mut buff) > 0 && buff[0] == b'r' { + let mut buf = [0u8; 1]; + if uart0.read_bytes(&mut buf) > 0 && buf[0] == b'r' { software_reset(); } } From cc96ed539478b3377b89b58141323d71c55c9d42 Mon Sep 17 00:00:00 2001 From: Juraj Sadel Date: Fri, 10 Jan 2025 14:02:19 +0100 Subject: [PATCH 08/11] add pub fn read_bytes --- esp-hal/src/spi/master.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/esp-hal/src/spi/master.rs b/esp-hal/src/spi/master.rs index 9aca27dd0fb..f43ac94fe05 100644 --- a/esp-hal/src/spi/master.rs +++ b/esp-hal/src/spi/master.rs @@ -512,6 +512,11 @@ where Ok(()) } + /// Read bytes from SPI. + 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) From f2c40fdbafb3cd18235274089975c8cc09065faa Mon Sep 17 00:00:00 2001 From: Juraj Sadel Date: Fri, 10 Jan 2025 14:02:34 +0100 Subject: [PATCH 09/11] migration guide update --- esp-hal/MIGRATING-0.22.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/esp-hal/MIGRATING-0.22.md b/esp-hal/MIGRATING-0.22.md index bd339de8ac6..8fcf6dad155 100644 --- a/esp-hal/MIGRATING-0.22.md +++ b/esp-hal/MIGRATING-0.22.md @@ -482,6 +482,11 @@ The Address and Command enums have similarly had their variants changed from e.g e.g. +```rust +let mut byte = [0u8; 1]; +spi.read_bytes(&mut byte); +``` + `driver.write_bytes(&[data])?` and `driver.read_bytes(&mut buffer)?` ## GPIO Changes From 8c48ab410aaf31bb5f2506683627038161946497 Mon Sep 17 00:00:00 2001 From: Juraj Sadel Date: Fri, 10 Jan 2025 14:10:03 +0100 Subject: [PATCH 10/11] another migration guide update --- esp-hal/MIGRATING-0.22.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/esp-hal/MIGRATING-0.22.md b/esp-hal/MIGRATING-0.22.md index 8fcf6dad155..efdbfae7557 100644 --- a/esp-hal/MIGRATING-0.22.md +++ b/esp-hal/MIGRATING-0.22.md @@ -429,7 +429,7 @@ e.g. + .with_tx(peripherals.GPIO2); ``` -`write_byte` and `read_byte` are now private. +`write_byte` and `read_byte` have been removed. e.g. @@ -487,8 +487,6 @@ let mut byte = [0u8; 1]; spi.read_bytes(&mut byte); ``` -`driver.write_bytes(&[data])?` and `driver.read_bytes(&mut buffer)?` - ## GPIO Changes The GPIO drive strength variants are renamed from e.g. `I5mA` to `_5mA`. From f15db5bd375753e150f8fb80219bba4c2f675979 Mon Sep 17 00:00:00 2001 From: Juraj Sadel Date: Fri, 10 Jan 2025 15:20:20 +0100 Subject: [PATCH 11/11] improve docs of read/write_bytes --- esp-hal/src/spi/master.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/esp-hal/src/spi/master.rs b/esp-hal/src/spi/master.rs index f43ac94fe05..edbd5776705 100644 --- a/esp-hal/src/spi/master.rs +++ b/esp-hal/src/spi/master.rs @@ -504,7 +504,8 @@ where } } - /// 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()?; @@ -512,12 +513,13 @@ where Ok(()) } - /// Read bytes from SPI. + /// 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 + /// 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) }