From 3a510af413cca81776b819d36f2934d5506ac0f4 Mon Sep 17 00:00:00 2001 From: Austin Chang Date: Fri, 10 Jul 2026 16:16:41 +0800 Subject: [PATCH 1/3] fix(wifi): hide DFS channels when DFS disabled; dedup channel parser (#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 --- lib/core/utils/wifi_channel.dart | 63 +++++++++ .../_shared/models/wifi_radio_ui_model.dart | 7 + .../views/dialogs/wifi_channel_dialog.dart | 45 ++++--- .../services/usp_wifi_data_service.dart | 37 +----- .../services/usp_wifi_settings_service.dart | 39 ++---- .../views/components/wifi_network_card.dart | 7 +- test/core/utils/wifi_channel_test.dart | 124 ++++++++++++++++++ .../dialogs/wifi_channel_dialog_test.dart | 115 +++++++++++++++- .../services/usp_wifi_data_service_test.dart | 16 ++- .../usp_wifi_settings_service_test.dart | 97 +++++++++++++- 10 files changed, 461 insertions(+), 89 deletions(-) create mode 100644 lib/core/utils/wifi_channel.dart create mode 100644 test/core/utils/wifi_channel_test.dart diff --git a/lib/core/utils/wifi_channel.dart b/lib/core/utils/wifi_channel.dart new file mode 100644 index 000000000..4b8120230 --- /dev/null +++ b/lib/core/utils/wifi_channel.dart @@ -0,0 +1,63 @@ +// Shared WiFi channel helpers: TR-181 PossibleChannels parsing and DFS +// (IEEE 802.11h) channel classification/filtering. Pure functions with no +// Flutter or provider dependencies — safe to import from services and widgets. + +/// Parses a TR-181 `PossibleChannels` string into a sorted list of channel +/// numbers. Handles comma-separated values and range notation. +/// e.g. "1-13,36,40,44,48" → [1,2,3,4,5,6,7,8,9,10,11,12,13,36,40,44,48] +List parsePossibleChannels(String raw) { + if (raw.isEmpty) return const []; + final result = []; + for (final part in raw.split(',')) { + final trimmed = part.trim(); + if (trimmed.contains('-')) { + final bounds = trimmed.split('-'); + // Skip malformed range tokens (e.g. "1-2-3"). + if (bounds.length != 2) continue; + final start = int.tryParse(bounds[0].trim()); + final end = int.tryParse(bounds[1].trim()); + if (start != null && end != null) { + // Inverted ranges (start > end) naturally yield nothing. + for (var i = start; i <= end; i++) { + result.add(i); + } + } + } else { + final ch = int.tryParse(trimmed); + if (ch != null) result.add(ch); + } + } + // Drop non-positive channels: TR-181 PossibleChannels "0" is an auto/any + // sentinel, not a real channel, and channel 0 must never reach the dropdown + // or be sent to firmware. + result.removeWhere((ch) => ch <= 0); + result.sort(); + return result; +} + +/// 5 GHz DFS channels (IEEE 802.11h): UNII-2A (52–64) + UNII-2C (100–144). +/// These are the only channels subject to Dynamic Frequency Selection; 2.4 GHz +/// and 6 GHz channels are never DFS. +const Set dfsChannels5GHz = { + 52, 56, 60, 64, // + 100, 104, 108, 112, 116, 120, 124, 128, 132, 136, 140, 144, +}; + +/// True only for 5 GHz DFS channels. [band] is the normalized band string +/// ("2.4GHz" / "5GHz" / "6GHz"). +bool isDfsChannel(int channel, {required String band}) => + band == '5GHz' && dfsChannels5GHz.contains(channel); + +/// Removes DFS channels when DFS is disabled. Returns [channels] unchanged when +/// DFS is enabled or when the band is not 5 GHz (DFS applies only to 5 GHz). +/// +/// The firmware does not trim `PossibleChannels` by DFS state, and TR-181 +/// exposes no DFS-vs-non-DFS channel field, so the client filters here. +List filterDfsChannels( + List channels, { + required String band, + required bool dfsEnabled, +}) => + (dfsEnabled || band != '5GHz') + ? channels + : channels.where((ch) => !dfsChannels5GHz.contains(ch)).toList(); diff --git a/lib/page/_shared/models/wifi_radio_ui_model.dart b/lib/page/_shared/models/wifi_radio_ui_model.dart index 8b96c8981..fa908ba39 100644 --- a/lib/page/_shared/models/wifi_radio_ui_model.dart +++ b/lib/page/_shared/models/wifi_radio_ui_model.dart @@ -21,6 +21,11 @@ class WifiRadioUIModel extends Equatable with DiagnosticLoggable { /// Empty when the band exposes no manual channels. final List possibleChannels; + /// Per-radio DFS (IEEE 802.11h) enabled state, from + /// `Device.WiFi.Radio.{i}.IEEE80211hEnabled`. When false, 5 GHz DFS channels + /// must be hidden from the channel dropdown. + final bool isDfsEnabled; + /// Access points grouped under this radio. final List accessPoints; @@ -35,6 +40,7 @@ class WifiRadioUIModel extends Equatable with DiagnosticLoggable { required this.channelBandwidth, required this.supportedStandards, this.possibleChannels = const [], + this.isDfsEnabled = false, this.accessPoints = const [], }); @@ -72,6 +78,7 @@ class WifiRadioUIModel extends Equatable with DiagnosticLoggable { 'channelBandwidth': channelBandwidth, 'supportedStandards': supportedStandards, 'possibleChannels': possibleChannels, + 'isDfsEnabled': isDfsEnabled, 'accessPoints': accessPoints, }; } diff --git a/lib/page/dashboard/views/dialogs/wifi_channel_dialog.dart b/lib/page/dashboard/views/dialogs/wifi_channel_dialog.dart index de45f506d..bacf8e6b6 100644 --- a/lib/page/dashboard/views/dialogs/wifi_channel_dialog.dart +++ b/lib/page/dashboard/views/dialogs/wifi_channel_dialog.dart @@ -1,4 +1,5 @@ import 'package:flutter/material.dart'; +import 'package:privacy_gui/core/utils/wifi_channel.dart'; import 'package:privacy_gui/localization/localization_hook.dart'; import 'package:privacy_gui/page/_shared/models/wifi_radio_ui_model.dart'; import 'package:ui_kit_library/ui_kit.dart'; @@ -27,6 +28,13 @@ class _WifiChannelDialogState extends State { /// Currently-selected dropdown value; [_autoValue] means Auto. late int _selected; + /// The value [_selected] held when the dialog opened. Apply is a no-op unless + /// the user moves away from this — compared against the initial *dropdown* + /// selection, not the radio's stored channel, so a stored channel that was + /// filtered out of the list (e.g. a DFS channel while DFS is off, which the + /// firmware leaves in place) is not mistaken for a user change. + late final int _initialSelected; + /// Manual channels available for this radio's band, sorted ascending. late final List _channels; @@ -37,22 +45,22 @@ class _WifiChannelDialogState extends State { @override void initState() { super.initState(); - _channels = widget.radio.possibleChannels; + // Hide 5 GHz DFS channels when DFS (IEEE 802.11h) is disabled — the + // firmware leaves them in PossibleChannels regardless of DFS state. + _channels = filterDfsChannels( + widget.radio.possibleChannels, + band: widget.radio.band, + dfsEnabled: widget.radio.isDfsEnabled, + ); // AC5: a stored channel that is no longer selectable defaults to Auto // (no ghost value is ever shown). final storedChannelSelectable = !widget.radio.autoChannelEnable && _channels.contains(widget.radio.channel); _selected = storedChannelSelectable ? widget.radio.channel : _autoValue; + _initialSelected = _selected; } - /// 5 GHz DFS channels (IEEE 802.11h). Used to annotate options with "· DFS". - static const _dfsChannels5 = { - 52, 56, 60, 64, // - 100, 104, 108, 112, 116, 120, 124, 128, 132, 136, 140, 144, - }; - - bool _isDfs(int channel) => - widget.radio.band == '5GHz' && _dfsChannels5.contains(channel); + bool _isDfs(int channel) => isDfsChannel(channel, band: widget.radio.band); String _labelFor(int value) { if (value == _autoValue) return loc(context).channelAutoRecommended; @@ -138,20 +146,21 @@ class _WifiChannelDialogState extends State { } void _onApply() { + // AC4: if the user did not move the dropdown from where it opened, Apply is + // a no-op — return null so the caller issues no mutation. Comparing against + // the initial dropdown selection (not the radio's stored channel) means an + // unselectable stored channel — e.g. a DFS channel the firmware left set + // while DFS is off — does not read as a user change and trigger a write. + if (_selected == _initialSelected) { + Navigator.of(context).pop(); + return; + } + final autoChannel = _autoChannel; // When Auto is selected the concrete channel is irrelevant to firmware; // keep the existing value so the returned record is stable. final channel = autoChannel ? widget.radio.channel : _selected; - // AC4: selection equal to the stored value is a no-op — return null so the - // caller issues no mutation. - final unchanged = autoChannel == widget.radio.autoChannelEnable && - (autoChannel || channel == widget.radio.channel); - if (unchanged) { - Navigator.of(context).pop(); - return; - } - Navigator.of(context).pop((channel: channel, autoChannel: autoChannel)); } } diff --git a/lib/page/wifi_settings/services/usp_wifi_data_service.dart b/lib/page/wifi_settings/services/usp_wifi_data_service.dart index 67247a597..b5b292012 100644 --- a/lib/page/wifi_settings/services/usp_wifi_data_service.dart +++ b/lib/page/wifi_settings/services/usp_wifi_data_service.dart @@ -9,6 +9,7 @@ import 'package:privacy_gui/generated/wifi_clients.g.dart'; import 'package:privacy_gui/core/usp/providers/usp_client_provider.dart'; import 'package:privacy_gui/core/usp/services/usp_client.dart'; import 'package:privacy_gui/core/utils/logger.dart'; +import 'package:privacy_gui/core/utils/wifi_channel.dart'; import 'package:privacy_gui/page/_shared/models/client_connection_detail.dart'; import 'package:privacy_gui/page/_shared/utils/wifi_guest_detection.dart'; import 'package:privacy_gui/page/_shared/models/wifi_client_ui_model.dart'; @@ -216,7 +217,8 @@ class UspWifiDataService { autoChannelEnable: radio.autoChannelEnable, channelBandwidth: radio.operatingChannelBandwidth, supportedStandards: radio.supportedStandards, - possibleChannels: _parsePossibleChannels(radio.possibleChannels), + possibleChannels: parsePossibleChannels(radio.possibleChannels), + isDfsEnabled: radio.ieee80211hEnabled, accessPoints: apModels, ); }).toList(); @@ -398,39 +400,6 @@ class UspWifiDataService { return rawBand; } - /// Parses a TR-181 `PossibleChannels` string into a sorted list of channel - /// numbers. Handles comma-separated values and range notation. - /// e.g. "1-13,36,40,44,48" → [1,2,3,4,5,6,7,8,9,10,11,12,13,36,40,44,48] - static List _parsePossibleChannels(String raw) { - if (raw.isEmpty) return const []; - final result = []; - for (final part in raw.split(',')) { - final trimmed = part.trim(); - if (trimmed.contains('-')) { - final bounds = trimmed.split('-'); - // Skip malformed range tokens (e.g. "1-2-3"). - if (bounds.length != 2) continue; - final start = int.tryParse(bounds[0].trim()); - final end = int.tryParse(bounds[1].trim()); - if (start != null && end != null) { - // Inverted ranges (start > end) naturally yield nothing. - for (var i = start; i <= end; i++) { - result.add(i); - } - } - } else { - final ch = int.tryParse(trimmed); - if (ch != null) result.add(ch); - } - } - // Drop non-positive channels: TR-181 PossibleChannels "0" is an - // auto/any sentinel, not a real channel, and channel 0 must never - // reach the dropdown or be sent to firmware. - result.removeWhere((ch) => ch <= 0); - result.sort(); - return result; - } - /// Builds a BSSID → band mapping from WiFi SSID and Radio data. /// /// Used by [MeshTopologyBuilder] to determine band for clients on slave nodes diff --git a/lib/page/wifi_settings/services/usp_wifi_settings_service.dart b/lib/page/wifi_settings/services/usp_wifi_settings_service.dart index bfe79f5af..ac9c4e449 100644 --- a/lib/page/wifi_settings/services/usp_wifi_settings_service.dart +++ b/lib/page/wifi_settings/services/usp_wifi_settings_service.dart @@ -1,6 +1,7 @@ import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:privacy_gui/core/errors/service_error.dart'; import 'package:privacy_gui/core/utils/logger.dart'; +import 'package:privacy_gui/core/utils/wifi_channel.dart'; import 'package:privacy_gui/core/usp/errors/usp_error.dart'; import 'package:privacy_gui/generated/wi_fi_access_points.g.dart'; import 'package:privacy_gui/generated/wi_fi_radios.g.dart'; @@ -83,8 +84,16 @@ class UspWifiSettingsService { final supportedModes = _parseModesSupported(ap?.modesSupported ?? ''); final band = _normalizeBand(radio?.operatingFrequencyBand ?? ''); - final possibleChannels = - _parsePossibleChannels(radio?.possibleChannels ?? ''); + // DFS (IEEE 802.11h) channels must not appear when DFS is disabled. The + // firmware leaves them in PossibleChannels regardless, so filter here — + // before computing per-bandwidth lists — so both the dropdown and the + // "N channels available" counts stay consistent. + final dfsEnabled = radio?.ieee80211hEnabled ?? false; + final possibleChannels = filterDfsChannels( + parsePossibleChannels(radio?.possibleChannels ?? ''), + band: band, + dfsEnabled: dfsEnabled, + ); final supportedBandwidths = _parseSupportedBandwidths( radio?.supportedOperatingChannelBandwidths ?? ''); @@ -642,32 +651,6 @@ List _parseModesSupported(String raw) { .toList(); } -/// Parses a TR-181 PossibleChannels string into a sorted list of channel numbers. -/// Handles both comma-separated values and range notation. -/// e.g. "1-13,36,40,44,48" → [1,2,3,4,5,6,7,8,9,10,11,12,13,36,40,44,48] -List _parsePossibleChannels(String raw) { - if (raw.isEmpty) return []; - final result = []; - for (final part in raw.split(',')) { - final trimmed = part.trim(); - if (trimmed.contains('-')) { - final bounds = trimmed.split('-'); - final start = int.tryParse(bounds[0].trim()); - final end = int.tryParse(bounds[1].trim()); - if (start != null && end != null) { - for (var i = start; i <= end; i++) { - result.add(i); - } - } - } else { - final ch = int.tryParse(trimmed); - if (ch != null) result.add(ch); - } - } - result.sort(); - return result; -} - /// Ensures a TR-181 path ends with a dot. String _ensureTrailingDot(String path) { if (path.isEmpty) return path; diff --git a/lib/page/wifi_settings/views/components/wifi_network_card.dart b/lib/page/wifi_settings/views/components/wifi_network_card.dart index 18119fd0d..c9782248c 100644 --- a/lib/page/wifi_settings/views/components/wifi_network_card.dart +++ b/lib/page/wifi_settings/views/components/wifi_network_card.dart @@ -465,6 +465,11 @@ class WifiNetworkCard extends ConsumerWidget { String selected = channelItems.any((e) => e.value == currentLabel) ? currentLabel : autoLabel; + // Baseline for the no-op check: the selection the dialog opens on. A stored + // channel that isn't selectable — e.g. a DFS channel the firmware left set + // while DFS is off — opens as Auto, and confirming without moving must NOT + // be treated as a change. + final initialSelected = selected; final result = await showSimpleAppDialog( context, @@ -485,7 +490,7 @@ class WifiNetworkCard extends ConsumerWidget { label: loc(context).ok, onTap: () => context.pop(selected)), ], ); - if (result != null && result != currentLabel && context.mounted) { + if (result != null && result != initialSelected && context.mounted) { if (result == autoLabel) { ref.read(uspWifiSettingsProvider.notifier).updateNetworkField( ssidInstancePath, diff --git a/test/core/utils/wifi_channel_test.dart b/test/core/utils/wifi_channel_test.dart new file mode 100644 index 000000000..b95831ed9 --- /dev/null +++ b/test/core/utils/wifi_channel_test.dart @@ -0,0 +1,124 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:privacy_gui/core/utils/wifi_channel.dart'; + +void main() { + // --------------------------------------------------------------------------- + // parsePossibleChannels — range notation, sentinels, malformed tokens + // --------------------------------------------------------------------------- + + group('parsePossibleChannels', () { + test('empty string returns empty list', () { + expect(parsePossibleChannels(''), isEmpty); + }); + + test('parses comma-separated single values ("1,6,11")', () { + expect(parsePossibleChannels('1,6,11'), [1, 6, 11]); + }); + + test('expands mixed range + single notation ("1-3,6")', () { + expect(parsePossibleChannels('1-3,6'), [1, 2, 3, 6]); + }); + + test('expands full range notation ("1-13")', () { + expect( + parsePossibleChannels('1-13'), + [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13], + ); + }); + + test('sorts unordered input', () { + expect(parsePossibleChannels('11,1,6'), [1, 6, 11]); + }); + + test('inverted range ("11-1") degrades to empty without throwing', () { + expect(parsePossibleChannels('11-1'), isEmpty); + }); + + test('filters out TR-181 "0" auto/any sentinel ("0,1,6,11")', () { + expect(parsePossibleChannels('0,1,6,11'), [1, 6, 11]); + }); + + test('drops non-positive channels from a range ("0-2")', () { + expect(parsePossibleChannels('0-2'), [1, 2]); + }); + + test('skips malformed range token ("1-2-3") without throwing', () { + expect(parsePossibleChannels('1-2-3'), isEmpty); + }); + + test('ignores whitespace around tokens (" 1 , 6 , 11 ")', () { + expect(parsePossibleChannels(' 1 , 6 , 11 '), [1, 6, 11]); + }); + }); + + // --------------------------------------------------------------------------- + // isDfsChannel — 5 GHz DFS classification + // --------------------------------------------------------------------------- + + group('isDfsChannel', () { + test('5 GHz DFS channels are DFS (52, 64, 100, 144)', () { + for (final ch in [52, 56, 60, 64, 100, 140, 144]) { + expect(isDfsChannel(ch, band: '5GHz'), isTrue, reason: 'channel $ch'); + } + }); + + test('5 GHz non-DFS channels are not DFS (36, 40, 44, 48, 149)', () { + for (final ch in [36, 40, 44, 48, 149]) { + expect(isDfsChannel(ch, band: '5GHz'), isFalse, reason: 'channel $ch'); + } + }); + + test('2.4 GHz channels are never DFS', () { + for (final ch in [1, 6, 11, 52, 100]) { + expect(isDfsChannel(ch, band: '2.4GHz'), isFalse); + } + }); + + test('6 GHz channels are never DFS', () { + expect(isDfsChannel(52, band: '6GHz'), isFalse); + expect(isDfsChannel(100, band: '6GHz'), isFalse); + }); + }); + + // --------------------------------------------------------------------------- + // filterDfsChannels — hide DFS channels when DFS disabled + // --------------------------------------------------------------------------- + + group('filterDfsChannels', () { + const fiveGhzAll = [ + 36, 40, 44, 48, // + 52, 56, 60, 64, // + 100, 104, 108, 112, 116, 120, 124, 128, 132, 136, 140, + ]; + + test('DFS disabled on 5 GHz drops DFS channels', () { + expect( + filterDfsChannels(fiveGhzAll, band: '5GHz', dfsEnabled: false), + [36, 40, 44, 48], + ); + }); + + test('DFS enabled on 5 GHz keeps all channels', () { + expect( + filterDfsChannels(fiveGhzAll, band: '5GHz', dfsEnabled: true), + fiveGhzAll, + ); + }); + + test('2.4 GHz is untouched regardless of DFS state', () { + const ch = [1, 6, 11]; + expect(filterDfsChannels(ch, band: '2.4GHz', dfsEnabled: false), ch); + expect(filterDfsChannels(ch, band: '2.4GHz', dfsEnabled: true), ch); + }); + + test('6 GHz is untouched regardless of DFS state', () { + const ch = [1, 5, 9, 213]; + expect(filterDfsChannels(ch, band: '6GHz', dfsEnabled: false), ch); + }); + + test('empty list stays empty', () { + expect(filterDfsChannels(const [], band: '5GHz', dfsEnabled: false), + isEmpty); + }); + }); +} diff --git a/test/page/dashboard/views/dialogs/wifi_channel_dialog_test.dart b/test/page/dashboard/views/dialogs/wifi_channel_dialog_test.dart index 9d6ef64f6..63df54b61 100644 --- a/test/page/dashboard/views/dialogs/wifi_channel_dialog_test.dart +++ b/test/page/dashboard/views/dialogs/wifi_channel_dialog_test.dart @@ -18,6 +18,7 @@ WifiRadioUIModel _radio({ String band = '5GHz', int channel = 36, bool autoChannelEnable = false, + bool isDfsEnabled = true, List possibleChannels = const [36, 40, 44, 48, 52, 149], }) { return WifiRadioUIModel( @@ -31,6 +32,7 @@ WifiRadioUIModel _radio({ channelBandwidth: '80MHz', supportedStandards: 'ax', possibleChannels: possibleChannels, + isDfsEnabled: isDfsEnabled, ); } @@ -138,8 +140,9 @@ void main() { expect(captured!.autoChannel, isTrue); }); - testWidgets('AC5: stored channel not in possibleChannels defaults to Auto', - (t) async { + testWidgets( + 'AC5: stored channel not in possibleChannels displays as Auto, and ' + 'confirming without moving is a no-op', (t) async { ({int channel, bool autoChannel})? captured; var called = false; await t.pumpWidget(host( @@ -162,10 +165,10 @@ void main() { await t.tap(find.text('Apply')); await t.pumpAndSettle(); - // Radio was NOT auto originally, now shows Auto -> this is a real change. + // The dialog opened on Auto because 165 is unselectable; the user did not + // move the selection, so Apply must NOT rewrite the radio to Auto. expect(called, isTrue); - expect(captured, isNotNull); - expect(captured!.autoChannel, isTrue); + expect(captured, isNull); }); testWidgets( @@ -236,6 +239,108 @@ void main() { expect(dd.itemAsString!(36), '36'); }); + testWidgets('#1025: DFS disabled hides 5GHz DFS channels from the dropdown', + (t) async { + await t.pumpWidget(host( + _radio( + band: '5GHz', + channel: 36, + autoChannelEnable: false, + isDfsEnabled: false, + possibleChannels: const [36, 40, 44, 48, 52, 100, 149], + ), + (_) {}, + )); + await openDialog(t); + + final dd = t.widget>(find.byType(AppDropdown)); + // -1 is the Auto sentinel; DFS channels 52 and 100 must be gone. + expect(dd.items, [-1, 36, 40, 44, 48, 149]); + }); + + testWidgets('#1025: DFS enabled keeps 5GHz DFS channels in the dropdown', + (t) async { + await t.pumpWidget(host( + _radio( + band: '5GHz', + channel: 36, + autoChannelEnable: false, + isDfsEnabled: true, + possibleChannels: const [36, 40, 44, 48, 52, 100, 149], + ), + (_) {}, + )); + await openDialog(t); + + final dd = t.widget>(find.byType(AppDropdown)); + expect(dd.items, [-1, 36, 40, 44, 48, 52, 100, 149]); + }); + + testWidgets( + '#1025: radio stuck on a DFS channel with DFS off — confirming without ' + 'moving does not rewrite to Auto', (t) async { + // SSH-confirmed firmware behaviour: disabling DFS leaves the radio on its + // manual DFS channel (e.g. 100). The dialog can only show Auto since 100 + // is filtered out, but merely opening and confirming must not mutate. + ({int channel, bool autoChannel})? captured; + var called = false; + await t.pumpWidget(host( + _radio( + band: '5GHz', + channel: 100, + autoChannelEnable: false, + isDfsEnabled: false, + possibleChannels: const [36, 40, 44, 48, 52, 100, 149], + ), + (r) { + captured = r; + called = true; + }, + )); + await openDialog(t); + + // 100 is filtered out, so the dialog opens on Auto. + final sw = t.widget(find.byType(AppSwitch)); + expect(sw.value, isTrue); + + await t.tap(find.text('Apply')); + await t.pumpAndSettle(); + + // No user interaction → no mutation. + expect(called, isTrue); + expect(captured, isNull); + }); + + testWidgets( + '#1025: after DFS-off fallback to Auto, actively picking a channel ' + 'still writes', (t) async { + // Guard against over-suppression: the no-op check must not block a real + // user selection made after the Auto fallback. + ({int channel, bool autoChannel})? captured; + await t.pumpWidget(host( + _radio( + band: '5GHz', + channel: 100, + autoChannelEnable: false, + isDfsEnabled: false, + possibleChannels: const [36, 40, 44, 48, 52, 100, 149], + ), + (r) => captured = r, + )); + await openDialog(t); + + // Turn Auto OFF, which selects the first manual channel (36). + await t.tap(find.byType(AppSwitch)); + await t.pumpAndSettle(); + + await t.tap(find.text('Apply')); + await t.pumpAndSettle(); + + expect(captured, isNotNull); + expect(captured!.autoChannel, isFalse); + expect(captured!.channel, 36); + }); + // Fix (#1023): UI-kit v2.26.1 gates the AppDropdown tap gesture when // onChanged is null (app_dropdown.dart:138,183), so passing a null // onChanged genuinely blocks interaction — no consumer-side IgnorePointer diff --git a/test/page/wifi_settings/services/usp_wifi_data_service_test.dart b/test/page/wifi_settings/services/usp_wifi_data_service_test.dart index cb1964bef..b5384297d 100644 --- a/test/page/wifi_settings/services/usp_wifi_data_service_test.dart +++ b/test/page/wifi_settings/services/usp_wifi_data_service_test.dart @@ -117,7 +117,7 @@ void _stubAllFetches(MockUspClient mockUsp) { } /// Stubs fetches for a single 5 GHz radio whose `PossibleChannels` value is -/// [possibleChannels]. Used to exercise `_parsePossibleChannels` (private) via +/// [possibleChannels]. Used to exercise the shared `parsePossibleChannels` via /// the public `fetch()` entry point. void _stubRadioWithPossibleChannels( MockUspClient mockUsp, @@ -234,6 +234,18 @@ void main() { expect(radio2.maxBitRate, 2400); }); + test('threads IEEE80211hEnabled into radio model isDfsEnabled', () async { + _stubAllFetches(mockUsp); + + final result = await svc.fetch(); + + // Both stub radios have IEEE80211hEnabled = false. The dashboard radio + // model carries the per-radio DFS flag verbatim (channel filtering happens + // in the channel dialog, not here). + expect(result.radioModels[0].isDfsEnabled, isFalse); + expect(result.radioModels[1].isDfsEnabled, isFalse); + }); + test('AC7: enriches possibleChannels from PossibleChannels at fetch time', () async { _stubAllFetches(mockUsp); @@ -259,7 +271,7 @@ void main() { // ------------------------------------------------------------------------- // PossibleChannels parsing — range notation, sentinels, malformed tokens - // (W-3 / W-4). Exercises the private _parsePossibleChannels via fetch(). + // (W-3 / W-4). Exercises the shared parsePossibleChannels via fetch(). // ------------------------------------------------------------------------- group('PossibleChannels parsing', () { diff --git a/test/page/wifi_settings/services/usp_wifi_settings_service_test.dart b/test/page/wifi_settings/services/usp_wifi_settings_service_test.dart index 6da8e59ef..eb166cffa 100644 --- a/test/page/wifi_settings/services/usp_wifi_settings_service_test.dart +++ b/test/page/wifi_settings/services/usp_wifi_settings_service_test.dart @@ -146,7 +146,9 @@ void main() { transmitPower: 100, maxBitRate: 2402, autoChannelEnable: false, - ieee80211hEnabled: false, + // DFS enabled so DFS channels (52–64) survive filtering — this test + // exercises bonding-group rules, not DFS filtering. + ieee80211hEnabled: true, supportedOperatingChannelBandwidths: 'Auto,20MHz,40MHz,80MHz,160MHz', ), ]); @@ -176,6 +178,99 @@ void main() { expect(bwMap['160MHz'], [36, 40, 44, 48, 52, 56, 60, 64]); }); + // ----------------------------------------------------------------------- + // DFS channel filtering (#1025). When IEEE80211hEnabled is false, 5 GHz + // DFS channels (52–64, 100–144) must be stripped from BOTH possibleChannels + // and availableChannelsPerBandwidth so the dropdown and the "N channels + // available" counts stay consistent. + // ----------------------------------------------------------------------- + + WiFiSsids singleSsid() => WiFiSsids(items: [ + WiFiSsid( + instancePath: 'Device.WiFi.SSID.1.', + ssid: 'DfsNet', + enable: true, + status: 'Up', + bssid: 'AA:BB:CC:DD:EE:FF', + lowerLayers: 'Device.WiFi.Radio.1.', + ), + ]); + + WiFiAccessPoints singleAp() => WiFiAccessPoints(items: [ + WiFiAccessPoint( + instancePath: 'Device.WiFi.AccessPoint.1.', + enable: true, + status: 'Enabled', + modesSupported: 'WPA2-Personal', + securityModeEnabled: 'WPA2-Personal', + encryptionMode: 'AES', + keyPassphrase: 'pass', + ssidAdvertisementEnabled: true, + ssidReference: 'Device.WiFi.SSID.1.', + ), + ]); + + WiFiRadios fiveGhzRadio({required bool dfsEnabled}) => WiFiRadios(items: [ + WiFiRadio( + instancePath: 'Device.WiFi.Radio.1.', + enable: true, + status: 'Up', + channel: 36, + operatingFrequencyBand: '5GHz', + operatingChannelBandwidth: '80MHz', + possibleChannels: '36,40,44,48,52,56,60,64,100,104,108,112', + operatingStandards: 'ax', + supportedStandards: 'a,n,ac,ax', + transmitPower: 100, + maxBitRate: 2402, + autoChannelEnable: false, + ieee80211hEnabled: dfsEnabled, + supportedOperatingChannelBandwidths: 'Auto,20MHz,40MHz,80MHz', + ), + ]); + + test('DFS disabled on 5 GHz strips DFS channels from possibleChannels', () { + final networks = svc.buildWifiNetworks( + ssids: singleSsid(), + accessPoints: singleAp(), + radios: fiveGhzRadio(dfsEnabled: false), + ); + + // Only non-DFS UNII-1 channels remain. + expect(networks.first.possibleChannels, [36, 40, 44, 48]); + }); + + test('DFS disabled on 5 GHz strips DFS from availableChannelsPerBandwidth', + () { + final networks = svc.buildWifiNetworks( + ssids: singleSsid(), + accessPoints: singleAp(), + radios: fiveGhzRadio(dfsEnabled: false), + ); + + final bwMap = networks.first.availableChannelsPerBandwidth; + expect(bwMap['Auto'], [36, 40, 44, 48]); + expect(bwMap['20MHz'], [36, 40, 44, 48]); + // No DFS channel should appear under any bandwidth. + for (final channels in bwMap.values) { + expect(channels.any((c) => c >= 52), isFalse, + reason: 'DFS channel leaked into bandwidth map'); + } + }); + + test('DFS enabled on 5 GHz retains DFS channels', () { + final networks = svc.buildWifiNetworks( + ssids: singleSsid(), + accessPoints: singleAp(), + radios: fiveGhzRadio(dfsEnabled: true), + ); + + expect( + networks.first.possibleChannels, + [36, 40, 44, 48, 52, 56, 60, 64, 100, 104, 108, 112], + ); + }); + test('empty supportedOperatingChannelBandwidths falls back to defaults', () { final ssids = WiFiSsids(items: [ From e4ac295665643d9ac3fb8d96de76bc028c4b3b54 Mon Sep 17 00:00:00 2001 From: Austin Chang Date: Mon, 13 Jul 2026 15:45:43 +0800 Subject: [PATCH 2/3] fix(wifi): move radios off DFS channel when DFS is disabled (#1025) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../providers/usp_wifi_advanced_provider.dart | 35 +++- .../services/usp_wifi_advanced_service.dart | 9 ++ .../usp_wifi_advanced_notifier_test.dart | 152 +++++++++++++++++- .../usp_wifi_advanced_service_test.dart | 31 ++++ 4 files changed, 223 insertions(+), 4 deletions(-) diff --git a/lib/page/wifi_settings/providers/usp_wifi_advanced_provider.dart b/lib/page/wifi_settings/providers/usp_wifi_advanced_provider.dart index 16a3db638..0220d3989 100644 --- a/lib/page/wifi_settings/providers/usp_wifi_advanced_provider.dart +++ b/lib/page/wifi_settings/providers/usp_wifi_advanced_provider.dart @@ -1,6 +1,7 @@ import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:privacy_gui/core/errors/service_error.dart'; import 'package:privacy_gui/core/utils/logger.dart'; +import 'package:privacy_gui/core/utils/wifi_channel.dart'; import 'package:privacy_gui/core/usp/providers/usp_mutation_lock.dart'; import 'package:privacy_gui/framework/preservable_contract.dart'; import 'package:privacy_gui/framework/preservable_notifier_mixin.dart'; @@ -106,15 +107,39 @@ class UspWifiAdvancedNotifier final radioPaths = current.ieee80211hByRadio.keys.toList(); final enabled = current.isDfsEnabled; + // When DFS is being disabled, any radio parked on a DFS channel must be + // moved off it — the firmware leaves the channel set on its own + // (SSH-verified). Force AutoChannelEnable on those radios so the firmware + // reselects a legal non-DFS channel. Only radios being turned off and + // currently sitting on a manual DFS channel are affected. + final forceAutoChannelPaths = []; + if (!enabled) { + final radios = ref.read(wifiDataProvider).valueOrNull?.radioModels ?? []; + final radioByPath = { + for (final r in radios) _withTrailingDot(r.instancePath): r, + }; + for (final path in radioPaths) { + // Radios staying on DFS need no channel remediation. + if (current.ieee80211hByRadio[path] == true) continue; + final radio = radioByPath[_withTrailingDot(path)]; + if (radio == null || radio.autoChannelEnable) continue; + if (isDfsChannel(radio.channel, band: radio.band)) { + forceAutoChannelPaths.add(path); + } + } + } + await ref.read(uspMutationLockProvider).withLock(() async { await _svc.setIeee80211hEnabled( radioPaths: radioPaths, enabled: enabled, + forceAutoChannelPaths: forceAutoChannelPaths, ); }); logger.d('[USP][WiFi][Advanced]: Save succeeded — ' - 'radios=${radioPaths.length}, enabled=$enabled'); + 'radios=${radioPaths.length}, enabled=$enabled, ' + 'forcedAutoChannel=${forceAutoChannelPaths.length}'); // Refresh Layer 1 cache so post-save fetch() reads fresh data. // Using refresh() instead of invalidate() because the latter only marks // the provider dirty — without an active subscriber it won't rebuild, @@ -155,3 +180,11 @@ class UspWifiAdvancedNotifier ); } } + +/// Normalizes a TR-181 instance path to a trailing-dot form so that radio +/// paths from [WifiAdvancedSettings.ieee80211hByRadio] and from +/// [WifiRadioUIModel.instancePath] compare equal regardless of source format. +String _withTrailingDot(String path) { + if (path.isEmpty) return path; + return path.endsWith('.') ? path : '$path.'; +} diff --git a/lib/page/wifi_settings/services/usp_wifi_advanced_service.dart b/lib/page/wifi_settings/services/usp_wifi_advanced_service.dart index 8fb8bbd16..9ed4299c1 100644 --- a/lib/page/wifi_settings/services/usp_wifi_advanced_service.dart +++ b/lib/page/wifi_settings/services/usp_wifi_advanced_service.dart @@ -43,14 +43,23 @@ class UspWifiAdvancedService { } /// Sets IEEE 802.11h on all given radio paths. + /// + /// [forceAutoChannelPaths] additionally receives `AutoChannelEnable = true` + /// in the same set() call. This is used when disabling DFS on a radio that is + /// parked on a DFS channel: the firmware does not vacate the channel on its + /// own (SSH-verified), so forcing auto-channel makes it reselect a legal + /// non-DFS channel. Paths not in this list keep their channel settings. Future setIeee80211hEnabled({ required List radioPaths, required bool enabled, + List forceAutoChannelPaths = const [], }) async { if (radioPaths.isEmpty) return; try { final params = { for (final path in radioPaths) '${path}IEEE80211hEnabled': enabled, + for (final path in forceAutoChannelPaths) + '${path}AutoChannelEnable': true, }; await _usp.set(params); } catch (e) { diff --git a/test/page/wifi_settings/providers/usp_wifi_advanced_notifier_test.dart b/test/page/wifi_settings/providers/usp_wifi_advanced_notifier_test.dart index 7a7c05a69..c8f617351 100644 --- a/test/page/wifi_settings/providers/usp_wifi_advanced_notifier_test.dart +++ b/test/page/wifi_settings/providers/usp_wifi_advanced_notifier_test.dart @@ -3,6 +3,7 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:mocktail/mocktail.dart'; import 'package:privacy_gui/core/errors/service_error.dart'; import 'package:privacy_gui/core/usp/providers/usp_mutation_lock.dart'; +import 'package:privacy_gui/page/_shared/models/wifi_radio_ui_model.dart'; import 'package:privacy_gui/page/wifi_settings/providers/usp_wifi_advanced_provider.dart'; import 'package:privacy_gui/page/wifi_settings/providers/wifi_data_provider.dart'; import 'package:privacy_gui/page/wifi_settings/services/usp_wifi_advanced_service.dart'; @@ -10,6 +11,24 @@ import 'package:privacy_gui/page/wifi_settings/services/usp_wifi_advanced_servic class MockUspWifiAdvancedService extends Mock implements UspWifiAdvancedService {} +WifiRadioUIModel _radioModel({ + required String instancePath, + required String band, + required int channel, + required bool autoChannelEnable, +}) => + WifiRadioUIModel( + instancePath: instancePath, + band: band, + enable: true, + transmitPower: 100, + maxBitRate: 2402, + channel: channel, + autoChannelEnable: autoChannelEnable, + channelBandwidth: '80MHz', + supportedStandards: 'a,n,ac,ax', + ); + void main() { late MockUspWifiAdvancedService mockService; @@ -17,12 +36,14 @@ void main() { mockService = MockUspWifiAdvancedService(); }); - ProviderContainer createContainer() { + ProviderContainer createContainer({ + List radios = const [], + }) { final container = ProviderContainer( overrides: [ uspWifiAdvancedServiceProvider.overrideWithValue(mockService), uspMutationLockProvider.overrideWithValue(UspMutationLock()), - wifiDataProvider.overrideWith(() => _StubWifiDataNotifier()), + wifiDataProvider.overrideWith(() => _StubWifiDataNotifier(radios)), ], ); container.listen(uspWifiAdvancedProvider, (_, __) {}); @@ -184,6 +205,7 @@ void main() { verifyNever(() => mockService.setIeee80211hEnabled( radioPaths: any(named: 'radioPaths'), enabled: any(named: 'enabled'), + forceAutoChannelPaths: any(named: 'forceAutoChannelPaths'), )); container.dispose(); }); @@ -220,6 +242,7 @@ void main() { when(() => mockService.setIeee80211hEnabled( radioPaths: any(named: 'radioPaths'), enabled: any(named: 'enabled'), + forceAutoChannelPaths: any(named: 'forceAutoChannelPaths'), )).thenAnswer((_) async {}); final container = createContainer(); @@ -232,6 +255,118 @@ void main() { verify(() => mockService.setIeee80211hEnabled( radioPaths: ['Device.WiFi.Radio.1.', 'Device.WiFi.Radio.2.'], enabled: true, + forceAutoChannelPaths: const [], + )).called(1); + container.dispose(); + }); + + test( + 'disabling DFS forces AutoChannelEnable on radios parked on a DFS ' + 'channel', () async { + when(() => mockService.fetchIeee80211h()).thenAnswer((_) async => { + 'Device.WiFi.Radio.1.': true, + 'Device.WiFi.Radio.2.': true, + }); + when(() => mockService.setIeee80211hEnabled( + radioPaths: any(named: 'radioPaths'), + enabled: any(named: 'enabled'), + forceAutoChannelPaths: any(named: 'forceAutoChannelPaths'), + )).thenAnswer((_) async {}); + + final container = createContainer(radios: [ + // 2.4 GHz on ch 6 — never DFS, must not be forced. + _radioModel( + instancePath: 'Device.WiFi.Radio.1.', + band: '2.4GHz', + channel: 6, + autoChannelEnable: false, + ), + // 5 GHz manually parked on DFS ch 100 — must be forced to auto. + _radioModel( + instancePath: 'Device.WiFi.Radio.2.', + band: '5GHz', + channel: 100, + autoChannelEnable: false, + ), + ]); + await Future.delayed(Duration.zero); + + final notifier = container.read(uspWifiAdvancedProvider.notifier); + notifier.setDfsEnabled(false); + await notifier.save(); + + verify(() => mockService.setIeee80211hEnabled( + radioPaths: ['Device.WiFi.Radio.1.', 'Device.WiFi.Radio.2.'], + enabled: false, + forceAutoChannelPaths: ['Device.WiFi.Radio.2.'], + )).called(1); + container.dispose(); + }); + + test( + 'disabling DFS does not force auto-channel when the 5 GHz radio is on ' + 'a non-DFS channel', () async { + when(() => mockService.fetchIeee80211h()).thenAnswer((_) async => { + 'Device.WiFi.Radio.2.': true, + }); + when(() => mockService.setIeee80211hEnabled( + radioPaths: any(named: 'radioPaths'), + enabled: any(named: 'enabled'), + forceAutoChannelPaths: any(named: 'forceAutoChannelPaths'), + )).thenAnswer((_) async {}); + + final container = createContainer(radios: [ + // 5 GHz on ch 36 (non-DFS) — no remediation needed. + _radioModel( + instancePath: 'Device.WiFi.Radio.2.', + band: '5GHz', + channel: 36, + autoChannelEnable: false, + ), + ]); + await Future.delayed(Duration.zero); + + final notifier = container.read(uspWifiAdvancedProvider.notifier); + notifier.setDfsEnabled(false); + await notifier.save(); + + verify(() => mockService.setIeee80211hEnabled( + radioPaths: ['Device.WiFi.Radio.2.'], + enabled: false, + forceAutoChannelPaths: const [], + )).called(1); + container.dispose(); + }); + + test('disabling DFS skips a radio already on auto-channel', () async { + when(() => mockService.fetchIeee80211h()).thenAnswer((_) async => { + 'Device.WiFi.Radio.2.': true, + }); + when(() => mockService.setIeee80211hEnabled( + radioPaths: any(named: 'radioPaths'), + enabled: any(named: 'enabled'), + forceAutoChannelPaths: any(named: 'forceAutoChannelPaths'), + )).thenAnswer((_) async {}); + + final container = createContainer(radios: [ + // Auto-channel already on: firmware will pick a legal channel itself. + _radioModel( + instancePath: 'Device.WiFi.Radio.2.', + band: '5GHz', + channel: 100, + autoChannelEnable: true, + ), + ]); + await Future.delayed(Duration.zero); + + final notifier = container.read(uspWifiAdvancedProvider.notifier); + notifier.setDfsEnabled(false); + await notifier.save(); + + verify(() => mockService.setIeee80211hEnabled( + radioPaths: ['Device.WiFi.Radio.2.'], + enabled: false, + forceAutoChannelPaths: const [], )).called(1); container.dispose(); }); @@ -243,6 +378,7 @@ void main() { when(() => mockService.setIeee80211hEnabled( radioPaths: any(named: 'radioPaths'), enabled: any(named: 'enabled'), + forceAutoChannelPaths: any(named: 'forceAutoChannelPaths'), )).thenThrow(const InvalidInputError(detail: 'read-only')); final container = createContainer(); @@ -262,6 +398,7 @@ void main() { when(() => mockService.setIeee80211hEnabled( radioPaths: any(named: 'radioPaths'), enabled: any(named: 'enabled'), + forceAutoChannelPaths: any(named: 'forceAutoChannelPaths'), )).thenAnswer((_) async {}); final container = createContainer(); @@ -326,6 +463,15 @@ void main() { // --------------------------------------------------------------------------- class _StubWifiDataNotifier extends WifiDataNotifier { + _StubWifiDataNotifier(this._radios); + + final List _radios; + @override - Future build() async => const WifiData.empty(); + Future build() async => _radios.isEmpty + ? const WifiData.empty() + : WifiData( + codegenContext: WifiCodegenContext.empty, + radioModels: _radios, + ); } diff --git a/test/page/wifi_settings/services/usp_wifi_advanced_service_test.dart b/test/page/wifi_settings/services/usp_wifi_advanced_service_test.dart index e4adaae00..d38594414 100644 --- a/test/page/wifi_settings/services/usp_wifi_advanced_service_test.dart +++ b/test/page/wifi_settings/services/usp_wifi_advanced_service_test.dart @@ -114,6 +114,37 @@ void main() { throwsA(isA()), ); }); + + test('forces AutoChannelEnable in the same set for given paths', () async { + when(() => mockUsp.set(any())).thenAnswer((_) async => {}); + + await svc.setIeee80211hEnabled( + radioPaths: ['Device.WiFi.Radio.1.', 'Device.WiFi.Radio.2.'], + enabled: false, + forceAutoChannelPaths: ['Device.WiFi.Radio.2.'], + ); + + // Radio.2 (parked on a DFS channel) also gets AutoChannelEnable=true; + // Radio.1 keeps its channel settings. + verify(() => mockUsp.set({ + 'Device.WiFi.Radio.1.IEEE80211hEnabled': false, + 'Device.WiFi.Radio.2.IEEE80211hEnabled': false, + 'Device.WiFi.Radio.2.AutoChannelEnable': true, + })).called(1); + }); + + test('empty forceAutoChannelPaths writes no AutoChannelEnable', () async { + when(() => mockUsp.set(any())).thenAnswer((_) async => {}); + + await svc.setIeee80211hEnabled( + radioPaths: ['Device.WiFi.Radio.1.'], + enabled: false, + ); + + verify(() => mockUsp.set({ + 'Device.WiFi.Radio.1.IEEE80211hEnabled': false, + })).called(1); + }); }); // ------------------------------------------------------------------------- From a9567059d09b49b0325bbc37d58fea98eb198235 Mon Sep 17 00:00:00 2001 From: Austin Chang Date: Mon, 13 Jul 2026 21:02:19 +0800 Subject: [PATCH 3/3] =?UTF-8?q?refactor(wifi):=20address=20PR=20#1124=20re?= =?UTF-8?q?view=20=E2=80=94=20dedup=20path=20helper,=20parse=20set=20resul?= =?UTF-8?q?t,=20doc=20DFS=20contracts?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the automated review on PR #1124. - W-6: extract the 4 identical `_ensureTrailingDot`/`_withTrailingDot` copies (ethernet + 2 wifi services + advanced provider) into a single shared `ensureTrailingDot` in lib/core/utils/tr181_path.dart. - W-3: parse the USP `set()` result in UspWifiAdvancedService.setIeee80211hEnabled via UspResultParser, throwing UspPartialFailureError / UspCompleteFailureError on partial/complete firmware rejection (mirrors the settings-service pattern). A partial rejection of the forced AutoChannelEnable write is no longer silently swallowed. Adds partial/complete-failure tests and updates the existing set() stubs to a success-shaped map. - W-2: document that WifiRadioUIModel.possibleChannels is raw (DFS filtering happens at display time in WifiChannelDialog on the dashboard path). - W-5: note that dfsChannels5GHz is the US/FCC (UNII-2A/2C) set — extend per regulatory domain if multi-market support is required. W-1 (no-op guard not auto-correcting a pre-existing DFS-parked radio) is intentionally not changed — it would contradict the deliberate "view-only Apply must not trigger a write" design and reintroduce a spurious radio restart. Co-Authored-By: Claude Opus 4.8 --- lib/core/utils/tr181_path.dart | 10 +++ lib/core/utils/wifi_channel.dart | 4 ++ .../_shared/models/wifi_radio_ui_model.dart | 8 +++ .../services/usp_ethernet_data_service.dart | 10 +-- .../providers/usp_wifi_advanced_provider.dart | 13 +--- .../services/usp_wifi_advanced_service.dart | 24 ++++++- .../services/usp_wifi_data_service.dart | 34 ++++----- .../services/usp_wifi_settings_service.dart | 19 ++--- .../usp_wifi_advanced_service_test.dart | 72 +++++++++++++++++-- 9 files changed, 141 insertions(+), 53 deletions(-) create mode 100644 lib/core/utils/tr181_path.dart diff --git a/lib/core/utils/tr181_path.dart b/lib/core/utils/tr181_path.dart new file mode 100644 index 000000000..06545306f --- /dev/null +++ b/lib/core/utils/tr181_path.dart @@ -0,0 +1,10 @@ +// Shared helpers for TR-181 object path manipulation. + +/// Ensures a TR-181 instance path ends with a dot so paths from different +/// sources (codegen `instancePath`, `lowerLayers`, `ssidReference`, provider +/// maps) compare and look up consistently. Returns [path] unchanged when empty +/// or already dot-terminated. +String ensureTrailingDot(String path) { + if (path.isEmpty) return path; + return path.endsWith('.') ? path : '$path.'; +} diff --git a/lib/core/utils/wifi_channel.dart b/lib/core/utils/wifi_channel.dart index 4b8120230..ea5febc88 100644 --- a/lib/core/utils/wifi_channel.dart +++ b/lib/core/utils/wifi_channel.dart @@ -38,6 +38,10 @@ List parsePossibleChannels(String raw) { /// 5 GHz DFS channels (IEEE 802.11h): UNII-2A (52–64) + UNII-2C (100–144). /// These are the only channels subject to Dynamic Frequency Selection; 2.4 GHz /// and 6 GHz channels are never DFS. +/// +/// Regulatory scope: this is the US/FCC (UNII-2A/2C) set. Other domains differ +/// (e.g. ETSI weather-radar restrictions, MIC/Japan assignments) — extend or +/// parameterize per regulatory domain if multi-market support is required. const Set dfsChannels5GHz = { 52, 56, 60, 64, // 100, 104, 108, 112, 116, 120, 124, 128, 132, 136, 140, 144, diff --git a/lib/page/_shared/models/wifi_radio_ui_model.dart b/lib/page/_shared/models/wifi_radio_ui_model.dart index fa908ba39..b7780b170 100644 --- a/lib/page/_shared/models/wifi_radio_ui_model.dart +++ b/lib/page/_shared/models/wifi_radio_ui_model.dart @@ -19,6 +19,14 @@ class WifiRadioUIModel extends Equatable with DiagnosticLoggable { /// dashboard data fetch ([UspWifiDataService.fetch]), so the edit-channel /// dialog can render its dropdown synchronously with no per-dialog fetch. /// Empty when the band exposes no manual channels. + /// + /// NOTE: this list is RAW — it is NOT DFS-filtered. On the dashboard path, + /// DFS (IEEE 802.11h) channels are hidden at display time in + /// [WifiChannelDialog] via `filterDfsChannels` keyed off [isDfsEnabled]. (The + /// WiFi Settings path pre-filters instead, in + /// `UspWifiSettingsService.buildWifiNetworks`, before building bandwidth + /// maps.) Any new consumer needing DFS-off filtering must call + /// `filterDfsChannels` itself. final List possibleChannels; /// Per-radio DFS (IEEE 802.11h) enabled state, from diff --git a/lib/page/local_network/services/usp_ethernet_data_service.dart b/lib/page/local_network/services/usp_ethernet_data_service.dart index b838e37e6..ddea07ff6 100644 --- a/lib/page/local_network/services/usp_ethernet_data_service.dart +++ b/lib/page/local_network/services/usp_ethernet_data_service.dart @@ -1,6 +1,7 @@ import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:privacy_gui/core/errors/service_error.dart'; import 'package:privacy_gui/core/utils/logger.dart'; +import 'package:privacy_gui/core/utils/tr181_path.dart'; import 'package:privacy_gui/core/usp/errors/usp_error.dart'; import 'package:privacy_gui/generated/ethernet_interfaces.g.dart'; import 'package:privacy_gui/core/usp/providers/usp_client_provider.dart'; @@ -129,11 +130,11 @@ class UspEthernetDataService { final result = []; final bridgeMemberPaths = - bridgePortMap.values.map(_ensureTrailingDot).toSet(); + bridgePortMap.values.map(ensureTrailingDot).toSet(); EthernetInterface? lanAggregate; for (final iface in ethernetInterfaces.items) { - final path = _ensureTrailingDot(iface.instancePath); + final path = ensureTrailingDot(iface.instancePath); if (bridgeMemberPaths.contains(path)) { lanAggregate ??= iface; } else { @@ -214,9 +215,4 @@ class UspEthernetDataService { return result; } - - static String _ensureTrailingDot(String path) { - if (path.isEmpty) return path; - return path.endsWith('.') ? path : '$path.'; - } } diff --git a/lib/page/wifi_settings/providers/usp_wifi_advanced_provider.dart b/lib/page/wifi_settings/providers/usp_wifi_advanced_provider.dart index 0220d3989..c562b0bc9 100644 --- a/lib/page/wifi_settings/providers/usp_wifi_advanced_provider.dart +++ b/lib/page/wifi_settings/providers/usp_wifi_advanced_provider.dart @@ -1,6 +1,7 @@ import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:privacy_gui/core/errors/service_error.dart'; import 'package:privacy_gui/core/utils/logger.dart'; +import 'package:privacy_gui/core/utils/tr181_path.dart'; import 'package:privacy_gui/core/utils/wifi_channel.dart'; import 'package:privacy_gui/core/usp/providers/usp_mutation_lock.dart'; import 'package:privacy_gui/framework/preservable_contract.dart'; @@ -116,12 +117,12 @@ class UspWifiAdvancedNotifier if (!enabled) { final radios = ref.read(wifiDataProvider).valueOrNull?.radioModels ?? []; final radioByPath = { - for (final r in radios) _withTrailingDot(r.instancePath): r, + for (final r in radios) ensureTrailingDot(r.instancePath): r, }; for (final path in radioPaths) { // Radios staying on DFS need no channel remediation. if (current.ieee80211hByRadio[path] == true) continue; - final radio = radioByPath[_withTrailingDot(path)]; + final radio = radioByPath[ensureTrailingDot(path)]; if (radio == null || radio.autoChannelEnable) continue; if (isDfsChannel(radio.channel, band: radio.band)) { forceAutoChannelPaths.add(path); @@ -180,11 +181,3 @@ class UspWifiAdvancedNotifier ); } } - -/// Normalizes a TR-181 instance path to a trailing-dot form so that radio -/// paths from [WifiAdvancedSettings.ieee80211hByRadio] and from -/// [WifiRadioUIModel.instancePath] compare equal regardless of source format. -String _withTrailingDot(String path) { - if (path.isEmpty) return path; - return path.endsWith('.') ? path : '$path.'; -} diff --git a/lib/page/wifi_settings/services/usp_wifi_advanced_service.dart b/lib/page/wifi_settings/services/usp_wifi_advanced_service.dart index 9ed4299c1..ac59f22b1 100644 --- a/lib/page/wifi_settings/services/usp_wifi_advanced_service.dart +++ b/lib/page/wifi_settings/services/usp_wifi_advanced_service.dart @@ -1,4 +1,5 @@ import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:privacy_gui/core/errors/service_error.dart'; import 'package:privacy_gui/core/usp/errors/usp_error.dart'; import 'package:privacy_gui/core/usp/providers/usp_client_provider.dart'; import 'package:privacy_gui/core/usp/services/usp_client.dart'; @@ -61,8 +62,29 @@ class UspWifiAdvancedService { for (final path in forceAutoChannelPaths) '${path}AutoChannelEnable': true, }; - await _usp.set(params); + final result = await _usp.set(params); + // Parse the batch result so a firmware partial rejection (e.g. accepts + // IEEE80211hEnabled but rejects a forced AutoChannelEnable) surfaces as an + // error instead of being silently swallowed. + final parsed = UspResultParser.parseSetResult(result); + switch (parsed) { + case UspSuccess(): + break; + case UspPartialSuccess(failures: final f): + throw UspPartialFailureError( + summary: + 'IEEE80211h update partial failure: ${f.first.errorMessage}', + successPaths: const [], + failures: f, + ); + case UspFailure(errors: final e): + throw UspCompleteFailureError( + summary: 'IEEE80211h update failed: ${e.first.errorMessage}', + failures: e, + ); + } } catch (e) { + if (e is ServiceError) rethrow; throw mapUspErrorToServiceError(e); } } diff --git a/lib/page/wifi_settings/services/usp_wifi_data_service.dart b/lib/page/wifi_settings/services/usp_wifi_data_service.dart index b5b292012..05ea60f67 100644 --- a/lib/page/wifi_settings/services/usp_wifi_data_service.dart +++ b/lib/page/wifi_settings/services/usp_wifi_data_service.dart @@ -9,6 +9,7 @@ import 'package:privacy_gui/generated/wifi_clients.g.dart'; import 'package:privacy_gui/core/usp/providers/usp_client_provider.dart'; import 'package:privacy_gui/core/usp/services/usp_client.dart'; import 'package:privacy_gui/core/utils/logger.dart'; +import 'package:privacy_gui/core/utils/tr181_path.dart'; import 'package:privacy_gui/core/utils/wifi_channel.dart'; import 'package:privacy_gui/page/_shared/models/client_connection_detail.dart'; import 'package:privacy_gui/page/_shared/utils/wifi_guest_detection.dart'; @@ -159,14 +160,14 @@ class UspWifiDataService { required WiFiAccessPoints accessPoints, }) { final ssidByPath = { - for (final s in ssids.items) _ensureTrailingDot(s.instancePath): s, + for (final s in ssids.items) ensureTrailingDot(s.instancePath): s, }; // Determine guest SSIDs via the canonical alias rule (see // wifi_guest_detection). Single source of truth shared across the app. final guestSsidPaths = { for (final ssid in ssids.items) - if (isGuestSsid(ssid)) _ensureTrailingDot(ssid.instancePath), + if (isGuestSsid(ssid)) ensureTrailingDot(ssid.instancePath), }; logger.t('[USP][WiFi] Total guest SSID paths: ${guestSsidPaths.length}'); // Diagnostic: multiple SSIDs but none matched the `-guest` alias rule @@ -182,18 +183,18 @@ class UspWifiDataService { final apsByRadioPath = >{}; for (final ap in accessPoints.items) { - final ssid = ssidByPath[_ensureTrailingDot(ap.ssidReference)]; + final ssid = ssidByPath[ensureTrailingDot(ap.ssidReference)]; if (ssid == null) continue; - final radioPath = _ensureTrailingDot(ssid.lowerLayers); + final radioPath = ensureTrailingDot(ssid.lowerLayers); apsByRadioPath.putIfAbsent(radioPath, () => []).add((ap: ap, ssid: ssid)); } return radios.items.map((radio) { final radioAps = - apsByRadioPath[_ensureTrailingDot(radio.instancePath)] ?? []; + apsByRadioPath[ensureTrailingDot(radio.instancePath)] ?? []; final apModels = radioAps.map((a) { final isGuest = - guestSsidPaths.contains(_ensureTrailingDot(a.ssid.instancePath)); + guestSsidPaths.contains(ensureTrailingDot(a.ssid.instancePath)); // Per-network enabled state = SSID.Enable. The Dashboard toggle mutates // both SSID.Enable and AccessPoint.Enable together, so either would do; // we read SSID.Enable as the single source of truth for the UI. @@ -328,14 +329,14 @@ class UspWifiDataService { }) { final apByPath = { for (final ap in accessPoints.items) - _ensureTrailingDot(ap.instancePath): ap, + ensureTrailingDot(ap.instancePath): ap, }; final ssidByPath = { - for (final s in ssids.items) _ensureTrailingDot(s.instancePath): s, + for (final s in ssids.items) ensureTrailingDot(s.instancePath): s, }; final bandByRadioPath = { for (final r in radios.items) - _ensureTrailingDot(r.instancePath): + ensureTrailingDot(r.instancePath): _normalizeBand(r.operatingFrequencyBand), }; @@ -344,18 +345,18 @@ class UspWifiDataService { final mac = entry.key; final client = entry.value; - final ap = apByPath[_ensureTrailingDot(client.parentPath)]; + final ap = apByPath[ensureTrailingDot(client.parentPath)]; if (ap == null) { logger.d( '[USP][Dashboard]Connection detail: no AP for parentPath=${client.parentPath}'); continue; } - final ssid = ssidByPath[_ensureTrailingDot(ap.ssidReference)]; + final ssid = ssidByPath[ensureTrailingDot(ap.ssidReference)]; final ssidName = ssid?.ssid ?? ''; final band = ssid != null - ? (bandByRadioPath[_ensureTrailingDot(ssid.lowerLayers)] ?? '') + ? (bandByRadioPath[ensureTrailingDot(ssid.lowerLayers)] ?? '') : ''; result[mac] = ClientConnectionDetail(band: band, ssidName: ssidName); @@ -387,11 +388,6 @@ class UspWifiDataService { // Helpers // --------------------------------------------------------------------------- - static String _ensureTrailingDot(String path) { - if (path.isEmpty) return path; - return path.endsWith('.') ? path : '$path.'; - } - static String _normalizeBand(String rawBand) { final lower = rawBand.toLowerCase(); if (lower.contains('6g') || lower.contains('6 g')) return '6GHz'; @@ -413,7 +409,7 @@ class UspWifiDataService { // Build Radio path → band lookup final bandByRadioPath = {}; for (final radio in radios.items) { - final path = _ensureTrailingDot(radio.instancePath); + final path = ensureTrailingDot(radio.instancePath); bandByRadioPath[path] = _normalizeBand(radio.operatingFrequencyBand); } @@ -423,7 +419,7 @@ class UspWifiDataService { final bssid = ssid.bssid.trim().toUpperCase(); if (bssid.isEmpty) continue; - final radioPath = _ensureTrailingDot(ssid.lowerLayers); + final radioPath = ensureTrailingDot(ssid.lowerLayers); final band = bandByRadioPath[radioPath]; if (band != null && band.isNotEmpty) { result[bssid] = band; diff --git a/lib/page/wifi_settings/services/usp_wifi_settings_service.dart b/lib/page/wifi_settings/services/usp_wifi_settings_service.dart index ac9c4e449..83fe62bce 100644 --- a/lib/page/wifi_settings/services/usp_wifi_settings_service.dart +++ b/lib/page/wifi_settings/services/usp_wifi_settings_service.dart @@ -1,6 +1,7 @@ import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:privacy_gui/core/errors/service_error.dart'; import 'package:privacy_gui/core/utils/logger.dart'; +import 'package:privacy_gui/core/utils/tr181_path.dart'; import 'package:privacy_gui/core/utils/wifi_channel.dart'; import 'package:privacy_gui/core/usp/errors/usp_error.dart'; import 'package:privacy_gui/generated/wi_fi_access_points.g.dart'; @@ -46,13 +47,13 @@ class UspWifiSettingsService { // Build lookup maps with normalized trailing-dot paths final apBySsidRef = {}; for (final ap in accessPoints.items) { - final key = _ensureTrailingDot(ap.ssidReference); + final key = ensureTrailingDot(ap.ssidReference); if (key.isNotEmpty) apBySsidRef[key] = ap; } final radioByPath = {}; for (final r in radios.items) { - radioByPath[_ensureTrailingDot(r.instancePath)] = r; + radioByPath[ensureTrailingDot(r.instancePath)] = r; } logger.d('[USP][WiFi]: Building networks: ' @@ -62,13 +63,13 @@ class UspWifiSettingsService { final networks = []; for (final ssid in ssids.items) { - final ssidPath = _ensureTrailingDot(ssid.instancePath); + final ssidPath = ensureTrailingDot(ssid.instancePath); // Find matching AccessPoint via ssidReference final ap = apBySsidRef[ssidPath]; // Find matching Radio via SSID.lowerLayers - final radioPath = _ensureTrailingDot(ssid.lowerLayers); + final radioPath = ensureTrailingDot(ssid.lowerLayers); final radio = radioByPath[radioPath]; logger.d('[USP][WiFi]: SSID ${ssid.ssid}: ' @@ -573,10 +574,10 @@ class UspWifiSettingsService { if (ssidPaths.isEmpty) return 0; // Resolve AccessPoint paths whose SSIDReference points at a matched SSID. - final matchedSsidPathSet = ssidPaths.map(_ensureTrailingDot).toSet(); + final matchedSsidPathSet = ssidPaths.map(ensureTrailingDot).toSet(); final apPaths = accessPoints.items .where((ap) => - matchedSsidPathSet.contains(_ensureTrailingDot(ap.ssidReference))) + matchedSsidPathSet.contains(ensureTrailingDot(ap.ssidReference))) .map((ap) => ap.instancePath) .toList(); @@ -651,12 +652,6 @@ List _parseModesSupported(String raw) { .toList(); } -/// Ensures a TR-181 path ends with a dot. -String _ensureTrailingDot(String path) { - if (path.isEmpty) return path; - return path.endsWith('.') ? path : '$path.'; -} - /// Parses a TR-181 SupportedOperatingChannelBandwidths string. /// e.g. "Auto,20MHz,40MHz,80MHz" → ['Auto', '20MHz', '40MHz', '80MHz'] List _parseSupportedBandwidths(String raw) { diff --git a/test/page/wifi_settings/services/usp_wifi_advanced_service_test.dart b/test/page/wifi_settings/services/usp_wifi_advanced_service_test.dart index d38594414..70dd7f499 100644 --- a/test/page/wifi_settings/services/usp_wifi_advanced_service_test.dart +++ b/test/page/wifi_settings/services/usp_wifi_advanced_service_test.dart @@ -6,6 +6,42 @@ import 'package:privacy_gui/page/wifi_settings/services/usp_wifi_advanced_servic class MockUspClient extends Mock implements UspClient {} +// WASM v0.11.0 set-result shapes consumed by UspResultParser.parseSetResult. +Map _setSuccess() => { + 'success': true, + 'result': {'data': {}}, + }; + +Map _setPartial({ + String path = 'Device.WiFi.Radio.2.AutoChannelEnable', + int errorCode = 7008, + String errorMessage = 'Invalid value', +}) => + { + 'success': true, + 'result': { + 'data': {'Device.WiFi.Radio.1.IEEE80211hEnabled': false}, + 'error': { + path: {'errorCode': errorCode, 'errorMessage': errorMessage}, + }, + }, + }; + +Map _setFailure({ + String path = 'bulk_operation', + int errorCode = 7004, + String errorMessage = 'Operation failed', +}) => + { + 'success': false, + 'result': { + 'data': {}, + 'error': { + path: {'errorCode': errorCode, 'errorMessage': errorMessage}, + }, + }, + }; + void main() { late MockUspClient mockUsp; late UspWifiAdvancedService svc; @@ -70,7 +106,7 @@ void main() { group('UspWifiAdvancedService - setIeee80211hEnabled', () { test('sets IEEE80211hEnabled on all provided radio paths', () async { - when(() => mockUsp.set(any())).thenAnswer((_) async => {}); + when(() => mockUsp.set(any())).thenAnswer((_) async => _setSuccess()); await svc.setIeee80211hEnabled( radioPaths: ['Device.WiFi.Radio.1.', 'Device.WiFi.Radio.2.'], @@ -90,7 +126,7 @@ void main() { }); test('sends false for all radios when disabling', () async { - when(() => mockUsp.set(any())).thenAnswer((_) async => {}); + when(() => mockUsp.set(any())).thenAnswer((_) async => _setSuccess()); await svc.setIeee80211hEnabled( radioPaths: ['Device.WiFi.Radio.1.'], @@ -116,7 +152,7 @@ void main() { }); test('forces AutoChannelEnable in the same set for given paths', () async { - when(() => mockUsp.set(any())).thenAnswer((_) async => {}); + when(() => mockUsp.set(any())).thenAnswer((_) async => _setSuccess()); await svc.setIeee80211hEnabled( radioPaths: ['Device.WiFi.Radio.1.', 'Device.WiFi.Radio.2.'], @@ -134,7 +170,7 @@ void main() { }); test('empty forceAutoChannelPaths writes no AutoChannelEnable', () async { - when(() => mockUsp.set(any())).thenAnswer((_) async => {}); + when(() => mockUsp.set(any())).thenAnswer((_) async => _setSuccess()); await svc.setIeee80211hEnabled( radioPaths: ['Device.WiFi.Radio.1.'], @@ -145,6 +181,34 @@ void main() { 'Device.WiFi.Radio.1.IEEE80211hEnabled': false, })).called(1); }); + + test('throws UspPartialFailureError on firmware partial rejection', + () async { + // Firmware accepts IEEE80211hEnabled but rejects the forced + // AutoChannelEnable write — must not be silently swallowed. + when(() => mockUsp.set(any())).thenAnswer((_) async => _setPartial()); + + expect( + () => svc.setIeee80211hEnabled( + radioPaths: ['Device.WiFi.Radio.1.', 'Device.WiFi.Radio.2.'], + enabled: false, + forceAutoChannelPaths: ['Device.WiFi.Radio.2.'], + ), + throwsA(isA()), + ); + }); + + test('throws UspCompleteFailureError on complete failure', () async { + when(() => mockUsp.set(any())).thenAnswer((_) async => _setFailure()); + + expect( + () => svc.setIeee80211hEnabled( + radioPaths: ['Device.WiFi.Radio.1.'], + enabled: false, + ), + throwsA(isA()), + ); + }); }); // -------------------------------------------------------------------------