Skip to content

Conversation

@ByteZhang1024
Copy link
Contributor

@ByteZhang1024 ByteZhang1024 commented Dec 10, 2025

Summary by CodeRabbit

  • Bug Fixes

    • Improved device connection error handling with enhanced logging and error reporting capabilities.
    • Refined connection flow stability and error recovery mechanisms.
  • Refactor

    • Reorganized device connection logic for better maintainability and centralized error handling.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 10, 2025

Walkthrough

Refactored device connection handling by promoting the connectDevice method to public with a background decorator in ServiceHardware, centralizing connection logic in ConnectYourDevice with improved error handling, and adding structured SDK logging for connection errors across the kit and shared packages.

Changes

Cohort / File(s) Summary
Core service refactoring
packages/kit-bg/src/services/ServiceHardware/ServiceHardware.ts
Changed connectDevice from private arrow function property to public async method with @backgroundMethod() decorator. Maintains delegation to getFeaturesWithoutCache(params).
Connection flow & error handling
packages/kit/src/views/Onboardingv2/pages/ConnectYourDevice.tsx, packages/kit/src/views/Onboardingv2/hooks/useDeviceConnect.tsx
Introduced centralized connectDevice helper managing hardware UI dialogs, background service calls, and navigation flow. Enhanced error handling with structured logging via defaultLogger.hardware.sdkLog.connectError instead of console logs. Added OneKeyHardwareError and HardwareErrorCode imports for specialized error branching.
Logging infrastructure
packages/shared/src/logger/scopes/hardware/scenes/sdk.ts
Added connectError() method to HardwareSDKScene for logging connection errors with connectId, deviceId, deviceType, uuid, and error details. Added IDeviceType import.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • ConnectYourDevice.tsx: Review new connectDevice helper logic, error branching for OneKeyHardwareError, and integration with UI dialog flow.
  • useDeviceConnect.tsx: Verify structured logging payload and error rethrow behavior.
  • ServiceHardware.ts: Confirm @backgroundMethod() decorator behavior and public API implications.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: converting connectDevice from a private property to a public async method and centralizing device connection handling in the scanPage flow.
Linked Issues check ✅ Passed Changes implement the Bluetooth pairing flow simplification by extracting device connection logic, adding proper error handling, and supporting the new pairing interaction model without Passkey entry.
Out of Scope Changes check ✅ Passed All changes directly support the device connection feature: refactoring connectDevice method, centralizing connection handling in ConnectYourDevice, adding structured logging, and introducing error handling for hardware errors.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/bleConnect

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to data retention organization setting

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 9ad85fb and 0b11774.

📒 Files selected for processing (4)
  • packages/kit-bg/src/services/ServiceHardware/ServiceHardware.ts (1 hunks)
  • packages/kit/src/views/Onboardingv2/hooks/useDeviceConnect.tsx (1 hunks)
  • packages/kit/src/views/Onboardingv2/pages/ConnectYourDevice.tsx (3 hunks)
  • packages/shared/src/logger/scopes/hardware/scenes/sdk.ts (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
packages/kit/src/views/Onboardingv2/hooks/useDeviceConnect.tsx (1)
packages/shared/src/logger/logger.ts (1)
  • defaultLogger (102-102)
packages/kit-bg/src/services/ServiceHardware/ServiceHardware.ts (1)
packages/shared/src/background/backgroundDecorators.ts (1)
  • backgroundMethod (218-218)
packages/kit/src/views/Onboardingv2/pages/ConnectYourDevice.tsx (3)
packages/kit-bg/src/services/ServiceHardware/ServiceHardware.ts (1)
  • connectDevice (621-623)
packages/shared/types/device.ts (1)
  • IConnectYourDeviceItem (418-423)
packages/shared/src/errors/errors/hardwareErrors.ts (1)
  • OneKeyHardwareError (33-44)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: lint (20.x)
  • GitHub Check: unittest (20.x)
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (5)
packages/kit/src/views/Onboardingv2/hooks/useDeviceConnect.tsx (1)

153-159: LGTM! Structured error logging added.

The SDK error logging provides comprehensive device context for debugging connection failures. Safe property access patterns are used throughout.

packages/shared/src/logger/scopes/hardware/scenes/sdk.ts (1)

6-7: LGTM! New logging method added for connection errors.

The connectError method provides structured logging with appropriate device context. Method signature and return structure are correct.

Also applies to: 19-34

packages/kit/src/views/Onboardingv2/pages/ConnectYourDevice.tsx (2)

1023-1073: Consider adding SDK logging for consistency.

The connectDevice helper handles errors differently from useDeviceConnect.tsx (lines 153-159 in that file). The original logs connection errors via defaultLogger.hardware.sdkLog.connectError() before rethrowing, but this new path only uses console logging for non-hardware errors.

Should this Bluetooth path also log to SDK for consistency?


1037-1042: Verify retryCount parameter value.

The connectDevice call uses retryCount: 0. Is this intentional for the Bluetooth connection flow, or should it allow retries like other connection paths?

packages/kit-bg/src/services/ServiceHardware/ServiceHardware.ts (1)

620-623: LGTM! Method promoted to public API.

The connectDevice method is now public with @backgroundMethod() decorator, enabling centralized connection handling from the UI layer. Functionality remains unchanged—it delegates to getFeaturesWithoutCache().


Comment @coderabbitai help to get the list of available commands and usage tips.

@revan-zhang
Copy link
Contributor

revan-zhang commented Dec 10, 2025

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@ByteZhang1024 ByteZhang1024 enabled auto-merge (squash) December 10, 2025 07:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants