Skip to content

Added method to change the ledc PWM frequency programmatically #5003

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

Merged
merged 4 commits into from
Apr 16, 2021

Conversation

iTacco
Copy link
Contributor

@iTacco iTacco commented Apr 1, 2021

Currently there is no real option to change the pwm frequency programmatically. The only "hack" solution is to use the ledcWriteTone(uint8_t chan, double freq) method.

But there is problem with this hack. Inside this method the bit resolution value is hard coded to 10. Which means if you had a different bit resolution inside your setup it would be overrided here.

So I added a simple method to change the pwm frequency programmatically. This method takes three parameters:
uint8_t chan, double freq, uint8_t bit_num

@iTacco iTacco changed the title Added method to change the ledc PWM frequence programmatically Added method to change the ledc PWM frequency programmatically Apr 2, 2021
@lbernstone
Copy link
Contributor

How about adding a boolean to ledcSetup that determines whether to reset the duty cycle instead?

@iTacco
Copy link
Contributor Author

iTacco commented Apr 6, 2021

But you could not change the frequency during runtime.
Or I am not getting what do you want to archive with resetting the duty cycle.
@lbernstone

@lbernstone
Copy link
Contributor

You would just call the setup a second time to change the frequency. Your code is almost identical to what ledcSetup calls, it just doesn't set the duty to 0. If that is not clear enough, you could then have your setFrequency function, but it would just be an alias to ledcSetup.

@iTacco
Copy link
Contributor Author

iTacco commented Apr 6, 2021

Ah okay. Now I understand what you meant.
But still. I guess it would be more intuitive and cleaner if there would be a seperate method to change the frequency with instead of add another paramater to the setup method.

But of couse. It could be solved by adding another parameter to the setup method.

@me-no-dev
Copy link
Member

any development here? :)

@iTacco
Copy link
Contributor Author

iTacco commented Apr 15, 2021

All development done in commits ;)
But I guess it needs to be discussed whether it should be done with my approach or should it be done with the approach by @lbernstone

@me-no-dev
Copy link
Member

I will accept this if you return the frequency as done in ledcSetup

- Returning frequency from ledcChangeFrequency
@iTacco
Copy link
Contributor Author

iTacco commented Apr 16, 2021

I will accept this if you return the frequency as done in ledcSetup

Done.

@me-no-dev me-no-dev merged commit 01c8cae into espressif:master Apr 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants