rust-embedded / embedded-hal

A Hardware Abstraction Layer (HAL) for embedded systems
Apache License 2.0
2.01k stars 202 forks source link

Tracking issue for PWM traits #358

Open eldruin opened 2 years ago

eldruin commented 2 years ago

The PWM traits available on the 0.2.x versions have been removed ahead of the 1.0.0 release. See: #357 This issue servers as a tracking issue until we add them back. The open (breaking) problems should be reasonably-well solved and all associated types should have appropriate bounds that enable generic code use.

PWM traits as of the 0.2.7 release:

As noted below, the PwmPin only has a Duty associated type and does not need a settlement on the Time type as Pwm does.

Relevant issues/PRs:

Please feel free to participate, highlight your current use cases, problems and provide solutions.

burrbull commented 2 years ago

As PwmPin doesn't have Time and only Duty, I think it is not so big issue. We just need to decide what type to use. Float is bad idea as timers are discrete. And could always be evaluated as (get_duty() as f32)/(get_max_duty() as f32). So u16 or u32? I think in most cases u16 is enough. But 32-bit timers can support both u16 and u32.

What if we move duty from associated type to generic and make u16 default type (like with I2c address type)?

pub trait PwmPin<Duty=u16> {
    fn disable(&mut self);
    fn enable(&mut self);
    fn get_duty(&self) -> Duty;
    fn get_max_duty(&self) -> Duty;
    fn set_duty(&mut self, duty: Duty);
}

In this case we "say" that u16 is required and other types are optional.

Alternative: return u32 always (like with Delay):

pub trait PwmPin {
    fn disable(&mut self);
    fn enable(&mut self);
    fn get_duty(&self) -> u32;
    fn get_max_duty(&self) -> u32;
    fn set_duty(&mut self, duty: u32);
}

But in this case we have potential problems with set_duty on 16-bit timers.

sirhcel commented 2 years ago

My gut instinct favors a generic Duty:

rursprung commented 1 year ago

i just ran into this (cf. https://github.com/dbrgn/embedded-hal-mock/pull/52#issuecomment-1336983674) as i so far use e-h 0.x which still has PwmPin. i understand the time-related issues and that this doesn't affect PwmPin itself and also see that there are solution suggestions here by @burrbull. is there anything specific still blocking re-adding PwmPin to e-h 1 with a generic Duty?

this affects e.g. drivers for typical h-bridge motor drivers, see e.g. the l298n crate or my own tb6612fng crate (both based on e-h 0.x). i had to hardcode the duty in my crate for the moment (it isn't released yet and i'm just using it on a single board), having the duty as a generic would mean that i could just let my consumer define it - my crate doesn't care about the exact type.

adamgreig commented 1 year ago

We discussed this at some length in today's meeting, and the result is #430.

overheat commented 1 year ago

How about async version PWM trait?