fix: support Pro2 and Neo BLE flows - #877
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
wabicai
changed the title
|
@codex review |
|
@codex security review |
|
You have reached your Codex usage limits for security reviews. Please try again later. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d42192d803
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
originalix
left a comment
originalix
left a comment
There was a problem hiding this comment.
One additional inline correctness finding.
| typeof firmwareMessage === 'string' && | ||
| firmwareMessage.trim().toLowerCase() === 'link disabled' | ||
| ) { | ||
| throw createProtocolV2LinkDisabledError(failureCode, firmwareMessage); |
There was a problem hiding this comment.
P1: Normalize this failure for established Protocol V2 sessions too
This conversion only runs during protocol probing. If the BLE session is already established and USB subsequently takes ownership, the same Failure_ProcessError / exact link disabled response goes through DeviceCommands' common Failure handling and falls through to HardwareErrorCode.RuntimeError (800), so callers still never receive 723. This affects ordinary commands such as device-state and portfolio-sync requests, not only connection setup.
Please centralize the exact code/message predicate and reuse it in both probeProtocolV2() and the normal DeviceCommands Failure mapper, then add an established-session test that asserts the public error is BleUnavailableWhileUsbConnected (723). This is separate from preserving the probe sentinel through DeviceConnector.acquire().
There was a problem hiding this comment.
已按这个建议做了。
isProtocolV2LinkDisabledFailure() 抽到 hd-transport/src/protocols/v2/errors.ts，probeProtocolV2() 和 DeviceCommands 的 Failure 映射共用。已建立的 V2 session 再收到 Failure_ProcessError / 5 + link disabled，也会直接变成 723，不再掉进 RuntimeError (800)。
测试：
packages/core/__tests__/DeviceCommands.test.ts（含 numeric5）packages/hd-transport/__tests__/protocol-v2.test.js
|
Review 意见已在
|
4 participants
Summary
BleUnavailableWhileUsbConnected(723) without resetting or retrying the BLE probefirmware-pro2to dev133389a08and regenerate the corrected TONsigning_messagefieldWhy
A Find My-bound Pro2 can advertise as
Pro2 22D8 - Find My, while connected device information may contain a truncated suffix. The matcher tolerates spacing, separator, and capitalization variations without requiring one exact suffix string, and the SDK displays the originalPro2 22D8name.Neo advertises as
Neofollowed by its four-character suffix. Both products use the same Protocol V2 discovery and communication path.When USB owns the firmware link, Pro2 and Neo answer the BLE Protocol V2 Ping with
Failure_ProcessError(5), messagelink disabled. This is expected USB-priority arbitration, not a busy command queue or broken BLE link. The SDK now returns code 723 immediately and preserves the firmware failure details; it does not reset or retry the probe.The updated Protocol V2 firmware schema corrects TON field 2 from
signning_messagetosigning_message. Pro2 and Neo do not retain compatibility with the old Protocol V2 spelling. Core continues acceptingsignning_messageonly for the already released Pro/Protocol V1 response schema.Physical-device verification
Pro2 22D8 - Find My; SDK search displaysPro2 22D8link disabledresponse is treated as the expected terminal USB-priority errorApp integration review
app-monorepoalready treats Pro2 and Neo as the same Protocol V2 product family for device settings. AfterDeviceSettingsSet, the SDK reloadsDeviceSettingsand emitsDEVICE.STATE; the App background runtime waits for DB persistence and then notifies the main runtime. No duplicate App-side settings write was added.The App consumes code 723 and uses its existing localized “unplug USB and retry” guidance. It no longer infers this condition from arbitrary error message text.
Validation
yarn agent:check --profile commithd-transport,hd-shared, andhd-corebuildsgit diff --checkNotes
The existing SDK
cancel(connectId?)API already covers Pro2 and Neo calls; no duplicate cancellation path is introduced. The SDK packages were published as1.2.0-alpha.124with npm dist-tagnext.