[ps2]

Relates to #199.

In the #199 traces, the first exchange after a fresh connect fails, and the pod hangs up 6–8 s after the connect (CBError 7, a remote close). There are two shapes:

• Omnipod 5 notReady about 2 s after connecting. Service or characteristic discovery timed out (2 s), so the pod's characteristics were never found and the session sent its command anyway. Nothing was written, but the command was logged as "Unacknowledged".
• DASH incorrectResponse 5 s after the send, then Pod disconnected … nil. The pod accepted the command at the Bluetooth level and never replied. The nil disconnect is OmnipodKit's own teardown after the read timeout.

The device log couldn't say which setup step failed, whether login succeeded, or who closed the link. This PR makes that visible and fixes the two code problems the traces show.

Changes

• Device-log lines for each connect and each failure.
  • [connect] configured in Ns: services …, characteristics …, write size …, hello …, notifications …, login …, or [connect] configuration failed after Ns: <error>; <steps so far>. Login errors used to be swallowed in completeConfiguration. They now reach this line, so a connect whose login failed isn't reported as configured. A fresh connection that skips setup is logged too.
  • [send] <error> at <stage>, sent / not sent / may have been received; peripheral …, central …, characteristics present/missing; <time since connect>, during <phase>, setup <outcome>.
  • [read] <error> at <RTS | data seq N | SUCCESS write | …> after Ns, disconnecting / link already down / sent NACK; <same context>. This replaces the earlier [connectOnDemand] no response from pod line.
  • [disconnect] on every disconnect. It gives the error domain and code, or for our own cancels the reason (idle, unresponsive pod, background, eager watchdog, probe wake, …), with the time since connect, the phase and the setup outcome.
  • The retry after a dropped link (link dropped after connecting) now says how setup went.
• No "unacknowledged" when nothing was sent. sendMessagePacket marks a message as possibly received before writing its last packet, and an Omnipod 5 command is a single packet. notReady is only thrown before a write is issued, so it now counts as unsent. The session gets a certain failure and no pending command to resolve.
• Unresponsive-pod teardown keeps keep-alive. The teardown goes through BluetoothManager.disconnectUnresponsive, on managerQueue, where the connect-on-demand state lives. When the link is held, the resulting didDisconnect reconnects, as before.
• The heartbeat probe re-arms after a drop when the link isn't held. handleDisconnect's decision is now DisconnectPolicy, which doesn't depend on CoreBluetooth and is unit tested.
  • A drop outside a command session clears commandConnectInFlight and arms the probe.
  • A drop during a session leaves the flag to the session. When the last session ends with the link down, the flag is cleared and the probe armed. This covers the pod hanging up mid-command, where readMessagePacket finds the link already down. Before, the flag stayed set and the background heartbeat stopped.
  • The session gate also keeps a cancelled probe's late didDisconnect from clearing the flag while a command connect is under way.
• Two log fixes. A connect that timed out is logged as [connect] not connected, setup not run, not as a services failure. A link iOS restored at launch is reported as "after restore", not "not connected".
• Removed the queue claims. Comments said a cancel from the PeripheralManager queue wedged the next connect, and nothing supports that. The earlier commits in this PR added [stuckConnect] logging to test it. Neither the Omnipod 5: Repeated "Peripheral Not Ready" errors after successful Bluetooth connection cause failed loops #199 traces nor the simulator tests point that way, so that logging is removed again, along with the remaining comments. The stuck-in-.connecting symptom is the InPlay link-layer wedge, which the eager watchdog handles.

On next-dev, a session is already refused when login hasn't completed on the connection (#201). That gate isn't on dev. There, the session still runs after a failed setup, and the unsent fix above applies.

Testing

• Built next-dev with this PR in the LoopWorkspace scheme (19 plugins). Ran it in the iOS Simulator against the pod simulator (Omnipod DASH, through ImpossiBLE).
• Failing connects (Add failNextConnects: leave service discovery unanswered, then hang up LoopKit/pod#29, failNextConnects: discovery unanswered, pod hangs up at 7 s with CBError 7):

  [disconnect] ours: background, 49.319s after connect, during idle, setup ok
  [connect] configuration failed after 2.021s: timeout([…discoverServices]); services failed after 2.018s
  [disconnect] CBErrorDomain 7, 7.005s after connect, during setup, setup failed at services
  [connectOnDemand] keep-alive — reconnecting after drop
  [connect] configured in 0.007s: services 0.002s, characteristics 0.000s, hello 0.000s, notifications 0.000s, login 0.004s
  

• Unresponsive pod (Add ignoreNextMessages: stay connected but leave messages unanswered LoopKit/pod#28, ignoreNextMessages), app open:

  [read] emptyValue at RTS after 5.010s, disconnecting; peripheral connected, central poweredOn, characteristics present; 70.942s after connect, during session Get detailed status, setup ok
  [disconnect] ours: unresponsive pod, 70.943s after connect, during session Get detailed status, setup ok
  [connectOnDemand] keep-alive — reconnecting after drop
  [connect] configured in 0.006s: …
  

• DisconnectPolicyTests: 13 tests. With c48aa64's policy put back, 4 fail. OmniTests: 202 run, 0 failures.
• Pod hangs up mid-command (Add hangUpOnNextMessages: hang up on a message, before or after answering LoopKit/pod#33, hangUpOnNextMessages), app in the background with Pod Keep Alive "when open":

  [disconnect] ours: command preempts probe, not connected, during connect, setup ok
  [disconnect] CBErrorDomain 7, 0.051s after connect, during session Get pod status, setup ok
  [read] emptyValue at RTS after 5.010s, link already down; …
  [connectOnDemand] connect
  [disconnect] ours: idle, 4.259s after connect, during idle, setup ok
  [delayedConnect] … issuing connect StartDelay=311s
  

  The stall itself isn't reproduced end to end. Loop's next cycle starts a session whose idle disconnect also clears the flag. Running this in the simulator needs Keep a connected peripheral's owner when a cancelled connect's disconnect arrives late mickeyl/ImpossiBLE#10.
• Earlier report corrected. I reported before that a reconnected link timed out in service discovery on every build. That was an ImpossiBLE bug: it dropped the peripheral when the app reconnected from its disconnect callback. It's fixed in Release a peripheral before telling the delegate it disconnected mickeyl/ImpossiBLE#8. With that fix, the reconnect after either teardown sets up normally, as above. CBError 7 in the simulator needs Carry the error domain and code on connection events mickeyl/ImpossiBLE#9.
• Not tested:
  • a real pod
  • the stall's exact timing (a drop after a cycle's last session, before the idle disconnect), which only the unit tests cover
  • a [send] failure from a failed setup: on next-dev, Refuse a pod session without a handshake; decode the LTK and BLE handle separately #201 refuses the session first, and I didn't build dev

[marionbarker]

Overview

I'm not competent to review this and I'm not sure how to test this solution.

I asked Claude instead. Here's the response:

Claude Review

The fix is sound, but it changes keep-alive behavior in a way I'd like confirmed before approving.

What checks out

• Queue-correct cancel. Moving the cancel to managerQueue via disconnectOnDemand matches the existing idle-disconnect path (PeripheralManager.swift:724). Its comment describes the same CoreBluetooth race.
• bluetoothManager is always wired. Every PeripheralManager that addPeripheral creates has bluetoothManager set, including during pairing (BluetoothManager.swift:696/702). So bluetoothManager? won't silently skip the disconnect where central? would have worked. The LTK-exchange reads during pairing are covered too.
• Ordering. managerQueue is serial, so a later session's beginCommandConnect can't run before the cancel and the flag reset.
• No in-session retry. Nothing in BleMessageTransport retries after incorrectResponse. The extra async hop before the cancel therefore doesn't let a retry write to the dying link.
• Config error names the step. PeripheralManagerError.timeout carries the CommandCondition list (service discovery, characteristic discovery, or notification state), as the description says.

Main concern: keep-alive no longer reconnects after this teardown (non-eager pods)

Clearing commandConnectInFlight lets the heartbeat probe re-arm. But didDisconnect also uses that flag to decide whether to reconnect while the connection should be held (BluetoothManager.swift:1714):

if shouldHoldConnection && commandConnectInFlight {
    // "a deliberate background/idle disconnect clears commandConnectInFlight first, so it won't reconnect"
    connectForCommand(peripheral)

Before this PR, an unresponsive-pod teardown in the foreground, or in the background with Pod Keep Alive, reconnected right away. Now it takes the idle path (fault scan + probe), and the link stays down until the next command connects on demand. Eager-connect pods aren't affected, because they reconnect earlier in didDisconnect regardless of the flag.

Until now disconnectOnDemand was only called from the idle disconnect, which already returns early under keep-alive, and from an unfinished connect. This is the first call site that can run while keep-alive is holding the connection. The comment at line 1716 also assumes only a deliberate disconnect clears the flag.

Suggestion: either clear the flag only when !shouldHoldConnection, or add a parameter to disconnectOnDemand that keeps it. If the new behavior is intended, please say so in the description and update the line 1716 comment.

Minor

• The flag is cleared while a session may still be counted in hasActiveCommandSession. If more sessions are queued, the probe can re-arm and then be cancelled by the next command connect ("command preempts heartbeat probe"). That path is handled, but it adds log churn.
• The new device events ("No response from pod…", "Configured in…") have no tag like the existing [connectOnDemand] / [eager] lines. A tag would make them easier to grep in issue reports.
• One new line uses String(format:) with %@ and the other uses interpolation. Both work on Darwin; this is just a consistency nit.

Testing

There are no new tests, and the PR notes it hasn't been run against a pod. Before merging, please test on a pod with Pod Keep Alive enabled and a forced read timeout, to confirm the reconnect behavior above.

Recommendation: request changes, or at least get an answer on the keep-alive reconnect. The rest looks good.

[ps2]

Thanks Marion. The keep-alive concern was right, and the pod simulator confirmed it. I added a pod simulator control that keeps the link up but leaves a message unanswered, then ran the teardown with the app open and the connection held:

• next-dev: logs keep-alive — reconnecting after drop and reconnects immediately.
• The first commit of this PR: doesn't reconnect; the pod is still disconnected 45 s later.
• With 1f60442: reconnects as before.

1f60442 takes your first suggestion. The teardown now goes through disconnectUnresponsive, which clears commandConnectInFlight only when the connection isn't held. A held link reconnects; an idle one re-arms the probe. The line 1716 comment is updated.

I also took out the claim that cancelling from the PeripheralManager queue "raced CoreBluetooth's teardown and wedged the next connect". It came from an older comment, I couldn't find any evidence for it, and PeripheralManager calls CoreBluetooth from its own queue on every command anyway. 109e834 adds [stuckConnect] device events, so if the suspected race does happen in the field, the logs will show it. The description is updated with the details.

[marionbarker]

Claude review

Re-review (049152c → 109e834)

The keep-alive concern is fixed. The new cancel-provenance logging has an ordering bug on the eager-watchdog path, and the PR description is now out of date.

Resolved

• Keep-alive reconnect (1f60442). The new disconnectUnresponsive clears commandConnectInFlight only when !shouldHoldConnection. A held link takes the didDisconnect keep-alive reconnect path as it did before this PR. An idle link re-arms the probe. The comment at the shouldHoldConnection && commandConnectInFlight branch now names this case. 👍
• All cancels are routed and recorded. Every BluetoothManager cancel now goes through cancelConnection. The only direct cancel left is PeripheralManager.reconnect, which calls noteCancel first and is kept off-queue on purpose for comparison, as the commit says.
• Thread safety of lastCancel. It is only touched on managerQueue. noteCancel writes synchronously when already on that queue, which setSpecific detects, and dispatches the write otherwise. managerQueue is a private serial queue per instance, so the static key is fine.

Bug: the watchdog [stuckConnect] line always reports its own cancel

In the eager watchdog (already running on managerQueue):

if target.state == .connecting {
    self.logStuckConnect(target, "eager watchdog")   // managerQueue.async { read lastCancel }
}
self.cancelConnection(target)                        // noteCancel writes lastCancel synchronously

logStuckConnect dispatches its read with async, so the read runs after cancelConnection has already overwritten lastCancel with the watchdog's own cancel. Every watchdog event will therefore say last cancel 0.0s earlier, from managerQueue. The connect being diagnosed here is the one stuck in .connecting, so this hides exactly what the logging is meant to show.

The on-demand-timeout path in PeripheralManager.connectOnDemand is fine: logStuckConnect and disconnectOnDemand are both dispatched to the same serial queue in that order, so the read happens first.

Fix: snapshot lastCancel synchronously when the caller is already on managerQueue. For example, read it at the top of logStuckConnect when getSpecific is true and only dispatch the logging, or call logStuckConnect after capturing and before cancelConnection with a synchronous read.

Minor

• Window between cancel and didDisconnect. shouldHoldConnection is checked when the cancel runs. If the app goes to the background before didDisconnect, the flag stays set and didDisconnect takes the idle path. There the probe re-arm is gated on !commandConnectInFlight, so the original "probe doesn't re-arm" problem comes back in that narrow window, until something else clears the flag. It's probably rare, but deciding in didDisconnect (for example with a "pending unresponsive teardown" marker) would close the window.
• PR description is stale. It still says the teardown goes through disconnectOnDemand, and its rationale relies on a715d9a's "cancel off the queue wedged the next connect". 1f60442 now calls that unproven and removes those comments. Please update the description to match: the queue routing is about keeping the connect-on-demand state in order, and [stuckConnect] is there to test the suspicion.
• Carried over from the first review (not blocking): the "No response from pod…" and "Configured in…" device events still have no [tag], and there are still no tests. disconnectUnresponsive's held vs. not-held branches would be easy to unit-test if the central can be faked.

Testing

On-pod testing is still pending as far as the PR shows. That matters more now that the keep-alive branch is in. The useful cases are:

1. App in the foreground, forced read timeout. Expect keep-alive — reconnecting after drop.
2. App in the background without Keep Alive. Expect the probe to re-arm.
3. An InPlay pod whose eager watchdog fires, once the ordering fix is in. Check that [stuckConnect] shows the earlier cancel, not the watchdog's own.

Recommendation: approve after the watchdog ordering fix and a description update. The rest is fine to merge as is.

[ps2]

Thanks Marion. 325c254 makes these changes:

• Watchdog ordering: you're right. logStuckConnect now reads lastCancel before returning when it's called on managerQueue, so the eager watchdog reports the earlier cancel, not its own. The on-demand timeout path still reads asynchronously, which was already in the right order.
• The window between cancel and didDisconnect: also right. disconnectUnresponsive now only marks the teardown and cancels. didDisconnect clears commandConnectInFlight if the link isn't held at that moment, so backgrounding in between no longer leaves the flag set. Before this change the flag stayed set until the next command connected.
• Tags: the events are now [connectOnDemand] no response from pod (…) — disconnecting, [connect] configured in …s and [connect] configuration failed after …s: …. All three use interpolation.
• Description: it was updated at 01:01 UTC, before your re-review. It names disconnectUnresponsive, and it calls the CoreBluetooth claim unsupported, so the review may have read an older copy. I've updated it again for this commit.
• Tests: there are still none for this path. BluetoothManager creates its own CBCentralManager and has no way to substitute a fake central. Adding one is a bigger change than this PR. The held case is covered by the pod simulator; the not-held case is checked by reading the code only.

I reran the pod-simulator test on 325c254. The held case behaves as before: the teardown completes with held=true, and keep-alive reconnects within 1 ms. The test also turned up something unrelated to this PR. On every build, next-dev included, the reconnected link times out in service discovery, and commands keep failing until the app relaunches. That may be an artifact of ImpossiBLE, the CoreBluetooth replacement the simulator uses, and it needs checking on a phone. Details are in the description.

[marionbarker]

Re-review: PR #209 (4 commits, head 325c254)

The fixes from the last round are in. The watchdog now reads the earlier cancel synchronously, the hold decision is made in didDisconnect, and the new events are tagged.

Critical: the background heartbeat stops if the pod hangs up first

The background ~5 min wake relies on the StartDelay probe. Every place the probe is armed, including issueDelayedConnectProbe's own guard, requires !commandConnectInFlight.

commandConnectInFlight is cleared only after our own teardown, the idle disconnect, or the background transition. When the pod drops the link during a command, which is the "hung up ~7 s after connect" case in #199:

• readMessagePacket sees state != .connected and skips the teardown, so unresponsiveTeardownPending is never set.
• didDisconnect takes the idle path with the flag still set, so the probe isn't armed.
• The idle disconnect requires .connected, so it never runs and never clears the flag.

The result is no further pump heartbeat wakes until a CGM wake, a manual open, or some other command connect clears the flag.

The same thing happens inside the PR's own path. If the pod's didDisconnect is queued before disconnectUnresponsive's block, the flag isn't cleared. The cancel then hits a peripheral that's already disconnected and gets no callback, so unresponsiveTeardownPending stays set until some later disconnect.

This gap is pre-existing on dev, but this PR is meant to restore probe re-arm after unresponsive-pod drops.

Suggestion: in didDisconnect, whenever !shouldHoldConnection, clear commandConnectInFlight for any drop, not just a marked teardown. A retrying session's next command connect already preempts the probe (the "command preempts heartbeat probe" path). That would also remove the need for unresponsiveTeardownPending.

Recommendation: fix the background re-arm above before merging. Everything else looks good.

[marionbarker]

Test

✅ Test phone behaved nominally while locked - not a wedging configuration

Configuration:

LoopWorkspace main v3.14.11, commit 5efba88

• OmnipodKit branch fix/unresponsive-pod-disconnect-queue, commit c48aa64
• iPhone SE 2nd gen running iOS 18.7.8
• rPi simulator (using mode DASH)

Test Details

This test was performed on 2026-11-04

After building, Pod Expired is left over from last test with this phone

• Deactivate pod
• Pair rPi DASH, prime and insert
• 08:34 Enter 45 g carbs but do not bolus
• 08:36 lock phone
• Phone is configured for closed loop, temp basal only, the glucose is from a constant CGM simulator but with carbs entered, should get temp basals to support the expected rise in glucose
• Let the phone sit for a while and then make sure it is behaving nominally

Activity in rPi terminal

08:39:57 PROGRAM_INSULIN; HEX, 17e8a53518201a0e494e532e0100b5013840001e001e160e0000012c005b8d80012c005b8d8000c4 

Return to the office and check rPi terminal

09:54:53 phone sent a getStatus request to pod
09:54:53 got an 0x1d response from Pod

Phone is working in the background. However, this is not a "wedging" configuration.

Saved data

Harvest the Xcode debug log and Loop Report.

• 20261104_0957_Xcode_OmnipodKitPR209.txt

• Loop Report 2026-11-04 09-59-51-08-00.md.zip

Plan for next test (next comment)

• Modify phone settings to Automatic Bolus.
• Modify build to indicate this is a wedging configuration for phone and pod.

[marionbarker]

Begin Data Collection

Configuration:

LoopWorkspace main v3.14.11, commit 5efba88

• OmnipodKit branch fix/unresponsive-pod-disconnect-queue, commit c48aa64
  • Make additional edits to OmnipodKit as shown at bottom of this comment
• iPhone SE 2nd gen running iOS 18.7.8
• rPi simulator (using mode DASH)

Test Details

This test was performed on 2026-11-04

Phone has the rPi DASH pod still connected from the previous test

• 10:13 Fresh build modified to mock up wedging configuration
• Realized the CGM was actually CGM simulator with constant value - edit prior comment with this information
• Switch to Nightscout as CGM and restart the UAM trace upload to Nightscout
• 10:17 Enter 45 g carbs but do not bolus
• 10:18 lock phone
• Phone is configured for closed loop, automatic bolus dosing strategy, the glucose trace mocks up an unannounce meal, so the app should provide an autobolus once every 5 minutes while locked
• Let the phone sit for a while and then make sure it is behaving nominally

Check rPi terminal and it had quit. Last command was at:
10:16:26 pkg response 0x1d; HEX, 17e8a535200a1d18002f3000000197ff01c8

Restart the rPi
./pod -mode dash

• 10:22:45 pkg command; 0x0e; GET_STATUS; HEX, 17e8a5351c030e010780d5
• 10:22:45 pkg response 0x1d; HEX, 17e8a535200a1d18002f30000001bfff03e8

Xcode fired a heartbeat at 10:26:41
From the rPi Terminal window:

• rPi reported new connection at same time, 10:26:41
• evidently no need to talk to pod at that time, next command happens in 5 minutes
• 10:31:01 pkg command; 0x1a; PROGRAM_INSULIN; HEX, 17e8a535301f1a0e494e532e02005801013000130013170d3f00be00030d40000000000000013a
• 10:31:01 pkg response 0x1d; HEX, 17e8a535340a1d58005360130001dfff818

Loop and rPi seem to be behaving nominally
Let the system collect data and report back later.

Post this comment now.

Modification to mock up wedging configuration

Make this modification to OmnipodKit

OmnipodKit % git diff

diff --git a/OmnipodKit/Common/UIDevice.swift b/OmnipodKit/Common/UIDevice.swift
index fd81b48..7e0c2a4 100644
--- a/OmnipodKit/Common/UIDevice.swift
+++ b/OmnipodKit/Common/UIDevice.swift
@@ -137,7 +137,7 @@ public extension UIDevice {
     /// checks (eager-connect gate, pod keep-alive defaulting, settings advisory).
     static var hasPossibleInPlayBLEIssues: Bool {
         let model = UIDevice.modelName
-        return model.contains("iPhone 16") || model == "iPhone 17e"
+        return true // return model.contains("iPhone 16") || model == "iPhone 17e"
     }
 
 }
diff --git a/OmnipodKit/PumpManager/OmniPumpManager.swift b/OmnipodKit/PumpManager/OmniPumpManager.swift
index 6eee9c9..a9d9392 100644
--- a/OmnipodKit/PumpManager/OmniPumpManager.swift
+++ b/OmnipodKit/PumpManager/OmniPumpManager.swift
@@ -2387,7 +2387,7 @@ extension OmniPumpManager {
     var usingInPlayPod: Bool? {
 
         if let blePodComms = podComms as? BlePodComms, let deviceBLEName = blePodComms.manager?.peripheral.name {
-            return deviceBLEName == BluetoothManager.inPlayPeripheralName
+            return true // return deviceBLEName == BluetoothManager.inPlayPeripheralName
         }
         return nil // don't know -- maybe not paired yet
     }

[marionbarker]

Data Collection

I configured to collect data faking a wedging scenario (hardware not wedging but code told that it was)

• See Log each connect's setup and every failed exchange and disconnect; don't mark unsent commands unacknowledged #209 (comment) for configuration details
• ✅ The system behaved nominally while locked

14:33 Unlock phone

• Pump events were coming in every 5 minutes, then 25 minutes when it was just repeating 0 TBR
• Issue Loop Report
• save Xcode debug logs

Loop Report 2026-11-04 14-34-56-08-00.md.zip

20261104_1435_Xcode_OmnipodKitPR209.txt.zip

[dnzxy]

We have various users on Trio reporting issues that could be linked to the issue this PR aims to solve.
There are 1-2 who'll run this PR to capture more logging. I'll provide logs as soon as they get to me, unless it is no longer needed by the time they are sent to me :)

[marionbarker]

I did another review with Claude. This time I made sure all information in the conversation was provided.

• Recommendation
• Section 1: code review
• Section 2: review of my long Xcode debug log file. There was a logging inconsistency and a suggestion that test be repeated with the addition of an rPi outage while the phone is locked.

Excerpt from Claude Review:

Recommendation

• Approve if the goal is the teardown fix plus field diagnostics. dnzxy's Trio logs, collected on this build, will be more informative with the new lines.
• Request changes if this PR is meant to close Omnipod 5: Repeated "Peripheral Not Ready" errors after successful Bluetooth connection cause failed loops #199's background-stop symptom. The fix is a small change in handleDisconnect; see Section 1.

Section 1: Review summary: OmnipodKit PR #209 (7 commits, head c48aa64)

Question: Does this PR prevent the app from stopping work in the background?

Answer: Not fully. It fixes one way the background heartbeat can stop, after OmnipodKit tears down an unresponsive pod. A second way is still open: the pod itself drops the link during a command. That gap is the critical item from the Nov 3 re-review, and it is unchanged at c48aa64.

The PR doesn't make background behavior worse than dev, and its logging is valuable. It is safe to merge as an improvement plus diagnostics, but not as the fix for "stops working in the background."

Open issue (critical for the background question)

A pod-initiated drop during a command still leaves commandConnectInFlight set, so the heartbeat probe never re-arms.

1. The pod hangs up mid-session. This matches the "hung up ~7 s after connect" in Omnipod 5: Repeated "Peripheral Not Ready" errors after successful Bluetooth connection cause failed loops #199.
2. readMessagePacket sees the link is already down, so it doesn't call disconnectUnresponsive, and the teardown marker is never set.
3. handleDisconnect clears the flag only when that marker is set. Otherwise it takes the idle path with the flag still set.
4. The idle path skips arming the probe because !commandConnectInFlight is false.
5. The idle disconnect requires .connected, so it never runs and never clears the flag.

Suggested fix: in handleDisconnect, clear commandConnectInFlight for any drop when !shouldHoldConnection. Optionally, gate it on !hasActiveCommandSession, and clear it when the last session ends with the link down.

• The "command preempts heartbeat probe" path already handles a probe that gets armed during a retry.
• This also makes unresponsiveTeardownPending unnecessary. That marker can currently stay set when the cancel lands on a link that is already down.

Section 2: Review of Xcode debug log

Logs from the ~4 h locked run (Loop Report and Xcode log, 10:18–14:33 PST):

• Heartbeat never stalled. 103 heartbeat fires; median gap 2 min, longest 4 min 20 s.
• Every disconnect was one of ours: probe wake 51, command preempts probe 36, idle 34, connect timed out 2.
  • No pod-initiated drop occurred while locked.
  • No unresponsive-pod teardown occurred either.
• The 25-minute pump-event spacing is expected. It is Loop renewing a 0 U/hr temp basal before its 30 minutes expire, not missed wakes.
• Failures before the lock were the simulator, not the PR. Between 10:13 and 10:18 there was a write timeout and empty CTS replies, and the link timed out at 10:16:26. That matches the time the rPi quit.

Two small logging inaccuracies seen in these logs (non-blocking):

• When an on-demand connect times out, the next line reads [connect] configuration failed … notReady; services failed. That blames setup for what was really a failed connect.
• After the app relaunches with a link iOS restored, linkContext says "not connected" while the peripheral is connected. This happens because connectedAt is only set in didConnect.

To test the gap:

1. Use a network CGM, with Pod Keep Alive off and the phone locked.
2. Have the simulator drop the link partway through a command.
3. Confirm whether a [delayedConnect] probe and heartbeat follow within about 5 minutes.

[marionbarker]

Giving an approval to remove block for merging.
Based on last Claude review - this does fix some issues.

• Warning - I am echoing Claude but do not necessarily understand the review, passing it on for experts to decide

Not sure if we want to include changes based on Claude review posted in this comment.

• A pod-initiated drop during a command still leaves commandConnectInFlight set, so the heartbeat probe never re-arms.
• Logging inconsistency reported

[ps2]

@marionbarker, thanks. The open issue is real, and 0d361d6 fixes it along the lines you suggested, with the session gate.

What changed

• handleDisconnect's decision now lives in DisconnectPolicy, a plain struct with no CoreBluetooth dependency:
  • A drop outside a command session clears commandConnectInFlight, so the probe re-arms.
  • A drop during a session leaves the flag set, since the session may retry its connect.
  • When the last session ends with the link down and not held, the flag is cleared and the probe is armed. That covers your sequence: the pod hangs up mid-command, then readMessagePacket sees the link already down.
• unresponsiveTeardownPending is gone.
• Both logging items are fixed:
  • A connect that timed out logs [connect] not connected, setup not run instead of blaming services.
  • A link iOS restored at launch reports "Ns after restore".

Why the gate matters. My first version cleared the flag on any drop when the link wasn't held, and the simulator run broke it. When a command preempts the probe, the cancelled probe's didDisconnect can arrive after the command's connect has started, and clearing the flag there pulled it out from under that connect. Real iOS also delivers didDisconnect for a cancelled pending connect. Your log has 36 "command preempts probe" disconnects, so this would have been common.

Tests

• OmniTests/DisconnectPolicyTests.swift adds 13 tests. They include your sequence, the late probe disconnect, held and keep-alive reconnects, the system-reconnect and watchdog paths, and eager pods.
• With c48aa64's behavior put back into the policy, 4 of them fail.
• OmniTests: 202 run, 0 failures.

Simulator. LoopKit/pod#33 adds hangUpOnNextMessages. The pod hangs up with CBError 7 when a message arrives, or after it has answered (afterAnswering), and then accepts the reconnect.

I ran next-dev plus this PR in the iOS Simulator, with the app in the background, Pod Keep Alive set to "when open" and a mock CGM. Here is a hang-up during a status read:

[connectOnDemand] command preempts heartbeat probe — cancelling probe
[connectOnDemand] connect
[disconnect] ours: command preempts probe, not connected, during connect, setup ok
[connect] configured in 0.038s: …
[disconnect] CBErrorDomain 7, 0.051s after connect, during session Get pod status, setup ok
[read] emptyValue at RTS after 5.010s, link already down; peripheral disconnected, …, during session Get pod status, setup ok
Unacknowledged message. seq:5, error = incorrectResponse
[connectOnDemand] connect
[connect] configured in 0.015s: …
[disconnect] ours: idle, 4.259s after connect, during idle, setup ok
[delayedConnect] … issuing connect StartDelay=311s

The third line is the late probe disconnect, landing during the command's connect.

Hang-ups after answering were handled too, and the probes and heartbeats kept going through every run.

What the simulator didn't show. I couldn't get the stall itself on c48aa64 end to end. After a status read, OmnipodKit issues a heartbeat if the last one was more than 2 minutes ago. Loop's cycle then opens another session, and that session's idle disconnect clears the flag. To stall, the pod has to drop the link after the cycle's last session and before the 4 s idle disconnect. The unit tests cover that case.

Running this also turned up an ImpossiBLE bug in the same ordering: the late disconnect orphaned the next connect. It's fixed in mickeyl/ImpossiBLE#10. You'll need it to run this in the simulator.

[marionbarker]

Claude review

PR #209 re-review — new commit 0d361d6 (2026-11-10)

PR: loopandlearn/OmnipodKit#209, head 0d361d6 (8 commits)
Developer comment: #issuecomment-6768935560
Checked against: OmnipodKit dev @ adf8773, with merge order #209, then #214 (#214 head f2e3aa6)

Verdict

Approve. 0d361d6 fixes the open item: a pod hang-up during a command no longer stops the background heartbeat. The fix is better than the one I suggested. The refactor keeps the old behavior except for the intended changes, and the decision logic now has unit tests. The findings below are low-severity or optional.

Correction to my earlier suggestion. In PR209_cleanup.md I proposed clearing commandConnectInFlight on any drop while the link isn't held. The developer's simulator run showed why that's wrong. When a command preempts the probe, the cancelled probe's didDisconnect can arrive after the command's connect has started, and clearing the flag there pulls it out from under that connect. Gating on "no active command session", and settling the flag when the last session ends, is the right shape.

Findings (excerpt)

Marion here - not Claude

"Passing on 2 findings I thought you might want to review. If you want all four, I can provide it."

returning to Claude:

Finding 2: A possible double probe issue.
handleCommandSessionsEnded checks peripheral.state == .disconnected, but the matching didDisconnect can still be queued behind it on managerQueue. If that happens, the probe is issued, then handleDisconnect sets delayedProbeInFlight = false and issues it again. CoreBluetooth coalesces the two connect calls, so the effect is a reset StartDelay timer and two [delayedConnect] issuing lines. In the common case, the 5 s read timeout means didDisconnect has long since arrived. Optional: in handleDisconnect, skip re-arming if a probe is already in flight.

Finding 4 Earlier cleanup items not taken (optional):

• Each disconnect still writes two device-log lines: [autoReconnect] didDisconnect isReconnecting=false … and [disconnect] ….
• A failed login still isn't retried on dev: needsConfiguration = false is set before completeConfiguration runs. The PR notes that next-dev's Refuse a pod session without a handshake; decode the LTK and BLE handle separately #201 gate covers this.
• There's still no else { noteSetupStep("no delegate — login skipped") } in configureAndRun. This one matters for Unhook a replaced or forgotten PeripheralManager from BlePodComms #214 (see below).

Then #214 on top

• dev + Log each connect's setup and every failed exchange and disconnect; don't mark unsent commands unacknowledged #209 (0d361d6) + Unhook a replaced or forgotten PeripheralManager from BlePodComms #214 merges cleanly (git merge-tree).
• 0d361d6 doesn't touch BlePodComms.swift or completeConfiguration, so the Unhook a replaced or forgotten PeripheralManager from BlePodComms #214 notes stand as written:
  • Add the explicit manager parameter and identity guard. The guard should return (with noteSetupStep), not throw, because Log each connect's setup and every failed exchange and disconnect; don't mark unsent commands unacknowledged #209 now rethrows.
  • Add the no delegate — login skipped note. Once Unhook a replaced or forgotten PeripheralManager from BlePodComms #214 clears old managers' delegates, a skipped login would otherwise be logged as setup ok.
  • Rebase onto Log each connect's setup and every failed exchange and disconnect; don't mark unsent commands unacknowledged #209's setupStep("login") { … } line.

[marionbarker]

LGTM