Skip to content

Commit

Permalink
fix: Improve startup time with proper settings loading.
Browse files Browse the repository at this point in the history
* Avoid doing duplicate calls to setings_load_subtree, which iterates
  NVS fully each time under the hood, and instead use on settings_load
  later in the lifecycle.
  • Loading branch information
petejohanson committed Jul 3, 2024
1 parent f18974e commit 80173f8
Show file tree
Hide file tree
Showing 11 changed files with 129 additions and 102 deletions.
4 changes: 4 additions & 0 deletions app/Kconfig.behaviors
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,16 @@ config ZMK_BEHAVIOR_LOCAL_IDS

if ZMK_BEHAVIOR_LOCAL_IDS

config ZMK_BEHAVIOR_LOCAL_IDS_IN_BINDINGS
bool "Track in behavior bindings"

choice ZMK_BEHAVIOR_LOCAL_ID_TYPE
prompt "Local ID Type"

config ZMK_BEHAVIOR_LOCAL_ID_TYPE_SETTINGS_TABLE
bool "Settings Table"
depends on SETTINGS
select ZMK_BEHAVIOR_LOCAL_IDS_IN_BINDINGS
help
Use persistent entries in the settings subsystem to identify
behaviors by local ID, which uses the device name to generate
Expand Down
7 changes: 5 additions & 2 deletions app/include/zmk/behavior.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,12 @@
#define ZMK_BEHAVIOR_OPAQUE 0
#define ZMK_BEHAVIOR_TRANSPARENT 1

typedef uint16_t zmk_behavior_local_id_t;

struct zmk_behavior_binding {
#if IS_ENABLED(CONFIG_ZMK_BEHAVIOR_LOCAL_IDS_IN_BINDINGS)
zmk_behavior_local_id_t local_id;
#endif // IS_ENABLED(CONFIG_ZMK_BEHAVIOR_LOCAL_IDS_IN_BINDINGS)
const char *behavior_dev;
uint32_t param1;
uint32_t param2;
Expand All @@ -23,8 +28,6 @@ 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.
*
Expand Down
14 changes: 8 additions & 6 deletions app/src/backlight.c
Original file line number Diff line number Diff line change
Expand Up @@ -58,19 +58,26 @@ static int zmk_backlight_update(void) {

#if IS_ENABLED(CONFIG_SETTINGS)
static int backlight_settings_load_cb(const char *name, size_t len, settings_read_cb read_cb,
void *cb_arg, void *param) {
void *cb_arg) {
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) {
rc = zmk_backlight_update();
}

return MIN(rc, 0);
}
return -ENOENT;
}

SETTINGS_STATIC_HANDLER_DEFINE(backlight, "backlight", NULL, backlight_settings_load_cb, NULL,
NULL);

static void backlight_save_work_handler(struct k_work *work) {
settings_save_one("backlight/state", &state, sizeof(state));
}
Expand All @@ -85,11 +92,6 @@ static int zmk_backlight_init(void) {
}

#if IS_ENABLED(CONFIG_SETTINGS)
settings_subsys_init();
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_work_init_delayable(&backlight_save_work, backlight_save_work_handler);
#endif
#if IS_ENABLED(CONFIG_ZMK_BACKLIGHT_AUTO_OFF_USB)
Expand Down
14 changes: 3 additions & 11 deletions app/src/behavior.c
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,8 @@ static int behavior_local_id_init(void) {
return 0;
}

SYS_INIT(behavior_local_id_init, APPLICATION, CONFIG_APPLICATION_INIT_PRIORITY);

#elif IS_ENABLED(CONFIG_ZMK_BEHAVIOR_LOCAL_ID_TYPE_SETTINGS_TABLE)

static zmk_behavior_local_id_t largest_local_id = 0;
Expand All @@ -239,7 +241,7 @@ static int behavior_handle_set(const char *name, size_t len, settings_read_cb re

if (settings_name_steq(name, "local_id", &next) && next) {
char *endptr;
uint8_t local_id = strtoul(next, &endptr, 10);
zmk_behavior_local_id_t local_id = strtoul(next, &endptr, 10);
if (*endptr != '\0') {
LOG_WRN("Invalid behavior local ID: %s with endptr %s", next, endptr);
return -EINVAL;
Expand Down Expand Up @@ -302,22 +304,12 @@ static int behavior_handle_commit(void) {
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)
Expand Down
48 changes: 24 additions & 24 deletions app/src/ble.c
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,11 @@ static int ble_profiles_handle_set(const char *name, size_t len, settings_read_c
return 0;
};

struct settings_handler profiles_handler = {.name = "ble", .h_set = ble_profiles_handle_set};
static int zmk_ble_complete_startup(void);

static struct settings_handler profiles_handler = {
.name = "ble", .h_set = ble_profiles_handle_set, .h_commit = zmk_ble_complete_startup};

#endif /* IS_ENABLED(CONFIG_SETTINGS) */

static bool is_conn_active_profile(const struct bt_conn *conn) {
Expand Down Expand Up @@ -644,29 +648,7 @@ static void zmk_ble_ready(int err) {
update_advertising();
}

static int zmk_ble_init(void) {
int err = bt_enable(NULL);

if (err) {
LOG_ERR("BLUETOOTH FAILED (%d)", err);
return err;
}

#if IS_ENABLED(CONFIG_SETTINGS)
settings_subsys_init();

err = settings_register(&profiles_handler);
if (err) {
LOG_ERR("Failed to setup the profile settings handler (err %d)", err);
return err;
}

k_work_init_delayable(&ble_save_work, ble_save_profile_work);

settings_load_subtree("ble");
settings_load_subtree("bt");

#endif
static int zmk_ble_complete_startup(void) {

#if IS_ENABLED(CONFIG_ZMK_BLE_CLEAR_BONDS_ON_START)
LOG_WRN("Clearing all existing BLE bond information from the keyboard");
Expand Down Expand Up @@ -706,6 +688,24 @@ static int zmk_ble_init(void) {
return 0;
}

static int zmk_ble_init(void) {
int err = bt_enable(NULL);

if (err < 0 && err != -EALREADY) {
LOG_ERR("BLUETOOTH FAILED (%d)", err);
return err;
}

#if IS_ENABLED(CONFIG_SETTINGS)
settings_register(&profiles_handler);
k_work_init_delayable(&ble_save_work, ble_save_profile_work);
#else
zmk_ble_complete_startup();
#endif

return 0;
}

#if IS_ENABLED(CONFIG_ZMK_BLE_PASSKEY_ENTRY)

static bool zmk_ble_numeric_usage_to_value(const zmk_key_t key, const zmk_key_t one,
Expand Down
13 changes: 2 additions & 11 deletions app/src/endpoints.c
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,8 @@ static int endpoints_handle_set(const char *name, size_t len, settings_read_cb r
return 0;
}

struct settings_handler endpoints_handler = {.name = "endpoints", .h_set = endpoints_handle_set};
SETTINGS_STATIC_HANDLER_DEFINE(endpoints, "endpoints", NULL, endpoints_handle_set, NULL, NULL);

#endif /* IS_ENABLED(CONFIG_SETTINGS) */

static bool is_usb_ready(void) {
Expand Down Expand Up @@ -322,17 +323,7 @@ static struct zmk_endpoint_instance get_selected_instance(void) {

static int zmk_endpoints_init(void) {
#if IS_ENABLED(CONFIG_SETTINGS)
settings_subsys_init();

int err = settings_register(&endpoints_handler);
if (err) {
LOG_ERR("Failed to register the endpoints settings handler (err %d)", err);
return err;
}

k_work_init_delayable(&endpoints_save_work, endpoints_save_preferred_work);

settings_load_subtree("endpoints");
#endif

current_instance = get_selected_instance();
Expand Down
39 changes: 18 additions & 21 deletions app/src/ext_power_generic.c
Original file line number Diff line number Diff line change
Expand Up @@ -121,12 +121,27 @@ static int ext_power_settings_set(const char *name, size_t len, settings_read_cb
return -ENOENT;
}

struct settings_handler ext_power_conf = {.name = "ext_power/state",
.h_set = ext_power_settings_set};
static int ext_power_settings_commit() {
const struct device *dev = DEVICE_DT_GET(DT_DRV_INST(0));
struct ext_power_generic_data *data = dev->data;

if (!data->settings_init) {

data->status = true;
k_work_schedule(&ext_power_save_work, K_NO_WAIT);

ext_power_enable(dev);
}

return 0;
}

SETTINGS_STATIC_HANDLER_DEFINE(ext_power, "ext_power/state", NULL, ext_power_settings_set,
ext_power_settings_commit, NULL);

#endif

static int ext_power_generic_init(const struct device *dev) {
struct ext_power_generic_data *data = dev->data;
const struct ext_power_generic_config *config = dev->config;

if (gpio_pin_configure_dt(&config->control, GPIO_OUTPUT_INACTIVE)) {
Expand All @@ -135,25 +150,7 @@ static int ext_power_generic_init(const struct device *dev) {
}

#if IS_ENABLED(CONFIG_SETTINGS)
settings_subsys_init();

int err = settings_register(&ext_power_conf);
if (err) {
LOG_ERR("Failed to register the ext_power settings handler (err %d)", err);
return err;
}

k_work_init_delayable(&ext_power_save_work, ext_power_save_state_work);

// Set default value (on) if settings isn't set
settings_load_subtree("ext_power");
if (!data->settings_init) {

data->status = true;
k_work_schedule(&ext_power_save_work, K_NO_WAIT);

ext_power_enable(dev);
}
#else
// Default to the ext_power being open when no settings
ext_power_enable(dev);
Expand Down
5 changes: 5 additions & 0 deletions app/src/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ int main(void) {
return -ENOTSUP;
}

#if IS_ENABLED(CONFIG_SETTINGS)
settings_subsys_init();
settings_load();
#endif

#ifdef CONFIG_ZMK_DISPLAY
zmk_display_init();
#endif /* CONFIG_ZMK_DISPLAY */
Expand Down
16 changes: 5 additions & 11 deletions app/src/rgb_underglow.c
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,10 @@ static int rgb_settings_set(const char *name, size_t len, settings_read_cb read_

rc = read_cb(cb_arg, &state, sizeof(state));
if (rc >= 0) {
if (state.on) {
k_timer_start(&underglow_tick, K_NO_WAIT, K_MSEC(50));
}

return 0;
}

Expand All @@ -230,7 +234,7 @@ static int rgb_settings_set(const char *name, size_t len, settings_read_cb read_
return -ENOENT;
}

struct settings_handler rgb_conf = {.name = "rgb/underglow", .h_set = rgb_settings_set};
SETTINGS_STATIC_HANDLER_DEFINE(rgb_underglow, "rgb/underglow", NULL, rgb_settings_set, NULL, NULL);

static void zmk_rgb_underglow_save_state_work(struct k_work *_work) {
settings_save_one("rgb/underglow/state", &state, sizeof(state));
Expand Down Expand Up @@ -262,17 +266,7 @@ static int zmk_rgb_underglow_init(void) {
};

#if IS_ENABLED(CONFIG_SETTINGS)
settings_subsys_init();

int err = settings_register(&rgb_conf);
if (err) {
LOG_ERR("Failed to register the ext_power settings handler (err %d)", err);
return err;
}

k_work_init_delayable(&underglow_save_work, zmk_rgb_underglow_save_state_work);

settings_load_subtree("rgb/underglow");
#endif

#if IS_ENABLED(CONFIG_ZMK_RGB_UNDERGLOW_AUTO_OFF_USB)
Expand Down
24 changes: 23 additions & 1 deletion app/src/split/bluetooth/central.c
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <zephyr/bluetooth/uuid.h>
#include <zephyr/bluetooth/gatt.h>
#include <zephyr/bluetooth/hci.h>
#include <zephyr/settings/settings.h>
#include <zephyr/sys/byteorder.h>

#include <zephyr/logging/log.h>
Expand Down Expand Up @@ -865,13 +866,34 @@ int zmk_split_bt_update_hid_indicator(zmk_hid_indicators_t indicators) {

#endif // IS_ENABLED(CONFIG_ZMK_SPLIT_PERIPHERAL_HID_INDICATORS)

static int finish_init() {
return IS_ENABLED(CONFIG_ZMK_BLE_CLEAR_BONDS_ON_START) ? 0 : start_scanning();
}

#if IS_ENABLED(CONFIG_SETTINGS)

static int central_ble_handle_set(const char *name, size_t len, settings_read_cb read_cb,
void *cb_arg) {
return 0;
}

static struct settings_handler ble_central_settings_handler = {
.name = "ble_central", .h_set = central_ble_handle_set, .h_commit = finish_init};

#endif // IS_ENABLED(CONFIG_SETTINGS)

static int zmk_split_bt_central_init(void) {
k_work_queue_start(&split_central_split_run_q, split_central_split_run_q_stack,
K_THREAD_STACK_SIZEOF(split_central_split_run_q_stack),
CONFIG_ZMK_BLE_THREAD_PRIORITY, NULL);
bt_conn_cb_register(&conn_callbacks);

return IS_ENABLED(CONFIG_ZMK_BLE_CLEAR_BONDS_ON_START) ? 0 : start_scanning();
#if IS_ENABLED(CONFIG_SETTINGS)
settings_register(&ble_central_settings_handler);
return 0;
#else
return finish_init();
#endif // IS_ENABLED(CONFIG_SETTINGS)
}

SYS_INIT(zmk_split_bt_central_init, APPLICATION, CONFIG_ZMK_BLE_INIT_PRIORITY);
Loading

2 comments on commit 80173f8

@Thempra
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Variable assignment before declaration

ccache /opt/zephyr-sdk-0.16.3/arm-zephyr-eabi/bin/arm-zephyr-eabi-gcc -DKERNEL -DNRF52840_XXAA -DPICOLIBC_INTEGER_PRINTF_SCANF -D_FORTIFY_SOURCE=1 -D_POSIX_C_SOURCE=200809 -D__LINUX_ERRNO_EXTENSIONS__ -D__PROGRAM_START -D__ZEPHYR__=1 -I/__w/zmk-config/zmk-config/zmk/app/include -I/__w/zmk-config/zmk-config/zephyr/include -I/tmp/tmp.UWeHllYMOO/zephyr/include/generated -I/__w/zmk-config/zmk-config/zephyr/soc/arm/nordic_nrf/nrf52 -I/__w/zmk-config/zmk-config/zephyr/soc/arm/nordic_nrf/common/. -I/__w/zmk-config/zmk-config/zephyr/subsys/usb/device -I/__w/zmk-config/zmk-config/zephyr/subsys/bluetooth/controller/ll_sw/nordic/hal/nrf5/nrfx_glue -I/__w/zmk-config/zmk-config/zephyr/subsys/bluetooth -I/__w/zmk-config/zmk-config/zephyr/subsys/settings/include -I/__w/zmk-config/zmk-config/modules/hal/cmsis/CMSIS/Core/Include -I/__w/zmk-config/zmk-config/zephyr/modules/cmsis/. -I/__w/zmk-config/zmk-config/modules/hal/nordic/nrfx -I/__w/zmk-config/zmk-config/modules/hal/nordic/nrfx/drivers/include -I/__w/zmk-config/zmk-config/modules/hal/nordic/nrfx/mdk -I/__w/zmk-config/zmk-config/zephyr/modules/hal_nordic/nrfx/. -I/__w/zmk-config/zmk-config/modules/crypto/tinycrypt/lib/include -I/__w/zmk-config/zmk-config/zmk/app/module/include -I/__w/zmk-config/zmk-config/zmk/app/module/drivers/sensor/battery/. -fno-strict-aliasing -Os -imacros /tmp/tmp.UWeHllYMOO/zephyr/include/generated/autoconf.h -fno-printf-return-value -fno-common -g -gdwarf-4 -fdiagnostics-color=always -mcpu=cortex-m4 -mthumb -mabi=aapcs -mfpu=fpv4-sp-d16 -mfloat-abi=hard -mfp16-format=ieee --sysroot=/opt/zephyr-sdk-0.16.3/arm-zephyr-eabi/arm-zephyr-eabi -imacros /__w/zmk-config/zmk-config/zephyr/include/zephyr/toolchain/zephyr_stdint.h -Wall -Wformat -Wformat-security -Wno-format-zero-length -Wno-pointer-sign -Wpointer-arith -Wexpansion-to-defined -Wno-unused-but-set-variable -Werror=implicit-int -fno-pic -fno-pie -fno-asynchronous-unwind-tables -ftls-model=local-exec -fno-reorder-functions --param=min-pagesize=0 -fno-defer-pop -fmacro-prefix-map=/__w/zmk-config/zmk-config/zmk/app=CMAKE_SOURCE_DIR -fmacro-prefix-map=/__w/zmk-config/zmk-config/zephyr=ZEPHYR_BASE -fmacro-prefix-map=/__w/zmk-config/zmk-config=WEST_TOPDIR -ffunction-sections -fdata-sections --specs=picolibc.specs -std=c99 -Wfatal-errors -MD -MT CMakeFiles/app.dir/src/ble.c.obj -MF CMakeFiles/app.dir/src/ble.c.obj.d -o CMakeFiles/app.dir/src/ble.c.obj -c /__w/zmk-config/zmk-config/zmk/app/src/ble.c
/__w/zmk-config/zmk-config/zmk/app/src/ble.c: In function 'zmk_ble_complete_startup':
/__w/zmk-config/zmk-config/zmk/app/src/ble.c:662:9: error: 'err' undeclared (first use in this function); did you mean 'erf'?
  662 |         err = settings_delete(setting_name);
      |         ^~~
      |         erf
compilation terminated due to -Wfatal-errors.

@lesshonor
Copy link
Contributor

@lesshonor lesshonor commented on 80173f8 Jul 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Variable assignment before declaration

@Thempra You need to delete this folder from your repository.

Please sign in to comment.