diff --git a/docs/SHADE_BEHAVIOUR_PLAN.md b/docs/SHADE_BEHAVIOUR_PLAN.md index 6c836cc..ca06fe6 100644 --- a/docs/SHADE_BEHAVIOUR_PLAN.md +++ b/docs/SHADE_BEHAVIOUR_PLAN.md @@ -88,7 +88,7 @@ The decisions are the user's. Each question comes with a recommended answer, and - ~~`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`). - - A remote step uses the step size this shade last *sent*, because `copy()` never copies `stepSize` (`test_remote_step_uses_the_step_size_last_sent`). + - ~~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 (`test_group_sun_flag_needs_a_sensor`). - The self-echo (Q5) and merging group commands into the frame rule (Q8). - `lastFrame` doing two jobs (the Q6 revisit). diff --git a/src/ShadeBehaviour.cpp b/src/ShadeBehaviour.cpp index 30e04bf..22b1c83 100644 --- a/src/ShadeBehaviour.cpp +++ b/src/ShadeBehaviour.cpp @@ -661,8 +661,8 @@ void ShadeBehaviour::onFrame(somfy_frame_t &frame, bool internal, uint32_t now) this->lastFrame.processed = true; if(this->settings.shadeType == shade_types::drycontact || this->settings.shadeType == shade_types::drycontact2) return; dir = 0; - if(this->lastFrame.stepSize == 0) this->lastFrame.stepSize = 1; - if(!this->applyStep(-1, this->lastFrame.stepSize)) return; + // The number of steps the frame asks for. + if(!this->applyStep(-1, frame.stepSize == 0 ? 1 : frame.stepSize)) return; this->emitCommand(cmd, internal ? "internal" : "remote", frame.remoteAddress); break; case somfy_commands::StepDown: @@ -670,8 +670,8 @@ void ShadeBehaviour::onFrame(somfy_frame_t &frame, bool internal, uint32_t now) this->lastFrame.processed = true; if(this->settings.shadeType == shade_types::drycontact || this->settings.shadeType == shade_types::drycontact2) return; dir = 1; - if(this->lastFrame.stepSize == 0) this->lastFrame.stepSize = 1; - if(!this->applyStep(1, this->lastFrame.stepSize)) return; + // The number of steps the frame asks for. + if(!this->applyStep(1, frame.stepSize == 0 ? 1 : frame.stepSize)) return; this->emitCommand(cmd, internal ? "internal" : "remote", frame.remoteAddress); break; case somfy_commands::Toggle: diff --git a/test/test_shadebehaviour/test_shadebehaviour.cpp b/test/test_shadebehaviour/test_shadebehaviour.cpp index c5fa0f7..cf4cba8 100644 --- a/test/test_shadebehaviour/test_shadebehaviour.cpp +++ b/test/test_shadebehaviour/test_shadebehaviour.cpp @@ -111,10 +111,11 @@ struct Rig { void onAir() { this->event(tx_event_t::started, this->lastSent().ticket); } const Sent &lastSent() const { return this->link.sent.back(); } // A remote press: the first frame, then repeats more of the same frame. - void press(somfy_commands cmd, uint8_t repeats = 0, uint32_t address = ADDRESS) { + void press(somfy_commands cmd, uint8_t repeats = 0, uint32_t address = ADDRESS, uint8_t stepSize = 0) { somfy_frame_t frame; frame.valid = true; frame.cmd = cmd; + frame.stepSize = stepSize; frame.remoteAddress = address; frame.rollingCode = ++this->remoteCode; this->b.onFrame(frame, false, this->now); @@ -353,16 +354,23 @@ static void test_remote_held_up_on_a_tilt_motor_tilts() { ASSERT_POS(50, h.st().target); ASSERT_POS(0, h.st().tiltTarget); } -static void test_remote_step_uses_the_step_size_last_sent() { - // today: copy() never copies stepSize, so a remote step uses whatever step - // size this shade last sent, or 1 when that was 0. +static void test_remote_step_uses_the_frames_step_size() { Rig r; r.b.restore(50, 0, -1, -1); + // A frame without a step size is one step. r.press(somfy_commands::StepDown); ASSERT_POS(51, r.st().target); - r.lastFrame.stepSize = 3; - r.lastFrame.processed = true; - r.press(somfy_commands::StepDown); + // What this shade last sent doesn't matter. + r.lastFrame.stepSize = 7; + r.press(somfy_commands::StepDown, 0, ADDRESS, 3); + // Steps count from where the shade is, still 50. + ASSERT_POS(53, r.st().target); +} +static void test_web_step_uses_the_step_size_sent() { + Rig r; + r.b.restore(50, 0, -1, -1); + r.b.sendCommand(somfy_commands::StepDown, 1, 3, r.now); + TEST_ASSERT_EQUAL_UINT8(3, r.lastSent().stepSize); ASSERT_POS(53, r.st().target); } static void test_wind_blocks_a_remote_down() { @@ -621,7 +629,8 @@ int main(int argc, char **argv) { RUN_TEST(test_remote_my_when_idle_goes_to_the_favorite_after_half_a_second); 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_step_size_last_sent); + RUN_TEST(test_remote_step_uses_the_frames_step_size); + 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); RUN_TEST(test_sun_flag_frame_turns_it_on_and_saves);