From e52224ac9678aba0eb6f24cbe83c0deec2cb1c56 Mon Sep 17 00:00:00 2001 From: David Zhang Date: Wed, 14 Oct 2026 00:58:03 +0700 Subject: [PATCH 1/4] fix(app): keep a live native BLE link from showing as Disconnected A launch connect that times out before the pendant arrives used to dispose its transport, which unregistered the peripheral from BleBridge while native kept connecting. The late device-ready then reached nobody and the app showed Disconnected over a live link until it was killed. #21302 keeps the transport on timeout; this closes the footguns that made the path possible and still leave siblings open: - BleBridge.registerPeripheral returns an owner-scoped disposer, so a retired transport cannot unregister its replacement. - NativeBleTransport.disconnect releases native based on its own intent, not the mirrored state, and ignores a ready in flight after release. - DeviceService.disconnectDevice disposes the transport it drops. - NativeLinkReconciler re-adopts a link native already holds when the app resumes; native replays device-ready without touching the radio. - Transports ignore state updates after dispose. Co-Authored-By: Claude Opus 5.5 --- .../20261014-pendant-reconnect-on-open.json | 3 + app/lib/main.dart | 5 +- app/lib/providers/device_provider.dart | 13 + app/lib/providers/native_link_reconciler.dart | 72 ++++ app/lib/services/bridges/ble_bridge.dart | 22 +- app/lib/services/devices.dart | 13 +- .../transports/native_ble_transport.dart | 22 +- .../transports/rayban_meta_transport.dart | 2 +- .../devices/transports/watch_transport.dart | 2 +- ...ce_provider_disconnect_telemetry_test.dart | 2 +- .../device_provider_late_ready_test.dart | 369 ++++++++++++++++++ ...pendant_launch_timeout_reconnect_test.dart | 2 +- .../unit/device_service_connections_test.dart | 2 +- 13 files changed, 511 insertions(+), 18 deletions(-) create mode 100644 app/changelog/unreleased/20261014-pendant-reconnect-on-open.json create mode 100644 app/lib/providers/native_link_reconciler.dart create mode 100644 app/test/providers/device_provider_late_ready_test.dart diff --git a/app/changelog/unreleased/20261014-pendant-reconnect-on-open.json b/app/changelog/unreleased/20261014-pendant-reconnect-on-open.json new file mode 100644 index 00000000000..13e2c776070 --- /dev/null +++ b/app/changelog/unreleased/20261014-pendant-reconnect-on-open.json @@ -0,0 +1,3 @@ +{ + "change": "Your Omi shows as connected again when you reopen the app, instead of staying stuck on Disconnected while it's actually connected" +} diff --git a/app/lib/main.dart b/app/lib/main.dart index 595633686c5..8af8ff298ed 100644 --- a/app/lib/main.dart +++ b/app/lib/main.dart @@ -675,9 +675,10 @@ class _MyAppState extends State with WidgetsBindingObserver { (previous?..setConversationProvider(conversation)) ?? LocalRecordingsProvider(), ), ChangeNotifierProxyProvider2( - create: (context) => DeviceProvider(), + create: (context) => DeviceProvider()..nativeLink.bindAppLifecycle(), update: (BuildContext context, captureProvider, localRecordings, DeviceProvider? previous) => - (previous?..setProviders(captureProvider, localRecordings)) ?? DeviceProvider(), + (previous?..setProviders(captureProvider, localRecordings)) ?? + (DeviceProvider()..nativeLink.bindAppLifecycle()), ), ChangeNotifierProxyProvider( create: (context) => OnboardingProvider(), diff --git a/app/lib/providers/device_provider.dart b/app/lib/providers/device_provider.dart index 0091455a7cf..6e22b66937d 100644 --- a/app/lib/providers/device_provider.dart +++ b/app/lib/providers/device_provider.dart @@ -18,6 +18,7 @@ import 'package:omi/pages/home/firmware_update.dart'; import 'package:omi/pages/home/omiglass_ota_update.dart'; import 'package:omi/providers/capture_provider.dart'; import 'package:omi/providers/local_recordings_provider.dart'; +import 'package:omi/providers/native_link_reconciler.dart'; import 'package:omi/services/capture/capture_wedge_monitor.dart'; import 'package:omi/services/devices.dart'; import 'package:omi/services/devices/connectors/device_connection.dart'; @@ -144,6 +145,8 @@ class DeviceProvider extends ChangeNotifier implements IDeviceServiceSubsciption /// coordinator wake, and capture streaming must use this token, not /// `_sessionGeneration` at callback time. -1 means no admitted connect. int _admittedConnectGeneration = -1; + late final NativeLinkReconciler nativeLink = NativeLinkReconciler(this); + (DeviceConnection, int)? _admittedLink; DeviceProvider({ BleDiagnosticsLoader? bleDiagnosticsLoader, @@ -203,6 +206,7 @@ class DeviceProvider extends ChangeNotifier implements IDeviceServiceSubsciption /// account publication from the retired generation must not land. void clearUserData() { _sessionGeneration++; + _admittedLink = null; _deviceHealthTimer?.cancel(); _deviceHealthTimer = null; _admittedConnectGeneration = -1; @@ -795,6 +799,8 @@ class DeviceProvider extends ChangeNotifier implements IDeviceServiceSubsciption void setIsConnected(bool value) { isConnected = value; + // Every path that drops the mirror must allow the live link to be re-admitted. + if (!value) _admittedLink = null; if (isConnected) { _discoveryTimer?.cancel(); } @@ -805,6 +811,7 @@ class DeviceProvider extends ChangeNotifier implements IDeviceServiceSubsciption void dispose() { _isDisposed = true; _sessionGeneration++; + nativeLink.dispose(); _deviceHealthTimer?.cancel(); _deviceHealthTimer = null; _admittedConnectGeneration = -1; @@ -1178,6 +1185,12 @@ class DeviceProvider extends ChangeNotifier implements IDeviceServiceSubsciption if (connection == null) { return; } + // One admission per physical connect: overlapping connected events (a + // native connect still initializing, a resume reconcile) must not run + // connect side effects twice. + final link = (connection, connection.connectionEpoch); + if (_admittedLink == link) return; + _admittedLink = link; _onDeviceConnected(connection.device, generation); } diff --git a/app/lib/providers/native_link_reconciler.dart b/app/lib/providers/native_link_reconciler.dart new file mode 100644 index 00000000000..6cc1876e89e --- /dev/null +++ b/app/lib/providers/native_link_reconciler.dart @@ -0,0 +1,72 @@ +import 'dart:async'; + +import 'package:flutter/widgets.dart'; + +import 'package:omi/backend/preferences.dart'; +import 'package:omi/gen/pigeon_communicator.g.dart'; +import 'package:omi/providers/device_provider.dart'; +import 'package:omi/services/devices.dart'; +import 'package:omi/services/services.dart'; +import 'package:omi/utils/logger.dart'; + +/// Native owns the BLE link and Dart mirrors it. Any lost device-ready (a +/// launch connect that timed out, a retired transport, a dropped callback) +/// otherwise leaves the UI disconnected over a live link until the app is +/// killed. On foreground, re-adopt a link native already holds: native answers +/// manageDevice for a connected peripheral by replaying device-ready, so this +/// never touches the radio and never starts a physical connect. +class NativeLinkReconciler { + NativeLinkReconciler(this._provider, {Future Function(String deviceId)? nativeLinkProbe}) + : _nativeLinkProbe = nativeLinkProbe ?? BleHostApi().isPeripheralConnected; + + final DeviceProvider _provider; + final Future Function(String deviceId) _nativeLinkProbe; + AppLifecycleListener? _lifecycle; + bool _disposed = false; + + void bindAppLifecycle() { + if (_disposed) return; + _lifecycle ??= AppLifecycleListener(onResume: () => unawaited(reconcile())); + } + + /// The paired device id while the mirror shows it disconnected, else null. + String? _disconnectedPairedDeviceId() { + if (_disposed || _provider.isConnected || _provider.isConnecting || _provider.isFirmwareUpdateInProgress) { + return null; + } + final id = SharedPreferencesUtil().btDevice.id; + return id.isEmpty ? null : id; + } + + Future reconcile() async { + final deviceId = _disconnectedPairedDeviceId(); + if (deviceId == null) return; + final bool linkUp; + try { + linkUp = await _nativeLinkProbe(deviceId); + } catch (_) { + return; + } + // Adopt only the device whose link was probed: pairing or the mirror may + // have changed while the probe was in flight. + if (!linkUp || _disconnectedPairedDeviceId() != deviceId) return; + Logger.debug('NativeLinkReconciler: native holds $deviceId while disconnected; re-adopting'); + final deviceService = ServiceManager.instance().device; + if (deviceService.connectionFor(deviceId)?.status == DeviceConnectionState.connected) { + // DeviceService holds the link but the provider never heard. Republish + // synchronously with the checks above: nothing can intervene, and no + // transport event will publish it. + _provider.onDeviceConnectionStateChanged(deviceId, DeviceConnectionState.connected); + return; + } + // Otherwise native replays device-ready into a fresh transport, whose own + // connected event publishes through the same path as any native connect. + await deviceService.ensureConnection(deviceId, force: true); + } + + void dispose() { + _disposed = true; + _lifecycle?.dispose(); + _lifecycle = null; + } +} diff --git a/app/lib/services/bridges/ble_bridge.dart b/app/lib/services/bridges/ble_bridge.dart index 1a2449e84b3..8ef094c4d33 100644 --- a/app/lib/services/bridges/ble_bridge.dart +++ b/app/lib/services/bridges/ble_bridge.dart @@ -32,6 +32,7 @@ class BleBridge implements BleFlutterApi { final Map _characteristicCallbacks = {}; final Map _disconnectCallbacks = {}; final Map _deviceReadyCallbacks = {}; + final Map _peripheralOwners = {}; final Map _rssiCallbacks = {}; final Map _ingressHealth = {}; final Set _nativeIngressOwners = {}; @@ -114,16 +115,28 @@ class BleBridge implements BleFlutterApi { return () => _scanBlockedListeners.remove(listener); } - void registerPeripheral({ + /// Routes this peripheral's native events to the caller, replacing any + /// previous owner. Returns a disposer that unregisters only while the caller + /// still owns the peripheral: a retired transport disposed after its + /// replacement registered must not silence the replacement's device-ready. + VoidCallback registerPeripheral({ required String peripheralUuid, CharacteristicValueCallback? onCharacteristicValue, ConnectionStateCallback? onConnectionState, DeviceReadyCallback? onDeviceReady, }) { final key = peripheralUuid.toUpperCase(); + final owner = Object(); + _peripheralOwners[key] = owner; + _characteristicCallbacks.remove(key); + _disconnectCallbacks.remove(key); + _deviceReadyCallbacks.remove(key); if (onCharacteristicValue != null) _characteristicCallbacks[key] = onCharacteristicValue; if (onConnectionState != null) _disconnectCallbacks[key] = onConnectionState; if (onDeviceReady != null) _deviceReadyCallbacks[key] = onDeviceReady; + return () { + if (identical(_peripheralOwners[key], owner)) _unregisterPeripheral(key); + }; } void registerRssiCallback(String peripheralUuid, RssiUpdateCallback callback) { @@ -134,8 +147,11 @@ class BleBridge implements BleFlutterApi { _rssiCallbacks.remove(peripheralUuid.toUpperCase()); } - void unregisterPeripheral(String peripheralUuid) { - final key = peripheralUuid.toUpperCase(); + @visibleForTesting + void clearPeripheralForTesting(String peripheralUuid) => _unregisterPeripheral(peripheralUuid.toUpperCase()); + + void _unregisterPeripheral(String key) { + _peripheralOwners.remove(key); _connectionRevisions[key] = (_connectionRevisions[key] ?? 0) + 1; _characteristicCallbacks.remove(key); _disconnectCallbacks.remove(key); diff --git a/app/lib/services/devices.dart b/app/lib/services/devices.dart index 8e2f310430c..aaa301649d9 100644 --- a/app/lib/services/devices.dart +++ b/app/lib/services/devices.dart @@ -369,12 +369,17 @@ class DeviceService { return null; } + /// A connection leaving [_connections] is always disposed: an orphaned + /// transport stays registered for native events and would swallow the next + /// device-ready, leaving the UI disconnected over a live link. Future disconnectDevice(String deviceId) async { - final connection = _connections[deviceId]; - if (connection != null) { - Logger.debug("DeviceService: Disconnecting device $deviceId..."); + final connection = _connections.remove(deviceId); + if (connection == null) return; + Logger.debug("DeviceService: Disconnecting device $deviceId..."); + try { await connection.disconnect(); - _connections.remove(deviceId); + } finally { + await connection.transport.dispose(); } } diff --git a/app/lib/services/devices/transports/native_ble_transport.dart b/app/lib/services/devices/transports/native_ble_transport.dart index 69bc5463fe1..81e4e33591c 100644 --- a/app/lib/services/devices/transports/native_ble_transport.dart +++ b/app/lib/services/devices/transports/native_ble_transport.dart @@ -37,6 +37,12 @@ class NativeBleTransport extends DeviceTransport implements CaptureSubscriptionE Completer>? _deviceReadyCompleter; DeviceTransportState _state = DeviceTransportState.disconnected; + late final VoidCallback _releaseBridge; + + /// Set when [disconnect] hands the peripheral back to native, cleared by + /// [connect]. Native keeps connecting and reconnecting after a Dart-side + /// timeout or drop, so release cannot be inferred from the mirrored [_state]. + bool _nativeReleased = false; Timer? _audioLivenessTimer; int _audioSilenceResubscribes = 0; int _subscriptionGeneration = 0; @@ -63,7 +69,7 @@ class NativeBleTransport extends DeviceTransport implements CaptureSubscriptionE NativeBleTransport(this._peripheralUuid, {this.requiresBond = false, BleHostApi? hostApi}) : _hostApi = hostApi ?? BleHostApi() { BleBridge.instance.addIngressListener(_ingressOwnershipChanged); - BleBridge.instance.registerPeripheral( + _releaseBridge = BleBridge.instance.registerPeripheral( peripheralUuid: _peripheralUuid, onConnectionState: _handleConnectionState, onDeviceReady: _handleDeviceReady, @@ -96,6 +102,7 @@ class NativeBleTransport extends DeviceTransport implements CaptureSubscriptionE deviceReady.ignore(); try { + _nativeReleased = false; await _hostApi.manageDevice(_peripheralUuid, requiresBond); } catch (e) { Logger.debug('[NativeBleTransport] manageDevice failed: $e'); @@ -122,7 +129,7 @@ class NativeBleTransport extends DeviceTransport implements CaptureSubscriptionE @override Future disconnect() async { _subscriptionGeneration++; - if (_state == DeviceTransportState.disconnected && _streamControllers.isEmpty) return; + if (_nativeReleased && _state == DeviceTransportState.disconnected && _streamControllers.isEmpty) return; // A liveness watch pending from a prior connection must not fire into a // subsequent one and force a spurious CCCD re-subscribe. @@ -147,8 +154,12 @@ class NativeBleTransport extends DeviceTransport implements CaptureSubscriptionE _services = []; try { + _nativeReleased = true; await _hostApi.unmanageDevice(_peripheralUuid); } catch (e) { + // Native may still own the link; keep release retryable and its readies + // adoptable rather than mirroring "released" over a live link. + _nativeReleased = false; Logger.debug('[NativeBleTransport] unmanageDevice failed: $e'); } @@ -288,7 +299,7 @@ class NativeBleTransport extends DeviceTransport implements CaptureSubscriptionE _subscriptionGeneration++; _audioLivenessTimer?.cancel(); BleBridge.instance.removeIngressListener(_ingressOwnershipChanged); - BleBridge.instance.unregisterPeripheral(_peripheralUuid); + _releaseBridge(); await _audioSubscriptionErrors.close(); _activeSubscriptionKeys.clear(); _subscribedSubscriptionKeys.clear(); @@ -299,7 +310,7 @@ class NativeBleTransport extends DeviceTransport implements CaptureSubscriptionE // MARK: - Private Helpers void _updateState(DeviceTransportState newState) { - if (_state != newState) { + if (_state != newState && !_connectionStateController.isClosed) { _state = newState; _connectionStateController.add(_state); } @@ -353,6 +364,9 @@ class NativeBleTransport extends DeviceTransport implements CaptureSubscriptionE } void _handleDeviceReady(List services) { + // A ready already in flight when this transport released native must not + // re-adopt the link behind the explicit disconnect. + if (_nativeReleased) return; _subscriptionGeneration++; _subscribedSubscriptionKeys.clear(); subscriptionFailures.clear(); diff --git a/app/lib/services/devices/transports/rayban_meta_transport.dart b/app/lib/services/devices/transports/rayban_meta_transport.dart index d1b3485bc46..5c8bfabaaae 100644 --- a/app/lib/services/devices/transports/rayban_meta_transport.dart +++ b/app/lib/services/devices/transports/rayban_meta_transport.dart @@ -55,7 +55,7 @@ class RayBanMetaTransport extends DeviceTransport { } void _updateState(DeviceTransportState newState) { - if (_state != newState) { + if (_state != newState && !_connectionStateController.isClosed) { _state = newState; _connectionStateController.add(_state); } diff --git a/app/lib/services/devices/transports/watch_transport.dart b/app/lib/services/devices/transports/watch_transport.dart index f6ea932a3ab..a081ecfec65 100644 --- a/app/lib/services/devices/transports/watch_transport.dart +++ b/app/lib/services/devices/transports/watch_transport.dart @@ -31,7 +31,7 @@ class WatchTransport extends DeviceTransport { Stream get connectionStateStream => _connectionStateController.stream; void _updateState(DeviceTransportState newState) { - if (_state != newState) { + if (_state != newState && !_connectionStateController.isClosed) { _state = newState; _connectionStateController.add(_state); } diff --git a/app/test/providers/device_provider_disconnect_telemetry_test.dart b/app/test/providers/device_provider_disconnect_telemetry_test.dart index ffacc56424f..0a220adfe01 100644 --- a/app/test/providers/device_provider_disconnect_telemetry_test.dart +++ b/app/test/providers/device_provider_disconnect_telemetry_test.dart @@ -345,7 +345,7 @@ void main() { final provider = DeviceProvider(bleDiagnosticsLoader: (_) async => throw StateError('no diagnostics')); addTearDown(provider.dispose); addTearDown(capture.dispose); - addTearDown(() => BleBridge.instance.unregisterPeripheral(id)); + addTearDown(() => BleBridge.instance.clearPeripheralForTesting(id)); provider.captureProvider = capture; provider.pairedDevice = _device(id); BleBridge.instance.onPeripheralDisconnected(id, recovery ? 'capture_recovery' : null); diff --git a/app/test/providers/device_provider_late_ready_test.dart b/app/test/providers/device_provider_late_ready_test.dart new file mode 100644 index 00000000000..e59d0c13a55 --- /dev/null +++ b/app/test/providers/device_provider_late_ready_test.dart @@ -0,0 +1,369 @@ +import 'dart:async'; + +import 'package:connectivity_plus_platform_interface/connectivity_plus_platform_interface.dart'; +import 'package:fake_async/fake_async.dart'; +import 'package:flutter/services.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:shared_preferences/shared_preferences.dart'; + +import 'package:omi/backend/preferences.dart'; +import 'package:omi/backend/schema/bt_device/bt_device.dart'; +import 'package:omi/gen/pigeon_communicator.g.dart'; +import 'package:omi/providers/device_provider.dart'; +import 'package:omi/providers/native_link_reconciler.dart'; +import 'package:omi/services/bridges/ble_bridge.dart'; +import 'package:omi/services/devices.dart'; +import 'package:omi/services/devices/discovery/device_locator.dart'; +import 'package:omi/services/devices/transports/device_transport.dart'; +import 'package:omi/services/services.dart'; +import 'package:omi/utils/logger.dart'; + +// A native link that is up while Dart shows "Disconnected" until the app is +// killed: the launch connect gave up at its 60s deadline, the pendant arrived +// later, and nothing in Dart adopted the late device-ready. + +const _deviceId = 'B56E190F-1298-F479-A89D-21C3B506DEA5'; +final _services = [ + BleService( + uuid: '19b10000-e8f2-537e-4f6c-d104768a1214', characteristicUuids: ['19b10001-e8f2-537e-4f6c-d104768a1214']), +]; + +class _TestConnectivityPlatform extends ConnectivityPlatform { + @override + Future> checkConnectivity() async => [ConnectivityResult.none]; + + @override + Stream> get onConnectivityChanged => const Stream.empty(); +} + +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + final channels = {}; + var linkUp = false; + var nativeReplaysReady = false; + var manageCalls = 0; + var unmanageCalls = 0; + var unmanageFails = false; + + void handle(String method, Object? Function(List args) reply) { + final name = 'dev.flutter.pigeon.omi_pigeon.BleHostApi.$method'; + channels.add(name); + TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger.setMockMessageHandler(name, (message) async { + final args = (BleHostApi.pigeonChannelCodec.decodeMessage(message) as List?) ?? const []; + return BleHostApi.pigeonChannelCodec.encodeMessage([reply(args)]); + }); + } + + void pairPendant() => SharedPreferencesUtil().btDeviceSet(BtDevice( + id: _deviceId, + name: 'Omi', + type: DeviceType.omi, + rssi: -60, + locator: DeviceLocator.bluetooth(deviceId: _deviceId), + )); + + setUpAll(() async { + SharedPreferences.setMockInitialValues({}); + await SharedPreferencesUtil.init(); + ConnectivityPlatform.instance = _TestConnectivityPlatform(); + try { + await ServiceManager.init(); + } catch (_) {} + }); + + setUp(() { + linkUp = false; + nativeReplaysReady = false; + unmanageFails = false; + manageCalls = 0; + unmanageCalls = 0; + handle('getBluetoothState', (_) => 'on'); + handle('manageDevice', (args) { + manageCalls++; + // Native answers manageDevice for an already-connected peripheral by + // replaying device-ready (iOS replayReady, Android adoptConnectedDevice). + if (nativeReplaysReady && args.first == _deviceId) { + scheduleMicrotask(() => BleBridge.instance.onDeviceReady(_deviceId, _services)); + } + return null; + }); + handle('unmanageDevice', (_) { + unmanageCalls++; + return null; + }); + const unmanage = 'dev.flutter.pigeon.omi_pigeon.BleHostApi.unmanageDevice'; + TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger.setMockMessageHandler(unmanage, (message) async { + unmanageCalls++; + // A pigeon error reply: [code, message, details]. + return BleHostApi.pigeonChannelCodec + .encodeMessage(unmanageFails ? ['ble', 'native busy', null] : [null]); + }); + handle('isPeripheralConnected', (_) => linkUp); + handle('subscribeCharacteristic', (_) => null); + handle('unsubscribeCharacteristic', (_) => null); + handle('writeCharacteristic', (_) => null); + handle('readCharacteristic', (_) => Uint8List.fromList([80])); + }); + + tearDown(() async { + await ServiceManager.instance().device.forgetDevice(_deviceId); + for (final name in channels) { + TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger.setMockMessageHandler(name, null); + } + channels.clear(); + BleBridge.instance.clearPeripheralForTesting(_deviceId); + }); + + test('a link that becomes ready after the launch connect timed out still reaches the provider', () { + fakeAsync((async) { + pairPendant(); + final provider = DeviceProvider(); + + provider.initiateConnection('HomePageWrapper', boundDeviceOnly: true); + async.elapse(const Duration(seconds: 61)); + expect(manageCalls, 1); + expect(provider.isConnected, isFalse, reason: 'the launch connect gave up at its deadline'); + + linkUp = true; + BleBridge.instance.onDeviceReady(_deviceId, _services); + async.elapse(const Duration(seconds: 5)); + + expect(provider.isConnected, isTrue); + expect(provider.connectedDevice?.id, _deviceId); + provider.dispose(); + async.flushTimers(); + }); + }); + + test('a retired registration cannot silence the transport that replaced it', () { + final readyFor = []; + final releaseOld = BleBridge.instance.registerPeripheral( + peripheralUuid: _deviceId, + onDeviceReady: (_) => readyFor.add('old'), + ); + final releaseNew = BleBridge.instance.registerPeripheral( + peripheralUuid: _deviceId, + onDeviceReady: (_) => readyFor.add('new'), + ); + + releaseOld(); + BleBridge.instance.onDeviceReady(_deviceId, _services); + expect(readyFor, ['new']); + + releaseNew(); + BleBridge.instance.onDeviceReady(_deviceId, _services); + expect(readyFor, ['new'], reason: 'the owner releases its own registration'); + }); + + test('disconnecting after a timed-out connect releases native and leaves no orphan transport', () { + fakeAsync((async) { + pairPendant(); + final service = ServiceManager.instance().device; + service.ensureConnection(_deviceId, force: true); + async.elapse(const Duration(seconds: 61)); + final timedOut = service.connectionFor(_deviceId); + expect(timedOut, isNotNull, reason: 'a launch timeout keeps the transport for native auto-reconnect'); + expect(timedOut!.status, DeviceConnectionState.disconnected); + final transportStates = []; + timedOut.transport.connectionStateStream.listen(transportStates.add); + + service.disconnectDevice(_deviceId); + async.elapse(const Duration(seconds: 1)); + + expect(unmanageCalls, 1, reason: 'native was still connecting; an explicit disconnect must stop it'); + expect(service.connectionFor(_deviceId), isNull); + + // A late ready must not land on the retired transport, where no + // connection listens and the link would be adopted invisibly. + BleBridge.instance.onDeviceReady(_deviceId, _services); + async.flushMicrotasks(); + expect(transportStates, isNot(contains(DeviceTransportState.connected))); + }); + }); + + test('disconnectDevice disposes the transport it drops', () async { + pairPendant(); + linkUp = true; + nativeReplaysReady = true; + final service = ServiceManager.instance().device; + final connection = await service.ensureConnection(_deviceId, force: true); + expect(connection?.status, DeviceConnectionState.connected); + var transportClosed = false; + connection!.transport.connectionStateStream.listen((_) {}, onDone: () => transportClosed = true); + + await service.disconnectDevice(_deviceId); + await pumpEventQueue(); + + expect(transportClosed, isTrue, reason: 'an undisposed transport stays registered for native events'); + }); + + test('foreground reconcile re-adopts a link native holds while the provider shows disconnected', () { + fakeAsync((async) { + pairPendant(); + final provider = DeviceProvider(); + linkUp = true; + nativeReplaysReady = true; + + provider.nativeLink.reconcile(); + async.elapse(const Duration(seconds: 5)); + + expect(manageCalls, 1); + expect(provider.isConnected, isTrue); + expect(provider.connectedDevice?.id, _deviceId); + provider.dispose(); + async.flushTimers(); + }); + }); + + test('foreground reconcile never starts a physical connect', () { + fakeAsync((async) { + pairPendant(); + final provider = DeviceProvider(); + + provider.nativeLink.reconcile(); + async.elapse(const Duration(seconds: 5)); + + expect(manageCalls, 0, + reason: 'native has no link; reconnecting is native auto-reconnect, not a resume side effect'); + expect(provider.isConnected, isFalse); + provider.dispose(); + async.flushTimers(); + }); + }); + + test('foreground reconcile republishes a link DeviceService already holds', () { + fakeAsync((async) { + pairPendant(); + linkUp = true; + nativeReplaysReady = true; + ServiceManager.instance().device.ensureConnection(_deviceId, force: true); + async.elapse(const Duration(seconds: 5)); + expect(ServiceManager.instance().device.connectionFor(_deviceId)?.status, DeviceConnectionState.connected); + + // A provider that missed the connect event (recreated, or the event was + // lost) mirrors disconnected over DeviceService's connected link. + final provider = DeviceProvider(); + expect(provider.isConnected, isFalse); + + provider.nativeLink.reconcile(); + async.elapse(const Duration(seconds: 5)); + + expect(provider.isConnected, isTrue); + expect(manageCalls, 1, reason: 'the held connection is republished, not reconnected'); + provider.dispose(); + async.flushTimers(); + }); + }); + + test('foreground reconcile adopts only the device whose link it probed', () { + fakeAsync((async) { + pairPendant(); + final provider = DeviceProvider(); + final probe = Completer(); + final reconciler = NativeLinkReconciler(provider, nativeLinkProbe: (_) => probe.future); + + reconciler.reconcile(); + SharedPreferencesUtil().btDeviceSet(BtDevice( + id: 'OTHER-PENDANT', + name: 'Omi', + type: DeviceType.omi, + rssi: -60, + locator: DeviceLocator.bluetooth(deviceId: 'OTHER-PENDANT'), + )); + probe.complete(true); + async.elapse(const Duration(seconds: 5)); + + expect(manageCalls, 0, reason: 'the newly paired device was never probed'); + reconciler.dispose(); + provider.dispose(); + async.flushTimers(); + }); + }); + + test('a failed native release stays retryable and keeps adopting native readies', () { + fakeAsync((async) { + pairPendant(); + final service = ServiceManager.instance().device; + service.ensureConnection(_deviceId, force: true); + async.elapse(const Duration(seconds: 61)); + final connection = service.connectionFor(_deviceId)!; + final transportStates = []; + connection.transport.connectionStateStream.listen(transportStates.add); + + unmanageFails = true; + connection.transport.disconnect(); + async.elapse(const Duration(seconds: 1)); + expect(unmanageCalls, 1); + + // Native still owns the link, so its ready must not be dropped. + BleBridge.instance.onDeviceReady(_deviceId, _services); + async.flushMicrotasks(); + expect(transportStates.last, DeviceTransportState.connected); + + unmanageFails = false; + connection.transport.disconnect(); + async.elapse(const Duration(seconds: 1)); + expect(unmanageCalls, 2, reason: 'release is retried after a failure'); + }); + }); + + test('overlapping connected events for one physical connect run connect side effects once', () { + fakeAsync((async) { + pairPendant(); + linkUp = true; + nativeReplaysReady = true; + final service = ServiceManager.instance().device; + service.ensureConnection(_deviceId, force: true); + async.elapse(const Duration(seconds: 5)); + + final provider = DeviceProvider(); + int admissions() => + Logger.instance.talker.history.where((e) => '${e.message}'.startsWith('_onConnected inside')).length; + final admittedBefore = admissions(); + // Connection handling now holds DeviceService's mutex on a slow native + // link check, so the first connected event is still mid-flight while + // the provider mirrors disconnected. + TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger + .setMockMessageHandler('dev.flutter.pigeon.omi_pigeon.BleHostApi.isPeripheralConnected', (_) async { + await Future.delayed(const Duration(seconds: 2)); + return BleHostApi.pigeonChannelCodec.encodeMessage([true]); + }); + final reconciler = NativeLinkReconciler(provider, nativeLinkProbe: (_) async => true); + + provider.onDeviceConnectionStateChanged(_deviceId, DeviceConnectionState.connected); + async.elapse(const Duration(milliseconds: 150)); + reconciler.reconcile(); + async.elapse(const Duration(seconds: 15)); + + expect(provider.isConnected, isTrue); + expect(admissions() - admittedBefore, 1); + reconciler.dispose(); + provider.dispose(); + async.flushTimers(); + }); + }); + + test('a provider-only disconnect lets the resume reconcile re-admit the live link', () { + fakeAsync((async) { + pairPendant(); + linkUp = true; + nativeReplaysReady = true; + ServiceManager.instance().device.ensureConnection(_deviceId, force: true); + async.elapse(const Duration(seconds: 5)); + final provider = DeviceProvider(); + provider.nativeLink.reconcile(); + async.elapse(const Duration(seconds: 5)); + expect(provider.isConnected, isTrue); + + // e.g. onboarding's error path drops the mirror without a link loss. + provider.setIsConnected(false); + provider.nativeLink.reconcile(); + async.elapse(const Duration(seconds: 5)); + + expect(provider.isConnected, isTrue); + provider.dispose(); + async.flushTimers(); + }); + }); +} diff --git a/app/test/services/devices/saved_pendant_launch_timeout_reconnect_test.dart b/app/test/services/devices/saved_pendant_launch_timeout_reconnect_test.dart index 551c727f2dc..ef6a30dcfb5 100644 --- a/app/test/services/devices/saved_pendant_launch_timeout_reconnect_test.dart +++ b/app/test/services/devices/saved_pendant_launch_timeout_reconnect_test.dart @@ -43,7 +43,7 @@ void main() { messenger.setMockMessageHandler(channelName, null); } hostApiChannelNames.clear(); - BleBridge.instance.unregisterPeripheral(_deviceId); + BleBridge.instance.clearPeripheralForTesting(_deviceId); }); setUp(() async { diff --git a/app/test/unit/device_service_connections_test.dart b/app/test/unit/device_service_connections_test.dart index 9ad2910e1aa..33a905cf82b 100644 --- a/app/test/unit/device_service_connections_test.dart +++ b/app/test/unit/device_service_connections_test.dart @@ -127,7 +127,7 @@ void main() { TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger.setMockMessageHandler(channel, null); } mockedChannels.clear(); - BleBridge.instance.unregisterPeripheral(audioId); + BleBridge.instance.clearPeripheralForTesting(audioId); }); setUp(() async { From 2c27dd92a78b81932681c15f12913ee7707516f5 Mon Sep 17 00:00:00 2001 From: David Zhang <9387252+Git-on-my-level@users.noreply.github.com> Date: Wed, 14 Oct 2026 04:27:14 +0700 Subject: [PATCH 2/4] fix(app): revalidate pairing when a queued readoption is admitted The reconciler's republish of a DeviceService-held link went through onDeviceConnectionStateChanged, so admission queued behind the 100 ms connect debouncer. The debounced handler never rechecked the saved device: a pairing persisted in that window (an input picker seeding another microphone) was overwritten when the stale admission called setConnectedDevice, writing the old device back to preferences and starting its connect side effects. Route the reconciler's republish through a dedicated readoptServiceLink admission that requires the persisted pairing to still match when the handler runs. The shared connected-event path is unchanged: onboarding legitimately connects before persisting a first pairing. Addresses chatgpt-codex-connector review thread PRRT_kwDOLkKqys6sA9ST. --- app/lib/providers/device_provider.dart | 25 +++++++++++- app/lib/providers/native_link_reconciler.dart | 8 ++-- .../device_provider_late_ready_test.dart | 39 +++++++++++++++++++ 3 files changed, 67 insertions(+), 5 deletions(-) diff --git a/app/lib/providers/device_provider.dart b/app/lib/providers/device_provider.dart index 6e22b66937d..450e53a4b1e 100644 --- a/app/lib/providers/device_provider.dart +++ b/app/lib/providers/device_provider.dart @@ -1177,7 +1177,7 @@ class DeviceProvider extends ChangeNotifier implements IDeviceServiceSubsciption } } - void _handleDeviceConnected(String deviceId, int generation) async { + void _handleDeviceConnected(String deviceId, int generation, {bool requirePersistedPair = false}) async { if (!_isCurrent(generation)) return; _admittedConnectGeneration = generation; var connection = await ServiceManager.instance().device.ensureConnection(deviceId); @@ -1185,6 +1185,14 @@ class DeviceProvider extends ChangeNotifier implements IDeviceServiceSubsciption if (connection == null) { return; } + // A readoption authorized the persisted pairing at probe time, but this + // admission waited on DeviceService: pairing may have moved on (an input + // picker persisting a different microphone) and publishing the old device + // would overwrite that choice in preferences and start its side effects. + if (requirePersistedPair && SharedPreferencesUtil().btDevice.id != deviceId) { + Logger.debug('Discarding readoption of $deviceId: pairing moved on before admission'); + return; + } // One admission per physical connect: overlapping connected events (a // native connect still initializing, a resume reconcile) must not run // connect side effects twice. @@ -1442,6 +1450,21 @@ class DeviceProvider extends ChangeNotifier implements IDeviceServiceSubsciption notifyListeners(); } + /// Readopts a service link native already holds while the mirror shows the + /// persisted pairing disconnected. The reconciler authorizes this at probe + /// time, but admission still queues behind the connect debouncer: pairing or + /// the mirror can move on in that window, so [_handleDeviceConnected] + /// revalidates the persisted pairing before publishing. + void readoptServiceLink(String deviceId) { + if (_isDisposed) return; + final generation = _sessionGeneration; + _disconnectDebouncer.cancel(); + _connectDebouncer.run(() { + if (!_isCurrent(generation)) return; + _handleDeviceConnected(deviceId, generation, requirePersistedPair: true); + }); + } + @override void onDeviceConnectionStateChanged(String deviceId, DeviceConnectionState state) async { Logger.debug("provider > device connection state changed...$deviceId...$state...${connectedDevice?.id}"); diff --git a/app/lib/providers/native_link_reconciler.dart b/app/lib/providers/native_link_reconciler.dart index 6cc1876e89e..27f3fbee9c0 100644 --- a/app/lib/providers/native_link_reconciler.dart +++ b/app/lib/providers/native_link_reconciler.dart @@ -53,10 +53,10 @@ class NativeLinkReconciler { Logger.debug('NativeLinkReconciler: native holds $deviceId while disconnected; re-adopting'); final deviceService = ServiceManager.instance().device; if (deviceService.connectionFor(deviceId)?.status == DeviceConnectionState.connected) { - // DeviceService holds the link but the provider never heard. Republish - // synchronously with the checks above: nothing can intervene, and no - // transport event will publish it. - _provider.onDeviceConnectionStateChanged(deviceId, DeviceConnectionState.connected); + // DeviceService holds the link but the provider never heard. Admission + // revalidates the persisted pairing when the debounced handler runs, so + // a pairing that moved on during the queue is not published. + _provider.readoptServiceLink(deviceId); return; } // Otherwise native replays device-ready into a fresh transport, whose own diff --git a/app/test/providers/device_provider_late_ready_test.dart b/app/test/providers/device_provider_late_ready_test.dart index e59d0c13a55..330b7e62f5d 100644 --- a/app/test/providers/device_provider_late_ready_test.dart +++ b/app/test/providers/device_provider_late_ready_test.dart @@ -256,6 +256,45 @@ void main() { }); }); + test('a readoption queued behind the connect debounce is dropped when pairing moves on', () { + fakeAsync((async) { + pairPendant(); + linkUp = true; + nativeReplaysReady = true; + ServiceManager.instance().device.ensureConnection(_deviceId, force: true); + async.elapse(const Duration(seconds: 5)); + expect(ServiceManager.instance().device.connectionFor(_deviceId)?.status, DeviceConnectionState.connected); + + final provider = DeviceProvider(); + expect(provider.isConnected, isFalse); + final probe = Completer(); + final reconciler = NativeLinkReconciler(provider, nativeLinkProbe: (_) => probe.future); + + reconciler.reconcile(); + probe.complete(true); + // Let the probe resolve and queue the readmission behind the 100 ms + // connect debounce, then move the pairing elsewhere before it fires. + async.flushMicrotasks(); + expect(provider.isConnected, isFalse, reason: 'admission is still queued'); + SharedPreferencesUtil().btDeviceSet(BtDevice( + id: 'OTHER-PENDANT', + name: 'Omi', + type: DeviceType.omi, + rssi: -60, + locator: DeviceLocator.bluetooth(deviceId: 'OTHER-PENDANT'), + )); + async.elapse(const Duration(seconds: 5)); + + expect(provider.isConnected, isFalse, reason: 'the old device must not publish over the newer pairing'); + expect(provider.connectedDevice?.id, isNot(_deviceId)); + expect(SharedPreferencesUtil().btDevice.id, 'OTHER-PENDANT', + reason: 'the stale admission must not rewrite pairing'); + reconciler.dispose(); + provider.dispose(); + async.flushTimers(); + }); + }); + test('foreground reconcile adopts only the device whose link it probed', () { fakeAsync((async) { pairPendant(); From a23da451358b573e4907ef5d7179a9ec274602d5 Mon Sep 17 00:00:00 2001 From: David Zhang <9387252+Git-on-my-level@users.noreply.github.com> Date: Wed, 14 Oct 2026 05:11:48 +0700 Subject: [PATCH 3/4] fix(app): guard fresh reconcile adoption and replay a ready lost to a failed release Two follow-ups from review: - The reconciler's fresh-connection branch still published through the unguarded generic connected path. A pairing persisted while the link formed (an input picker seeding another microphone) was overwritten when that admission ran. The reconciler now queues its guarded readoptServiceLink admission in place of the pending generic one: the connect debouncer is single-slot and the reconcile continuation runs as the next microtask inside the 100 ms window, so the guarded admission always replaces the unguarded one. - A ready delivered while unmanageDevice was pending was dropped by the _nativeReleased guard even when the release then failed, stranding a live native link until the next resume. The transport now buffers the pending ready and replays it after a failed release, exactly like a ready arriving after the failure. Addresses chatgpt-codex-connector threads PRRT_kwDOLkKqys6sEx56 and PRRT_kwDOLkKqys6sEx59. --- app/lib/providers/native_link_reconciler.dart | 13 +++- .../transports/native_ble_transport.dart | 27 ++++++- .../device_provider_late_ready_test.dart | 71 +++++++++++++++++++ 3 files changed, 105 insertions(+), 6 deletions(-) diff --git a/app/lib/providers/native_link_reconciler.dart b/app/lib/providers/native_link_reconciler.dart index 27f3fbee9c0..623426de5a0 100644 --- a/app/lib/providers/native_link_reconciler.dart +++ b/app/lib/providers/native_link_reconciler.dart @@ -59,9 +59,16 @@ class NativeLinkReconciler { _provider.readoptServiceLink(deviceId); return; } - // Otherwise native replays device-ready into a fresh transport, whose own - // connected event publishes through the same path as any native connect. - await deviceService.ensureConnection(deviceId, force: true); + // Otherwise native replays device-ready into a fresh transport. Its + // connected event queues an unguarded admission through the generic path; + // queue the guarded readoption in its place — the debounced action is + // single-slot, and this runs as the next microtask after the transport's + // event, well inside its 100 ms window — so a pairing that moved on while + // the link was forming is never published. + final connection = await deviceService.ensureConnection(deviceId, force: true); + if (connection?.status == DeviceConnectionState.connected) { + _provider.readoptServiceLink(deviceId); + } } void dispose() { diff --git a/app/lib/services/devices/transports/native_ble_transport.dart b/app/lib/services/devices/transports/native_ble_transport.dart index 81e4e33591c..b738b3ca31d 100644 --- a/app/lib/services/devices/transports/native_ble_transport.dart +++ b/app/lib/services/devices/transports/native_ble_transport.dart @@ -43,6 +43,11 @@ class NativeBleTransport extends DeviceTransport implements CaptureSubscriptionE /// [connect]. Native keeps connecting and reconnecting after a Dart-side /// timeout or drop, so release cannot be inferred from the mirrored [_state]. bool _nativeReleased = false; + // True only while disconnect() awaits unmanageDevice: a ready arriving in + // this window is buffered in [_readyWhileReleasing] because a failed release + // must replay it (native kept the link), while a successful one discards it. + bool _releasePending = false; + List? _readyWhileReleasing; Timer? _audioLivenessTimer; int _audioSilenceResubscribes = 0; int _subscriptionGeneration = 0; @@ -153,17 +158,28 @@ class NativeBleTransport extends DeviceTransport implements CaptureSubscriptionE _closeAllStreams(); _services = []; + List? replayReady; try { _nativeReleased = true; + _releasePending = true; await _hostApi.unmanageDevice(_peripheralUuid); + _releasePending = false; + _readyWhileReleasing = null; } catch (e) { // Native may still own the link; keep release retryable and its readies - // adoptable rather than mirroring "released" over a live link. + // adoptable rather than mirroring "released" over a live link. A ready + // that arrived while the release was pending is still native's word that + // the link is live, so replay it below — after disconnect's final state, + // exactly like a ready arriving after a failed release. + _releasePending = false; _nativeReleased = false; + replayReady = _readyWhileReleasing; + _readyWhileReleasing = null; Logger.debug('[NativeBleTransport] unmanageDevice failed: $e'); } _updateState(DeviceTransportState.disconnected); + if (replayReady != null) _handleDeviceReady(replayReady); } @override @@ -365,8 +381,13 @@ class NativeBleTransport extends DeviceTransport implements CaptureSubscriptionE void _handleDeviceReady(List services) { // A ready already in flight when this transport released native must not - // re-adopt the link behind the explicit disconnect. - if (_nativeReleased) return; + // re-adopt the link behind the explicit disconnect. If a release is still + // pending, buffer the ready: a failed release replays it (native kept the + // link), a successful one discards it. + if (_nativeReleased) { + if (_readyWhileReleasing == null && _releasePending) _readyWhileReleasing = services; + return; + } _subscriptionGeneration++; _subscribedSubscriptionKeys.clear(); subscriptionFailures.clear(); diff --git a/app/test/providers/device_provider_late_ready_test.dart b/app/test/providers/device_provider_late_ready_test.dart index 330b7e62f5d..74c8a3dcb18 100644 --- a/app/test/providers/device_provider_late_ready_test.dart +++ b/app/test/providers/device_provider_late_ready_test.dart @@ -347,6 +347,77 @@ void main() { }); }); + test('a ready delivered while a release is pending is replayed when that release fails', () { + fakeAsync((async) { + pairPendant(); + final service = ServiceManager.instance().device; + service.ensureConnection(_deviceId, force: true); + async.elapse(const Duration(seconds: 61)); + final connection = service.connectionFor(_deviceId)!; + final transportStates = []; + connection.transport.connectionStateStream.listen(transportStates.add); + + // The release hangs; native delivers the ready while it is pending, then + // the release fails. The buffered ready must be replayed, not dropped. + final release = Completer(); + TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger + .setMockMessageHandler('dev.flutter.pigeon.omi_pigeon.BleHostApi.unmanageDevice', (message) async { + unmanageCalls++; + await release.future; + return BleHostApi.pigeonChannelCodec + .encodeMessage(unmanageFails ? ['ble', 'native busy', null] : [null]); + }); + unmanageFails = true; + connection.transport.disconnect(); + BleBridge.instance.onDeviceReady(_deviceId, _services); + release.complete(); + async.flushMicrotasks(); + + expect(transportStates.last, DeviceTransportState.connected, + reason: 'the pending ready survives the failed release and re-adopts the live link'); + + // Settle the transport like the failed-release test above so tearDown + // finds a released, disconnected transport. + unmanageFails = false; + connection.transport.disconnect(); + async.elapse(const Duration(seconds: 1)); + expect(unmanageCalls, 2); + }); + }); + + test('a reconcile adopt of a forming connection is dropped when pairing moves on', () { + fakeAsync((async) { + pairPendant(); + linkUp = true; + // manageDevice replays ready only for the *held* case; a fresh ensure + // connection must still complete, driven here by the native replay. + nativeReplaysReady = true; + final provider = DeviceProvider(); + final probe = Completer(); + final reconciler = NativeLinkReconciler(provider, nativeLinkProbe: (_) => probe.future); + + reconciler.reconcile(); + probe.complete(true); + async.flushMicrotasks(); + // The transport is forming; the persisted pairing moves on before the + // connected admission can run. + SharedPreferencesUtil().btDeviceSet(BtDevice( + id: 'OTHER-PENDANT', + name: 'Omi', + type: DeviceType.omi, + rssi: -60, + locator: DeviceLocator.bluetooth(deviceId: 'OTHER-PENDANT'), + )); + async.elapse(const Duration(seconds: 5)); + + expect(provider.isConnected, isFalse, reason: 'the forming link belongs to a retired pairing'); + expect(SharedPreferencesUtil().btDevice.id, 'OTHER-PENDANT'); + reconciler.dispose(); + provider.dispose(); + async.flushTimers(); + }); + }); + test('overlapping connected events for one physical connect run connect side effects once', () { fakeAsync((async) { pairPendant(); From cf6a8aa287383ff1717795c78ebe669217d24aae Mon Sep 17 00:00:00 2001 From: David Zhang <9387252+Git-on-my-level@users.noreply.github.com> Date: Wed, 14 Oct 2026 08:17:17 +0700 Subject: [PATCH 4/4] fix(app): carry the readoption pairing constraint through publication Threads 4/5 on the late-ready review: a resume reconcile authorizes the persisted pairing at probe time, but the fresh path's generic connected admission can start running while the link is still forming (DeviceConnection.connect emits connected before awaiting ping and getDeviceInfo), and device setup publication awaits link checks after admission. Arm the constraint on the provider before the link forms, gate admission, both btDevice persists, and the setup continuation (before battery/capture/WAL side effects and before the final publication), and clear it on session reset. A pairing persisted in any of those windows (the Ray-Ban input picker seeding another microphone before awaiting its connection) is no longer published over; verified by two provider-level regressions that fail on the previous head. --- app/lib/providers/device_provider.dart | 64 +++++++++++-- app/lib/providers/native_link_reconciler.dart | 14 ++- .../device_provider_late_ready_test.dart | 91 +++++++++++++++++++ 3 files changed, 158 insertions(+), 11 deletions(-) diff --git a/app/lib/providers/device_provider.dart b/app/lib/providers/device_provider.dart index 450e53a4b1e..8a39e6d6d63 100644 --- a/app/lib/providers/device_provider.dart +++ b/app/lib/providers/device_provider.dart @@ -147,6 +147,14 @@ class DeviceProvider extends ChangeNotifier implements IDeviceServiceSubsciption int _admittedConnectGeneration = -1; late final NativeLinkReconciler nativeLink = NativeLinkReconciler(this); (DeviceConnection, int)? _admittedLink; + // When a resume reconcile arms a readoption, admissions for that device are + // constrained to the pairing the reconcile probed: the persisted pairing + // must still name that device when an admission or its async continuation + // runs, or a pairing persisted in the windows between (an input picker + // seeding another microphone) is silently overwritten. Armed before the + // link starts forming, so the constraint holds for whichever connected + // event queues the admission. + String? _readoptExpectedPair; DeviceProvider({ BleDiagnosticsLoader? bleDiagnosticsLoader, @@ -207,6 +215,7 @@ class DeviceProvider extends ChangeNotifier implements IDeviceServiceSubsciption void clearUserData() { _sessionGeneration++; _admittedLink = null; + _readoptExpectedPair = null; _deviceHealthTimer?.cancel(); _deviceHealthTimer = null; _admittedConnectGeneration = -1; @@ -391,14 +400,14 @@ class DeviceProvider extends ChangeNotifier implements IDeviceServiceSubsciption if (connectedDevice != null) { if (pairedDevice?.firmwareRevision != null && pairedDevice?.firmwareRevision != 'Unknown') { if (!_isCurrent(generation)) return; - SharedPreferencesUtil().btDevice = pairedDevice!; + _persistPairedDevice(); return; } var connection = await ServiceManager.instance().device.ensureConnection(connectedDevice!.id); if (!_isCurrent(generation)) return; pairedDevice = await connectedDevice?.getDeviceInfo(connection); if (!_isCurrent(generation)) return; - SharedPreferencesUtil().btDevice = pairedDevice!; + _persistPairedDevice(); } else { if (!_isCurrent(generation)) return; if (SharedPreferencesUtil().btDevice.id.isEmpty) { @@ -411,6 +420,23 @@ class DeviceProvider extends ChangeNotifier implements IDeviceServiceSubsciption notifyListeners(); } + /// Publishes the paired device to preferences unless a readoption armed a + /// pairing constraint and the persisted pairing has since moved on: the + /// armed publication belongs to the probe-time pairing, and writing here + /// would silently overwrite the newer choice (an input picker's selection). + void _persistPairedDevice() { + if (_readoptPairMovedOn(pairedDevice!.id)) { + Logger.debug('Skipping paired-device persist: pairing moved on from $_readoptExpectedPair'); + return; + } + SharedPreferencesUtil().btDevice = pairedDevice!; + } + + /// True when a readoption armed a pairing constraint for [deviceId] and the + /// persisted pairing no longer names it. + bool _readoptPairMovedOn(String deviceId) => + _readoptExpectedPair == deviceId && SharedPreferencesUtil().btDevice.id != deviceId; + /// Hardware find-device LED. Not account publication; left unfenced. Future findDevice() { final device = connectedDevice ?? pairedDevice; @@ -955,6 +981,14 @@ class DeviceProvider extends ChangeNotifier implements IDeviceServiceSubsciption Logger.debug('Discarding device setup continuation from a stale connection session'); return; } + // A readoption's authorization is only as fresh as its last check: device + // setup awaits BLE work, and a pairing persisted during that window (an + // input picker seeding another microphone) must not get battery, capture, + // and WAL side effects started for the retired pairing. + if (_readoptPairMovedOn(device.id)) { + Logger.debug('Discarding device setup continuation of ${device.id}: pairing moved on during setup'); + return; + } final normalizedDevice = pairedDevice ?? device; if (captureProvider != null) { @@ -968,6 +1002,10 @@ class DeviceProvider extends ChangeNotifier implements IDeviceServiceSubsciption // Read initial battery level int currentLevel = await _retrieveBatteryLevel(device.id); if (!_isCurrent(generation)) return; + if (_readoptPairMovedOn(device.id)) { + Logger.debug('Discarding readoption of ${device.id}: pairing moved on before side effects'); + return; + } if (currentLevel != -1) { batteryLevel = currentLevel; BatteryWidgetService().updateBatteryInfo( @@ -993,6 +1031,10 @@ class DeviceProvider extends ChangeNotifier implements IDeviceServiceSubsciption await getDeviceInfo(); if (!_isCurrent(generation)) return; + if (_readoptPairMovedOn(device.id)) { + Logger.debug('Discarding readoption of ${device.id}: pairing moved on before final publication'); + return; + } SharedPreferencesUtil().deviceName = device.name; // getDeviceInfo() may have reclassified the discovery object — an Omi-typed @@ -1189,7 +1231,7 @@ class DeviceProvider extends ChangeNotifier implements IDeviceServiceSubsciption // admission waited on DeviceService: pairing may have moved on (an input // picker persisting a different microphone) and publishing the old device // would overwrite that choice in preferences and start its side effects. - if (requirePersistedPair && SharedPreferencesUtil().btDevice.id != deviceId) { + if ((requirePersistedPair || _readoptExpectedPair == deviceId) && SharedPreferencesUtil().btDevice.id != deviceId) { Logger.debug('Discarding readoption of $deviceId: pairing moved on before admission'); return; } @@ -1452,11 +1494,12 @@ class DeviceProvider extends ChangeNotifier implements IDeviceServiceSubsciption /// Readopts a service link native already holds while the mirror shows the /// persisted pairing disconnected. The reconciler authorizes this at probe - /// time, but admission still queues behind the connect debouncer: pairing or - /// the mirror can move on in that window, so [_handleDeviceConnected] - /// revalidates the persisted pairing before publishing. + /// time — and arms the pairing constraint before the link forms — so every + /// admission for this device revalidates the persisted pairing at admission + /// and through the publication continuation. void readoptServiceLink(String deviceId) { if (_isDisposed) return; + _readoptExpectedPair = deviceId; final generation = _sessionGeneration; _disconnectDebouncer.cancel(); _connectDebouncer.run(() { @@ -1465,6 +1508,15 @@ class DeviceProvider extends ChangeNotifier implements IDeviceServiceSubsciption }); } + /// Arms the readoption pairing constraint for [deviceId] before the link + /// starts forming: whichever connected event queues the admission — the + /// transport's own event or the reconciler's — it must still match the + /// persisted pairing when it and its continuation run. + void armReadoptPairConstraint(String deviceId) { + if (_isDisposed) return; + _readoptExpectedPair = deviceId; + } + @override void onDeviceConnectionStateChanged(String deviceId, DeviceConnectionState state) async { Logger.debug("provider > device connection state changed...$deviceId...$state...${connectedDevice?.id}"); diff --git a/app/lib/providers/native_link_reconciler.dart b/app/lib/providers/native_link_reconciler.dart index 623426de5a0..17cd11880d1 100644 --- a/app/lib/providers/native_link_reconciler.dart +++ b/app/lib/providers/native_link_reconciler.dart @@ -60,11 +60,15 @@ class NativeLinkReconciler { return; } // Otherwise native replays device-ready into a fresh transport. Its - // connected event queues an unguarded admission through the generic path; - // queue the guarded readoption in its place — the debounced action is - // single-slot, and this runs as the next microtask after the transport's - // event, well inside its 100 ms window — so a pairing that moved on while - // the link was forming is never published. + // connected event queues an admission through the generic path, which can + // start running before this reconcile finishes (DeviceConnection.connect + // emits connected before awaiting ping/getDeviceInfo). Arm the pairing + // constraint on the provider *before* the link starts forming so every + // admission for this device — whichever event queued it — revalidates the + // probe-time pairing, and keep it armed through publication: a pairing + // persisted while the link was forming (an input picker seeding another + // microphone) is never published over. + _provider.armReadoptPairConstraint(deviceId); final connection = await deviceService.ensureConnection(deviceId, force: true); if (connection?.status == DeviceConnectionState.connected) { _provider.readoptServiceLink(deviceId); diff --git a/app/test/providers/device_provider_late_ready_test.dart b/app/test/providers/device_provider_late_ready_test.dart index 74c8a3dcb18..fec0ff69bd3 100644 --- a/app/test/providers/device_provider_late_ready_test.dart +++ b/app/test/providers/device_provider_late_ready_test.dart @@ -454,6 +454,97 @@ void main() { }); }); + test('a generic admission racing a fresh reconcile is dropped when pairing moves on', () { + fakeAsync((async) { + pairPendant(); + linkUp = true; + nativeReplaysReady = true; + final provider = DeviceProvider(); + // DeviceConnection.connect emits connected before awaiting ping and + // getDeviceInfo, so the transport's own connected event queues the + // generic admission while the reconcile is still forming the link. + // Block that admission inside ensureConnection behind DeviceService's + // mutex on a slow native link check. + TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger + .setMockMessageHandler('dev.flutter.pigeon.omi_pigeon.BleHostApi.isPeripheralConnected', (_) async { + await Future.delayed(const Duration(seconds: 2)); + return BleHostApi.pigeonChannelCodec.encodeMessage([true]); + }); + final reconciler = NativeLinkReconciler(provider, nativeLinkProbe: (_) async => true); + + reconciler.reconcile(); + async.elapse(const Duration(milliseconds: 50)); + // The transport's connected event has queued the generic admission; it + // runs while the link is still forming, and the persisted pairing moves + // on before it passes its checks. + SharedPreferencesUtil().btDeviceSet(BtDevice( + id: 'OTHER-PENDANT', + name: 'Omi', + type: DeviceType.omi, + rssi: -60, + locator: DeviceLocator.bluetooth(deviceId: 'OTHER-PENDANT'), + )); + async.elapse(const Duration(seconds: 15)); + + expect(provider.isConnected, isFalse, + reason: 'the racing generic admission must not publish the retired pairing'); + expect(SharedPreferencesUtil().btDevice.id, 'OTHER-PENDANT'); + reconciler.dispose(); + provider.dispose(); + async.flushTimers(); + }); + }); + + test('a pairing persisted during device setup does not get the retired device published over it', () { + fakeAsync((async) { + pairPendant(); + linkUp = true; + nativeReplaysReady = true; + final service = ServiceManager.instance().device; + service.ensureConnection(_deviceId, force: true); + async.elapse(const Duration(seconds: 5)); + expect(service.connectionFor(_deviceId)?.status, DeviceConnectionState.connected); + + final provider = DeviceProvider(); + // Every step of device setup that touches the link re-checks it via + // ensureConnection -> isConnected. Park the *second* such check (the + // one after admission) and drift the persisted pairing while it is + // parked — exactly the window the Ray-Ban input picker creates when it + // persists a new microphone before awaiting its own connection. + var linkChecks = 0; + final linkCheckBlocked = Completer(); + TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger + .setMockMessageHandler('dev.flutter.pigeon.omi_pigeon.BleHostApi.isPeripheralConnected', (message) async { + linkChecks++; + if (linkChecks == 2) { + await linkCheckBlocked.future; + } + return BleHostApi.pigeonChannelCodec.encodeMessage([true]); + }); + final reconciler = NativeLinkReconciler(provider, nativeLinkProbe: (_) async => true); + + reconciler.reconcile(); + async.elapse(const Duration(seconds: 5)); + // Device setup is parked mid-setup; the persisted pairing moves on + // inside the window. + SharedPreferencesUtil().btDeviceSet(BtDevice( + id: 'OTHER-PENDANT', + name: 'Omi', + type: DeviceType.omi, + rssi: -60, + locator: DeviceLocator.bluetooth(deviceId: 'OTHER-PENDANT'), + )); + linkCheckBlocked.complete(); + async.elapse(const Duration(seconds: 20)); + + expect(SharedPreferencesUtil().btDevice.id, 'OTHER-PENDANT', + reason: 'device setup must not publish the retired pairing over the newer choice'); + reconciler.dispose(); + provider.dispose(); + async.flushTimers(); + }); + }); + test('a provider-only disconnect lets the resume reconcile re-admit the live link', () { fakeAsync((async) { pairPendant();