Defer forgetting sensor on suspected session end to avoid long reconnection outages - #62
Open
cheets wants to merge 2 commits into
Open
Defer forgetting sensor on suspected session end to avoid long reconnection outages#62cheets wants to merge 2 commits into
cheets wants to merge 2 commits into
Conversation
A remote disconnect before authentication completes is treated as a session end, immediately forgetting the sensor and scanning from scratch. The same disconnect signature occurs on transient BLE handshake failures (auth notification timeouts, encryption failures), where forgetting the tracked peripheral downgrades reconnection from an OS-level pending connect to a throttled background scan, causing 10-40 minute glucose outages. Field logs showed 118 suspected session ends in 7 days of which 1 was a real session end. Keep tracking the sensor on a suspected session end and defer the scan-for-new-sensor by a 15 minute wall-clock grace period, cancelled when any glucose or backfill message arrives. A real session end still switches sensors: the stopped sensor stays silent, the grace period expires, and the new sensor is discovered during its warmup. Immediate switch on sensorFailed/sessionEnded algorithm states is unchanged. Also add DI seams (central manager factory, injectable bluetooth manager, internal manager init) so G7CGMManager is unit-testable, plus a shared scheme with a test action.
daghaian
reviewed
Aug 20, 2026
| } | ||
|
|
||
| // A grace period is already running; keep its original deadline. | ||
| guard scheduled else { return } |
There was a problem hiding this comment.
Could we add a log here for future troubleshooting?
guard scheduled else { logDeviceCommunication("Suspected session end during active grace period; original deadline unchanged.", type: .connection) return }
|
This is a great contribution @cheets. I have been experiencing these same nuances myself with G7 so thank you for taking the time to look into this. One question I do have (which I think we can safely assume is very low probability), is how your code is handling a situation where at the 15 minute mark, you receive a glucose reading. As it stands, there is the possibility, however finite, that if a glucose reading comes in at 15 minutes, the work item has already been dispatched, and a new sensor scan is going to be triggered. Are we alright with acknowledging this as a low-probability edge case? |
A reading arriving as the grace timer fires could still trigger a spurious sensor scan: the work item is already dispatched and can no longer be cancelled. Track the last received message time and skip the scan if any communication arrived after the grace period began. Also log when a suspected session end occurs during an active grace period, and when an expiry is skipped due to resumed communication.
Author
|
I looked at the grace period and made a fix for it. Expiry now re-checks last communication time and aborts the scan so the race window shrinks from timer-granularity to microseconds. The worst residual outcome equals pre-fix behavior -> rediscovery next cycle. I also added the guard log. |
Contributor
|
Thanks! I think this looks good. On small observation. Not a blocker. The pending grace period doesn't survive app termination. Today's forget is A cheap guard would be a check when the manager is restored from |
ps2
added a commit
that referenced
this pull request
Aug 29, 2026
* fix: defer sensor forget on suspected session end A remote disconnect before authentication completes is treated as a session end, immediately forgetting the sensor and scanning from scratch. The same disconnect signature occurs on transient BLE handshake failures (auth notification timeouts, encryption failures), where forgetting the tracked peripheral downgrades reconnection from an OS-level pending connect to a throttled background scan, causing 10-40 minute glucose outages. Field logs showed 118 suspected session ends in 7 days of which 1 was a real session end. Keep tracking the sensor on a suspected session end and defer the scan-for-new-sensor by a 15 minute wall-clock grace period, cancelled when any glucose or backfill message arrives. A real session end still switches sensors: the stopped sensor stays silent, the grace period expires, and the new sensor is discovered during its warmup. Immediate switch on sensorFailed/sessionEnded algorithm states is unchanged. Also add DI seams (central manager factory, injectable bluetooth manager, internal manager init) so G7CGMManager is unit-testable, plus a shared scheme with a test action. * fix: re-check for comms when session end grace period expires A reading arriving as the grace timer fires could still trigger a spurious sensor scan: the work item is already dispatched and can no longer be cancelled. Track the last received message time and skip the scan if any communication arrived after the grace period began. Also log when a suspected session end occurs during an active grace period, and when an expiry is skipped due to resumed communication. * Persist the suspected session end so a grace period survives termination The deferred scan is an in-memory DispatchWorkItem, so an app killed inside the 15-minute window loses it and nothing re-arms it on launch. For a session that genuinely ended, that leaves the manager tracking a sensor which will never advertise again -- the user has to scan manually. The behaviour it replaced forgot the sensor synchronously, so it always happened. Record the grace start in G7CGMManagerState and re-establish the deferral when the manager is constructed: - A reading timestamped after the grace start proves the session survived; clear the marker and keep the sensor. - A window that elapsed while we were not running, with no reading since, forgets the sensor and scans, as it would have done live. - A window still open re-arms for the remaining time. The marker is cleared whenever the deferral is cancelled, guarded on it being set, since that path runs for every glucose and backfill message and mutateState notifies observers and persists. The restore runs from the shared init(state:sensor:) rather than the rawState initialiser, so it is covered by the existing bluetooth seam; constructing through the public rawState path builds a real CBCentralManager with a restore identifier, which throws in a test bundle. Six tests over the restore and persistence paths. G7SensorKitTests green at 27, and the workspace builds. * Drive the grace period from persisted state instead of a work item With the grace start recorded in G7CGMManagerState, the DispatchWorkItem machinery is redundant. suspectedSessionEndAt already says whether a grace period is running and identifies which one, so: - Cancellation is clearing the marker. A pending expiry re-reads it, finds a grace start that is no longer current, and does nothing. - Double-scheduling is prevented by the marker being non-nil rather than by a work item slot and a `scheduled` flag. - The race between an arriving reading and an already-dispatched expiry is covered by the same identity check, so the separate lastSensorCommsDate is no longer needed. - Restore after termination schedules through the same path with the remaining time, instead of duplicating the scheduling logic. Removes two Locked members and the work item lifecycle; the timer is now a plain asyncAfter. An uncancelled closure lives at most one grace period and no-ops when it fires. A timer is still required: when a session genuinely ends the sensor stops advertising, so no reading, disconnect or other callback would ever re-evaluate. Two tests invoked the expiry with a fabricated graceStart, which the identity check now correctly ignores; they pass the recorded value. Added a test that a superseded expiry does not forget the sensor. 28 tests green, workspace builds. * Log the authentication handshake at .default so it reaches sysdiagnose iOS keeps info and debug os_log entries in a memory ring buffer and does not write them to the log archive, so none of the authentication path is visible in a sysdiagnose. Investigating a false session end, the archive showed 26 connects and 74 control responses but not one line about auth, which is indistinguishable from the handshake never happening. pendingAuth is what decides whether a disconnect is treated as a session end, so whether the gate fires on every connection or only after pairing is exactly the question field diagnostics need to answer, and today they cannot. Promoted to .default: - "Listening for authentication responses" (was .info) -- the gate armed - "Observed authenticated session" (was .debug) -- the gate fired - "Ignoring authentication response" (was .debug) -- a response arrived but was not bonded/authenticated, which is the interesting failure to tell apart from silence - "Listening for backfill responses" (was .debug) -- shows whether the subscribe happens per connection Diagnostics only; no behaviour change. --------- Co-authored-by: Henri Koskenranta <henri.koskenranta@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When a G7 disconnects before authentication completes,
G7SensorreportssuspectedEndOfSession=trueandG7CGMManagerimmediately forgets the sensor (sensorID = nil, peripheral forgotten) and starts scanning for a new one.The same disconnect signature is produced by ordinary transient BLE handshake failures — auth notification enable timeouts,
unknownCharacteristicduring incomplete service discovery,CBErrorencryption failures. In a week of device logs I collected, 118 suspected session ends fired, of which exactly 1 was a real session end. Each false positive forgets the tracked peripheral, which downgrades reconnection from an OS-level pending connect (instant wake when the sensor next transmits) to a throttled background scan plus full re-discovery. Result: recurring 10–40 minute glucose outages ("Searching for sensor"), during which Loop cannot dose. Worst observed: 45 minutes.Change
On a suspected session end, keep tracking the current sensor and defer
scanForNewSensor()by a 15-minute wall-clock grace period:sensorFailed/sessionEndedalgorithm states is unchanged — a sensor that announces its own session end still triggers an instant scan.The grace period uses a wall-clock deadline so device sleep cannot postpone detection of a genuinely ended session.
Also adds unit tests for the new behavior, with small DI seams to make
G7CGMManagertestable (central-manager factory to avoid the state-restoration exception in test bundles, injectableG7BluetoothManager, internal manager init), and a shared scheme with a test action.Field results
I have been running this for a month on my own Loop with no issues. From exported device logs:
A real sensor swap during the trial was handled by the unchanged
sessionEndedmessage path: old sensor forgotten immediately, new sensor discovered 3 minutes after activation, during warmup.Notes
This overlaps in intent with the
scanning-fixbranch ("Continue with sensor even after auth without control msg", "Use remote disconnect without auth/data as end-of-session detection again") — same underlying observation that auth-less remote disconnects are unreliable as a session-end signal. This change keeps the existing detection but makes acting on it tolerant to transient failures. Happy to adapt if maintainers prefer thescanning-fixdirection.