-
Notifications
You must be signed in to change notification settings - Fork 15
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
Conversation
TestsInitial test on "working" phone looks good. Test on phone with no issuesI built this onto my personal Looping phone
Configuration
Test Results
|
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 |
@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. |
@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. |
StatusThis looks like a good fix to me. TestsThere seemed to be 2 kinds of failures where Loop stops picking up G7 sensor data, sometimes for hours at a time.
Test Sensor SwapA new sensor was picked up by Loop 5 minutes after the Dexcom app paired with it with no action required by the user. Test DetailsI used a technique where I make the G7 App think a Sensor is stopped without interrupting the Sensor Session.
Begin with G7 app and Loop reading glucose from DXCMPa, Loop is using the scanning-fix branch of G7SensorKit, SHA b0d424d
Loop Report is attached. Sysdiagnose link sent to @ps2 via private message. |
There was a problem hiding this 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
@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) |
@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 |
More testsThese tests are from Katie - she installed G7SensorKit, PR 34, SHA She performed a successful quit / restart test after a glucose reading came in 14:50 PDT (21:50 UTC) and restarted at:
New IssueThis may be a new issue with values and timestamps for I noticed several times in Katie's NS where it looked like 2 readings were plotted at the same (or very similar) timestamps.
This is an excerpt of log messages for the last row of the table above. I removed the repeated
|
Testing ContinuedThis is from Katie again.
The graphic below is from Nightscout, 2025-04-12. Loop Report 2025-04-13 082219-0700.md Screenshot of the G7 app on Katie's phone 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. |
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.