Merge pull request #57 from IRNAS/release/v1.7.0

Release v1.7.0

CHANGELOG.md

@@ -6,6 +6,12 @@ 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.
+
 ## [1.6.0] - 2023-05-12
 
 ### Added
@@ -99,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
 

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"

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

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.

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};

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;
-}
+}

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");
-}
+}
+
+/* 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
+ */

tests/user_settings/src/main.c

@@ -4,6 +4,8 @@
 #include <zephyr/ztest.h>
 #include <zephyr/ztest_error_hook.h>
 
+#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
- */
+ */

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