From ecac94d384a5b2a26e27267de00856f824149005 Mon Sep 17 00:00:00 2001 From: Peter Johanson Date: Mon, 22 Jan 2024 13:39:08 -0800 Subject: [PATCH] fix: Cleanups of sideband and direct kscan from review. * Add dedicated init priority for the sideband kscan. * Refactor sideband code for clarity. * Tweaks to direct kscan for clarity. * Make sideband behavior row optional for brevity. * Allow overriding ZMK Uno sideband behaviors. --- app/Kconfig | 9 +++ app/boards/shields/zmk_uno/zmk_uno.keymap | 22 +++--- app/boards/shields/zmk_uno/zmk_uno.overlay | 9 +-- .../shields/zmk_uno/zmk_uno_split.keymap | 21 +++--- .../kscan/zmk,kscan-sideband-behaviors.yaml | 7 +- app/module/drivers/kscan/kscan_gpio_direct.c | 4 +- app/src/kscan_sideband_behaviors.c | 69 ++++++++++++------- 7 files changed, 79 insertions(+), 62 deletions(-) diff --git a/app/Kconfig b/app/Kconfig index bba381dc15a..5f634a0a219 100644 --- a/app/Kconfig +++ b/app/Kconfig @@ -500,6 +500,15 @@ config ZMK_KSCAN_SIDEBAND_BEHAVIORS depends on DT_HAS_ZMK_KSCAN_SIDEBAND_BEHAVIORS_ENABLED select KSCAN +if ZMK_KSCAN_SIDEBAND_BEHAVIORS + +config ZMK_KSCAN_SIDEBAND_BEHAVIORS_INIT_PRIORITY + int "Keyboard scan sideband behaviors driver init priority" + # The default kscan init priority is 90, so be sure we are lower. + default 95 + +endif # ZMK_KSCAN_SIDEBAND_BEHAVIORS + menu "Logging" config ZMK_LOGGING_MINIMAL diff --git a/app/boards/shields/zmk_uno/zmk_uno.keymap b/app/boards/shields/zmk_uno/zmk_uno.keymap index e416ff5e6c8..a7f6a2675d5 100644 --- a/app/boards/shields/zmk_uno/zmk_uno.keymap +++ b/app/boards/shields/zmk_uno/zmk_uno.keymap @@ -11,21 +11,17 @@ #include #include -// Uncomment the following block if using the "Direct Wire" jumper to switch the matrix to a direct wire. +// Uncomment the following lines if using the "Direct Wire" jumper to switch the matrix to a direct wire. -/* :REMOVE ME +// &kscan_direct { status = "okay"; }; +// &kscan_matrix { status = "disabled"; }; -&kscan_direct { status = "okay"; }; -&kscan_matrix { status = "disabled"; }; - -/ { - chosen { - zmk,matrix-transform = &direct_matrix_transform; - zmk,kscan = &kscan_direct; - }; -}; - -REMOVE ME: */ +// / { +// chosen { +// zmk,matrix-transform = &direct_matrix_transform; +// zmk,kscan = &kscan_direct; +// }; +// }; / { keymap { diff --git a/app/boards/shields/zmk_uno/zmk_uno.overlay b/app/boards/shields/zmk_uno/zmk_uno.overlay index 7b4ae903e44..47bb8ddfb4b 100644 --- a/app/boards/shields/zmk_uno/zmk_uno.overlay +++ b/app/boards/shields/zmk_uno/zmk_uno.overlay @@ -40,20 +40,17 @@ compatible = "zmk,kscan-sideband-behaviors"; kscan = <&kscan_sp3t_toggle>; - usb { - row = <0>; + first_toggle_sideband: first_toggle_sideband { column = <0>; bindings = <&out OUT_USB>; }; - ble_zero { - row = <0>; + second_toggle_sideband: second_toggle_sideband { column = <1>; bindings = <&ble_zero>; }; - ble_one { - row = <0>; + third_toggle_sideband: third_toggle_sideband { column = <2>; bindings = <&ble_one>; }; diff --git a/app/boards/shields/zmk_uno/zmk_uno_split.keymap b/app/boards/shields/zmk_uno/zmk_uno_split.keymap index 1e46042a7e1..37672a7dbdb 100644 --- a/app/boards/shields/zmk_uno/zmk_uno_split.keymap +++ b/app/boards/shields/zmk_uno/zmk_uno_split.keymap @@ -12,21 +12,18 @@ #include #include -// Uncomment the following block if using the "Direct Wire" jumper to switch the matrix to a direct wire. +// Uncomment the following lines if using the "Direct Wire" jumper to switch the matrix to a direct wire. -/* :REMOVE ME -&kscan_direct { status = "okay"; }; -&kscan_matrix { status = "disabled"; }; +// &kscan_direct { status = "okay"; }; +// &kscan_matrix { status = "disabled"; }; -/ { - chosen { - zmk,matrix-transform = &split_direct_matrix_transform; - zmk,kscan = &kscan_direct_comp; - }; -}; - -REMOVE ME: */ +// / { +// chosen { +// zmk,matrix-transform = &split_direct_matrix_transform; +// zmk,kscan = &kscan_direct_comp; +// }; +// }; / { diff --git a/app/dts/bindings/kscan/zmk,kscan-sideband-behaviors.yaml b/app/dts/bindings/kscan/zmk,kscan-sideband-behaviors.yaml index 57b54a6041c..f3ed180d14b 100644 --- a/app/dts/bindings/kscan/zmk,kscan-sideband-behaviors.yaml +++ b/app/dts/bindings/kscan/zmk,kscan-sideband-behaviors.yaml @@ -2,8 +2,9 @@ # SPDX-License-Identifier: MIT description: | - kscan sideband behavior runner. Only basic system behavior should be used, - since no keymap processing occurs when using them. + kscan sideband behavior runner. Only basic system behaviors should be used, + since no keymap processing occurs when using them. Primarily, that means avoiding + using tap-holds, sticky keys, etc. as sideband behaviors. compatible: "zmk,kscan-sideband-behaviors" @@ -20,7 +21,7 @@ child-binding: properties: row: type: int - required: true + default: 0 column: type: int required: true diff --git a/app/module/drivers/kscan/kscan_gpio_direct.c b/app/module/drivers/kscan/kscan_gpio_direct.c index 2c8551c90df..930db68304d 100644 --- a/app/module/drivers/kscan/kscan_gpio_direct.c +++ b/app/module/drivers/kscan/kscan_gpio_direct.c @@ -46,7 +46,7 @@ LOG_MODULE_DECLARE(zmk, CONFIG_ZMK_LOG_LEVEL); COND_CODE_1(DT_INST_NODE_HAS_PROP(n, input_gpios), (DT_INST_PROP_LEN(n, input_gpios)), \ (DT_INST_PROP_LEN(n, input_keys))) -#define KSCAN_DIRECT_INPUT_CFG_INIT(idx, inst_idx) \ +#define KSCAN_GPIO_DIRECT_INPUT_CFG_INIT(idx, inst_idx) \ KSCAN_GPIO_GET_BY_IDX(DT_DRV_INST(inst_idx), input_gpios, idx) #define KSCAN_KEY_DIRECT_INPUT_CFG_INIT(idx, inst_idx) \ KSCAN_GPIO_GET_BY_IDX(DT_INST_PROP_BY_IDX(inst_idx, input_keys, idx), gpios, 0) @@ -353,7 +353,7 @@ static const struct kscan_driver_api kscan_direct_api = { \ static struct kscan_gpio kscan_direct_inputs_##n[] = { \ COND_CODE_1(DT_INST_NODE_HAS_PROP(n, input_gpios), \ - (LISTIFY(INST_INPUTS_LEN(n), KSCAN_DIRECT_INPUT_CFG_INIT, (, ), n)), \ + (LISTIFY(INST_INPUTS_LEN(n), KSCAN_GPIO_DIRECT_INPUT_CFG_INIT, (, ), n)), \ (LISTIFY(INST_INPUTS_LEN(n), KSCAN_KEY_DIRECT_INPUT_CFG_INIT, (, ), n)))}; \ \ static struct zmk_debounce_state kscan_direct_state_##n[INST_INPUTS_LEN(n)]; \ diff --git a/app/src/kscan_sideband_behaviors.c b/app/src/kscan_sideband_behaviors.c index cff28e49d18..5b8a30a85e3 100644 --- a/app/src/kscan_sideband_behaviors.c +++ b/app/src/kscan_sideband_behaviors.c @@ -19,9 +19,9 @@ LOG_MODULE_DECLARE(zmk, CONFIG_ZMK_LOG_LEVEL); struct ksbb_entry { + struct zmk_behavior_binding binding; uint8_t row; uint8_t column; - struct zmk_behavior_binding binding; }; struct ksbb_config { @@ -39,32 +39,53 @@ struct ksbb_data { // The kscan callback has no context with it, so we keep a static array of all possible // KSBBs to check when a kscan callback from the "wrapped" inner kscan fires. -static const struct device *ksbbs[DT_NUM_INST_STATUS_OKAY(DT_DRV_COMPAT)] = { - DT_INST_FOREACH_STATUS_OKAY(GET_KSBB_DEV)}; +static const struct device *ksbbs[] = {DT_INST_FOREACH_STATUS_OKAY(GET_KSBB_DEV)}; -void ksbb_inner_kscan_callback(const struct device *dev, uint32_t row, uint32_t column, - bool pressed) { +int find_ksbb_for_inner(const struct device *inner_dev, const struct device **ksbb_dev) { for (int i = 0; i < ARRAY_SIZE(ksbbs); i++) { const struct device *ksbb = ksbbs[i]; const struct ksbb_config *cfg = ksbb->config; - struct ksbb_data *data = ksbb->data; - if (cfg->kscan != dev) { - continue; + if (cfg->kscan == inner_dev) { + *ksbb_dev = ksbb; + return 0; + } + } + + return -ENODEV; +} + +int find_sideband_behavior(const struct device *dev, uint32_t row, uint32_t column, + struct ksbb_entry **entry) { + const struct ksbb_config *cfg = dev->config; + + for (int e = 0; e < cfg->entries_len; e++) { + struct ksbb_entry *candidate = &cfg->entries[e]; + + if (candidate->row == row && candidate->column == column) { + *entry = candidate; + return 0; } + } - for (int e = 0; e < cfg->entries_len; e++) { - struct ksbb_entry *entry = &cfg->entries[e]; - if (entry->row == row && entry->column == column) { - struct zmk_behavior_binding_event event = {.position = INT32_MAX, - .timestamp = k_uptime_get()}; - - if (pressed) { - behavior_keymap_binding_pressed(&entry->binding, event); - } else { - behavior_keymap_binding_released(&entry->binding, event); - } - return; + return -ENODEV; +} + +void ksbb_inner_kscan_callback(const struct device *dev, uint32_t row, uint32_t column, + bool pressed) { + struct ksbb_entry *entry = NULL; + const struct device *ksbb = NULL; + + if (find_ksbb_for_inner(dev, &ksbb) >= 0) { + struct ksbb_data *data = ksbb->data; + if (find_sideband_behavior(ksbb, row, column, &entry) >= 0) { + struct zmk_behavior_binding_event event = {.position = INT32_MAX, + .timestamp = k_uptime_get()}; + + if (pressed) { + behavior_keymap_binding_pressed(&entry->binding, event); + } else { + behavior_keymap_binding_released(&entry->binding, event); } } @@ -78,10 +99,6 @@ static int ksbb_configure(const struct device *dev, kscan_callback_t callback) { const struct ksbb_config *cfg = dev->config; struct ksbb_data *data = dev->data; - if (!callback) { - return -EINVAL; - } - data->callback = callback; #if IS_ENABLED(CONFIG_PM_DEVICE) @@ -161,7 +178,7 @@ static int ksbb_pm_action(const struct device *dev, enum pm_device_action action struct ksbb_data ksbb_data_##n = {}; \ PM_DEVICE_DT_INST_DEFINE(n, ksbb_pm_action); \ DEVICE_DT_INST_DEFINE(n, ksbb_init, PM_DEVICE_DT_INST_GET(n), &ksbb_data_##n, \ - &ksbb_config_##n, APPLICATION, CONFIG_KERNEL_INIT_PRIORITY_DEFAULT, \ - &ksbb_api); + &ksbb_config_##n, POST_KERNEL, \ + CONFIG_ZMK_KSCAN_SIDEBAND_BEHAVIORS_INIT_PRIORITY, &ksbb_api); DT_INST_FOREACH_STATUS_OKAY(KSBB_INST)