From 92cd54f097318aa01b2500d89046db7a88ce3630 Mon Sep 17 00:00:00 2001 From: cjkas Date: Thu, 29 Oct 2026 17:25:07 +0100 Subject: [PATCH] fix: review fixes for the shades.cfg records - A group in a backup from before version 23 has no rolling code, and a restore wrote 0 to NVS for it. It keeps the code NVS has. - A restore reads each record into one that starts as the shade, group or settings it replaces, so what an older version lacks stays as it was (the update check below version 20 among them), as before. - restoreFile and loadFile read the rooms, shades and groups through one readModel(); the settings, network and radio are applied by helpers. - The shade's tilt is forgotten where the shades are written, not in the conversion to a record. - rollingCodeAfterLoad moves to RollingCode.h and its tests to test_rollingcode. - The topics a deleted shade or group clears are one list each (MqttTopics.h), tested against what is published. - e2e: a restore of the shades brings back every shade and group, and the README has the rollback check by hand. Co-Authored-By: Claude Opus 5.5 --- src/ConfigFile.cpp | 162 ++++++++++-------- src/ConfigFile.h | 1 + src/ConfigRecords.cpp | 9 +- src/ConfigRecords.h | 16 +- src/MqttTopics.h | 19 ++ src/RollingCode.h | 13 +- src/outputs/MqttPublisher.cpp | 31 +--- test/e2e/README.md | 7 + test/e2e/test_restore.py | 19 ++ test/e2e/test_update.py | 4 + .../test_configrecords/test_configrecords.cpp | 103 ++++++++--- test/test_mqttdisco/test_mqttdisco.cpp | 29 ++++ test/test_rollingcode/test_rollingcode.cpp | 12 ++ 13 files changed, 277 insertions(+), 148 deletions(-) create mode 100644 src/MqttTopics.h diff --git a/src/ConfigFile.cpp b/src/ConfigFile.cpp index 69f6cde..27926cd 100644 --- a/src/ConfigFile.cpp +++ b/src/ConfigFile.cpp @@ -5,6 +5,7 @@ #include "ConfigFile.h" #include "Utils.h" #include "ConfigSettings.h" +#include "RollingCode.h" static const char *TAG = "ConfigFile"; @@ -73,10 +74,6 @@ static room_record_t roomRecord(const SomfyRoom *room) { return rec; } static shade_record_t shadeRecord(SomfyShade *shade) { - if(shade->tiltType == tilt_types::none || shade->shadeType != shade_types::blind) { - shade->clearTilt(); - shade->tiltType = tilt_types::none; - } shade_record_t rec; rec.shadeId = shade->getShadeId(); rec.paired = shade->paired; @@ -178,6 +175,62 @@ static trans_record_t transRecord(const transceiver_config_t &cfg) { rec.noiseDetection = cfg.noiseDetection; return rec; } +static void applySettingsRecord(const settings_record_t &rec) { + strlcpy(settings.hostname, rec.hostname, sizeof(settings.hostname)); + strlcpy(settings.NTP.ntpServer, rec.ntpServer, sizeof(settings.NTP.ntpServer)); + strlcpy(settings.NTP.posixZone, rec.posixZone, sizeof(settings.NTP.posixZone)); + settings.ssdpBroadcast = rec.ssdpBroadcast; + settings.checkForUpdate = rec.checkForUpdate; +} +static void applyNetRecord(const net_record_t &rec, const restore_options_t &opts, bool sameBoard) { + if(opts.network) { + settings.connType = static_cast(rec.connType); + settings.IP.dhcp = rec.dhcp; + settings.IP.ip.fromString(rec.ip); + settings.IP.gateway.fromString(rec.gateway); + settings.IP.subnet.fromString(rec.subnet); + settings.IP.dns1.fromString(rec.dns1); + settings.IP.dns2.fromString(rec.dns2); +#if HAS_ETHERNET + if(sameBoard) { + ESP_LOGI(TAG, "Restoring Ethernet adapter settings"); + settings.Ethernet.boardType = rec.ethBoardType; + settings.Ethernet.phyType = rec.ethPhyType; + settings.Ethernet.CLKMode = static_cast(rec.ethClkMode); + settings.Ethernet.phyAddress = rec.ethPhyAddress; + settings.Ethernet.PWRPin = rec.ethPWRPin; + settings.Ethernet.MDCPin = rec.ethMDCPin; + settings.Ethernet.MDIOPin = rec.ethMDIOPin; + } +#else + (void)sameBoard; +#endif + } + if(opts.mqtt) { + strlcpy(settings.MQTT.protocol, rec.mqttProtocol, sizeof(settings.MQTT.protocol)); + strlcpy(settings.MQTT.hostname, rec.mqttHostname, sizeof(settings.MQTT.hostname)); + settings.MQTT.port = rec.mqttPort; + settings.MQTT.pubDisco = rec.mqttPubDisco; + strlcpy(settings.MQTT.rootTopic, rec.mqttRootTopic, sizeof(settings.MQTT.rootTopic)); + strlcpy(settings.MQTT.discoTopic, rec.mqttDiscoTopic, sizeof(settings.MQTT.discoTopic)); + } +} +static void applyTransRecord(const trans_record_t &rec, transceiver_config_t &cfg) { + cfg.enabled = rec.enabled; + cfg.proto = rec.proto; + cfg.type = rec.type; + cfg.SCKPin = rec.SCKPin; + cfg.CSNPin = rec.CSNPin; + cfg.MOSIPin = rec.MOSIPin; + cfg.MISOPin = rec.MISOPin; + cfg.TXPin = rec.TXPin; + cfg.RXPin = rec.RXPin; + cfg.frequency = rec.frequency; + cfg.rxBandwidth = rec.rxBandwidth; + cfg.deviation = rec.deviation; + cfg.txPower = rec.txPower; + cfg.noiseDetection = rec.noiseDetection; +} // NVS keeps the last rolling code each remote sent. static uint16_t keepRollingCode(Preferences &pref, SomfyRemote *remote, uint16_t inFile, bool restoring) { const uint16_t stored = pref.getUShort(remote->getRemotePrefId(), 0); @@ -187,14 +240,14 @@ static uint16_t keepRollingCode(Preferences &pref, SomfyRemote *remote, uint16_t } void ShadeConfigFile::readRoom(SomfyRoom *room) { - room_record_t rec; - readRoomRecord(*this, this->header, rec); + room_record_t rec = roomRecord(room); + readRoomRecord(*this, rec); room->roomId = rec.roomId; strlcpy(room->name, rec.name, sizeof(room->name)); room->sortOrder = rec.sortOrder; } void ShadeConfigFile::readShade(SomfyShade *shade) { - shade_record_t rec; + shade_record_t rec = shadeRecord(shade); readShadeRecord(*this, this->header, rec); if(rec.shadeId == 255) { shade->clear(); @@ -242,7 +295,7 @@ void ShadeConfigFile::readShade(SomfyShade *shade) { pinMode(shade->gpioMy, OUTPUT); } void ShadeConfigFile::readGroup(SomfyGroup *group) { - group_record_t rec; + group_record_t rec = groupRecord(group); readGroupRecord(*this, this->header, rec); if(rec.groupId == 255) { group->clear(); @@ -264,7 +317,8 @@ void ShadeConfigFile::readGroup(SomfyGroup *group) { if(group->getRemoteAddress() != 0) { Preferences pref; pref.begin("ShadeCodes"); - group->lastRollingCode = keepRollingCode(pref, group, rec.lastRollingCode, this->restoring); + // A backup from before version 23 has no code for the group to restore. + group->lastRollingCode = keepRollingCode(pref, group, rec.lastRollingCode, this->restoring && rec.hasRollingCode); pref.end(); } } @@ -274,6 +328,15 @@ void ShadeConfigFile::readRepeaters(SomfyShadeController *s) { for(uint8_t i = 0; i < SOMFY_MAX_REPEATERS; i++) s->linkRepeater(repeaters[i]); } +// The rooms, shades and groups, in place of the ones the controller has. +void ShadeConfigFile::readModel(SomfyShadeController *s) { + for(uint8_t i = 0; i < this->header.roomRecords; i++) this->readRoom(&s->rooms[i]); + for(uint8_t i = this->header.roomRecords; i < SOMFY_MAX_ROOMS; i++) s->rooms[i].clear(); + for(uint8_t i = 0; i < this->header.shadeRecords; i++) this->readShade(&s->shades[i]); + for(uint8_t i = this->header.shadeRecords; i < SOMFY_MAX_SHADES; i++) s->shades[i].clear(); + for(uint8_t i = 0; i < this->header.groupRecords; i++) this->readGroup(&s->groups[i]); + for(uint8_t i = this->header.groupRecords; i < SOMFY_MAX_GROUPS; i++) s->groups[i].clear(); +} bool ShadeConfigFile::begin(bool readOnly) { return this->begin("/shades.cfg", readOnly); } bool ShadeConfigFile::begin(const char *filename, bool readOnly) { return ConfigFile::begin(filename, readOnly); } void ShadeConfigFile::end() { ConfigFile::end(); } @@ -296,7 +359,13 @@ bool ShadeConfigFile::writeModel(SomfyShadeController *s) { } for(uint8_t i = 0; i < SOMFY_MAX_SHADES; i++) { SomfyShade *shade = &s->shades[i]; - if(shade->getShadeId() != 255) writeShadeRecord(*this, shadeRecord(shade)); + if(shade->getShadeId() == 255) continue; + // Only a blind tilts: the others forget any tilt they were given. + if(shade->tiltType == tilt_types::none || shade->shadeType != shade_types::blind) { + shade->clearTilt(); + shade->tiltType = tilt_types::none; + } + writeShadeRecord(*this, shadeRecord(shade)); } for(uint8_t i = 0; i < SOMFY_MAX_GROUPS; i++) { SomfyGroup *group = &s->groups[i]; @@ -459,15 +528,8 @@ bool ShadeConfigFile::restoreFile(SomfyShadeController *s, const char *filename, } this->restoring = true; if(opts.shades) { - ESP_LOGI(TAG, "Restoring Rooms..."); - for(uint8_t i = 0; i < this->header.roomRecords; i++) this->readRoom(&s->rooms[i]); - for(uint8_t i = this->header.roomRecords; i < SOMFY_MAX_ROOMS; i++) s->rooms[i].clear(); - ESP_LOGI(TAG, "Restoring Shades..."); - for(uint8_t i = 0; i < this->header.shadeRecords; i++) this->readShade(&s->shades[i]); - for(uint8_t i = this->header.shadeRecords; i < SOMFY_MAX_SHADES; i++) s->shades[i].clear(); - ESP_LOGI(TAG, "Restoring Groups..."); - for(uint8_t i = 0; i < this->header.groupRecords; i++) this->readGroup(&s->groups[i]); - for(uint8_t i = this->header.groupRecords; i < SOMFY_MAX_GROUPS; i++) s->groups[i].clear(); + ESP_LOGI(TAG, "Restoring the rooms, shades and groups"); + this->readModel(s); } else { ESP_LOGI(TAG, "Shade data ignored"); @@ -481,13 +543,9 @@ bool ShadeConfigFile::restoreFile(SomfyShadeController *s, const char *filename, else skipRecords(*this, this->header.repeaterRecords); if(this->header.settingsRecordSize > 0) { if(opts.settings) { - settings_record_t rec; + settings_record_t rec = settingsRecord(); readSettingsRecord(*this, this->header, rec); - strlcpy(settings.hostname, rec.hostname, sizeof(settings.hostname)); - strlcpy(settings.NTP.ntpServer, rec.ntpServer, sizeof(settings.NTP.ntpServer)); - strlcpy(settings.NTP.posixZone, rec.posixZone, sizeof(settings.NTP.posixZone)); - settings.ssdpBroadcast = rec.ssdpBroadcast; - settings.checkForUpdate = rec.checkForUpdate; + applySettingsRecord(rec); } else skipRecords(*this, 1); } @@ -495,37 +553,9 @@ bool ShadeConfigFile::restoreFile(SomfyShadeController *s, const char *filename, if(opts.network || opts.mqtt) { // Only the board that wrote the Ethernet settings takes them back. const bool sameBoard = HAS_ETHERNET && strncmp(settings.serverId, this->header.serverId, sizeof(settings.serverId)) == 0; - net_record_t rec; + net_record_t rec = netRecord(); readNetRecord(*this, this->header, rec, sameBoard); - if(opts.network) { - settings.connType = static_cast(rec.connType); - settings.IP.dhcp = rec.dhcp; - settings.IP.ip.fromString(rec.ip); - settings.IP.gateway.fromString(rec.gateway); - settings.IP.subnet.fromString(rec.subnet); - settings.IP.dns1.fromString(rec.dns1); - settings.IP.dns2.fromString(rec.dns2); -#if HAS_ETHERNET - if(sameBoard) { - ESP_LOGI(TAG, "Restoring Ethernet adapter settings"); - settings.Ethernet.boardType = rec.ethBoardType; - settings.Ethernet.phyType = rec.ethPhyType; - settings.Ethernet.CLKMode = static_cast(rec.ethClkMode); - settings.Ethernet.phyAddress = rec.ethPhyAddress; - settings.Ethernet.PWRPin = rec.ethPWRPin; - settings.Ethernet.MDCPin = rec.ethMDCPin; - settings.Ethernet.MDIOPin = rec.ethMDIOPin; - } -#endif - } - if(opts.mqtt && this->header.version >= 22) { - strlcpy(settings.MQTT.protocol, rec.mqttProtocol, sizeof(settings.MQTT.protocol)); - strlcpy(settings.MQTT.hostname, rec.mqttHostname, sizeof(settings.MQTT.hostname)); - settings.MQTT.port = rec.mqttPort; - settings.MQTT.pubDisco = rec.mqttPubDisco; - strlcpy(settings.MQTT.rootTopic, rec.mqttRootTopic, sizeof(settings.MQTT.rootTopic)); - strlcpy(settings.MQTT.discoTopic, rec.mqttDiscoTopic, sizeof(settings.MQTT.discoTopic)); - } + applyNetRecord(rec, opts, sameBoard); } else skipRecords(*this, 1); } @@ -537,20 +567,7 @@ bool ShadeConfigFile::restoreFile(SomfyShadeController *s, const char *filename, trans_record_t rec = transRecord(s->transceiver.config); readTransRecord(*this, rec); transceiver_config_t cfg = s->transceiver.config; - cfg.enabled = rec.enabled; - cfg.proto = rec.proto; - cfg.type = rec.type; - cfg.SCKPin = rec.SCKPin; - cfg.CSNPin = rec.CSNPin; - cfg.MOSIPin = rec.MOSIPin; - cfg.MISOPin = rec.MISOPin; - cfg.TXPin = rec.TXPin; - cfg.RXPin = rec.RXPin; - cfg.frequency = rec.frequency; - cfg.rxBandwidth = rec.rxBandwidth; - cfg.deviation = rec.deviation; - cfg.txPower = rec.txPower; - cfg.noiseDetection = rec.noiseDetection; + applyTransRecord(rec, cfg); s->transceiver.setConfig(cfg); } if(opts.settings || opts.network) settings.save(); @@ -578,12 +595,7 @@ bool ShadeConfigFile::loadFile(SomfyShadeController *s, const char *filename) { return false; } this->restoring = false; - for(uint8_t i = 0; i < this->header.roomRecords; i++) this->readRoom(&s->rooms[i]); - for(uint8_t i = this->header.roomRecords; i < SOMFY_MAX_ROOMS; i++) s->rooms[i].clear(); - for(uint8_t i = 0; i < this->header.shadeRecords; i++) this->readShade(&s->shades[i]); - for(uint8_t i = this->header.shadeRecords; i < SOMFY_MAX_SHADES; i++) s->shades[i].clear(); - for(uint8_t i = 0; i < this->header.groupRecords; i++) this->readGroup(&s->groups[i]); - for(uint8_t i = this->header.groupRecords; i < SOMFY_MAX_GROUPS; i++) s->groups[i].clear(); + this->readModel(s); if(this->header.repeaterRecords > 0) { memset(s->repeaters, 0x00, sizeof(s->repeaters)); for(uint8_t i = 0; i < this->header.repeaterRecords; i++) this->readRepeaters(s); diff --git a/src/ConfigFile.h b/src/ConfigFile.h index 3e8777c..cbfc8ee 100644 --- a/src/ConfigFile.h +++ b/src/ConfigFile.h @@ -33,6 +33,7 @@ class ShadeConfigFile : public ConfigFile { // A restore takes the rolling codes in the file (rollingCodeAfterLoad). bool restoring = false; bool writeModel(SomfyShadeController *s); + void readModel(SomfyShadeController *s); void readRoom(SomfyRoom *room); void readShade(SomfyShade *shade); void readGroup(SomfyGroup *group); diff --git a/src/ConfigRecords.cpp b/src/ConfigRecords.cpp index 6ba37b8..a73a7bf 100644 --- a/src/ConfigRecords.cpp +++ b/src/ConfigRecords.cpp @@ -66,11 +66,6 @@ bool ConfigReader::readVarString(char *buff, size_t len) { } return false; } -char ConfigReader::readChar(const char defVal) { - uint8_t ch; - if(this->next(ch)) return (char)ch; - return defVal; -} int8_t ConfigReader::readInt8(const int8_t defVal) { char buff[5]; if(this->readString(buff, sizeof(buff))) return static_cast(atoi(buff)); @@ -213,8 +208,7 @@ bool writeRoomRecord(ConfigWriter &w, const room_record_t &rec) { w.writeString(rec.name, sizeof(rec.name)); return w.writeUInt8(static_cast(rec.sortOrder), CFG_REC_END); } -bool readRoomRecord(ConfigReader &r, const config_header_t &header, room_record_t &rec) { - (void)header; +bool readRoomRecord(ConfigReader &r, room_record_t &rec) { r.beginRecord(); rec.roomId = r.readUInt8(0); r.readFixedString(rec.name, sizeof(rec.name)); @@ -349,6 +343,7 @@ bool readGroupRecord(ConfigReader &r, const config_header_t &header, group_recor rec.bitLength = r.readUInt8(56); // Version 23 put the rolling code here, version 24 at the end. if(ver == 23) rec.lastRollingCode = r.readUInt16(0); + rec.hasRollingCode = ver >= 23; memset(rec.linkedShades, 0x00, sizeof(rec.linkedShades)); uint8_t linked = 0; for(uint8_t j = 0; j < SOMFY_MAX_GROUPED_SHADES; j++) { diff --git a/src/ConfigRecords.h b/src/ConfigRecords.h index 462c367..54c6b73 100644 --- a/src/ConfigRecords.h +++ b/src/ConfigRecords.h @@ -61,7 +61,6 @@ class ConfigReader { bool readFixedString(char *buff, size_t len); // A quoted value of any length, cut to fit buff. bool readVarString(char *buff, size_t len); - char readChar(const char defVal = '\0'); int8_t readInt8(const int8_t defVal = 0); uint8_t readUInt8(const uint8_t defVal = 0); uint16_t readUInt16(const uint16_t defVal = 0); @@ -139,6 +138,8 @@ struct group_record_t { bool flipCommands = false; uint8_t roomId = 0; uint16_t lastRollingCode = 0; + // Versions before 23 have none. + bool hasRollingCode = false; }; struct settings_record_t { // The firmware that wrote the backup. Not restored. @@ -190,10 +191,12 @@ struct trans_record_t { bool noiseDetection = false; }; +// The readers leave alone what the record's version lacks, so a record that +// starts as the shade, group or settings it is read into keeps their values. bool writeConfigHeader(ConfigWriter &w, const config_header_t &header); bool readConfigHeader(ConfigReader &r, config_header_t &header); bool writeRoomRecord(ConfigWriter &w, const room_record_t &rec); -bool readRoomRecord(ConfigReader &r, const config_header_t &header, room_record_t &rec); +bool readRoomRecord(ConfigReader &r, room_record_t &rec); bool writeShadeRecord(ConfigWriter &w, const shade_record_t &rec); bool readShadeRecord(ConfigReader &r, const config_header_t &header, shade_record_t &rec); bool writeGroupRecord(ConfigWriter &w, const group_record_t &rec); @@ -210,13 +213,4 @@ bool readTransRecord(ConfigReader &r, trans_record_t &rec); // Moves past count records without reading them. bool skipRecords(ConfigReader &r, uint32_t count); -// The rolling code a remote goes on with. NVS holds the last code sent and -// shades.cfg the code when it was saved, so a load keeps the higher. A -// restore takes the backup's code even when it is lower. A motor ignores -// codes below the last one it heard, so after restoring an older backup to -// the same device it ignores the shade until the codes pass that one. -inline uint16_t rollingCodeAfterLoad(uint16_t stored, uint16_t inFile, bool restoring) { - if(restoring) return inFile; - return stored > inFile ? stored : inFile; -} #endif diff --git a/src/MqttTopics.h b/src/MqttTopics.h new file mode 100644 index 0000000..0a9cc5d --- /dev/null +++ b/src/MqttTopics.h @@ -0,0 +1,19 @@ +#ifndef MQTT_TOPICS_H +#define MQTT_TOPICS_H +#include + +// The topics the broker keeps for a shade (shades//) and a group +// (groups//), which deleting one clears. Kept free of MQTT so the lists +// can be checked against what is published in the unit tests on the PC +// (pio test -e native). +inline constexpr const char *SHADE_TOPICS[] = { + "shadeId", "name", "remoteAddress", "shadeType", "tiltType", "flags", "flipCommands", "flipPosition", + "position", "direction", "target", "lastRollingCode", "mypos", "myTiltPos", + "tiltDirection", "tiltPosition", "tiltTarget", "windy", "sunny" +}; +inline constexpr const char *GROUP_TOPICS[] = { + "groupId", "name", "remoteAddress", "groupType", "direction", "lastRollingCode", "flags", "sunSensor", "flipCommands" +}; +inline constexpr size_t SHADE_TOPIC_COUNT = sizeof(SHADE_TOPICS) / sizeof(SHADE_TOPICS[0]); +inline constexpr size_t GROUP_TOPIC_COUNT = sizeof(GROUP_TOPICS) / sizeof(GROUP_TOPICS[0]); +#endif diff --git a/src/RollingCode.h b/src/RollingCode.h index 7eb58fe..a6e1c27 100644 --- a/src/RollingCode.h +++ b/src/RollingCode.h @@ -2,7 +2,8 @@ #define ROLLING_CODE_H #include -// Working out a remote's next rolling code (src/SomfyRemote.cpp). Kept free of NVS +// Working out a remote's next rolling code (src/SomfyRemote.cpp) and the one +// it goes on with after a load (src/ConfigFile.cpp). Kept free of NVS // so it can be unit tested on the PC (pio test -e native). // The code the next frame uses. It is only read here and consumed once the @@ -13,4 +14,14 @@ template inline uint16_t nextRollingCode(bool synced, uint16_t last, LoadStored loadStored) { return (uint16_t)((synced ? last : loadStored()) + 1); } +// The rolling code a remote goes on with after shades.cfg or a backup is +// read. NVS holds the last code sent and shades.cfg the code when it was +// saved, so a load keeps the higher. A restore takes the backup's code even +// when it is lower. A motor ignores codes below the last one it heard, so +// after restoring an older backup to the same device it ignores the remote +// until the codes pass that one. +inline uint16_t rollingCodeAfterLoad(uint16_t stored, uint16_t inFile, bool restoring) { + if(restoring) return inFile; + return stored > inFile ? stored : inFile; +} #endif diff --git a/src/outputs/MqttPublisher.cpp b/src/outputs/MqttPublisher.cpp index 43928a0..51f11f3 100644 --- a/src/outputs/MqttPublisher.cpp +++ b/src/outputs/MqttPublisher.cpp @@ -4,6 +4,7 @@ #include "SomfySnapshot.h" #include "MqttConnection.h" #include "MqttDisco.h" +#include "MqttTopics.h" extern SomfyShadeController somfy; extern ConfigSettings settings; @@ -118,25 +119,7 @@ void SomfyGroup::unpublish() { SomfyGroup::unpublish(this->groupId); } void SomfyShade::unpublish() { SomfyShade::unpublish(this->shadeId); } void SomfyShade::unpublish(uint8_t id) { if(mqtt.connected()) { - SomfyShade::unpublish(id, "shadeId"); - SomfyShade::unpublish(id, "name"); - SomfyShade::unpublish(id, "remoteAddress"); - SomfyShade::unpublish(id, "shadeType"); - SomfyShade::unpublish(id, "tiltType"); - SomfyShade::unpublish(id, "flags"); - SomfyShade::unpublish(id, "flipCommands"); - SomfyShade::unpublish(id, "flipPosition"); - SomfyShade::unpublish(id, "position"); - SomfyShade::unpublish(id, "direction"); - SomfyShade::unpublish(id, "target"); - SomfyShade::unpublish(id, "lastRollingCode"); - SomfyShade::unpublish(id, "mypos"); - SomfyShade::unpublish(id, "myTiltPos"); - SomfyShade::unpublish(id, "tiltDirection"); - SomfyShade::unpublish(id, "tiltPosition"); - SomfyShade::unpublish(id, "tiltTarget"); - SomfyShade::unpublish(id, "windy"); - SomfyShade::unpublish(id, "sunny"); + for(const char *topic : SHADE_TOPICS) SomfyShade::unpublish(id, topic); if(settings.MQTT.pubDisco) { char topic[128] = ""; shadeDiscoTopic(settings.MQTT.discoTopic, id, false, topic, sizeof(topic)); @@ -148,15 +131,7 @@ void SomfyShade::unpublish(uint8_t id) { } void SomfyGroup::unpublish(uint8_t id) { if(mqtt.connected()) { - SomfyGroup::unpublish(id, "groupId"); - SomfyGroup::unpublish(id, "name"); - SomfyGroup::unpublish(id, "remoteAddress"); - SomfyGroup::unpublish(id, "groupType"); - SomfyGroup::unpublish(id, "direction"); - SomfyGroup::unpublish(id, "lastRollingCode"); - SomfyGroup::unpublish(id, "flags"); - SomfyGroup::unpublish(id, "sunSensor"); - SomfyGroup::unpublish(id, "flipCommands"); + for(const char *topic : GROUP_TOPICS) SomfyGroup::unpublish(id, topic); } } void SomfyGroup::unpublish(uint8_t id, const char *topic) { diff --git a/test/e2e/README.md b/test/e2e/README.md index 4bdf4bd..f52a207 100644 --- a/test/e2e/README.md +++ b/test/e2e/README.md @@ -21,4 +21,11 @@ A plain run sends nothing on air, changes no setting (it saves only values the d The web UI holds one of the device's 5 socket slots for each open tab, and Home Assistant holds one. The tests that fill every slot fill the ones left, so close the UI's tabs for the full load. +A rollback is checked by hand, on a release build with an OTA slot: + +1. Note the version the web UI shows. +2. Build firmware that restarts before it is marked good: add `abort();` at the end of `setup()` in `src/SomfyController.cpp`, and change `data-src/appversion` so the two versions differ. +3. Update to it through the web UI, with the serial monitor open. +4. The crash restarts the device, and the bootloader boots the old firmware ([ADR 0003](../../docs/adr/0003-pioarduino-idf55.md)). The web UI shows the version from step 1 again. + The tests run in whatever security mode the device is in; the suite never changes it. To test both modes, run the suite once in each. diff --git a/test/e2e/test_restore.py b/test/e2e/test_restore.py index 45e9ff9..d3845ce 100644 --- a/test/e2e/test_restore.py +++ b/test/e2e/test_restore.py @@ -91,3 +91,22 @@ def test_a_restore_of_the_radio_alone_brings_it_back_as_it_was(request, host, to wait_for_restart(host, token, schema, booted) c, _, _ = synced_model(connect, schema) assert settings(c, schema, "radio") == radio + + +def test_a_restore_of_the_shades_brings_them_back_as_they_were(request, host, token, connect, schema): + """Every shade and group, their rolling codes included, from a backup taken a moment before.""" + if not request.config.getoption("--reboot"): + pytest.skip("restores a backup and reboots the device: pass --reboot") + _, shades, groups = synced_model(connect, schema) + _, frames, _ = run_sync(connect(schema=schema.hash)) + booted = boot_time(schema, frames) + status, _, backup = http(host, "/backup", headers=auth(token)) + assert status == 200 + + options = json.dumps({"shades": True}) + status, _, body = upload(host, "/restore", auth(token), file=backup, fields={"data": options}) + assert status == 200, body + wait_for_restart(host, token, schema, booted) + _, shades_after, groups_after = synced_model(connect, schema) + assert shades_after == shades + assert groups_after == groups diff --git a/test/e2e/test_update.py b/test/e2e/test_update.py index d3c1a2c..56323ef 100644 --- a/test/e2e/test_update.py +++ b/test/e2e/test_update.py @@ -3,6 +3,10 @@ Pass --firmware with a firmware.bin built for the device's chip, e.g. .pio/build/esp32s3/firmware.bin. The device must have an OTA slot: the release builds do, the C5, C6 and debug builds don't. + +It can't tell the new firmware from the old when both have the same +version, and it doesn't make the update fail. test/e2e/README.md has the +rollback check by hand. """ import time diff --git a/test/test_configrecords/test_configrecords.cpp b/test/test_configrecords/test_configrecords.cpp index c8e741f..7d9900a 100644 --- a/test/test_configrecords/test_configrecords.cpp +++ b/test/test_configrecords/test_configrecords.cpp @@ -272,7 +272,7 @@ static void test_a_room_reads_back() { writeRoomRecord(w, room); TextReader r(w.text); room_record_t got; - readRoomRecord(r, header(), got); + readRoomRecord(r, got); TEST_ASSERT_EQUAL_UINT8(2, got.roomId); TEST_ASSERT_EQUAL_STRING("Living room", got.name); TEST_ASSERT_EQUAL_INT8(5, got.sortOrder); @@ -401,7 +401,7 @@ static void test_the_longest_name_reads_back() { writeRoomRecord(w, room); TextReader r(w.text); room_record_t got; - readRoomRecord(r, header(), got); + readRoomRecord(r, got); TEST_ASSERT_EQUAL_STRING("12345678901234567890", got.name); TEST_ASSERT_EQUAL_INT8(7, got.sortOrder); } @@ -506,8 +506,8 @@ static void test_values_a_newer_version_added_are_skipped() { writeRoomRecord(w, room_record_t{2, "Attic", 1}); TextReader r(w.text); room_record_t got; - readRoomRecord(r, header(), got); - readRoomRecord(r, header(), got); + readRoomRecord(r, got); + readRoomRecord(r, got); TEST_ASSERT_EQUAL_UINT8(2, got.roomId); TEST_ASSERT_EQUAL_STRING("Attic", got.name); } @@ -522,11 +522,11 @@ static void test_a_short_record_never_reads_the_next() { writeRoomRecord(w, room_record_t{2, "Attic", 1}); TextReader r(text + w.text); room_record_t got; - readRoomRecord(r, header(), got); + readRoomRecord(r, got); TEST_ASSERT_EQUAL_UINT8(1, got.roomId); TEST_ASSERT_EQUAL_STRING("Hall", got.name); TEST_ASSERT_EQUAL_INT8(0, got.sortOrder); - readRoomRecord(r, header(), got); + readRoomRecord(r, got); TEST_ASSERT_EQUAL_UINT8(2, got.roomId); TEST_ASSERT_EQUAL_INT8(1, got.sortOrder); } @@ -538,7 +538,7 @@ static void test_skipping_records_passes_their_newlines() { TextReader r(w.text); TEST_ASSERT_TRUE(skipRecords(r, 2)); room_record_t got; - readRoomRecord(r, header(), got); + readRoomRecord(r, got); TEST_ASSERT_EQUAL_STRING("Last", got.name); TEST_ASSERT_FALSE(skipRecords(r, 1)); } @@ -551,7 +551,7 @@ static void writeShadeStart(TextWriter &w, uint8_t id) { w.writeUInt32(0x111111); w.writeString("Old", CFG_NAME_LEN); } -static void test_a_version_4_shade() { +static std::string version4Shade() { TextWriter w; writeShadeStart(w, 2); w.writeUInt8(0); // tiltType @@ -565,23 +565,62 @@ static void test_a_version_4_shade() { w.writeFloat(-1, 5); // myTiltPos w.writeFloat(25, 5); // currentPos w.writeFloat(0, 5, CFG_REC_END); - TextReader r(w.text); + return w.text; +} +static void test_a_version_4_shade() { + const std::string text = version4Shade(); + TextReader r(text); shade_record_t got; readShadeRecord(r, header(4), got); TEST_ASSERT_EQUAL_UINT8(2, got.shadeId); + TEST_ASSERT_TRUE(got.paired); + TEST_ASSERT_EQUAL_UINT8(static_cast(shade_types::roller), static_cast(got.shadeType)); + TEST_ASSERT_EQUAL_HEX32(0x111111, got.remoteAddress); TEST_ASSERT_EQUAL_STRING("Old", got.name); + TEST_ASSERT_EQUAL_UINT8(static_cast(tilt_types::none), static_cast(got.tiltType)); TEST_ASSERT_EQUAL_UINT8(static_cast(radio_proto::RTS), static_cast(got.proto)); + TEST_ASSERT_EQUAL_UINT8(56, got.bitLength); TEST_ASSERT_EQUAL_UINT32(15000, got.upTime); + TEST_ASSERT_EQUAL_UINT32(14000, got.downTime); + TEST_ASSERT_EQUAL_UINT32(0, got.tiltTime); TEST_ASSERT_EQUAL_UINT16(100, got.stepSize); TEST_ASSERT_EQUAL_HEX32(0x222222, got.linkedRemotes[0]); + TEST_ASSERT_EQUAL_HEX32(0, got.linkedRemotes[4]); TEST_ASSERT_EQUAL_HEX32(0, got.linkedRemotes[5]); TEST_ASSERT_EQUAL_UINT16(300, got.lastRollingCode); + TEST_ASSERT_EQUAL_HEX8(0, got.flags); TEST_ASSERT_EQUAL_FLOAT(50, got.myPos); + TEST_ASSERT_EQUAL_FLOAT(-1, got.myTiltPos); TEST_ASSERT_EQUAL_FLOAT(25, got.currentPos); + TEST_ASSERT_EQUAL_FLOAT(0, got.currentTiltPos); + TEST_ASSERT_FALSE(got.flipCommands); + TEST_ASSERT_FALSE(got.flipPosition); TEST_ASSERT_EQUAL_UINT8(1, got.repeats); TEST_ASSERT_EQUAL_INT8(1, got.sortOrder); + TEST_ASSERT_EQUAL_UINT8(0, got.gpioUp); TEST_ASSERT_EQUAL_UINT8(0, got.roomId); - TEST_ASSERT_EQUAL(w.text.size(), r.position()); + TEST_ASSERT_EQUAL(text.size(), r.position()); +} +// A restore reads into a record that starts as the shade it replaces, and +// what an older version lacks stays as it was. +static void test_what_an_older_version_lacks_stays_as_it_was() { + const std::string text = version4Shade(); + TextReader r(text); + shade_record_t got = aShade(); + readShadeRecord(r, header(4), got); + TEST_ASSERT_EQUAL_UINT32(15000, got.upTime); + TEST_ASSERT_EQUAL_HEX32(0x222222, got.linkedRemotes[0]); + TEST_ASSERT_EQUAL_UINT8(static_cast(radio_proto::GP_Remote), static_cast(got.proto)); + TEST_ASSERT_EQUAL_UINT16(250, got.stepSize); + TEST_ASSERT_EQUAL_HEX32(0x200005, got.linkedRemotes[5]); + TEST_ASSERT_EQUAL_HEX8(0x21, got.flags); + TEST_ASSERT_TRUE(got.flipCommands); + TEST_ASSERT_TRUE(got.flipPosition); + TEST_ASSERT_EQUAL_UINT8(4, got.repeats); + TEST_ASSERT_EQUAL_UINT8(25, got.gpioUp); + TEST_ASSERT_EQUAL_UINT8(27, got.gpioMy); + TEST_ASSERT_EQUAL_UINT8(2, got.gpioFlags); + TEST_ASSERT_EQUAL_UINT8(5, got.roomId); } // Version 2 saved positions as fractions and the favorite as a byte. static void test_a_version_2_shade_has_fractions() { @@ -664,6 +703,7 @@ static void test_a_version_23_group() { group_record_t got; readGroupRecord(r, header(23), got); TEST_ASSERT_EQUAL_UINT16(4321, got.lastRollingCode); + TEST_ASSERT_TRUE(got.hasRollingCode); TEST_ASSERT_EQUAL_UINT8(2, got.linkedShades[1]); TEST_ASSERT_EQUAL_UINT8(6, got.roomId); TEST_ASSERT_EQUAL(w.text.size(), r.position()); @@ -681,14 +721,34 @@ static void test_a_version_11_group() { TEST_ASSERT_FALSE(got.flipCommands); TEST_ASSERT_EQUAL_UINT16(0, got.lastRollingCode); } -// Before version 20 the settings had no update check. -static void test_version_19_settings_check_for_updates() { +// Before version 23 a group had no rolling code, so a restore leaves the one +// NVS holds. +static void test_a_group_before_version_23_has_no_rolling_code() { + TextWriter w; + writeGroupStart(w); + for(uint8_t j = 0; j < SOMFY_MAX_GROUPED_SHADES; j++) w.writeUInt8(0, j == SOMFY_MAX_GROUPED_SHADES - 1 ? CFG_REC_END : CFG_VALUE_SEP); + TextReader r(w.text); + group_record_t got = aGroup(); + readGroupRecord(r, header(11), got); + TEST_ASSERT_FALSE(got.hasRollingCode); + TEST_ASSERT_EQUAL_UINT16(1234, got.lastRollingCode); + TEST_ASSERT_TRUE(got.flipCommands); + TEST_ASSERT_EQUAL_UINT8(4, got.roomId); + TextWriter v24; + writeGroupRecord(v24, aGroup()); + TextReader r24(v24.text); + readGroupRecord(r24, header(), got); + TEST_ASSERT_TRUE(got.hasRollingCode); +} +// Before version 20 the settings had no update check, which stays as it was. +static void test_version_19_settings_leave_the_update_check() { TextReader r("\"v2.0.0\",\"host\",\"pool.ntp.org\",\"\",true \n"); settings_record_t got; + got.checkForUpdate = false; readSettingsRecord(r, header(19), got); TEST_ASSERT_EQUAL_STRING("host", got.hostname); TEST_ASSERT_TRUE(got.ssdpBroadcast); - TEST_ASSERT_TRUE(got.checkForUpdate); + TEST_ASSERT_FALSE(got.checkForUpdate); } // Before version 22 the network record had no MQTT settings. static void test_a_version_21_network_has_no_mqtt() { @@ -703,15 +763,6 @@ static void test_a_version_21_network_has_no_mqtt() { TEST_ASSERT_EQUAL_UINT8(9, r.readUInt8(0)); } -static void test_a_load_keeps_the_higher_rolling_code() { - TEST_ASSERT_EQUAL_UINT16(500, rollingCodeAfterLoad(500, 300, false)); - TEST_ASSERT_EQUAL_UINT16(700, rollingCodeAfterLoad(500, 700, false)); -} -static void test_a_restore_takes_the_backups_rolling_code() { - TEST_ASSERT_EQUAL_UINT16(300, rollingCodeAfterLoad(500, 300, true)); - TEST_ASSERT_EQUAL_UINT16(700, rollingCodeAfterLoad(500, 700, true)); -} - // A real backup, which isn't in the repo: it holds the remotes' addresses. // CONFIG_BACKUP= pio test -e native -f test_configrecords static void test_a_real_backup_reads_to_its_end() { @@ -727,7 +778,7 @@ static void test_a_real_backup_reads_to_its_end() { TEST_ASSERT_EQUAL(static_cast(h.length), r.position()); for(uint8_t i = 0; i < h.roomRecords; i++) { room_record_t rec; - readRoomRecord(r, h, rec); + readRoomRecord(r, rec); TEST_ASSERT_NOT_EQUAL(0, rec.roomId); } for(uint8_t i = 0; i < h.shadeRecords; i++) { @@ -786,14 +837,14 @@ int main(int argc, char **argv) { RUN_TEST(test_a_short_record_never_reads_the_next); RUN_TEST(test_skipping_records_passes_their_newlines); RUN_TEST(test_a_version_4_shade); + RUN_TEST(test_what_an_older_version_lacks_stays_as_it_was); RUN_TEST(test_a_version_2_shade_has_fractions); RUN_TEST(test_a_version_13_shade); RUN_TEST(test_a_version_23_group); RUN_TEST(test_a_version_11_group); - RUN_TEST(test_version_19_settings_check_for_updates); + RUN_TEST(test_a_group_before_version_23_has_no_rolling_code); + RUN_TEST(test_version_19_settings_leave_the_update_check); RUN_TEST(test_a_version_21_network_has_no_mqtt); - RUN_TEST(test_a_load_keeps_the_higher_rolling_code); - RUN_TEST(test_a_restore_takes_the_backups_rolling_code); RUN_TEST(test_a_real_backup_reads_to_its_end); return UNITY_END(); } diff --git a/test/test_mqttdisco/test_mqttdisco.cpp b/test/test_mqttdisco/test_mqttdisco.cpp index 687972e..095dcc3 100644 --- a/test/test_mqttdisco/test_mqttdisco.cpp +++ b/test/test_mqttdisco/test_mqttdisco.cpp @@ -1,5 +1,9 @@ #include +#include +#include #include "MqttDisco.h" +#include "MqttTopics.h" +#include "ShadeChanges.h" void setUp() {} void tearDown() {} @@ -170,6 +174,29 @@ static void test_the_config_topic_of_each_kind() { TEST_ASSERT_FALSE(isDiscoSwitch(shade_types::garage1)); } +static bool hasTopic(const char *const *topics, size_t count, const std::string &topic) { + for(size_t i = 0; i < count; i++) if(topic == topics[i]) return true; + return false; +} +static void keepRetained(const char *topic, int32_t value, bool retain, void *ctx) { + (void)value; + if(retain) static_cast *>(ctx)->push_back(topic); +} +// A deleted shade leaves none of its topics at the broker. +static void test_deleting_a_shade_clears_every_state_topic_the_broker_keeps() { + shade_snapshot_t shade{}; + std::vector retained; + publishShadeTopics(SHADE_CHANGED_TOPICS, shade, keepRetained, &retained); + TEST_ASSERT_FALSE(retained.empty()); + for(const std::string &topic : retained) + TEST_ASSERT_TRUE_MESSAGE(hasTopic(SHADE_TOPICS, SHADE_TOPIC_COUNT, topic), topic.c_str()); +} +// Group unpublish named it SunSensor, so a deleted group's stayed. +static void test_deleting_a_group_clears_its_sun_sensor_topic() { + TEST_ASSERT_TRUE(hasTopic(GROUP_TOPICS, GROUP_TOPIC_COUNT, "sunSensor")); + TEST_ASSERT_FALSE(hasTopic(GROUP_TOPICS, GROUP_TOPIC_COUNT, "SunSensor")); +} + int main(int argc, char **argv) { UNITY_BEGIN(); RUN_TEST(test_a_roller_is_a_cover_under_the_discovery_topic); @@ -183,5 +210,7 @@ int main(int argc, char **argv) { RUN_TEST(test_a_tilt_only_blind_has_only_the_tilt); RUN_TEST(test_a_dry_contact_is_a_switch); RUN_TEST(test_the_config_topic_of_each_kind); + RUN_TEST(test_deleting_a_shade_clears_every_state_topic_the_broker_keeps); + RUN_TEST(test_deleting_a_group_clears_its_sun_sensor_topic); return UNITY_END(); } diff --git a/test/test_rollingcode/test_rollingcode.cpp b/test/test_rollingcode/test_rollingcode.cpp index 8de99aa..0029878 100644 --- a/test/test_rollingcode/test_rollingcode.cpp +++ b/test/test_rollingcode/test_rollingcode.cpp @@ -47,6 +47,16 @@ static void test_wraps_to_zero_after_0xFFFF() { TEST_ASSERT_EQUAL_UINT16(0, next(true, 0xFFFF, store)); } +// NVS is newer than shades.cfg, so a load keeps the higher code. +static void test_a_load_keeps_the_higher_rolling_code() { + TEST_ASSERT_EQUAL_UINT16(500, rollingCodeAfterLoad(500, 300, false)); + TEST_ASSERT_EQUAL_UINT16(700, rollingCodeAfterLoad(500, 700, false)); +} +static void test_a_restore_takes_the_backups_rolling_code() { + TEST_ASSERT_EQUAL_UINT16(300, rollingCodeAfterLoad(500, 300, true)); + TEST_ASSERT_EQUAL_UINT16(700, rollingCodeAfterLoad(500, 700, true)); +} + int main(int argc, char **argv) { (void)argc; (void)argv; @@ -55,5 +65,7 @@ int main(int argc, char **argv) { RUN_TEST(test_synced_never_reads_the_store); RUN_TEST(test_reading_does_not_use_up_a_code); RUN_TEST(test_wraps_to_zero_after_0xFFFF); + RUN_TEST(test_a_load_keeps_the_higher_rolling_code); + RUN_TEST(test_a_restore_takes_the_backups_rolling_code); return UNITY_END(); }