movement settings: add timeout and LED duration

6 files changed, 67 insertions, 31 deletions

diff --git a/movement/README.md b/movement/README.md
index e4eeb1c4..009234f8 100644
--- a/movement/README.md
+++ b/movement/README.md
@@ -158,10 +158,10 @@ case EVENT_MODE_BUTTON_UP:
     break;
 ```
 
-Similarly in response to the user releasing the LIGHT button, we tell Movement to illuminate the LED. Movement does not do this automatically, in case your watch face UI has another use for the LIGHT button.
+Similarly in response to the user pressing the LIGHT button, we tell Movement to illuminate the LED. Movement does not do this automatically, in case your watch face UI has another use for the LIGHT button.
 
 ```c
-case EVENT_LIGHT_BUTTON_UP:
+case EVENT_LIGHT_BUTTON_DOWN:
     movement_illuminate_led();
     break;
 ```
diff --git a/movement/movement.c b/movement/movement.c
index 8d28cc55..a95501a4 100644
--- a/movement/movement.c
+++ b/movement/movement.c
@@ -19,8 +19,8 @@ void cb_alarm_fired();
 void cb_tick();
 
 static inline void _movement_reset_inactivity_countdown() {
-    movement_state.le_mode_ticks = movement_le_inactivity_deadlines[movement_state.settings.bit.le_inactivity_interval];
-    movement_state.timeout_ticks = movement_timeout_inactivity_deadlines[movement_state.settings.bit.to_inactivity_interval];
+    movement_state.le_mode_ticks = movement_le_inactivity_deadlines[movement_state.settings.bit.le_interval];
+    movement_state.timeout_ticks = movement_timeout_inactivity_deadlines[movement_state.settings.bit.to_interval];
 }
 
 void movement_request_tick_frequency(uint8_t freq) {
@@ -31,7 +31,10 @@ void movement_request_tick_frequency(uint8_t freq) {
 }
 
 void movement_illuminate_led() {
-    movement_state.light_ticks = movement_state.settings.bit.led_duration;
+    watch_set_led_color(movement_state.settings.bit.led_red_color ? (0xF | movement_state.settings.bit.led_red_color << 4) : 0,
+                        movement_state.settings.bit.led_green_color ? (0xF | movement_state.settings.bit.led_green_color << 4) : 0);
+    movement_state.led_on = true;
+    movement_state.light_ticks = movement_state.settings.bit.led_duration * 2;
 }
 
 void movement_move_to_face(uint8_t watch_face_index) {
@@ -48,8 +51,8 @@ void app_init() {
 
     movement_state.settings.bit.led_green_color = 0xF;
     movement_state.settings.bit.button_should_sound = true;
-    movement_state.settings.bit.le_inactivity_interval = 1;
-    movement_state.settings.bit.led_duration = 3;
+    movement_state.settings.bit.le_interval = 1;
+    movement_state.settings.bit.led_duration = 1;
     _movement_reset_inactivity_countdown();
 }
 
@@ -112,14 +115,6 @@ bool app_loop() {
         movement_state.watch_face_changed = false;
     }
 
-    // If the LED is off and should be on, turn it on
-    if (movement_state.light_ticks > 0 && !movement_state.led_on) {
-        watch_set_led_color(movement_state.settings.bit.led_red_color ? (0xF | movement_state.settings.bit.led_red_color << 4) : 0,
-                            movement_state.settings.bit.led_green_color ? (0xF | movement_state.settings.bit.led_green_color << 4) : 0);
-        movement_state.led_on = true;
-
-    }
-
     // if the LED is on and should be off, turn it off
     if (movement_state.led_on && movement_state.light_ticks == 0) {
         // unless the user is holding down the LIGHT button, in which case, give them more time.
@@ -215,7 +210,7 @@ void cb_tick() {
     watch_date_time date_time = watch_rtc_get_date_time();
     if (date_time.unit.second != movement_state.last_second) {
         if (movement_state.light_ticks) movement_state.light_ticks--;
-        if (movement_state.settings.bit.le_inactivity_interval && movement_state.le_mode_ticks > 0) movement_state.le_mode_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--;
 
         movement_state.last_second = date_time.unit.second;
diff --git a/movement/movement.h b/movement/movement.h
index d116a6e0..41446a9b 100644
--- a/movement/movement.h
+++ b/movement/movement.h
@@ -6,12 +6,12 @@
 // TODO: none of this is implemented
 typedef union {
     struct {
-        uint32_t reserved : 14;
+        uint32_t reserved : 15;
         uint32_t clock_mode_24h : 1;        // determines whether clock should use 12 or 24 hour mode.
         uint32_t button_should_sound : 1;   // if true, pressing a button emits a sound.
-        uint32_t to_inactivity_interval : 2;// an inactivity interval for asking the active face to resign.
-        uint32_t le_inactivity_interval : 3;// 0 to disable low energy mode, or an inactivity interval for going into low energy mode.
-        uint32_t led_duration : 3;          // how many seconds to shine the LED for, or 0 to disable it.
+        uint32_t to_interval : 2;           // an inactivity interval for asking the active face to resign.
+        uint32_t le_interval : 3;           // 0 to disable low energy mode, or an inactivity interval for going into low energy mode.
+        uint32_t led_duration : 2;          // how many seconds to shine the LED for (x2), or 0 to disable it.
         uint32_t led_red_color : 4;         // for general purpose illumination, the red LED value (0-15)
         uint32_t led_green_color : 4;       // for general purpose illumination, the green LED value (0-15)
     } bit;
diff --git a/movement/watch_faces/clock/simple_clock_face.c b/movement/watch_faces/clock/simple_clock_face.c
index 95db3901..70e35047 100644
--- a/movement/watch_faces/clock/simple_clock_face.c
+++ b/movement/watch_faces/clock/simple_clock_face.c
@@ -65,7 +65,7 @@ bool simple_clock_face_loop(movement_event_t event, movement_settings_t *setting
         case EVENT_MODE_BUTTON_UP:
             movement_move_to_next_face();
             return false;
-        case EVENT_LIGHT_BUTTON_UP:
+        case EVENT_LIGHT_BUTTON_DOWN:
             movement_illuminate_led();
             break;
         case EVENT_ALARM_BUTTON_UP:
diff --git a/movement/watch_faces/complications/pulsometer_face.c b/movement/watch_faces/complications/pulsometer_face.c
index abe002fb..d54d8ddf 100644
--- a/movement/watch_faces/complications/pulsometer_face.c
+++ b/movement/watch_faces/complications/pulsometer_face.c
@@ -24,7 +24,7 @@ bool pulsometer_face_loop(movement_event_t event, movement_settings_t *settings,
         case EVENT_MODE_BUTTON_UP:
             movement_move_to_next_face();
             break;
-        case EVENT_LIGHT_BUTTON_UP:
+        case EVENT_LIGHT_BUTTON_DOWN:
             movement_illuminate_led();
             break;
         case EVENT_ALARM_BUTTON_DOWN:
diff --git a/movement/watch_faces/settings/preferences_face.c b/movement/watch_faces/settings/preferences_face.c
index 98a2372d..b68f2a39 100644
--- a/movement/watch_faces/settings/preferences_face.c
+++ b/movement/watch_faces/settings/preferences_face.c
@@ -2,8 +2,16 @@
 #include "preferences_face.h"
 #include "watch.h"
 
-#define PREFERENCES_FACE_NUM_PREFEFENCES (5)
-const char preferences_face_titles[PREFERENCES_FACE_NUM_PREFEFENCES][11] = {"CL        ", "Bt  Beep  ", "SC        ", "Lt   grn  ", "Lt   red  "};
+#define PREFERENCES_FACE_NUM_PREFEFENCES (7)
+const char preferences_face_titles[PREFERENCES_FACE_NUM_PREFEFENCES][11] = {
+    "CL        ",   // Clock: 12 or 24 hour
+    "BT  Beep  ",   // Buttons: should they beep?
+    "TO        ",   // Timeout: how long before we snap back to the clock face?
+    "LE        ",   // Low Energy mode: how long before it engages?
+    "LT        ",   // Light: duration
+    "LT   grn  ",   // Light: green component
+    "LT   red  ",   // Light: red component
+};
 
 void preferences_face_setup(movement_settings_t *settings, void ** context_ptr) {
     (void) settings;
@@ -36,12 +44,18 @@ bool preferences_face_loop(movement_event_t event, movement_settings_t *settings
                     settings->bit.button_should_sound = !(settings->bit.button_should_sound);
                     break;
                 case 2:
-                    settings->bit.le_inactivity_interval = settings->bit.le_inactivity_interval + 1;
+                    settings->bit.to_interval = settings->bit.to_interval + 1;
                     break;
                 case 3:
-                    settings->bit.led_green_color = settings->bit.led_green_color + 1;
+                    settings->bit.le_interval = settings->bit.le_interval + 1;
                     break;
                 case 4:
+                    settings->bit.led_duration = settings->bit.led_duration + 1;
+                    break;
+                case 5:
+                    settings->bit.led_green_color = settings->bit.led_green_color + 1;
+                    break;
+                case 6:
                     settings->bit.led_red_color = settings->bit.led_red_color + 1;
                     break;
             }
@@ -56,7 +70,7 @@ 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[3];
+    char buf[8];
     switch (current_page) {
         case 0:
             if (settings->bit.clock_mode_24h) watch_display_string("24h", 4);
@@ -67,7 +81,23 @@ bool preferences_face_loop(movement_event_t event, movement_settings_t *settings
             else watch_display_string("n", 9);
             break;
         case 2:
-            switch (settings->bit.le_inactivity_interval) {
+            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;
@@ -94,17 +124,28 @@ bool preferences_face_loop(movement_event_t event, movement_settings_t *settings
                     break;
             }
             break;
-        case 3:
+        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 4:
+        case 6:
             sprintf(buf, "%2d", settings->bit.led_red_color);
             watch_display_string(buf, 8);
             break;
     }
 
-    if (current_page > 2) {
+    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;