Add an hour option to the timer application
This adds an hours column to the timer app.
Addresses: #1678
Technical Details:
I thought this would be a simple task until I found that xTimerChangePeriod doesn't support periods longer than an hour and 10ish minutes (1hr 9m 54s is what I found). So my solution to this was to divide the hours (referred to as intervals in the code) out of the duration given when the timer is started and store it in timerOverflowIntervals, then set the period in xTimerChangePeriod to the leftover duration (an hour). Then when Messages::TimerDone: is triggered, it checks if any hours/intervals are stored, if there are then it starts a new hour timer and subtracts an hour/interval from timerOverflowIntervals. This allows for setting very large timers (256 hours) with the timer component without making the system hold large tick/millisecond values (however the timer app only allows up to 100 hours).
I may also have missed something simple with xTimerChangePeriod and way over engineered this, in which case this was a great learning exercise. Anyways, let me know what you think. To code reviewers: I'm a C++ novice so please go easy :).
Not sure why the GIF appears sped up, but the timer counts down seconds normally in the simulator.
Build size and comparison to main:
| Section | Size | Difference |
|---|---|---|
| text | 373376B | 352B |
| data | 948B | 0B |
| bss | 22544B | 8B |
Thanks for the feature! I have a feeling xTimerChangePeriod works fine for any tick count, but using pdMS_TO_TICKS isn't suitable as it causes an integer overflow. This macro is defined as #define pdMS_TO_TICKS( xTimeInMs ) ( ( TickType_t ) ( ( ( TickType_t ) ( xTimeInMs ) * ( TickType_t ) configTICK_RATE_HZ ) / ( TickType_t ) 1000 ) ), which minus the awkward type casts is (xTimeInMs*configTICK_RATE_HZ)/1000, and the tick rate is 1024Hz. The time you found as the maximum working is 4194000ms, which when multiplied by 1024 is 4294656000 - almost exactly 2^32! The solution to this is to not use pdMS_TO_TICKS as you have an integer number of seconds. To convert seconds to ticks, multiply the number of seconds by configTICK_RATE_HZ. This way the longest possible timer is 2^32-1 ticks which equals 4194304s, which is 48 days. Should be enough :)
@mark9064 After passing the product of duration.count() in seconds multiplied by configTICK_RATE_HZ to xTimerChangePeriod, the timer is still only setting to a maximum of 1h 9m 54s, I even tried just hard coding it xTimerChangePeriod(timer, 4199 * configTICK_RATE_HZ, 0); but this just overflowed as well... which leads me to believe xTimerChangePeriod is the problem, unless there's something wrong with TickType_t remainingTime = xTimerGetExpiryTime(timer) - xTaskGetTickCount(); in GetTimeRemaining, because I checked the ticks returned by this and its showing the same issue there as well (I even tried using uint32_t and uint64_t instead of TickType_t incase that was the issue). I tried looking around the FreeRTOS kernel but was struggling to understand it well enough to find the issue. Perhaps even if we can find a way to set this to go for the 48 hours you mentioned above, it would still be better to go with an approach like mine of restarting the timer according to a number of intervals, allowing us to set much longer timers (easily increasing the limit by changing timerOverflowIntervals from uint8_t).
Well in GetTimeRemaining it also multiplies by 1000 which will cause overflow. Basically all code paths involving it will need to be refactored to make sure they don't multiply tick counts beyond the range of uint32. To fix this one, you could split building the time into milliseconds and seconds (seconds with tick count / tick rate, milliseconds with tick count % tick rate * 1000 / tick rate)
I asked about the same thing!
I asked about the same thing!
@codeguylevi About what, the timer limit issue I'm having?
I asked about the same thing!
@codeguylevi About what, the timer limit issue I'm having?
Yes.
Yes.
You mean the timer currently being limited to 59:59 right?
Also Scott if you'd like me to take a look at the overflow issue I'd be happy to - if so let me know exactly what you've tried so far re removing pdMS_TO_TICKS etc.
Yes.
You mean the timer currently being limited to 59:59 right?
Yup!