movement: fix preferences glitch, add some notes

2 files changed, 83 insertions, 74 deletions

diff --git a/movement/movement.c b/movement/movement.c
index a95501a4..df3cbf0b 100644
--- a/movement/movement.c
+++ b/movement/movement.c
@@ -209,7 +209,11 @@ void cb_tick() {
     event.event_type = EVENT_TICK;
     watch_date_time date_time = watch_rtc_get_date_time();
     if (date_time.unit.second != movement_state.last_second) {
+        // TODO: since we time the LED with the 1 Hz tick, the actual time lit can vary depending on whether the
+        // user hit it just before or just after a tick. If we time this with the system tick we can do better.
         if (movement_state.light_ticks) movement_state.light_ticks--;
+
+        // TODO: can we consolidate these two ticks?
         if (movement_state.settings.bit.le_interval && movement_state.le_mode_ticks > 0) movement_state.le_mode_ticks--;
         if (movement_state.timeout_ticks > 0) movement_state.timeout_ticks--;
 
diff --git a/movement/watch_faces/settings/preferences_face.c b/movement/watch_faces/settings/preferences_face.c
index b68f2a39..afe71034 100644
--- a/movement/watch_faces/settings/preferences_face.c
+++ b/movement/watch_faces/settings/preferences_face.c
@@ -69,85 +69,90 @@ bool preferences_face_loop(movement_event_t event, movement_settings_t *settings
 
     watch_display_string((char *)preferences_face_titles[current_page], 0);
 
-    if (event.subsecond % 2) return current_page <= 2;
-    char buf[8];
-    switch (current_page) {
-        case 0:
-            if (settings->bit.clock_mode_24h) watch_display_string("24h", 4);
-            else watch_display_string("12h", 4);
-            break;
-        case 1:
-            if (settings->bit.button_should_sound) watch_display_string("y", 9);
-            else watch_display_string("n", 9);
-            break;
-        case 2:
-            switch (settings->bit.to_interval) {
-                case 0:
-                    watch_display_string("60 sec", 4);
-                    break;
-                case 1:
-                    watch_display_string("2 n&in", 4);
-                    break;
-                case 2:
-                    watch_display_string("5 n&in", 4);
-                    break;
-                case 3:
-                    watch_display_string("30n&in", 4);
-                    break;
-            }
-            break;
-        case 3:
-            switch (settings->bit.le_interval) {
-                case 0:
-                    watch_display_string(" never", 4);
-                    break;
-                case 1:
-                    watch_display_string("1 hour", 4);
-                    break;
-                case 2:
-                    watch_display_string("2 hour", 4);
-                    break;
-                case 3:
-                    watch_display_string("6 hour", 4);
-                    break;
-                case 4:
-                    watch_display_string("12 hr", 4);
-                    break;
-                case 5:
-                    watch_display_string(" 1 day", 4);
-                    break;
-                case 6:
-                    watch_display_string(" 2 day", 4);
-                    break;
-                case 7:
-                    watch_display_string(" 7 day", 4);
-                    break;
-            }
-            break;
-        case 4:
-            if (settings->bit.led_duration) {
-                // FIXME: since we time the LED with the 1 Hz tick, the actual time lit can vary depending
-                // on whether the user hit it just before or just after a tick. so the setting is "1-2 s",
-                // "3-4 s", or "5-6 s". If we time this with the system tick we can do better.
-                sprintf(buf, " %1d-%1d s", settings->bit.led_duration * 2 - 1, settings->bit.led_duration * 2);
-                watch_display_string(buf, 4);
-            } else {
-                watch_display_string("no LEd", 4);
-            }
-            break;
-        case 5:
-            sprintf(buf, "%2d", settings->bit.led_green_color);
-            watch_display_string(buf, 8);
-            break;
-        case 6:
-            sprintf(buf, "%2d", settings->bit.led_red_color);
-            watch_display_string(buf, 8);
-            break;
+    // blink active setting on even-numbered quarter-seconds
+    if (event.subsecond % 2) {
+        char buf[8];
+        switch (current_page) {
+            case 0:
+                if (settings->bit.clock_mode_24h) watch_display_string("24h", 4);
+                else watch_display_string("12h", 4);
+                break;
+            case 1:
+                if (settings->bit.button_should_sound) watch_display_string("y", 9);
+                else watch_display_string("n", 9);
+                break;
+            case 2:
+                switch (settings->bit.to_interval) {
+                    case 0:
+                        watch_display_string("60 sec", 4);
+                        break;
+                    case 1:
+                        watch_display_string("2 n&in", 4);
+                        break;
+                    case 2:
+                        watch_display_string("5 n&in", 4);
+                        break;
+                    case 3:
+                        watch_display_string("30n&in", 4);
+                        break;
+                }
+                break;
+            case 3:
+                switch (settings->bit.le_interval) {
+                    case 0:
+                        watch_display_string(" never", 4);
+                        break;
+                    case 1:
+                        watch_display_string("1 hour", 4);
+                        break;
+                    case 2:
+                        watch_display_string("2 hour", 4);
+                        break;
+                    case 3:
+                        watch_display_string("6 hour", 4);
+                        break;
+                    case 4:
+                        watch_display_string("12 hr", 4);
+                        break;
+                    case 5:
+                        watch_display_string(" 1 day", 4);
+                        break;
+                    case 6:
+                        watch_display_string(" 2 day", 4);
+                        break;
+                    case 7:
+                        watch_display_string(" 7 day", 4);
+                        break;
+                }
+                break;
+            case 4:
+                if (settings->bit.led_duration) {
+                    // TODO: since we time the LED with the 1 Hz tick, the actual time lit can vary depending
+                    // on whether the user hit it just before or just after a tick. so the setting is "1-2 s",
+                    // "3-4 s", or "5-6 s". If we time this with the system tick we can do better.
+                    // see also cb_tick at the bottom of movement.c
+                    sprintf(buf, " %1d-%1d s", settings->bit.led_duration * 2 - 1, settings->bit.led_duration * 2);
+                    watch_display_string(buf, 4);
+                } else {
+                    watch_display_string("no LEd", 4);
+                }
+                break;
+            case 5:
+                sprintf(buf, "%2d", settings->bit.led_green_color);
+                watch_display_string(buf, 8);
+                break;
+            case 6:
+                sprintf(buf, "%2d", settings->bit.led_red_color);
+                watch_display_string(buf, 8);
+                break;
+        }
     }
 
+    // on LED color select screns, preview the color.
     if (current_page >= 5) {
         watch_set_led_color(settings->bit.led_red_color ? (0xF | settings->bit.led_red_color << 4) : 0,
                             settings->bit.led_green_color ? (0xF | settings->bit.led_green_color << 4) : 0);
+        // return false so the watch stays awake (needed for the PWM driver to function).
         return false;
     }