fix(wifi): hide DFS channels when DFS disabled + dedup channel parser (#1025, #1038)#1124
fix(wifi): hide DFS channels when DFS disabled + dedup channel parser (#1025, #1038)#1124AustinChangLinksys wants to merge 2 commits into
Conversation
…1025, #1038) when DFS (IEEE 802.11h) was disabled. Firmware leaves them in PossibleChannels regardless of DFS state and TR-181 exposes no DFS-vs-non-DFS field, so the client now classifies and filters DFS channels itself: - WiFi Settings filters in the service (buildWifiNetworks), before computing per-bandwidth lists, so the dropdown and "N channels available" counts agree. - Dashboard threads a per-radio isDfsEnabled flag onto WifiRadioUIModel and the channel dialog filters at display time. single hardened parsePossibleChannels in lib/core/utils/wifi_channel.dart, and converge both services onto it. The shared copy carries the hardened logic (malformed-range guard + "0" sentinel filtering) as the baseline. No-op guard: when the radio's current channel is a DFS channel that the firmware left set while DFS is off, it is filtered out of the option list and the editor opens on Auto. Confirming without moving the selection now compares against the initial dropdown selection (not the radio's stored channel), so it is correctly treated as a no-op and issues no mutation. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
When DFS (IEEE 802.11h) is disabled from the Advanced tab, a radio manually parked on a DFS channel (e.g. 5 GHz ch 100) stays there — SSH-verified that the firmware does not vacate the channel on its own, leaving the radio on an illegal channel with no radar detection. On save, when DFS is being turned off, any radio currently on a manual DFS channel now also gets AutoChannelEnable=true in the same set() call, so the firmware reselects a legal non-DFS channel. Radios already on auto-channel, on non-DFS channels, or on non-5 GHz bands are untouched. Reuses isDfsChannel() from wifi_channel.dart for the classification. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
AustinChangLinksys
left a comment
There was a problem hiding this comment.
🤖 Automated Review — Round 1 · 69eb8f1..e4ac295 (full)
Verdict: 💬 Self-review (comment only) — Austin's own PR; GitHub prohibits self-approve. Full review for reference.
| Conf. | Where | Issue (one-liner) | |
|---|---|---|---|
| 🟢High | wifi_channel_dialog.dart:_onApply() |
[both reviewers] _initialSelected no-op check regression: pre-disabled DFS + SSE-update scenarios both miss mandatory Auto-write |
|
| 🟢High | usp_wifi_settings_service.dart:86-95 vs wifi_channel_dialog.dart:initState vs wifi_radio_ui_model.dart |
DFS filtering at 3 independent sites with different contracts: settings path strips DFS before bandwidth maps; dashboard model stores raw channels + filters only in dialog | |
| 🟡Med | usp_wifi_advanced_service.dart:setIeee80211hEnabled() |
USP set() result not parsed; firmware partial rejection of new AutoChannelEnable writes silently swallowed |
|
| 🟡Med | usp_wifi_advanced_provider.dart:performSave() |
TOCTOU: wifiDataProvider read at save time may be stale vs. state at DFS toggle time |
|
| 🟡Med | wifi_channel.dart:dfsChannels5GHz |
Hardcoded as US/FCC-only, undocumented; silently wrong for ETSI/MIC/Japan regulatory domains | |
| 💡 | 🟢High | usp_wifi_advanced_provider.dart + usp_wifi_data_service.dart:388 + usp_wifi_settings_service.dart:672 |
[both reviewers] _withTrailingDot is 3rd copy of _ensureTrailingDot — deduplicate into shared util |
| 💡 | ⚪Low | wifi_channel.dart:parsePossibleChannels() |
No dedup step; overlapping firmware ranges (e.g. "36-48,36") produce duplicate dropdown entries |
| 💡 | ⚪Low | wifi_channel.dart:filterDfsChannels() |
Condition `(dfsEnabled |
Confidence: 🟢High = code-verified · 🟡Med = located + reasoned, not fully confirmed · ⚪Low = speculative, please double-check.
Items marked [both reviewers] were independently flagged by two agents → higher confidence.
⚠️ Warning Details
W-1 — _initialSelected no-op check regression — 🟢High [both reviewers]
lib/page/dashboard/views/dialogs/wifi_channel_dialog.dart — _onApply() (PR diff)
Scenario A (pre-disabled DFS state — Reviewer A):
// initState (PR diff):
_channels = filterDfsChannels(widget.radio.possibleChannels,
band: widget.radio.band, dfsEnabled: widget.radio.isDfsEnabled);
// If isDfsEnabled=false AND channel=100 (DFS) AND autoChannelEnable=false:
// → _channels does NOT contain 100
// → storedChannelSelectable = false
// → _selected = _autoValue = -1 ; _initialSelected = -1
// _onApply():
if (_selected == _initialSelected) { // -1 == -1 → true
Navigator.of(context).pop(); // returns null — NO write
return;
}If DFS was disabled in a previous session (before this PR's forceAutoChannelPaths logic ran, e.g. factory reset or pre-deployment firmware state) and the radio is still parked on a DFS channel, opening the dialog shows Auto (DFS channel filtered out). Tapping Apply without changing is a no-op → firmware stays with DFS=off, channel=100 (inconsistent). The old _onApply correctly detected this as a change (autoChannel=true != radio.autoChannelEnable=false) and would have written.
Scenario B (mid-dialog SSE model update — Reviewer B):
The old check compared against widget.radio (live). If an SSE event updates radio.channel mid-dialog (e.g. router auto-selects a non-DFS channel), the new _initialSelected-based comparison cannot detect this model change — it may either suppress a needed write or allow a spurious one depending on timing.
Fix (Scenario A): In initState, detect the DFS-forced-auto case:
final _dfsChannelWasForced = !widget.radio.isDfsEnabled &&
!widget.radio.autoChannelEnable &&
isDfsChannel(widget.radio.channel, band: widget.radio.band);In _onApply, bypass the no-op check when _dfsChannelWasForced is true.
W-2 — DFS filtering inconsistency across data paths — 🟢High [single reviewer B]
Three filtering sites with different contracts:
usp_wifi_settings_service.dart:86-95: filters DFS beforecomputeChannelsPerBandwidth→ bothpossibleChannelsand bandwidth maps are DFS-clean.wifi_channel_dialog.dart:initState: filters DFS at dialog open time.WifiRadioUIModel.possibleChannelsstores raw channels.wifi_radio_ui_model.dart:possibleChannelsis unfiltered on the dashboard path.
Any future consumer of WifiRadioUIModel.possibleChannels (tooltip, new feature) gets raw DFS channels regardless of DFS state, silently inconsistent with the settings path.
Fix: Document the contract explicitly on WifiRadioUIModel.possibleChannels. Consider filtering at model construction in usp_wifi_data_service.dart for consistency.
W-3 — setIeee80211hEnabled() USP result not parsed — 🟡Med [single reviewer A]
lib/page/wifi_settings/services/usp_wifi_advanced_service.dart
try {
final params = <String, dynamic>{
for (final path in radioPaths) '${path}IEEE80211hEnabled': enabled,
for (final path in forceAutoChannelPaths)
'${path}AutoChannelEnable': true, // ← new in PR
};
await _usp.set(params); // ← result discarded
} catch (e) {
throw mapUspErrorToServiceError(e);
}Contrast with usp_wifi_settings_service.dart:520-537 which uses UspResultParser.parseSetResult() and throws on UspPartialSuccess. If firmware accepts IEEE80211hEnabled=false but rejects AutoChannelEnable=true writes (partial failure), the UI shows DFS disabled while affected radios stay on DFS channels.
Fix: Parse result with UspResultParser.parseSetResult() and throw on UspPartialSuccess.
W-4 — TOCTOU: stale wifiDataProvider at save time — 🟡Med [single reviewer A]
lib/page/wifi_settings/providers/usp_wifi_advanced_provider.dart — performSave()
forceAutoChannelPaths is built from ref.read(wifiDataProvider) at save time. An SSE event (~500ms debounce) between the DFS toggle and the save could update wifiDataProvider, causing the path list to under-include (radio moved onto DFS channel mid-debounce) or over-include (spurious Auto-force, harmless).
Fix: Snapshot the radio state synchronously in setDfsEnabled() at toggle time before any async path.
W-5 — dfsChannels5GHz regulatory scope undocumented — 🟡Med [single reviewer B]
lib/core/utils/wifi_channel.dart
const Set<int> dfsChannels5GHz = {
52, 56, 60, 64,
100, 104, 108, 112, 116, 120, 124, 128, 132, 136, 140, 144,
};This is the US/FCC UNII-2A/2C set. ETSI adds 120/124/128 to weather-radar restrictions (stricter CAC). MIC (Japan) has different DFS channel assignments entirely. Devices shipped to Japan would show wrong channel filtering.
Fix: Rename to dfsChannels5GhzUsFcc or add a /// doc comment stating // US/FCC (UNII-2A/2C) — extend per regulatory domain if multi-market required.
✅ What looks good
- Core DFS filter logic (
filterDfsChannels,isDfsChannel,parsePossibleChannels): pure functions, well-implemented, comprehensive tests intest/core/utils/wifi_channel_test.dart(channel-0 sentinel, malformed range, inverted range, band gating — all covered). WifiRadioUIModel.isDfsEnabled: Equatable equality correctly derived viaDiagnosticLoggable.namedProps(includesisDfsEnabledautomatically — no explicitpropsoverride needed).- Deduplication: Both private
_parsePossibleChannelscopies (inusp_wifi_data_service.dartandusp_wifi_settings_service.dart) correctly replaced by shared function per PR diff. forceAutoChannelPathslogic inperformSave(): correctly handles the primary case (DFS being disabled NOW with radio on DFS channel); 2.4GHz/auto-channel/non-DFS-channel radios all correctly skipped.- Test coverage:
wifi_channel_test.dart(124 lines), DFS filter tests inusp_wifi_settings_service_test.dart,forceAutoChannelPathsnotifier tests (4 cases), service test — thorough. - Mutation lock discipline:
withLock()preserved inperformSave(). - No hardcoded secrets, no injection vectors, no privilege bypass paths.
Cross-reviewed by two independent agents (security+correctness / architecture+maintainability). Automated — please sanity-check before merge.
Summary
Fixes two related WiFi-channel issues plus a source-level remediation, all on the channel-list build path.
#1025 — DFS channels shown / radio parked on DFS channel when DFS is off
The channel dropdown listed 5 GHz DFS channels (52–64, 100–144) even when DFS (IEEE 802.11h) was disabled. SSH-verified on M60TB FW 1.2.2: firmware leaves DFS channels in
PossibleChannelsregardless of DFS state, and TR-181 exposes no DFS-vs-non-DFS field — so the client classifies and filters them itself.buildWifiNetworks), before computing per-bandwidth lists, so the dropdown and the "N channels available" counts stay consistent.isDfsEnabledflag ontoWifiRadioUIModel; the channel dialog filters at display time.AutoChannelEnable=truein the sameset()call, so the firmware reselects a legal non-DFS channel. End-to-end verified on-device: manual ch 100 + disable DFS → radio moves to ch 36, UI shows Auto.#1038 — deduplicate diverged
_parsePossibleChannelsThe parser was duplicated in two services and had diverged (one hardened, one not). Extracted a single hardened
parsePossibleChannelsintolib/core/utils/wifi_channel.dart(malformed-range guard +"0"sentinel filtering as the baseline) and converged both services onto it.No-op guard
When a radio's current channel is a DFS channel the firmware left set while DFS is off, it is filtered out of the option list and the editor opens on Auto. Confirming without moving the selection now compares against the initial dropdown selection (not the radio's stored channel), so it is correctly a no-op and issues no mutation.
Testing
test/core/utils/wifi_channel_test.dart— parser edge cases + DFS classify/filter.possibleChannels+availableChannelsPerBandwidth;forceAutoChannelPathsin the sameset().dart formatclean,flutter analyzeclean.Closes #1025
Closes #1038