diff --git a/radio/src/datastructs_private.h b/radio/src/datastructs_private.h index f9531dc3a..fc4e2c828 100644 --- a/radio/src/datastructs_private.h +++ b/radio/src/datastructs_private.h @@ -715,8 +715,9 @@ PACK(struct customSwitch { #endif PACK(struct PartialModel { + CUST_ATTR(semver,nullptr,w_semver); ModelHeader header; - TimerData timers[MAX_TIMERS]; + ModuleData moduleData[NUM_MODULES]; }); /* @@ -756,8 +757,10 @@ PACK(struct USBJoystickChData { }); PACK(struct ModelData { + // Must match start of PartialModel CUST_ATTR(semver,nullptr,w_semver); ModelHeader header; + TimerData timers[MAX_TIMERS]; uint8_t telemetryProtocol:3; uint8_t thrTrim:1; // Enable Throttle Trim diff --git a/radio/src/storage/modelslist.cpp b/radio/src/storage/modelslist.cpp index 990a62ab9..a3681c544 100644 --- a/radio/src/storage/modelslist.cpp +++ b/radio/src/storage/modelslist.cpp @@ -102,11 +102,11 @@ void ModelCell::setModelId(uint8_t moduleIdx, uint8_t id) modelId[moduleIdx] = id; } -void ModelCell::setRfData(ModelData *model) +void ModelCell::setRfData(ModelHeader *header, ModuleData* modData) { for (uint8_t i = 0; i < NUM_MODULES; i++) { - modelId[i] = model->header.modelId[i]; - setRfModuleData(i, &(model->moduleData[i])); + modelId[i] = header->modelId[i]; + setRfModuleData(i, &(modData[i])); TRACE("<%s/%i> : %X,%X,%X", strlen(modelName) ? modelName : modelFilename, i, moduleData[i].type, moduleData[i].subType, modelId[i]); } @@ -660,17 +660,6 @@ bool ModelMap::renameLabel(const std::string &from, std::string to, } } - ModelData *modeldata = (ModelData *)malloc(sizeof(ModelData)); - if (!modeldata) { - TRACE("Labels: Out Of Memory"); - if (progress != nullptr) progress("", 100); // Kill progress dialog - return true; - } - - // Force a write of any changes in memory - storageCheck(true); - - bool fault = false; ModelsVector mods = getModelsByLabel(from); // Find all models to be renamed // Scan all these models first, recombine their labels to a csv, @@ -685,22 +674,26 @@ bool ModelMap::renameLabel(const std::string &from, std::string to, if(curlen + csvto.size() - csvfrom.size() > LABELS_LENGTH - 1) { TRACE("Labels: Rename Error! Labels too long on %s", model->modelName); if (progress != nullptr) progress("", 100); // Kill progress dialog - free(modeldata); return true; } } + // Force a write of any changes in memory + storageCheck(true); + + bool fault = false; int i = 0; for (const auto &modcell : mods) { if (progress != nullptr) { progress(modcell->modelFilename, (i++) * 100 / mods.size()); } - readModelYaml(modcell->modelFilename, (uint8_t *)modeldata, - sizeof(ModelData)); + PartialModel partial; + memclear(&partial, sizeof(PartialModel)); + readModelYaml(modcell->modelFilename, (uint8_t*)&partial, sizeof(PartialModel)); // Separate Curent CSV - LabelsVector lbls = ModelMap::fromCSV(modeldata->header.labels); + LabelsVector lbls = ModelMap::fromCSV(partial.header.labels); // Replace from->to strings for (auto &lbl : lbls) { @@ -714,20 +707,14 @@ bool ModelMap::renameLabel(const std::string &from, std::string to, lbls.resize(std::distance(lbls.begin(), last)); // Write back - strncpy(modeldata->header.labels, ModelMap::toCSV(lbls).c_str(), LABELS_LENGTH); - modeldata->header.labels[LABELS_LENGTH-1] = '\0'; - - char path[256]; - getModelPath(path, modcell->modelFilename); + strAppend(partial.header.labels, ModelMap::toCSV(lbls).c_str(), LABELS_LENGTH - 1); if (modcell == modelslist.getCurrentModel()) { - // If working on the current model, write current data to file instead - memcpy(g_model.header.labels, modeldata->header.labels, LABELS_LENGTH); - fault = (writeFileYaml(path, get_modeldata_nodes(), - (uint8_t *)&g_model, 0) != NULL); + // If working on the current model, copy new labels mark dirty + memcpy(g_model.header.labels, partial.header.labels, LABELS_LENGTH); + storageDirty(EE_MODEL); } else { - fault = (writeFileYaml(path, get_modeldata_nodes(), - (uint8_t *)modeldata, 0) != NULL); + fault = writeModelLabels(modcell, partial.header.labels); } #if defined(SIMU) sleep_ms(100); @@ -745,8 +732,6 @@ bool ModelMap::renameLabel(const std::string &from, std::string to, // Make sure to leave at 100, to kill rename dialog if (progress != nullptr) progress("", 100); - free(modeldata); - // Issue a rescan all of all models. modelslist.clear(); modelslist.load(); @@ -808,6 +793,122 @@ bool ModelMap::removeModels(ModelCell *cell) return rv; } +/** + * @brief Update the labels in an existing model yaml file on SD card + * + * @param cell Model to update + * @return true Success + * @return false Failure + */ +bool ModelMap::writeModelLabels(ModelCell* cell, const char* labels) +{ + TRACE("Updating labels in %s",cell->modelFilename); + + UINT bytes_cnt; + char buf[512]; + char tempPath[256]; + FIL out; + FIL file; + + // Read exiting model header + PartialModel partial; + memclear(&partial, sizeof(PartialModel)); + readModelYaml(cell->modelFilename, (uint8_t*)&partial, sizeof(PartialModel)); + + // Update header with new labels + strAppend(partial.header.labels, labels, LABELS_LENGTH - 1); + // Remove module data - only want to write the header + memclear(&partial.moduleData, sizeof(ModuleData) * NUM_MODULES); + + // Write new header to a temp file + getModelPath(tempPath, "tmp.yml"); + if (writeFileYaml(tempPath, get_partialmodel_nodes(), (uint8_t *)&partial, 0) != NULL) { + TRACE("ERROR writing temp model file"); + f_unlink(tempPath); + return false; + } + + // Open tmp file for appending + FRESULT result = f_open(&out, tempPath, FA_OPEN_EXISTING | FA_WRITE | FA_OPEN_APPEND); + if (result != FR_OK) { + TRACE("ERROR opening temp file"); + f_unlink(tempPath); + return false; + } + + // Copy rest of model yaml from original file to temp file + getModelPath(buf, cell->modelFilename); + result = f_open(&file, buf, FA_OPEN_EXISTING | FA_READ); + if (result != FR_OK) { + f_close(&out); + f_unlink(tempPath); + return false; + } + + // Read old header - assumes header fits within first 512 bytes + // header has not changed significantly for a long time so should be safe + result = f_read(&file, buf, sizeof(buf), &bytes_cnt); + if (result != FR_OK) { + f_close(&out); + f_close(&file); + f_unlink(tempPath); + return false; + } + + // Find header section + int n = 0; + while (n < sizeof(buf) - 7 && strncmp(&buf[n], "header:", 7) != 0) + n += 1; + + if (n >= sizeof(buf) - 7) { + TRACE("ERROR model header not found in %s", cell->modelFilename); + f_close(&out); + f_close(&file); + f_unlink(tempPath); + return false; + } + + // Skip header section - look for next section after 'header:' + do { + // Skip current line + while (n < sizeof(buf) && buf[n] != '\n') n += 1; + n += 1; + } while ((n < sizeof(buf)) && buf[n] == ' '); + + if (n >= sizeof(buf)) { + TRACE("ERROR could not match model header in %s", cell->modelFilename); + f_close(&out); + f_close(&file); + f_unlink(tempPath); + return false; + } + + // Write remainder of first buffer after header + result = f_write(&out, &buf[n], sizeof(buf) - n, &bytes_cnt); + // Block copy the rest of the original file to the temp file + while (result == FR_OK && bytes_cnt != 0) { + result = f_read(&file, buf, sizeof(buf), &bytes_cnt); + if (result == FR_OK && bytes_cnt != 0) + result = f_write(&out, buf, bytes_cnt, &bytes_cnt); + } + + f_close(&out); + f_close(&file); + + if (result != FR_OK) { + TRACE("ERROR copying to temp file"); + f_unlink(tempPath); + return false; + } + + // Delete original file and rename temp file + getModelPath(buf, cell->modelFilename); + f_unlink(buf); + f_rename(tempPath, buf); + + return true; +} + /** * @brief Opens a model.yml File and writes labels data into it. * @details If the cell is current model then write the labels data to g_model @@ -829,25 +930,7 @@ bool ModelMap::updateModelFile(ModelCell *cell) return false; } - ModelData *modeldata = (ModelData *)malloc(sizeof(ModelData)); - if (!modeldata) { - TRACE("Labels: Out Of Memory"); - return true; - } - - bool fault = false; - readModelYaml(cell->modelFilename, (uint8_t *)modeldata, sizeof(ModelData)); - - strncpy(modeldata->header.labels, ModelMap::toCSV(getLabelsByModel(cell)).c_str(), - LABELS_LENGTH - 1); - modeldata->header.labels[LABELS_LENGTH - 1] = '\0'; - - char path[256]; - getModelPath(path, cell->modelFilename); - fault = (writeFileYaml(path, get_modeldata_nodes(), (uint8_t *)modeldata, 0) != - NULL); - - free(modeldata); + bool fault = writeModelLabels(cell, ModelMap::toCSV(getLabelsByModel(cell)).c_str()); #if defined(DEBUG_TIMERS) DEBUG_TIMER_SAMPLE(debugTimerYamlScan); @@ -930,28 +1013,23 @@ void ModelMap::updateModelCell(ModelCell *cell) { modelslabels.removeModels(cell); - ModelData *model = (ModelData *)malloc(sizeof(ModelData)); - if (!model) { - TRACE("Labels: Out Of Memory"); - return; - } - TRACE("Labels: Updating model %s", cell->modelFilename); - readModelYaml(cell->modelFilename, (uint8_t *)model, sizeof(ModelData)); - strncpy(cell->modelName, model->header.name, LEN_MODEL_NAME); - cell->modelName[LEN_MODEL_NAME] = '\0'; - strncpy(cell->modelBitmap, model->header.bitmap, LEN_BITMAP_NAME); - cell->modelBitmap[LEN_BITMAP_NAME] = '\0'; - LabelsVector labels = ModelMap::fromCSV(model->header.labels); + + PartialModel partial; + memclear(&partial, sizeof(PartialModel)); + readModelYaml(cell->modelFilename, (uint8_t*)&partial, sizeof(PartialModel)); + + strAppend(cell->modelName, partial.header.name, LEN_MODEL_NAME); + strAppend(cell->modelBitmap, partial.header.bitmap, LEN_BITMAP_NAME); + LabelsVector labels = ModelMap::fromCSV(partial.header.labels); for(const auto &lbl : labels ) { modelslabels.addLabelToModel(lbl,cell); } // Save Module Data - cell->setRfData(model); + cell->setRfData(&partial.header, partial.moduleData); cell->_isDirty = false; - free(model); } /** @@ -1326,13 +1404,11 @@ void ModelsList::updateCurrentModelCell() { if (currentModel) { #if LEN_BITMAP_NAME > 0 - strncpy(currentModel->modelBitmap, g_model.header.bitmap, LEN_BITMAP_NAME); - currentModel->modelBitmap[LEN_BITMAP_NAME] = '\0'; + strAppend(currentModel->modelBitmap, g_model.header.bitmap, LEN_BITMAP_NAME); #endif - strncpy(currentModel->modelFilename, g_eeGeneral.currModelFilename, LEN_MODEL_FILENAME); - currentModel->modelFilename[LEN_MODEL_FILENAME] = '\0'; + strAppend(currentModel->modelFilename, g_eeGeneral.currModelFilename, LEN_MODEL_FILENAME); currentModel->setModelName(g_model.header.name); - currentModel->setRfData(&g_model); + currentModel->setRfData(&g_model.header, g_model.moduleData); modelslabels.setDirty(); } else { TRACE("ModelList Error - No Current Model"); diff --git a/radio/src/storage/modelslist.h b/radio/src/storage/modelslist.h index fe2e2e2e4..d06934442 100644 --- a/radio/src/storage/modelslist.h +++ b/radio/src/storage/modelslist.h @@ -43,7 +43,7 @@ #define DEFAULT_MODEL_SORT NAME_ASC -struct ModelData; +struct ModelHeader; struct ModuleData; struct SimpleModuleData { @@ -80,7 +80,7 @@ class ModelCell void setModelName(char *name); void setModelName(char *name, uint8_t len); - void setRfData(ModelData *model); + void setRfData(ModelHeader *header, ModuleData* moduleData); void setModelId(uint8_t moduleIdx, uint8_t id); void setRfModuleData(uint8_t moduleIdx, ModuleData *modData); @@ -168,6 +168,8 @@ class ModelMap : protected std::multimap void updateModelCell(ModelCell *); bool removeModels(ModelCell *); + bool writeModelLabels(ModelCell*, const char*); + protected: ModelsSortBy _sortOrder = DEFAULT_MODEL_SORT; bool _isDirty = true; diff --git a/radio/src/storage/yaml/yaml_datastructs_128x64.cpp b/radio/src/storage/yaml/yaml_datastructs_128x64.cpp index e597a33af..b3bb957b7 100644 --- a/radio/src/storage/yaml/yaml_datastructs_128x64.cpp +++ b/radio/src/storage/yaml/yaml_datastructs_128x64.cpp @@ -881,8 +881,9 @@ static const struct YamlNode struct_ModelData[] = { YAML_END }; static const struct YamlNode struct_PartialModel[] = { + YAML_CUSTOM("semver",nullptr,w_semver), YAML_STRUCT("header", 96, struct_ModelHeader, NULL), - YAML_ARRAY("timers", 96, 3, struct_TimerData, NULL), + YAML_ARRAY("moduleData", 232, 2, struct_ModuleData, NULL), YAML_END }; diff --git a/radio/src/storage/yaml/yaml_datastructs_c14.cpp b/radio/src/storage/yaml/yaml_datastructs_c14.cpp index 443f73f2d..c0edf57f6 100644 --- a/radio/src/storage/yaml/yaml_datastructs_c14.cpp +++ b/radio/src/storage/yaml/yaml_datastructs_c14.cpp @@ -977,8 +977,9 @@ static const struct YamlNode struct_ModelData[] = { YAML_END }; static const struct YamlNode struct_PartialModel[] = { + YAML_CUSTOM("semver",nullptr,w_semver), YAML_STRUCT("header", 1048, struct_ModelHeader, NULL), - YAML_ARRAY("timers", 136, 3, struct_TimerData, NULL), + YAML_ARRAY("moduleData", 232, 2, struct_ModuleData, NULL), YAML_END }; diff --git a/radio/src/storage/yaml/yaml_datastructs_f16.cpp b/radio/src/storage/yaml/yaml_datastructs_f16.cpp index 6172ec13f..6ae2ad493 100644 --- a/radio/src/storage/yaml/yaml_datastructs_f16.cpp +++ b/radio/src/storage/yaml/yaml_datastructs_f16.cpp @@ -978,8 +978,9 @@ static const struct YamlNode struct_ModelData[] = { YAML_END }; static const struct YamlNode struct_PartialModel[] = { + YAML_CUSTOM("semver",nullptr,w_semver), YAML_STRUCT("header", 1048, struct_ModelHeader, NULL), - YAML_ARRAY("timers", 136, 3, struct_TimerData, NULL), + YAML_ARRAY("moduleData", 232, 2, struct_ModuleData, NULL), YAML_END }; diff --git a/radio/src/storage/yaml/yaml_datastructs_gx12.cpp b/radio/src/storage/yaml/yaml_datastructs_gx12.cpp index 3abdb34d3..073cee189 100644 --- a/radio/src/storage/yaml/yaml_datastructs_gx12.cpp +++ b/radio/src/storage/yaml/yaml_datastructs_gx12.cpp @@ -929,8 +929,9 @@ static const struct YamlNode struct_ModelData[] = { YAML_END }; static const struct YamlNode struct_PartialModel[] = { + YAML_CUSTOM("semver",nullptr,w_semver), YAML_STRUCT("header", 96, struct_ModelHeader, NULL), - YAML_ARRAY("timers", 96, 3, struct_TimerData, NULL), + YAML_ARRAY("moduleData", 232, 2, struct_ModuleData, NULL), YAML_END }; diff --git a/radio/src/storage/yaml/yaml_datastructs_gx15.cpp b/radio/src/storage/yaml/yaml_datastructs_gx15.cpp index 5933e4aaf..274ab0333 100644 --- a/radio/src/storage/yaml/yaml_datastructs_gx15.cpp +++ b/radio/src/storage/yaml/yaml_datastructs_gx15.cpp @@ -1026,8 +1026,9 @@ static const struct YamlNode struct_ModelData[] = { YAML_END }; static const struct YamlNode struct_PartialModel[] = { + YAML_CUSTOM("semver",nullptr,w_semver), YAML_STRUCT("header", 1048, struct_ModelHeader, NULL), - YAML_ARRAY("timers", 136, 3, struct_TimerData, NULL), + YAML_ARRAY("moduleData", 232, 2, struct_ModuleData, NULL), YAML_END }; diff --git a/radio/src/storage/yaml/yaml_datastructs_nb4p.cpp b/radio/src/storage/yaml/yaml_datastructs_nb4p.cpp index 652730433..13244fbd5 100644 --- a/radio/src/storage/yaml/yaml_datastructs_nb4p.cpp +++ b/radio/src/storage/yaml/yaml_datastructs_nb4p.cpp @@ -968,8 +968,9 @@ static const struct YamlNode struct_ModelData[] = { YAML_END }; static const struct YamlNode struct_PartialModel[] = { + YAML_CUSTOM("semver",nullptr,w_semver), YAML_STRUCT("header", 1048, struct_ModelHeader, NULL), - YAML_ARRAY("timers", 136, 3, struct_TimerData, NULL), + YAML_ARRAY("moduleData", 232, 2, struct_ModuleData, NULL), YAML_END }; diff --git a/radio/src/storage/yaml/yaml_datastructs_nv14.cpp b/radio/src/storage/yaml/yaml_datastructs_nv14.cpp index ad8eb54bb..3183ba727 100644 --- a/radio/src/storage/yaml/yaml_datastructs_nv14.cpp +++ b/radio/src/storage/yaml/yaml_datastructs_nv14.cpp @@ -976,8 +976,9 @@ static const struct YamlNode struct_ModelData[] = { YAML_END }; static const struct YamlNode struct_PartialModel[] = { + YAML_CUSTOM("semver",nullptr,w_semver), YAML_STRUCT("header", 1048, struct_ModelHeader, NULL), - YAML_ARRAY("timers", 136, 3, struct_TimerData, NULL), + YAML_ARRAY("moduleData", 232, 2, struct_ModuleData, NULL), YAML_END }; diff --git a/radio/src/storage/yaml/yaml_datastructs_pa01.cpp b/radio/src/storage/yaml/yaml_datastructs_pa01.cpp index 9a01dc9fc..517ab5ddf 100644 --- a/radio/src/storage/yaml/yaml_datastructs_pa01.cpp +++ b/radio/src/storage/yaml/yaml_datastructs_pa01.cpp @@ -1025,8 +1025,9 @@ static const struct YamlNode struct_ModelData[] = { YAML_END }; static const struct YamlNode struct_PartialModel[] = { + YAML_CUSTOM("semver",nullptr,w_semver), YAML_STRUCT("header", 1048, struct_ModelHeader, NULL), - YAML_ARRAY("timers", 136, 3, struct_TimerData, NULL), + YAML_ARRAY("moduleData", 232, 2, struct_ModuleData, NULL), YAML_END }; diff --git a/radio/src/storage/yaml/yaml_datastructs_pl18.cpp b/radio/src/storage/yaml/yaml_datastructs_pl18.cpp index 31bde01d5..66bedbcec 100644 --- a/radio/src/storage/yaml/yaml_datastructs_pl18.cpp +++ b/radio/src/storage/yaml/yaml_datastructs_pl18.cpp @@ -976,8 +976,9 @@ static const struct YamlNode struct_ModelData[] = { YAML_END }; static const struct YamlNode struct_PartialModel[] = { + YAML_CUSTOM("semver",nullptr,w_semver), YAML_STRUCT("header", 1048, struct_ModelHeader, NULL), - YAML_ARRAY("timers", 136, 3, struct_TimerData, NULL), + YAML_ARRAY("moduleData", 232, 2, struct_ModuleData, NULL), YAML_END }; diff --git a/radio/src/storage/yaml/yaml_datastructs_pl18u.cpp b/radio/src/storage/yaml/yaml_datastructs_pl18u.cpp index 3548ae842..17e0ed75c 100644 --- a/radio/src/storage/yaml/yaml_datastructs_pl18u.cpp +++ b/radio/src/storage/yaml/yaml_datastructs_pl18u.cpp @@ -968,8 +968,9 @@ static const struct YamlNode struct_ModelData[] = { YAML_END }; static const struct YamlNode struct_PartialModel[] = { + YAML_CUSTOM("semver",nullptr,w_semver), YAML_STRUCT("header", 1048, struct_ModelHeader, NULL), - YAML_ARRAY("timers", 136, 3, struct_TimerData, NULL), + YAML_ARRAY("moduleData", 232, 2, struct_ModuleData, NULL), YAML_END }; diff --git a/radio/src/storage/yaml/yaml_datastructs_st16.cpp b/radio/src/storage/yaml/yaml_datastructs_st16.cpp index 667ad4317..6d8a62570 100644 --- a/radio/src/storage/yaml/yaml_datastructs_st16.cpp +++ b/radio/src/storage/yaml/yaml_datastructs_st16.cpp @@ -1025,8 +1025,9 @@ static const struct YamlNode struct_ModelData[] = { YAML_END }; static const struct YamlNode struct_PartialModel[] = { + YAML_CUSTOM("semver",nullptr,w_semver), YAML_STRUCT("header", 1048, struct_ModelHeader, NULL), - YAML_ARRAY("timers", 136, 3, struct_TimerData, NULL), + YAML_ARRAY("moduleData", 232, 2, struct_ModuleData, NULL), YAML_END }; diff --git a/radio/src/storage/yaml/yaml_datastructs_t15.cpp b/radio/src/storage/yaml/yaml_datastructs_t15.cpp index cc6d78672..bc3331313 100644 --- a/radio/src/storage/yaml/yaml_datastructs_t15.cpp +++ b/radio/src/storage/yaml/yaml_datastructs_t15.cpp @@ -1003,8 +1003,9 @@ static const struct YamlNode struct_ModelData[] = { YAML_END }; static const struct YamlNode struct_PartialModel[] = { + YAML_CUSTOM("semver",nullptr,w_semver), YAML_STRUCT("header", 1048, struct_ModelHeader, NULL), - YAML_ARRAY("timers", 136, 3, struct_TimerData, NULL), + YAML_ARRAY("moduleData", 232, 2, struct_ModuleData, NULL), YAML_END }; diff --git a/radio/src/storage/yaml/yaml_datastructs_t15pro.cpp b/radio/src/storage/yaml/yaml_datastructs_t15pro.cpp index d37bf8576..e739e85fe 100644 --- a/radio/src/storage/yaml/yaml_datastructs_t15pro.cpp +++ b/radio/src/storage/yaml/yaml_datastructs_t15pro.cpp @@ -1025,8 +1025,9 @@ static const struct YamlNode struct_ModelData[] = { YAML_END }; static const struct YamlNode struct_PartialModel[] = { + YAML_CUSTOM("semver",nullptr,w_semver), YAML_STRUCT("header", 1048, struct_ModelHeader, NULL), - YAML_ARRAY("timers", 136, 3, struct_TimerData, NULL), + YAML_ARRAY("moduleData", 232, 2, struct_ModuleData, NULL), YAML_END }; diff --git a/radio/src/storage/yaml/yaml_datastructs_t20.cpp b/radio/src/storage/yaml/yaml_datastructs_t20.cpp index 8fe2d3e42..7eb523e79 100644 --- a/radio/src/storage/yaml/yaml_datastructs_t20.cpp +++ b/radio/src/storage/yaml/yaml_datastructs_t20.cpp @@ -907,8 +907,9 @@ static const struct YamlNode struct_ModelData[] = { YAML_END }; static const struct YamlNode struct_PartialModel[] = { + YAML_CUSTOM("semver",nullptr,w_semver), YAML_STRUCT("header", 96, struct_ModelHeader, NULL), - YAML_ARRAY("timers", 96, 3, struct_TimerData, NULL), + YAML_ARRAY("moduleData", 232, 2, struct_ModuleData, NULL), YAML_END }; diff --git a/radio/src/storage/yaml/yaml_datastructs_t22.cpp b/radio/src/storage/yaml/yaml_datastructs_t22.cpp index d37bf8576..e739e85fe 100644 --- a/radio/src/storage/yaml/yaml_datastructs_t22.cpp +++ b/radio/src/storage/yaml/yaml_datastructs_t22.cpp @@ -1025,8 +1025,9 @@ static const struct YamlNode struct_ModelData[] = { YAML_END }; static const struct YamlNode struct_PartialModel[] = { + YAML_CUSTOM("semver",nullptr,w_semver), YAML_STRUCT("header", 1048, struct_ModelHeader, NULL), - YAML_ARRAY("timers", 136, 3, struct_TimerData, NULL), + YAML_ARRAY("moduleData", 232, 2, struct_ModuleData, NULL), YAML_END }; diff --git a/radio/src/storage/yaml/yaml_datastructs_tpro.cpp b/radio/src/storage/yaml/yaml_datastructs_tpro.cpp index ed10a9a02..5628d781b 100644 --- a/radio/src/storage/yaml/yaml_datastructs_tpro.cpp +++ b/radio/src/storage/yaml/yaml_datastructs_tpro.cpp @@ -907,8 +907,9 @@ static const struct YamlNode struct_ModelData[] = { YAML_END }; static const struct YamlNode struct_PartialModel[] = { + YAML_CUSTOM("semver",nullptr,w_semver), YAML_STRUCT("header", 96, struct_ModelHeader, NULL), - YAML_ARRAY("timers", 96, 3, struct_TimerData, NULL), + YAML_ARRAY("moduleData", 232, 2, struct_ModuleData, NULL), YAML_END }; diff --git a/radio/src/storage/yaml/yaml_datastructs_tx15.cpp b/radio/src/storage/yaml/yaml_datastructs_tx15.cpp index 5933e4aaf..274ab0333 100644 --- a/radio/src/storage/yaml/yaml_datastructs_tx15.cpp +++ b/radio/src/storage/yaml/yaml_datastructs_tx15.cpp @@ -1026,8 +1026,9 @@ static const struct YamlNode struct_ModelData[] = { YAML_END }; static const struct YamlNode struct_PartialModel[] = { + YAML_CUSTOM("semver",nullptr,w_semver), YAML_STRUCT("header", 1048, struct_ModelHeader, NULL), - YAML_ARRAY("timers", 136, 3, struct_TimerData, NULL), + YAML_ARRAY("moduleData", 232, 2, struct_ModuleData, NULL), YAML_END }; diff --git a/radio/src/storage/yaml/yaml_datastructs_tx16smk3.cpp b/radio/src/storage/yaml/yaml_datastructs_tx16smk3.cpp index 6a8f45507..0e16f08ce 100644 --- a/radio/src/storage/yaml/yaml_datastructs_tx16smk3.cpp +++ b/radio/src/storage/yaml/yaml_datastructs_tx16smk3.cpp @@ -1027,8 +1027,9 @@ static const struct YamlNode struct_ModelData[] = { YAML_END }; static const struct YamlNode struct_PartialModel[] = { + YAML_CUSTOM("semver",nullptr,w_semver), YAML_STRUCT("header", 1048, struct_ModelHeader, NULL), - YAML_ARRAY("timers", 136, 3, struct_TimerData, NULL), + YAML_ARRAY("moduleData", 232, 2, struct_ModuleData, NULL), YAML_END }; diff --git a/radio/src/storage/yaml/yaml_datastructs_v12.cpp b/radio/src/storage/yaml/yaml_datastructs_v12.cpp index 026401122..56f73b7bd 100644 --- a/radio/src/storage/yaml/yaml_datastructs_v12.cpp +++ b/radio/src/storage/yaml/yaml_datastructs_v12.cpp @@ -1021,8 +1021,9 @@ static const struct YamlNode struct_ModelData[] = { YAML_END }; static const struct YamlNode struct_PartialModel[] = { + YAML_CUSTOM("semver",nullptr,w_semver), YAML_STRUCT("header", 1048, struct_ModelHeader, NULL), - YAML_ARRAY("timers", 136, 3, struct_TimerData, NULL), + YAML_ARRAY("moduleData", 232, 2, struct_ModuleData, NULL), YAML_END }; diff --git a/radio/src/storage/yaml/yaml_datastructs_x10.cpp b/radio/src/storage/yaml/yaml_datastructs_x10.cpp index 764884183..31b8131cc 100644 --- a/radio/src/storage/yaml/yaml_datastructs_x10.cpp +++ b/radio/src/storage/yaml/yaml_datastructs_x10.cpp @@ -978,8 +978,9 @@ static const struct YamlNode struct_ModelData[] = { YAML_END }; static const struct YamlNode struct_PartialModel[] = { + YAML_CUSTOM("semver",nullptr,w_semver), YAML_STRUCT("header", 1048, struct_ModelHeader, NULL), - YAML_ARRAY("timers", 136, 3, struct_TimerData, NULL), + YAML_ARRAY("moduleData", 232, 2, struct_ModuleData, NULL), YAML_END }; diff --git a/radio/src/storage/yaml/yaml_datastructs_x9d.cpp b/radio/src/storage/yaml/yaml_datastructs_x9d.cpp index ff690575c..3f5d78614 100644 --- a/radio/src/storage/yaml/yaml_datastructs_x9d.cpp +++ b/radio/src/storage/yaml/yaml_datastructs_x9d.cpp @@ -883,8 +883,9 @@ static const struct YamlNode struct_ModelData[] = { YAML_END }; static const struct YamlNode struct_PartialModel[] = { + YAML_CUSTOM("semver",nullptr,w_semver), YAML_STRUCT("header", 192, struct_ModelHeader, NULL), - YAML_ARRAY("timers", 136, 3, struct_TimerData, NULL), + YAML_ARRAY("moduleData", 232, 2, struct_ModuleData, NULL), YAML_END }; diff --git a/radio/src/storage/yaml/yaml_datastructs_x9dp2019.cpp b/radio/src/storage/yaml/yaml_datastructs_x9dp2019.cpp index ff690575c..3f5d78614 100644 --- a/radio/src/storage/yaml/yaml_datastructs_x9dp2019.cpp +++ b/radio/src/storage/yaml/yaml_datastructs_x9dp2019.cpp @@ -883,8 +883,9 @@ static const struct YamlNode struct_ModelData[] = { YAML_END }; static const struct YamlNode struct_PartialModel[] = { + YAML_CUSTOM("semver",nullptr,w_semver), YAML_STRUCT("header", 192, struct_ModelHeader, NULL), - YAML_ARRAY("timers", 136, 3, struct_TimerData, NULL), + YAML_ARRAY("moduleData", 232, 2, struct_ModuleData, NULL), YAML_END }; diff --git a/radio/src/storage/yaml/yaml_datastructs_x9e.cpp b/radio/src/storage/yaml/yaml_datastructs_x9e.cpp index 7cee0fadf..774550d85 100644 --- a/radio/src/storage/yaml/yaml_datastructs_x9e.cpp +++ b/radio/src/storage/yaml/yaml_datastructs_x9e.cpp @@ -884,8 +884,9 @@ static const struct YamlNode struct_ModelData[] = { YAML_END }; static const struct YamlNode struct_PartialModel[] = { + YAML_CUSTOM("semver",nullptr,w_semver), YAML_STRUCT("header", 192, struct_ModelHeader, NULL), - YAML_ARRAY("timers", 136, 3, struct_TimerData, NULL), + YAML_ARRAY("moduleData", 232, 2, struct_ModuleData, NULL), YAML_END }; diff --git a/radio/src/storage/yaml/yaml_datastructs_xlite.cpp b/radio/src/storage/yaml/yaml_datastructs_xlite.cpp index a00e82368..c86d9498d 100644 --- a/radio/src/storage/yaml/yaml_datastructs_xlite.cpp +++ b/radio/src/storage/yaml/yaml_datastructs_xlite.cpp @@ -882,8 +882,9 @@ static const struct YamlNode struct_ModelData[] = { YAML_END }; static const struct YamlNode struct_PartialModel[] = { + YAML_CUSTOM("semver",nullptr,w_semver), YAML_STRUCT("header", 96, struct_ModelHeader, NULL), - YAML_ARRAY("timers", 96, 3, struct_TimerData, NULL), + YAML_ARRAY("moduleData", 232, 2, struct_ModuleData, NULL), YAML_END }; diff --git a/radio/src/storage/yaml/yaml_datastructs_xlites.cpp b/radio/src/storage/yaml/yaml_datastructs_xlites.cpp index 0ec87c706..1351124a6 100644 --- a/radio/src/storage/yaml/yaml_datastructs_xlites.cpp +++ b/radio/src/storage/yaml/yaml_datastructs_xlites.cpp @@ -887,8 +887,9 @@ static const struct YamlNode struct_ModelData[] = { YAML_END }; static const struct YamlNode struct_PartialModel[] = { + YAML_CUSTOM("semver",nullptr,w_semver), YAML_STRUCT("header", 96, struct_ModelHeader, NULL), - YAML_ARRAY("timers", 96, 3, struct_TimerData, NULL), + YAML_ARRAY("moduleData", 232, 2, struct_ModuleData, NULL), YAML_END }; diff --git a/radio/src/tests/modelslist.cpp b/radio/src/tests/modelslist.cpp new file mode 100644 index 000000000..1d2bb3988 --- /dev/null +++ b/radio/src/tests/modelslist.cpp @@ -0,0 +1,166 @@ +/* + * Copyright (C) EdgeTX + * + * Based on code named + * opentx - https://github.com/opentx/opentx + * th9x - http://code.google.com/p/th9x + * er9x - http://code.google.com/p/er9x + * gruvin9x - http://code.google.com/p/gruvin9x + * + * License GPLv2: http://www.gnu.org/licenses/gpl-2.0.html + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +// Targeted tests for the PartialModel/ModelHeader restructuring in PR #7709 +// ("fix(color): modifying model labels may corrupt model yaml files"). +// +// These deliberately avoid FatFS/SD-card I/O and instead exercise the two +// things that PR touches which can silently drift out of sync across the +// 27 hand-edited yaml_datastructs_*.cpp files: the PartialModel YAML node +// table, and the strAppend() length constants used to write header.labels. + +#include "gtests.h" + +#include "storage/yaml/yaml_tree_walker.h" +#include "storage/yaml/yaml_parser.h" +#include "storage/yaml/yaml_datastructs.h" + +// Find an immediate child of an array/struct YamlNode by tag. +static const YamlNode* findChild(const YamlNode* root, const char* tag) +{ + const YamlNode* child = root->u._array.child; + for (; child->type != YDT_NONE; child++) { + if (child->tag && strcmp(child->tag, tag) == 0) return child; + } + return nullptr; +} + +// The PR hand-edited struct_PartialModel's "moduleData" YAML_ARRAY entry in +// 27 generated files to read `YAML_ARRAY("moduleData", , , +// struct_ModuleData, NULL)`, replacing the old "timers" entry. must +// equal sizeof(ModuleData)*8 and the element count must equal NUM_MODULES, +// or the YAML (de)serializer will misalign every field after moduleData in +// any file that uses struct_PartialModel (readModelYaml/writeModelLabels). +TEST(PartialModel, ModuleDataNodeMatchesStructLayout) +{ + const YamlNode* root = get_partialmodel_nodes(); + const YamlNode* moduleData = findChild(root, "moduleData"); + ASSERT_NE(moduleData, nullptr); + + EXPECT_EQ(moduleData->elmts, NUM_MODULES); + EXPECT_EQ(moduleData->size, sizeof(ModuleData) * 8); +} + +// Same idea for the "header" entry: its declared bit size must equal +// sizeof(ModelHeader)*8, or moduleData (and thus every array element after +// it) parses starting at the wrong bit offset. +TEST(PartialModel, HeaderNodeMatchesStructLayout) +{ + const YamlNode* root = get_partialmodel_nodes(); + const YamlNode* header = findChild(root, "header"); + ASSERT_NE(header, nullptr); + + EXPECT_EQ(header->size, sizeof(ModelHeader) * 8); +} + +// PartialModel must be layout-compatible with the start of ModelData, per +// the comment added at datastructs_private.h ModelData::header ("Must match +// start of PartialModel") -- readModelCell()/updateModelCell() rely on this +// so that data read into a PartialModel (a stack buffer) reflects the same +// bytes that would land in the corresponding fields of a full ModelData. +TEST(PartialModel, HeaderOffsetMatchesModelData) +{ + EXPECT_EQ(offsetof(PartialModel, header), offsetof(ModelData, header)); +} + +// End-to-end: parse the same YAML "header:" block once into a PartialModel +// and once into a full ModelData, and check they agree field-for-field. +// This is the strongest guard against the two generated tables silently +// drifting apart, because it doesn't hardcode any bit offsets itself -- it +// just requires both parsers to produce the same result for the same input. +TEST(PartialModel, HeaderParsesIdenticallyToModelData) +{ + static const char yaml[] = + "header: \n" + " name: \"Tst Name\"\n" + " modelId: [3, 5]\n" +#if LEN_BITMAP_NAME > 0 + " bitmap: \"pic.bmp\"\n" +#endif +#if defined(STORAGE_MODELSLIST) + " labels: \"alpha,bravo\"\n" +#endif + ; + + PartialModel partial; + memclear(&partial, sizeof(partial)); + { + YamlTreeWalker tree; + tree.reset(get_partialmodel_nodes(), (uint8_t*)&partial); + YamlParser yp; + yp.init(YamlTreeWalker::get_parser_calls(), &tree); + yp.parse(yaml, sizeof(yaml) - 1); + } + + ModelData model; + memclear(&model, sizeof(model)); + { + YamlTreeWalker tree; + tree.reset(get_modeldata_nodes(), (uint8_t*)&model); + YamlParser yp; + yp.init(YamlTreeWalker::get_parser_calls(), &tree); + yp.parse(yaml, sizeof(yaml) - 1); + } + + EXPECT_STREQ(partial.header.name, model.header.name); + EXPECT_EQ(0, memcmp(partial.header.modelId, model.header.modelId, + sizeof(model.header.modelId))); +#if LEN_BITMAP_NAME > 0 + EXPECT_STREQ(partial.header.bitmap, model.header.bitmap); +#endif +#if defined(STORAGE_MODELSLIST) + EXPECT_STREQ(partial.header.labels, model.header.labels); +#endif + + // Sanity: the fixture actually populated something non-zero, otherwise a + // parser that silently no-ops on both sides would pass trivially. + EXPECT_STREQ("Tst Name", partial.header.name); +} + +// Behavioural contract for ModelHeader::labels: it holds the CSV-joined +// list of *every* label attached to a model (declared as +// char[LABELS_LENGTH], currently 100 bytes) -- as distinct from the length +// of one individual label name (LABEL_LENGTH, 16 bytes). A model with +// several labels attached should keep all of them when this field gets +// (re)written, up to the field's own declared capacity, not just as much +// as would fit in a single label name. +// +// header.labels only exists on targets with STORAGE_MODELSLIST. +#if defined(STORAGE_MODELSLIST) +TEST(PartialModel, LabelsFieldRetainsFullCsvUpToItsOwnCapacity) +{ + PartialModel partial; + memclear(&partial, sizeof(partial)); + + // A multi-label CSV comfortably inside LABELS_LENGTH, but longer than a + // single LABEL_LENGTH-sized name -- i.e. more than one label attached. + const char* csv = "alpha,bravo,charlie,delta,echo"; + ASSERT_GT(strlen(csv), (size_t)LABEL_LENGTH); + ASSERT_LT(strlen(csv), (size_t)LABELS_LENGTH); + + strAppend(partial.header.labels, csv, LABELS_LENGTH - 1); + + EXPECT_STREQ(csv, partial.header.labels) + << "header.labels should retain the full label list up to " + "LABELS_LENGTH-1 characters, not just a single label name's " + "worth."; +} +#endif // defined(STORAGE_MODELSLIST)