Dynamically enable/disable and order faces at runtime

[alesgenova]

First of all I would like to thank you for this great project!!

The developer experience has been great thanks to the documentation, code organization and the emulator: I'm not a C programmer (unless C++ counts :P) and don't even have the watch yet, but I was still able to put this together in a couple of evenings.

This is my first contribution to the project, and as you'll see it's quite far reaching. Should you decide to accept it, make sure to give it a careful review, cause I am not to be trusted :)

Changes to movement (first commit)

I would like to propose some changes to movement app so that we can dynamically enable/disable the faces that were built into the current firmware, as well as reorder them.

For example, lets assume the following faces have been specified in movement_config.h:

const watch_face_t watch_faces[] = {
    simple_clock_face, // CL
    alarm_face,        // AL
    stopwatch_face,    // ST
    world_clock_face,  // DT
    countdown_face,    // CD
};

When the watch first starts, the user will navigate through the pages in the following order:

CL -> AL -> ST -> DT -> CD

Let's say that now we would like to have a different arrangement (and to achieve it at runtime without rebuilding/reflashing the firmware), for example:

CL -> DT -> ST | CD

where AL is no longer there, the order of ST and DT has been swapped, and CD is a secondary face.

To achieve this, I have introduced the concept of page which is a dynamic redirect to one of the static faces. When navigating, we will now ask to go to the next page, or to the i-th page, instead of navigating based of the face like before.

By calling the following methods that have been added in this PR, we obtain the desired navigation:

movement_enable_page(1, false);
movement_swap_page_order(2, 3);
movement_set_secondary_page(4);

Internally I have added a couple of auxiliary arrays to keep track of which face is on each page and the on/off status of each face:

uint16_t page_to_face[MOVEMENT_NUM_FACES]; // i-th el. is the face_index for page i
bool watch_face_status[MOVEMENT_NUM_FACES]; // i-th el. indicates if face i is enabled

So when the the watch starts the arrays are in the following state:

watch_faces:       [ CL , AL , ST , DT, CD ]  // This array never changes
watch_face_status: [ T  , T  , T  , T , T  ]
page_to_face:      [ 0  , 1  , 2  , 3 , 4  ]
secondary_page:    0

and after calling the three methods above the arrays are in the following state:

watch_faces:       [ CL , AL , ST , DT, CD ]  // This array never changes
watch_face_status: [ T  , F  , T  , T , T  ]
page_to_face:      [ 0  , 1  , 3  , 2 , 4  ]
secondary_page:    4

New face page_ordering_face (second commit)

I have created a new face that leverages the new API and gives the user complete control over rearranging their faces:

Screencast.from.08-18-2023.09.50.26.PM.webm

Changes to watch_face_t (third commit)

In order to display a human readable label when using the page_ordering_face to help the user be aware of which face/page is being manipulated, I have extended the watch_face_t struct to include an additional optional method called label with the following signature:

typedef void (*watch_face_label)(movement_settings_t *settings, void *context, char* label, uint8_t size);

If changing the watch_face_t is considered out of scope for this improvement, you can simply revert the 3rd commit in this PR, and the watch faces will be labelled based on their static order.

[bademux]

sorry for my 5¢:

while I like the idea, watch face runtime customization is hackfix to:

• lack of load\store face settings
• no easy access (IRDA\NFC\Pin Connector) for firmware upgrade, settings load\store

[alesgenova]

sorry for my 5¢:

while I like the idea, watch face runtime customization is hackfix to:

* lack of load\store face settings

* no easy access (IRDA\NFC\Pin Connector) for firmware upgrade, settings load\store

Even if easy wireless firmware upgrade was available, I think there is still value in customizing active faces at runtime.

For example, you could build a monster firmware that includes say 100 faces, most of which will be disabled most of the time.
But then when a specific situation arises in the field, you can still enable them without needing access to any other device, straight from the watch.

[rieck]

Woah! This is highly appreciated. I regularly use the watch, and depending on the context (work, traveling, vacation, leisure), I would disable specific faces to cycle through the important ones quickly. I will test the face in the next days.

[kingannoy]

This is amazing! Especially for things like a world clock it makes a lot of sense to be able to activate/deactivate that easily. But even for a face you just want to test-drive it would be really nice to be able to deactivate it if you don't end up using it after a while. Also moving things between the primary and secondary layer is also great! ❤️

[814d3]

Am I right that it would be a bad idea to disable faces like finetune and nanosec as it would not only hide them but also stop there execution?
How should one set MOVEMENT_SECONDARY_FACE_INDEX (0 // or (MOVEMENT_NUM_FACES - 2)) in movement_config.h or is it independently of page_ordering_face?
Thank you.

[alesgenova]

Am I right that it would be a bad idea to disable faces like finetune and nanosec as it would not only hide them but also stop there execution? How should one set MOVEMENT_SECONDARY_FACE_INDEX (0 // or (MOVEMENT_NUM_FACES - 2)) in movement_config.h or is it independently of page_ordering_face? Thank you.

Can't speak for those faces since I don't use them, but it's up to the user I guess?

The initial secondary face can be set at build time through MOVEMENT_SECONDARY_FACE_INDEX,
but that can be changed at runtime by calling the new provided movement_set_secondary_face() method.
The page_orderin_face I have added in this PR also can change the secondary face, see the demo video above

[814d3]

Thanks for the fast reply.
Maybe anyone using this feature/face can tell what happens if one disables faces that "interact" with movement/the core, like preferences_face, set_time_face or the two mentioned above.

[joeycastillo]

Hey! So sorry for the delay in reviewing this. This was a huge and impressive effort; thanks so much for your thoughtful approach and detailed notes!

Alas I think this is a bit of a big change to make to the main Movement firmware, especially inasmuch as it would require rewriting a lot of documentation, and changing expectations both among folks who have previously made watch faces, and folks picking up the watch and using it for the first time.

My thought: might it make sense to make this version of Movement its own app, separate from the default Movement firmware, in the apps directory at the root of the repository? It might be a lot of duplicated code, yes, but it seems like the easiest way to let folks make use of this very cool functionality, while keeping the original Movement developer experience intact.

Let me know what you think!

[alesgenova]

@joeycastillo
The only breaking changes from a face point of view are very straightforward:

• movement_move_to_next_face() -> movement_move_to_next_page()
• movement_move_to_face(i) -> movement_move_to_page(i)

I have already taken care of fixing these changes throughout the existing faces in the repo, as well as the face template generator, and the various readmes. The only other place that would need to be changed is the docs website.

The other more invasive change is the addition of the watch_face_label function to the face struct, but as I mentioned we can just simply drop the third commit in this PR if this is a change you are not comfortable with.

Finally, I don't think making it an alternative movement app would be a good idea, because despite the minor breaking changes above, it means that all the existing and future faces wouldn't work with this alternative movement.

What do you think?

PS:
I have been using this feature on the watch for about 6 weeks, and it has been working great so far.