diff --git a/ROMFS/px4fmu_common/init.d/rcS b/ROMFS/px4fmu_common/init.d/rcS index b6d517b45c2..8e44fecca3d 100644 --- a/ROMFS/px4fmu_common/init.d/rcS +++ b/ROMFS/px4fmu_common/init.d/rcS @@ -195,10 +195,15 @@ else param load /fs/mtd_caldata fi + # Load parameters from storage. A blank store (first boot, or after + # switching firmware) is seeded from the backup or defaults and persisted + # by 'load-or-init' itself - that is not an error and does not beep. A + # corrupt or unwritable store, or a backup that fails to import, returns + # failure. param select $PARAM_FILE - if ! param import + if ! param load-or-init $PARAM_BACKUP_FILE then - echo "ERROR [init] param import failed" + echo "ERROR [init] param load-or-init failed" set STARTUP_TUNE 2 # tune 2 = ERROR_TUNE bsondump $PARAM_FILE @@ -219,10 +224,13 @@ else # dump current backup file contents for comparison bsondump $PARAM_BACKUP_FILE - param import $PARAM_BACKUP_FILE - - # overwrite invalid $PARAM_FILE with backup - cp $PARAM_BACKUP_FILE $PARAM_FILE + # only persist when the backup actually imported, so a + # corrupt backup keeps the storage invalid and the + # operator keeps being alerted on the next boot + if param import $PARAM_BACKUP_FILE + then + param save + fi fi param status diff --git a/src/lib/parameters/flashparams/flashfs.c b/src/lib/parameters/flashparams/flashfs.c index 97847fa1a9d..0c18da2facf 100644 --- a/src/lib/parameters/flashparams/flashfs.c +++ b/src/lib/parameters/flashparams/flashfs.c @@ -782,6 +782,35 @@ int parameter_flashfs_read(flash_file_token_t token, uint8_t **buffer, size_t return rv; } +/**************************************************************************** + * Name: parameter_flashfs_blank + * + * Description: + * Reports whether the parameter storage is fully erased, so callers can + * tell a blank store from one that holds unparseable data. + * + ****************************************************************************/ + +int parameter_flashfs_blank(void) +{ + if (sector_map == NULL) { + return -ENXIO; + } + + for (int s = 0; sector_map[s].address; s++) { + uint32_t *pm = (uint32_t *) sector_map[s].address; + uint32_t *pe = pm + (sector_map[s].size / sizeof(uint32_t)); + + while (pm < pe) { + if (!blank_flash(pm++)) { + return 0; + } + } + } + + return 1; +} + /**************************************************************************** * Name: parameter_flashfs_write * @@ -1074,6 +1103,12 @@ int parameter_flashfs_init(sector_descriptor_t *fconfig, uint8_t *buffer, uint16 /* No paramates and no free space => neeed erase */ rv = parameter_flashfs_erase(); + + /* A positive return value means flash space has been erased successfully */ + + if (rv > 0) { + rv = 0; + } } } diff --git a/src/lib/parameters/flashparams/flashfs.h b/src/lib/parameters/flashparams/flashfs.h index c2f3db4f627..840935b0555 100644 --- a/src/lib/parameters/flashparams/flashfs.h +++ b/src/lib/parameters/flashparams/flashfs.h @@ -156,6 +156,24 @@ __EXPORT int parameter_flashfs_init(sector_descriptor_t *fconfig, uint8_t *buffe __EXPORT int parameter_flashfs_read(flash_file_token_t ft, uint8_t **buffer, size_t *buf_size); +/**************************************************************************** + * Name: parameter_flashfs_blank + * + * Description: + * Reports whether the parameter storage is fully erased. This lets callers + * tell a never-written store (first boot, or after switching firmware) from + * one that holds data but no valid entry (torn write, bit-rot, or foreign + * data) - both of which surface as -ENOENT from parameter_flashfs_read. + * + * Returned value: + * 1 if every configured sector is fully erased + * 0 if any byte is programmed + * A negative errno if the flashfs has not been initialized + * + ****************************************************************************/ + +__EXPORT int parameter_flashfs_blank(void); + /**************************************************************************** * Name: parameter_flashfs_write * diff --git a/src/lib/parameters/flashparams/flashfs32.c b/src/lib/parameters/flashparams/flashfs32.c index 25e97d25631..ae8994d8096 100644 --- a/src/lib/parameters/flashparams/flashfs32.c +++ b/src/lib/parameters/flashparams/flashfs32.c @@ -853,6 +853,35 @@ int parameter_flashfs_read(flash_file_token_t token, uint8_t **buffer, size_t return rv; } +/**************************************************************************** + * Name: parameter_flashfs_blank + * + * Description: + * Reports whether the parameter storage is fully erased, so callers can + * tell a blank store from one that holds unparseable data. + * + ****************************************************************************/ + +int parameter_flashfs_blank(void) +{ + if (sector_map == NULL) { + return -ENXIO; + } + + for (int s = 0; sector_map[s].address; s++) { + uint32_t *pm = (uint32_t *) sector_map[s].address; + uint32_t *pe = pm + (sector_map[s].size / sizeof(uint32_t)); + + while (pm < pe) { + if (!blank_flash(pm++)) { + return 0; + } + } + } + + return 1; +} + /**************************************************************************** * Name: parameter_flashfs_write * diff --git a/src/lib/parameters/flashparams/flashparams.cpp b/src/lib/parameters/flashparams/flashparams.cpp index 198bb7b2ff8..ba055dd5bdb 100644 --- a/src/lib/parameters/flashparams/flashparams.cpp +++ b/src/lib/parameters/flashparams/flashparams.cpp @@ -255,9 +255,28 @@ param_import_internal() bson_decoder_s decoder{}; int result = -1; - uint8_t *buffer = 0; - size_t buf_size; - parameter_flashfs_read(parameters_token, &buffer, &buf_size); + uint8_t *buffer = nullptr; + size_t buf_size = 0; + int read_result = parameter_flashfs_read(parameters_token, &buffer, &buf_size); + + if (read_result == -ENOENT || (read_result >= 0 && (buffer == nullptr || buf_size == 0))) { + /* No valid entry found. A fully erased store is blank (first boot, or + * after switching firmware): report "not yet stored" (1), matching the + * convention used by param_load_default(). A store that holds data but + * no valid entry (torn write, bit-rot, or foreign data) is corrupt and + * must not be silently reseeded - report it so the boot recovery runs. */ + if (parameter_flashfs_blank() == 1) { + return 1; + } + + debug("flash holds data but no valid entry"); + return -EILSEQ; + } + + if (read_result < 0) { + debug("flash read failed (%d)", read_result); + return read_result; + } if (bson_decoder_init_buf(&decoder, buffer, buf_size, param_import_callback)) { debug("decoder init failed"); diff --git a/src/lib/parameters/param.h b/src/lib/parameters/param.h index 16a3ec34b2f..4bb73da3ff9 100644 --- a/src/lib/parameters/param.h +++ b/src/lib/parameters/param.h @@ -359,8 +359,9 @@ __EXPORT int param_export(const char *filename, 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). - * @return Zero on success, nonzero if an error occurred during import. - * Note that in the failure case, parameters may be inconsistent. + * @return Zero on success, 1 if the source is blank (nothing stored + * yet), negative if an error occurred during import. Note that + * in the error case, parameters may be inconsistent. */ __EXPORT int param_import(int fd); @@ -371,8 +372,10 @@ __EXPORT int param_import(int fd); * values from a file. * * @param fd File descriptor to import from (-1 selects the FLASH storage). - * @return Zero on success, nonzero if an error occurred during import. - * Note that in the failure case, parameters may be inconsistent. + * @return Zero on success, 1 if the source is blank (all parameters + * were reset to defaults), negative if an error occurred during + * import. Note that in the error case, parameters may be + * inconsistent. */ __EXPORT int param_load(int fd); diff --git a/src/lib/parameters/parameters.cpp b/src/lib/parameters/parameters.cpp index 1508fcb4a5d..3aa317fdcfc 100644 --- a/src/lib/parameters/parameters.cpp +++ b/src/lib/parameters/parameters.cpp @@ -50,6 +50,7 @@ #include #include #include +#include #include #include @@ -920,6 +921,11 @@ param_load_default() int result = param_load(fd_load); ::close(fd_load); + if (result == 1) { + /* blank file: nothing stored yet */ + return 1; + } + if (result != 0) { PX4_ERR("error reading parameters from '%s'", filename); return -2; @@ -1226,6 +1232,35 @@ param_import_callback(bson_decoder_t decoder, bson_node_t node) static int param_import_internal(int fd) { + /* blank source (nothing stored yet): an empty file, or zeroed/erased + * storage where the BSON document length would be. Unlike the flash + * backend, which scans the whole store, this only inspects the leading + * document length - a length of 0 or -1 can never start a valid BSON + * document, so nothing parseable is diverted here regardless of what + * follows. */ + + /* A zero-length regular file is blank. NuttX FAT can't read() a 0-byte + * file (no cluster chain -> error, not EOF), so the content check below + * would miss it; detect it by size. Char-device backends (e.g. FRAM) + * report no meaningful size and fall through to the content check. */ + struct stat st; + + if (fstat(fd, &st) == 0 && S_ISREG(st.st_mode) && st.st_size == 0) { + return 1; + } + + int32_t doc_size = 0; + ssize_t len = read(fd, &doc_size, sizeof(doc_size)); + + if (len == 0 || (len == (ssize_t)sizeof(doc_size) && (doc_size == 0 || doc_size == -1))) { + return 1; + } + + if (lseek(fd, 0, SEEK_SET) != 0) { + PX4_ERR("import lseek failed (%d)", errno); + return -1; + } + static constexpr int MAX_ATTEMPTS = 3; for (int attempt = 1; attempt <= MAX_ATTEMPTS; attempt++) { diff --git a/src/modules/commander/worker_thread.cpp b/src/modules/commander/worker_thread.cpp index 9859da498b3..bccbc635be0 100644 --- a/src/modules/commander/worker_thread.cpp +++ b/src/modules/commander/worker_thread.cpp @@ -150,6 +150,13 @@ void WorkerThread::threadEntry() case Request::ParamLoadDefault: _ret_value = param_load_default(); + // blank storage (1) is not an error: all parameters were reset to defaults + if (_ret_value == 1) { + mavlink_log_warning(&_mavlink_log_pub, "Blank storage, parameters reset to default\t"); + events::send(events::ID("commander_load_param_blank"), events::Log::Warning, "Blank storage, parameters reset to default"); + _ret_value = 0; + } + if (_ret_value != 0) { mavlink_log_critical(&_mavlink_log_pub, "Error loading settings\t"); events::send(events::ID("commander_load_param_failed"), events::Log::Critical, "Error loading settings"); diff --git a/src/systemcmds/param/param.cpp b/src/systemcmds/param/param.cpp index f9bd28365c2..f74365bc58e 100644 --- a/src/systemcmds/param/param.cpp +++ b/src/systemcmds/param/param.cpp @@ -78,6 +78,7 @@ static int do_save(const char *param_file_name); static int do_save_default(); static int do_load(const char *param_file_name); static int do_import(const char *param_file_name = nullptr); +static int do_load_or_init(const char *backup_file_name); static int do_show(const char *search_string, bool only_changed); static int do_show_for_airframe(); static int do_show_all(); @@ -127,6 +128,9 @@ $ reboot PRINT_MODULE_USAGE_ARG("", "File name (use default if not given)", true); PRINT_MODULE_USAGE_COMMAND_DESCR("import", "Import params from a file"); PRINT_MODULE_USAGE_ARG("", "File name (use default if not given)", true); + PRINT_MODULE_USAGE_COMMAND_DESCR("load-or-init", + "Load params from storage; if blank, seed from a backup file or defaults and persist"); + PRINT_MODULE_USAGE_ARG("", "Backup file to seed from when storage is blank", true); PRINT_MODULE_USAGE_COMMAND_DESCR("save", "Save params to a file"); PRINT_MODULE_USAGE_ARG("", "File name (use default if not given)", true); @@ -224,6 +228,15 @@ param_main(int argc, char *argv[]) } } + if (!strcmp(argv[1], "load-or-init")) { + if (argc >= 3) { + return do_load_or_init(argv[2]); + + } else { + return do_load_or_init(nullptr); + } + } + if (!strcmp(argv[1], "select")) { if (argc >= 3) { param_set_default_file(argv[2]); @@ -511,6 +524,105 @@ do_import(const char *param_file_name) return 0; } +/** + * Load parameters from the default storage, initializing it if blank. + * + * A blank storage (nothing stored yet - e.g. first boot, or after switching + * firmware) is not an error: the parameters are seeded from the backup file + * if one is given and readable, otherwise the firmware defaults are kept, + * and the result is persisted so the storage is valid on the next boot. + * + * @return 0 if parameters were loaded or the storage was initialized, + * 1 if the storage is corrupt, a backup exists but could not be + * imported, or the result could not be persisted + */ +static int +do_load_or_init(const char *backup_file_name) +{ + /* resolve the storage like 'param import' does: the selected default + * file, or the flash backend when none is set */ + const char *param_file_name = param_get_default_file(); + int result; + + if (param_file_name) { + int storage_fd = open(param_file_name, O_RDONLY); + + if (storage_fd >= 0) { + result = param_import(storage_fd); + close(storage_fd); + + } else if (errno == ENOENT) { + /* no file yet */ + result = 1; + + } else { + PX4_ERR("open '%s' failed (%i)", param_file_name, errno); + result = -1; + } + + } else { + result = param_import(-1); + } + + if (result == 0) { + PX4_INFO("parameters loaded from storage"); + return 0; + } + + if (result < 0) { + PX4_ERR("parameter storage is corrupt (%i)", result); + return 1; + } + + /* result > 0: storage is blank, nothing has been stored yet */ + PX4_INFO("parameter storage empty, initializing"); + + int fd = -1; + + if (backup_file_name) { + fd = open(backup_file_name, O_RDONLY); + + if (fd < 0 && errno != ENOENT) { + /* a backup exists but cannot be read: report failure so the boot + * recovery alerts the operator instead of silently persisting + * defaults over it */ + PX4_ERR("open backup '%s' failed (%i)", backup_file_name, errno); + return 1; + } + } + + if (fd >= 0) { + PX4_INFO("seeding parameters from backup '%s'", backup_file_name); + + int backup_result = param_import(fd); + close(fd); + + if (backup_result < 0) { + /* a backup exists but does not import cleanly: report failure so + * the boot recovery alerts the operator. Do not reset to defaults + * - that would also discard parameters loaded before this call + * (e.g. factory calibration from a caldata partition). */ + PX4_ERR("backup import failed (%i)", backup_result); + return 1; + } + + if (backup_result > 0) { + PX4_INFO("backup is blank, using firmware defaults"); + } + + } else { + PX4_INFO("no backup file, using firmware defaults"); + } + + if (do_save_default() != 0) { + PX4_ERR("failed to persist initial parameters"); + return 1; + } + + PX4_INFO("parameter storage initialized"); + return 0; +} + static int do_save_default() { diff --git a/src/systemcmds/tests/test_parameters.cpp b/src/systemcmds/tests/test_parameters.cpp index 77b45ce55e8..d223f8eb95a 100644 --- a/src/systemcmds/tests/test_parameters.cpp +++ b/src/systemcmds/tests/test_parameters.cpp @@ -82,6 +82,8 @@ private: bool ResetAllExcludesWildcard(); bool CustomDefaults(); bool exportImport(); + bool blankImport(); + bool corruptImport(); // tests on system parameters // WARNING, can potentially trash your system @@ -237,6 +239,10 @@ bool ParameterTest::CustomDefaults() param_get_default_value(param_test_1, &default_value); ut_compare("value for param default doesn't match default value", default_value, 2); + // custom defaults persist in RAM until reboot; remember the firmware + // default so it can be restored and the test stays repeatable in one boot + const int32_t original_default_value = default_value; + // change default value int32_t new_default_value = 123456789; param_set_default_value(param_test_1, &new_default_value); @@ -265,6 +271,11 @@ bool ParameterTest::CustomDefaults() param_get(param_test_1, &value); ut_compare("param value not reset to custom default", value, 123456789); + // restore the firmware default and value so a second run in the same boot + // starts from the same state + param_set_default_value(param_test_1, &original_default_value); + param_reset(param_test_1); + return true; } @@ -395,6 +406,102 @@ bool ParameterTest::exportImport() return ret; } +bool ParameterTest::blankImport() +{ + // a blank source (empty file, zeroed or erased storage) imports as "not yet stored" (1) + const char *blank_file_name = PX4_STORAGEDIR "/param_blank"; + + const struct { + const char *label; + uint8_t fill; + size_t size; + } blank_cases[] = { + {"empty file", 0x00, 0}, + {"zeroed storage", 0x00, 64}, + {"erased storage", 0xff, 64}, + }; + + for (const auto &c : blank_cases) { + int fd = open(blank_file_name, O_WRONLY | O_CREAT | O_TRUNC, 0666); + + if (fd < 0) { + PX4_ERR("creating '%s' failed (%i)", blank_file_name, errno); + return false; + } + + for (size_t i = 0; i < c.size; i++) { + if (write(fd, &c.fill, 1) != 1) { + PX4_ERR("writing '%s' failed (%i)", blank_file_name, errno); + close(fd); + return false; + } + } + + close(fd); + + fd = open(blank_file_name, O_RDONLY); + + if (fd < 0) { + PX4_ERR("open '%s' failed (%i)", blank_file_name, errno); + return false; + } + + int result = param_import(fd); + close(fd); + + if (result != 1) { + PX4_ERR("import of %s returned %d, expected 1", c.label, result); + unlink(blank_file_name); + return false; + } + } + + unlink(blank_file_name); + return true; +} + +bool ParameterTest::corruptImport() +{ + // a source that holds data but no valid BSON document fails to import + const char *corrupt_file_name = PX4_STORAGEDIR "/param_corrupt"; + + // garbage document length followed by an unsupported node type + const uint8_t garbage[] = {0xde, 0xad, 0xbe, 0x6f, 0x42, 0x13, 0x37, 0x00}; + + int fd = open(corrupt_file_name, O_WRONLY | O_CREAT | O_TRUNC, 0666); + + if (fd < 0) { + PX4_ERR("creating '%s' failed (%i)", corrupt_file_name, errno); + return false; + } + + if (write(fd, garbage, sizeof(garbage)) != (ssize_t)sizeof(garbage)) { + PX4_ERR("writing '%s' failed (%i)", corrupt_file_name, errno); + close(fd); + return false; + } + + close(fd); + + fd = open(corrupt_file_name, O_RDONLY); + + if (fd < 0) { + PX4_ERR("open '%s' failed (%i)", corrupt_file_name, errno); + return false; + } + + int result = param_import(fd); + close(fd); + unlink(corrupt_file_name); + + if (result >= 0) { + PX4_ERR("import of corrupt file returned %d, expected failure", result); + return false; + } + + return true; +} + bool ParameterTest::exportImportAll() { static constexpr float MAGIC_FLOAT_VAL = 0.217828f; @@ -582,6 +689,8 @@ bool ParameterTest::run_tests() ut_run_test(ResetAllExcludesWildcard); ut_run_test(CustomDefaults); ut_run_test(exportImport); + ut_run_test(blankImport); + ut_run_test(corruptImport); // WARNING, can potentially trash your system #ifdef __PX4_POSIX