Skip to content
This repository has been archived by the owner on Nov 7, 2020. It is now read-only.

Fixes issue #16 #17

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

ThiagoVinicius
Copy link

Rewrite the way timers keep track of time, preventing interference caused by intrinsic delays on app launch, background, persist tasks, system timer scheduling, etc.

* Break down Timer.current_time into two fields:
  * Timer.start_timestamp
  * Timer.paused_offset

  Combining those two with a reference timestamp (usually 'now'), it is
  possible to calculate the ellapsed time. The benefits are: that we don't have
  to rely on the app_timer_register callback; Timers are able to be restored
  to the correct time, after being backgrounded, or even if the Pebble
  Smartwatch has been powered off in between launches; It leaves the task of
  tracking the passage of time up to Pebble itself, where it excels.

  The downside is that the timestamp depends on the local time, since there
  is currently no API to use another time. Because of that, the timers will
  be affected by changing timezones or when entering or leaving DST. In order
  to avoid this, we need to use a time source that is increasing and monotonic.

* Get rid of app_timer_register.

* Use TickTimerService firing at each SECOND_UNIT to update the timers.

NOTE: This commit changes the timer data structure, but leaves migration. This
      is going to be fixed soon.
This commit adds code to properly migrate data to the new storage version.
We should only account for offset if the timer was running.
@matthewtole
Copy link
Member

This looks great! I'm going to review this as soon as I get a chance. Thanks for doing this.

@ThiagoVinicius
Copy link
Author

I'm glad to be able to help improve this app.

 * Dealing with the update_marked flag only in timers_fire_update_handlers and
   timers_mark_update.
 * Fix derp on timers_update_timestamp, which would only disable queueing if
   something had been actually queued. It should always disable.
This field is useless, as of current timer implementation.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants