[Refactor] Extract AddNodesService and AddWiredNodesService from Notifiers

[HankYuLinksys]

User description

Summary

• Extracts AddNodesService and AddWiredNodesService from their respective Notifiers to enforce three-layer architecture compliance
• Services encapsulate all JNAP communication, Providers delegate to Services
• Providers no longer import jnap/models, jnap/result, or jnap/actions directly
• Creates BackhaulInfoUIModel to replace JNAP model BackHaulInfoData in State layer
• Adds comprehensive test coverage: 119 tests (45 wired nodes, 74 nodes specific)

Changes

New Files

• lib/page/nodes/services/add_nodes_service.dart - Service for wireless node onboarding
• lib/page/nodes/services/add_wired_nodes_service.dart - Service for wired node onboarding
• lib/page/nodes/models/backhaul_info_ui_model.dart - UI model replacing JNAP BackHaulInfoData

Refactored Files

• lib/page/nodes/providers/add_nodes_provider.dart - Now delegates to AddNodesService
• lib/page/nodes/providers/add_wired_nodes_provider.dart - Now delegates to AddWiredNodesService
• lib/page/nodes/providers/add_wired_nodes_state.dart - Uses BackhaulInfoUIModel instead of BackHaulInfoData

Test Files

• test/page/nodes/services/add_nodes_service_test.dart - 21 tests
• test/page/nodes/services/add_wired_nodes_service_test.dart - 14 tests
• test/page/nodes/providers/add_nodes_provider_test.dart - 20 tests
• test/page/nodes/providers/add_wired_nodes_provider_test.dart - 12 tests
• test/page/nodes/providers/add_nodes_state_test.dart - 13 tests
• test/page/nodes/providers/add_wired_nodes_state_test.dart - 9 tests
• test/page/nodes/models/backhaul_info_ui_model_test.dart - 10 tests
• test/mocks/test_data/add_nodes_test_data.dart - Test data builders
• test/mocks/test_data/add_wired_nodes_test_data.dart - Test data builders

Architecture Compliance

✅ Provider imports NO jnap/actions (PASS)
✅ Provider imports NO jnap/models (PASS)
✅ Provider imports NO jnap/result (PASS)
✅ Service imports RouterRepository (PASS)
✅ Service maps JNAPError → ServiceError (PASS)

Test plan

• All 119 tests pass (flutter test test/page/nodes/)
• flutter analyze lib/page/nodes/ - No issues
• Architecture compliance grep checks pass

---

PR Type

Enhancement, Tests, Documentation

---

Description

• Extracts AddNodesService and AddWiredNodesService from their respective Notifiers to enforce three-layer architecture compliance

• Services encapsulate all JNAP communication; Providers delegate to Services

• Providers no longer import jnap/models, jnap/result, or jnap/actions directly

• Creates BackhaulInfoUIModel to replace JNAP model BackHaulInfoData in State layer

• Adds comprehensive test coverage: 119 tests across services, providers, states, and models

• Includes detailed specification documentation with contracts, data models, research findings, and implementation quickstart

• All architecture compliance checks pass; no direct JNAP imports in Provider layer

---

Diagram Walkthrough

flowchart LR
  A["Providers<br/>add_nodes_provider<br/>add_wired_nodes_provider"] -->|"delegates to"| B["Services<br/>AddNodesService<br/>AddWiredNodesService"]
  B -->|"uses"| C["JNAP Layer<br/>actions, models, result"]
  A -->|"manages"| D["State<br/>AddNodesState<br/>AddWiredNodesState"]
  D -->|"contains"| E["UI Models<br/>BackhaulInfoUIModel"]
  F["Test Data Builders<br/>add_nodes_test_data<br/>add_wired_nodes_test_data"] -->|"supports"| G["119 Unit Tests<br/>services, providers,<br/>states, models"]

Loading

File Walkthrough

	Relevant files
Enhancement	3 files add_nodes_service.dart Add Nodes Service Layer for Bluetooth Onboarding                  lib/page/nodes/services/add_nodes_service.dart New service class encapsulating JNAP communication for Bluetooth auto-onboarding Implements polling methods for auto-onboarding status, node online detection, and backhaul info Provides collectChildNodeData to merge backhaul info into device objects Maps JNAPError to ServiceError for consistent error handling +321/-0  add_wired_nodes_service.dart Add Wired Nodes Service Layer for Wired Onboarding              lib/page/nodes/services/add_wired_nodes_service.dart New service class for wired node auto-onboarding JNAP operations Implements pollBackhaulChanges with timestamp-based new node detection Provides fetchNodes to retrieve device list filtered by nodeType Returns BackhaulPollResult containing found count and onboarded status +250/-0  backhaul_info_ui_model.dart Create BackhaulInfoUIModel for State Layer                              lib/page/nodes/models/backhaul_info_ui_model.dart New UI model replacing JNAP BackHaulInfoData in state/provider layers Contains only essential fields: deviceUUID, connectionType, timestamp Implements serialization/deserialization and Equatable for state comparison Enforces architecture separation per constitution Article V Section 5.3.1 +40/-0
Refactoring	3 files add_nodes_provider.dart Refactor Add Nodes Provider to Use Service Layer                  lib/page/nodes/providers/add_nodes_provider.dart Refactored to delegate all JNAP operations to AddNodesService Removed direct imports of jnap/actions, jnap/models, jnap/result Simplified polling logic by using service stream methods Updated state management to use service-provided data structures +33/-200 add_wired_nodes_provider.dart Refactor Wired Nodes Provider to Use Service Layer              lib/page/nodes/providers/add_wired_nodes_provider.dart Refactored to delegate JNAP operations to AddWiredNodesService Removed direct imports of jnap/actions, jnap/models, jnap/result Updated _checkBackhaulChanges to use service polling with BackhaulInfoUIModel Simplified auto-onboarding flow by delegating to service methods +63/-234 add_wired_nodes_state.dart Update Wired Nodes State to Use BackhaulInfoUIModel            lib/page/nodes/providers/add_wired_nodes_state.dart Changed backhaulSnapshot field type from List to List Updated copyWith and serialization methods to use new UI model Removed import of JNAP BackHaulInfoData model +5/-5
Tests	11 files add_nodes_service_test.dart Add Nodes Service Unit Tests                                                          test/page/nodes/services/add_nodes_service_test.dart Comprehensive test suite with 21 tests covering all service methods Tests for setAutoOnboardingSettings, getAutoOnboardingSettings, pollAutoOnboardingStatus Tests for pollForNodesOnline, pollNodesBackhaulInfo, and collectChildNodeData Verifies error handling and stream transformations +407/-0  add_wired_nodes_service_test.dart Add Wired Nodes Service Unit Tests                                              test/page/nodes/services/add_wired_nodes_service_test.dart Test suite with 14 tests for wired nodes service methods Tests for setAutoOnboardingEnabled, getAutoOnboardingEnabled, pollBackhaulChanges Tests for fetchNodes with various scenarios including error handling Verifies polling configuration and backhaul change detection logic +301/-0  add_nodes_provider_test.dart Add Nodes Provider Unit Tests                                                        test/page/nodes/providers/add_nodes_provider_test.dart Test suite with 20 tests verifying provider delegates to service Tests for setAutoOnboardingSettings, getAutoOnboardingSettings, startAutoOnboarding Tests for startRefresh and state management with mocked service Verifies correct service method invocation order and state updates +344/-0  add_wired_nodes_provider_test.dart Add Wired Nodes Provider Unit Tests                                            test/page/nodes/providers/add_wired_nodes_provider_test.dart Test suite with 12 tests for wired nodes provider delegation Tests for service method delegation and error propagation Tests for startAutoOnboarding flow with mocked service Verifies state management and BackhaulInfoUIModel parameter handling +248/-0  add_nodes_state_test.dart Add Nodes State Unit Tests                                                              test/page/nodes/providers/add_nodes_state_test.dart Test suite with 13 tests for AddNodesState immutability and serialization Tests for copyWith, equality, and toJson/fromJson methods Tests for default values and props list for Equatable Verifies device list handling and state transitions +242/-0  add_wired_nodes_state_test.dart Add Wired Nodes State Unit Tests                                                  test/page/nodes/providers/add_wired_nodes_state_test.dart Test suite with 9 tests for AddWiredNodesState with BackhaulInfoUIModel Tests for backhaulSnapshot field accepting UI model lists Tests for copyWith, serialization, and Equatable behavior Verifies state immutability and model round-trip serialization +185/-0  backhaul_info_ui_model_test.dart Backhaul Info UI Model Unit Tests                                                test/page/nodes/models/backhaul_info_ui_model_test.dart Test suite with 10 tests for BackhaulInfoUIModel serialization and equality Tests for toMap/fromMap and toJson/fromJson methods Tests for Equatable implementation and props list Verifies model immutability and data integrity +146/-0  add_nodes_test_data.dart Add Nodes Test Data Builders                                                          test/mocks/test_data/add_nodes_test_data.dart Test data builder class with factory methods for creating JNAP mock responses Provides helpers for creating device data, connections, backhaul info, and error responses Centralizes test data creation to improve test readability and maintainability Includes builders for complete device scenarios with nested structures +243/-0  add_wired_nodes_test_data.dart Add Wired Nodes Test Data Builders                                              test/mocks/test_data/add_wired_nodes_test_data.dart Test data builder class for wired nodes service and provider tests Provides factory methods for creating backhaul devices, JNAP responses, and error scenarios Includes helpers for device creation and backhaul info structures Supports test scenarios with various connection types and timestamps +105/-0  backhaul_info_ui_model_test.dart BackhaulInfoUIModel serialization and equality tests          test/page/nodes/models/backhaul_info_ui_model_test.dart Comprehensive test suite for BackhaulInfoUIModel with 10 test cases covering Equatable equality, toMap/fromMap roundtrip, and toJson/fromJson serialization Tests verify that two instances with identical values are equal and have matching hash codes Tests validate roundtrip preservation of data through map and JSON serialization Tests confirm proper handling of missing fields with empty string defaults +146/-0  add_wired_nodes_test_data.dart Test data builders for wired nodes service testing              test/mocks/test_data/add_wired_nodes_test_data.dart Test data builder class providing factory methods for creating JNAP mock responses used in AddWiredNodesService tests Methods create success responses for wired auto-onboarding settings, backhaul info, and device lists Helper method createBackhaulDevice() generates backhaul device maps compatible with BackHaulInfoData.fromMap() Helper method createDevice() creates complete device maps with nested structures (connections, properties, unit, model, knownInterfaces) +105/-0
Documentation	9 files tasks.md Complete task breakdown for nodes service extraction          specs/001-add-nodes-service/tasks.md Comprehensive task breakdown organized into 12 phases covering both Bluetooth (AddNodesService) and wired (AddWiredNodesService) implementations Phase 1-6 document completed Bluetooth service extraction with 41 tasks marked complete Phase 7-12 document new wired service scope extension with 49 tasks for UI model, service implementation, state updates, and provider refactoring Includes detailed dependency graphs, parallel execution opportunities, and implementation strategies for single developers +476/-0  spec.md Feature specification for nodes service extraction              specs/001-add-nodes-service/spec.md Feature specification defining extraction of AddNodesService (Bluetooth) and AddWiredNodesService (wired) from their respective Notifiers Seven user stories covering architecture compliance, auto-onboarding operations, polling, testability, and state model compliance 28 functional requirements (FR-001 to FR-028) with 11 success criteria (SC-001 to SC-011) Edge cases and clarifications documented for both Bluetooth and wired implementations +220/-0  quickstart.md Implementation quickstart guide for nodes services              specs/001-add-nodes-service/quickstart.md Step-by-step implementation guide for AddNodesService (Bluetooth) with 6 steps covering service creation, test data builder, tests, provider refactoring, and architecture verification Extended quickstart for AddWiredNodesService (wired) with 6 additional steps including BackhaulInfoUIModel creation, service implementation, state updates, and compliance verification Code examples for each step showing patterns for service creation, test data builders, and refactoring patterns Common pitfalls and reference files documented +520/-0  data-model.md Data models and architecture flow documentation                    specs/001-add-nodes-service/data-model.md Data model documentation showing before/after architecture for AddNodesService (Bluetooth) with data flow diagrams Defines new BackhaulInfoUIModel entity with fields deviceUUID, connectionType, timestamp extending Equatable Defines BackhaulPollResult class containing backhaulList, foundCounting, and anyOnboarded for stream emissions Documents modified AddWiredNodesState replacing List with List +325/-0  research.md Design research and decision documentation                              specs/001-add-nodes-service/research.md Research findings addressing 11 key design questions about service patterns, error mapping, UI models, and timestamp comparison logic Decisions documented for stream-based operations, error mapping strategy, LinksysDevice location, and BackhaulInfoUIModel design Clarifies that timestamp comparison logic should move to AddWiredNodesService as data transformation responsibility References existing patterns from router_password_service.dart and constitution articles +289/-0  add_wired_nodes_service_contract.md AddWiredNodesService method contracts and specifications  specs/001-add-nodes-service/contracts/add_wired_nodes_service_contract.md Detailed contract specification for AddWiredNodesService with provider definition and class structure Four method contracts: setAutoOnboardingEnabled(), getAutoOnboardingEnabled(), pollBackhaulChanges(), and fetchNodes() Documents JNAP actions, authentication requirements, polling configurations, and error handling patterns Includes supporting models (BackhaulInfoUIModel, BackhaulPollResult), usage examples, and testing contract +298/-0  add_nodes_service_contract.md AddNodesService method contracts and specifications            specs/001-add-nodes-service/contracts/add_nodes_service_contract.md Detailed contract specification for AddNodesService (Bluetooth) with provider definition and class structure Six method contracts: setAutoOnboardingSettings(), getAutoOnboardingSettings(), pollAutoOnboardingStatus(), startAutoOnboarding(), pollForNodesOnline(), and pollNodesBackhaulInfo() Documents JNAP actions, authentication requirements, polling configurations with specific retry counts and delays Includes error handling contract, usage examples, and testing requirements +260/-0  plan.md Implementation plan and constitution compliance                    specs/001-add-nodes-service/plan.md Implementation plan for extracting AddNodesService (Bluetooth) and AddWiredNodesService (wired) from their respective Notifiers Technical context including language version, dependencies, testing framework, and performance goals Constitution compliance checklist confirming all articles (I, V, VI, VII, XI, XIII) are satisfied Project structure documenting file locations for source code, tests, and documentation +84/-0    requirements.md Specification quality and completeness checklist                  specs/001-add-nodes-service/checklists/requirements.md Quality checklist validating specification completeness with 23 checkpoints across content quality, requirement completeness, and feature readiness All items marked complete including no [NEEDS CLARIFICATION] markers, testable requirements, and measurable success criteria Scope extension validation confirming AddWiredNodesNotifier violations, new functional requirements (FR-015 to FR-028), and new success criteria (SC-007 to SC-011) Notes indicate spec is ready for planning phase +49/-0

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Sensitive data logging Description: Debug logging includes potentially sensitive device identifiers and network metadata (e.g., onboardedMACList MAC addresses and device.getDeviceLocation()), which could leak user/network information if logs are collected or exposed. add_nodes_service.dart [164-205] Referred Code logger.d( '[AddNodesService]: [pollForNodesOnline] Start by MACs: $onboardedMACList'); return _routerRepository .scheduledCommand( action: JNAPAction.getDevices, firstDelayInMilliSec: refreshing ? 1000 : 20000, retryDelayInMilliSec: refreshing ? 3000 : 20000, // Basic 3 minutes, add 2 minutes for each one more node maxRetry: refreshing ? 5 : 9 + onboardedMACList.length * 6, auth: true, condition: (result) { if (result is JNAPSuccess) { final deviceList = List.from(result.output['devices']) .map((e) => LinksysDevice.fromMap(e)) .where((device) => device.isAuthority == false) .toList(); // Check all MAC addresses can be found in the device list bool allFound = onboardedMACList.every((mac) => deviceList.any( (device) => ... (clipped 21 lines)
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
🔴	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Sensitive data in logs: New debug logging serializes full device JSON (likely including identifiers such as MAC addresses/device IDs) which risks leaking sensitive data into logs. Referred Code logger.d( '[AddNodes]: [pollForNodesOnline] added devices: ${addedDevices.map((d) => d.toJson()).join(', ')}'); } Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Missing audit context: The new service layer performs critical router state changes (auto-onboarding enable/start) without any explicit audit-trail fields (e.g., actor/user id, outcome) visible in this diff, so audit compliance cannot be confirmed. Referred Code Future<void> setAutoOnboardingSettings() async { try { await _routerRepository.send( JNAPAction.setBluetoothAutoOnboardingSettings, data: {'isAutoOnboardingEnabled': true}, auth: true, ); } on JNAPError catch (e) { throw mapJnapErrorToServiceError(e); } } /// Fetches current Bluetooth auto-onboarding setting /// /// Returns: /// - `true` if auto-onboarding is enabled /// - `false` if disabled or not configured /// /// Throws: /// - [NetworkError] if router communication fails /// - [UnauthorizedError] if authentication expired ... (clipped 83 lines) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Silent stream failures: The new polling streams drop non-success results without emitting an error or logging details, which may lead to silent failures and makes diagnosing edge cases harder. Referred Code Stream<BackhaulPollResult> pollBackhaulChanges( List<BackhaulInfoUIModel> snapshot, { bool refreshing = false, }) { final now = DateTime.now(); final dateFormat = DateFormat("yyyy-MM-ddThh:mm:ssZ"); bool anyOnboarded = false; return _routerRepository .scheduledCommand( action: JNAPAction.getBackhaulInfo, auth: true, firstDelayInMilliSec: 1 * 1000, retryDelayInMilliSec: 10 * 1000, maxRetry: refreshing ? 6 : 60, condition: (result) { if (result is! JNAPSuccess) { return false; } // Keep polling until timeout - no early termination return false; ... (clipped 53 lines) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Timestamp parsing assumptions: New logic relies on parsing router-provided timestamp strings and defaults parse failures to epoch (0), which could misclassify nodes without explicit validation or fallback handling shown in this diff. Referred Code final now = DateTime.now(); final dateFormat = DateFormat("yyyy-MM-ddThh:mm:ssZ"); bool anyOnboarded = false; return _routerRepository .scheduledCommand( action: JNAPAction.getBackhaulInfo, auth: true, firstDelayInMilliSec: 1 * 1000, retryDelayInMilliSec: 10 * 1000, maxRetry: refreshing ? 6 : 60, condition: (result) { if (result is! JNAPSuccess) { return false; } // Keep polling until timeout - no early termination return false; }, onCompleted: (_) { logger.i('[AddWiredNodesService]: poll backhaul info is completed'); }, ... (clipped 96 lines) Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Prevent infinite polling for backhaul info In pollNodesBackhaulInfo, add a check to prevent an infinite polling loop by returning an empty stream if the childNodes list is empty. lib/page/nodes/services/add_nodes_service.dart [242-293] Stream<List<BackHaulInfoData>> pollNodesBackhaulInfo( List<LinksysDevice> nodes, { bool refreshing = false, }) { final childNodes = nodes.where((e) => e.nodeType == 'Slave' && e.isOnline()).toList(); logger.d( '[AddNodesService]: [pollNodesBackhaulInfo] check child nodes backhaul info data: $childNodes'); + + // If there are no child nodes to check, return an empty stream to avoid + // polling indefinitely. + if (childNodes.isEmpty) { + return Stream.value([]); + } return _routerRepository .scheduledCommand( action: JNAPAction.getBackhaulInfo, firstDelayInMilliSec: refreshing ? 1000 : 3000, retryDelayInMilliSec: refreshing ? 3000 : 3000, maxRetry: refreshing ? 1 : 20, auth: true, condition: (result) { if (result is JNAPSuccess) { final backhaulList = List.from(result.output['backhaulDevices'] ?? []) .map((e) => BackHaulInfoData.fromMap(e)) .toList(); // Check all MAC addresses can be found in the backhaul list bool allFound = backhaulList.isNotEmpty && childNodes.every((n) => backhaulList .any((backhaul) => backhaul.deviceUUID == n.deviceID)); logger.d( '[AddNodesService]: [pollNodesBackhaulInfo] are All child deviceUUID in backhaul info data? $allFound'); return allFound; } return false; }, onCompleted: (_) { logger.d('[AddNodesService]: [pollNodesBackhaulInfo] Done!'); }, ) .transform( StreamTransformer<JNAPResult, List<BackHaulInfoData>>.fromHandlers( handleData: (result, sink) { if (result is JNAPSuccess) { final backhaulList = List.from(result.output['backhaulDevices'] ?? []) .map((e) => BackHaulInfoData.fromMap(e)) .toList(); sink.add(backhaulList); } }, ), ); } Apply / Chat Suggestion importance[1-10]: 8 __ Why: This suggestion correctly identifies a potential infinite loop when childNodes is empty, which could lead to excessive network requests and timeouts. Adding a guard clause is a critical fix for this edge case.	Medium
	Enforce data contract in fromMap In the fromMap factory, enforce the data contract by casting required fields to String instead of defaulting to an empty string. This ensures model validity and surfaces data issues early by throwing an error for missing or null values. lib/page/nodes/models/backhaul_info_ui_model.dart [29-34] factory BackhaulInfoUIModel.fromMap(Map<String, dynamic> map) => BackhaulInfoUIModel( - deviceUUID: map['deviceUUID'] ?? '', - connectionType: map['connectionType'] ?? '', - timestamp: map['timestamp'] ?? '', + deviceUUID: map['deviceUUID'] as String, + connectionType: map['connectionType'] as String, + timestamp: map['timestamp'] as String, ); Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly points out that defaulting to empty strings can hide data integrity issues. Enforcing the data contract by throwing an error on missing required fields is a more robust, "fail-fast" approach, even though the PR author intentionally tested for the default behavior.	Medium
General	Use fixed timestamp for test data In createBackhaulDevice, replace the non-deterministic DateTime.now() with a fixed timestamp string to ensure test repeatability and prevent flakiness. test/mocks/test_data/add_wired_nodes_test_data.dart [55] -'timestamp': timestamp ?? DateTime.now().toIso8601String(), +'timestamp': timestamp ?? '2026-01-07T12:00:00.000Z', Apply / Chat Suggestion importance[1-10]: 8 __ Why: This is a critical improvement for test stability. Using DateTime.now() in test data makes tests non-deterministic, which is a significant issue for time-sensitive logic like the timestamp comparison mentioned in the PR's documentation.	Medium
Possible issue	Always clean up onboarding state Wrap the asynchronous operations within a try/finally block to guarantee that cleanup functions like stopAutoOnboarding() are always executed, even if an error occurs during the process. lib/page/nodes/providers/add_wired_nodes_provider.dart [47-87] Future startAutoOnboarding(BuildContext context) async { logger.d('[AddWiredNode]: start auto onboarding'); final log = BenchMarkLogger(name: 'Add Wired Node Process')..start(); ref.read(idleCheckerPauseProvider.notifier).state = true; ref.read(pollingProvider.notifier).stopPolling(); state = state.copyWith( isLoading: true, loadingMessage: loc(context).addNodesOnboardingStart); - // Set router auto onboarding to true - await setAutoOnboardingSettings(true); - if (!context.mounted) return; - // Get backhaul info from deviceManager and convert to UI model - final backhaulData = ref.read(deviceManagerProvider).backhaulInfoData; - final backhaulSnapshot = backhaulData - .map((data) => BackhaulInfoUIModel( - deviceUUID: data.deviceUUID, - connectionType: data.connectionType, - timestamp: data.timestamp, - )) - .toList(); - state = state.copyWith(backhaulSnapshot: backhaulSnapshot); + try { + await setAutoOnboardingSettings(true); + if (!context.mounted) return; - logger.d('[AddWiredNode]: check backhaul changes'); - await _checkBackhaulChanges(context, backhaulSnapshot); - // Set router auto onboarding to false - await stopAutoOnboarding(); - // Fetch latest status - logger.d('[AddWiredNode]: fetch nodes'); - final nodes = await _service.fetchNodes(); - if (!context.mounted) return; - state = state.copyWith(nodes: nodes); - stopCheckingBackhaul(context); + final backhaulData = ref.read(deviceManagerProvider).backhaulInfoData; + final backhaulSnapshot = backhaulData + .map((data) => BackhaulInfoUIModel( + deviceUUID: data.deviceUUID, + connectionType: data.connectionType, + timestamp: data.timestamp, + )) + .toList(); + state = state.copyWith(backhaulSnapshot: backhaulSnapshot); - // End - log.end(); - final delta = log.delta; - logger.d('[AddWiredNode]: end auto onboarding, cost time: ${delta}ms'); + logger.d('[AddWiredNode]: check backhaul changes'); + await _checkBackhaulChanges(context, backhaulSnapshot); + + logger.d('[AddWiredNode]: fetch nodes'); + final nodes = await _service.fetchNodes(); + if (!context.mounted) return; + state = state.copyWith(nodes: nodes); + } finally { + await stopAutoOnboarding(); + if (context.mounted) { + stopCheckingBackhaul(context); + } + log.end(); + final delta = log.delta; + logger.d('[AddWiredNode]: end auto onboarding, cost time: ${delta}ms'); + } } Apply / Chat Suggestion importance[1-10]: 9 __ Why: This suggestion addresses a critical flaw in error handling by proposing a try/finally block. This ensures essential cleanup logic runs, preventing the router and application from being left in an inconsistent state after an exception, which is a significant improvement to the code's robustness.	High
	Prevent false new-node counts Fix a bug in timestamp parsing by using DateTime.tryParse instead of an incorrect DateFormat string. Additionally, refine the logic to only count new nodes with a connectionType of 'Wired' to prevent false positives. lib/page/nodes/services/add_wired_nodes_service.dart [104-224] Stream<BackhaulPollResult> pollBackhaulChanges( List<BackhaulInfoUIModel> snapshot, { bool refreshing = false, }) { - final now = DateTime.now(); - final dateFormat = DateFormat("yyyy-MM-ddThh:mm:ssZ"); + final pollStartTime = DateTime.now(); bool anyOnboarded = false; return _routerRepository .scheduledCommand( action: JNAPAction.getBackhaulInfo, auth: true, firstDelayInMilliSec: 1 * 1000, retryDelayInMilliSec: 10 * 1000, maxRetry: refreshing ? 6 : 60, condition: (result) { if (result is! JNAPSuccess) { return false; } - // Keep polling until timeout - no early termination return false; }, onCompleted: (_) { logger.i('[AddWiredNodesService]: poll backhaul info is completed'); }, ) .transform( StreamTransformer<JNAPResult, BackhaulPollResult>.fromHandlers( handleData: (result, sink) { if (result is JNAPSuccess) { final backhaulInfoList = List.from( result.output['backhaulDevices'] ?? [], ).map((e) => BackHaulInfoData.fromMap(e)).toList(); - // Calculate found counting - detect new wired nodes final foundCounting = backhaulInfoList.fold<int>(0, (value, infoData) { - // Find nodes that are: - // 1. Connection type is "Wired" - // 2. Timestamp is after poll start time (now) - // 3. Either new UUID or timestamp newer than snapshot + if (infoData.connectionType != 'Wired') { + return value; + } final isNewNode = !_isExistingWiredNode( infoData, snapshot, - now, - dateFormat, + pollStartTime, ); return isNewNode ? value + 1 : value; }); if (foundCounting > 0) { anyOnboarded = true; } logger.i('[AddWiredNodesService]: Found $foundCounting new nodes'); - // Convert to UI models final uiModels = backhaulInfoList .map((data) => BackhaulInfoUIModel( deviceUUID: data.deviceUUID, connectionType: data.connectionType, timestamp: data.timestamp, )) .toList(); sink.add(BackhaulPollResult( backhaulList: uiModels, foundCounting: foundCounting, anyOnboarded: anyOnboarded, )); } }, ), ); } bool _isExistingWiredNode( BackHaulInfoData infoData, List<BackhaulInfoUIModel> snapshot, DateTime pollStartTime, - DateFormat dateFormat, ) { - // Check if this device was in the snapshot final snapshotEntry = snapshot.firstWhere( (e) => e.deviceUUID == infoData.deviceUUID, orElse: () => const BackhaulInfoUIModel( deviceUUID: '', connectionType: '', timestamp: '', ), ); - // If not found in snapshot, it's new if (snapshotEntry.deviceUUID.isEmpty) { return false; } - // Must be wired connection if (infoData.connectionType != 'Wired') { return false; } final infoTimestamp = - dateFormat.tryParse(infoData.timestamp)?.millisecondsSinceEpoch ?? 0; + DateTime.tryParse(infoData.timestamp)?.millisecondsSinceEpoch ?? 0; final snapshotTimestamp = - dateFormat.tryParse(snapshotEntry.timestamp)?.millisecondsSinceEpoch ?? - 0; + DateTime.tryParse(snapshotEntry.timestamp)?.millisecondsSinceEpoch ?? 0; final nowTimestamp = pollStartTime.millisecondsSinceEpoch; - // Check timestamps: - // - Info timestamp must be before poll start (now) - // - Info timestamp must be same or earlier than snapshot timestamp return nowTimestamp > infoTimestamp && infoTimestamp <= snapshotTimestamp; } Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a bug in the date parsing logic by using hh instead of HH, and proposes the robust DateTime.tryParse fix. It also correctly points out a logical flaw where non-wired nodes would be counted as new, improving the accuracy of the feature.	Medium
	Handle missing devices payload safely Safely handle the JNAP response by using the null-aware operator (?? []) on result.output['devices']. This prevents potential crashes if the devices field is null in the API response. lib/page/nodes/services/add_nodes_service.dart [160-229] Stream<List<LinksysDevice>> pollForNodesOnline( List<String> onboardedMACList, { bool refreshing = false, }) { logger.d( '[AddNodesService]: [pollForNodesOnline] Start by MACs: $onboardedMACList'); return _routerRepository .scheduledCommand( action: JNAPAction.getDevices, firstDelayInMilliSec: refreshing ? 1000 : 20000, retryDelayInMilliSec: refreshing ? 3000 : 20000, - // Basic 3 minutes, add 2 minutes for each one more node maxRetry: refreshing ? 5 : 9 + onboardedMACList.length * 6, auth: true, condition: (result) { if (result is JNAPSuccess) { - final deviceList = List.from(result.output['devices']) + final deviceList = List.from(result.output['devices'] ?? []) .map((e) => LinksysDevice.fromMap(e)) .where((device) => device.isAuthority == false) .toList(); - // Check all MAC addresses can be found in the device list bool allFound = onboardedMACList.every((mac) => deviceList.any( (device) => device.nodeType == 'Slave' && (device.knownInterfaces?.any((knownInterface) => knownInterface.macAddress == mac) ?? false))); logger.d( '[AddNodesService]: [pollForNodesOnline] are All MACs in device list? $allFound'); - // Check if nodes in the mac list all have connections bool ret = deviceList .where((device) => device.nodeType == 'Slave' && (device.knownInterfaces?.any((knownInterface) => onboardedMACList .contains(knownInterface.macAddress)) ?? false)) .every((device) { final hasConnections = device.isOnline(); logger.d( '[AddNodesService]: [pollForNodesOnline] <${device.getDeviceLocation()}> has connections: $hasConnections'); return hasConnections; }); return allFound && ret; } return false; }, onCompleted: (_) { logger.d('[AddNodesService]: [pollForNodesOnline] Done!'); }, ) .transform( StreamTransformer<JNAPResult, List<LinksysDevice>>.fromHandlers( handleData: (result, sink) { if (result is JNAPSuccess) { - final deviceList = List.from(result.output['devices']) + final deviceList = List.from(result.output['devices'] ?? []) .map((e) => LinksysDevice.fromMap(e)) .where((device) => device.nodeType != null) .toList(); sink.add(deviceList); } }, ), ); } Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a potential null pointer exception if the devices key is missing from the API response and provides a simple, effective fix. This improves the robustness of the polling logic by preventing the stream from crashing due to an unexpected payload.	Medium
	Validate JSON shape before mapping Add a type check in the fromJson factory to validate that the decoded JSON is a map before deserialization, preventing potential runtime errors from malformed data. lib/page/nodes/models/backhaul_info_ui_model.dart [38-39] -factory BackhaulInfoUIModel.fromJson(String source) => - BackhaulInfoUIModel.fromMap(json.decode(source)); +factory BackhaulInfoUIModel.fromJson(String source) { + final decoded = json.decode(source); + if (decoded is! Map) { + throw const FormatException('BackhaulInfoUIModel JSON must be an object'); + } + return BackhaulInfoUIModel.fromMap(Map<String, dynamic>.from(decoded)); +} Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that json.decode can return a non-map value, which would cause a runtime crash, and proposes a robust check to handle this case, improving the code's resilience.	Medium
	Normalize decoded snapshot element types In the AddWiredNodesState.fromMap factory, add type checks for elements within the backhaulSnapshot list to handle both Map and BackhaulInfoUIModel instances safely during deserialization. lib/page/nodes/providers/add_wired_nodes_state.dart [69-72] backhaulSnapshot: map['backhaulSnapshot'] != null - ? List<BackhaulInfoUIModel>.from(map['backhaulSnapshot'] - ?.map((x) => BackhaulInfoUIModel.fromMap(x))) + ? List<BackhaulInfoUIModel>.from( + (map['backhaulSnapshot'] as List).map((x) { + if (x is BackhaulInfoUIModel) return x; + if (x is Map) { + return BackhaulInfoUIModel.fromMap(Map<String, dynamic>.from(x)); + } + return BackhaulInfoUIModel.fromMap(const <String, dynamic>{}); + }), + ) : null, Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly points out that deserializing a list of dynamic objects without type checking is unsafe and can lead to runtime crashes. The proposed change adds robust type handling for each element, making the state hydration process more resilient.	Medium
More