Add redundant page (#50)

* add sim_params.yml generator on paramsSave for sitl

* delete tests dir changes

* fixed unit tests

* initial commit

* init commit

* refundant include

* fix rebase, integrated redundant storage feature into main functions of storage

* fix offset issue

* fix param_idxs overflow

* adapted functions and addressing for redundant paging

* paramsInitRedundantPage excluded from paramsInit fn

* add explicit rom drivers pointers, fix bug in ubuntu version for application without redundant pages

* fix standby rom erase

* fix tests

* extend storage tests

* upd error msgs

libparams/rom.h

@@ -28,6 +28,7 @@ typedef struct {
     size_t total_size;
     size_t pages_amount;
     bool inited;
+    bool erased;
 } RomDriverInstance;
 
 

libparams/storage.c

@@ -22,56 +22,96 @@ 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 uint32_t _getStringMemoryPoolAddress(RomDriverInstance* rom_driver);
+static int8_t _save(RomDriverInstance* rom_driver);
+static int8_t _chooseRom();
 
 #define INT_POOL_SIZE           integers_amount * sizeof(IntegerParamValue_t)
 #define STR_POOL_SIZE           MAX_STRING_LENGTH * strings_amount
 
-
 ///< Default values correspond to the last page access only.
-static RomDriverInstance rom = {
+static RomDriverInstance primary_rom = {
+    .inited = false,
+};
+static RomDriverInstance redundant_rom = {
     .inited = false,
 };
 
+RomDriverInstance* active_rom;
+RomDriverInstance* standby_rom;
 
 int8_t paramsInit(ParamIndex_t int_num,
                   ParamIndex_t str_num,
                   int32_t first_page_idx,
                   size_t pages_num) {
+    if (int_num > 512 || str_num > 512) {
+        return LIBPARAMS_WRONG_ARGS;
+    }
     uint32_t need_memory_bytes = sizeof(IntegerParamValue_t) * int_num +\
                                  MAX_STRING_LENGTH * str_num;
-    rom = romInit(first_page_idx, pages_num);
+    primary_rom = romInit(first_page_idx, pages_num);
 
-    if (!rom.inited) {
+    if (!primary_rom.inited) {
         return LIBPARAMS_UNKNOWN_ERROR;
     }
 
-    if (romGetAvailableMemory(&rom) < need_memory_bytes) {
+    if (romGetAvailableMemory(&primary_rom) < need_memory_bytes) {
         return LIBPARAMS_WRONG_ARGS;
     }
-
     integers_amount = int_num;
     strings_amount = str_num;
     all_params_amount = integers_amount + strings_amount;
+    active_rom = &primary_rom;
+    return LIBPARAMS_OK;
+}
+
+int8_t paramsInitRedundantPage() {
+    redundant_rom =
+        romInit(primary_rom.first_page_idx - primary_rom.pages_amount, primary_rom.pages_amount);
+    if (!redundant_rom.inited) {
+        return LIBPARAMS_UNKNOWN_ERROR;
+    }
+    _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);
+    if (active_rom == NULL) {
+        return LIBPARAMS_NOT_INITIALIZED;
+    }
+    romRead(active_rom, 0, (uint8_t*)integer_values_pool, INT_POOL_SIZE);
+    romRead(active_rom, _getStringMemoryPoolAddress(active_rom),
+            (uint8_t*)&string_values_pool, STR_POOL_SIZE);
 
-    for (uint_fast8_t idx = 0; idx < integers_amount; idx++) {
+    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;
         }
     }
 
-    for (uint_fast8_t idx = 0; idx < strings_amount; idx++) {
+    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);
+        } 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(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) {
+            rom_instance->erased = true;
+            return LIBPARAMS_OK;
         }
     }
 
@@ -82,10 +122,28 @@ int8_t paramsSave() {
     if (all_params_amount == 0) {
         return LIBPARAMS_NOT_INITIALIZED;
     }
-
-    romBeginWrite(&rom);
-    int8_t res = _save();
-    romEndWrite(&rom);
+    int8_t res = 0;
+    if (standby_rom != NULL) {
+        // write params to standby rom
+        romBeginWrite(standby_rom);
+        res = _save(standby_rom);
+        romEndWrite(standby_rom);
+        // if save was successful erase active rom
+        // and switch roms
+        if (res == 0) {
+            RomDriverInstance* buffer = standby_rom;
+            standby_rom = active_rom;
+            active_rom = buffer;
+            standby_rom->erased = true;
+            romBeginWrite(standby_rom);
+            romEndWrite(standby_rom);
+        }
+        return res;
+    }
+    romBeginWrite(active_rom);
+    active_rom->erased = true;
+    res = _save(active_rom);
+    romEndWrite(active_rom);
     return res;
 }
 
@@ -95,7 +153,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;
         }
     }
@@ -191,8 +249,7 @@ StringParamValue_t* paramsGetStringValue(ParamIndex_t param_idx) {
     return str;
 }
 
-uint8_t paramsSetStringValue(ParamIndex_t param_idx,
-                             uint8_t str_len,
+uint8_t paramsSetStringValue(ParamIndex_t param_idx, uint8_t str_len,
                              const StringParamValue_t param_value) {
     if (str_len > MAX_STRING_LENGTH || _isCorrectStringParamIndex(param_idx)) {
         return 0;
@@ -225,26 +282,45 @@ 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;
 }
 
 /**
  * @note From romWrite() it is always expected to be successfully executed withing this file.
  * 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;
     }
+    rom_driver->erased = false;
+    return LIBPARAMS_OK;
+}
+
 
+/**
+ * @brief           Choose a rom which addreses a non-erased part of flash memory
+ * **/
+int8_t _chooseRom() {
+    paramsLoadRom(&primary_rom);
+    paramsLoadRom(&redundant_rom);
+    active_rom = &primary_rom;
+    standby_rom = &redundant_rom;
+    if (primary_rom.erased) {
+        if (!redundant_rom.erased) {
+            active_rom = &redundant_rom;
+            standby_rom = &primary_rom;
+        }
+    }
     return LIBPARAMS_OK;
 }
+

libparams/storage.h

@@ -84,6 +84,12 @@ 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 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 paramsInitRedundantPage();
+
 /**
  * @brief           Load parameters from a persistent memory: flash for stm32 and file for ubuntu.
  * @return          LIBPARAMS_OK on success, otherwise < 0.

platform_specific/ubuntu/FlashMemoryLayout.hpp

@@ -15,12 +15,12 @@ typedef struct {
     IntegerDesc_t* integer_desc_pool;
     StringDesc_t* string_desc_pool;
 
-    uint8_t num_int_params;
-    uint8_t num_str_params;
+    uint16_t num_int_params;
+    uint16_t num_str_params;
 } ParametersLayout_t;
 
 typedef struct {
-    const uint8_t* memory_ptr;
+    uint8_t* memory_ptr;
     uint16_t page_size;
     uint8_t num_pages;
     uint32_t flash_size = page_size * num_pages;

platform_specific/ubuntu/YamlParameters.cpp

@@ -10,9 +10,9 @@
 #include <filesystem>
 #include <fstream>
 #include <iostream>
+#include <iomanip>
 
 #include "storage.h"
-#include "libparams_error_codes.h"
 #include "YamlParameters.hpp"
 
 YamlParameters::YamlParameters(FlashMemoryLayout_t flash_desc,
@@ -60,10 +60,10 @@ int8_t YamlParameters::read_from_dir(const std::string& path) {
     }
     int8_t res;
     char file_name[256];
-    uint8_t int_param_idx = 0;
-    uint8_t str_param_idx = 0;
+    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
@@ -83,7 +83,7 @@ int8_t YamlParameters::read_from_dir(const std::string& path) {
         logger.info("data read from ", file_name);
 
         if ((int_param_idx > params.num_int_params) || (str_param_idx > params.num_str_params)) {
-            break;;
+            break;
         }
         res = __read_page(params_storage_file, &int_param_idx, &str_param_idx);
         params_storage_file.close();
@@ -110,9 +110,9 @@ int8_t YamlParameters::write_to_dir(const std::string& path) {
     int8_t res;
     char file_name[256];
     // remember last written indexes
-    uint8_t int_param_idx = 0;
-    uint8_t str_param_idx = 0;
-    for (uint8_t idx = 0; idx < flash.num_pages; idx++) {
+    uint16_t int_param_idx = 0;
+    uint16_t str_param_idx = 0;
+    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;
@@ -127,16 +127,17 @@ int8_t YamlParameters::write_to_dir(const std::string& path) {
     if (int_param_idx != params.num_int_params || str_param_idx != params.num_str_params) {
         logger.error("Number of parameters in the file isn't equal",
                      " to the one specified in the constructor",
-                     "int real: ", (int)int_param_idx, " expected: ", (int)params.num_int_params,
-                     "\n",
-                     "str real: ", (int)str_param_idx, " expected: ", (int)params.num_str_params);
+                     "int real: ", (int)int_param_idx + 1,
+                     " expected: ", (int)params.num_int_params, "\n",
+                     "str real: ", (int)str_param_idx + 1,
+                     " expected: ", (int)params.num_str_params);
         return LIBPARAMS_WRONG_ARGS;
     }
     return LIBPARAMS_OK;
 }
 
-int8_t YamlParameters::__read_page(std::ifstream& params_storage_file, uint8_t* int_param_idx,
-                                   uint8_t* str_param_idx) {
+int8_t YamlParameters::__read_page(std::ifstream& params_storage_file, uint16_t* int_param_idx,
+                                   uint16_t* str_param_idx) {
     std::string line;
     std::string value;
 
@@ -148,7 +149,7 @@ int8_t YamlParameters::__read_page(std::ifstream& params_storage_file, uint8_t*
         }
         value = line.substr(delimiter_pos + 1);
         try {
-            if (*int_param_idx > params.num_int_params) {
+            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 +161,18 @@ int8_t YamlParameters::__read_page(std::ifstream& params_storage_file, uint8_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) {
-            int offset = flash.flash_size - MAX_STRING_LENGTH *
-                         (params.num_str_params - (*str_param_idx ));
-            if (*str_param_idx + 1> 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) + 1);
                 return LIBPARAMS_WRONG_ARGS;
             }
 
             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_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;
@@ -186,17 +188,17 @@ int8_t YamlParameters::__read_page(std::ifstream& params_storage_file, uint8_t*
     return LIBPARAMS_OK;
 }
 
-int8_t YamlParameters::__write_page(std::ofstream& params_storage_file, uint8_t* int_param_idx,
-                                    uint8_t* str_param_idx) {
+int8_t YamlParameters::__write_page(std::ofstream& params_storage_file, uint16_t* int_param_idx,
+                                    uint16_t* str_param_idx) {
     if (*int_param_idx > params.num_int_params || *str_param_idx > params.num_str_params) {
         logger.error("int_param_idx or str_param_idx is bigger than defined by num_int_params\n");
         return LIBPARAMS_WRONG_ARGS;
     }
 
     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);
@@ -218,7 +220,7 @@ int8_t YamlParameters::__write_page(std::ofstream& params_storage_file, uint8_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;