Add some checks and logging in Omnipod packet and message exchange

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

@@ -971,9 +971,7 @@ public class OmnipodPumpPlugin extends PumpPluginAbstract implements OmnipodPump
 
                 // Don't trigger an alert when we exceeded the thresholds, but the last communication was successful
                 // This happens when we simply didn't need to send any commands to the pump
-                return false;
             }
-
         }
 
         return false;

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

@@ -15,6 +15,15 @@ import info.nightscout.androidaps.plugins.pump.common.hw.rileylink.ble.defs.RLMe
 import info.nightscout.androidaps.plugins.pump.common.hw.rileylink.ble.defs.RileyLinkBLEError;
 import info.nightscout.androidaps.plugins.pump.common.utils.ByteUtil;
 import info.nightscout.androidaps.plugins.pump.omnipod.comm.action.OmnipodAction;
+import info.nightscout.androidaps.plugins.pump.omnipod.comm.exception.CommunicationException;
+import info.nightscout.androidaps.plugins.pump.omnipod.comm.exception.IllegalPacketTypeException;
+import info.nightscout.androidaps.plugins.pump.omnipod.comm.exception.IllegalResponseException;
+import info.nightscout.androidaps.plugins.pump.omnipod.comm.exception.IllegalSequenceNumberException;
+import info.nightscout.androidaps.plugins.pump.omnipod.comm.exception.NonceOutOfSyncException;
+import info.nightscout.androidaps.plugins.pump.omnipod.comm.exception.NonceResyncException;
+import info.nightscout.androidaps.plugins.pump.omnipod.comm.exception.NotEnoughDataException;
+import info.nightscout.androidaps.plugins.pump.omnipod.comm.exception.PodFaultException;
+import info.nightscout.androidaps.plugins.pump.omnipod.comm.exception.PodReturnedErrorResponseException;
 import info.nightscout.androidaps.plugins.pump.omnipod.comm.message.MessageBlock;
 import info.nightscout.androidaps.plugins.pump.omnipod.comm.message.OmnipodMessage;
 import info.nightscout.androidaps.plugins.pump.omnipod.comm.message.OmnipodPacket;
@@ -28,15 +37,7 @@ import info.nightscout.androidaps.plugins.pump.omnipod.defs.MessageBlockType;
 import info.nightscout.androidaps.plugins.pump.omnipod.defs.PacketType;
 import info.nightscout.androidaps.plugins.pump.omnipod.defs.PodInfoType;
 import info.nightscout.androidaps.plugins.pump.omnipod.defs.state.PodState;
-import info.nightscout.androidaps.plugins.pump.omnipod.comm.exception.CommunicationException;
-import info.nightscout.androidaps.plugins.pump.omnipod.comm.exception.IllegalPacketTypeException;
-import info.nightscout.androidaps.plugins.pump.omnipod.comm.exception.IllegalResponseException;
-import info.nightscout.androidaps.plugins.pump.omnipod.comm.exception.NonceOutOfSyncException;
-import info.nightscout.androidaps.plugins.pump.omnipod.comm.exception.NonceResyncException;
-import info.nightscout.androidaps.plugins.pump.omnipod.comm.exception.NotEnoughDataException;
 import info.nightscout.androidaps.plugins.pump.omnipod.exception.OmnipodException;
-import info.nightscout.androidaps.plugins.pump.omnipod.comm.exception.PodFaultException;
-import info.nightscout.androidaps.plugins.pump.omnipod.comm.exception.PodReturnedErrorResponseException;
 
 /**
  * Created by andy on 6/29/18.
@@ -153,6 +154,8 @@ public class OmnipodCommunicationService extends RileyLinkCommunicationManager {
             packetAddress = addressOverride;
         }
 
+        podState.increaseMessageNumber();
+
         boolean firstPacket = true;
         byte[] encodedMessage;
         // this does not work well with the deactivate pod command, we somehow either
@@ -207,7 +210,6 @@ public class OmnipodCommunicationService extends RileyLinkCommunicationManager {
         }
 
         if (response.getPacketType() == PacketType.ACK) {
-            podState.increasePacketNumber(1);
             throw new IllegalPacketTypeException(null, PacketType.ACK);
         }
 
@@ -216,6 +218,9 @@ public class OmnipodCommunicationService extends RileyLinkCommunicationManager {
         while (receivedMessage == null) {
             try {
                 receivedMessage = OmnipodMessage.decodeMessage(receivedMessageData);
+                if (receivedMessage.getSequenceNumber() != podState.getMessageNumber()) {
+                    throw new IllegalSequenceNumberException(podState.getMessageNumber(), receivedMessage.getSequenceNumber());
+                }
             } catch (NotEnoughDataException ex) {
                 // Message is (probably) not complete yet
                 OmnipodPacket ackForCon = createAckPacket(podState, packetAddress, ackAddressOverride);
@@ -235,8 +240,6 @@ public class OmnipodCommunicationService extends RileyLinkCommunicationManager {
             }
         }
 
-        podState.increaseMessageNumber(2);
-
         ackUntilQuiet(podState, packetAddress, ackAddressOverride);
 
         List<MessageBlock> messageBlocks = receivedMessage.getMessageBlocks();
@@ -250,7 +253,13 @@ public class OmnipodCommunicationService extends RileyLinkCommunicationManager {
             }
         }
 
-        return messageBlocks.get(0);
+        MessageBlock messageBlock = messageBlocks.get(0);
+
+        if (messageBlock.getType() != MessageBlockType.ERROR_RESPONSE) {
+            podState.increaseMessageNumber();
+        }
+
+        return messageBlock;
     }
 
     private OmnipodPacket createAckPacket(PodState podState, Integer packetAddress, Integer messageAddress) {
@@ -286,7 +295,7 @@ public class OmnipodCommunicationService extends RileyLinkCommunicationManager {
             throw new CommunicationException(CommunicationException.Type.UNEXPECTED_EXCEPTION, ex);
         }
 
-        podState.increasePacketNumber(1);
+        podState.increasePacketNumber();
     }
 
     private OmnipodPacket exchangePackets(PodState podState, OmnipodPacket packet) {
@@ -300,6 +309,8 @@ public class OmnipodCommunicationService extends RileyLinkCommunicationManager {
     private OmnipodPacket exchangePackets(PodState podState, OmnipodPacket packet, int repeatCount, int responseTimeoutMilliseconds, int exchangeTimeoutMilliseconds, int preambleExtensionMilliseconds) {
         long timeoutTime = System.currentTimeMillis() + exchangeTimeoutMilliseconds;
 
+        podState.increasePacketNumber();
+
         while (System.currentTimeMillis() < timeoutTime) {
             OmnipodPacket response = null;
             try {
@@ -311,17 +322,34 @@ public class OmnipodCommunicationService extends RileyLinkCommunicationManager {
             } catch (Exception ex) {
                 throw new CommunicationException(CommunicationException.Type.UNEXPECTED_EXCEPTION, ex);
             }
-            if (response == null || !response.isValid()) {
+            if (response == null) {
+                if (isLoggingEnabled()) {
+                    LOG.debug("exchangePackets response is null");
+                }
+                continue;
+            } else if (!response.isValid()) {
+                if (isLoggingEnabled()) {
+                    LOG.debug("exchangePackets response is invalid: " + response);
+                }
                 continue;
             }
             if (response.getAddress() != packet.getAddress()) {
-                continue;
+                if (isLoggingEnabled()) {
+                    LOG.debug("Packet address " + response.getAddress() + " doesn't match " + packet.getAddress());
                 }
-            if (response.getSequenceNumber() != ((podState.getPacketNumber() + 1) & 0b11111)) {
                 continue;
             }
 
-            podState.increasePacketNumber(2);
+            if (response.getSequenceNumber() != podState.getPacketNumber()) {
+                if (isLoggingEnabled()) {
+                    LOG.debug("Packet sequence number " + response.getSequenceNumber() + " does not match " + podState.getPacketNumber());
+                }
+                continue;
+            }
+
+            // Once we have verification that the POD heard us, we can increment our counters
+            podState.increasePacketNumber();
+
             return response;
         }
         throw new CommunicationException(CommunicationException.Type.TIMEOUT);

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

@@ -0,0 +1,22 @@
+package info.nightscout.androidaps.plugins.pump.omnipod.comm.exception;
+
+import info.nightscout.androidaps.plugins.pump.omnipod.exception.OmnipodException;
+
+public class IllegalSequenceNumberException extends OmnipodException {
+    private final int expected;
+    private final int actual;
+
+    public IllegalSequenceNumberException(int expected, int actual) {
+        super("Invalid sequence number. Expected="+ expected +", actual="+ actual, false);
+        this.expected = expected;
+        this.actual = actual;
+    }
+
+    public int getExpected() {
+        return expected;
+    }
+
+    public int getActual() {
+        return actual;
+    }
+}

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

@@ -1,5 +1,7 @@
 package info.nightscout.androidaps.plugins.pump.omnipod.comm.message;
 
+import java.util.Arrays;
+
 import info.nightscout.androidaps.plugins.pump.common.hw.rileylink.ble.data.RLMessage;
 import info.nightscout.androidaps.plugins.pump.common.utils.ByteUtil;
 import info.nightscout.androidaps.plugins.pump.omnipod.defs.PacketType;
@@ -79,4 +81,14 @@ public class OmnipodPacket implements RLMessage {
         return valid;
     }
 
+    @Override
+    public String toString() {
+        return "OmnipodPacket{" +
+                "packetAddress=" + packetAddress +
+                ", packetType=" + packetType +
+                ", sequenceNumber=" + sequenceNumber +
+                ", encodedMessage=" + ByteUtil.shortHexStringWithoutSpaces(encodedMessage) +
+                ", valid=" + valid +
+                '}';
+    }
 }

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

@@ -46,12 +46,12 @@ public abstract class PodState {
         this.packetNumber = packetNumber;
     }
 
-    public void increaseMessageNumber(int increment) {
+    public void increaseMessageNumber() {
-        setMessageNumber((messageNumber + increment) & 0b1111);
+        setMessageNumber((messageNumber + 1) & 0b1111);
     }
 
-    public void increasePacketNumber(int increment) {
+    public void increasePacketNumber() {
-        setPacketNumber((packetNumber + increment) & 0b11111);
+        setPacketNumber((packetNumber + 1) & 0b11111);
     }
 
     public boolean hasFaultEvent() {

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

@@ -37,6 +37,7 @@ import info.nightscout.androidaps.plugins.pump.common.utils.DateTimeUtil;
 import info.nightscout.androidaps.plugins.pump.omnipod.comm.OmnipodCommunicationService;
 import info.nightscout.androidaps.plugins.pump.omnipod.comm.OmnipodManager;
 import info.nightscout.androidaps.plugins.pump.omnipod.comm.SetupActionResult;
+import info.nightscout.androidaps.plugins.pump.omnipod.comm.exception.IllegalSequenceNumberException;
 import info.nightscout.androidaps.plugins.pump.omnipod.comm.message.response.StatusResponse;
 import info.nightscout.androidaps.plugins.pump.omnipod.comm.message.response.podinfo.PodInfoRecentPulseLog;
 import info.nightscout.androidaps.plugins.pump.omnipod.comm.message.response.podinfo.PodInfoResponse;
@@ -612,6 +613,8 @@ public class AapsOmnipodManager implements OmnipodCommunicationManagerInterface
                 comment = getStringResource(R.string.omnipod_driver_error_invalid_progress_state);
             } else if (ex instanceof IllegalResponseException) {
                 comment = getStringResource(R.string.omnipod_driver_error_invalid_response);
+            } else if (ex instanceof IllegalSequenceNumberException) {
+                comment = getStringResource(R.string.omnipod_driver_error_invalid_sequence_number);
             } else if (ex instanceof MessageDecodingException) {
                 comment = getStringResource(R.string.omnipod_driver_error_message_decoding_failed);
             } else if (ex instanceof NonceOutOfSyncException) {

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

@@ -1756,6 +1756,7 @@
     <string name="omnipod_driver_error_invalid_packet_type">Communication failed: received an invalid packet from the Pod.</string>
     <string name="omnipod_driver_error_invalid_progress_state">Communication failed: the Pod is in a wrong state.</string>
     <string name="omnipod_driver_error_invalid_response">Communication failed: received an invalid response from the Pod.</string>
+    <string name="omnipod_driver_error_invalid_sequence_number">Communication failed: received a message with an invalid sequence number from the Pod.</string>
     <string name="omnipod_driver_error_message_decoding_failed">Communication failed: failed to decode message from the Pod.</string>
     <string name="omnipod_driver_error_nonce_resync_failed">Communication failed: nonce resync failed.</string>
     <string name="omnipod_driver_error_nonce_out_of_sync">Communication failed: nonce out of sync.</string>