Cancel in-flight cover/climate action before starting a new one - #28
Merged
Conversation
Tapping Stop (or the icon) while a cover is opening/closing appeared to stop
it, but ~2s later it snapped fully open/closed anyway. The VM assigned
actionTask = Task { ... } for each action without cancelling the previous task,
so the still-running demo open/close bypassed its `guard !Task.isCancelled`
guard and overwrote the state Stop had just set.
Cancel the outstanding actionTask before starting each new action
(open/close/stop/setPosition/toggle) so the in-flight demo travel bails.
Add a Swift Testing case with a slow spy CoverControlling that mirrors the
demo guard, verifying stop during open cancels the open so it never completes.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The same fire-and-forget `actionTask = Task { ... }` pattern in
ClimateCardViewModel replaced the task reference without cancelling the prior
task, so a still-running demo action (post-sleep `guard !Task.isCancelled`)
could overwrite the state a newer action just applied — the same defect just
fixed for Cover. Cancel the outstanding actionTask before starting each new
climate action.
Also extend cover cancellation coverage to close(), setPosition(), and
toggle() (via iconTapped), which the original fix changed but left untested.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
CoverCardViewModel: cancel the outstandingactionTaskbefore each new action (setPosition,open,close,stop,toggle).ClimateCardViewModel: apply the same cancel-before-reassign to all action methods (togglePower,setHVACMode,setTemperature,setTemperatureRange,setFanMode,setSwingMode,setPresetMode,setHumidity) — it had the identical latent race.stop/close/setPosition/toggle(viaiconTapped) and a climate cancellation test, each using a hand-written slow spy that mirrors the demo controller's post-sleepguard !Task.isCancelled.Why
Tapping Stop (or the card icon) while a cover was opening/closing appeared to stop it, but ~2s later it snapped fully open/closed anyway. Each action assigned
actionTask = Task { ... }without cancelling the previous task, so the still-running demo open/close bypassed itsguard !Task.isCancelledand overwrote the state the newer action had just set.ClimateCardViewModelshared the exact same pattern and demo guard, so the same fix is applied there.Production behavior is unchanged:
HAServiceCaller.callServiceis continuation-based and not cancellation-aware, socancel()only sets the flag that the demo controllers check — it never aborts a real WebSocket call.Notes
HemeraTestspass (372 tests). The cancellation tests complete in milliseconds, confirming the in-flight 2s demo task is cancelled and bails at its guard rather than running to completion.