better not as a constructor?

embassy-nrf/src/pwm.rs

@@ -55,7 +55,7 @@ pub struct Pwm<'d, T: Instance> {
 }
 
 #[derive(Debug, Eq, PartialEq, Clone, Copy)]
-pub enum LoopMode {
+pub enum SequenceMode {
     /// Run sequence n Times total
     Times(u16),
     /// Repeat until `stop` is called.
@@ -63,7 +63,7 @@ pub enum LoopMode {
 }
 
 /// Configure an infinite looping sequence for `simple_playback`
-pub struct LoopingConfig<'a> {
+pub struct SequenceConfig<'a> {
     /// Selects up mode or up-and-down mode for the counter
     pub counter_mode: CounterMode,
     // Top value to be compared against buffer values
@@ -79,7 +79,7 @@ pub struct LoopingConfig<'a> {
     /// Number of Times PWM periods after the sequence ends before starting the next sequence
     pub end_delay: u32,
     /// How many times to play the sequence
-    pub times: LoopMode,
+    pub times: SequenceMode,
 }
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
@@ -132,6 +132,9 @@ impl<'d, T: Instance> Pwm<'d, T> {
             pin.set_low();
             pin.conf().write(|w| w.dir().output());
         }
+
+        // if NoPin provided writes disconnected (top bit 1) 0x80000000 else
+        // writes pin number ex 13 (0x0D) which is connected (top bit 0)
         r.psel.out[0].write(|w| unsafe { w.bits(ch0.psel_bits()) });
         r.psel.out[1].write(|w| unsafe { w.bits(ch1.psel_bits()) });
         r.psel.out[2].write(|w| unsafe { w.bits(ch2.psel_bits()) });
@@ -166,54 +169,18 @@ impl<'d, T: Instance> Pwm<'d, T> {
     }
 
     /// Returns a configured pwm that has had start called on it
-    pub fn simple_playback(
+    pub fn play_sequence(&self, config: SequenceConfig) -> Result<(), Error> {
-        _pwm: impl Unborrow<Target = T> + 'd,
-        ch0: impl Unborrow<Target = impl GpioOptionalPin> + 'd,
-        ch1: impl Unborrow<Target = impl GpioOptionalPin> + 'd,
-        ch2: impl Unborrow<Target = impl GpioOptionalPin> + 'd,
-        ch3: impl Unborrow<Target = impl GpioOptionalPin> + 'd,
-        config: LoopingConfig,
-    ) -> Result<Self, Error> {
         slice_in_ram_or(config.sequence, Error::DMABufferNotInDataMemory)?;
 
         if config.sequence.len() > 32767 {
             return Err(Error::SequenceTooLong);
         }
-        if let LoopMode::Times(0) = config.times {
+        if let SequenceMode::Times(0) = config.times {
             return Err(Error::SequenceTooShort);
         }
 
-        unborrow!(ch0, ch1, ch2, ch3);
-
         let r = T::regs();
 
-        if let Some(pin) = ch0.pin_mut() {
-            pin.set_low();
-            pin.conf().write(|w| w.dir().output());
-        }
-
-        if let Some(pin) = ch1.pin_mut() {
-            pin.set_low();
-            pin.conf().write(|w| w.dir().output());
-        }
-        if let Some(pin) = ch2.pin_mut() {
-            pin.set_low();
-            pin.conf().write(|w| w.dir().output());
-        }
-        if let Some(pin) = ch3.pin_mut() {
-            pin.set_low();
-            pin.conf().write(|w| w.dir().output());
-        }
-
-        // if NoPin provided writes disconnected (top bit 1) 0x80000000 else
-        // writes pin number ex 13 (0x0D) which is connected (top bit 0)
-        r.psel.out[0].write(|w| unsafe { w.bits(ch0.psel_bits()) });
-        r.psel.out[1].write(|w| unsafe { w.bits(ch1.psel_bits()) });
-        r.psel.out[2].write(|w| unsafe { w.bits(ch2.psel_bits()) });
-        r.psel.out[3].write(|w| unsafe { w.bits(ch3.psel_bits()) });
-
-        r.enable.write(|w| w.enable().enabled());
-
         r.mode
             .write(|w| unsafe { w.bits(config.counter_mode as u32) });
         r.prescaler
@@ -250,7 +217,7 @@ impl<'d, T: Instance> Pwm<'d, T> {
 
         match config.times {
             // just the one time, no loop count
-            LoopMode::Times(1) => {
+            SequenceMode::Times(1) => {
                 r.loop_.write(|w| w.cnt().disabled());
                 // tasks_seqstart doesnt exist in all svds so write its bit instead
                 r.tasks_seqstart[0].write(|w| unsafe { w.bits(0x01) });
@@ -258,7 +225,7 @@ impl<'d, T: Instance> Pwm<'d, T> {
             // loop count is how many times to play BOTH sequences
             // 2 total  (1 x 2)
             // 3 total, (2 x 2) - 1
-            LoopMode::Times(n) => {
+            SequenceMode::Times(n) => {
                 let odd = n & 1 == 1;
                 let times = if odd { (n / 2) + 1 } else { n / 2 };
 
@@ -273,17 +240,14 @@ impl<'d, T: Instance> Pwm<'d, T> {
                 }
             }
             // to play infinitely, repeat the sequence one time, then have loops done self trigger seq0 again
-            LoopMode::Infinite => {
+            SequenceMode::Infinite => {
                 r.loop_.write(|w| unsafe { w.cnt().bits(0x1) });
                 r.shorts.write(|w| w.loopsdone_seqstart0().enabled());
                 // tasks_seqstart doesnt exist in all svds so write its bit instead
                 r.tasks_seqstart[0].write(|w| unsafe { w.bits(0x01) });
             }
         }
-
+        Ok(())
-        Ok(Self {
-            phantom: PhantomData,
-        })
     }
 
     /// Stop playback

examples/nrf/src/bin/pwm_sequence.rs

@@ -7,7 +7,7 @@ mod example_common;
 use defmt::*;
 use embassy::executor::Spawner;
 use embassy::time::{Duration, Timer};
-use embassy_nrf::pwm::{CounterMode, LoopMode, LoopingConfig, Prescaler, Pwm, SequenceLoad};
+use embassy_nrf::pwm::{CounterMode, Prescaler, Pwm, SequenceConfig, SequenceLoad, SequenceMode};
 use embassy_nrf::Peripherals;
 
 #[embassy::main]
@@ -16,7 +16,7 @@ async fn main(_spawner: Spawner, p: Peripherals) {
         0x8000, 0, 0, 0, 0, 0x8000, 0, 0, 0, 0, 0x8000, 0, 0, 0, 0, 0x8000,
     ];
 
-    let config = LoopingConfig {
+    let config = SequenceConfig {
         counter_mode: CounterMode::Up,
         top: 15625,
         prescaler: Prescaler::Div128,
@@ -24,12 +24,11 @@ async fn main(_spawner: Spawner, p: Peripherals) {
         sequence_load: SequenceLoad::Individual,
         refresh: 0,
         end_delay: 0,
-        times: LoopMode::Times(5),
+        times: SequenceMode::Times(5),
     };
 
-    let _pwm = unwrap!(Pwm::simple_playback(
+    let pwm = Pwm::new(p.PWM0, p.P0_13, p.P0_15, p.P0_16, p.P0_14);
-        p.PWM0, p.P0_13, p.P0_15, p.P0_16, p.P0_14, config
+    unwrap!(pwm.play_sequence(config));
-    ));
     info!("pwm started!");
 
     loop {

examples/nrf/src/bin/pwm_simple_sin.rs

@@ -9,7 +9,7 @@ use defmt::*;
 use embassy::executor::Spawner;
 use embassy::time::{Duration, Timer};
 use embassy_nrf::gpio::NoPin;
-use embassy_nrf::pwm::{CounterMode, LoopMode, LoopingConfig, Prescaler, Pwm, SequenceLoad};
+use embassy_nrf::pwm::{CounterMode, Prescaler, Pwm, SequenceConfig, SequenceLoad, SequenceMode};
 use embassy_nrf::Peripherals;
 use micromath::F32Ext;
 
@@ -20,7 +20,7 @@ async fn main(_spawner: Spawner, p: Peripherals) {
     // probably not best use of resources to create the table at runtime, but makes testing fast
     let seq_values: [u16; 220] = core::array::from_fn(|n| ((W1 * n as f32).sin() * 10000.0) as u16);
 
-    let config = LoopingConfig {
+    let config = SequenceConfig {
         counter_mode: CounterMode::UpAndDown,
         top: 12000,
         prescaler: Prescaler::Div16,
@@ -28,12 +28,11 @@ async fn main(_spawner: Spawner, p: Peripherals) {
         sequence_load: SequenceLoad::Common,
         refresh: 0,
         end_delay: 0,
-        times: LoopMode::Infinite,
+        times: SequenceMode::Infinite,
     };
 
-    let pwm = unwrap!(Pwm::simple_playback(
+    let pwm = Pwm::new(p.PWM0, p.P0_13, NoPin, NoPin, NoPin);
-        p.PWM0, p.P0_13, NoPin, NoPin, NoPin, config
+    unwrap!(pwm.play_sequence(config));
-    ));
     info!("pwm started!");
 
     Timer::after(Duration::from_millis(20000)).await;