From 9a5cd3c9021916aa590d96e541fb42127056e665 Mon Sep 17 00:00:00 2001 From: Bart Sopers Date: Sun, 26 Apr 2020 21:11:47 +0200 Subject: [PATCH] Add some checks and logging in Omnipod packet and message exchange --- .../pump/omnipod/OmnipodPumpPlugin.java | 2 - .../comm/OmnipodCommunicationService.java | 64 +++++++++++++------ .../IllegalSequenceNumberException.java | 22 +++++++ .../omnipod/comm/message/OmnipodPacket.java | 12 ++++ .../pump/omnipod/defs/state/PodState.java | 8 +-- .../driver/comm/AapsOmnipodManager.java | 3 + app/src/main/res/values/strings.xml | 1 + 7 files changed, 88 insertions(+), 24 deletions(-) create mode 100644 app/src/main/java/info/nightscout/androidaps/plugins/pump/omnipod/comm/exception/IllegalSequenceNumberException.java diff --git a/app/src/main/java/info/nightscout/androidaps/plugins/pump/omnipod/OmnipodPumpPlugin.java b/app/src/main/java/info/nightscout/androidaps/plugins/pump/omnipod/OmnipodPumpPlugin.java index 39b8075e88..cb575a2246 100644 --- a/app/src/main/java/info/nightscout/androidaps/plugins/pump/omnipod/OmnipodPumpPlugin.java +++ b/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; diff --git a/app/src/main/java/info/nightscout/androidaps/plugins/pump/omnipod/comm/OmnipodCommunicationService.java b/app/src/main/java/info/nightscout/androidaps/plugins/pump/omnipod/comm/OmnipodCommunicationService.java index 2fc3a4339a..9ab8693acf 100644 --- a/app/src/main/java/info/nightscout/androidaps/plugins/pump/omnipod/comm/OmnipodCommunicationService.java +++ b/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 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 (response.getSequenceNumber() != ((podState.getPacketNumber() + 1) & 0b11111)) { + if (isLoggingEnabled()) { + LOG.debug("Packet address " + response.getAddress() + " doesn't match " + packet.getAddress()); + } 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); diff --git a/app/src/main/java/info/nightscout/androidaps/plugins/pump/omnipod/comm/exception/IllegalSequenceNumberException.java b/app/src/main/java/info/nightscout/androidaps/plugins/pump/omnipod/comm/exception/IllegalSequenceNumberException.java new file mode 100644 index 0000000000..d47b1e8d0b --- /dev/null +++ b/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; + } +} diff --git a/app/src/main/java/info/nightscout/androidaps/plugins/pump/omnipod/comm/message/OmnipodPacket.java b/app/src/main/java/info/nightscout/androidaps/plugins/pump/omnipod/comm/message/OmnipodPacket.java index fd90ffb640..157e72e51b 100644 --- a/app/src/main/java/info/nightscout/androidaps/plugins/pump/omnipod/comm/message/OmnipodPacket.java +++ b/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 + + '}'; + } } diff --git a/app/src/main/java/info/nightscout/androidaps/plugins/pump/omnipod/defs/state/PodState.java b/app/src/main/java/info/nightscout/androidaps/plugins/pump/omnipod/defs/state/PodState.java index 2e19de1286..c6987c879e 100644 --- a/app/src/main/java/info/nightscout/androidaps/plugins/pump/omnipod/defs/state/PodState.java +++ b/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) { - setMessageNumber((messageNumber + increment) & 0b1111); + public void increaseMessageNumber() { + setMessageNumber((messageNumber + 1) & 0b1111); } - public void increasePacketNumber(int increment) { - setPacketNumber((packetNumber + increment) & 0b11111); + public void increasePacketNumber() { + setPacketNumber((packetNumber + 1) & 0b11111); } public boolean hasFaultEvent() { diff --git a/app/src/main/java/info/nightscout/androidaps/plugins/pump/omnipod/driver/comm/AapsOmnipodManager.java b/app/src/main/java/info/nightscout/androidaps/plugins/pump/omnipod/driver/comm/AapsOmnipodManager.java index 8399bfdf4b..bbc5345618 100644 --- a/app/src/main/java/info/nightscout/androidaps/plugins/pump/omnipod/driver/comm/AapsOmnipodManager.java +++ b/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) { diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 2d5da40098..be040f4ac8 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -1756,6 +1756,7 @@ Communication failed: received an invalid packet from the Pod. Communication failed: the Pod is in a wrong state. Communication failed: received an invalid response from the Pod. + Communication failed: received a message with an invalid sequence number from the Pod. Communication failed: failed to decode message from the Pod. Communication failed: nonce resync failed. Communication failed: nonce out of sync.