Merge branch 'main' into spi-interrupts

documentation/API-GUIDELINES.md

@@ -80,6 +80,9 @@ In general, the [Rust API Guidelines](https://rust-lang.github.io/api-guidelines
 - If a driver only implements a subset of a peripheral's capabilities, it should be placed in the `peripheral::subcategory` module.
   - For example, if a driver implements the slave-mode I2C driver, it should be placed into `i2c::slave`.
   - This helps us reducing the need of introducing breaking changes if we implement additional functionalities.
+- Avoid abbreviations and contractions in the API, where possible.
+  - Saving a few characters may introduce ambiguity, e.g `SpiTransDone`, is it `Transmit` or `Transfer`?
+  - Common abbreviations, that are well understood such as `Dma` are perfectly fine.
 
 ## Maintainability
 

esp-hal/CHANGELOG.md

@@ -45,6 +45,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - `DmaRxBuf`, `DmaTxBuf` and `DmaRxTxBuf` now implement `Debug` and `defmt::Format` (#2823)
 - DMA channels (`AnyGdmaChannel`, `SpiDmaChannel`, `I2sDmaChannel`, `CryptoDmaChannel`) and their RX/TX halves now implement `Debug` and `defmt::Format` (#2823)
 - `DmaDescriptor` and `DmaDescriptorFlags` now implement `PartialEq` and `Eq` (#2823)
+- `gpio::{Event, WakeEvent, GpioRegisterAccess}` now implement `Debug`, `Eq`, `PartialEq` and `Hash` (#2842)
+- `gpio::{Level, Pull, AlternateFunction, RtcFunction}` now implement `Hash` (#2842)
+- `gpio::{GpioPin, AnyPin, Io, Output, OutputOpenDrain, Input, Flex}` now implement `Debug`, `defmt::Format` (#2842)
 - More interrupts are available in `esp_hal::spi::master::SpiInterrupt`, add `enable_listen`,`interrupts` and `clear_interrupts` for ESP32/ESP32-S2 (#2833)
 
 ### Changed
@@ -79,6 +82,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Updated `esp-pacs` with support for Wi-Fi on the ESP32 and made the peripheral non virtual
 - `SpiBitOrder`, `SpiDataMode`, `SpiMode` were renamed to `BitOder`, `DataMode` and `Mode` (#2828)
 - `crate::Mode` was renamed to `crate::DriverMode` (#2828)
+- Renamed some I2C error variants (#2844)
+
 ### Fixed
 
 - Xtensa devices now correctly enable the `esp-hal-procmacros/rtc-slow` feature (#2594)

esp-hal/MIGRATING-0.22.md

@@ -300,3 +300,20 @@ is not compatible with the hardware.
 - spi_dma.transfer(dma_rx_buf, dma_tx_buf);
 + spi_dma.transfer(5, dma_rx_buf, 5, dma_tx_buf);
 ```
+
+## I2C Error changes
+
+To avoid abbreviations and contractions (as per the esp-hal guidelines), some error variants have changed
+
+```diff
+- Error::ExecIncomplete
++ Error::ExecutionIncomplete
+- Error::CommandNrExceeded
++ Error::CommandNumberExceeded
+- Error::ExceedingFifo
++ Error::FifoExceeded
+- Error::TimeOut
++ Error::Timeout
+- Error::InvalidZeroLength
++ Error::ZeroLengthInvalid
+```

esp-hal/src/gpio/mod.rs

@@ -114,7 +114,7 @@ impl CFnPtr {
 }
 
 /// Event used to trigger interrupts.
-#[derive(Copy, Clone)]
+#[derive(Debug, Eq, PartialEq, Copy, Clone, Hash)]
 #[cfg_attr(feature = "defmt", derive(defmt::Format))]
 pub enum Event {
     /// Interrupts trigger on rising pin edge.
@@ -139,7 +139,7 @@ impl From<WakeEvent> for Event {
 }
 
 /// Event used to wake up from light sleep.
-#[derive(Copy, Clone)]
+#[derive(Debug, Eq, PartialEq, Copy, Clone, Hash)]
 #[cfg_attr(feature = "defmt", derive(defmt::Format))]
 pub enum WakeEvent {
     /// Wake on low level
@@ -157,7 +157,7 @@ pub enum WakeEvent {
 /// input, the peripheral will read the corresponding level from that signal.
 ///
 /// When connected to a peripheral output, the level will be ignored.
-#[derive(Debug, Eq, PartialEq, Copy, Clone)]
+#[derive(Debug, Eq, PartialEq, Copy, Clone, Hash)]
 #[cfg_attr(feature = "defmt", derive(defmt::Format))]
 pub enum Level {
     /// Low
@@ -198,7 +198,7 @@ impl From<Level> for bool {
 }
 
 /// Pull setting for an input.
-#[derive(Debug, Eq, PartialEq, Copy, Clone)]
+#[derive(Debug, Eq, PartialEq, Copy, Clone, Hash)]
 #[cfg_attr(feature = "defmt", derive(defmt::Format))]
 pub enum Pull {
     /// No pull
@@ -210,7 +210,7 @@ pub enum Pull {
 }
 
 /// Drive strength (values are approximates)
-#[derive(Debug, Eq, PartialEq, Copy, Clone)]
+#[derive(Debug, Eq, PartialEq, Copy, Clone, Hash, PartialOrd, Ord)]
 #[cfg_attr(feature = "defmt", derive(defmt::Format))]
 pub enum DriveStrength {
     /// Drive strength of approximately 5mA.
@@ -234,7 +234,7 @@ pub enum DriveStrength {
 /// The different variants correspond to different functionality depending on
 /// the chip and the specific pin. For more information, refer to your chip's
 #[doc = crate::trm_markdown_link!("iomuxgpio")]
-#[derive(Debug, Eq, PartialEq, Copy, Clone)]
+#[derive(Debug, Eq, PartialEq, Copy, Clone, Hash)]
 #[cfg_attr(feature = "defmt", derive(defmt::Format))]
 pub enum AlternateFunction {
     /// Alternate function 0.
@@ -268,7 +268,7 @@ impl TryFrom<usize> for AlternateFunction {
 }
 
 /// RTC function
-#[derive(Debug, Eq, PartialEq, Copy, Clone)]
+#[derive(Debug, Eq, PartialEq, Copy, Clone, Hash)]
 #[cfg_attr(feature = "defmt", derive(defmt::Format))]
 pub enum RtcFunction {
     /// RTC mode.
@@ -547,7 +547,8 @@ pub trait TouchPin: Pin {
 }
 
 #[doc(hidden)]
-#[derive(Clone, Copy, EnumCount)]
+#[derive(Debug, Eq, PartialEq, Copy, Clone, Hash, EnumCount)]
+#[cfg_attr(feature = "defmt", derive(defmt::Format))]
 pub enum GpioRegisterAccess {
     Bank0,
     #[cfg(gpio_bank_1)]
@@ -758,9 +759,13 @@ impl Bank1GpioRegisterAccess {
 
 /// GPIO pin
 #[non_exhaustive]
+#[derive(Debug)]
+#[cfg_attr(feature = "defmt", derive(defmt::Format))]
 pub struct GpioPin<const GPIONUM: u8>;
 
 /// Type-erased GPIO pin
+#[derive(Debug)]
+#[cfg_attr(feature = "defmt", derive(defmt::Format))]
 pub struct AnyPin(pub(crate) AnyPinInner);
 
 impl<const GPIONUM: u8> GpioPin<GPIONUM>
@@ -858,6 +863,8 @@ pub(crate) fn bind_default_interrupt_handler() {
 }
 
 /// General Purpose Input/Output driver
+#[derive(Debug)]
+#[cfg_attr(feature = "defmt", derive(defmt::Format))]
 pub struct Io {
     _io_mux: IO_MUX,
 }
@@ -1073,6 +1080,8 @@ macro_rules! gpio {
                 }
             )+
 
+            #[derive(Debug)]
+            #[cfg_attr(feature = "defmt", derive(defmt::Format))]
             pub(crate) enum AnyPinInner {
                 $(
                     [<Gpio $gpionum >]($crate::gpio::GpioPin<$gpionum>),
@@ -1174,6 +1183,8 @@ macro_rules! gpio {
 /// This driver configures the GPIO pin to be a push-pull output driver.
 /// Push-pull means that the driver actively sets the output voltage level
 /// for both high and low logical [`Level`]s.
+#[derive(Debug)]
+#[cfg_attr(feature = "defmt", derive(defmt::Format))]
 pub struct Output<'d, P = AnyPin> {
     pin: Flex<'d, P>,
 }
@@ -1345,6 +1356,8 @@ where
 ///
 /// This driver configures the GPIO pin to be an input. Input drivers read the
 /// voltage of their pins and convert it to a logical [`Level`].
+#[derive(Debug)]
+#[cfg_attr(feature = "defmt", derive(defmt::Format))]
 pub struct Input<'d, P = AnyPin> {
     pin: Flex<'d, P>,
 }
@@ -1615,6 +1628,8 @@ where
 /// for the low logical [`Level`], but leaves the high level floating, which is
 /// then determined by external hardware, or internal pull-up/pull-down
 /// resistors.
+#[derive(Debug)]
+#[cfg_attr(feature = "defmt", derive(defmt::Format))]
 pub struct OutputOpenDrain<'d, P = AnyPin> {
     pin: Flex<'d, P>,
 }
@@ -1838,6 +1853,8 @@ where
 /// Flexible pin driver.
 ///
 /// This driver allows changing the pin mode between input and output.
+#[derive(Debug)]
+#[cfg_attr(feature = "defmt", derive(defmt::Format))]
 pub struct Flex<'d, P = AnyPin> {
     pin: PeripheralRef<'d, P>,
 }

esp-hal/src/i2c/master/mod.rs

@@ -91,35 +91,37 @@ const MAX_ITERATIONS: u32 = 1_000_000;
 #[non_exhaustive]
 pub enum Error {
     /// The transmission exceeded the FIFO size.
-    ExceedingFifo,
+    FifoExceeded,
     /// The acknowledgment check failed.
     AckCheckFailed,
     /// A timeout occurred during transmission.
-    TimeOut,
+    Timeout,
     /// The arbitration for the bus was lost.
     ArbitrationLost,
     /// The execution of the I2C command was incomplete.
-    ExecIncomplete,
+    ExecutionIncomplete,
     /// The number of commands issued exceeded the limit.
-    CommandNrExceeded,
+    CommandNumberExceeded,
     /// Zero length read or write operation.
-    InvalidZeroLength,
+    ZeroLengthInvalid,
 }
 
 impl core::error::Error for Error {}
 
 impl core::fmt::Display for Error {
     fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
         match self {
-            Error::ExceedingFifo => write!(f, "The transmission exceeded the FIFO size"),
+            Error::FifoExceeded => write!(f, "The transmission exceeded the FIFO size"),
             Error::AckCheckFailed => write!(f, "The acknowledgment check failed"),
-            Error::TimeOut => write!(f, "A timeout occurred during transmission"),
+            Error::Timeout => write!(f, "A timeout occurred during transmission"),
             Error::ArbitrationLost => write!(f, "The arbitration for the bus was lost"),
-            Error::ExecIncomplete => write!(f, "The execution of the I2C command was incomplete"),
-            Error::CommandNrExceeded => {
+            Error::ExecutionIncomplete => {
+                write!(f, "The execution of the I2C command was incomplete")
+            }
+            Error::CommandNumberExceeded => {
                 write!(f, "The number of commands issued exceeded the limit")
             }
-            Error::InvalidZeroLength => write!(f, "Zero length read or write operation"),
+            Error::ZeroLengthInvalid => write!(f, "Zero length read or write operation"),
         }
     }
 }
@@ -204,7 +206,7 @@ impl embedded_hal::i2c::Error for Error {
         use embedded_hal::i2c::{ErrorKind, NoAcknowledgeSource};
 
         match self {
-            Self::ExceedingFifo => ErrorKind::Overrun,
+            Self::FifoExceeded => ErrorKind::Overrun,
             Self::ArbitrationLost => ErrorKind::ArbitrationLoss,
             Self::AckCheckFailed => ErrorKind::NoAcknowledge(NoAcknowledgeSource::Unknown),
             _ => ErrorKind::Other,
@@ -643,7 +645,7 @@ impl<'a> I2cFuture<'a> {
         }
 
         if r.time_out().bit_is_set() {
-            return Err(Error::TimeOut);
+            return Err(Error::Timeout);
         }
 
         if r.nack().bit_is_set() {
@@ -1337,7 +1339,7 @@ impl Driver<'_> {
         let max_len = if start { 254usize } else { 255usize };
         if bytes.len() > max_len {
             // we could support more by adding multiple write operations
-            return Err(Error::ExceedingFifo);
+            return Err(Error::FifoExceeded);
         }
 
         let write_len = if start { bytes.len() + 1 } else { bytes.len() };
@@ -1386,7 +1388,7 @@ impl Driver<'_> {
         I: Iterator<Item = &'a COMD>,
     {
         if buffer.is_empty() {
-            return Err(Error::InvalidZeroLength);
+            return Err(Error::ZeroLengthInvalid);
         }
         let (max_len, initial_len) = if will_continue {
             (255usize, buffer.len())
@@ -1395,7 +1397,7 @@ impl Driver<'_> {
         };
         if buffer.len() > max_len {
             // we could support more by adding multiple read operations
-            return Err(Error::ExceedingFifo);
+            return Err(Error::FifoExceeded);
         }
 
         if start {
@@ -1579,7 +1581,7 @@ impl Driver<'_> {
 
             tout -= 1;
             if tout == 0 {
-                return Err(Error::TimeOut);
+                return Err(Error::Timeout);
             }
 
             embassy_futures::yield_now().await;
@@ -1607,7 +1609,7 @@ impl Driver<'_> {
 
             tout -= 1;
             if tout == 0 {
-                return Err(Error::TimeOut);
+                return Err(Error::Timeout);
             }
         }
         self.check_all_commands_done()?;
@@ -1623,7 +1625,7 @@ impl Driver<'_> {
             let cmd = cmd_reg.read();
 
             if cmd.bits() != 0x0 && !cmd.opcode().is_end() && !cmd.command_done().bit_is_set() {
-                return Err(Error::ExecIncomplete);
+                return Err(Error::ExecutionIncomplete);
             }
         }
 
@@ -1646,7 +1648,7 @@ impl Driver<'_> {
             if #[cfg(esp32)] {
                 // Handle error cases
                 let retval = if interrupts.time_out().bit_is_set() {
-                    Err(Error::TimeOut)
+                    Err(Error::Timeout)
                 } else if interrupts.nack().bit_is_set() {
                     Err(Error::AckCheckFailed)
                 } else if interrupts.arbitration_lost().bit_is_set() {
@@ -1657,7 +1659,7 @@ impl Driver<'_> {
             } else {
                 // Handle error cases
                 let retval = if interrupts.time_out().bit_is_set() {
-                    Err(Error::TimeOut)
+                    Err(Error::Timeout)
                 } else if interrupts.nack().bit_is_set() {
                     Err(Error::AckCheckFailed)
                 } else if interrupts.arbitration_lost().bit_is_set() {
@@ -2181,7 +2183,7 @@ fn add_cmd<'a, I>(cmd_iterator: &mut I, command: Command) -> Result<(), Error>
 where
     I: Iterator<Item = &'a COMD>,
 {
-    let cmd = cmd_iterator.next().ok_or(Error::CommandNrExceeded)?;
+    let cmd = cmd_iterator.next().ok_or(Error::CommandNumberExceeded)?;
 
     cmd.write(|w| match command {
         Command::Start => w.opcode().rstart(),

esp-hal/src/spi/master.rs

@@ -1738,8 +1738,8 @@ mod dma {
 
                 unsafe {
                     self.spi_dma.start_dma_transfer(
-                        chunk.len(),
                         0,
+                        chunk.len(),
                         &mut EmptyBuf,
                         &mut self.tx_buf,
                     )?;

hil-test/tests/spi_full_duplex.rs

@@ -447,6 +447,39 @@ mod tests {
         );
     }
 
+    #[test]
+    #[cfg(pcnt)]
+    fn test_dma_bus_read_write_pcnt(ctx: Context) {
+        const TRANSFER_SIZE: usize = 4;
+        let (rx_buffer, rx_descriptors, tx_buffer, tx_descriptors) = dma_buffers!(4);
+        let dma_rx_buf = DmaRxBuf::new(rx_descriptors, rx_buffer).unwrap();
+        let dma_tx_buf = DmaTxBuf::new(tx_descriptors, tx_buffer).unwrap();
+
+        ctx.pcnt_unit.channel0.set_edge_signal(ctx.pcnt_source);
+        ctx.pcnt_unit
+            .channel0
+            .set_input_mode(EdgeMode::Hold, EdgeMode::Increment);
+
+        let mut spi = ctx
+            .spi
+            .with_dma(ctx.dma_channel)
+            .with_buffers(dma_rx_buf, dma_tx_buf);
+
+        // Fill the buffer where each byte has 3 pos edges.
+        let tx_buf = [0b0110_1010; TRANSFER_SIZE];
+        let mut rx_buf = [0; TRANSFER_SIZE];
+
+        for i in 1..4 {
+            // Preset as 5, expect 0 repeated receive
+            rx_buf.copy_from_slice(&[5; TRANSFER_SIZE]);
+            spi.read(&mut rx_buf).unwrap();
+            assert_eq!(rx_buf, [0; TRANSFER_SIZE]);
+
+            spi.write(&tx_buf).unwrap();
+            assert_eq!(ctx.pcnt_unit.value(), (i * 3 * TRANSFER_SIZE) as _);
+        }
+    }
+
     #[test]
     fn test_dma_bus_symmetric_transfer(ctx: Context) {
         let (rx_buffer, rx_descriptors, tx_buffer, tx_descriptors) = dma_buffers!(4);