refactor: Fixes for soft-off based on review.

* Better naming for gpio-key behavior triggers. * Tweaks to scanned behavior trigger to avoid bad semaphore use, and reduce chance of issues with slowly scanned matrixes. * Various code cleanups of style issues.
zmkfirmware · Dec 21, 2023 · 141dbd3 · 141dbd3
1 parent a7851d0
commit 141dbd3
Show file tree

Hide file tree

Showing 19 changed files with 227 additions and 231 deletions.
diff --git a/app/CMakeLists.txt b/app/CMakeLists.txt
@@ -22,11 +22,11 @@ target_sources(app PRIVATE src/matrix_transform.c)
 target_sources(app PRIVATE src/sensors.c)
 target_sources_ifdef(CONFIG_ZMK_WPM app PRIVATE src/wpm.c)
 target_sources(app PRIVATE src/event_manager.c)
-target_sources_ifdef(CONFIG_ZMK_BEHAVIOR_KEY app PRIVATE src/behavior_key.c)
-target_sources_ifdef(CONFIG_ZMK_BEHAVIOR_KEY_SCANNED app PRIVATE src/behavior_key_scanned.c)
+target_sources_ifdef(CONFIG_ZMK_GPIO_KEY_BEHAVIOR_TRIGGER app PRIVATE src/gpio_key_behavior_trigger.c)
+target_sources_ifdef(CONFIG_ZMK_GPIO_SCANNED_KEY_BEHAVIOR_TRIGGER app PRIVATE src/gpio_scanned_key_behavior_trigger.c)
 target_sources_ifdef(CONFIG_ZMK_PM_SOFT_OFF app PRIVATE src/pm.c)
 target_sources_ifdef(CONFIG_ZMK_EXT_POWER app PRIVATE src/ext_power_generic.c)
-target_sources_ifdef(CONFIG_ZMK_WAKEUP_TRIGGER_KEY app PRIVATE src/wakeup_trigger_key.c)
+target_sources_ifdef(CONFIG_ZMK_GPIO_KEY_WAKEUP_TRIGGER app PRIVATE src/gpio_key_wakeup_trigger.c)
 target_sources(app PRIVATE src/events/activity_state_changed.c)
 target_sources(app PRIVATE src/events/position_state_changed.c)
 target_sources(app PRIVATE src/events/sensor_event.c)

diff --git a/app/Kconfig b/app/Kconfig
@@ -410,10 +410,10 @@ config ZMK_PM_SOFT_OFF
     bool "Soft-off support"
     select PM_DEVICE
 
-config ZMK_WAKEUP_TRIGGER_KEY
+config ZMK_GPIO_KEY_WAKEUP_TRIGGER
     bool "Hardware supported wakeup (GPIO)"
     default y
-    depends on DT_HAS_ZMK_WAKEUP_TRIGGER_KEY_ENABLED && ZMK_PM_SOFT_OFF
+    depends on DT_HAS_ZMK_GPIO_KEY_WAKEUP_TRIGGER_ENABLED && ZMK_PM_SOFT_OFF
 
 #Power Management
 endmenu

diff --git a/app/Kconfig.behaviors b/app/Kconfig.behaviors
@@ -1,15 +1,15 @@
 # Copyright (c) 2023 The ZMK Contributors
 # SPDX-License-Identifier: MIT
 
-config ZMK_BEHAVIOR_KEY
+config ZMK_GPIO_KEY_BEHAVIOR_TRIGGER
     bool
     default y
-    depends on DT_HAS_ZMK_BEHAVIOR_KEY_ENABLED
+    depends on DT_HAS_ZMK_GPIO_KEY_BEHAVIOR_TRIGGER_ENABLED
 
-config ZMK_BEHAVIOR_KEY_SCANNED
+config ZMK_GPIO_SCANNED_KEY_BEHAVIOR_TRIGGER
     bool
     default y
-    depends on DT_HAS_ZMK_BEHAVIOR_KEY_SCANNED_ENABLED
+    depends on DT_HAS_ZMK_GPIO_SCANNED_KEY_BEHAVIOR_TRIGGER_ENABLED
 
 config ZMK_BEHAVIOR_KEY_TOGGLE
     bool

diff --git a/app/boards/shields/zmk_uno/boards/nrf52840dk_nrf52840.overlay b/app/boards/shields/zmk_uno/boards/nrf52840dk_nrf52840.overlay
@@ -43,7 +43,7 @@ encoder: &qdec0 {
     };
 
     wakeup_source: wakeup_source {
-        compatible = "zmk,wakeup-trigger-key";
+        compatible = "zmk,gpio-key-wakeup-trigger";
         status = "okay";
 
         trigger = <&button0>;
@@ -58,7 +58,7 @@ encoder: &qdec0 {
     };
 
     soft_off_behavior_key {
-        compatible = "zmk,behavior-key";
+        compatible = "zmk,gpio-key-behavior-trigger";
         status = "okay";
         bindings = <&soft_off>;
         key = <&button0>;

diff --git a/app/dts/behaviors/soft_off.dtsi b/app/dts/behaviors/soft_off.dtsi
@@ -8,7 +8,6 @@
     behaviors {
         /omit-if-no-ref/ soft_off: soft_off {
             compatible = "zmk,behavior-soft-off";
-            label = "SOFTOFF";
             #binding-cells = <0>;
         };
     };

diff --git a/app/dts/bindings/zmk,behavior-key.yaml → ...ndings/zmk,gpio-key-behavior-trigger.yaml b/app/dts/bindings/zmk,behavior-key.yaml → ...ndings/zmk,gpio-key-behavior-trigger.yaml
@@ -4,7 +4,7 @@
 description: |
   Driver for a dedicated key for invoking a connected behavior.
 
-compatible: "zmk,behavior-key"
+compatible: "zmk,gpio-key-behavior-trigger"
 
 include: base.yaml
 
@@ -16,7 +16,7 @@ properties:
   bindings:
     type: phandle
     required: true
-    description: The GPIO key that triggers wake via interrupt
+    description: The behavior to invoke when the GPIO key is pressed
   debounce-press-ms:
     type: int
     default: 5

diff --git a/app/dts/bindings/zmk,wakeup-trigger-key.yaml → ...bindings/zmk,gpio-key-wakeup-trigger.yaml b/app/dts/bindings/zmk,wakeup-trigger-key.yaml → ...bindings/zmk,gpio-key-wakeup-trigger.yaml
@@ -4,7 +4,7 @@
 description: |
   Driver for a dedicated key for waking the device from sleep
 
-compatible: "zmk,wakeup-trigger-key"
+compatible: "zmk,gpio-key-wakeup-trigger"
 
 include: base.yaml
 

diff --git a/...ts/bindings/zmk,behavior-key-scanned.yaml → ...mk,gpio-scanned-key-behavior-trigger.yaml b/...ts/bindings/zmk,behavior-key-scanned.yaml → ...mk,gpio-scanned-key-behavior-trigger.yaml
@@ -4,7 +4,7 @@
 description: |
   Driver for a dedicated key triggered by matrix scanning for invoking a connected behavior.
 
-compatible: "zmk,behavior-key-scanned"
+compatible: "zmk,gpio-scanned-key-behavior-trigger"
 
 include: base.yaml
 
@@ -16,7 +16,7 @@ properties:
   bindings:
     type: phandle
     required: true
-    description: The GPIO key that triggers wake via interrupt
+    description: The behavior to invoke when the GPIO key is pressed
   debounce-press-ms:
     type: int
     default: 5

diff --git a/app/dts/bindings/zmk,soft-off-wakeup-sources.yaml b/app/dts/bindings/zmk,soft-off-wakeup-sources.yaml
@@ -2,7 +2,7 @@
 # SPDX-License-Identifier: MIT
 
 description: |
-  Description of all possible wakeup-sources from a forces
+  Description of all possible wakeup-sources from a forced
   soft-off state.
 
 compatible: "zmk,soft-off-wakeup-sources"
@@ -11,4 +11,4 @@ properties:
   wakeup-sources:
     type: phandles
     required: true
-    description: List of wakeup-sources that should be enabled to wake the system from forces soft-off state.
+    description: List of wakeup-sources that should be enabled to wake the system from forced soft-off state.
diff --git a/app/module/drivers/kscan/kscan_gpio_direct.c b/app/module/drivers/kscan/kscan_gpio_direct.c
@@ -322,8 +322,6 @@ static int kscan_direct_init(const struct device *dev) {
 #if IS_ENABLED(CONFIG_PM_DEVICE)
 
 static int kscan_direct_pm_action(const struct device *dev, enum pm_device_action action) {
-    int ret = 0;
-
     switch (action) {
     case PM_DEVICE_ACTION_SUSPEND:
         kscan_direct_disable(dev);
@@ -332,11 +330,8 @@ static int kscan_direct_pm_action(const struct device *dev, enum pm_device_actio
         kscan_direct_enable(dev);
         break;
     default:
-        ret = -ENOTSUP;
-        break;
+        return -ENOTSUP;
     }
-
-    return ret;
 }
 
 #endif // IS_ENABLED(CONFIG_PM_DEVICE)

diff --git a/app/module/drivers/kscan/kscan_gpio_matrix.c b/app/module/drivers/kscan/kscan_gpio_matrix.c
@@ -425,8 +425,6 @@ static int kscan_matrix_init(const struct device *dev) {
 #if IS_ENABLED(CONFIG_PM_DEVICE)
 
 static int kscan_matrix_pm_action(const struct device *dev, enum pm_device_action action) {
-    int ret = 0;
-
     switch (action) {
     case PM_DEVICE_ACTION_SUSPEND:
         kscan_matrix_disable(dev);
@@ -435,11 +433,8 @@ static int kscan_matrix_pm_action(const struct device *dev, enum pm_device_actio
         kscan_matrix_enable(dev);
         break;
     default:
-        ret = -ENOTSUP;
-        break;
+        return -ENOTSUP;
     }
-
-    return ret;
 }
 
 #endif // IS_ENABLED(CONFIG_PM_DEVICE)

diff --git a/app/src/behaviors/behavior_soft_off.c b/app/src/behaviors/behavior_soft_off.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020 The ZMK Contributors
+ * Copyright (c) 2023 The ZMK Contributors
  *
  * SPDX-License-Identifier: MIT
  */

diff --git a/app/src/behavior_key.c → app/src/gpio_key_behavior_trigger.c b/app/src/behavior_key.c → app/src/gpio_key_behavior_trigger.c
@@ -4,7 +4,7 @@
  * SPDX-License-Identifier: MIT
  */
 
-#define DT_DRV_COMPAT zmk_behavior_key
+#define DT_DRV_COMPAT zmk_gpio_key_behavior_trigger
 
 #include <zephyr/device.h>
 #include <drivers/behavior.h>
@@ -19,13 +19,13 @@
 
 LOG_MODULE_DECLARE(zmk, CONFIG_ZMK_LOG_LEVEL);
 
-struct behavior_key_config {
+struct gkbt_config {
     struct zmk_debounce_config debounce_config;
     int32_t debounce_scan_period_ms;
     struct gpio_dt_spec key;
 };
 
-struct behavior_key_data {
+struct gkbt_data {
     struct zmk_behavior_binding binding;
     struct zmk_debounce_state debounce_state;
     struct gpio_callback key_callback;
@@ -34,21 +34,21 @@ struct behavior_key_data {
     uint32_t read_time;
 };
 
-static void bk_enable_interrupt(const struct device *dev) {
-    const struct behavior_key_config *config = dev->config;
+static void gkbt_enable_interrupt(const struct device *dev) {
+    const struct gkbt_config *config = dev->config;
 
     gpio_pin_interrupt_configure_dt(&config->key, GPIO_INT_LEVEL_ACTIVE);
 }
 
-static void bk_disable_interrupt(const struct device *dev) {
-    const struct behavior_key_config *config = dev->config;
+static void gkbt_disable_interrupt(const struct device *dev) {
+    const struct gkbt_config *config = dev->config;
 
     gpio_pin_interrupt_configure_dt(&config->key, GPIO_INT_DISABLE);
 }
 
-static void bk_read(const struct device *dev) {
-    const struct behavior_key_config *config = dev->config;
-    struct behavior_key_data *data = dev->data;
+static void gkbt_read(const struct device *dev) {
+    const struct gkbt_config *config = dev->config;
+    struct gkbt_data *data = dev->data;
 
     zmk_debounce_update(&data->debounce_state, gpio_pin_get_dt(&config->key),
                         config->debounce_scan_period_ms, &config->debounce_config);
@@ -71,65 +71,72 @@ static void bk_read(const struct device *dev) {
 
         k_work_reschedule(&data->update_work, K_TIMEOUT_ABS_MS(data->read_time));
     } else {
-        bk_enable_interrupt(dev);
+        gkbt_enable_interrupt(dev);
     }
 }
 
-static void bk_update_work(struct k_work *work) {
+static void gkbt_update_work(struct k_work *work) {
     struct k_work_delayable *dwork = CONTAINER_OF(work, struct k_work_delayable, work);
-    struct behavior_key_data *data = CONTAINER_OF(dwork, struct behavior_key_data, update_work);
-    bk_read(data->dev);
+    struct gkbt_data *data = CONTAINER_OF(dwork, struct gkbt_data, update_work);
+    gkbt_read(data->dev);
 }
 
-static void bk_gpio_irq_callback(const struct device *port, struct gpio_callback *cb,
-                                 const gpio_port_pins_t pin) {
-    struct behavior_key_data *data = CONTAINER_OF(cb, struct behavior_key_data, key_callback);
+static void gkbt_gpio_irq_callback(const struct device *port, struct gpio_callback *cb,
+                                   const gpio_port_pins_t pin) {
+    struct gkbt_data *data = CONTAINER_OF(cb, struct gkbt_data, key_callback);
 
-    bk_disable_interrupt(data->dev);
+    gkbt_disable_interrupt(data->dev);
 
     data->read_time = k_uptime_get();
     k_work_reschedule(&data->update_work, K_NO_WAIT);
 }
 
-static int behavior_key_init(const struct device *dev) {
-    const struct behavior_key_config *config = dev->config;
-    struct behavior_key_data *data = dev->data;
+static void gkbt_wait_for_key_release(const struct device *dev) {
+    const struct gkbt_config *config = dev->config;
+
+    while (gpio_pin_get_dt(&config->key)) {
+        k_sleep(K_MSEC(100));
+    }
+}
+
+static int gkbt_init(const struct device *dev) {
+    const struct gkbt_config *config = dev->config;
+    struct gkbt_data *data = dev->data;
 
     if (!device_is_ready(config->key.port)) {
-        LOG_ERR("GPIO port is not ready");
+        LOG_ERR("GPIO port %s is not ready", config->key.port->name);
         return -ENODEV;
     }
 
-    k_work_init_delayable(&data->update_work, bk_update_work);
+    k_work_init_delayable(&data->update_work, gkbt_update_work);
     data->dev = dev;
 
     gpio_pin_configure_dt(&config->key, GPIO_INPUT);
-    gpio_init_callback(&data->key_callback, bk_gpio_irq_callback, BIT(config->key.pin));
+    gpio_init_callback(&data->key_callback, gkbt_gpio_irq_callback, BIT(config->key.pin));
     gpio_add_callback(config->key.port, &data->key_callback);
 
-    while (gpio_pin_get_dt(&config->key)) {
-        k_sleep(K_MSEC(100));
-    }
+    // Be sure our wakeup key is released before startup continues to avoid wake/sleep loop.
+    gkbt_wait_for_key_release(dev);
 
-    bk_enable_interrupt(dev);
+    gkbt_enable_interrupt(dev);
 
     return 0;
 }
 
-static int behavior_key_pm_action(const struct device *dev, enum pm_device_action action) {
-    const struct behavior_key_config *config = dev->config;
-    struct behavior_key_data *data = dev->data;
+static int gkbt_pm_action(const struct device *dev, enum pm_device_action action) {
+    const struct gkbt_config *config = dev->config;
+    struct gkbt_data *data = dev->data;
 
     int ret;
 
     switch (action) {
     case PM_DEVICE_ACTION_SUSPEND:
-        bk_disable_interrupt(dev);
+        gkbt_disable_interrupt(dev);
         ret = gpio_remove_callback(config->key.port, &data->key_callback);
         break;
     case PM_DEVICE_ACTION_RESUME:
         ret = gpio_add_callback(config->key.port, &data->key_callback);
-        bk_enable_interrupt(dev);
+        gkbt_enable_interrupt(dev);
         break;
     default:
         ret = -ENOTSUP;
@@ -139,8 +146,8 @@ static int behavior_key_pm_action(const struct device *dev, enum pm_device_actio
     return ret;
 }
 
-#define BK_INST(n)                                                                                 \
-    const struct behavior_key_config bk_config_##n = {                                             \
+#define GKBT_INST(n)                                                                               \
+    const struct gkbt_config gkbt_config_##n = {                                                   \
         .key = GPIO_DT_SPEC_GET(DT_INST_PHANDLE(n, key), gpios),                                   \
         .debounce_config =                                                                         \
             {                                                                                      \
@@ -149,11 +156,12 @@ static int behavior_key_pm_action(const struct device *dev, enum pm_device_actio
             },                                                                                     \
         .debounce_scan_period_ms = DT_INST_PROP(n, debounce_scan_period_ms),                       \
     };                                                                                             \
-    struct behavior_key_data bk_data_##n = {                                                       \
+    struct gkbt_data gkbt_data_##n = {                                                             \
         .binding = ZMK_KEYMAP_EXTRACT_BINDING(0, DT_DRV_INST(n)),                                  \
     };                                                                                             \
-    PM_DEVICE_DT_INST_DEFINE(n, behavior_key_pm_action);                                           \
-    DEVICE_DT_INST_DEFINE(n, behavior_key_init, PM_DEVICE_DT_INST_GET(n), &bk_data_##n,            \
-                          &bk_config_##n, POST_KERNEL, CONFIG_KERNEL_INIT_PRIORITY_DEFAULT, NULL);
+    PM_DEVICE_DT_INST_DEFINE(n, gkbt_pm_action);                                                   \
+    DEVICE_DT_INST_DEFINE(n, gkbt_init, PM_DEVICE_DT_INST_GET(n), &gkbt_data_##n,                  \
+                          &gkbt_config_##n, POST_KERNEL, CONFIG_KERNEL_INIT_PRIORITY_DEFAULT,      \
+                          NULL);
 
-DT_INST_FOREACH_STATUS_OKAY(BK_INST)
+DT_INST_FOREACH_STATUS_OKAY(GKBT_INST)