WIP: don't kill the pod when switching profile

app/src/main/java/info/nightscout/androidaps/plugins/pump/omnipod/comm/OmnipodCommunicationService.java

@@ -164,10 +164,6 @@ public class OmnipodCommunicationService extends RileyLinkCommunicationManager {
                 // We actually ignore previous (ack) responses if it was not last packet to send
                 response = exchangePackets(podState, packet);
             } catch (Exception ex) {
-                // If this is not the last packet, the message wasn't fully sent,
-                // so it's impossible for the pod to have received the message
-                boolean isCertainFailure = encodedMessage.length > 0;
-
                 OmnipodException newException;
                 if (ex instanceof OmnipodException) {
                     newException = (OmnipodException) ex;
@@ -175,10 +171,15 @@ public class OmnipodCommunicationService extends RileyLinkCommunicationManager {
                     newException = new CommunicationException(CommunicationException.Type.UNEXPECTED_EXCEPTION, ex);
                 }
 
+                boolean lastPacket = encodedMessage.length == 0;
+
+                // If this is not the last packet, the message wasn't fully sent,
+                // so it's impossible for the pod to have received the message
+                newException.setCertainFailure(!lastPacket);
+
                 if (isLoggingEnabled()) {
-                    LOG.debug("Caught exception in transportMessages. Setting certainFailure to {} because encodedMessage.length={}", isCertainFailure, encodedMessage.length);
+                    LOG.debug("Caught exception in transportMessages. Set certainFailure to {} because encodedMessage.length={}", newException.isCertainFailure(), encodedMessage.length);
                 }
-                newException.setCertainFailure(isCertainFailure);
 
                 throw newException;
             }

app/src/main/java/info/nightscout/androidaps/plugins/pump/omnipod/comm/OmnipodManager.java

@@ -167,14 +167,32 @@ public class OmnipodManager {
         }
     }
 
+    // CAUTION: cancels all delivery
+    // CAUTION: suspends and then resumes delivery. An OmnipodException[certainFailure=false] indicates that the pod is or might be suspended
     public synchronized void setBasalSchedule(BasalSchedule schedule, boolean acknowledgementBeep) {
         assertReadyForDelivery();
 
         logStartingCommandExecution("setBasalSchedule [basalSchedule=" + schedule + ", acknowledgementBeep=" + acknowledgementBeep + "]");
 
         try {
-            executeAndVerify(() -> communicationService.executeAction(new SetBasalScheduleAction(podState, schedule,
+            // Never emit a beep for suspending delivery, so if the user has beeps enabled,
-                    false, podState.getScheduleOffset(), acknowledgementBeep)));
+            // they can verify that setting the basal schedule succeeded (not suspending the delivery)
+            cancelDelivery(EnumSet.allOf(DeliveryType.class), false);
+        } catch (Exception ex) {
+            logCommandExecutionFinished("setBasalSchedule");
+            throw ex;
+        }
+
+        try {
+            try {
+                executeAndVerify(() -> communicationService.executeAction(new SetBasalScheduleAction(podState, schedule,
+                        false, podState.getScheduleOffset(), acknowledgementBeep)));
+            } catch (OmnipodException ex) {
+                // Treat all exceptions as uncertain failures, because all delivery has been suspended here.
+                // Setting this to an uncertain failure will enable for the user to get an appropriate warning
+                ex.setCertainFailure(false);
+                throw ex;
+            }
         } finally {
             logCommandExecutionFinished("setBasalSchedule");
         }
@@ -195,14 +213,29 @@ public class OmnipodManager {
     }
 
     public synchronized void cancelTemporaryBasal(boolean acknowledgementBeep) {
+        cancelDelivery(EnumSet.of(DeliveryType.TEMP_BASAL), acknowledgementBeep);
+    }
+
+    private synchronized void cancelDelivery(EnumSet<DeliveryType> deliveryTypes, boolean acknowledgementBeep) {
         assertReadyForDelivery();
 
-        logStartingCommandExecution("cancelTemporaryBasal [acknowledgementBeep=" + acknowledgementBeep + "]");
+        logStartingCommandExecution("cancelDelivery [deliveryTypes=" + deliveryTypes + ", acknowledgementBeep=" + acknowledgementBeep + "]");
 
         try {
-            executeAndVerify(() -> communicationService.executeAction(new CancelDeliveryAction(podState, DeliveryType.TEMP_BASAL, acknowledgementBeep)));
+            // As the cancel delivery command is a single packet command,
+            // first verify that the pod is reachable by obtaining a status response
+            // FIXME is this actually necessary?
+            StatusResponse podStatus = getPodStatus();
+        } catch (OmnipodException ex) {
+            logCommandExecutionFinished("cancelDelivery");
+            ex.setCertainFailure(true);
+            throw ex;
+        }
+
+        try {
+            executeAndVerify(() -> communicationService.executeAction(new CancelDeliveryAction(podState, deliveryTypes, acknowledgementBeep)));
         } finally {
-            logCommandExecutionFinished("cancelTemporaryBasal");
+            logCommandExecutionFinished("cancelDelivery");
         }
     }
 
@@ -300,7 +333,7 @@ public class OmnipodManager {
             logStartingCommandExecution("cancelBolus [acknowledgementBeep=" + acknowledgementBeep + "]");
 
             try {
-                executeAndVerify(() -> communicationService.executeAction(new CancelDeliveryAction(podState, DeliveryType.BOLUS, acknowledgementBeep)));
+                cancelDelivery(EnumSet.of(DeliveryType.BOLUS), acknowledgementBeep);
             } catch (PodFaultException ex) {
                 if (isLoggingEnabled()) {
                     LOG.info("Ignoring PodFaultException in cancelBolus", ex);
@@ -315,48 +348,51 @@ public class OmnipodManager {
         }
     }
 
-    // CAUTION: cancels TBR and bolus
     public synchronized void suspendDelivery(boolean acknowledgementBeep) {
-        assertReadyForDelivery();
+        cancelDelivery(EnumSet.allOf(DeliveryType.class), acknowledgementBeep);
-
-        logStartingCommandExecution("suspendDelivery [acknowledgementBeep=" + acknowledgementBeep + "]");
-
-        try {
-            executeAndVerify(() -> communicationService.executeAction(new CancelDeliveryAction(podState, EnumSet.allOf(DeliveryType.class), acknowledgementBeep)));
-        } finally {
-            logCommandExecutionFinished("suspendDelivery");
-        }
     }
 
+    // Same as setting basal schedule, but without suspending delivery first
     public synchronized void resumeDelivery(boolean acknowledgementBeep) {
         assertReadyForDelivery();
-
+        logStartingCommandExecution("resumeDelivery");
-        logStartingCommandExecution("resumeDelivery [acknowledgementBeep=" + acknowledgementBeep + "]");
 
         try {
             executeAndVerify(() -> communicationService.executeAction(new SetBasalScheduleAction(podState, podState.getBasalSchedule(),
-                    true, podState.getScheduleOffset(), acknowledgementBeep)));
+                    false, podState.getScheduleOffset(), acknowledgementBeep)));
         } finally {
             logCommandExecutionFinished("resumeDelivery");
         }
     }
 
-    // CAUTION: cancels TBR and bolus
+    // CAUTION: cancels all delivery
-    // CAUTION: suspends and then resumes delivery.
+    // CAUTION: suspends and then resumes delivery. An OmnipodException[certainFailure=false] indicates that the pod is or might be suspended
-    //  If any error occurs during the command sequence, delivery could be suspended
     public synchronized void setTime(boolean acknowledgementBeeps) {
         assertReadyForDelivery();
 
         logStartingCommandExecution("setTime [acknowledgementBeeps=" + acknowledgementBeeps + "]");
 
         try {
-            suspendDelivery(acknowledgementBeeps);
+            cancelDelivery(EnumSet.allOf(DeliveryType.class), acknowledgementBeeps);
+        } catch (Exception ex) {
+            logCommandExecutionFinished("setTime");
+            throw ex;
+        }
 
+        DateTimeZone oldTimeZone = podState.getTimeZone();
+
+        try {
             // Joda seems to cache the default time zone, so we use the JVM's
             DateTimeZone.setDefault(DateTimeZone.forTimeZone(TimeZone.getDefault()));
             podState.setTimeZone(DateTimeZone.getDefault());
 
-            resumeDelivery(acknowledgementBeeps);
+            setBasalSchedule(podState.getBasalSchedule(), acknowledgementBeeps);
+        } catch (OmnipodException ex) {
+            // Treat all exceptions as uncertain failures, because all delivery has been suspended here.
+            // Setting this to an uncertain failure will enable for the user to get an appropriate warning
+            podState.setTimeZone(oldTimeZone);
+            ex.setCertainFailure(false);
+            throw ex;
         } finally {
             logCommandExecutionFinished("setTime");
         }

app/src/main/java/info/nightscout/androidaps/plugins/pump/omnipod/defs/DeliveryType.java

@@ -4,8 +4,7 @@ public enum DeliveryType {
     NONE((byte) 0x00),
     BASAL((byte) 0x01),
     TEMP_BASAL((byte) 0x02),
-    BOLUS((byte) 0x04),
+    BOLUS((byte) 0x04);
-    EXTENDED_BOLUS((byte) 0x08);
 
     private byte value;
 

app/src/main/java/info/nightscout/androidaps/plugins/pump/omnipod/driver/comm/AapsOmnipodManager.java

@@ -200,11 +200,15 @@ public class AapsOmnipodManager implements OmnipodCommunicationManagerInterface
         return new PumpEnactResult().success(true).enacted(true);
     }
 
+    // TODO cancels TBR. Notify treatments plugin
     @Override
     public PumpEnactResult setBasalProfile(Profile basalProfile) {
         try {
             delegate.setBasalSchedule(mapProfileToBasalSchedule(basalProfile), isBasalBeepsEnabled());
         } catch (Exception ex) {
+            if ((ex instanceof OmnipodException) && !((OmnipodException) ex).isCertainFailure()) {
+                return new PumpEnactResult().success(false).enacted(false).comment(getStringResource(R.string.omnipod_error_set_basal_failed_uncertain));
+            }
             String comment = handleAndTranslateException(ex);
             return new PumpEnactResult().success(false).enacted(false).comment(comment);
         }
@@ -358,12 +362,14 @@ public class AapsOmnipodManager implements OmnipodCommunicationManagerInterface
 
     // TODO should we add this to the OmnipodCommunicationManager interface?
     // Updates the pods current time based on the device timezone and the pod's time zone
+    // TODO cancels TBR. Notify treatments plugin
     public PumpEnactResult setTime() {
         try {
-            // CAUTION cancels TBR
             delegate.setTime(isBasalBeepsEnabled());
         } catch (Exception ex) {
-            // CAUTION pod could be suspended
+            if ((ex instanceof OmnipodException) && !((OmnipodException) ex).isCertainFailure()) {
+                return new PumpEnactResult().success(false).enacted(false).comment(getStringResource(R.string.omnipod_error_set_time_failed_uncertain));
+            }
             String comment = handleAndTranslateException(ex);
             return new PumpEnactResult().success(false).enacted(false).comment(comment);
         }

app/src/main/res/values/strings.xml

@@ -1742,6 +1742,8 @@
     <string name="omnipod_alert_shutdown_imminent">Shutdown is imminent</string>
     <string name="omnipod_alert_low_reservoir">Low reservoir</string>
     <string name="omnipod_alert_unknown_alert">Unknown alert</string>
+    <string name="omnipod_error_set_basal_failed_uncertain">Setting basal profile failed. Delivery might be suspended! Please refresh pod status.</string>
+    <string name="omnipod_error_set_time_failed_uncertain">Setting time failed. Delivery might be suspended! Please refresh pod status.</string>
 
 
 </resources>