Improved unacknowledged command recovery

[itsmojo]

Fix for LoopKit/Loop#2275
Use new tryToResolvePendingCommand() function that try to use getStatus() to resolve a
pending unacknowledged command in most PodCommSession functions instead of just
failing if called with a pending unacknowledged command. Reworked logic to prevent
possible incorrect unacknowledged command handling during pod setup & deactivation.

[marionbarker]

Initial Review

The pair of PR, OmniBLE #139 and OmniKit PR 47: Improved unacknowledged command recovery, will be reviewed and tested in tandem.

Summary

For every instance in which the PodCommsError.unacknowledgedCommandPending is thrown in the current code, this PR attempts a recovery by using the new tryToResolvePendingCommand function. If that attempt fails, then the code throws PodCommsError.unacknowledgedCommandPending as before.

If the recovery succeeds, then the app continues without alerting the user. This should provide a smoother user experience.

Code Review

• By code inspection, this PR makes the code changes to support the statement in the Summary.
• The OmniBLE and OmniKit PR were compared side-by-side and both make identical changes.

Build Test

• The app builds using these versions of OmniBLE and OmniKit with no problems.

Testing

Most of the testing will be done with OmniBLE using the rPi simulator. After that testing is completed and reported in the OmniBLE PR, an Eros pod will be tested. These test results will be provided in future comments.

[marionbarker]

Test: Part 1

This PR will take several sessions to test.
All tests in this comment were successful and behaved as expected.

These tests use the rPi DASH simulator and make use of the front-end to cause the simulator to crash after the next command is processed. Because the tryToResolvePendingCommand can never be successful (because the rPi is not available to respond), this series of tests were used to confirm that all comm loss error states are still exhibited when messages are not received from the rPi.

List of functions tested:

There are 11 functions that use the tryToResolvePendingCommand:

• configureAlerts
• beepConfig
• bolus
• setTempBasal
• suspendDelivery
• cancelDelivery
• setTime
• setBasalSchedule
• resumeBasal
• deactivatePod
• acknowledgeAlerts

Test each in turn.

Note - the changes to the pod setup process will be tested and reported in a different comment.

Test Method and Results

• Disable Closed Loop to prevent automated commands from Loop
• For each function, exercise the function using the Crash after processing next command feature of the rPi front-end.

configureAlerts
Pod, Notification, Modify low reservoir reminder

• got expected error when rPi crashed
• resume rPi and save

beepConfig
Pod, Confidence Reminder, change from enabled to disabled

• got expected error when rPi crashed
• resume rPi and save

bolus
Main screen, bolus, issue 1 U

• got Unable to Reach Pump screen
• resume rPi and bolus starts

cancelDelivery
cancel bolus in progress (from prior step)

• got 2 messages - could not cancel and then pump not reachable
• resume rPi, both screens resolved and bolus (which was still going) was interrupted (1.15 U delivered of 3 U requested)

setTempBasal
Pod, Manual Temp Basal

• unable to reach pump screen
• resume rPi, loop says pod is at requested MTB
• check the commands in rPi window, 0x1d shows temp basal is active

suspendDelivery
Pod, suspend for 30 minutes

• unable to reach pump screen
• resume rPi, loop shows insulin delivery is suspended
• check the commands in rPi window, 0x1d shows all delivery is halted

setBasalSchedule and resumeBasal
Resume delivery (after last test). This will run the setBasalSchedule function

• failed to resume, then unable to reach pump screen
• resume rPi, both messages go away and loop shows insulin delivery resumed
• check the commands in rPi window, 0x1d shows scheduled basal is active

setTime
Change time zone on test phone
This will be tricky because it requires a suspend followed by setBasalSchedule to change the time

• clock icon appears on main screen
• sync to current time
• unable to reach pump screen
• resume rPi, screen goes away; but now see that insulin is suspended (confirm by 0x1d hex code)
• sync to current time without the crash on the front-end and all is fine.

deactivatePod
Can't really test this with the rPi front-end crash, multiple commands sent to deactivate

acknowledgeAlerts
Use front end to set LowReservoirAlert
wait for next get status command

• get "Low Reservoir" modal alert and tap OK
• get "Unable To Clear Alert" modal screen on loop screen and alert stays set on the front end
• resume rPi and 0x11; SILENCE_ALERTS is sent again, alert clears from front end.

[marionbarker]

Test: Part 2

Test Method

While developing this PR, @itsmojo developed a testing patch (OmniBLE-PR139-testing.patch) which he shared with me. I modified that patch to simplify testing of the code in this PR.

• OmniBLE-PR139-testing_toggle.patch

This patch uses two private vars fakeUncertainBeforeSendPercent and fakeUncertainAfterSendPercent from the original testing patch but modifies the usage slightly.

These vars can be configured between 0.0 and 1.0, inclusive. Based on the value of those parameters, the code inserts a fake error with the probability provided by that var.

• In other words, 0.0 never fails, 1.0 always fails and 0.01 fails 1% of the time.

In order to allow the code to keep operating when 1.0 is selected for one of these vars, I added a toggle right before those values are used in func send<T: MessageBlock>. If a value is 0.0, it is changed to 1.0. If it is 1.0, it is changed to 0.0. Otherwise, it is not modified.

I then configured the initial values to 0.01 (or 1% failure rate) and added a set point after the toggle section and before the usage of the vars in func send<T: MessageBlock>.

For most of the testing below, that set point is disabled. It is only needed for commands that require 2-command (4-message) sequences that must all go through (like setTime, deactivatePod and parts of the pod initialization sequence). For all other cases, the first try for a command fails but the subsequent retry succeeds or restores app to prior state.

Test comm errors

I tested each command listed below with the indicated configurations.
All tests were successful.

1. Test all commands below one time with default configuration

   • fakeUncertainAfterSendPercent=0.01 and fakeUncertainBeforeSendPercent=0.01

2. Use breakpoint to enable testing of comm failures after a command is sent:

   • Configure fakeUncertainAfterSendPercent=1.0 and fakeUncertainBeforeSendPercent=0.01

3. Use breakpoint to enable testing of comm failures before a command is:

   • Configure fakeUncertainAfterSendPercent=0.01 and fakeUncertainBeforeSendPercent=1.0
   • For these tests need to intervene using the break point to set value to 0.0 in order for commands like bolus, setTempBasal, resumeBasal, suspendDelivery to be enacted - the app auto-recovers, but command was not issued

2-message commands

If one of the fakeUncertain is set to 1.0, then first attempt fails, retry succeeds (or restores prior state).

Test the commands below:

• configureAlerts
• beepConfig
• bolus
• cancelDelivery
• setTempBasal
• resumeBasal
• suspendDelivery
• setBasalSchedule (included in several prior commands)
• acknowledgeAlerts (use front-end to set alert)

4-message commands
For these, enable the break point and set the fakeUncertain var to 0.0 for 2 successive sends

• setTime
• deactivatePod

Special Cases
These are exercised during initial pod set-up
Go through pod pair, prime and insert with appropriate adjustment of fakeUncertain var as needed so the sequences can complete

• prime
• programInitialBasalSchedule
• insertCannula
• checkInsertionCompleted

[marionbarker]

Status

Code review: Approve
Test: Part 1: Success
Test: Part 2: Success

I approve this PR.