Skip to content
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

Update FreeAPS code to remove extra pod getStatus messages #353

Merged
merged 2 commits into from
Jul 20, 2024

Conversation

marionbarker
Copy link
Contributor

@marionbarker marionbarker commented Jul 17, 2024

This PR is in response to Issue #265.

Examination of this problem led to updates to the OmniBLE and OmniKit submodules. Once those changes are incorporated, the code changes in the FreeAPS files can be implemented. The Trio app continues to behave as before but with significantly fewer getStatus calls to the pods. This should improve battery life.

Important This only works properly if the submodule updates are included in the Trio build.

While the PR for these two submodules are under final review, this PR remains a draft. Once merged, the updated index for the submodules will be incorporated in the PR.

Until such time, this PR can be tested using the OmniXXX PR.

@marionbarker marionbarker marked this pull request as ready for review July 18, 2024 13:43
@marionbarker
Copy link
Contributor Author

The pod state improvements were approved and merged in LoopKit and are now available to Trio.
This PR is ready for review.

@itsmojo
Copy link
Contributor

itsmojo commented Jul 18, 2024

Using “update podState” in the titles of OmmiBLE 125 and OmniKit 36 (which allow this APSManager change to work) is a misleading description that wasn't noticed until after these PR's landed in LoopKit. These PR's fix the bolusState and basalDeliveryState calculations that are part of the public pumpManager status variable that is used by the APSManager. This status var is type PumpManagerStatus and is actually is some simplified derived state outside of podState.

I have done a lot of testing of OmniXXX changes (particularly for OmniKit which was also missing needed changes made to the OmniBLE setStateWithResult() func). I also have been running Trio with OmniBLE with an updated APSManager with all traces of updateStatus() removed since I first tracked down this problem on the 14th of July (N.B. I didn't pick up the one line change to DeviceDataManager.swift until today however). Everything seems to run great with these changes in all my testing and personal use and it definitely reduces the overall Trio pod traffic which is a very good thing. The problem with the bolusState and basalDeliveryState calculations were introduced into LoopKit's OmniXXXPumpManagers during the Loop 3 rework. These changes ended up messing up iAPS which uses the pumpManager status variable for a number of decisions, while it didn't break anything in Loop which wasn't using this particular status variable any more for delivery decisions.

So this PR is very safe, well tested (by at least me & Marion so far), and reduces the # of Trio pod comms which is very beneficial for all Trio pod users. This PR looks good to me and I'd suggest landing this sooner rather than later.

@dnzxy
Copy link
Contributor

dnzxy commented Jul 18, 2024

Since Joe cannot approve this PR (so it counts towards the required 2 approvals), I'll approve this in his place.
I think this should make it into 1.0.0, for the pod battery effects alone.

@marionbarker
Copy link
Contributor Author

Summary

This PR is ready to merge. Please expedite.

Tests

Medtronic

  • Ran with MDT 515 pump in closed loop on test phone for several days.
  • No unusual behavior.
  • Tested that bolusing, interrupted bolusing, suspend/resume all behave the same way they did before applying the code in this PR.

Omnipod DASH

  • Ran using rPi DASH simulator in closed loop on test phone for several days.
  • No unusual behavior.
  • Tested that bolusing, interrupted bolusing, suspend/resume, set and cancel manual TempBasal all behave the same way they did before applying the code in this PR with the important exception that the extra getStatus commands are no longer observed.

Pod getStatus Fix

Show the table entries where there used to be extra getStatus commands issued by Trio. Repeat this test with the code from this PR. Only the commands from the app to the pod are shown. Every command sent is responded to by the pod with an 0x1d message.

Button Trio Loop Trio+Mod
Pod from Main Screen 0x0e 0x0e 0x0e
suspend 0x1f7
0x0e
0x0e
0x1f7 0x1f7
resume 0x1a
0x19
0x0e
0x0e
0x0e
0x1a
0x19
0x1a
0x19
manual TB 0x1a
0x0e
0x0e
0x0e
0x1a 0x1a
cancel MTB 0x1f2
0x0e
0x0e
0x0e
0x0e
0x1f2 0x1f2
manual bolus 0x1a
0x0e
pause
0x0e
0x0e
0x1a
pause
0x1a
pause

For message descriptions, please see the wiki.

@MikePlante1 MikePlante1 merged commit 8303f0a into nightscout:dev Jul 20, 2024
@marionbarker marionbarker deleted the update_pump_manager branch July 21, 2024 03:09
mountrcg pushed a commit to mountrcg/Trio that referenced this pull request Jul 26, 2024
Update FreeAPS code to remove extra pod getStatus messages
@MikePlante1 MikePlante1 mentioned this pull request Jul 27, 2024
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.

4 participants