-
Notifications
You must be signed in to change notification settings - Fork 21
Fix/android ble #557
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: onekey
Are you sure you want to change the base?
Fix/android ble #557
Conversation
WalkthroughReduces BLE connect retry attempts in core and updates Android bonding to check existing bonded devices before pairing. Exports Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant Core as Core BLE Connect
participant Device
rect rgba(0,128,0,0.05)
App->>Core: connectDeviceForBle(uuid)
loop Attempt (max 1 retry on BleTimeoutError)
Core->>Device: Connect
alt Success
Device-->>Core: Connected
Core-->>App: Connection handle
else BleTimeoutError
Core->>Core: bleTimeoutRetry++, wait 3s
Note right of Core: Retry threshold: <= 1
else Other error
Core-->>App: Throw error
end
end
end
sequenceDiagram
participant App
participant RN as React Native Layer
participant BM as BleManager
participant Dev as Device
App->>RN: ensureBonded(uuid)
RN->>BM: getBondedDevices()
BM-->>RN: [bonded list]
alt Already bonded (case-insensitive match)
RN-->>App: Skip pairing
else Not bonded
RN->>BM: pairDevice(uuid)
BM-->>RN: bond state
RN->>BM: onDeviceBondState(uuid) (if bonded)
RN-->>App: Pairing result
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) ✅ license/snyk check is complete. No issues have been found. (View Details) |
48e6606
to
534efb8
Compare
There was a problem hiding this 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/core/index.ts (1)
595-614
: Localize BLE retry state, fix typo, and avoid recursion
- Global bleTimeoutRetry is shared across calls and devices. Scope it per attempt.
- Replace recursion with a loop to avoid stack growth and make backoff clearer.
- Fix "Bletooth" typo and update the stale "retry 6 times" comment.
Apply:
-/** - * If the Bluetooth connection times out, retry 6 times - */ -let bleTimeoutRetry = 0; - -async function connectDeviceForBle(method: BaseMethod, device: Device) { - try { - await device.acquire(); - await device.initialize(parseInitOptions(method)); - } catch (err) { - if (err.errorCode === HardwareErrorCode.BleTimeoutError && bleTimeoutRetry <= 1) { - bleTimeoutRetry += 1; - Log.debug(`Bletooth connect timeout and will retry, retry count: ${bleTimeoutRetry}`); - await wait(3000); - await connectDeviceForBle(method, device); - } else { - throw err; - } - } -} +/** + * If the Bluetooth connection times out, retry up to 2 times with backoff. + */ +async function connectDeviceForBle(method: BaseMethod, device: Device) { + let retries = 0; + let delay = 3000; + // try acquire + initialize with bounded retries + // eslint-disable-next-line no-constant-condition + while (true) { + try { + await device.acquire(); + await device.initialize(parseInitOptions(method)); + return; + } catch (err: any) { + if (err?.errorCode === HardwareErrorCode.BleTimeoutError && retries < 2) { + retries += 1; + Log.debug(`Bluetooth connect timeout; retrying (${retries}/2) after ${delay}ms`); + await wait(delay); + delay = Math.min(delay * 2, 8000); + continue; + } + throw err; + } + } +}Also remove the now-unused reset:
- bleTimeoutRetry = 0;
Also applies to: 707-713
📜 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
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/core/src/core/index.ts
(1 hunks)packages/hd-transport-react-native/src/index.ts
(2 hunks)
⏰ 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). (1)
- GitHub Check: lint (22)
🔇 Additional comments (1)
packages/hd-transport-react-native/src/index.ts (1)
22-27
: Export and document getBondedDevices
EnsureBleManager.ts
actually exportsgetBondedDevices
, add a JSDoc describing its return shape (avoidany[]
), and confirm it’s properly guarded for Android.
8d6e0b5
to
c342d12
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
📜 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
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (15)
packages/connect-examples/electron-example/package.json
(2 hunks)packages/connect-examples/expo-example/package.json
(2 hunks)packages/connect-examples/expo-playground/package.json
(2 hunks)packages/core/package.json
(2 hunks)packages/hd-ble-sdk/package.json
(2 hunks)packages/hd-common-connect-sdk/package.json
(2 hunks)packages/hd-transport-electron/package.json
(2 hunks)packages/hd-transport-emulator/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-web-device/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)
⏰ 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: Publish v1.1.11-alpha.1
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (11)
packages/shared/package.json (1)
3-3
: Version bump looks good.No other manifest changes. Safe metadata update.
packages/connect-examples/expo-playground/package.json (1)
3-3
: App version bump is fine.Private example app; no publish impact.
packages/hd-transport/package.json (1)
3-3
: Transport package version bump is OK.No script/deps changes. Proceed.
packages/core/package.json (1)
3-3
: Core version bump is OK.Matches the alpha series.
packages/hd-transport-emulator/package.json (2)
3-3
: Emulator package version bump is fine.No other changes.
27-28
: Internal deps aligned — LGTM.Both @onekeyfe deps point to 1.1.11-alpha.1.
packages/hd-transport-lowlevel/package.json (1)
3-3
: All prerelease deps at 1.1.11-alpha.1; no 1.1.10 pins found.packages/hd-transport-electron/package.json (1)
3-3
: Approve prerelease bumpExample apps reference @onekeyfe/[email protected]. Publish hd-core and hd-transport before this package to avoid install errors.
packages/hd-transport-react-native/package.json (1)
3-3
: Verify Metro alias for BLE utils
Only ‘@onekeyfe/react-native-ble-utils’ is imported (packages/hd-transport-react-native/src/BleManager.ts); no direct ‘react-native-ble-utils’ imports found. Confirm your Metro bundler resolves the alias mapping correctly.packages/connect-examples/electron-example/package.json (1)
5-5
: Approve changes: version alignment confirmed. Bothpackage.json
references for@onekeyfe/hd-transport-electron
are pinned to"1.1.11-alpha.1"
. Smoke-test the electron example after publishing to catch any prerelease resolution hiccups.packages/connect-examples/expo-example/package.json (1)
3-3
: Example app pinned to alpha — LGTM.Exact pins avoid mixing stable and alpha. No action needed.
Also applies to: 22-25
c342d12
to
4c2adec
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/hd-transport-lowlevel/package.json (1)
8-10
: Tag prerelease as alpha in publishConfig.Prevent accidental
latest
tag on publish."publishConfig": { - "access": "public" + "access": "public", + "tag": "alpha" },packages/hd-transport-http/package.json (1)
14-16
: Set prerelease npm dist-tag to “alpha”.Keep prereleases off
latest
."publishConfig": { - "access": "public" + "access": "public", + "tag": "alpha" },packages/hd-transport-emulator/package.json (1)
14-16
: Add alpha dist-tag to publishConfig.Avoid pushing prereleases to latest.
"publishConfig": { - "access": "public" + "access": "public", + "tag": "alpha" },
♻️ Duplicate comments (7)
packages/core/package.json (1)
3-3
: Lockfile and publish order still need confirmation.Prior note stands: update the lockfile and publish in dep order for alpha.2 to avoid 404s.
Run:
#!/bin/bash set -euo pipefail echo "Lockfiles present:"; fd -a '^(pnpm-lock\.yaml|yarn\.lock|package-lock\.json)$' || true # Compare against the default branch if available git fetch origin main || true echo "Lockfile changes vs origin/main:"; git diff --name-only origin/main...HEAD | rg -N '(^|/)pnpm-lock\.yaml|yarn\.lock|package-lock\.json' || echo "No lockfile changes detected" git fetch origin master || true echo "Lockfile changes vs origin/master:"; git diff --name-only origin/master...HEAD | rg -N '(^|/)pnpm-lock\.yaml|yarn\.lock|package-lock\.json' || echo "No lockfile changes detected"packages/hd-ble-sdk/package.json (1)
13-15
: Set npm dist-tag to “alpha” for prereleases.Without it, publishing may update
latest
. Add the tag."publishConfig": { - "access": "public" + "access": "public", + "tag": "alpha" },packages/connect-examples/expo-playground/package.json (1)
20-22
: Use workspace protocol for internal deps.Avoid manual bumps in examples; follow workspace versions.
- "@onekeyfe/hd-core": "1.1.11-alpha.2", - "@onekeyfe/hd-shared": "1.1.11-alpha.2", - "@onekeyfe/hd-web-sdk": "1.1.11-alpha.2", + "@onekeyfe/hd-core": "workspace:*", + "@onekeyfe/hd-shared": "workspace:*", + "@onekeyfe/hd-web-sdk": "workspace:*",packages/hd-transport-http/package.json (1)
27-31
: Plan axios upgrade to ^1.x in a follow-up.^0.27.2 is old. Upgrade in a separate PR, run tests, and address any API changes.
packages/hd-transport-web-device/package.json (1)
9-11
: Add alpha dist-tag to prevent prerelease from becoming latest.Keep prereleases off the default npm tag.
"publishConfig": { - "access": "public" + "access": "public", + "tag": "alpha" },packages/hd-web-sdk/package.json (1)
10-12
: Pin prerelease to alpha dist-tag.Prevents accidental promotion to latest on npm.
"publishConfig": { - "access": "public" + "access": "public", + "tag": "alpha" },packages/hd-common-connect-sdk/package.json (1)
13-15
: Add alpha dist-tag to publishConfig.Keep prereleases off npm latest.
"publishConfig": { - "access": "public" + "access": "public", + "tag": "alpha" },
📜 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
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (15)
packages/connect-examples/electron-example/package.json
(2 hunks)packages/connect-examples/expo-example/package.json
(2 hunks)packages/connect-examples/expo-playground/package.json
(2 hunks)packages/core/package.json
(2 hunks)packages/hd-ble-sdk/package.json
(2 hunks)packages/hd-common-connect-sdk/package.json
(2 hunks)packages/hd-transport-electron/package.json
(2 hunks)packages/hd-transport-emulator/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-web-device/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)
⏰ 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: Publish v1.1.11-alpha.2
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (10)
packages/hd-transport-react-native/package.json (1)
22-24
: Alias support confirmed. Yarn 1.22.22 supportsnpm:
aliases and the lockfile includes
@onekeyfe/react-native-ble-utils@npm:react-native-ble-utils@^0.1.7
.packages/hd-transport/package.json (1)
3-3
: LGTM on the version bump.No other changes. Looks clean.
packages/hd-transport-electron/package.json (2)
3-3
: LGTM on the version bump.No issues spotted here.
28-30
: Check downstream apps’ Electron versions
Peer dependencies now requireelectron >=25.0.0
andelectron-log >=4.0.0
. Confirm all consuming apps upgrade to Electron 25+ (and electron-log 4+) before publishing.packages/hd-transport-web-device/package.json (1)
3-3
: Version and deps aligned. LGTM.Also applies to: 23-28
packages/connect-examples/expo-example/package.json (1)
3-3
: Example app versions aligned. LGTM.Private package; no publish config needed.
Also applies to: 22-25
packages/hd-transport-emulator/package.json (2)
3-3
: Version and deps aligned. LGTM.Also applies to: 27-28
29-29
: Upgrade axios to ^1.x and verify compatibility across all modules that import axios.packages/hd-web-sdk/package.json (1)
3-3
: Version and deps aligned. LGTM.Also applies to: 24-27
packages/hd-common-connect-sdk/package.json (1)
3-3
: Version and deps aligned. LGTM.Also applies to: 23-28
"version": "1.1.11-alpha.2", | ||
"author": "OneKey", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Mark example app as private to avoid accidental publish.
Add "private": true
. It’s an example; don’t risk publishing it.
"executableName": "onekey-hardware-example",
"version": "1.1.11-alpha.2",
+ "private": true,
"author": "OneKey",
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
"version": "1.1.11-alpha.2", | |
"author": "OneKey", | |
"executableName": "onekey-hardware-example", | |
"version": "1.1.11-alpha.2", | |
"private": true, | |
"author": "OneKey", |
🤖 Prompt for AI Agents
In packages/connect-examples/electron-example/package.json around lines 5 to 6,
the package is not marked private and could be accidentally published; add a
"private": true field to package.json (e.g., as a sibling to "version" and
"author") to prevent npm publishing and commit the change.
"dependencies": { | ||
"@onekeyfe/hd-transport-electron": "1.1.10", | ||
"@onekeyfe/hd-transport-electron": "1.1.11-alpha.2", | ||
"@stoprocent/noble": "2.3.4", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Use workspace protocol for internal deps to cut version-churn.
Point internal packages to workspace:*
if you use Yarn/PNPM workspaces.
"dependencies": {
- "@onekeyfe/hd-transport-electron": "1.1.11-alpha.2",
+ "@onekeyfe/hd-transport-electron": "workspace:*",
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
"dependencies": { | |
"@onekeyfe/hd-transport-electron": "1.1.10", | |
"@onekeyfe/hd-transport-electron": "1.1.11-alpha.2", | |
"@stoprocent/noble": "2.3.4", | |
"dependencies": { | |
"@onekeyfe/hd-transport-electron": "workspace:*", | |
"@stoprocent/noble": "2.3.4", |
🤖 Prompt for AI Agents
In packages/connect-examples/electron-example/package.json around lines 24 to
26, the dependency entries for internal packages are pinned to explicit
versions; replace their version strings with the workspace protocol (e.g.,
"workspace:*") so workspace-aware package managers (Yarn/PNPM) will resolve them
to the local workspace packages and avoid version churn. Update each internal
package dependency to use "workspace:*" and leave external dependencies
unchanged.
"@onekeyfe/hd-shared": "1.1.11-alpha.2", | ||
"@onekeyfe/hd-transport": "1.1.11-alpha.2", | ||
"@onekeyfe/react-native-ble-utils": "npm:react-native-ble-utils@^0.1.7", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Pin internal deps consistently across the monorepo.
All internal @onekeyfe deps here use 1.1.11-alpha.2. Make sure every package matches to prevent install 404s.
Run:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Internal versions:"
jq -r 'select(.name|startswith("@onekeyfe/")) | "\(.name)@\(.version)"' packages/**/package.json | sort
echo
echo "Cross-dep pins not matching their package version:"
python - <<'PY'
import json,glob,os,sys
issues=[]
for path in glob.glob("packages/**/package.json", recursive=True):
data=json.load(open(path))
name=data.get("name","")
version=data.get("version","")
for sec in ("dependencies","devDependencies","peerDependencies"):
for k,v in (data.get(sec) or {}).items():
if k.startswith("@onekeyfe/") and v==version: continue
# naive check: ensure internal deps are pinned to some 1.1.11-alpha.2
for sec in ("dependencies","devDependencies"):
deps=(data.get(sec) or {})
for k,v in deps.items():
if k.startswith("@onekeyfe/") and "1.1.11-alpha.2" not in v:
issues.append((path,k,v))
for p,k,v in issues: print(f"{p}: {k} -> {v}")
PY
Length of output: 1550
Pin internal @onekeyfe deps to 1.1.11-alpha.2 across the repo
Detected mismatches:
- packages/connect-examples/expo-example/package.json: @onekeyfe/react-native-ble-utils → ^0.1.3
- packages/hd-transport-react-native/package.json: @onekeyfe/react-native-ble-utils → npm:react-native-ble-utils@^0.1.7
- packages/hd-web-sdk/package.json: @onekeyfe/cross-inpage-provider-core → ^0.0.17
Update these to 1.1.11-alpha.2 to prevent install errors.
🤖 Prompt for AI Agents
In packages/hd-transport-react-native/package.json lines 22-24, the internal
@onekeyfe dependency versions are inconsistent; update the mismatched entries
across the repo to the pinned version 1.1.11-alpha.2. Replace
"@onekeyfe/react-native-ble-utils": "npm:react-native-ble-utils@^0.1.7" (and any
other occurrences noted in packages/connect-examples/expo-example/package.json
and packages/hd-web-sdk/package.json) with the exact internal package reference
and version "@onekeyfe/react-native-ble-utils": "1.1.11-alpha.2" (and set
"@onekeyfe/cross-inpage-provider-core": "1.1.11-alpha.2" and any other @onekeyfe
packages listed to "1.1.11-alpha.2") so all internal @onekeyfe deps match and
avoid install errors.
{ | ||
"name": "@onekeyfe/hd-shared", | ||
"version": "1.1.10", | ||
"version": "1.1.11-alpha.2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Version bump looks good. Add a pre-release publish tag.
To avoid publishing this alpha as "latest" on npm, set a publish tag.
Apply:
- "publishConfig": {
- "access": "public"
- },
+ "publishConfig": {
+ "access": "public",
+ "tag": "next"
+ },
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
"version": "1.1.11-alpha.2", | |
"publishConfig": { | |
"access": "public", | |
"tag": "next" | |
}, |
🤖 Prompt for AI Agents
packages/shared/package.json around line 3: add a pre-release publish tag so
this alpha version is not published as "latest"; update the package.json to
include a publishConfig with a tag (for example "alpha") or otherwise ensure the
CI/npm publish step uses --tag alpha; specifically add a publishConfig: { "tag":
"alpha" } entry in package.json so npm will publish this release under the alpha
tag.
4c2adec
to
8fdf473
Compare
8fdf473
to
89b77e0
Compare
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.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/core/src/core/index.ts (2)
605-610
: BLE timeout retries cut to 2 — make it configurable and fix log typoTwo retries are fine, but some devices may need more. Read an optional max from method.payload and fix “Bletooth” typo.
Apply:
} catch (err) { - if (err.errorCode === HardwareErrorCode.BleTimeoutError && bleTimeoutRetry <= 1) { - bleTimeoutRetry += 1; - Log.debug(`Bletooth connect timeout and will retry, retry count: ${bleTimeoutRetry}`); + const maxRetries = + typeof method?.payload?.bleTimeoutMaxRetry === 'number' + ? method.payload.bleTimeoutMaxRetry + : 2; + if (err?.errorCode === HardwareErrorCode.BleTimeoutError && bleTimeoutRetry < maxRetries) { + bleTimeoutRetry += 1; + Log.debug(`Bluetooth connect timeout. Retrying (${bleTimeoutRetry}/${maxRetries})...`); await wait(3000); await connectDeviceForBle(method, device); } else { throw err; }
600-614
: Prefer loop over recursion for retriesAvoid stack growth and simplify flow with an iterative retry.
Apply:
-async function connectDeviceForBle(method: BaseMethod, device: Device) { - try { - await device.acquire(); - await device.initialize(parseInitOptions(method)); - } catch (err) { - if (err.errorCode === HardwareErrorCode.BleTimeoutError && bleTimeoutRetry <= 1) { - bleTimeoutRetry += 1; - Log.debug(`Bletooth connect timeout and will retry, retry count: ${bleTimeoutRetry}`); - await wait(3000); - await connectDeviceForBle(method, device); - } else { - throw err; - } - } -} +async function connectDeviceForBle(method: BaseMethod, device: Device) { + const maxRetries = + typeof method?.payload?.bleTimeoutMaxRetry === 'number' + ? method.payload.bleTimeoutMaxRetry + : 2; + // ensure counter starts from zero for each ensureConnected cycle + bleTimeoutRetry = Math.min(bleTimeoutRetry, maxRetries); + // attempt + retries + // eslint-disable-next-line no-constant-condition + while (true) { + try { + await device.acquire(); + await device.initialize(parseInitOptions(method)); + return; + } catch (err: any) { + if (err?.errorCode !== HardwareErrorCode.BleTimeoutError || bleTimeoutRetry >= maxRetries) { + throw err; + } + bleTimeoutRetry += 1; + Log.debug(`Bluetooth connect timeout. Retrying (${bleTimeoutRetry}/${maxRetries})...`); + await wait(3000); + } + } +}packages/connect-examples/electron-example/package.json (1)
7-9
: Fix misleading description (nit).Text doesn’t match an Electron hardware example.
- "description": "End-to-end encrypted workspaces for teams", + "description": "Electron example app for OneKey hardware SDK",
♻️ Duplicate comments (9)
packages/shared/package.json (1)
3-3
: Pre-release: add dist-tag to avoid publishing as latestSame as earlier note: tag the alpha release.
Apply:
"publishConfig": { - "access": "public" + "access": "public", + "tag": "next" },packages/hd-transport-react-native/src/index.ts (1)
268-277
: Fix: avoid shadowing, handle bonded/bonding, and remap pairing errors
- Don’t shadow the outer
device
var in.find
.- Treat both
bonded
andbonding
as success.- Wrap
pairDevice
in try/catch and callremapError
.- Compute lowercase UUID once.
- const bondedDevices = await getBondedDevices(); - const existsBondedList = bondedDevices.find( - device => device.id.toLowerCase() === uuid.toLowerCase() - ); - - if (!existsBondedList) { - const bondState = await pairDevice(uuid); - if (bondState.bonding) { - await onDeviceBondState(uuid); - } - } + const targetId = uuid.toLowerCase(); + const bondedDevices = await getBondedDevices(); + const bondedDevice = bondedDevices.find(d => d.id?.toLowerCase?.() === targetId); + if (!bondedDevice) { + try { + const bondState = await pairDevice(uuid); + if (bondState?.bonded || bondState?.bonding) { + await onDeviceBondState(uuid); + } + } catch (e) { + this.Log?.debug('pairDevice error:', e); + remapError(e as any); + } + }packages/hd-transport-web-device/package.json (1)
3-3
: Add publish dist-tag for prereleases.Set
publishConfig.tag
toalpha
to avoid promoting prereleases tolatest
."publishConfig": { - "access": "public" + "access": "public", + "tag": "alpha" },Also applies to: 23-24, 27-27
packages/connect-examples/expo-playground/package.json (1)
3-3
: Optional: use workspace protocol for internal deps.Cuts churn on example apps during version bumps.
- "@onekeyfe/hd-core": "1.1.11-alpha.2", - "@onekeyfe/hd-shared": "1.1.11-alpha.2", - "@onekeyfe/hd-web-sdk": "1.1.11-alpha.2", + "@onekeyfe/hd-core": "workspace:*", + "@onekeyfe/hd-shared": "workspace:*", + "@onekeyfe/hd-web-sdk": "workspace:*",Run to spot any stragglers not on alpha.2 (adjust if using workspaces):
#!/bin/bash # Find internal @onekeyfe deps not pinned to 1.1.11-alpha.2 or workspace:*. fd package.json -E node_modules | while read -r f; do jq -r '.name as $n | [(.dependencies//{}),(.devDependencies//{}),(.peerDependencies//{})] | add | to_entries[] | select(.key|startswith("@onekeyfe/")) | "\($n)\t\(.key)\t\(.value)"' "$f" done | awk -F'\t' '$3!="1.1.11-alpha.2" && $3!="workspace:*" {print}'Also applies to: 20-22
packages/hd-ble-sdk/package.json (1)
23-26
: Deps aligned. Add npm dist-tag “alpha”.Same reasoning as other packages.
"publishConfig": { - "access": "public" + "access": "public", + "tag": "alpha" },packages/connect-examples/electron-example/package.json (2)
5-6
: Mark example app as private.Avoid accidental publish.
"executableName": "onekey-hardware-example", "version": "1.1.11-alpha.2", + "private": true,
25-25
: Use workspace protocol for internal dep (optional).Cuts version churn in examples if using workspaces.
- "@onekeyfe/hd-transport-electron": "1.1.11-alpha.2", + "@onekeyfe/hd-transport-electron": "workspace:*",packages/hd-web-sdk/package.json (1)
10-12
: Add npm dist-tag to avoid publishing alpha as latest.Ensure this prerelease always publishes under the alpha tag.
Apply:
"publishConfig": { - "access": "public" + "access": "public", + "tag": "alpha" },packages/hd-common-connect-sdk/package.json (1)
13-15
: Add npm dist-tag to keep prereleases off latest.Same as hd-web-sdk: add tag: "alpha".
"publishConfig": { - "access": "public" + "access": "public", + "tag": "alpha" },
📜 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
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (18)
packages/connect-examples/electron-example/package.json
(2 hunks)packages/connect-examples/expo-example/package.json
(2 hunks)packages/connect-examples/expo-playground/package.json
(2 hunks)packages/connect-examples/shared-constants/package.json
(1 hunks)packages/core/package.json
(2 hunks)packages/core/src/core/index.ts
(1 hunks)packages/hd-ble-sdk/package.json
(2 hunks)packages/hd-common-connect-sdk/package.json
(2 hunks)packages/hd-transport-electron/package.json
(2 hunks)packages/hd-transport-emulator/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-react-native/src/index.ts
(2 hunks)packages/hd-transport-web-device/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)
⏰ 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: Publish v1.1.11-alpha.2
- GitHub Check: lint (22)
- GitHub Check: build (22)
🔇 Additional comments (10)
packages/hd-transport/package.json (1)
3-3
: Add npm dist-tag for alpha release
- In packages/hd-transport/package.json, add or update the publishConfig block to include
"tag": "next"
alongside"access": "public"
.- Confirm this aligns with your npm publish plan and manually verify no other packages have stray alpha versions.
packages/connect-examples/shared-constants/package.json (1)
3-3
: LGTMPrivate package version bump is fine; no publish impact.
packages/hd-transport-react-native/src/index.ts (1)
22-27
: Imports look good.New BleManager exports are referenced cleanly.
packages/hd-transport-lowlevel/package.json (1)
3-3
: Versions aligned to alpha.2 — OK.No issues spotted with the sync of internal deps.
Also applies to: 22-24
packages/hd-transport-react-native/package.json (1)
3-3
: Version alignment looks good.Matches the repo-wide 1.1.11-alpha.2 pin.
packages/hd-transport-electron/package.json (1)
3-3
: Good: version synced to 1.1.11-alpha.2.packages/hd-transport-http/package.json (1)
3-3
: HTTP transport version bump is consistent.packages/hd-ble-sdk/package.json (1)
3-3
: BLE SDK version pin OK.packages/connect-examples/expo-example/package.json (1)
3-3
: Alpha pin alignment looks consistent.Version and internal deps all pinned to 1.1.11-alpha.2. Private example app is fine to pin exact. No action.
Also applies to: 22-25
packages/hd-common-connect-sdk/package.json (1)
3-3
: Deps pinned coherently to 1.1.11-alpha.2.Looks consistent across transports and core. Nice.
Also applies to: 23-29
{ | ||
"name": "@onekeyfe/hd-core", | ||
"version": "1.1.12-alpha.0", | ||
"version": "1.1.11-alpha.2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Versions aligned — add npm dist-tag for alpha publish
Core and its @onekeyfe deps are pinned to 1.1.11-alpha.2. Looks consistent. Add a dist-tag to keep “latest” clean.
Apply:
"publishConfig": {
- "access": "public"
+ "access": "public",
+ "tag": "next"
},
Also applies to: 28-29
🤖 Prompt for AI Agents
In packages/core/package.json around line 3 (and also apply the same change to
the other package.jsons referenced at lines 28-29), the package is using an
alpha version but no npm dist-tag is set; add a publishConfig entry to pin alpha
releases to the "alpha" dist-tag by adding "publishConfig": { "tag": "alpha" }
(place it near the top-level fields, e.g., after "version") so npm publish will
use the alpha tag and keep "latest" clean.
"@onekeyfe/hd-core": "1.1.11-alpha.2", | ||
"@onekeyfe/hd-shared": "1.1.11-alpha.2", | ||
"@onekeyfe/hd-transport": "1.1.11-alpha.2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Deps are consistent. Add alpha dist-tag.
Prevent prerelease from becoming latest on npm.
"publishConfig": {
- "access": "public"
+ "access": "public",
+ "tag": "alpha"
},
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
"@onekeyfe/hd-core": "1.1.11-alpha.2", | |
"@onekeyfe/hd-shared": "1.1.11-alpha.2", | |
"@onekeyfe/hd-transport": "1.1.11-alpha.2", | |
"publishConfig": { | |
"access": "public", | |
"tag": "alpha" | |
}, |
🤖 Prompt for AI Agents
In packages/hd-transport-electron/package.json around lines 28–30, the
prerelease dependency versions need an npm dist-tag so the alpha release does
not become the latest; add a publishConfig with "tag": "alpha" to package.json
(or ensure CI invokes npm publish --tag alpha) so published prereleases use the
"alpha" dist-tag instead of "latest".
{ | ||
"name": "@onekeyfe/hd-transport-emulator", | ||
"version": "1.1.12-alpha.0", | ||
"version": "1.1.11-alpha.2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Use alpha dist-tag on publish.
Same prerelease safeguard here.
"publishConfig": {
- "access": "public"
+ "access": "public",
+ "tag": "alpha"
},
Also applies to: 27-28
🤖 Prompt for AI Agents
In packages/hd-transport-emulator/package.json around line 3 (and also update
lines 27-28), add a publishConfig entry to set the npm dist-tag to "alpha" for
this prerelease; specifically add or update "publishConfig": { "tag": "alpha" }
in package.json so publishing uses the alpha dist-tag, and ensure any other
package.json entries referenced at lines 27-28 follow the same change.
"@onekeyfe/hd-shared": "1.1.11-alpha.2", | ||
"@onekeyfe/hd-transport": "1.1.11-alpha.2", | ||
"axios": "^0.27.2", | ||
"secure-json-parse": "^4.0.0" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Plan axios upgrade to ^1.x in a follow-up.
^0.27.2 is dated; upgrade separately and run tests.
Also add an alpha dist-tag:
"publishConfig": {
- "access": "public"
+ "access": "public",
+ "tag": "alpha"
},
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
"@onekeyfe/hd-shared": "1.1.11-alpha.2", | |
"@onekeyfe/hd-transport": "1.1.11-alpha.2", | |
"axios": "^0.27.2", | |
"secure-json-parse": "^4.0.0" | |
} | |
"publishConfig": { | |
"access": "public", | |
"tag": "alpha" | |
}, |
🤖 Prompt for AI Agents
In packages/hd-transport-http/package.json around lines 27 to 31, keep the
current axios dependency as-is for now but add a follow-up task to upgrade to
^1.x and run the test suite; also add a publishConfig with "tag": "alpha" so npm
publishes use the alpha dist-tag (e.g. add "publishConfig": {"tag":"alpha"})
before publishing. Ensure tests pass after adding publishConfig and create a
short issue/PR to perform the axios^1.x upgrade and test changes separately.
"@onekeyfe/hd-shared": "1.1.11-alpha.2", | ||
"@onekeyfe/hd-transport": "1.1.11-alpha.2", | ||
"@onekeyfe/react-native-ble-utils": "npm:react-native-ble-utils@^0.1.7", | ||
"react-native-ble-plx": "3.5.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify npm alias for @onekeyfe/react-native-ble-utils.
You map the scoped name to external react-native-ble-utils@^0.1.7
. If this is intentional, consider depending on react-native-ble-utils
directly to avoid scope confusion. If not, pin to the internal version like other @onekeyfe deps.
Proposed options:
- "@onekeyfe/react-native-ble-utils": "npm:react-native-ble-utils@^0.1.7",
+ "react-native-ble-utils": "^0.1.7",
or
- "@onekeyfe/react-native-ble-utils": "npm:react-native-ble-utils@^0.1.7",
+ "@onekeyfe/react-native-ble-utils": "1.1.11-alpha.2",
Also add an alpha dist-tag to avoid publishing under latest:
"publishConfig": {
- "access": "public"
+ "access": "public",
+ "tag": "alpha"
},
🏁 Script executed:
#!/bin/bash
# Locate internal @onekeyfe/react-native-ble-utils package.json
internal_file=$(rg -l '"name": "@onekeyfe/react-native-ble-utils"' -n .)
echo "Internal file: ${internal_file:-not found}"
if [ -n "$internal_file" ]; then
grep -n '"version"' "$internal_file"
fi
# Check publishConfig in hd-transport-react-native
pkg="packages/hd-transport-react-native/package.json"
echo ""
echo "publishConfig snippet from $pkg:"
grep -n '"publishConfig"' -C3 "$pkg" || echo "publishConfig not found"
Length of output: 616
Use direct react-native-ble-utils
and add an alpha dist-tag
The scoped package @onekeyfe/react-native-ble-utils
doesn’t exist internally. Swap the alias for the external package and tag this alpha release to avoid publishing under “latest”.
packages/hd-transport-react-native/package.json
@@ Lines 22-25
- "@onekeyfe/react-native-ble-utils": "npm:react-native-ble-utils@^0.1.7",
+ "react-native-ble-utils": "^0.1.7",
@@ publishConfig
- "access": "public"
+ "access": "public",
+ "tag": "alpha"
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
"@onekeyfe/hd-shared": "1.1.11-alpha.2", | |
"@onekeyfe/hd-transport": "1.1.11-alpha.2", | |
"@onekeyfe/react-native-ble-utils": "npm:react-native-ble-utils@^0.1.7", | |
"react-native-ble-plx": "3.5.0" | |
// In packages/hd-transport-react-native/package.json | |
"dependencies": { | |
"@onekeyfe/hd-shared": "1.1.11-alpha.2", | |
"@onekeyfe/hd-transport": "1.1.11-alpha.2", | |
"react-native-ble-utils": "^0.1.7", | |
"react-native-ble-plx": "3.5.0" | |
}, | |
// … other fields … | |
"publishConfig": { | |
"access": "public", | |
"tag": "alpha" | |
} |
🤖 Prompt for AI Agents
In packages/hd-transport-react-native/package.json around lines 22 to 25, the
dependency uses a nonexistent scoped alias "@onekeyfe/react-native-ble-utils"
and the package needs an alpha dist-tag; replace the aliased dependency with the
upstream package name "react-native-ble-utils" at the appropriate version (e.g.,
^0.1.7 or exact 0.1.7 as needed) and add a publishConfig with "tag": "alpha" (or
ensure npm publish is run with --tag alpha) so this release is published under
the alpha dist-tag instead of latest.
{ | ||
"name": "@onekeyfe/hd-web-sdk", | ||
"version": "1.1.12-alpha.0", | ||
"version": "1.1.11-alpha.2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify workspace-wide prerelease tagging and version sync.
Confirm all packages on 1.1.11-alpha.2 have publishConfig.tag set to alpha and no stragglers remain.
Also applies to: 24-27
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find package.json files under packages/
fd -a package.json packages | while read -r f; do
python - <<'PY' "$f"
import json,sys,os
p=sys.argv[1]
with open(p) as fh:
j=json.load(fh)
name=j.get("name", os.path.dirname(p))
ver=j.get("version","")
pc=j.get("publishConfig",{})
tag=(pc or {}).get("tag")
if "-" in ver and not (isinstance(pc,dict) and tag=="alpha"):
print(f"MISSING alpha tag: {p} name={name} version={ver} publishConfig={pc}")
PY
done
echo
echo "Check internal dep pins:"
rg -nP '"@onekeyfe/[^"]+":\s*"1\.1\.11-alpha\.2"' packages | wc -l
Length of output: 4256
Add missing alpha publish tags
- 16 packages at version
1.1.11-alpha.2
lack apublishConfig.tag: "alpha"
entry. - 37 internal dependency references to
"1.1.11-alpha.2"
were found—verify these are intentional.
🤖 Prompt for AI Agents
packages/hd-web-sdk/package.json lines 3-3: package version "1.1.11-alpha.2" is
missing a publishConfig.tag set to "alpha"; add a publishConfig block with
"tag": "alpha" to this package.json (and the other 15 packages at the same
version) so npm publishes them under the alpha dist-tag, and review/update the
37 internal dependency entries that reference "1.1.11-alpha.2" to ensure they
intentionally point to the pre-release (adjust to the correct version or range
if they should reference a stable or workspace reference instead).
Summary by CodeRabbit