From 468141e05ab79fc13a7702057cc3740a8350db32 Mon Sep 17 00:00:00 2001 From: ryan Date: Fri, 16 Oct 2020 10:35:26 +1300 Subject: [PATCH 1/4] Refactor PWM for simpler generic use --- src/pwm.rs | 51 +++++++++++++++++++++++---------------------------- 1 file changed, 23 insertions(+), 28 deletions(-) diff --git a/src/pwm.rs b/src/pwm.rs index 203feafd5..f3cdc4f39 100644 --- a/src/pwm.rs +++ b/src/pwm.rs @@ -1,5 +1,7 @@ //! Pulse Width Modulation +use core::time::Duration; + /// Pulse Width Modulation /// /// # Examples @@ -9,6 +11,7 @@ /// ``` /// extern crate embedded_hal as hal; /// +/// use core::time::Duration; /// use hal::prelude::*; /// /// fn main() { @@ -22,47 +25,39 @@ /// let max_duty = pwm.try_get_max_duty().unwrap(); /// /// // brightest LED -/// pwm.try_set_duty(Channel::_1, max_duty).unwrap(); +/// pwm.try_set_duty(0, max_duty).unwrap(); /// /// // dimmer LED -/// pwm.try_set_duty(Channel::_2, max_duty / 4).unwrap(); +/// pwm.try_set_duty(1, max_duty / 4).unwrap(); /// } /// /// # use core::convert::Infallible; -/// # struct KiloHertz(u32); -/// # trait U32Ext { fn khz(self) -> KiloHertz; } -/// # impl U32Ext for u32 { fn khz(self) -> KiloHertz { KiloHertz(self) } } +/// # trait U32Ext { fn khz(self) -> Duration; } +/// # impl U32Ext for u32 { fn khz(self) -> Duration { Duration::from_nanos( 1_000_000u64 / self as u64 ) } } /// # enum Channel { _1, _2 } /// # struct Pwm1; /// # impl hal::pwm::Pwm for Pwm1 { +/// # const CHANNELS: usize = 4; /// # type Error = Infallible; -/// # type Channel = Channel; -/// # type Time = KiloHertz; /// # type Duty = u16; -/// # fn try_disable(&mut self, _: Channel) -> Result<(), Self::Error> { unimplemented!() } -/// # fn try_enable(&mut self, _: Channel) -> Result<(), Self::Error> { unimplemented!() } -/// # fn try_get_duty(&self, _: Channel) -> Result { unimplemented!() } +/// # fn try_disable(&mut self, _: usize) -> Result<(), Self::Error> { unimplemented!() } +/// # fn try_enable(&mut self, _: usize) -> Result<(), Self::Error> { unimplemented!() } +/// # fn try_get_duty(&self, _: usize) -> Result { unimplemented!() } /// # fn try_get_max_duty(&self) -> Result { Ok(0) } -/// # fn try_set_duty(&mut self, _: Channel, _: u16) -> Result<(), Self::Error> { Ok(()) } -/// # fn try_get_period(&self) -> Result { unimplemented!() } -/// # fn try_set_period(&mut self, _: T) -> Result<(), Self::Error> where T: Into { Ok(()) } +/// # fn try_set_duty(&mut self, _: usize, _: u16) -> Result<(), Self::Error> { Ok(()) } +/// # fn try_get_period(&self) -> Result { unimplemented!() } +/// # fn try_set_period(&mut self, _: T) -> Result<(), Self::Error> where T: Into { Ok(()) } /// # } /// ``` // unproven reason: pre-singletons API. The `PwmPin` trait seems more useful because it models independent // PWM channels. Here a certain number of channels are multiplexed in a single implementer. pub trait Pwm { + /// Number of available channels + const CHANNELS: usize; + /// Enumeration of `Pwm` errors type Error; - /// Enumeration of channels that can be used with this `Pwm` interface - /// - /// If your `Pwm` interface has no channels you can use the type `()` - /// here - type Channel; - - /// A time unit that can be converted into a human time unit (e.g. seconds) - type Time; - /// Type for the `duty` methods /// /// The implementer is free to choose a float / percentage representation @@ -70,31 +65,31 @@ pub trait Pwm { type Duty; /// Disables a PWM `channel` - fn try_disable(&mut self, channel: Self::Channel) -> Result<(), Self::Error>; + fn try_disable(&mut self, channel: usize) -> Result<(), Self::Error>; /// Enables a PWM `channel` - fn try_enable(&mut self, channel: Self::Channel) -> Result<(), Self::Error>; + fn try_enable(&mut self, channel: usize) -> Result<(), Self::Error>; /// Returns the current PWM period - fn try_get_period(&self) -> Result; + fn try_get_period(&self) -> Result; /// Returns the current duty cycle /// /// While the pin is transitioning to the new duty cycle after a `try_set_duty` call, this may /// return the old or the new duty cycle depending on the implementation. - fn try_get_duty(&self, channel: Self::Channel) -> Result; + fn try_get_duty(&self, channel: usize) -> Result; /// Returns the maximum duty cycle value fn try_get_max_duty(&self) -> Result; /// Sets a new duty cycle - fn try_set_duty(&mut self, channel: Self::Channel, duty: Self::Duty) + fn try_set_duty(&mut self, channel: usize, duty: Self::Duty) -> Result<(), Self::Error>; /// Sets a new PWM period fn try_set_period

(&mut self, period: P) -> Result<(), Self::Error> where - P: Into; + P: Into; } /// A single PWM channel / pin From 9183f60cab5694d21cbe3e64ef590a6480108501 Mon Sep 17 00:00:00 2001 From: ryan Date: Fri, 16 Oct 2020 10:47:34 +1300 Subject: [PATCH 2/4] run fmt --- src/pwm.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/pwm.rs b/src/pwm.rs index f3cdc4f39..775f83bb0 100644 --- a/src/pwm.rs +++ b/src/pwm.rs @@ -83,8 +83,7 @@ pub trait Pwm { fn try_get_max_duty(&self) -> Result; /// Sets a new duty cycle - fn try_set_duty(&mut self, channel: usize, duty: Self::Duty) - -> Result<(), Self::Error>; + fn try_set_duty(&mut self, channel: usize, duty: Self::Duty) -> Result<(), Self::Error>; /// Sets a new PWM period fn try_set_period

(&mut self, period: P) -> Result<(), Self::Error> From abe693bd648856b327ab6b3059f9641bc07b084a Mon Sep 17 00:00:00 2001 From: ryan Date: Sat, 17 Oct 2020 14:14:10 +1300 Subject: [PATCH 3/4] split pwm traits, example with markers and split/join --- src/pwm.rs | 118 +++++++++++++++++++++++++++++++++++------------------ 1 file changed, 78 insertions(+), 40 deletions(-) diff --git a/src/pwm.rs b/src/pwm.rs index 775f83bb0..bdf422e03 100644 --- a/src/pwm.rs +++ b/src/pwm.rs @@ -12,98 +12,136 @@ use core::time::Duration; /// extern crate embedded_hal as hal; /// /// use core::time::Duration; -/// use hal::prelude::*; +/// # use core::marker::PhantomData; +/// use hal::pwm::*; /// /// fn main() { -/// let mut pwm: Pwm1 = { +/// let mut pwm = { /// // .. -/// # Pwm1 +/// # Pwm1(HasPins) /// }; /// /// pwm.try_set_period(1.khz()).unwrap(); /// -/// let max_duty = pwm.try_get_max_duty().unwrap(); +/// let max_duty = Pwm1::::MAX_DUTY; /// /// // brightest LED /// pwm.try_set_duty(0, max_duty).unwrap(); /// /// // dimmer LED /// pwm.try_set_duty(1, max_duty / 4).unwrap(); +/// +/// let (pwm, [mut ch1, mut ch2]) = pwm.split(); +/// +/// ch1.try_set_duty(max_duty / 5).unwrap(); +/// ch2.try_set_duty(max_duty / 5).unwrap(); +/// /// } /// /// # use core::convert::Infallible; /// # trait U32Ext { fn khz(self) -> Duration; } /// # impl U32Ext for u32 { fn khz(self) -> Duration { Duration::from_nanos( 1_000_000u64 / self as u64 ) } } -/// # enum Channel { _1, _2 } -/// # struct Pwm1; -/// # impl hal::pwm::Pwm for Pwm1 { +/// // Note: possibly you could genericise this over [PwmPinImpl; CHANNELS], which would be great for accessing channels internally +/// // except, these are probably not all going to be the same type, so instead we're using marker traits to specify whether it has been split or not +/// # struct Pwm1(S); +/// # struct Pwm1Pin; +/// # struct HasPins; +/// # impl Pwm1 { +/// # pub fn split(self) -> (Pwm1<()>, [Pwm1Pin; 2]) { +/// # (Pwm1(()), [Pwm1Pin, Pwm1Pin]) +/// # } +/// # } +/// # impl Pwm1<()> { +/// # pub fn join(self, pins: [Pwm1Pin; 2]) -> Pwm1 { +/// # Pwm1(HasPins) +/// # } +/// # } +/// # +/// # impl hal::pwm::Pwm for Pwm1 { /// # const CHANNELS: usize = 4; +/// # const MAX_DUTY: u16 = 1024; /// # type Error = Infallible; -/// # type Duty = u16; +/// # /// # fn try_disable(&mut self, _: usize) -> Result<(), Self::Error> { unimplemented!() } /// # fn try_enable(&mut self, _: usize) -> Result<(), Self::Error> { unimplemented!() } -/// # fn try_get_duty(&self, _: usize) -> Result { unimplemented!() } -/// # fn try_get_max_duty(&self) -> Result { Ok(0) } -/// # fn try_set_duty(&mut self, _: usize, _: u16) -> Result<(), Self::Error> { Ok(()) } /// # fn try_get_period(&self) -> Result { unimplemented!() } /// # fn try_set_period(&mut self, _: T) -> Result<(), Self::Error> where T: Into { Ok(()) } /// # } +/// # +/// # impl hal::pwm::PwmDuty for Pwm1 { +/// # fn try_get_duty(&self, _: usize) -> Result { unimplemented!() } +/// # fn try_set_duty(&mut self, _: usize, _: D) -> Result<(), Self::Error> where D: Into { Ok(()) } +/// # } +/// # impl hal::pwm::PwmPin for Pwm1Pin { +/// # const MAX_DUTY: u16 = 1024; +/// # type Error = Infallible; +/// # fn try_disable(&mut self) -> Result<(), Self::Error> { unimplemented!() } +/// # fn try_enable(&mut self) -> Result<(), Self::Error> { unimplemented!() } +/// # fn try_get_duty(&self) -> Result { unimplemented!() } +/// # fn try_set_duty(&mut self, _: D) -> Result<(), Self::Error> where D: Into { Ok(()) } +/// # } /// ``` // unproven reason: pre-singletons API. The `PwmPin` trait seems more useful because it models independent // PWM channels. Here a certain number of channels are multiplexed in a single implementer. -pub trait Pwm { +pub trait Pwm { /// Number of available channels const CHANNELS: usize; + /// Maximum duty cycle value + const MAX_DUTY: Duty; + /// Enumeration of `Pwm` errors type Error; - /// Type for the `duty` methods - /// - /// The implementer is free to choose a float / percentage representation - /// (e.g. `0.0 .. 1.0`) or an integer representation (e.g. `0 .. 65535`) - type Duty; - /// Disables a PWM `channel` fn try_disable(&mut self, channel: usize) -> Result<(), Self::Error>; /// Enables a PWM `channel` fn try_enable(&mut self, channel: usize) -> Result<(), Self::Error>; + // Note: should you be able to _set_ the period once the channels have been split? + // my feeling is, probably not + /// Returns the current PWM period fn try_get_period(&self) -> Result; + /// Sets a new PWM period + fn try_set_period

(&mut self, period: P) -> Result<(), Self::Error> + where + P: Into; + + // Note: technically there could be a `channel` or `split` method here but this is + // rather extremely difficult prior to const-generics landing (and CHANNELS would need to be a const generic) +} + +/// PwmDuty trait allows PWM pins duty-cycles to be set per-channel +/// +/// This should be implemented for a `Pwm` type that currently holds channel references +pub trait PwmDuty: Pwm { + /// Returns the current duty cycle /// /// While the pin is transitioning to the new duty cycle after a `try_set_duty` call, this may /// return the old or the new duty cycle depending on the implementation. - fn try_get_duty(&self, channel: usize) -> Result; - - /// Returns the maximum duty cycle value - fn try_get_max_duty(&self) -> Result; + fn try_get_duty(&self, channel: usize) -> Result; /// Sets a new duty cycle - fn try_set_duty(&mut self, channel: usize, duty: Self::Duty) -> Result<(), Self::Error>; - - /// Sets a new PWM period - fn try_set_period

(&mut self, period: P) -> Result<(), Self::Error> + fn try_set_duty(&mut self, channel: usize, duty: D) -> Result<(), Self::Error> where - P: Into; + D: Into; } /// A single PWM channel / pin /// -/// See `Pwm` for details -pub trait PwmPin { +/// This trait should be implemented over split PWM channels, see `Pwm` for details +pub trait PwmPin { + + /// Maximum duty cycle value + const MAX_DUTY: Duty; + /// Enumeration of `PwmPin` errors type Error; - /// Type for the `duty` methods - /// - /// The implementer is free to choose a float / percentage representation - /// (e.g. `0.0 .. 1.0`) or an integer representation (e.g. `0 .. 65535`) - type Duty; - /// Disables a PWM `channel` fn try_disable(&mut self) -> Result<(), Self::Error>; @@ -114,11 +152,11 @@ pub trait PwmPin { /// /// While the pin is transitioning to the new duty cycle after a `try_set_duty` call, this may /// return the old or the new duty cycle depending on the implementation. - fn try_get_duty(&self) -> Result; - - /// Returns the maximum duty cycle value - fn try_get_max_duty(&self) -> Result; + fn try_get_duty(&self) -> Result; /// Sets a new duty cycle - fn try_set_duty(&mut self, duty: Self::Duty) -> Result<(), Self::Error>; + fn try_set_duty(&mut self, duty: D) -> Result<(), Self::Error> + where + D: Into; } + From f10ce7c4a1f5e4941828e1eeafda59ecd4c06a9c Mon Sep 17 00:00:00 2001 From: ryan Date: Sat, 17 Oct 2020 14:23:35 +1300 Subject: [PATCH 4/4] remove associated type bounds for pwm functions swap to concrete types for Duty and Period to support boxing / dyn dispatch of different types --- src/pwm.rs | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/pwm.rs b/src/pwm.rs index bdf422e03..888059341 100644 --- a/src/pwm.rs +++ b/src/pwm.rs @@ -65,12 +65,12 @@ use core::time::Duration; /// # fn try_disable(&mut self, _: usize) -> Result<(), Self::Error> { unimplemented!() } /// # fn try_enable(&mut self, _: usize) -> Result<(), Self::Error> { unimplemented!() } /// # fn try_get_period(&self) -> Result { unimplemented!() } -/// # fn try_set_period(&mut self, _: T) -> Result<(), Self::Error> where T: Into { Ok(()) } +/// # fn try_set_period(&mut self, _: Duration) -> Result<(), Self::Error> { Ok(()) } /// # } /// # /// # impl hal::pwm::PwmDuty for Pwm1 { /// # fn try_get_duty(&self, _: usize) -> Result { unimplemented!() } -/// # fn try_set_duty(&mut self, _: usize, _: D) -> Result<(), Self::Error> where D: Into { Ok(()) } +/// # fn try_set_duty(&mut self, _: usize, _: u16) -> Result<(), Self::Error> { Ok(()) } /// # } /// # impl hal::pwm::PwmPin for Pwm1Pin { /// # const MAX_DUTY: u16 = 1024; @@ -78,7 +78,7 @@ use core::time::Duration; /// # fn try_disable(&mut self) -> Result<(), Self::Error> { unimplemented!() } /// # fn try_enable(&mut self) -> Result<(), Self::Error> { unimplemented!() } /// # fn try_get_duty(&self) -> Result { unimplemented!() } -/// # fn try_set_duty(&mut self, _: D) -> Result<(), Self::Error> where D: Into { Ok(()) } +/// # fn try_set_duty(&mut self, _: u16) -> Result<(), Self::Error> { Ok(()) } /// # } /// ``` // unproven reason: pre-singletons API. The `PwmPin` trait seems more useful because it models independent @@ -106,9 +106,7 @@ pub trait Pwm { fn try_get_period(&self) -> Result; /// Sets a new PWM period - fn try_set_period

(&mut self, period: P) -> Result<(), Self::Error> - where - P: Into; + fn try_set_period(&mut self, period: Duration) -> Result<(), Self::Error>; // Note: technically there could be a `channel` or `split` method here but this is // rather extremely difficult prior to const-generics landing (and CHANNELS would need to be a const generic) @@ -126,9 +124,7 @@ pub trait PwmDuty: Pwm { fn try_get_duty(&self, channel: usize) -> Result; /// Sets a new duty cycle - fn try_set_duty(&mut self, channel: usize, duty: D) -> Result<(), Self::Error> - where - D: Into; + fn try_set_duty(&mut self, channel: usize, duty: Duty) -> Result<(), Self::Error>; } /// A single PWM channel / pin @@ -155,8 +151,6 @@ pub trait PwmPin { fn try_get_duty(&self) -> Result; /// Sets a new duty cycle - fn try_set_duty(&mut self, duty: D) -> Result<(), Self::Error> - where - D: Into; + fn try_set_duty(&mut self, duty: Duty) -> Result<(), Self::Error>; }