Merge pull request #59 from cjkas/cleanup/waiting-frame-steps

cleanup: drop the step cases onWaitingFrame never reaches

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;

src/ShadeBehaviour.cpp

@@ -364,24 +364,7 @@ void ShadeBehaviour::onWaitingFrame(uint32_t now) {
   if(this->lastFrame.await > 0 && static_cast<int32_t>(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.

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