Consider Implementing the embedded_hal::PwmPin trait for PwmOutput Pins

[ericPrince]

The avr_hal_generic::simple_pwm::PwmPinOps trait maps pretty exactly to embedded_hal::PwmPin. Implementing PwmPin would be nice so users can make use of generic code that uses it.

It would be nice to implement the Pwm trait in embedded-hal too, but I'd suggest that just doing PwmPin is lower hanging fruit. Not sure how this matches up with #130 (in particular this comment) but adding this impl as is was super straightforward.

The implementation looks like this in simple_pwm - I've tested this locally. The only other change needed is replacing use of u8 in a couple places in simple_pwm with the Duty type (this should be done anyway). I'd be happy to open a PR for this if adding the impl is given the 👍.

So any Pin<PwmOutput...> that impls PwmPinOps (which they all do) also now impls PwmPin:

use embedded_hal::PwmPin;

// ...

impl<TC, PIN> PwmPin for Pin<mode::PwmOutput<TC>, PIN>
where
    Pin<mode::PwmOutput<TC>, PIN>: PwmPinOps<TC>,
{
    type Duty = <Pin<mode::PwmOutput<TC>, PIN> as PwmPinOps<TC>>::Duty;

    fn enable(&mut self) {
        PwmPinOps::enable(self)
    }
    fn disable(&mut self) {
        PwmPinOps::disable(self)
    }
    fn get_duty(&self) -> Self::Duty {
        PwmPinOps::get_duty(self)
    }
    fn get_max_duty(&self) -> Self::Duty {
        PwmPinOps::get_max_duty(self)
    }

    fn set_duty(&mut self, value: Self::Duty) {
        PwmPinOps::set_duty(self, value)
    }
}

[Rahix]

I'd be happy to open a PR for this

Please do! I personally didn't want to put much effort into e-h implementations until the e-h 1.0 version settles but if you have a current need for this, I don't object to adding it. :)

[ericPrince]

Sorry for not getting back to this! I actually had totally overlooked that PwmPin is one of the traits that's removed in embedded-hal. (Seems to me like they could/should just have PwmPin::Duty be a type that impls num::Num or even num::Zero or similar.

I think it's better to wait on embedded-hal, and if I really need it I can fork avr-hal temporarily.

[Rahix]

As long as we're still depending on the old version of embedded-hal, it's fine to implement the trait that's included in it. If it helps you, I'd rather have the implementation included than for you to have to fork avr-hal ;)