Made safer reset of timer #12442
Made safer reset of timer #12442
Conversation
There was a problem hiding this comment.
@roangel - can you provide some timing capture for this change. From my understanding oneshot is to respond with 0 latency. If the CPU hits this (rCNT(timer) < CNT_VALUE_SAFE_TO_WRITE) regularly the update will never happen or be so delayed it will not be on shot.
|
Hey @davids5, thanks for the reply!
The thing is that CNT_VALUE_SAFE_TO_WRITE for 8MHz is 375 us, which should never be reached because the maximum value that we would be able to write from the controller is 250 us. This guard is only protecting us from writing in the conditions specified in the above image, where we reset the timer after it having overflown (should never happen unless our module has a delay on the update), and the signal still being in HIGH. The CPU should never hit this condition in normal circumstances, let alone hitting it continuously. |
|
For one shot what is the minimum timing (low) between pulses? Would we be better off using the HRT to trigger a 260 us watchdog (triggered on the last UG) to stop the timer? |
The 375 us value was taken from the fact that the minimum pulse duration at 8 MHz with normal PWM is 125us. An update that respects that limit seems to be in the safe side.
If I understand correctly, this would mean that we stop the timer until we receive the next update, whenever it is. It sounds like a clean way of doing it, but what would the servos and motors do when you don't update them for such a long time? Will they stop or will they keep their previous value? Do they have an internal watchdog that makes them stop or go to an unexpected/undesired value when we stop updating the PWM signal? |
|
Thank you for the drawing. What rate is the output from your controller?
I think that the oneshots ESC are pulse to pulse reactive, but we should review the spec. The repeat at 8 ms is an artifact of the implementation, we wanted "one pulse mode." So I think we should test adding code to stop the timer based on the longest ccr value + 10 us. (or what ever the min pulse to pulse time is for one shot (inter pulse low)) That will give us "one pulse mode." That will be close to synchronous. |
|
@roangel - sorry hit sent too quick - see updated |
I'm using 50 Hz (20 ms) update rate, however, I'm also lowering the BOARD_ONESHOT_FREQ to 1000000, so it works fine in OneShot mode. The only change that we aim for is to remove the delay between the update of the controller and the actuator, but we are not interested in the FastPWM part of it (we still use 1000us as lowest pulse width instead of 125 us). The analysis is done for the case where for whatever reason we miss some updates. But about the solution that you offer, why wouldn't it be possible to just use the one-pulse mode already built-in in the timers? That would offer us exactly what we need and we wouldn't need a watchdog, right? |
|
This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions. |
|
Thanks for your earlier contribution. Unfortunately as the project has overall evolved quite a bit, this change doesn't apply any more. Closing stale pull requests like this one is part of us working aggressively to bring down the PR review time so that we will be able to merge or reject PRs in the future in a much more timely fashion. |
Describe problem solved by the proposed pull request
With PWM in one shot mode, at 8 MHz frequency of the timer, the overflow time for a 16-bit timer is known to be 8.192 ms (2^16/8e6 s). As we are running our controllers faster than that, the timers should never overflow. However, if there is an unexpected delay on the update of the timers' value, they will overflow and start again the PWM cycle with the compare value that was set the previous time.
The problem comes when we are so unlucky that the update of the compare value and reset of the timer happens when we are sending a HIGH signal in the PWM sequence, i.e., at 8 MHz, when we are in the first 250 us of signal. In this case, we would reset the counter in the middle of a HIGH PWM value and keep it high during more time than desired, rendering unexpected behaviour and possibly undesired full thrust/full flap deflections.
With this PR we only reset the timers if the current CNT value is well above a safe value, when we are 100% sure that the state of the PWM signal is low.