From 291dd250cdc9a1d91c69b21479cb6a930aa6f8aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tja=C5=BE=20Vra=C4=8Dko?= Date: Fri, 18 Aug 2023 12:22:15 +0200 Subject: [PATCH 1/3] Add totos --- library/protocol/binary/README.md | 14 ++++++++++++++ .../executor/user_settings_protocol_executor.c | 9 ++++++--- tests/protocol_binary/src/main.c | 11 ++++++++++- 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/library/protocol/binary/README.md b/library/protocol/binary/README.md index 3c5f60d..2678e55 100644 --- a/library/protocol/binary/README.md +++ b/library/protocol/binary/README.md @@ -69,6 +69,20 @@ For example, to set the default value of setting with id 13 to 10 (the setting i A valid restore command is encoded as `07`. +TODO: maybe list some and list some full should be removed? We should implement support for multiple commands in a single buffer in the executor instead. + +## LIST SOME (0x08) + +A valid list some command is encoded as [1 byte command (0x08), 1 byte number of settings (N), N * 2 byte setting ID]. + +Each setting is encoded separately as specified in the GET command. + +## LIST SOME FULL (0x09) + +A valid list some full command is encoded as [1 byte command (0x09), 1 byte number of settings (N), N * 2 byte setting ID]. + +Each setting is encoded separately as specified in the GET FULL command. + ## Additional examples The following list gives a settings description (in text), its short (GET) and full (GET FULL) encoding. diff --git a/library/protocol/executor/user_settings_protocol_executor.c b/library/protocol/executor/user_settings_protocol_executor.c index a54217b..319575f 100644 --- a/library/protocol/executor/user_settings_protocol_executor.c +++ b/library/protocol/executor/user_settings_protocol_executor.c @@ -161,9 +161,12 @@ static int prv_exec_restore(void) } /* will this return the number of bytes parsed? negative error code otherwise? This way you can have - * a buffer holding multiple commands in a row and this will always parse 1 command ant tell teh - * user where it finished */ -int usp_executor_parse_and_execute(struct usp_executor *usp_executor, uint8_t *buffer, size_t len) + * a buffer holding multiple commands in a row and this will always parse 1 command ant tell the + * user where it finished + * + * TODO: Implement the idea in the above comment */ +int usp_executor_parse_and_execute(struct usp_executor *usp_executor, uint8_t *buffer, size_t len, + void *user_data) { int ret; struct user_settings_protocol_command cmd = {0}; diff --git a/tests/protocol_binary/src/main.c b/tests/protocol_binary/src/main.c index d0e1e6f..430e2ec 100644 --- a/tests/protocol_binary/src/main.c +++ b/tests/protocol_binary/src/main.c @@ -347,4 +347,13 @@ ZTEST(protocol_binary_suite, test_user_setting_encode_correct_bytes_full) zassert_mem_equal(&buffer[11], default_value, sizeof(default_value), "default value should be here"); zassert_equal(buffer[13], us.max_size, "max size should be here"); -} \ No newline at end of file +} + +/* TODO: test list_some commands */ +/* TODO: test that the number of bytes decoded is correct for each command */ + +/* TODO: add multi-command support to executor. Since decode returns number of bytes decoded, we + * could have a loop and move that number of bytes in the buffer and decode+execute a new command + * + * Add tests for all of the new stuff + */ From 8be1625064e77aadcd429ed6aafca439b71196d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tja=C5=BE=20Vra=C4=8Dko?= Date: Tue, 2 Apr 2024 12:08:56 +0200 Subject: [PATCH 2/3] Add CONFIG_USER_SETTINGS_DEFAULT_OVERWRITE Kconfig option If set, the default values of settings can be changed by calling user_settings_set_default_with_key. The tests have been updated to test this new feature. --- CHANGELOG.md | 4 +++ library/Kconfig | 7 +++++ library/include/user_settings.h | 7 ++++- library/user_settings/user_settings.c | 4 +-- tests/user_settings/src/main.c | 45 ++++++++++++++++++++++++--- tests/user_settings/testcase.yaml | 13 ++++++++ 6 files changed, 73 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c7772fa..ceb48b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) ## [Unreleased] +### Added + +- The CONFIG_USER_SETTINGS_DEFAULT_OVERWRITE option. If set, default values can be overwritten. + ## [1.6.0] - 2023-05-12 ### Added diff --git a/library/Kconfig b/library/Kconfig index 307c4e8..0086d32 100644 --- a/library/Kconfig +++ b/library/Kconfig @@ -26,6 +26,13 @@ config USER_SETTINGS_JSON depends on CJSON_LIB default false +config USER_SETTINGS_DEFAULT_OVERWRITE + bool "Allow default values to be overwritten" + default false + help + If enabled, user_settings_set_default_with_key() will no longer return an + error if a default already exists, but will replace it. + module = USER_SETTINGS module-str = User settings source "subsys/logging/Kconfig.template.log_config" diff --git a/library/include/user_settings.h b/library/include/user_settings.h index 67b94c1..5524650 100644 --- a/library/include/user_settings.h +++ b/library/include/user_settings.h @@ -95,12 +95,17 @@ int user_settings_load(void); * If the key input for this function is unknown to the application (i.e. parsed from user), then * it should first be checked with user_settings_exists_with_key(). * + * @note If overwriting default values is required, set CONFIG_USER_SETTINGS_DEFAULT_OVERWRITE=y. + * This can be used in use-cases where the default values are set form within the application, and a + * firmware update can change the default values. In that case, -EALREADY is never returned. + * * @param[in] key The key of the setting to set * @param[in] data The default value * @param[in] len The length of the default value (in bytes) * * @retval 0 on success - * @retval -EALREADY if the default value has already been set + * @retval -EALREADY if the default value has already been set. Only returned if + * CONFIG_USER_SETTINGS_DEFAULT_OVERWRITE=n. * @retval -ENOMEM if len is >= then the max_size specified when adding the setting with * user_settings_add() or user_settings_add_sized() * @retval -EIO if the setting value could not be stored to NVS diff --git a/library/user_settings/user_settings.c b/library/user_settings/user_settings.c index 0999f90..ccb0422 100644 --- a/library/user_settings/user_settings.c +++ b/library/user_settings/user_settings.c @@ -267,7 +267,7 @@ static int prv_user_settings_set_default(struct user_setting *s, void *data, siz } /* check if default already set */ - if (s->default_is_set) { + if (!IS_ENABLED(CONFIG_USER_SETTINGS_DEFAULT_OVERWRITE) && s->default_is_set) { LOG_ERR("Default already set for setting %s. Not setting new default. Clear NVS " "first if you wish to change the default.", s->key); @@ -790,4 +790,4 @@ bool user_settings_any_changed(void) } } return false; -} \ No newline at end of file +} diff --git a/tests/user_settings/src/main.c b/tests/user_settings/src/main.c index e4d561c..530cc9d 100644 --- a/tests/user_settings/src/main.c +++ b/tests/user_settings/src/main.c @@ -4,6 +4,8 @@ #include #include +#define NUM_SETTINGS 5 + static void *user_settings_suite_setup(void) { user_settings_init(); @@ -13,6 +15,7 @@ static void *user_settings_suite_setup(void) user_settings_add(2, "t2", USER_SETTINGS_TYPE_U32); user_settings_add(3, "t3", USER_SETTINGS_TYPE_I8); user_settings_add_sized(4, "t4", USER_SETTINGS_TYPE_STR, 10); + user_settings_add(5, "t5", USER_SETTINGS_TYPE_U32); user_settings_load(); @@ -51,7 +54,7 @@ ZTEST(user_settings_suite, test_settings_exist) ZTEST(user_settings_suite, test_nonexistand_settings_dont_exist) { - zassert_equal(user_settings_exists_with_id(5), false, "Setting should not exist"); + zassert_equal(user_settings_exists_with_id(0), false, "Setting should not exist"); zassert_equal(user_settings_exists_with_key("t0"), false, "Setting should not exist"); } @@ -130,6 +133,35 @@ ZTEST(user_settings_suite, test_settings_default_value) zassert_equal(value_out, value, "Value should be unchanged"); } +ZTEST(user_settings_suite, test_settings_default_value_twice) +{ + int err; + + /* Set default value for t5 */ + uint32_t default_value = 69; + + err = user_settings_set_default_with_id(5, &default_value, sizeof(default_value)); + zassert_ok(err, "set default should not error here"); + + /* Set default with same value should not error */ + err = user_settings_set_default_with_id(5, &default_value, sizeof(default_value)); + zassert_ok(err, "set default should not error here"); + + /* Set default with new value should error if CONFIG_USER_SETTINGS_DEFAULT_OVERWRITE=n. + * + * NOTE: If running these test with Twister, both cases will be tested. + * The command is: east twister -T . -p native_posix + */ + uint32_t new_default_value = 70; + err = user_settings_set_default_with_id(5, &new_default_value, sizeof(new_default_value)); + + if (IS_ENABLED(CONFIG_USER_SETTINGS_DEFAULT_OVERWRITE)) { + zassert_ok(err, "set default should not error here, err: %d", err); + } else { + zassert_equal(err, -EALREADY, "set default should error here, err: %d", err); + } +} + ZTEST(user_settings_suite, test_settings_restore_one) { /* set value */ @@ -257,7 +289,7 @@ ZTEST(user_settings_suite, test_settings_iter_correct_count) n_settings++; zassert_equal(id, n_settings, "wrong id"); } - zassert_equal(n_settings, 4, "number of settings should be 4"); + zassert_equal(n_settings, NUM_SETTINGS, "number of settings should be %d", NUM_SETTINGS); } ZTEST(user_settings_suite, test_settings_iter_correct_key_and_id) @@ -290,7 +322,12 @@ ZTEST(user_settings_suite, test_settings_iter_correct_key_and_id) zassert_equal(id, 4, "Id should be 4, was %d", id); zassert_ok(strcmp(key, "t4"), "Key should be t4, was: %s", key); - /* Since we have 4 settings, we should get NULL here */ + ret = user_settings_iter_next(&key, &id); + zassert_true(ret, "Return value should be true"); + zassert_equal(id, 5, "Id should be 5, was %d", id); + zassert_ok(strcmp(key, "t5"), "Key should be t5, was: %s", key); + + /* Since we have 5 settings, we should get NULL here */ ret = user_settings_iter_next(&key, &id); zassert_false(ret, "Return value should be false"); } @@ -450,4 +487,4 @@ ZTEST(user_settings_suite, test_settings_user_settings_any_changed) * NOT TESTED: * * - assertions when getting/setting nonexistant settings - */ \ No newline at end of file + */ diff --git a/tests/user_settings/testcase.yaml b/tests/user_settings/testcase.yaml index 39ec96c..df52f5d 100644 --- a/tests/user_settings/testcase.yaml +++ b/tests/user_settings/testcase.yaml @@ -1,3 +1,16 @@ tests: user_settings.user_settings: platform_allow: native_posix + extra_configs: + # Disable fancy test, otherwise stdout parsing does not work. + - CONFIG_FANCY_ZTEST=n + - CONFIG_TEST_LOGGING_DEFAULTS=n + - CONFIG_ASSERT=n + user_settings.user_settings_with_default_override: + platform_allow: native_posix + extra_configs: + # Disable fancy test, otherwise stdout parsing does not work. + - CONFIG_FANCY_ZTEST=n + - CONFIG_TEST_LOGGING_DEFAULTS=n + - CONFIG_ASSERT=n + - CONFIG_USER_SETTINGS_DEFAULT_OVERWRITE=y From 6be36212babdcbc9ab286e4c5597fe3550e6713d Mon Sep 17 00:00:00 2001 From: "github-bot :robot" Date: Thu, 4 Apr 2024 13:38:24 +0000 Subject: [PATCH 3/3] Update CHANGELOG.md for Release v1.7.0 --- CHANGELOG.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ceb48b5..c243116 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,9 +6,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) ## [Unreleased] +## [1.7.0] - 2024-04-04 + ### Added -- The CONFIG_USER_SETTINGS_DEFAULT_OVERWRITE option. If set, default values can be overwritten. +- The CONFIG_USER_SETTINGS_DEFAULT_OVERWRITE option. If set, default values can be overwritten. ## [1.6.0] - 2023-05-12 @@ -103,7 +105,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Basic sample. - Callbacks sample. -[Unreleased]: https://github.com/IRNAS/irnas-usersettings-lib/compare/v1.6.0...HEAD +[Unreleased]: https://github.com/IRNAS/irnas-usersettings-lib/compare/v1.7.0...HEAD + +[1.7.0]: https://github.com/IRNAS/irnas-usersettings-lib/compare/v1.6.0...v1.7.0 [1.6.0]: https://github.com/IRNAS/irnas-usersettings-lib/compare/v1.5.0...v1.6.0