Skip to content

Improved dose handling on faults for more consistent state - #139

Merged
marionbarker merged 2 commits into
loopandlearn:next-devfrom
itsmojo:fault-dose-lastReconciliation-fix
Aug 31, 2026
Merged

marionbarker merged 2 commits into
loopandlearn:next-devfrom
itsmojo:fault-dose-lastReconciliation-fix

Conversation

@itsmojo

@itsmojo itsmojo commented Aug 30, 2026

Copy link
Copy Markdown
Collaborator

As per the Conformance checklist in Pump Manager Dose Reporting, pump managers should always be passing a correct lastReconciliation value for each hasNewPumpEvents dose store. With the implementation for immediate dose reporting on pod fault done in [Omnipod Kit #119], this semantic wasn't being maintained as the special store() done for faults was done using the updated cancelled doses that will have newer times than lastReconciliation because updateFromStatusResponse() hadn't yet been called to update the variables used to derive its value.

This newer and cleaner approach simply has handlePodFault() call podState.updateFromStatusResponse() before setting podState.fault. This call will finalize all the canceled doses and update all the internal OmnipodKit variables as any other normal cancel command would do. With this change, when setStateWithResult() detects a transition to a faulted state, it simply has to call store() with the finalizeDoses to ensure the app gets all the updated and finalized dosing info for the pod fault (just as it would for any cancel command that didn't fault) before actually being notified of the pod fault. Since updateFromStatusResponse() has already been called, lastReconciliation will be consistent with the dose times.

@ps2 ps2 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM! just a small nit

@marionbarker
marionbarker merged commit 8b14632 into loopandlearn:next-dev Aug 31, 2026
marionbarker added a commit that referenced this pull request Sep 1, 2026
* Store updated doses immediately before a pod fault is reported (#119)

* Improved dose handling on faults for more consistent state (#139)

* Improved dose handling on faults for more consistent state

* Nit cleanup

---------

Co-authored-by: Joe Moran <mojo@moranfoundation.org>
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>
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.

3 participants