Huawei: Quick fix issue with concatenated packets.

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/devices/huawei/HuaweiPacket.java

@@ -235,7 +235,7 @@ public class HuaweiPacket {
         }
     }
 
-    protected static final int PACKET_MINIMAL_SIZE = 6;
+    protected static final int PACKET_MINIMAL_SIZE = 3; // magic + data size
 
     protected ParamsProvider paramsProvider;
 
@@ -288,6 +288,7 @@ public class HuaweiPacket {
         this.partialPacket = packet.partialPacket;
         this.payload = packet.payload;
         this.complete = packet.complete;
+        this.left = packet.left;
 
         if (packet.isEncrypted)
             this.isEncrypted = true;
@@ -303,7 +304,14 @@ public class HuaweiPacket {
      */
     public void parseTlv() throws ParseException {}
 
+    private int left = 0;
+
+    public int getLeft() {
+        return this.left;
+    }
+
     private void parseData(byte[] data) throws ParseException {
+        this.left = 0;
         if (partialPacket != null) {
             int newCapacity = partialPacket.length + data.length;
             data = ByteBuffer.allocate(newCapacity)
@@ -313,23 +321,13 @@ public class HuaweiPacket {
         }
 
         ByteBuffer buffer = ByteBuffer.wrap(data);
-
-        if (buffer.capacity() < PACKET_MINIMAL_SIZE) {
+        if (buffer.capacity() < 1) {
             throw new LengthMismatchException("Packet length mismatch : "
                     + buffer.capacity()
-                    + " != 6");
+                    + " < 1");
         }
 
         byte magic = buffer.get();
-        short expectedSize = buffer.getShort();
-        int isSliced = buffer.get();
-        if (isSliced == 1 || isSliced == 2 || isSliced == 3) {
-            buffer.get(); // Throw away slice flag
-        }
-        byte[] newPayload = new byte[buffer.remaining() - 2];
-        buffer.get(newPayload, 0, buffer.remaining() - 2);
-        short expectedChecksum = buffer.getShort();
-        buffer.rewind();
 
         if (magic != HUAWEI_MAGIC) {
             throw new MagicMismatchException("Magic mismatch : "
@@ -337,26 +335,40 @@ public class HuaweiPacket {
                     + " != 0x5A");
         }
 
-        int newPayloadLen = newPayload.length + 1;
-        if (isSliced == 1 || isSliced == 2 || isSliced == 3) {
-            newPayloadLen = newPayload.length + 2;
+        if (buffer.capacity() < PACKET_MINIMAL_SIZE) {
+            this.partialPacket = data;
+            return;
         }
-        if (expectedSize != (short) newPayloadLen) {
-            if (expectedSize > (short) newPayloadLen) {
-                // Older band and BT version do not handle message with more than 256 bits.
-                this.partialPacket = data;
-                return;
-            } else {
-                throw new LengthMismatchException("Expected length mismatch : "
-                    + expectedSize
-                    + " < "
-                    + (short) newPayloadLen);
-            }
+
+        short expectedSize = buffer.getShort();
+
+        if(expectedSize < 0) {
+            throw new LengthMismatchException("Expected length mismatch : " + expectedSize);
         }
+
+        if (expectedSize + 2 > buffer.remaining()) {
+            // Older band and BT version do not handle message with more than 256 bits.
+            this.partialPacket = data;
+            return;
+        }
+
         this.partialPacket = null;
 
-        byte[] dataNoCRC = new byte[buffer.capacity() - 2];
-        buffer.get(dataNoCRC, 0, buffer.capacity() - 2);
+        int addLen = 1;
+        int isSliced = buffer.get();
+        if (isSliced == 1 || isSliced == 2 || isSliced == 3) {
+            buffer.get(); // Throw away slice flag
+            addLen++;
+        }
+
+        byte[] newPayload = new byte[expectedSize - addLen];
+        buffer.get(newPayload, 0, expectedSize - addLen);
+        short expectedChecksum = buffer.getShort();
+        this.left = buffer.remaining();
+        buffer.rewind();
+
+        byte[] dataNoCRC = new byte[expectedSize + 3];
+        buffer.get(dataNoCRC, 0, expectedSize + 3);
         short actualChecksum = (short) CheckSums.getCRC16(dataNoCRC, 0x0000);
         if (actualChecksum != expectedChecksum) {
             throw new ChecksumIncorrectException("Checksum mismatch : "
@@ -388,7 +400,8 @@ public class HuaweiPacket {
         if (
                 (serviceId == 0x0a && commandId == 0x05) ||
                 (serviceId == 0x28 && commandId == 0x06) ||
-                (serviceId == 0x2c && commandId == 0x05)
+                (serviceId == 0x2c && commandId == 0x05) ||
+                (serviceId == 0x1c && commandId == 0x05)
         ) {
             // TODO: this doesn't seem to be TLV
             this.payload = newPayload;

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/huawei/HuaweiSupportProvider.java

@@ -16,6 +16,8 @@
     along with this program.  If not, see <https://www.gnu.org/licenses/>. */
 package nodomain.freeyourgadget.gadgetbridge.service.devices.huawei;
 
+import static nodomain.freeyourgadget.gadgetbridge.devices.huawei.HuaweiConstants.HUAWEI_MAGIC;
+
 import android.bluetooth.BluetoothGattCharacteristic;
 import android.content.Context;
 import android.content.SharedPreferences;
@@ -934,23 +936,7 @@ public class HuaweiSupportProvider {
     }
 
     public void onSocketRead(byte[] data) {
-        // The data can contain multiple packets, which need to be split.
-        // But we also need to take into account partial packets (where data does not contain a full packet)
-        if (data[0] != 0x5a) {
-            // Part of partial packet, just parse
             responseManager.handleData(data);
-            return;
-        }
-
-        ByteBuffer bData = ByteBuffer.wrap(data);
-        while (bData.remaining() != 0) {
-            int dataLen = bData.getShort(bData.position() + 1) + 0x05;
-            if (dataLen > bData.remaining())
-                dataLen = bData.remaining(); // Part of partial packet, just parse the remainder
-            byte[] newData = new byte[dataLen];
-            bData.get(newData, 0, dataLen);
-            responseManager.handleData(newData);
-        }
     }
 
     public void removeInProgressRequests(Request req) {

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/huawei/ResponseManager.java

@@ -20,6 +20,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 
@@ -80,47 +81,57 @@ public class ResponseManager {
      * @param data The received data
      */
     public void handleData(byte[] data) {
-        try {
-            if (receivedPacket == null)
-                receivedPacket = new HuaweiPacket(support.getParamsProvider()).parse(data);
-            else
-                receivedPacket = receivedPacket.parse(data);
-        } catch (HuaweiPacket.ParseException e) {
-            LOG.error("Packet parse exception", e);
+        //NOTE: This is a quick fix issue with concatenated packets.
+        //TODO: Extract transport related code from packet.
+        int left = 0;
+        do {
+            if(left > 0)
+                data = Arrays.copyOfRange(data, data.length - left, data.length);
 
-            // Clean up so the next message may be parsed correctly
-            this.receivedPacket = null;
-            return;
-        }
+            try {
+                if (receivedPacket == null)
+                    receivedPacket = new HuaweiPacket(support.getParamsProvider()).parse(data);
+                else
+                    receivedPacket = receivedPacket.parse(data);
 
-        if (receivedPacket.complete) {
-            Request handler = null;
-            synchronized (handlers) {
-                for (Request req : handlers) {
-                    if (req.handleResponse(receivedPacket)) {
-                        handler = req;
-                        break;
-                    }
-                }
+                left = receivedPacket.getLeft();
+            } catch (HuaweiPacket.ParseException e) {
+                LOG.error("Packet parse exception", e);
+
+                // Clean up so the next message may be parsed correctly
+                this.receivedPacket = null;
+                return;
             }
 
-            if (handler == null) {
-                LOG.debug("Service: " + Integer.toHexString(receivedPacket.serviceId & 0xff) + ", command: " + Integer.toHexString(receivedPacket.commandId & 0xff)  + ", asynchronous response.");
-
-                // Asynchronous response
-                asynchronousResponse.handleResponse(receivedPacket);
-            } else {
-                LOG.debug("Service: " + Integer.toHexString(receivedPacket.serviceId & 0xff)  + ", command: " + Integer.toHexString(receivedPacket.commandId & 0xff)  + ", handled by: " + handler.getClass());
-
-                if (handler.autoRemoveFromResponseHandler()) {
-                    synchronized (handlers) {
-                        handlers.remove(handler);
+            if (receivedPacket.complete) {
+                Request handler = null;
+                synchronized (handlers) {
+                    for (Request req : handlers) {
+                        if (req.handleResponse(receivedPacket)) {
+                            handler = req;
+                            break;
+                        }
                     }
                 }
 
-                handler.handleResponse();
+                if (handler == null) {
+                    LOG.debug("Service: " + Integer.toHexString(receivedPacket.serviceId & 0xff) + ", command: " + Integer.toHexString(receivedPacket.commandId & 0xff) + ", asynchronous response.");
+
+                    // Asynchronous response
+                    asynchronousResponse.handleResponse(receivedPacket);
+                } else {
+                    LOG.debug("Service: " + Integer.toHexString(receivedPacket.serviceId & 0xff) + ", command: " + Integer.toHexString(receivedPacket.commandId & 0xff) + ", handled by: " + handler.getClass());
+
+                    if (handler.autoRemoveFromResponseHandler()) {
+                        synchronized (handlers) {
+                            handlers.remove(handler);
+                        }
+                    }
+
+                    handler.handleResponse();
+                }
+                receivedPacket = null;
             }
-            receivedPacket = null;
-        }
+        } while (left > 0);
     }
 }

app/src/test/java/nodomain/freeyourgadget/gadgetbridge/service/devices/huawei/TestHuaweiSupportProvider.java

@@ -19,20 +19,20 @@ public class TestHuaweiSupportProvider {
         Mockito.verify(supportProvider.responseManager, Mockito.times(1)).handleData(data1);
     }
 
-    @Test
-    public void testOnSocketReadMultiplePacket() {
-        byte[] expected = {(byte) 0x5A, (byte) 0x00, (byte) 0x06, (byte) 0x00, (byte) 0x04, (byte) 0x01, (byte) 0x01, (byte) 0x01, (byte) 0x02, (byte) 0x99, (byte) 0x6B};
-        byte[] data1 = {(byte) 0x5A, (byte) 0x00, (byte) 0x06, (byte) 0x00, (byte) 0x04, (byte) 0x01, (byte) 0x01, (byte) 0x01, (byte) 0x02, (byte) 0x99, (byte) 0x6B, (byte) 0x5A, (byte) 0x00, (byte) 0x06, (byte) 0x00, (byte) 0x04, (byte) 0x01, (byte) 0x01, (byte) 0x01, (byte) 0x02, (byte) 0x99, (byte) 0x6B};
-
-        HuaweiBRSupport support = new HuaweiBRSupport();
-
-        HuaweiSupportProvider supportProvider = new HuaweiSupportProvider(support);
-        supportProvider.responseManager = Mockito.mock(ResponseManager.class);
-
-        supportProvider.onSocketRead(data1);
-
-        Mockito.verify(supportProvider.responseManager, Mockito.times(2)).handleData(expected);
-    }
+//    @Test
+//    public void testOnSocketReadMultiplePacket() {
+//        byte[] expected = {(byte) 0x5A, (byte) 0x00, (byte) 0x06, (byte) 0x00, (byte) 0x04, (byte) 0x01, (byte) 0x01, (byte) 0x01, (byte) 0x02, (byte) 0x99, (byte) 0x6B};
+//        byte[] data1 = {(byte) 0x5A, (byte) 0x00, (byte) 0x06, (byte) 0x00, (byte) 0x04, (byte) 0x01, (byte) 0x01, (byte) 0x01, (byte) 0x02, (byte) 0x99, (byte) 0x6B, (byte) 0x5A, (byte) 0x00, (byte) 0x06, (byte) 0x00, (byte) 0x04, (byte) 0x01, (byte) 0x01, (byte) 0x01, (byte) 0x02, (byte) 0x99, (byte) 0x6B};
+//
+//        HuaweiBRSupport support = new HuaweiBRSupport();
+//
+//        HuaweiSupportProvider supportProvider = new HuaweiSupportProvider(support);
+//        supportProvider.responseManager = Mockito.mock(ResponseManager.class);
+//
+//        supportProvider.onSocketRead(data1);
+//
+//        Mockito.verify(supportProvider.responseManager, Mockito.times(2)).handleData(expected);
+//    }
 
     @Test
     public void testOnSocketReadPartialPacket() {
@@ -50,4 +50,22 @@ public class TestHuaweiSupportProvider {
         Mockito.verify(supportProvider.responseManager, Mockito.times(1)).handleData(data1);
         Mockito.verify(supportProvider.responseManager, Mockito.times(1)).handleData(data2);
     }
+
+    @Test
+    public void testOnSocketReadPartialPacket5a() {
+        byte[] data1 = {(byte) 0x5A, (byte) 0x00, (byte) 0x06, (byte) 0x00, (byte) 0x04, (byte) 0x01};
+        byte[] data2 = {(byte) 0x5A, (byte) 0x91, (byte) 0x92, (byte) 0x99, (byte) 0x6B};
+
+        HuaweiBRSupport support = new HuaweiBRSupport();
+
+        HuaweiSupportProvider supportProvider = new HuaweiSupportProvider(support);
+        supportProvider.responseManager = Mockito.mock(ResponseManager.class);
+
+        supportProvider.onSocketRead(data1);
+        supportProvider.onSocketRead(data2);
+
+        Mockito.verify(supportProvider.responseManager, Mockito.times(1)).handleData(data1);
+        Mockito.verify(supportProvider.responseManager, Mockito.times(1)).handleData(data2);
+    }
+
 }

app/src/test/java/nodomain/freeyourgadget/gadgetbridge/service/devices/huawei/TestResponseManager.java

@@ -467,4 +467,24 @@ public class TestResponseManager {
         verify(request2, times(1)).handleResponse((HuaweiPacket) any());
         verify(request2, times(0)).handleResponse();
     }
+
+    @Test
+    public void testOnSocketReadMultiplePacketSplit() throws IllegalAccessException, HuaweiPacket.ParseException {
+        byte[] expected = {(byte) 0x5A, (byte) 0x00, (byte) 0x06, (byte) 0x00, (byte) 0x04, (byte) 0x01, (byte) 0x01, (byte) 0x01, (byte) 0x02, (byte) 0x99, (byte) 0x6B};
+
+        byte[] data1 = {(byte) 0x5A, (byte) 0x00, (byte) 0x06, (byte) 0x00, (byte) 0x04, (byte) 0x01, (byte) 0x01};
+        byte[] data2 = {(byte) 0x01, (byte) 0x02, (byte) 0x99, (byte) 0x6B, (byte) 0x5A, (byte) 0x00, (byte) 0x06, (byte) 0x00, (byte) 0x04, (byte) 0x01, (byte) 0x01, (byte) 0x01, (byte) 0x02, (byte) 0x99, (byte) 0x6B};
+
+        HuaweiPacket expectedPacket = new HuaweiPacket(supportProvider.getParamsProvider()).parse(expected);
+
+        AsynchronousResponse mockAsynchronousResponse = Mockito.mock(AsynchronousResponse.class);
+
+        ResponseManager responseManager = new ResponseManager(supportProvider);
+        asynchronousResponseField.set(responseManager, mockAsynchronousResponse);
+
+        responseManager.handleData(data1);
+        responseManager.handleData(data2);
+
+        verify(mockAsynchronousResponse, times(2)).handleResponse(expectedPacket);
+    }
 }