It feels weird to me that the <widget>_init() function defined by ZMK_DISPLAY_WIDGET_LISTENER passes null to the get state function, which expects an event pointer. As far as I can tell, this is the only place in ZMK where a zmk_event_t* parameter may be null. While making the event type check function support being given null does fix it in this case, it think that is just hiding a bigger design issue with the display code.

The battery status widget is the only widget that actually uses the event parameter. If it is a battery event, it reads from the event. If it is some other type of event, it still has to populate the entire state struct though, and it can't get it from the event, so it has to read it from elsewhere.

The rest of the widgets just ignore the event entirely. Why even pass the event if we're mostly not going to use it?

Some alternative designs to consider:

1. Don't pass the event to the <widget>_get_state() function at all. Update battery_status_get_state() to always read from zmk_battery_state_of_charge().
   • Then there is no difference between initialization and handling a state change.
   • This is potentially less efficient though, because the data we need to copy to the state variable is probably already in the event data, and functions like zmk_ble_active_profile_is_connected() do a lot more work than just returning the value of a static variable.
   • It also requires updating the entire state even if only a part of it changed.
2. Separate <widget>_get_state() into init and update functions.
   • The init function takes no parameters. It is called once on startup, and it should populate the entire state struct.
   • The update function takes a pointer to the current state and an event pointer. It is called whenever the widget listener handles an event. It should only update the parts of the state struct that are relevant to the event.
   • The update function could potentially run directly on the display thread, provided it can always get the data it needs from events. If all the init functions are called prior to starting the display thread, then it would also be safe to remove the mutex around each state variable.
   • This design feels cleaner to me, but it also requires more code.

// Option 1
static struct foo_state foo_get_state(void) {
    // Regardless of which of a or b changed (or neither on startup), we must populate every field
    return (struct foo_state) {
        .a = get_current_a(),
        .b = get_current_b(),
    };
}

ZMK_DISPLAY_WIDGET_LISTENER(widget_foo, struct foo_state, foo_update_cb, foo_get_state);
ZMK_SUBSCRIPTION(widget_foo, a_changed);
ZMK_SUBSCRIPTION(widget_foo, b_changed);

// Option 2
static struct foo_state foo_init_state(void) {
    // This is called once on startup and has to populate every field
    return (struct foo_state) {
        .a = get_current_a(),
        .b = get_current_b(),
    };
}

static void foo_update_state(struct foo_state* state, const zmk_event_t* eh) {
    const struct a_changed* a;
    const struct b_changed* b;

    // Only update the fields that changed using the event data instead of calling other functions.
    if ((a = as_a_changed(eh)) != NULL) {
        state->a = a->value;
    } else if ((b = as_b_changed(eh)) != NULL) {
        state->b = b->value;
    }
}

ZMK_DISPLAY_WIDGET_LISTENER(widget_foo, struct foo_state, foo_update_cb, foo_init_state, foo_update_state);
ZMK_SUBSCRIPTION(widget_foo, a_changed);
ZMK_SUBSCRIPTION(widget_foo, b_changed);

@petejohanson what are your thoughts on this?