fix: skip redundant WiFi version probe during firmware update#552
Open
tylerkron wants to merge 3 commits into
Open
fix: skip redundant WiFi version probe during firmware update#552tylerkron wants to merge 3 commits into
tylerkron wants to merge 3 commits into
Conversation
Consolidates the still-relevant version bumps from Dependabot PRs #533, #536, and #539, which were all failing build-and-test with NU1605 package-downgrade errors. Root cause: Dependabot bumped Daqifi.Core in the DataModel and IO library projects but left the direct reference in Daqifi.Desktop (and, transitively, Daqifi.Desktop.UITest) at 0.21.0. NuGet treats a transitive version that is higher than a direct reference as a downgrade, which is an error by default. Aligning all three direct Daqifi.Core references clears it. Changes: - Daqifi.Core 0.21.0 -> 0.22.0 (DataModel, IO, Daqifi.Desktop) [non-breaking] - Microsoft.NET.Test.Sdk 18.5.1 -> 18.6.0 (all 5 test projects) - Sentry 6.5.0 -> 6.6.0 (Common) The Google.Protobuf 3.34.1 -> 3.35.0 bumps from #533/#536 are dropped: #537 removed all direct Google.Protobuf references (it now resolves transitively via Daqifi.Core). The coverlet.collector and System.Management bumps already landed in #538. Verified locally: dotnet restore on the full solution now exits 0 with no NU1605 errors (restore is where all three PRs failed). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Daqifi.Core 0.22.0 added a fifth optional `bool skipVersionCheck` parameter to IFirmwareUpdateService.UpdateWifiModuleAsync (part of the "WiFi planning split from execution" work). Production calls the method with the default, so it compiles unchanged, but Moq validates a .Callback delegate against the method's full parameter list at setup time, so the four-parameter callback threw at runtime: System.ArgumentException: Invalid callback. Setup on method with parameters (IStreamingDevice, string, IProgress<FirmwareUpdateProgress>, CancellationToken, bool) cannot invoke callback with parameters (IStreamingDevice, string, IProgress<FirmwareUpdateProgress>, CancellationToken). Update the Setup, Callback, and Verify for UpdateWifiModuleAsync to the five-parameter signature (It.IsAny<bool>() for the new arg). UpdateFirmwareAsync is unchanged in 0.22.0, so its mocks are left as-is. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
DaqifiViewModel.UpdateWifiModuleAsync already probes the WiFi firmware version (via ILanChipInfoProvider) to decide whether to skip the update and to surface versions in the UI, returning early when the module is already up to date. It then called Daqifi.Core's UpdateWifiModuleAsync which, as of 0.22.0, performs its own internal version probe by default, so the device was queried twice. Pass skipVersionCheck: true (added in Daqifi.Core 0.22.0) so Core flashes directly without re-probing. This is safe because the desktop only reaches the Core call once it has decided a flash is needed. Tighten the test's Verify to assert the flag is passed as true. Requires the Daqifi.Core 0.22.0 bump from #551 (the parameter does not exist in 0.21.0); stacked on that branch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Review Summary by QodoSkip redundant WiFi version probe during firmware update
WalkthroughsDescription• Skip redundant WiFi version probe during firmware update • Pass skipVersionCheck: true to Core's UpdateWifiModuleAsync • Desktop already probes version before calling Core, eliminating duplicate device queries • Tighten test verification to assert flag is passed correctly Diagramflowchart LR
A["Desktop probes WiFi version"] --> B{"Update needed?"}
B -->|No| C["Return early"]
B -->|Yes| D["Call Core UpdateWifiModuleAsync"]
D --> E["skipVersionCheck: true"]
E --> F["Flash without re-probing"]
File Changes1. Daqifi.Desktop/ViewModels/DaqifiViewModel.cs
|
Contributor
Code Review by Qodo
Context used✅ Compliance rules (platform):
47 rules 1. Always flashes on unknown versions
|
📊 Code Coverage ReportSummarySummary
CoverageDAQiFi - 19.1%
Daqifi.Desktop.Common - 38.1%
Daqifi.Desktop.IO - 100%
Coverage report generated by ReportGenerator • View full report in build artifacts |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
DaqifiViewModel.UpdateWifiModuleAsyncalready probes the WiFi firmware version (viaILanChipInfoProvider) to decide whether to skip the update and to surface the current/target version in the UI — it returns early when the module is already up to date (DaqifiViewModel.cs:803). It then called Core'sUpdateWifiModuleAsync, which — as ofDaqifi.Core 0.22.0— runs its own internal version probe by default, so the device was queried twice per WiFi update.This passes the new
skipVersionCheck: trueparameter so Core flashes directly without re-probing. The 0.22.0 XML docs explicitly recommend this for callers that already did the check ("PassskipVersionCheck: trueto that call to avoid a second probe — see issue #143 for the motivating callsite").Behavioral note (important for review + on-device testing)
The flag is passed unconditionally. The desktop reaches the Core call in four paths, but has only truly "done the version check" in the first:
skipVersionCheck: trueILanChipInfoProvidercoreDeviceeitherIn the two "unsure" paths this removes Core's fallback probe: a user-initiated update will now re-flash the WiFi module even if it happened to already be current, rather than Core short-circuiting. This is idempotent and matches the desktop's logged intent in those paths, so it is considered acceptable — but it is a real behavioral change, and there is no unit coverage for the probe-failure paths.
I do not have a device attached, so this has not been validated against hardware. Per
CLAUDE.md/ Daqifi.Desktop.UITest/README.md, run it through the FlaUI integration gate against a physically connected device, and specifically exercise:Compile + unit tests pass on CI; that does not cover the on-device flash path.
Changes
DaqifiViewModel.cs: passskipVersionCheck: true; update the now-stale comment that said Core also probes.DaqifiViewModelFirmwareUpdateTests.cs: tighten theUpdateWifiModuleAsyncVerifyto assert the flag istrue(theSetup/Callbackstay permissive so they still exercise device routing).Stacking
Stacked on #551 (base branch
deps/nuget-version-alignment) becauseskipVersionCheckonly exists inDaqifi.Core 0.22.0, which #551 introduces — it will not compile against currentmain. After #551 squash-merges and its branch is deleted, GitHub auto-retargets this PR tomain; confirm the diff still shows only the 2 files (rebase ontomainonly if the squash causes divergence).🤖 Generated with Claude Code