Correction based on comments.

Re-write modulo256 checksum's parameter to byte array, not varargs.
Split the tests to two methods for HEADER ver1 and ver2.
Remove above-class copyrights, added top-file copyrights.
Rename parcelNumber to index.
Rename recordStartSign to recordType.
Remove crearing additional buffer for record data, use origin buffer instead.
Added check on minimal length of packet to avoid the out-of-range exceptions.

7 files changed, 77 insertions, 38 deletions

diff --git a/src/org/traccar/helper/Checksum.java b/src/org/traccar/helper/Checksum.java
index 7a0151d36..44cef9635 100644
--- a/src/org/traccar/helper/Checksum.java
+++ b/src/org/traccar/helper/Checksum.java
@@ -243,7 +243,7 @@ public final class Checksum {
         return (10 - (checksum % 10)) % 10;
     }
 
-    public static int modulo256(byte... bytes) {
+    public static int modulo256(byte[] bytes) {
         int sum = 0;
         for (byte b : bytes) {
             sum = (sum + b) & 0xFF;
diff --git a/src/org/traccar/protocol/Arnavi4FrameDecoder.java b/src/org/traccar/protocol/Arnavi4FrameDecoder.java
index 97bf75a20..2587420c9 100644
--- a/src/org/traccar/protocol/Arnavi4FrameDecoder.java
+++ b/src/org/traccar/protocol/Arnavi4FrameDecoder.java
@@ -1,3 +1,18 @@
+/*
+ * Copyright 2017 Ivan Muratov (binakot@gmail.com)
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
 package org.traccar.protocol;
 
 import org.jboss.netty.buffer.ChannelBuffer;
@@ -5,11 +20,10 @@ import org.jboss.netty.channel.Channel;
 import org.jboss.netty.channel.ChannelHandlerContext;
 import org.jboss.netty.handler.codec.frame.FrameDecoder;
 
-/**
- * Created by Ivan Muratov @binakot on 12.07.2017.
- */
 public class Arnavi4FrameDecoder extends FrameDecoder {
 
+    static final int PACKET_MIN_LENGTH = 4;
+
     static final byte HEADER_START_SIGN = (byte) 0xFF;
     static final byte HEADER_VERSION_1 = 0x22;
     static final byte HEADER_VERSION_2 = 0x23;
@@ -26,7 +40,7 @@ public class Arnavi4FrameDecoder extends FrameDecoder {
             Channel channel,
             ChannelBuffer buf) throws Exception {
 
-        if (buf.readableBytes() == 0) {
+        if (buf.readableBytes() < PACKET_MIN_LENGTH) {
             return null;
         }
 
diff --git a/src/org/traccar/protocol/Arnavi4Protocol.java b/src/org/traccar/protocol/Arnavi4Protocol.java
index 227397980..381a9b457 100644
--- a/src/org/traccar/protocol/Arnavi4Protocol.java
+++ b/src/org/traccar/protocol/Arnavi4Protocol.java
@@ -1,3 +1,18 @@
+/*
+ * Copyright 2017 Ivan Muratov (binakot@gmail.com)
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
 package org.traccar.protocol;
 
 import org.jboss.netty.bootstrap.ServerBootstrap;
@@ -8,9 +23,6 @@ import org.traccar.TrackerServer;
 import java.nio.ByteOrder;
 import java.util.List;
 
-/**
- * Created by Ivan Muratov @binakot on 11.07.2017.
- */
 public class Arnavi4Protocol extends BaseProtocol {
 
     public Arnavi4Protocol() {
diff --git a/src/org/traccar/protocol/Arnavi4ProtocolDecoder.java b/src/org/traccar/protocol/Arnavi4ProtocolDecoder.java
index c5ec3f31f..232e8f053 100644
--- a/src/org/traccar/protocol/Arnavi4ProtocolDecoder.java
+++ b/src/org/traccar/protocol/Arnavi4ProtocolDecoder.java
@@ -1,3 +1,18 @@
+/*
+ * Copyright 2017 Ivan Muratov (binakot@gmail.com)
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
 package org.traccar.protocol;
 
 import org.jboss.netty.buffer.ChannelBuffer;
@@ -21,9 +36,6 @@ import static org.traccar.protocol.Arnavi4FrameDecoder.HEADER_VERSION_2;
 import static org.traccar.protocol.Arnavi4FrameDecoder.PACKAGE_START_SIGN;
 import static org.traccar.protocol.Arnavi4FrameDecoder.PACKAGE_END_SIGN;
 
-/**
- * Created by Ivan Muratov @binakot on 11.07.2017.
- */
 public class Arnavi4ProtocolDecoder extends BaseProtocolDecoder {
 
     private static final byte RECORD_PING = 0x00;
@@ -40,18 +52,19 @@ public class Arnavi4ProtocolDecoder extends BaseProtocolDecoder {
         super(protocol);
     }
 
-    private Position decodePosition(DeviceSession deviceSession, ChannelBuffer buf, long timestamp) {
+    private Position decodePosition(DeviceSession deviceSession, ChannelBuffer buf, int length, Date time) {
 
         final Position position = new Position();
         position.setProtocol(getProtocolName());
         position.setDeviceId(deviceSession.getDeviceId());
 
-        position.setTime(new Date(timestamp));
+        position.setTime(time);
 
-        while (buf.readableBytes() > 0) {
-            short tagId = buf.readUnsignedByte();
+        int readBytes = 0;
+        while (readBytes < length) {
+            short tag = buf.readUnsignedByte();
             int tagValue = buf.readInt();
-            switch (tagId) {
+            switch (tag) {
                 case TAG_LATITUDE:
                     position.setLatitude(Float.intBitsToFloat(tagValue));
                     position.setValid(true);
@@ -72,6 +85,8 @@ public class Arnavi4ProtocolDecoder extends BaseProtocolDecoder {
                 default:
                     break; // Skip other tags
             }
+
+            readBytes += 5; // 1 byte tag + 4 bytes value
         }
 
         return position;
@@ -127,37 +142,36 @@ public class Arnavi4ProtocolDecoder extends BaseProtocolDecoder {
 
             List<Position> positions = new LinkedList<>();
 
-            int parcelNumber = buf.readUnsignedByte();
+            int index = buf.readUnsignedByte();
 
-            byte recordStartSign = buf.readByte();
-            while (recordStartSign != PACKAGE_END_SIGN) {
-                switch (recordStartSign) {
+            byte recordType = buf.readByte();
+            while (recordType != PACKAGE_END_SIGN) {
+                switch (recordType) {
                     case RECORD_PING:
                     case RECORD_DATA:
                     case RECORD_TEXT:
                     case RECORD_FILE:
                     case RECORD_BINARY:
                         int length = buf.readUnsignedShort();
-                        long timestamp = buf.readUnsignedInt() * 1000;
-                        ChannelBuffer recordBuf = buf.readBytes(length);
+                        Date time = new Date(buf.readUnsignedInt() * 1000);
 
-                        if (recordStartSign == RECORD_DATA) {
-                            positions.add(decodePosition(deviceSession, recordBuf, timestamp));
+                        if (recordType == RECORD_DATA) {
+                            positions.add(decodePosition(deviceSession, buf, length, time));
                         }
 
                         buf.readUnsignedByte(); // crc
                         break;
 
                     default:
-                        throw new IllegalArgumentException("unsupported record type");
+                        return null; // Unsupported types of package
                 }
 
-                recordStartSign = buf.readByte();
+                recordType = buf.readByte();
             }
 
             if (channel != null) {
                 final ChannelBuffer response = ChannelBuffers.dynamicBuffer(ByteOrder.LITTLE_ENDIAN, 4);
-                response.writeBytes(new byte[]{0x7B, 0x00, (byte) parcelNumber, 0x7D});
+                response.writeBytes(new byte[]{0x7B, 0x00, (byte) index, 0x7D});
                 channel.write(response);
             }
 
diff --git a/test/org/traccar/helper/ChecksumTest.java b/test/org/traccar/helper/ChecksumTest.java
index 737b65c62..c7c5031df 100644
--- a/test/org/traccar/helper/ChecksumTest.java
+++ b/test/org/traccar/helper/ChecksumTest.java
@@ -4,7 +4,6 @@ import org.jboss.netty.buffer.ChannelBuffer;
 import org.jboss.netty.buffer.ChannelBuffers;
 import org.junit.Assert;
 import org.junit.Test;
-import org.traccar.protocol.Arnavi4ProtocolDecoder;
 
 import java.nio.charset.StandardCharsets;
 
@@ -31,9 +30,9 @@ public class ChecksumTest {
 
     @Test
     public void testModulo256() {
-        Assert.assertEquals(0x00, Checksum.modulo256((byte)0x00));
-        Assert.assertEquals(0x00, Checksum.modulo256((byte)0x00, (byte)0x00, (byte)0x00));
-        Assert.assertEquals(0x06, Checksum.modulo256((byte)0x01, (byte)0x02, (byte)0x03));
+        Assert.assertEquals(0x00, Checksum.modulo256(new byte[] {0x00}));
+        Assert.assertEquals(0x00, Checksum.modulo256(new byte[] {0x00, 0x00, 0x00}));
+        Assert.assertEquals(0x06, Checksum.modulo256(new byte[] {0x01, 0x02, 0x03}));
     }
 
 }
diff --git a/test/org/traccar/protocol/Arnavi4FrameDecoderTest.java b/test/org/traccar/protocol/Arnavi4FrameDecoderTest.java
index 08abd3835..0b502bc36 100644
--- a/test/org/traccar/protocol/Arnavi4FrameDecoderTest.java
+++ b/test/org/traccar/protocol/Arnavi4FrameDecoderTest.java
@@ -6,9 +6,6 @@ import org.traccar.ProtocolTest;
 
 import java.nio.ByteOrder;
 
-/**
- * Created by Ivan Muratov @binakot on 13.07.2017.
- */
 public class Arnavi4FrameDecoderTest extends ProtocolTest {
 
     @Test
diff --git a/test/org/traccar/protocol/Arnavi4ProtocolDecoderTest.java b/test/org/traccar/protocol/Arnavi4ProtocolDecoderTest.java
index 2395572a1..d789b1c9c 100644
--- a/test/org/traccar/protocol/Arnavi4ProtocolDecoderTest.java
+++ b/test/org/traccar/protocol/Arnavi4ProtocolDecoderTest.java
@@ -5,13 +5,10 @@ import org.traccar.ProtocolTest;
 
 import java.nio.ByteOrder;
 
-/**
- * Created by Ivan Muratov @binakot on 11.07.2017.
- */
 public class Arnavi4ProtocolDecoderTest extends ProtocolTest {
 
     @Test
-    public void testDecode() throws Exception {
+    public void testHeader1Decode() throws Exception {
 
         Arnavi4ProtocolDecoder decoder;
 
@@ -23,6 +20,12 @@ public class Arnavi4ProtocolDecoderTest extends ProtocolTest {
         verifyPositions(decoder, binary(ByteOrder.LITTLE_ENDIAN, // Valid PACKAGE packet with one DATA packet
                 "5b01012800a3175f5903513934420447221c42055402781E0900f0c5215b4e0084005c00007c005d0000a300fa37010000295d"),
                 position("2017-07-07 05:09:55.000", true, 45.05597, 39.03347));
+    }
+
+    @Test
+    public void testHeader2Decode() throws Exception {
+
+        Arnavi4ProtocolDecoder decoder;
 
         decoder = new Arnavi4ProtocolDecoder(new Arnavi4Protocol());