From 50aa4914373134cbe92d4b213cc0522a353c6839 Mon Sep 17 00:00:00 2001 From: cjkas Date: Sat, 17 Oct 2026 22:53:18 +0200 Subject: [PATCH] fix: skip the room records too when a restore leaves the shades out A restore without "shades" skipped the shade and group records but not the room records, which come first, so it read the repeaters and the settings from room data. The skip is now shadeModelBytes(), in a ConfigHeader.h the native tests can reach, which counts rooms, shades and groups. Found during LOOP_QUEUE_PLAN.md's review. Co-Authored-By: Claude Opus 5.5 --- src/ConfigFile.cpp | 6 +-- src/ConfigFile.h | 17 +----- src/ConfigHeader.h | 33 ++++++++++++ test/test_configheader/test_configheader.cpp | 55 ++++++++++++++++++++ 4 files changed, 91 insertions(+), 20 deletions(-) create mode 100644 src/ConfigHeader.h create mode 100644 test/test_configheader/test_configheader.cpp diff --git a/src/ConfigFile.cpp b/src/ConfigFile.cpp index 05622c6..52bc09c 100644 --- a/src/ConfigFile.cpp +++ b/src/ConfigFile.cpp @@ -574,10 +574,8 @@ bool ShadeConfigFile::restoreFile(SomfyShadeController *s, const char *filename, } else { ESP_LOGI(TAG, "Shade data ignored"); - // FF past the shades and groups. - this->file.seek(this->file.position() - + (this->header.shadeRecords * this->header.shadeRecordSize) - + (this->header.groupRecords * this->header.groupRecordSize), SeekSet); // Start at the beginning of the file after the header. + // FF past the rooms, shades and groups. + this->file.seek(this->file.position() + shadeModelBytes(this->header), SeekSet); } if(opts.repeaters) { ESP_LOGI(TAG, "Restoring Repeaters..."); diff --git a/src/ConfigFile.h b/src/ConfigFile.h index 011e7a9..3759ba2 100644 --- a/src/ConfigFile.h +++ b/src/ConfigFile.h @@ -2,6 +2,7 @@ #include #include "Somfy.h" #include "ConfigSettings.h" +#include "ConfigHeader.h" #ifndef configfile_h #define configfile_h @@ -12,22 +13,6 @@ #define CFG_TOK_QUOTE '"' -struct config_header_t { - uint8_t version = 1; - uint8_t repeaterRecords = 0; - uint8_t repeaterRecordSize = 0; - uint8_t roomRecords = 0; - uint16_t roomRecordSize = 0; - uint16_t shadeRecordSize = 0; - uint8_t shadeRecords = 0; - uint16_t groupRecordSize = 0; - uint8_t groupRecords = 0; - uint16_t settingsRecordSize = 0; - uint16_t netRecordSize = 0; - uint16_t transRecordSize = 0; - char serverId[10] = ""; // This must match the server id size in the ConfigSettings. - int8_t length = 0; -}; class ConfigFile { protected: File file; diff --git a/src/ConfigHeader.h b/src/ConfigHeader.h new file mode 100644 index 0000000..bf21a34 --- /dev/null +++ b/src/ConfigHeader.h @@ -0,0 +1,33 @@ +#ifndef CONFIG_HEADER_H +#define CONFIG_HEADER_H +#include + +// The header of a shades.cfg file: its version and how many records of each +// kind follow, and their sizes. Kept free of the file system so it can be +// unit tested on the PC (pio test -e native). +struct config_header_t { + uint8_t version = 1; + uint8_t repeaterRecords = 0; + uint8_t repeaterRecordSize = 0; + uint8_t roomRecords = 0; + uint16_t roomRecordSize = 0; + uint16_t shadeRecordSize = 0; + uint8_t shadeRecords = 0; + uint16_t groupRecordSize = 0; + uint8_t groupRecords = 0; + uint16_t settingsRecordSize = 0; + uint16_t netRecordSize = 0; + uint16_t transRecordSize = 0; + char serverId[10] = ""; // This must match the server id size in the ConfigSettings. + int8_t length = 0; +}; + +// The bytes of the shade model's records, which follow the header in this +// order: rooms, shades, groups. A restore that leaves the shades out skips +// this many bytes. +inline uint32_t shadeModelBytes(const config_header_t &header) { + return static_cast(header.roomRecords) * header.roomRecordSize + + static_cast(header.shadeRecords) * header.shadeRecordSize + + static_cast(header.groupRecords) * header.groupRecordSize; +} +#endif diff --git a/test/test_configheader/test_configheader.cpp b/test/test_configheader/test_configheader.cpp new file mode 100644 index 0000000..1b37ef2 --- /dev/null +++ b/test/test_configheader/test_configheader.cpp @@ -0,0 +1,55 @@ +#include +#include "ConfigHeader.h" + +void setUp() {} +void tearDown() {} + +static config_header_t header() { + config_header_t h; + h.version = 22; + h.roomRecords = 3; + h.roomRecordSize = 29; + h.shadeRecords = 5; + h.shadeRecordSize = 276; + h.groupRecords = 2; + h.groupRecordSize = 200; + h.repeaterRecords = 1; + h.repeaterRecordSize = 77; + h.settingsRecordSize = 100; + return h; +} + +static void test_shade_model_counts_rooms_shades_and_groups() { + // A restore that leaves the shades out has to skip the rooms too, or it + // reads the repeaters and settings from the room records. + TEST_ASSERT_EQUAL_UINT32(3 * 29 + 5 * 276 + 2 * 200, shadeModelBytes(header())); +} +static void test_shade_model_leaves_out_the_rest() { + config_header_t h = header(); + const uint32_t before = shadeModelBytes(h); + h.repeaterRecordSize = 1; + h.settingsRecordSize = 1; + h.netRecordSize = 1; + h.transRecordSize = 1; + TEST_ASSERT_EQUAL_UINT32(before, shadeModelBytes(h)); +} +static void test_an_old_file_without_rooms() { + config_header_t h = header(); + h.roomRecords = 0; + TEST_ASSERT_EQUAL_UINT32(5 * 276 + 2 * 200, shadeModelBytes(h)); +} +static void test_a_full_model_does_not_overflow() { + config_header_t h; + h.roomRecords = h.shadeRecords = h.groupRecords = 255; + h.roomRecordSize = h.shadeRecordSize = h.groupRecordSize = 0xFFFF; + TEST_ASSERT_EQUAL_UINT32(3UL * 255 * 0xFFFF, shadeModelBytes(h)); +} + +int main(int argc, char **argv) { + UNITY_BEGIN(); + RUN_TEST(test_shade_model_counts_rooms_shades_and_groups); + RUN_TEST(test_shade_model_leaves_out_the_rest); + RUN_TEST(test_an_old_file_without_rooms); + RUN_TEST(test_a_full_model_does_not_overflow); + return UNITY_END(); +}