From 9df2638f03a504fc3fc142be0a6aae914a52fc4b Mon Sep 17 00:00:00 2001 From: cjkas Date: Sat, 17 Oct 2026 23:25:16 +0200 Subject: [PATCH] cleanup: drop the step cases onWaitingFrame never reaches onWaitingFrame only acts on a frame that isn't processed yet, and the step rule in onFrame marks a step frame processed as soon as it arrives; a repeat or synonym keeps the mark. So the "fixed 1% step of a held step button" never ran. A new test pins what a held step button does: one step by the frame's step size. SOMFY_SPLIT_PLAN.md is corrected. Co-Authored-By: Claude Opus 5.5 --- docs/SOMFY_SPLIT_PLAN.md | 2 +- src/ShadeBehaviour.cpp | 19 +------------------ .../test_shadebehaviour.cpp | 13 +++++++++++++ 3 files changed, 15 insertions(+), 19 deletions(-) diff --git a/docs/SOMFY_SPLIT_PLAN.md b/docs/SOMFY_SPLIT_PLAN.md index 1dcd82f..5b61edd 100644 --- a/docs/SOMFY_SPLIT_PLAN.md +++ b/docs/SOMFY_SPLIT_PLAN.md @@ -90,7 +90,7 @@ What changed in behaviour: - **Step edge cases**: a group step on a tilt-only shade already at its end now goes through like a remote step. A step that can't be sized (no travel time) leaves the targets alone. - The 8 debug logs of the sun and wind timers are gone. They were compiled out at `CORE_DEBUG_LEVEL=3`. -Kept as it was: the fixed 1% step of a held step button in `processWaitingFrame`. +Kept as it was: the fixed 1% step of a held step button in `processWaitingFrame`. It later turned out never to run: a step frame is acted on as soon as it arrives, so it is never left waiting. The branch was deleted, and `test_a_held_step_button_steps_once` pins what a held step button does. Left out on purpose, since neither fixes a bug or can be reached by the native tests: - the four near-copy arrive-at-target blocks in `checkMovement`. They are now in `ShadeBehaviour::tick` ([SHADE_BEHAVIOUR_PLAN.md](SHADE_BEHAVIOUR_PLAN.md)), still four copies, but pinned by native tests; diff --git a/src/ShadeBehaviour.cpp b/src/ShadeBehaviour.cpp index 647888c..b6c3d37 100644 --- a/src/ShadeBehaviour.cpp +++ b/src/ShadeBehaviour.cpp @@ -364,24 +364,7 @@ void ShadeBehaviour::onWaitingFrame(uint32_t now) { if(this->lastFrame.await > 0 && static_cast(now - this->lastFrame.await) > 0) { somfy_commands cmd = this->transformCommand(this->lastFrame.cmd); switch(cmd) { - case somfy_commands::StepUp: - this->lastFrame.processed = true; - // Simply move the shade up by 1%. - if(this->s.currentPos > 0) { - this->p_target(floor(this->s.currentPos) - 1); - this->setMovement(-1, now); - this->emitCommand(cmd, "remote", this->lastFrame.remoteAddress); - } - break; - case somfy_commands::StepDown: - this->lastFrame.processed = true; - // Simply move the shade down by 1%. - if(this->s.currentPos < 100) { - this->p_target(floor(this->s.currentPos) + 1); - this->setMovement(1, now); - this->emitCommand(cmd, "remote", this->lastFrame.remoteAddress); - } - break; + // A step acts as soon as it arrives (onFrame), so it is never waiting here. case somfy_commands::Down: case somfy_commands::Up: if(this->settings.tiltType == tilt_types::tiltmotor) { // Theoretically this should get here unless it does have a tilt motor. diff --git a/test/test_shadebehaviour/test_shadebehaviour.cpp b/test/test_shadebehaviour/test_shadebehaviour.cpp index 920d30d..0f63b11 100644 --- a/test/test_shadebehaviour/test_shadebehaviour.cpp +++ b/test/test_shadebehaviour/test_shadebehaviour.cpp @@ -377,6 +377,18 @@ static void test_remote_step_uses_the_frames_step_size() { // Steps count from where the shade is, still 50. ASSERT_POS(53, r.st().target); } +static void test_a_held_step_button_steps_once() { + // The repeats of a held step button are the same frame, so it steps once, + // by the frame's step size, and nothing more once the press is over. + Rig r; + r.b.restore(50, 0, -1, -1); + r.press(somfy_commands::StepDown, 20, ADDRESS, 2); + ASSERT_POS(52, r.st().target); + r.wait(1000); + ASSERT_POS(52, r.st().target); + ASSERT_POS(52, r.st().currentPos); + TEST_ASSERT_EQUAL(1, r.events.commands.size()); +} static void test_web_step_uses_the_step_size_sent() { Rig r; r.b.restore(50, 0, -1, -1); @@ -652,6 +664,7 @@ int main(int argc, char **argv) { RUN_TEST(test_remote_my_held_sets_the_favorite); RUN_TEST(test_remote_held_up_on_a_tilt_motor_tilts); RUN_TEST(test_remote_step_uses_the_frames_step_size); + RUN_TEST(test_a_held_step_button_steps_once); RUN_TEST(test_web_step_uses_the_step_size_sent); RUN_TEST(test_wind_blocks_a_remote_down); RUN_TEST(test_wind_retracts_the_shade);