nightscout / Trio

Trio - an automated insulin delivery system for iOS based on the OpenAPS algorithm with adaptations.
https://docs.diy-trio.org/en/latest/
MIT License
98 stars 531 forks source link

Update FreeAPS code to remove extra pod getStatus messages #353

Closed marionbarker closed 4 months ago

marionbarker commented 4 months ago

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 commented 4 months ago

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

itsmojo commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

Summary

This PR is ready to merge. Please expedite.

Tests

Medtronic

Omnipod DASH

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.