From 2776b21a571340c60be92a87b3390860718d85f8 Mon Sep 17 00:00:00 2001 From: Peter Johanson Date: Wed, 17 Apr 2024 16:44:22 -0700 Subject: [PATCH] feat(behaviors): Add local ID system for behaviors * Add a new feature for tracking a given behavior by a new concept of a "behavior local ID" which is a stable 16-bit identifier for a given behavior, that is resilient to new behaviors being added and requires no additional work on the part of the behavior authors. * Add implementations for either settings lookup table, or CRC16 hashing of behavior device names for generating behavior local IDs. --- app/CMakeLists.txt | 4 + app/Kconfig.behaviors | 29 ++++ app/include/drivers/behavior.h | 19 ++- .../linker/zmk-behavior-local-id-map.ld | 11 ++ app/include/zmk/behavior.h | 22 +++ app/src/behavior.c | 137 ++++++++++++++++++ 6 files changed, 221 insertions(+), 1 deletion(-) create mode 100644 app/include/linker/zmk-behavior-local-id-map.ld diff --git a/app/CMakeLists.txt b/app/CMakeLists.txt index 0b681ea95312..2818e932256b 100644 --- a/app/CMakeLists.txt +++ b/app/CMakeLists.txt @@ -11,6 +11,10 @@ project(zmk) zephyr_linker_sources(SECTIONS include/linker/zmk-behaviors.ld) zephyr_linker_sources(RODATA include/linker/zmk-events.ld) +if(CONFIG_ZMK_BEHAVIOR_LOCAL_IDS) + zephyr_linker_sources(DATA_SECTIONS include/linker/zmk-behavior-local-id-map.ld) +endif() + zephyr_syscall_header(${APPLICATION_SOURCE_DIR}/include/drivers/behavior.h) zephyr_syscall_header(${APPLICATION_SOURCE_DIR}/include/drivers/ext_power.h) diff --git a/app/Kconfig.behaviors b/app/Kconfig.behaviors index c6cc45f3b87d..0fa345462dc8 100644 --- a/app/Kconfig.behaviors +++ b/app/Kconfig.behaviors @@ -7,6 +7,35 @@ config ZMK_BEHAVIOR_METADATA Enabling this option adds APIs for documenting and fetching metadata describing a behaviors name, and supported parameters. +config ZMK_BEHAVIOR_LOCAL_IDS + bool "Local IDs" + +if ZMK_BEHAVIOR_LOCAL_IDS + +choice ZMK_BEHAVIOR_LOCAL_ID_TYPE + prompt "Local ID Type" + +config ZMK_BEHAVIOR_LOCAL_ID_TYPE_SETTINGS_TABLE + bool "Settings Table" + depends on SETTINGS + help + Use persistent entries in the settings subsystem to identify + behaviors by local ID, which uses the device name to generate + a new settings entry tying a presistant local ID to that name. + This guarantees stable, colllision-free local IDs at the expense + of settings storage used. + +config ZMK_BEHAVIOR_LOCAL_ID_TYPE_CRC16 + bool "CRC16 Hash" + help + Use the CRC16-ANSI hash of behavior device names to generate + stable behavior local IDs. This saves on settings storage at + the expense of (highly unlikely) risk of collisions. + +endchoice + +endif + config ZMK_BEHAVIOR_KEY_TOGGLE bool default y diff --git a/app/include/drivers/behavior.h b/app/include/drivers/behavior.h index 3dd6e0623f74..0b814ff28288 100644 --- a/app/include/drivers/behavior.h +++ b/app/include/drivers/behavior.h @@ -108,6 +108,15 @@ struct zmk_behavior_ref { const struct zmk_behavior_metadata metadata; }; +#if IS_ENABLED(CONFIG_ZMK_BEHAVIOR_LOCAL_IDS) + +struct zmk_behavior_local_id_map { + const struct device *device; + zmk_behavior_local_id_t local_id; +}; + +#endif // IS_ENABLED(CONFIG_ZMK_BEHAVIOR_LOCAL_IDS) + #define ZMK_BEHAVIOR_REF_DT_NAME(node_id) _CONCAT(zmk_behavior_, DEVICE_DT_NAME_GET(node_id)) #if IS_ENABLED(CONFIG_ZMK_BEHAVIOR_METADATA) @@ -125,9 +134,17 @@ struct zmk_behavior_ref { #define ZMK_BEHAVIOR_REF_INITIALIZER(node_id, _dev) \ { .device = _dev, .metadata = ZMK_BEHAVIOR_METADATA_INITIALIZER(node_id), } +#define ZMK_BEHAVIOR_LOCAL_ID_MAP_INITIALIZER(node_id, _dev) \ + { .device = _dev, } + #define ZMK_BEHAVIOR_REF_DEFINE(name, node_id, _dev) \ static const STRUCT_SECTION_ITERABLE(zmk_behavior_ref, name) = \ - ZMK_BEHAVIOR_REF_INITIALIZER(node_id, _dev) + ZMK_BEHAVIOR_REF_INITIALIZER(node_id, _dev); \ + COND_CODE_1(IS_ENABLED(CONFIG_ZMK_BEHAVIOR_LOCAL_IDS), \ + (static const STRUCT_SECTION_ITERABLE(zmk_behavior_local_id_map, \ + _CONCAT(_zmk_behavior_local_id_map, name)) = \ + ZMK_BEHAVIOR_LOCAL_ID_MAP_INITIALIZER(node_id, _dev)), \ + ()); #define ZMK_BEHAVIOR_REF_DT_DEFINE(node_id) \ ZMK_BEHAVIOR_REF_DEFINE(ZMK_BEHAVIOR_REF_DT_NAME(node_id), node_id, DEVICE_DT_GET(node_id)) diff --git a/app/include/linker/zmk-behavior-local-id-map.ld b/app/include/linker/zmk-behavior-local-id-map.ld new file mode 100644 index 000000000000..0a314784d9e1 --- /dev/null +++ b/app/include/linker/zmk-behavior-local-id-map.ld @@ -0,0 +1,11 @@ +/* + * Copyright (c) 2023 The ZMK Contributors + * + * SPDX-License-Identifier: MIT + */ + +#include + +ITERABLE_SECTION_RAM(zmk_behavior_local_id_map, 4) + + diff --git a/app/include/zmk/behavior.h b/app/include/zmk/behavior.h index 016fa3bc063a..34a415ca06e1 100644 --- a/app/include/zmk/behavior.h +++ b/app/include/zmk/behavior.h @@ -23,6 +23,8 @@ struct zmk_behavior_binding_event { int64_t timestamp; }; +typedef uint16_t zmk_behavior_local_id_t; + /** * @brief Get a const struct device* for a behavior from its @p name field. * @@ -36,3 +38,23 @@ struct zmk_behavior_binding_event { * unrelated node which shares the same name as a behavior. */ const struct device *zmk_behavior_get_binding(const char *name); + +/** + * @brief Get a local ID for a behavior from its @p name field. + * + * @param name Behavior name to search for. + * + * @retval The local ID value that can be used to reference the behavior later, across reboots. + * @retval UINT16_MAX if the behavior is not found or its initialization function failed. + */ +zmk_behavior_local_id_t zmk_behavior_get_local_id(const char *name); + +/** + * @brief Get a behavior name for a behavior from its @p local_id . + * + * @param local_id Behavior local ID used to search for the behavior + * + * @retval The name of the behavior that is associated with that local ID. + * @retval NULL if the behavior is not found or its initialization function failed. + */ +const char *zmk_behavior_find_behavior_name_from_local_id(zmk_behavior_local_id_t local_id); diff --git a/app/src/behavior.c b/app/src/behavior.c index 7777155f404a..0ccdc04c32f1 100644 --- a/app/src/behavior.c +++ b/app/src/behavior.c @@ -6,9 +6,17 @@ #include #include +#include #include #include +#if IS_ENABLED(CONFIG_ZMK_BEHAVIOR_LOCAL_IDS) && \ + IS_ENABLED(CONFIG_ZMK_BEHAVIOR_LOCAL_ID_TYPE_SETTINGS_TABLE) + +#include + +#endif + #include #include #include @@ -185,6 +193,135 @@ int zmk_behavior_validate_binding(const struct zmk_behavior_binding *binding) { #endif // IS_ENABLED(CONFIG_ZMK_BEHAVIOR_METADATA) } +#if IS_ENABLED(CONFIG_ZMK_BEHAVIOR_LOCAL_IDS) + +zmk_behavior_local_id_t zmk_behavior_get_local_id(const char *name) { + if (!name) { + return UINT16_MAX; + } + + STRUCT_SECTION_FOREACH(zmk_behavior_local_id_map, item) { + if (z_device_is_ready(item->device) && strcmp(item->device->name, name) == 0) { + return item->local_id; + } + } + + return UINT16_MAX; +} + +const char *zmk_behavior_find_behavior_name_from_local_id(zmk_behavior_local_id_t local_id) { + STRUCT_SECTION_FOREACH(zmk_behavior_local_id_map, item) { + if (z_device_is_ready(item->device) && item->local_id == local_id) { + return item->device->name; + } + } + + return NULL; +} + +#if IS_ENABLED(CONFIG_ZMK_BEHAVIOR_LOCAL_ID_TYPE_CRC16) + +static int behavior_local_id_init(void) { + STRUCT_SECTION_FOREACH(zmk_behavior_local_id_map, item) { + item->local_id = crc16_ansi(item->device->name, strlen(item->device->name)); + } + + return 0; +} + +#elif IS_ENABLED(CONFIG_ZMK_BEHAVIOR_LOCAL_ID_TYPE_SETTINGS_TABLE) + +static zmk_behavior_local_id_t largest_local_id = 0; + +static int behavior_handle_set(const char *name, size_t len, settings_read_cb read_cb, + void *cb_arg) { + const char *next; + + LOG_DBG("Setting Behavior setting %s", name); + + if (settings_name_steq(name, "local_id", &next) && next) { + char *endptr; + uint8_t local_id = strtoul(next, &endptr, 10); + if (*endptr != '\0') { + LOG_WRN("Invalid behavior local ID: %s with endptr %s", next, endptr); + return -EINVAL; + } + + if (len >= 64) { + LOG_ERR("Too large binding setting size (got %d expected less than %d)", len, 64); + return -EINVAL; + } + + char name[len + 1]; + + int err = read_cb(cb_arg, name, len); + if (err <= 0) { + LOG_ERR("Failed to handle keymap binding from settings (err %d)", err); + return err; + } + + name[len] = '\0'; + STRUCT_SECTION_FOREACH(zmk_behavior_local_id_map, item) { + if (strcmp(name, item->device->name) == 0) { + item->local_id = local_id; + largest_local_id = MAX(largest_local_id, local_id); + return 0; + } + } + + return -EINVAL; + } + + return 0; +} + +static int behavior_handle_commit(void) { + STRUCT_SECTION_FOREACH(zmk_behavior_local_id_map, item) { + if (item->local_id != 0) { + continue; + } + + if (!item->device || !item->device->name || !device_is_ready(item->device)) { + LOG_WRN("Skipping ID for device that doesn't exist or isn't ready"); + continue; + } + + item->local_id = ++largest_local_id; + char setting_name[32]; + sprintf(setting_name, "behavior/local_id/%d", item->local_id); + + // If the `device->name` is readonly in flash, settings save can fail to copy/read it while + // persisting to flash, so copy the device name into memory first before saving. + char device_name[32]; + snprintf(device_name, ARRAY_SIZE(device_name), "%s", item->device->name); + + settings_save_one(setting_name, device_name, strlen(device_name)); + } + + return 0; +} + +SETTINGS_STATIC_HANDLER_DEFINE(behavior, "behavior", NULL, behavior_handle_set, + behavior_handle_commit, NULL); + +static int behavior_local_id_init(void) { + settings_subsys_init(); + + settings_load_subtree("behavior"); + + return 0; +} + +#else + +#error "A behavior local ID mechanism must be selected" + +#endif + +SYS_INIT(behavior_local_id_init, APPLICATION, CONFIG_APPLICATION_INIT_PRIORITY); + +#endif + #if IS_ENABLED(CONFIG_LOG) static int check_behavior_names(void) { // Behavior names must be unique, but we don't have a good way to enforce this