tock
timer: handle timer overflow
PR fixes https://github.com/tock/tock/issues/3879
Tock does not support setting alarms more than 2^32 ticks in the future as the alarm_at syscall takes in ticks. For the default nrf52840 tock configuration at 32KHz, the greatest possible value for the syscall alarm is 37 hours.
timer_in takes in milliseconds, but is flawed in that it is just a wrapper around alarm_at which can only accept a max value of 2^32 ticks. Thus, calling timer_in longer than 2^32 ticks in milliseconds will not have the expected behavior. delay_ms calls timer_in, so it is flawed for the same reason.
We fixed this by modifying timer_in. If the timer length is longer than the time that alarm_at can count up to, then timer_in will schedule multiple alarms to reach the full length. We calculate the number of full overflows and the remainder ticks to reach the target length of time. The overflows use the overflow_callback for each intermediate overflow.
Testing
We verified this code works by creating a testing program that simulates setting a timer longer than the threshold of what would overflow. We changed alarm_timer.c in the following way: replace all UINT_MAX with a hard coded variable with about 1 second of ticks. This tests the overflow logic, which works correctly.
Testing program
#include <assert.h>
#include <openthread/dataset_ftd.h>
#include <openthread/instance.h>
#include <openthread/platform/alarm-milli.h>
#include <stdio.h>
#include <timer.h>
static void overflow_callback(int last_timer_fire_time,
__attribute__ ((unused)) int unused1,
__attribute__ ((unused)) int unused2,
void* ud) {
printf("\tLETS GO OOOOOOO\n");
}
// we wanted busy wait because calling `delay_ms` uses `timer_in`, which
// is what we are trying to test. it was triggering our debug statements.
volatile int x = 0;
static void busy_wait(int num) {
int loop = 0;
while (loop < num) {
loop++;
x = loop;
}
}
int main(int argc, char *argv[])
{
busy_wait(10000000);
tock_timer_t* timer = malloc(sizeof(tock_timer_t));
timer_in(2000, overflow_callback, NULL, timer);
busy_wait(10000000);
}
Following on Tyler's comment, it would be nice to capture the more complete testing program in the tests/ folder, so that we have something to validate with down the road as well.
Considering this PR has stalled for a bit, I propose that we split this into two PRs:
- The improvements to calculating ticks to ms and ms to ticks.
- The
timer_inwrapping logic to set timers greater than $2^{32}$ and accompanying test.
We can keep this PR as (2) blocking on the new PR (1).
What is left to do here? Move code from openthread to the general alarm code?
@hudson-ayers Do you think there is any reason to keep the libtock_alarm_repeating_t type around? It would essentially be an alias for libtock_alarm_t right now, but if we need to keep more state for repeating alarms in the future then it would be helpful. But I can't think of what that state might be.
I think it is fine to get rid of libtock_alarm_repeating_t with this change since they would become identical, I cannot think of any other data we would really want to add to repeating alarms.
