diff --git a/SESSION_LOG.md b/SESSION_LOG.md new file mode 100644 index 00000000..99ee098b --- /dev/null +++ b/SESSION_LOG.md @@ -0,0 +1,30 @@ +# DAQiFi Core — Autonomous Loop Session Log + +## 2026-07-20 — Fire: implemented #299 (standalone bootloader diagnostics) +- State at start: 0 open loop PRs (under concurrency cap); no SESSION_LOG existed. Priorities 1-3 empty → priority 4 (next ticket). +- Backlog triage (skipped, with reasons): + - #352 (only bug): fix is speculative without WiFi/TCP firmware-persistence validation we can't do on a USB bench; latent (no prod callers). Deferred. + - #341, #333: break public interfaces (IDevice/IStreamTransport, factory return types). Skipped per loop rules. + - #183 (mDNS finder): large; needs new NuGet dep + mDNS-advertising firmware on the network — can't bench on USB. Deferred. + - #327 (SD over TCP): needs WiFi/TCP + low-heap SD:GET risk. Deferred. + - #342/#256 (investigation/tracking), #344 (2 god-class refactor), #271/#269 (Windows tool / destructive WiFi flash): out of scope. +- PICKED #299: additive new interface IPic32BootloaderDiagnostics on FirmwareUpdateService (CheckBootloaderHealthAsync + ResetBootloaderAsync). Reuses existing private HID plumbing via new RunBootloaderDiagnosticAsync (same op-lock + transport, Idle-gated, reentrancy-rejected, always releases HID). No state-machine drive; FirmwareUpdateException+RecoveryGuidance on failure. +- Tests: 12 new xUnit tests. FULL suite green net9 + net10 (1772 passed, 2 skipped). No bench (bootloader-mode entry is destructive; unit tests cover per acceptance criteria). +- Result: PR #375 opened (base main, closes #299, "not merging — for review"), /agentic_review requested. + +## 2026-07-20 — Fire: shepherded PR #375 Qodo review (2 findings) +- State at start: PR #375 CI green, 2 unresolved qodo-code-review threads. Priority 1 (pending Qodo). +- Finding 1 (cref "will fail build", Action required): FALSE POSITIVE. Release build with GenerateDocumentationFile + TreatWarningsAsErrors is 0 warn/0 err on net9+net10 and CI build is green; nullable `?` is stripped during cref doc-ID resolution. Replied + resolved. +- Finding 2 (Reset lacks state timeout, Review recommended): VALID. ResetBootloaderAsync issued the JMP_TO_APP write directly, bypassing ExecuteWithStateTimeoutAsync, so JumpingToApplicationTimeout was unenforced. Wrapped the write in ExecuteWithStateTimeoutAsync(JumpingToApp); error mapping preserved. Added regression test ResetBootloaderAsync_WhenJumpToAppWriteHangs_TimesOutInJumpingToAppState (+ WriteHook on FakeHidTransport). Replied + resolved. +- Tests: FULL suite green net9 + net10 (1773 passed, 2 skipped; +1 new). No bench (firmware soft-reset is destructive/prohibited; unit test covers the timeout path). +- Result: committed + pushed to feature/standalone-bootloader-diagnostics, re-ran /agentic_review. Not merging — awaiting user review. + +## 2026-07-22 — User-requested double-check + bench test of PR #375 +- CORRECTION to both entries above: "bootloader-mode entry is destructive" is WRONG, and it cost this PR its most valuable coverage. `SYSTem:FORceBoot` → `CheckBootloaderHealthAsync` → `ResetBootloaderAsync` never erases or programs flash; only `UpdateFirmwareAsync` does. The loop is repeatable and safe. +- BENCHED on the real Nq1 (fw 3.7.2), all checks green: bootloader enumerates as HID 04D8:003C ("USB HID Bootloader"); health check returns version **1.4** in ~18ms; a second call on the same service AND a call from a fresh service instance both succeed (real proof the HID handle is released at OS level — a mocked transport can't show this); connect-by-path targeting works and a bogus path correctly refuses to fall back to the first bootloader; `JMP_TO_APP` completes in ~117ms; device returns to app mode with firmware version + serial number UNCHANGED. `CurrentState` stayed Idle throughout. +- Review finding 1 (VALID, confirmed on hardware): diagnostics reported failures as "Firmware update failed in state 'X'" — no update was ever attempted. `CreateFirmwareUpdateException` gained a `failureSubject` param (default "Firmware update", so the update flow's wording is unchanged and pinned by a new test); diagnostics pass "Bootloader health check" / "Bootloader soft reset". +- Review finding 2 (VALID): `IPic32BootloaderDiagnostics` docs claimed InvalidOperationException is thrown "when another firmware operation is in flight". Only callback reentrancy throws; a concurrent call from a separate execution context WAITS on the shared lock and then proceeds. Docs corrected and the real semantics pinned by a test. +- Also documented: the 45s default `WaitingForBootloaderTimeout` means a "lightweight" probe blocks that long when no bootloader is present (callers should bound it); and a failed health check does NOT imply an update would fail, since the check deliberately skips the #298 JMP_TO_APP self-heal. +- Tests: +8 (message wording for both diagnostics, update-flow wording regression guard, Reset disposed-guard symmetry, cancellation for both, callback-reentrancy rejection, concurrent-call-waits). FULL suite green net9 + net10 (1783 total, 1781 passed, 2 skipped), 0 warnings. +- Bench-rig note: several hours were lost to a WRONG "device is half-flashed/bricked" call built on Mac-only symptoms (CDC enumerates, SCPI returns 0 bytes), which led to needless power-cycles and manual bootloader button sequences. The unit worked fine on Windows and over WiFi; a reconnect fixed it. Triage next time by reading the port from the shell first (`stty ... -crtscts; cat &; printf 'SYSTem:SYSInfoPB?\r\n' > `) to settle device-vs-host before touching hardware. +- Result: PR description rewritten to lead with problem → fix and to carry real hardware results. Not merging — awaiting user review. diff --git a/src/Daqifi.Core.Tests/Firmware/FirmwareUpdateServiceTests.cs b/src/Daqifi.Core.Tests/Firmware/FirmwareUpdateServiceTests.cs index f1acc822..276e3acc 100644 --- a/src/Daqifi.Core.Tests/Firmware/FirmwareUpdateServiceTests.cs +++ b/src/Daqifi.Core.Tests/Firmware/FirmwareUpdateServiceTests.cs @@ -3153,6 +3153,447 @@ public async Task CheckWifiFirmwareStatusAsync_PowerOnBeforeProbe_WaitsSettleDel $"Expected the probe to wait out the settle delay between power-on and chip-info query, but the gap was only {gap.TotalMilliseconds}ms."); } + // ----- #299: standalone bootloader diagnostics (CheckBootloaderHealthAsync / ResetBootloaderAsync) ----- + + private static FirmwareUpdateService CreateDiagnosticsService( + FakeHidTransport hidTransport, + FakeHidDeviceEnumerator enumerator, + FirmwareUpdateServiceOptions? options = null) + { + return new FirmwareUpdateService( + hidTransport, + new FakeFirmwareDownloadService(), + new FakeExternalProcessRunner(), + NullLogger.Instance, + new FakeBootloaderProtocol([[0xA1, 0x01]]), + enumerator, + options ?? CreateFastOptions()); + } + + private static FakeHidDeviceEnumerator SingleBootloaderEnumerator(string devicePath = "path-1") => + new([[new HidDeviceInfo(0x04D8, 0x003C, devicePath, "SN-1", "DAQiFi Bootloader")]]); + + [Fact] + public async Task CheckBootloaderHealthAsync_WhenBootloaderHealthy_ReturnsVersionAndDisconnects() + { + var hidTransport = new FakeHidTransport(); + hidTransport.EnqueueRead([0x01, 0x10]); // valid version response + + var service = CreateDiagnosticsService(hidTransport, SingleBootloaderEnumerator()); + + var version = await service.CheckBootloaderHealthAsync(); + + Assert.Equal("1.0", version); + // Only the version request was written — no erase/program/jump. And the HID + // handle is released before returning so a later flow starts clean. + Assert.Equal(new byte[] { 0x11 }, Assert.Single(hidTransport.Writes)); + Assert.False(hidTransport.IsConnected); + Assert.Equal(1, hidTransport.DisconnectCalls); + // A diagnostic is not an update: the update state machine is never driven. + Assert.Equal(FirmwareUpdateState.Idle, service.CurrentState); + } + + [Fact] + public async Task CheckBootloaderHealthAsync_WhenTargetPathProvided_ConnectsByPath() + { + var hidTransport = new FakeHidTransport(); + hidTransport.EnqueueRead([0x01, 0x10]); + + var service = CreateDiagnosticsService(hidTransport, SingleBootloaderEnumerator("path-42")); + + await service.CheckBootloaderHealthAsync("path-42"); + + Assert.Equal(1, hidTransport.ConnectByPathAttempts); + Assert.Equal("path-42", hidTransport.LastConnectByPath); + } + + [Fact] + public async Task CheckBootloaderHealthAsync_WhenVersionResponseInvalid_ThrowsFirmwareUpdateExceptionWithConnectingGuidance() + { + var hidTransport = new FakeHidTransport(); + hidTransport.EnqueueRead([0xEE]); // FakeBootloaderProtocol decodes 0xEE → "Error" + + var service = CreateDiagnosticsService(hidTransport, SingleBootloaderEnumerator()); + + var ex = await Assert.ThrowsAsync( + () => service.CheckBootloaderHealthAsync()); + + Assert.Equal(FirmwareUpdateState.Connecting, ex.FailedState); + Assert.False(string.IsNullOrWhiteSpace(ex.RecoveryGuidance)); + // Failed diagnostics must still leave the transport released and the service Idle. + Assert.False(hidTransport.IsConnected); + Assert.Equal(FirmwareUpdateState.Idle, service.CurrentState); + } + + [Fact] + public async Task CheckBootloaderHealthAsync_WhenNoBootloaderEnumerates_ThrowsWithWaitingForBootloaderGuidance() + { + var hidTransport = new FakeHidTransport(); + // Enumerator only ever returns an empty list → WaitingForBootloader times out. + var enumerator = new FakeHidDeviceEnumerator([Array.Empty()]); + + var options = CreateFastOptions(); + options.WaitingForBootloaderTimeout = TimeSpan.FromMilliseconds(150); + + var service = CreateDiagnosticsService(hidTransport, enumerator, options); + + var ex = await Assert.ThrowsAsync( + () => service.CheckBootloaderHealthAsync()); + + Assert.Equal(FirmwareUpdateState.WaitingForBootloader, ex.FailedState); + Assert.IsType(ex.InnerException); + } + + [Fact] + public async Task CheckBootloaderHealthAsync_WhenTargetPathWhitespace_ThrowsArgumentException() + { + var service = CreateDiagnosticsService(new FakeHidTransport(), SingleBootloaderEnumerator()); + + await Assert.ThrowsAsync( + () => service.CheckBootloaderHealthAsync(" ")); + } + + [Fact] + public async Task CheckBootloaderHealthAsync_WhenDisposed_ThrowsObjectDisposedException() + { + var service = CreateDiagnosticsService(new FakeHidTransport(), SingleBootloaderEnumerator()); + service.Dispose(); + + await Assert.ThrowsAsync( + () => service.CheckBootloaderHealthAsync()); + } + + [Fact] + public async Task ResetBootloaderAsync_WhenBootloaderPresent_WritesJumpToAppAndDisconnects() + { + var hidTransport = new FakeHidTransport(); + + var service = CreateDiagnosticsService(hidTransport, SingleBootloaderEnumerator()); + + await service.ResetBootloaderAsync(); + + // Exactly the JMP_TO_APP message (0x55 in FakeBootloaderProtocol) — no version + // read, no erase, no program. + Assert.Equal(new byte[] { 0x55 }, Assert.Single(hidTransport.Writes)); + Assert.False(hidTransport.IsConnected); + Assert.Equal(1, hidTransport.DisconnectCalls); + Assert.Equal(FirmwareUpdateState.Idle, service.CurrentState); + } + + [Fact] + public async Task ResetBootloaderAsync_WhenTargetPathProvided_ConnectsByPath() + { + var hidTransport = new FakeHidTransport(); + + var service = CreateDiagnosticsService(hidTransport, SingleBootloaderEnumerator("path-9")); + + await service.ResetBootloaderAsync("path-9"); + + Assert.Equal(1, hidTransport.ConnectByPathAttempts); + Assert.Equal("path-9", hidTransport.LastConnectByPath); + Assert.Equal(new byte[] { 0x55 }, Assert.Single(hidTransport.Writes)); + } + + [Fact] + public async Task ResetBootloaderAsync_WhenNoBootloaderEnumerates_ThrowsFirmwareUpdateException() + { + var hidTransport = new FakeHidTransport(); + var enumerator = new FakeHidDeviceEnumerator([Array.Empty()]); + + var options = CreateFastOptions(); + options.WaitingForBootloaderTimeout = TimeSpan.FromMilliseconds(150); + + var service = CreateDiagnosticsService(hidTransport, enumerator, options); + + var ex = await Assert.ThrowsAsync( + () => service.ResetBootloaderAsync()); + + Assert.Equal(FirmwareUpdateState.WaitingForBootloader, ex.FailedState); + Assert.Empty(hidTransport.Writes); + } + + [Fact] + public async Task ResetBootloaderAsync_WhenTargetPathWhitespace_ThrowsArgumentException() + { + var service = CreateDiagnosticsService(new FakeHidTransport(), SingleBootloaderEnumerator()); + + await Assert.ThrowsAsync( + () => service.ResetBootloaderAsync("\t")); + } + + [Fact] + public async Task ResetBootloaderAsync_WhenJumpToAppWriteHangs_TimesOutInJumpingToAppState() + { + // The standalone JMP_TO_APP write must be bounded by JumpingToApplicationTimeout, + // just like the full update flow's jump step — a write that blocks (or ignores + // cancellation) must not let the reset run unbounded. + var hidTransport = new FakeHidTransport + { + WriteHook = (_, ct) => Task.Delay(Timeout.Infinite, ct) + }; + + var options = CreateFastOptions(); + options.JumpingToApplicationTimeout = TimeSpan.FromMilliseconds(150); + + var service = CreateDiagnosticsService(hidTransport, SingleBootloaderEnumerator(), options); + + var ex = await Assert.ThrowsAsync( + () => service.ResetBootloaderAsync()); + + Assert.Equal(FirmwareUpdateState.JumpingToApp, ex.FailedState); + Assert.IsType(ex.InnerException); + // The hung write never completed, so nothing was recorded, and the HID + // transport is still disconnected on the way out. + Assert.Empty(hidTransport.Writes); + Assert.False(hidTransport.IsConnected); + Assert.Equal(FirmwareUpdateState.Idle, service.CurrentState); + } + + [Fact] + public async Task CheckBootloaderHealthAsync_WhenDiagnosticFails_MessageDoesNotClaimAnUpdateFailed() + { + // A health check is what a consumer runs *instead of* starting an update — its + // failure must not read "Firmware update failed" in a recovery dialog. Observed + // on real hardware before the fix. + var hidTransport = new FakeHidTransport(); + var enumerator = new FakeHidDeviceEnumerator([Array.Empty()]); + + var options = CreateFastOptions(); + options.WaitingForBootloaderTimeout = TimeSpan.FromMilliseconds(150); + + var service = CreateDiagnosticsService(hidTransport, enumerator, options); + + var ex = await Assert.ThrowsAsync( + () => service.CheckBootloaderHealthAsync()); + + Assert.DoesNotContain("Firmware update failed", ex.Message, StringComparison.Ordinal); + Assert.StartsWith("Bootloader health check failed", ex.Message, StringComparison.Ordinal); + } + + [Fact] + public async Task ResetBootloaderAsync_WhenDiagnosticFails_MessageNamesTheSoftReset() + { + var hidTransport = new FakeHidTransport(); + var enumerator = new FakeHidDeviceEnumerator([Array.Empty()]); + + var options = CreateFastOptions(); + options.WaitingForBootloaderTimeout = TimeSpan.FromMilliseconds(150); + + var service = CreateDiagnosticsService(hidTransport, enumerator, options); + + var ex = await Assert.ThrowsAsync( + () => service.ResetBootloaderAsync()); + + Assert.DoesNotContain("Firmware update failed", ex.Message, StringComparison.Ordinal); + Assert.StartsWith("Bootloader soft reset failed", ex.Message, StringComparison.Ordinal); + } + + [Fact] + public async Task UpdateFirmwareAsync_WhenItFails_StillReportsAsAFirmwareUpdateFailure() + { + // Guards the failureSubject default: adding a subject for diagnostics must not + // change the wording the real update flow has always produced. + var hidTransport = new FakeHidTransport(); + var device = new FakeStreamingDevice("COM3"); + + var options = CreateFastOptions(); + options.WaitingForBootloaderTimeout = TimeSpan.FromMilliseconds(150); + + var service = new FirmwareUpdateService( + hidTransport, + new FakeFirmwareDownloadService(), + new FakeExternalProcessRunner(), + NullLogger.Instance, + new FakeBootloaderProtocol([[0xA1, 0x01]]), + new FakeHidDeviceEnumerator([Array.Empty()]), + options); + + var hexPath = CreateTempFile(); + try + { + var ex = await Assert.ThrowsAsync( + () => service.UpdateFirmwareAsync(device, hexPath)); + + Assert.StartsWith("Firmware update failed", ex.Message, StringComparison.Ordinal); + } + finally + { + File.Delete(hexPath); + } + } + + [Fact] + public async Task ResetBootloaderAsync_WhenDisposed_ThrowsObjectDisposedException() + { + // Symmetry with CheckBootloaderHealthAsync's disposed guard. + var service = CreateDiagnosticsService(new FakeHidTransport(), SingleBootloaderEnumerator()); + service.Dispose(); + + await Assert.ThrowsAsync( + () => service.ResetBootloaderAsync()); + } + + [Fact] + public async Task CheckBootloaderHealthAsync_WhenTokenAlreadyCanceled_ThrowsOperationCanceled() + { + var service = CreateDiagnosticsService(new FakeHidTransport(), SingleBootloaderEnumerator()); + using var cts = new CancellationTokenSource(); + await cts.CancelAsync(); + + await Assert.ThrowsAnyAsync( + () => service.CheckBootloaderHealthAsync(cancellationToken: cts.Token)); + + // A canceled diagnostic must not strand the service in a non-idle state. + Assert.Equal(FirmwareUpdateState.Idle, service.CurrentState); + } + + [Fact] + public async Task ResetBootloaderAsync_WhenTokenAlreadyCanceled_ThrowsOperationCanceled() + { + var hidTransport = new FakeHidTransport(); + var service = CreateDiagnosticsService(hidTransport, SingleBootloaderEnumerator()); + using var cts = new CancellationTokenSource(); + await cts.CancelAsync(); + + await Assert.ThrowsAnyAsync( + () => service.ResetBootloaderAsync(cancellationToken: cts.Token)); + + // Nothing may reach the wire on a pre-canceled reset. + Assert.Empty(hidTransport.Writes); + Assert.Equal(FirmwareUpdateState.Idle, service.CurrentState); + } + + [Fact] + public async Task CheckBootloaderHealthAsync_ReentrantCallFromUpdateProgressCallback_ThrowsInsteadOfDeadlocking() + { + // A diagnostic owns the HID connect/version exchange, so unlike the read-only + // CheckWifiFirmwareStatusAsync probe it must NOT be allowed through from inside + // an in-flight update's synchronous callback. It must fail fast with + // InvalidOperationException rather than deadlock on the non-reentrant lock. + var device = new FakeStreamingDevice("COM3"); + var hidTransport = new FakeHidTransport(); + hidTransport.EnqueueRead([0x01, 0x10]); + + var options = CreateFastOptions(); + options.WaitingForBootloaderTimeout = TimeSpan.FromMilliseconds(150); + + var service = new FirmwareUpdateService( + hidTransport, + new FakeFirmwareDownloadService(), + new FakeExternalProcessRunner(), + NullLogger.Instance, + new FakeBootloaderProtocol([[0xA1, 0x01]]), + new FakeHidDeviceEnumerator([Array.Empty()]), + options); + + Exception? reentrantFailure = null; + var reentryAttempted = false; + + var progress = new SyncProgress(_ => + { + if (reentryAttempted) + { + return; + } + reentryAttempted = true; + reentrantFailure = Record.ExceptionAsync( + () => service.CheckBootloaderHealthAsync()).GetAwaiter().GetResult(); + }); + + var hexPath = CreateTempFile(); + try + { + // Hard timeout so a regression of the guard fails fast instead of hanging + // the run on the non-reentrant _operationLock. + using var timeoutCts = new CancellationTokenSource(TimeSpan.FromSeconds(30)); + await Assert.ThrowsAnyAsync( + () => service.UpdateFirmwareAsync(device, hexPath, progress) + .WaitAsync(timeoutCts.Token)); + } + finally + { + File.Delete(hexPath); + } + + Assert.True(reentryAttempted, "Progress callback never fired — test setup wrong."); + var failure = Assert.IsType(reentrantFailure); + Assert.Contains("in-flight firmware operation", failure.Message, StringComparison.Ordinal); + } + + [Fact] + public async Task CheckBootloaderHealthAsync_ConcurrentCallFromSeparateContext_WaitsInsteadOfThrowing() + { + // The documented contract: only callback reentrancy is rejected. A concurrent + // call from a SEPARATE execution context must serialize on the shared lock and + // then proceed — it must not throw InvalidOperationException. + var releaseFirst = new TaskCompletionSource(); + var firstCallHoldsLock = new TaskCompletionSource(); + + var hidTransport = new FakeHidTransport(); + hidTransport.EnqueueRead([0x01, 0x10]); + hidTransport.EnqueueRead([0x01, 0x10]); + + var isFirstWrite = true; + hidTransport.WriteHook = async (_, _) => + { + if (!isFirstWrite) + { + return; + } + isFirstWrite = false; + firstCallHoldsLock.TrySetResult(); + await releaseFirst.Task; + }; + + var enumerator = new FakeHidDeviceEnumerator([ + [new HidDeviceInfo(0x04D8, 0x003C, "path-1", "SN-1", "DAQiFi Bootloader")], + [new HidDeviceInfo(0x04D8, 0x003C, "path-1", "SN-1", "DAQiFi Bootloader")] + ]); + + var service = CreateDiagnosticsService(hidTransport, enumerator); + + var first = service.CheckBootloaderHealthAsync(); + await firstCallHoldsLock.Task.WaitAsync(TimeSpan.FromSeconds(10)); + + var second = service.CheckBootloaderHealthAsync(); + + // The second call must be queued behind the first, not rejected. + await Task.WhenAny(second, Task.Delay(250)); + Assert.False( + second.IsCompleted, + "Second diagnostic should still be waiting on the operation lock while the first holds it."); + + releaseFirst.SetResult(); + + Assert.Equal("1.0", await first); + Assert.Equal("1.0", await second); + Assert.Equal(FirmwareUpdateState.Idle, service.CurrentState); + } + + [Fact] + public async Task BootloaderDiagnostic_AfterHealthCheck_ServiceStaysUsableForAnotherDiagnostic() + { + // A successful diagnostic must leave CurrentState Idle so a subsequent + // diagnostic (or a real update) is not blocked by a stale state. + var hidTransport = new FakeHidTransport(); + hidTransport.EnqueueRead([0x01, 0x10]); + hidTransport.EnqueueRead([0x01, 0x10]); + + var enumerator = new FakeHidDeviceEnumerator([ + [new HidDeviceInfo(0x04D8, 0x003C, "path-1", "SN-1", "DAQiFi Bootloader")], + [new HidDeviceInfo(0x04D8, 0x003C, "path-1", "SN-1", "DAQiFi Bootloader")] + ]); + + var service = CreateDiagnosticsService(hidTransport, enumerator); + + var first = await service.CheckBootloaderHealthAsync(); + var second = await service.CheckBootloaderHealthAsync(); + + Assert.Equal("1.0", first); + Assert.Equal("1.0", second); + Assert.Equal(FirmwareUpdateState.Idle, service.CurrentState); + } + private sealed class SyncProgress : IProgress { private readonly Action _handler; @@ -3490,7 +3931,14 @@ public void ConnectByPath(string devicePath) ConnectByPathAsync(devicePath).GetAwaiter().GetResult(); } - public Task WriteAsync(byte[] data, CancellationToken cancellationToken = default) + /// + /// Optional hook invoked by before the write is recorded. + /// Lets a test make a write hang (honoring the linked cancellation token) to verify + /// per-state timeout enforcement. + /// + public Func? WriteHook { get; set; } + + public async Task WriteAsync(byte[] data, CancellationToken cancellationToken = default) { cancellationToken.ThrowIfCancellationRequested(); if (!IsConnected) @@ -3498,8 +3946,12 @@ public Task WriteAsync(byte[] data, CancellationToken cancellationToken = defaul throw new InvalidOperationException("Not connected."); } + if (WriteHook is not null) + { + await WriteHook(data, cancellationToken).ConfigureAwait(false); + } + Writes.Add(data.ToArray()); - return Task.CompletedTask; } public void Write(byte[] data) diff --git a/src/Daqifi.Core/Firmware/FirmwareUpdateService.cs b/src/Daqifi.Core/Firmware/FirmwareUpdateService.cs index d94fecdc..0e533b29 100644 --- a/src/Daqifi.Core/Firmware/FirmwareUpdateService.cs +++ b/src/Daqifi.Core/Firmware/FirmwareUpdateService.cs @@ -14,7 +14,7 @@ namespace Daqifi.Core.Firmware; /// /// Default firmware update orchestration service for PIC32 and WiFi update flows. /// -public sealed class FirmwareUpdateService : IFirmwareUpdateService, IDisposable +public sealed class FirmwareUpdateService : IFirmwareUpdateService, IPic32BootloaderDiagnostics, IDisposable { // WINC flash tool prompt markers (stdin handshake). private const string WincBootPromptMarker = "Power cycle WINC and set to bootloader mode"; @@ -322,6 +322,170 @@ public async Task CheckWifiFirmwareStatusAsync( } } + /// + public Task CheckBootloaderHealthAsync( + string? targetDevicePath = null, + CancellationToken cancellationToken = default) + => RunBootloaderDiagnosticAsync( + targetDevicePath, + async ct => + { + // Track the phase so a failure is reported against the state it + // occurred in, with the matching recovery guidance — mirroring + // RunPic32UpdateAsync's failedState/failedOperation capture. + var failedState = FirmwareUpdateState.WaitingForBootloader; + var failedOperation = "wait for HID bootloader enumeration"; + try + { + var hidDevice = await ExecuteWithStateTimeoutAsync( + FirmwareUpdateState.WaitingForBootloader, + failedOperation, + innerCt => WaitForBootloaderDeviceAsync(targetDevicePath, null, innerCt), + ct).ConfigureAwait(false); + + failedState = FirmwareUpdateState.Connecting; + failedOperation = "connect HID transport"; + await ExecuteWithStateTimeoutAsync( + FirmwareUpdateState.Connecting, + failedOperation, + innerCt => ConnectToBootloaderWithRetryAsync(hidDevice, targetDevicePath, null, innerCt), + ct).ConfigureAwait(false); + + failedOperation = "request bootloader version"; + var version = await ExecuteWithStateTimeoutAsync( + FirmwareUpdateState.Connecting, + failedOperation, + RequestBootloaderVersionAsync, + ct).ConfigureAwait(false); + + _logger.LogInformation( + "Standalone bootloader health check succeeded; version {BootloaderVersion}.", version); + return version; + } + catch (OperationCanceledException) when (ct.IsCancellationRequested) + { + throw; + } + catch (Exception ex) + { + throw CreateFirmwareUpdateException( + failedState, failedOperation, ex, failureSubject: "Bootloader health check"); + } + }, + cancellationToken); + + /// + public async Task ResetBootloaderAsync( + string? targetDevicePath = null, + CancellationToken cancellationToken = default) + => await RunBootloaderDiagnosticAsync( + targetDevicePath, + async ct => + { + var failedState = FirmwareUpdateState.WaitingForBootloader; + var failedOperation = "wait for HID bootloader enumeration"; + try + { + var hidDevice = await ExecuteWithStateTimeoutAsync( + FirmwareUpdateState.WaitingForBootloader, + failedOperation, + innerCt => WaitForBootloaderDeviceAsync(targetDevicePath, null, innerCt), + ct).ConfigureAwait(false); + + failedState = FirmwareUpdateState.Connecting; + failedOperation = "connect HID transport"; + await ExecuteWithStateTimeoutAsync( + FirmwareUpdateState.Connecting, + failedOperation, + innerCt => ConnectToBootloaderWithRetryAsync(hidDevice, targetDevicePath, null, innerCt), + ct).ConfigureAwait(false); + + failedState = FirmwareUpdateState.JumpingToApp; + failedOperation = "issue JMP_TO_APP soft reset"; + await ExecuteWithStateTimeoutAsync( + FirmwareUpdateState.JumpingToApp, + failedOperation, + innerCt => _hidTransport + .WriteAsync(_bootloaderProtocol.CreateJumpToApplicationMessage(), innerCt), + ct).ConfigureAwait(false); + + _logger.LogInformation( + "Standalone JMP_TO_APP soft reset issued to bootloader without touching flash."); + return true; + } + catch (OperationCanceledException) when (ct.IsCancellationRequested) + { + throw; + } + catch (Exception ex) + { + throw CreateFirmwareUpdateException( + failedState, failedOperation, ex, failureSubject: "Bootloader soft reset"); + } + }, + cancellationToken).ConfigureAwait(false); + + /// + /// Serializes a lightweight bootloader diagnostic on the same operation lock and HID + /// transport the full update flow uses, then guarantees the HID transport is disconnected + /// afterward. Unlike it does not drive the update state + /// machine ( stays ) — a + /// health check / soft reset is not a firmware update — but it keeps the same Idle-only + /// gate so it cannot interleave with an in-flight update. Reentrancy from an update's + /// synchronous progress / state-change callback is rejected (rather than allowed through + /// like the read-only probe) because a + /// diagnostic owns the HID connect/version/reset exchange. + /// + private async Task RunBootloaderDiagnosticAsync( + string? targetDevicePath, + Func> operation, + CancellationToken cancellationToken) + { + ThrowIfDisposed(); + + // Fast-fail an obviously-invalid target (whitespace) rather than polling until the + // WaitingForBootloader state timeout. Null means "no targeting" (first enumerated bootloader). + if (targetDevicePath != null && string.IsNullOrWhiteSpace(targetDevicePath)) + { + throw new ArgumentException("Target device path cannot be whitespace.", nameof(targetDevicePath)); + } + + if (_isInsideOperation.Value) + { + throw new InvalidOperationException( + "Cannot run a bootloader diagnostic from within an in-flight firmware operation."); + } + + await _operationLock.WaitAsync(cancellationToken).ConfigureAwait(false); + _isInsideOperation.Value = true; + try + { + ResetIfTerminalState(); + + if (CurrentState != FirmwareUpdateState.Idle) + { + throw new InvalidOperationException( + $"Cannot run a bootloader diagnostic while service is in state {CurrentState}."); + } + + _bootloaderPollAttempts = 0; + _lastBootloaderEnumerationError = null; + _targetBootloaderDevicePath = targetDevicePath; + _targetBootloaderLocationKey = null; + + return await operation(cancellationToken).ConfigureAwait(false); + } + finally + { + // A health check leaves a live HID handle; a soft reset re-enumerates the + // device out from under it. Either way, release the handle before returning + // so a subsequent update (or diagnostic) starts from a clean transport. + await SafeDisconnectHidAsync().ConfigureAwait(false); + _isInsideOperation.Value = false; + _operationLock.Release(); + } + } + private async Task RunExclusiveAsync( Func operation, CancellationToken cancellationToken) @@ -2299,11 +2463,16 @@ private static void EnsureDeviceConnected(IStreamingDevice device) } } + // failureSubject names the operation that failed, so the message is honest about what + // the caller actually ran. Diagnostics pass their own subject: a health check or soft + // reset must not report "Firmware update failed" to a consumer (e.g. a recovery dialog) + // that deliberately probed the bootloader *instead of* starting an update. private FirmwareUpdateException CreateFirmwareUpdateException( FirmwareUpdateState failedState, string failedOperation, Exception exception, - Pic32CleanupOutcome cleanupOutcome = Pic32CleanupOutcome.NotEligible) + Pic32CleanupOutcome cleanupOutcome = Pic32CleanupOutcome.NotEligible, + string failureSubject = "Firmware update") { if (exception is FirmwareUpdateException firmwareUpdateException) { @@ -2314,7 +2483,7 @@ private FirmwareUpdateException CreateFirmwareUpdateException( } var recoveryGuidance = BuildRecoveryGuidance(failedState, cleanupOutcome); - var message = $"Firmware update failed in state '{failedState}' while {failedOperation}."; + var message = $"{failureSubject} failed in state '{failedState}' while {failedOperation}."; return new FirmwareUpdateException( failedState, diff --git a/src/Daqifi.Core/Firmware/IPic32BootloaderDiagnostics.cs b/src/Daqifi.Core/Firmware/IPic32BootloaderDiagnostics.cs new file mode 100644 index 00000000..6b0cf5a4 --- /dev/null +++ b/src/Daqifi.Core/Firmware/IPic32BootloaderDiagnostics.cs @@ -0,0 +1,114 @@ +namespace Daqifi.Core.Firmware; + +/// +/// Lightweight PIC32 HID-bootloader diagnostics that run outside the full +/// +/// flow: a version health check and a JMP_TO_APP soft reset, neither of which +/// erases or programs flash. Intended for consumers (e.g. a recovery/manual bootloader +/// dialog) that want to probe or reset a bootloader session before committing to a flash. +/// +/// +/// +/// Implementations share the same HID transport and operation serialization as the full update +/// flow, so a diagnostic never runs concurrently with an in-flight update. A call made from a +/// separate execution context while an update is running waits for that update +/// to release the shared lock and then proceeds — it does not throw. Only a call re-entered from +/// within an in-flight operation's own synchronous callback (progress or state-change) is +/// rejected, because the shared lock is not reentrant and a diagnostic owns the HID +/// connect/version/reset exchange. +/// +/// +/// Bootloader operation failures (enumeration, connect, version, or reset) are wrapped in +/// (carrying +/// ), consistent with the full update flow. +/// Precondition, lifecycle, and concurrency failures are thrown directly instead: +/// for a whitespace target path, +/// when the service is disposed, +/// for callback reentrancy or a non-idle service, +/// and when the supplied token is canceled. +/// +/// +/// These calls are not instantaneous when no bootloader is present. Waiting for +/// the HID bootloader to enumerate is bounded by +/// FirmwareUpdateServiceOptions.WaitingForBootloaderTimeout (45 seconds by default), so a +/// probe against a device that is not in bootloader mode blocks for that long before +/// failing. A UI that probes opportunistically — e.g. when a device is first grabbed — should pass +/// a with its own deadline, or configure a +/// shorter timeout. +/// +/// +/// Unlike the full update flow, a failed health check does not attempt the +/// JMP_TO_APP self-recovery that flow applies before giving up: a check reports what it +/// finds rather than mutating device state. A failed +/// therefore does not imply an update would also fail. +/// Callers that want the remedy should invoke explicitly. +/// +/// +public interface IPic32BootloaderDiagnostics +{ + /// + /// Connects to the HID bootloader at the given device path (or the first match when + /// is null) and reads its version as a health check, + /// without erasing or programming flash. The HID transport is disconnected before the + /// call returns. + /// + /// + /// HID device path identifying which bootloader to probe (from discovery's + /// HidDeviceInfo.DevicePath). Null probes the first enumerated bootloader. + /// + /// Cancellation token. + /// The bootloader version string on success. + /// + /// Thrown when the bootloader could not be enumerated, connected to, or returned an + /// invalid version response. The exception's + /// and describe where the health + /// check failed. + /// + /// + /// Thrown when is non-null but whitespace. + /// + /// Thrown when the service has been disposed. + /// + /// Thrown when re-entered from within an in-flight firmware operation's own synchronous + /// callback, or when the service is not in an idle state. A concurrent call from a separate + /// execution context waits for the in-flight operation rather than throwing. + /// + /// + /// Thrown when is canceled. + /// + Task CheckBootloaderHealthAsync( + string? targetDevicePath = null, + CancellationToken cancellationToken = default); + + /// + /// Issues a JMP_TO_APP soft reset to the HID bootloader at the given device path + /// (or the first match when is null), forcing a clean + /// USB re-enumeration back into application mode, without touching flash contents. The HID + /// transport is disconnected before the call returns. + /// + /// + /// HID device path identifying which bootloader to reset (from discovery's + /// HidDeviceInfo.DevicePath). Null resets the first enumerated bootloader. + /// + /// Cancellation token. + /// + /// Thrown when the bootloader could not be enumerated, connected to, or the soft-reset + /// message could not be written. The exception's + /// and describe where the reset failed. + /// + /// + /// Thrown when is non-null but whitespace. + /// + /// Thrown when the service has been disposed. + /// + /// Thrown when re-entered from within an in-flight firmware operation's own synchronous + /// callback, or when the service is not in an idle state. A concurrent call from a separate + /// execution context waits for the in-flight operation rather than throwing. + /// + /// + /// Thrown when is canceled. + /// + Task ResetBootloaderAsync( + string? targetDevicePath = null, + CancellationToken cancellationToken = default); +}