fix: step a remote's step by the frame's own step size

The step rule read lastFrame.stepSize, which copy() never updates, so a
remote's step moved by whatever step size this shade last sent. It now
reads the frame's step size, 1 when the frame has none. Reconcile Q19 in
SHADE_BEHAVIOUR_PLAN.md.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
cjkas 2026-10-16 22:39:58 +02:00
parent 9e519d4d5f
commit 2cf3c4b542
3 changed files with 22 additions and 13 deletions

View file

@ -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). - ~~`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`). - 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`). - 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`). - 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). - The self-echo (Q5) and merging group commands into the frame rule (Q8).
- `lastFrame` doing two jobs (the Q6 revisit). - `lastFrame` doing two jobs (the Q6 revisit).

View file

@ -661,8 +661,8 @@ void ShadeBehaviour::onFrame(somfy_frame_t &frame, bool internal, uint32_t now)
this->lastFrame.processed = true; this->lastFrame.processed = true;
if(this->settings.shadeType == shade_types::drycontact || this->settings.shadeType == shade_types::drycontact2) return; if(this->settings.shadeType == shade_types::drycontact || this->settings.shadeType == shade_types::drycontact2) return;
dir = 0; dir = 0;
if(this->lastFrame.stepSize == 0) this->lastFrame.stepSize = 1; // The number of steps the frame asks for.
if(!this->applyStep(-1, this->lastFrame.stepSize)) return; if(!this->applyStep(-1, frame.stepSize == 0 ? 1 : frame.stepSize)) return;
this->emitCommand(cmd, internal ? "internal" : "remote", frame.remoteAddress); this->emitCommand(cmd, internal ? "internal" : "remote", frame.remoteAddress);
break; break;
case somfy_commands::StepDown: case somfy_commands::StepDown:
@ -670,8 +670,8 @@ void ShadeBehaviour::onFrame(somfy_frame_t &frame, bool internal, uint32_t now)
this->lastFrame.processed = true; this->lastFrame.processed = true;
if(this->settings.shadeType == shade_types::drycontact || this->settings.shadeType == shade_types::drycontact2) return; if(this->settings.shadeType == shade_types::drycontact || this->settings.shadeType == shade_types::drycontact2) return;
dir = 1; dir = 1;
if(this->lastFrame.stepSize == 0) this->lastFrame.stepSize = 1; // The number of steps the frame asks for.
if(!this->applyStep(1, this->lastFrame.stepSize)) return; if(!this->applyStep(1, frame.stepSize == 0 ? 1 : frame.stepSize)) return;
this->emitCommand(cmd, internal ? "internal" : "remote", frame.remoteAddress); this->emitCommand(cmd, internal ? "internal" : "remote", frame.remoteAddress);
break; break;
case somfy_commands::Toggle: case somfy_commands::Toggle:

View file

@ -111,10 +111,11 @@ struct Rig {
void onAir() { this->event(tx_event_t::started, this->lastSent().ticket); } void onAir() { this->event(tx_event_t::started, this->lastSent().ticket); }
const Sent &lastSent() const { return this->link.sent.back(); } const Sent &lastSent() const { return this->link.sent.back(); }
// A remote press: the first frame, then repeats more of the same frame. // 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; somfy_frame_t frame;
frame.valid = true; frame.valid = true;
frame.cmd = cmd; frame.cmd = cmd;
frame.stepSize = stepSize;
frame.remoteAddress = address; frame.remoteAddress = address;
frame.rollingCode = ++this->remoteCode; frame.rollingCode = ++this->remoteCode;
this->b.onFrame(frame, false, this->now); 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(50, h.st().target);
ASSERT_POS(0, h.st().tiltTarget); ASSERT_POS(0, h.st().tiltTarget);
} }
static void test_remote_step_uses_the_step_size_last_sent() { static void test_remote_step_uses_the_frames_step_size() {
// today: copy() never copies stepSize, so a remote step uses whatever step
// size this shade last sent, or 1 when that was 0.
Rig r; Rig r;
r.b.restore(50, 0, -1, -1); r.b.restore(50, 0, -1, -1);
// A frame without a step size is one step.
r.press(somfy_commands::StepDown); r.press(somfy_commands::StepDown);
ASSERT_POS(51, r.st().target); ASSERT_POS(51, r.st().target);
r.lastFrame.stepSize = 3; // What this shade last sent doesn't matter.
r.lastFrame.processed = true; r.lastFrame.stepSize = 7;
r.press(somfy_commands::StepDown); 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); ASSERT_POS(53, r.st().target);
} }
static void test_wind_blocks_a_remote_down() { 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_when_idle_goes_to_the_favorite_after_half_a_second);
RUN_TEST(test_remote_my_held_sets_the_favorite); RUN_TEST(test_remote_my_held_sets_the_favorite);
RUN_TEST(test_remote_held_up_on_a_tilt_motor_tilts); 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_blocks_a_remote_down);
RUN_TEST(test_wind_retracts_the_shade); RUN_TEST(test_wind_retracts_the_shade);
RUN_TEST(test_sun_flag_frame_turns_it_on_and_saves); RUN_TEST(test_sun_flag_frame_turns_it_on_and_saves);