diff --git a/Hemera/Entities/Climate/UI/ClimateCardViewModel.swift b/Hemera/Entities/Climate/UI/ClimateCardViewModel.swift index 5322d93..1add9b7 100644 --- a/Hemera/Entities/Climate/UI/ClimateCardViewModel.swift +++ b/Hemera/Entities/Climate/UI/ClimateCardViewModel.swift @@ -200,6 +200,7 @@ final class ClimateCardViewModel: Identifiable { func togglePower() { guard climate.isAvailable else { return } + actionTask?.cancel() if climate.state == .off { actionTask = Task { await controller.turnOnClimate(id) } } else { @@ -209,6 +210,7 @@ final class ClimateCardViewModel: Identifiable { func setHVACMode(_ mode: ClimateEntity.HVACMode) { guard climate.isAvailable else { return } + actionTask?.cancel() actionTask = Task { await controller.setHVACMode(id, mode: mode.rawValue) } } @@ -217,6 +219,7 @@ final class ClimateCardViewModel: Identifiable { resetPending() pendingTargetTemp = temperature cooldown.commit() + actionTask?.cancel() actionTask = Task { await controller.setTemperature(id, temperature: temperature) } } @@ -226,6 +229,7 @@ final class ClimateCardViewModel: Identifiable { pendingTargetTempLow = low pendingTargetTempHigh = high cooldown.commit() + actionTask?.cancel() actionTask = Task { await controller.setTemperatureRange(id, low: low, high: high) } } @@ -243,21 +247,25 @@ final class ClimateCardViewModel: Identifiable { func setFanMode(_ mode: String) { guard climate.isAvailable else { return } + actionTask?.cancel() actionTask = Task { await controller.setFanMode(id, mode: mode) } } func setSwingMode(_ mode: String) { guard climate.isAvailable else { return } + actionTask?.cancel() actionTask = Task { await controller.setSwingMode(id, mode: mode) } } func setPresetMode(_ mode: String) { guard climate.isAvailable else { return } + actionTask?.cancel() actionTask = Task { await controller.setPresetMode(id, mode: mode) } } func setHumidity(_ humidity: Double) { guard climate.isAvailable else { return } + actionTask?.cancel() actionTask = Task { await controller.setHumidity(id, humidity: humidity) } } diff --git a/Hemera/Entities/Cover/UI/CoverCardViewModel.swift b/Hemera/Entities/Cover/UI/CoverCardViewModel.swift index a596f51..2faa555 100644 --- a/Hemera/Entities/Cover/UI/CoverCardViewModel.swift +++ b/Hemera/Entities/Cover/UI/CoverCardViewModel.swift @@ -134,6 +134,7 @@ final class CoverCardViewModel: Identifiable { func setPosition(to position: Int) { guard cover.isAvailable else { return } + actionTask?.cancel() pendingPosition = position cooldown.commit() actionTask = Task { @@ -171,6 +172,7 @@ final class CoverCardViewModel: Identifiable { func open() { guard cover.isAvailable else { return } + actionTask?.cancel() actionTask = Task { await controller.openCover(id) } @@ -178,6 +180,7 @@ final class CoverCardViewModel: Identifiable { func close() { guard cover.isAvailable else { return } + actionTask?.cancel() actionTask = Task { await controller.closeCover(id) } @@ -185,12 +188,14 @@ final class CoverCardViewModel: Identifiable { func stop() { guard cover.isAvailable else { return } + actionTask?.cancel() actionTask = Task { await controller.stopCover(id) } } private func toggle() { + actionTask?.cancel() actionTask = Task { await controller.toggleCover(id) } diff --git a/HemeraTests/Entities/ClimateCardViewModelTests.swift b/HemeraTests/Entities/ClimateCardViewModelTests.swift index 446d1db..125f001 100644 --- a/HemeraTests/Entities/ClimateCardViewModelTests.swift +++ b/HemeraTests/Entities/ClimateCardViewModelTests.swift @@ -282,6 +282,29 @@ struct ClimateCardViewModelTests { #expect(spy.setTemperatureCalls.isEmpty) } + // MARK: - Action Cancellation + + /** + A second action must cancel the in-flight one. Otherwise the demo controller's + post-sleep `guard !Task.isCancelled` never fires and the still-running earlier + action overwrites the state the newer action just applied. + */ + @Test func setTemperature_whileActionInFlight_cancelsPriorAction() async { + let controller = SlowClimateControlling() + let vm = makeViewModel(state: .heat, controller: controller) + + vm.setTemperature(20) + let firstTask = vm.actionTask + vm.setHVACMode(.cool) + + await firstTask?.value + await vm.actionTask?.value + + #expect(firstTask?.isCancelled == true) + #expect(controller.didCompleteSetTemperature == false) + #expect(controller.setHVACModeCalls == ["cool"]) + } + // MARK: - Available HVAC Modes @Test func availableHVACModes_parsesFromRaw() { @@ -408,3 +431,31 @@ private final class SpyClimateControlling: ClimateControlling { turnOffCalls.append(id) } } + +/** + Mirrors the demo controller: `setTemperature` simulates a delayed apply, then only + completes if its Task was not cancelled — the same `guard !Task.isCancelled` the bug hinges on. + */ +@MainActor +private final class SlowClimateControlling: ClimateControlling { + var didCompleteSetTemperature = false + var setHVACModeCalls: [String] = [] + + func setHVACMode(_ id: String, mode: String) async { + setHVACModeCalls.append(mode) + } + + func setTemperature(_ id: String, temperature: Double) async { + try? await Task.sleep(for: .seconds(2)) + guard !Task.isCancelled else { return } + didCompleteSetTemperature = true + } + + func setTemperatureRange(_ id: String, low: Double, high: Double) async {} + func setFanMode(_ id: String, mode: String) async {} + func setSwingMode(_ id: String, mode: String) async {} + func setPresetMode(_ id: String, mode: String) async {} + func setHumidity(_ id: String, humidity: Double) async {} + func turnOnClimate(_ id: String) async {} + func turnOffClimate(_ id: String) async {} +} diff --git a/HemeraTests/Entities/CoverCardViewModelTests.swift b/HemeraTests/Entities/CoverCardViewModelTests.swift index b3d96b5..6f10f6e 100644 --- a/HemeraTests/Entities/CoverCardViewModelTests.swift +++ b/HemeraTests/Entities/CoverCardViewModelTests.swift @@ -131,6 +131,82 @@ struct CoverCardViewModelTests { #expect(spy.openedIds.isEmpty) } + // MARK: - Action Cancellation + + /** + Stopping mid-open must cancel the in-flight open Task. Otherwise the demo + controller's post-sleep `guard !Task.isCancelled` never fires and the still-running + open overwrites the state Stop just set (cover snaps fully open ~2s after Stop). + */ + @Test + func stop_duringOpen_cancelsInFlightOpenSoItDoesNotOverwriteState() async { + let controller = SlowCoverControlling() + let vm = makeViewModel(features: [.open, .stop], controller: controller) + + vm.open() + let openTask = vm.actionTask + vm.stop() + + await openTask?.value + await vm.actionTask?.value + + #expect(openTask?.isCancelled == true) + #expect(controller.didCompleteOpen == false) + #expect(controller.stoppedIds == ["cover.test"]) + } + + @Test + func close_duringOpen_cancelsInFlightOpen() async { + let controller = SlowCoverControlling() + let vm = makeViewModel(features: [.open, .close], controller: controller) + + vm.open() + let openTask = vm.actionTask + vm.close() + + await openTask?.value + await vm.actionTask?.value + + #expect(openTask?.isCancelled == true) + #expect(controller.didCompleteOpen == false) + #expect(controller.closedIds == ["cover.test"]) + } + + @Test + func setPosition_duringOpen_cancelsInFlightOpen() async { + let controller = SlowCoverControlling() + let vm = makeViewModel(features: [.open, .setPosition], controller: controller) + + vm.open() + let openTask = vm.actionTask + vm.setPosition(to: 50) + + await openTask?.value + await vm.actionTask?.value + + #expect(openTask?.isCancelled == true) + #expect(controller.didCompleteOpen == false) + #expect(controller.positionCalls == [50]) + } + + @Test + func toggleViaIconTapped_duringOpen_cancelsInFlightOpen() async { + let controller = SlowCoverControlling() + // No .open feature → iconTapped's .closed case falls back to toggle(). + let vm = makeViewModel(state: .closed, features: [], controller: controller) + + vm.open() + let openTask = vm.actionTask + vm.iconTapped() + + await openTask?.value + await vm.actionTask?.value + + #expect(openTask?.isCancelled == true) + #expect(controller.didCompleteOpen == false) + #expect(controller.toggledIds == ["cover.test"]) + } + // MARK: - Simple State Description @Test @@ -317,3 +393,38 @@ private final class SpyCoverControlling: CoverControlling { toggledIds.append(id) } } + +/** + Mirrors the demo controller: `openCover` simulates travel, then only completes if + its Task was not cancelled — the same `guard !Task.isCancelled` the real bug hinges on. + */ +@MainActor +private final class SlowCoverControlling: CoverControlling { + var didCompleteOpen = false + var stoppedIds: [String] = [] + var positionCalls: [Int] = [] + var closedIds: [String] = [] + var toggledIds: [String] = [] + + func setPosition(of id: String, to position: Int) async { + positionCalls.append(position) + } + + func openCover(_ id: String) async { + try? await Task.sleep(for: .seconds(2)) + guard !Task.isCancelled else { return } + didCompleteOpen = true + } + + func closeCover(_ id: String) async { + closedIds.append(id) + } + + func stopCover(_ id: String) async { + stoppedIds.append(id) + } + + func toggleCover(_ id: String) async { + toggledIds.append(id) + } +}