-
Notifications
You must be signed in to change notification settings - Fork 250
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
Watch face for tracking deadlines. #266
Conversation
You can enter and monitor up to four different deadlines by providing their respective date and time. The watch face displays the remaining time at matching granularity, ranging from years to seconds.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks amazing, I just think it could be improved even further by eliminating the need for higher tick frequencies via screen updates on user input.
#define SETTINGS_NUM (5) | ||
const char settings_titles[SETTINGS_NUM][3] = { "YR", "MO", "DA", "HR", "M1" }; | ||
|
||
static uint8_t tick_freq; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be a member of the deadline_state_t
structure. There might be multiple instances of a face on the watch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for pointing this out. Fixed with 1ea192c
return movement_default_loop_handler(event, settings); | ||
} | ||
|
||
/* Slow down frequency after first loop for snappiness */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be increasing power consumption unnecessarily. Updating the screen when user input events arrive should make the user interface responsive and snappy without raising the tick frequency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure why I set the tick frequency to 1/4 Hz on the first run of the main loop. Snappiness? I could not notice any difference in the emulator. I removed this exception and set the frequency to 1 Hz for the entire face, except for the settings loop with blinking digits. Fix is in 7e28d40.
switch (state->mode) { | ||
case DEADLINE_FACE_SETTING: | ||
_setting_loop(event, settings, context); | ||
break; | ||
default: | ||
case DEADLINE_FACE_RUNNING: | ||
_running_loop(event, settings, context); | ||
break; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The individual loop functions for each mode of the watch face is really good design!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code seems great, the watch face is useful and the few remaining concerns have been resolved. Only constructive suggestions for future improvements remain. Therefore the reviewed changes are approved.
I have also tested this branch in the simulator and found it to work well and have a responsive user interface.
case EVENT_TICK: | ||
if (state->tick_freq == 8) { | ||
if (watch_get_pin_level(BTN_ALARM)) { | ||
_increment_date(settings, state, date_time); | ||
_setting_display(event, settings, state, date_time); | ||
} else { | ||
_change_tick_freq(4, state); | ||
} | ||
} | ||
break; | ||
case EVENT_ALARM_LONG_PRESS: | ||
_change_tick_freq(8, state); | ||
break; | ||
case EVENT_ALARM_LONG_UP: | ||
_change_tick_freq(4, state); | ||
break; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The higher frequency is necessary for the blinking animations. This is consistent with the user interface of the stock watch and other sensor watch faces.
if (settings->bit.clock_mode_24h) { | ||
watch_set_indicator(WATCH_INDICATOR_24H); | ||
sprintf(buf, "%s%2d%2d%02d ", settings_titles[state->current_page], i, date_time.unit.hour, date_time.unit.minute); | ||
} else { | ||
sprintf(buf, "%s%2d%2d%02d ", settings_titles[state->current_page], i, (date_time.unit.hour % 12) ? (date_time.unit.hour % 12) : 12, | ||
date_time.unit.minute); | ||
if (date_time.unit.hour < 12) | ||
watch_clear_indicator(WATCH_INDICATOR_PM); | ||
else | ||
watch_set_indicator(WATCH_INDICATOR_PM); | ||
} | ||
} else { | ||
watch_clear_colon(); | ||
watch_clear_indicator(WATCH_INDICATOR_24H); | ||
watch_clear_indicator(WATCH_INDICATOR_PM); | ||
sprintf(buf, "%s%2d%2d%02d%02d", settings_titles[state->current_page], i, date_time.unit.year + 20, date_time.unit.month, date_time.unit.day); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code is similar to the simple clock watch face's time display logic. I have refactored this code to be more functional and organized in PR #373. If accepted, it could result in further improvements to this new watch face.
Perhaps it would be possible to make more shared time display functions which could also be used here in future versions.
case 2: | ||
buf[8] = buf[9] = ' '; | ||
break; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is consistent with the user interface of digital quartz watches in general.
I have no data regarding the power consumption of the animations. I speculate that a menu-driven interface would consume less power but that's a minor separate discussion that has no impact on the quality of this PR.
This watch face draws inspiration from other faces of the project but focuses on keeping track of up to four different deadlines by displaying time remaining in varying granularities. Tested-by: Konrad Rieck <[email protected]> Tested-by: Matheus Afonso Martins Moreira <[email protected]> Reviewed-by: Matheus Afonso Martins Moreira <[email protected]> Signed-off-by: Matheus Afonso Martins Moreira <[email protected]> GitHub-Pull-Request: joeycastillo#266
This watch face draws inspiration from other faces of the project but focuses on keeping track of up to four different deadlines by displaying time remaining in varying granularities. Tested-by: Konrad Rieck <[email protected]> Tested-by: Matheus Afonso Martins Moreira <[email protected]> Reviewed-by: Matheus Afonso Martins Moreira <[email protected]> Signed-off-by: Matheus Afonso Martins Moreira <[email protected]> GitHub-Pull-Request: joeycastillo#266
This watch face draws inspiration from other faces of the project but focuses on keeping track of up to four different deadlines by displaying time remaining in varying granularities. Tested-by: Konrad Rieck <[email protected]> Tested-by: Matheus Afonso Martins Moreira <[email protected]> Reviewed-by: Matheus Afonso Martins Moreira <[email protected]> Signed-off-by: Matheus Afonso Martins Moreira <[email protected]> GitHub-Pull-Request: joeycastillo#266
This watch face draws inspiration from other faces of the project but focuses on keeping track of up to four different deadlines by displaying time remaining in varying granularities. Tested-by: Konrad Rieck <[email protected]> Tested-by: Matheus Afonso Martins Moreira <[email protected]> Reviewed-by: Matheus Afonso Martins Moreira <[email protected]> Signed-off-by: Matheus Afonso Martins Moreira <[email protected]> GitHub-Pull-Request: joeycastillo#266
Am now running this code on real hardware. Tracking two deadlines right now. No issues. |
I was having high power usage issues on my watch and although I initially suspected it might have something to do with silicon errata workaround code I eventually isolated the problem to this watch face by disassembling and resetting the watch and then testing each face until sleep mode no longer worked: it happened after a deadline was set. It reduced battery voltage from 2.97 V to 2.91 V in a span of about 18 hours. I will review this PR again in order to try and figure out what the problem is. Do you have any ideas why this might be happening? |
Oh, that's a nasty surprise. I have been running the face for over 6 months on my watch, but have not monitored the voltage. Most of the time, the face was in the background. Did you have it in the foreground when you measured the voltage drop? Guess 1: If the face was in the foreground, I already have some ideas about where the problem is coming from and how to mitigate it. The face calculates the remaining time with different granularity when in the foreground. This can be a bit involved due to the complexity of the Gregorian calendar. I can think of two improvements:
Guess 2: Before I get into the above optimizations, however, I want to rule out other issues. I could also imagine that this is a problem with the tick frequency. The calculations for time remaining should happen at 1 Hz, which might be acceptable (I don't know). However, if for some reason, the clock is still running at 4 Hz or more we would definitely waste a lot of energy with unnecessary computations. I hope to have some time this weekend. |
In my case, the face was in the background. I set a deadline until my birthday and then moved on to test other faces and finetune the watch. I set it down for a bit and when I came back I noticed it was not in sleep mode: the clock face still displayed the seconds. So I hacked in a 10 second low energy timer, reassembled the watch and tested the faces one by one by using it, moving back to the clock and waiting for the low power tick tock animation. Verified that sleep no longer engaged after setting a deadline. I looked at the code again and saw that it was using scheduled tasks in the background. I don't think those are supposed to prevent sleep though, at least that's my impression from reading Can you reproduce this issue? Or does your watch enter sleep mode normally? I'm asking because I flashed a branch with more features than just this PR. They might be interacting with each other. |
Yes, I can reproduce the problem. 😢 The watch no longer goes into sleep mode when a deadline is set. I assume this is a bug introduced with some other changes to movement. At least, I don't remember seeing this behavior when the watch face was first developed. Unfortunately, it's a showstopper as of now. We need to figure out what's going on and prevent entering sleep mode. I will have a look at this over the weekend. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
High power consumption issue. Reproduced by author.
Which commit was it originally based on? |
That is surprisingly hard to say. I have two watches and merged in the changes from |
- A background task is only scheduled if the alarm option is activated. If the option is enabled, an alarm sounds when the next deadline is reached. If the option is disabled, no alarm sounds and the watch can enter low energy sleep. - Fixed tick frequency error. During running mode, the clock ticks at 1Hz. This is set in the init function `_running_init` and thus ensures that we do not run too fast when returning from the setting mode. - Minor corrections to comments and indentations.
I spent a little time investigating this problem. We have two independent issues here: (a) the watch face does not enter sleep mode, and (b) the battery is draining badly. (a) I did some research, and movement cannot enter sleep mode when a task is still scheduled. In this case, the main loop calls I have slightly changed the implementation of the deadline face so that the user can now enable or disable the alarm function (long press on the light button). When the alarm function is enabled, an alarm will sound when a deadline is reached. This requires a background task, and sleep mode is not available. However, no task is scheduled if the alarm function is deactivated and sleep mode works as expected. (b) I ran the face with a deadline set and the alarm disabled on my RED board for 8 hours and could not observe any change in voltage. I repeated the test for 8 hours with alarm enabled. Again, no voltage drop could be detected. I suppose the face is not draining the battery, and what you have observed is a result of other patches to the deployed firmware. We didn't immediately realize there were two problems because we took the absence of sleep mode as an indication of this problem, which is not the case. |
I see. Thanks, I didn't catch that. I wonder why that's the case. I thought movement set up a timer for the task. I mean the whole point of scheduling a task in the background is to be interrupted when the time is right in order to handle it. Perhaps the real solution lies in somehow making the scheduler compatible with sleep mode.
Suggestion: use a minutely background tick when alarm is enabled but schedule a task only when the time left is less than one minute. That way the sleep mode will only be unavailable in the last 60 seconds until the deadline. What do you think?
It's possible. Although I observed a reduction in voltage while I had the deadline face flashed, I also had other code running which is a source of confounding. I have removed the deadline face from the build and reflashed the firmware, and the voltage returned to a nominal 3 V. I also observe voltage drops of about 0.05 V when I illuminate the LED and switch to the voltage readout face. It returns to nominal immediately after the LED is turned off. I assume the same thing is happening here. Seems to be a sign the face is consuming more processor resources than anticipated. I definitely overestimated the battery drain the first time around though. |
I am confused. What is a background tick? I thought the lowest frequency for faces is 1 Hz. Are you suggesting to intercept the
Could you run the experiment again by flashing the deadline face back on the firmware? I have found that the voltage is not really a reliable source of information. Looking at the code of the face, I wonder if there is any computation at all when it is running in the background and no task is scheduled. I suspect something else is causing problems, including voltage being a poor indicator. |
My current understanding of the source code is that there are two types of tasks:
Scheduled tasks are run when the time equals the scheduled time. It seems we just discovered that they stop the device from entering low power mode. Background tasks execute once a minute and are handled even by the low power movement loop. They are executed by this function: Sensor-Watch/movement/movement.c Lines 160 to 170 in af18673
Both the normal and low power movement loops call this function when Sensor-Watch/movement/movement.c Lines 399 to 403 in af18673
In order to use them, the watch face must provide a I suggested that a Alternatively, limit the deadline resolution to minutes and check whether the targeted minute has been reached and activate the buzzer when handling the background event. This is how the other watch faces seem to work, including the clock face's hourly chime.
Sure. I'll do that as soon as my branch gets merged, I'm going to be testing it over the next few days. I should buy a second sensor watch board and a used watch for testing...
It's possible. I assumed it was reliable because I saw the battery endurance tests run by @joeycastillo which graphed battery voltage over time, thereby establishing that battery voltage remained above the minimum for reliable operation for over one year. It's certainly proving to be hard to pin down. Sadly I don't have those power profilers... |
The alarm for deadlines is now handled via a background task. Instead of a scheduled task that prevents sleep mode, the face checks in the background every minute whether an deadline is due. If this is the case, the face wakes up from sleep mode and starts a scheduled task for the remaining seconds.
Now, I got it. That's a great idea. I was not aware of the background task feature. I have implemented the alarm functionality as you suggested. When the alarm is enabled (bell visible), the face checks every minute and schedules a task only at the last minute for the remaining seconds. If the alarm function is deactivated, nothing happens. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good! Have you tested it on your watch yet? I will soon.
/* Determine closest deadline */ | ||
watch_date_time now = watch_rtc_get_date_time(); | ||
uint32_t now_ts = watch_utility_date_time_to_unix_time(now, _get_tz_offset(settings)); | ||
uint32_t next_ts = state->deadlines[_closest_deadline(settings, state)]; | ||
|
||
/* No active deadline */ | ||
if (next_ts < now_ts) | ||
return false; | ||
|
||
/* No deadline within next 60 seconds */ | ||
if (next_ts >= now_ts + 60) | ||
return false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you considered a simpler implementation that ignores seconds? Then you'd be able to avoid the need for scheduled tasks in the first place. The minutely background task would suffice.
// Filter out the seconds
current_date_time >>= 6;
closest_deadline_date_time >>= 6;
// Handle deadline reached event in loop
return current_date_time == closest_deadline_date_time;
It would reduce the resolution of the deadlines from seconds to minutes but I have a feeling that's an acceptable tradeoff. This face is designed to keep track of deadlines spanning years. Timing so precise as to require seconds resolution feels like the domain of countdown timers.
It's up to you. The code seems significantly more sleep mode friendly now and I think it's perfectly acceptable. I will be testing it on my watch soon.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've thought about it, and I'm not really convinced. I usually track deadlines within days, and I don't want to miss the ticking seconds when the last day starts. Moreover, I think that people who use finetune_face.c
to make their clock as accurate as possible deserve accurate tracking of deadlines.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Absolutely. All good points as well.
I'll be testing the new version soon!
/* Deadline within next minute. Let's set up an alarm */ | ||
watch_date_time next = watch_utility_date_time_from_unix_time(next_ts, _get_tz_offset(settings)); | ||
movement_request_wake(); | ||
movement_schedule_background_task_for_face(state->face_idx, next); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this code be in the watch face's loop function?
I'm not exactly sure if there's any problem with having this logic in this callback. I think there's no problem. I'm not sure why movement goes to the trouble of returning from this function only to call the main loop with a background event. Not sure if there's a rationale for it. I'm just highlighting the fact other faces seem to work this way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right. Changing this would require a bit more logic for the background event, as it could now be both a background and a scheduled task. I would leave it as is, despite the slight violation of the original intent of ...wants_background_task
.
I have been running the face on my watch for the last 3 months. No issues observed. |
Adds a timing watch face that focuses on keeping track of specific deadlines. Dates and times can be set on the watch face and it will display the time remaining at ever decreasing levels of granularity for the sake of brevity and ease of understanding. For example, it will display years and months if over a year is left, months and days if less than one year is left, days and hours if less than one month is left and the full remaining time if less than a day is left. It also notifies the user when a deadline has passed recently. Reviewed-by: Matheus Afonso Martins Moreira <[email protected]> Tested-on-hardware-by: Konrad Rieck <[email protected]> GitHub-Pull-Request: #465
Adds a timing watch face that focuses on keeping track of specific deadlines. Dates and times can be set on the watch face and it will display the time remaining at ever decreasing levels of granularity for the sake of brevity and ease of understanding. For example, it will display years and months if over a year is left, months and days if less than one year is left, days and hours if less than one month is left and the full remaining time if less than a day is left. It also notifies the user when a deadline has passed recently. Reviewed-by: Matheus Afonso Martins Moreira <[email protected]> Tested-on-hardware-by: Konrad Rieck <[email protected]> GitHub-Pull-Request: #266
Adds a timing watch face that focuses on keeping track of specific deadlines. Dates and times can be set on the watch face and it will display the time remaining at ever decreasing levels of granularity for the sake of brevity and ease of understanding. For example, it will display years and months if over a year is left, months and days if less than one year is left, days and hours if less than one month is left and the full remaining time if less than a day is left. It also notifies the user when a deadline has passed recently. Reviewed-by: Matheus Afonso Martins Moreira <[email protected]> Tested-on-hardware-by: Konrad Rieck <[email protected]> GitHub-Pull-Request: #266
Adds a timing watch face that focuses on keeping track of specific deadlines. Dates and times can be set on the watch face and it will display the time remaining at ever decreasing levels of granularity for the sake of brevity and ease of understanding. For example, it will display years and months if over a year is left, months and days if less than one year is left, days and hours if less than one month is left and the full remaining time if less than a day is left. It also notifies the user when a deadline has passed recently. Reviewed-by: Matheus Afonso Martins Moreira <[email protected]> Tested-on-hardware-by: Konrad Rieck <[email protected]> GitHub-Pull-Request: #266
This PR has been merged into |
This watch face draws inspiration from other faces of the project but focuses on keeping track of deadlines. You can enter and monitor up to four different deadlines by providing their respective date and time. The face then display the remaining time at different granularity. It has two modes: running mode and settings mode.
Running Mode
When the watch face is activated, it defaults to running mode. The top right corner shows the current deadline number, and the main display presents the time left until the deadline. The format of the display varies depending on the remaining time.
When less than a day is left, the display shows the remaining hours, minutes, and seconds in the form
HH:MM:SS
.When less than a month is left, the display shows the remaining days and hours in the form
DD:HH
with the unitdy
for days.When less than a year is left, the display shows the remaining months and days in the form
MM:DD
with the unitmo
for months.When more than a year is left, the years and months are displayed in the form
YY:MM
with the unityr
for years.When a deadline has passed in the last 24 hours, the display shows
over
to indicate that the deadline has just recently been reached.When no deadline is set for a particular slot, or if a deadline has already passed by more than 24 hours,
--:--
is displayed.The user can navigate in running mode using the following buttons:
The alarm button moves the next deadline. There are currently four slots available for deadlines. When the last slot has been reached, pressing the button moves to the first slot.
A long press on the alarm button activates settings mode and enables configuring the currently selected deadline.
Settings Mode
In settings mode, the currently selected slot for a deadline can be configured by providing the date and the time. Like running mode, the top right corner of the display indicates the current deadline number. The main display shows the date and, on the next page, the time to be configured.
The user can use the following buttons in settings mode.
The light button navigates through the different date and time settings, going from year, month, day, hour, to minute. The selected position is blinking.
A long press on the light button resets the date and time to the next day at midnight. This is the default deadline.
The alarm button increments the currently selected position. A long press on the alarm button changes the value faster.
The mode button exists setting mode and returns to running mode. Here the selected deadline slot can be changed.