`Pwm` API about embedded-hal HOT 8 CLOSED

I'm wondering how useful are the get_period and set_period methods get it comes to building other abstractions, or in even in generic programming. cf. #3 (comment)

from embedded-hal.

Update: this trait is available in release v0.1.0 behind the "unproven" Cargo feature. But there's also a PwmPin trait that's not feature gated.

from embedded-hal.

I could not implement either of the PWM traits for the PCA9685 driver because all the methods are infallible. I did not feel like starting another digital::v2 debacle so I've just dropped it.

About set_period/get_period I think it would be cumbersome to implement since it depends on the "real world" frequency on which everything is running, which may be simply an external input and thus subject to external change plus configurable through a prescaler or power configuration or so.
Even when solving this, the driver will need to calculate an approximation to a combination of prescaler and/or other configs and I feel this is something that the driver user would rather determine themselves so that the authoritatively provided frequency is followed exactly.
Additionally, choosing a Time type seems difficult because of precision and conversion issues.

from embedded-hal.

@eldruin i believe the fallible component is resolved, do you still have concerns with the current implementation?

from embedded-hal.

Indeed. I believe I could implement a split yielding PwmPins in the PCA9685 driver using the embedded-hal 1.0.0 alpha. I will have a try at this soon and see how it works. It will probably be similar to the pcf857x I/O expander driver.

Regarding set_period/get_period for the Pwm trait, my concerns about the frequency calculation still stand.

from embedded-hal.

Can you describe your API concerns in a separate issue for better visibility so we can close this ticket and avoid the impression PWM still doesn't exist? 😅

from embedded-hal.

Sure. I filed #226. This can be closed now.

from embedded-hal.

Thank you!

from embedded-hal.