Add a 10 minute inactivity deadline.

[maxz]

I like to use the 10 minute timeout on my watch and added the 30 minute deadline to have an option for people who want something in-between. The default setting is still the 1 hour timeout.

The patch was tested on hardware and the watch properly switches into the low energy mode after the respective deadlines.

[814d3]

Very nice, I will test it and report back after 1 to 2 weeks.

[WesleyAC]

I do wonder if at some point it would make more sense to make a more granular time picker, so people could have the option of entering a value they select freely, rather than from a fixed list of options. But merging this seems fine to me — although I do wonder if the 30 minutes option will actually be used.

Could you spell "min" as n&in, as the the other parts of this face do? I would expect that to look a little nicer.

[maxz]

I adjusted the spelling. It displays alright: while it looks slightly funky, it certainly is the best to be consistent.

Like you, I'm also not sure whether the 30 minute deadline will find many users. But I though there surely are people who prefer a 10 minute timeout like I do and then there are probably people for whom 1 hour seems too long and 10 minutes too short. And it seemed like a good measure to avoid having 10 minute steps within the first hour.

I think a free selection would come with different problems. By using a 24 hour clock for the input, intervals of up to 1 day could be processed. That might work in the long run. Although the selection of never would be hard to convey through that.

[joeycastillo]

Can you double check that all the intervals still work after your change? Having ten options instead of eight would make it overflow the three bits set aside for it in the settings struct…

[814d3]

Short Feedback: Looks good to me and is running well. I testet it (green board) with "n&in" spelling also within timer_face and interval_face. But I testet only the 10 min. setting.

[joeycastillo]

After a very quick test of this in the simulator, I'm seeing where even on the settings screen, this change pushes the 2-day and 7-day options out of rotation.

I'm going to be honest, my instinct is not to accept this pull request. Shorter low energy timeouts really aren't necessary; at this point the difference in power consumption between sleep mode and wake mode is on the order of a couple of microamperes, so managing to get into this mode for an extra 30 or 50 minutes isn't likely to have a massive impact on battery life.

Low energy mode was always meant for the long intervals of days or weeks when the watch might not be worn, either because you're wearing a different watch or you forgot Sensor Watch in a drawer. Hence the desire for longer timeout options, like 7 days: this would keep the watch in wake mode effectively all the time while you're wearing it, but still allow it to go to sleep if it's very obviously not in use.

[maxz]

You are correct. I only tested the new lower timeouts. The 2 and 7 day timeouts are pushed out.

Personally I like to wear my watch and for it to switch back to the low energy mode as soon as I'm finished with accessing a TOTP code or stopping some time. I don't care too much about the seconds on the display.

I'd like to at least add the 10 minute deadline so that I don't have to indefinitely maintain it as a custom patch in addition to the other stuff that I already have and because there are surely other people which would prefer this mode of operation.

Is there some way we could add this? One option would obviously be to increase the field from 3 to 4 bits (depending on how the restraints on that struct are, I will take a look at that in a second.) Another way would be to replace one of the existing entries.

[joeycastillo]

Let’s keep 10 minutes and remove 2 days. That’ll bring it down to eight, and still offer a good range of options.

[814d3]

In addition to your points: I like the LE-mode as a button lock as it "disables" two buttons. So the shorter the better (for me) :-)

[maxz]

I made the changes and tested them in the simulator. This time all entries are obviously there. I will test them on hardware in a few minutes.

[maxz]

I tested the new commit on hardware now (and flashed the wrong firmware first, so I had to disassemble, flash and reassemble the watch twice.)

All the expected entries are there and the 10 minute and 1 hour entry work as expected (including the timeout after the proper period.)

In addition to your points: I like the LE-mode as a button lock as it "disables" two buttons. So the shorter the better (for me) :-)

I also use it for that purpose in addition to the other mentioned ones.

I though about a timeout of 5 minutes, but decided to go for 10, because 5 might sometimes be too short 10 should cover most interactions for many people. What I had in mind was among other things, how much time I would require the seconds to be visible when setting all clocks in the house after a DST change.

[maxz]

In addition to your points: I like the LE-mode as a button lock as it "disables" two buttons. So the shorter the better (for me) :-)

@814d3: Test this patch if you'd like. Something like this IMO does not belong in the mainline repository (at least not unless it's behind some optional switch) and it currently is nothing more than a very unpolished proof of concept.

Pressing the alarm button after applying this patch will switch your watch into the low energy mode. I tested it on hardware and apart from some timing issues it should work (you might sometimes have to press the button twice to wake up properly.) Although due to the timing issue it is not very pleasant to use. The transition into the low energy mode should set everything properly just as if the timeout had occurred naturally. It just sets the timeout to 0 and then lets the movement program do what it would do in such a case.

diff --git a/movement/movement.c b/movement/movement.c
index 0642221..d306efe 100644
--- a/movement/movement.c
+++ b/movement/movement.c
@@ -288,6 +288,11 @@ void movement_cancel_background_task_for_face(uint8_t watch_face_index) {
     movement_state.has_scheduled_background_task = other_tasks_scheduled;
 }

+void movement_request_low_energy_mode() {
+    movement_state.le_mode_ticks = 0;
+    movement_state.timeout_ticks = 0;
+}
+
 void movement_request_wake() {
     movement_state.needs_wake = true;
     _movement_reset_inactivity_countdown();
diff --git a/movement/movement.h b/movement/movement.h
index 1dabfbc..805dc25 100644
--- a/movement/movement.h
+++ b/movement/movement.h
@@ -304,6 +304,8 @@ void movement_cancel_background_task(void);
 void movement_schedule_background_task_for_face(uint8_t watch_face_index, watch_date_time date_time);
 void movement_cancel_background_task_for_face(uint8_t watch_face_index);

+void movement_request_low_energy_mode(void);
+
 void movement_request_wake(void);

 void movement_play_signal(void);
diff --git a/movement/watch_faces/clock/simple_clock_face.c b/movement/watch_faces/clock/simple_clock_face.c
index fbc2c4b..e357ccc 100644
--- a/movement/watch_faces/clock/simple_clock_face.c
+++ b/movement/watch_faces/clock/simple_clock_face.c
@@ -133,6 +133,9 @@ bool simple_clock_face_loop(movement_event_t event, movement_settings_t *setting
             if (state->signal_enabled) watch_set_indicator(WATCH_INDICATOR_BELL);
             else watch_clear_indicator(WATCH_INDICATOR_BELL);
             break;
+        case EVENT_ALARM_BUTTON_UP:
+            movement_request_low_energy_mode();
+            break;
         case EVENT_BACKGROUND_TASK:
             // uncomment this line to snap back to the clock face when the hour signal sounds:
             // movement_move_to_face(state->watch_face_index);

[814d3]

Thanks for the additional patch, but I think I'll continue to use the initial patch with a 10 min timeout, which has been working without problems for 1,5 weeks. From my point of view it can be merged.

Greetings