whiterose: add new clock face

[jdek]

A simple sub-hour audible signal useful for tracking time without having to actually look at the watch itself.

Inspired by the TV show Mr. Robot:

Each beep indicates one minute of my time that has passed. I have allotted you no more than three minutes. -- Whiterose

https://www.youtube.com/watch?v=GHI1Rq9oTh8 (spoiler)

[joeycastillo]

While I like this as a concept, I'm hesitant to add it to the core of Movement just because it is a bit of an edge case, and it feels like a lot to ship this feature to everyone (including the additional support burden of explaining another settings screen).

Instead I think you could accomplish this with a watch face, using the background task request to make the signals and the watch face itself as the configuration interface. It would be a way to make the feature opt-in for folks that wanted it, without going too deep into Movement's core code.

[jdek]

I'll give implementing it as a separate face a go, but I'm concerned about divergence with the simple_clock_face since it should have feature parity apart from the extra audible signals. What about a compile time flag?

[jdek]

@joeycastillo what do you think of this? Essentially just simple_clock with the variable beep feature inside the face itself.

[WesleyAC]

@jdek I believe @joeycastillo's suggestion was to make a new face that isn't a clock, but rather just has a setting for whether the beep is enabled and how frequently it happens, letting the user use whatever clock face they want. The beep will still happen, even if the face is in the background. Removing the clock from this, and adding a option to disable the beep altogether would get it there, I think.

[jdek]

Hi, I'm looking at this again. @WesleyAC, if the face is only a settings face managing the granular beep, wouldn't this conflict with simple_clock_face's hourly signal? I'm not understanding how this is supposed to work smoothly. If the settings face only handles beeps within the hour (let's use 15 min signal as an example): Settings: xx:15 xx:30 xx:45; Clock: xx:00. Then we have a scenario whereby you have to turn off both the signal in simple clock and beep in settings. Even if we give the settings face full control of both the signal and the beep then we can no longer easily turn off the beeps from the main clock (which already has a signal function).

The user should be able to easily turn on/off all signals/beeps from the main clock, where this is configured isn't particularly an issue since in normal use I don't think you end up changing that too much. The only way to achieve this is to have either:

• Communication between core & settings (read: beep interval saved and used in every clock which implements an hourly signal)
• A standalone clock implementing both clock feature & settings

The concern was that it would be confusing for users, but then sure don't make it on by default. What exactly is the downside to using a compile time flag to add this support to core? We only need 3 bits of data to indicate a reasonable amount of options: 60, 30, 15, 10, 5, 2, 1, 0 (off).

[jdek]

@matheusmoreira any thoughts on this? could it be integrated into the new clock_face.c or where should we go from here

[matheusmoreira]

@jdek

could it be integrated into the new clock_face.c

Absolutely! Would you like to do the honors? Something like this should work:

/*
 * Sound the time signal every CHIME_PERIOD minutes.
 * The defualt is 0 for an hourly chime.
 * Set to 15 for quarter hourly signals or 1 for whiterose mode!
 */
#ifndef CLOCK_FACE_CHIME_PERIOD
#define CLOCK_FACE_CHIME_PERIOD 0
#endif

bool clock_face_wants_background_task(movement_settings_t *settings, void *context) {
    // ...
    watch_date_time date_time = watch_rtc_get_date_time();

    if (CLOCK_FACE_CHIME_PERIOD == 0) {
        return date_time.unit.minute == 0;
    } else {
        return date_time.unit.minute % CLOCK_FACE_CHIME_PERIOD == 0;
    }
}

[jdek]

@matheusmoreira sure, for the simple case, but I'd really like this to be configurable at runtime (the entire functionality can be build time #ifdef I don't mind with that though). Very rarely do I actually need 1 min intervals, but I use 5 or 15 minutes quite often. How would you suggest this to be done, in a way which will actually get accepted?

[matheusmoreira]

To make it configurable at runtime, we'd have to add a preferences screen to the clock face. There are plenty of examples in the repository. Currently, long pressing ALARM toggles the chime. We could make short press toggle it and long press switch to the chime configuration screen.

I'm thinking about making an API to let individual watch faces register their preferences with the framework. Then the preferences face would be able to present a single configuration interface for all of them. It'd make things much easier, and free up controls for use in the faces.

[jdek]

Honestly, I'm fine for it to go in as unconfigurable until you rework the preferences (I have two sensor watches, so I'll just carry one which each interval I use). I don't like the idea of having preferences outside of the preferences face, it's messy and bad UI. Setting an interval should be done from a preferences face, turning it on and off could be done from the face itself. Difference between a preference and a setting is how I would look at it.

Will update the PR later today with an #ifdef'd & rebased version. Thanks.

[matheusmoreira]

Honestly, I'm fine for it to go in as unconfigurable until you rework the preferences

Will update the PR later today with an #ifdef'd & rebased version. Thanks.

Great! I'm preparing my next branch with the next batch of pull requests right now. I'll merge this feature in and test it on my own watch.

I encourage you to test my branch as well, that way you can test your proposed feature and several others in one go. It helps ensure everything is stable before merging. I'll make sure you are fully credited for your effort!

I have two sensor watches, so I'll just carry one which each interval I use

Yeah... That you're resorting to carrying two watches is the best possible evidence that we need runtime settings for this. Due to work and life, I can't give you a timeframe guarantee on how fast I can get this done. Hopefully soon but I can't make promises.

I don't like the idea of having preferences outside of the preferences face, it's messy and bad UI.

I wouldn't say "bad" as it's clearly been working... I'd say it's a little inconsistent. Most developers are individually defining the user interface for each face. There's variations in how they approach the problem. This variation can be confusing for the user. Centralizing it in the movement framework would normalize all that.

Setting an interval should be done from a preferences face, turning it on and off could be done from the face itself.

I agree with this. I believe this the ability to toggle some settings from within the watch face should be maintained as it is useful, fast and improves usability. Other settings which are more complex and less frequently modified however should probably be in a dedicated preferences face with a polished and consistent user interface.

In any case, to do this I'll be adding quite a bit of state to movement. I need to consider the impact this will have on RAM and ROM usage more carefully. I'll probably need to figure out a way to make all this wiring up as compile time static as possible.

[jdek]

@matheusmoreira apologies for my delay on this. Updated and force pushed to the PR as https://github.com/joeycastillo/Sensor-Watch/pull/279/commits/0336a0a74d5fb0269099f6ef46406527d1affb48.