mirror of
https://github.com/rstrouse/ESPSomfy-RTS.git
synced 2026-10-31 22:02:17 +01:00
fix: end the positioning when the lift reaches its target
Arriving at a lift target never cleared settingPos (the tilt arrival clears settingTiltPos), so the next step or group move also sent a stop when it arrived. A stop to a motor that has already stopped is a My, which sends it to its favorite. Reconcile Q18 in SHADE_BEHAVIOUR_PLAN.md. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
4f8eac03ac
commit
9e519d4d5f
3 changed files with 30 additions and 8 deletions
|
|
@ -85,7 +85,7 @@ The decisions are the user's. Each question comes with a recommended answer, and
|
||||||
### Still open
|
### Still open
|
||||||
|
|
||||||
- **Reconcile: what is decided first, and in what order.** The candidates, each pinned by a "today:" test in `test_shadebehaviour` unless marked:
|
- **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 (`test_today_a_step_after_move_to_target_sends_a_stop`).
|
- ~~`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*, because `copy()` never copies `stepSize` (`test_remote_step_uses_the_step_size_last_sent`).
|
||||||
|
|
@ -152,3 +152,11 @@ The decisions are the user's. Each question comes with a recommended answer, and
|
||||||
- **`onTxEvent` takes no `now`:** the event carries its own time.
|
- **`onTxEvent` takes no `now`:** the event carries its own time.
|
||||||
- **A known ordering change:** when a simMy shade's Favorite or My sends again from inside its own echo, a shade linked to it now hears the outer frame before the inner one. Before, it depended on the two shades' slots. Nothing else changes order.
|
- **A known ordering change:** when a simMy shade's Favorite or My sends again from inside its own echo, a shade linked to it now hears the outer frame before the inner one. Before, it depended on the two shades' slots. Nothing else changes order.
|
||||||
- **Flash:** `esp32dev` `firmware.bin` grows 1,397,776 → 1,399,776 B (+2,000 B, 88.6%), mostly the virtual calls through the two seams.
|
- **Flash:** `esp32dev` `firmware.bin` grows 1,397,776 → 1,399,776 B (+2,000 B, 88.6%), mostly the virtual calls through the two seams.
|
||||||
|
|
||||||
|
**Reconcile round 1 (2026-10-16), all as recommended.** Each fix is one PR stacked on #47, and each changes a "today:" test.
|
||||||
|
|
||||||
|
- **Q17. Order.** Clear bugs first: `settingPos` (Q18), the remote step size (Q19), group sun flags (Q20), the long-press wrap (Q21). Then the rules where the copies disagree: tilt-only Up/Down/My, and the integrated tilt turning back. The structural changes (self-echo, merging group commands, splitting `lastFrame`) wait until those are done and may be dropped.
|
||||||
|
- **Q18. `settingPos` is cleared on a lift arrival,** as `settingTiltPos` is on a tilt arrival. A later step or group move then stops by itself instead of getting a My, which would have sent a stopped motor to its favorite.
|
||||||
|
- **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`.
|
||||||
|
|
|
||||||
|
|
@ -135,6 +135,7 @@ void ShadeBehaviour::tick(uint32_t now) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
this->p_direction(0);
|
this->p_direction(0);
|
||||||
|
this->settingPos = false;
|
||||||
this->tiltStart = curTime;
|
this->tiltStart = curTime;
|
||||||
this->startTiltPos = this->s.currentTiltPos;
|
this->startTiltPos = this->s.currentTiltPos;
|
||||||
if(this->isAtTarget()) this->commitShadePosition();
|
if(this->isAtTarget()) this->commitShadePosition();
|
||||||
|
|
@ -161,6 +162,7 @@ void ShadeBehaviour::tick(uint32_t now) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
this->p_direction(0);
|
this->p_direction(0);
|
||||||
|
this->settingPos = false;
|
||||||
this->tiltStart = curTime;
|
this->tiltStart = curTime;
|
||||||
this->startTiltPos = this->s.currentTiltPos;
|
this->startTiltPos = this->s.currentTiltPos;
|
||||||
if(this->isAtTarget()) this->commitShadePosition();
|
if(this->isAtTarget()) this->commitShadePosition();
|
||||||
|
|
|
||||||
|
|
@ -251,10 +251,9 @@ static void test_refused_stop_runs_on_to_the_limit() {
|
||||||
TEST_ASSERT_EQUAL(1, r.link.sent.size());
|
TEST_ASSERT_EQUAL(1, r.link.sent.size());
|
||||||
ASSERT_POS(100, r.st().target);
|
ASSERT_POS(100, r.st().target);
|
||||||
}
|
}
|
||||||
static void test_today_a_step_after_move_to_target_sends_a_stop() {
|
static void test_a_step_after_move_to_target_sends_no_stop() {
|
||||||
// today: arriving at a lift target never clears settingPos (the tilt
|
// The stop at the target ends the positioning, so a later step stops by
|
||||||
// blocks do clear settingTiltPos), so a later step also sends a stop when
|
// itself. A stop sent then would be a My to a still motor: its favorite.
|
||||||
// it arrives. On the "still open" list of LOOP_QUEUE_PLAN.md.
|
|
||||||
Rig r;
|
Rig r;
|
||||||
r.b.moveToTarget(40, -1, r.now);
|
r.b.moveToTarget(40, -1, r.now);
|
||||||
r.onAir();
|
r.onAir();
|
||||||
|
|
@ -264,8 +263,20 @@ static void test_today_a_step_after_move_to_target_sends_a_stop() {
|
||||||
r.b.sendCommand(somfy_commands::StepDown, 1, 1, r.now);
|
r.b.sendCommand(somfy_commands::StepDown, 1, 1, r.now);
|
||||||
r.onAir();
|
r.onAir();
|
||||||
r.run(2000);
|
r.run(2000);
|
||||||
TEST_ASSERT_EQUAL(before + 2, r.link.sent.size());
|
TEST_ASSERT_EQUAL(before + 1, r.link.sent.size());
|
||||||
ASSERT_CMD(somfy_commands::My, r.lastSent().cmd);
|
ASSERT_CMD(somfy_commands::StepDown, r.lastSent().cmd);
|
||||||
|
TEST_ASSERT_EQUAL_INT8(0, r.st().direction);
|
||||||
|
}
|
||||||
|
static void test_a_group_step_after_move_to_target_sends_no_stop() {
|
||||||
|
Rig r;
|
||||||
|
r.b.moveToTarget(40, -1, r.now);
|
||||||
|
r.onAir();
|
||||||
|
r.run(4100);
|
||||||
|
const size_t before = r.link.sent.size();
|
||||||
|
r.b.onGroupCommand(somfy_commands::StepDown, 1, r.now);
|
||||||
|
r.run(1000);
|
||||||
|
ASSERT_POS(41, r.st().currentPos);
|
||||||
|
TEST_ASSERT_EQUAL(before, r.link.sent.size());
|
||||||
}
|
}
|
||||||
static void test_toggle_on_a_56_bit_shade_sends_my() {
|
static void test_toggle_on_a_56_bit_shade_sends_my() {
|
||||||
Rig r;
|
Rig r;
|
||||||
|
|
@ -602,7 +613,8 @@ int main(int argc, char **argv) {
|
||||||
RUN_TEST(test_move_to_target_sends_a_stop_there);
|
RUN_TEST(test_move_to_target_sends_a_stop_there);
|
||||||
RUN_TEST(test_dropped_stop_runs_on_to_the_limit);
|
RUN_TEST(test_dropped_stop_runs_on_to_the_limit);
|
||||||
RUN_TEST(test_refused_stop_runs_on_to_the_limit);
|
RUN_TEST(test_refused_stop_runs_on_to_the_limit);
|
||||||
RUN_TEST(test_today_a_step_after_move_to_target_sends_a_stop);
|
RUN_TEST(test_a_step_after_move_to_target_sends_no_stop);
|
||||||
|
RUN_TEST(test_a_group_step_after_move_to_target_sends_no_stop);
|
||||||
RUN_TEST(test_toggle_on_a_56_bit_shade_sends_my);
|
RUN_TEST(test_toggle_on_a_56_bit_shade_sends_my);
|
||||||
RUN_TEST(test_flipped_shade_sends_the_other_way);
|
RUN_TEST(test_flipped_shade_sends_the_other_way);
|
||||||
RUN_TEST(test_remote_down_and_my_stop);
|
RUN_TEST(test_remote_down_and_my_stop);
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue