Unacknowledged command handling fixes and PodCommsSession improvements

[itsmojo]

• Don't append an optional beep block to a getStatus with a pending unacknowledged command
• Handle unacknowledged commands cases in acknowledgeAlerts(), beepConfig() & configureAlert()
• Use checkCommandAgainstStatus() for seq # mismatch to determine insulin delivery status
• Remove unneeded @discardResult attribute on various PodCommsSessions functions
• Comment improvements and better source code synchronization between OmniKit and OmniBLE

[itsmojo]

To test the new code that suppresses appending beepBlocks when there is an outstanding unacknowledgedCommand (which breaks any chance of correctly resolving the unacknowledgedCommand) was to modify getStatus() to always append a beepBlock and then create unacknowledged message situations using the rPi front end and then single step thru the code in send() to verify that the specified beepBlock was not being appended. Then tests were run again using the modified getStatus() while creating unacknowledged command situations with a single breakpoint on the single line in send<> that actually appends the beepBlock (to ensure that it is never executed) as well as to verify that no beep blocks were ever appended over an extended period of running like this by an examination of the resulting pod comms logs using OmniBLEParser.

To develop and test checkCommandAgainstStatus(), I used debug code I had created to introduce inconsistent podStatus delivery states in February 2024 when working on the code for OmniBLE PR #110 back in February 2024. I then commented out the call to unacknowledgedCommandWasReceived() on a message # match [that updates the podState’s unfinalizedBolus, unfinalizedTempBasal, suspendState appropriately depending on whether it’s an insulin or a cancel command] to simulate a breakdown in the msg # based recovery code for whatever reason. Using this setup along with the rPi pod sim and front end, I was able to force test checkCommandStatus() to resolve the pendingCommand against the actual received pod delivery state for all 6 possible cases (starting/cancelling a basal, tempBasal or bolus). This is actually a safe and sane thing to do whenever the message #’s don’t match since if the pod ever reports that it is bolusing, temp basaling or not suspended and the podState doesn’t match but there is a pendingCommand for that insulin delivery, it is better to use the dosing/time information from the pendingCommand for better dosing reconciliation (instead of having to rely on the podState’s lastDeliveryStatusReceived variable which can only prevent a 0x31 fault, but can’t do any dose reconciliation).

The added guards to configureAlerts(), beepConfig() and acknowledgeAlerts() to protect against a pending unacknowledgedCommand situation were not formally tested. But without these guards, any of these commands being executed would definitely break any chance of correctly resolving an unacknowledgedCommand, so they definitely do close another potential unacknowledgedCommand resolution issue. These guards are limited until after the pod setup is complete to ensure sure that these guards can’t possible mess up a pod setup.

[marionbarker]

Status

LGTM

Code Review

The code review looks good. The comparison of OmniBLE PR 136 and OmniKit PR 45, side-by-side, agreed. The only differences between OmniBLE and OmniKit are the rileylink vs Bluetooth differences.

Testing

Hard to test some of this code - relying on the work @itmojo did by inserting errors.

This modification has been running in-vivo for @itsmojo for several months.
I added it to my personal phone with no issues.
I exercised the rPi simulator: used the front end to crash the rPi in a variety of scenarios: bolus, temp basal, suspend, resume, cancel bolus, cancel temp basal, scheduled basal, alerts. After restarting the rPi, the app resumed as expected.