Open mciantyre opened 3 years ago
Our ESC
interface already has an API for atomic changes. The default implementation just does the non-optimal loop. If we think that this feature is necessary, let's make sure to design to that API.
I'm on the fence between must-have and nice-to-have for this feature. If we were directly driving motors with our PWM outputs, I'd say this is very useful. But, since we're talking to a different ESC today, and we don't control the behaviors on that ESC, I'd think that atomic PWM changes on our end would be an over-optimization.
In practice, I'd expect a loop to set the PWM outputs to happen so fast that it's basically atomic (should verify this). We could prevent software preemption with a critical section.
Edit:
I'd expect a loop to set the PWM outputs to happen so fast that it's basically atomic
If the sequence to set PWM outputs crosses a PWM cycle, this won't be the case.
Did you have something in mind for verifying this? Such as connecting it to a logic analyzer?
First thing to verify is that the chip supports a way to atomically set the PWM outputs. Given the description from the fields in this PWM MCTRL register (image below), my interpretation is "yes." I still need to dig in.
Each PWM peripheral supports eight outputs (four submodules, each driving two pins). So to set the values across all eight simultaneously, the flow resembles
0b1111
to CLDOK
, or wait for LDOK
to read zero.0b1111
to LDOK
Once 3 happens, the peripheral loads the PWM values for all outputs, and starts driving with the new values.
Showing this happens with a logic analyzer would be good evidence. But, it wouldn't be complete verification. Today, we can show that we appear to be changing all PWM outputs at the same time; we're lucky.
I'll look into this as I play with the HAL. Could be cool to implement, but still not convinced we need it for today's system.
I tend to agree that we don't need this. Since we prove (or can prove) that they're setting roughly at the same time, I think that's good enough.
When we start to load the Teensy with more processes we will want to verify this doesn't change.
I believe the i.MX RT PWM peripherals could let us set PWM duty cycles atomically across a single peripheral's outputs. This could give us a guarantee that, when we say "set all motor throttles to [value]" we've changed the duty cycle for all PWM outputs at exactly the same time (within the bounds of a PWM cycle).
We can't express atomic changes with today's driver. We can only loop over the N outputs, and set them all to the same throttle. With this API, there's a chance that the first few motors are set to the throttle before the other motors.
This issue tracks atomic changes to PWM outputs.