Hi all,
I was not happy with my first attempt at the interface between config and
subsystems.
So here’s a 2nd version.
The config items are still identified as strings.
Subsystem registers a set of handlers, and a root for it’s config tree.
struct conf_handler {
SLIST_ENTRY(conf_handler) ch_list;
char *ch_name;
char *(*ch_get)(int argc, char **argv, char *val, int val_len_max);
int (*ch_set)(int argc, char **argv, char *val);
int (*ch_commit)();
};
ch_name would be aforementioned root for subsystem config.
ch_get/ch_set do the obvious thing; get is for getting the current value, set
is used in setting a value.
Config items have names like subsystem/item1, or subsystem/item2/item3.
‘/‘ would be used as a delimiter between components of the name. This name gets
split into
components and gets passed in as argc/argv value to subsystem handler.
When a blob of configuration comes in, it would be translated into a bunch of
calls to set(),
followed by commit.
The idea is to have set() validate the input, stash the value, and then ‘apply’
it when commit
gets called.
As you can see, the value also is passed in a form of a string. There’s some
helper functions
that you can use to translate between string and integers and so forth. Use of
those is
optional, but probably a good idea.
There is still no persistent storage of config. We should have at least 2
options for this:
storage of config in files, or in a raw flash region (when FS is not available).
I’m thinking the API should be similar for both, so decision of which one to
use will be
a simple build time knob.
There was a request to be able to load configuration from multiple sources;
e.g. from
a number of files and from the flash variable as well.
Any system will have some config items that are going to be set at
manufacturing time
(serial numbers, for example) and then there will items that change more often.
So one could for example have the manufacturing config live somewhere else from
everything else. This way reverting back to manufacturing config should be easy.
I hope folks can take a look at what’s there now, and give feedback on this.
Thanks,
M
> Begin forwarded message:
>
> From: [email protected]
> Subject: [3/3] incubator-mynewt-larva git commit: Second take on the config
> system.
> Date: January 29, 2016 at 12:50:22 PM PST
> To: [email protected]
> Reply-To: [email protected]
>
> Second take on the config system.
>
>
> Project: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-larva/repo
> Commit:
> http://git-wip-us.apache.org/repos/asf/incubator-mynewt-larva/commit/ccda6d4b
> Tree:
> http://git-wip-us.apache.org/repos/asf/incubator-mynewt-larva/tree/ccda6d4b
> Diff:
> http://git-wip-us.apache.org/repos/asf/incubator-mynewt-larva/diff/ccda6d4b
>
> Branch: refs/heads/master
> Commit: ccda6d4b166c1034cb7ec1dacd88378ffa6b7b56
> Parents: 6689163
> Author: Marko Kiiskila <[email protected]>
> Authored: Fri Jan 29 12:49:47 2016 -0800
> Committer: Marko Kiiskila <[email protected]>
> Committed: Fri Jan 29 12:49:47 2016 -0800
>
> ----------------------------------------------------------------------
> project/blinky/src/main.c | 2 +-
> project/slinky/src/main.c | 74 +++++----
> sys/config/include/config/config.h | 48 +++---
> sys/config/src/config.c | 218 ++++++++++++++++----------
> sys/config/src/config_cli.c | 43 ++----
> sys/config/src/config_nmgr.c | 34 +---
> sys/config/src/config_priv.h | 1 +
> sys/config/src/test/conf_test.c | 266 +++++++++++++-------------------
> 8 files changed, 326 insertions(+), 360 deletions(-)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/incubator-mynewt-larva/blob/ccda6d4b/project/blinky/src/main.c
> ----------------------------------------------------------------------
> diff --git a/project/blinky/src/main.c b/project/blinky/src/main.c
> index 43b985e..81637c6 100755
> --- a/project/blinky/src/main.c
> +++ b/project/blinky/src/main.c
> @@ -158,7 +158,7 @@ main(int argc, char **argv)
> mcu_sim_parse_args(argc, argv);
> #endif
>
> - conf_module_init();
> + conf_init();
>
> cbmem_init(&log_mem, log_buf, sizeof(log_buf));
> util_log_cbmem_handler_init(&log_mem_handler, &log_mem);
>
> http://git-wip-us.apache.org/repos/asf/incubator-mynewt-larva/blob/ccda6d4b/project/slinky/src/main.c
> ----------------------------------------------------------------------
> diff --git a/project/slinky/src/main.c b/project/slinky/src/main.c
> index f0c8f7a..dd2ffad 100755
> --- a/project/slinky/src/main.c
> +++ b/project/slinky/src/main.c
> @@ -85,35 +85,54 @@ uint8_t
> default_mbuf_mpool_data[DEFAULT_MBUF_MPOOL_BUF_LEN *
> struct os_mbuf_pool default_mbuf_pool;
> struct os_mempool default_mbuf_mpool;
>
> +static char *test_conf_get(int argc, char **argv, char *val, int max_len);
> +static int test_conf_set(int argc, char **argv, char *val);
> +static int test_conf_commit(void);
> +
> +static struct conf_handler test_conf_handler = {
> + .ch_name = "test",
> + .ch_get = test_conf_get,
> + .ch_set = test_conf_set,
> + .ch_commit = test_conf_commit
> +};
> +
> static uint8_t test8;
> +static uint8_t test8_shadow;
> static char test_str[32];
>
> -static const struct conf_entry test_conf_array[2] = {
> - [0] = {
> - .c_name = "8",
> - .c_type = CONF_INT8,
> - .c_val.single.val = &test8
> - },
> - [1] = {
> - .c_name = "str",
> - .c_type = CONF_STRING,
> - .c_val.array.val = test_str,
> - .c_val.array.maxlen = sizeof(test_str)
> +static char *
> +test_conf_get(int argc, char **argv, char *buf, int max_len)
> +{
> + if (argc == 1) {
> + if (!strcmp(argv[0], "8")) {
> + return conf_str_from_value(CONF_INT8, &test8, buf, max_len);
> + } else if (!strcmp(argv[0], "str")) {
> + return test_str;
> + }
> }
> -};
> -static struct conf_node test_conf_array_node = {
> - .cn_cnt = 2,
> - .cn_array = (struct conf_entry *)test_conf_array
> -};
> + return NULL;
> +}
>
> -static const struct conf_entry test_conf_dir = {
> - .c_name = "test",
> - .c_type = CONF_DIR
> -};
> -static struct conf_node test_conf_dir_node = {
> - .cn_cnt = 1,
> - .cn_array = (struct conf_entry *)&test_conf_dir
> -};
> +static int
> +test_conf_set(int argc, char **argv, char *val)
> +{
> + if (argc == 1) {
> + if (!strcmp(argv[0], "8")) {
> + return CONF_VALUE_SET(val, CONF_INT8, test8_shadow);
> + } else if (!strcmp(argv[0], "str")) {
> + return CONF_VALUE_SET(val, CONF_STRING, test_str);
> + }
> + }
> + return OS_ENOENT;
> +}
> +
> +static int
> +test_conf_commit(void)
> +{
> + test8 = test8_shadow;
> +
> + return 0;
> +}
>
> void
> task1_handler(void *arg)
> @@ -205,11 +224,8 @@ main(int argc, char **argv)
> mcu_sim_parse_args(argc, argv);
> #endif
>
> - conf_module_init();
> - rc = conf_register(NULL, &test_conf_dir_node);
> - assert(rc == 0);
> -
> - rc = conf_register(&test_conf_dir_node, &test_conf_array_node);
> + conf_init();
> + rc = conf_register(&test_conf_handler);
> assert(rc == 0);
>
> cbmem_init(&log_mem, log_buf, sizeof(log_buf));
>
> http://git-wip-us.apache.org/repos/asf/incubator-mynewt-larva/blob/ccda6d4b/sys/config/include/config/config.h
> ----------------------------------------------------------------------
> diff --git a/sys/config/include/config/config.h
> b/sys/config/include/config/config.h
> index 59b68b2..717f711 100644
> --- a/sys/config/include/config/config.h
> +++ b/sys/config/include/config/config.h
> @@ -37,39 +37,27 @@ enum conf_type {
> CONF_DOUBLE
> } __attribute__((__packed__));
>
> -struct conf_entry {
> - const char *c_name;
> - enum conf_type c_type;
> - union {
> - struct { /* INT8, INT16, INT32, INT64, FLOAT, DOUBLE */
> - void *val;
> - } single;
> - struct { /* STRING, BYTES */
> - uint16_t maxlen;
> - uint16_t len;
> - void *val;
> - } array;
> - } c_val;
> +struct conf_handler {
> + SLIST_ENTRY(conf_handler) ch_list;
> + char *ch_name;
> + char *(*ch_get)(int argc, char **argv, char *val, int val_len_max);
> + int (*ch_set)(int argc, char **argv, char *val);
> + int (*ch_commit)();
> };
>
> -struct conf_entry_dir {
> - const char *c_name;
> - enum conf_type c_type; /* DIR */
> -};
> -
> -struct conf_node {
> - SLIST_ENTRY(conf_node) cn_next;
> - SLIST_HEAD(, conf_node) cn_children;
> - struct conf_entry *cn_array;
> - int cn_cnt;
> -};
> +int conf_init(void);
> +int conf_register(struct conf_handler *);
> +int conf_load(void);
>
> -int conf_module_init(void);
> -int conf_register(struct conf_node *parent, struct conf_node *child);
> -struct conf_entry *conf_lookup(int argc, char **argv);
> +int conf_set_value(char *name, char *val_str);
> +char *conf_get_value(char *name, char *buf, int buf_len);
> +int conf_commit(char *name);
>
> -int conf_parse_name(char *name, int *name_argc, char *name_argv[]);
> -int conf_set_value(struct conf_entry *ce, char *val_str);
> -char *conf_get_value(struct conf_entry *ce, char *buf, int buf_len);
> +int conf_value_from_str(char *val_str, enum conf_type type, void *vp,
> + int maxlen);
> +char *conf_str_from_value(enum conf_type type, void *vp, char *buf,
> + int buf_len);
> +#define CONF_VALUE_SET(str, type, val) \
> + conf_value_from_str((str), (type), &(val), sizeof(val))
>
> #endif /* __UTIL_CONFIG_H_ */
>
> http://git-wip-us.apache.org/repos/asf/incubator-mynewt-larva/blob/ccda6d4b/sys/config/src/config.c
> ----------------------------------------------------------------------
> diff --git a/sys/config/src/config.c b/sys/config/src/config.c
> index d7f6d54..ff2b6ab 100644
> --- a/sys/config/src/config.c
> +++ b/sys/config/src/config.c
> @@ -17,77 +17,61 @@
> #include <string.h>
> #include <stdio.h>
>
> -#include "config/config.h"
> +#include <os/os.h>
>
> -#ifdef SHELL_PRESENT
> -#include <shell/shell.h>
> -#include <console/console.h>
> -#endif
> +#include "config/config.h"
> +#include "config_priv.h"
>
> -static struct conf_node g_conf_root;
> +static SLIST_HEAD(, conf_handler) conf_handlers =
> + SLIST_HEAD_INITIALIZER(&conf_handlers);
>
> -static struct conf_entry *
> -conf_node_match(struct conf_node *cn, const char *name)
> +int
> +conf_init(void)
> {
> - int i;
> + int rc = 0;
>
> - for (i = 0; i < cn->cn_cnt; i++) {
> - if (!strcmp(name, cn->cn_array[i].c_name)) {
> - return &cn->cn_array[i];
> - }
> - }
> - return NULL;
> +#ifdef SHELL_PRESENT
> + rc = conf_cli_register();
> +#endif
> +#ifdef NEWTMGR_PRESENT
> + rc = conf_nmgr_register();
> +#endif
> + return rc;
> }
>
> -/*
> - * Register config node to a specific spot in the tree.
> - */
> int
> -conf_register(struct conf_node *parent, struct conf_node *child)
> +conf_register(struct conf_handler *handler)
> {
> - struct conf_node *cn;
> - int i;
> -
> - if (!parent) {
> - parent = &g_conf_root;
> - }
> + SLIST_INSERT_HEAD(&conf_handlers, handler, ch_list);
> + return 0;
> +}
>
> - for (i = 0; i < child->cn_cnt; i++) {
> - SLIST_FOREACH(cn, &parent->cn_children, cn_next) {
> - if (conf_node_match(cn, child->cn_array[i].c_name)) {
> - return -1;
> - }
> - }
> - }
> - SLIST_INSERT_HEAD(&parent->cn_children, child, cn_next);
> +int
> +conf_load(void)
> +{
> + /*
> + * for every config source
> + * load config
> + * apply config
> + * commit all
> + */
> return 0;
> }
>
> /*
> - * Lookup conf_entry based on name.
> + * Find conf_handler based on name.
> */
> -struct conf_entry *
> -conf_lookup(int argc, char **argv)
> +struct conf_handler *
> +conf_handler_lookup(char *name)
> {
> - int i;
> - struct conf_node *parent = &g_conf_root;
> - struct conf_entry *ret = NULL;
> - struct conf_node *cn;
> -
> - for (i = 0; i < argc; i++) {
> - ret = NULL;
> - SLIST_FOREACH(cn, &parent->cn_children, cn_next) {
> - ret = conf_node_match(cn, argv[i]);
> - if (ret) {
> - break;
> - }
> - }
> - parent = cn;
> - if (!parent) {
> - return NULL;
> + struct conf_handler *ch;
> +
> + SLIST_FOREACH(ch, &conf_handlers, ch_list) {
> + if (!strcmp(name, ch->ch_name)) {
> + return ch;
> }
> }
> - return ret;
> + return NULL;
> }
>
> /*
> @@ -113,13 +97,25 @@ conf_parse_name(char *name, int *name_argc, char
> *name_argv[])
> return 0;
> }
>
> +static struct conf_handler *
> +conf_parse_and_lookup(char *name, int *name_argc, char *name_argv[])
> +{
> + int rc;
> +
> + rc = conf_parse_name(name, name_argc, name_argv);
> + if (rc) {
> + return NULL;
> + }
> + return conf_handler_lookup(name_argv[0]);
> +}
> +
> int
> -conf_set_value(struct conf_entry *ce, char *val_str)
> +conf_value_from_str(char *val_str, enum conf_type type, void *vp, int maxlen)
> {
> int32_t val;
> char *eptr;
>
> - switch (ce->c_type) {
> + switch (type) {
> case CONF_INT8:
> case CONF_INT16:
> case CONF_INT32:
> @@ -127,61 +123,53 @@ conf_set_value(struct conf_entry *ce, char *val_str)
> if (*eptr != '\0') {
> goto err;
> }
> - if (ce->c_type == CONF_INT8) {
> + if (type == CONF_INT8) {
> if (val < INT8_MIN || val > UINT8_MAX) {
> goto err;
> }
> - *(int8_t *)ce->c_val.single.val = val;
> - } else if (ce->c_type == CONF_INT16) {
> + *(int8_t *)vp = val;
> + } else if (type == CONF_INT16) {
> if (val < INT16_MIN || val > UINT16_MAX) {
> goto err;
> }
> - *(int16_t *)ce->c_val.single.val = val;
> - } else if (ce->c_type == CONF_INT32) {
> - *(int32_t *)ce->c_val.single.val = val;
> + *(int16_t *)vp = val;
> + } else if (type == CONF_INT32) {
> + *(int32_t *)vp = val;
> }
> break;
> case CONF_STRING:
> val = strlen(val_str);
> - if (val + 1 > ce->c_val.array.maxlen) {
> + if (val + 1 > maxlen) {
> goto err;
> }
> - strcpy(ce->c_val.array.val, val_str);
> - ce->c_val.array.len = val;
> + strcpy(vp, val_str);
> break;
> default:
> - console_printf("Can't parse type %d\n", ce->c_type);
> - break;
> + goto err;
> }
> return 0;
> err:
> - return -1;
> + return OS_INVALID_PARM;
> }
>
> -/*
> - * Get value in printable string form. If value is not string, the value
> - * will be filled in *buf.
> - * Return value will be pointer to beginning of that buffer,
> - * except for string it will pointer to beginning of string.
> - */
> char *
> -conf_get_value(struct conf_entry *ce, char *buf, int buf_len)
> +conf_str_from_value(enum conf_type type, void *vp, char *buf, int buf_len)
> {
> int32_t val;
>
> - if (ce->c_type == CONF_STRING) {
> - return ce->c_val.array.val;
> + if (type == CONF_STRING) {
> + return vp;
> }
> - switch (ce->c_type) {
> + switch (type) {
> case CONF_INT8:
> case CONF_INT16:
> case CONF_INT32:
> - if (ce->c_type == CONF_INT8) {
> - val = *(int8_t *)ce->c_val.single.val;
> - } else if (ce->c_type == CONF_INT16) {
> - val = *(int16_t *)ce->c_val.single.val;
> + if (type == CONF_INT8) {
> + val = *(int8_t *)vp;
> + } else if (type == CONF_INT16) {
> + val = *(int16_t *)vp;
> } else {
> - val = *(int32_t *)ce->c_val.single.val;
> + val = *(int32_t *)vp;
> }
> snprintf(buf, buf_len, "%ld", (long)val);
> return buf;
> @@ -190,3 +178,69 @@ conf_get_value(struct conf_entry *ce, char *buf, int
> buf_len)
> }
> }
>
> +int
> +conf_set_value(char *name, char *val_str)
> +{
> + int name_argc;
> + char *name_argv[CONF_MAX_DIR_DEPTH];
> + struct conf_handler *ch;
> +
> + ch = conf_parse_and_lookup(name, &name_argc, name_argv);
> + if (!ch) {
> + return OS_INVALID_PARM;
> + }
> +
> + return ch->ch_set(name_argc - 1, &name_argv[1], val_str);
> +}
> +
> +/*
> + * Get value in printable string form. If value is not string, the value
> + * will be filled in *buf.
> + * Return value will be pointer to beginning of that buffer,
> + * except for string it will pointer to beginning of string.
> + */
> +char *
> +conf_get_value(char *name, char *buf, int buf_len)
> +{
> + int name_argc;
> + char *name_argv[CONF_MAX_DIR_DEPTH];
> + struct conf_handler *ch;
> +
> + ch = conf_parse_and_lookup(name, &name_argc, name_argv);
> + if (!ch) {
> + return NULL;
> + }
> +
> + return ch->ch_get(name_argc - 1, &name_argv[1], buf, buf_len);
> +}
> +
> +int
> +conf_commit(char *name)
> +{
> + int name_argc;
> + char *name_argv[CONF_MAX_DIR_DEPTH];
> + struct conf_handler *ch;
> + int rc;
> +
> + if (name) {
> + ch = conf_parse_and_lookup(name, &name_argc, name_argv);
> + if (!ch) {
> + return OS_INVALID_PARM;
> + }
> + if (ch->ch_commit) {
> + return ch->ch_commit();
> + } else {
> + return 0;
> + }
> + } else {
> + SLIST_FOREACH(ch, &conf_handlers, ch_list) {
> + if (ch->ch_commit) {
> + rc = ch->ch_commit();
> + if (rc) {
> + return rc;
> + }
> + }
> + }
> + return 0;
> + }
> +}
>
> http://git-wip-us.apache.org/repos/asf/incubator-mynewt-larva/blob/ccda6d4b/sys/config/src/config_cli.c
> ----------------------------------------------------------------------
> diff --git a/sys/config/src/config_cli.c b/sys/config/src/config_cli.c
> index b0a2891..bfe1966 100644
> --- a/sys/config/src/config_cli.c
> +++ b/sys/config/src/config_cli.c
> @@ -32,15 +32,10 @@ shell_conf_command(int argc, char **argv)
> {
> char *name = NULL;
> char *val = NULL;
> - char *name_argv[CONF_MAX_DIR_DEPTH];
> char tmp_buf[16];
> - int name_argc;
> int rc;
> - struct conf_entry *ce;
>
> switch (argc) {
> - case 1:
> - break;
> case 2:
> name = argv[1];
> break;
> @@ -52,26 +47,25 @@ shell_conf_command(int argc, char **argv)
> goto err;
> }
>
> - rc = conf_parse_name(name, &name_argc, name_argv);
> - if (rc) {
> - goto err;
> - }
> -
> - ce = conf_lookup(name_argc, name_argv);
> - if (!ce) {
> - console_printf("No such config variable\n");
> - goto err;
> + if (!strcmp(name, "commit")) {
> + rc = conf_commit(val);
> + if (rc) {
> + val = "Failed to commit\n";
> + } else {
> + val = "Done\n";
> + }
> + console_printf(val);
> + return 0;
> }
> -
> if (!val) {
> - val = conf_get_value(ce, tmp_buf, sizeof(tmp_buf));
> + val = conf_get_value(name, tmp_buf, sizeof(tmp_buf));
> if (!val) {
> console_printf("Cannot display value\n");
> goto err;
> }
> - console_printf("%s", val);
> + console_printf("%s\n", val);
> } else {
> - rc = conf_set_value(ce, val);
> + rc = conf_set_value(name, val);
> if (rc) {
> console_printf("Failed to set\n");
> goto err;
> @@ -82,16 +76,11 @@ err:
> console_printf("Invalid args\n");
> return 0;
> }
> -#endif
>
> -int conf_module_init(void)
> +int
> +conf_cli_register(void)
> {
> -#ifdef SHELL_PRESENT
> - shell_cmd_register(&shell_conf_cmd, "config", shell_conf_command);
> -#endif
> -#ifdef NEWTMGR_PRESENT
> - conf_nmgr_register();
> -#endif
> - return 0;
> + return shell_cmd_register(&shell_conf_cmd, "config", shell_conf_command);
> }
> +#endif
>
>
> http://git-wip-us.apache.org/repos/asf/incubator-mynewt-larva/blob/ccda6d4b/sys/config/src/config_nmgr.c
> ----------------------------------------------------------------------
> diff --git a/sys/config/src/config_nmgr.c b/sys/config/src/config_nmgr.c
> index 8d0cf34..81004d8 100644
> --- a/sys/config/src/config_nmgr.c
> +++ b/sys/config/src/config_nmgr.c
> @@ -42,9 +42,6 @@ conf_nmgr_read(struct nmgr_jbuf *njb)
> {
> char tmp_str[max(CONF_MAX_DIR_DEPTH * 8, 16)];
> char *val_str;
> - char *name_argv[CONF_MAX_DIR_DEPTH];
> - int name_argc;
> - struct conf_entry *ce;
> const struct json_attr_t attr[2] = {
> [0] = {
> .attribute = "name",
> @@ -64,16 +61,7 @@ conf_nmgr_read(struct nmgr_jbuf *njb)
> return OS_EINVAL;
> }
>
> - if (conf_parse_name(tmp_str, &name_argc, name_argv)) {
> - return OS_EINVAL;
> - }
> -
> - ce = conf_lookup(name_argc, name_argv);
> - if (!ce) {
> - return OS_EINVAL;
> - }
> -
> - val_str = conf_get_value(ce, tmp_str, sizeof(tmp_str));
> + val_str = conf_get_value(tmp_str, tmp_str, sizeof(tmp_str));
> if (!val_str) {
> return OS_EINVAL;
> }
> @@ -89,17 +77,14 @@ conf_nmgr_read(struct nmgr_jbuf *njb)
> static int
> conf_nmgr_write(struct nmgr_jbuf *njb)
> {
> - char tmp_str[CONF_MAX_DIR_DEPTH * 8];
> + char name_str[CONF_MAX_DIR_DEPTH * 8];
> char val_str[256];
> - char *name_argv[CONF_MAX_DIR_DEPTH];
> - int name_argc;
> - struct conf_entry *ce;
> struct json_attr_t attr[3] = {
> [0] = {
> .attribute = "name",
> .type = t_string,
> - .addr.string = tmp_str,
> - .len = sizeof(tmp_str)
> + .addr.string = name_str,
> + .len = sizeof(name_str)
> },
> [1] = {
> .attribute = "val",
> @@ -118,16 +103,7 @@ conf_nmgr_write(struct nmgr_jbuf *njb)
> return OS_EINVAL;
> }
>
> - if (conf_parse_name(tmp_str, &name_argc, name_argv)) {
> - return OS_EINVAL;
> - }
> -
> - ce = conf_lookup(name_argc, name_argv);
> - if (!ce) {
> - return OS_EINVAL;
> - }
> -
> - rc = conf_set_value(ce, val_str);
> + rc = conf_set_value(name_str, val_str);
> if (rc) {
> return OS_EINVAL;
> }
>
> http://git-wip-us.apache.org/repos/asf/incubator-mynewt-larva/blob/ccda6d4b/sys/config/src/config_priv.h
> ----------------------------------------------------------------------
> diff --git a/sys/config/src/config_priv.h b/sys/config/src/config_priv.h
> index a457eb9..6217fee 100644
> --- a/sys/config/src/config_priv.h
> +++ b/sys/config/src/config_priv.h
> @@ -17,6 +17,7 @@
> #ifndef __CONFIG_PRIV_H_
> #define __CONFIG_PRIV_H_
>
> +int conf_cli_register(void);
> int conf_nmgr_register(void);
>
> #endif /* __CONFIG_PRIV_H_ */
>
> http://git-wip-us.apache.org/repos/asf/incubator-mynewt-larva/blob/ccda6d4b/sys/config/src/test/conf_test.c
> ----------------------------------------------------------------------
> diff --git a/sys/config/src/test/conf_test.c b/sys/config/src/test/conf_test.c
> index 5d0a4a0..8556275 100644
> --- a/sys/config/src/test/conf_test.c
> +++ b/sys/config/src/test/conf_test.c
> @@ -20,212 +20,154 @@
> #include <testutil/testutil.h>
> #include <config/config.h>
>
> -uint8_t val8;
> +static uint8_t val8;
>
> -static struct conf_entry ce1 = {
> - .c_name = "ce1",
> - .c_type = CONF_INT8,
> - .c_val.single.val = &val8
> -};
> -
> -static struct conf_node cn1 = {
> - .cn_cnt = 1,
> - .cn_array = &ce1
> -};
> -
> -static struct conf_entry ce2 = {
> - .c_name = "ce2",
> - .c_type = CONF_INT8,
> - .c_val.single.val = &val8
> -};
> -
> -static struct conf_node cn2 = {
> - .cn_cnt = 1,
> - .cn_array = &ce2
> -};
> +static int test_get_called;
> +static int test_set_called;
> +static int test_commit_called;
>
> -static struct conf_entry ce_arr1[2] = {
> - [0] = {
> - .c_name = "cea1",
> - .c_type = CONF_INT8,
> - .c_val.single.val = &val8
> - },
> - [1] = {
> - .c_name = "cea2",
> - .c_type = CONF_INT8,
> - .c_val.single.val = &val8
> +static char *
> +ctest_handle_get(int argc, char **argv, char *val, int val_len_max)
> +{
> + test_get_called = 1;
> + if (argc == 1 && !strcmp(argv[0], "mybar")) {
> + return conf_str_from_value(CONF_INT8, &val8, val, val_len_max);
> }
> -};
> + return NULL;
> +}
>
> -static struct conf_node cn_arr1 = {
> - .cn_cnt = 2,
> - .cn_array = ce_arr1
> -};
> +static int
> +ctest_handle_set(int argc, char **argv, char *val)
> +{
> + uint8_t newval;
> + int rc;
>
> -static struct conf_entry ce_arr2[2] = {
> - [0] = {
> - .c_name = "ce21",
> - .c_type = CONF_INT8,
> - .c_val.single.val = &val8
> - },
> - [1] = {
> - .c_name = "cea2",
> - .c_type = CONF_INT8,
> - .c_val.single.val = &val8
> + test_set_called = 1;
> + if (argc == 1 && !strcmp(argv[0], "mybar")) {
> + rc = CONF_VALUE_SET(val, CONF_INT8, newval);
> + TEST_ASSERT(rc == 0);
> + val8 = newval;
> + return 0;
> }
> -};
> -
> -static struct conf_node cn_arr2 = {
> - .cn_cnt = 2,
> - .cn_array = ce_arr2
> -};
> + return OS_ENOENT;
> +}
>
> -static struct conf_entry_dir ce_dir = {
> - .c_name = "foo",
> - .c_type = CONF_DIR
> -};
> +static int
> +ctest_handle_commit(void)
> +{
> + test_commit_called = 1;
> + return 0;
> +}
>
> -static struct conf_node cn_dir = {
> - .cn_cnt = 1,
> - .cn_array = (struct conf_entry *)&ce_dir
> +struct conf_handler config_test_handler = {
> + .ch_name = "myfoo",
> + .ch_get = ctest_handle_get,
> + .ch_set = ctest_handle_set,
> + .ch_commit = ctest_handle_commit
> };
>
> -static struct conf_entry ce_foo_arr1[2] = {
> - [0] = {
> - .c_name = "foo1",
> - .c_type = CONF_INT8,
> - .c_val.single.val = &val8
> - },
> - [1] = {
> - .c_name = "foo2",
> - .c_type = CONF_INT8,
> - .c_val.single.val = &val8
> - }
> -};
> +static void
> +ctest_clear_call_state(void)
> +{
> + test_get_called = 0;
> + test_set_called = 0;
> + test_commit_called = 0;
> +}
>
> -static struct conf_node cn_foo_arr1 = {
> - .cn_cnt = 2,
> - .cn_array = ce_foo_arr1
> -};
> +static int
> +ctest_get_call_state(void)
> +{
> + return test_get_called + test_set_called + test_commit_called;
> +}
>
> TEST_CASE(config_empty_lookups)
> {
> - struct conf_entry *ce;
> - char *names1[] = { "foo", "bar" };
> -
> - ce = conf_lookup(0, NULL);
> - TEST_ASSERT(ce == NULL);
> + int rc;
> + char tmp[64], *str;
>
> - ce = conf_lookup(1, names1);
> - TEST_ASSERT(ce == NULL);
> + rc = conf_set_value("foo/bar", "tmp");
> + TEST_ASSERT(rc != 0);
>
> - ce = conf_lookup(2, names1);
> - TEST_ASSERT(ce == NULL);
> + str = conf_get_value("foo/bar", tmp, sizeof(tmp));
> + TEST_ASSERT(str == NULL);
> }
>
> TEST_CASE(config_test_insert)
> {
> int rc;
>
> - /*
> - * Add 2 new ones
> - */
> - rc = conf_register(NULL, &cn1);
> + rc = conf_register(&config_test_handler);
> TEST_ASSERT(rc == 0);
> - rc = conf_register(NULL, &cn2);
> - TEST_ASSERT(rc == 0);
> -
> - /*
> - * Fail adding same ones again
> - */
> - rc = conf_register(NULL, &cn1);
> - TEST_ASSERT(rc != 0);
> - rc = conf_register(NULL, &cn2);
> - TEST_ASSERT(rc != 0);
> -
> - /*
> - * Add with multiple conf_entries
> - */
> - rc = conf_register(NULL, &cn_arr1);
> - TEST_ASSERT(rc == 0);
> -
> - /*
> - * Cannot add it again
> - */
> - rc = conf_register(NULL, &cn_arr1);
> - TEST_ASSERT(rc != 0);
> -
> - /*
> - * Should fail right away
> - */
> - rc = conf_register(NULL, &cn_arr2);
> - TEST_ASSERT(rc != 0);
> -
> }
>
> -TEST_CASE(config_test_lookup)
> +TEST_CASE(config_test_getset_unknown)
> {
> - struct conf_entry *ce;
> - char *names1[] = { "foo", "bar" };
> - char *names2[] = { "ce1" };
> - char *names3[] = { "cea2" };
> -
> - ce = conf_lookup(0, NULL);
> - TEST_ASSERT(ce == NULL);
> + char tmp[64], *str;
> + int rc;
>
> - ce = conf_lookup(1, names1);
> - TEST_ASSERT(ce == NULL);
> + rc = conf_set_value("foo/bar", "tmp");
> + TEST_ASSERT(rc != 0);
> + TEST_ASSERT(ctest_get_call_state() == 0);
>
> - ce = conf_lookup(2, names1);
> - TEST_ASSERT(ce == NULL);
> + str = conf_get_value("foo/bar", tmp, sizeof(tmp));
> + TEST_ASSERT(str == NULL);
> + TEST_ASSERT(ctest_get_call_state() == 0);
>
> - ce = conf_lookup(1, names2);
> - TEST_ASSERT(ce != NULL);
> - TEST_ASSERT(!strcmp(ce->c_name, names2[0]));
> + rc = conf_set_value("myfoo/bar", "tmp");
> + TEST_ASSERT(rc == OS_ENOENT);
> + TEST_ASSERT(test_set_called == 1);
> + ctest_clear_call_state();
>
> - ce = conf_lookup(1, names3);
> - TEST_ASSERT(ce != NULL);
> - TEST_ASSERT(!strcmp(ce->c_name, names3[0]));
> + str = conf_get_value("myfoo/bar", tmp, sizeof(tmp));
> + TEST_ASSERT(str == NULL);
> + TEST_ASSERT(test_get_called == 1);
> + ctest_clear_call_state();
> }
>
> -TEST_CASE(config_test_dir)
> +TEST_CASE(config_test_getset_int)
> {
> + char tmp[64], *str;
> int rc;
> - struct conf_entry *ce;
> - char *names1[] = { "foo", "foo1" };
> - char *names2[] = { "foo", "foo2" };
> - char *names3[] = { "foo", "foo3" };
> -
> - /*
> - * Add directory node, and node under.
> - */
> - rc = conf_register(NULL, &cn_dir);
> - TEST_ASSERT(rc == 0);
> - rc = conf_register(&cn_dir, &cn_foo_arr1);
> +
> + rc = conf_set_value("myfoo/mybar", "42");
> TEST_ASSERT(rc == 0);
> + TEST_ASSERT(test_set_called == 1);
> + TEST_ASSERT(val8 == 42);
> + ctest_clear_call_state();
> +
> + str = conf_get_value("myfoo/mybar", tmp, sizeof(tmp));
> + TEST_ASSERT(str);
> + TEST_ASSERT(test_get_called == 1);
> + TEST_ASSERT(!strcmp("42", tmp));
> + ctest_clear_call_state();
> +}
>
> - ce = conf_lookup(1, names1);
> - TEST_ASSERT(ce != NULL);
> - TEST_ASSERT(ce->c_type == CONF_DIR);
> +TEST_CASE(config_test_commit)
> +{
> + int rc;
>
> - ce = conf_lookup(2, names1);
> - TEST_ASSERT(ce != NULL);
> - TEST_ASSERT(!strcmp(ce->c_name, names1[1]));
> + rc = conf_commit("bar");
> + TEST_ASSERT(rc == 0);
> + TEST_ASSERT(ctest_get_call_state());
>
> - ce = conf_lookup(2, names2);
> - TEST_ASSERT(ce != NULL);
> - TEST_ASSERT(!strcmp(ce->c_name, names2[1]));
> + rc = conf_commit(NULL);
> + TEST_ASSERT(rc == 0);
> + TEST_ASSERT(test_commit_called == 1);
> + ctest_clear_call_state();
>
> - ce = conf_lookup(2, names3);
> - TEST_ASSERT(ce == NULL);
> + rc = conf_commit("myfoo");
> + TEST_ASSERT(rc == 0);
> + TEST_ASSERT(test_commit_called == 1);
> + ctest_clear_call_state();
> }
>
> TEST_SUITE(config_test_suite)
> {
> config_empty_lookups();
> config_test_insert();
> - config_test_lookup();
> - config_test_dir();
> + config_test_getset_unknown();
> + config_test_getset_int();
> + config_test_commit();
> }
>
> #ifdef PKG_TEST
> @@ -236,7 +178,7 @@ main(int argc, char **argv)
> tu_config.tc_print_results = 1;
> tu_init();
>
> - config_test_suite();
> + conf_init();
>
> return tu_any_failed;
> }
>
