Align the top of the second with the 1Hz periodic interrupt

movement.c

@@ -155,17 +155,36 @@ static udatetime_t _movement_convert_date_time_to_udate(watch_date_time_t date_t
 
 static void _movement_set_top_of_minute_alarm() {
     uint32_t counter = watch_rtc_get_counter();
+    uint32_t next_minute_counter;
     watch_date_time_t date_time = watch_rtc_get_date_time();
     uint32_t freq = watch_rtc_get_frequency();
+    uint32_t half_freq = freq >> 1;
+    uint32_t subsecond_mask = freq - 1;
+    uint32_t ticks_per_minute = watch_rtc_get_ticks_per_minute();
 
-    // remove subsecond from counter
+    // get the counter at the last second tick
-    counter &= ~(freq - 1);
+    next_minute_counter = counter & (~subsecond_mask);
+    // add/subtract half second shift to sync up second tick with the 1Hz interrupt
+    next_minute_counter += (counter & subsecond_mask) >= half_freq ? half_freq : -half_freq;
     // counter at the next top of the minute
-    counter += (60 - date_time.unit.second) * freq;
+    next_minute_counter += (60 - date_time.unit.second) * freq;
 
-    movement_volatile_state.minute_counter = counter;
+    // Since the minute alarm is very important, double/triple check to make sure that it will fire.
+    // These are theoretical corner cases that probably can't even happen, but since we do a subtraction
+    // above I wanna be certain that we don't schedule the next alarm at a counter value just before the
+    // current counter, which would result in the alarm firing after more than one year.
+    // This should be robust to the counter overflow, and we should ever iterate once at most.
+    if (next_minute_counter == counter) {
+        next_minute_counter += ticks_per_minute;
+    }
 
-    watch_rtc_register_comp_callback(cb_minute_alarm_fired, counter, MINUTE_TIMEOUT);
+    while ((next_minute_counter - counter) > ticks_per_minute) {
+        next_minute_counter += ticks_per_minute;
+    }
+
+    movement_volatile_state.minute_counter = next_minute_counter;
+
+    watch_rtc_register_comp_callback(cb_minute_alarm_fired, next_minute_counter, MINUTE_TIMEOUT);
 }
 
 static bool _movement_update_dst_offset_cache(void) {
@@ -850,9 +869,6 @@ void app_init(void) {
         watch_rtc_set_date_time(date_time);
     }
 
-    // set up the 1 minute alarm (for background tasks and low power updates)
-    _movement_set_top_of_minute_alarm();
-
     // register callbacks to be notified when buzzer starts/stops playing.
     // this is so movement can be notified even when triggered by a face bypassing movement
     watch_buzzer_register_global_callbacks(cb_buzzer_start, cb_buzzer_stop);
@@ -865,6 +881,9 @@ void app_init(void) {
     movement_state.light_on = false;
     movement_state.next_available_backup_register = 2;
     _movement_reset_inactivity_countdown();
+
+    // set up the 1 minute alarm (for background tasks and low power updates)
+    _movement_set_top_of_minute_alarm();
 }
 
 void app_wake_from_backup(void) {
@@ -1069,21 +1088,6 @@ bool app_loop(void) {
             movement_volatile_state.turn_led_off = false;
             movement_force_led_off();
         }
-    } 
-
-    // actually play the note sequence we were asked to play while in deep sleep.
-    if (movement_volatile_state.has_pending_sequence) {
-        movement_volatile_state.has_pending_sequence = false;
-        watch_buzzer_play_sequence_with_volume(_pending_sequence, movement_request_sleep, movement_button_volume());
-        // When this sequence is done playing, movement_request_sleep is invoked and the watch will go,
-        // back to sleep (unless the user interacts with it in the meantime)
-        _pending_sequence = NULL;
-    }
-
-    // handle top-of-minute tasks, if the alarm handler told us we need to
-    if (movement_volatile_state.minute_alarm_fired) {
-        movement_volatile_state.minute_alarm_fired = false;
-        _movement_handle_top_of_minute();
     }
 
     // if we have a scheduled background task, handle that here:
@@ -1123,6 +1127,12 @@ bool app_loop(void) {
         event_type++;
     }
 
+    // handle top-of-minute tasks, if the alarm handler told us we need to
+    if (movement_volatile_state.minute_alarm_fired) {
+        movement_volatile_state.minute_alarm_fired = false;
+        _movement_handle_top_of_minute();
+    }
+
     // Now handle the EVENT_TIMEOUT
     if (resign_timeout && movement_state.current_face_idx != 0) {
         event.event_type = EVENT_TIMEOUT;
@@ -1153,6 +1163,15 @@ bool app_loop(void) {
         // // this is a hack tho: waking from sleep mode, app_setup does get called, but it happens before we have reset our ticks.
         // // need to figure out if there's a better heuristic for determining how we woke up.
         app_setup();
+
+        // If we woke up to play a note sequence, actually play the note sequence we were asked to play while in deep sleep.
+        if (movement_volatile_state.has_pending_sequence) {
+            movement_volatile_state.has_pending_sequence = false;
+            watch_buzzer_play_sequence_with_volume(_pending_sequence, movement_request_sleep, movement_button_volume());
+            // When this sequence is done playing, movement_request_sleep is invoked and the watch will go,
+            // back to sleep (unless the user interacts with it in the meantime)
+            _pending_sequence = NULL;
+        }
     }
 #endif
 
@@ -1300,9 +1319,10 @@ void cb_minute_alarm_fired(void) {
 void cb_tick(void) {
     rtc_counter_t counter = watch_rtc_get_counter();
     uint32_t freq = watch_rtc_get_frequency();
+    uint32_t half_freq = freq >> 1;
     uint32_t subsecond_mask = freq - 1;
     movement_volatile_state.pending_events |= 1 << EVENT_TICK;
-    movement_volatile_state.subsecond = (counter & subsecond_mask) >> movement_state.tick_pern;
+    movement_volatile_state.subsecond = ((counter + half_freq) & subsecond_mask) >> movement_state.tick_pern;
 }
 
 void cb_accelerometer_event(void) {

watch-library/hardware/watch/watch_rtc.c

@@ -34,6 +34,7 @@ static const uint32_t RTC_OSC_DIV = 10;
 static const uint32_t RTC_OSC_HZ = 1 << RTC_OSC_DIV; // 2^10 = 1024
 static const uint32_t RTC_PRESCALER_DIV = 3;
 static const uint32_t RTC_CNT_HZ = RTC_OSC_HZ >> RTC_PRESCALER_DIV; // 1024 / 2^3 = 128
+static const uint32_t RTC_CNT_SUBSECOND_MASK = RTC_CNT_HZ - 1;
 static const uint32_t RTC_CNT_DIV = RTC_OSC_DIV - RTC_PRESCALER_DIV; // 7
 static const uint32_t RTC_CNT_TICKS_PER_MINUTE = RTC_CNT_HZ * 60;
 static const uint32_t RTC_CNT_TICKS_PER_HOUR = RTC_CNT_TICKS_PER_MINUTE * 60;
@@ -88,17 +89,31 @@ rtc_date_time_t watch_rtc_get_date_time(void) {
 }
 
 void watch_rtc_set_unix_time(unix_timestamp_t unix_time) {
-    //  time_backup + counter / RTC_CNT_HZ = unix_time
+    /* unix_time = time_backup + counter / RTC_CNT_HZ - 0.5
+     *
+     * Because of the way the hardware is designed, the periodic interrupts fire at the subsecond tick values
+     * according to the table below (for a 128Hz counter).
+     * since the 1Hz periodic interrupt is the most important, we shift the conversion from counter to timestamp by 64 ticks,
+     * so that the second changes at the top of the 1Hz interrupt. Hence the 0.5 factor in the equation above.
+     * 1Hz:   64
+     * 2Hz:   32, 96
+     * 4Hz:   16, 48, 80, 112
+     * 8Hz:   8, 24, 40, 56, 72, 88, 104, 120
+     * 16Hz:  4, 12, 20, ..., 124
+     * 32Hz:  2, 6, 10, ..., 126
+     * 64Hz:  1, 3, 5, ..., 127
+     * 128Hz: 0, 1, 2, ..., 127
+    */
     rtc_counter_t counter = rtc_get_counter();
-    unix_timestamp_t tb = unix_time - (counter >> RTC_CNT_DIV);
+    unix_timestamp_t tb = unix_time - (counter >> RTC_CNT_DIV) - ((counter & RTC_CNT_SUBSECOND_MASK) >> (RTC_CNT_DIV - 1)) + 1;
     watch_store_backup_data(tb, TB_BKUP_REG);
 }
 
 unix_timestamp_t watch_rtc_get_unix_time(void) {
-    //  time_backup + counter / RTC_CNT_HZ = unix_time
+    // unix_time = time_backup + counter / RTC_CNT_HZ - 0.5
     rtc_counter_t counter = rtc_get_counter();
     unix_timestamp_t tb = watch_get_backup_data(TB_BKUP_REG);
-    return tb + (counter >> RTC_CNT_DIV);
+    return tb + (counter >> RTC_CNT_DIV) + ((counter & RTC_CNT_SUBSECOND_MASK) >> (RTC_CNT_DIV - 1)) - 1;
 }
 
 rtc_counter_t watch_rtc_get_counter(void) {

watch-library/simulator/watch/watch_rtc.c

@@ -31,6 +31,7 @@
 #include <emscripten/html5.h>
 
 static const uint32_t RTC_CNT_HZ = 128;
+static const uint32_t RTC_CNT_SUBSECOND_MASK = RTC_CNT_HZ - 1;
 static const uint32_t RTC_CNT_DIV = 7;
 static const uint32_t RTC_CNT_TICKS_PER_MINUTE = RTC_CNT_HZ * 60;
 static const uint32_t RTC_CNT_TICKS_PER_HOUR = RTC_CNT_TICKS_PER_MINUTE * 60;
@@ -98,15 +99,15 @@ rtc_date_time_t watch_rtc_get_date_time(void) {
 }
 
 void watch_rtc_set_unix_time(unix_timestamp_t unix_time) {
-    //  time_backup + counter / RTC_CNT_HZ = unix_time
+    // unix_time = time_backup + counter / RTC_CNT_HZ - 0.5
     rtc_counter_t counter = watch_rtc_get_counter();
-    reference_timestamp = unix_time - (counter >> RTC_CNT_DIV);
+    reference_timestamp = unix_time - (counter >> RTC_CNT_DIV) - ((counter & RTC_CNT_SUBSECOND_MASK) >> (RTC_CNT_DIV - 1)) + 1;
 }
 
 unix_timestamp_t watch_rtc_get_unix_time(void) {
-    //  time_backup + counter / RTC_CNT_HZ = unix_time
+    // unix_time = time_backup + counter / RTC_CNT_HZ - 0.5
     rtc_counter_t counter = watch_rtc_get_counter();
-    return reference_timestamp + (counter >> RTC_CNT_DIV);
+    return reference_timestamp + (counter >> RTC_CNT_DIV) + ((counter & RTC_CNT_SUBSECOND_MASK) >> (RTC_CNT_DIV - 1)) - 1;
 }
 
 rtc_counter_t watch_rtc_get_counter(void) {