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

Improve analogWrite frequency specification #775

Open
matthijskooijman opened this issue Nov 15, 2019 · 2 comments
Open

Improve analogWrite frequency specification #775

matthijskooijman opened this issue Nov 15, 2019 · 2 comments
Labels

Comments

@matthijskooijman
Copy link
Contributor

Currently, there is an analogWriteFrequency() that sets the frequency for all future analogWrite() calls on all timer channels. This works, but when working with different libraries or code files (with different frequency requirements), this can be a bit cumbersome because this function hides some global state. e.g. to really make this work generically, you would need to call analogWriteFrequency() before every call to analogWrite() (and again afterwards to restore the default if there is also code that is not aware of analogWriteFrequency().

To solve this, it might be good if you could pass the frequency to use as an (optional) argument to analogWrite() directly. This removes the hidden global state and makes the analogWrite() calls predictable again.

Having said that, there are certainly sketches where a "set frequency for all analogWrites"-approach is easier, so the current analogWriteFrequency() still makes sense (but can nicely co-exist with the above suggestion by using the analogWriteFrequency-specified default when no frequency is passed to analogWrite().

Currently, every time analogWrite() is called, the timer is configured completely, including the PWM frequency. A more obvious approach is to setup the timer frequency (prescaler and ARR value) once and then just use the existing value when calling analogWrite(). Something like this could be added using something like:

 analogWriteFrequence(pin, 100 /* Hz */);     
 analogWrite(pin, PWM_FREQ_UNCHANGED);

This explicitly specifies to not change the frequency. This could also be the default, but then there is no longer any easy way to change the frequency for all future analogWrite() calls.

Of course, the timer frequency is shared between channels of the same timer, so this API would change the frequency of all pins on the same timer. This is expected and should be documented properly, but is not otherwise a problem AFAICS.

I just also realized that the current code produces quite unexpected results when you start analogWrite() on one pin using one frequency, then start analogWrite() on another pin on the same timer using a different frequency. Then the timer gets reconfigured to a new frequency, but the CC values of other channels are unchanged, so if the ARR value changed, the effective duty cycle of those other channels now changed. I'm not sure how to fix this properly (other than encouraging people to set up frequencies first and then start analogWrite() afterwards), but it's something to keep in mind.

All of this could mostly be applied to the analogWriteResolution as well (except for the UNCHANGED part, since the resolution is really only a property of the value passed to analogWrite(), not so much a configuration stored in the timer which always runs at maximum resolution I think). That would suggest adding a bits argument to analogWrite() as well (probably before the frequency argument` suggested above). This is something that the official Arduino API does not support yet, see arduino/ArduinoCore-API#33 for some thoughts about that.

@fpistm
Copy link
Member

fpistm commented Nov 15, 2019

The first issue is the analog API's are C functions 😉

@matthijskooijman
Copy link
Contributor Author

Good point. OTOH, you could define the default function as C, and offer the extra functions with more arguments only in C++. Or use different names, but that gets cumbersome quickly...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants