Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get actual running duty from LedcDriver? #452

Open
hongquan opened this issue Jul 11, 2024 · 0 comments
Open

Get actual running duty from LedcDriver? #452

hongquan opened this issue Jul 11, 2024 · 0 comments
Labels
enhancement New feature or request

Comments

@hongquan
Copy link

The context:

I'm writing an application to control stepper motor and use esp_idf_hal::ledc::LedcDriver to generate PWM for that motor. The motor will be on / off depending on some condition (our application is to control peristaltic pump for hydroponic farm). I currently use LedcDriver::enable, LedcDriver::disable to turn on / off the motor. We also need to regularly report the motor status (on or off). Tried LedcDriver::get_duty() but it is not what I am looking for, because it returns the original-set duty cycle, no matter if the motor is on or off. I found that the esp_idf_sys::ledc_get_duty returns what we want, but LedcDriver doesn't have a wrapper method for this function yet.

Discussed

I have discussed with @Vollbrecht on Matrix, here is his opinion:

Personally i also don't find it appropriate to call the method that does this disable(). Disable is ambiguous here, as it could mean the timer or the output pin. And in what state the output itself is in disable is also ambiguous e.g high/low.
What probably should be done:

deprecate the disable() method for ambiguity
add wrappers around the ledc_timer_pause / ledc_timer_resume functions
add convenient methods that set the duty to 0 or 100% maybe something like set_low, set_high or set_min_duty()/set_max_duty() ( this methods would update the state value and you would get the current correct duty value)
adding just a second method that returns the "true" duty cycle in that case would only add more confusion. So that's why i think we should rework it.

So I bring this issue up here to get a final decision.

@Vollbrecht Vollbrecht added the enhancement New feature or request label Jul 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Todo
Development

No branches or pull requests

2 participants