I'm working on augmenting an existing product that is based in the nRF52832. We've been using a handful of timers for some time without apparent issue. I added another timer that was behaving properly but caused a different timer to malfunction sometimes. The original timer was a 60-second timer that forces a BLE disconnect after 60 seconds of idle time. The new timer is a five-minute timer that forces a BLE disconnect after five minutes, even if the connection is active. This means the timers are both running simultaneously. The 60-second timer is the one that we've been using successfully for some time without issue but then started malfunctioning.
Each time a BLE event is detected, we restart this 60-second timer. In my test, I restarted the timer multiple times but was nearing the 5-minutes of the other timer. Right before the timer malfunctioned (still about 25 seconds left on the 5-minute timer), I restarted the timer (app_timer_stop followed shortly afterwards by app_timer_start) and unlike the last 20 restarts where the timer runs as expected, the timer fired immediately this time.
Note that we're using a timer prescalar of 0 using real-timer counter 0 (RTC0) so a 60-second timer means 1966080 ticks, well within the 24-bit available in the RTC.
After a lot of digging, I found that the code to insert a new handler into the internal, singly-linked list was determining that the timer should already be expired; this means it was entering the second else statement. This happened despite the fact that for the previous restarts, it determined the timer duration was just fine and let the timer run.
The following code snippet comes from components\libraries\timer\app_timer.c in the function list_insertions_handler.
if (
((p_timer->ticks_at_start - m_ticks_latest) & MAX_RTC_COUNTER_VAL)
<
(MAX_RTC_COUNTER_VAL / 2)
)
{
p_timer->ticks_to_expire = ticks_diff_get(p_timer->ticks_at_start, m_ticks_latest) +
p_timer->ticks_first_interval;
}
else
{
uint32_t delta_current_start;
delta_current_start = ticks_diff_get(m_ticks_latest, p_timer->ticks_at_start);
if (p_timer->ticks_first_interval > delta_current_start)
{
p_timer->ticks_to_expire = p_timer->ticks_first_interval - delta_current_start;
}
else
{
p_timer->ticks_to_expire = 0;
}
}
To rectify the situation, I modified the code to do what I thought was the "right" thing to do, namely, given the tick counter when the timer started and last updated tick counter, compute the ticks that should occur from now until the timer expires, including handling the possibility of rollover.
if (p_timer->ticks_at_start > m_ticks_latest)
{
p_timer->ticks_to_expire = ticks_diff_get(p_timer->ticks_at_start, m_ticks_latest);
}
else
{
p_timer->ticks_to_expire = p_timer->ticks_at_start + MAX_RTC_COUNTER_VAL - m_ticks_latest;
}
p_timer->ticks_to_expire = (p_timer->ticks_to_expire + p_timer->ticks_first_interval) & MAX_RTC_COUNTER_VAL;
Can anyone explain why the original code was written the way it is (I'm running SDK 12.3 but the same code persists even in SDK version 17.0)? If I have that information, then I will perhaps see that my changes are ignoring a possible issue.
Is there anything wrong with the changed code?
