Arnavi4 frame decoder's code reorganization based on traccar maintainer comments.

4 files changed, 55 insertions, 70 deletions

diff --git a/setup/default.xml b/setup/default.xml
index 223b9a585..1e84c5218 100644
--- a/setup/default.xml
+++ b/setup/default.xml
@@ -579,6 +579,9 @@
     <entry key='vt200.port'>5143</entry>
     <entry key='owntracks.port'>5144</entry>
     <entry key='vtfms.port'>5145</entry>
-    <entry key='arnavi4.port'>5146</entry>
+    <entry key='tlv.port'>5146</entry>
+    <entry key='esky.port'>5147</entry>
+    <entry key='genx.port'>5148</entry>
+    <entry key='arnavi4.port'>5149</entry>
 
 </properties>
diff --git a/src/org/traccar/protocol/Arnavi4FrameDecoder.java b/src/org/traccar/protocol/Arnavi4FrameDecoder.java
index 7c60cb934..b13f3fd7d 100644
--- a/src/org/traccar/protocol/Arnavi4FrameDecoder.java
+++ b/src/org/traccar/protocol/Arnavi4FrameDecoder.java
@@ -22,53 +22,50 @@ import org.jboss.netty.handler.codec.frame.FrameDecoder;
 
 public class Arnavi4FrameDecoder extends FrameDecoder {
 
-    private static final int PACKET_MINIMUM_LENGTH = 4;
-    private static final int RECORD_MINIMUM_LENGTH = 8;
-
-    static final byte HEADER_START_SIGN = (byte) 0xff;
-    static final byte HEADER_VERSION_1 = 0x22;
-    static final byte HEADER_VERSION_2 = 0x23;
+    private static final int MIN_LENGTH = 4;
     private static final int HEADER_LENGTH = 10;
-
-    private static final byte PACKAGE_START_SIGN = 0x5b;
+    private static final int PACKET_WRAPPER_LENGTH = 8;
+    private static final int COMMAND_ANSWER_PACKET_LENGTH = 4;
+    private static final int COMMAND_ANSWER_PARCEL_NUMBER = 0xfd;
     private static final byte PACKAGE_END_SIGN = 0x5d;
-    private static final int PACKAGE_MIN_PARCEL_NUMBER = 0x01;
-    private static final int PACKAGE_MAX_PARCEL_NUMBER = 0xfb;
+
+    private boolean firstPacket = true;
 
     @Override
     protected Object decode(
-            ChannelHandlerContext ctx,
-            Channel channel,
-            ChannelBuffer buf) throws Exception {
+            ChannelHandlerContext ctx, Channel channel, ChannelBuffer buf) throws Exception {
 
-        if (buf.readableBytes() < PACKET_MINIMUM_LENGTH) {
+        if (buf.readableBytes() < MIN_LENGTH) {
             return null;
         }
 
-        if (buf.getByte(0) == HEADER_START_SIGN
-                && buf.readableBytes() == HEADER_LENGTH
-                && (buf.getByte(1) == HEADER_VERSION_1 || buf.getByte(1) == HEADER_VERSION_2)) {
-
-            return buf.readBytes(HEADER_LENGTH);
-        }
-
-        int index = buf.getUnsignedByte(1); // parcel number
-        if (buf.getByte(0) == PACKAGE_START_SIGN
-                && index >= PACKAGE_MIN_PARCEL_NUMBER && index <= PACKAGE_MAX_PARCEL_NUMBER) {
-
-            int bufferPosition = 2; // start sign + parcel number
-            while (bufferPosition + RECORD_MINIMUM_LENGTH < buf.readableBytes()
-                    && buf.getByte(bufferPosition) != PACKAGE_END_SIGN) {
-
-                int dataLength = buf.getUnsignedShort(bufferPosition + 1);
-                bufferPosition += RECORD_MINIMUM_LENGTH + dataLength; // type + data length + unixtime + data + crc
+        int length;
+        if (firstPacket) {
+            firstPacket = false;
+            length = HEADER_LENGTH;
+        } else {
+            int index = buf.getUnsignedByte(1); // parcel number
+            if (index == COMMAND_ANSWER_PARCEL_NUMBER) {
+                length = COMMAND_ANSWER_PACKET_LENGTH;
+            } else {
+                int pos = 2; // start sign + parcel number
+                while (pos + PACKET_WRAPPER_LENGTH < buf.readableBytes()
+                        && buf.getByte(pos) != PACKAGE_END_SIGN) {
+
+                    int dataLength = buf.getUnsignedShort(pos + 1);
+                    pos += PACKET_WRAPPER_LENGTH + dataLength; // packet type + data length + unixtime + data + crc
+                }
+
+                if (buf.getByte(pos) != PACKAGE_END_SIGN) { // end sign
+                    return null;
+                }
+
+                length = pos + 1;
             }
+        }
 
-            if (bufferPosition < buf.readableBytes()
-                    && buf.getByte(bufferPosition) == PACKAGE_END_SIGN) {
-
-                return buf.readBytes(bufferPosition + 1); // end sign
-            }
+        if (buf.readableBytes() >= length) {
+            return buf.readBytes(length);
         }
 
         return null;
diff --git a/src/org/traccar/protocol/Arnavi4ProtocolDecoder.java b/src/org/traccar/protocol/Arnavi4ProtocolDecoder.java
index e3e027993..06abb563f 100644
--- a/src/org/traccar/protocol/Arnavi4ProtocolDecoder.java
+++ b/src/org/traccar/protocol/Arnavi4ProtocolDecoder.java
@@ -30,12 +30,12 @@ import java.util.Date;
 import java.util.LinkedList;
 import java.util.List;
 
-import static org.traccar.protocol.Arnavi4FrameDecoder.HEADER_START_SIGN;
-import static org.traccar.protocol.Arnavi4FrameDecoder.HEADER_VERSION_1;
-import static org.traccar.protocol.Arnavi4FrameDecoder.HEADER_VERSION_2;
-
 public class Arnavi4ProtocolDecoder extends BaseProtocolDecoder {
 
+    private static final byte HEADER_START_SIGN = (byte) 0xff;
+    private static final byte HEADER_VERSION_1 = 0x22;
+    private static final byte HEADER_VERSION_2 = 0x23;
+
     private static final byte RECORD_PING = 0x00;
     private static final byte RECORD_DATA = 0x01;
     private static final byte RECORD_TEXT = 0x03;
diff --git a/test/org/traccar/protocol/Arnavi4FrameDecoderTest.java b/test/org/traccar/protocol/Arnavi4FrameDecoderTest.java
index 2203cdafc..b634f0cdc 100644
--- a/test/org/traccar/protocol/Arnavi4FrameDecoderTest.java
+++ b/test/org/traccar/protocol/Arnavi4FrameDecoderTest.java
@@ -17,14 +17,6 @@ public class Arnavi4FrameDecoderTest extends ProtocolTest {
                 binary(ByteOrder.LITTLE_ENDIAN, "ff22f30c45f5c90f0300"),
                 decoder.decode(null, null, binary(ByteOrder.LITTLE_ENDIAN, "ff22f30c45f5c90f0300")));
 
-        Assert.assertEquals( // Valid HEADER v2 packet with IMEI
-                binary(ByteOrder.LITTLE_ENDIAN, "ff23f30c45f5c90f0300"),
-                decoder.decode(null, null, binary(ByteOrder.LITTLE_ENDIAN, "ff23f30c45f5c90f0300")));
-
-        Assert.assertEquals( // Valid PACKAGE with answer to server on file transfer
-                null,
-                decoder.decode(null, null, binary(ByteOrder.LITTLE_ENDIAN, "5bfd005d")));
-
         Assert.assertEquals( // Valid PACKAGE with one DATA packet
                 binary(ByteOrder.LITTLE_ENDIAN, "5b01012800a3175f5903513934420447221c42055402781E0900f0c5215b4e0084005c00007c005d0000a300fa37010000295d"),
                 decoder.decode(null, null, binary(ByteOrder.LITTLE_ENDIAN, "5b01012800a3175f5903513934420447221c42055402781E0900f0c5215b4e0084005c00007c005d0000a300fa37010000295d")));
@@ -34,31 +26,24 @@ public class Arnavi4FrameDecoderTest extends ProtocolTest {
                 decoder.decode(null, null, binary(ByteOrder.LITTLE_ENDIAN, "5b01012800a3175f5903513934420447221c42055402781E0900f0c5215b4e0084005c00007c005d0000a300fa3701000029012800a3175f5903513934420447221c42055402781E0900f0c5215b4e0084005c00007c005d0000a300fa37010000295d")));
 
         Assert.assertEquals( // Valid PACKAGE with one TEXT packet.
-                null,
-                decoder.decode(null, null, binary(ByteOrder.LITTLE_ENDIAN, "030700e3f16b50747261636361721b")));
+                binary(ByteOrder.LITTLE_ENDIAN, "5b01030700e3f16b50747261636361721b5d"),
+                decoder.decode(null, null, binary(ByteOrder.LITTLE_ENDIAN, "5b01030700e3f16b50747261636361721b5d")));
 
-        Assert.assertEquals( // Valid PACKAGE with one BINARY packet.
-                null,
-                decoder.decode(null, null, binary(ByteOrder.LITTLE_ENDIAN, "062000e3f16b5003298b5e4204cbd514420500191000080400ff021b")));
-
-    }
+        Assert.assertEquals( // Valid PACKAGE with two TEXT packet.
+                binary(ByteOrder.LITTLE_ENDIAN, "5b01030700e3f16b50747261636361721b030700e3f16b50747261636361721b5d"),
+                decoder.decode(null, null, binary(ByteOrder.LITTLE_ENDIAN, "5b01030700e3f16b50747261636361721b030700e3f16b50747261636361721b5d")));
 
-    @Test
-    public void testDecodeInvalidPackets() throws Exception {
-
-        Arnavi4FrameDecoder decoder = new Arnavi4FrameDecoder();
-
-        Assert.assertEquals( // Invalid PACKAGE with one DATA packet (missing last byte with end sign)
-                null,
-                decoder.decode(null, null, binary(ByteOrder.LITTLE_ENDIAN, "5b01012800a3175f5903513934420447221c42055402781E0900f0c5215b4e0084005c00007c005d0000a300fa3701000029")));
+        Assert.assertEquals( // Valid PACKAGE with one BINARY packet.
+                binary(ByteOrder.LITTLE_ENDIAN, "5b01061400e3f16b5003298b5e4204cbd514420500191000080400ff021b5d"),
+                decoder.decode(null, null, binary(ByteOrder.LITTLE_ENDIAN, "5b01061400e3f16b5003298b5e4204cbd514420500191000080400ff021b5d")));
 
-        Assert.assertEquals( // Invalid PACKAGE with two DATA packet (missing last 10 bytes)
-                null,
-                decoder.decode(null, null, binary(ByteOrder.LITTLE_ENDIAN, "5b01012800a3175f5903513934420447221c42055402781E0900f0c5215b4e0084005c00007c005d0000a300fa3701000029012800a3175f5903513934420447221c42055402781E0900f0c5215b4e0084005c00007c005d00")));
+        Assert.assertEquals( // Valid PACKAGE with two BINARY packet.
+                binary(ByteOrder.LITTLE_ENDIAN, "5b01061400e3f16b5003298b5e4204cbd514420500191000080400ff021b061400e3f16b5003298b5e4204cbd514420500191000080400ff021b5d"),
+                decoder.decode(null, null, binary(ByteOrder.LITTLE_ENDIAN, "5b01061400e3f16b5003298b5e4204cbd514420500191000080400ff021b061400e3f16b5003298b5e4204cbd514420500191000080400ff021b5d")));
 
-        Assert.assertEquals( // Valid PACKAGE with useless extra bytes at the end
-                binary(ByteOrder.LITTLE_ENDIAN, "5b01012800a3175f5903513934420447221c42055402781E0900f0c5215b4e0084005c00007c005d0000a300fa37010000295d"),
-                decoder.decode(null, null, binary(ByteOrder.LITTLE_ENDIAN, "5b01012800a3175f5903513934420447221c42055402781E0900f0c5215b4e0084005c00007c005d0000a300fa37010000295d010203040506070809")));
+        Assert.assertEquals( // Valid PACKAGE with answer to server on file transfer
+                binary(ByteOrder.LITTLE_ENDIAN, "5bfd005d"),
+                decoder.decode(null, null, binary(ByteOrder.LITTLE_ENDIAN, "5bfd005d")));
 
     }