Refactor providers - nodes and connectivity

[HankYuLinksys]

User description

• Create NodeLightSettingsService for JNAP communication
  - fetchSettings() with forceRemote parameter
  - saveSettings() with auto re-fetch after save
  - Error mapping to ServiceError types

  • Refactor NodeLightSettingsNotifier to delegate to service
    • Remove direct JNAP imports (better_action.dart, router_repository.dart)
    • Keep currentStatus getter in Provider (UI transformation logic)
  • Add comprehensive test coverage
    • 16 model tests for NodeLightSettings
    • 9 service tests for NodeLightSettingsService
    • 10 provider tests for delegation and currentStatus

• Create NodeDetailService with LED blink methods (startBlinkNodeLED, stopBlinkNodeLED)

  • Add transformation helpers (transformDeviceToUIValues, transformConnectedDevices)
  • Refactor NodeDetailNotifier to delegate JNAP operations to Service
  • Remove JNAP imports from node_detail_provider.dart and node_detail_state.dart
  • Add currentStatus getter to NodeLightSettingsNotifier for architecture compliance
  • Update quick_panel.dart and node_detail_view.dart to use Provider getter instead of Service
  • Move NodeLightStatus.getStatus() logic from State to Provider layer
  • Add comprehensive unit tests for Service, Provider, and State

• Create ConnectivityService class with testRouterType() and fetchRouterConfiguredData() methods

  • Move JNAP communication logic from Provider to Service layer
  • Remove JNAP imports from connectivity_provider.dart (jnap/models, jnap/result, jnap/actions)
  • Add ServiceError mapping via mapJnapErrorToServiceError() for fetchRouterConfiguredData
  • Add null safety handling for isDefaultPassword and isSetByUser fields
  • Add unit tests for ConnectivityService (12 tests)
  • Add unit tests for ConnectivityNotifier delegation (7 tests)
  • Add unit tests for ConnectivityState, ConnectivityInfo, AvailabilityInfo (28 tests)
  • Create test data builder at test/mocks/test_data/connectivity_test_data.dart

---

PR Type

Enhancement, Tests, Documentation

---

Description

• Create three new service classes for JNAP communication delegation:

  • NodeLightSettingsService: LED night mode settings fetch/save with auto re-fetch
  • NodeDetailService: Node LED blinking and device transformation helpers
  • ConnectivityService: Router type detection and configuration status checking

• Refactor three provider notifiers to delegate JNAP operations to service layer:

  • NodeLightSettingsNotifier: Remove JNAP imports, add currentStatus getter
  • NodeDetailNotifier: Delegate device transformation and LED blinking to service
  • ConnectivityNotifier: Delegate router type and configuration checks to service

• Update UI components to use provider currentStatus getter instead of static methods:

  • quick_panel.dart and node_detail_view.dart now read from provider notifier

• Add comprehensive test coverage across all layers:

  • 9 service tests for NodeLightSettingsService
  • 8 service tests for NodeDetailService
  • 12 service tests for ConnectivityService
  • 10 provider tests for NodeLightSettingsNotifier
  • 10 provider tests for NodeDetailNotifier
  • 7 provider tests for ConnectivityNotifier
  • 16 model tests for NodeLightSettings
  • 16 state tests for NodeDetailState
  • 28 state tests for connectivity models

• Create test data builders for service mocking and test scenarios

• Document specifications with contracts, task breakdowns, quickstart guides, and implementation plans for all three services

---

Diagram Walkthrough

flowchart LR
  UI["UI Components<br/>quick_panel, node_detail_view"]
  Provider["Provider Layer<br/>NodeLightSettingsNotifier<br/>NodeDetailNotifier<br/>ConnectivityNotifier"]
  Service["Service Layer<br/>NodeLightSettingsService<br/>NodeDetailService<br/>ConnectivityService"]
  Repo["RouterRepository<br/>JNAP Communication"]
  
  UI -- "read currentStatus" --> Provider
  Provider -- "delegate operations" --> Service
  Service -- "JNAP actions" --> Repo
  Service -- "error mapping" --> Provider

Loading

File Walkthrough

	Relevant files
Enhancement	5 files node_light_settings_service.dart New NodeLightSettingsService for LED settings JNAP communication lib/core/jnap/services/node_light_settings_service.dart Created new stateless service for LED night mode settings JNAP operations Implements fetchSettings() with optional forceRemote parameter for cache control Implements saveSettings() that persists settings and auto re-fetches from device Maps JNAP errors to ServiceError types (UnauthorizedError, UnexpectedError) +90/-0    node_detail_service.dart New NodeDetailService with LED blink and device transformation methods lib/page/nodes/services/node_detail_service.dart Created new stateless service for node detail operations and LED blinking Implements startBlinkNodeLED() and stopBlinkNodeLED() methods with error mapping Provides transformDeviceToUIValues() helper to convert JNAP device data to UI primitives Provides transformConnectedDevices() helper and getNodeLightStatus() transformation method +143/-0  connectivity_service.dart New ConnectivityService for router type and configuration detection lib/providers/connectivity/services/connectivity_service.dart Created new stateless service for connectivity-related JNAP operations Implements testRouterType() to detect router connection type via serial number comparison Implements fetchRouterConfiguredData() to check router password configuration status Maps JNAP errors to ServiceError types with graceful error handling +102/-0  quick_panel.dart Update quick_panel to use provider currentStatus getter    lib/page/dashboard/views/components/quick_panel.dart Updated to use nodeLightSettingsProvider.notifier.currentStatus getter instead of NodeLightStatus.getStatus() Removed dependency on static method for status transformation Improved code by reading status from provider notifier +6/-2      node_detail_view.dart Update node_detail_view to use provider currentStatus getter lib/page/nodes/views/node_detail_view.dart Updated to use nodeLightSettingsProvider.notifier.currentStatus getter instead of NodeLightStatus.getStatus() Removed unnecessary watch of nodeLightSettingsProvider state Simplified status retrieval by reading from provider notifier +3/-2
Refactoring	3 files node_light_settings_provider.dart Refactor NodeLightSettingsNotifier to delegate to service layer lib/core/jnap/providers/node_light_settings_provider.dart Refactored NodeLightSettingsNotifier to delegate JNAP operations to NodeLightSettingsService Removed direct JNAP imports (JNAPAction, routerRepositoryProvider) Added currentStatus getter to provide UI-friendly NodeLightStatus enum derived from settings Simplified fetch() and save() methods to call service layer +19/-21  node_detail_provider.dart Refactor NodeDetailNotifier to delegate to NodeDetailService lib/page/nodes/providers/node_detail_provider.dart Refactored NodeDetailNotifier.createState() to delegate device transformation to NodeDetailService Removed direct JNAP imports and device utility logic from provider Updated toggleBlinkNode() to use service methods with improved error handling for ServiceError Simplified state creation by using service's transformDeviceToUIValues() and transformConnectedDevices() +55/-86  connectivity_provider.dart Refactor ConnectivityNotifier to delegate to ConnectivityService lib/providers/connectivity/connectivity_provider.dart Refactored ConnectivityNotifier._testRouterType() to delegate to ConnectivityService Refactored isRouterConfigured() to delegate to ConnectivityService.fetchRouterConfiguredData() Removed direct JNAP imports and communication logic from provider Simplified methods to act as thin delegation layer to service +16/-51
Tests	12 files node_light_settings_service_test.dart Add comprehensive tests for NodeLightSettingsService          test/core/jnap/services/node_light_settings_service_test.dart Added 9 comprehensive unit tests for NodeLightSettingsService Tests cover fetchSettings() with default and forced remote fetch Tests cover saveSettings() with null field exclusion and auto re-fetch Tests verify error handling for UnauthorizedError and UnexpectedError +234/-0  node_detail_service_test.dart Add comprehensive tests for NodeDetailService                        test/page/nodes/services/node_detail_service_test.dart Added 8 comprehensive unit tests for NodeDetailService Tests cover startBlinkNodeLED() and stopBlinkNodeLED() JNAP communication Tests verify correct parameters passed to repository Tests verify error handling for UnauthorizedError and UnexpectedError +223/-0  connectivity_service_test.dart Add comprehensive tests for ConnectivityService                    test/providers/connectivity/connectivity_service_test.dart Added 12 comprehensive unit tests for ConnectivityService Tests cover testRouterType() with serial number matching scenarios Tests cover fetchRouterConfiguredData() with various password states Tests verify error handling and edge cases (null gateway IP, missing serial number) +353/-0  node_detail_provider_test.dart Add comprehensive tests for NodeDetailNotifier                      test/page/nodes/providers/node_detail_provider_test.dart Added 10 comprehensive unit tests for NodeDetailNotifier Tests cover createState() delegation to service transformation methods Tests cover toggleBlinkNode() with start/stop scenarios and error handling Tests verify service method calls and state updates +428/-0  node_light_settings_provider_test.dart Add comprehensive tests for NodeLightSettingsNotifier        test/core/jnap/providers/node_light_settings_provider_test.dart Added 10 comprehensive unit tests for NodeLightSettingsNotifier Tests cover delegation of fetch() and save() to service Tests cover currentStatus getter with all status scenarios (on, off, night) Tests verify state updates after successful service operations +198/-0  connectivity_provider_test.dart Add comprehensive tests for ConnectivityNotifier                  test/providers/connectivity/connectivity_provider_test.dart Added 7 comprehensive unit tests for ConnectivityNotifier Tests cover delegation of isRouterConfigured() to service Tests verify error propagation from service layer Tests verify service provider dependency injection +180/-0  node_light_settings_test.dart Add comprehensive tests for NodeLightSettings model            test/core/jnap/models/node_light_settings_test.dart Added 16 comprehensive unit tests for NodeLightSettings model Tests cover fromMap() and toMap() serialization with null field handling Tests cover factory constructors (on(), off(), night()) and fromStatus() Tests cover copyWith(), equality, and JSON roundtrip serialization +191/-0  node_detail_state_test.dart Add comprehensive tests for NodeDetailState                            test/page/nodes/providers/node_detail_state_test.dart Added 16 comprehensive unit tests for NodeDetailState Tests cover copyWith() with partial and full updates Tests cover equality, JSON serialization, and BlinkingStatus enum Tests verify default values and state immutability +261/-0  connectivity_state_test.dart Add comprehensive tests for connectivity state models        test/providers/connectivity/connectivity_state_test.dart Added 28 comprehensive unit tests for connectivity state models Tests cover ConnectivityState, ConnectivityInfo, AvailabilityInfo, and RouterType Tests verify copyWith(), equality, and default values for all models Tests cover serialization and enum behavior +392/-0  node_light_settings_test_data.dart Add test data builder for NodeLightSettingsService              test/mocks/test_data/node_light_settings_test_data.dart Created test data builder for NodeLightSettingsService tests Provides factory methods for JNAP mock responses (night mode, on, off, save) Provides error response builders for unauthorized and unexpected errors +61/-0    node_detail_test_data.dart Add test data builder for NodeDetailService                            test/mocks/test_data/node_detail_test_data.dart Created test data builder for NodeDetailService tests Provides factory methods for JNAP mock responses (blink start, blink stop) Provides error response builders for unauthorized and unexpected errors +31/-0    connectivity_test_data.dart Add test data builder for ConnectivityService                        test/mocks/test_data/connectivity_test_data.dart Created test data builder for ConnectivityService tests Provides factory methods for getDeviceInfo and fetchIsConfigured JNAP responses Supports partial override pattern with named parameters for flexible test scenarios +107/-0
Documentation	25 files node_light_settings_service_contract.md Add service contract specification for NodeLightSettingsService specs/002-node-light-settings-service/contracts/node_light_settings_service_contract.md Created service contract specification for NodeLightSettingsService Documents class definition, provider definition, and method contracts Includes JNAP action mapping, usage examples, and comprehensive test scenarios Defines error mapping strategy and provider delegation pattern +182/-0  tasks.md ConnectivityService extraction tasks and execution plan    specs/001-connectivity-service/tasks.md Created comprehensive task breakdown for ConnectivityService extraction with 31 tasks organized into 6 phases Defined parallel execution opportunities and dependency chains for efficient implementation Included detailed success criteria mapping and implementation strategy with MVP-first approach Established test-first methodology with explicit task ordering for User Stories 1-3 +236/-0  tasks.md NodeLightSettings service extraction implementation tasks specs/002-node-light-settings-service/tasks.md Created 18 implementation tasks organized into 6 phases for NodeLightSettingsService extraction Defined parallel opportunities for model and service tests before implementation Included detailed success criteria verification and architecture compliance checks Established incremental delivery strategy with clear phase dependencies +275/-0  tasks.md NodeDetail service extraction comprehensive task breakdown specs/001-node-detail-service/tasks.md Created 36 implementation tasks across 6 phases for NodeDetailService extraction Defined parallel execution opportunities within each phase (14 tasks can run in parallel) Included comprehensive test coverage requirements and architecture compliance verification Established task-to-requirement traceability and success criteria mapping +228/-0  node_detail_service_contract.md NodeDetailService API contract and method specifications  specs/001-node-detail-service/contracts/node_detail_service_contract.md Defined complete API contract for NodeDetailService with provider definition and class structure Documented four public methods: startBlinkNodeLED, stopBlinkNodeLED, transformDeviceToUIValues, transformConnectedDevices Specified error handling strategy using mapJnapErrorToServiceError() for JNAP error mapping Included detailed usage examples and integration patterns for Provider layer +252/-0  spec.md NodeDetail service feature specification and requirements specs/001-node-detail-service/spec.md Defined feature specification with three user stories (P1, P1, P2 priorities) Established 11 functional requirements covering service creation, JNAP delegation, and error handling Specified 7 measurable success criteria including architecture compliance and test coverage targets Documented edge cases and clarifications from specification sessions +113/-0  quickstart.md ConnectivityService extraction quickstart implementation guide specs/001-connectivity-service/quickstart.md Provided step-by-step implementation guide for ConnectivityService extraction in 6 steps Included code examples for service creation, method implementation, and provider refactoring Documented verification checklist and common issues with solutions Referenced existing service patterns and error mapping utilities +228/-0  quickstart.md NodeDetail service extraction quickstart guide                      specs/001-node-detail-service/quickstart.md Created 10-step implementation guide for NodeDetailService extraction Included code examples for service file creation, JNAP method implementation, and transformation helpers Documented architecture compliance verification commands and test creation patterns Provided verification checklist and common pitfalls with solutions +210/-0  research.md NodeDetail service extraction research and technical decisions specs/001-node-detail-service/research.md Documented research findings on service pattern implementation following RouterPasswordService reference Analyzed error mapping strategy using centralized mapJnapErrorToServiceError() function Clarified state transformation approach with service providing helpers and provider creating state Identified JNAP actions to extract and analyzed dependency imports to remove/add +202/-0  spec.md ConnectivityService feature specification and requirements specs/001-connectivity-service/spec.md Defined feature specification for ConnectivityService extraction with three user stories (all P1) Established 10 functional requirements covering service creation, JNAP delegation, and error mapping Specified 5 measurable success criteria including architecture compliance and test coverage Documented edge cases and assumptions for router connectivity detection +106/-0  spec.md NodeLightSettings service feature specification                    specs/002-node-light-settings-service/spec.md Defined feature specification with three user stories (all P1 priorities) Established 11 functional requirements covering service creation, settings fetch/save, and error handling Specified 7 measurable success criteria including zero JNAP imports and test coverage targets Documented edge cases for network failures and state consistency +111/-0  service-contract.md Service contract quality assurance checklist                          specs/001-node-detail-service/checklists/service-contract.md Created 32-item quality checklist for service contract validation Organized checklist into 9 categories: method signatures, error handling, statelessness, dependency injection, transformation helpers, API consistency, boundary conditions, contract-spec alignment, and testability Included specific references to contract sections and specification requirements Provided summary table mapping checklist items to quality dimensions +103/-0  data-model.md NodeDetail service data model classification and mapping  specs/001-node-detail-service/data-model.md Classified existing data models by layer: Data Layer (JNAP models), Application Layer (Service/Provider), Error Layer Documented field mapping from RawDevice to NodeDetailState with transformation logic Specified required modification: remove NodeLightSettings import from state file Illustrated data transformation flow from JNAP models through Service to Provider state +135/-0  data-model.md ConnectivityService data model and entity definitions        specs/001-connectivity-service/data-model.md Defined four entities: RouterType enum, RouterConfiguredData class, ConnectivityService class, and provider definition Documented state transitions for router type detection with decision flow diagram Specified validation rules for data fields with enforcement mechanisms Illustrated relationships between ConnectivityNotifier, Service, and data models +156/-0  plan.md NodeDetail service implementation plan and constitution compliance specs/001-node-detail-service/plan.md Provided implementation plan with technical context and constitution compliance checks Documented project structure for source code and tests with file organization Established pre-design and post-design constitution gate checks with all articles verified Included complexity tracking and decision rationale +96/-0    connectivity_service_contract.md ConnectivityService API contract and method specifications specs/001-connectivity-service/contracts/connectivity_service_contract.md Defined complete API contract for ConnectivityService with provider definition Documented two public methods: testRouterType() and fetchRouterConfiguredData() with detailed signatures Specified error handling behavior (never throws for testRouterType, throws ServiceError for fetchRouterConfiguredData) Included integration patterns for ConnectivityNotifier and comprehensive testing contract +175/-0  research.md ConnectivityService extraction research and technical decisions specs/001-connectivity-service/research.md Documented research findings on service layer patterns following existing implementations Analyzed error handling approach using centralized mapJnapErrorToServiceError() function Clarified RouterConfiguredData location decision and SharedPreferences dependency handling Reviewed existing code logic for _testRouterType() and isRouterConfigured() extraction +125/-0  data-model.md NodeLightSettings service data model and entity definitions specs/002-node-light-settings-service/data-model.md Documented existing entities with no changes required: NodeLightSettings, NodeLightStatus enum Defined new NodeLightSettingsService entity with stateless design and two core methods Specified JNAP action mapping and error mapping strategy Illustrated data flow from View through Provider and Service to RouterRepository +108/-0  requirements.md NodeLightSettings specification quality validation checklist specs/002-node-light-settings-service/checklists/requirements.md Created specification quality checklist with 8 validation categories Confirmed all mandatory sections completed and no clarification markers remain Verified requirements are testable, unambiguous, and technology-agnostic Validated feature readiness for planning phase +38/-0    quickstart.md NodeLightSettings Service Extraction Quickstart Guide        specs/002-node-light-settings-service/quickstart.md Provides step-by-step implementation guide for extracting NodeLightSettingsService from the notifier Includes code templates for service creation, provider refactoring, and test structure Documents verification commands to ensure JNAP imports are removed and tests pass Contains file checklist and success criteria for architecture compliance +133/-0  plan.md NodeLightSettings Service Extraction Implementation Plan  specs/002-node-light-settings-service/plan.md Outlines implementation plan for service extraction with technical context and dependencies Defines project structure showing new service file location and test organization Includes constitution check validating three-layer architecture compliance Tracks complexity metrics indicating low risk refactoring task +89/-0    plan.md ConnectivityService Extraction Implementation Plan              specs/001-connectivity-service/plan.md Establishes implementation plan for ConnectivityService extraction from provider Details technical context including Dart/Flutter versions and testing framework Maps constitution requirements to implementation tasks for architecture compliance Defines project structure for service placement in lib/providers/connectivity/services/ +73/-0    research.md NodeLightSettings Service Extraction Phase 0 Research        specs/002-node-light-settings-service/research.md Analyzes existing DeviceManagerService pattern as reference implementation Documents current NodeLightSettingsNotifier JNAP actions and methods Identifies data models (NodeLightSettings, NodeLightStatus) and error mapping patterns Confirms no external research needed and no risks identified +97/-0    requirements.md NodeDetail Service Specification Quality Checklist              specs/001-node-detail-service/checklists/requirements.md Validates specification completeness for NodeDetail service extraction feature Confirms all mandatory sections completed and no clarification markers remain Verifies requirements are testable, unambiguous, and technology-agnostic Confirms spec readiness for planning phase with all checklist items passing +37/-0    requirements.md ConnectivityService Specification Quality Checklist            specs/001-connectivity-service/checklists/requirements.md Validates specification quality and completeness for ConnectivityService extraction Confirms content quality with no implementation details leaking into specification Verifies all functional requirements have clear acceptance criteria Confirms spec is ready for planning phase with all validation items passed +36/-0
Additional files	1 files node_detail_state.dart +0/-16

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Sensitive log exposure Description: Error logging in toggleBlinkNode() logs raw ServiceError/error.toString() which may include sensitive operational details (e.g., router identifiers, device IDs, network info, or backend error messages) and could be exposed via logs in production. node_detail_provider.dart [102-131] Referred Code _startBlinkNodeLED(deviceId).then((_) { prefs.setString(blinkingDeviceId, deviceId); state = state.copyWith(blinkingStatus: BlinkingStatus.stopBlinking); _blinkTimer?.cancel(); _blinkTimer = Timer(const Duration(seconds: 24), () { _stopBlinkNodeLED().then((_) { state = state.copyWith(blinkingStatus: BlinkingStatus.blinkNode); }); }); }).onError((error, stackTrace) { state = state.copyWith(blinkingStatus: BlinkingStatus.blinkNode); if (error is ServiceError) { logger.e('ServiceError: $error'); } else { logger.e(error.toString()); } }); } else { _stopBlinkNodeLED().then((_) { _blinkTimer?.cancel(); prefs.remove(blinkingDeviceId); ... (clipped 9 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 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 logged: The provider logs NodeDetailState JSON after populating fields like serial number, MAC address, and IP addresses, which can expose sensitive device/network identifiers in application logs. Referred Code final state = newState.copyWith( deviceId: targetId, location: values['location'] as String, isMaster: values['isMaster'] as bool, isOnline: values['isOnline'] as bool, connectedDevices: connectedDevices, upstreamDevice: values['upstreamDevice'] as String, isWiredConnection: values['isWiredConnection'] as bool, signalStrength: values['signalStrength'] as int, serialNumber: values['serialNumber'] as String, modelNumber: values['modelNumber'] as String, firmwareVersion: values['firmwareVersion'] as String, hardwareVersion: values['hardwareVersion'] as String, lanIpAddress: values['lanIpAddress'] as String, wanIpAddress: values['wanIpAddress'] as String, isMLO: values['isMLO'] as bool, macAddress: values['macAddress'] as String, ); logger.d('[State]:[NodeDetailsState]: ${state.toJson()}'); return state; 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: Unsafe map casts: The provider assumes service transformation results always contain specific keys and non-null types (via as String/bool/int), which can throw at runtime and lacks explicit handling for missing/invalid values. Referred Code final state = newState.copyWith( deviceId: targetId, location: values['location'] as String, isMaster: values['isMaster'] as bool, isOnline: values['isOnline'] as bool, connectedDevices: connectedDevices, upstreamDevice: values['upstreamDevice'] as String, isWiredConnection: values['isWiredConnection'] as bool, signalStrength: values['signalStrength'] as int, serialNumber: values['serialNumber'] as String, modelNumber: values['modelNumber'] as String, firmwareVersion: values['firmwareVersion'] as String, hardwareVersion: values['hardwareVersion'] as String, lanIpAddress: values['lanIpAddress'] as String, wanIpAddress: values['wanIpAddress'] as String, isMLO: values['isMLO'] as bool, macAddress: values['macAddress'] as String, ); 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: Verbose error logs: The provider logs raw ServiceError objects (logger.e('ServiceError: $error')) which may include internal details depending on ServiceError.toString() implementation. Referred Code }).onError((error, stackTrace) { state = state.copyWith(blinkingStatus: BlinkingStatus.blinkNode); if (error is ServiceError) { logger.e('ServiceError: $error'); } else { logger.e(error.toString()); } }); } else { _stopBlinkNodeLED().then((_) { _blinkTimer?.cancel(); prefs.remove(blinkingDeviceId); state = state.copyWith(blinkingStatus: BlinkingStatus.blinkNode); }).onError((error, stackTrace) { state = state.copyWith(blinkingStatus: BlinkingStatus.stopBlinking); if (error is ServiceError) { logger.e('ServiceError: $error'); } else { logger.e(error.toString()); } Learn more about managing compliance generic rules or creating your own custom rules
	Update

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	✅ Use async/await for predictable behavior Suggestion Impact: The commit replaced the .then()/.onError() chains in toggleBlinkNode with await calls wrapped in try/catch, including making the Timer callback async and awaiting _stopBlinkNodeLED(), and awaiting SharedPreferences set/remove operations. code diff: @@ -99,36 +99,45 @@ final deviceId = ref.read(nodeDetailIdProvider); if (!stopOnly && blinkDevice == null) { state = state.copyWith(blinkingStatus: BlinkingStatus.blinking); - _startBlinkNodeLED(deviceId).then((_) { - prefs.setString(blinkingDeviceId, deviceId); + try { + await _startBlinkNodeLED(deviceId); + await prefs.setString(blinkingDeviceId, deviceId); state = state.copyWith(blinkingStatus: BlinkingStatus.stopBlinking); _blinkTimer?.cancel(); - _blinkTimer = Timer(const Duration(seconds: 24), () { - _stopBlinkNodeLED().then((_) { + _blinkTimer = Timer(const Duration(seconds: 24), () async { + try { + await _stopBlinkNodeLED(); state = state.copyWith(blinkingStatus: BlinkingStatus.blinkNode); - }); + } catch (error) { + if (error is ServiceError) { + logger.e('ServiceError: $error'); + } else { + logger.e(error.toString()); + } + } }); - }).onError((error, stackTrace) { + } catch (error) { state = state.copyWith(blinkingStatus: BlinkingStatus.blinkNode); if (error is ServiceError) { logger.e('ServiceError: $error'); } else { logger.e(error.toString()); } - }); + } } else { - _stopBlinkNodeLED().then((_) { + try { + await _stopBlinkNodeLED(); _blinkTimer?.cancel(); - prefs.remove(blinkingDeviceId); + await prefs.remove(blinkingDeviceId); state = state.copyWith(blinkingStatus: BlinkingStatus.blinkNode); - }).onError((error, stackTrace) { + } catch (error) { state = state.copyWith(blinkingStatus: BlinkingStatus.stopBlinking); if (error is ServiceError) { logger.e('ServiceError: $error'); } else { logger.e(error.toString()); } - }); + } } Refactor the toggleBlinkNode method to use async/await and try/catch instead of .then() to ensure its Future completes only after all asynchronous operations are finished. lib/page/nodes/providers/node_detail_provider.dart [96-133] Future<void> toggleBlinkNode([bool stopOnly = false]) async { final prefs = await SharedPreferences.getInstance(); final blinkDevice = prefs.getString(blinkingDeviceId); final deviceId = ref.read(nodeDetailIdProvider); + if (!stopOnly && blinkDevice == null) { state = state.copyWith(blinkingStatus: BlinkingStatus.blinking); - _startBlinkNodeLED(deviceId).then((_) { - prefs.setString(blinkingDeviceId, deviceId); + try { + await _startBlinkNodeLED(deviceId); + await prefs.setString(blinkingDeviceId, deviceId); state = state.copyWith(blinkingStatus: BlinkingStatus.stopBlinking); _blinkTimer?.cancel(); - _blinkTimer = Timer(const Duration(seconds: 24), () { - _stopBlinkNodeLED().then((_) { + _blinkTimer = Timer(const Duration(seconds: 24), () async { + try { + await _stopBlinkNodeLED(); state = state.copyWith(blinkingStatus: BlinkingStatus.blinkNode); - }); + } catch (e) { + // Handle timer-based stop error if needed + } }); - }).onError((error, stackTrace) { + } catch (error) { state = state.copyWith(blinkingStatus: BlinkingStatus.blinkNode); if (error is ServiceError) { logger.e('ServiceError: $error'); } else { logger.e(error.toString()); } - }); + } } else { - _stopBlinkNodeLED().then((_) { + try { + await _stopBlinkNodeLED(); _blinkTimer?.cancel(); - prefs.remove(blinkingDeviceId); + await prefs.remove(blinkingDeviceId); state = state.copyWith(blinkingStatus: BlinkingStatus.blinkNode); - }).onError((error, stackTrace) { + } catch (error) { state = state.copyWith(blinkingStatus: BlinkingStatus.stopBlinking); if (error is ServiceError) { logger.e('ServiceError: $error'); } else { logger.e(error.toString()); } - }); + } } } [Suggestion processed] Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that using .then() in an async function leads to "fire-and-forget" behavior, which is misleading for callers and complicates testing. Refactoring to async/await improves code clarity, predictability, and testability.	Medium
	Refine WAN IP and upstream logic Refine the logic in the transformDeviceToUIValues example to only assign wanIpAddress for the master device. specs/001-node-detail-service/quickstart.md [105-110] -'wanIpAddress': wanStatus?.wanConnection?.ipAddress ?? '', +'wanIpAddress': isMaster ? (wanStatus?.wanConnection?.ipAddress ?? '') : '', 'upstreamDevice': isMaster ? 'INTERNET' - : (device.upstream?.getDeviceLocation() ?? + : device.upstream?.getDeviceLocation() ?? masterDevice?.getDeviceLocation() ?? - ''), + '', Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies a logical flaw in the example code where wanIpAddress should only be set for the master node, improving the correctness of the implementation guide.	Low
General	Use centralized error mapping utility Replace the private _mapJnapError function in the contract with the project's centralized mapJnapErrorToServiceError utility to maintain architectural consistency. specs/002-node-light-settings-service/contracts/node_light_settings_service_contract.md [88-94] -/// Maps JNAP errors to ServiceError types. -ServiceError _mapJnapError(JNAPError error) { - return switch (error.result) { - '_ErrorUnauthorized' => const UnauthorizedError(), - _ => UnexpectedError(originalError: error, message: error.result), - }; -} +// This private method should be removed. +// Instead, the service should use the centralized public error mapper: +// import 'package:privacy_gui/core/errors/jnap_error_mapper.dart'; +// +// try { +// // ... JNAP call +// } on JNAPError catch (e) { +// throw mapJnapErrorToServiceError(e); +// } Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly enforces architectural consistency by advocating for a centralized error mapper, which aligns with the project's documented standards and prevents code duplication.	Medium
	Early return on null gateway IP Add a guard clause to the testRouterType method to check if gatewayIp is null or empty and return RouterType.others to prevent unnecessary JNAP calls. lib/providers/connectivity/services/connectivity_service.dart [44-54] Future<RouterType> testRouterType(String? gatewayIp) async { + if (gatewayIp == null || gatewayIp.isEmpty) { + return RouterType.others; + } final routerSN = await _routerRepository .send( JNAPAction.getDeviceInfo, type: CommandType.local, fetchRemote: true, cacheLevel: CacheLevel.noCache, ) .then<String>( (value) => NodeDeviceInfo.fromJson(value.output).serialNumber) .onError((error, stackTrace) => ''); Apply / Chat Suggestion importance[1-10]: 5 __ Why: The suggestion correctly points out that the gatewayIp parameter is unused and proposes adding a guard clause. This improves robustness by preventing unnecessary network calls when the IP is invalid, making the code safer.	Low
	Improve preference handling and redundancy In the testRouterType example, use prefs.getString() for type-safe access to SharedPreferences and remove the redundant routerSN.isNotEmpty check. specs/001-connectivity-service/quickstart.md [77-82] final prefs = await SharedPreferences.getInstance(); -final currentSN = prefs.get(pCurrentSN); +final currentSN = prefs.getString(pCurrentSN); -if (routerSN.isNotEmpty && routerSN == currentSN) { +if (routerSN == currentSN) { return RouterType.behindManaged; } Apply / Chat Suggestion importance[1-10]: 5 __ Why: The suggestion improves the example code's quality by promoting type safety with getString() and removing a redundant conditional check, leading to more robust and cleaner code.	Low
Update