Medtrum: Refactor Notification packet;

Reduce cognitive complexity
    add check for size of maskedMessage

pump/medtrum/src/main/java/info/nightscout/pump/medtrum/comm/packets/NotificationPacket.kt

@@ -12,6 +12,8 @@ import info.nightscout.pump.medtrum.extension.toLong
 import info.nightscout.pump.medtrum.util.MedtrumTimeUtil
 import javax.inject.Inject
 
+typealias MaskHandler = (ByteArray, Int) -> Int
+
 class NotificationPacket(val injector: HasAndroidInjector) {
 
     /**
@@ -53,14 +55,53 @@ class NotificationPacket(val injector: HasAndroidInjector) {
         private const val MASK_STORAGE = 0x100
         private const val MASK_ALARM = 0x200
         private const val MASK_AGE = 0x400
-        private const val MASK_UNKNOWN_1 = 0x800
+        private const val MASK_MAGNETO_PLACE = 0x800
 
         private const val MASK_UNUSED_CGM = 0x1000
         private const val MASK_UNUSED_COMMAND_CONFIRM = 0x2000
         private const val MASK_UNUSED_AUTO_STATUS = 0x4000
         private const val MASK_UNUSED_LEGACY = 0x8000
+
+        private const val SIZE_FIELD_MASK = 2
+        private const val SIZE_SUSPEND = 4
+        private const val SIZE_NORMAL_BOLUS = 3
+        private const val SIZE_EXTENDED_BOLUS = 3
+        private const val SIZE_BASAL = 12
+        private const val SIZE_SETUP = 1
+        private const val SIZE_RESERVOIR = 2
+        private const val SIZE_START_TIME = 4
+        private const val SIZE_BATTERY = 3
+        private const val SIZE_STORAGE = 4
+        private const val SIZE_ALARM = 4
+        private const val SIZE_AGE = 4
+        private const val SIZE_MAGNETO_PLACE = 2
+        private const val SIZE_UNUSED_CGM = 5
+        private const val SIZE_UNUSED_COMMAND_CONFIRM = 2
+        private const val SIZE_UNUSED_AUTO_STATUS = 2
+        private const val SIZE_UNUSED_LEGACY = 2
     }
 
+    val maskHandlers: Map<Int, MaskHandler> = mapOf(
+        MASK_SUSPEND to ::handleSuspend,
+        MASK_NORMAL_BOLUS to ::handleNormalBolus,
+        MASK_EXTENDED_BOLUS to ::handleExtendedBolus,
+        MASK_BASAL to ::handleBasal,
+        MASK_SETUP to ::handleSetup,
+        MASK_RESERVOIR to ::handleReservoir,
+        MASK_START_TIME to ::handleStartTime,
+        MASK_BATTERY to ::handleBattery,
+        MASK_STORAGE to ::handleStorage,
+        MASK_ALARM to ::handleAlarm,
+        MASK_AGE to ::handleAge,
+        MASK_MAGNETO_PLACE to ::handleUnknown1,
+        MASK_UNUSED_CGM to ::handleUnusedCGM,
+        MASK_UNUSED_COMMAND_CONFIRM to ::handleUnusedCommandConfirm,
+        MASK_UNUSED_AUTO_STATUS to ::handleUnusedAutoStatus,
+        MASK_UNUSED_LEGACY to ::handleUnusedLegacy
+    )
+
+    var newPatchStartTime = 0L
+
     init {
         injector.androidInjector().inject(this)
     }
@@ -74,7 +115,7 @@ class NotificationPacket(val injector: HasAndroidInjector) {
             medtrumPump.pumpState = state
         }
 
-        if (notification.size > NOTIF_STATE_END) {
+        if (notification.size > NOTIF_STATE_END + SIZE_FIELD_MASK) {
             handleMaskedMessage(notification.copyOfRange(NOTIF_STATE_END, notification.size))
         }
     }
@@ -82,21 +123,66 @@ class NotificationPacket(val injector: HasAndroidInjector) {
     /**
      * Handle a message with a field mask, can be used by other packets as well
      */
-    fun handleMaskedMessage(data: ByteArray) {
+    fun handleMaskedMessage(data: ByteArray): Boolean {
         val fieldMask = data.copyOfRange(0, 2).toInt()
         var offset = 2
-        var newPatchStartTime: Long? = null
+    
+        val expectedLength = calculateExpectedLengthBasedOnFieldMask(fieldMask)
+        if (data.size < expectedLength) {
+            aapsLogger.error(LTag.PUMPCOMM, "Incorrect message length. Expected at least $expectedLength bytes.")
+            return false
+        }
     
         aapsLogger.debug(LTag.PUMPCOMM, "Message field mask: $fieldMask")
     
-        if (fieldMask and MASK_SUSPEND != 0) {
+        for ((mask, handler) in maskHandlers) {
+            if (fieldMask and mask != 0) {
+                offset = handler(data, offset)
+            }
+        }
+    
+        return true
+    }
+
+    private fun calculateExpectedLengthBasedOnFieldMask(fieldMask: Int): Int {
+        var expectedLength = SIZE_FIELD_MASK
+        
+        val sizeMap = mapOf(
+            MASK_SUSPEND to SIZE_SUSPEND,
+            MASK_NORMAL_BOLUS to SIZE_NORMAL_BOLUS,
+            MASK_EXTENDED_BOLUS to SIZE_EXTENDED_BOLUS,
+            MASK_BASAL to SIZE_BASAL,
+            MASK_SETUP to SIZE_SETUP,
+            MASK_RESERVOIR to SIZE_RESERVOIR,
+            MASK_START_TIME to SIZE_START_TIME,
+            MASK_BATTERY to SIZE_BATTERY,
+            MASK_STORAGE to SIZE_STORAGE,
+            MASK_ALARM to SIZE_ALARM,
+            MASK_AGE to SIZE_AGE,
+            MASK_MAGNETO_PLACE to SIZE_MAGNETO_PLACE,
+            MASK_UNUSED_CGM to SIZE_UNUSED_CGM,
+            MASK_UNUSED_COMMAND_CONFIRM to SIZE_UNUSED_COMMAND_CONFIRM,
+            MASK_UNUSED_AUTO_STATUS to SIZE_UNUSED_AUTO_STATUS,
+            MASK_UNUSED_LEGACY to SIZE_UNUSED_LEGACY
+        )
+    
+        for ((mask, size) in sizeMap) {
+            if (fieldMask and mask != 0) {
+                expectedLength += size
+            }
+        }
+    
+        return expectedLength
+    }
+
+    private fun handleSuspend(data: ByteArray, offset: Int): Int {
         aapsLogger.debug(LTag.PUMPCOMM, "Suspend notification received")
         medtrumPump.suspendTime = MedtrumTimeUtil().convertPumpTimeToSystemTimeMillis(data.copyOfRange(offset, offset + 4).toLong())
         aapsLogger.debug(LTag.PUMPCOMM, "Suspend time: ${medtrumPump.suspendTime}")
-            offset += 4
+        return offset + SIZE_SUSPEND
     }
     
-        if (fieldMask and MASK_NORMAL_BOLUS != 0) {
+    private fun handleNormalBolus(data: ByteArray, offset: Int): Int {
         aapsLogger.debug(LTag.PUMPCOMM, "Normal bolus notification received")
         val bolusData = data.copyOfRange(offset, offset + 1).toInt()
         val bolusType = bolusData and 0x7F
@@ -104,15 +190,16 @@ class NotificationPacket(val injector: HasAndroidInjector) {
         val bolusDelivered = data.copyOfRange(offset + 1, offset + 3).toInt() * 0.05
         aapsLogger.debug(LTag.PUMPCOMM, "Bolus type: $bolusType, bolusData: $bolusData bolus completed: $bolusCompleted, bolus delivered: $bolusDelivered")
         medtrumPump.handleBolusStatusUpdate(bolusType, bolusCompleted, bolusDelivered)
-            offset += 3
+        return offset + SIZE_NORMAL_BOLUS
     }
     
-        if (fieldMask and MASK_EXTENDED_BOLUS != 0) {
+    private fun handleExtendedBolus(data: ByteArray, offset: Int): Int {
         aapsLogger.error(LTag.PUMPCOMM, "Extended bolus notification received, extended bolus not supported!")
-            offset += 3
+        aapsLogger.debug(LTag.PUMPCOMM, "Extended bolus data: ${data.copyOfRange(offset, offset + SIZE_EXTENDED_BOLUS).toLong()}")
+        return offset + SIZE_EXTENDED_BOLUS
     }
     
-        if (fieldMask and MASK_BASAL != 0) {
+    private fun handleBasal(data: ByteArray, offset: Int): Int {
         aapsLogger.debug(LTag.PUMPCOMM, "Basal notification received")
         val basalType = enumValues<BasalType>()[data.copyOfRange(offset, offset + 1).toInt()]
         val basalSequence = data.copyOfRange(offset + 1, offset + 3).toInt()
@@ -129,24 +216,24 @@ class NotificationPacket(val injector: HasAndroidInjector) {
         if (medtrumPump.lastBasalRate != basalRate || medtrumPump.lastBasalStartTime != basalStartTime) {
             medtrumPump.handleBasalStatusUpdate(basalType, basalRate, basalSequence, basalPatchId, basalStartTime)
         }
-            offset += 12
+        return offset + SIZE_BASAL
     }
     
-        if (fieldMask and MASK_SETUP != 0) {
+    private fun handleSetup(data: ByteArray, offset: Int): Int {
         aapsLogger.debug(LTag.PUMPCOMM, "Setup notification received")
         medtrumPump.primeProgress = data.copyOfRange(offset, offset + 1).toInt()
         aapsLogger.debug(LTag.PUMPCOMM, "Prime progress: ${medtrumPump.primeProgress}")
-            offset += 1
+        return offset + SIZE_SETUP
     }
     
-        if (fieldMask and MASK_RESERVOIR != 0) {
+    private fun handleReservoir(data: ByteArray, offset: Int): Int {
         aapsLogger.debug(LTag.PUMPCOMM, "Reservoir notification received")
         medtrumPump.reservoir = data.copyOfRange(offset, offset + 2).toInt() * 0.05
         aapsLogger.debug(LTag.PUMPCOMM, "Reservoir: ${medtrumPump.reservoir}")
-            offset += 2
+        return offset + SIZE_RESERVOIR
     }
     
-        if (fieldMask and MASK_START_TIME != 0) {
+    private fun handleStartTime(data: ByteArray, offset: Int): Int {
         aapsLogger.debug(LTag.PUMPCOMM, "Start time notification received")
         newPatchStartTime = MedtrumTimeUtil().convertPumpTimeToSystemTimeMillis(data.copyOfRange(offset, offset + 4).toLong())
         if (medtrumPump.patchStartTime != newPatchStartTime) {
@@ -154,20 +241,20 @@ class NotificationPacket(val injector: HasAndroidInjector) {
             medtrumPump.patchStartTime = newPatchStartTime
         }
         aapsLogger.debug(LTag.PUMPCOMM, "Patch start time: $newPatchStartTime")
-            offset += 4
+        return offset + SIZE_START_TIME
     }
     
-        if (fieldMask and MASK_BATTERY != 0) {
+    private fun handleBattery(data: ByteArray, offset: Int): Int {
         aapsLogger.debug(LTag.PUMPCOMM, "Battery notification received")
         val parameter = data.copyOfRange(offset, offset + 3).toInt()
         // Precision for voltage A is a guess, voltage B is the important one, threshold: < 2.64
         medtrumPump.batteryVoltage_A = (parameter and 0xFFF) / 512.0
         medtrumPump.batteryVoltage_B = (parameter shr 12) / 512.0
         aapsLogger.debug(LTag.PUMPCOMM, "Battery voltage A: ${medtrumPump.batteryVoltage_A}, battery voltage B: ${medtrumPump.batteryVoltage_B}")
-            offset += 3
+        return offset + SIZE_BATTERY
     }
     
-        if (fieldMask and MASK_STORAGE != 0) {
+    private fun handleStorage(data: ByteArray, offset: Int): Int {
         aapsLogger.debug(LTag.PUMPCOMM, "Storage notification received")
         val sequence = data.copyOfRange(offset, offset + 2).toInt()
         if (sequence > medtrumPump.currentSequenceNumber) {
@@ -176,17 +263,17 @@ class NotificationPacket(val injector: HasAndroidInjector) {
         val patchId = data.copyOfRange(offset + 2, offset + 4).toLong()
         if (patchId != medtrumPump.patchId) {
             aapsLogger.warn(LTag.PUMPCOMM, "handleMaskedMessage: We got wrong patch id!")
-                if (newPatchStartTime != null) {
+            if (newPatchStartTime != 0L) {
                 // This is a fallback for when the activate packet did not receive the ack but the patch activated anyway
                 aapsLogger.error(LTag.PUMPCOMM, "handleMaskedMessage: Also Received start time in this packet, registering new patch id: $patchId")
                 medtrumPump.handleNewPatch(patchId, sequence, newPatchStartTime)
             }
         }
         aapsLogger.debug(LTag.PUMPCOMM, "Last known sequence number: ${medtrumPump.currentSequenceNumber}, patch id: ${patchId}")
-            offset += 4
+        return offset + SIZE_STORAGE
     }
     
-        if (fieldMask and MASK_ALARM != 0) {
+    private fun handleAlarm(data: ByteArray, offset: Int): Int {
         val alarmFlags = data.copyOfRange(offset, offset + 2).toInt()
         val alarmParameter = data.copyOfRange(offset + 2, offset + 4).toInt()
         aapsLogger.debug(LTag.PUMPCOMM, "Alarm notification received, Alarm flags: $alarmFlags, alarm parameter: $alarmParameter")
@@ -211,35 +298,44 @@ class NotificationPacket(val injector: HasAndroidInjector) {
                 }
             }
         }
-            offset += 4
+        return offset + SIZE_ALARM
     }
     
-        if (fieldMask and MASK_AGE != 0) {
+    private fun handleAge(data: ByteArray, offset: Int): Int {
         aapsLogger.debug(LTag.PUMPCOMM, "Age notification received")
         medtrumPump.patchAge = data.copyOfRange(offset, offset + 4).toLong()
         aapsLogger.debug(LTag.PUMPCOMM, "Patch age: ${medtrumPump.patchAge}")
-            offset += 4
+        return offset + SIZE_AGE
     }
     
-        if (fieldMask and MASK_UNKNOWN_1 != 0) {
+    private fun handleUnknown1(data: ByteArray, offset: Int): Int {
-            aapsLogger.debug(LTag.PUMPCOMM, "Unknown 1 notification received, not handled!")
+        aapsLogger.debug(LTag.PUMPCOMM, "Magneto placement notification received!")
+        val magnetoPlacement = data.copyOfRange(offset, offset + 2).toInt()
+        aapsLogger.debug(LTag.PUMPCOMM, "Magneto placement: $magnetoPlacement")
+        return offset + SIZE_MAGNETO_PLACE
     }
     
-        if (fieldMask and MASK_UNUSED_CGM != 0) {
+    private fun handleUnusedCGM(data: ByteArray, offset: Int): Int {
         aapsLogger.debug(LTag.PUMPCOMM, "Unused CGM notification received, not handled!")
+        aapsLogger.debug(LTag.PUMPCOMM, "Unused CGM data: ${data.copyOfRange(offset, offset + SIZE_UNUSED_CGM).toLong()}")
+        return offset + SIZE_UNUSED_CGM
     }
     
-        if (fieldMask and MASK_UNUSED_COMMAND_CONFIRM != 0) {
+    private fun handleUnusedCommandConfirm(data: ByteArray, offset: Int): Int {
-            // This one is a warning, as this happens we need to know about it, and maybe implement
         aapsLogger.warn(LTag.PUMPCOMM, "Unused command confirm notification received, not handled!")
+        aapsLogger.debug(LTag.PUMPCOMM, "Unused command confirm data: ${data.copyOfRange(offset, offset + SIZE_UNUSED_COMMAND_CONFIRM).toLong()}")
+        return offset + SIZE_UNUSED_COMMAND_CONFIRM
     }
     
-        if (fieldMask and MASK_UNUSED_AUTO_STATUS != 0) {
+    private fun handleUnusedAutoStatus(data: ByteArray, offset: Int): Int {
         aapsLogger.debug(LTag.PUMPCOMM, "Unused auto status notification received, not handled!")
+        aapsLogger.debug(LTag.PUMPCOMM, "Unused auto status data: ${data.copyOfRange(offset, offset + SIZE_UNUSED_AUTO_STATUS).toLong()}")
+        return offset + SIZE_UNUSED_AUTO_STATUS
     }
     
-        if (fieldMask and MASK_UNUSED_LEGACY != 0) {
+    private fun handleUnusedLegacy(data: ByteArray, offset: Int): Int {
         aapsLogger.debug(LTag.PUMPCOMM, "Unused legacy notification received, not handled!")
-        }
+        aapsLogger.debug(LTag.PUMPCOMM, "Unused legacy data: ${data.copyOfRange(offset, offset + SIZE_UNUSED_LEGACY).toLong()}")
+        return offset + SIZE_UNUSED_LEGACY
     }
 }

pump/medtrum/src/main/java/info/nightscout/pump/medtrum/comm/packets/SynchronizePacket.kt

@@ -31,7 +31,7 @@ class SynchronizePacket(injector: HasAndroidInjector) : MedtrumPacket(injector)
     }
 
     override fun handleResponse(data: ByteArray): Boolean {
-        val success = super.handleResponse(data)
+        var success = super.handleResponse(data)
         if (success) {
             val state = MedtrumPumpState.fromByte(data[RESP_STATE_START])
 
@@ -63,7 +63,7 @@ class SynchronizePacket(injector: HasAndroidInjector) : MedtrumPacket(injector)
             }
 
             // Let the notification packet handle the rest of the sync data
-            NotificationPacket(injector).handleMaskedMessage(fieldMask.toByteArray(2) + syncData)
+            success = NotificationPacket(injector).handleMaskedMessage(fieldMask.toByteArray(2) + syncData)
         }
 
         return success

pump/medtrum/src/test/java/info/nightscout/pump/medtrum/comm/packets/NotificationPacketTest.kt

@@ -87,4 +87,17 @@ class NotificationPacketTest : MedtrumTestBase() {
         assertThat(medtrumPump.bolusingTreatment!!.insulin).isWithin(0.01).of(1.65)
         assertThat(medtrumPump.reservoir).isWithin(0.01).of(161.95)
     }
+
+    @Test fun handleNotificationGivenFieldMaskButMessageTooShortThenNothingSaved() {
+        // Inputs
+        val data = byteArrayOf(67, 41, 67, -1, 122, 95, 18, 0, 73, 1, 19, 0, 1, 0, 20, 0, 0, 0, 0, 16)
+
+        // Call
+        NotificationPacket(packetInjector).handleNotification(data)
+
+        // Expected values
+        assertThat(medtrumPump.suspendTime).isEqualTo(0)
+        assertThat(medtrumPump.lastBasalStartTime).isEqualTo(0)
+        assertThat(medtrumPump.currentSequenceNumber).isEqualTo(0)
+    }
 }