zmkfirmware / zmk

ZMK Firmware Repository
https://zmk.dev/
MIT License
2.63k stars 2.69k forks source link

Improvements in event subscriptions #794

Open okke-formsma opened 3 years ago

okke-formsma commented 3 years ago

A possible solution is to keep a (linked) list of event handlers for each event. The event handlers would be ordered by "priority". Event handlers are called in order of priority. In addition to the current ZMK_SUBSCRIPTION macro, functions should be exposed for (de)registering event handlers. This could be done similar to zehyr's K_WORK_DEFINE / k_work.

example 1: fine grained event handlers with priority

old code:

int hid_listener(const zmk_event_t *eh) {
    const struct zmk_keycode_state_changed *kc_ev = as_zmk_keycode_state_changed(eh);
    if (kc_ev) {
        if (kc_ev->state) {
            hid_listener_keycode_pressed(kc_ev);
        } else {
            hid_listener_keycode_released(kc_ev);
        }
        return 0;
    }
    const struct zmk_mouse_move_state_changed *ms_ev = as_zmk_mouse_move_state_changed(eh);
    if (ms_ev) {
        if (ms_ev->state) {
            hid_listener_mouse_move_pressed(ms_ev);
        } else {
            hid_listener_mouse_move_released(ms_ev);
        }
        return 0;
    }
    return 0;
}

ZMK_LISTENER(hid_listener, hid_listener);
ZMK_SUBSCRIPTION(hid_listener, zmk_keycode_state_changed);
ZMK_SUBSCRIPTION(hid_listener, zmk_mouse_move_state_changed);

new code;

int hid_listener_keycode_pressed(const struct zmk_keycode_pressed *kc_ev) { ... }
int hid_listener_keycode_released(const struct zmk_keycode_released *kc_ev) { ... }
int hid_listener_mouse_move_pressed(const struct zmk_mouse_move_pressed *kc_ev) { ... }
int hid_listener_mouse_move_released(const struct zmk_mouse_move_released *kc_ev) { ... }

ZMK_SUBSCRIPTION(hid_listener_keycode_pressed, zmk_keycode_pressed, 100);
ZMK_SUBSCRIPTION(hid_listener_keycode_released, zmk_keycode_released, 100);
ZMK_SUBSCRIPTION(hid_listener_mouse_move_pressed, zmk_mouse_move_pressed, 200);
ZMK_SUBSCRIPTION(hid_listener_mouse_move_released, zmk_mouse_move_released, 200);

example 2: dynamic event handlers

old code

# somewhere `undecided_hold_tap` is set to non-null if we care about the event listener
undecided_hold_tap = NULL;
...
undecided_hold_tap = some_pointer;
static int keycode_state_changed_listener(const zmk_event_t *eh) {
    struct zmk_keycode_state_changed *ev = as_zmk_keycode_state_changed(eh);

    if (undecided_hold_tap == NULL) {
        // LOG_DBG("0x%02X bubble (no undecided hold_tap active)", ev->keycode);
        return ZMK_EV_EVENT_BUBBLE;
    }
    ...
}

new code

# somewhere the subscription is set/released
zmk_subscribe(keycode_pressed_listener, zmk_keycode_pressed, 500);
...
zmk_unsubscribe(keycode_pressed_listener, zmk_keycode_pressed);
static int keycode_state_changed_listener(const struct zmk_keycode_state_changed *ev) {
    ...
}
okke-formsma commented 3 years ago

Something else that's been a burden for more complex behaviors, is the problem of context.

A subscription for a listener is not associated with any context. This forced introduction of global variables in many behaviors such as 'hold-tap' to keep track of the 'current hold-tap'. If the keycode listener could be registered in a 'context', it could be associated with the currently active hold-tap.

An interface similar to how work-items are handled would be interesting:

from https://docs.zephyrproject.org/latest/reference/kernel/threads/workqueue.html?highlight=container_of#work-item-lifecycle:

If the handler function requires additional information about the work it is to perform, the work item can be embedded in a larger data structure. The handler function can then use the argument value to compute the address of the enclosing data structure with CONTAINER_OF, and thereby obtain access to the additional information it needs.

static void work_handler(struct k_work *work)
{
        struct k_work_delayable *dwork = k_work_delayable_from_work(work);
        struct work_context *ctx = CONTAINER_OF(dwork, struct work_context,
                                                timed_work);

this would translate to something like

static int keycode_state_changed_listener(const struct zmk_keycode_state_changed *ev)
        struct hold_tap_context *hold_tap = CONTAINER_OF(ev, struct hold_tap_context, keycode_listener);