Skip to content

G7 End of session detection bugfix #34

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Apr 14, 2025
Merged

G7 End of session detection bugfix #34

merged 7 commits into from
Apr 14, 2025

Conversation

ps2
Copy link
Contributor

@ps2 ps2 commented Mar 22, 2025

Attempt at fixing LoopKit/Loop#2286 (and maybe LoopKit/Loop#2278).

This change listens for connectionEventDidOccur, like #33, adds additional end of session detection based on algorithm state decodeing.

@marionbarker
Copy link
Contributor

Tests

Initial test on "working" phone looks good.

Test on phone with no issues

I built this onto my personal Looping phone

  • I have no history of missing G7 data with this phone

Configuration

  • iPhone 15 pro running iOS 18.3.2, customized version of 3.4.4

Test Results

  • Glucose continues to be displayed on main screen with this version of G7SensorKit
    • observe that values update over next 2 cycles
  • quit the Dexcom app
  • Remove CGM from Loop
  • Add a G6 just to make sure G7 vestiges are removed, enter XXXXXX as transmitter ID, remove Share credentials from G6, wait for one more cycle
    • tapping on glucose took me to Dexcom G6 app (which is not running)
    • Delete G6 as CGM
  • Add G7 back as CGM
    • Get "Searching for Sensor" icon
    • Restart the G7 app (it is stale, shows 102 from from 10:16, currently 10:25)
    • Updated value to 103 on G7 app, check Loop, it shows 103.

@motinis
Copy link

motinis commented Mar 22, 2025

Hopefully this works - although if it does, it makes me think that in the scan-fix the backup handling for connecting of the sensor could have just relied on the SensorServiceUUID.cgmService.cbUUID and not the advertisementService as well (?).

@dnzxy
Copy link

dnzxy commented Mar 31, 2025

@ps2 Could you please share some insights on the issue you have identified (for the two linked Loop issues) and how the proposed fix here is attempting to fix it. Just from the description alone, it is highlighting possible limitations of the proposed solution while not really going into detail what the identified, underlying problem seems to be or what the proposed solution aims to solve; the code change itself just shows that a guard is being extended by a time-based guarding component and more recently added logging.
Thank you.

@ps2
Copy link
Contributor Author

ps2 commented Apr 1, 2025

@dnzxy The code checking for a connect/disconnect with no glucose reading was attempting to detect end-of-session, as that's what we see after a session end: the sensor continues to connect and disconnect, but no data is sent (probably because the g7 app is not asking for any). It seems that recently, perhaps due to iOS changes, sometimes we don't get the data during a connection period, and so this code was triggering prematurely.

So I've changed it to not trigger prematurely, but this means that we're left without a way to detect early end of session.

The recent commits to this PR are attempts at adding back automatic detection of early end of session due to manual ends or sensor errors, so we can start scanning for the new sensor proactively.

@ps2 ps2 changed the title Continue with sensor even after auth without control msg G7 End of session detection bugfix Apr 1, 2025
@marionbarker
Copy link
Contributor

marionbarker commented Apr 12, 2025

Status

This looks like a good fix to me.
I don't know if we want to keep all the logging.

Tests

There seemed to be 2 kinds of failures where Loop stops picking up G7 sensor data, sometimes for hours at a time.

  • The errors only happen to some people
  • The symptom is intermittent interruption in Loop pulling in G7 data (while G7 app is working)
  • Most people reported earlier tests with this PR improved their connectivity: see #development > Loop stops receiving G7 data, but G7 app is ok
  • One individual reported that whenever she quit and restarted Loop, it could take hours for Loop to start picking up the G7 data again
  • One of the early changes in this PR meant that Loop did not automatically scan for a new sensor when the old one finished early see #development > G7SensorKit PR tests @ 💬
  • The most recent commit "fixed" the problem of Loop not automatically picking up the new sensor.

Test Sensor Swap

A new sensor was picked up by Loop 5 minutes after the Dexcom app paired with it with no action required by the user.

Test Details

I used a technique where I make the G7 App think a Sensor is stopped without interrupting the Sensor Session.

  • Left arm sensor is DXCMPa
  • Right arm sensor is DXCMTm

Begin with G7 app and Loop reading glucose from DXCMPa, Loop is using the scanning-fix branch of G7SensorKit, SHA b0d424d

16:41 forget DXCMPa, disable Bluetooth
16:42 stop sensor in G7 app, and quit app
16:43 enable Bluetooth
16:43 start Dexcom G7 app and pair to code for new sensor
16:44 pair and read in G7 app, DXCMTm is device name
16:44 Loop shows glucose from DXCMPa sensor at 16:40, Scan new is an option, but don't choose it
16:49 Dexcom reads glucose from DXCMTm, Loop picks up new sensor

Loop Report is attached.

Sysdiagnose link sent to @ps2 via private message.

@marionbarker marionbarker self-requested a review April 12, 2025 00:22
Copy link
Contributor

@marionbarker marionbarker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One question is how much logging to leave turned on.
The method appears to work.
LGTM

@motinis
Copy link

motinis commented Apr 12, 2025

@ps2 since you've returned in the last commit the pendingAuth for end of session detection won't this likely go back to roughly the state of PR33 (minus the additional delay when it resumes scanning)? I think that things were working better without the end of session detection (and having people manually rescan is not a huge deal)

@ps2
Copy link
Contributor Author

ps2 commented Apr 12, 2025

@motinis The noAuth/noData detection for end of session is not something I love; I added it initially because we had no other way of detecting end of session. We have seen some evidence that we can detect end of session in the protocol, at least for some sensors/phones, and I've added that, but we see some phone/sensor combos don't give us those signals. So I've added the hacky end-of-session detection back.

The other issue (that PR #33 did actually fix, I think), is that some iOS versions we're not seeing the didDiscover callback when scanning. I think this is probably a bug in iOS. That combined with the hacky eos detection meant sometimes sensors would get dropped and not reconnected to. Adding listening for connectionEventDidOccur signals as a fallback should hopefully help with that.

@marionbarker
Copy link
Contributor

marionbarker commented Apr 12, 2025

More tests

These tests are from Katie - she installed G7SensorKit, PR 34, SHA b0d424d at 2025-04-12 16:49:13 +0000 (09:49 PDT).

She performed a successful quit / restart test after a glucose reading came in 14:50 PDT (21:50 UTC) and restarted at:

  • 2025-04-12 21:56:56 +0000
  • Loop picked up the next reading at
    • 2025-04-12 22:00:37 +0000

New Issue

This may be a new issue with values and timestamps for G7BackfillMessage - or it may be related to an Issue previously reported, see LoopKit/Loop#2291 - or it may be a response to a Dexcom calibration by the user.

I noticed several times in Katie's NS where it looked like 2 readings were plotted at the same (or very similar) timestamps.
I found all of these instances in her log, see graphic for 2 cases and table for all 3 cases.

Katie-offset-readings-same-time

PDT UTC didRead didReadBackfill Backfill time
2025-04-10 18:50 2025-04-11 01:50:35 123 (read & backfill) 133 01:55:37
2025-04-12 10:50 2025-04-12 17:50:40 183 (read & backfill) 164 17:55:41
2025-04-12 11:35 2025-04-12 18:35:39 162 (read & backfill) 155 18:40:38

This is an excerpt of log messages for the last row of the table above. I removed the repeated +0000 G7CGMManager DXCMRy found in each line. The full log is attached below.

* 2025-04-12 18:35:39 connection Sensor connected
* 2025-04-12 18:35:39 receive Sensor didRead G7GlucoseMessage(glucose:Optional(162), sequence:552 glucoseIsDisplayOnly:false state:ok messageTimestamp:162563 age:6, data:4e00037b0200280200010600a20006fca1000f)
* 2025-04-12 18:35:40 receive Sensor comms 320001890000008b000e70020003010544710200
* 2025-04-12 18:35:40 receive Sensor comms ea0003030102000000431400007fffffff
* 2025-04-12 18:35:40 connection Sensor disconnected: suspectedEndOfSession=false
* 2025-04-12 18:40:36 connection Sensor connected
* 2025-04-12 18:40:38 receive Sensor didRead G7GlucoseMessage(glucose:Optional(150), sequence:554 glucoseIsDisplayOnly:false state:ok messageTimestamp:162861 age:4, data:4e002d7c02002a0200010400960006fb91000f)
* 2025-04-12 18:40:38 receive Sensor comms 320001890000008b002a7a0200020105587b0200
* 2025-04-12 18:40:38 receive Sensor comms ea0003020103000000450c00007fffffff
* 2025-04-12 18:40:38 receive Sensor didReadBackfill G7BackfillMessage(glucose:Optional(162), glucoseIsDisplayOnly:false timestamp:162557, data:fd7a0200a200060ffc)
* 2025-04-12 18:40:38 receive Sensor didReadBackfill G7BackfillMessage(glucose:Optional(155), glucoseIsDisplayOnly:false timestamp:162565, data:057b02009b00061ffc)
* 2025-04-12 18:40:38 receive Sensor didReadBackfill G7BackfillMessage(glucose:Optional(150), glucoseIsDisplayOnly:false timestamp:162857, data:297c02009600060ffb)
* 2025-04-12 18:40:38 connection Sensor disconnected: suspectedEndOfSession=false
* 2025-04-12 18:45:42 connection Sensor connected
* 2025-04-12 18:45:43 receive Sensor didRead G7GlucoseMessage(glucose:Optional(142), sequence:555 glucoseIsDisplayOnly:false state:ok messageTimestamp:163166 age:9, data:4e005e7d02002b02000109008e0006f884000f)
* 2025-04-12 18:45:43 receive Sensor comms 320001890000008b002a7a0200030105587b0200
* 2025-04-12 18:45:43 receive Sensor comms ea0003010302000000621400007fffffff
* 2025-04-12 18:45:43 connection Sensor disconnected: suspectedEndOfSession=false

Loop Report 2025-04-12 150232-0700.md

@marionbarker
Copy link
Contributor

Testing Continued

This is from Katie again.
There was a drop-out at 07:50 PDT, then one reading, then another gap, then Loop recovered. Katie said the phone was near her. She sent a Loop report, which shows a launchDate: 2025-04-12 21:56:56 +0000; so this was not a quit/restart issue.

2025-04-13 14:45:41 Glucose reading, Sensor didRead followed by Sensor didReadBackfill
2025-04-13 14:50:45 Missed reading, got a timeout and `suspectedEndOfSession=true` (missing reading)
2025-04-13 14:55:41 Glucose reading, new connection with same sensor and didRead
gap in connections
2025-04-13 15:20:38 Glucose reading, connections resume

The graphic below is from Nightscout, 2025-04-12.

katie-ns-suspected-end-true-20250413-0750

Loop Report 2025-04-13 082219-0700.md

Screenshot of the G7 app on Katie's phone

IMG_5556

She will attempt to get a sysdiagnose. If successful, it will need to be uploaded later. I'll let @ps2 know if she is successful with the sysdiagnose.

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.

4 participants