Open matthijskooijman opened 4 years ago
The first issue is the analog API's are C functions 😉
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...
Currently, there is an
analogWriteFrequency()
that sets the frequency for all futureanalogWrite()
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 callanalogWriteFrequency()
before every call toanalogWrite()
(and again afterwards to restore the default if there is also code that is not aware ofanalogWriteFrequency()
.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 theanalogWrite()
calls predictable again.Having said that, there are certainly sketches where a "set frequency for all
analogWrites
"-approach is easier, so the currentanalogWriteFrequency()
still makes sense (but can nicely co-exist with the above suggestion by using theanalogWriteFrequency
-specified default when no frequency is passed toanalogWrite()
.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 callinganalogWrite()
. Something like this could be added using something like: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 startanalogWrite()
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 startanalogWrite()
afterwards), but it's something to keep in mind.All of this could mostly be applied to the
analogWriteResolution
as well (except for theUNCHANGED
part, since the resolution is really only a property of the value passed toanalogWrite()
, not so much a configuration stored in the timer which always runs at maximum resolution I think). That would suggest adding abits
argument toanalogWrite()
as well (probably before thefrequency
argument` suggested above). This is something that the official Arduino API does not support yet, see https://github.com/arduino/ArduinoCore-API/issues/33 for some thoughts about that.