Skip to content

Add stopwatch face to movement #21

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 11, 2021

Conversation

tahnok
Copy link
Collaborator

@tahnok tahnok commented Nov 4, 2021

Simple stopwatch that only counts seconds (not subseconds), minutes and yes, even hours

Current status: broken because the low energy mode locks me out after a minute! I see there's a _movement_reset_inactivity_countdown in movement.c I could expose, but I'm wondering if you had a different idea in mind for letting a complication opt out of the low power mode

@joeycastillo
Copy link
Owner

joeycastillo commented Nov 5, 2021

So, my thought process was that (for Movement, at least) watch faces would not be able to opt out of the low energy mode. The once-per-minute updates allow for regular screen refreshes, and background tasks allow for occasional tasks, but the goal would be for watch faces to not require active ticking to accomplish their tasks beyond the timeout period. This might require some creative thinking for situations like this.

In this case, one could make a basic stopwatch (i.e. without a stop / restart function) by capturing just the time the stopwatch started, and then calculating the amount of time that elapsed since that time. That way even if the watch goes into deep sleep, when it wakes up it can see that the stopwatch started at 9:00 PM, now it's 10:08, and therefore 68 minutes have passed. Off top of head, one might be able to add the stop/start function by tracking three variables: a running boolean, the timestamp when the stopwatch was last started, and a duration that's added to whenever the stopwatch is stopped. The stopwatch display would be the sum of the duration and, if running, the time interval since the watch was last started.

This would admittedly require some date and time calculation functions that the watch library doesn't have yet, and I sense it's going to be subtle to get those right. but in my mind this is how the stopwatch could work while still sipping power in the low energy mode.

@tahnok
Copy link
Collaborator Author

tahnok commented Nov 8, 2021

I like the idea of putting in the work to make the stopwatch face really power efficient by storing the 'start' instant and calculating based on that how much time has elapsed to that it doesn't matter how many ticks / time has elapsed

Howeever... I think there's a few other faces (like a countdown timer) that would benefit from being able to be updated once a second like the 'main' clock face is, at least until the low power timeout kicks in

@tahnok
Copy link
Collaborator Author

tahnok commented Nov 8, 2021

Ah, I re-read through the simple_clock and movement code and I see I can get longer updates if I treat the TICK and TIMEOUT the same, which works but doesn't seem to be in the spirit of things

@joeycastillo
Copy link
Owner

I think you can safely ignore EVENT_TIMEOUT now (I fixed a bug in ef40d58 that was causing faces to break there), but for a stopwatch to ignore this is very much is in the spirit of things! The timeout is meant for things like a settings screen, where you want the watch to return home after a while. If a watch face is clock or time-oriented, it probably should ignore the timeout and remain on screen since it's something the wearer will want to look at.

FWIW, the main face doesn't get any special privileges; it doesn't tick when other faces are on screen, and I'm hesitant to have any off-screen face getting ticks because of the low power implications of several watch faces opting in to this (anything that happens once a second is going to add up). A countdown timer is the main use case that deeply challenges my assumptions here. Still, I think that we can refactor Movement's current use of the RTC's alarm function to make this work, without the watch face needing to count down on its own.

@tahnok
Copy link
Collaborator Author

tahnok commented Nov 9, 2021

Ah, I see the confusion now. I thought it was intentional that after 60s (or whatever the user chooses as the timeout) a watch face stopped getting EVENT_TICK events while 'active', and I was looking for away around that. I wasn't trying to figure out a way to keep getting tick events in the background.

I've cleaned things up a bit and put up a version that doesn't use the start instant + run time, save that for a follow up PR

You also mentioned the countdown / RTC alarm refactor. Would love to discuss that further and even take a crack at implementing it if you'd like. I can see a few uses cases (alarm, countdown, sensor measurements) where a nice API like set_alarm_callback(time, callback_fn) would be quite useful

Copy link
Owner

@joeycastillo joeycastillo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good! I'm down to merge it with some very minor changes, detailed inline. And then if at a later date we want to implement the start time + run time method, we can do it here instead of adding a second stopwatch face.

}
}

sprintf(buf, "sw%2d%02d%02d", stopwatch_state->hours, stopwatch_state->minutes, stopwatch_state->seconds);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One change I'd make: the W doesn't work well in position 1 (there's no downstroke, so it ends up looking like SU). I'd change this and the string on line 26 to say ST, which seems clearer.

#include "movement.h"

typedef struct {
bool measuring;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest renaming this variable to running; I sense it's a more descriptive name and would add more clarity to the code.

stopwatch_state->seconds = 0;
stopwatch_state->minutes = 0;
stopwatch_state->hours = 0;
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you add a watch_display_string("sw 00000", 0); line here, the display will update immediately on the button press, instead of having to wait for the next tick :)

Simple stopwatch that only counts seconds (not subseconds), minutes
and yes, even hours
@tahnok
Copy link
Collaborator Author

tahnok commented Nov 11, 2021

Great suggestions, I've made the changes and force pushed a new version

@joeycastillo joeycastillo merged commit 323e6df into joeycastillo:main Nov 11, 2021
@tahnok tahnok deleted the stopwatch_face branch November 12, 2021 01:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants