From b92809eb6ccfb6d239ad1a20ecc5002e7195c276 Mon Sep 17 00:00:00 2001 From: Tyler Kron Date: Wed, 5 Aug 2026 13:05:59 -0600 Subject: [PATCH 1/3] fix(firmware): stop reflashing supported WiFi modules when the release lookup fails (part of #269) CheckWifiFirmwareStatusAsync compared the module's version against the latest GitHub release. When that lookup failed - offline bench, blocked egress, rate limit - it reported LatestReleaseUnavailable with no version opinion at all, and UpdateWifiModuleAsync's `default:` arm fell through to "flash conservatively". A network outage therefore triggered a multi-minute destructive WINC reflash of a module whose own reported version was already fine. Core now owns the minimum supported WINC firmware (19.7.7, the policy resolved on #269) and answers that question without a network: - FirmwareUpdateServiceOptions.MinimumSupportedWifiFirmwareVersion, defaulting to the new DefaultMinimumSupportedWifiFirmwareVersion const, validated at construction so a typo fails loudly instead of silently disabling the check. - WifiFirmwareStatus.MinimumSupportedVersion / MeetsMinimumSupportedVersion, populated on every result path. The latter is a three-state bool? on purpose: false means "read the device, it is below the bar", null means "could not tell". - The update flow's version gate honors a true verdict and skips the flash. null still proceeds to flash, so an unreadable module is never mistaken for a good one. IsUpToDate keeps its exact latest-release meaning and its existing values, so no current behavior of the status record changes; the new answer sits beside it. Co-Authored-By: Claude Opus 5 --- .../Firmware/FirmwareUpdateServiceTests.cs | 398 +++++++++++++++++- .../Firmware/FirmwareUpdateServiceOptions.cs | 37 ++ .../Firmware/WifiFirmwareStatus.cs | 35 ++ src/Daqifi.Core/Firmware/WifiModuleUpdater.cs | 55 ++- 4 files changed, 522 insertions(+), 3 deletions(-) diff --git a/src/Daqifi.Core.Tests/Firmware/FirmwareUpdateServiceTests.cs b/src/Daqifi.Core.Tests/Firmware/FirmwareUpdateServiceTests.cs index c00aea82..0d65586b 100644 --- a/src/Daqifi.Core.Tests/Firmware/FirmwareUpdateServiceTests.cs +++ b/src/Daqifi.Core.Tests/Firmware/FirmwareUpdateServiceTests.cs @@ -2012,6 +2012,140 @@ public async Task UpdateWifiModuleAsync_WhenDeviceVersionMatchesLatest_SkipsFlas Assert.Contains("already up to date", completeEvent.CurrentOperation, StringComparison.OrdinalIgnoreCase); } + [Fact] + public async Task UpdateWifiModuleAsync_WhenLatestReleaseUnavailableButDeviceMeetsMinimum_SkipsFlash() + { + // The defect this closes: a release lookup failure is a *network* failure, but it + // used to fall through to "flash conservatively" — erasing and reprogramming a WINC + // module whose own reported version was already supported, for eight minutes, because + // GitHub was unreachable. The device answered; that answer is enough. + var downloadService = new FakeFirmwareDownloadService + { + LatestWifiReleaseException = new HttpRequestException("no route to host") + }; + var processRunner = new FakeExternalProcessRunner(); + var device = new FakeLanChipInfoStreamingDevice("COM47", chipInfo: new LanChipInfo + { + ChipId = 1234, + FwVersion = "19.7.7", + BuildDate = "Jan 8 2019" + }); + + var service = new FirmwareUpdateService( + new FakeHidTransport(), + downloadService, + processRunner, + NullLogger.Instance, + new FakeBootloaderProtocol([[0x10]]), + new FakeHidDeviceEnumerator([]), + CreateFastOptions()); + + var progressEvents = new List(); + var progress = new CapturingProgress(progressEvents); + + var firmwareDir = CreateTempDirectory(); + File.WriteAllText(Path.Combine(firmwareDir, "winc_flash_tool.cmd"), "@echo off"); + + try + { + await service.UpdateWifiModuleAsync(device, firmwareDir, progress); + } + finally + { + Directory.Delete(firmwareDir, recursive: true); + } + + // The strongest assertion available: the flash tool was never launched at all. + Assert.Equal(0, processRunner.RunCount); + Assert.DoesNotContain("SYSTem:COMMUnicate:LAN:FWUpdate", device.SentCommands); + + Assert.Equal(FirmwareUpdateState.Complete, service.CurrentState); + var completeEvent = Assert.Single(progressEvents, p => p.State == FirmwareUpdateState.Complete); + Assert.Equal(100, completeEvent.PercentComplete); + Assert.Contains("minimum supported version", completeEvent.CurrentOperation, StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public async Task UpdateWifiModuleAsync_WhenLatestReleaseUnavailableAndDeviceBelowMinimum_ProceedsWithFlash() + { + // The guard on the skip above: it is a verdict, not a blanket "give up and assume + // fine". A module genuinely below the supported minimum still gets flashed even + // though the release lookup failed. + var downloadService = new FakeFirmwareDownloadService { LatestWifiRelease = null }; + var processRunner = new FakeExternalProcessRunner + { + NextResult = new ExternalProcessResult( + 0, false, TimeSpan.Zero, ["Operation completed successfully"], []) + }; + var device = new FakeLanChipInfoStreamingDevice("COM48", chipInfo: new LanChipInfo + { + ChipId = 1234, + FwVersion = "19.5.4", + BuildDate = "Jan 8 2019" + }); + + var service = new FirmwareUpdateService( + new FakeHidTransport(), + downloadService, + processRunner, + NullLogger.Instance, + new FakeBootloaderProtocol([[0x10]]), + new FakeHidDeviceEnumerator([]), + CreateFastOptions()); + + var firmwareDir = CreateTempDirectory(); + File.WriteAllText(Path.Combine(firmwareDir, "winc_flash_tool.cmd"), "@echo off"); + + try + { + await service.UpdateWifiModuleAsync(device, firmwareDir); + } + finally + { + Directory.Delete(firmwareDir, recursive: true); + } + + Assert.True(processRunner.RunCount > 0); + } + + [Fact] + public async Task UpdateWifiModuleAsync_WhenChipInfoUnavailable_StillProceedsWithFlash() + { + // Regression guard for the null case: "could not read the device" must never be + // mistaken for "meets the minimum". This is the path that would silently stop + // flashing broken modules if the three-state verdict were collapsed to a bool. + var downloadService = new FakeFirmwareDownloadService { LatestWifiRelease = null }; + var processRunner = new FakeExternalProcessRunner + { + NextResult = new ExternalProcessResult( + 0, false, TimeSpan.Zero, ["Operation completed successfully"], []) + }; + var device = new FakeLanChipInfoStreamingDevice("COM49", chipInfo: null); + + var service = new FirmwareUpdateService( + new FakeHidTransport(), + downloadService, + processRunner, + NullLogger.Instance, + new FakeBootloaderProtocol([[0x10]]), + new FakeHidDeviceEnumerator([]), + CreateFastOptions()); + + var firmwareDir = CreateTempDirectory(); + File.WriteAllText(Path.Combine(firmwareDir, "winc_flash_tool.cmd"), "@echo off"); + + try + { + await service.UpdateWifiModuleAsync(device, firmwareDir); + } + finally + { + Directory.Delete(firmwareDir, recursive: true); + } + + Assert.True(processRunner.RunCount > 0); + } + [Fact] public async Task UpdateWifiModuleAsync_WhenDeviceVersionIsOlder_ProceedsWithFlash() { @@ -2606,6 +2740,258 @@ public async Task CheckWifiFirmwareStatusAsync_WhenDeviceDoesNotSupportLanQuery_ Assert.Equal(WifiFirmwareStatusReason.DeviceDoesNotSupportLanQuery, status.Reason); Assert.Null(status.CurrentChipInfo); Assert.Null(status.LatestRelease); + + // The bar is a property of Core's policy, not of the device, so it is reported + // even when no device version was ever read. The verdict is not. + Assert.Equal(new FirmwareVersion(19, 7, 7, null, 0), status.MinimumSupportedVersion); + Assert.Null(status.MeetsMinimumSupportedVersion); + } + + [Fact] + public void MinimumSupportedWifiFirmwareVersion_DefaultsToTheFirmwareContractValue() + { + // 19.7.7 is a firmware-contract fact (#269), not a tuning default. Pinned here so a + // change to it is a deliberate edit to this assertion rather than a silent drift that + // would start accepting or rejecting modules in the field. + Assert.Equal("19.7.7", FirmwareUpdateServiceOptions.DefaultMinimumSupportedWifiFirmwareVersion); + Assert.Equal( + FirmwareUpdateServiceOptions.DefaultMinimumSupportedWifiFirmwareVersion, + new FirmwareUpdateServiceOptions().MinimumSupportedWifiFirmwareVersion); + } + + [Theory] + [InlineData("")] + [InlineData(" ")] + [InlineData("not-a-version")] + public void Constructor_WithUnparseableMinimumSupportedWifiFirmwareVersion_Throws(string minimum) + { + // A typo'd minimum must fail loudly at construction. If it were ignored at compare + // time instead, the check would silently degrade to "no version opinion" — exactly + // the state this option exists to remove — and the degradation would be invisible. + var options = CreateFastOptions(); + options.MinimumSupportedWifiFirmwareVersion = minimum; + + var ex = Assert.Throws(() => new FirmwareUpdateService( + new FakeHidTransport(), + new FakeFirmwareDownloadService(), + new FakeExternalProcessRunner(), + NullLogger.Instance, + new FakeBootloaderProtocol([[0xA1, 0x01]]), + new FakeHidDeviceEnumerator([Array.Empty()]), + options)); + + Assert.Equal(nameof(FirmwareUpdateServiceOptions.MinimumSupportedWifiFirmwareVersion), ex.ParamName); + } + + [Fact] + public async Task CheckWifiFirmwareStatusAsync_WhenLatestReleaseLookupReturnsNull_StillAnswersMinimumSupported() + { + // The point of the minimum: a release lookup that fails (offline bench, blocked + // egress, rate limit) leaves IsUpToDate unanswerable, but the device already told + // us its version, so "is this module supported" is still answerable — with no + // network at all. + var device = new FakeLanChipInfoStreamingDevice("COM40", chipInfo: new LanChipInfo + { + ChipId = 1234, + FwVersion = "19.7.7", + BuildDate = "Jan 8 2019" + }); + + var service = new FirmwareUpdateService( + new FakeHidTransport(), + new FakeFirmwareDownloadService { LatestWifiRelease = null }, + new FakeExternalProcessRunner(), + NullLogger.Instance, + new FakeBootloaderProtocol([[0x10]]), + new FakeHidDeviceEnumerator([]), + CreateFastOptions()); + + var status = await service.CheckWifiFirmwareStatusAsync(device); + + // Unchanged: IsUpToDate stays a strictly latest-release answer and stays false. + Assert.False(status.IsUpToDate); + Assert.Equal(WifiFirmwareStatusReason.LatestReleaseUnavailable, status.Reason); + Assert.Null(status.LatestRelease); + + // New: the network-independent verdict. + Assert.True(status.MeetsMinimumSupportedVersion); + Assert.Equal(new FirmwareVersion(19, 7, 7, null, 0), status.MinimumSupportedVersion); + } + + [Fact] + public async Task CheckWifiFirmwareStatusAsync_WhenLatestReleaseLookupThrows_StillAnswersMinimumSupported() + { + // The throwing half of the same failure — a different catch block in the + // implementation, so it needs its own coverage. + var device = new FakeLanChipInfoStreamingDevice("COM41", chipInfo: new LanChipInfo + { + ChipId = 1234, + FwVersion = "19.8.0", + BuildDate = "Jan 8 2019" + }); + + var service = new FirmwareUpdateService( + new FakeHidTransport(), + new FakeFirmwareDownloadService + { + LatestWifiReleaseException = new HttpRequestException("no route to host") + }, + new FakeExternalProcessRunner(), + NullLogger.Instance, + new FakeBootloaderProtocol([[0x10]]), + new FakeHidDeviceEnumerator([]), + CreateFastOptions()); + + var status = await service.CheckWifiFirmwareStatusAsync(device); + + Assert.Equal(WifiFirmwareStatusReason.LatestReleaseUnavailable, status.Reason); + Assert.False(status.IsUpToDate); + Assert.True(status.MeetsMinimumSupportedVersion); + } + + [Fact] + public async Task CheckWifiFirmwareStatusAsync_WhenDeviceIsBelowMinimum_ReportsFalseNotNull() + { + // "Below the minimum" and "could not tell" must stay distinguishable — collapsing + // them is the conflation the three-state property exists to prevent. + var device = new FakeLanChipInfoStreamingDevice("COM42", chipInfo: new LanChipInfo + { + ChipId = 1234, + FwVersion = "19.5.4", + BuildDate = "Jan 8 2019" + }); + + var service = new FirmwareUpdateService( + new FakeHidTransport(), + new FakeFirmwareDownloadService { LatestWifiRelease = null }, + new FakeExternalProcessRunner(), + NullLogger.Instance, + new FakeBootloaderProtocol([[0x10]]), + new FakeHidDeviceEnumerator([]), + CreateFastOptions()); + + var status = await service.CheckWifiFirmwareStatusAsync(device); + + Assert.False(status.MeetsMinimumSupportedVersion); + Assert.NotNull(status.MeetsMinimumSupportedVersion); + } + + [Fact] + public async Task CheckWifiFirmwareStatusAsync_WhenChipInfoUnavailable_LeavesMinimumVerdictUnknown() + { + var device = new FakeLanChipInfoStreamingDevice("COM43", chipInfo: null); + + var service = new FirmwareUpdateService( + new FakeHidTransport(), + new FakeFirmwareDownloadService { LatestWifiRelease = null }, + new FakeExternalProcessRunner(), + NullLogger.Instance, + new FakeBootloaderProtocol([[0x10]]), + new FakeHidDeviceEnumerator([]), + CreateFastOptions()); + + var status = await service.CheckWifiFirmwareStatusAsync(device); + + Assert.Equal(WifiFirmwareStatusReason.ChipInfoUnavailable, status.Reason); + Assert.Null(status.MeetsMinimumSupportedVersion); + Assert.Equal(new FirmwareVersion(19, 7, 7, null, 0), status.MinimumSupportedVersion); + } + + [Fact] + public async Task CheckWifiFirmwareStatusAsync_WhenDeviceVersionUnparseable_LeavesMinimumVerdictUnknown() + { + var wifiRelease = new FirmwareReleaseInfo + { + Version = new FirmwareVersion(19, 7, 7, null, 0), + TagName = "19.7.7", + IsPreRelease = false + }; + var device = new FakeLanChipInfoStreamingDevice("COM44", chipInfo: new LanChipInfo + { + ChipId = 1234, + FwVersion = "garbage", + 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.Equal(WifiFirmwareStatusReason.VersionUnparseable, status.Reason); + Assert.Null(status.MeetsMinimumSupportedVersion); + } + + [Fact] + public async Task CheckWifiFirmwareStatusAsync_UsesTheConfiguredMinimum_NotOnlyTheDefault() + { + // A manufacturing line raising the bar must actually move the verdict; a device that + // passes the shipped default has to fail a higher configured minimum. + var options = CreateFastOptions(); + options.MinimumSupportedWifiFirmwareVersion = "19.9.0"; + + var device = new FakeLanChipInfoStreamingDevice("COM45", chipInfo: new LanChipInfo + { + ChipId = 1234, + FwVersion = "19.7.7", + BuildDate = "Jan 8 2019" + }); + + var service = new FirmwareUpdateService( + new FakeHidTransport(), + new FakeFirmwareDownloadService { LatestWifiRelease = null }, + new FakeExternalProcessRunner(), + NullLogger.Instance, + new FakeBootloaderProtocol([[0x10]]), + new FakeHidDeviceEnumerator([]), + options); + + var status = await service.CheckWifiFirmwareStatusAsync(device); + + Assert.Equal(new FirmwareVersion(19, 9, 0, null, 0), status.MinimumSupportedVersion); + Assert.False(status.MeetsMinimumSupportedVersion); + } + + [Fact] + public async Task CheckWifiFirmwareStatusAsync_WhenLatestReleaseIsKnown_StillReportsTheMinimumVerdict() + { + // The minimum is reported on the happy path too, so a caller can use one property + // consistently instead of switching on Reason to know whether it is populated. + var wifiRelease = new FirmwareReleaseInfo + { + Version = new FirmwareVersion(19, 8, 0, null, 0), + TagName = "19.8.0", + IsPreRelease = false + }; + var device = new FakeLanChipInfoStreamingDevice("COM46", chipInfo: new LanChipInfo + { + ChipId = 1234, + FwVersion = "19.7.7", + 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); + + // Supported, but not the newest — the two questions genuinely disagree here, which + // is the case that motivated modeling them separately. + Assert.Equal(WifiFirmwareStatusReason.UpdateAvailable, status.Reason); + Assert.False(status.IsUpToDate); + Assert.True(status.MeetsMinimumSupportedVersion); } [Fact] @@ -4383,6 +4769,14 @@ private sealed class FakeFirmwareDownloadService : IFirmwareDownloadService { public FirmwareReleaseInfo? LatestWifiRelease { get; set; } + /// + /// When set, faults with this instead of + /// returning . Models the *throwing* half of the + /// release-lookup failure (offline / DNS / rate limit), which reaches a different + /// catch block than the null-return half. + /// + public Exception? LatestWifiReleaseException { get; set; } + public Task GetLatestReleaseAsync(bool includePreRelease = false, CancellationToken cancellationToken = default) { return Task.FromResult(null); @@ -4413,7 +4807,9 @@ public Task CheckForUpdateAsync(string deviceVersionS public Task GetLatestWifiReleaseAsync(CancellationToken cancellationToken = default) { - return Task.FromResult(LatestWifiRelease); + return LatestWifiReleaseException is not null + ? Task.FromException(LatestWifiReleaseException) + : Task.FromResult(LatestWifiRelease); } public void InvalidateCache() diff --git a/src/Daqifi.Core/Firmware/FirmwareUpdateServiceOptions.cs b/src/Daqifi.Core/Firmware/FirmwareUpdateServiceOptions.cs index ea93fb48..2560aff0 100644 --- a/src/Daqifi.Core/Firmware/FirmwareUpdateServiceOptions.cs +++ b/src/Daqifi.Core/Firmware/FirmwareUpdateServiceOptions.cs @@ -299,6 +299,33 @@ public sealed class FirmwareUpdateServiceOptions /// public bool KickLanApplyOnNotInitialized { get; set; } = true; + /// + /// The minimum WINC1500 firmware version DAQiFi supports. This is a firmware-contract + /// fact rather than a tuning knob, so Core owns the value instead of leaving every + /// consumer to hard-code its own copy. + /// + public const string DefaultMinimumSupportedWifiFirmwareVersion = "19.7.7"; + + /// + /// Minimum WINC firmware version treated as supported, used to populate + /// . Defaults to + /// . + /// + /// + /// This exists because "is the module supported" and "is the module newest" are + /// different questions, and only the first can be answered without a network. The + /// latest-release comparison behind needs a + /// GitHub lookup; when that lookup fails (offline bench, rate limit, blocked egress) + /// Core previously had no version opinion at all and callers defaulted to "needs + /// flash", reflashing modules whose reported version was already fine. Comparing + /// against this minimum keeps the check answerable offline. + /// + /// Raising this rejects modules Core would otherwise accept, so it is a policy + /// decision; it is settable so a manufacturing line can raise the bar without waiting + /// on a Core release. Must parse as a . + /// + public string MinimumSupportedWifiFirmwareVersion { get; set; } = DefaultMinimumSupportedWifiFirmwareVersion; + /// /// Gets the configured timeout for a given firmware update state. /// @@ -421,6 +448,16 @@ public void Validate() // toggled on later without re-touching this value. ValidateNonNegative(PowerOnWifiModuleSettleDelay, nameof(PowerOnWifiModuleSettleDelay)); + // Rejected here rather than silently ignored at compare time: a typo'd minimum + // would otherwise degrade the check back to "no version opinion" invisibly, which + // is precisely the failure this option exists to remove. + if (!FirmwareVersion.TryParse(MinimumSupportedWifiFirmwareVersion, out _)) + { + throw new ArgumentException( + $"Minimum supported WiFi firmware version '{MinimumSupportedWifiFirmwareVersion}' is not a parseable version.", + nameof(MinimumSupportedWifiFirmwareVersion)); + } + if (BootloaderVendorId < 0 || BootloaderVendorId > 0xFFFF) { throw new ArgumentOutOfRangeException( diff --git a/src/Daqifi.Core/Firmware/WifiFirmwareStatus.cs b/src/Daqifi.Core/Firmware/WifiFirmwareStatus.cs index 160ed94f..ee0eb9af 100644 --- a/src/Daqifi.Core/Firmware/WifiFirmwareStatus.cs +++ b/src/Daqifi.Core/Firmware/WifiFirmwareStatus.cs @@ -33,8 +33,43 @@ public sealed record WifiFirmwareStatus /// at least the latest release. Any unknown is reported as false so the /// caller defaults to "needs update". /// + /// + /// This is a "newest available" answer and therefore depends on the GitHub + /// lookup succeeding. For the network-independent "is this module supported + /// at all" question — the one a manufacturing or field check actually asks — + /// use . + /// public required bool IsUpToDate { get; init; } + /// + /// The minimum WINC firmware version Core considers supported + /// (), + /// or null if that option could not be parsed. Reported on every result — including + /// the ones where the device could not be read — so a caller can always state the bar + /// it was judging against. + /// + public FirmwareVersion? MinimumSupportedVersion { get; init; } + + /// + /// Whether the device's reported WiFi firmware is at least + /// , or null when that could not be determined + /// (no chip info was read, the device version did not parse, or the configured + /// minimum did not parse). + /// + /// + /// Unlike this needs no network access: it compares the + /// device's own reported version against a firmware-contract constant Core owns. That + /// makes it the right signal for a manufacturing check ("device >= minimum") and the + /// reason an offline or rate-limited GitHub lookup no longer forces a reflash of a + /// module that is demonstrably supported. + /// + /// Deliberately a three-state ?: false means "read the device + /// and it is below the minimum", while null means "could not tell". Collapsing + /// those two into one false is exactly the conflation that made an unreadable + /// device indistinguishable from an outdated one. + /// + public bool? MeetsMinimumSupportedVersion { get; init; } + /// /// Why has its current value — lets callers /// distinguish "definitively up to date" from "couldn't check, assuming not". diff --git a/src/Daqifi.Core/Firmware/WifiModuleUpdater.cs b/src/Daqifi.Core/Firmware/WifiModuleUpdater.cs index cb245d0f..b5c5433e 100644 --- a/src/Daqifi.Core/Firmware/WifiModuleUpdater.cs +++ b/src/Daqifi.Core/Firmware/WifiModuleUpdater.cs @@ -173,12 +173,23 @@ internal async Task CheckStatusAsync( IStreamingDevice device, CancellationToken cancellationToken) { + // Resolved up front so every return path below can state the bar it judged + // against, including the ones that never get to read the device. Parsed + // defensively rather than trusting Validate(): the options object stays mutable + // after the service is constructed, and an unparseable minimum must degrade to + // "no minimum opinion" (null) instead of throwing out of a read-only probe. + FirmwareVersion? minimumSupported = + FirmwareVersion.TryParse(Options.MinimumSupportedWifiFirmwareVersion, out var parsedMinimum) + ? parsedMinimum + : null; + if (device is not ILanChipInfoProvider lanChipInfoProvider) { return new WifiFirmwareStatus { IsUpToDate = false, Reason = WifiFirmwareStatusReason.DeviceDoesNotSupportLanQuery, + MinimumSupportedVersion = minimumSupported, }; } @@ -233,9 +244,17 @@ internal async Task CheckStatusAsync( Reason = wasLanNotInitialized ? WifiFirmwareStatusReason.LanNotInitialized : WifiFirmwareStatusReason.ChipInfoUnavailable, + MinimumSupportedVersion = minimumSupported, }; } + // Parsed once here, before the release lookup, because the minimum-supported + // answer must survive that lookup failing — that is the whole point of it. + var deviceVersionParsed = FirmwareVersion.TryParse(chipInfo.FwVersion, out var deviceVersion); + bool? meetsMinimum = deviceVersionParsed && minimumSupported is { } minimum + ? deviceVersion >= minimum + : null; + FirmwareReleaseInfo? latestWifi; try { @@ -251,6 +270,8 @@ internal async Task CheckStatusAsync( CurrentChipInfo = chipInfo, IsUpToDate = false, Reason = WifiFirmwareStatusReason.LatestReleaseUnavailable, + MinimumSupportedVersion = minimumSupported, + MeetsMinimumSupportedVersion = meetsMinimum, }; } @@ -261,6 +282,8 @@ internal async Task CheckStatusAsync( CurrentChipInfo = chipInfo, IsUpToDate = false, Reason = WifiFirmwareStatusReason.LatestReleaseUnavailable, + MinimumSupportedVersion = minimumSupported, + MeetsMinimumSupportedVersion = meetsMinimum, }; } @@ -268,7 +291,7 @@ internal async Task CheckStatusAsync( // 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)) + if (!deviceVersionParsed) { return new WifiFirmwareStatus { @@ -276,6 +299,8 @@ internal async Task CheckStatusAsync( LatestRelease = latestWifi, IsUpToDate = false, Reason = WifiFirmwareStatusReason.VersionUnparseable, + MinimumSupportedVersion = minimumSupported, + MeetsMinimumSupportedVersion = meetsMinimum, }; } @@ -286,6 +311,8 @@ internal async Task CheckStatusAsync( LatestRelease = latestWifi, IsUpToDate = isCurrent, Reason = isCurrent ? WifiFirmwareStatusReason.UpToDate : WifiFirmwareStatusReason.UpdateAvailable, + MinimumSupportedVersion = minimumSupported, + MeetsMinimumSupportedVersion = meetsMinimum, }; } @@ -321,7 +348,31 @@ private async Task IsWifiFirmwareUpToDateAsync( default: // DeviceDoesNotSupportLanQuery, ChipInfoUnavailable, // LanNotInitialized, LatestReleaseUnavailable, - // VersionUnparseable — proceed with the flash conservatively. + // VersionUnparseable — no latest-release verdict is available. + // + // "Conservative" here used to mean "flash", but a WINC reflash is a + // multi-minute, destructive operation, so flashing on a *network* + // failure is the expensive guess, not the safe one. When the device + // itself answered and its reported version meets the minimum Core + // supports, that is a real verdict reached without the network, so + // honor it and skip the flash. Only LatestReleaseUnavailable can + // reach here with a non-null answer: every other default reason + // means the device version is unknown or unparseable, which leaves + // MeetsMinimumSupportedVersion null and still falls through to the + // flash. A caller that wants to reflash regardless already has + // skipVersionCheck: true. + if (status.MeetsMinimumSupportedVersion == true) + { + var minimumMessage = + $"WiFi firmware meets the minimum supported version (device: {status.CurrentChipInfo!.FwVersion}, " + + $"minimum: {status.MinimumSupportedVersion}); latest-release lookup was unavailable ({status.Reason}), " + + "so skipping the flash rather than reflashing a supported module."; + Logger.LogInformation(minimumMessage); + _context.TransitionToState(FirmwareUpdateState.Complete, minimumMessage); + _context.ReportProgress(progress, FirmwareUpdateState.Complete, 100, minimumMessage, 100, 100); + return true; + } + return false; } } From 0b17b1f0475ea8a59d218290153e519f4412dbae Mon Sep 17 00:00:00 2001 From: Tyler Kron Date: Wed, 5 Aug 2026 17:34:55 -0600 Subject: [PATCH 2/3] docs: log the #434 branch-update fire in SESSION_LOG --- SESSION_LOG.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/SESSION_LOG.md b/SESSION_LOG.md index 8c105f85..d826f1c6 100644 --- a/SESSION_LOG.md +++ b/SESSION_LOG.md @@ -40,3 +40,13 @@ - BENCH (real Nq1, fw 3.7.2, USB, non-destructive) via a scratch harness, because the example CLI surfaces none of these counters. **Session 1 (ch 0,1,2 @200 Hz, 3 s):** `rawFrames=475` and `decodedCh0=475` — every delivered frame reached BOTH consumer paths exactly once (the re-raise multiplicity contract, on hardware); `discarded=1` with `PartialAnalogFrame[an=1/en=3]` — the firmware's malformed leading frame caught by the moved guard and withheld from raw consumers; all 3 channels decoded 475 samples each in ascending order; `decodeFailures=0`, `gaps=0`; every event's `sender` asserted to be the device. **Session 2 (ch 0 only @100 Hz, same instance):** `discarded=0`/`failures=0` (BeginSession reset both, no leftover tripped the gate); `rawFrames=238` vs `decodedCh0=237` — one post-stop frame re-raised but NOT decoded, exactly the `if (!IsStreaming)` branch on hardware; `ch1=ch2=0` decoded, so the disable reached the device and the snapshot the decode maps against is still right. Only channel enable/disable + stream start/stop; no NVM write, no reboot, no SD. - Bench-rig note: the example CLI's `--channels` takes a **bitmask** (`7` = ch 0,1,2), not a comma list, and `--format` accepts only `text|csv|jsonl` (no `json`). - Result: PR #435 opened (base main, part of #344, "not merging — for review"), /agentic_review requested. Now **3 loop PRs awaiting review (#433, #434, #435) — at the concurrency cap**, so the next fire should shepherd only, not start a new ticket. + +## 2026-08-05 — Fire: brought #434 up to date with main, re-validated the merged tree on hardware +- State at start (re-derived from `gh`, not this log): 3 open loop PRs — **#434, #436, #437 — at the concurrency cap**, so priority 4 was out of scope by rule and this fire was shepherd-only. Backlog unchanged at 5 open issues (#344, #333, #271, #269, #183), 4 of them standing skips. +- Priority 1: GraphQL `reviewThreads` shows **0 unresolved threads on all three**; #437's two threads are both resolved. Priority 2: all three already carry ready-notes. Priority 3: CI `build` green on every head SHA. Nothing in 1-3 looked actionable on the first pass. +- The one real gap was in the `mergeStateStatus` field rather than in any review: **#434 was `BEHIND`**, while #436/#437 were `BLOCKED` (the ordinary "needs a code-owner approval" state). `BEHIND` is only reported when the ruleset requires branches to be up to date, so #434 was **not mergeable as it stood** — the user would have hit a required update before being able to merge a PR this loop had already declared ready. That is a shepherding gap, so this fire closed it. +- Checked for conflict before touching it rather than after: main's only new commit (`cf5e757`, the #435 frame-decode extraction) touches `Device/*` + `SESSION_LOG.md`, and #434 touches `Firmware/*` only — **zero file overlap**, `mergeable: MERGEABLE`. Updated via `gh pr update-branch` (server-side, no local checkout in the shared clone — the incident two fires ago stands). `0bd9337..84f68d0`; state went `BEHIND` → `BLOCKED`, i.e. now merge-ready pending the user's approval. +- Re-verified locally rather than trusting CI alone, because merging main in produced a tree that had never been built: FULL suite green net9 + net10 (**2,581 passed, 2 skipped each**) + `Daqifi.Mcp.Tests` 23, 0 warnings/errors. CI `build` then passed on `84f68d0` too. +- BENCH (real Nq1 on the bench USB CDC port, fw 3.7.2, non-destructive — connect/stream/disconnect only; no NVM write, no reboot, no SD): the merged tree is the actual thing worth benching here, since main's `StreamFrameDecoder` refactor and #434's firmware changes had **never run together on hardware**. Example CLI built against this worktree's core (0 warnings), 3 process-level connect → populate → stream → disconnect cycles, **all exit 0**, each reporting `analogIn=16 digital=16 fw=3.7.2` and a stable serial, 30 CSV sample rows each at 20 Hz × 2 s — identical run to run, no drift, no wedge across repeated open/close of the same port. +- #434's **own** change was deliberately not benched, and the reason is specific rather than a shrug: it is about what happens when the **GitHub release lookup fails** — a network condition on the host, not a device condition — and the WINC paths around it can only be exercised by a destructive flash. There is no non-destructive device-side trigger for it, so hardware could not have observed the change either way. +- Result: no source change this fire. #434 updated to `84f68d0`, re-noted (the earlier ready-note pinned `0bd9337`, now stale), `/agentic_review` re-run so Qodo pins the merged head. Not merging. Still **3 loop PRs awaiting review (#434, #436, #437) — at the cap**; the next fire shepherds only until the user merges one. From 3e985253f33c89145af2008f1cd0d5bce731e78c Mon Sep 17 00:00:00 2001 From: Tyler Kron Date: Wed, 5 Aug 2026 18:05:55 -0600 Subject: [PATCH 3/3] chore: drop the loop scratch journal from this branch SESSION_LOG.md is an agent scratch journal, not a project artifact. Every branch that appends to it conflicts every other open PR. Removing the delta here; a follow-up untracks it and adds it to .gitignore. Co-Authored-By: Claude Opus 5 --- SESSION_LOG.md | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/SESSION_LOG.md b/SESSION_LOG.md index 82d75345..d6536126 100644 --- a/SESSION_LOG.md +++ b/SESSION_LOG.md @@ -78,13 +78,3 @@ - Pushing the log entry drew a Qodo finding of its own ("Hardware identifiers committed", `SESSION_LOG.md`): the bench note pinned the unit's **serial number** into a public repo. **Valid and taken** — the serial carried no analytical weight (the point was "the device came back populated", not *which* device), so it is now described rather than quoted. Kept the `/dev/cu.usbmodem*` shape, which `README.md` already documents as the ordinary macOS port example and is not an identifier. The same redaction was applied to the PR comment. **Convention for future fires: bench notes state what the device reported, never its serial.** - CI flake, NOT a regression: the docs-only push failed `DaqifiDeviceOperationSerializationTests.TextExchange_CancelledWhileTheOutboundQueueDrains_DoesNotResubscribeTheConsumer` on **net10 only** — a commit that touched one markdown file cannot regress it. The prior push (`ff2e30b`) failed a *different* net10 timing test (`StreamMessageConsumerStallingReaderTests.Start_WhenStoppedReaderExitsWithinGrace_WaitsAndRestartsSameInstance`). Two different timing tests failing on the same TFM across two runs is runner slowness, not one broken test; both pass locally on net9 + net10. Worth watching: **net10 CI appears to be the flaky lane on this repo**, and if it keeps costing reruns it deserves its own issue rather than a rerun each time. - Result: no source change this fire. #437 ready-note + bench comment posted; log appended to the branch it shepherded, per the convention set by the previous shepherd fire. Still **3 loop PRs awaiting review (#434, #436, #437) — at the cap**; the next fire shepherds only until the user merges one. - -## 2026-08-05 — Fire: brought #434 up to date with main, re-validated the merged tree on hardware -- State at start (re-derived from `gh`, not this log): 3 open loop PRs — **#434, #436, #437 — at the concurrency cap**, so priority 4 was out of scope by rule and this fire was shepherd-only. Backlog unchanged at 5 open issues (#344, #333, #271, #269, #183), 4 of them standing skips. -- Priority 1: GraphQL `reviewThreads` shows **0 unresolved threads on all three**; #437's two threads are both resolved. Priority 2: all three already carry ready-notes. Priority 3: CI `build` green on every head SHA. Nothing in 1-3 looked actionable on the first pass. -- The one real gap was in the `mergeStateStatus` field rather than in any review: **#434 was `BEHIND`**, while #436/#437 were `BLOCKED` (the ordinary "needs a code-owner approval" state). `BEHIND` is only reported when the ruleset requires branches to be up to date, so #434 was **not mergeable as it stood** — the user would have hit a required update before being able to merge a PR this loop had already declared ready. That is a shepherding gap, so this fire closed it. -- Checked for conflict before touching it rather than after: main's only new commit (`cf5e757`, the #435 frame-decode extraction) touches `Device/*` + `SESSION_LOG.md`, and #434 touches `Firmware/*` only — **zero file overlap**, `mergeable: MERGEABLE`. Updated via `gh pr update-branch` (server-side, no local checkout in the shared clone — the incident two fires ago stands). `0bd9337..84f68d0`; state went `BEHIND` → `BLOCKED`, i.e. now merge-ready pending the user's approval. -- Re-verified locally rather than trusting CI alone, because merging main in produced a tree that had never been built: FULL suite green net9 + net10 (**2,581 passed, 2 skipped each**) + `Daqifi.Mcp.Tests` 23, 0 warnings/errors. CI `build` then passed on `84f68d0` too. -- BENCH (real Nq1 on the bench USB CDC port, fw 3.7.2, non-destructive — connect/stream/disconnect only; no NVM write, no reboot, no SD): the merged tree is the actual thing worth benching here, since main's `StreamFrameDecoder` refactor and #434's firmware changes had **never run together on hardware**. Example CLI built against this worktree's core (0 warnings), 3 process-level connect → populate → stream → disconnect cycles, **all exit 0**, each reporting `analogIn=16 digital=16 fw=3.7.2` and a stable serial, 30 CSV sample rows each at 20 Hz × 2 s — identical run to run, no drift, no wedge across repeated open/close of the same port. -- #434's **own** change was deliberately not benched, and the reason is specific rather than a shrug: it is about what happens when the **GitHub release lookup fails** — a network condition on the host, not a device condition — and the WINC paths around it can only be exercised by a destructive flash. There is no non-destructive device-side trigger for it, so hardware could not have observed the change either way. -- Result: no source change this fire. #434 updated to `84f68d0`, re-noted (the earlier ready-note pinned `0bd9337`, now stale), `/agentic_review` re-run so Qodo pins the merged head. Not merging. Still **3 loop PRs awaiting review (#434, #436, #437) — at the cap**; the next fire shepherds only until the user merges one.