Merge pull request #37 from AAPS-Omnipod/omnipod_eros_dev_remove_duplicate_verification

Remove delivery status verification in favor of nonce verification

omnipod/src/main/java/info/nightscout/androidaps/plugins/pump/omnipod/driver/exception/DeliveryStatusVerificationFailedException.java

@@ -1,16 +0,0 @@
-package info.nightscout.androidaps.plugins.pump.omnipod.driver.exception;
-
-import info.nightscout.androidaps.plugins.pump.omnipod.driver.definition.DeliveryStatus;
-
-public class DeliveryStatusVerificationFailedException extends OmnipodException {
-    private final DeliveryStatus expectedStatus;
-
-    public DeliveryStatusVerificationFailedException(DeliveryStatus expectedStatus, Throwable cause) {
-        super("Failed to verify delivery status (expected=" + expectedStatus + ")", cause, false);
-        this.expectedStatus = expectedStatus;
-    }
-
-    public DeliveryStatus getExpectedStatus() {
-        return expectedStatus;
-    }
-}

omnipod/src/main/java/info/nightscout/androidaps/plugins/pump/omnipod/driver/exception/PrecedingCommandFailedUncertainlyException.java

@@ -0,0 +1,7 @@
+package info.nightscout.androidaps.plugins.pump.omnipod.driver.exception;
+
+public class PrecedingCommandFailedUncertainlyException extends OmnipodException {
+    public PrecedingCommandFailedUncertainlyException(Throwable cause) {
+        super("Preceding command failed", cause, false);
+    }
+}

omnipod/src/main/java/info/nightscout/androidaps/plugins/pump/omnipod/driver/manager/OmnipodManager.java

@@ -41,7 +41,6 @@ import info.nightscout.androidaps.plugins.pump.omnipod.driver.definition.PodInfo
 import info.nightscout.androidaps.plugins.pump.omnipod.driver.definition.PodProgressStatus;
 import info.nightscout.androidaps.plugins.pump.omnipod.driver.definition.schedule.BasalSchedule;
 import info.nightscout.androidaps.plugins.pump.omnipod.driver.exception.CommandFailedAfterChangingDeliveryStatusException;
-import info.nightscout.androidaps.plugins.pump.omnipod.driver.exception.DeliveryStatusVerificationFailedException;
 import info.nightscout.androidaps.plugins.pump.omnipod.driver.exception.IllegalActivationProgressException;
 import info.nightscout.androidaps.plugins.pump.omnipod.driver.exception.IllegalDeliveryStatusException;
 import info.nightscout.androidaps.plugins.pump.omnipod.driver.exception.IllegalPodProgressException;
@@ -49,6 +48,7 @@ import info.nightscout.androidaps.plugins.pump.omnipod.driver.exception.NonceOut
 import info.nightscout.androidaps.plugins.pump.omnipod.driver.exception.OmnipodException;
 import info.nightscout.androidaps.plugins.pump.omnipod.driver.exception.PodFaultException;
 import info.nightscout.androidaps.plugins.pump.omnipod.driver.exception.PodProgressStatusVerificationFailedException;
+import info.nightscout.androidaps.plugins.pump.omnipod.driver.exception.PrecedingCommandFailedUncertainlyException;
 import info.nightscout.androidaps.plugins.pump.omnipod.rileylink.manager.OmnipodRileyLinkCommunicationManager;
 import info.nightscout.androidaps.utils.sharedPreferences.SP;
 import io.reactivex.Completable;
@@ -175,7 +175,16 @@ public class OmnipodManager {
 
         boolean wasSuspended = podStateManager.isSuspended();
         if (!wasSuspended) {
+            try {
                 suspendDelivery(acknowledgementBeep);
+            } catch (OmnipodException ex) {
+                if (ex.isCertainFailure()) {
+                    throw ex;
+                }
+
+                // Uncertain failure
+                throw new PrecedingCommandFailedUncertainlyException(ex);
+            }
         }
 
         try {
@@ -190,18 +199,10 @@ public class OmnipodManager {
                 throw ex;
             }
 
-            // verifyDeliveryStatus will throw an exception if verification fails
+            // Uncertain failure
-            if (verifyDeliveryStatus(DeliveryStatus.NORMAL, ex)) {
-                podStateManager.setBasalSchedule(schedule);
-            } else {
-                if (!wasSuspended) {
-                    throw new CommandFailedAfterChangingDeliveryStatusException("Suspending delivery succeeded but setting the new basal schedule did not", ex);
-                }
-                ex.setCertainFailure(true);
             throw ex;
         }
     }
-    }
 
     // CAUTION: cancels temp basal and then sets new temp basal. An OmnipodException[certainFailure=false] indicates that the pod might have cancelled the previous temp basal, but did not set a new temp basal
     public synchronized void setTemporaryBasal(double rate, Duration duration, boolean acknowledgementBeep, boolean completionBeep) {
@@ -217,15 +218,9 @@ public class OmnipodManager {
                     throw ex;
                 }
 
-                try {
+                // Uncertain failure
-                    if (!verifyDeliveryStatus(DeliveryStatus.NORMAL, ex)) {
-                        ex.setCertainFailure(true);
-                        throw ex;
-                    }
-                } catch (DeliveryStatusVerificationFailedException ex2) {
                 podStateManager.setTempBasalCertain(false);
-                    throw ex2;
+                throw new PrecedingCommandFailedUncertainlyException(ex);
-                }
             }
         }
 
@@ -241,51 +236,14 @@ public class OmnipodManager {
                 throw ex;
             }
 
-            // verifyDeliveryStatus will throw an exception if verification fails
+            // Uncertain failure
-            try {
-                if (verifyDeliveryStatus(DeliveryStatus.TEMP_BASAL_RUNNING, ex)) {
-                    podStateManager.setTempBasal(DateTime.now().minus(OmnipodConstants.AVERAGE_TEMP_BASAL_COMMAND_COMMUNICATION_DURATION), rate, duration, true);
-                } else {
-                    if (cancelCurrentTbr) {
-                        throw new CommandFailedAfterChangingDeliveryStatusException("Failed to set new TBR while cancelling old TBR succeeded", ex);
-                    }
-
-                    ex.setCertainFailure(true);
-                    throw ex;
-                }
-            } catch (CommandFailedAfterChangingDeliveryStatusException ex2) {
-                // Don't set temp basal in Pod State for this Exception
-                throw ex2;
-            } catch (OmnipodException ex2) {
-                if (!ex2.isCertainFailure()) {
-                    // We're not sure that setting the new TBR failed, so we assume that it succeeded
-                    // If it didn't, PodStateManager.updateFromResponse() will fix the state
-                    // upon receiving the next StatusResponse
             podStateManager.setTempBasal(DateTime.now().minus(OmnipodConstants.AVERAGE_TEMP_BASAL_COMMAND_COMMUNICATION_DURATION), rate, duration, false);
-                }
+            throw ex;
-                throw ex2;
-            }
         }
     }
 
     public synchronized void cancelTemporaryBasal(boolean acknowledgementBeep) {
-        try {
         cancelDelivery(EnumSet.of(DeliveryType.TEMP_BASAL), acknowledgementBeep);
-        } catch (OmnipodException ex) {
-            if (ex.isCertainFailure()) {
-                throw ex;
-            }
-
-            try {
-                if (!verifyDeliveryStatus(DeliveryStatus.NORMAL, ex)) {
-                    ex.setCertainFailure(true);
-                    throw ex;
-                }
-            } catch (DeliveryStatusVerificationFailedException ex2) {
-                podStateManager.setTempBasalCertain(false);
-                throw ex2;
-            }
-        }
     }
 
     private synchronized StatusResponse cancelDelivery(EnumSet<DeliveryType> deliveryTypes, boolean acknowledgementBeep) {
@@ -423,20 +381,7 @@ public class OmnipodManager {
     public synchronized void suspendDelivery(boolean acknowledgementBeep) {
         assertReadyForDelivery();
 
-
-        try {
         cancelDelivery(EnumSet.allOf(DeliveryType.class), acknowledgementBeep);
-        } catch (OmnipodException ex) {
-            if (ex.isCertainFailure()) {
-                throw ex;
-            }
-
-            // verifyDeliveryStatus will throw an exception if verification fails
-            if (!verifyDeliveryStatus(DeliveryStatus.SUSPENDED, ex)) {
-                ex.setCertainFailure(true);
-                throw ex;
-            }
-        }
     }
 
     // CAUTION: cancels all delivery
@@ -573,28 +518,6 @@ public class OmnipodManager {
         throw new PodProgressStatusVerificationFailedException(expectedPodProgressStatus, ex);
     }
 
-    /**
-     * @param expectedStatus    expected delivery status
-     * @param verificationCause the Exception causing us to verify the delivery status
-     * @return true if the Pod's status matches the expected status, otherwise false
-     * @throws DeliveryStatusVerificationFailedException in case reading the Pod status fails
-     */
-    private boolean verifyDeliveryStatus(DeliveryStatus expectedStatus, Throwable verificationCause) {
-        aapsLogger.debug(LTag.PUMPCOMM, "Attempting to verify delivery status (expected={})", expectedStatus);
-        for (int i = 0; 3 > i; i++) {
-            try {
-                StatusResponse podStatus = getPodStatus();
-                aapsLogger.debug(LTag.PUMPCOMM, "Resolved delivery status (expected={}, actual={})", expectedStatus, podStatus.getDeliveryStatus());
-                return podStatus.getDeliveryStatus().equals(expectedStatus);
-            } catch (Exception ex) {
-                aapsLogger.debug(LTag.PUMPCOMM, "Ignoring exception thrown in getPodStatus() during attempt to verify delivery status: {}: {}",
-                        ex.getClass().getSimpleName(), ex.getMessage());
-            }
-        }
-        aapsLogger.warn(LTag.PUMPCOMM, "Failed to verify delivery status");
-        throw new DeliveryStatusVerificationFailedException(expectedStatus, verificationCause);
-    }
-
     private Duration calculateEstimatedBolusDuration(DateTime startTime, double units, double deliveryRateInUnitsPerSecond) {
         if (!podStateManager.isPodActivationCompleted()) {
             // No basal or temp basal is active yet

omnipod/src/main/java/info/nightscout/androidaps/plugins/pump/omnipod/manager/AapsOmnipodManager.java

@@ -46,7 +46,6 @@ import info.nightscout.androidaps.plugins.pump.omnipod.driver.communication.mess
 import info.nightscout.androidaps.plugins.pump.omnipod.driver.communication.message.response.podinfo.PodInfoRecentPulseLog;
 import info.nightscout.androidaps.plugins.pump.omnipod.driver.communication.message.response.podinfo.PodInfoResponse;
 import info.nightscout.androidaps.plugins.pump.omnipod.driver.definition.AlertConfiguration;
-import info.nightscout.androidaps.plugins.pump.omnipod.driver.definition.DeliveryStatus;
 import info.nightscout.androidaps.plugins.pump.omnipod.driver.definition.FaultEventCode;
 import info.nightscout.androidaps.plugins.pump.omnipod.driver.definition.OmnipodConstants;
 import info.nightscout.androidaps.plugins.pump.omnipod.driver.definition.PodInfoType;
@@ -55,7 +54,6 @@ import info.nightscout.androidaps.plugins.pump.omnipod.driver.definition.schedul
 import info.nightscout.androidaps.plugins.pump.omnipod.driver.exception.ActivationTimeExceededException;
 import info.nightscout.androidaps.plugins.pump.omnipod.driver.exception.CommandFailedAfterChangingDeliveryStatusException;
 import info.nightscout.androidaps.plugins.pump.omnipod.driver.exception.CrcMismatchException;
-import info.nightscout.androidaps.plugins.pump.omnipod.driver.exception.DeliveryStatusVerificationFailedException;
 import info.nightscout.androidaps.plugins.pump.omnipod.driver.exception.IllegalActivationProgressException;
 import info.nightscout.androidaps.plugins.pump.omnipod.driver.exception.IllegalDeliveryStatusException;
 import info.nightscout.androidaps.plugins.pump.omnipod.driver.exception.IllegalMessageAddressException;
@@ -72,6 +70,7 @@ import info.nightscout.androidaps.plugins.pump.omnipod.driver.exception.OmnipodE
 import info.nightscout.androidaps.plugins.pump.omnipod.driver.exception.PodFaultException;
 import info.nightscout.androidaps.plugins.pump.omnipod.driver.exception.PodProgressStatusVerificationFailedException;
 import info.nightscout.androidaps.plugins.pump.omnipod.driver.exception.PodReturnedErrorResponseException;
+import info.nightscout.androidaps.plugins.pump.omnipod.driver.exception.PrecedingCommandFailedUncertainlyException;
 import info.nightscout.androidaps.plugins.pump.omnipod.driver.exception.RileyLinkInterruptedException;
 import info.nightscout.androidaps.plugins.pump.omnipod.driver.exception.RileyLinkTimeoutException;
 import info.nightscout.androidaps.plugins.pump.omnipod.driver.exception.RileyLinkUnexpectedException;
@@ -281,24 +280,22 @@ public class AapsOmnipodManager {
             String errorMessage = translateException(ex.getCause());
             addFailureToHistory(historyEntryType, errorMessage);
             return new PumpEnactResult(injector).success(false).enacted(false).comment(errorMessage);
-        } catch (DeliveryStatusVerificationFailedException ex) {
+        } catch (PrecedingCommandFailedUncertainlyException ex) {
             if (showNotifications) {
-                String note;
+                showNotification(Notification.FAILED_UDPATE_PROFILE, getStringResource(R.string.omnipod_error_set_basal_failed_delivery_might_be_suspended), Notification.URGENT, R.raw.boluserror);
-                if (ex.getExpectedStatus() == DeliveryStatus.SUSPENDED) {
-                    // Happened when suspending delivery before setting the new profile
-                    note = getStringResource(R.string.omnipod_error_set_basal_failed_delivery_might_be_suspended);
-                } else {
-                    // Happened when setting the new profile (after suspending delivery)
-                    note = getStringResource(R.string.omnipod_error_set_basal_might_have_failed_delivery_might_be_suspended);
-                }
-                showNotification(Notification.FAILED_UDPATE_PROFILE, note, Notification.URGENT, R.raw.boluserror);
             }
             String errorMessage = translateException(ex.getCause());
             addFailureToHistory(historyEntryType, errorMessage);
             return new PumpEnactResult(injector).success(false).enacted(false).comment(errorMessage);
         } catch (Exception ex) {
             if (showNotifications) {
-                showNotification(Notification.FAILED_UDPATE_PROFILE, getStringResource(R.string.omnipod_error_set_basal_failed), Notification.URGENT, R.raw.boluserror);
+                String note;
+                if (OmnipodManager.isCertainFailure(ex)) {
+                    note = getStringResource(R.string.omnipod_error_set_basal_failed);
+                } else {
+                    note = getStringResource(R.string.omnipod_error_set_basal_might_have_failed_delivery_might_be_suspended);
+                }
+                showNotification(Notification.FAILED_UDPATE_PROFILE, note, Notification.URGENT, R.raw.boluserror);
             }
             String errorMessage = translateException(ex);
             addFailureToHistory(historyEntryType, errorMessage);
@@ -455,13 +452,19 @@ public class AapsOmnipodManager {
             String errorMessage = translateException(ex.getCause());
             addFailureToHistory(PodHistoryEntryType.SET_TEMPORARY_BASAL, errorMessage);
             return new PumpEnactResult(injector).success(false).enacted(false).comment(errorMessage);
-        } catch (DeliveryStatusVerificationFailedException ex) {
+        } catch (PrecedingCommandFailedUncertainlyException ex) {
             String errorMessage = translateException(ex.getCause());
+            addFailureToHistory(PodHistoryEntryType.SET_TEMPORARY_BASAL, errorMessage);
 
-            String note;
+            showNotification(getStringResource(R.string.omnipod_error_set_temp_basal_failed_old_tbr_might_be_cancelled), Notification.URGENT, isNotificationUncertainTbrSoundEnabled() ? R.raw.boluserror : null);
-            if (ex.getExpectedStatus() == DeliveryStatus.TEMP_BASAL_RUNNING) {
+
-                // Happened after cancelling the old TBR, when attempting to set new TBR
+            return new PumpEnactResult(injector).success(false).enacted(false).comment(errorMessage);
-                note = getStringResource(R.string.omnipod_error_set_temp_basal_failed_old_tbr_cancelled_new_might_have_failed);
+        } catch (Exception ex) {
+            String errorMessage = translateException(ex);
+            long pumpId = addFailureToHistory(PodHistoryEntryType.SET_TEMPORARY_BASAL, errorMessage);
+
+            if (!OmnipodManager.isCertainFailure(ex)) {
+                showNotification(getStringResource(R.string.omnipod_error_set_temp_basal_failed_old_tbr_cancelled_new_might_have_failed), Notification.URGENT, isNotificationUncertainTbrSoundEnabled() ? R.raw.boluserror : null);
 
                 // Assume that setting the temp basal succeeded here, because in case it didn't succeed,
                 // The next StatusResponse that we receive will allow us to recover from the wrong state
@@ -469,19 +472,10 @@ public class AapsOmnipodManager {
                 // If we would assume that the TBR didn't succeed, we couldn't properly recover upon the next StatusResponse,
                 // as we could only see that the Pod is running a TBR, but we don't know the rate and duration as
                 // the Pod doesn't provide this information
-                long pumpId = addFailureToHistory(PodHistoryEntryType.SET_TEMPORARY_BASAL, errorMessage);
-                addTempBasalTreatment(System.currentTimeMillis(), pumpId, tempBasalPair);
-            } else {
-                // Happened when attempting to cancel the old TBR
-                note = getStringResource(R.string.omnipod_error_set_temp_basal_failed_old_tbr_might_be_cancelled);
-                addFailureToHistory(PodHistoryEntryType.SET_TEMPORARY_BASAL, errorMessage);
-            }
-            showNotification(note, Notification.URGENT, isNotificationUncertainTbrSoundEnabled() ? R.raw.boluserror : null);
 
-            return new PumpEnactResult(injector).success(false).enacted(false).comment(errorMessage);
+                addTempBasalTreatment(System.currentTimeMillis(), pumpId, tempBasalPair);
-        } catch (Exception ex) {
+            }
-            String errorMessage = translateException(ex);
+
-            addFailureToHistory(PodHistoryEntryType.SET_TEMPORARY_BASAL, errorMessage);
             return new PumpEnactResult(injector).success(false).enacted(false).comment(errorMessage);
         }
 
@@ -560,7 +554,7 @@ public class AapsOmnipodManager {
             String errorMessage = translateException(ex.getCause());
             addFailureToHistory(PodHistoryEntryType.SET_TIME, errorMessage);
             return new PumpEnactResult(injector).success(false).enacted(false).comment(errorMessage);
-        } catch (DeliveryStatusVerificationFailedException ex) {
+        } catch (PrecedingCommandFailedUncertainlyException ex) {
             if (showNotifications) {
                 showNotification(getStringResource(R.string.omnipod_error_set_time_failed_delivery_might_be_suspended), Notification.URGENT, R.raw.boluserror);
             }
@@ -568,6 +562,9 @@ public class AapsOmnipodManager {
             addFailureToHistory(PodHistoryEntryType.SET_TIME, errorMessage);
             return new PumpEnactResult(injector).success(false).enacted(false).comment(errorMessage);
         } catch (Exception ex) {
+            if (showNotifications) {
+                showNotification(getStringResource(R.string.omnipod_error_set_time_failed_delivery_might_be_suspended), Notification.URGENT, R.raw.boluserror);
+            }
             String errorMessage = translateException(ex);
             addFailureToHistory(PodHistoryEntryType.SET_TIME, errorMessage);
             return new PumpEnactResult(injector).success(false).enacted(false).comment(errorMessage);