From 7277f412011a295c8a10bbed9feae10facc1ae0a Mon Sep 17 00:00:00 2001 From: Tyler Kron Date: Mon, 20 Jul 2026 17:33:28 -0600 Subject: [PATCH 1/6] feat(firmware): expose standalone bootloader health-check and soft-reset (closes #299) Add IPic32BootloaderDiagnostics with CheckBootloaderHealthAsync and ResetBootloaderAsync, implemented on FirmwareUpdateService, so consumers (e.g. daqifi-desktop's recovery/manual bootloader dialog) can probe or reset a bootloader session without kicking off a full erase/program flash. Both reuse the existing private connect/retry/version/soft-reset plumbing via a new RunBootloaderDiagnosticAsync helper that serializes on the same operation lock and HID transport as the full update flow, rejects reentrancy from an in-flight update, and always releases the HID handle. Unlike an update these do not drive the update state machine (CurrentState stays Idle). Failures throw FirmwareUpdateException with RecoveryGuidance. Co-Authored-By: Claude Opus 4.8 --- .../Firmware/FirmwareUpdateServiceTests.cs | 192 ++++++++++++++++++ .../Firmware/FirmwareUpdateService.cs | 161 ++++++++++++++- .../Firmware/IPic32BootloaderDiagnostics.cs | 60 ++++++ 3 files changed, 412 insertions(+), 1 deletion(-) create mode 100644 src/Daqifi.Core/Firmware/IPic32BootloaderDiagnostics.cs diff --git a/src/Daqifi.Core.Tests/Firmware/FirmwareUpdateServiceTests.cs b/src/Daqifi.Core.Tests/Firmware/FirmwareUpdateServiceTests.cs index bdac143f..59288b60 100644 --- a/src/Daqifi.Core.Tests/Firmware/FirmwareUpdateServiceTests.cs +++ b/src/Daqifi.Core.Tests/Firmware/FirmwareUpdateServiceTests.cs @@ -3143,6 +3143,198 @@ 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 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; diff --git a/src/Daqifi.Core/Firmware/FirmwareUpdateService.cs b/src/Daqifi.Core/Firmware/FirmwareUpdateService.cs index d94fecdc..fd5c2a39 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,165 @@ 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); + } + }, + 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 _hidTransport + .WriteAsync(_bootloaderProtocol.CreateJumpToApplicationMessage(), 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); + } + }, + 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) diff --git a/src/Daqifi.Core/Firmware/IPic32BootloaderDiagnostics.cs b/src/Daqifi.Core/Firmware/IPic32BootloaderDiagnostics.cs new file mode 100644 index 00000000..6d48cd1c --- /dev/null +++ b/src/Daqifi.Core/Firmware/IPic32BootloaderDiagnostics.cs @@ -0,0 +1,60 @@ +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 these operations cannot run concurrently with — nor be re-entered from a +/// callback of — an in-flight update. Both throw +/// (carrying ) on failure, consistent +/// with the full update flow. +/// +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. + /// + 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. + /// + Task ResetBootloaderAsync( + string? targetDevicePath = null, + CancellationToken cancellationToken = default); +} From 5b0e14f8b275897cd031fc34aff3fd0221ed1c59 Mon Sep 17 00:00:00 2001 From: Tyler Kron Date: Mon, 20 Jul 2026 17:34:05 -0600 Subject: [PATCH 2/6] docs: log #299 bootloader-diagnostics fire in SESSION_LOG Co-Authored-By: Claude Opus 4.8 --- SESSION_LOG.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 SESSION_LOG.md diff --git a/SESSION_LOG.md b/SESSION_LOG.md new file mode 100644 index 00000000..5171b760 --- /dev/null +++ b/SESSION_LOG.md @@ -0,0 +1,13 @@ +# 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. From 4a94caa5db957c024f91a41ed3ee2bb1ffc8353d Mon Sep 17 00:00:00 2001 From: Tyler Kron Date: Mon, 20 Jul 2026 17:50:09 -0600 Subject: [PATCH 3/6] fix(firmware): enforce JumpingToApp timeout on standalone bootloader soft reset ResetBootloaderAsync issued the JMP_TO_APP HID write directly, bypassing ExecuteWithStateTimeoutAsync, so the configured JumpingToApplicationTimeout was not applied to the standalone soft-reset step. A blocking write (or one that ignores cancellation) could run unbounded and appear hung to callers. Wrap the write in ExecuteWithStateTimeoutAsync(JumpingToApp) to match the full update flow, preserving the existing JumpingToApp error mapping. Co-Authored-By: Claude Opus 4.8 --- .../Firmware/FirmwareUpdateServiceTests.cs | 43 ++++++++++++++++++- .../Firmware/FirmwareUpdateService.cs | 9 ++-- 2 files changed, 47 insertions(+), 5 deletions(-) diff --git a/src/Daqifi.Core.Tests/Firmware/FirmwareUpdateServiceTests.cs b/src/Daqifi.Core.Tests/Firmware/FirmwareUpdateServiceTests.cs index 59288b60..b04519e3 100644 --- a/src/Daqifi.Core.Tests/Firmware/FirmwareUpdateServiceTests.cs +++ b/src/Daqifi.Core.Tests/Firmware/FirmwareUpdateServiceTests.cs @@ -3311,6 +3311,34 @@ 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 BootloaderDiagnostic_AfterHealthCheck_ServiceStaysUsableForAnotherDiagnostic() { @@ -3662,7 +3690,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) @@ -3670,8 +3705,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 fd5c2a39..d2e4e4d9 100644 --- a/src/Daqifi.Core/Firmware/FirmwareUpdateService.cs +++ b/src/Daqifi.Core/Firmware/FirmwareUpdateService.cs @@ -401,9 +401,12 @@ await ExecuteWithStateTimeoutAsync( failedState = FirmwareUpdateState.JumpingToApp; failedOperation = "issue JMP_TO_APP soft reset"; - await _hidTransport - .WriteAsync(_bootloaderProtocol.CreateJumpToApplicationMessage(), ct) - .ConfigureAwait(false); + 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."); From e6e392bbe95bf7e23352116375c3e7e2cecad27b Mon Sep 17 00:00:00 2001 From: Tyler Kron Date: Mon, 20 Jul 2026 17:51:15 -0600 Subject: [PATCH 4/6] docs: log #375 Qodo-shepherding fire in SESSION_LOG Co-Authored-By: Claude Opus 4.8 --- SESSION_LOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/SESSION_LOG.md b/SESSION_LOG.md index 5171b760..be5b1c46 100644 --- a/SESSION_LOG.md +++ b/SESSION_LOG.md @@ -11,3 +11,10 @@ - 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. From ad47226605b8b7a6972dd653e9c9cedfe64922f4 Mon Sep 17 00:00:00 2001 From: Tyler Kron Date: Mon, 20 Jul 2026 18:07:37 -0600 Subject: [PATCH 5/6] docs(firmware): document non-FirmwareUpdateException throws on IPic32BootloaderDiagnostics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The diagnostic methods also throw ArgumentException (whitespace path), ObjectDisposedException (disposed service), InvalidOperationException (reentrancy / non-idle), and propagate OperationCanceledException — none of which were on the interface contract. Add tags and clarify so consumers handle the right surface. Co-Authored-By: Claude Opus 4.8 --- .../Firmware/IPic32BootloaderDiagnostics.cs | 34 +++++++++++++++++-- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/src/Daqifi.Core/Firmware/IPic32BootloaderDiagnostics.cs b/src/Daqifi.Core/Firmware/IPic32BootloaderDiagnostics.cs index 6d48cd1c..adefebd8 100644 --- a/src/Daqifi.Core/Firmware/IPic32BootloaderDiagnostics.cs +++ b/src/Daqifi.Core/Firmware/IPic32BootloaderDiagnostics.cs @@ -10,9 +10,15 @@ namespace Daqifi.Core.Firmware; /// /// Implementations share the same HID transport and operation serialization as the full /// update flow, so these operations cannot run concurrently with — nor be re-entered from a -/// callback of — an in-flight update. Both throw -/// (carrying ) on failure, consistent -/// with the full update flow. +/// callback of — an in-flight update. 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, +/// when invoked while another firmware operation +/// is in flight (including reentrancy from its callbacks) or the service is not idle, and +/// when the supplied token is canceled. /// public interface IPic32BootloaderDiagnostics { @@ -34,6 +40,17 @@ public interface IPic32BootloaderDiagnostics /// and describe where the health /// check failed. /// + /// + /// Thrown when is non-null but whitespace. + /// + /// Thrown when the service has been disposed. + /// + /// Thrown when another firmware operation is in flight (including reentrancy from its + /// callbacks) or the service is not in an idle state. + /// + /// + /// Thrown when is canceled. + /// Task CheckBootloaderHealthAsync( string? targetDevicePath = null, CancellationToken cancellationToken = default); @@ -54,6 +71,17 @@ Task CheckBootloaderHealthAsync( /// 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 another firmware operation is in flight (including reentrancy from its + /// callbacks) or the service is not in an idle state. + /// + /// + /// Thrown when is canceled. + /// Task ResetBootloaderAsync( string? targetDevicePath = null, CancellationToken cancellationToken = default); From 3c61d5fcbdac95e32ad613d0299f16ce402a20a3 Mon Sep 17 00:00:00 2001 From: Tyler Kron Date: Wed, 22 Jul 2026 16:01:53 -0600 Subject: [PATCH 6/6] fix(firmware): stop bootloader diagnostics reporting "Firmware update failed" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A health check or soft reset is what a consumer runs *instead of* starting an update, but both routed through CreateFirmwareUpdateException, which hard-codes "Firmware update failed in state 'X' while Y." A recovery dialog would tell a user their firmware update failed when none was ever attempted. Confirmed on a real Nq1: probing with no bootloader present emitted "Firmware update failed in state 'WaitingForBootloader'". CreateFirmwareUpdateException gains a failureSubject parameter defaulting to "Firmware update", so the update flow's wording is unchanged (pinned by a new regression test); the diagnostics pass "Bootloader health check" and "Bootloader soft reset". Also corrects the IPic32BootloaderDiagnostics throw contract, which claimed InvalidOperationException is raised "when another firmware operation is in flight". Only reentrancy from an in-flight operation's own synchronous callback throws — a concurrent call from a separate execution context waits on the shared lock and then proceeds. Documents two further behaviours callers need: the 45s default WaitingForBootloaderTimeout means a probe against a device that is not in bootloader mode blocks that long, and a failed health check does not imply an update would fail, since the check deliberately skips the #298 JMP_TO_APP self-heal the update flow applies. Adds 8 tests: message wording for both diagnostics plus an update-flow regression guard, ResetBootloaderAsync disposed-guard symmetry, cancellation for both methods, callback-reentrancy rejection, and the concurrent-call-waits semantics the docs now promise. Bench-tested end to end on a real Nq1 (fw 3.7.2) — no flash written: FORceBoot -> HID 04D8:003C -> health check returns bootloader version 1.4 in ~18ms -> repeat on the same service and from a fresh instance both succeed (the real proof the HID handle is released at OS level) -> connect-by-path targeting works, bogus path correctly refuses to fall back -> JMP_TO_APP in ~117ms -> device returns to app mode with firmware version and serial number unchanged. Full suite green on net9.0 and net10.0 (1783 tests, 1781 passed, 2 skipped), 0 warnings. Co-Authored-By: Claude Opus 4.8 --- SESSION_LOG.md | 10 + .../Firmware/FirmwareUpdateServiceTests.cs | 221 ++++++++++++++++++ .../Firmware/FirmwareUpdateService.cs | 15 +- .../Firmware/IPic32BootloaderDiagnostics.cs | 52 +++-- 4 files changed, 281 insertions(+), 17 deletions(-) diff --git a/SESSION_LOG.md b/SESSION_LOG.md index be5b1c46..99ee098b 100644 --- a/SESSION_LOG.md +++ b/SESSION_LOG.md @@ -18,3 +18,13 @@ - 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 b04519e3..6df65b42 100644 --- a/src/Daqifi.Core.Tests/Firmware/FirmwareUpdateServiceTests.cs +++ b/src/Daqifi.Core.Tests/Firmware/FirmwareUpdateServiceTests.cs @@ -3339,6 +3339,227 @@ public async Task ResetBootloaderAsync_WhenJumpToAppWriteHangs_TimesOutInJumping 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() { diff --git a/src/Daqifi.Core/Firmware/FirmwareUpdateService.cs b/src/Daqifi.Core/Firmware/FirmwareUpdateService.cs index d2e4e4d9..0e533b29 100644 --- a/src/Daqifi.Core/Firmware/FirmwareUpdateService.cs +++ b/src/Daqifi.Core/Firmware/FirmwareUpdateService.cs @@ -368,7 +368,8 @@ await ExecuteWithStateTimeoutAsync( } catch (Exception ex) { - throw CreateFirmwareUpdateException(failedState, failedOperation, ex); + throw CreateFirmwareUpdateException( + failedState, failedOperation, ex, failureSubject: "Bootloader health check"); } }, cancellationToken); @@ -418,7 +419,8 @@ await ExecuteWithStateTimeoutAsync( } catch (Exception ex) { - throw CreateFirmwareUpdateException(failedState, failedOperation, ex); + throw CreateFirmwareUpdateException( + failedState, failedOperation, ex, failureSubject: "Bootloader soft reset"); } }, cancellationToken).ConfigureAwait(false); @@ -2461,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) { @@ -2476,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 index adefebd8..6b0cf5a4 100644 --- a/src/Daqifi.Core/Firmware/IPic32BootloaderDiagnostics.cs +++ b/src/Daqifi.Core/Firmware/IPic32BootloaderDiagnostics.cs @@ -8,17 +8,41 @@ namespace Daqifi.Core.Firmware; /// 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 these operations cannot run concurrently with — nor be re-entered from a -/// callback of — an in-flight update. 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: +/// +/// 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, -/// when invoked while another firmware operation -/// is in flight (including reentrancy from its callbacks) or the service is not idle, and -/// when the supplied token is canceled. +/// 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 { @@ -45,8 +69,9 @@ public interface IPic32BootloaderDiagnostics /// /// Thrown when the service has been disposed. /// - /// Thrown when another firmware operation is in flight (including reentrancy from its - /// callbacks) or the service is not in an idle state. + /// 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. @@ -76,8 +101,9 @@ Task CheckBootloaderHealthAsync( /// /// Thrown when the service has been disposed. /// - /// Thrown when another firmware operation is in flight (including reentrancy from its - /// callbacks) or the service is not in an idle state. + /// 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.