fix(devices): MeshNetwork architecture + fix #1043 #1044 #1047 #1048#1068
fix(devices): MeshNetwork architecture + fix #1043 #1044 #1047 #1048#1068AustinChangLinksys wants to merge 9 commits into
Conversation
…1043, #1044) Phase 1-3 of DeviceUIModel/NodeUIModel refactoring: New models (lib/page/_shared/models/): - NetworkEntity: abstract base class for network entities - ClientDevice: client device model with WifiConnectionInfo - NodeEntity: sealed class (MasterNode/SlaveNode) with BackhaulInfo - MeshNetwork: top-level SSoT container with lookup helpers - WifiConnectionInfo/BackhaulInfo: value objects for connection details New builder (lib/page/_shared/utils/): - MeshNetworkBuilder: constructs MeshNetwork from Hosts + DataElements Integration: - DevicesData: added meshNetwork field alongside legacy deviceModels/nodeModels - UspDevicesDataService: builds MeshNetwork in fetch() and rebuild methods - Compatibility layer maintains existing API for gradual migration Also fixes Device Analytics card (#1043, #1044): - Simplified to 4 tabs: Overview, Signal, Trend, Activity - Fixed Y-axis duplicate labels in trend chart - Use node hostname instead of model name for child node clients - Exclude mesh nodes from client distribution counts Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- UspTopologyBuilder: add buildFromMeshNetwork() method, deprecate old build() - UspNetworkTopologyCard: use meshNetwork parameter instead of device/node lists - UspTopologyView: use buildFromMeshNetwork() - uspNodeDetailProvider: support NodeEntity with legacy model conversion The provider now uses MeshNetwork.findNode() for direct lookup and pre-organized connectedClients, while maintaining backward compatibility by converting to NodeUIModel/DeviceUIModel for existing views. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
#1043, #1044) Complete migration to MeshNetwork architecture as Single Source of Truth: - Remove DeviceUIModel class and related extensions - Remove NodeUIModel class and legacy conversion helpers - Remove obsolete test files for deleted models - Migrate all 26+ consumers to use ClientDevice/NodeEntity directly - Update MeshNetworkBuilder to patch parentNodeName on all clients - Improve Device Analytics card UI with LayoutBlock grid layout - Show connected node name for ALL devices (including master node clients) Architecture benefits: - SSoT: MeshNetwork contains all network entities in one place - Sealed classes: NodeEntity (MasterNode/SlaveNode) enable pattern matching - Direct ownership: Nodes own their connectedClients list - Cleaner lookups: meshNetwork.findNode()/findClient() replace manual loops Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
PR Summary by Qodofix(devices): MeshNetwork SSoT architecture + fix Y-axis & child node signal
AI Description
Diagram
High-Level Assessment
Files changed (29)
|
Code Review by Qodo
Context used 1. Test data helper in wrong folder
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| import 'package:privacy_gui/page/_shared/models/client_device.dart'; | ||
| import 'package:privacy_gui/page/_shared/models/mesh_network.dart'; | ||
| import 'package:privacy_gui/page/_shared/models/node_entity.dart'; | ||
| import 'package:privacy_gui/page/devices/providers/devices_data_provider.dart'; | ||
|
|
||
| /// Creates an empty MeshNetwork for testing purposes. | ||
| MeshNetwork createEmptyMeshNetwork() { | ||
| return MeshNetwork( | ||
| master: MasterNode( | ||
| deviceId: 'GATEWAY', | ||
| model: 'Test Router', | ||
| connectedClients: [], | ||
| ), | ||
| ); | ||
| } | ||
|
|
||
| /// Creates a MeshNetwork with the given clients for testing purposes. | ||
| MeshNetwork createMeshNetworkWithClients(List<ClientDevice> clients) { | ||
| return MeshNetwork( | ||
| master: MasterNode( | ||
| deviceId: 'GATEWAY', | ||
| model: 'Test Router', | ||
| connectedClients: clients, | ||
| ), | ||
| ); | ||
| } | ||
|
|
||
| /// Creates a MeshNetwork with both master and slave nodes for testing purposes. | ||
| MeshNetwork createMeshNetworkWithNodes({ | ||
| required MasterNode master, | ||
| List<SlaveNode> slaves = const [], | ||
| }) { | ||
| return MeshNetwork(master: master, slaves: slaves); | ||
| } | ||
|
|
||
| /// Creates an empty DevicesData for testing purposes. | ||
| DevicesData createEmptyDevicesData() { | ||
| return DevicesData(meshNetwork: createEmptyMeshNetwork()); | ||
| } | ||
|
|
||
| /// Creates DevicesData with the given clients for testing purposes. | ||
| DevicesData createDevicesDataWithClients(List<ClientDevice> clients) { | ||
| return DevicesData(meshNetwork: createMeshNetworkWithClients(clients)); | ||
| } |
There was a problem hiding this comment.
1. Test data helper in wrong folder 📘 Rule violation ⚙ Maintainability
A new reusable test data builder file was added under test/test_helpers/, but compliance requires test data builders be centralized under test/mocks/test_data/ using the [feature]_test_data.dart convention. Keeping builders outside the standard location increases duplication and makes test data harder to discover and reuse consistently.
Agent Prompt
## Issue description
A reusable test data builder was added at `test/test_helpers/mesh_network_test_helper.dart`, but compliance requires such builders to live under `test/mocks/test_data/` following the `[feature]_test_data.dart` naming convention.
## Issue Context
This helper provides reusable factories (`createEmptyMeshNetwork()`, `createDevicesDataWithClients()`, etc.), which qualifies as centralized test data/builder code.
## Fix Focus Areas
- test/test_helpers/mesh_network_test_helper.dart[1-44]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| macAddress: master.deviceId, | ||
| deviceUUID: master.deviceId, // fallback - may need refinement |
There was a problem hiding this comment.
2. Wrong ra deviceuuid 🐞 Bug ≡ Correctness
deviceCredentialsProvider sets DeviceCredentials.deviceUUID to master.deviceId, which is the node identifier (typically MAC-like) rather than the Hosts DeviceID/UUID previously used for Guardian Remote Assistance calls. This will break Remote Assistance session lookup/PIN creation because requests are sent with an incorrect deviceUUID.
Agent Prompt
## Issue description
Remote Assistance credentials currently set `deviceUUID` to `master.deviceId`, which is not the Hosts DeviceID/UUID needed by Guardian API calls.
## Issue Context
- The UUID previously came from the Hosts table (`ConnectedDevice.deviceId`), and the codebase still preserves that value for clients as `ClientDevice.hostsDeviceId`.
- The new `MasterNode` model does not currently retain the Hosts DeviceID, so the provider can’t fetch it.
## Fix Focus Areas
- lib/core/cloud/providers/remote_assistance/device_credentials_provider.dart[19-29]
- lib/page/_shared/models/node_entity.dart[84-190]
- lib/page/_shared/utils/mesh_network_builder.dart[75-125]
## Suggested fix
1. Add a `hostsDeviceId` (or similarly named) field to `MasterNode` (and optionally `NodeEntity` if you want it shared).
2. Populate it in `MeshNetworkBuilder._buildMasterNode` from the Hosts record (`masterDevice?.deviceId`).
3. Update `deviceCredentialsProvider` to use `devicesData.master.hostsDeviceId` for `deviceUUID` and return null if it’s missing.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| String _getDeviceCategory(ClientDevice d) { | ||
| final isChildNodeClient = d.parentNodeName != null && d.band == null; | ||
| if (isChildNodeClient) { | ||
| return d.parentNodeName!; | ||
| } | ||
| if (d.isWifi) { | ||
| return d.band ?? 'WiFi'; | ||
| } | ||
| return 'Wired'; |
There was a problem hiding this comment.
3. Wired devices miscategorized 🐞 Bug ≡ Correctness
UspDeviceAnalyticsNotifier._getDeviceCategory classifies any device with parentNodeName != null and band == null as a “child node client”, but MeshNetworkBuilder patches parentNodeName onto *all* master clients and wired devices always have band == null. This causes master wired clients to be bucketed under the gateway name instead of "Wired", skewing bandDistribution and any UI/PDF that depends on it.
Agent Prompt
## Issue description
Device analytics categorization treats `(parentNodeName != null && band == null)` as “child node client”, which incorrectly captures master wired devices because:
- wired => `band == null`
- builder patches `parentNodeName` for master clients
## Issue Context
This directly impacts `DeviceDistribution.bandDistribution` (now used as category distribution) and can mislabel wired totals.
## Fix Focus Areas
- lib/page/_shared/providers/usp_device_analytics_notifier.dart[171-185]
- lib/page/_shared/utils/mesh_network_builder.dart[105-125]
- lib/page/_shared/models/client_device.dart[172-188]
## Suggested fix
Update `_getDeviceCategory` to handle wired devices first, then use band/name only for WiFi, e.g.:
```dart
String _getDeviceCategory(ClientDevice d) {
if (!d.isWifi) return 'Wired';
if (d.band != null && d.band!.isNotEmpty) return d.band!;
if (d.parentNodeName != null && d.parentNodeName!.isNotEmpty) return d.parentNodeName!;
return 'WiFi';
}
```
This matches the method’s own comment (“Master Wired: 'Wired'”) and prevents master wired devices from being categorized under the gateway display name.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
🤖 Automated Review — Oversize PR This round's changes exceed the automated-review limit (6,692 lines / 72 files, limit 4,000 lines / 60 files); AI review was not run. Manual review recommended. First 15 changed files (for a quick scan): |
Resolve conflicts: - Use ClientDevice model (this branch) instead of DeviceUIModel - Keep AppLineChart for trend view (this branch's design choice) - Remove duplicate variable definitions in analytics card - Use deviceId for router MAC filtering (NodeEntity API) - Delete obsolete usp_topology_builder_test.dart (uses old DeviceUIModel API) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Fix DevicesData.props to use full objects instead of .length for proper Riverpod state comparison. Add comprehensive unit tests for MeshNetwork, ClientDevice, NodeEntity models and UspTopologyBuilder. - Fix props bug: meshTopology/hostNameByMac now compared as objects - Add DevicesTestData builder (484 lines) for centralized test factories - Add MeshNetwork tests (45 tests): accessors, lookups, Equatable - Add ClientDevice tests (44 tests): displayName, WiFi, multi-interface - Add NodeEntity tests (28 tests): Master/Slave, backhaul, extensions - Add UspTopologyBuilder tests (28 tests): nodes, links, edge cases Total: 145 new tests, all 3047 tests passing. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
🤖 Automated Review — Oversize PR This round's changes exceed the automated-review limit (4738 lines / 58 files, limit 4000 lines / 60 files); AI review was not run. Manual review recommended. First 15 changed files (for a quick scan): |
- Add clientBandSsidMap to MeshTopologyInfo for slave client band/SSID - Add buildBssidToBandMap() to resolve BSSID → band via SSID LowerLayers - Use meshNetwork.allClients as data source instead of wifiClientMap - Add band/SSID fallback to clientBandSsidMap for slave node clients - Truncate long client names in Speed tab chart to prevent overlap - Add comprehensive tests for MeshNetworkBuilder, MeshTopologyBuilder, and UspWifiDataService Closes #1043, #1044 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
AustinChangLinksys
changed the title
AustinChangLinksys
left a comment
AustinChangLinksys
left a comment
There was a problem hiding this comment.
🤖 Automated Review — Round 3 · 936cdda..784d17c (incremental)
Verdict: 💬 Self-review (comment only) — The clientBandSsidMap band/SSID fallback for slave node clients is well-designed and correctly wired end-to-end. One Warning about a sibling SNR bug and one about a stale-snapshot race; several Suggestions.
| Conf. | Where | Issue (one-liner) | |
|---|---|---|---|
| High | stats_wifi_channels_section.dart:80 |
[single, NEW] SNR fix not applied to sibling file — c.client.signalStrength/noise still used; part of the same #1043/#1044 fix context |
|
| High | devices_data_provider.dart:172,286 |
[single, NEW] Stale wifiData snapshot used in fire-and-forget _fetchMeshAndUpdate; SSE WiFi updates can overtake the mesh fetch |
|
| 💡 | Med | usp_wifi_data_service.dart:407 |
_ensureTrailingDot("") returns "" — empty instancePath can cause phantom radio path match for malformed USP responses |
| 💡 | Med | devices_data_provider.dart (diff call site) |
No log warning when bssidToBandMap is empty due to WiFi data timeout; silent degradation |
| 💡 | Med | mesh_topology_builder.dart (diff) |
bss.bssid not guarded for empty before map lookup (harmless today but asymmetric with buildBssidToBandMap) |
| ✅ | High | mesh_topology_builder.dart + usp_wifi_data_service.dart |
BSSID normalization is consistent in both paths — both use trim().toUpperCase() |
| ✅ | High | usp_wifi_data_service.dart:buildBssidToBandMap |
Empty BSSID guard (if (bssid.isEmpty) continue) is correct and covers the null-BSSID codegen case |
| ✅ | High | usp_wifi_performance_card.dart:347 |
SNR fix applied correctly — c.signalStrength, c.noise at PR head (verified via git show 784d17c) |
| ✅ | High | test/...mesh_network_builder_test.dart |
12-scenario test suite comprehensively covers builder paths including signal fallback, band/SSID fallback, precedence, MAC normalization |
Confidence: High = code-verified · Med = located + reasoned, not fully confirmed · Low = speculative, please double-check.
Warning Details
W-1: SNR fix not applied to sibling file [single, High]
lib/page/statistics/views/sections/stats_wifi_channels_section.dart:80 (NOT in this PR's diff):
final snr = computeSNR(c.client.signalStrength, c.client.noise); // c.client.* still presentusp_wifi_performance_card.dart was fixed in this PR to use c.signalStrength, c.noise (confirmed via git show 784d17cf). Both files have a local _ClientInfo class that wraps WifiClientUIModel client. If the intent of this PR is to flatten _ClientInfo to expose signalStrength/noise directly (eliminating the .client nesting), then stats_wifi_channels_section.dart:80 must be updated in the same PR, or the two files diverge in their SNR computation model.
Trigger condition: Always — every render of the WiFi channels statistics view calls this path.
Fix: Either apply the same _ClientInfo flattening to stats_wifi_channels_section.dart in this PR, or document that the two files intentionally use different data models and the stats file is out-of-scope.
W-2: Stale wifiData used in fire-and-forget mesh fetch [single, High]
lib/page/devices/providers/devices_data_provider.dart:172:
_fetchMeshAndUpdate(svc, wifiData, gatewayName, sysData, result);
// ^^^^^^^^ snapshot from before the mesh fetch starts_fetchMeshAndUpdate is called fire-and-forget (no await). By the time fetchMeshTopology completes (potentially slow on large mesh networks), wifiDataProvider may have already emitted a fresh SSE-triggered state. The wifiData passed in will be stale, causing the rebuilt device models to use outdated SSID names, bands, and connection details for the current mesh topology.
The same issue exists at line 286 in _refetchPreservingMesh.
Trigger condition: SSE WiFi update fires between the start of _fetch() and completion of the background fetchMeshTopology() call.
Fix: Re-read WiFi data from Riverpod after the mesh fetch completes inside _fetchMeshAndUpdate:
// After: final meshTopology = await svc.fetchMeshTopology(...)
final freshWifi = ref.read(wifiDataProvider).valueOrNull;
final effectiveWifi = freshWifi ?? wifiData; // use freshest availableSuggestion Details
S-1: _ensureTrailingDot("") phantom match [Med]
lib/page/wifi_settings/services/usp_wifi_data_service.dart:407-409:
static String _ensureTrailingDot(String path) {
if (path.isEmpty) return path; // returns "" unchanged
return path.endsWith('.') ? path : '$path.';
}If both radio.instancePath and ssid.lowerLayers are empty strings, both map to "" and will incorrectly match each other in buildBssidToBandMap, associating an SSID with a band from a radio with an empty path. This is malformed USP data but worth guarding.
Fix: if (path.isEmpty) return path; // keep returning "", but also add guard at call site: if (path.isEmpty) continue;
S-2: No log warning on empty bssidToBandMap [Med]
When wifiDataProvider times out, buildBssidToBandMap is called with WiFiSsids(items: []) and returns {}. Band enrichment silently degrades to no-op. There is no diagnostic trace of why slave clients show no band.
Fix:
if (bssidToBandMap.isEmpty) {
logger.w('[DevicesData]: bssidToBandMap is empty — slave client band/SSID enrichment skipped');
}S-3: bss.bssid not guarded for empty in MeshTopologyBuilder [Med]
lib/page/_shared/utils/mesh_topology_builder.dart (new code):
final bssidUpper = bss.bssid.trim().toUpperCase();
final band = bssidToBandMap[bssidUpper] ?? ''; // empty lookup miss — harmlessEmpty BSSID causes a harmless map miss. However, it's asymmetric with buildBssidToBandMap which explicitly guards empty BSSIDs. For clarity and symmetry:
Fix: Add if (bssidUpper.isEmpty) continue; after normalizing bssidUpper.
What looks good
clientBandSsidMapdesign — clean record type({String band, String ssid})stored by uppercase MAC; theconnectionDetailMapprecedence overclientBandSsidMapis correctly implemented and tested.- BSSID normalization consistency — both
buildBssidToBandMapandMeshTopologyBuilderusetrim().toUpperCase()for lookup keys. No mismatch. buildBssidToBandMapstatic method — correctly uses_ensureTrailingDotfor consistent path normalization; skips empty BSSIDs and empty bands.- Data flow end-to-end —
DevicesDataNotifier→buildBssidToBandMap→fetchMeshTopology(bssidToBandMap:)→MeshTopologyBuilder.build(bssidToBandMap:)→clientBandSsidMap→MeshNetworkBuilder→WifiConnectionInfo.bandis a coherent, traceable chain. - Test suite (
mesh_network_builder_test.dart,mesh_topology_builder_test.dart,usp_wifi_data_service_test.dart) — 12-scenarioMeshNetworkBuildertest suite covers signal fallback, band/SSID fallback, precedence, multi-interface merging, MAC normalization, wired/WiFi detection.MeshTopologyBuildertest coversclientBandSsidMappopulation and empty band case. WifiData.codegenContextin equality (from PR 1075) —DevicesDataNotifiercorrectly readswifiData.codegenContext.rawto build the BSSID map; not bypassing the codegen boundary from the provider layer.- No architecture layer violations —
UspWifiDataService.buildBssidToBandMapis a static utility method callable from other services;DevicesDataNotifierreadswifiDataProviderviaref.read(correct for a Notifier);UspDevicesDataServicestays in the service layer.
Cross-reviewed by two independent agents (security+correctness / architecture+maintainability). Automated — please sanity-check before merge.
AustinChangLinksys
added
the
need-decide
PeterJhongLinksys
left a comment
PeterJhongLinksys
left a comment
There was a problem hiding this comment.
Manual review confirms the two blocking issues already flagged inline by qodo — both should be resolved before merge:
-
RA
deviceUUIDregression (device_credentials_provider.dart) —deviceUUIDis now set to the master's MAC (master.deviceId) instead of the Hosts DeviceID/UUID that the legacy path used.MasterNodecarries no field retaining the Hosts UUID, and the inline// fallback - may need refinementconfirms it's unfinished. This identifier flows into every Guardian Remote Assistance call, so RA session lookup / PIN creation will receive the wrong value — a functional regression. -
Wired-client miscategorization (
usp_device_analytics_notifier.dart,_getDeviceCategory) —parentNodeName != null && band == nullis treated as "child-node client", butMeshNetworkBuilderpatchesparentNodeNameonto all master clients and wired devices always haveband == null, so master wired clients get bucketed under the gateway name instead of "Wired", skewingbandDistribution.
Otherwise the migration looks sound (l10n keys present, MeshNetwork.master non-null so no null-crash, no dead _getRouterMacs()). Holding approval on these two.
…, fallback Resolve the two blocking review issues plus a rule violation and a band-fallback logic bug found in MeshNetworkBuilder: - RA deviceUUID regression: add hostsDeviceId (Hosts UUID) to MasterNode, plumb it through MeshNetworkBuilder, and use it in deviceCredentialsProvider instead of the MAC. Returns null when no UUID is available (matches the legacy behavior; avoids sending a wrong deviceUUID to Guardian RA). - Wired-client miscategorization: _getDeviceCategory now keys off parentNodeId (null only for master clients) instead of the parentNodeName that the builder patches onto ALL clients — master wired clients bucket under "Wired" again. - Remove dead test/test_helpers/mesh_network_test_helper.dart (unused; duplicates DevicesTestData; violated constitution §1.6.2 location rule). - Band/SSID fallback: treat empty-string band/ssid from connectionDetailMap as absent so the DataElements value is used (empty string no longer masks it). Tests: add device_credentials_provider_test; update notifier test to model the real patched shape (master clients carry parentNodeName) as a regression guard; add empty-string fallback case and MasterNode.hostsDeviceId equality test. Note: slave-node client band display (#1044 follow-up) needs codegen YAML changes and is tracked separately — see doc/usp/dataelements-sta-band-enhancement.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Addressed review feedback — commit Verified all flagged issues against the code and pushed fixes. ✅ Fixed in this PR1. RA 2. Wired-client miscategorization (blocking) — 3. Test data helper in wrong folder (rule violation) — ✅ Also fixed — band/SSID fallback logic bug (found during review)4. Empty-string band masks the DataElements fallback — 🔀 Split out to a follow-up issue — #1118Slave-node WiFi client band display can't work today: Tests
Verification: |
|
Addressed review feedback — commit Verified all flagged issues against the code and pushed fixes. ✅ Fixed in this PR1. RA 2. Wired-client miscategorization (blocking) — 3. Test data helper in wrong folder (rule violation) — ✅ Also fixed — band/SSID fallback logic bug (found during review)4. Empty-string band masks the DataElements fallback — 🔀 Split out to a follow-up issue — #1118Slave-node WiFi client band display can't work today: Tests
Verification: |
AustinChangLinksys
left a comment
AustinChangLinksys
left a comment
There was a problem hiding this comment.
🤖 Automated Review — Round 4 · 784d17c..7f1c12f (incremental)
Verdict: 💬 Self-review (comment only) — GitHub prohibits self-approval. The two blocking issues from Round 3 (RA deviceUUID regression and wired-client miscategorization) are correctly fixed in this round. One carry-over Warning (stale wifiData snapshot) remains open; band/SSID empty-string fallback bug is now fixed.
| Conf. | Where | Issue (one-liner) | |
|---|---|---|---|
| High | devices_data_provider.dart:176,294 |
[CARRY-OVER W-2] Stale wifiData snapshot passed into fire-and-forget _fetchMeshAndUpdate — pre-existing, not introduced in this diff |
|
| ✅ | High | device_credentials_provider.dart:24-30 |
[FIXED] deviceUUID regression resolved — hostsDeviceId from Hosts table used; null/empty guard prevents wrong value to Guardian API |
| ✅ | High | usp_device_analytics_notifier.dart:248 |
[FIXED] Wired-client miscategorization resolved — parentNodeId correctly discriminates master vs slave clients; master wired no longer falls into gateway-name bucket |
| ✅ | High | mesh_network_builder.dart:293-294 |
[FIXED] Empty-string band/SSID fallback fixed — _nonEmpty() helper prevents empty-string from masking ?? fallback to DataElements value |
| ✅ | High | test/.../device_credentials_provider_test.dart |
New regression guard: 5 cases covering UUID used, null when no UUID, null when empty, null when no session, null when no devices |
| 💡 | Med | node_entity.dart:158 |
copyWith(hostsDeviceId:) cannot explicitly set field to null (nullable-param-without-sentinel pattern, consistent with rest of codebase) — minor limitation |
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-2 [CARRY-OVER, not in this diff] · High — Stale wifiData in fire-and-forget _fetchMeshAndUpdate
lib/page/devices/providers/devices_data_provider.dart:176 and :294
// Line 176: fire-and-forget — no await
_fetchMeshAndUpdate(svc, wifiData, gatewayName, sysData, result);
// ^^^^^^^^ snapshot from before the mesh fetch starts
// Line 294 (_refetchPreservingMesh): same pattern
_fetchMeshAndUpdate(svc, wifiData, gatewayName, sysData, result);_fetchMeshAndUpdate is called without await. By the time fetchMeshTopology() completes (potentially slow on larger mesh networks), wifiDataProvider may have emitted a fresh SSE-triggered state. The stale wifiData snapshot causes rebuilt device models to use outdated SSID names, bands, and connection details. Not introduced in this diff (pre-existing).
Austin split slave-band-display enhancement to #1118 (requires USP framework codegen changes). This stale-snapshot issue is a separate correctness concern affecting all mesh fetches, not dependent on #1118.
Fix (same as Round 3 suggestion): Re-read WiFi data after the mesh fetch completes inside _fetchMeshAndUpdate:
// After: final meshTopology = await svc.fetchMeshTopology(...)
final freshWifi = ref.read(wifiDataProvider).valueOrNull;
final effectiveWifi = freshWifi ?? wifiData; // prefer freshest availableWhat looks good
deviceUUIDfix is correct end-to-end:ConnectedDevice.deviceIdin the Hosts table is the Hosts DeviceID/UUID (not the MAC — that ismacAddress)._buildMasterNodecorrectly mapshostsDeviceId: masterDevice?.deviceId. The provider adds a null/empty guard before returning credentials. Regression testdevice_credentials_provider_test.dartexplicitly verifiesdeviceUUID != macAddress.patchedMasterpropagation:mesh_network_builder.dart:125correctly carrieshostsDeviceId: master.hostsDeviceIdthrough the patchedMaster rebuild, ensuring the fix is not lost in the multi-pass build.parentNodeIddiscrimination: The comment inusp_device_analytics_notifier.dart:243-246correctly explains whyparentNodeNameis unreliable (patched onto all clients by the builder) and whyparentNodeIdis the correct discriminant (null only for master clients). Test regression guard withparentNodeName: 'MyGateway'on master clients explicitly validatesbandDistribution.containsKey('MyGateway')is false._nonEmpty()helper: Clean and documented.mesh_network_builder_test.dartregression test correctly validates the fallback chain with empty-stringClientConnectionDetail.mesh_network_test_helper.dartdeletion: Confirmed unused (zero references acrosstest/); duplication withDevicesTestDatafactories correctly identified.hostsDeviceIdin Equatableprops: Correctly participates in equality (line 192).copyWithnull-passthrough pattern is consistent with all other nullable fields inMasterNode.- 3074 tests passing (per commit note).
flutter analyzeclean on changed files.
Cross-reviewed by two independent agents (security+correctness / architecture+maintainability). Automated — please sanity-check before merge.
|
Merged latest
Conflict resolution
Verification
PR is |
Integrate the dev-2.6.0 device multi-select filter + DeviceCategory feature onto this branch's MeshNetwork architecture (which removed DeviceUIModel / NodeUIModel / device_ui_extensions). Conflict resolutions: - device_filter_state.dart / device_filter_provider.dart: keep ClientDevice / NodeEntity types; port dev-2.6.0's multi-select + DeviceCategory + private-MAC filtering onto ClientDevice. DeviceConnectionType → ConnectionType. - usp_device_filter_panel.dart: DeviceConnectionType → ConnectionType, hide ui_kit's ConnectionType to avoid the name clash. - device_filter_provider_test.dart: rebuilt dev-2.6.0's multi-select test suite (49 tests incl. category / private-MAC / cross-dimension AND) on ClientDevice fixtures. - dhcp_data_provider_test / usp_dhcp_reservations_notifier_test / dhcp_reservation_edit_dialog_test / device_filter_state_test: migrated DeviceUIModel fixtures + DevicesData.deviceModels to MeshNetwork/ClientDevice; dropped const on non-const model constructors. flutter analyze: 0 errors. ./run_tests.sh: 3209 tests passed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
AustinChangLinksys
force-pushed
the
fix/device-analytics-display-1043-1044
branch
from
c7557aa to
dcd9bb2
Compare
|
🤖 Automated Review — Oversize PR This round's changes exceed the automated-review limit (8563 lines / 135 files, limit 6000 lines / 100 files); AI review was not run. Manual review recommended. First 15 changed files (for a quick scan): |
1 similar comment
|
🤖 Automated Review — Oversize PR This round's changes exceed the automated-review limit (8563 lines / 135 files, limit 6000 lines / 100 files); AI review was not run. Manual review recommended. First 15 changed files (for a quick scan): |
|
🤖 Automated Review — Oversize PR This round's changes exceed the automated-review limit (8563 lines / 135 files, First 15 changed files (for a quick scan): |
|
🤖 Automated Review — Oversize PR This round's changes exceed the automated-review limit (8563 lines / 135 files, limit 6000 lines / 100 files); AI review was not run. Manual review recommended. First 15 changed files (for a quick scan): |
|
🤖 Automated Review — Oversize PR This round's changes exceed the automated-review limit (8563 lines / 135 files, First 15 changed files (for a quick scan): |
2 participants
Summary
MeshNetworksealed class architecture as Single Source of Truth (SSoT) for mesh nodes and client devicesclientSignalMapChanges
New Architecture (Phase 1-5)
MeshNetworkcontainer withMasterNode,SlaveNode(sealed),ClientDeviceMeshNetworkBuilder— unified builder from ConnectedDevices + DataElementsDeviceUIModel/NodeUIModelafter migrating all consumersKey Files
lib/page/_shared/models/mesh_network.dart— SSoT containerlib/page/_shared/models/node_entity.dart— sealed MasterNode/SlaveNodelib/page/_shared/models/client_device.dart— withparentNodeNamefieldlib/page/_shared/utils/mesh_network_builder.dart— builder logiclib/page/dashboard/views/components/usp_device_analytics_card.dart— Y-axis fixWiFi Performance Card Enhancement
meshNetwork.allClientsas data source (includes slave node clients)clientBandSsidMaptoMeshTopologyInfofor slave client band/SSID resolutionbuildBssidToBandMap()to resolve BSSID → band via SSID LowerLayers → RadioSignal Resolution
WiFi.AccessPoint.*.AssociatedDeviceDataElements.Network.Device.*.Radio.*.BSS.*.STA.SignalStrength(RCPI→RSSI)Test plan
flutter analyzepasses./run_tests.shpasses (3067 tests)Related Issues
Closes #1043, #1044, #1047, #1048
🤖 Generated with Claude Code