From ce843825e8343354d3e9bcc497adcc898602b294 Mon Sep 17 00:00:00 2001 From: Alessandro Bortolin Date: Mon, 20 Dec 2021 11:00:22 +0100 Subject: [PATCH] refactor(backlight): code cleanup --- app/Kconfig | 2 - app/dts/behaviors/backlight.dtsi | 2 +- app/include/dt-bindings/zmk/backlight.h | 8 +- app/include/zmk/backlight.h | 10 +- app/src/backlight.c | 137 +++++++----------------- app/src/behaviors/behavior_backlight.c | 37 ++++--- docs/docs/features/backlight.md | 4 +- 7 files changed, 67 insertions(+), 133 deletions(-) diff --git a/app/Kconfig b/app/Kconfig index 49eec835..02fef66a 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 6127f605..f9bd02b8 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 70572437..fa6dc9b1 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 8711d884..dd7d966a 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 fc8831c7..6ef7f8f0 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 3dcbd3c4..8dd6ee58 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 a917b166..f43c7357 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** = `` 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>`.