refactor(backlight): code cleanup

7 files changed, 67 insertions, 133 deletions

diff --git a/app/Kconfig b/app/Kconfig
index 49eec83..02fef66 100644
--- a/app/Kconfig
+++ b/app/Kconfig
@@ -352,11 +352,9 @@ config ZMK_BACKLIGHT_ON_START
 
 config ZMK_BACKLIGHT_AUTO_OFF_IDLE
 	bool "Turn off backlight when keyboard goes into idle state"
-	default y
 
 config ZMK_BACKLIGHT_AUTO_OFF_USB
 	bool "Turn off backlight when USB is disconnected"
-	default n
 
 #ZMK_BACKLIGHT
 endif
diff --git a/app/dts/behaviors/backlight.dtsi b/app/dts/behaviors/backlight.dtsi
index 6127f60..f9bd02b 100644
--- a/app/dts/behaviors/backlight.dtsi
+++ b/app/dts/behaviors/backlight.dtsi
@@ -8,7 +8,7 @@
 	behaviors {
 		/omit-if-no-ref/ bl: behavior_backlight {
 			compatible = "zmk,behavior-backlight";
-			label = "BACKLIGHT";
+			label = "BCKLGHT";
 			#binding-cells = <2>;
 		};
 	};
diff --git a/app/include/dt-bindings/zmk/backlight.h b/app/include/dt-bindings/zmk/backlight.h
index 7057243..fa6dc9b 100644
--- a/app/include/dt-bindings/zmk/backlight.h
+++ b/app/include/dt-bindings/zmk/backlight.h
@@ -4,16 +4,16 @@
  * SPDX-License-Identifier: MIT
  */
 
-#define BL_TOG_CMD 0
-#define BL_ON_CMD 1
-#define BL_OFF_CMD 2
+#define BL_ON_CMD 0
+#define BL_OFF_CMD 1
+#define BL_TOG_CMD 2
 #define BL_INC_CMD 3
 #define BL_DEC_CMD 4
 #define BL_SET_CMD 5
 
-#define BL_TOG BL_TOG_CMD 0
 #define BL_ON BL_ON_CMD 0
 #define BL_OFF BL_OFF_CMD 0
+#define BL_TOG BL_TOG_CMD 0
 #define BL_INC BL_INC_CMD 0
 #define BL_DEC BL_DEC_CMD 0
 #define BL_SET BL_SET_CMD
diff --git a/app/include/zmk/backlight.h b/app/include/zmk/backlight.h
index 8711d88..dd7d966 100644
--- a/app/include/zmk/backlight.h
+++ b/app/include/zmk/backlight.h
@@ -6,11 +6,11 @@
 
 #pragma once
 
-int zmk_backlight_toggle();
-bool zmk_backlight_get_on();
 int zmk_backlight_on();
 int zmk_backlight_off();
-uint8_t zmk_backlight_calc_brt(int direction);
+int zmk_backlight_toggle();
+bool zmk_backlight_is_on();
+
 int zmk_backlight_set_brt(uint8_t brightness);
-int zmk_backlight_adjust_brt(int direction);
-int zmk_backlight_get_brt();
+uint8_t zmk_backlight_get_brt();
+uint8_t zmk_backlight_calc_brt(int direction);
diff --git a/app/src/backlight.c b/app/src/backlight.c
index fc8831c..6ef7f8f 100644
--- a/app/src/backlight.c
+++ b/app/src/backlight.c
@@ -43,10 +43,11 @@ static struct backlight_state state = {.brightness = CONFIG_ZMK_BACKLIGHT_BRT_ST
                                        .on = IS_ENABLED(CONFIG_ZMK_BACKLIGHT_ON_START)};
 
 static int zmk_backlight_update() {
-    uint8_t brt = state.on ? state.brightness : 0;
+    uint8_t brt = zmk_backlight_get_brt();
     for (int i = 0; i < BACKLIGHT_NUM_LEDS; i++) {
         int rc = led_set_brightness(backlight_dev, i, brt);
         if (rc != 0) {
+            LOG_ERR("Failed to update backlight LED %d: %d", i, rc);
             return rc;
         }
     }
@@ -54,34 +55,25 @@ static int zmk_backlight_update() {
 }
 
 #if IS_ENABLED(CONFIG_SETTINGS)
-static int backlight_settings_set(const char *name, size_t len, settings_read_cb read_cb,
-                                  void *cb_arg) {
+static int backlight_settings_load_cb(const char *name, size_t len, settings_read_cb read_cb,
+                                      void *cb_arg, void *param) {
     const char *next;
-
     if (settings_name_steq(name, "state", &next) && !next) {
         if (len != sizeof(state)) {
             return -EINVAL;
         }
 
         int rc = read_cb(cb_arg, &state, sizeof(state));
-        if (rc < 0) {
-            return rc;
-        }
-
-        return zmk_backlight_update();
+        return MIN(rc, 0);
     }
-
     return -ENOENT;
 }
 
-static struct settings_handler backlight_conf = {.name = "backlight",
-                                                 .h_set = backlight_settings_set};
-
-static void zmk_backlight_save_state_work() {
+static void backlight_save_work_handler(struct k_work *work) {
     settings_save_one("backlight/state", &state, sizeof(state));
 }
 
-static struct k_delayed_work backlight_save_work;
+static K_DELAYED_WORK_DEFINE(backlight_save_work, backlight_save_work_handler);
 #endif
 
 static int zmk_backlight_init(const struct device *_arg) {
@@ -92,22 +84,21 @@ static int zmk_backlight_init(const struct device *_arg) {
 
 #if IS_ENABLED(CONFIG_SETTINGS)
     settings_subsys_init();
-
-    int err = settings_register(&backlight_conf);
-    if (err) {
-        LOG_ERR("Failed to register the backlight settings handler (err %d)", err);
-        return err;
+    int rc = settings_load_subtree_direct("backlight", backlight_settings_load_cb, NULL);
+    if (rc != 0) {
+        LOG_ERR("Failed to load backlight settings: %d", rc);
     }
-
-    k_delayed_work_init(&backlight_save_work, zmk_backlight_save_state_work);
-
-    settings_load_subtree("backlight");
 #endif
 
     return zmk_backlight_update();
 }
 
-static int zmk_backlight_save_state() {
+static int zmk_backlight_update_and_save() {
+    int rc = zmk_backlight_update();
+    if (rc != 0) {
+        return rc;
+    }
+
 #if IS_ENABLED(CONFIG_SETTINGS)
     k_delayed_work_cancel(&backlight_save_work);
     return k_delayed_work_submit(&backlight_save_work, K_MSEC(CONFIG_ZMK_SETTINGS_SAVE_DEBOUNCE));
@@ -116,111 +107,57 @@ static int zmk_backlight_save_state() {
 #endif
 }
 
-bool zmk_backlight_get_on() { return state.on; }
-
 int zmk_backlight_on() {
-    if (!state.on && state.brightness == 0) {
-        state.brightness = CONFIG_ZMK_BACKLIGHT_BRT_STEP;
-    }
+    state.brightness = MAX(state.brightness, CONFIG_ZMK_BACKLIGHT_BRT_STEP);
     state.on = true;
-
-    int rc = zmk_backlight_update();
-    if (rc != 0) {
-        return rc;
-    }
-
-    return zmk_backlight_save_state();
+    return zmk_backlight_update_and_save();
 }
 
 int zmk_backlight_off() {
-
     state.on = false;
-
-    int rc = zmk_backlight_update();
-    if (rc != 0) {
-        return rc;
-    }
-
-    return zmk_backlight_save_state();
+    return zmk_backlight_update_and_save();
 }
 
-int zmk_backlight_get_brt() { return state.on ? state.brightness : 0; }
-
 int zmk_backlight_toggle() { return state.on ? zmk_backlight_off() : zmk_backlight_on(); }
 
-int zmk_backlight_set_brt(uint8_t brightness) {
-    if (brightness > BRT_MAX) {
-        brightness = BRT_MAX;
-    }
-
-    state.brightness = brightness;
-    state.on = (brightness > 0);
-
-    int rc = zmk_backlight_update();
-    if (rc != 0) {
-        return rc;
-    }
-
-    return zmk_backlight_save_state();
-}
-
-uint8_t zmk_backlight_calc_brt(int direction) {
-    uint8_t brightness = state.brightness;
-
-    int b = state.brightness + (direction * CONFIG_ZMK_BACKLIGHT_BRT_STEP);
-    return CLAMP(b, 0, BRT_MAX);
-}
-
-int zmk_backlight_adjust_brt(int direction) {
+bool zmk_backlight_is_on() { return state.on; }
 
-    state.brightness = zmk_backlight_calc_brt(direction);
+int zmk_backlight_set_brt(uint8_t brightness) {
+    state.brightness = MIN(brightness, BRT_MAX);
     state.on = (state.brightness > 0);
-
-    int rc = zmk_backlight_update();
-    if (rc != 0) {
-        return rc;
-    }
-
-    return zmk_backlight_save_state();
+    return zmk_backlight_update_and_save();
 }
 
-#if IS_ENABLED(CONFIG_ZMK_BACKLIGHT_AUTO_OFF_IDLE)
-static bool auto_off_idle_prev_state = false;
-#endif
+uint8_t zmk_backlight_get_brt() { return state.on ? state.brightness : 0; }
 
-#if IS_ENABLED(CONFIG_ZMK_BACKLIGHT_AUTO_OFF_USB)
-static bool auto_off_usb_prev_state = false;
-#endif
+uint8_t zmk_backlight_calc_brt(int direction) {
+    int brt = state.brightness + (direction * CONFIG_ZMK_BACKLIGHT_BRT_STEP);
+    return CLAMP(brt, 0, BRT_MAX);
+}
 
 #if IS_ENABLED(CONFIG_ZMK_BACKLIGHT_AUTO_OFF_IDLE) || IS_ENABLED(CONFIG_ZMK_BACKLIGHT_AUTO_OFF_USB)
-static int backlight_auto_state(bool *prev_state, bool *new_state) {
-    if (state.on == *new_state) {
+static int backlight_auto_state(bool *prev_state, bool new_state) {
+    if (state.on == new_state) {
         return 0;
     }
-    if (*new_state) {
-        state.on = *prev_state;
-        *prev_state = false;
-        return zmk_backlight_on();
-    } else {
-        state.on = false;
-        *prev_state = true;
-        return zmk_backlight_off();
-    }
+    state.on = new_state && *prev_state;
+    *prev_state = !new_state;
+    return zmk_backlight_update();
 }
 
 static int backlight_event_listener(const zmk_event_t *eh) {
 
 #if IS_ENABLED(CONFIG_ZMK_BACKLIGHT_AUTO_OFF_IDLE)
     if (as_zmk_activity_state_changed(eh)) {
-        bool new_state = (zmk_activity_get_state() == ZMK_ACTIVITY_ACTIVE);
-        return backlight_auto_state(&auto_off_idle_prev_state, &new_state);
+        static bool prev_state = false;
+        return backlight_auto_state(&prev_state, zmk_activity_get_state() == ZMK_ACTIVITY_ACTIVE);
     }
 #endif
 
 #if IS_ENABLED(CONFIG_ZMK_BACKLIGHT_AUTO_OFF_USB)
     if (as_zmk_usb_conn_state_changed(eh)) {
-        bool new_state = zmk_usb_is_powered();
-        return backlight_auto_state(&auto_off_usb_prev_state, &new_state);
+        static bool prev_state = false;
+        return backlight_auto_state(&prev_state, zmk_usb_is_powered());
     }
 #endif
 
diff --git a/app/src/behaviors/behavior_backlight.c b/app/src/behaviors/behavior_backlight.c
index 3dcbd3c..8dd6ee5 100644
--- a/app/src/behaviors/behavior_backlight.c
+++ b/app/src/behaviors/behavior_backlight.c
@@ -24,24 +24,17 @@ static int
 on_keymap_binding_convert_central_state_dependent_params(struct zmk_behavior_binding *binding,
                                                          struct zmk_behavior_binding_event event) {
     switch (binding->param1) {
-    case BL_TOG_CMD: {
-        binding->param1 = zmk_backlight_get_on() ? BL_OFF_CMD : BL_ON_CMD;
+    case BL_TOG_CMD:
+        binding->param1 = zmk_backlight_is_on() ? BL_OFF_CMD : BL_ON_CMD;
         break;
-    }
-    case BL_INC_CMD: {
-        uint8_t brightness = zmk_backlight_calc_brt(1);
-
+    case BL_INC_CMD:
         binding->param1 = BL_SET_CMD;
-        binding->param2 = brightness;
+        binding->param2 = zmk_backlight_calc_brt(1);
         break;
-    }
-    case BL_DEC_CMD: {
-        uint8_t brightness = zmk_backlight_calc_brt(-1);
-
+    case BL_DEC_CMD:
         binding->param1 = BL_SET_CMD;
-        binding->param2 = brightness;
+        binding->param2 = zmk_backlight_calc_brt(-1);
         break;
-    }
     default:
         return 0;
     }
@@ -54,18 +47,24 @@ on_keymap_binding_convert_central_state_dependent_params(struct zmk_behavior_bin
 static int on_keymap_binding_pressed(struct zmk_behavior_binding *binding,
                                      struct zmk_behavior_binding_event event) {
     switch (binding->param1) {
-    case BL_TOG_CMD:
-        return zmk_backlight_toggle();
     case BL_ON_CMD:
         return zmk_backlight_on();
     case BL_OFF_CMD:
         return zmk_backlight_off();
-    case BL_INC_CMD:
-        return zmk_backlight_adjust_brt(1);
-    case BL_DEC_CMD:
-        return zmk_backlight_adjust_brt(-1);
+    case BL_TOG_CMD:
+        return zmk_backlight_toggle();
+    case BL_INC_CMD: {
+        uint8_t brt = zmk_backlight_calc_brt(1);
+        return zmk_backlight_set_brt(brt);
+    }
+    case BL_DEC_CMD: {
+        uint8_t brt = zmk_backlight_calc_brt(-1);
+        return zmk_backlight_set_brt(brt);
+    }
     case BL_SET_CMD:
         return zmk_backlight_set_brt(binding->param2);
+    default:
+        LOG_ERR("Unknown backlight command: %d", binding->param1);
     }
 
     return -ENOTSUP;
diff --git a/docs/docs/features/backlight.md b/docs/docs/features/backlight.md
index a917b16..f43c735 100644
--- a/docs/docs/features/backlight.md
+++ b/docs/docs/features/backlight.md
@@ -24,7 +24,7 @@ There are various Kconfig options used to configure the backlight feature. These
 | `CONFIG_ZMK_BACKLIGHT_BRT_STEP`      | Brightness step in percent                            | 20      |
 | `CONFIG_ZMK_BACKLIGHT_BRT_START`     | Default brightness in percent                         | 40      |
 | `CONFIG_ZMK_BACKLIGHT_ON_START`      | Default backlight state                               | y       |
-| `CONFIG_ZMK_BACKLIGHT_AUTO_OFF_IDLE` | Turn off backlight when keyboard goes into idle state | y       |
+| `CONFIG_ZMK_BACKLIGHT_AUTO_OFF_IDLE` | Turn off backlight when keyboard goes into idle state | n       |
 | `CONFIG_ZMK_BACKLIGHT_AUTO_OFF_USB`  | Turn off backlight when USB is disconnected           | n       |
 
 ## Adding Backlight to a Board
@@ -43,7 +43,7 @@ First, you need to enable PWM by adding the following lines to your `.overlay` f
 };
 ```
 
-The value `ch0-pin` represents the pin that controls the LEDs. To calculate the value to use, you need a bit of math. You need the hardware port and run it through a function.
+The value `ch0-pin` represents the pin that controls the LEDs. With nRF52 boards, you can calculate the value to use in the following way: you need the hardware port and run it through a function.
 **32 \* X + Y** = `<Pin number>` where X is first part of the hardware port "PX.01" and Y is the second part of the hardware port "P1.Y".
 
 For example, _P1.13_ would give you _32 \* 1 + 13_ = `<45>` and _P0.15_ would give you _32 \* 0 + 15_ = `<15>`.