diff --git a/src/Daqifi.Core.Tests/Firmware/FirmwareUpdateServiceTests.cs b/src/Daqifi.Core.Tests/Firmware/FirmwareUpdateServiceTests.cs index d86346b..be848e5 100644 --- a/src/Daqifi.Core.Tests/Firmware/FirmwareUpdateServiceTests.cs +++ b/src/Daqifi.Core.Tests/Firmware/FirmwareUpdateServiceTests.cs @@ -491,6 +491,462 @@ public async Task UpdateWifiModuleAsync_WhenDeviceDoesNotSupportLanQuery_Proceed Assert.Contains("SYSTem:COMMUnicate:LAN:FWUpdate", device.SentCommands); } + [Fact] + public async Task CheckWifiFirmwareStatusAsync_WhenVersionMatches_ReturnsUpToDate() + { + // Closes #143: callers can now make the version decision themselves + // without triggering UpdateWifiModuleAsync's hidden internal probe. + // This test asserts the new public planning method returns the + // expected status object and DOES NOT mutate service state. + var wifiRelease = new FirmwareReleaseInfo + { + Version = new FirmwareVersion(19, 5, 4, null, 0), + TagName = "19.5.4", + IsPreRelease = false + }; + var device = new FakeLanChipInfoStreamingDevice("COM9", chipInfo: new LanChipInfo + { + ChipId = 1234, + FwVersion = "19.5.4", + BuildDate = "Jan 8 2019" + }); + + var service = new FirmwareUpdateService( + new FakeHidTransport(), + new FakeFirmwareDownloadService { LatestWifiRelease = wifiRelease }, + new FakeExternalProcessRunner(), + NullLogger.Instance, + new FakeBootloaderProtocol([[0x10]]), + new FakeHidDeviceEnumerator([]), + CreateFastOptions()); + + var status = await service.CheckWifiFirmwareStatusAsync(device); + + Assert.True(status.IsUpToDate); + Assert.Equal(WifiFirmwareStatusReason.UpToDate, status.Reason); + Assert.NotNull(status.CurrentChipInfo); + Assert.Equal("19.5.4", status.CurrentChipInfo!.FwVersion); + Assert.NotNull(status.LatestRelease); + Assert.Equal("19.5.4", status.LatestRelease!.TagName); + + // The planning method must NOT mutate service state (the internal + // IsWifiFirmwareUpToDateAsync transitions to Complete on its + // hit-path; CheckWifiFirmwareStatusAsync must not, so callers can + // call it freely without locking out a subsequent flash. + Assert.Equal(FirmwareUpdateState.Idle, service.CurrentState); + } + + [Fact] + public async Task CheckWifiFirmwareStatusAsync_WhenVersionOlder_ReturnsUpdateAvailable() + { + var wifiRelease = new FirmwareReleaseInfo + { + Version = new FirmwareVersion(19, 6, 1, null, 0), + TagName = "19.6.1", + IsPreRelease = false + }; + var device = new FakeLanChipInfoStreamingDevice("COM10", chipInfo: new LanChipInfo + { + ChipId = 1234, + FwVersion = "19.5.4", + BuildDate = "Jan 8 2019" + }); + + var service = new FirmwareUpdateService( + new FakeHidTransport(), + new FakeFirmwareDownloadService { LatestWifiRelease = wifiRelease }, + new FakeExternalProcessRunner(), + NullLogger.Instance, + new FakeBootloaderProtocol([[0x10]]), + new FakeHidDeviceEnumerator([]), + CreateFastOptions()); + + var status = await service.CheckWifiFirmwareStatusAsync(device); + + Assert.False(status.IsUpToDate); + Assert.Equal(WifiFirmwareStatusReason.UpdateAvailable, status.Reason); + Assert.Equal(FirmwareUpdateState.Idle, service.CurrentState); + } + + [Fact] + public async Task CheckWifiFirmwareStatusAsync_WhenDeviceDoesNotSupportLanQuery_ReturnsDeviceDoesNotSupportLanQuery() + { + var device = new FakeStreamingDevice("COM11"); + + var service = new FirmwareUpdateService( + new FakeHidTransport(), + new FakeFirmwareDownloadService(), + new FakeExternalProcessRunner(), + NullLogger.Instance, + new FakeBootloaderProtocol([[0x10]]), + new FakeHidDeviceEnumerator([]), + CreateFastOptions()); + + var status = await service.CheckWifiFirmwareStatusAsync(device); + + Assert.False(status.IsUpToDate); + Assert.Equal(WifiFirmwareStatusReason.DeviceDoesNotSupportLanQuery, status.Reason); + Assert.Null(status.CurrentChipInfo); + Assert.Null(status.LatestRelease); + } + + [Fact] + public async Task UpdateWifiModuleAsync_WithSkipVersionCheck_BypassesProbeAndAlwaysFlashes() + { + // The motivating case for #143: caller already made the version + // decision (via CheckWifiFirmwareStatusAsync). Pass skipVersionCheck:true + // so Core does NOT re-probe the device — even when the device's + // current version equals the latest, the flash flow runs. + var wifiRelease = new FirmwareReleaseInfo + { + Version = new FirmwareVersion(19, 5, 4, null, 0), + TagName = "19.5.4", + IsPreRelease = false + }; + var device = new FakeLanChipInfoStreamingDevice("COM12", chipInfo: new LanChipInfo + { + ChipId = 1234, + FwVersion = "19.5.4", // Matches latest — would normally short-circuit + BuildDate = "Jan 8 2019" + }); + + var externalProcessRunner = new FakeExternalProcessRunner + { + NextResult = new ExternalProcessResult(0, false, TimeSpan.FromMilliseconds(10), [], []) + }; + + var options = CreateFastOptions(); + options.PostLanFirmwareModeDelay = TimeSpan.FromMilliseconds(5); + options.PostWifiReconnectDelay = TimeSpan.FromMilliseconds(5); + + var service = new FirmwareUpdateService( + new FakeHidTransport(), + new FakeFirmwareDownloadService { LatestWifiRelease = wifiRelease }, + externalProcessRunner, + NullLogger.Instance, + new FakeBootloaderProtocol([[0x10]]), + new FakeHidDeviceEnumerator([]), + options); + + var firmwareDir = CreateTempDirectory(); + File.WriteAllText(Path.Combine(firmwareDir, "winc_flash_tool.cmd"), "@echo off"); + + try + { + await service.UpdateWifiModuleAsync( + device, + firmwareDir, + progress: null, + skipVersionCheck: true); + } + finally + { + Directory.Delete(firmwareDir, recursive: true); + } + + // Even though device version matched latest, flash ran because + // skipVersionCheck bypassed the probe. + Assert.Equal(FirmwareUpdateState.Complete, service.CurrentState); + Assert.Contains("SYSTem:COMMUnicate:LAN:FWUpdate", device.SentCommands); + } + + [Fact] + public async Task CheckWifiFirmwareStatusAsync_ReentrantCallFromUpdateProgressCallback_DoesNotDeadlock() + { + // Closes a Qodo finding on PR #198: UpdateWifiModuleAsync holds + // _operationLock while synchronously firing progress.Report(). + // A handler that calls back into CheckWifiFirmwareStatusAsync + // would deadlock waiting for the same lock the update flow owns + // (SemaphoreSlim is not re-entrant). The AsyncLocal _isInsideOperation + // flag detects this case and skips the second acquisition. + // + // The test invokes CheckWifiFirmwareStatusAsync from inside a + // progress callback fired by UpdateWifiModuleAsync. Without the + // re-entrancy guard, this hangs until xunit's per-test budget + // kills it. With the guard, the inner call returns quickly and + // the update completes normally. + var wifiRelease = new FirmwareReleaseInfo + { + Version = new FirmwareVersion(19, 6, 1, null, 0), + TagName = "19.6.1", + IsPreRelease = false + }; + var device = new FakeLanChipInfoStreamingDevice("COM13", chipInfo: new LanChipInfo + { + ChipId = 1234, + FwVersion = "19.5.4", + BuildDate = "Jan 8 2019" + }); + + var externalProcessRunner = new FakeExternalProcessRunner + { + NextResult = new ExternalProcessResult(0, false, TimeSpan.FromMilliseconds(10), [], []) + }; + + var options = CreateFastOptions(); + options.PostLanFirmwareModeDelay = TimeSpan.FromMilliseconds(5); + options.PostWifiReconnectDelay = TimeSpan.FromMilliseconds(5); + + var service = new FirmwareUpdateService( + new FakeHidTransport(), + new FakeFirmwareDownloadService { LatestWifiRelease = wifiRelease }, + externalProcessRunner, + NullLogger.Instance, + new FakeBootloaderProtocol([[0x10]]), + new FakeHidDeviceEnumerator([]), + options); + + var firmwareDir = CreateTempDirectory(); + File.WriteAllText(Path.Combine(firmwareDir, "winc_flash_tool.cmd"), "@echo off"); + + WifiFirmwareStatus? reentrantStatus = null; + var reentryAttempted = false; + + // Progress handler that re-enters CheckWifiFirmwareStatusAsync + // exactly once (on the first event) — mirrors a UI consumer + // that might want to refresh status display when an update + // transitions state. + var progress = new SyncProgress(_ => + { + if (reentryAttempted) + { + return; + } + reentryAttempted = true; + reentrantStatus = service.CheckWifiFirmwareStatusAsync(device).GetAwaiter().GetResult(); + }); + + // Hard timeout via WaitAsync so a regression of the deadlock fails + // fast instead of stalling the whole xunit run waiting for the + // global per-test budget. The fast-options + ms-scale delays mean + // the happy path completes in well under a second; 30s is plenty + // of margin under heavy CI load while still being a clear "stuck". + try + { + using var timeoutCts = new CancellationTokenSource(TimeSpan.FromSeconds(30)); + await service.UpdateWifiModuleAsync(device, firmwareDir, progress) + .WaitAsync(timeoutCts.Token); + } + catch (OperationCanceledException) when (!reentryAttempted) + { + throw new Xunit.Sdk.XunitException( + "Test setup failed before reentrancy was attempted."); + } + catch (OperationCanceledException) + { + throw new Xunit.Sdk.XunitException( + "UpdateWifiModuleAsync did not complete within 30s — the " + + "AsyncLocal re-entrancy guard likely regressed and the " + + "inner CheckWifiFirmwareStatusAsync call is deadlocked."); + } + finally + { + Directory.Delete(firmwareDir, recursive: true); + } + + Assert.True(reentryAttempted, "Progress callback never fired — test setup wrong."); + Assert.NotNull(reentrantStatus); + Assert.Equal(FirmwareUpdateState.Complete, service.CurrentState); + } + + [Fact] + public async Task CheckWifiFirmwareStatusAsync_WhenLanChipInfoFailsTransiently_RetriesUntilSuccess() + { + // Closes #144: post-PIC32-reboot the WiFi subsystem can lag the + // application by a few seconds, so the first chip-info query + // transiently fails. Without retry, the WiFi version decision + // would short-circuit and trigger an unnecessary multi-minute + // reflash. The retry budget covers the startup window. + var wifiRelease = new FirmwareReleaseInfo + { + Version = new FirmwareVersion(19, 5, 4, null, 0), + TagName = "19.5.4", + IsPreRelease = false + }; + var device = new FakeLanChipInfoStreamingDevice( + "COM14", + chipInfo: new LanChipInfo + { + ChipId = 1234, + FwVersion = "19.5.4", + BuildDate = "Jan 8 2019" + }, + transientFailuresBeforeSuccess: 2); + + var options = CreateFastOptions(); + options.LanChipInfoMaxAttempts = 3; + options.LanChipInfoRetryDelay = TimeSpan.FromMilliseconds(5); // Keep test fast + + var service = new FirmwareUpdateService( + new FakeHidTransport(), + new FakeFirmwareDownloadService { LatestWifiRelease = wifiRelease }, + new FakeExternalProcessRunner(), + NullLogger.Instance, + new FakeBootloaderProtocol([[0x10]]), + new FakeHidDeviceEnumerator([]), + options); + + var status = await service.CheckWifiFirmwareStatusAsync(device); + + Assert.Equal(3, device.GetLanChipInfoCallCount); + Assert.True(status.IsUpToDate); + Assert.Equal(WifiFirmwareStatusReason.UpToDate, status.Reason); + } + + [Fact] + public async Task CheckWifiFirmwareStatusAsync_WhenLanChipInfoFailsAllAttempts_ReturnsChipInfoUnavailable() + { + // After exhausting LanChipInfoMaxAttempts the planning method must + // fall through to ChipInfoUnavailable, NOT hang or surface the + // raw exception. This preserves the "couldn't check, default to + // running update" semantics for genuinely-broken devices. + var device = new FakeLanChipInfoStreamingDevice( + "COM15", + chipInfo: new LanChipInfo + { + ChipId = 1234, + FwVersion = "19.5.4", + BuildDate = "Jan 8 2019" + }, + transientFailuresBeforeSuccess: 99); + + var options = CreateFastOptions(); + options.LanChipInfoMaxAttempts = 3; + options.LanChipInfoRetryDelay = TimeSpan.FromMilliseconds(5); + + var service = new FirmwareUpdateService( + new FakeHidTransport(), + new FakeFirmwareDownloadService(), + new FakeExternalProcessRunner(), + NullLogger.Instance, + new FakeBootloaderProtocol([[0x10]]), + new FakeHidDeviceEnumerator([]), + options); + + var status = await service.CheckWifiFirmwareStatusAsync(device); + + Assert.Equal(3, device.GetLanChipInfoCallCount); + Assert.False(status.IsUpToDate); + Assert.Equal(WifiFirmwareStatusReason.ChipInfoUnavailable, status.Reason); + } + + [Fact] + public async Task CheckWifiFirmwareStatusAsync_TotalTimeoutHit_ShortCircuitsToChipInfoUnavailable() + { + // Closes a Qodo follow-up on PR #199: per-attempt query timeouts + // compound with retry delays, so a high MaxAttempts × non-trivial + // per-attempt latency could block far beyond the configured retry + // budget while holding _operationLock. The total-timeout cap caps + // wall-clock time independent of attempt counts. + // + // The fake's per-attempt latency is the Task.Delay below; with + // 200ms latency × 10 max attempts × 100ms retry delay, a naive + // implementation would block ~2.9s. The 100ms total timeout + // forces an early ChipInfoUnavailable return after the first + // attempt's latency exceeds the budget. + var device = new SlowFakeLanChipInfoStreamingDevice( + "COM17", + attemptLatency: TimeSpan.FromMilliseconds(200)); + + var options = CreateFastOptions(); + options.LanChipInfoMaxAttempts = 10; + options.LanChipInfoRetryDelay = TimeSpan.FromMilliseconds(100); + options.LanChipInfoTotalTimeout = TimeSpan.FromMilliseconds(100); + + var service = new FirmwareUpdateService( + new FakeHidTransport(), + new FakeFirmwareDownloadService(), + new FakeExternalProcessRunner(), + NullLogger.Instance, + new FakeBootloaderProtocol([[0x10]]), + new FakeHidDeviceEnumerator([]), + options); + + var stopwatch = System.Diagnostics.Stopwatch.StartNew(); + var status = await service.CheckWifiFirmwareStatusAsync(device); + stopwatch.Stop(); + + Assert.False(status.IsUpToDate); + Assert.Equal(WifiFirmwareStatusReason.ChipInfoUnavailable, status.Reason); + // Should bail well before attempting all 10 × (200ms + 100ms) = 3s. + // Allowing 1500ms for CI variance / xunit overhead. + Assert.True(stopwatch.Elapsed < TimeSpan.FromMilliseconds(1500), + $"Probe took {stopwatch.ElapsedMilliseconds}ms, expected <1500ms with TotalTimeout=100ms."); + } + + private sealed class SlowFakeLanChipInfoStreamingDevice : IStreamingDevice, ILanChipInfoProvider + { + private readonly TimeSpan _attemptLatency; + public SlowFakeLanChipInfoStreamingDevice(string name, TimeSpan attemptLatency) + { + Name = name; + _attemptLatency = attemptLatency; + IsConnected = true; + } + public string Name { get; } + public IPAddress? IpAddress => null; + public bool IsConnected { get; private set; } + public ConnectionStatus Status => ConnectionStatus.Connected; + public int StreamingFrequency { get; set; } + public bool IsStreaming { get; private set; } + public event EventHandler? StatusChanged { add { } remove { } } + public event EventHandler? MessageReceived { add { } remove { } } + public void Connect() => IsConnected = true; + public void Disconnect() => IsConnected = false; + public void Send(IOutboundMessage message) { } + public void StartStreaming() => IsStreaming = true; + public void StopStreaming() => IsStreaming = false; + public async Task GetLanChipInfoAsync(CancellationToken cancellationToken = default) + { + await Task.Delay(_attemptLatency, cancellationToken).ConfigureAwait(false); + return null; + } + } + + [Fact] + public async Task CheckWifiFirmwareStatusAsync_FirstAttemptSucceeds_DoesNotRetry() + { + // Steady-state path: no retry overhead when the first call works. + var wifiRelease = new FirmwareReleaseInfo + { + Version = new FirmwareVersion(19, 5, 4, null, 0), + TagName = "19.5.4", + IsPreRelease = false + }; + var device = new FakeLanChipInfoStreamingDevice( + "COM16", + chipInfo: new LanChipInfo + { + ChipId = 1234, + FwVersion = "19.5.4", + BuildDate = "Jan 8 2019" + }); + + var options = CreateFastOptions(); + options.LanChipInfoMaxAttempts = 3; + options.LanChipInfoRetryDelay = TimeSpan.FromMilliseconds(5); + + var service = new FirmwareUpdateService( + new FakeHidTransport(), + new FakeFirmwareDownloadService { LatestWifiRelease = wifiRelease }, + new FakeExternalProcessRunner(), + NullLogger.Instance, + new FakeBootloaderProtocol([[0x10]]), + new FakeHidDeviceEnumerator([]), + options); + + await service.CheckWifiFirmwareStatusAsync(device); + + Assert.Equal(1, device.GetLanChipInfoCallCount); + } + + private sealed class SyncProgress : IProgress + { + private readonly Action _handler; + public SyncProgress(Action handler) => _handler = handler; + public void Report(T value) => _handler(value); + } + private static FirmwareUpdateServiceOptions CreateFastOptions() { return new FirmwareUpdateServiceOptions @@ -602,14 +1058,18 @@ private sealed class FakeLanChipInfoStreamingDevice : IStreamingDevice, ILanChip { private readonly LanChipInfo? _chipInfo; private ConnectionStatus _status = ConnectionStatus.Connected; + private int _remainingTransientFailures; - public FakeLanChipInfoStreamingDevice(string name, LanChipInfo? chipInfo) + public FakeLanChipInfoStreamingDevice(string name, LanChipInfo? chipInfo, int transientFailuresBeforeSuccess = 0) { Name = name; _chipInfo = chipInfo; + _remainingTransientFailures = transientFailuresBeforeSuccess; IsConnected = true; } + public int GetLanChipInfoCallCount { get; private set; } + public string Name { get; } public IPAddress? IpAddress => null; public bool IsConnected { get; private set; } @@ -653,6 +1113,16 @@ public void Send(IOutboundMessage message) public Task GetLanChipInfoAsync(CancellationToken cancellationToken = default) { + GetLanChipInfoCallCount++; + if (_remainingTransientFailures > 0) + { + _remainingTransientFailures--; + // Faulted task (not sync throw) more accurately simulates how + // a real async method surfaces failure — caller's await sees a + // genuinely-async exception path. + return Task.FromException( + new InvalidOperationException("Simulated transient post-reboot failure.")); + } return Task.FromResult(_chipInfo); } } diff --git a/src/Daqifi.Core/Firmware/FirmwareUpdateService.cs b/src/Daqifi.Core/Firmware/FirmwareUpdateService.cs index 7118aa5..41c2ff3 100644 --- a/src/Daqifi.Core/Firmware/FirmwareUpdateService.cs +++ b/src/Daqifi.Core/Firmware/FirmwareUpdateService.cs @@ -75,6 +75,16 @@ private static readonly IReadOnlyDictionary _isInsideOperation = new(); private readonly IHidTransport _hidTransport; private readonly IExternalProcessRunner _externalProcessRunner; private readonly ILogger _logger; @@ -154,11 +164,14 @@ await RunExclusiveAsync( } /// +#pragma warning disable CA1068 public async Task UpdateWifiModuleAsync( IStreamingDevice device, string firmwarePath, IProgress? progress = null, - CancellationToken cancellationToken = default) + CancellationToken cancellationToken = default, + bool skipVersionCheck = false) +#pragma warning restore CA1068 { ArgumentNullException.ThrowIfNull(device); @@ -174,10 +187,51 @@ public async Task UpdateWifiModuleAsync( } await RunExclusiveAsync( - ct => RunWifiUpdateAsync(device, firmwarePath, progress, ct), + ct => RunWifiUpdateAsync(device, firmwarePath, progress, skipVersionCheck, ct), cancellationToken).ConfigureAwait(false); } + /// + public async Task CheckWifiFirmwareStatusAsync( + IStreamingDevice device, + CancellationToken cancellationToken = default) + { + ArgumentNullException.ThrowIfNull(device); + ThrowIfDisposed(); + + // Serialize device I/O with UpdateFirmwareAsync / UpdateWifiModuleAsync. + // GetLanChipInfoAsync runs a SCPI text exchange on the same transport + // those updates use; a concurrent caller would interleave on the wire + // and either corrupt the consumer swap or get partial replies. + // Acquired without the RunExclusiveAsync state check — a status probe + // is read-only and should be available to UI even when an update is + // in flight (it just waits for the in-flight I/O to release the lock). + // + // Reentrancy guard: an update fires progress / state-change callbacks + // synchronously while it holds _operationLock. If a callback handler + // calls back into CheckWifiFirmwareStatusAsync, WaitAsync would + // deadlock waiting for the same lock the caller's flow already owns + // (SemaphoreSlim is not re-entrant). The AsyncLocal flag detects + // this case and skips the second acquisition; we're already in a + // serialized device-I/O context. + if (_isInsideOperation.Value) + { + return await CheckWifiFirmwareStatusCoreAsync(device, cancellationToken).ConfigureAwait(false); + } + + await _operationLock.WaitAsync(cancellationToken).ConfigureAwait(false); + _isInsideOperation.Value = true; + try + { + return await CheckWifiFirmwareStatusCoreAsync(device, cancellationToken).ConfigureAwait(false); + } + finally + { + _isInsideOperation.Value = false; + _operationLock.Release(); + } + } + private async Task RunExclusiveAsync( Func operation, CancellationToken cancellationToken) @@ -185,6 +239,7 @@ private async Task RunExclusiveAsync( ThrowIfDisposed(); await _operationLock.WaitAsync(cancellationToken).ConfigureAwait(false); + _isInsideOperation.Value = true; try { ResetIfTerminalState(); @@ -202,6 +257,7 @@ private async Task RunExclusiveAsync( } finally { + _isInsideOperation.Value = false; _operationLock.Release(); } } @@ -330,13 +386,15 @@ private async Task RunWifiUpdateAsync( IStreamingDevice device, string firmwarePath, IProgress? progress, + bool skipVersionCheck, CancellationToken cancellationToken) { const long totalBytes = 100; try { - if (await IsWifiFirmwareUpToDateAsync(device, progress, cancellationToken).ConfigureAwait(false)) + if (!skipVersionCheck + && await IsWifiFirmwareUpToDateAsync(device, progress, cancellationToken).ConfigureAwait(false)) { return; } @@ -433,25 +491,67 @@ private async Task IsWifiFirmwareUpToDateAsync( IProgress? progress, CancellationToken cancellationToken) { - if (device is not ILanChipInfoProvider lanChipInfoProvider) + // Internal callsite: in addition to deciding the boolean, we must + // transition to Complete + report 100% progress so the caller's + // single UpdateWifiModuleAsync(...) call observes the same end-state + // as a successful flash. CheckWifiFirmwareStatusAsync (the public + // planning API) does not have that side effect — its callers own + // their own logging / UI transitions. + var status = await CheckWifiFirmwareStatusCoreAsync(device, cancellationToken).ConfigureAwait(false); + + switch (status.Reason) { - return false; - } + case WifiFirmwareStatusReason.UpdateAvailable: + _logger.LogInformation( + "WiFi firmware update available: device has {DeviceVersion}, latest is {LatestVersion}.", + status.CurrentChipInfo!.FwVersion, + status.LatestRelease!.TagName); + return false; + + case WifiFirmwareStatusReason.UpToDate: + var message = $"WiFi firmware is already up to date (device: {status.CurrentChipInfo!.FwVersion}, latest: {status.LatestRelease!.TagName})."; + _logger.LogInformation(message); + TransitionToState(FirmwareUpdateState.Complete, message); + ReportProgress(progress, FirmwareUpdateState.Complete, 100, message, 100, 100); + return true; - LanChipInfo? chipInfo; - try - { - chipInfo = await lanChipInfoProvider.GetLanChipInfoAsync(cancellationToken).ConfigureAwait(false); + default: + // DeviceDoesNotSupportLanQuery, ChipInfoUnavailable, + // LatestReleaseUnavailable, VersionUnparseable — proceed + // with the flash conservatively. + return false; } - catch (Exception ex) when (ex is not OperationCanceledException) + } + + private async Task CheckWifiFirmwareStatusCoreAsync( + IStreamingDevice device, + CancellationToken cancellationToken) + { + if (device is not ILanChipInfoProvider lanChipInfoProvider) { - _logger.LogDebug(ex, "Failed to query LAN chip info; proceeding with WiFi firmware update."); - return false; + return new WifiFirmwareStatus + { + IsUpToDate = false, + Reason = WifiFirmwareStatusReason.DeviceDoesNotSupportLanQuery, + }; } + // Bounded retry for the LAN chip-info probe (closes #144). Right + // after a PIC32 firmware update the application is up while WiFi + // is still finishing startup, so the first chip-info query can + // transiently fail; without retry, the WiFi version decision + // would short-circuit to ChipInfoUnavailable and flow on to a + // multi-minute reflash of already-current WiFi firmware. The + // retry budget is bounded (LanChipInfoMaxAttempts × RetryDelay) + // and observes cancellation between attempts. + var chipInfo = await TryGetLanChipInfoWithRetryAsync(lanChipInfoProvider, cancellationToken).ConfigureAwait(false); if (chipInfo == null) { - return false; + return new WifiFirmwareStatus + { + IsUpToDate = false, + Reason = WifiFirmwareStatusReason.ChipInfoUnavailable, + }; } FirmwareReleaseInfo? latestWifi; @@ -463,40 +563,144 @@ private async Task IsWifiFirmwareUpToDateAsync( } catch (Exception ex) when (ex is not OperationCanceledException) { - _logger.LogDebug(ex, "Failed to query latest WiFi firmware release; proceeding with update."); - return false; + _logger.LogDebug(ex, "Failed to query latest WiFi firmware release; reporting status as LatestReleaseUnavailable."); + return new WifiFirmwareStatus + { + CurrentChipInfo = chipInfo, + IsUpToDate = false, + Reason = WifiFirmwareStatusReason.LatestReleaseUnavailable, + }; } if (latestWifi == null) { - return false; + return new WifiFirmwareStatus + { + CurrentChipInfo = chipInfo, + IsUpToDate = false, + Reason = WifiFirmwareStatusReason.LatestReleaseUnavailable, + }; } - if (!IsWifiVersionCurrent(chipInfo.FwVersion, latestWifi.TagName)) + // Only the device-reported version needs parsing; latestWifi.Version + // is already a strongly-typed FirmwareVersion from FirmwareDownloadService. + // Re-parsing TagName would risk divergence from the canonical Version + // (different tag prefix conventions, etc.) and cost an extra parse. + if (!FirmwareVersion.TryParse(chipInfo.FwVersion, out var deviceVersion)) { - _logger.LogInformation( - "WiFi firmware update available: device has {DeviceVersion}, latest is {LatestVersion}.", - chipInfo.FwVersion, - latestWifi.TagName); - return false; + return new WifiFirmwareStatus + { + CurrentChipInfo = chipInfo, + LatestRelease = latestWifi, + IsUpToDate = false, + Reason = WifiFirmwareStatusReason.VersionUnparseable, + }; } - var message = $"WiFi firmware is already up to date (device: {chipInfo.FwVersion}, latest: {latestWifi.TagName})."; - _logger.LogInformation(message); - TransitionToState(FirmwareUpdateState.Complete, message); - ReportProgress(progress, FirmwareUpdateState.Complete, 100, message, 100, 100); - return true; + var isCurrent = deviceVersion >= latestWifi.Version; + return new WifiFirmwareStatus + { + CurrentChipInfo = chipInfo, + LatestRelease = latestWifi, + IsUpToDate = isCurrent, + Reason = isCurrent ? WifiFirmwareStatusReason.UpToDate : WifiFirmwareStatusReason.UpdateAvailable, + }; } - private static bool IsWifiVersionCurrent(string deviceVersion, string latestTagName) + private async Task TryGetLanChipInfoWithRetryAsync( + ILanChipInfoProvider lanChipInfoProvider, + CancellationToken cancellationToken) { - if (!FirmwareVersion.TryParse(deviceVersion, out var device) || - !FirmwareVersion.TryParse(latestTagName, out var latest)) + var maxAttempts = Math.Max(1, _options.LanChipInfoMaxAttempts); + var retryDelay = _options.LanChipInfoRetryDelay; + var totalTimeout = _options.LanChipInfoTotalTimeout; + + // Wall-clock budget guards against the pathological case where + // attempt-count × per-attempt-timeout + retry-delay sum vastly + // exceeds the configured retry budget (e.g., 3 × 2s device timeout + // + 2 × 2s delay = ~10s while _operationLock is held). Linking + // the caller's CT preserves cancellation semantics; the timeout + // CTS just adds a deadline. + using var timeoutCts = new CancellationTokenSource(totalTimeout); + using var linkedCts = CancellationTokenSource.CreateLinkedTokenSource( + cancellationToken, timeoutCts.Token); + var linkedToken = linkedCts.Token; + + for (var attempt = 1; attempt <= maxAttempts; attempt++) { - return false; + try + { + linkedToken.ThrowIfCancellationRequested(); + } + catch (OperationCanceledException) when (timeoutCts.IsCancellationRequested && !cancellationToken.IsCancellationRequested) + { + _logger.LogDebug( + "LAN chip-info probe hit total timeout ({Timeout}) before attempt {Attempt}/{Max}.", + totalTimeout, + attempt, + maxAttempts); + return null; + } + + try + { + var chipInfo = await lanChipInfoProvider.GetLanChipInfoAsync(linkedToken).ConfigureAwait(false); + if (chipInfo != null) + { + if (attempt > 1) + { + _logger.LogDebug( + "LAN chip-info query succeeded on attempt {Attempt}/{Max}.", + attempt, + maxAttempts); + } + return chipInfo; + } + _logger.LogDebug( + "LAN chip-info query returned null on attempt {Attempt}/{Max}.", + attempt, + maxAttempts); + } + catch (OperationCanceledException) when (timeoutCts.IsCancellationRequested && !cancellationToken.IsCancellationRequested) + { + _logger.LogDebug( + "LAN chip-info probe hit total timeout ({Timeout}) during attempt {Attempt}/{Max}.", + totalTimeout, + attempt, + maxAttempts); + return null; + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + _logger.LogDebug( + ex, + "LAN chip-info query failed on attempt {Attempt}/{Max}.", + attempt, + maxAttempts); + } + + if (attempt < maxAttempts) + { + try + { + await Task.Delay(retryDelay, linkedToken).ConfigureAwait(false); + } + catch (OperationCanceledException) when (timeoutCts.IsCancellationRequested && !cancellationToken.IsCancellationRequested) + { + _logger.LogDebug( + "LAN chip-info probe hit total timeout ({Timeout}) during retry delay after attempt {Attempt}/{Max}.", + totalTimeout, + attempt, + maxAttempts); + return null; + } + } } - return device >= latest; + _logger.LogDebug( + "LAN chip-info query exhausted {Max} attempts; reporting status as ChipInfoUnavailable.", + maxAttempts); + return null; } private ExternalProcessRequest BuildWifiProcessRequest( diff --git a/src/Daqifi.Core/Firmware/FirmwareUpdateServiceOptions.cs b/src/Daqifi.Core/Firmware/FirmwareUpdateServiceOptions.cs index 4b75984..8d920b4 100644 --- a/src/Daqifi.Core/Firmware/FirmwareUpdateServiceOptions.cs +++ b/src/Daqifi.Core/Firmware/FirmwareUpdateServiceOptions.cs @@ -119,6 +119,43 @@ public sealed class FirmwareUpdateServiceOptions /// public string? WifiPortOverride { get; set; } + /// + /// Total attempts (initial + retries) for LAN chip-info queries before + /// the WiFi version decision falls through to "couldn't check, proceed + /// with flash". Right after a PIC32 firmware update the application is + /// typically up while the WiFi subsystem is still finishing startup, so + /// the first chip-info query can transiently fail; bounded retry covers + /// that window so callers don't unnecessarily reflash up-to-date WiFi + /// firmware (closes #144). + /// + /// + /// Each attempt also incurs the per-attempt timeout from the device + /// implementation (e.g., DaqifiStreamingDevice.GetLanChipInfoAsync + /// uses 2s). Total wall-clock budget is therefore + /// sum(attempt durations) + (MaxAttempts-1) * RetryDelay; with the + /// 2s device default, 3 attempts and 2s delay sum to ~10s in the worst + /// case. The retry loop holds _operationLock, so use + /// to cap the actual wall-clock + /// time independent of attempt counts. + /// + public int LanChipInfoMaxAttempts { get; set; } = 3; + + /// + /// Delay between LAN chip-info retry attempts (cancellation-aware). + /// + public TimeSpan LanChipInfoRetryDelay { get; set; } = TimeSpan.FromSeconds(2); + + /// + /// Hard upper bound on wall-clock time spent in the LAN chip-info + /// probe (including per-attempt query timeouts and retry delays). + /// When exceeded, the loop short-circuits to ChipInfoUnavailable + /// regardless of remaining attempts. Prevents pathological multi- + /// attempt cases (e.g., 3 attempts × 2s device timeout + 2 × 2s delays + /// = ~10s) from stalling firmware flows or UI status probes that hold + /// the operation lock. + /// + public TimeSpan LanChipInfoTotalTimeout { get; set; } = TimeSpan.FromSeconds(8); + /// /// Gets the configured timeout for a given firmware update state. /// @@ -176,6 +213,17 @@ public void Validate() "Flash write retry count must be at least 1."); } + if (LanChipInfoMaxAttempts < 1) + { + throw new ArgumentOutOfRangeException( + nameof(LanChipInfoMaxAttempts), + LanChipInfoMaxAttempts, + "LAN chip-info max attempts must be at least 1."); + } + + ValidatePositive(LanChipInfoRetryDelay, nameof(LanChipInfoRetryDelay)); + ValidatePositive(LanChipInfoTotalTimeout, nameof(LanChipInfoTotalTimeout)); + if (BootloaderVendorId < 0 || BootloaderVendorId > 0xFFFF) { throw new ArgumentOutOfRangeException( diff --git a/src/Daqifi.Core/Firmware/IFirmwareUpdateService.cs b/src/Daqifi.Core/Firmware/IFirmwareUpdateService.cs index 46df9df..de57b85 100644 --- a/src/Daqifi.Core/Firmware/IFirmwareUpdateService.cs +++ b/src/Daqifi.Core/Firmware/IFirmwareUpdateService.cs @@ -38,9 +38,36 @@ Task UpdateFirmwareAsync( /// Path to a WiFi tool executable/script or directory containing it. /// Optional progress reporter. /// Cancellation token. + /// + /// When true, bypass the internal version probe and always run the flash + /// flow. Use after a separate + /// call so the device isn't queried twice — see issue #143 for the + /// motivating callsite. + /// + // skipVersionCheck added AFTER cancellationToken (technically violates + // CA1068 "CancellationToken should be last") to avoid breaking source + // compat for any existing positional caller passing CancellationToken + // as the 4th argument. Additivity wins over strict style here. +#pragma warning disable CA1068 Task UpdateWifiModuleAsync( IStreamingDevice device, string firmwarePath, IProgress? progress = null, + CancellationToken cancellationToken = default, + bool skipVersionCheck = false); +#pragma warning restore CA1068 + + /// + /// Probes the device for its current WiFi chip info and looks up the + /// latest WiFi firmware release, returning the comparison result without + /// mutating service state. Lets callers (typically GUI/desktop integrations) + /// surface their own logging, retry policy, or UI before deciding whether + /// to call . Pass + /// skipVersionCheck: true to that call to avoid a second probe. + /// + /// The connected streaming device to inspect. + /// Cancellation token. + Task CheckWifiFirmwareStatusAsync( + IStreamingDevice device, CancellationToken cancellationToken = default); } diff --git a/src/Daqifi.Core/Firmware/WifiFirmwareStatus.cs b/src/Daqifi.Core/Firmware/WifiFirmwareStatus.cs new file mode 100644 index 0000000..7a5cb83 --- /dev/null +++ b/src/Daqifi.Core/Firmware/WifiFirmwareStatus.cs @@ -0,0 +1,67 @@ +namespace Daqifi.Core.Firmware; + +/// +/// Result of : +/// the inputs Core uses to decide whether a WiFi update is needed plus the +/// boolean conclusion. Returned without mutating service state, so callers can +/// inspect, log, retry, or surface UI before deciding to call +/// . +/// +/// +/// When is +/// or , both +/// and are non-null. +/// Other reasons leave one or both null and conservatively report +/// = false so callers default to running the update. +/// +public sealed record WifiFirmwareStatus +{ + /// + /// The current WiFi chip info read from the device, or null if the device + /// did not expose or the query failed. + /// + public LanChipInfo? CurrentChipInfo { get; init; } + + /// + /// The latest WiFi firmware release on GitHub, or null if the lookup + /// failed (e.g. offline, rate-limited). + /// + public FirmwareReleaseInfo? LatestRelease { get; init; } + + /// + /// True only when both versions are available AND the device version is + /// at least the latest release. Any unknown is reported as false so the + /// caller defaults to "needs update". + /// + public required bool IsUpToDate { get; init; } + + /// + /// Why has its current value — lets callers + /// distinguish "definitively up to date" from "couldn't check, assuming not". + /// + public required WifiFirmwareStatusReason Reason { get; init; } +} + +/// +/// Categorical outcome for . +/// +public enum WifiFirmwareStatusReason +{ + /// Device version >= latest release version. + UpToDate, + + /// Device version < latest release version. + UpdateAvailable, + + /// The device does not implement . + DeviceDoesNotSupportLanQuery, + + /// Querying the device for chip info failed. + ChipInfoUnavailable, + + /// Looking up the latest release on GitHub failed. + LatestReleaseUnavailable, + + /// Either version string failed to parse. + VersionUnparseable, +}