fix(home): restore off state on home_undo after set_temperature (#455)#460
Conversation
When HVAC was off before a setpoint change, capture turn_off as undo_restore (mirror set_brightness off handling). Fixes GeniePod#455.
The off->turn_off restore is fully covered by the actuation.rs unit test, and the home_control->ledger->home_undo round-trip is already exercised by GeniePod#435's home_undo_restores_prior_brightness_after_dim (exec_home_control_inner reads prior state identically for any setpoint action). Keeping the diff to the 6-line fix + its unit test and not growing the shared RecordingHomeProvider fixture.
|
@kiannidev — I trimmed this PR directly: dropped the dispatch.rs changes (the Why I pulled it back: that end-to-end test was ~131 lines of new fixture for coverage we already have — the off→ A note for future PRs, please: keep them tight and scoped — the smallest diff that fixes the issue with proof. Favor a focused unit test over a heavy end-to-end one when the logic is already unit-testable, and don't expand shared test fixtures unless several tests genuinely need it. It keeps review fast and the suite lean. This is the same over-scoping that got #457 closed, so worth internalizing. The fix itself is correct — thanks for catching #455. |
ai-hpc
left a comment
There was a problem hiding this comment.
Approve ✅ — with the dispatch.rs noise dropped, this is now the minimal correct fix: undo_restore_from_prior("set_temperature") returns turn_off when the prior state was off (mirroring the set_brightness arm), covered by the focused undo_restore_from_prior_set_temperature_when_off unit test. Verified locally — the unit test passes; the end-to-end ledger/undo path is already covered by #435. Closes #455.
Fixes #455
Summary
Mirror the
set_brightnessoff-state handling inundo_restore_from_priorforset_temperature. When the HVAC wasoffbefore a setpoint change,home_undonow recordsturn_offas the restore action instead of leavingundo_restoreempty (which skipped the thermostat change and reversed an unrelated earlier action).Changes
undo_restore_from_prior("set_temperature"): priorstate == "off"→UndoRestore { action: "turn_off" }.set_temperature(72)→home_undo→turn_off(climate back off).RecordingHomeProviderstub with climate domain for the integration test.Real Behavior Proof
Tested profile / hardware (check all that apply):
jetsonraspberry_piportable_sbclaptopmacWhat I ran
Environment: x86_64 Linux dev host (
laptopprofile). Stub provider test + unit tests; optional live HA repro below.Optional live HA repro:
What I observed
Before (
main):undo_restore_from_prior("set_temperature", prior_off)→None.home_undoafter heating an off thermostat could skip the temperature change.After (this PR):
turn_off; heating prior withtemperature: 68→ undo restoresset_temperature(68).Repro caveat: Some HA climate entities expose
temperaturewhilestateisoff; this fix targets thestate == "off"path aligned with #435 brightness handling.Test plan
cargo test -p genie-core undo_restore_from_prior_set_temperaturecargo test -p genie-core home_undo_restores_off_state_after_set_temperaturecargo test -p genie-core home_undocargo clippy -p genie-core -- -D warningsNotes for reviewers