Open marionbarker opened 1 month ago
APSManager.swift
because commenting out the code breaks another part of the behavior - handling pod state following a bolus is no longer handled correctly
c4b4588
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.
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).
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 |
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 |
Note: commenting out the code breaks another part of the app - handling pod state following a bolus is no longer handled correctly
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 }
}
}
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.
I returned Trio to the original state, alpha, commit 217d1d65
I tested the case of trying to bolus while already bolusing.
I tested the case of trying to bolus after canceling the earlier bolus (which would otherwise still in progress)
Now just comment out the one line: self.updateStatus
with no other changes.
Repeat the 2 tests above.
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
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
This did not solve the problem. I'll post more details soon.
This issue may be related to UI : Bolus Progress Wheel Persisting After Bolus Completed
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:
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 inclearBolusReporter()
inFreeAPS/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.