Skip to content

Conversation

@wabicai
Copy link
Member

@wabicai wabicai commented Dec 22, 2025

Summary by CodeRabbit

  • Bug Fixes

    • Improved firmware update stability with enhanced timeout handling for device communication.
    • Reduced unnecessary WebUSB reauthorization prompts and refined reconnect prompting during firmware updates.
  • Chores

    • Bumped package versions across examples, SDKs, transports, and core packages to 1.1.21-alpha.2 for consistency.

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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 22, 2025

Walkthrough

Version bumps from 1.1.21-alpha.1 to 1.1.21-alpha.2 across multiple package.json files. Firmware update flow changed: GetFeatures timeout counting, limited retries before reconnect, and consolidated WebUSB bootloader prompt behavior and accessibility adjustments in the base firmware class.

Changes

Cohort / File(s) Summary
Example packages
packages/connect-examples/electron-example/package.json, packages/connect-examples/expo-example/package.json, packages/connect-examples/expo-playground/package.json
Bumped package versions to 1.1.21-alpha.2 and updated internal @onekeyfe dependency versions to 1.1.21-alpha.2
Core package
packages/core/package.json
Bumped package version to 1.1.21-alpha.2; bumped @onekeyfe/hd-shared and @onekeyfe/hd-transport deps to alpha.2
Firmware update implementation
packages/core/src/api/FirmwareUpdateV3.ts
Added GetFeatures timeout counter and max-retry logic; specialized 3s GetFeatures timeout → HardwareError; retry vs reconnect decision based on timeout counter; added per-reconnect web-device prompt counter and integrated DEVICE event import
Firmware update base class
packages/core/src/api/firmware/FirmwareUpdateBaseMethod.ts
Removed hasPromptedWebUsbBootloaderReauth field; changed _promptDeviceInBootloaderForWebDevice visibility to protected; removed ensureWebUsbBootloaderReauthPrompt method
Common/connect & SDK packages
packages/hd-common-connect-sdk/package.json, packages/hd-web-sdk/package.json, packages/shared/package.json
Bumped versions and internal @onekeyfe dependencies to 1.1.21-alpha.2
BLE & transport SDKs
packages/hd-ble-sdk/package.json, packages/hd-transport-electron/package.json, packages/hd-transport-emulator/package.json, packages/hd-transport-http/package.json, packages/hd-transport-lowlevel/package.json, packages/hd-transport-react-native/package.json, packages/hd-transport-web-device/package.json, packages/hd-transport/package.json
Bumped package versions to 1.1.21-alpha.2; updated @onekeyfe/* transport/shared dependency pins to alpha.2

Sequence Diagram(s)

mermaid
sequenceDiagram
participant UI as User Interface
participant Updater as FirmwareUpdateV3
participant Transport as Transport Layer
participant Device as Hardware Device
Note over Updater,Transport: New timeout & retry flow
UI->>Updater: start firmware update
Updater->>Transport: send GetFeatures (with 3s timeout)
Transport->>Device: request features
alt GetFeatures succeeds
Device-->>Transport: features
Transport-->>Updater: features (reset timeout counter)
Updater->>Updater: continue update loop
else GetFeatures timeout
Transport--xUpdater: timeout error (classified)
Updater->>Updater: increment getFeaturesTimeoutCount
alt count < max
Updater->>Transport: retry GetFeatures
else count >= max
Updater->>UI: prompt user to re-select device (WebUSB)
UI->>Updater: user action
Updater->>Transport: trigger reconnect flow
end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Pay special attention to:
    • packages/core/src/api/FirmwareUpdateV3.ts (timeout counting, error classification, retry vs reconnect branches).
    • packages/core/src/api/firmware/FirmwareUpdateBaseMethod.ts (removed method and visibility change — check inheritance and callers).
  • Remaining changes are widespread package.json version bumps (low-review overhead).

Pre-merge checks and finishing touches

✅ Passed checks (3 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 code change: adding GetFeatures retry logic with timeout handling in the firmware update process, not just a vague version bump.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ 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 fix/getFeature-retry

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

@revan-zhang
Copy link
Contributor

revan-zhang commented Dec 22, 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.

@socket-security
Copy link

socket-security bot commented Dec 22, 2025

Warning

Review the following alerts detected in dependencies.

According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.

Action Severity Alert  (click "▶" to expand/collapse)
Warn High
Obfuscated code: npm entities is 91.0% likely obfuscated

Confidence: 0.91

Location: Package overview

From: ?npm/react-native-svg@15.12.1npm/entities@4.5.0

ℹ Read more on: This package | This alert | What is obfuscated code?

Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support@socket.dev.

Suggestion: Packages should not obfuscate their code. Consider not using packages with obfuscated code.

Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity ignore npm/entities@4.5.0. You can also ignore all packages with @SocketSecurity ignore-all. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.

Warn High
Obfuscated code: npm entities is 91.0% likely obfuscated

Confidence: 0.91

Location: Package overview

From: ?npm/react-native-svg@15.12.1npm/entities@4.5.0

ℹ Read more on: This package | This alert | What is obfuscated code?

Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support@socket.dev.

Suggestion: Packages should not obfuscate their code. Consider not using packages with obfuscated code.

Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity ignore npm/entities@4.5.0. You can also ignore all packages with @SocketSecurity ignore-all. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.

View full report

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

📜 Review details

Configuration used: Organization 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 ed487b9 and 9c5fcaa.

📒 Files selected for processing (1)
  • packages/core/src/api/FirmwareUpdateV3.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/core/src/api/FirmwareUpdateV3.ts (3)
packages/shared/src/HardwareError.ts (2)
  • HardwareErrorCode (79-481)
  • HardwareError (51-77)
packages/core/src/utils/index.ts (1)
  • wait (40-43)
packages/core/src/events/device.ts (1)
  • DEVICE (6-31)
⏰ 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). (2)
  • GitHub Check: build (22)
  • GitHub Check: lint (22)
🔇 Additional comments (4)
packages/core/src/api/FirmwareUpdateV3.ts (4)

1-1: LGTM!

Imports are correct. Both HardwareError and DEVICE are used in the new retry and WebUSB prompt logic.

Also applies to: 20-20


389-425: LGTM!

The retry logic is clean. Transient timeouts retry without reconnect. Other errors trigger reconnect. Counter resets are placed correctly.


444-450: LGTM!

The helper is clean and precise. It correctly identifies GetFeatures timeout errors by checking the error type, code, and method parameter.


515-519: Method exists in FirmwareUpdateBaseMethod as a protected async method (line 95). The call in FirmwareUpdateV3 is correct.

@wabicai wabicai merged commit 7411fcd into onekey Dec 22, 2025
10 checks passed
@wabicai wabicai deleted the fix/getFeature-retry branch December 22, 2025 13:24
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.

4 participants