Skip to content

refactor: simplify device_provider — remove legacy polling, use reactive native reconnect#5915

Merged
mdmohsin7 merged 9 commits intomainfrom
fix/skip-reconnect-on-manual-disconnect
Mar 25, 2026
Merged

refactor: simplify device_provider — remove legacy polling, use reactive native reconnect#5915
mdmohsin7 merged 9 commits intomainfrom
fix/skip-reconnect-on-manual-disconnect

Conversation

@mdmohsin7
Copy link
Copy Markdown
Member

@mdmohsin7 mdmohsin7 commented Mar 22, 2026

Summary

  • Remove Dart-side 15s polling reconnection timer that raced with NativeBleTransport auto-reconnect (iOS CoreBluetooth / Android CompanionDeviceService)
  • Replace periodicConnect with initiateConnection — single ensureConnection call, native handles retries
  • Simplify scanAndConnectToDevice — direct connection, no fragile 2s delays or redundant queries
  • Fix prepareDFU to use setFirmwareUpdateInProgress instead of _reconnectAt DateTime hack that only suppressed polling for 30s
  • Fix transport BleBridge registration leak when replacing stale connections in DeviceService._connectToDevice
  • Fix OmiGlass OTA page not resetting _isFirmwareUpdateInProgress in dispose()
  • Keep discovery-only timer for onboarding (scanning for new devices every 10s)
Screenshot 2026-03-25 at 3 18 32 PM

Test plan

  • Cold start with paired device — connects via native auto-reconnect
  • Device goes out of range — shows disconnected, reconnects when back in range
  • Manual disconnect from device info — no reconnect attempt
  • DFU firmware update — disconnect, update, auto-reconnect after reboot
  • OmiGlass OTA — firmware flag resets when leaving page
  • Onboarding — devices discovered even if turned on after page loads
  • flutter test passes (191/197, 6 pre-existing failures in unrelated widget tests)

🤖 Generated with Claude Code

When the user disconnects from Device Info page, set _manualDisconnect
flag so onDeviceDisconnected skips periodicConnect and cancels the
reconnection timer.
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 22, 2026

Greptile Summary

This PR removes the Dart-side 15-second polling/reconnection timer (periodicConnect / _reconnectionTimer) and replaces it with a single initiateConnection call that delegates reconnection entirely to the native BLE layer (iOS CoreBluetooth / Android CompanionDeviceService). It also fixes a BleBridge registration leak in DeviceService._connectToDevice, fixes prepareDFU to use the proper setFirmwareUpdateInProgress flag instead of a 30-second _reconnectAt DateTime hack, and fixes the OmiGlass OTA page not resetting _isFirmwareUpdateInProgress on dispose().

Key changes:

  • periodicConnectinitiateConnection: single ensureConnection(force: true) call; native handles all retries
  • _startDiscoveryScanning / _runDiscoveryScan: retained as a 10-second periodic scan only for onboarding (no paired device)
  • _manualDisconnect flag: suppresses the 30-second disconnect notification for deliberate disconnects (DFU, user-initiated) — but see P1 comment: the flag is set before the await ensureConnection call, so a null-connection early return leaves it permanently true
  • DeviceService._connectToDevice: now calls transport.dispose() on the stale connection before replacing it, preventing leaked BleBridge callbacks
  • OmiGlassOtaUpdate.dispose(): now calls resetFirmwareUpdateState(), fixing a pre-existing state leak

Confidence Score: 4/5

  • Safe to merge after addressing the _manualDisconnect early-reset bug; all other issues are minor style/edge-case concerns.
  • The overall architecture is sound — native auto-reconnect is the right approach, the BleBridge leak fix is correct, and the OmiGlass dispose fix is clean. The one P1 bug (_manualDisconnect set before awaiting ensureConnection) is a small ordering fix that prevents a missed disconnect notification in a failure edge case. The discovery timer leak is a P2 that was equivalent in behavior to the old polling timer. No data-loss or security risks are introduced.
  • app/lib/providers/device_provider.dart — _bleDisconnectDevice flag ordering (P1) and _discoveryTimer cancellation on onboarding exit (P2).

Important Files Changed

Filename Overview
app/lib/providers/device_provider.dart Core refactor: replaces 15s polling timer with a single initiateConnection call + native auto-reconnect. Has three issues: _manualDisconnect not reset on null connection (P1), _discoveryTimer leaks if onboarding is abandoned (P2), and inconsistent navigator key reference (P2).
app/lib/services/devices.dart Fixes transport BleBridge registration leak: now calls _connection!.transport.dispose() before nulling _connection, ensuring callbacks are unregistered even for non-connected stale transports.
app/lib/pages/home/omiglass_ota_update.dart Adds resetFirmwareUpdateState() call in dispose(), ensuring _isFirmwareUpdateInProgress is cleared when leaving the OTA page — fixes a pre-existing state leak.
app/lib/pages/home/page.dart Trivial rename: periodicConnectinitiateConnection at the HomePageWrapper call site.
app/lib/pages/onboarding/find_device/found_devices.dart Renames call from periodicConnect to initiateConnection; minor formatting fix to a ternary chain.
app/lib/providers/onboarding_provider.dart Single-line rename: periodicConnectinitiateConnection; no logic changes.

Sequence Diagram

sequenceDiagram
    participant App as Flutter App
    participant DP as DeviceProvider
    participant DS as DeviceService
    participant NT as NativeBleTransport
    participant OS as iOS/Android BLE

    Note over App,OS: Cold start / HomePageWrapper init
    App->>DP: initiateConnection('HomePageWrapper')
    DP->>DS: ensureConnection(pairedId, force: true)
    DS->>DS: _connectToDevice(id)<br/>dispose old transport first
    DS->>NT: connect(onConnectionStateChanged)
    NT->>OS: native connect request
    OS-->>NT: connected callback
    NT-->>DS: onDeviceConnectionStateChanged(connected)
    DS-->>DP: onDeviceConnectionStateChanged(connected)
    DP->>DP: _handleDeviceConnected → _onDeviceConnected

    Note over App,OS: Device goes out of range
    OS-->>NT: disconnected callback
    NT-->>DS: onDeviceConnectionStateChanged(disconnected)
    DS-->>DP: onDeviceConnectionStateChanged(disconnected)
    DP->>DP: onDeviceDisconnected<br/>[_manualDisconnect=false → schedule 30s notification]
    OS->>NT: native auto-reconnect (no Dart polling)
    NT-->>DS: onDeviceConnectionStateChanged(connected)
    DS-->>DP: _handleDeviceConnected

    Note over App,OS: User-initiated disconnect / DFU
    App->>DP: prepareDFU()
    DP->>DP: setFirmwareUpdateInProgress(true)
    DP->>DP: _bleDisconnectDevice → _manualDisconnect=true
    DP->>DS: ensureConnection + disconnect()
    DS-->>DP: onDeviceConnectionStateChanged(disconnected)
    DP->>DP: onDeviceDisconnected<br/>[_manualDisconnect=true → return early, no notification]

    Note over App,OS: Onboarding (no paired device)
    App->>DP: initiateConnection('FoundDevices')
    DP->>DP: _startDiscoveryScanning()<br/>Timer.periodic 10s
    DP->>DS: discover() every 10s
    DS-->>DP: onDevices(foundDevices)
Loading

Comments Outside Diff (3)

  1. app/lib/providers/device_provider.dart, line 103-110 (link)

    _manualDisconnect not reset when connection is null

    _manualDisconnect is set to true synchronously at the top of _bleDisconnectDevice. If ensureConnection returns null (e.g., the transport can't locate the device), the function returns early without ever calling disconnect(), so no disconnect event fires. _manualDisconnect is therefore left as true indefinitely.

    The next time the device disconnects unexpectedly (e.g., goes out of range), onDeviceDisconnected will see _manualDisconnect == true, reset it to false, and return early — silently swallowing the 30-second "device disconnected" notification for an involuntary disconnect.

  2. app/lib/providers/device_provider.dart, line 362-363 (link)

    Inconsistent navigator key reference

    This line uses MyApp.navigatorKey.currentContext while the rest of device_provider.dart (lines 167, 447, 489) consistently uses globalNavigatorKey.currentContext. MyApp.navigatorKey is a getter that delegates to globalNavigatorKey (confirmed in main.dart), so they are functionally identical — but mixing the two styles within the same file creates unnecessary confusion.

  3. app/lib/providers/device_provider.dart, line 254-273 (link)

    _discoveryTimer leaks if user exits onboarding without pairing

    _startDiscoveryScanning() is triggered by initiateConnection('FoundDevices') (and initiateConnection('Onboarding')) when no device is paired. The timer calls deviceService.discover() every 10 seconds. _runDiscoveryScan self-cancels only when btDevice.id.isNotEmpty || isConnected.

    If the user opens the onboarding/found-devices page and then navigates away without ever pairing a device, neither cancellation condition is met and the timer continues firing indefinitely — running a 5-second BLE scan every 10 seconds in the background.

    Consider also cancelling _discoveryTimer when the provider receives an explicit signal that the user has left onboarding, or expose a stopDiscovery() method that the onboarding page can call in its own dispose().

Reviews (2): Last reviewed commit: "Merge branch 'main' into fix/skip-reconn..." | Re-trigger Greptile

…tive reconnect

Remove Dart-side 15s polling timer (_reconnectionTimer, _startPollingReconnect,
_scanConnectDevice) that raced with NativeBleTransport auto-reconnect. Replace
periodicConnect with initiateConnection that calls ensureConnection once. Native
iOS CoreBluetooth / Android CompanionDeviceService handles reconnection. Keep a
discovery-only timer for onboarding (scanning for new devices). Fix prepareDFU
to use setFirmwareUpdateInProgress instead of _reconnectAt DateTime hack.
Always call transport.dispose() in _connectToDevice before nulling _connection.
Previously, if a connection was in disconnected state (e.g. after timeout),
the old NativeBleTransport stayed registered in BleBridge, leaking callbacks.
OmiGlass OTA page was missing resetFirmwareUpdateState() in dispose(),
unlike firmware_update.dart. This left _isFirmwareUpdateInProgress true
after leaving the page, permanently suppressing firmware update checks.
@mdmohsin7 mdmohsin7 changed the title fix: skip Dart-side reconnect polling after manual disconnect refactor: simplify device_provider — remove legacy polling, use reactive native reconnect Mar 24, 2026
@mdmohsin7
Copy link
Copy Markdown
Member Author

@greptile-apps re-review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant