diff --git a/ios/Bisque/Bisque/ContentView.swift b/ios/Bisque/Bisque/ContentView.swift index 6a97b74..d1f3f7f 100644 --- a/ios/Bisque/Bisque/ContentView.swift +++ b/ios/Bisque/Bisque/ContentView.swift @@ -2,6 +2,7 @@ import SwiftUI struct ContentView: View { @Environment(KilnConnection.self) private var connection + @Environment(\.scenePhase) private var scenePhase @State private var store = KilnStore() @State private var hasAttemptedAutoConnect = false @@ -28,6 +29,31 @@ struct ContentView: View { await connection.autoConnect() } } + /* iOS suspends the WebSocket within seconds of backgrounding, and the app + declares no background modes. Without this the socket came back dead + while connectionState still read .connected, so returning to the app + mid-firing showed whatever reading it had when the phone went away — + until the user happened to pull to refresh (#147). + + REST first, then the socket: the snapshot is what makes the screen + correct immediately, and reconnecting alone would leave it wrong until + the next broadcast. It is also the only place a transition that + happened while suspended can be noticed, which is why refreshStatus + runs it rather than just adopting the new value. + + refreshStatus, not refreshAll: the latter also awaits profiles and + settings, so a stall on either would hold both the corrected reading + and the socket restart behind an unrelated request. Those two change + rarely and are refreshed on connect and on pull-to-refresh. */ + .onChange(of: scenePhase) { _, phase in + guard phase == .active else { return } + Task { + if let client = connection.apiClient { + await store.refreshStatus(using: client) + } + connection.resumeIfConnected() + } + } } } diff --git a/ios/Bisque/Bisque/LiveActivity/FiringActivityManager.swift b/ios/Bisque/Bisque/LiveActivity/FiringActivityManager.swift index 1fea5c3..12e918e 100644 --- a/ios/Bisque/Bisque/LiveActivity/FiringActivityManager.swift +++ b/ios/Bisque/Bisque/LiveActivity/FiringActivityManager.swift @@ -7,6 +7,25 @@ final class FiringActivityManager { private var currentActivity: Activity? private let log = Logger(subsystem: "com.bisque.kiln-controller", category: "liveactivity") + /// How long a posted reading stays presentable before ActivityKit renders it + /// as stale. + /// + /// The app has no background modes and no ActivityKit push token, so iOS + /// suspends the WebSocket within seconds of backgrounding and there is + /// nothing to update the activity until the app is opened again (#147). + /// With no `staleDate` the lock screen kept showing that last temperature as + /// current — indefinitely, across an 8–12 hour firing. Wrong and confident is + /// worse than visibly out of date, for a number an operator may act on. + /// + /// The firmware broadcasts every second, so any gap beyond a few seconds is + /// already abnormal; 90s is loose enough to ride out a Wi-Fi blip while the + /// app is foregrounded without flickering to stale. + private static let staleAfter: TimeInterval = 90 + + private static func staleDate(from now: Date = Date()) -> Date { + now.addingTimeInterval(staleAfter) + } + func start(profileName: String) { guard ActivityAuthorizationInfo().areActivitiesEnabled else { return } @@ -28,7 +47,7 @@ final class FiringActivityManager { do { currentActivity = try Activity.request( attributes: attributes, - content: .init(state: initialState, staleDate: nil), + content: .init(state: initialState, staleDate: Self.staleDate()), pushType: nil ) } catch { @@ -56,7 +75,7 @@ final class FiringActivityManager { ) Task { - await activity.update(.init(state: state, staleDate: nil)) + await activity.update(.init(state: state, staleDate: Self.staleDate())) } } @@ -75,6 +94,9 @@ final class FiringActivityManager { ) Task { + /* No staleDate on the terminal content: "Complete" is not a + reading that decays, and marking it stale after 90s would make a + finished firing look like a lost connection. */ await activity.end(.init(state: finalState, staleDate: nil), dismissalPolicy: .after(.now + 300)) } } diff --git a/ios/Bisque/Bisque/Networking/KilnWebSocketManager.swift b/ios/Bisque/Bisque/Networking/KilnWebSocketManager.swift index 5d9eba8..4c30774 100644 --- a/ios/Bisque/Bisque/Networking/KilnWebSocketManager.swift +++ b/ios/Bisque/Bisque/Networking/KilnWebSocketManager.swift @@ -15,6 +15,17 @@ final class KilnWebSocketManager { private var receiveTask: Task? private var shouldReconnect = false + /// Bumped by every `openConnection()`. A receive loop carries the value it + /// was started with and stays silent once it no longer matches. + /// + /// Without it, a loop whose socket has been replaced still reaches its catch + /// block and schedules a reconnect — and that reconnect cancels the *live* + /// socket, whose loop then schedules the next one. Self-sustaining churn at + /// 1–8s intervals, losing updates the whole time. Cancelling the task is not + /// enough on its own: `Task.isCancelled` guards loop entry, not the catch, + /// and `openConnection()` can supersede a loop that was never cancelled. + private var generation = 0 + let updateSubject = PassthroughSubject() let otaSubject = PassthroughSubject() @@ -32,6 +43,10 @@ final class KilnWebSocketManager { } func disconnect() { + // Bumped here too, so a loop cancelled by an explicit disconnect is + // silenced by generation alone rather than relying on shouldReconnect — + // which connect() may already have set back to true. + generation += 1 shouldReconnect = false reconnectTask?.cancel() reconnectTask = nil @@ -45,6 +60,12 @@ final class KilnWebSocketManager { private func openConnection() { guard let url = url else { return } + generation += 1 + let thisGeneration = generation + + // Supersede any loop still running against the outgoing socket, so it + // cannot outlive this call and report the socket it owned as lost. + receiveTask?.cancel() webSocketTask?.cancel(with: .goingAway, reason: nil) // Via URLRequest, not the bare URL, so the handshake can carry the // Authorization header. URLSession permits this on a WebSocket where @@ -62,11 +83,11 @@ final class KilnWebSocketManager { reconnectDelay = 1 receiveTask = Task { [weak self] in - await self?.receiveLoop(task: task) + await self?.receiveLoop(task: task, myGeneration: thisGeneration) } } - nonisolated private func receiveLoop(task: URLSessionWebSocketTask) async { + nonisolated private func receiveLoop(task: URLSessionWebSocketTask, myGeneration: Int) async { while !Task.isCancelled { do { let message = try await task.receive() @@ -100,8 +121,14 @@ final class KilnWebSocketManager { } } catch { await MainActor.run { [weak self] in - self?.isConnected = false - self?.scheduleReconnect() + guard let self, myGeneration == self.generation else { + // This socket was replaced deliberately — by disconnect() + // or by a newer openConnection(). Its failure is expected + // and says nothing about the connection that is live now. + return + } + self.isConnected = false + self.scheduleReconnect() } return } diff --git a/ios/Bisque/Bisque/State/KilnConnection.swift b/ios/Bisque/Bisque/State/KilnConnection.swift index 7b39d08..6ff7a09 100644 --- a/ios/Bisque/Bisque/State/KilnConnection.swift +++ b/ios/Bisque/Bisque/State/KilnConnection.swift @@ -75,6 +75,23 @@ final class KilnConnection { connectionState = .disconnected } + /// Re-establishes the live feed after the app has been suspended (#147). + /// + /// iOS tears the WebSocket down within seconds of backgrounding, and the app + /// declares no background modes, so on return the socket is dead while + /// `connectionState` still reads `.connected` and the UI presents whatever + /// reading it had when the phone went in a pocket. Nothing noticed until the + /// user pulled to refresh. + /// + /// Only meaningful when a connection was already established: a foreground + /// event must not start dialling a kiln the user never connected to, or + /// overwrite an `.error` they still need to read. + func resumeIfConnected() { + guard case .connected = connectionState, !host.isEmpty else { return } + webSocket.disconnect() + webSocket.connect(host: host, port: port, apiToken: apiToken) + } + /// Returns false if the token could not be persisted, so the caller can say /// so rather than letting the user discover it as a lockout on next launch /// (#151). The in-memory token is still adopted either way — the current diff --git a/ios/Bisque/Bisque/State/KilnStore.swift b/ios/Bisque/Bisque/State/KilnStore.swift index 5e8b846..d8d76fc 100644 --- a/ios/Bisque/Bisque/State/KilnStore.swift +++ b/ios/Bisque/Bisque/State/KilnStore.swift @@ -15,6 +15,14 @@ final class KilnStore { private var cancellables = Set() private var previousStatus: String = "idle" + /// False until the first status has been seen from either source. + /// + /// `previousStatus` starts at "idle", which is indistinguishable from an + /// observed idle — so without this the first snapshot after launch would + /// treat a kiln found mid-error, or already complete, as a fresh transition + /// and fire a notification for a firing the app never watched. + private var hasObservedStatus = false + /// Highest temperature seen across the current firing. /// /// The completion notification used to report `progress.currentTemp` at the @@ -38,37 +46,70 @@ final class KilnStore { .store(in: &cancellables) } + /// Applies a `/status` snapshot, running any status transition it reveals. + /// + /// Separate from `refreshAll` so the live figures are never held hostage to + /// the profile and settings requests: those were awaited together, so a + /// failure or a stall on either discarded a perfectly good status and left + /// the suspended reading on screen until the resource timeout expired. + func refreshStatus(using client: KilnAPIClient) async { + do { + let status = try await client.getStatus() + apply(status: status) + } catch { + self.error = error.localizedDescription + } + } + + private func apply(status: StatusResponse) { + progress = FiringProgress( + isActive: status.isActive, + profileId: status.profileId, + startTime: nil, + currentTemp: status.currentTemp, + targetTemp: status.targetTemp, + currentSegment: status.currentSegment, + totalSegments: status.totalSegments, + elapsedTime: status.elapsedTime, + estimatedTimeRemaining: status.estimatedTimeRemaining, + status: status.status + ) + + /* Run the transition rather than just adopting the new value. + A firing that completed while the app was suspended is only ever + visible in this snapshot: assigning previousStatus directly meant the + next WebSocket frame carried the same status, so `newStatus != + previousStatus` was false and the transition was lost for good — the + Live Activity was never ended, and peakTempC kept its high-water mark + into the following firing's completion notification. The same lie + #146 was about, arriving by a different route. + + Gated on having observed a status before, so the first load after + launch does not announce a firing that ended before the app ran. */ + if hasObservedStatus, status.status != previousStatus { + handleStatusTransition(from: previousStatus, to: status.status) + } + previousStatus = status.status + hasObservedStatus = true + } + func refreshAll(using client: KilnAPIClient) async { isLoading = true error = nil + // Status first and on its own, so it lands even if the rest fails. + await refreshStatus(using: client) + do { - async let statusTask = client.getStatus() async let profilesTask = client.getProfiles() async let settingsTask = client.getSettings() - - let (status, profiles, settings) = try await (statusTask, profilesTask, settingsTask) - - self.progress = FiringProgress( - isActive: status.isActive, - profileId: status.profileId, - startTime: nil, - currentTemp: status.currentTemp, - targetTemp: status.targetTemp, - currentSegment: status.currentSegment, - totalSegments: status.totalSegments, - elapsedTime: status.elapsedTime, - estimatedTimeRemaining: status.estimatedTimeRemaining, - status: status.status - ) - self.previousStatus = status.status + let (profiles, settings) = try await (profilesTask, settingsTask) self.profiles = profiles self.settings = settings - self.isLoading = false } catch { self.error = error.localizedDescription - self.isLoading = false } + self.isLoading = false } func loadHistory(using client: KilnAPIClient) async { @@ -122,10 +163,11 @@ final class KilnStore { // Detect status transitions for notifications and Live Activity let newStatus = update.status - if newStatus != previousStatus { + if hasObservedStatus, newStatus != previousStatus { handleStatusTransition(from: previousStatus, to: newStatus) - previousStatus = newStatus } + previousStatus = newStatus + hasObservedStatus = true // Update Live Activity if update.isActive { diff --git a/ios/Bisque/BisqueLiveActivity/BisqueLiveActivity.swift b/ios/Bisque/BisqueLiveActivity/BisqueLiveActivity.swift index 455e2a5..24094f0 100644 --- a/ios/Bisque/BisqueLiveActivity/BisqueLiveActivity.swift +++ b/ios/Bisque/BisqueLiveActivity/BisqueLiveActivity.swift @@ -30,6 +30,9 @@ struct BisqueLiveActivity: Widget { .font(.caption2) .foregroundStyle(.secondary) } + // Dimmed rather than hidden: the layout should not jump, and + // a faded number reads as "last known" (#147). + .opacity(context.isStale ? 0.4 : 1) } DynamicIslandExpandedRegion(.bottom) { @@ -41,9 +44,15 @@ struct BisqueLiveActivity: Widget { .font(.caption2) .foregroundStyle(.secondary) Spacer() - Text(liveActivityFormatDuration(context.state.estimatedSecondsRemaining)) - .font(.caption2) - .foregroundStyle(.secondary) + if context.isStale { + Text("not updating") + .font(.caption2) + .foregroundStyle(.orange) + } else { + Text(liveActivityFormatDuration(context.state.estimatedSecondsRemaining)) + .font(.caption2) + .foregroundStyle(.secondary) + } } } } @@ -54,12 +63,22 @@ struct BisqueLiveActivity: Widget { .font(.caption.bold()) .foregroundStyle(liveActivityStatusColor(context.state.status)) } + .opacity(context.isStale ? 0.4 : 1) } compactTrailing: { - Text(liveActivityFormatDuration(context.state.estimatedSecondsRemaining)) - .font(.caption2) - .foregroundStyle(.secondary) + // There is no room for an explanation here, so the countdown — + // the most obviously wrong thing to keep showing from a frozen + // frame — is replaced by a warning glyph. + if context.isStale { + Image(systemName: "exclamationmark.triangle.fill") + .foregroundStyle(.orange) + } else { + Text(liveActivityFormatDuration(context.state.estimatedSecondsRemaining)) + .font(.caption2) + .foregroundStyle(.secondary) + } } minimal: { Text("🔥") + .opacity(context.isStale ? 0.4 : 1) } } } diff --git a/ios/Bisque/BisqueLiveActivity/FiringLockScreenView.swift b/ios/Bisque/BisqueLiveActivity/FiringLockScreenView.swift index 63dadeb..e29cd10 100644 --- a/ios/Bisque/BisqueLiveActivity/FiringLockScreenView.swift +++ b/ios/Bisque/BisqueLiveActivity/FiringLockScreenView.swift @@ -31,6 +31,7 @@ struct FiringLockScreenView: View { .font(.caption2) .foregroundStyle(.secondary) } + .opacity(context.isStale ? 0.4 : 1) } // Progress @@ -43,9 +44,22 @@ struct FiringLockScreenView: View { .font(.caption2) .foregroundStyle(.secondary) Spacer() - Text("~\(remainingText) remaining") - .font(.caption2) - .foregroundStyle(.secondary) + /* Setting a staleDate on the content only makes ActivityKit + flip `context.isStale` — it does not change what is drawn. + Without reading it here the reading kept looking current + indefinitely, which was the whole complaint in #147. The + countdown is dropped rather than dimmed: "~2h remaining" + extrapolated from a frozen frame is a claim, not a stale + observation. */ + if context.isStale { + Label("Not updating — open Bisque", systemImage: "exclamationmark.triangle.fill") + .font(.caption2) + .foregroundStyle(.orange) + } else { + Text("~\(remainingText) remaining") + .font(.caption2) + .foregroundStyle(.secondary) + } } } .padding()