app_timer.c in SDK 17.0.2 adds additional parameter check, not mentioned in migration guide

Hi there,

That part was added to report an error when the app_timer_start() was called with a timeout larger than the 24 bit RTC range. 

The code in fact does have handling for timers exceeding the duration of the RTC. 

Are you referring to your own code or previous versions of the app timer module?

That app_timer2.c was recommended as a solution, however our testing with app_timer2 showed significant problems and we reverted back to app_timer.c  

Have you already opened tickets for these issues?

Shouldn't the Release Notes and Migration Guide be updated to note this change and provide some explanation of the reason for the change?

I agree that this could have been included. We usually only highlight major changes in our migration guide, so my guess is that someone decided that this wasn't a big enough change to include in the migration guide. 

Since in our past usage of this function, the app_timer code did correctly handle timer values greater than MAX_RTC_COUNTER_VAL, can we safely disable this check?

The app timer will still be limited by the RTC, but if you handle this somewhere else in your code then I think it should be ok. 

regards

Jared 

The app_timer.c code already handles situations where the timeout is greater than MAX_RTC_COUNTER_VAL.

When the timer is initially inserted into the list a calculation is performed to determine number of tickets until expired.  This calculation can handle up to uint32_t ticks.   From this point onward, every call to timer_timeouts_check (from the ISR) decrements the ticks_to_expire by the time that has passed.  The RTC1 can roll over multiple times before ticks_to_expire reaches zero.

Does Nordic have a testcase that actually fails on the older SDK's or was this parameter check added without an actual requirement?

The app_timer.c code has been unchanged since at least SDK 15 (e.g. 2018).  Were there complaints about timer duration exceeding MAX_RTC_COUNTER_VAL.  Why was this new parameter range check added?

The app_timer.c code already handles situations where the timeout is greater than MAX_RTC_COUNTER_VAL.

When the timer is initially inserted into the list a calculation is performed to determine number of tickets until expired.  This calculation can handle up to uint32_t ticks.   From this point onward, every call to timer_timeouts_check (from the ISR) decrements the ticks_to_expire by the time that has passed.  The RTC1 can roll over multiple times before ticks_to_expire reaches zero.

Does Nordic have a testcase that actually fails on the older SDK's or was this parameter check added without an actual requirement?

The app_timer.c code has been unchanged since at least SDK 15 (e.g. 2018).  Were there complaints about timer duration exceeding MAX_RTC_COUNTER_VAL.  Why was this new parameter range check added?

Hi,

There was a reported bug that the app timer module behaved inconsistently when using timeouts values larger than the RTC wrap around. The reported behavior was that timeout handlers would not fire if there weren't any other timers present. But if there another timer present with shorter timeout, then the timeout handler for the longer timeout would fire as expected.

We therefore decided to limit the app timer timeout range to get a consistent behavior. 

regards

Jared