Fix reporting cancelled TBR upon suspending delivery for Omnipod

omnipod/src/main/java/info/nightscout/androidaps/plugins/pump/omnipod/OmnipodPumpPlugin.java

@@ -116,8 +116,8 @@ import static info.nightscout.androidaps.plugins.pump.omnipod.driver.definition.
  */
 @Singleton
 public class OmnipodPumpPlugin extends PumpPluginBase implements PumpInterface, RileyLinkPumpDevice {
-    private static final long RILEY_LINK_CONNECT_TIMEOUT_MILLIS = 3 * 60 * 1000L; // 3 minutes
+    private static final long RILEY_LINK_CONNECT_TIMEOUT_MILLIS = 3 * 60 * 1_000L; // 3 minutes
-    private static final long STATUS_CHECK_INTERVAL_MILLIS = 60 * 1000L; // 1 minute
+    private static final long STATUS_CHECK_INTERVAL_MILLIS = 60 * 1_000L; // 1 minute
     public static final int STARTUP_STATUS_REQUEST_TRIES = 2;
     public static final double RESERVOIR_OVER_50_UNITS_DEFAULT = 75.0;
 
@@ -155,7 +155,6 @@ public class OmnipodPumpPlugin extends PumpPluginBase implements PumpInterface,
     private final Handler loopHandler = new Handler(Looper.getMainLooper());
 
     private final Runnable statusChecker;
-    private boolean isCancelTempBasalRunning;
 
     @Inject
     public OmnipodPumpPlugin(
@@ -238,6 +237,7 @@ public class OmnipodPumpPlugin extends PumpPluginBase implements PumpInterface,
 
         statusChecker = new Runnable() {
             @Override public void run() {
+                if (commandQueue.size() == 0) {
                     if (podStateManager.isPodRunning() && !podStateManager.isSuspended()) {
                         aapsOmnipodManager.cancelSuspendedFakeTbrIfExists();
                     } else {
@@ -257,6 +257,9 @@ public class OmnipodPumpPlugin extends PumpPluginBase implements PumpInterface,
                     }
 
                     doPodCheck();
+                } else {
+                    aapsLogger.debug(LTag.PUMPCOMM, "Skipping Pod status check because command queue is not empty");
+                }
 
                 loopHandler.postDelayed(this, STATUS_CHECK_INTERVAL_MILLIS);
             }
@@ -360,10 +363,6 @@ public class OmnipodPumpPlugin extends PumpPluginBase implements PumpInterface,
     }
 
     private void handleCancelledTbr() {
-        // Only report TBR cancellations if they haven't been explicitly requested
-        if (isCancelTempBasalRunning) {
-            return;
-        }
         if (!podStateManager.isTempBasalRunning() && activePlugin.getActiveTreatments().isTempBasalInProgress() && !aapsOmnipodManager.hasSuspendedFakeTbr()) {
             aapsOmnipodManager.reportCancelledTbr();
         }
@@ -695,12 +694,7 @@ public class OmnipodPumpPlugin extends PumpPluginBase implements PumpInterface,
             return new PumpEnactResult(getInjector()).success(true).enacted(false);
         }
 
-        isCancelTempBasalRunning = true;
-        try {
         return executeCommand(OmnipodCommandType.CANCEL_TEMPORARY_BASAL, aapsOmnipodManager::cancelTemporaryBasal);
-        } finally {
-            isCancelTempBasalRunning = false;
-        }
     }
 
     // TODO improve (i8n and more)

omnipod/src/main/java/info/nightscout/androidaps/plugins/pump/omnipod/driver/communication/action/CancelDeliveryAction.java

@@ -1,13 +1,17 @@
 package info.nightscout.androidaps.plugins.pump.omnipod.driver.communication.action;
 
+import org.joda.time.Duration;
+
 import java.util.ArrayList;
 import java.util.EnumSet;
 import java.util.List;
 
 import info.nightscout.androidaps.plugins.pump.omnipod.driver.communication.message.MessageBlock;
 import info.nightscout.androidaps.plugins.pump.omnipod.driver.communication.message.OmnipodMessage;
+import info.nightscout.androidaps.plugins.pump.omnipod.driver.communication.message.command.BeepConfigCommand;
 import info.nightscout.androidaps.plugins.pump.omnipod.driver.communication.message.command.CancelDeliveryCommand;
 import info.nightscout.androidaps.plugins.pump.omnipod.driver.communication.message.response.StatusResponse;
+import info.nightscout.androidaps.plugins.pump.omnipod.driver.definition.BeepConfigType;
 import info.nightscout.androidaps.plugins.pump.omnipod.driver.definition.BeepType;
 import info.nightscout.androidaps.plugins.pump.omnipod.driver.definition.DeliveryType;
 import info.nightscout.androidaps.plugins.pump.omnipod.driver.manager.PodStateManager;
@@ -35,23 +39,14 @@ public class CancelDeliveryAction implements OmnipodAction<StatusResponse> {
     public StatusResponse execute(OmnipodRileyLinkCommunicationManager communicationService) {
         List<MessageBlock> messageBlocks = new ArrayList<>();
 
-        if (acknowledgementBeep && deliveryTypes.size() > 1) {
+        messageBlocks.add(new CancelDeliveryCommand(podStateManager.getCurrentNonce(), BeepType.NO_BEEP, deliveryTypes));
-            // Workaround for strange beep behaviour when cancelling multiple delivery types
-            List<DeliveryType> deliveryTypeList = new ArrayList<>(deliveryTypes);
 
-            EnumSet<DeliveryType> deliveryTypeWithBeep = EnumSet.of(deliveryTypeList.remove(deliveryTypeList.size() - 1));
+        // Workaround for strange behavior where the Pod beeps for each specified delivery type
-            EnumSet<DeliveryType> deliveryTypesWithoutBeep = EnumSet.copyOf(deliveryTypeList);
+        if (acknowledgementBeep) {
-
+            messageBlocks.add(new BeepConfigCommand(BeepConfigType.BEEP, false, Duration.ZERO, false, Duration.ZERO, false, Duration.ZERO));
-            messageBlocks.add(new CancelDeliveryCommand(podStateManager.getCurrentNonce(), BeepType.NO_BEEP, deliveryTypesWithoutBeep));
-            messageBlocks.add(new CancelDeliveryCommand(podStateManager.getCurrentNonce(), BeepType.BEEP, deliveryTypeWithBeep));
-        } else {
-            messageBlocks.add(new CancelDeliveryCommand(podStateManager.getCurrentNonce(),
-                    acknowledgementBeep && deliveryTypes.size() == 1 ? BeepType.BEEP : BeepType.NO_BEEP, deliveryTypes));
         }
 
-        StatusResponse statusResponse = communicationService.exchangeMessages(StatusResponse.class, podStateManager,
+        return communicationService.exchangeMessages(StatusResponse.class, podStateManager,
                 new OmnipodMessage(podStateManager.getAddress(), messageBlocks, podStateManager.getMessageNumber()));
-
-        return statusResponse;
     }
 }

omnipod/src/main/java/info/nightscout/androidaps/plugins/pump/omnipod/driver/communication/message/OmnipodMessage.java

@@ -1,6 +1,7 @@
 package info.nightscout.androidaps.plugins.pump.omnipod.driver.communication.message;
 
 import java.util.ArrayList;
+import java.util.EnumSet;
 import java.util.List;
 
 import info.nightscout.androidaps.logging.AAPSLogger;
@@ -31,7 +32,7 @@ public class OmnipodMessage {
 
     public OmnipodMessage(int address, List<MessageBlock> messageBlocks, int sequenceNumber) {
         this.address = address;
-        this.messageBlocks = messageBlocks;
+        this.messageBlocks = new ArrayList<>(messageBlocks);
         this.sequenceNumber = sequenceNumber;
     }
 
@@ -54,7 +55,7 @@ public class OmnipodMessage {
             throw new CrcMismatchException(calculatedCrc, crc);
         }
         List<MessageBlock> blocks = decodeBlocks(ByteUtil.substring(data, 6, data.length - 6 - 2));
-        if (blocks == null || blocks.size() == 0) {
+        if (blocks.size() == 0) {
             throw new MessageDecodingException("No blocks decoded");
         }
 
@@ -110,7 +111,7 @@ public class OmnipodMessage {
     }
 
     public List<MessageBlock> getMessageBlocks() {
-        return messageBlocks;
+        return new ArrayList<>(messageBlocks);
     }
 
     public int getSequenceNumber() {
@@ -147,12 +148,16 @@ public class OmnipodMessage {
         return false;
     }
 
-    public boolean isSetTempBasalMessage() {
+    public boolean isGetStatusMessage() {
-        return messageBlocks.size() >= 2 && messageBlocks.get(0).getType() == MessageBlockType.SET_INSULIN_SCHEDULE && messageBlocks.get(1).getType() == MessageBlockType.TEMP_BASAL_EXTRA;
+        return messageBlocks.size() == 1 && messageBlocks.get(0).getType() == MessageBlockType.GET_STATUS;
     }
 
-    public boolean isCancelTempBasalMessage() {
+    public boolean isSuspendDeliveryMessage() {
-        return messageBlocks.size() >= 1 && messageBlocks.get(0).getType() == MessageBlockType.CANCEL_DELIVERY && ((CancelDeliveryCommand) messageBlocks.get(0)).getDeliveryTypes().contains(DeliveryType.TEMP_BASAL);
+        return isCancelDeliveryMessage() && EnumSet.allOf(DeliveryType.class).equals(((CancelDeliveryCommand) messageBlocks.get(0)).getDeliveryTypes());
+    }
+
+    private boolean isCancelDeliveryMessage() {
+        return messageBlocks.size() >= 1 && messageBlocks.get(0).getType() == MessageBlockType.CANCEL_DELIVERY;
     }
 
     @Override

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

@@ -277,6 +277,15 @@ public class OmnipodManager {
     private synchronized StatusResponse cancelDelivery(EnumSet<DeliveryType> deliveryTypes, boolean acknowledgementBeep) {
         assertReadyForDelivery();
 
+        if (!podStateManager.isTempBasalCertain() || !podStateManager.isBasalCertain()) {
+            try {
+                getPodStatus();
+            } catch (OmnipodException ex) {
+                ex.setCertainFailure(true);
+                throw ex;
+            }
+        }
+
         if (deliveryTypes.contains(DeliveryType.BASAL)) {
             podStateManager.setBasalCertain(false);
         }

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

@@ -568,10 +568,10 @@ public abstract class PodStateManager {
             podState.setPodProgressStatus(status.getPodProgressStatus());
             podState.setTimeActive(status.getTimeActive());
 
-            boolean isBasalCertain = podState.isBasalCertain() == null || podState.isBasalCertain();
+            boolean wasBasalCertain = podState.isBasalCertain() == null || podState.isBasalCertain();
-            boolean isTempBasalCertain = podState.isTempBasalCertain() == null || podState.isTempBasalCertain();
+            boolean wasTempBasalCertain = podState.isTempBasalCertain() == null || podState.isTempBasalCertain();
             if (!status.getDeliveryStatus().isTbrRunning() && hasTempBasal()) {
-                if (isTempBasalCertain) {
+                if (wasTempBasalCertain || requestMessage.isSuspendDeliveryMessage()) {
                     clearTempBasal(); // Triggers onTbrChanged when appropriate
                 } else {
                     // Don't trigger onTbrChanged as we will trigger onUncertainTbrRecovered below
@@ -580,14 +580,17 @@ public abstract class PodStateManager {
                     podState.setTempBasalDuration(null);
                 }
             }
-            if (!isTempBasalCertain) {
+
+            if (!wasTempBasalCertain) {
                 podState.setTempBasalCertain(true);
-                if (!requestMessage.isSetTempBasalMessage() // We always set TBR to uncertain before sending the set temp basal command, so this is not an actual recovery
+
-                        && !requestMessage.isCancelTempBasalMessage()) { // Delivery status changed, so we can't recover here
+                // We exclusively use get status messages to recover from uncertain TBRs
+                // DO NOT change this as the recovery mechanism will otherwise interfere with normal delivery commands
+                if (requestMessage.isGetStatusMessage()) {
                     onUncertainTbrRecovered();
                 }
             }
-            if (!isBasalCertain) {
+            if (!wasBasalCertain) {
                 podState.setBasalCertain(true);
             }