From 290b49fcdf57c9e1b26a44a77535f0b020667929 Mon Sep 17 00:00:00 2001 From: AsiiaPine Date: Wed, 7 Aug 2024 12:20:35 +0300 Subject: [PATCH] adapted functions and addressing for redundant paging --- libparams/storage.c | 107 +++++++++----------- libparams/storage.h | 11 -- platform_specific/ubuntu/YamlParameters.cpp | 25 ++--- platform_specific/ubuntu/YamlParameters.hpp | 2 +- platform_specific/ubuntu/flash_driver.cpp | 8 +- tests/params/temp_params_0.yaml | 5 + tests/unit_tests/CMakeLists.txt | 44 ++------ 7 files changed, 78 insertions(+), 124 deletions(-) create mode 100644 tests/params/temp_params_0.yaml diff --git a/libparams/storage.c b/libparams/storage.c index 3b69485..9737691 100644 --- a/libparams/storage.c +++ b/libparams/storage.c @@ -22,9 +22,10 @@ static ParamIndex_t strings_amount = 0; static ParamIndex_t all_params_amount = 0; static bool _isCorrectStringParamIndex(ParamIndex_t param_idx); -static uint32_t _getStringMemoryPoolAddress(); -static int8_t _save(); -static int8_t _save_redundant(RomDriverInstance* rom_driver); +static uint32_t _getStringMemoryPoolAddress(RomDriverInstance* rom_driver); +static int8_t _save(RomDriverInstance* rom_driver); +static int8_t _chooseRom(); +static int8_t _redundantRomInit(); #define INT_POOL_SIZE integers_amount * sizeof(IntegerParamValue_t) #define STR_POOL_SIZE MAX_STRING_LENGTH * strings_amount @@ -56,23 +57,14 @@ int8_t paramsInit(ParamIndex_t int_num, integers_amount = int_num; strings_amount = str_num; all_params_amount = integers_amount + strings_amount; - paramsInitRedundantPage(); - paramsChooseRom(); - return LIBPARAMS_OK; -} - -int8_t paramsInitRedundantPage() { - redundant_rom = - romInit(rom.first_page_idx -rom.pages_amount, rom.pages_amount); - if (!redundant_rom.inited) { - return LIBPARAMS_UNKNOWN_ERROR; - } + _redundantRomInit(); + _chooseRom(); return LIBPARAMS_OK; } int8_t paramsLoad() { romRead(&rom, 0, (uint8_t*)integer_values_pool, INT_POOL_SIZE); - romRead(&rom, _getStringMemoryPoolAddress(), (uint8_t*)&string_values_pool, STR_POOL_SIZE); + romRead(&rom, _getStringMemoryPoolAddress(&rom), (uint8_t*)&string_values_pool, STR_POOL_SIZE); for (uint_fast16_t idx = 0; idx < integers_amount; idx++) { IntegerParamValue_t val = integer_values_pool[idx]; @@ -85,50 +77,30 @@ int8_t paramsLoad() { // 255 value is default value for stm32, '\0' for ubuntu if (string_values_pool[idx][0] == 255 || string_values_pool[idx][0] == '\0') { memcpy(string_values_pool[idx], string_desc_pool[idx].def, MAX_STRING_LENGTH); + } else { + break; } } return LIBPARAMS_OK; } -int8_t paramsLoadRom(RomDriverInstance rom_instance) { - romRead(&rom_instance, 0, (uint8_t*)integer_values_pool, INT_POOL_SIZE); - romRead(&rom_instance, _getStringMemoryPoolAddress(), +int8_t paramsLoadRom(RomDriverInstance* rom_instance) { + romRead(rom_instance, 0, (uint8_t*)integer_values_pool, INT_POOL_SIZE); + romRead(rom_instance, _getStringMemoryPoolAddress(rom_instance), (uint8_t*)&string_values_pool, STR_POOL_SIZE); for (uint_fast16_t idx = 0; idx < integers_amount; idx++) { IntegerParamValue_t val = integer_values_pool[idx]; if (val < integer_desc_pool[idx].min || val > integer_desc_pool[idx].max) { - integer_values_pool[idx] = integer_desc_pool[idx].def; - rom_instance.erased = true; + rom_instance->erased = true; + return LIBPARAMS_OK; } } - for (uint_fast16_t idx = 0; idx < strings_amount; idx++) { - // 255 value is default value for stm32, '\0' for ubuntu - if (string_values_pool[idx][0] == 255 || string_values_pool[idx][0] == '\0') { - memcpy(string_values_pool[idx], string_desc_pool[idx].def, MAX_STRING_LENGTH); - } - } - - return LIBPARAMS_OK; -} - -int8_t paramsChooseRom() { - paramsLoadRom(rom); - paramsLoadRom(redundant_rom); - RomDriverInstance buffer; - if (rom.erased) { - if (!redundant_rom.erased) { - buffer = redundant_rom; - redundant_rom = rom; - rom = buffer; - } - } return LIBPARAMS_OK; } - int8_t paramsSave() { if (all_params_amount == 0) { return LIBPARAMS_NOT_INITIALIZED; @@ -137,7 +109,7 @@ int8_t paramsSave() { if (redundant_rom.inited) { // write params to redundant rom romBeginWrite(&redundant_rom); - res = _save_redundant(&redundant_rom); + res = _save(&redundant_rom); romEndWrite(&redundant_rom); // erase rom if save was successful if (res == 0) { @@ -151,7 +123,7 @@ int8_t paramsSave() { } romBeginWrite(&rom); rom.erased = true; - res = _save(); + res = _save(&rom); romEndWrite(&rom); return res; } @@ -162,7 +134,7 @@ int8_t paramsResetToDefault() { } for (ParamIndex_t idx = 0; idx < integers_amount; idx++) { - if (!integer_desc_pool[idx].is_required) { + if (!integer_desc_pool[idx].is_required || integer_desc_pool[idx].is_mutable) { integer_values_pool[idx] = integer_desc_pool[idx].def; } } @@ -291,8 +263,8 @@ static bool _isCorrectStringParamIndex(ParamIndex_t param_idx) { return param_idx < integers_amount || param_idx >= all_params_amount; } -static uint32_t _getStringMemoryPoolAddress() { - return romGetAvailableMemory(&rom) - MAX_STRING_LENGTH * strings_amount; +static uint32_t _getStringMemoryPoolAddress(RomDriverInstance* rom_driver) { + return romGetAvailableMemory(rom_driver) - MAX_STRING_LENGTH * strings_amount; } /** @@ -300,33 +272,48 @@ static uint32_t _getStringMemoryPoolAddress() { * An error means either a library internal error or the provided flash driver is incorrect. * If such errir is detected, stop writing immediately to avoid doing something wrong. */ -static int8_t _save() { +static int8_t _save(RomDriverInstance* rom_driver) { if (INT_POOL_SIZE != 0 && - 0 == romWrite(&rom, 0, (uint8_t*)integer_values_pool, INT_POOL_SIZE)) { + 0 == romWrite(rom_driver, 0, (uint8_t*)integer_values_pool, INT_POOL_SIZE)) { return LIBPARAMS_UNKNOWN_ERROR; } - size_t offset = _getStringMemoryPoolAddress(); + size_t offset = _getStringMemoryPoolAddress(rom_driver); if (STR_POOL_SIZE != 0 && - 0 == romWrite(&rom, offset, (uint8_t*)string_values_pool, STR_POOL_SIZE)) { + 0 == romWrite(rom_driver, offset, (uint8_t*)string_values_pool, STR_POOL_SIZE)) { return LIBPARAMS_UNKNOWN_ERROR; } return LIBPARAMS_OK; } - -static int8_t _save_redundant(RomDriverInstance* rom_driver) { - if (INT_POOL_SIZE != 0 && - 0 == romWrite(rom_driver, 0, (uint8_t*)integer_values_pool, INT_POOL_SIZE)) { +/** + * @brief Initialize the redundant parameters pages if the rom page_idx was 256, then the redundant pages will be allocated at (256 - rom.pages_num idx). Call this on paramsSave() to backup parameters. + * @return LIBPARAMS_OK on success, otherwise < 0. + */ +int8_t _redundantRomInit() { + redundant_rom = + romInit(rom.first_page_idx - rom.pages_amount, rom.pages_amount); + if (!redundant_rom.inited) { return LIBPARAMS_UNKNOWN_ERROR; } + return LIBPARAMS_OK; +} - size_t offset = _getStringMemoryPoolAddress(); - if (STR_POOL_SIZE != 0 && - 0 == romWrite(rom_driver, offset, (uint8_t*)string_values_pool, STR_POOL_SIZE)) { - return LIBPARAMS_UNKNOWN_ERROR; +/** + * @brief Choose a rom which addreses a non-erased part of flash memory + * **/ +int8_t _chooseRom() { + paramsLoadRom(&rom); + paramsLoadRom(&redundant_rom); + RomDriverInstance buffer; + if (rom.erased) { + if (!redundant_rom.erased) { + buffer = redundant_rom; + redundant_rom = rom; + rom = buffer; + } } - rom_driver->erased = false; return LIBPARAMS_OK; } + diff --git a/libparams/storage.h b/libparams/storage.h index 51f30ab..6a16ba7 100644 --- a/libparams/storage.h +++ b/libparams/storage.h @@ -84,22 +84,11 @@ typedef uint16_t ParamIndex_t; int8_t paramsInit(ParamIndex_t int_num, ParamIndex_t str_num, int32_t first_page_idx, size_t pages_num); -/** - * @brief Initialize the redundant parameters pages if the rom page_idx was 256, then the redundant pages are allocated at 256 - rom.pages_num idx. Call this on paramsSave() to backup parameters. - * @return LIBPARAMS_OK on success, otherwise < 0. - */ -int8_t paramsInitRedundantPage(); -/** - * @brief Choose a rom from redundant rom and main rom. - * **/ -int8_t paramsChooseRom(); - /** * @brief Load parameters from a persistent memory: flash for stm32 and file for ubuntu. * @return LIBPARAMS_OK on success, otherwise < 0. */ int8_t paramsLoad(); -// int8_t paramsLoadRom(RomDriverInstance rom); /** * @brief Save parameters to a persistent memory: flash for stm32 and file for ubuntu. diff --git a/platform_specific/ubuntu/YamlParameters.cpp b/platform_specific/ubuntu/YamlParameters.cpp index 2865d37..77d3e58 100644 --- a/platform_specific/ubuntu/YamlParameters.cpp +++ b/platform_specific/ubuntu/YamlParameters.cpp @@ -63,7 +63,7 @@ int8_t YamlParameters::read_from_dir(const std::string& path) { uint16_t int_param_idx = 0; uint16_t str_param_idx = 0; // read params values for each page - for (uint8_t idx = 0; idx < flash.num_pages; idx++) { + for (uint16_t idx = 0; idx < flash.num_pages; idx++) { std::ifstream params_storage_file; // check if temp file for the page already exists, else read from init file @@ -112,7 +112,7 @@ int8_t YamlParameters::write_to_dir(const std::string& path) { // remember last written indexes uint16_t int_param_idx = 0; uint16_t str_param_idx = 0; - for (uint8_t idx = 0; idx < flash.num_pages; idx++) { + for (uint16_t idx = 0; idx < flash.num_pages; idx++) { snprintf(file_name, sizeof(file_name), "%s/%s_%d.yaml", path.c_str(), temp_file_name.c_str(), idx); std::ofstream params_storage_file; @@ -148,7 +148,7 @@ int8_t YamlParameters::__read_page(std::ifstream& params_storage_file, uint16_t* } value = line.substr(delimiter_pos + 1); try { - if ((*int_param_idx > params.num_int_params) || (*int_param_idx == 512)) { + if ((*int_param_idx) > params.num_int_params) { logger.error("Got more integer params than defined by num_int_params"); return LIBPARAMS_WRONG_ARGS; } @@ -160,17 +160,18 @@ int8_t YamlParameters::__read_page(std::ifstream& params_storage_file, uint16_t* memcpy((void*)(flash.memory_ptr + 4 * (*int_param_idx)), &int_value, 4); *int_param_idx = *int_param_idx + 1; } catch (std::invalid_argument const& ex) { - if ((*str_param_idx == 512) || (*str_param_idx >= params.num_str_params)) { - logger.error("Wrong num_str_params\n"); + uint32_t offset = flash.flash_size - MAX_STRING_LENGTH * + (params.num_str_params - (*str_param_idx )); + if ((*str_param_idx) >= params.num_str_params) { + logger.error("Wrong num_str_params expected: ", params.num_str_params, + " got: ", (*str_param_idx)); return LIBPARAMS_WRONG_ARGS; } - int offset = flash.flash_size - MAX_STRING_LENGTH * - int(params.num_str_params - (*str_param_idx )); size_t quote_pos = value.find('"'); size_t quote_end_pos = value.find('"', quote_pos + 1); std::string str_value = value.substr(quote_pos + 1, quote_end_pos - quote_pos - 1); - if (offset < int(*int_param_idx) * 4) { + if (offset < (*int_param_idx) * 4) { logger.error("params overlap last int param addr", *int_param_idx * 4, ", str param offset ", offset); return LIBPARAMS_WRONG_ARGS; @@ -194,9 +195,9 @@ int8_t YamlParameters::__write_page(std::ofstream& params_storage_file, uint16_t } uint32_t n_bytes = 0; - uint8_t param_idx = *int_param_idx; + uint16_t param_idx = *int_param_idx; - for (uint8_t index = param_idx; index < params.num_int_params; index++) { + for (uint16_t index = param_idx; index < params.num_int_params; index++) { int32_t int_param_value; const char* name = params.integer_desc_pool[index].name; memcpy(&int_param_value, flash.memory_ptr + index * 4, 4); @@ -205,7 +206,7 @@ int8_t YamlParameters::__write_page(std::ofstream& params_storage_file, uint16_t logger.info(std::left, std::setw(32), name, ":\t", int_param_value); n_bytes += 4; *int_param_idx = *int_param_idx + 1; - if (n_bytes +4 > flash.page_size) { + if (n_bytes + 4 > flash.page_size) { return LIBPARAMS_OK; } } @@ -218,7 +219,7 @@ int8_t YamlParameters::__write_page(std::ofstream& params_storage_file, uint16_t if (available_str_params < str_params_remained) { last_str_param_idx = prev_str_idx + available_str_params; } - for (uint8_t index = prev_str_idx; index < last_str_param_idx; index++) { + for (uint16_t index = prev_str_idx; index < last_str_param_idx; index++) { const char* name = params.string_desc_pool[index].name; if (name == nullptr) { return LIBPARAMS_OK; diff --git a/platform_specific/ubuntu/YamlParameters.hpp b/platform_specific/ubuntu/YamlParameters.hpp index cba2f05..a0db837 100644 --- a/platform_specific/ubuntu/YamlParameters.hpp +++ b/platform_specific/ubuntu/YamlParameters.hpp @@ -15,12 +15,12 @@ static SimpleLogger logger("YamlParameters"); class YamlParameters { - FlashMemoryLayout_t flash; ParametersLayout_t params; std::string init_file_name = "init_params"; std::string temp_file_name = "temp_params"; public: + FlashMemoryLayout_t flash; explicit YamlParameters(FlashMemoryLayout_t flash_desc, ParametersLayout_t params_desc); int8_t read_from_dir(const std::string& path); diff --git a/platform_specific/ubuntu/flash_driver.cpp b/platform_specific/ubuntu/flash_driver.cpp index 9833266..18d5aea 100644 --- a/platform_specific/ubuntu/flash_driver.cpp +++ b/platform_specific/ubuntu/flash_driver.cpp @@ -82,16 +82,16 @@ size_t flashRead(uint8_t* data, size_t offset, size_t bytes_to_read) { } int8_t flashWrite(const uint8_t* data, size_t offset, size_t bytes_to_write) { - if (is_locked || offset < FLASH_START_ADDR || offset >= FLASH_START_ADDR + 2 * PAGE_SIZE_BYTES) + if (is_locked || offset < FLASH_START_ADDR || offset >= FLASH_START_ADDR + sizeof(flash_memory)) { return LIBPARAMS_WRONG_ARGS; } uint8_t* rom = &(flashGetPointer()[offset - FLASH_START_ADDR]); memcpy(rom, data, bytes_to_write); - uint8_t redundant = (offset - FLASH_START_ADDR) / PAGE_SIZE_BYTES; - mem_layout.memory_ptr = &flashGetPointer() - [offset - redundant * mem_layout.flash_size]; + uint8_t redundant = (offset - FLASH_START_ADDR) / mem_layout.flash_size; + yaml_params.flash.memory_ptr = &flashGetPointer() + [redundant * mem_layout.flash_size]; return __save_to_files(); } diff --git a/tests/params/temp_params_0.yaml b/tests/params/temp_params_0.yaml new file mode 100644 index 0000000..e93f6e6 --- /dev/null +++ b/tests/params/temp_params_0.yaml @@ -0,0 +1,5 @@ +uavcan.node.id : 67305985 +uavcan.pub.mag.id : 134678021 +uavcan.can.baudrate : 0 +name : "" +uavcan.pub.mag.type : "" diff --git a/tests/unit_tests/CMakeLists.txt b/tests/unit_tests/CMakeLists.txt index 8ef01ad..a25fc20 100644 --- a/tests/unit_tests/CMakeLists.txt +++ b/tests/unit_tests/CMakeLists.txt @@ -27,30 +27,9 @@ include_directories(${GTEST_INCLUDE_DIR}) set(LIBPARAMS_PLATFORM ubuntu) include(${ROOT_DIR}/CMakeLists.txt) -function(generate_random_params) - set(LIBPARAMS_PARAMS_INIT_NAME init_params) - set(LIBPARAMS_PARAMS_TEMP_NAME temp_params) - add_definitions(-DLIBPARAMS_PARAMS_DIR="${CMAKE_CURRENT_BINARY_DIR}") - execute_process( - COMMAND ${ROOT_DIR}/scripts/generate_random_params.py - --out-dir ${CMAKE_CURRENT_BINARY_DIR} - RESULT_VARIABLE ret - ) - if(NOT ret EQUAL 0) - message( FATAL_ERROR "Random Params Generator has been failed. Abort.") - endif() - execute_process( - COMMAND ${ROOT_DIR}/scripts/generate_default_params.py --out-dir ${CMAKE_CURRENT_BINARY_DIR} -f ${CMAKE_CURRENT_BINARY_DIR}/params.cpp - --out-file-name ${LIBPARAMS_PARAMS_INIT_NAME} - RESULT_VARIABLE ret - ) - if(NOT ret EQUAL 0) - message( FATAL_ERROR "Default Params Generator has been failed. Abort.") - endif() -endfunction() +add_definitions(-DLIBPARAMS_PARAMS_DIR="${TESTS_DIR}/params") function(gen_test app_name test_file) - # Create the executable target add_executable(${app_name} ${test_file} @@ -59,22 +38,15 @@ function(gen_test app_name test_file) target_include_directories(${app_name} PUBLIC ${libparamsHeaders}) # Conditional source file based on app_name - if(${app_name} STREQUAL yaml_parameters) - message(STATUS "${app_name} params dir ${CMAKE_CURRENT_BINARY_DIR}") - generate_random_params() - target_sources(${app_name} PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/params.cpp) - target_include_directories(${app_name} PRIVATE ${CMAKE_CURRENT_BINARY_DIR}) - else () - message(STATUS "${app_name} params dir ${UNIT_TESTS_DIR}") - target_sources(${app_name} PRIVATE ${TESTS_DIR}/params/params.c) - target_include_directories(${app_name} PRIVATE ${TESTS_DIR}/params/) - endif() + message(STATUS "${app_name} params dir ${UNIT_TESTS_DIR}") + target_sources(${app_name} PRIVATE ${TESTS_DIR}/params/params.c) + target_include_directories(${app_name} PRIVATE ${TESTS_DIR}/params/) # Link libraries to the target target_link_libraries(${app_name} gtest) endfunction() -gen_test(flash_driver ${UNIT_TESTS_DIR}/test_flash_driver.cpp) -gen_test(rom ${UNIT_TESTS_DIR}/test_rom.cpp) -gen_test(storage ${UNIT_TESTS_DIR}/test_storage.cpp) -gen_test(yaml_parameters ${UNIT_TESTS_DIR}/test_yaml_parameters.cpp) +gen_test(flash_driver ${UNIT_TESTS_DIR}/test_flash_driver.cpp) +gen_test(rom ${UNIT_TESTS_DIR}/test_rom.cpp) +gen_test(storage ${UNIT_TESTS_DIR}/test_storage.cpp) +gen_test(yaml_parameters ${UNIT_TESTS_DIR}/test_yaml_parameters.cpp)