Adafruit16CServoDriver Service PWM Frequency error

[Cyber-One]

Describe the bug
The PCA9685 chip has a known issue,
it's inbuilt clock isn't the most accurate, in fact it's not even close :-(
What's more different batches of the chip will see it's clock running at different speed for the same settings.

To Reproduce
With the Adafruit16CServoDriver service set to default settings and the servo on pin 0 set to 90°,
it should be outputting a 1500uS pulse at 60Hz.
What I am measuring is 1327uS pulse at 68.9916865Hz.
This is where @Shido/CyberSyntek#1360 was having issues with servos not centring correctly

Additional context
In the Adafruit16CServoDriver.java line 407 the frequency has a 0.9 factor applied to correct for the error in the chips clock frequency.
But testing across multiple PCA9685 has shown this error is not the same for all batches.
This 0.9 should be made available as a setting in the WebGUI and saved in the config.

The function starting at line 499
public void onServoWriteMicroseconds(ServoControl servo, int uS)
Also has an error.
It is using a constant for the calculation for the set microSeconds.
int pulseWidthOff = (int) (uS * 0.45) - 300;
This is based off the PWM frequency is set at 60Hz.
There is a function which will allow you to adjust the PWM frequency between 150Hz and 600Hz at line 390.
This function ignores the current PWM frequency

The function that sets the PWM frequency starting at line 390 also requires the pin passed as a parameter.
This pin value is never used in the function and should be removed.
It should be noted there is only one PWM frequency setting for the chip, all 16 output use this same setting.
You can not have pin 0 running at 60Hz and pin 5 running at 300 Hz at the same time, this is a hardware limitation.
The Pin parameter suggest to the user, that each pin can be set individually when it can not.

Servo positioning is also calculated based on the constants defined on line 160 and 166
This is based on a PWM frequency of 60Hz and is not updated at all if the PWMfrequency is changed.
This results in not being able to use the new digital servos at the higher 300Hz update rates.
SERVOMIN is used on lines 120, 474 and 955
SERVOMAX is used on lines 120, 474 and 955 as well
These values should be int's and updated by the setPWMFreq() function.

[Temik007]

This will help you
https://github.com/Temik007/Adafruit-PWM-Servo-Driver-Library/blob/master/examples/oscillatorAVR/oscillatorAVR.ino

[supertick]

This will help you https://github.com/Temik007/Adafruit-PWM-Servo-Driver-Library/blob/master/examples/oscillatorAVR/oscillatorAVR.ino

Thanks @Temik007 :)
Does this mean the PCA9685 oscillators can vary between 50-330Hz 😱 ?

[Cyber-One]

Normally the oscillators will hold a fixed frequency, however this frequency can vary from batch to batch and even between the start and end of a batch of the PCA9685 chips.
For this reason they have the option of an external clock source when you need multiple modules to work together.

In our application the frequency of the clock is very important, thankfully it doesn't drift much if at all, but the module you have will be different than the unit I have.

Add to this that newer servos now like the faster frequencies as high as 330Hz up from the 50Hz - 60Hz we normally use.
The high frequency allows for faster response times, not really an issue for us diving over I2C
When controlling a servo, the pulse width is the critical thing with 1500uS being the centre of the rotation.

When using the PCA9685 as a PWM source for a speed controller or a LED driver, the higher frequencies do need to be used.

If we want to use the PCA9685 for two different functions, such as driving a servo and driving an LED as well and potentially driving a pair of motors, then the user will need to decide on the best frequency to use.
This unfortunately is a whole chip selection, there is only one clock and divider.
A change in the clock divider results in all 16 PWM output periods being changed as well.

In the early days of using the PCA9685 LED driver chip as a servo driver with the Arduino it was found that while the developer had the centre position correct, other builders would find it was not centred, this is when the builder community at large discovered the clock errors.

The onboard clock runs at approximately 25MHz (This is the one we normally use) This is where our errors are coming from.
We then need to set the pre-scale value using the formulae:
pre-scale = round(osc_clock / (4096 x update_rate)) - 1

By default MRL tries for 60Hz update rate

MRL uses the code:

226:-   // public static final int PWM_FREQ = 60; // default frequency for servos
227:-   public static final float osc_clock = 25000000; // clock frequency of the
228:-   // internal clock
229:-   public static final float precision = 4096; // pwm_precision
|
407: - prescale_value = Math.round(0.9 * osc_clock / precision / hz) - 1;

the 0.9 in the code is to try and correct for an error in the oscillator frequency, but this only really works for the developer when testing with his/her device.
I have multiple devices source at different times and different vendors and found that they have different oscillator clock frequencies (units from the same batch tend to match units from the same batch)

at line 390:- public void setPWMFreq(String pin, Integer hz) { // Analog servos run at ~60
the value pin is required by the function but is never used, this is a bit misleading for the users.
This function sets the chip prescaler, there are no pin prescalers, so all the pins are affected.

Possible solution

Add to the Adafruit16CServoDriver GUI the ability to set the device output frequency and also to set the device oscillator frequency.
The default output frequency can be set to 60Hz by default (24Hz - 1526Hz)
I would remove the 0.9 adjustment from the code and set the oscillator frequency to 22.5MHz
Both the frequencies should be saved in the config as well.

This will allow for generic code to be used and the users can fine tune the device oscillator to match their own unit.
A change in the device output frequency should not change a servo output position.
Running higher output frequencies will also increase the fine resolution of the servo positioning as well, but not all servos may support this :-)