Extra messages exchanged with Omnipod

[marionbarker]

Describe the bug

The number of messages exchanged for the same actions with Trio is greater than with Loop. The extra exchanges with Trio appear to be getStatus followed by status response. These should not require a lot of power, but some people are complaining of pod faults with Trio. Extra messages can mean more battery drain and thus increase the likelihood of faults.

Attach a Log

I will add some specific comparisons in a comment. I specifically observed this when looking at a suspend resume action. I will evaluate additional actions when preparing my comment.

To Reproduce

Steps to reproduce the behavior:
I will include the specific steps when I add some specific comparisons in a comment.

Expected behavior

The number of messages and message types exchanged between the phone and the pod were carefully adjusted during Loop development to be as small as possible while doing the required task.

(There was some relaxation of this design with the advent of BLE communications, but most of the efficiencies have been reinstated for Loop and the OmniBLE/Kit submodules.)

Screenshots

N/A

Setup Information (please complete the following information):

Smartphone:

N/A

Pump:

• Manufacturer: Insulet
• Model: probably both Eros and DASH, use the rPi DASH simulator for testing

CGM:

N/A

Trio Version:

The explicit version / commit numbers will be listed in the comment with the comparisons

Technical Details

@itsmojo suggests that the extra get status calls observed when suspending and resuming in Trio come from the self.updateStatus() call in clearBolusReporter() in FreeAPS/Sources/APS/APSManager.swift. He wants to reserve his focus for the OmniBLE/Kit submodules, so is turning analysis of extra messages over to the Trio developers.

Additional context

Add any other context about the problem here.

[marionbarker]

Summary

• I established exactly which pod buttons are sending too many getStatus commands to the pod
• Based off the hint provided by @itsmojo, I commented out some code (see end of this comment) and repeated the tests
• This needs someone else to delve into APSManager.swift because commenting out the code breaks another part of the behavior - handling pod state following a bolus is no longer handled correctly
  • The behavior following a bolus is reported in the next comment
• I'll be happy to test the PR when it is prepared

Configuration

• iPhone 8 running Trio alpha, commit 217d1d6
• iPhone 7+ running Loop dev, commit c4b4588

Method

Examine the response to each command in the rPi DASH simulator.
Compare to Loop.

For each command, there is an 0x1d message returned from the pod.

• Each command / response is parsed as needed using a python parser
• The first 0x1d message shows the correct state
• The additional 0x0e (getStatus) commands are completely unnecessary
• When doing a manual bolus with Trio, there is a pause after bolus starts, then more unneeded getStatus after enough time for bolus to complete

It is important to minimize communications with the pods to limit the load on the battery of the pod.

The PumpManager in the Omnipod code is smart enough decide whether to get a status before starting to bolus or before setting a TempBasal. If it is sure it knows the state of the pod, it skips the extra getStatus. (Note the difference in the Loop vs Trio+Mod test - last row).

Manual Commands

Make a table of standard manual commands for the pod (keep closed loop disabled)

Button	Trio	Loop	Trio+Mod
Pod from Main Screen	0x0e	0x0e	0x0e
beep	0x1e	0x1e	0x1e
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
notification	0x19	0x19	0x19
confidence	0x1e	0x1e	0x1e
silence pod	0x11	0x11	0x11
unsilence	0x11	0x11	0x11
manual bolus	0x1a 0x0e pause 0x0e 0x0e	0x1a pause	0x1a pause
manual bolus cancel partway	-	0x0e 0x1a cancel 0x1f1	0x1a cancel 0x1f1

Diagnostic Commands

Different table mostly because each one gets a special response from the pod.
So for these, indicate what is sent and what comes back.
This should be the same for all apps - confirm it but don't make a table for each.

Button	Command	Response
read pod status	0x0e type 0x02	0x0202
play test beeps	0x1e	0x1d
read pulse log	0x0e type 0x50	0x0250
read pulse log plus	0x0e type 0x03	0x0203
read activation time	0x0e type 0x05	0x0205
read triggered alerts	0x0e type 0x01	0x0201

Modification made to the Trio code

Note: commenting out the code breaks another part of the app - handling pod state following a bolus is no longer handled correctly

• The behavior following a bolus is reported in the next comment

This works as desired to remove the extra getStatus commands being issued to the pod, but is not the correct solution.

For the Trio+Mod column in the table Manual Commands, this is the modification applied to the code:

% git diff
diff --git a/FreeAPS/Sources/APS/APSManager.swift b/FreeAPS/Sources/APS/APSManager.swift
index ac64f24b..6e111b2f 100644
--- a/FreeAPS/Sources/APS/APSManager.swift
+++ b/FreeAPS/Sources/APS/APSManager.swift
@@ -909,10 +909,12 @@ final class BaseAPSManager: APSManager, Injectable {
         }
 
         if let omnipod = pump as? OmnipodPumpManager {
-            omnipod.getPodStatus { _ in }
+            debug(.apsManager, "no need to get status from pod")
+            // omnipod.getPodStatus { _ in }
         }
         if let omnipodBLE = pump as? OmniBLEPumpManager {
-            omnipodBLE.getPodStatus { _ in }
+            debug(.apsManager, "no need to get status from pod")
+            // omnipodBLE.getPodStatus { _ in }
         }
     }

[marionbarker]

Summary

There was a suggestion in the discord chat of just deleting the updateStatus function in APSManager.swift (this would have the same result as commenting out the two lines as was done in the prior test.)

Neither of the options (removing the function or commenting out lines in the function) is an appropriate solution, as shown in the testing below.

The function should be updated to get the status of the Pod from OmniXXX without triggering an extra set of communications with the pod.

Testing:

Test the way Trio works now

I returned Trio to the original state, alpha, commit 217d1d6

I tested the case of trying to bolus while already bolusing.

• manual bolus
• attempt manual bolus while previous bolus is in progress
  • the app allows entry but when issuing the bolus, it is rejected
  • appropriate error message appears

I tested the case of trying to bolus after canceling the earlier bolus (which would otherwise still in progress)

• manual bolus
• cancel bolus in progress
• attempt manual bolus while canceled bolus would have been in progress
  • success

Test total removal of updateStatus call

Now just comment out the one line: self.updateStatus with no other changes.
Repeat the 2 tests above.

• The first test is OK.
• The second test is not ok - rejects the manual bolus.
• Subsequent attempts to bolus fail until I manually issue a command that talks to the pod (used the beeps command)

So updateStatus needs to be there but it should not require the pod to be queried. This information is available in the pump manager.

Tagging @avouspierre and @itsmojo

[kskandis]

I think canceling should be excluded from sending a trigger to clearReporter? This can be done in DeviceDataManager.

This is because if the Reporter observer is prematurely removed then the cancel completion will not be observed. APSManager seoarately observes CancelBolus and then will remove the bolusReporter so there is no reason to include cancelling in the trigger.

I tested this in Mock and Cancel Bolus succeeds. I also commented outt getting status updates.

Could you see if you get the same results, @marionbarker by applying patch or I could do a PR?
extra-messagges-with-omnipod.patch

[marionbarker]

This did not solve the problem. I'll post more details soon.

[kskandis]

This issue may be related to UI : Bolus Progress Wheel Persisting After Bolus Completed
#309

[marionbarker]

This was fixed with the merge of #353