Add post runSession onFault handler to store reconciled doses

[itsmojo]

Background

During the analysis of Trio issue #1236, it was found that even there have always been delays between when a pod fault is noticed by OmniBLE or OmnipodKit and reported to the controlling app (Loop or Trio) and when any reconciled interrupted doses are actually stored back to the controlling app. Generally speaking without forgetting the pod, the reconciled dose would require a subsequent disconnect and post-connect status fetch to actually store() the interrupted dose to the app which could be minutes later (which is now even less likely to occur with the Pod Keep Alive stuff for Altas pods). For this discussion, it'll use Loop for the app name and the current OmnipodKit for the PumpManager (but this discussion also generally applies for Trio and OmniBLE as well).

PodCommSessions.handlePodFault() will be called when a pod fault is noticed. The a pod first faults, the podState.fault is set to the return fault status, podState.handleCancelDosing() will handle any needed dosing updates and podState.updateFromStatusResponse() handles the rest of the podState updates from derived status in the pod fault. Loop will be notified of the pod fault from the end of OmniPumpManager.setStateWithResult() when the observers are notified of this state change showing a pod fault. However, in this context, the updated dose values cannot yet be stored (they might not have even been updated yet).

The functions that typically handle the doses for storage generally occur after a successful session.XXX() call (in places getDetailedStatus(), cancelBolus(), suspendDelivery(), resumeDelivery(), enactBolus(), cancelBolus(), enactTempBasal()) and are of the form:

            let result = session.XXX()
            switch result {
            case .success:
                session.dosesForStorage() { (doses) -> Bool in
                    return self.store(doses: doses, in: session)
                }
                ....

The store() func being used here cannot be called within the lockedState lock (this why it cannot be used within or around setStateWithResult()), but it does depend on being called with the transport session lock held. It is important to note that because of the logic involving managing lastSync time stamp, it would also be an error to invoke sessions.dosesForStorage() after any actual pod communications error. But a pod fault being received is a unique case in that a response was indeed received from the pod, but the failure return is not a success which implied a more common pod comms error. So a potential way to make sure that session.dosesForStorage() would be called in pod fault cases would be to add yet another value for the DeliveryCommandResult enum to denote that a pod fault was received during that operation and update all the callers to handle this new value correctly (i.e., do the store() operation, but then return failure to the app). But this approach seems too messy and error prone.

Draft Implementation

This PR shows one way to make sure any updated doses are stored at the end of each runSession whenever a fault has been defected via a pod response for whatever reason. This way, there is no need to add any special case testing for a pod fault case within any session.XXX failure to get any reconciled doses immediately stored on same PM call when Loop is notified of the pod fault whereas currently the user must wait for some subsequent laster PumpManager call to hopefully trigger the store operation, otherwise this will not happen until the pod is deactivated and OmniPumpManager.forgetPod() is called.

Just noticed that the version of store() used by forgetPod() doesn't take a PodCommsSession and it appears that PodState can be locked, so other approaches might also be possible using this store() variant.

GetStatus() Differences in OmniBLE vs OmnipodKit

OmniBLE had code to in its podCommsDidEstablishSession() to always call session.dosesForStorage() independent of the result of the session.getStatus() call. This would be the only place that would force a reconciled dose to be stored() after a pod fault and before a pod deactivation. In all the other places a sessions.getStatus() call is used, session.dosesForStorage() would only be called on a successful return. However, with a user using Pod Keep Alive, podCommsDidEstablishSession() should not be getting called every 3 minutes and thus the gap between when a pod fault is noticed by Loop and when the interrupted bolus dose get stored (without deactivating the pod) could be forever.

OmnipodKit had too many cases of back to back getStatus pod calls due to the different timing of pod disconnects with O5 pods and uses a new handlePodUpdatesAsNeeded() function that allows some podStatus calls to be skipped if allowed by the caller, but it still handles other various common state updates. This function originally ended up being too big of a difference from OmniBLE's behavior when called by the podCommsDidEstablishSession() as it only did the store() on success (matching other place OmniBLE getStatus() store semantics, but not OmniBLEPumpManager.podCommsDidEstablishSession()'s store semantics). This difference was corrected in OmnipodKit PR #99 by having handlePodUpdatesAsNeeded() invoke session.dosesForStorage() if either a getStatus was successfully performed OR if the pod is currently faulted. This ends up having nearly the same semantics as OmniBLE.podCommsDidEstablishSession() which always does the session.dosesForStorage() regardless of the getStatus result.