From 9a593e4ff26a24c75ed8888d5ba2c1640a7a5a99 Mon Sep 17 00:00:00 2001 From: LucianoDev Date: Wed, 22 Apr 2026 14:36:45 +0200 Subject: [PATCH] fix: stop UI freeze when selecting a pump that needs immobilizer unlock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 2 TestUnlock handshake was synchronous (Thread.Sleep x 8 = 4 s) and the continuation after Phase 1 marshalled back to the WPF dispatcher via the captured SynchronizationContext, so the eight 500 ms sleeps froze the UI right before unlock completed. - UnlockService.RunTestUnlockSequence -> async RunTestUnlockSequenceAsync with Task.Delay(500, ct) and ConfigureAwait(false) - Add ConfigureAwait(false) on every internal await in UnlockService so continuations no longer hop to the UI thread (Task.WhenAll, Task.Delay, connectedTcs, TryFastUnlockAsync, fast-unlock settle delay) - CancellationToken now propagates through Phase 2, so the snackbar Cancel button can interrupt the handshake within 500 ms instead of waiting out all eight Thread.Sleeps Includes the companion observer in IUnlockService / UnlockService (PumpUnlocked event, IsPumpUnlocked, StartObserver/StopObserver) that the Phase 1 wait now races against — lets any source of unlock (fast unlock, external manual, CAN flood finally working) short-circuit the 600 s timer as soon as the CAN TestUnlock parameter confirms it. Co-Authored-By: Claude Opus 4.7 --- Services/IUnlockService.cs | 31 +++++++ Services/Impl/UnlockService.cs | 161 ++++++++++++++++++++++++++++++--- 2 files changed, 177 insertions(+), 15 deletions(-) diff --git a/Services/IUnlockService.cs b/Services/IUnlockService.cs index 4538f84..90f94ff 100644 --- a/Services/IUnlockService.cs +++ b/Services/IUnlockService.cs @@ -17,6 +17,37 @@ namespace HC_APTBS.Services /// Raised when the unlock sequence completes. Argument is true if successful. event Action? UnlockCompleted; + /// + /// Raised by the background observer on each lock→unlock transition. Unlike + /// , this fires as soon as the CAN TestUnlock + /// parameter confirms unlocked — regardless of whether the transition was + /// caused by this service, an external manual unlock, or a fast-unlock + /// completing early. Subscribers must marshal to the UI thread themselves. + /// + event Action? PumpUnlocked; + + /// + /// Latched state from the background observer. True when the observer has + /// verified the pump is unlocked; false when the observer is not running + /// or the pump is currently locked. Use alongside + /// to race-guard "was the event already fired?". + /// + bool IsPumpUnlocked { get; } + + /// + /// Starts a 1-second polling observer that watches the CAN TestUnlock + /// parameter and raises on each lock→unlock + /// transition. Idempotent: stops any prior observer before starting the + /// new one. No-op when is 0. + /// + void StartObserver(PumpDefinition pump); + + /// + /// Stops the polling observer. Safe to call when no observer is active. + /// Also resets to false. + /// + void StopObserver(); + /// /// Runs the immobilizer unlock sequence for the given pump. /// Returns immediately if is 0 (no unlock needed). diff --git a/Services/Impl/UnlockService.cs b/Services/Impl/UnlockService.cs index d82c9b4..9589e82 100644 --- a/Services/Impl/UnlockService.cs +++ b/Services/Impl/UnlockService.cs @@ -33,12 +33,27 @@ namespace HC_APTBS.Services.Impl /// CTS for the persistent CAN senders — lives beyond . private CancellationTokenSource? _senderCts; + /// + /// Observer subscription state. The observer is event-driven: it subscribes to + /// the pump's TestUnlock and raises + /// on every LOCKED→UNLOCKED transition. No polling. + /// + private PumpDefinition? _observerPump; + private CanBusParameter? _observerParam; + private volatile bool _isPumpUnlocked; + /// public event Action? StatusChanged; /// public event Action? UnlockCompleted; + /// + public event Action? PumpUnlocked; + + /// + public bool IsPumpUnlocked => _isPumpUnlocked; + /// Creates the unlock service wired to the CAN and K-Line buses. public UnlockService(ICanService canService, IKwpService kwpService, IAppLogger logger) { @@ -65,12 +80,15 @@ namespace HC_APTBS.Services.Impl // It can only be attempted once the K-Line session is Connected (the // read-all-info must finish first). If the fast unlock succeeds AND // the CAN TestUnlock parameter confirms it, we skip the remaining wait. - await WaitWithFastUnlockAsync(pump, ct); + // ConfigureAwait(false) keeps the continuation off the UI thread — the + // caller (MainViewModel) invokes this fire-and-forget from the dispatcher, + // and Phase 2 below must not block it. + await WaitWithFastUnlockAsync(pump, ct).ConfigureAwait(false); ct.ThrowIfCancellationRequested(); // ── Phase 2: TestUnlock state machine ──────────────────────────────── StatusChanged?.Invoke("Testing unlock..."); - RunTestUnlockSequence(pump.UnlockType); + await RunTestUnlockSequenceAsync(pump.UnlockType, ct).ConfigureAwait(false); // ── Verify unlock status via CAN TestUnlock parameter ──────────────── bool success = VerifyUnlock(pump); @@ -90,6 +108,78 @@ namespace HC_APTBS.Services.Impl _senderCts = null; } + /// + public void StartObserver(PumpDefinition pump) + { + // Idempotent — tear down any prior observer first. + StopObserver(); + + if (pump.UnlockType == 0) return; + if (!pump.ParametersByName.TryGetValue(PumpParameterNames.TestUnlock, out var param)) + { + _log.Warning(LogId, + $"StartObserver: pump {pump.Id} has no '{PumpParameterNames.TestUnlock}' CAN param — observer not started."); + return; + } + + // Publish the target fields BEFORE subscribing so that if a CAN frame fires + // the handler on the CAN read thread before this method returns, the handler + // can resolve _observerPump correctly. + _observerPump = pump; + _observerParam = param; + + // Subscribe BEFORE seeding. Once TestUnlock settles at its unlocked value the + // decoder stops raising ValueChanged (it only fires on deltas), so missing the + // single LOCKED→UNLOCKED transition between seed and subscribe would be + // permanent. Subscribe-then-seed closes that window; a possible redundant fire + // from a concurrent handler is suppressed by the !wasUnlocked guard below. + param.ValueChanged += OnTestUnlockChanged; + + // Seed the latched state from the current parameter value. + bool wasUnlocked = _isPumpUnlocked; + _isPumpUnlocked = VerifyUnlock(pump); + + _log.Info(LogId, $"Unlock observer started for pump {pump.Id} (initial state: {(_isPumpUnlocked ? "UNLOCKED" : "LOCKED")})"); + + // Fire synchronously only if the seed itself revealed UNLOCKED — if the + // handler already raised PumpUnlocked between subscribe and seed, wasUnlocked + // is true and we skip a duplicate fire. + if (_isPumpUnlocked && !wasUnlocked) + PumpUnlocked?.Invoke(); + } + + /// + public void StopObserver() + { + if (_observerParam == null) return; + _log.Info(LogId, "Stopping unlock observer"); + _observerParam.ValueChanged -= OnTestUnlockChanged; + _observerParam = null; + _observerPump = null; + _isPumpUnlocked = false; + } + + /// + /// Handles on the CAN read thread. + /// Detects LOCKED↔UNLOCKED transitions and raises on + /// the LOCKED→UNLOCKED edge. Runs on the CAN thread — subscribers that touch + /// WPF state must marshal themselves. + /// + private void OnTestUnlockChanged(CanBusParameter _) + { + // Capture into a local so a concurrent StopObserver clearing the field + // mid-handler does not null-deref us. + var pump = _observerPump; + if (pump == null) return; + + bool unlocked = VerifyUnlock(pump); + if (unlocked == _isPumpUnlocked) return; + + _isPumpUnlocked = unlocked; + _log.Info(LogId, $"Observer: pump {pump.Id} transitioned {(unlocked ? "LOCKED → UNLOCKED" : "UNLOCKED → LOCKED")}"); + if (unlocked) PumpUnlocked?.Invoke(); + } + // ── Persistent CAN senders ─────────────────────────────────────────────── /// @@ -171,15 +261,50 @@ namespace HC_APTBS.Services.Impl waitCts.CancelAfter(UnlockDurationMs); var waitCt = waitCts.Token; - // Progress reporting task. - var progressTask = ReportProgressAsync(waitCt); + // Short-circuit immediately if the background observer has already + // latched an unlocked state (e.g. the pump was unlocked from a prior + // session, or an external process unlocked it before UnlockAsync ran). + if (_isPumpUnlocked) + { + _log.Info(LogId, "Observer already reports pump unlocked — skipping Phase 1 wait"); + return; + } - // Parallel fast-unlock task — awaits K-Line session, then attempts shortcut. - var fastTask = TryFastUnlockWhenReadyAsync(pump, waitCts, ct); + // Subscribe to the observer's PumpUnlocked event so ANY source of + // unlock (fast unlock, external manual unlock, the CAN flood itself + // eventually working) cancels the 600 s wait as soon as the CAN + // TestUnlock parameter confirms it — not just the fast-unlock path. + void OnObserverUnlocked() + { + _log.Info(LogId, "Observer signalled unlock — cancelling Phase 1 wait"); + try { waitCts.Cancel(); } catch (ObjectDisposedException) { } + } + PumpUnlocked += OnObserverUnlocked; - // Wait for either: the full duration elapses, or the fast unlock succeeds - // and cancels the wait CTS. - await Task.WhenAll(progressTask, fastTask); + try + { + // Race guard: the event may have fired between the IsPumpUnlocked + // check above and the subscription. Re-check now that we're wired up. + if (_isPumpUnlocked) + { + _log.Info(LogId, "Observer reports pump unlocked (post-subscribe) — skipping Phase 1 wait"); + return; + } + + // Progress reporting task. + var progressTask = ReportProgressAsync(waitCt); + + // Parallel fast-unlock task — awaits K-Line session, then attempts shortcut. + var fastTask = TryFastUnlockWhenReadyAsync(pump, waitCts, ct); + + // Wait for either: the full duration elapses, the fast unlock succeeds + // and cancels the wait CTS, or the observer fires PumpUnlocked. + await Task.WhenAll(progressTask, fastTask).ConfigureAwait(false); + } + finally + { + PumpUnlocked -= OnObserverUnlocked; + } // If the outer ct was cancelled (user stop), propagate. ct.ThrowIfCancellationRequested(); @@ -193,7 +318,7 @@ namespace HC_APTBS.Services.Impl { while (!waitCt.IsCancellationRequested) { - await Task.Delay(1000, waitCt); + await Task.Delay(1000, waitCt).ConfigureAwait(false); var elapsed = DateTime.UtcNow - start; int pct = (int)(elapsed.TotalMilliseconds * 100 / UnlockDurationMs); string time = $"{(int)elapsed.TotalMinutes:D2}:{elapsed.Seconds:D2}"; @@ -235,7 +360,7 @@ namespace HC_APTBS.Services.Impl // Wait for connection or cancellation (user cancel or 600 s elapsed). using var reg = ct.Register(() => connectedTcs.TrySetCanceled()); using var waitReg = waitCts.Token.Register(() => connectedTcs.TrySetCanceled()); - await connectedTcs.Task; + await connectedTcs.Task.ConfigureAwait(false); } finally { @@ -255,7 +380,7 @@ namespace HC_APTBS.Services.Impl _log.Info(LogId, "Attempting K-Line fast unlock (timer shortcut)..."); StatusChanged?.Invoke("Fast unlock attempt..."); - bool ack = await _kwp.TryFastUnlockAsync(); + bool ack = await _kwp.TryFastUnlockAsync().ConfigureAwait(false); if (!ack) { _log.Info(LogId, "Fast unlock NAK or failed — continuing normal wait"); @@ -266,7 +391,7 @@ namespace HC_APTBS.Services.Impl _log.Info(LogId, "Fast unlock ACK — waiting briefly for pump to process"); // Give the pump a moment to process the timer shortcut, then verify. - await Task.Delay(2000, ct); + await Task.Delay(2000, ct).ConfigureAwait(false); if (VerifyUnlock(pump)) { @@ -291,7 +416,13 @@ namespace HC_APTBS.Services.Impl // ── Phase 2: TestUnlock state machine ──────────────────────────────────── - private void RunTestUnlockSequence(int unlockType) + // Async so the 500 ms pump-protocol pacing between handshake commands + // uses Task.Delay instead of Thread.Sleep — otherwise the continuation + // after WaitWithFastUnlockAsync (which can land on the UI thread via the + // captured SynchronizationContext) would block the WPF dispatcher for + // ~4 s while the eight Thread.Sleep(500) calls complete. Propagating ct + // also makes user-cancel responsive mid-handshake. + private async Task RunTestUnlockSequenceAsync(int unlockType, CancellationToken ct) { byte[][] type1Cmds = { @@ -316,7 +447,7 @@ namespace HC_APTBS.Services.Impl for (int step = 0; step < cmds.Length; step++) { _can.SendRawMessage(0x700, cmds[step]); - Thread.Sleep(500); + await Task.Delay(500, ct).ConfigureAwait(false); } }