Repository navigation
Revamped Pod Keep Alive OmnipodKit implementation - #125
Merged
marionbarker merged 2 commits intoAug 23, 2026
Merged
Conversation
Collaborator
Author
Collaborator
Author
|
Also removed two AI generated .md files that were left at the top level. |
ps2
self-requested a review
August 23, 2026 19:44
ps2
approved these changes
Aug 23, 2026
ps2
added a commit
that referenced
this pull request
Aug 24, 2026
… title #134) Two conflicts, both straight combinations: - shouldHoldConnection: keep the eager-pod hold, use #125's podKeepAliveKeepsConnectedInBackground accessor for the PKA case. - Pairing keep-alive auto-enable: keep UIDevice.hasPossibleInPlayBLEIssues (the consolidated predicate) with #125's state.podKeepAlive storage. #125 still referenced the iPhoneWithPossibleInPlayIssues var removed on this branch, so this resolution is required for the merged tree to compile.
marionbarker
pushed a commit
that referenced
this pull request
Sep 1, 2026
ps2
added a commit
that referenced
this pull request
Sep 4, 2026
Backport of #133 from next-dev, squashed. Mitigates BLE connection wedges on InPlay-firmware DASH pods paired with an iPhone 16 (any variant) or 17e: a connect that stalls with no callback is torn down and retried rather than left to time out, and the pod settings view gains a persistent notice explaining the slower connections. Also carries the later fixes from that branch: the O5 fresh-advert connect, which cut foreground connects on Omnipod 5 from about six seconds to under one; the foreground-state seed, so a pump manager built after the app has already become active does not idle-disconnect between commands; and the test-beeps button no longer gating on hasConnection. Two conflicts against this line, both because dev predates the Pod Keep Alive rework (#125). Resolved by keeping dev's accessor and taking the eager-connect logic: - shouldHoldConnection keeps podType.isDash && Storage.shared.podKeepAlive.value.keepsPodConnectedInBackground rather than podKeepAliveKeepsConnectedInBackground - the pairing auto-enable keeps Storage.shared.podKeepAlive.value while adopting the consolidated UIDevice.hasPossibleInPlayBLEIssues Does not build against LoopWorkspace/dev as it stands, for reasons that predate this change -- see the pull request.
ps2
added a commit
that referenced
this pull request
Sep 15, 2026
* Eager connect watchdog: mitigate InPlay/iPhone-16 connection wedges Backport of #133 from next-dev, squashed. Mitigates BLE connection wedges on InPlay-firmware DASH pods paired with an iPhone 16 (any variant) or 17e: a connect that stalls with no callback is torn down and retried rather than left to time out, and the pod settings view gains a persistent notice explaining the slower connections. Also carries the later fixes from that branch: the O5 fresh-advert connect, which cut foreground connects on Omnipod 5 from about six seconds to under one; the foreground-state seed, so a pump manager built after the app has already become active does not idle-disconnect between commands; and the test-beeps button no longer gating on hasConnection. Two conflicts against this line, both because dev predates the Pod Keep Alive rework (#125). Resolved by keeping dev's accessor and taking the eager-connect logic: - shouldHoldConnection keeps podType.isDash && Storage.shared.podKeepAlive.value.keepsPodConnectedInBackground rather than podKeepAliveKeepsConnectedInBackground - the pairing auto-enable keeps Storage.shared.podKeepAlive.value while adopting the consolidated UIDevice.hasPossibleInPlayBLEIssues Does not build against LoopWorkspace/dev as it stands, for reasons that predate this change -- see the pull request. * Provide the BLE heartbeat for DASH pods not using a RileyLink setBLEHeartbeatRequest branched on podType.mayUseRileyLink, which is true for DASH as well as Eros because DASH can use a RileyLink under the Pod Keep Alive option. A DASH pod on direct BLE with Pod Keep Alive off therefore took the RileyLink branch and only set timerTickEnabled on a RileyLink provider that has no devices. provideHeartbeat was never assigned and BlePodComms.setHeartbeatRequest was never called, so the delayed-connect probe that produces periodic background wakes was never scheduled. issueHeartbeatIfNeeded() also gates on provideHeartbeat, so even a probe that did fire would not have notified Loop. Loop was requesting the heartbeat correctly -- DeviceDataManager sets pumpManagerMustProvideBLEHeartbeat when the CGM cannot provide one -- and the request was logged and then dropped. With no wake source the app stayed suspended in the background until something else woke it. Seen as Loop Failure alerts after gaps of 20 to 264 minutes that end the instant the app is foregrounded; a report from an affected DASH pod shows provideHeartbeat: false moments after a setBLEHeartbeatRequest log line. Only reachable when the CGM cannot supply the heartbeat itself, so it needs a remote or networked CGM such as Nightscout. A BLE Dexcom provides its own heartbeat and masks it. Branch on whether a RileyLink is actually in use instead: Eros always, DASH only under the RileyLink Pod Keep Alive option. The other mayUseRileyLink call sites want the "may" semantics and are unchanged. * Do not cancel a pairing connect we just issued as a zombie The zombie check in didDiscover treats a pairable pod heard while .connecting with no watchdog as a stale connect and cancels it. That is right for a connect abandoned by an earlier attempt, but it cannot tell that case from a connect we issued moments ago: a peripheral keeps advertising until the link is actually established, and CoreBluetooth reports .connecting from the instant connect() is called, so an advert inside the first second or two is the normal case. discoverPods pre-connects any already-known disconnected pod and then starts the discovery scan. timedConnect only arms the watchdog when shouldUseEagerConnect is true, which needs an affected iPhone model, so on any other phone that pre-connect is unsupervised. The scan hears the pod ~0.8s later at state=.connecting, the zombie branch cancels it, the teardown timer reissues timedConnect (again unsupervised), the next advert ~1.5s later cancels that, and pairing loops at the advert interval without ever completing. Two Xcode captures from an iPhone SE pairing an Omnipod 5 show 62 and 104 consecutive cycles of [pairing] heard pod ... pairable=true state=1 [pairing] ... stuck in .connecting with no watchdog -- cancelling zombie connect [pairing] reconnecting to pairable pod ... after zombie teardown Pairing with a pod that is not yet a known device takes the state=0 path and races the same window: it worked in earlier testing because the ~1.1s connect landed before the next advert. Stamp when the current pairing connect was issued and only treat a .connecting pod as a zombie once that connect is older than pairingConnectZombieSeconds (default 5s, past a healthy connect and inside the pairing budget). The stamp is separate from connectRequestedAt, which is deliberately kept across retries for the latency metric, and is cleared on didConnect and didFailToConnect. * Cancel stale pairing connects deterministically; drop the zombie heuristic A "zombie" was a pending connect left behind by a pairing attempt that gave up without cancelling. The didDiscover branch tried to detect one from an advert heard while the pod was .connecting, but that is equally true of a connect issued a second ago, and on phones where the watchdog does not arm it cancelled every in-flight pairing connect and looped (see 5a32565, which only narrowed the misfire). Remove the guess and close the source instead: - endPodDiscovery cancels any .connecting peripheral. The previous autoConnectIDs exemption is what leaked the orphans: discovery adds the pod to autoConnectIDs before ending, so any later abandoned connect to a pod that had once been found was never cancelled. Only a .connected paired pod keeps the exemption. - discoverPods cancels any unsupervised .connecting peripheral before issuing its own connects, so during discovery the only pending connect is ours and the discovery timeout owns it. This also covers a connect left pending across an app suspend or kill, which comes back through state restoration already .connecting. didDiscover goes back to connecting only when .disconnected. The pairingConnectIssuedAt stamp and pairingConnectZombieSeconds from 5a32565 go with the heuristic. * Simplify rileyLinkIsInUse
jeremybarnum
pushed a commit
to jeremybarnum/OmnipodKit
that referenced
this pull request
Sep 16, 2026
Lean-out step 1: onto Pete's current next-dev. Ten upstream commits (loopandlearn#119, loopandlearn#125 PodKeepAlive/SilentTune revamp, loopandlearn#133 HostAppState + eager-connect watchdog, loopandlearn#134, loopandlearn#136, loopandlearn#138, loopandlearn#139, loopandlearn#140, loopandlearn#147, loopandlearn#155). Fourteen conflict hunks, resolved to keep his code wherever ours modified the same lines: - lifecycle goes through his HostAppState (its watchOS branch compiles now; our WatchKit import and notification mapping are gone) - his fresh-discovery connect (hoisted out of the DASH branch) replaces ours, which drops our connect-intent guard from that path - his handleDisconnect/iOS-17 variant; our loan disconnect stamp moves into it - his keep-alive tail in shouldHoldConnection, under our watchOS early return - his UIDevice.hasPossibleInPlayBLEIssues and state.podKeepAlive replace our iPhoneWithPossibleInPlayIssues; a watch-only UIDevice stand-in (Common/UIDevice+watchOS.swift) keeps the watch target compiling, since his UIDevice.swift is excluded there - our pod-adoption block in didDiscover and the adoption-marker log in didFailToConnect stay, ahead of his pairing and watchdog code - SilentTune (depends on PumpManagerUI) and silent.wav excluded from the watch target; the audio keep-alive stays behind #if os(iOS) - InPlayConnectionInfoView (his new UI file) excluded from the watch target Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.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
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.
Background
The original Pod Keep Alive implementation was an adaptation of code from the
LoopFollowapp intended as a temporary bandaid measure for the Atlas pods when used with iPhone 16s and 17e'e that can have significant BLE reconnect delay times resulting in long periods of pod communication losses. Unforunately no iOS/iPhone or Insulet solutions for this reconnect problems between these devices has happened and no better software workarounds have been found in over a year. Now that that PKA is part of the unified OmnipodKit pump manager, it is due for some long overdue updates and a more viable long term implementation.Some issues of the original PKA implmementation include:
OmnipodKit.Pump Manager Detailsor Loop Reports (since it's not part of the state being held inOmniPumpManagerState).Switch pump typeis used to switch to a different pump type,OmnipodKitwill still be held in memory.Switch pump typeis used to switch to a different pump type after the DASH pod type has been selected,RileyLinkBLEKitwill also still be held in memory.RileyLinkPKA type is selected and a RileyLink is connected, then no RileyLinks will be available when switching the pump manager fromAll Omnipod Types(OmnipodKit) toMinimed(MinimedKit). This will also occur if the PKA type has been changed back toDisabledbefore switching pump types.LoopFollowbased PKA implementation.OmnipodKitfor both BLE Omnipod pod types.New Implementation
This new PKA implementation is completely revamped and fixes all of the above listed issues. The uses of Storage()/StrorageValue() classes for remembering persistent state have been eliminated and replaced by a single
PodKeepAlivevalue kept inOmniPumpManageStatewhich is managed just like any other pump manager state. The various logical pieces have been split into multiple source files that make more sense. While this new PKA implementation doesn't (currently at least) support a RileyLink option to do BLE based wakeups for PKA, it does provide a framework for doing this in a way that would integrate better withOmnipodKitwhich already has its own RileyLink mechanisms included for use with older Eros pods.Other highlights of the new PKA implementation include:
OmnipodKitorRileyLinkBLEKit.All Omnipod Types(OmnipodKit) toMinimed(MinimedKit).Testing Notes
To verify that
OmnipodKitandRileyLinkBLEKitare no longer being held in memory, it is necessary to run Loop under Xcode. After switching out from theOmnipodKitpump manager to the main page showingAdd Pump, tap on the XcodeDebug Memory Graphicon (3 connected dots pointing to the right) in the diagnostic visual debugger section of the debug bar. In the older PKA implemenation, there will be several held references forOmnipodKit. If anOmnipod DASHpod type was selected before switching out, there will be even more held references toOmnipodKitand there will be at least one held reference toRileyLinkBLEKit. With this updated PKA implementation, there should be no held references to eitherOmnipodKitorRileyLinkBLEKitafter switching out from theOmnipodKit(All Omnipod Types) pump manager no matter which pod type or PKA option is selected as should be expected.The Silent Tune option now plays the
silent.wavfile found in the OmnipodKit/Resources directory which is a copy of the still includedblank.wavfile in the same directory. For testing purposes, thesilent.wavfile can be easily overwritten by the contents of theheartbeat.wavfile for a heartbeat sound that can be heard with Silent Tune PKA option is selected and the app is not in the foreground. The app can be easily restored to its default non-test configuration by coping theblank.wavfile back tosilent.wav.Other issues cited for the poor code layout and incorrect descriptions can be verified by examination of the old vs new implementations in the source code and by examination of the screen views.
Possible Future Work
With the unified Omnipod pump manager that includes all supported Omnipod pod types,
RileyLinkBLEKitis always linked withOmnipodKit. If a RileyLink PKA option is eventually re-implemented forOmnipodKit, it should use the existingRileyLinkBLEKitcode that is already included for the Eros pod type instead of using any code from the orignal PKA implementation which has a conflicting and overly messy standalone BLE and RileyLink implementation that was originally adapted fromLoopFollow.