Fix pod pairing retries: double completion and a discovery Timer that never fires - #116
Conversation
OverviewIn order to test this - we have 3 open branches in OmnipodKit to consider:
We need to confirm the changes in this PR apply to the Evaluation of other branchesDownload the patch for PR 116 and use git apply to see if this applies cleanly for the The only conflict is the logging change in OmnipodKit/Bluetooth/BluetoothManager.swift that does not apply for those two branches. The patch was modified to remove that change and then it applies cleanly to those two branches. Test Plan
|
Test 1 (from Test Plan)✅ successful test ConfigurationLoopWorkspace branch
Build onto SE 2nd gen phone running iOS 18.
|
Test 2 (from Test Plan)✅ successful test of new BLE method using LoopWorkspace 3.14.4 (not next-dev) with modification from this PR ConfigurationLoopWorkspace branch
Test with rPi DASHBuild onto SE 2nd gen phone running iOS 18.
Pair with actual Omnipod 5
Xcode Debug logThis log contains the rPi DASH plus Omnipod 5 pairing with the new code from PR 116 using the new BLE heartbeat Green Loop with Locked Phone✅ Unlock the phone and observe that app is looping while locked with a CGM that does not have a heartbeat. Leave the Omnipod 5 connected to this test phone with this configuration to get longer term log files. |
Test 3 (from Test Plan)✅ successful test of next-dev (v3.15.1) with modification from this PR ConfigurationLoopWorkspace branch
Build onto SE 3rd gen phone running iOS 26.5.2.
Xcode debug logThe debug logs from before the app hung just before insertion and after phone reboot, rebuild and successful insertion are in this zip file. Green Loop with Locked Phone✅ Unlock the phone and observe that app is looping while locked with a CGM that does not have a heartbeat. Leave the Omnipod Atlas DASH connected to this test phone with this configuration to get longer term log files with the phone locked. Testing MistakeIt does not affect what we are testing, but I did forget to the check the state of Pod Keep Alive. Since this is an SE phone that does not need the PKA running, disable PKA at 13:15 PDT. Network issuesThe phone was also not picking up the Nightscout site. Not getting CGM values. And I was seeing a lot of "network error" in the xcode log with upload failures.
Lock the phone and leave it alone for longer term testing. |
marionbarker
left a comment
marionbarker
left a comment
There was a problem hiding this comment.
Approve by test.
Applied this modification to 3 different versions of OmnipodKit with successful tests of each of the following:
- original BLE method tested with LoopWorkspace "dev" branch, using rPi simulator
- backport modified BLE-heartbeat method to LoopWorkspace "dev" like branch
- pair actual Omnipod 5 Pod
- next-dev modified BLE-heartbeat tested with LoopWorkspace next-dev branch
- pair actual Omnipod Atlas DASH Pod
Three related fixes in the DASH/O5 pairing path:
- pairAndPrime: a failed connectToNewPod invoked the caller completion
twice (completion + completionFailure). The first call schedules the
pairing UI's silent auto-retry; the second immediately resets the
auto-retry latch and shows the error, leaving a hidden retry running
concurrently with the user's manual Retry. OmniBLE calls the
completion once here; the second call crept in during the OmnipodKit
rewrite.
- connectToNewPod: the discovery polling Timer was scheduled on the
calling thread. On a thread with no running run loop (the pairing
auto-retry path) the timer never fires, so that discovery attempt
never completes, never times out, and never calls endPodDiscovery,
leaving discovery mode enabled and the scan running. Schedule the
timer on the main run loop, and return after each terminal condition
so a single tick cannot invoke the completion twice.
- PairPodViewModel: perform the pairing auto-retry via
DispatchQueue.main.asyncAfter instead of Thread.sleep on a utility
QoS thread, so the retry attempt runs with a live run loop.
Also fixes a malformed %{public} os_log format specifier in
BluetoothManager.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
marionbarker
left a comment
marionbarker
left a comment
There was a problem hiding this comment.
Approve again. Thanks for removing the comments in OmnipodKit/PumpManager/OmniPumpManager.swift.
itsmojo
left a comment
itsmojo
left a comment
There was a problem hiding this comment.
Looks good! Thanks to @scrappy for tracking this stuff down and to @marionbarker for all her fabulous testing & git management work.
|
Thanks for your input @scrappy. These changes were merged into |
4 participants
Fixes #115.
Three small changes in the DASH/O5 pairing path, plus a format-specifier fix:
OmniPumpManager.pairAndPrime— on theconnectToNewPodfailure path, delete the extracompletion(...)socompletionFailure(...)is the only completion call, per your comment on the issue. This stops the second invocation from resetting the pairing UI's auto-retry latch and leaving a hidden retry running alongside the user's manual Retry.BlePodComms.connectToNewPod— schedule the discovery pollingTimeron the main run loop. The completion fordiscoverPodscan run on a caller thread with no active run loop, where a timer scheduled on the current thread never fires and that discovery attempt hangs without ever timing out or callingendPodDiscovery(). Alsoreturnafter each terminal branch so a single tick can't invoke the completion twice, and passtargetPodto the success completion instead of re-readingdevices.first!.PairPodViewModel— perform the pairing auto-retry viaDispatchQueue.main.asyncAfterinstead ofThread.sleepon a utility-QoS worker, so the retry attempt runs with a live run loop regardless of the fix above.BluetoothManager— malformed%{public}→%{public}@in one os_log call.Status: I'm running these fixes on my own phone now (cherry-picked onto the current iAPS submodule pin). The first two are straightforward to confirm from the code; my next pod change is the real test of the retry path — I'll follow up here with the result either way.