Skip to content

Conversation

@originalix
Copy link
Contributor

@originalix originalix commented Mar 20, 2025

Summary by CodeRabbit

  • New Features

    • Introduced a new connection type selection tool in the user interface, letting users choose between different connection methods.
    • Enabled dynamic transport switching, enhancing connectivity and streamlining event handling.
    • Improved event logging for better visibility into connection events.
    • Added new localized messages for connection options in English and Chinese.
    • Added a toggle for skipping the web device prompt in the settings.
  • Chores

    • Upgraded version numbers and updated dependencies across multiple components to ensure consistency and reliability.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 20, 2025

Walkthrough

Multiple packages were updated to version 1.0.24-alpha.4. New localization entries for connection type and WebUSB were added. An atom and UI modifications in the expo example allow users to select a connection type. Core and various SDK modules now support a new transport switching mechanism. New event interfaces and message types were introduced to support this feature. Dependency versions were updated accordingly.

Changes

File(s) Change Summary
packages/connect-examples/electron-example/package.json, packages/connect-examples/expo-example/package.json, packages/core/package.json, packages/hd-ble-sdk/package.json, packages/hd-common-connect-sdk/package.json, packages/hd-transport-http/package.json, packages/hd-transport-lowlevel/package.json, packages/hd-transport-react-native/package.json, packages/hd-transport-webusb/package.json, packages/hd-transport/package.json, packages/hd-web-sdk/package.json, packages/shared/package.json Updated version numbers from 1.0.24-alpha.2 to 1.0.24-alpha.4; dependency versions were updated to match.
packages/connect-examples/expo-example/locale/en-US.json, packages/connect-examples/expo-example/locale/zh-CN.json Added new message entries for "Connection Type" and "Use WebUSB" (or their Chinese equivalents).
packages/connect-examples/expo-example/src/atoms/deviceConnectAtoms.ts Added new ConnectionType type and connectionTypeAtom for state management.
packages/connect-examples/expo-example/src/components/DeviceList.tsx Integrated connection type selection using a Picker and introduced onSwitchConnectionType to trigger SDK transport switching.
packages/connect-examples/expo-example/src/components/HandleSDKEvents.tsx Added conditional logging for events with type LOG_EVENT.
packages/connect-examples/expo-example/src/utils/hardwareInstance.ts, packages/connect-examples/expo-example/src/utils/importSdk/index.ts Updated import statements and initialization logic; modified function signatures to accept optional parameters.
packages/core/src/core/index.ts, packages/core/src/data-manager/DataManager.ts, packages/core/src/device/DeviceCommands.ts, packages/core/src/device/DevicePool.ts, packages/core/src/events/*, packages/core/src/inject.ts, packages/core/src/lowLevelInject.ts, packages/core/src/topLevelInject.ts, packages/core/src/types/api/index.ts Changed the return type in findUiPromise; added switchTransport to update environment settings, reset state, and set transport; added new event interfaces and updated API definitions for transport switching.
packages/hd-ble-sdk/src/index.ts, packages/hd-common-connect-sdk/src/index.ts Added new switchTransport functions returning a resolved promise.
packages/hd-web-sdk/src/iframe/builder.ts, packages/hd-web-sdk/src/iframe/index.ts, packages/hd-web-sdk/src/index.ts Removed conditional check for USB access; added switchCoreTransport and new async switchTransport for iframe-based transport switching.

Sequence Diagram(s)

sequenceDiagram
    participant U as User (DeviceList)
    participant S as SDK
    participant C as Core Module
    participant DM as DataManager
    participant TM as TransportManager
    participant DP as DevicePool

    U->>S: Select connection type (Bridge/WebUSB)
    S->>C: Call switchTransport(newEnv)
    C->>DM: updateEnv(newEnv)
    C->>TM: setTransport(Transport, plugin)
    C->>DP: resetState()
    C-->>S: Return success response
Loading
sequenceDiagram
    participant UI as User Interface
    participant HW as hd-web-sdk
    participant IF as Iframe
    participant SC as switchCoreTransport

    UI->>HW: Invoke switchTransport(env)
    HW->>IF: Send message (SWITCH_TRANSPORT event)
    IF->>SC: Process message & select transport based on env
    SC-->>HW: Return success response
Loading

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro (Legacy)

📥 Commits

Reviewing files that changed from the base of the PR and between 29ce56f and 9e47718.

📒 Files selected for processing (16)
  • packages/connect-examples/electron-example/package.json (1 hunks)
  • packages/connect-examples/expo-example/package.json (2 hunks)
  • packages/connect-examples/expo-example/src/components/CommonParamsView.tsx (1 hunks)
  • packages/connect-examples/expo-example/src/provider/CommonParamsProvider.tsx (1 hunks)
  • packages/core/package.json (2 hunks)
  • packages/core/src/core/index.ts (3 hunks)
  • packages/core/src/types/params.ts (1 hunks)
  • packages/hd-ble-sdk/package.json (2 hunks)
  • packages/hd-common-connect-sdk/package.json (2 hunks)
  • packages/hd-transport-http/package.json (2 hunks)
  • packages/hd-transport-lowlevel/package.json (2 hunks)
  • packages/hd-transport-react-native/package.json (2 hunks)
  • packages/hd-transport-webusb/package.json (2 hunks)
  • packages/hd-transport/package.json (1 hunks)
  • packages/hd-web-sdk/package.json (2 hunks)
  • packages/shared/package.json (1 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
packages/connect-examples/expo-example/src/components/CommonParamsView.tsx (1)
packages/connect-examples/expo-example/src/components/SwitchInput.tsx (1) (1)
  • SwitchInput (12-37)
packages/core/src/core/index.ts (6)
packages/core/src/constants/ui-request.ts (1) (1)
  • UI_REQUEST (1-17)
packages/core/src/types/settings.ts (1) (1)
  • ConnectSettings (3-23)
packages/hd-transport/src/types/transport.ts (2) (2)
  • LowlevelTransportSharedPlugin (62-71)
  • Transport (35-59)
packages/hd-transport/src/index.ts (2) (2)
  • LowlevelTransportSharedPlugin (23-23)
  • Transport (17-17)
packages/core/src/data-manager/DataManager.ts (1) (1)
  • DataManager (32-350)
packages/core/src/data-manager/TransportManager.ts (1) (1)
  • TransportManager (20-131)
🔇 Additional comments (21)
packages/shared/package.json (1)

3-3: Version bumped to 1.0.24-alpha.4

Version number updated from 1.0.24-alpha.2 to 1.0.24-alpha.4, keeping it in sync with other packages in the project.

packages/hd-transport/package.json (1)

3-3: Version bumped to 1.0.24-alpha.4

Version number updated from 1.0.24-alpha.2 to 1.0.24-alpha.4, matching other packages in the project.

packages/connect-examples/expo-example/src/provider/CommonParamsProvider.tsx (1)

12-12: Added skipWebDevicePrompt parameter

New parameter supports transport switching functionality by allowing control over web device connection prompts. Default is properly set to false.

packages/connect-examples/electron-example/package.json (1)

5-5: Version bumped to 1.0.24-alpha.4

Version number updated from 1.0.24-alpha.2 to 1.0.24-alpha.4, keeping it consistent with other packages in the project.

packages/core/src/types/params.ts (1)

35-39: Clean addition of skipWebDevicePrompt property

The new property is well-documented and follows the existing pattern in the interface. Good job keeping the implementation consistent.

packages/connect-examples/expo-example/src/components/CommonParamsView.tsx (1)

63-67: Good implementation of the switch for skipWebDevicePrompt

The new SwitchInput follows the same pattern as other switches in this component. It's correctly bound to the commonParams state.

packages/core/package.json (2)

3-3: Version bump looks good

Version increment to 1.0.24-alpha.4 from alpha.2 aligns with the feature addition.


28-29: Dependency versions properly updated

Dependencies correctly updated to match the new version.

packages/hd-transport-http/package.json (2)

3-3: Version bump looks good

Version increment aligns with other package updates in this PR.


27-28: Dependencies properly updated

Dependencies correctly updated to match the new version across packages.

packages/hd-transport-webusb/package.json (1)

3-3: Precise version and dependency bump.
The package version was correctly updated to "1.0.24-alpha.4" and both dependency versions now align with the new release.

Also applies to: 23-24

packages/hd-transport-react-native/package.json (1)

3-3: Consistent update applied.
The version and dependency bumps to "1.0.24-alpha.4" look clean and are consistent with related packages.

Also applies to: 22-23

packages/hd-transport-lowlevel/package.json (1)

3-3: Version update confirmed.
The updates to "1.0.24-alpha.4" for both the package and its dependencies are correctly applied.

Also applies to: 22-23

packages/hd-ble-sdk/package.json (1)

3-3: Accurate version and dependency revisions.
The new version "1.0.24-alpha.4" and the updated dependency versions for "@onekeyfe/hd-core", "@onekeyfe/hd-shared", and "@onekeyfe/hd-transport-react-native" ensure consistency across the ecosystem.

Also applies to: 23-25

packages/hd-web-sdk/package.json (1)

3-3: Solid package.json update.
The version bump and dependency updates to "1.0.24-alpha.4" are applied correctly. This helps maintain compatibility across packages.

Also applies to: 24-27

packages/core/src/core/index.ts (2)

401-403: Good addition of user control for web device prompts.

The new conditional check prevents unnecessary UI prompts when specifically indicated, improving user experience when working with web devices.


834-849:

Details

✅ Verification successful

Great implementation of transport switching mechanism.

The new switchTransport function provides a clean way to dynamically change transport layers. It properly updates the environment, sets the transport, and resets all necessary state to ensure a smooth transition.

Run this script to verify all exported functions are properly exposed:


🏁 Script executed:

#!/bin/bash
# Check if the switchTransport function is properly exported and accessible

# Look for export statements for switchTransport
grep -n "export const switchTransport" packages/core/src/core/index.ts

# Check if the function is imported and used elsewhere in the codebase
rg "switchTransport" --type ts --no-filename | grep -v "packages/core/src/core/index.ts"

Length of output: 1227


Transport switching mechanism verified.

The switchTransport function is clearly exported and used consistently across the codebase. The verification script confirms that its export and usage meet expectations. The method properly updates the environment, sets the transport, resets internal states, and reinitializes the connector for a smooth transition.

Great work!

packages/connect-examples/expo-example/package.json (2)

3-3: Version bump is consistent with feature additions.

Updated version to 1.0.24-alpha.4 aligns with the new transport switching functionality.


22-25: Dependencies properly updated to match main package version.

All OneKey dependencies now use consistent version numbers, ensuring compatibility between packages.

packages/hd-common-connect-sdk/package.json (2)

3-3: Version bump matches other package updates.

Version increased to 1.0.24-alpha.4 to align with the expo-example package.


23-27: Dependencies correctly synchronized.

All dependencies updated to 1.0.24-alpha.4, maintaining version consistency across the ecosystem.


🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary or Summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@socket-security
Copy link

socket-security bot commented Mar 20, 2025

Report too large to display inline

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: 10

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 40bbd1a and 3ab54b3.

📒 Files selected for processing (36)
  • packages/connect-examples/electron-example/package.json (1 hunks)
  • packages/connect-examples/expo-example/locale/en-US.json (1 hunks)
  • packages/connect-examples/expo-example/locale/zh-CN.json (1 hunks)
  • packages/connect-examples/expo-example/package.json (2 hunks)
  • packages/connect-examples/expo-example/src/atoms/deviceConnectAtoms.ts (1 hunks)
  • packages/connect-examples/expo-example/src/components/DeviceList.tsx (5 hunks)
  • packages/connect-examples/expo-example/src/components/HandleSDKEvents.tsx (1 hunks)
  • packages/connect-examples/expo-example/src/utils/hardwareInstance.ts (2 hunks)
  • packages/connect-examples/expo-example/src/utils/importSdk/index.ts (1 hunks)
  • packages/core/package.json (2 hunks)
  • packages/core/src/core/index.ts (2 hunks)
  • packages/core/src/data-manager/DataManager.ts (1 hunks)
  • packages/core/src/device/DeviceCommands.ts (0 hunks)
  • packages/core/src/device/DevicePool.ts (1 hunks)
  • packages/core/src/events/call.ts (2 hunks)
  • packages/core/src/events/core.ts (2 hunks)
  • packages/core/src/events/iframe.ts (2 hunks)
  • packages/core/src/index.ts (5 hunks)
  • packages/core/src/inject.ts (4 hunks)
  • packages/core/src/lowLevelInject.ts (3 hunks)
  • packages/core/src/topLevelInject.ts (2 hunks)
  • packages/core/src/types/api/index.ts (2 hunks)
  • packages/hd-ble-sdk/package.json (2 hunks)
  • packages/hd-ble-sdk/src/index.ts (2 hunks)
  • packages/hd-common-connect-sdk/package.json (2 hunks)
  • packages/hd-common-connect-sdk/src/index.ts (2 hunks)
  • packages/hd-transport-http/package.json (2 hunks)
  • packages/hd-transport-lowlevel/package.json (2 hunks)
  • packages/hd-transport-react-native/package.json (2 hunks)
  • packages/hd-transport-webusb/package.json (2 hunks)
  • packages/hd-transport/package.json (1 hunks)
  • packages/hd-web-sdk/package.json (2 hunks)
  • packages/hd-web-sdk/src/iframe/builder.ts (1 hunks)
  • packages/hd-web-sdk/src/iframe/index.ts (3 hunks)
  • packages/hd-web-sdk/src/index.ts (3 hunks)
  • packages/shared/package.json (1 hunks)
💤 Files with no reviewable changes (1)
  • packages/core/src/device/DeviceCommands.ts
🧰 Additional context used
🧬 Code Definitions (12)
packages/core/src/types/api/index.ts (1)
packages/core/src/types/settings.ts (1) (1)
  • ConnectSettings (3-23)
packages/core/src/events/core.ts (1)
packages/core/src/events/call.ts (1) (1)
  • IFrameSwitchTransportMessage (55-59)
packages/connect-examples/expo-example/src/utils/hardwareInstance.ts (2)
packages/connect-examples/expo-example/src/utils/importSdk/index.ts (2) (2)
  • importSdk (7-12)
  • importLowLevelSDK (21-22)
packages/connect-examples/expo-example/src/utils/importSdk/index.native.ts (1) (1)
  • importSdk (3-3)
packages/core/src/inject.ts (1)
packages/core/src/types/api/index.ts (1) (1)
  • CoreApi (164-432)
packages/core/src/data-manager/DataManager.ts (1)
packages/core/src/types/settings.ts (1) (1)
  • ConnectSettings (3-23)
packages/connect-examples/expo-example/src/utils/importSdk/index.ts (2)
packages/connect-examples/expo-example/src/utils/importSdk/index.native.ts (1) (1)
  • importSdk (3-3)
packages/core/src/types/api/index.ts (1) (1)
  • CoreApi (164-432)
packages/core/src/lowLevelInject.ts (1)
packages/core/src/types/api/index.ts (1) (1)
  • CoreApi (164-432)
packages/hd-common-connect-sdk/src/index.ts (1)
packages/core/src/core/index.ts (1) (1)
  • switchTransport (832-847)
packages/core/src/events/iframe.ts (1)
packages/core/src/types/settings.ts (1) (1)
  • ConnectSettings (3-23)
packages/hd-web-sdk/src/index.ts (4)
packages/core/src/core/index.ts (1) (1)
  • switchTransport (832-847)
packages/core/src/types/settings.ts (1) (1)
  • ConnectSettings (3-23)
packages/hd-web-sdk/src/utils/bridgeUtils.ts (1) (1)
  • sendMessage (29-57)
packages/core/src/events/iframe.ts (1) (1)
  • IFRAME (5-11)
packages/connect-examples/expo-example/src/components/DeviceList.tsx (1)
packages/connect-examples/expo-example/src/atoms/deviceConnectAtoms.ts (2) (2)
  • connectionTypeAtom (7-7)
  • ConnectionType (4-4)
packages/core/src/events/call.ts (2)
packages/core/src/events/iframe.ts (1) (1)
  • IFRAME (5-11)
packages/core/src/types/settings.ts (1) (1)
  • ConnectSettings (3-23)
🪛 Biome (1.9.4)
packages/core/src/device/DevicePool.ts

[error] 252-252: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)


[error] 253-253: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)


[error] 254-254: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)


[error] 255-255: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)


[error] 256-256: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)


[error] 259-259: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)

packages/core/src/data-manager/DataManager.ts

[error] 310-310: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)


[error] 311-311: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)


[error] 312-312: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)


[error] 313-313: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)

🪛 GitHub Check: lint (18.x)
packages/core/src/data-manager/DataManager.ts

[failure] 314-314:
Insert ,

🪛 GitHub Actions: lint
packages/core/src/data-manager/DataManager.ts

[error] 314-314: Insert , prettier/prettier

🔇 Additional comments (59)
packages/core/src/events/core.ts (1)

3-3: New transport switching message type correctly integrated

Added IFrameSwitchTransportMessage to both imports and the CoreMessage union type. This enables the core event system to handle transport switching messages, fulfilling the PR's primary objective.

Also applies to: 20-20

packages/core/src/types/api/index.ts (1)

160-160: Good addition of required import.

The ConnectSettings import is correctly added to support the new switchTransport method.

packages/connect-examples/expo-example/src/atoms/deviceConnectAtoms.ts (1)

1-7: Clean implementation of connection type state management.

The atom setup is simple and effective. Good use of comments and clear type definition.

packages/connect-examples/expo-example/locale/zh-CN.json (1)

126-127: Appropriate localization entries added.

The Chinese translations for connection type and WebUSB match the functionality being added.

packages/hd-ble-sdk/src/index.ts (2)

181-181: Proper SDK function export.

The switchTransport function is correctly added to the HardwareBleSdk object.


171-171:

Details

❓ Verification inconclusive

Simple implementation of switchTransport function.

The function always returns a resolved promise with true. This works as a stub but lacks actual transport switching logic.

Verify whether this minimal implementation is sufficient for BLE SDK or if additional logic is needed:


🌐 Web query:

Does WebUSB transport work with BLE devices?

💡 Result:

WebUSB transport can interface with Bluetooth Low Energy (BLE) devices when the BLE functionality is implemented over a USB interface. This typically requires specific firmware and hardware configurations to bridge USB and BLE protocols. Here's a detailed breakdown:

Key Considerations

  1. USB-Based BLE Adapters:
    WebUSB can communicate with USB-connected BLE adapters (e.g., Nordic Semiconductor's nRF52 series with custom connectivity firmware). These devices translate BLE operations into USB-compatible commands. For example, the pc-ble-driver-webusb project enables BLE functionalities like scanning and connecting to devices via WebUSB[1].

  2. Required Firmware:
    The BLE device must run modified firmware to expose BLE operations over USB. The WebUSB library interacts with this firmware to execute SoftDevice (Nordic’s BLE stack) functions such as sd_ble_gap_scan_start()[1].

  3. Event Handling:
    BLE events received via USB are decoded using Emscripten functions to access C-style structs in JavaScript. For example, extracting data from a BLE advertisement report involves low-level memory operations similar to native C code[1].

Comparison with Web Bluetooth

Feature WebUSB with BLE Adapters Web Bluetooth API
Transport Protocol USB Bluetooth (wireless)
Use Case Advanced BLE control via USB Direct BLE device interaction
Complexity Requires custom firmware/drivers Browser-managed pairing
Implementation Low-level USB transfers High-level BLE abstractions

Example Use Case

A Nordic nRF52 USB dongle with WebUSB-enabled firmware allows web apps to:

  • Start BLE scans:
    await sd_ble_gap_scan_start(adapter, scanParam);[1]
  • Parse BLE events by accessing C structs in JavaScript using Emscripten memory functions[1].

Limitations

  • Browser Support: WebUSB requires HTTPS and user-granted permissions[2][6].
  • Hardware Dependency: BLE functionality via WebUSB depends on vendor-specific USB firmware (e.g., Nordic's experimental connectivity firmware)[1][4].

In summary, WebUSB can transport BLE protocol data when the hardware bridges USB and BLE layers, enabling direct browser-based control for specialized use cases[1][4][6]. For most BLE interactions, the Web Bluetooth API is more straightforward[7][8].

Citations:


Enhance switchTransport Implementation for Real BLE Transport Switching

The current implementation serves as a stub by simply returning a resolved promise with true. While WebUSB can interface with BLE devices, it only works with specific USB-based BLE adapters running custom firmware. This means that to meet production needs, you’ll eventually need logic that detects if the device supports USB transport and then handles switching appropriately.

  • Confirm the target hardware supports USB-based BLE via custom adapters.
  • Add logic to detect and switch transports if needed.
packages/hd-common-connect-sdk/src/index.ts (1)

172-172: LGTM: Added switchTransport to the SDK exports

The function is properly added to the HardwareCommonConnectSdk object, making it available for consumers.

packages/core/src/topLevelInject.ts (2)

5-5: LGTM: Added ConnectSettings import

Import added correctly to support type checking for the new function parameter.


55-56: LGTM: Transport switching implementation added to API

The implementation correctly delegates to the lowLevelApi or returns a resolved promise with false if not available.

packages/connect-examples/expo-example/locale/en-US.json (1)

125-126: Good addition of localization strings for connection management.

These new entries clearly support the connection type selection feature, making the UI more user-friendly.

packages/connect-examples/expo-example/src/components/HandleSDKEvents.tsx (1)

55-57: Implemented logging for LOG_EVENT.

This code properly logs payload data from LOG_EVENT events. It replaces the commented-out logging implementation at lines 85-90.

packages/hd-web-sdk/src/iframe/builder.ts (1)

27-27: USB permissions now always enabled.

The iframe now always gets USB permissions rather than only when in WebUSB mode. This supports the transport switching feature but could present security implications.

Consider if this broader permission is necessary or if more granular control would be better. Does this match your security requirements?

packages/core/package.json (1)

3-3: Version bumped to 1.0.24-alpha.3.

Package version and dependencies properly updated to maintain consistency across the codebase.

Also applies to: 28-29

packages/core/src/lowLevelInject.ts (3)

16-16: Good addition of the switchTransport method.

The new method nicely follows the existing pattern of reusing types from CoreApi. This ensures type consistency across the codebase.


32-32: Parameter addition looks good.

The switchTransport parameter integrates well with the existing function signature.


53-54: Clean implementation of switchTransport in the API object.

The method is properly positioned in the returned API object, maintaining the established code style.

packages/core/src/inject.ts (4)

13-13: Good addition to InjectApi interface.

This change mirrors the pattern used in LowLevelInjectApi, ensuring consistency between interfaces.


23-23: Parameter addition is consistent.

The switchTransport parameter follows the same pattern as other parameters in the function signature.


53-54: Clean implementation in the API object.

The switchTransport method is properly added to the returned API object.


74-74: Properly updated Omit type.

The switchTransport property is correctly added to the Omit type, ensuring it's handled separately from createCoreApi.

packages/hd-transport-http/package.json (1)

3-3: Version bump is appropriate.

Good synchronization of version numbers across the package and its dependencies.

Also applies to: 27-28

packages/core/src/index.ts (3)

9-9: Good export of switchTransport function.

Making switchTransport available alongside Core and initCore provides a consistent API surface.


26-26: Proper integration in HardwareSdk.

The switchTransport parameter is correctly added to both the function parameters and the inject call.

Also applies to: 36-36


48-48: Seamless integration in HardwareSDKLowLevel.

The implementation mirrors the changes in HardwareSdk, ensuring consistency across both API layers.

Also applies to: 59-59

packages/hd-transport-webusb/package.json (2)

3-3: Package version updated correctly

Version bump from 1.0.24-alpha.2 to 1.0.24-alpha.3 aligns with the switch transport feature addition.


23-24: Dependencies updated consistently

Dependencies properly updated to match the new version. Good practice to keep all related packages in sync.

packages/hd-transport-react-native/package.json (2)

3-3: Package version updated correctly

Version bump from 1.0.24-alpha.2 to 1.0.24-alpha.3 matches other packages in the codebase.


22-23: Dependencies updated consistently

Dependencies properly updated to maintain version parity across the SDK.

packages/hd-transport-lowlevel/package.json (2)

3-3: Package version updated correctly

Version bump from 1.0.24-alpha.2 to 1.0.24-alpha.3 aligns with other packages.


22-23: Dependencies updated consistently

Dependencies properly updated to maintain version consistency.

packages/hd-web-sdk/src/index.ts (2)

278-278: LGTM! Function properly exported

The switchTransport function is correctly added to the HardwareSDKLowLevel exports.


291-291: LGTM! Function properly exported

The switchTransport function is correctly added to the HardwareWebSdk exports.

packages/core/src/events/iframe.ts (3)

10-10: Well-defined constant for transport switching

Good addition of a clear, descriptive constant for the transport switching feature.


25-30: Clean interface definition

The interface is well-typed and properly uses ConnectSettings['env'] to ensure type safety.


32-32: Updated type definition

The IFrameEvent type correctly includes the new interface.

packages/connect-examples/expo-example/src/utils/hardwareInstance.ts (4)

4-4: Import updated to include low-level SDK

Clean import statement that includes both required SDK imports.


33-33: Simplified SDK import call

The call to importSdk() is now cleaner without unnecessary parameters.


40-40: Moved low-level SDK initialization

Good positioning of the low-level SDK import after the platform check, making the code more efficient.


38-38:

Details

❓ Verification inconclusive

Environment changed from 'webusb' to 'web'

This is a significant change in the environment setting. Verify that this change works with the transport switching mechanism.


🏁 Script executed:

#!/bin/bash
# Check how the environment is used in the codebase

echo "Checking environment usage in transport handling code..."
rg -A 5 "env === 'web'" --glob "*.ts" --glob "*.js"
echo "---"
rg -A 5 "env === 'webusb'" --glob "*.ts" --glob "*.js"

Length of output: 1797


Ensure Transport Switching Mechanism Aligns with the New Environment Setting

  • The example now sets settings.env = 'web' instead of 'webusb'.
  • The transport switching code (e.g., in packages/hd-web-sdk/src/iframe/index.ts, packages/hd-common-connect-sdk/src/index.ts, and packages/core/src/data-manager/DataManager.ts) still checks for 'webusb'.
  • Please verify that the transport logic properly handles 'web' or ensure the comparisons are updated accordingly.
packages/connect-examples/expo-example/src/utils/importSdk/index.ts (2)

7-8: Made options parameter optional

Good improvement. The optional parameter with optional chaining makes the function more flexible and robust.


14-15: Made options parameter optional in importTopLevelSdk

Consistent pattern applied to both functions improves the API's usability.

packages/core/src/events/call.ts (2)

3-3: Import addition looks good.

The ConnectSettings import is correctly added to support the new message interface, making the code more type-safe.


55-59: Well-structured new interface.

The IFrameSwitchTransportMessage interface follows the existing pattern and properly types the payload with ConnectSettings['env']. This ensures type safety when switching transports.

packages/core/src/core/index.ts (1)

832-847: Well-implemented transport switching function.

The new switchTransport function correctly updates environment settings, sets the transport, and resets state. This makes dynamic transport selection possible.

Consider adding a comment explaining why each reset is necessary for clearer maintenance in the future.

packages/hd-ble-sdk/package.json (1)

3-3: Version update is consistent.

The package version and its dependencies are correctly bumped to 1.0.24-alpha.3, maintaining version consistency across related packages.

Also applies to: 23-25

packages/hd-web-sdk/package.json (1)

3-3: Version update is consistent.

The package version and its dependencies are correctly bumped to 1.0.24-alpha.3, maintaining version consistency across related packages.

Also applies to: 24-27

packages/connect-examples/expo-example/package.json (2)

3-3: Version bump to alpha.3

Version updated consistently with other packages in the PR.


22-25: Dependency versions updated consistently

All OneKey dependencies now use the same alpha.3 version. This keeps packages in sync.

packages/hd-common-connect-sdk/package.json (2)

3-3: Version bump to alpha.3

Version updated to match other packages.


23-27: Dependencies updated to alpha.3

All dependencies now use the same version. This ensures compatibility across the SDK.

packages/connect-examples/expo-example/src/components/DeviceList.tsx (6)

11-13: New imports added for transport switching feature

Added imports for UI components and state management to support connection type selection.

Also applies to: 16-16, 22-22


104-104: Connection type state management added

Using Jotai atom for connection type state - good approach for sharing state across components.


131-139: Connection type check logic updated

Changed from environment variable to state-based connection type checking. Code now properly branches based on user selection.


146-146: Added connectionType to dependency array

Properly added connectionType to the dependency array of useCallback. This ensures the callback refreshes when connection type changes.


204-216: Added UI for connection type selection

Added picker component for switching between Bridge and WebUSB. The UI is clean and integrates well with the existing layout.


153-162:

Details

❓ Verification inconclusive

New callback for switching connection type

Added handler for connection type changes. The TypeScript error suppression with @ts-expect-error needs attention.

Let's check if the SDK interface has the switchTransport method properly typed:


🏁 Script executed:

#!/bin/bash
# Find SDK interface definition
rg -A 5 "switchTransport|class.*SDK" --type typescript

Length of output: 91


SDK Method Type Verification Needed
The new callback for switching connection types is added correctly. However, the use of @ts-expect-error to suppress errors when calling sdk?.switchTransport indicates that the SDK interface might not have switchTransport properly typed. We couldn’t automatically verify the type definition from the repository, so please confirm manually that the SDK interface has a correctly typed switchTransport method. Remove the suppression if it's no longer necessary.

  • File: packages/connect-examples/expo-example/src/components/DeviceList.tsx (lines 153-162)
packages/shared/package.json (1)

3-3: Version bump confirmed.
The version is updated to "1.0.24-alpha.3" as required.

packages/hd-transport/package.json (1)

3-3: Version update looks good.
The version change to "1.0.24-alpha.3" is applied correctly.

packages/connect-examples/electron-example/package.json (1)

5-5: Version updated successfully.
The version is bumped to "1.0.24-alpha.3" correctly to align with other packages.

originalix and others added 4 commits March 20, 2025 23:40
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@originalix originalix enabled auto-merge (squash) March 20, 2025 15:49
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: 1

♻️ Duplicate comments (1)
packages/hd-common-connect-sdk/src/index.ts (1)

162-162: ⚠️ Potential issue

Implementation needs to handle parameters.

This implementation doesn't process any parameters and always returns success: true.

Update the function to accept and handle parameters:

-const switchTransport = () => Promise.resolve({ success: true });
+const switchTransport = (env: ConnectSettings['env']) => Promise.resolve({ success: true });
📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro (Legacy)

📥 Commits

Reviewing files that changed from the base of the PR and between 27d7ad1 and 29ce56f.

📒 Files selected for processing (5)
  • packages/core/src/topLevelInject.ts (2 hunks)
  • packages/core/src/types/api/index.ts (2 hunks)
  • packages/hd-ble-sdk/src/index.ts (2 hunks)
  • packages/hd-common-connect-sdk/src/index.ts (2 hunks)
  • packages/hd-web-sdk/src/index.ts (3 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
packages/core/src/types/api/index.ts (1)
packages/core/src/types/settings.ts (1) (1)
  • ConnectSettings (3-23)
packages/hd-web-sdk/src/index.ts (5)
packages/core/src/core/index.ts (1) (1)
  • switchTransport (832-847)
packages/core/src/types/settings.ts (1) (1)
  • ConnectSettings (3-23)
packages/hd-web-sdk/src/utils/bridgeUtils.ts (1) (1)
  • sendMessage (29-57)
packages/core/src/events/iframe.ts (1) (1)
  • IFRAME (5-11)
packages/shared/src/HardwareError.ts (1) (1)
  • HardwareErrorCode (46-397)
🔇 Additional comments (9)
packages/core/src/types/api/index.ts (2)

160-160: Good addition of ConnectSettings import.

The import is correctly added to support the new switchTransport method's type parameter.


178-178: The return type is properly defined.

The method signature uses a specific return type Promise<{ success: boolean }> instead of a generic Promise. This provides better type safety.

packages/core/src/topLevelInject.ts (2)

5-5: Good import addition for ConnectSettings.

This import correctly supports the type used in the new switchTransport method.


55-57: Implementation follows established pattern.

The switchTransport implementation follows the same pattern as other methods in this file - delegating to lowLevelApi when available and providing a fallback when not.

packages/hd-common-connect-sdk/src/index.ts (1)

172-172: Properly added to exported SDK.

The function is correctly added to the HardwareCommonConnectSdk export.

packages/hd-ble-sdk/src/index.ts (1)

181-181: Properly added to exported SDK.

The function is correctly added to the HardwareBleSdk export.

packages/hd-web-sdk/src/index.ts (3)

230-240: Well-implemented transport switching function!

The function looks good. It properly handles the case when the iframe isn't initialized by throwing an appropriate error, as previously suggested.


279-279: Great integration of switchTransport into HardwareSDKLowLevel!

The function is now properly integrated into the HardwareSDKLowLevel interface, allowing users to switch transport methods from the low-level API.


292-292: Good job adding switchTransport to HardwareWebSdk!

The function is correctly integrated into the HardwareWebSdk interface, providing a consistent API across different SDK variants.

@originalix originalix merged commit 7231f9f into onekey Mar 21, 2025
9 checks passed
@originalix originalix deleted the fix/web-usb branch March 21, 2025 07:11
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.

3 participants