From 85086bceed1d0629295d2d533da1e315f5f620cb Mon Sep 17 00:00:00 2001 From: Bart Sopers Date: Wed, 11 Nov 2020 19:50:06 +0100 Subject: [PATCH] Remove delivery status verification in favor of nonce verification --- ...veryStatusVerificationFailedException.java | 16 --- ...dingCommandFailedUncertainlyException.java | 7 ++ .../driver/manager/OmnipodManager.java | 119 ++++-------------- .../omnipod/manager/AapsOmnipodManager.java | 50 ++++---- 4 files changed, 51 insertions(+), 141 deletions(-) delete mode 100644 omnipod/src/main/java/info/nightscout/androidaps/plugins/pump/omnipod/driver/exception/DeliveryStatusVerificationFailedException.java create mode 100644 omnipod/src/main/java/info/nightscout/androidaps/plugins/pump/omnipod/driver/exception/PrecedingCommandFailedUncertainlyException.java diff --git a/omnipod/src/main/java/info/nightscout/androidaps/plugins/pump/omnipod/driver/exception/DeliveryStatusVerificationFailedException.java b/omnipod/src/main/java/info/nightscout/androidaps/plugins/pump/omnipod/driver/exception/DeliveryStatusVerificationFailedException.java deleted file mode 100644 index f6c86561dc..0000000000 --- a/omnipod/src/main/java/info/nightscout/androidaps/plugins/pump/omnipod/driver/exception/DeliveryStatusVerificationFailedException.java +++ /dev/null @@ -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; - } -} diff --git a/omnipod/src/main/java/info/nightscout/androidaps/plugins/pump/omnipod/driver/exception/PrecedingCommandFailedUncertainlyException.java b/omnipod/src/main/java/info/nightscout/androidaps/plugins/pump/omnipod/driver/exception/PrecedingCommandFailedUncertainlyException.java new file mode 100644 index 0000000000..a1bbd6c7ab --- /dev/null +++ b/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); + } +} diff --git a/omnipod/src/main/java/info/nightscout/androidaps/plugins/pump/omnipod/driver/manager/OmnipodManager.java b/omnipod/src/main/java/info/nightscout/androidaps/plugins/pump/omnipod/driver/manager/OmnipodManager.java index e4601e2a68..963aaa1535 100644 --- a/omnipod/src/main/java/info/nightscout/androidaps/plugins/pump/omnipod/driver/manager/OmnipodManager.java +++ b/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) { - suspendDelivery(acknowledgementBeep); + try { + suspendDelivery(acknowledgementBeep); + } catch (OmnipodException ex) { + if (ex.isCertainFailure()) { + throw ex; + } + + // Uncertain failure + throw new PrecedingCommandFailedUncertainlyException(ex); + } } try { @@ -190,16 +199,8 @@ public class OmnipodManager { throw ex; } - // verifyDeliveryStatus will throw an exception if verification fails - 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; - } + // Uncertain failure + throw ex; } } @@ -217,15 +218,9 @@ public class OmnipodManager { throw ex; } - try { - if (!verifyDeliveryStatus(DeliveryStatus.NORMAL, ex)) { - ex.setCertainFailure(true); - throw ex; - } - } catch (DeliveryStatusVerificationFailedException ex2) { - podStateManager.setTempBasalCertain(false); - throw ex2; - } + // Uncertain failure + podStateManager.setTempBasalCertain(false); + throw new PrecedingCommandFailedUncertainlyException(ex); } } @@ -241,51 +236,14 @@ public class OmnipodManager { throw ex; } - // verifyDeliveryStatus will throw an exception if verification fails - 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 ex2; - } + // Uncertain failure + podStateManager.setTempBasal(DateTime.now().minus(OmnipodConstants.AVERAGE_TEMP_BASAL_COMMAND_COMMUNICATION_DURATION), rate, duration, false); + throw ex; } } 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; - } - } + cancelDelivery(EnumSet.of(DeliveryType.TEMP_BASAL), acknowledgementBeep); } private synchronized StatusResponse cancelDelivery(EnumSet 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; - } - } + cancelDelivery(EnumSet.allOf(DeliveryType.class), acknowledgementBeep); } // 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 diff --git a/omnipod/src/main/java/info/nightscout/androidaps/plugins/pump/omnipod/manager/AapsOmnipodManager.java b/omnipod/src/main/java/info/nightscout/androidaps/plugins/pump/omnipod/manager/AapsOmnipodManager.java index 51c7fb499d..4f621b97d6 100644 --- a/omnipod/src/main/java/info/nightscout/androidaps/plugins/pump/omnipod/manager/AapsOmnipodManager.java +++ b/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; - 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); + showNotification(Notification.FAILED_UDPATE_PROFILE, getStringResource(R.string.omnipod_error_set_basal_failed_delivery_might_be_suspended), 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,17 @@ 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; - if (ex.getExpectedStatus() == DeliveryStatus.TEMP_BASAL_RUNNING) { - // Happened after cancelling the old TBR, when attempting to set new TBR - note = getStringResource(R.string.omnipod_error_set_temp_basal_failed_old_tbr_cancelled_new_might_have_failed); + showNotification(getStringResource(R.string.omnipod_error_set_temp_basal_failed_old_tbr_might_be_cancelled), Notification.URGENT, isNotificationUncertainTbrSoundEnabled() ? R.raw.boluserror : null); + + return new PumpEnactResult(injector).success(false).enacted(false).comment(errorMessage); + } catch (Exception ex) { + String errorMessage = translateException(ex); + 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 @@ -471,16 +472,8 @@ public class AapsOmnipodManager { // 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); - } 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 +553,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 +561,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);