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 <noreply@anthropic.com>
This commit is contained in:
cjkas 2026-10-29 17:25:07 +01:00
parent b525a800b0
commit 92cd54f097
13 changed files with 277 additions and 148 deletions

View file

@ -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<conn_types_t>(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<eth_clock_mode_t>(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<conn_types_t>(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<eth_clock_mode_t>(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);

View file

@ -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);

View file

@ -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<int8_t>(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<uint8_t>(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++) {

View file

@ -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

19
src/MqttTopics.h Normal file
View file

@ -0,0 +1,19 @@
#ifndef MQTT_TOPICS_H
#define MQTT_TOPICS_H
#include <stddef.h>
// The topics the broker keeps for a shade (shades/<id>/) and a group
// (groups/<id>/), 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

View file

@ -2,7 +2,8 @@
#define ROLLING_CODE_H
#include <stdint.h>
// 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<typename LoadStored>
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

View file

@ -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) {