parameters: simplify import mark_unsaved and don't fail bson decode unnecessarily

src/lib/parameters/flashparams/flashparams.cpp

@@ -187,18 +187,14 @@ out:
 	return result;
 }
 
-struct param_import_state {
-	bool mark_saved;
-};
 
 static int
-param_import_callback(bson_decoder_t decoder, void *priv, bson_node_t node)
+param_import_callback(bson_decoder_t decoder, bson_node_t node)
 {
 	float f;
 	int32_t i;
 	void *v = nullptr;
 	int result = -1;
-	struct param_import_state *state = (struct param_import_state *)priv;
 
 	/*
 	 * EOO means the end of the parameter object. (Currently not supporting
@@ -255,7 +251,7 @@ param_import_callback(bson_decoder_t decoder, void *priv, bson_node_t node)
 		goto out;
 	}
 
-	if (param_set_external(param, v, state->mark_saved, true)) {
+	if (param_set_external(param, v, true, true)) {
 		debug("error setting value for '%s'", node->name);
 		goto out;
 	}
@@ -268,23 +264,20 @@ out:
 }
 
 static int
-param_import_internal(bool mark_saved)
+param_import_internal()
 {
 	bson_decoder_s decoder{};
 	int result = -1;
-	struct param_import_state state;
 
 	uint8_t *buffer = 0;
 	size_t buf_size;
 	parameter_flashfs_read(parameters_token, &buffer, &buf_size);
 
-	if (bson_decoder_init_buf(&decoder, buffer, buf_size, param_import_callback, &state)) {
+	if (bson_decoder_init_buf(&decoder, buffer, buf_size, param_import_callback)) {
 		debug("decoder init failed");
 		goto out;
 	}
 
-	state.mark_saved = mark_saved;
-
 	do {
 		result = bson_decoder_next(&decoder);
 
@@ -307,10 +300,10 @@ int flash_param_save(param_filter_func filter)
 int flash_param_load()
 {
 	param_reset_all();
-	return param_import_internal(true);
+	return param_import_internal();
 }
 
 int flash_param_import()
 {
-	return param_import_internal(true);
+	return param_import_internal();
 }

src/lib/parameters/param.h

@@ -330,11 +330,10 @@ __EXPORT int		param_export(int fd, param_filter_func filter);
  * This function merges the imported parameters with the current parameter set.
  *
  * @param fd		File descriptor to import from (-1 selects the FLASH storage).
- * @param mark_saved	Whether to mark imported parameters as already saved
  * @return		Zero on success, nonzero if an error occurred during import.
  *			Note that in the failure case, parameters may be inconsistent.
  */
-__EXPORT int		param_import(int fd, bool mark_saved);
+__EXPORT int		param_import(int fd);
 
 /**
  * Load parameters from a file.

src/lib/parameters/parameters.cpp

@@ -1222,7 +1222,7 @@ int param_save_default()
 	} else {
 		param_lock_writer();
 		perf_begin(param_export_perf);
-		res = flash_param_save(false, nullptr);
+		res = flash_param_save(nullptr);
 		params_unsaved.reset();
 		perf_end(param_export_perf);
 		param_unlock_writer();
@@ -1402,19 +1402,9 @@ out:
 	return result;
 }
 
-struct param_import_state {
-	bool mark_saved;
-};
-
 static int
-param_import_callback(bson_decoder_t decoder, void *priv, bson_node_t node)
+param_import_callback(bson_decoder_t decoder, bson_node_t node)
 {
-	float f = 0.0f;
-	int32_t i = 0;
-	void *v = nullptr;
-	int result = -1;
-	param_import_state *state = (param_import_state *)priv;
-
 	/*
 	 * EOO means the end of the parameter object. (Currently not supporting
 	 * nested BSON objects).
@@ -1426,10 +1416,7 @@ param_import_callback(bson_decoder_t decoder, void *priv, bson_node_t node)
 
 	param_modify_on_import(node);
 
-	/*
+	// Find the parameter this node represents.  If we don't know it, ignore the node.
-	 * Find the parameter this node represents.  If we don't know it,
-	 * ignore the node.
-	 */
 	param_t param = param_find_no_notification(node->name);
 
 	if (param == PARAM_INVALID) {
@@ -1437,59 +1424,42 @@ param_import_callback(bson_decoder_t decoder, void *priv, bson_node_t node)
 		return 1;
 	}
 
-	/*
+	// Handle setting the parameter from the node
-	 * Handle setting the parameter from the node
-	 */
-
 	switch (node->type) {
 	case BSON_INT32: {
-			if (param_type(param) != PARAM_TYPE_INT32) {
+			if (param_type(param) == PARAM_TYPE_INT32) {
+				int32_t i = node->i32;
+				param_set_internal(param, &i, true, true);
+				PX4_DEBUG("Imported %s with value %d", param_name(param), i);
+
+			} else {
 				PX4_WARN("unexpected type for %s", node->name);
-				result = 1; // just skip this entry
-				goto out;
 			}
-
-			i = node->i32;
-			v = &i;
-
-			PX4_DEBUG("Imported %s with value %d", param_name(param), i);
 		}
 		break;
 
 	case BSON_DOUBLE: {
-			if (param_type(param) != PARAM_TYPE_FLOAT) {
+			if (param_type(param) == PARAM_TYPE_FLOAT) {
+				float f = node->d;
+				param_set_internal(param, &f, true, true);
+				PX4_DEBUG("Imported %s with value %f", param_name(param), (double)f);
+
+			} else {
 				PX4_WARN("unexpected type for %s", node->name);
-				result = 1; // just skip this entry
-				goto out;
 			}
-
-			f = node->d;
-			v = &f;
-
-			PX4_DEBUG("Imported %s with value %f", param_name(param), (double)f);
 		}
 		break;
 
 	default:
-		PX4_ERR("%s unrecognised node type %d", node->name, node->type);
+		PX4_ERR("import: unrecognised node type for '%s'", node->name);
-		result = 1; // just skip this entry
-		goto out;
 	}
 
-	if (param_set_internal(param, v, state->mark_saved, true)) {
+	// don't return zero, that means EOF
-		PX4_DEBUG("error setting value for '%s'", node->name);
+	return 1;
-		goto out;
-	}
-
-	/* don't return zero, that means EOF */
-	result = 1;
-
-out:
-	return result;
 }
 
 static int
-param_dump_callback(bson_decoder_t decoder, void *priv, bson_node_t node)
+param_dump_callback(bson_decoder_t decoder, bson_node_t node)
 {
 	switch (node->type) {
 	case BSON_EOO:
@@ -1521,15 +1491,12 @@ param_dump_callback(bson_decoder_t decoder, void *priv, bson_node_t node)
 }
 
 static int
-param_import_internal(int fd, bool mark_saved)
+param_import_internal(int fd)
 {
 	for (int attempt = 1; attempt < 5; attempt++) {
 		bson_decoder_s decoder{};
-		param_import_state state;
-
-		if (bson_decoder_init_file(&decoder, fd, param_import_callback, &state) == 0) {
-			state.mark_saved = mark_saved;
 
+		if (bson_decoder_init_file(&decoder, fd, param_import_callback) == 0) {
 			int result = -1;
 
 			do {
@@ -1564,13 +1531,13 @@ param_import_internal(int fd, bool mark_saved)
 }
 
 int
-param_import(int fd, bool mark_saved)
+param_import(int fd)
 {
 	if (fd < 0) {
 		return flash_param_import();
 	}
 
-	return param_import_internal(fd, mark_saved);
+	return param_import_internal(fd);
 }
 
 int
@@ -1581,16 +1548,15 @@ param_load(int fd)
 	}
 
 	param_reset_all_internal(false);
-	return param_import_internal(fd, true);
+	return param_import_internal(fd);
 }
 
 int
 param_dump(int fd)
 {
 	bson_decoder_s decoder{};
-	param_import_state state;
 
-	if (bson_decoder_init_file(&decoder, fd, param_dump_callback, &state) == 0) {
+	if (bson_decoder_init_file(&decoder, fd, param_dump_callback) == 0) {
 		PX4_INFO_RAW("BSON document size %" PRId32 "\n", decoder.total_document_size);
 
 		int result = -1;

src/lib/parameters/tinybson/tinybson.cpp

@@ -115,11 +115,10 @@ read_double(bson_decoder_t decoder, double *d)
 }
 
 int
-bson_decoder_init_file(bson_decoder_t decoder, int fd, bson_decoder_callback callback, void *priv)
+bson_decoder_init_file(bson_decoder_t decoder, int fd, bson_decoder_callback callback)
 {
 	decoder->fd = fd;
 	decoder->callback = callback;
-	decoder->priv = priv;
 	decoder->nesting = 1;
 	decoder->node.type = BSON_UNDEFINED;
 
@@ -135,8 +134,7 @@ bson_decoder_init_file(bson_decoder_t decoder, int fd, bson_decoder_callback cal
 }
 
 int
-bson_decoder_init_buf(bson_decoder_t decoder, void *buf, unsigned bufsize, bson_decoder_callback callback,
+bson_decoder_init_buf(bson_decoder_t decoder, void *buf, unsigned bufsize, bson_decoder_callback callback)
-		      void *priv)
 {
 	/* argument sanity */
 	if ((buf == nullptr) || (callback == nullptr)) {
@@ -157,7 +155,6 @@ bson_decoder_init_buf(bson_decoder_t decoder, void *buf, unsigned bufsize, bson_
 
 	decoder->bufpos = 0;
 	decoder->callback = callback;
-	decoder->priv = priv;
 	decoder->nesting = 1;
 	decoder->pending = 0;
 	decoder->node.type = BSON_UNDEFINED;
@@ -318,7 +315,7 @@ bson_decoder_next(bson_decoder_t decoder)
 	}
 
 	/* call the callback and pass its results back */
-	return decoder->callback(decoder, decoder->priv, &decoder->node);
+	return decoder->callback(decoder, &decoder->node);
 }
 
 int

src/lib/parameters/tinybson/tinybson.h

@@ -101,7 +101,7 @@ typedef struct bson_decoder_s *bson_decoder_t;
  *
  * The node callback function's return value is returned by bson_decoder_next.
  */
-typedef int	(* bson_decoder_callback)(bson_decoder_t decoder, void *priv, bson_node_t node);
+typedef int	(* bson_decoder_callback)(bson_decoder_t decoder, bson_node_t node);
 
 struct bson_decoder_s {
 	/* file reader state */
@@ -114,7 +114,6 @@ struct bson_decoder_s {
 
 	bool			dead{false};
 	bson_decoder_callback	callback;
-	void			*priv{nullptr};
 	unsigned		nesting{0};
 	struct bson_node_s	node {};
 	int32_t			pending{0};
@@ -136,10 +135,9 @@ struct bson_decoder_s {
  * @param decoder		Decoder state structure to be initialised.
  * @param fd			File to read BSON data from.
  * @param callback		Callback to be invoked by bson_decoder_next
- * @param priv          Callback private data, stored in node.
  * @return			Zero on success.
  */
-__EXPORT int bson_decoder_init_file(bson_decoder_t decoder, int fd, bson_decoder_callback callback, void *priv);
+__EXPORT int bson_decoder_init_file(bson_decoder_t decoder, int fd, bson_decoder_callback callback);
 
 /**
  * Initialise the decoder to read from a buffer in memory.
@@ -150,11 +148,9 @@ __EXPORT int bson_decoder_init_file(bson_decoder_t decoder, int fd, bson_decoder
  *				passed as zero if the buffer size should be extracted from the
  *				BSON header only.
  * @param callback		Callback to be invoked by bson_decoder_next
- * @param priv		Callback private data, stored in node.
  * @return			Zero on success.
  */
-__EXPORT int bson_decoder_init_buf(bson_decoder_t decoder, void *buf, unsigned bufsize, bson_decoder_callback callback,
+__EXPORT int bson_decoder_init_buf(bson_decoder_t decoder, void *buf, unsigned bufsize, bson_decoder_callback callback);
-				   void *priv);
 
 /**
  * Process the next node from the stream and invoke the callback.

src/systemcmds/param/param.cpp

@@ -529,11 +529,8 @@ do_load(const char *param_file_name)
 static int
 do_import(const char *param_file_name)
 {
-	bool mark_saved = false;
-
 	if (param_file_name == nullptr) {
 		param_file_name = param_get_default_file();
-		mark_saved = true; // if imported from default storage, mark as saved
 	}
 
 	int fd = -1;
@@ -549,7 +546,7 @@ do_import(const char *param_file_name)
 		PX4_INFO("importing from '%s'", param_file_name);
 	}
 
-	int result = param_import(fd, mark_saved);
+	int result = param_import(fd);
 
 	if (fd >= 0) {
 		close(fd);

src/systemcmds/tests/test_bson.cpp

@@ -96,7 +96,7 @@ encode(bson_encoder_t encoder)
 }
 
 static int
-decode_callback(bson_decoder_t decoder, void *priv, bson_node_t node)
+decode_callback(bson_decoder_t decoder, bson_node_t node)
 {
 	unsigned len;
 
@@ -287,7 +287,7 @@ test_bson(int argc, char *argv[])
 	}
 
 	/* now test-decode it */
-	if (bson_decoder_init_buf(&decoder, buf, len, decode_callback, nullptr)) {
+	if (bson_decoder_init_buf(&decoder, buf, len, decode_callback)) {
 		PX4_ERR("FAIL: bson_decoder_init_buf");
 		return 1;
 	}

src/systemcmds/tests/test_parameters.cpp

@@ -561,7 +561,7 @@ bool ParameterTest::exportImportAll()
 		return false;
 	}
 
-	result = param_import(fd, false);
+	result = param_import(fd);
 	close(fd);
 
 	if (result < 0) {