Switch to freertos timers #1095
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Avamander
previously requested changes
Apr 17, 2022
rev22
force-pushed
the
alarm-reliability-and-switch-to-freertos-timers
branch
from
681c700 to
aca605d
Compare
|
It's great! Because alarm reliability is a very important thing... A unreliable alarm is may be even worse than absense of any alarm. |
Riksu9000
reviewed
May 2, 2022
rev22
force-pushed
the
alarm-reliability-and-switch-to-freertos-timers
branch
from
aca605d to
34b0717
Compare
|
Thank you @Riksu9000 for reviewing the code! I have now rebased and updated the commits for the latest develop branch, and applied few cosmetical changes. |
1 task
…tainabilty. Firmware size seems unchanged.
rev22
force-pushed
the
alarm-reliability-and-switch-to-freertos-timers
branch
from
34b0717 to
201d09a
Compare
|
I have rebased the code again and excluded the commit about resilience to time adjustments, so that it's possible to focus better on the switch to FreeRTOS timers (per code review). A new commit was added that may address code clarity. |
Riksu9000
suggested changes
May 19, 2022
…g timers (removes ScheduleAlarm message)
rev22
changed the title
Riksu9000
suggested changes
May 20, 2022
Riksu9000
approved these changes
May 20, 2022
NeroBurner
requested changes
Jun 5, 2022
Co-authored-by: NeroBurner <[email protected]>
NeroBurner
approved these changes
Jun 6, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Addresses observations and issues reported in #1086 and possibly others.
This switches alarm, timer and motor controller to use FreeRTOS timers instead of the app_timer component.
The logic was changed slightly for MotorController, to avoid two timers being started simultaneosily but being executed in an unintended order.
An event is introduced to differentiate between time settings and time adjustments, this is to make alarms resilient to time zone or daylight saving changes, and to prevent companion apps from postponing alarms by periodically adjusting the time in the watch (this could happen if the time is adjusted by the companion app to the exact hour and minute an alarm was set to, which would result in the alarm being set forward 24h by ScheduleAlarm).
I have positively tested the changes, though testing was done in my personal branch mainly so far, and it's still preliminary. I only own one PineTime at the moment, and testing alarms and recurring alarms can take days.
Edit: per discussion with Riksu9000 below, this PR currently focuses on the switch to FreeRTOS timers