From c68ff8b99ce62870f7a448d8d37c0d9fd928c863 Mon Sep 17 00:00:00 2001 From: Radoslaw Koppel Date: Wed, 18 Sep 2019 16:07:01 +0200 Subject: [PATCH] settings: Generic function to call set handler This commit implements generic function to decide witch functions to call for selected value name with given loading parameters. Signed-off-by: Radoslaw Koppel --- include/settings/settings.h | 60 ++++++++++++++++----- samples/bluetooth/peripheral_dis/src/main.c | 10 ++-- subsys/settings/src/settings.c | 40 ++++++++++++++ subsys/settings/src/settings_fcb.c | 15 ++---- subsys/settings/src/settings_file.c | 15 ++---- subsys/settings/src/settings_line.c | 38 ++----------- subsys/settings/src/settings_nvs.c | 38 +++++-------- subsys/settings/src/settings_priv.h | 6 --- subsys/settings/src/settings_store.c | 12 ++++- 9 files changed, 128 insertions(+), 106 deletions(-) diff --git a/include/settings/settings.h b/include/settings/settings.h index 9b08ed57559..2e9827dd566 100644 --- a/include/settings/settings.h +++ b/include/settings/settings.h @@ -342,7 +342,6 @@ int settings_commit_subtree(const char *subtree); struct settings_store_itf; /** - * @struct settings_store * Backend handler node for storage handling. */ struct settings_store { @@ -354,23 +353,43 @@ struct settings_store { }; /** - * @struct settings_store_itf + * Arguments for data loading. + * Holds all parameters that changes the way data should be loaded from backend. + */ +struct settings_load_arg { + /** + * @brief Name of the subtree to be loaded + * + * If NULL, all values would be loaded. + */ + const char *subtree; + /** + * @brief Pointer to the callback function. + * + * If NULL then matching registered function would be used. + */ + settings_load_direct_cb cb; + /** + * @brief Parameter for callback function + * + * Parameter to be passed to the callback function. + */ + void *param; +}; + +/** * Backend handler functions. * Sources are registered using a call to @ref settings_src_register. * Destinations are registered using a call to @ref settings_dst_register. */ struct settings_store_itf { - int (*csi_load)(struct settings_store *cs, const char *subtree, - settings_load_direct_cb cb, void *param); + int (*csi_load)(struct settings_store *cs, + const struct settings_load_arg *arg); /**< Loads values from storage limited to subtree defined by subtree. * * Parameters: - * - cs - Corresponding backend handler node - * - subtree - subtree name of the subtree to be loaded, - * if NULL loads all values. - * - cb - pointer to the callback function, - * if NULL then matching registered function would be used. - * - param - parameter to be passed when callback function is called. + * - cs - Corresponding backend handler node, + * - arg - Structure that holds additional data for data loading. */ int (*csi_save_start)(struct settings_store *cs); @@ -429,9 +448,26 @@ void settings_dst_register(struct settings_store *cs); * @return settings_handler_static on success, NULL on failure. */ struct settings_handler_static *settings_parse_and_lookup(const char *name, - const char **next); - + const char **next); +/** + * Calls settings handler. + * + * @param[in] name The name of the data found in the backend. + * @param[in] len The size of the data found in the backend. + * @param[in] read_cb Function provided to read the data from + * the backend. + * @param[in,out] read_cb_arg Arguments for the read function provided by + * the backend. + * @param[in,out] load_arg Arguments for data loading. + * + * @return 0 or negative error code + */ +int settings_call_set_handler(const char *name, + size_t len, + settings_read_cb read_cb, + void *read_cb_arg, + const struct settings_load_arg *load_arg); /* * API for const name processing */ diff --git a/samples/bluetooth/peripheral_dis/src/main.c b/samples/bluetooth/peripheral_dis/src/main.c index f07247e6d1f..a0b07cf0a3e 100644 --- a/samples/bluetooth/peripheral_dis/src/main.c +++ b/samples/bluetooth/peripheral_dis/src/main.c @@ -46,9 +46,7 @@ static struct bt_conn_cb conn_callbacks = { }; static int zephyr_settings_fw_load(struct settings_store *cs, - const char *subtree, - settings_load_direct_cb cb, - void *param); + const struct settings_load_arg *arg); static const struct settings_store_itf zephyr_settings_fw_itf = { .csi_load = zephyr_settings_fw_load, @@ -59,12 +57,10 @@ static struct settings_store zephyr_settings_fw_store = { }; static int zephyr_settings_fw_load(struct settings_store *cs, - const char *subtree, - settings_load_direct_cb cb, - void *param) + const struct settings_load_arg *arg) { /* Ignore subtree and direct loading */ - if (subtree || cb) + if (arg && (arg->subtree || arg->cb)) return 0; #if defined(CONFIG_BT_GATT_DIS_SETTINGS) settings_runtime_set("bt/dis/model", diff --git a/subsys/settings/src/settings.c b/subsys/settings/src/settings.c index 9861dfdeda3..93a75053b48 100644 --- a/subsys/settings/src/settings.c +++ b/subsys/settings/src/settings.c @@ -190,6 +190,46 @@ struct settings_handler_static *settings_parse_and_lookup(const char *name, return bestmatch; } +int settings_call_set_handler(const char *name, + size_t len, + settings_read_cb read_cb, + void *read_cb_arg, + const struct settings_load_arg *load_arg) +{ + int rc; + const char *name_key; + + if (load_arg && load_arg->subtree && + !settings_name_steq(name, load_arg->subtree, &name_key)) { + return 0; + } + + if (load_arg && load_arg->cb) { + rc = load_arg->cb(name_key, len, read_cb, read_cb_arg, + load_arg->param); + } else { + struct settings_handler_static *ch; + + ch = settings_parse_and_lookup(name, &name_key); + if (!ch) { + return 0; + } + + rc = ch->h_set(name_key, len, read_cb, read_cb_arg); + + if (rc != 0) { + LOG_ERR("set-value failure. key: %s error(%d)", + log_strdup(name), rc); + /* Ignoring the error */ + rc = 0; + } else { + LOG_DBG("set-value OK. key: %s", + log_strdup(name)); + } + } + return rc; +} + int settings_commit(void) { return settings_commit_subtree(NULL); diff --git a/subsys/settings/src/settings_fcb.c b/subsys/settings/src/settings_fcb.c index be2d205ea6c..d47c6cfd597 100644 --- a/subsys/settings/src/settings_fcb.c +++ b/subsys/settings/src/settings_fcb.c @@ -23,8 +23,8 @@ struct settings_fcb_load_cb_arg { void *cb_arg; }; -static int settings_fcb_load(struct settings_store *cs, const char *subtree, - settings_load_direct_cb cb, void *param); +static int settings_fcb_load(struct settings_store *cs, + const struct settings_load_arg *arg); static int settings_fcb_save(struct settings_store *cs, const char *name, const char *value, size_t val_len); @@ -118,15 +118,10 @@ static int settings_fcb_load_priv(struct settings_store *cs, line_load_cb cb, return 0; } -static int settings_fcb_load(struct settings_store *cs, const char *subtree, - settings_load_direct_cb cb, void *param) +static int settings_fcb_load(struct settings_store *cs, + const struct settings_load_arg *arg) { - struct settings_line_load_arg arg = { - .subtree = subtree, - .direct_cb = cb, - .param = param - }; - return settings_fcb_load_priv(cs, settings_line_load_cb, &arg); + return settings_fcb_load_priv(cs, settings_line_load_cb, (void *)arg); } diff --git a/subsys/settings/src/settings_file.c b/subsys/settings/src/settings_file.c index 77cb94aa860..07fef8bbd4f 100644 --- a/subsys/settings/src/settings_file.c +++ b/subsys/settings/src/settings_file.c @@ -14,8 +14,8 @@ #include "settings/settings_file.h" #include "settings_priv.h" -static int settings_file_load(struct settings_store *cs, const char *subtree, - settings_load_direct_cb cb, void *param); +static int settings_file_load(struct settings_store *cs, + const struct settings_load_arg *arg); static int settings_file_save(struct settings_store *cs, const char *name, const char *value, size_t val_len); @@ -110,15 +110,10 @@ static int settings_file_load_priv(struct settings_store *cs, line_load_cb cb, /* * Called to load configuration items. */ -static int settings_file_load(struct settings_store *cs, const char *subtree, - settings_load_direct_cb cb, void *param) +static int settings_file_load(struct settings_store *cs, + const struct settings_load_arg *arg) { - struct settings_line_load_arg arg = { - .subtree = subtree, - .direct_cb = cb, - .param = param - }; - return settings_file_load_priv(cs, settings_line_load_cb, &arg); + return settings_file_load_priv(cs, settings_line_load_cb, (void *)arg); } static void settings_tmpfile(char *dst, const char *src, char *pfx) diff --git a/subsys/settings/src/settings_line.c b/subsys/settings/src/settings_line.c index fbf957137f0..f31dea30b55 100644 --- a/subsys/settings/src/settings_line.c +++ b/subsys/settings/src/settings_line.c @@ -542,43 +542,15 @@ static ssize_t settings_line_read_cb(void *cb_arg, void *data, size_t len) } int settings_line_load_cb(const char *name, void *val_read_cb_ctx, off_t off, - void *cb_arg) + void *cb_arg) { - const char *name_key; - struct settings_handler_static *ch; - struct settings_line_read_value_cb_ctx value_ctx; - struct settings_line_load_arg *arg = cb_arg; - int rc; size_t len; - - if (arg && arg->subtree && - !settings_name_steq(name, arg->subtree, &name_key)) { - return 0; - } - + struct settings_line_read_value_cb_ctx value_ctx; + struct settings_load_arg *arg = cb_arg; value_ctx.read_cb_ctx = val_read_cb_ctx; value_ctx.off = off; len = settings_line_val_get_len(off, val_read_cb_ctx); - if (arg && arg->direct_cb) { - rc = arg->direct_cb(name_key, len, settings_line_read_cb, - (void *)&value_ctx, arg->param); - } else { - ch = settings_parse_and_lookup(name, &name_key); - if (!ch) { - return 0; - } - - rc = ch->h_set(name_key, len, settings_line_read_cb, - (void *)&value_ctx); - - if (rc != 0) { - LOG_ERR("set-value failure. key: %s error(%d)", - log_strdup(name), rc); - } else { - LOG_DBG("set-value OK. key: %s", - log_strdup(name)); - } - } - return rc; + return settings_call_set_handler(name, len, settings_line_read_cb, + &value_ctx, arg); } diff --git a/subsys/settings/src/settings_nvs.c b/subsys/settings/src/settings_nvs.c index a05acba5742..0b8656cb056 100644 --- a/subsys/settings/src/settings_nvs.c +++ b/subsys/settings/src/settings_nvs.c @@ -20,8 +20,8 @@ struct settings_nvs_read_fn_arg { u16_t id; }; -static int settings_nvs_load(struct settings_store *cs, const char *subtree, - settings_load_direct_cb cb, void *param); +static int settings_nvs_load(struct settings_store *cs, + const struct settings_load_arg *arg); static int settings_nvs_save(struct settings_store *cs, const char *name, const char *value, size_t val_len); @@ -63,15 +63,14 @@ int settings_nvs_dst(struct settings_nvs *cf) return 0; } -static int settings_nvs_load(struct settings_store *cs, const char *subtree, - settings_load_direct_cb cb, void *param) +static int settings_nvs_load(struct settings_store *cs, + const struct settings_load_arg *arg) { + int ret = 0; struct settings_nvs *cf = (struct settings_nvs *)cs; struct settings_nvs_read_fn_arg read_fn_arg; - struct settings_handler_static *ch; char name[SETTINGS_MAX_NAME_LEN + SETTINGS_EXTRA_LEN + 1]; char buf; - const char *name_argv; ssize_t rc1, rc2; u16_t name_id = NVS_NAMECNT_ID; @@ -114,31 +113,18 @@ static int settings_nvs_load(struct settings_store *cs, const char *subtree, /* Found a name, this might not include a trailing \0 */ name[rc1] = '\0'; - - if (subtree && !settings_name_steq(name, subtree, &name_argv)) { - continue; - } - read_fn_arg.fs = &cf->cf_nvs; read_fn_arg.id = name_id + NVS_NAME_ID_OFFSET; - if (cb) { - int ret; - ret = cb(name_argv, rc2, settings_nvs_read_fn, - (void *) &read_fn_arg, param); - if (ret) - return ret; - } else { - ch = settings_parse_and_lookup(name, &name_argv); - if (!ch) { - continue; - } - - ch->h_set(name_argv, rc2, settings_nvs_read_fn, - (void *) &read_fn_arg); + ret = settings_call_set_handler( + name, rc2, + settings_nvs_read_fn, &read_fn_arg, + (void *)arg); + if (ret) { + break; } } - return 0; + return ret; } static int settings_nvs_save(struct settings_store *cs, const char *name, diff --git a/subsys/settings/src/settings_priv.h b/subsys/settings/src/settings_priv.h index 1dbafe0fdd4..b62ecbd50d7 100644 --- a/subsys/settings/src/settings_priv.h +++ b/subsys/settings/src/settings_priv.h @@ -46,12 +46,6 @@ int settings_line_load_cb(const char *name, void *val_read_cb_ctx, typedef int (*line_load_cb)(const char *name, void *val_read_cb_ctx, off_t off, void *cb_arg); -struct settings_line_load_arg { - const char *subtree; - settings_load_direct_cb direct_cb; - void *param; -}; - struct settings_line_read_value_cb_ctx { void *read_cb_ctx; off_t off; diff --git a/subsys/settings/src/settings_store.c b/subsys/settings/src/settings_store.c index a41587ccc28..922ffd64ef4 100644 --- a/subsys/settings/src/settings_store.c +++ b/subsys/settings/src/settings_store.c @@ -51,6 +51,9 @@ int settings_load_subtree(const char *subtree) { struct settings_store *cs; int rc; + const struct settings_load_arg arg = { + .subtree = subtree + }; /* * for every config store @@ -60,7 +63,7 @@ int settings_load_subtree(const char *subtree) */ k_mutex_lock(&settings_lock, K_FOREVER); SYS_SLIST_FOR_EACH_CONTAINER(&settings_load_srcs, cs, cs_next) { - cs->cs_itf->csi_load(cs, subtree, NULL, NULL); + cs->cs_itf->csi_load(cs, &arg); } rc = settings_commit_subtree(subtree); k_mutex_unlock(&settings_lock); @@ -74,6 +77,11 @@ int settings_load_subtree_direct( { struct settings_store *cs; + const struct settings_load_arg arg = { + .subtree = subtree, + .cb = cb, + .param = param + }; /* * for every config store * load config @@ -82,7 +90,7 @@ int settings_load_subtree_direct( */ k_mutex_lock(&settings_lock, K_FOREVER); SYS_SLIST_FOR_EACH_CONTAINER(&settings_load_srcs, cs, cs_next) { - cs->cs_itf->csi_load(cs, subtree, cb, param); + cs->cs_itf->csi_load(cs, &arg); } k_mutex_unlock(&settings_lock); return 0;