-
Notifications
You must be signed in to change notification settings - Fork 528
Feat/hw fixes on x #12911
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: x
Are you sure you want to change the base?
Feat/hw fixes on x #12911
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,6 +20,8 @@ import { getAppStaticResourcesPath } from '../resoucePath'; | |
|
|
||
| const RESOURCE = 'ble-pair'; | ||
| const PROCESS_NAME = 'onekey-ble-pair'; | ||
| // Matches the SDK's /connect cancelled/i -> BlePairingCancelled (10310). | ||
| const PAIR_CANCELLED_REASON = 'connect cancelled: BLE pairing declined by user'; | ||
| // The user has up to the OS pairing window to confirm; keep some headroom. | ||
| const PAIR_TIMEOUT_MS = 60_000; | ||
|
|
||
|
|
@@ -76,13 +78,15 @@ export function isBlePairAvailable(): boolean { | |
| function runHelper( | ||
| args: string[], | ||
| onEvent?: (event: IBlePairEvent) => void, | ||
| registerDecide?: (decide: (decision: IPairDecision) => void) => void, | ||
| ): Promise<IBlePairEvent[]> { | ||
| return new Promise((resolve, reject) => { | ||
| const helperPath = resolveHelperPath(); | ||
| logger.info(`[BlePair] spawning ${helperPath} ${args.join(' ')}`); | ||
|
|
||
| // stdin carries the pair decision (confirm/cancel). | ||
| const child = spawn(helperPath, args, { | ||
| stdio: ['ignore', 'pipe', 'pipe'], | ||
| stdio: ['pipe', 'pipe', 'pipe'], | ||
| }); | ||
|
|
||
| const spawnedAt = Date.now(); | ||
|
|
@@ -98,6 +102,23 @@ function runHelper( | |
| child.kill(); | ||
| }, PAIR_TIMEOUT_MS); | ||
|
|
||
| // Cancel goes through stdin, not kill(): only a live helper can decline the | ||
| // request, which is what makes Windows send the device an SMP Pairing Failed. | ||
| registerDecide?.((decision) => { | ||
| if (settled) return; | ||
| if (decision === 'cancel') { | ||
| lastError = PAIR_CANCELLED_REASON; | ||
|
Comment on lines
+109
to
+110
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 在 Windows Desktop(Electron main 为单 JS runtime,WinRT 配对状态由独立 helper 子进程持有)中,用户取消时这里先写入可映射为 AGENTS.md reference: AGENTS.md:L36-L38 Useful? React with 👍 / 👎. |
||
| } | ||
| child.stdin?.write(`${decision}\n`, (error) => { | ||
| if (!error) return; | ||
| logger.warn( | ||
| `[BlePair] failed to send '${decision}' to helper: ${error.message}`, | ||
| ); | ||
| // Pipe is gone; no clean decline left. | ||
| if (decision === 'cancel') child.kill(); | ||
| }); | ||
| }); | ||
|
|
||
| const settle = (fn: () => void) => { | ||
| if (settled) return; | ||
| settled = true; | ||
|
|
@@ -170,6 +191,22 @@ function runHelper( | |
| }); | ||
| } | ||
|
|
||
| /** The host's half of the BLE numeric comparison. */ | ||
| export type IPairDecision = 'confirm' | 'cancel'; | ||
|
|
||
| // Only `pair` registers here. | ||
| let decideActivePair: ((decision: IPairDecision) => void) | null = null; | ||
|
|
||
| /** | ||
| * Answer the in-flight OS pairing ceremony. Returns false when none is waiting. | ||
| */ | ||
| export function decideActivePairing(decision: IPairDecision): boolean { | ||
| const decide = decideActivePair; | ||
| if (!decide) return false; | ||
| decide(decision); | ||
| return true; | ||
| } | ||
|
|
||
| /** | ||
| * Pair `address` (a colon/dash BLE MAC) at the OS level, streaming the | ||
| * numeric-comparison pin to `onPin` so the UI can show it. Resolves once the | ||
|
|
@@ -190,10 +227,18 @@ export async function ensureDevicePaired( | |
| // of the "who is holding the device" experiment. | ||
| args.push('--keep-link'); | ||
| } | ||
| const events = await runHelper(args, (event) => { | ||
| if (event.type === 'pairing') { | ||
| onPin(event.pin); | ||
| } | ||
| const events = await runHelper( | ||
| args, | ||
| (event) => { | ||
| if (event.type === 'pairing') { | ||
| onPin(event.pin); | ||
| } | ||
| }, | ||
| (decide) => { | ||
| decideActivePair = decide; | ||
| }, | ||
| ).finally(() => { | ||
| decideActivePair = null; | ||
| }); | ||
| if (events.some((e) => e.type === 'paired')) { | ||
| return 'paired'; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,7 +37,7 @@ | |
| //! Usage: onekey-ble-pair <pair|is-paired|forget|inspect> --address AA:BB:..:FF | ||
|
|
||
| #[cfg(windows)] | ||
| use std::sync::atomic::AtomicU64; | ||
| use std::sync::atomic::{AtomicU64, AtomicU8, Ordering}; | ||
| #[cfg(windows)] | ||
| use std::sync::{Mutex, OnceLock}; | ||
| #[cfg(windows)] | ||
|
|
@@ -83,6 +83,99 @@ fn init_log_file() { | |
| #[cfg(windows)] | ||
| static ACCEPT_MS: AtomicU64 = AtomicU64::new(u64::MAX); | ||
|
|
||
| // The host's half of the numeric comparison, fed by the parent over stdin. | ||
| // Accepting unconditionally skips that comparison and leaves no way to cancel: | ||
| // after Accept() an abort can only drop the link, which reads as a dead peer. | ||
| #[cfg(windows)] | ||
| const DECISION_PENDING: u8 = 0; | ||
| #[cfg(windows)] | ||
| const DECISION_CONFIRM: u8 = 1; | ||
| #[cfg(windows)] | ||
| const DECISION_CANCEL: u8 = 2; | ||
| #[cfg(windows)] | ||
| static DECISION: AtomicU8 = AtomicU8::new(DECISION_PENDING); | ||
|
|
||
| /// Poll interval while the delegate waits for the parent's decision. | ||
| #[cfg(windows)] | ||
| const DECISION_POLL_MS: u64 = 25; | ||
|
|
||
| /// Backstop so a parent that never answers cannot wedge the helper; the device | ||
| /// gives up on its own pairing window long before this. | ||
| #[cfg(windows)] | ||
| const DECISION_TIMEOUT_MS: u64 = 120_000; | ||
|
Comment on lines
+102
to
+105
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 在 Windows Desktop 的无人应答路径中,Electron main 会在 60 秒时直接 AGENTS.md reference: AGENTS.md:L36-L38 Useful? React with 👍 / 👎. |
||
|
|
||
| /// Hold the ceremony until the parent decides, then Accept (or not) and release | ||
| /// the deferral. Completing WITHOUT Accept is what makes Windows send the device | ||
| /// an SMP Pairing Failed, so it shows the cancel and leaves pairing mode. | ||
| #[cfg(windows)] | ||
| fn await_decision( | ||
| args: &windows::Devices::Enumeration::DevicePairingRequestedEventArgs, | ||
| kind: &str, | ||
| ) -> windows::core::Result<()> { | ||
| let deferral = args.GetDeferral()?; | ||
| let started = t_ms(); | ||
| diag(&format!("{kind}: awaiting host confirmation")); | ||
|
|
||
| let decision = loop { | ||
| let current = DECISION.load(Ordering::SeqCst); | ||
| if current != DECISION_PENDING { | ||
| break current; | ||
| } | ||
| if t_ms().saturating_sub(started) >= DECISION_TIMEOUT_MS { | ||
| // Treat silence as refusal: never bond a device nobody confirmed. | ||
| break DECISION_CANCEL; | ||
| } | ||
| std::thread::sleep(std::time::Duration::from_millis(DECISION_POLL_MS)); | ||
| }; | ||
|
|
||
| let waited = t_ms().saturating_sub(started); | ||
| if decision == DECISION_CONFIRM { | ||
| args.Accept()?; | ||
| ACCEPT_MS.store(t_ms(), Ordering::SeqCst); | ||
| diag(&format!( | ||
| "accepted {kind} after {waited}ms (device confirmation pending)" | ||
| )); | ||
| } else { | ||
| diag(&format!( | ||
| "declined {kind} after {waited}ms; the device is told via SMP Pairing Failed" | ||
| )); | ||
| } | ||
| deferral.Complete()?; | ||
| Ok(()) | ||
| } | ||
|
|
||
| /// Watch stdin for a `confirm` / `cancel` line. EOF (parent closed the pipe or | ||
| /// died) counts as a cancel, so the device is told rather than left to time out. | ||
| #[cfg(windows)] | ||
| fn spawn_decision_reader() { | ||
| std::thread::spawn(|| { | ||
| use std::io::BufRead; | ||
| let stdin = std::io::stdin(); | ||
| for line in stdin.lock().lines() { | ||
| let Ok(line) = line else { break }; | ||
| let decision = match line.trim() { | ||
| "confirm" => DECISION_CONFIRM, | ||
| "cancel" => DECISION_CANCEL, | ||
| _ => continue, | ||
| }; | ||
| // First decision wins; a later line cannot flip a settled ceremony. | ||
| let _ = DECISION.compare_exchange( | ||
| DECISION_PENDING, | ||
| decision, | ||
| Ordering::SeqCst, | ||
| Ordering::SeqCst, | ||
| ); | ||
| return; | ||
| } | ||
| let _ = DECISION.compare_exchange( | ||
| DECISION_PENDING, | ||
| DECISION_CANCEL, | ||
| Ordering::SeqCst, | ||
| Ordering::SeqCst, | ||
| ); | ||
| }); | ||
| } | ||
|
|
||
| #[cfg(windows)] | ||
| fn t_ms() -> u64 { | ||
| START | ||
|
|
@@ -121,6 +214,12 @@ fn main() { | |
| // again. Runtime-selectable so both halves can be tried from one build. | ||
| let keep_link = args.iter().any(|a| a == "--keep-link"); | ||
|
|
||
| // Only `pair` has a ceremony to gate; the read-only commands never block on | ||
| // a decision, and starting the reader for them would just hold their stdin. | ||
| if command == "pair" { | ||
| spawn_decision_reader(); | ||
| } | ||
|
|
||
| let result = pollster::block_on(async { | ||
| match command { | ||
| "pair" => win::run_pair(address, keep_link).await, | ||
|
|
@@ -482,29 +581,26 @@ mod win { | |
| let kind = args.PairingKind()?; | ||
| diag(&format!("PairingRequested kind={kind:?}")); | ||
| match kind { | ||
| // Numeric comparison: surface the pin so the user can check | ||
| // it against the device screen, then accept. | ||
| // Numeric comparison: surface the pin, then hold the | ||
| // ceremony until the parent says the user matched it. | ||
| DevicePairingKinds::ConfirmPinMatch => { | ||
| let pin = args.Pin()?; | ||
| super::emit(&format!( | ||
| r#"{{"type":"pairing","pin":"{}"}}"#, | ||
| super::json_escape(&pin.to_string()) | ||
| )); | ||
| args.Accept()?; | ||
| ACCEPT_MS.store(t_ms(), Ordering::SeqCst); | ||
| diag("accepted ConfirmPinMatch (device confirmation pending)"); | ||
| super::await_decision(&args, "ConfirmPinMatch")?; | ||
| } | ||
| // Device shows a pin; Windows just needs a yes. Surface it too. | ||
| // Device shows a pin; Windows just needs a yes. Same gate: | ||
| // the user is still confirming a code they can read. | ||
| DevicePairingKinds::DisplayPin => { | ||
| if let Ok(pin) = args.Pin() { | ||
| super::emit(&format!( | ||
| r#"{{"type":"pairing","pin":"{}"}}"#, | ||
| super::json_escape(&pin.to_string()) | ||
| )); | ||
| } | ||
| args.Accept()?; | ||
| ACCEPT_MS.store(t_ms(), Ordering::SeqCst); | ||
| diag("accepted DisplayPin"); | ||
| super::await_decision(&args, "DisplayPin")?; | ||
| } | ||
| // Just-works: no code, so nothing ties the bond to the device | ||
| // in front of the user. A Safe 7 has a screen and always | ||
|
|
||
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.
Cancelling BLE pairing shows a confusing hardware failure instead of a cancellation
Reported at
apps/desktop/app/process/BlePair.ts:156-158(outside the diff of this PR, so this is a file-level comment)Severity:
non-severeThe user-cancelled reason recorded for the pairing attempt is immediately replaced by the helper's own generic failure text (
lastError = event.messageatapps/desktop/app/process/BlePair.ts:157) before the attempt finishes, so a deliberate cancel is reported as an unexplained pairing failure.Impact: A user who presses Cancel on the pairing-code dialog (or cancels from the app) sees a scary low-level Bluetooth failure message rather than a clean "cancelled" result.
How the cancel reason is overwritten before the promise rejects
When the decision callback receives
cancelit setslastError = PAIR_CANCELLED_REASON(apps/desktop/app/process/BlePair.ts:109-111), whose whole purpose is to match the SDK's/connect cancelled/itest so it maps toBlePairingCancelled(see comment atapps/desktop/app/process/BlePair.ts:23-24).The helper then declines the ceremony,
PairAsyncreturns a non-Pairedstatus, andrun_pairreturnsErr(format!("pairing failed with status {status:?}"))(apps/desktop/native-modules/onekey-ble-pair/src/main.rs:787).mainturns that into{"type":"error","message":"pairing failed with status DevicePairingResultStatus(N)"}on stdout, which the stdout parser assigns tolastError, clobbering the cancel reason. Theexithandler (apps/desktop/app/process/BlePair.ts:175-190) then rejects with the clobbered message.A
cancelRequestedflag that makes the cancel reason win (or that skips overwritinglastErroronce a cancel has been sent) restores the intended mapping.