SPI pins are no longer optional (#2133)

* SPI pins are no longer optional, rename DummyPin

* Swap QSPI test expected levels

* Tweak documentation around Level, implement PeripheralOutput

* Fmt

esp-hal/CHANGELOG.md

@@ -12,7 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Added
 
 - Introduce traits for the DMA buffer objects (#1976)
-- Implement `embedded-hal` output pin traits for `DummyPin` (#2019)
+- Implement `embedded-hal` output pin traits for `NoPin` (#2019, #2133)
 - Added `esp_hal::init` to simplify HAL initialisation (#1970, #1999)
 - Added GpioPin::degrade to create ErasePins easily. Same for AnyPin by accident. (#2075)
 - Added missing functions to `Flex`: `unlisten`, `is_interrupt_set`, `wakeup_enable`, `wait_for_high`, `wait_for_low`, `wait_for_rising_edge`, `wait_for_falling_edge`, `wait_for_any_edge`. (#2075)
@@ -39,6 +39,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - ESP32: Added support for touch sensing on GPIO32 and 33 (#2109)
 - Replaced `AnyPin` with `InputSignal` and `OutputSignal` and renamed `ErasedPin` to `AnyPin` (#2128)
 - Replaced the `ErasedTimer` enum with the `AnyTimer` struct. (#?)
+- Changed the parameters of `Spi::with_pins` to no longer be optional (#2133)
+- Renamed `DummyPin` to `NoPin` and removed all internal logic from it. (#2133)
+- The `NO_PIN` constant has been removed. (#2133)
 
 ### Fixed
 

esp-hal/MIGRATING-0.20.md

@@ -146,3 +146,16 @@ configure an input pin, and pass it to `set_edge_signal` or `set_ctrl_signal`.
 -   ));
 +   ch0.set_edge_signal(Input::new(io.pins.gpio5, Pull::Down));
 ```
+
+## SPI pins and `NO_PIN`
+
+Use `NoPin` in place of the now-removed `NO_PIN` constant.
+
+SPI pins, when using the `with_pin` function, are no longer optional.
+You can pass `NoPin` or `Level` as inputs, and `NoPin` as output if you don't need a particular pin.
+
+```diff
+ let spi = Spi::new(peripherals.SPI2, 100.kHz(), SpiMode::Mode0)
+-    .with_pins(Some(sclk), Some(mosi), NO_PIN, NO_PIN);
++    .with_pins(sclk, mosi, Level::Low, NoPin);
+```

esp-hal/src/dma/mod.rs

@@ -35,7 +35,7 @@
 //!     100.kHz(),
 //!     SpiMode::Mode0,
 //! )
-//! .with_pins(Some(sclk), Some(mosi), Some(miso), Some(cs))
+//! .with_pins(sclk, mosi, miso, cs)
 //! .with_dma(dma_channel.configure(
 //!     false,
 //!     DmaPriority::Priority0,

esp-hal/src/gpio/interconnect.rs

@@ -5,11 +5,11 @@ use crate::{
         self,
         AlternateFunction,
         AnyPin,
-        DummyPin,
         GpioPin,
         GpioProperties,
         InputPin,
         Level,
+        NoPin,
         OutputSignalType,
         PeripheralInput,
         PeripheralOutput,
@@ -331,7 +331,7 @@ impl PeripheralOutput for OutputSignal {
 enum AnyInputSignalInner {
     Input(InputSignal),
     Constant(Level),
-    Dummy(DummyPin),
+    Dummy(NoPin),
 }
 
 /// A type-erased input signal.
@@ -358,8 +358,8 @@ impl From<Level> for AnyInputSignal {
     }
 }
 
-impl From<DummyPin> for AnyInputSignal {
-    fn from(pin: DummyPin) -> Self {
+impl From<NoPin> for AnyInputSignal {
+    fn from(pin: NoPin) -> Self {
         Self(AnyInputSignalInner::Dummy(pin))
     }
 }

esp-hal/src/gpio/mod.rs

@@ -26,8 +26,8 @@
 //! - [Output] and [OutputOpenDrain] pins can be used as digital outputs.
 //! - [Flex] pin is a pin that can be used as an input and output pin.
 //! - [AnyPin] is a type-erased GPIO pin with support for inverted signalling.
-//! - [DummyPin] is a useful for cases where peripheral driver requires a pin,
-//!   but real pin cannot be used.
+//! - [NoPin] is a useful for cases where peripheral driver requires a pin, but
+//!   real pin cannot be used.
 //!
 //! ### GPIO interconnect
 //!
@@ -77,10 +77,10 @@ use crate::{
     InterruptConfigurable,
 };
 
-mod dummy_pin;
 pub mod interconnect;
+mod placeholder;
 
-pub use dummy_pin::DummyPin;
+pub use placeholder::NoPin;
 
 #[cfg(soc_etm)]
 pub mod etm;
@@ -90,7 +90,6 @@ pub mod lp_io;
 pub mod rtc_io;
 
 /// Convenience constant for `Option::None` pin
-pub const NO_PIN: Option<DummyPin> = None;
 
 static USER_INTERRUPT_HANDLER: CFnPtr = CFnPtr::NULL;
 
@@ -147,6 +146,14 @@ pub enum WakeEvent {
 }
 
 /// Digital input or output level.
+///
+/// `Level` can be used to control a GPIO output, and it can act as a peripheral
+/// signal and be connected to peripheral inputs and outputs.
+///
+/// When connected to a peripheral
+/// 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)]
 #[cfg_attr(feature = "defmt", derive(defmt::Format))]
 pub enum Level {

esp-hal/src/gpio/dummy_pin.rs → esp-hal/src/gpio/placeholder.rs

@@ -1,34 +1,17 @@
-//! Placeholder pins.
+//! Placeholder pins/signals.
 //!
 //! These are useful to pass them into peripheral drivers where you don't want
 //! an actual pin but one is required.
+// This module also contains peripheral signal impls for `Level` to avoid
+// polluting the main module.
 
 use super::*;
 
-/// DummyPin, not useful everywhere as it panics if number() is called
-#[derive(Default, Clone)]
-pub struct DummyPin {
-    value: bool,
-}
-
-impl DummyPin {
-    /// Create a dummy pin.
-    pub fn new() -> Self {
-        Self { value: false }
-    }
-}
-
-impl crate::peripheral::Peripheral for DummyPin {
-    type P = Self;
-
-    unsafe fn clone_unchecked(&mut self) -> Self::P {
-        Self { value: self.value }
-    }
+impl PeripheralSignal for Level {
+    fn pull_direction(&self, _pull: Pull, _internal: private::Internal) {}
 }
 
-impl private::Sealed for DummyPin {}
-
-impl PeripheralInput for DummyPin {
+impl PeripheralInput for Level {
     fn input_signals(&self, _: private::Internal) -> [Option<InputSignal>; 6] {
         [None; 6]
     }
@@ -40,11 +23,14 @@ impl PeripheralInput for DummyPin {
     fn enable_input_in_sleep_mode(&mut self, _on: bool, _: private::Internal) {}
 
     fn is_input_high(&self, _: private::Internal) -> bool {
-        self.value
+        *self == Level::High
     }
 
     fn connect_input_to_peripheral(&mut self, signal: InputSignal, _: private::Internal) {
-        let value = if self.value { ONE_INPUT } else { ZERO_INPUT };
+        let value = match self {
+            Level::High => ONE_INPUT,
+            Level::Low => ZERO_INPUT,
+        };
 
         unsafe { &*GPIO::PTR }
             .func_in_sel_cfg(signal as usize - FUNC_IN_SEL_OFFSET)
@@ -61,48 +47,14 @@ impl PeripheralInput for DummyPin {
     fn disconnect_input_from_peripheral(&mut self, _signal: InputSignal, _: private::Internal) {}
 }
 
-impl PeripheralSignal for Level {
-    delegate::delegate! {
-        to match self {
-            Level::High => DummyPin { value: true },
-            Level::Low => DummyPin { value: false },
-        } {
-            fn pull_direction(&self, pull: Pull, _internal: private::Internal);
-        }
-    }
-}
-
-impl PeripheralInput for Level {
-    delegate::delegate! {
-        to match self {
-            Level::High => DummyPin { value: true },
-            Level::Low => DummyPin { value: false },
-        } {
-            fn init_input(&self, _pull: Pull, _internal: private::Internal);
-            fn enable_input(&mut self, _on: bool, _internal: private::Internal);
-            fn enable_input_in_sleep_mode(&mut self, _on: bool, _internal: private::Internal);
-            fn is_input_high(&self, _internal: private::Internal) -> bool;
-            fn connect_input_to_peripheral(&mut self, _signal: InputSignal, _internal: private::Internal);
-            fn disconnect_input_from_peripheral(&mut self, _signal: InputSignal, _internal: private::Internal);
-            fn input_signals(&self, _internal: private::Internal) -> [Option<InputSignal>; 6];
-        }
-    }
-}
-
-impl PeripheralSignal for DummyPin {
-    fn pull_direction(&self, _pull: Pull, _internal: private::Internal) {}
-}
-
-impl PeripheralOutput for DummyPin {
+impl PeripheralOutput for Level {
     fn set_to_open_drain_output(&mut self, _: private::Internal) {}
 
     fn set_to_push_pull_output(&mut self, _: private::Internal) {}
 
     fn enable_output(&mut self, _on: bool, _: private::Internal) {}
 
-    fn set_output_high(&mut self, on: bool, _: private::Internal) {
-        self.value = on;
-    }
+    fn set_output_high(&mut self, _on: bool, _: private::Internal) {}
 
     fn set_drive_strength(&mut self, _strength: DriveStrength, _: private::Internal) {}
 
@@ -115,7 +67,7 @@ impl PeripheralOutput for DummyPin {
     fn internal_pull_down_in_sleep_mode(&mut self, _on: bool, _: private::Internal) {}
 
     fn is_set_high(&self, _: private::Internal) -> bool {
-        self.value
+        false
     }
 
     fn output_signals(&self, _: private::Internal) -> [Option<OutputSignal>; 6] {
@@ -127,49 +79,99 @@ impl PeripheralOutput for DummyPin {
     fn disconnect_from_peripheral_output(&mut self, _signal: OutputSignal, _: private::Internal) {}
 }
 
-impl embedded_hal_02::digital::v2::OutputPin for DummyPin {
+/// Placeholder pin, used when no pin is required when using a peripheral.
+///
+/// When used as a peripheral signal, `NoPin` is equivalent to [`Level::Low`].
+#[derive(Default, Clone, Copy)]
+pub struct NoPin;
+
+impl crate::peripheral::Peripheral for NoPin {
+    type P = Self;
+
+    unsafe fn clone_unchecked(&mut self) -> Self::P {
+        Self
+    }
+}
+
+impl private::Sealed for NoPin {}
+
+impl PeripheralSignal for NoPin {
+    fn pull_direction(&self, _pull: Pull, _internal: private::Internal) {}
+}
+
+impl PeripheralInput for NoPin {
+    delegate::delegate! {
+        to Level::Low {
+            fn init_input(&self, _pull: Pull, _internal: private::Internal);
+            fn enable_input(&mut self, _on: bool, _internal: private::Internal);
+            fn enable_input_in_sleep_mode(&mut self, _on: bool, _internal: private::Internal);
+            fn is_input_high(&self, _internal: private::Internal) -> bool;
+            fn connect_input_to_peripheral(&mut self, _signal: InputSignal, _internal: private::Internal);
+            fn disconnect_input_from_peripheral(&mut self, _signal: InputSignal, _internal: private::Internal);
+            fn input_signals(&self, _internal: private::Internal) -> [Option<InputSignal>; 6];
+        }
+    }
+}
+
+impl PeripheralOutput for NoPin {
+    delegate::delegate! {
+        to Level::Low {
+            fn set_to_open_drain_output(&mut self, _internal: private::Internal);
+            fn set_to_push_pull_output(&mut self, _internal: private::Internal);
+            fn enable_output(&mut self, _on: bool, _internal: private::Internal);
+            fn set_output_high(&mut self, _on: bool, _internal: private::Internal);
+            fn set_drive_strength(&mut self, _strength: DriveStrength, _internal: private::Internal);
+            fn enable_open_drain(&mut self, _on: bool, _internal: private::Internal);
+            fn enable_output_in_sleep_mode(&mut self, _on: bool, _internal: private::Internal);
+            fn internal_pull_up_in_sleep_mode(&mut self, _on: bool, _internal: private::Internal);
+            fn internal_pull_down_in_sleep_mode(&mut self, _on: bool, _internal: private::Internal);
+            fn is_set_high(&self, _internal: private::Internal) -> bool;
+            fn output_signals(&self, _internal: private::Internal) -> [Option<OutputSignal>; 6];
+            fn connect_peripheral_to_output(&mut self, _signal: OutputSignal, _internal: private::Internal);
+            fn disconnect_from_peripheral_output(&mut self, _signal: OutputSignal, _internal: private::Internal);
+        }
+    }
+}
+
+impl embedded_hal_02::digital::v2::OutputPin for NoPin {
     type Error = core::convert::Infallible;
 
     fn set_high(&mut self) -> Result<(), Self::Error> {
-        self.set_output_high(true, private::Internal);
         Ok(())
     }
     fn set_low(&mut self) -> Result<(), Self::Error> {
-        self.set_output_high(false, private::Internal);
         Ok(())
     }
 }
-impl embedded_hal_02::digital::v2::StatefulOutputPin for DummyPin {
+impl embedded_hal_02::digital::v2::StatefulOutputPin for NoPin {
     fn is_set_high(&self) -> Result<bool, Self::Error> {
-        Ok(PeripheralOutput::is_set_high(self, private::Internal))
+        Ok(false)
     }
     fn is_set_low(&self) -> Result<bool, Self::Error> {
-        Ok(!PeripheralOutput::is_set_high(self, private::Internal))
+        Ok(false)
     }
 }
 
-impl embedded_hal::digital::ErrorType for DummyPin {
+impl embedded_hal::digital::ErrorType for NoPin {
     type Error = core::convert::Infallible;
 }
 
-impl embedded_hal::digital::OutputPin for DummyPin {
+impl embedded_hal::digital::OutputPin for NoPin {
     fn set_low(&mut self) -> Result<(), Self::Error> {
-        self.set_output_high(true, private::Internal);
         Ok(())
     }
 
     fn set_high(&mut self) -> Result<(), Self::Error> {
-        self.set_output_high(false, private::Internal);
         Ok(())
     }
 }
 
-impl embedded_hal::digital::StatefulOutputPin for DummyPin {
+impl embedded_hal::digital::StatefulOutputPin for NoPin {
     fn is_set_high(&mut self) -> Result<bool, Self::Error> {
-        Ok(PeripheralOutput::is_set_high(self, private::Internal))
+        Ok(false)
     }
 
     fn is_set_low(&mut self) -> Result<bool, Self::Error> {
-        Ok(!PeripheralOutput::is_set_high(self, private::Internal))
+        Ok(false)
     }
 }