From 38d4ce505878fd5fd2b5d3f606dbd38d67179eb5 Mon Sep 17 00:00:00 2001 From: cjkas Date: Fri, 16 Oct 2026 22:47:53 +0200 Subject: [PATCH] fix: keep a tilt-only shade's lift at 100 whatever sends the command The web and group Up/Down put a tilt-only shade's lift back at 100, but a remote's Up/Down/My, a group's My and the web's My while moving left a lift that something had moved, and the simulation then ran it. Every Up/Down/My path now pins the lift at 100 and acts on the tilt only. Reconcile Q22 in SHADE_BEHAVIOUR_PLAN.md, which also records Q23 (the integrated tilt turn-back) as deferred. Co-Authored-By: Claude Opus 5.5 --- docs/SHADE_BEHAVIOUR_PLAN.md | 9 +- src/ShadeBehaviour.cpp | 17 ++- src/ShadeBehaviour.h | 2 + .../test_shadebehaviour.cpp | 107 +++++++++--------- 4 files changed, 79 insertions(+), 56 deletions(-) diff --git a/docs/SHADE_BEHAVIOUR_PLAN.md b/docs/SHADE_BEHAVIOUR_PLAN.md index 371c6d6..8871259 100644 --- a/docs/SHADE_BEHAVIOUR_PLAN.md +++ b/docs/SHADE_BEHAVIOUR_PLAN.md @@ -86,8 +86,8 @@ The decisions are the user's. Each question comes with a recommended answer, and - **Reconcile: what is decided first, and in what order.** The candidates, each pinned by a "today:" test in `test_shadebehaviour` unless marked: - ~~`settingPos` is never cleared on a lift arrival, so a later step also sends a stop.~~ Fixed (Q18). - - Up and My on tilt-only shades differ between the web, a remote and a group (`test_today_up_on_tilt_only_*`, `test_today_my_while_tilt_only_moves`). - - An integrated tilt sent to a lift target without a tilt turns its slats back with an Up after the stop (`test_today_integrated_tilt_turns_back_after_a_lift_target`). + - ~~Up and My on tilt-only shades differ between the web, a remote and a group.~~ Fixed (Q22). + - An integrated tilt sent to a lift target without a tilt turns its slats back with an Up after the stop (`test_today_integrated_tilt_turns_back_after_a_lift_target`). Deferred (Q23). - ~~A remote step uses the step size this shade last *sent*.~~ Fixed (Q19). - ~~A group's SunFlag or Flag doesn't make other groups work their flags out again.~~ Fixed (Q20). - The self-echo (Q5) and merging group commands into the frame rule (Q8). @@ -160,3 +160,8 @@ The decisions are the user's. Each question comes with a recommended answer, and - **Q19. A remote's step uses the frame's own step size** (`frame.stepSize`, 1 when 0) instead of `lastFrame.stepSize`, which `copy()` never updates. `copy()` itself stays as it is. - **Q20. A group's SunFlag/Flag calls `onSensorFlags()`,** as the frame rule does, so every group works its flags out again. - **Q21. The long-press wait is wrap-safe:** `(int32_t)(now - await) > 0`. + +**Reconcile round 2 (2026-10-16):** + +- **Q22. A tilt-only shade's lift is pinned at 100 on every path** (web, remote, group; Up, Down, My, idle or moving), and those commands act on the tilt only. Before, the remote and group My paths and the web's My while moving left a lift that something had moved, and `tick` then simulated it travelling. `test_tilt_only_lift_stays_down_whatever_the_source` checks the 18 combinations. +- **Q23. Integrated tilt turning back: deferred.** No integrated-tilt motor is at hand to watch, and reconcile only changes what has been checked on a real motor. The behaviour stays, still labelled "today:", until someone with such a shade can say whether the turn-back is clean. diff --git a/src/ShadeBehaviour.cpp b/src/ShadeBehaviour.cpp index c89c940..647888c 100644 --- a/src/ShadeBehaviour.cpp +++ b/src/ShadeBehaviour.cpp @@ -450,6 +450,7 @@ void ShadeBehaviour::onWaitingFrame(uint32_t now) { else { if(this->s.myPos >= 0.0f && this->s.myPos <= 100.0f) this->p_target(this->s.myPos); if(this->s.myTiltPos >= 0.0f && this->s.myTiltPos <= 100.0f) this->p_tiltTarget(this->s.myTiltPos); + this->pinTiltOnlyLift(); } this->setMovement(0, now); this->lastFrame.processed = true; @@ -458,6 +459,7 @@ void ShadeBehaviour::onWaitingFrame(uint32_t now) { else { this->p_target(this->s.currentPos); this->p_tiltTarget(this->s.currentTiltPos); + this->pinTiltOnlyLift(); } if(this->lastFrame.repeats > SETMY_REPEATS + 2) this->lastFrame.processed = true; if(this->lastFrame.processed) { @@ -572,6 +574,7 @@ void ShadeBehaviour::onFrame(somfy_frame_t &frame, bool internal, uint32_t now) } else { // If from a remote we will simply be going up. + this->pinTiltOnlyLift(); if(this->settings.tiltType == tilt_types::tiltonly && !internal) this->p_tiltTarget(0.0f); else if(!internal) { if(this->settings.tiltType != tilt_types::tiltonly) this->p_target(0.0f); @@ -601,6 +604,7 @@ void ShadeBehaviour::onFrame(somfy_frame_t &frame, bool internal, uint32_t now) } else { this->lastFrame.processed = true; + this->pinTiltOnlyLift(); if(!internal) { if(this->settings.tiltType != tilt_types::tiltonly) this->p_target(100.0f); if(this->settings.tiltType != tilt_types::none) this->p_tiltTarget(100.0f); @@ -642,6 +646,7 @@ void ShadeBehaviour::onFrame(somfy_frame_t &frame, bool internal, uint32_t now) if(this->lastFrame.processed) return; ESP_LOGI(TAG, "Moving to My target"); this->lastFrame.processed = true; + this->pinTiltOnlyLift(); if(this->s.myTiltPos >= 0.0f && this->s.myTiltPos <= 100.0f) this->p_tiltTarget(this->s.myTiltPos); if(this->s.myPos >= 0.0f && this->s.myPos <= 100.0f && this->settings.tiltType != tilt_types::tiltonly) this->p_target(this->s.myPos); this->emitCommand(cmd, internal ? "internal" : "remote", frame.remoteAddress); @@ -650,6 +655,7 @@ void ShadeBehaviour::onFrame(somfy_frame_t &frame, bool internal, uint32_t now) else { if(this->lastFrame.processed) return; this->lastFrame.processed = true; + this->pinTiltOnlyLift(); if(!internal) { if(this->settings.tiltType != tilt_types::tiltonly) this->p_target(this->s.currentPos); this->p_tiltTarget(this->s.currentTiltPos); @@ -767,12 +773,11 @@ void ShadeBehaviour::onGroupCommand(somfy_commands cmd, uint8_t repeat, uint32_t else { if(this->s.myTiltPos >= 0.0f && this->s.myTiltPos <= 100.0f) this->p_tiltTarget(this->s.myTiltPos); if(this->s.myPos >= 0.0f && this->s.myPos <= 100.0f && this->settings.tiltType != tilt_types::tiltonly) this->p_target(this->s.myPos); + this->pinTiltOnlyLift(); } } else { - if(this->settings.tiltType == tilt_types::tiltonly) { - this->p_target(100.0f); - } + if(this->settings.tiltType == tilt_types::tiltonly) this->pinTiltOnlyLift(); else this->p_target(this->s.currentPos); this->p_tiltTarget(this->s.currentTiltPos); } @@ -895,6 +900,11 @@ bool ShadeBehaviour::sendSetMy(uint16_t delayMs, float oldPos, float oldTiltPos, this->p_myTiltPos(oldTiltPos); return false; } +void ShadeBehaviour::pinTiltOnlyLift() { + if(this->settings.tiltType != tilt_types::tiltonly) return; + this->p_target(100.0f); + this->p_currentPos(100.0f); +} void ShadeBehaviour::runToLimit(int8_t dir, bool tilt, uint32_t since) { // The motor didn't get a stop at the target so it keeps going until its end // limit. Carry on simulating that from where and when it should have stopped. @@ -1123,6 +1133,7 @@ bool ShadeBehaviour::sendCommand(somfy_commands cmd, uint8_t repeat, uint8_t ste if(!this->transmit(cmd, repeat, 0, 0, false, now).sent) return false; if(this->settings.tiltType != tilt_types::tiltonly) this->p_target(this->s.currentPos); this->p_tiltTarget(this->s.currentTiltPos); + this->pinTiltOnlyLift(); } } else if(cmd == somfy_commands::Toggle) { diff --git a/src/ShadeBehaviour.h b/src/ShadeBehaviour.h index 7af0d21..1c42558 100644 --- a/src/ShadeBehaviour.h +++ b/src/ShadeBehaviour.h @@ -135,6 +135,8 @@ class ShadeBehaviour { bool storeMyPosition(float pos, float tiltPos, uint32_t now); // Repeats for a tilt command, more for a separate tilt motor. uint8_t tiltRepeats() const; + // A tilt-only shade has no lift to move: it stays all the way down. + void pinTiltOnlyLift(); void runToLimit(int8_t dir, bool tilt, uint32_t since); // Sets the targets the sun or wind sends the shade to. void applySensorMove(SensorMove move); diff --git a/test/test_shadebehaviour/test_shadebehaviour.cpp b/test/test_shadebehaviour/test_shadebehaviour.cpp index 2b96a26..920d30d 100644 --- a/test/test_shadebehaviour/test_shadebehaviour.cpp +++ b/test/test_shadebehaviour/test_shadebehaviour.cpp @@ -457,58 +457,66 @@ static void test_today_integrated_tilt_turns_back_after_a_lift_target() { } /********************************************************************* - * Tilt-only shades: the copies disagree + * Tilt-only shades: the lift stays all the way down ********************************************************************/ -static Rig *tiltOnly(Rig &r) { +enum class Source { web, remote, group }; +static const char *sourceName(Source src) { + switch(src) { + case Source::web: return "web"; + case Source::remote: return "remote"; + default: return "group"; + } +} +// A tilt-only shade whose favorite tilt is 30, and whose lift something has +// moved to 60. moving starts the tilt turning first. +static void tiltOnly(Rig &r, bool moving) { r.settings.shadeType = shade_types::blind; r.settings.tiltType = tilt_types::tiltonly; - r.b.restore(100, 50, -1, -1); - return &r; + r.b.restore(100, 50, -1, 30); + if(moving) { + r.b.onGroupCommand(somfy_commands::Down, 1, r.now); + r.run(500); + } + r.b.setPosition(60, -1); } -static void test_today_up_on_tilt_only_from_the_web() { - Rig r; - tiltOnly(r); - r.b.sendCommand(somfy_commands::Up, 1, 0, r.now); - ASSERT_POS(100, r.st().target); - ASSERT_POS(100, r.st().currentPos); - ASSERT_POS(0, r.st().tiltTarget); +static void tiltOnlyCommand(Rig &r, Source src, somfy_commands cmd) { + switch(src) { + case Source::web: + r.b.sendCommand(cmd, 1, 0, r.now); + r.onAir(); + break; + case Source::remote: + r.press(cmd); + // Past the wait that tells a short press from a long one. + r.wait(600); + break; + case Source::group: + r.b.onGroupCommand(cmd, 1, r.now); + break; + } } -static void test_today_up_on_tilt_only_from_a_remote() { - // today: a remote only sets the tilt target, where the web and groups also - // put the lift at 100. - Rig r; - tiltOnly(r); - r.b.restore(60, 50, -1, -1); - r.press(somfy_commands::Up); - ASSERT_POS(60, r.st().target); - ASSERT_POS(0, r.st().tiltTarget); -} -static void test_today_up_on_tilt_only_from_a_group() { - Rig r; - tiltOnly(r); - r.b.restore(60, 50, -1, -1); - r.b.onGroupCommand(somfy_commands::Up, 1, r.now); - ASSERT_POS(100, r.st().target); - ASSERT_POS(100, r.st().currentPos); - ASSERT_POS(0, r.st().tiltTarget); -} -static void test_today_my_while_tilt_only_moves() { - // today: a group's My puts the lift target at 100, a remote's leaves it. - Rig g; - tiltOnly(g); - g.b.restore(60, 50, -1, -1); - g.b.onGroupCommand(somfy_commands::Down, 1, g.now); - g.run(1000); - g.b.onGroupCommand(somfy_commands::My, 1, g.now); - ASSERT_POS(100, g.st().target); - - Rig r; - tiltOnly(r); - r.b.restore(60, 50, -1, -1); - r.press(somfy_commands::Down); - r.run(1000); - r.press(somfy_commands::My); - ASSERT_POS(60, r.st().target); +static void test_tilt_only_lift_stays_down_whatever_the_source() { + const Source sources[] = {Source::web, Source::remote, Source::group}; + const somfy_commands cmds[] = {somfy_commands::Up, somfy_commands::Down, somfy_commands::My}; + for(Source src : sources) { + for(somfy_commands cmd : cmds) { + for(bool moving : {false, true}) { + Rig r; + tiltOnly(r, moving); + tiltOnlyCommand(r, src, cmd); + r.run(1000); + char msg[64]; + snprintf(msg, sizeof(msg), "%s %s%s", sourceName(src), translateSomfyCommand(cmd).c_str(), moving ? " while moving" : ""); + TEST_ASSERT_FLOAT_WITHIN_MESSAGE(0.5f, 100, r.st().target, msg); + TEST_ASSERT_FLOAT_WITHIN_MESSAGE(0.5f, 100, r.st().currentPos, msg); + TEST_ASSERT_EQUAL_INT8_MESSAGE(0, r.st().direction, msg); + // What the tilt does. + if(cmd == somfy_commands::Up) TEST_ASSERT_FLOAT_WITHIN_MESSAGE(0.5f, 0, r.st().tiltTarget, msg); + else if(cmd == somfy_commands::Down) TEST_ASSERT_FLOAT_WITHIN_MESSAGE(0.5f, 100, r.st().tiltTarget, msg); + else if(!moving) TEST_ASSERT_FLOAT_WITHIN_MESSAGE(0.5f, 30, r.st().tiltTarget, msg); + } + } + } } /********************************************************************* @@ -650,10 +658,7 @@ int main(int argc, char **argv) { RUN_TEST(test_sun_flag_frame_turns_it_on_and_saves); RUN_TEST(test_integrated_tilt_turns_the_slats_first); RUN_TEST(test_today_integrated_tilt_turns_back_after_a_lift_target); - RUN_TEST(test_today_up_on_tilt_only_from_the_web); - RUN_TEST(test_today_up_on_tilt_only_from_a_remote); - RUN_TEST(test_today_up_on_tilt_only_from_a_group); - RUN_TEST(test_today_my_while_tilt_only_moves); + RUN_TEST(test_tilt_only_lift_stays_down_whatever_the_source); RUN_TEST(test_group_down_and_step); RUN_TEST(test_group_sun_flag_needs_a_sensor); RUN_TEST(test_set_favorite_here_is_committed_when_all_on_air);