Merge pull request #46 from cjkas/refactor/shade-settings

refactor: hold shade settings in ShadeSettings and RemoteSettings

GLOSSARY.md

@@ -60,6 +60,16 @@ The per-shade switch that lets the sun move a shade. While it is on, lasting sun
 **Extend / retract**
 Where the weather sends a shade: extend to its favorite, or fully when it has none; retract fully. A tilt-only shade extends and retracts its tilt, every other shade its lift.
 
+**ShadeSettings**
+What the user sets up for a shade and its behaviour only reads: shade and tilt type, travel times, step size, flip position, simulated My and sun sensor.
+_Avoid_: config (the whole shade model is saved in the config file)
+
+**RemoteSettings**
+What shades and groups share about how their frames go on air: flip commands, bit length and repeats.
+
+**Flags**
+The byte a shade is saved and sent with. Its sun sensor, light and simulated My bits are settings; its sun flag, sunny, windy and demo mode bits are state. A group's flags are its shades' flags together.
+
 ## Entry points and outputs
 
 **Entry point**

docs/SHADE_BEHAVIOUR_PLAN.md

@@ -1,6 +1,6 @@
 # Plan: deepen the shade behaviour
 
-**Status:** design in progress. Round 1 of the design questions is open (below); nothing is built.
+**Status:** design settled (rounds 1–3, see "Decisions"), apart from the reconcile order, which waits for PR B's pinning tests. PR A (`refactor/shade-settings`) is built; PR B is next.
 **Written:** 2026-10-16, on top of `refactor/split-transports` (PR #44, the top of the LoopQueue stack #40–#44, which is not yet merged or run on a device). Line numbers refer to that branch.
 **Start here:** read [GLOSSARY.md](../GLOSSARY.md), [ADR-0002](adr/0002-loop-task-is-the-only-writer-of-the-shade-model.md) and [LOOP_QUEUE_PLAN.md](LOOP_QUEUE_PLAN.md) first. Their decisions are not reopened here.
 
@@ -66,37 +66,25 @@ One module holding the shade's behaviour behind a small interface:
 
 The decisions are the user's. Each question comes with a recommended answer, and they are asked in rounds, as in the LoopQueue plan. Settled answers move to "Decisions".
 
-### Round 1 (open)
+### Found while preparing round 2
 
-**Q1. What moves into the module.**
+- **The echo does part of the sender's work, and it varies by command.** For a frame marked internal, `processFrame` leaves the targets alone on Up and Down (the sender sets them itself after the send). It does set them on My (when idle), Stop, Toggle, Favorite, StepUp and StepDown. So for Stop, Step, Toggle and Favorite sent from the web, the shade's *only* reaction is in its echo. A linked shade gets the same internal frame, so it follows My/Stop/Step/Toggle/Favorite but not Up/Down. The echo also resets `moveStart`/`startPos` and copies the frame into `lastFrame` on every shade it reaches.
-- **Into the module:** everything that decides where the shade is and where it is going: `processFrame`, `processWaitingFrame`, `processInternalCommand`, `sendCommand`, `sendTiltCommand`, `moveToTarget`, `moveToTiltTarget`, `setMyPosition`, `moveToMyPosition`, `checkMovement` and `processTxEvent`. Their state goes too: positions, targets, directions, start times, the `setting*` flags, the favorite, `ShadeSensors` and `ShadeTxState`.
+- **The order inside a send is:** encode → submit → commit rolling code → `lastFrame` → echo to every shade → `markTxPending` on the sender, its linked shades and the members of a group with that address. The sender's caller (`moveToTarget`, `sendCommand`, …) sets its targets *after* all of that.
-- **Stays in `SomfyShade`:** identity and settings (id, name, remotes, types, times), persistence, GPIO, JSON and rolling codes.
+- **`lastFrame` serves two purposes.** It is the template `sendCommandAfter` builds the next frame from, and the long-press state `processWaitingFrame` reads (`await`, `repeats`, `processed`, `cmd`, `stepSize`).
+- **`emitCommand` is still a direct output.** It builds a socket event with ArduinoJson and publishes three MQTT topics on the spot, about 40 times in the shade code and once per member in `SomfyGroup::sendCommand`. `emitState()` only marks the shade changed.
+- **`isInGroup()` isn't used by the behaviour code.** Only the delete check in SomfyJson and the snapshot call it, so it stays in `SomfyShade`.
+- **`flags` mixes settings and state.** SimMy, SunSensor and Light are settings; SunFlag, Sunny, Windy and DemoMode are sensor state the behaviour changes. ConfigFile saves the whole byte.
+- **Outside writers of motion state:** ConfigFile at boot (positions, targets, favorite) and `SomfyShadeOps` setting a position without moving (`p_target`/`p_currentPos`). The other files only read it.
 
-Recommended: yes.
+### Found while preparing round 3
 
-**Q2. Three phases: pin, move, reconcile.**
+- **The sender's own echo needs the frame as sent, not just the command.** The Flag and SunFlag rules ignore a frame whose rolling code is 0x8000 or above (a guard against bogus sensors), and they apply it to the shade's own echo too. `flipCommands` flips the command on the way out and flips it back when the echo comes in.
-1. **Pin:** native scenario tests of today's behaviour, including where the copies disagree, each labelled "today: …". Every later behaviour change then shows up as a test change.
+- **`sendSensorCommand` doesn't move.** ShadeRequests calls it through `SomfyShadeOps`, and it stays in `SomfyRemote`. It keeps echoing to every shade, the sender included, so the shade hears it through `onFrame`.
-2. **Move:** the code goes into the module unchanged, and the pinning tests stay green. Today's code can't be built natively, so phases 1 and 2 happen together: the seams go in while the tests are written against the moved, unchanged code.
+- **GPIO:** `GP_Remote` pulses its pins inside the send; `GP_Relay` sends nothing and `setGPIOs()` drives the relays from `loop()` by reading the direction. Both stay outside the module.
-3. **Reconcile:** the copies are made to agree, one decision per commit, each checked on real motors.
 
-Recommended: yes.
+### Still open
 
-**Q3. Time is passed in.** Every input carries `now` (`onFrame(frame, now)`, `tick(now)`, ...), as `ShadeSensors::check(…, now)` and `TxHold` already do. The alternative is a clock interface the module asks. Recommended: pass it in.
+- **Reconcile: what is decided first, and in what order.** Asked once PR B's pinning tests have listed every "today: …" disagreement. Known candidates: `settingPos` on arrival (may send extra stops), Up/Down/My on tilt-only shades, the self-echo (Q5), and merging group commands into the frame rule (Q8).
-
-**Q4. One behaviour object per shade, settings as plain data.** Each `SomfyShade` holds one module object with the motion state. Its settings come in as a plain struct: shade type, tilt type, up/down/tilt times, step size, `simMy`, `flipCommands`. The alternative is functions with no state of their own, over a state struct the caller owns. Recommended: an object per shade.
-
-### Round 2 (waits for round 1)
-
-- **The echo.** Should the module apply its own commands directly, instead of reacting to its own echoed frame? This is the biggest decision; it decides what the radio seam looks like, and whether linked shades still learn about the command from the echo.
-- **The radio seam.** What it takes (command, repeats, step size, delay, priority) and returns (ticket or refused). Rolling codes, NVS and frame encoding stay behind it in `SomfyRemote`. GPIO-driven shades sit behind the same seam.
-- **Linked remotes and groups.** How a frame from a linked remote and a group command reach the module, and whether `processInternalCommand` stays a separate input.
-- **The global `somfy`.** Where `isDirty`/`commit()` go (a change bit the caller acts on?), `updateGroupFlags()` (the controller after the pass?), `transceiver.config` (part of the settings struct?), and `isInGroup()` (given by the caller?).
-
-### Later rounds
-
-- The names, which go into GLOSSARY.md (for example **ShadeBehaviour**, **MotorLink**). Check for clashes with `ShadeMotion`, `ShadeOps`, `ShadeRequests` and `ShadeChanges`.
-- The order of the steps and the PR split.
-- What the reconcile phase decides first: Up/Down/My on tilt-only shades, `settingPos` on arrival.
 
 ## Constraints
 
@@ -107,4 +95,43 @@ Recommended: yes.
 
 ## Decisions
 
-None yet.
+**Round 1 (2026-10-16), all as recommended:**
+
+- **Q1. What moves in.** `processFrame`, `processWaitingFrame`, `processInternalCommand`, `sendCommand`, `sendTiltCommand`, `moveToTarget`, `moveToTiltTarget`, `setMyPosition`, `moveToMyPosition`, `checkMovement` and `processTxEvent` move into the module. So do their helpers (`sendStop`, `sendSetMy`, `storeMyPosition`, `runToLimit`, `applySensorMove`, `applyStep`, `tiltRepeats`) and their state: positions, targets, directions, `lastMovement`, start times and positions, the `setting*` flags, `awaitMy`, the favorite, `ShadeSensors` and `ShadeTxState`. The `p_*` setters and change marking also move in. `SomfyShade` keeps identity, settings, persistence, GPIO, JSON, MQTT topics and rolling codes.
+- **Q2. Pin, move, reconcile.** Phases 1 and 2 land together and change no behaviour. Disagreements are pinned as "today: …" tests. Reconcile makes one decision per commit, each checked on real motors.
+- **Q3. Time is passed in.** Every input carries `now` as `uint32_t`, with wrap-safe elapsed times as in `TxHold`.
+- **Q4. One object per shade.** It holds a `const ShadeSettings&`. The settings fields move into that struct, which `SomfyShade` owns, so there is one copy and nothing to drift.
+
+**Round 2 (2026-10-16), all as recommended.** The principle: the module never touches anything outside itself; `SomfyShade` wraps it and does the outside work.
+
+- **Q5. The echo.** Today a sent frame is handed back to every shade as if a remote had sent it, and some commands (Stop, Step, Toggle, Favorite, My when idle) do their work only there. The module calls its own frame rule directly after a successful send, at the same point as today. Other shades (linked remotes) still hear the frame through the controller. Whether the sender should hear itself at all is a reconcile decision.
+- **Q6. Held-button state.** The module keeps its own record of the held button (`cmd`, `repeats`, `await`, `processed`, `stepSize`, `remoteAddress`). `SomfyRemote::lastFrame` stays the send template, and `SomfyShade` keeps copying frames into it as today.
+- **Q7. Command events.** They leave through an output seam, `onCommand(cmd, source, sourceAddress)`, which calls today's `emitCommand` on the device and records in tests. The timing is unchanged, and ArduinoJson stays out of the module.
+- **Q8. Group commands.** They stay a separate input, `onGroupCommand(cmd, repeat, now)`, with today's rule. `markTxPending` becomes `onTxQueued(ticket, now)`. Merging group commands into the frame rule is a reconcile decision.
+- **Q9. The global `somfy`.** The module reports "save wanted" and "sensor flags changed"; `SomfyShade` sets `somfy.isDirty` and calls `updateGroupFlags()`. `bitLength` goes into `ShadeSettings`, still filled in lazily by `SomfyShade`. `isInGroup()` stays in `SomfyShade`.
+- **Q10. The `flags` byte.** SunFlag, Sunny, Windy and DemoMode go into the module; SimMy and SunSensor into `ShadeSettings`; Light stays in `SomfyShade`. The byte is put back together for ConfigFile, JSON and MQTT, so nothing saved or sent changes.
+- **Q11. Reading the motion state.** A read-only `state()`. ConfigFile at boot writes through `restore(...)` and `SomfyShadeOps` through `setPosition(pos, tilt)`. The 12 reading files switch over in the move phase.
+
+**Round 3 (2026-10-16), all as recommended:**
+
+- **Q12. The radio seam.** The module sends with `send(cmd, repeats, stepSize, delayMs, priority)` and gets back `{sent, ticket, frame}`. On the device, `SomfyRemote` does what `sendCommandAfter` does today (rolling code, encode, submit or pulse GPIO, commit the code, `lastFrame`), but it echoes and marks the radio hold only for the *other* shades and group members. The module runs its own echo on the returned frame and marks its own hold. In tests a fake records each send, hands out tickets 1, 2, 3, … and can be told to refuse.
+- **Q13. Names.** These go into GLOSSARY.md with PR B, when the code exists:
+  - **ShadeBehaviour**: the module (`src/ShadeBehaviour.h/.cpp`, `test/test_shadebehaviour`).
+  - **ShadeSettings**: the read-only settings.
+  - **ShadeBehaviour::State**: the motion state `state()` returns.
+  - **MotorLink**: the send seam, radio or GPIO.
+  - **ShadeEvents**: the `onCommand(...)` output.
+- **Q14. Order and PRs**, stacked on #44:
+  1. **PR A:** `ShadeSettings` and the `flags` split, with tests that the saved byte and JSON don't change.
+  2. **PR B:** the move: `ShadeBehaviour`, `MotorLink`, `ShadeEvents`, `restore`/`setPosition`, the 12 readers and the pinning scenario tests. No behaviour change.
+  3. **Reconcile:** one PR per decision. These start only after #40–#44, A and B have run on a device.
+
+**Before PR A (2026-10-16), all as recommended.** Q4 assumed every setting lives on `SomfyShade`. But `flipCommands`, `bitLength`, `repeats` and the `flags` byte are on the `SomfyRemote` base, which groups share and the send path uses.
+
+- **Q15. Two settings structs, held as bases.** `RemoteSettings` (`flipCommands`, `bitLength`, `repeats`) is a base of `SomfyRemote`, so shades and groups each have one. `ShadeSettings` (types, travel times, step size, `flipPosition`, `simMy`, `sunSensor`) is a second base of `SomfyShade`. Existing code like `shade->upTime` keeps compiling. The module will get two read-only references. That is still one copy of each, so nothing can drift.
+- **Q16. The `flags` byte.**
+  - SimMy and SunSensor are `bool`s in `ShadeSettings`, and Light is a `bool` on `SomfyShade`.
+  - SunFlag, Sunny, Windy, DemoMode and the unused Lighted stay on `SomfyShade` as `stateFlags` until PR B moves them into the module.
+  - A shade's byte is `getFlags()` (put together) and `setFlags()` (split), used by ConfigFile, JSON, MQTT, events and group flags.
+  - Groups keep their own `flags` byte (the OR of their shades), and `SomfyRemote::getFlags()` is virtual for `sendSensorCommand`.
+  - `test_shadesettings` checks that all 256 byte values come back unchanged.

src/ConfigFile.cpp

@@ -827,7 +827,7 @@ bool ShadeConfigFile::readShadeRecord(SomfyShade *shade) {
     if(this->header.version < 5 && j == 4) break; // Prior to version 5 we only supported 5 linked remotes.
   }
   shade->lastRollingCode = this->readUInt16(0);
-  if(this->header.version > 7) shade->flags = this->readUInt8(0);
+  if(this->header.version > 7) shade->setFlags(this->readUInt8(0));
   if(shade->getRemoteAddress() != 0) {
     // If the last rolling code stored on the nvs is less than the rc we currently have
     // then we need to set it.
@@ -995,7 +995,7 @@ bool ShadeConfigFile::writeShadeRecord(SomfyShade *shade) {
   }
   this->writeUInt16(shade->lastRollingCode);
   if(shade->getShadeId() != 255) {
-    this->writeUInt8(shade->flags & 0xFF);
+    this->writeUInt8(shade->getFlags());
     this->writeFloat(shade->myPos, 5);
     this->writeFloat(shade->myTiltPos, 5);
     this->writeFloat(shade->currentPos, 5);

src/ShadeSettings.h

@@ -0,0 +1,50 @@
+#ifndef SHADE_SETTINGS_H
+#define SHADE_SETTINGS_H
+#include <Arduino.h>
+#include "ShadeTypes.h"
+
+// The settings a shade's behaviour reads and never changes.  The user sets
+// them through JSON, the config file and ModelEditor.  Kept free of the shade
+// model so they can be used in the unit tests on the PC (pio test -e native).
+
+// What shades and groups share: how their frames go on air.
+struct RemoteSettings {
+  bool flipCommands = false;
+  uint8_t bitLength = 0;
+  uint8_t repeats = 1;
+};
+// What only a shade has.
+struct ShadeSettings {
+  shade_types shadeType = shade_types::roller;
+  tilt_types tiltType = tilt_types::none;
+  uint32_t upTime = 10000;
+  uint32_t downTime = 10000;
+  uint32_t tiltTime = 7000;
+  uint16_t stepSize = 100;
+  bool flipPosition = false;
+  // Reaches the favorite by moving to it rather than sending My.
+  bool simMy = false;
+  bool sunSensor = false;
+};
+
+// The bits of a shade's flags byte that are settings.  The rest are state the
+// shade changes (sun flag, sunny, windy, demo mode) or unused (lighted).
+static const uint8_t SHADE_SETTING_FLAGS = static_cast<uint8_t>(somfy_flags_t::SunSensor) |
+  static_cast<uint8_t>(somfy_flags_t::Light) | static_cast<uint8_t>(somfy_flags_t::SimMy);
+
+// The flags byte as it is saved and sent, put together from where its bits live.
+inline uint8_t joinShadeFlags(const ShadeSettings &settings, bool light, uint8_t stateFlags) {
+  uint8_t flags = stateFlags & ~SHADE_SETTING_FLAGS;
+  if(settings.sunSensor) flags |= static_cast<uint8_t>(somfy_flags_t::SunSensor);
+  if(light) flags |= static_cast<uint8_t>(somfy_flags_t::Light);
+  if(settings.simMy) flags |= static_cast<uint8_t>(somfy_flags_t::SimMy);
+  return flags;
+}
+// Splits a saved or sent flags byte back to where its bits live.
+inline void splitShadeFlags(uint8_t flags, ShadeSettings &settings, bool &light, uint8_t &stateFlags) {
+  settings.sunSensor = flags & static_cast<uint8_t>(somfy_flags_t::SunSensor);
+  light = flags & static_cast<uint8_t>(somfy_flags_t::Light);
+  settings.simMy = flags & static_cast<uint8_t>(somfy_flags_t::SimMy);
+  stateFlags = flags & ~SHADE_SETTING_FLAGS;
+}
+#endif

src/Somfy.h

@@ -8,6 +8,7 @@
 #include "TxBuffer.h"
 #include "RadioScheduler.h"
 #include "ShadeTypes.h"
+#include "ShadeSettings.h"
 #include "ShadeMotion.h"
 #include "ShadeSensors.h"
 #include "ShadeChanges.h"
@@ -43,7 +44,7 @@ struct send_result_t {
   uint32_t ticket = 0;
   explicit operator bool() const { return this->sent; }
 };
-class SomfyRemote {
+class SomfyRemote : public RemoteSettings {
   // These sizes for the data have been
   // confirmed.  The address is actually 24bits
   // and the rolling code is 16 bits.
@@ -62,11 +63,7 @@ class SomfyRemote {
     uint8_t gpioMy = 0;
     uint32_t gpioRelease = 0;
     somfy_frame_t lastFrame;
-    bool flipCommands = false;
     uint16_t lastRollingCode = 0;
-    uint8_t flags = 0;
-    uint8_t bitLength = 0;
-    uint8_t repeats = 1;
     virtual bool isLastCommand(somfy_commands cmd);
     char *getRemotePrefId() {return m_remotePrefId;}
     virtual void setRemoteAddress(uint32_t address);
@@ -74,12 +71,8 @@ class SomfyRemote {
     uint16_t peekNextRollingCode();
     void commitRollingCode(uint16_t code);
     virtual uint16_t setRollingCode(uint16_t code);
-    bool hasSunSensor();
+    // The flags byte (somfy_flags_t) as it is saved and sent.
-    bool hasLight();
+    virtual uint8_t getFlags() { return 0; }
-    bool simMy();
-    void setSunSensor(bool bHasSensor);
-    void setLight(bool bHasLight);
-    void setSimMy(bool bSimMy);
     // The send functions return false only when the radio could not take the
     // command.  Nothing about the remote or shade state has changed in that case.
     virtual bool sendCommand(somfy_commands cmd);
@@ -96,7 +89,7 @@ class SomfyLinkedRemote : public SomfyRemote {
   public:
     SomfyLinkedRemote();    
 };
-class SomfyShade : public SomfyRemote {
+class SomfyShade : public SomfyRemote, public ShadeSettings {
   protected:
     uint8_t shadeId = 255;
     uint64_t moveStart = 0;
@@ -108,6 +101,9 @@ class SomfyShade : public SomfyRemote {
     bool settingPos = false;
     bool settingTiltPos = false;
     uint32_t awaitMy = 0;
+    // The flags bits that are state: sun flag, sunny, windy, demo mode and the
+    // unused lighted.  The setting bits are in ShadeSettings and light.
+    uint8_t stateFlags = 0;
     // Radio jobs whose outcome decides what the motor really did.
     ShadeTxState tx;
     bool sendStop(int8_t dir, bool tilt);
@@ -124,9 +120,7 @@ class SomfyShade : public SomfyRemote {
   public:
     uint8_t roomId = 0;
     int8_t sortOrder = 0;
-    bool flipPosition = false;
+    bool light = false;
-    shade_types shadeType = shade_types::roller;
-    tilt_types tiltType = tilt_types::none;
     float currentPos = 0.0f;
     float currentTiltPos = 0.0f;
     int8_t lastMovement = 0;
@@ -144,10 +138,9 @@ class SomfyShade : public SomfyRemote {
     char name[21] = "";
     void setShadeId(uint8_t id) { shadeId = id; }
     uint8_t getShadeId() { return shadeId; }
-    uint32_t upTime = 10000;
+    uint8_t getFlags() override;
-    uint32_t downTime = 10000;
+    // Sets the setting and state bits from a saved or sent flags byte.
-    uint32_t tiltTime = 7000;
+    void setFlags(uint8_t flags);
-    uint16_t stepSize = 100;
     bool save();
     bool isIdle();
     bool isInGroup();
@@ -232,6 +225,10 @@ class SomfyGroup : public SomfyRemote {
     int8_t sortOrder = 0;
     group_types groupType = group_types::channel;
     int8_t direction = 0; // 0 = stopped, 1=down, -1=up.
+    // Its linked shades' flags together, see updateFlags.
+    uint8_t flags = 0;
+    uint8_t getFlags() override { return this->flags; }
+    bool hasSunSensor() { return (this->flags & static_cast<uint8_t>(somfy_flags_t::SunSensor)) > 0; }
     char name[21] = "";
     uint8_t linkedShades[SOMFY_MAX_GROUPED_SHADES];
     void setGroupId(uint8_t id) { groupId = id; }

src/SomfyGroup.cpp

@@ -33,7 +33,7 @@ void SomfyGroup::updateFlags() {
   for(uint8_t i = 0; i < SOMFY_MAX_GROUPED_SHADES; i++) {
     if(this->linkedShades[i] != 0) {
       SomfyShade *shade = somfy.getShadeById(this->linkedShades[i]);
-      if(shade) this->flags |= shade->flags;
+      if(shade) this->flags |= shade->getFlags();
     }
     else break;
   }

src/SomfyJson.cpp

@@ -80,9 +80,9 @@ int8_t SomfyShade::fromJSON(JsonObject &obj) {
     if(!obj["hasTilt"].isNull()) this->tiltType = static_cast<bool>(obj["hasTilt"]) ? tilt_types::none : tilt_types::tiltmotor;
     if(!obj["bitLength"].isNull()) this->bitLength = obj["bitLength"];
     if(!obj["proto"].isNull()) this->proto = static_cast<radio_proto>(obj["proto"].as<uint8_t>());
-    if(!obj["sunSensor"].isNull()) this->setSunSensor(obj["sunSensor"]);
+    if(!obj["sunSensor"].isNull()) this->sunSensor = obj["sunSensor"];
-    if(!obj["simMy"].isNull()) this->setSimMy(obj["simMy"]);
+    if(!obj["simMy"].isNull()) this->simMy = obj["simMy"];
-    if(!obj["light"].isNull()) this->setLight(obj["light"]);
+    if(!obj["light"].isNull()) this->light = obj["light"];
     if(!obj["gpioFlags"].isNull()) this->gpioFlags = obj["gpioFlags"];
     if(!obj["gpioLLTrigger"].isNull()) {
       if(obj["gpioLLTrigger"].as<bool>())
@@ -150,7 +150,7 @@ int8_t SomfyShade::fromJSON(JsonObject &obj) {
         this->linkedRemotes[j].setRemoteAddress(linkedAddresses[j]);
       }
     }
-    if(!obj["flags"].isNull()) this->flags = obj["flags"];
+    if(!obj["flags"].isNull()) this->setFlags(obj["flags"].as<uint8_t>());
     if(this->proto == radio_proto::GP_Remote || this->proto == radio_proto::GP_Relay) {
       if(!obj["gpioUp"].isNull()) this->gpioUp = obj["gpioUp"];
       if(!obj["gpioDown"].isNull()) this->gpioDown = obj["gpioDown"];

src/SomfyRemote.cpp

@@ -22,17 +22,11 @@ static bool startsMovement(somfy_commands cmd) {
       return false;
   }
 }
-bool SomfyRemote::simMy() { return (this->flags & static_cast<uint8_t>(somfy_flags_t::SimMy)) > 0; }
-void SomfyRemote::setSimMy(bool bSimMy) { bSimMy ? this->flags |= static_cast<uint8_t>(somfy_flags_t::SimMy) : this->flags &= ~(static_cast<uint8_t>(somfy_flags_t::SimMy)); }
-bool SomfyRemote::hasSunSensor() { return (this->flags & static_cast<uint8_t>(somfy_flags_t::SunSensor)) > 0;}
-bool SomfyRemote::hasLight() { return (this->flags & static_cast<uint8_t>(somfy_flags_t::Light)) > 0; }
-void SomfyRemote::setSunSensor(bool bHasSensor ) { bHasSensor ? this->flags |= static_cast<uint8_t>(somfy_flags_t::SunSensor) : this->flags &= ~(static_cast<uint8_t>(somfy_flags_t::SunSensor)); }
-void SomfyRemote::setLight(bool bHasLight ) { bHasLight ? this->flags |= static_cast<uint8_t>(somfy_flags_t::Light) : this->flags &= ~(static_cast<uint8_t>(somfy_flags_t::Light)); }
 void SomfyRemote::setRemoteAddress(uint32_t address) { this->m_remoteAddress = address; this->m_rollingCodeSynced = false; snprintf(this->m_remotePrefId, sizeof(this->m_remotePrefId), "_%lu", (unsigned long)this->m_remoteAddress); }
 uint32_t SomfyRemote::getRemoteAddress() { return this->m_remoteAddress; }
 somfy_commands SomfyRemote::transformCommand(somfy_commands cmd) { return this->flipCommands ? flipCommand(cmd) : cmd; }
 bool SomfyRemote::sendSensorCommand(int8_t isWindy, int8_t isSunny, uint8_t repeat) {
-  uint8_t flags = (this->flags >> 4) & 0x0F;
+  uint8_t flags = (this->getFlags() >> 4) & 0x0F;
   if(isWindy > 0) flags |= 0x01;
   if(isSunny > 0) flags |= 0x02;
   if(isWindy == 0) flags &= ~0x01;

src/SomfyShade.cpp

@@ -113,9 +113,9 @@ bool SomfyShade::isInGroup() {
 }
 void SomfyShade::checkMovement() {
   const uint64_t curTime = millis();
-  const bool sunFlag = this->flags & static_cast<uint8_t>(somfy_flags_t::SunFlag);
+  const bool sunFlag = this->stateFlags & static_cast<uint8_t>(somfy_flags_t::SunFlag);
-  const bool isSunny = this->flags & static_cast<uint8_t>(somfy_flags_t::Sunny);
+  const bool isSunny = this->stateFlags & static_cast<uint8_t>(somfy_flags_t::Sunny);
-  const bool isWindy = this->flags & static_cast<uint8_t>(somfy_flags_t::Windy);
+  const bool isWindy = this->stateFlags & static_cast<uint8_t>(somfy_flags_t::Windy);
   // We need to first evaluate the sensor flags as these could be triggering movement from previous sensor inputs. So
   // we must check this before setting the directional items or it will not get processed until the next loop.
   int32_t downTime = (int32_t)this->downTime;
@@ -318,13 +318,15 @@ uint16_t SomfyShade::p_lastRollingCode(uint16_t code) {
   return old;
 }
 bool SomfyShade::p_flag(somfy_flags_t flag, bool val) {
-  bool old = !!(this->flags & static_cast<uint8_t>(flag));
+  bool old = !!(this->stateFlags & static_cast<uint8_t>(flag));
   if(val) 
-      this->flags |= static_cast<uint8_t>(flag);
+      this->stateFlags |= static_cast<uint8_t>(flag);
   else
-      this->flags &= ~(static_cast<uint8_t>(flag));
+      this->stateFlags &= ~(static_cast<uint8_t>(flag));
   return old;
 }
+uint8_t SomfyShade::getFlags() { return joinShadeFlags(*this, this->light, this->stateFlags); }
+void SomfyShade::setFlags(uint8_t flags) { splitShadeFlags(flags, *this, this->light, this->stateFlags); }
 bool SomfyShade::p_sunFlag(bool val) {
   bool old = this->p_flag(somfy_flags_t::SunFlag, val);
   if(old != val) this->changed |= SHADE_CHANGED_SUN_FLAG;
@@ -487,7 +489,7 @@ void SomfyShade::processWaitingFrame() {
           this->emitState();
         }
         else if(this->isIdle()) {
-          if(this->simMy())
+          if(this->simMy)
             this->moveToMyPosition(); // Call out like this (instead of move to target) so that we don't get some of the goofy tilt only problems.
           else {
             if(this->myPos >= 0.0f && this->myPos <= 100.0f) this->p_target(this->myPos);
@@ -542,7 +544,7 @@ void SomfyShade::processFrame(somfy_frame_t &frame, bool internal) {
       this->lastFrame.processed = true;
       if(this->shadeType == shade_types::drycontact || this->shadeType == shade_types::drycontact2) return;
       {
-        const uint8_t prevFlags = this->flags;
+        const uint8_t prevFlags = this->stateFlags;
         const bool wasSunny = prevFlags & static_cast<uint8_t>(somfy_flags_t::Sunny);
         const bool wasWindy = prevFlags & static_cast<uint8_t>(somfy_flags_t::Windy);
         const uint16_t status = frame.rollingCode << 4;
@@ -559,11 +561,11 @@ void SomfyShade::processFrame(somfy_frame_t &frame, bool internal) {
           this->p_windy(false);
           //this->flags &= ~(static_cast<uint8_t>(somfy_flags_t::Windy));
         if(frame.rollingCode & static_cast<uint8_t>(somfy_flags_t::DemoMode))
-          this->flags |= static_cast<uint8_t>(somfy_flags_t::DemoMode);
+          this->stateFlags |= static_cast<uint8_t>(somfy_flags_t::DemoMode);
         else
-          this->flags &= ~(static_cast<uint8_t>(somfy_flags_t::DemoMode));
+          this->stateFlags &= ~(static_cast<uint8_t>(somfy_flags_t::DemoMode));
-        const bool isSunny = this->flags & static_cast<uint8_t>(somfy_flags_t::Sunny);
+        const bool isSunny = this->stateFlags & static_cast<uint8_t>(somfy_flags_t::Sunny);
-        const bool isWindy = this->flags & static_cast<uint8_t>(somfy_flags_t::Windy);
+        const bool isWindy = this->stateFlags & static_cast<uint8_t>(somfy_flags_t::Windy);
         this->sensors.onSensorFrame(wasSunny, wasWindy, isSunny, isWindy, (uint32_t)curTime);
         this->emitState();
         somfy.updateGroupFlags();
@@ -594,8 +596,8 @@ void SomfyShade::processFrame(somfy_frame_t &frame, bool internal) {
       if(this->shadeType == shade_types::drycontact || this->shadeType == shade_types::drycontact2) return;
       if(this->lastFrame.rollingCode & 0x8000) return; // Some sensors send bogus frames with a rollingCode >= 32768 that cause them to change the state.
       {
-        const bool isWindy = this->flags & static_cast<uint8_t>(somfy_flags_t::Windy);
+        const bool isWindy = this->stateFlags & static_cast<uint8_t>(somfy_flags_t::Windy);
-        const bool isSunny = this->flags & static_cast<uint8_t>(somfy_flags_t::Sunny);
+        const bool isSunny = this->stateFlags & static_cast<uint8_t>(somfy_flags_t::Sunny);
         //this->flags |= static_cast<uint8_t>(somfy_flags_t::SunFlag);
         this->p_sunFlag(true);
         this->applySensorMove(this->sensors.onSunFlag(isSunny, isWindy));
@@ -746,7 +748,7 @@ void SomfyShade::processFrame(somfy_frame_t &frame, bool internal) {
     case somfy_commands::Favorite:
       if(this->lastFrame.processed) return;
       this->lastFrame.processed = true;
-      if(this->simMy()) {
+      if(this->simMy) {
         this->moveToMyPosition();
       }
       else {
@@ -813,7 +815,7 @@ void SomfyShade::processInternalCommand(somfy_commands cmd, uint8_t repeat) {
     case somfy_commands::My:
       if(this->isIdle()) {
         ESP_LOGD(TAG, "Shade #%d is idle", this->getShadeId());
-        if(this->simMy()) {
+        if(this->simMy) {
           this->moveToMyPosition();
         }
         else {
@@ -839,18 +841,18 @@ void SomfyShade::processInternalCommand(somfy_commands cmd, uint8_t repeat) {
       break;
     case somfy_commands::Flag:
       this->p_sunFlag(false);
-      if(this->hasSunSensor()) {
+      if(this->sunSensor) {
         somfy.isDirty = true;
         this->emitState();
       }
       else {
-        ESP_LOGW(TAG, "Shade does not have sensor %d", this->flags);
+        ESP_LOGW(TAG, "Shade does not have sensor %d", this->getFlags());
       }
       break;    
     case somfy_commands::SunFlag:
-      if(this->hasSunSensor()) {
+      if(this->sunSensor) {
-        const bool isWindy = this->flags & static_cast<uint8_t>(somfy_flags_t::Windy);
+        const bool isWindy = this->stateFlags & static_cast<uint8_t>(somfy_flags_t::Windy);
-        const bool isSunny = this->flags & static_cast<uint8_t>(somfy_flags_t::Sunny);
+        const bool isSunny = this->stateFlags & static_cast<uint8_t>(somfy_flags_t::Sunny);
         this->p_sunFlag(true);
         //this->flags |= static_cast<uint8_t>(somfy_flags_t::SunFlag);
         this->applySensorMove(this->sensors.onSunFlag(isSunny, isWindy));
@@ -858,7 +860,7 @@ void SomfyShade::processInternalCommand(somfy_commands cmd, uint8_t repeat) {
         this->emitState();
       }
       else
-        ESP_LOGW(TAG, "Shade does not have sensor %d", this->flags);
+        ESP_LOGW(TAG, "Shade does not have sensor %d", this->getFlags());
       break;
     default:
       dir = 0;
@@ -1095,7 +1097,7 @@ bool SomfyShade::moveToMyPosition() {
   }
   if(this->myPos == -1 && (this->tiltType == tilt_types::none || this->myTiltPos == -1)) return true;
   this->settingPos = false;
-  if(this->simMy()) {
+  if(this->simMy) {
     ESP_LOGI(TAG, "Moving to simulated favorite");
     if(!this->moveToTarget(this->myPos, this->myTiltPos)) return false;
   }

src/SomfySnapshot.cpp

@@ -36,13 +36,13 @@ void snapshotShade(SomfyShade &shade, shade_snapshot_t &out) {
   out.shadeType = static_cast<uint8_t>(shade.shadeType);
   out.bitLength = shade.bitLength;
   out.proto = static_cast<uint8_t>(shade.proto);
-  out.flags = shade.flags;
+  out.flags = shade.getFlags();
   out.flipCommands = shade.flipCommands;
   out.flipPosition = shade.flipPosition;
   out.inGroup = shade.isInGroup();
-  out.sunSensor = shade.hasSunSensor();
+  out.sunSensor = shade.sunSensor;
-  out.light = shade.hasLight();
+  out.light = shade.light;
-  out.simMy = shade.simMy();
+  out.simMy = shade.simMy;
   out.repeats = shade.repeats;
   out.sortOrder = shade.sortOrder;
   out.gpioUp = shade.gpioUp;
@@ -74,7 +74,7 @@ void snapshotGroup(SomfyGroup &group, group_snapshot_t &out) {
   out.flags = 0;
   for(uint8_t i = 0; i < SOMFY_MAX_GROUPED_SHADES && group.linkedShades[i] != 0; i++) {
     SomfyShade *shade = somfy.getShadeById(group.linkedShades[i]);
-    if(shade) out.flags |= shade->flags;
+    if(shade) out.flags |= shade->getFlags();
   }
   out.linkedShadeCount = 0;
   for(uint8_t i = 0; i < SOMFY_MAX_GROUPED_SHADES; i++) {

src/outputs/MqttPublisher.cpp

@@ -193,7 +193,7 @@ void SomfyShade::publish() {
     this->publish("remoteAddress", this->getRemoteAddress(), true);
     this->publish("shadeType", static_cast<uint8_t>(this->shadeType), true);
     this->publish("tiltType", static_cast<uint8_t>(this->tiltType), true);
-    this->publish("flags", this->flags, true);
+    this->publish("flags", this->getFlags(), true);
     this->publish("flipCommands", this->flipCommands, true);
     this->publish("flipPosition", this->flipPosition, true);
     this->publishState();

src/outputs/SomfyEvents.cpp

@@ -24,9 +24,9 @@ void SomfyShade::emitState(uint8_t num, const char *evt) {
   json->addElem("tiltType", static_cast<uint8_t>(this->tiltType));
   json->addElem("flipCommands", this->flipCommands);
   json->addElem("flipPosition", this->flipPosition);
-  json->addElem("flags", this->flags);
+  json->addElem("flags", this->getFlags());
-  json->addElem("sunSensor", this->hasSunSensor());
+  json->addElem("sunSensor", this->sunSensor);
-  json->addElem("light", this->hasLight());
+  json->addElem("light", this->light);
   json->addElem("sortOrder", this->sortOrder);
   if(this->tiltType != tilt_types::none) {
     json->addElem("tiltDirection", this->tiltDirection);
@@ -82,7 +82,7 @@ void SomfyGroup::emitState(uint8_t num, const char *evt) {
       SomfyShade *shade = somfy.getShadeById(this->linkedShades[i]);
       if(shade) {
         json->addElem(this->linkedShades[i]);
-        flags |= shade->flags;
+        flags |= shade->getFlags();
       }
     }
   }

test/test_shadesettings/test_shadesettings.cpp

@@ -0,0 +1,77 @@
+#include <unity.h>
+#include "ShadeSettings.h"
+
+static const uint8_t SUN_FLAG = static_cast<uint8_t>(somfy_flags_t::SunFlag);
+static const uint8_t SUN_SENSOR = static_cast<uint8_t>(somfy_flags_t::SunSensor);
+static const uint8_t DEMO_MODE = static_cast<uint8_t>(somfy_flags_t::DemoMode);
+static const uint8_t LIGHT = static_cast<uint8_t>(somfy_flags_t::Light);
+static const uint8_t WINDY = static_cast<uint8_t>(somfy_flags_t::Windy);
+static const uint8_t SUNNY = static_cast<uint8_t>(somfy_flags_t::Sunny);
+static const uint8_t LIGHTED = static_cast<uint8_t>(somfy_flags_t::Lighted);
+static const uint8_t SIM_MY = static_cast<uint8_t>(somfy_flags_t::SimMy);
+
+void setUp() {}
+void tearDown() {}
+
+static void test_every_byte_comes_back_unchanged() {
+  // The config file and JSON hold the whole byte, so splitting it and putting
+  // it back together must not lose or move a bit.
+  for(int flags = 0; flags <= 0xFF; flags++) {
+    ShadeSettings settings;
+    bool light = false;
+    uint8_t state = 0;
+    splitShadeFlags(static_cast<uint8_t>(flags), settings, light, state);
+    TEST_ASSERT_EQUAL_HEX8(flags, joinShadeFlags(settings, light, state));
+  }
+}
+static void test_setting_bits_go_to_the_settings() {
+  ShadeSettings settings;
+  bool light = false;
+  uint8_t state = 0xFF;
+  splitShadeFlags(SUN_SENSOR | LIGHT | SIM_MY, settings, light, state);
+  TEST_ASSERT_TRUE(settings.sunSensor);
+  TEST_ASSERT_TRUE(light);
+  TEST_ASSERT_TRUE(settings.simMy);
+  TEST_ASSERT_EQUAL_HEX8(0, state);
+}
+static void test_state_bits_stay_state() {
+  ShadeSettings settings;
+  settings.sunSensor = settings.simMy = true;
+  bool light = true;
+  uint8_t state = 0;
+  const uint8_t stateBits = SUN_FLAG | DEMO_MODE | WINDY | SUNNY | LIGHTED;
+  splitShadeFlags(stateBits, settings, light, state);
+  TEST_ASSERT_FALSE(settings.sunSensor);
+  TEST_ASSERT_FALSE(light);
+  TEST_ASSERT_FALSE(settings.simMy);
+  TEST_ASSERT_EQUAL_HEX8(stateBits, state);
+}
+static void test_join_ignores_setting_bits_in_the_state() {
+  // The settings decide those bits, whatever the state byte holds.
+  ShadeSettings settings;
+  TEST_ASSERT_EQUAL_HEX8(WINDY, joinShadeFlags(settings, false, WINDY | SUN_SENSOR | LIGHT | SIM_MY));
+  settings.simMy = true;
+  TEST_ASSERT_EQUAL_HEX8(SIM_MY | SUNNY, joinShadeFlags(settings, false, SUNNY));
+}
+static void test_defaults() {
+  ShadeSettings shade;
+  TEST_ASSERT_EQUAL_HEX8(0, joinShadeFlags(shade, false, 0));
+  TEST_ASSERT_EQUAL_UINT32(10000, shade.upTime);
+  TEST_ASSERT_EQUAL_UINT32(10000, shade.downTime);
+  TEST_ASSERT_EQUAL_UINT32(7000, shade.tiltTime);
+  TEST_ASSERT_EQUAL_UINT16(100, shade.stepSize);
+  RemoteSettings remote;
+  TEST_ASSERT_FALSE(remote.flipCommands);
+  TEST_ASSERT_EQUAL_UINT8(0, remote.bitLength);
+  TEST_ASSERT_EQUAL_UINT8(1, remote.repeats);
+}
+
+int main(int argc, char **argv) {
+  UNITY_BEGIN();
+  RUN_TEST(test_every_byte_comes_back_unchanged);
+  RUN_TEST(test_setting_bits_go_to_the_settings);
+  RUN_TEST(test_state_bits_stay_state);
+  RUN_TEST(test_join_ignores_setting_bits_in_the_state);
+  RUN_TEST(test_defaults);
+  return UNITY_END();
+}