From 2128b2b55f85a6190194d83696f7419eb53c6642 Mon Sep 17 00:00:00 2001 From: Peter Johanson Date: Thu, 12 Aug 2021 03:44:38 +0000 Subject: [PATCH] refactor(display): Output/layer/battery thread safety. * Submit widget updates to display queue. * Use mutex to control access to shared state for widgets. --- app/include/zmk/display.h | 51 +++++++++++++++- app/src/display/main.c | 5 ++ app/src/display/widgets/battery_status.c | 53 +++++++++++------ app/src/display/widgets/layer_status.c | 50 +++++++++------- app/src/display/widgets/output_status.c | 76 +++++++++++++++--------- app/src/display/widgets/wpm_status.c | 37 +++++++----- 6 files changed, 190 insertions(+), 82 deletions(-) diff --git a/app/include/zmk/display.h b/app/include/zmk/display.h index d3bd042e..45a4bee0 100644 --- a/app/include/zmk/display.h +++ b/app/include/zmk/display.h @@ -4,8 +4,57 @@ * SPDX-License-Identifier: MIT */ +/** @file display.h + * @brief Display functions and macros. + */ + #pragma once struct k_work_q *zmk_display_work_q(); -int zmk_display_init(); \ No newline at end of file +bool zmk_display_is_initialized(); +int zmk_display_init(); + +/** + * @brief Macro to define a ZMK event listener that handles the thread safety of fetching + * the necessary state from the system work queue context, invoking a work callback + * in the display queue context, and properly accessing that state safely when performing + * display/LVGL updates. + * + * @param listener THe ZMK Event manager listener name. + * @param state_type The struct/enum type used to store/transfer state. + * @param cb The callback to invoke in the dispaly queue context to update the UI. Should be `void + * func(state_type)` signature. + * @param state_func The callback function to invoke to fetch the updated state from ZMK core. + * Should be `state type func(const zmk_event_t *eh)` signature. + * @retval listner##_init Generates a function `listener##_init` that should be called by the widget + * once ready to be updated. + **/ +#define ZMK_DISPLAY_WIDGET_LISTENER(listener, state_type, cb, state_func) \ + K_MUTEX_DEFINE(listener##_mutex); \ + static state_type __##listener##_state; \ + static state_type listener##_get_local_state() { \ + k_mutex_lock(&listener##_mutex, K_FOREVER); \ + state_type copy = __##listener##_state; \ + k_mutex_unlock(&listener##_mutex); \ + return copy; \ + }; \ + static void listener##_work_cb(struct k_work *work) { cb(listener##_get_local_state()); }; \ + K_WORK_DEFINE(listener##_work, listener##_work_cb); \ + static void listener##_refresh_state(const zmk_event_t *eh) { \ + k_mutex_lock(&listener##_mutex, K_FOREVER); \ + __##listener##_state = state_func(eh); \ + k_mutex_unlock(&listener##_mutex); \ + }; \ + static void listener##_init() { \ + listener##_refresh_state(NULL); \ + listener##_work_cb(NULL); \ + } \ + static int listener##_cb(const zmk_event_t *eh) { \ + if (zmk_display_is_initialized()) { \ + listener##_refresh_state(eh); \ + k_work_submit_to_queue(zmk_display_work_q(), &listener##_work); \ + } \ + return ZMK_EV_EVENT_BUBBLE; \ + } \ + ZMK_LISTENER(listener, listener##_cb); diff --git a/app/src/display/main.c b/app/src/display/main.c index 95b607e8..57fab2b0 100644 --- a/app/src/display/main.c +++ b/app/src/display/main.c @@ -22,6 +22,7 @@ LOG_MODULE_DECLARE(zmk, CONFIG_ZMK_LOG_LEVEL); #define ZMK_DISPLAY_NAME CONFIG_LVGL_DISPLAY_DEV_NAME static const struct device *display; +static bool initialized = false; static lv_obj_t *screen; @@ -82,6 +83,8 @@ static void stop_display_updates() { k_timer_stop(&display_timer); } +int zmk_display_is_initialized() { return initialized; } + int zmk_display_init() { LOG_DBG(""); @@ -108,6 +111,8 @@ int zmk_display_init() { start_display_updates(); + initialized = true; + LOG_DBG(""); return 0; } diff --git a/app/src/display/widgets/battery_status.c b/app/src/display/widgets/battery_status.c index 309c3e9d..7503c616 100644 --- a/app/src/display/widgets/battery_status.c +++ b/app/src/display/widgets/battery_status.c @@ -4,11 +4,13 @@ * SPDX-License-Identifier: MIT */ +#include #include #include LOG_MODULE_DECLARE(zmk, CONFIG_ZMK_LOG_LEVEL); +#include #include #include #include @@ -33,12 +35,20 @@ void battery_status_init() { lv_style_set_text_line_space(&label_style, LV_STATE_DEFAULT, 1); } -void set_battery_symbol(lv_obj_t *label) { +struct battery_status_state { + uint8_t level; +#if IS_ENABLED(CONFIG_USB) + bool usb_present; +#endif +}; + +static void set_battery_symbol(lv_obj_t *label, struct battery_status_state state) { char text[2] = " "; - uint8_t level = bt_bas_get_battery_level(); + + uint8_t level = state.level; #if IS_ENABLED(CONFIG_USB) - if (zmk_usb_is_powered()) { + if (state.usb_present) { strcpy(text, LV_SYMBOL_CHARGE); } #endif /* IS_ENABLED(CONFIG_USB) */ @@ -57,32 +67,41 @@ void set_battery_symbol(lv_obj_t *label) { lv_label_set_text(label, text); } +void battery_status_update_cb(struct battery_status_state state) { + struct zmk_widget_battery_status *widget; + SYS_SLIST_FOR_EACH_CONTAINER(&widgets, widget, node) { set_battery_symbol(widget->obj, state); } +} + +static struct battery_status_state battery_status_get_state(const zmk_event_t *eh) { + return (struct battery_status_state) { + .level = bt_bas_get_battery_level(), +#if IS_ENABLED(CONFIG_USB) + .usb_present = zmk_usb_is_powered(), +#endif /* IS_ENABLED(CONFIG_USB) */ + }; +} + +ZMK_DISPLAY_WIDGET_LISTENER(widget_battery_status, struct battery_status_state, + battery_status_update_cb, battery_status_get_state) + +ZMK_SUBSCRIPTION(widget_battery_status, zmk_battery_state_changed); +#if IS_ENABLED(CONFIG_USB) +ZMK_SUBSCRIPTION(widget_battery_status, zmk_usb_conn_state_changed); +#endif /* IS_ENABLED(CONFIG_USB) */ + int zmk_widget_battery_status_init(struct zmk_widget_battery_status *widget, lv_obj_t *parent) { battery_status_init(); widget->obj = lv_label_create(parent, NULL); lv_obj_add_style(widget->obj, LV_LABEL_PART_MAIN, &label_style); lv_obj_set_size(widget->obj, 40, 15); - set_battery_symbol(widget->obj); sys_slist_append(&widgets, &widget->node); + widget_battery_status_init(); return 0; } lv_obj_t *zmk_widget_battery_status_obj(struct zmk_widget_battery_status *widget) { - LOG_DBG("Label: %p", widget->obj); return widget->obj; } - -int battery_status_listener(const zmk_event_t *eh) { - struct zmk_widget_battery_status *widget; - SYS_SLIST_FOR_EACH_CONTAINER(&widgets, widget, node) { set_battery_symbol(widget->obj); } - return ZMK_EV_EVENT_BUBBLE; -} - -ZMK_LISTENER(widget_battery_status, battery_status_listener) -ZMK_SUBSCRIPTION(widget_battery_status, zmk_battery_state_changed); -#if IS_ENABLED(CONFIG_USB) -ZMK_SUBSCRIPTION(widget_battery_status, zmk_usb_conn_state_changed); -#endif /* IS_ENABLED(CONFIG_USB) */ diff --git a/app/src/display/widgets/layer_status.c b/app/src/display/widgets/layer_status.c index 9960f2a0..3c63565b 100644 --- a/app/src/display/widgets/layer_status.c +++ b/app/src/display/widgets/layer_status.c @@ -4,9 +4,11 @@ * SPDX-License-Identifier: MIT */ +#include #include LOG_MODULE_DECLARE(zmk, CONFIG_ZMK_LOG_LEVEL); +#include #include #include #include @@ -18,7 +20,12 @@ static lv_style_t label_style; static bool style_initialized = false; -void layer_status_init() { +struct layer_status_state { + uint8_t index; + const char *label; +}; + +static void layer_status_init() { if (style_initialized) { return; } @@ -31,49 +38,50 @@ void layer_status_init() { lv_style_set_text_line_space(&label_style, LV_STATE_DEFAULT, 1); } -void set_layer_symbol(lv_obj_t *label) { - int active_layer_index = zmk_keymap_highest_layer_active(); - - LOG_DBG("Layer changed to %i", active_layer_index); - - const char *layer_label = zmk_keymap_layer_label(active_layer_index); - if (layer_label == NULL) { +static void set_layer_symbol(lv_obj_t *label, struct layer_status_state state) { + if (state.label == NULL) { char text[6] = {}; - sprintf(text, LV_SYMBOL_KEYBOARD "%i", active_layer_index); + sprintf(text, LV_SYMBOL_KEYBOARD "%i", state.index); lv_label_set_text(label, text); } else { char text[12] = {}; - snprintf(text, 12, LV_SYMBOL_KEYBOARD "%s", layer_label); + snprintf(text, 12, LV_SYMBOL_KEYBOARD "%s", state.label); lv_label_set_text(label, text); } } +static void layer_status_update_cb(struct layer_status_state state) { + struct zmk_widget_layer_status *widget; + SYS_SLIST_FOR_EACH_CONTAINER(&widgets, widget, node) { set_layer_symbol(widget->obj, state); } +} + +static struct layer_status_state layer_status_get_state(const zmk_event_t *eh) { + uint8_t index = zmk_keymap_highest_layer_active(); + return (struct layer_status_state){.index = index, .label = zmk_keymap_layer_label(index)}; +} + +ZMK_DISPLAY_WIDGET_LISTENER(widget_layer_status, struct layer_status_state, layer_status_update_cb, + layer_status_get_state) + +ZMK_SUBSCRIPTION(widget_layer_status, zmk_layer_state_changed); + int zmk_widget_layer_status_init(struct zmk_widget_layer_status *widget, lv_obj_t *parent) { layer_status_init(); widget->obj = lv_label_create(parent, NULL); lv_obj_add_style(widget->obj, LV_LABEL_PART_MAIN, &label_style); lv_obj_set_size(widget->obj, 40, 15); - set_layer_symbol(widget->obj); sys_slist_append(&widgets, &widget->node); + widget_layer_status_init(); return 0; } lv_obj_t *zmk_widget_layer_status_obj(struct zmk_widget_layer_status *widget) { return widget->obj; -} - -int layer_status_listener(const zmk_event_t *eh) { - struct zmk_widget_layer_status *widget; - SYS_SLIST_FOR_EACH_CONTAINER(&widgets, widget, node) { set_layer_symbol(widget->obj); } - return 0; -} - -ZMK_LISTENER(widget_layer_status, layer_status_listener) -ZMK_SUBSCRIPTION(widget_layer_status, zmk_layer_state_changed); \ No newline at end of file +} \ No newline at end of file diff --git a/app/src/display/widgets/output_status.c b/app/src/display/widgets/output_status.c index 3bfbebf6..b1a3710d 100644 --- a/app/src/display/widgets/output_status.c +++ b/app/src/display/widgets/output_status.c @@ -4,11 +4,13 @@ * SPDX-License-Identifier: MIT */ +#include #include #include LOG_MODULE_DECLARE(zmk, CONFIG_ZMK_LOG_LEVEL); +#include #include #include #include @@ -23,7 +25,14 @@ static lv_style_t label_style; static bool style_initialized = false; -void output_status_init() { +struct output_status_state { + enum zmk_endpoint selected_endpoint; + bool active_profile_connected; + bool active_profile_bonded; + uint8_t active_profile_index; +}; + +static void output_status_init() { if (style_initialized) { return; } @@ -36,26 +45,34 @@ void output_status_init() { lv_style_set_text_line_space(&label_style, LV_STATE_DEFAULT, 1); } -void set_status_symbol(lv_obj_t *label) { - enum zmk_endpoint selected_endpoint = zmk_endpoints_selected(); - bool active_profile_connected = zmk_ble_active_profile_is_connected(); - bool active_profie_bonded = !zmk_ble_active_profile_is_open(); - uint8_t active_profile_index = zmk_ble_active_profile_index(); +static struct output_status_state get_state(const zmk_event_t *_eh) { + return (struct output_status_state){.selected_endpoint = zmk_endpoints_selected(), + .active_profile_connected = + zmk_ble_active_profile_is_connected(), + .active_profile_bonded = !zmk_ble_active_profile_is_open(), + .active_profile_index = zmk_ble_active_profile_index()}; + ; +} + +static void set_status_symbol(lv_obj_t *label, struct output_status_state state) { char text[6] = {}; - switch (selected_endpoint) { + switch (state.selected_endpoint) { case ZMK_ENDPOINT_USB: strcat(text, LV_SYMBOL_USB " "); break; case ZMK_ENDPOINT_BLE: - if (active_profie_bonded) { - if (active_profile_connected) { - sprintf(text, LV_SYMBOL_WIFI "%i " LV_SYMBOL_OK, active_profile_index); + if (state.active_profile_bonded) { + if (state.active_profile_connected) { + snprintf(text, sizeof(text), LV_SYMBOL_WIFI "%i " LV_SYMBOL_OK, + state.active_profile_index); } else { - sprintf(text, LV_SYMBOL_WIFI "%i " LV_SYMBOL_CLOSE, active_profile_index); + snprintf(text, sizeof(text), LV_SYMBOL_WIFI "%i " LV_SYMBOL_CLOSE, + state.active_profile_index); } } else { - sprintf(text, LV_SYMBOL_WIFI "%i " LV_SYMBOL_SETTINGS, active_profile_index); + snprintf(text, sizeof(text), LV_SYMBOL_WIFI "%i " LV_SYMBOL_SETTINGS, + state.active_profile_index); } break; } @@ -63,35 +80,36 @@ void set_status_symbol(lv_obj_t *label) { lv_label_set_text(label, text); } +static void output_status_update_cb(struct output_status_state state) { + struct zmk_widget_output_status *widget; + SYS_SLIST_FOR_EACH_CONTAINER(&widgets, widget, node) { set_status_symbol(widget->obj, state); } +} + +ZMK_DISPLAY_WIDGET_LISTENER(widget_output_status, struct output_status_state, + output_status_update_cb, get_state) +ZMK_SUBSCRIPTION(widget_output_status, zmk_endpoint_selection_changed); + +#if defined(CONFIG_USB) +ZMK_SUBSCRIPTION(widget_output_status, zmk_usb_conn_state_changed); +#endif +#if defined(CONFIG_ZMK_BLE) +ZMK_SUBSCRIPTION(widget_output_status, zmk_ble_active_profile_changed); +#endif + int zmk_widget_output_status_init(struct zmk_widget_output_status *widget, lv_obj_t *parent) { output_status_init(); + widget->obj = lv_label_create(parent, NULL); lv_obj_add_style(widget->obj, LV_LABEL_PART_MAIN, &label_style); lv_obj_set_size(widget->obj, 40, 15); - set_status_symbol(widget->obj); sys_slist_append(&widgets, &widget->node); + widget_output_status_init(); return 0; } lv_obj_t *zmk_widget_output_status_obj(struct zmk_widget_output_status *widget) { return widget->obj; } - -int output_status_listener(const zmk_event_t *eh) { - struct zmk_widget_output_status *widget; - SYS_SLIST_FOR_EACH_CONTAINER(&widgets, widget, node) { set_status_symbol(widget->obj); } - return ZMK_EV_EVENT_BUBBLE; -} - -ZMK_LISTENER(widget_output_status, output_status_listener) -ZMK_SUBSCRIPTION(widget_output_status, zmk_endpoint_selection_changed); - -#if defined(CONFIG_USB) -ZMK_SUBSCRIPTION(widget_output_status, zmk_usb_conn_state_changed); -#endif -#if defined(CONFIG_ZMK_BLE) -ZMK_SUBSCRIPTION(widget_output_status, zmk_ble_active_profile_changed); -#endif diff --git a/app/src/display/widgets/wpm_status.c b/app/src/display/widgets/wpm_status.c index 41ee3685..bee34135 100644 --- a/app/src/display/widgets/wpm_status.c +++ b/app/src/display/widgets/wpm_status.c @@ -7,6 +7,7 @@ #include LOG_MODULE_DECLARE(zmk, CONFIG_ZMK_LOG_LEVEL); +#include #include #include #include @@ -18,6 +19,10 @@ static lv_style_t label_style; static bool style_initialized = false; +struct wpm_status_state { + uint8_t wpm; +}; + void wpm_status_init() { if (style_initialized) { return; @@ -31,37 +36,41 @@ void wpm_status_init() { lv_style_set_text_line_space(&label_style, LV_STATE_DEFAULT, 1); } -void set_wpm_symbol(lv_obj_t *label, int wpm) { +struct wpm_status_state wpm_status_get_state(const zmk_event_t *eh) { + return (struct wpm_status_state){.wpm = zmk_wpm_get_state()}; +}; + +void set_wpm_symbol(lv_obj_t *label, struct wpm_status_state state) { char text[4] = {}; - LOG_DBG("WPM changed to %i", wpm); - sprintf(text, "%i ", wpm); + LOG_DBG("WPM changed to %i", state.wpm); + snprintf(text, sizeof(text), "%i ", state.wpm); lv_label_set_text(label, text); } +void wpm_status_update_cb(struct wpm_status_state state) { + struct zmk_widget_wpm_status *widget; + SYS_SLIST_FOR_EACH_CONTAINER(&widgets, widget, node) { set_wpm_symbol(widget->obj, state); } +} + +ZMK_DISPLAY_WIDGET_LISTENER(widget_wpm_status, struct wpm_status_state, wpm_status_update_cb, + wpm_status_get_state) +ZMK_SUBSCRIPTION(widget_wpm_status, zmk_wpm_state_changed); + int zmk_widget_wpm_status_init(struct zmk_widget_wpm_status *widget, lv_obj_t *parent) { wpm_status_init(); + widget->obj = lv_label_create(parent, NULL); lv_obj_add_style(widget->obj, LV_LABEL_PART_MAIN, &label_style); lv_label_set_align(widget->obj, LV_LABEL_ALIGN_RIGHT); lv_obj_set_size(widget->obj, 40, 15); - set_wpm_symbol(widget->obj, 0); sys_slist_append(&widgets, &widget->node); + widget_wpm_status_init(); return 0; } lv_obj_t *zmk_widget_wpm_status_obj(struct zmk_widget_wpm_status *widget) { return widget->obj; } - -int wpm_status_listener(const zmk_event_t *eh) { - struct zmk_wpm_state_changed *ev = as_zmk_wpm_state_changed(eh); - struct zmk_widget_wpm_status *widget; - SYS_SLIST_FOR_EACH_CONTAINER(&widgets, widget, node) { set_wpm_symbol(widget->obj, ev->state); } - return 0; -} - -ZMK_LISTENER(widget_wpm_status, wpm_status_listener) -ZMK_SUBSCRIPTION(widget_wpm_status, zmk_wpm_state_changed); \ No newline at end of file