BLE: tell user to forget existing Omi when pairing is broken#7132
BLE: tell user to forget existing Omi when pairing is broken#7132
Conversation
…ost, skip retry When iOS surfaces CBError.peerRemovedPairingInformation (code 14), the OS-level bond between phone and peripheral is broken. Auto-retrying the connect loops forever with the same error until the user forgets the device in iOS Settings, flooding logs and preventing the app from telling the user what to do. Map this case to a stable 'pairing_lost' token in bleReasonString and pass that token (instead of Apple's localized description) up to Flutter via onPeripheralDisconnected, so the Dart layer can detect the condition without matching English-only strings. Skip the auto-reconnect attempt for both didFailToConnect and didDisconnectPeripheral when the error is pairing_lost.
…skip retry Mirrors the iOS handling: GATT statuses 137 (GATT_AUTH_FAIL) and 15 (GATT_INSUF_ENCRYPTION) both indicate the OS-level bond is out of sync with the peripheral's pairing record. Auto-retry loops with the same status until the user forgets the device in system Bluetooth settings. Map these statuses to a 'pairing_lost' token in handleDisconnection, surface that token through onPeripheralDisconnected so the Dart layer can detect it without parsing platform-specific strings, and short-circuit handleRetryLogic to stop the retry loop until the user manually forgets the device.
BleBridge gains a top-level pairingLostCallback fired from onPeripheralDisconnected whenever the error string carries the 'pairing_lost' token from native (iOS peerRemovedPairingInformation / Android GATT_AUTH_FAIL / GATT_INSUF_ENCRYPTION). DeviceProvider registers the callback in its constructor and shows a non-dismissible ConfirmationDialog via the global navigator key, with text explaining how to recover. A guard flag keeps the dialog from stacking when multiple disconnect events fire during the failed-retry storm. Adds the new l10n keys (pairingLostTitle / pairingLostBody / pairingLostButton) to the call site; the .arb additions land in a follow-up commit.
|
@morpheus review — Approved ✅ Well-structured four-layer fix. Reviewed all semantic diffs (native iOS/Android, Flutter bridge/provider, l10n keys + translations). Spot-checked ja/ar/ko translations — clean. What I checked:
One non-blocking note: The confirm button reads "Open Bluetooth Settings" ( |
Adds three keys (pairingLostTitle / pairingLostBody / pairingLostButton) to app_en.arb and machine-translates them into all 48 other shipped locales via Google Translate. Regenerates the AppLocalizations classes via flutter gen-l10n. Copy is plain-language and instruction-only — no jargon, no platform-specific UI symbols (no iOS '(i)' chevron) — so the same dialog reads naturally on both iOS and Android.
mdmohsin7
force-pushed
the
fix-pairing-lost-dialog
branch
from
036bb5a to
2f15a91
Compare
|
@morpheus re-review after force-push — Approval stands ✅ Verified the updated diff (783 insertions / 55 deletions, down from 261k/260k):
Previous review findings all still apply. Non-blocking note about the "Open Bluetooth Settings" button text still stands. Process note: Force-push rewrites branch history. Team non-negotiable is "fix forward, never rewrite." Noting for the record — ops approved, and the branch hadn't been merged, so no downstream impact. |
The pairing-lost dialog's confirm button reads 'Open Bluetooth Settings' but the original implementation just popped the dialog — leaving the user to hunt for the right settings page on their own. This wires the button to actually open the system Bluetooth settings page. Adds a new openBluetoothSettings() method to the Pigeon BleHostApi: - iOS: tries the App-Prefs:root=Bluetooth deep-link first, then falls back to UIApplication.openSettingsURLString. Apple has restricted the Bluetooth-specific URL on newer iOS versions; the fallback at least gets the user into Settings rather than nowhere. - Android: fires Settings.ACTION_BLUETOOTH_SETTINGS, with a fallback to Settings.ACTION_SETTINGS if the Bluetooth-specific intent isn't resolvable on the target device. The dialog's onConfirm now invokes BleHostApi().openBluetoothSettings() fire-and-forget after popping the dialog, with a try/catch that logs but does not surface failures (the user is already on their way to Settings; better to let them recover than to flash an error). Also re-formats the pre-existing battery-throttle ternary in device_provider to the project's --line-length 120 style; the line was below the threshold in main but now triggers the CI lint check because this PR modifies the file.
|
@greptile-apps review |
Greptile SummaryThis PR adds a recovery path for broken BLE bond state: native layers (iOS
Confidence Score: 4/5Safe to merge on iOS; the Android path carries a P1 risk of showing a misleading recovery dialog to first-time users if GATT status 15 fires during initial bond negotiation. One P1 finding (GATT 15 false-positive on Android initial pairing) sets the ceiling at 4. iOS changes and the Flutter/Dart layer look correct. The P2 suggestion on contains() vs == does not affect the score. app/android/app/src/main/kotlin/com/friend/ios/OmiBleForegroundService.kt — GATT status 15 handling needs re-evaluation. Important Files Changed
Sequence DiagramsequenceDiagram
participant Peripheral as Omi Device
participant Native as Native BLE Layer(iOS / Android)
participant Bridge as BleBridge (Flutter)
participant Provider as DeviceProvider
participant Dialog as ConfirmationDialog
participant Settings as System BT Settings
Peripheral->>Native: Disconnect (broken bond)
Note over Native: iOS: CBError.peerRemovedPairingInformation<br/>Android: GATT 137 (or 15)
Native->>Native: Map to pairing_lost token
Native->>Native: Skip auto-retry
Native->>Bridge: onPeripheralDisconnected(uuid, pairing_lost)
Bridge->>Bridge: isPairingLostError() true
Bridge->>Provider: pairingLostCallback(uuid)
Provider->>Provider: _pairingLostDialogShowing = true
Provider->>Dialog: showDialog (barrierDismissible: false)
Dialog-->>Provider: User taps Open Bluetooth Settings
Provider->>Provider: _pairingLostDialogShowing = false
Provider->>Settings: BleHostApi.openBluetoothSettings()
Note over Settings: User forgets Omi, re-pairs from scratch
Reviews (2): Last reviewed commit: "Address greptile P1+P2: await openBlueto..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| try { | ||
| BleHostApi().openBluetoothSettings(); | ||
| } catch (e) { | ||
| Logger.debug('openBluetoothSettings failed: $e'); |
There was a problem hiding this comment.
await on async openBluetoothSettings()
BleHostApi().openBluetoothSettings() returns Future<void>, but it is called without await. The surrounding try/catch only intercepts synchronous exceptions — any PlatformException thrown by the platform channel will escape as an unhandled future error, silently failing without the intended Logger.debug fallback.
| DispatchQueue.main.async { | ||
| if let url = bluetoothUrl, UIApplication.shared.canOpenURL(url) { | ||
| UIApplication.shared.open(url, options: [:], completionHandler: nil) | ||
| } else if let url = appSettingsUrl { | ||
| UIApplication.shared.open(url, options: [:], completionHandler: nil) | ||
| } | ||
| } |
There was a problem hiding this comment.
canOpenURL for App-Prefs:root=Bluetooth always returns false without LSApplicationQueriesSchemes entry
On iOS 9+, canOpenURL requires the queried URL scheme to be listed in LSApplicationQueriesSchemes inside Info.plist. App-Prefs: is not a registered first-party scheme, so canOpenURL always returns false, the branch is unreachable, and the fallback to UIApplication.openSettingsURLString (the app's own settings page, not Bluetooth settings) always fires. Use openURL directly and check success in the completion handler instead.
| DispatchQueue.main.async { | |
| if let url = bluetoothUrl, UIApplication.shared.canOpenURL(url) { | |
| UIApplication.shared.open(url, options: [:], completionHandler: nil) | |
| } else if let url = appSettingsUrl { | |
| UIApplication.shared.open(url, options: [:], completionHandler: nil) | |
| } | |
| } | |
| DispatchQueue.main.async { | |
| if let url = bluetoothUrl { | |
| UIApplication.shared.open(url, options: [:]) { success in | |
| if !success, let settingsUrl = appSettingsUrl { | |
| UIApplication.shared.open(settingsUrl, options: [:], completionHandler: nil) | |
| } | |
| } | |
| } else if let url = appSettingsUrl { | |
| UIApplication.shared.open(url, options: [:], completionHandler: nil) | |
| } | |
| } |
…drop canOpenURL Three issues from greptile review on PR #7132: - (P1) device_provider: BleHostApi().openBluetoothSettings() returns Future<void>; calling it without await left PlatformExceptions from the Pigeon channel as unhandled future errors. Make onConfirm async and await inside the existing try/catch so the failure is caught and logged. - (P1) device_provider: pairingLostCallback was set on the BleBridge singleton in the constructor but never cleared in dispose(), so a torn-down provider would keep receiving pairing_lost events on a dead instance. Null the callback in dispose(). - (P2) iOS BleHostApiImpl: canOpenURL on App-Prefs:root=Bluetooth always returns false on iOS 9+ without an LSApplicationQueriesSchemes entry, making the Bluetooth-specific branch dead code. Replace canOpenURL with the open() completion handler's success flag, falling back to the app settings URL only when the Bluetooth deep-link actually fails to launch.
|
@greptile-apps re-review |
| // Android GATT statuses that indicate the OS-level bond is broken / out of sync | ||
| // with the peripheral's pairing record. Auto-retry will loop forever until the | ||
| // user forgets the device in system Bluetooth settings. | ||
| // 137 = GATT_AUTH_FAIL (most common when the peer wiped its bonding info) | ||
| // 15 = GATT_INSUF_ENCRYPTION | ||
| val pairingLost = status == 137 || status == 15 | ||
|
|
||
| val error = when { |
There was a problem hiding this comment.
GATT_INSUF_ENCRYPTION (15) is returned by Android when a peripheral requires an encrypted link but bonding hasn't been established yet — which is exactly what happens the first time a new user tries to connect to an Omi that mandates bonding. In that flow Android typically disconnects with status 15 before presenting the system pairing dialog; the re-bond occurs on the next automatic reconnect attempt. Mapping status 15 to pairing_lost here will: (1) display "Can't connect to your Omi — forget the device" to a user who has never paired before, and (2) skip the retry that would let Android complete the bond negotiation. GATT_AUTH_FAIL (137) is the reliable broken-bond signal; the status 15 guard should either be removed or restricted to cases where a prior bond record is already known to exist.
1 participant
Problem
When iOS or Android still has a Bluetooth bond record for an Omi but the peripheral has wiped its bonding state (or vice versa), the BLE stack surfaces a specific error every time the app tries to connect:
CBError.peerRemovedPairingInformation(code 14) →[OmiBle] didFailToConnect: Omi, error=Peer removed pairing informationGATT_AUTH_FAIL) or 15 (GATT_INSUF_ENCRYPTION)Today the native layer just logs it, sends Apple's localized string up to Flutter, and immediately retries — which fails again with the same error in a tight loop. The Flutter side drops the error string in
NativeBleTransport._handleConnectionState, the onboarding catch block silently removes the device from the scan list, and the user is left with no idea why their Omi keeps vanishing.There's no recovery path because there's no way for the user to know that their phone needs to forget the device and re-pair from system Bluetooth settings.
Fix
Four-layer change keeping the platform-specific bits in the native side and the user-facing copy in shared Flutter code:
iOS (
app/ios/Runner/OmiBleManager.swift)bleReasonStringadds a case for.peerRemovedPairingInformationreturning the stable token"pairing_lost".didFailToConnectanddidDisconnectPeripheralsend"pairing_lost"to Flutter viaonPeripheralDisconnectedinstead of Apple's localized description (so Flutter doesn't have to match English-only strings).Android (
app/android/app/src/main/kotlin/com/friend/ios/OmiBleForegroundService.kt)handleDisconnectionmaps GATT status 137 and 15 to"pairing_lost"and forwards via the sameonPeripheralDisconnectedchannel.handleRetryLogicshort-circuits on those statuses to stop the retry loop.Flutter routing (
app/lib/services/bridges/ble_bridge.dart,app/lib/providers/device_provider.dart)BleBridgegains a top-levelpairingLostCallback, fired fromonPeripheralDisconnectedwhenever the error string carries thepairing_losttoken.DeviceProviderregisters the callback in its constructor and shows a non-dismissibleConfirmationDialogvia the global navigator key. A guard flag prevents the dialog from stacking when multiple disconnect events fire during the failed-retry storm.l10n (
app/lib/l10n/*)pairingLostTitle,pairingLostBody,pairingLostButton) added toapp_en.arb:AppLocalizationsclasses regenerated viaflutter gen-l10n.(i)chevron) — same dialog reads naturally on both iOS and Android.The button is dismiss-only — the user manually navigates to system Bluetooth settings. No flaky deep-link via
App-Prefs:Bluetooth/ AndroidSettings.ACTION_BLUETOOTH_SETTINGS(those are inconsistent across OS versions and would add scope without solving the core issue).Test plan
_pairingLostDialogShowingresets (re-trigger to verify).onPeripheralDisconnectedevents fire in quick succession.Note on diff size
The l10n commit shows a large insert/delete count because the pre-commit hook normalized all 49 touched
.arbfiles from 2-space to 4-space JSON indent. The actual semantic delta is the 3 new keys per locale plus the 50 regeneratedAppLocalizationsgetters; the rest is whitespace.