You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@plc4x.apache.org by jm...@apache.org on 2018/01/08 03:17:56 UTC

[incubator-plc4x] branch master updated (464d125 -> 44e01ea)

This is an automated email from the ASF dual-hosted git repository.

jmclean pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-plc4x.git.


    from 464d125  SONAR Some further fixing of issues sonarqube reported
     new 74617fc  mask slot number
     new ecba61f  add calling parameter test
     new 7b849a1  get rid of a level of nesting
     new 5ad5d2d  no real need to calculate size/length every time through the loop
     new bd85953  add more encode and decode parameter tests
     new 44e01ea  Sort divided by a int is always an int no need to round up with Math.ceil

The 6 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../plc4x/java/isotp/netty/IsoTPProtocol.java      | 152 +++++------
 .../plc4x/java/s7/netty/Plc4XS7Protocol.java       |  34 +--
 .../org/apache/plc4x/java/s7/netty/S7Protocol.java |   2 +-
 .../plc4x/java/isotp/netty/IsoTPProtocolTest.java  | 294 ++++++++++++++++++++-
 4 files changed, 386 insertions(+), 96 deletions(-)

-- 
To stop receiving notification emails like this one, please contact
['"commits@plc4x.apache.org" <co...@plc4x.apache.org>'].

Re: [incubator-plc4x] 06/06: Sort divided by a int is always an int no need to round up with Math.ceil

Posted by Justin Mclean <ju...@classsoftware.com>.
Hi,

Ping?

Justin

Re: [incubator-plc4x] 06/06: Sort divided by a int is always an int no need to round up with Math.ceil

Posted by Christofer Dutz <ch...@c-ware.de>.
Yup … exactly

Chris

Am 09.01.18, 09:42 schrieb "Justin Mclean" <ju...@me.com>:

    Hi,
    
    > you are correct with your assumption. However as soon as at least one floating point number is used in the equation, the result should be too, so it should be enough to make 8 an 8.0
    
    So this what I was thinking:
    (short) Math.ceil(userData.readShort() / 8.0)
    
    Thanks,
    Justin


Re: [incubator-plc4x] 06/06: Sort divided by a int is always an int no need to round up with Math.ceil

Posted by Justin Mclean <ju...@me.com>.
Hi,

> you are correct with your assumption. However as soon as at least one floating point number is used in the equation, the result should be too, so it should be enough to make 8 an 8.0

So this what I was thinking:
(short) Math.ceil(userData.readShort() / 8.0)

Thanks,
Justin

Re: [incubator-plc4x] 06/06: Sort divided by a int is always an int no need to round up with Math.ceil

Posted by Christofer Dutz <ch...@c-ware.de>.
Hi Justin,

you are correct with your assumption. However as soon as at least one floating point number is used in the equation, the result should be too, so it should be enough to make 8 an 8.0

And no, size in bits is a Boolean and it simply provides information on if the size parameter is the number of bytes (false) or the number of bits (true). And no, if I want to read 3 consecutive bits for example, then we have to transport this in 1 byte, if we read 9 bits, then in 2 and so on.

Chris




Am 08.01.18, 04:24 schrieb "Justin Mclean" <jm...@apache.org>:

    Hi,
    
    With this change here:
    
    >                         DataTransportSize dataTransportSize = DataTransportSize.valueOf(userData.readByte());
    >                         short length = (dataTransportSize.isSizeInBits()) ?
    > -                            (short) Math.ceil(userData.readShort() / 8) : userData.readShort();
    > +                            (short) (userData.readShort() / 8) : userData.readShort();
    
    Math.ceil would never round up as a short divided by an init is never a floating point number.
    
    Do we need to worry about non byte boundaries? i.e. is isSizeInBits always multiple of 8? If not the code will need to be modified as it would still be doing what it was doing before and sometimes missing a byte. 
    
    Thanks,
    Justin
    
    


Re: [incubator-plc4x] 06/06: Sort divided by a int is always an int no need to round up with Math.ceil

Posted by Justin Mclean <jm...@apache.org>.
Hi,

With this change here:

>                         DataTransportSize dataTransportSize = DataTransportSize.valueOf(userData.readByte());
>                         short length = (dataTransportSize.isSizeInBits()) ?
> -                            (short) Math.ceil(userData.readShort() / 8) : userData.readShort();
> +                            (short) (userData.readShort() / 8) : userData.readShort();

Math.ceil would never round up as a short divided by an init is never a floating point number.

Do we need to worry about non byte boundaries? i.e. is isSizeInBits always multiple of 8? If not the code will need to be modified as it would still be doing what it was doing before and sometimes missing a byte. 

Thanks,
Justin


[incubator-plc4x] 06/06: Sort divided by a int is always an int no need to round up with Math.ceil

Posted by jm...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jmclean pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-plc4x.git

commit 44e01ea0483f092cc6df9248bb6658211acd82bf
Author: Justin Mclean <jm...@apache.org>
AuthorDate: Mon Jan 8 14:17:48 2018 +1100

    Sort divided by a int is always an int no need to round up with Math.ceil
---
 .../s7/src/main/java/org/apache/plc4x/java/s7/netty/S7Protocol.java     | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/S7Protocol.java b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/S7Protocol.java
index 585173f..bd6ea9c 100644
--- a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/S7Protocol.java
+++ b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/S7Protocol.java
@@ -242,7 +242,7 @@ public class S7Protocol extends MessageToMessageCodec<IsoTPMessage, S7Message> {
                         (messageType == MessageType.ACK_DATA)) {
                         DataTransportSize dataTransportSize = DataTransportSize.valueOf(userData.readByte());
                         short length = (dataTransportSize.isSizeInBits()) ?
-                            (short) Math.ceil(userData.readShort() / 8) : userData.readShort();
+                            (short) (userData.readShort() / 8) : userData.readShort();
                         byte[] data = new byte[length];
                         userData.readBytes(data);
                         // Initialize a rudimentary payload (This is updated in the Plc4XS7Protocol class

-- 
To stop receiving notification emails like this one, please contact
"commits@plc4x.apache.org" <co...@plc4x.apache.org>.

[incubator-plc4x] 02/06: add calling parameter test

Posted by jm...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jmclean pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-plc4x.git

commit ecba61f5c19821a9063fbfc79573acaea78b7311
Author: Justin Mclean <jm...@apache.org>
AuthorDate: Mon Jan 8 11:14:05 2018 +1100

    add calling parameter test
---
 .../plc4x/java/isotp/netty/IsoTPProtocolTest.java  | 34 +++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/plc4j/protocols/s7/src/test/java/org/apache/plc4x/java/isotp/netty/IsoTPProtocolTest.java b/plc4j/protocols/s7/src/test/java/org/apache/plc4x/java/isotp/netty/IsoTPProtocolTest.java
index a536bd4..55f3529 100644
--- a/plc4j/protocols/s7/src/test/java/org/apache/plc4x/java/isotp/netty/IsoTPProtocolTest.java
+++ b/plc4j/protocols/s7/src/test/java/org/apache/plc4x/java/isotp/netty/IsoTPProtocolTest.java
@@ -23,7 +23,9 @@ import io.netty.buffer.Unpooled;
 import io.netty.channel.ChannelHandlerContext;
 import org.apache.plc4x.java.isoontcp.netty.model.IsoOnTcpMessage;
 import org.apache.plc4x.java.isotp.netty.model.IsoTPMessage;
+import org.apache.plc4x.java.isotp.netty.model.params.CallingTsapParameter;
 import org.apache.plc4x.java.isotp.netty.model.params.Parameter;
+import org.apache.plc4x.java.isotp.netty.model.params.TsapParameter;
 import org.apache.plc4x.java.isotp.netty.model.tpdus.*;
 import org.apache.plc4x.java.isotp.netty.model.types.*;
 import org.apache.plc4x.java.netty.NettyTestBase;
@@ -38,7 +40,7 @@ import java.util.List;
 
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
-public class IsoTPProtocolTest extends NettyTestBase {
+public class IsoTPProtocolTest {
 
     private  IsoTPProtocol isoTPProtocol;
 
@@ -340,6 +342,36 @@ public class IsoTPProtocolTest extends NettyTestBase {
 
     @Test
     @Tag("fast")
+    public void encodeCallingParameters() throws Exception {
+        ChannelHandlerContext ctx = new MockChannelHandlerContext();
+        ByteBuf buf = Unpooled.buffer();
+        ArrayList<Parameter> parmameters = new ArrayList<>();
+        CallingTsapParameter callingParameter = new CallingTsapParameter(DeviceGroup.PG_OR_PC, (byte) 0x7, (byte) 0xe1); // slot number too big and overflows into rack
+        parmameters.add(callingParameter);
+        ErrorTpdu tpdu = new ErrorTpdu((short)0x1, RejectCause.REASON_NOT_SPECIFIED, parmameters, buf);
+        ArrayList<Object> out = new ArrayList<>();
+
+        isoTPProtocol.encode(ctx, tpdu, out);
+
+        assertTrue(out.size() == 1, "Message not decoded");
+
+        ByteBuf userData = ((IsoOnTcpMessage)out.get(0)).getUserData();
+
+        assertTrue(userData.writerIndex() == 9, "Incorrect message length");
+        assertTrue(userData.readByte() == (byte)0x8, "Incorrect header length");
+        assertTrue(userData.readByte() == TpduCode.TPDU_ERROR.getCode(), "Incorrect Tpdu code");
+        assertTrue(userData.readShort() == (short)0x1, "Incorrect destination reference code");
+        assertTrue(userData.readByte() == RejectCause.REASON_NOT_SPECIFIED.getCode(), "Incorrect reject cause code");
+        assertTrue(userData.readByte() == ParameterCode.CALLING_TSAP.getCode(), "Incorrect parameter code");
+        assertTrue(userData.readByte() == (byte)0x2, "Incorrect parameter length");
+        assertTrue(userData.readByte() == DeviceGroup.PG_OR_PC.getCode(), "Incorrect device group code");
+        byte rackAndSlot = userData.readByte();
+        assertTrue((rackAndSlot & 0xf0) >> 4 == 0x7, "Incorrect rack number");
+        assertTrue((rackAndSlot & 0x0f) == (0xe1 & 0x0f), "Incorrect slot number");
+    }
+
+    @Test
+    @Tag("fast")
     public void decodeError() throws Exception {
         ChannelHandlerContext ctx = new MockChannelHandlerContext();
         ByteBuf buf = Unpooled.buffer();

-- 
To stop receiving notification emails like this one, please contact
"commits@plc4x.apache.org" <co...@plc4x.apache.org>.

[incubator-plc4x] 03/06: get rid of a level of nesting

Posted by jm...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jmclean pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-plc4x.git

commit 7b849a1cf697fd97129be661c9a7021c2ff7ced4
Author: Justin Mclean <jm...@apache.org>
AuthorDate: Mon Jan 8 14:15:15 2018 +1100

    get rid of a level of nesting
---
 .../plc4x/java/isotp/netty/IsoTPProtocol.java      | 145 +++++++++++----------
 1 file changed, 74 insertions(+), 71 deletions(-)

diff --git a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/isotp/netty/IsoTPProtocol.java b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/isotp/netty/IsoTPProtocol.java
index 7033777..d2ac23c 100644
--- a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/isotp/netty/IsoTPProtocol.java
+++ b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/isotp/netty/IsoTPProtocol.java
@@ -261,34 +261,36 @@ public class IsoTPProtocol extends MessageToMessageCodec<IsoOnTcpMessage, Tpdu>
     }
 
     private void outputParameters(ByteBuf out, List<Parameter> parameters) {
-        if (parameters != null) {
-            for (Parameter parameter : parameters) {
-                out.writeByte(parameter.getType().getCode());
-                out.writeByte((byte) (getParameterLength(parameter) - 2));
-                switch (parameter.getType()) {
-                    case CALLED_TSAP:
-                    case CALLING_TSAP:
-                        TsapParameter tsap = (TsapParameter) parameter;
-                        out.writeByte(tsap.getDeviceGroup().getCode());
-                        out.writeByte((byte) ((tsap.getRackNumber() << 4) | (tsap.getSlotNumber() & 0x0F)));
-                        break;
-                    case CHECKSUM:
-                        ChecksumParameter checksum = (ChecksumParameter) parameter;
-                        out.writeByte(checksum.getChecksum());
-                        break;
-                    case DISCONNECT_ADDITIONAL_INFORMATION:
-                        DisconnectAdditionalInformationParameter disconnectAdditionalInformation = (DisconnectAdditionalInformationParameter) parameter;
-                        out.writeBytes(disconnectAdditionalInformation.getData());
-                        break;
-                    case TPDU_SIZE:
-                        TpduSizeParameter sizeParameter = (TpduSizeParameter) parameter;
-                        out.writeByte(sizeParameter.getTpduSize().getCode());
-                        break;
-                    default:
-                        logger.error("TDPU tarameter type {} not implemented yet",
-                            new Object[]{parameter.getType().name()});
-                        return;
-                }
+        if (parameters == null) {
+            return;
+        }
+        
+        for (Parameter parameter : parameters) {
+            out.writeByte(parameter.getType().getCode());
+            out.writeByte((byte) (getParameterLength(parameter) - 2));
+            switch (parameter.getType()) {
+                case CALLED_TSAP:
+                case CALLING_TSAP:
+                    TsapParameter tsap = (TsapParameter) parameter;
+                    out.writeByte(tsap.getDeviceGroup().getCode());
+                    out.writeByte((byte) ((tsap.getRackNumber() << 4) | (tsap.getSlotNumber() & 0x0F)));
+                    break;
+                case CHECKSUM:
+                    ChecksumParameter checksum = (ChecksumParameter) parameter;
+                    out.writeByte(checksum.getChecksum());
+                    break;
+                case DISCONNECT_ADDITIONAL_INFORMATION:
+                    DisconnectAdditionalInformationParameter disconnectAdditionalInformation = (DisconnectAdditionalInformationParameter) parameter;
+                    out.writeBytes(disconnectAdditionalInformation.getData());
+                    break;
+                case TPDU_SIZE:
+                    TpduSizeParameter sizeParameter = (TpduSizeParameter) parameter;
+                    out.writeByte(sizeParameter.getTpduSize().getCode());
+                    break;
+                default:
+                    logger.error("TDPU tarameter type {} not implemented yet",
+                        new Object[]{parameter.getType().name()});
+                    return;
             }
         }
     }
@@ -302,32 +304,33 @@ public class IsoTPProtocol extends MessageToMessageCodec<IsoOnTcpMessage, Tpdu>
      * @return length of the iso tp header
      */
     private short getHeaderLength(Tpdu tpdu) {
-        if (tpdu != null) {
-            short headerLength;
-            switch (tpdu.getTpduCode()) {
-                case CONNECTION_REQUEST:
-                case CONNECTION_CONFIRM:
-                    headerLength = 7;
-                    break;
-                case DATA:
-                    headerLength = 3;
-                    break;
-                case DISCONNECT_REQUEST:
-                    headerLength = 7;
-                    break;
-                case DISCONNECT_CONFIRM:
-                    headerLength = 6;
-                    break;
-                case TPDU_ERROR:
-                    headerLength = 5;
-                    break;
-                default:
-                    headerLength = 0;
-                    break;
-            }
-            return (short) (headerLength + getParametersLength(tpdu.getParameters()));
+        if (tpdu == null) {
+            return 0;
         }
-        return 0;
+        
+        short headerLength;
+        switch (tpdu.getTpduCode()) {
+            case CONNECTION_REQUEST:
+            case CONNECTION_CONFIRM:
+                headerLength = 7;
+                break;
+            case DATA:
+                headerLength = 3;
+                break;
+            case DISCONNECT_REQUEST:
+                headerLength = 7;
+                break;
+            case DISCONNECT_CONFIRM:
+                headerLength = 6;
+                break;
+            case TPDU_ERROR:
+                headerLength = 5;
+                break;
+            default:
+                headerLength = 0;
+                break;
+        }
+        return (short) (headerLength + getParametersLength(tpdu.getParameters()));
     }
 
     private short getParametersLength(List<Parameter> parameters) {
@@ -341,25 +344,25 @@ public class IsoTPProtocol extends MessageToMessageCodec<IsoOnTcpMessage, Tpdu>
     }
 
     private short getParameterLength(Parameter parameter) {
-        if (parameter != null) {
-            switch (parameter.getType()) {
-                case CALLED_TSAP:
-                case CALLING_TSAP:
-                    return 4;
-                case CHECKSUM:
-                    return 3;
-                case DISCONNECT_ADDITIONAL_INFORMATION:
-                    DisconnectAdditionalInformationParameter disconnectAdditionalInformationParameter =
-                        (DisconnectAdditionalInformationParameter) parameter;
-                    return (short) (2 + ((disconnectAdditionalInformationParameter.getData() != null) ?
-                        disconnectAdditionalInformationParameter.getData().length : 0));
-                case TPDU_SIZE:
-                    return 3;
-                default:
-                    return 0;
-            }
+        if (parameter == null) {
+            return 0;
+        }
+        switch (parameter.getType()) {
+            case CALLED_TSAP:
+            case CALLING_TSAP:
+                return 4;
+            case CHECKSUM:
+                return 3;
+            case DISCONNECT_ADDITIONAL_INFORMATION:
+                DisconnectAdditionalInformationParameter disconnectAdditionalInformationParameter =
+                    (DisconnectAdditionalInformationParameter) parameter;
+                return (short) (2 + ((disconnectAdditionalInformationParameter.getData() != null) ?
+                    disconnectAdditionalInformationParameter.getData().length : 0));
+            case TPDU_SIZE:
+                return 3;
+            default:
+                return 0;
         }
-        return 0;
     }
 
     private Parameter parseParameter(ByteBuf out) {

-- 
To stop receiving notification emails like this one, please contact
"commits@plc4x.apache.org" <co...@plc4x.apache.org>.

[incubator-plc4x] 01/06: mask slot number

Posted by jm...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jmclean pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-plc4x.git

commit 74617fc50da51dca38bda4b4bdce890d7c113669
Author: Justin Mclean <jm...@apache.org>
AuthorDate: Mon Jan 8 11:13:13 2018 +1100

    mask slot number
---
 .../java/org/apache/plc4x/java/isotp/netty/IsoTPProtocol.java    | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/isotp/netty/IsoTPProtocol.java b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/isotp/netty/IsoTPProtocol.java
index bdaa722..7033777 100644
--- a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/isotp/netty/IsoTPProtocol.java
+++ b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/isotp/netty/IsoTPProtocol.java
@@ -270,8 +270,7 @@ public class IsoTPProtocol extends MessageToMessageCodec<IsoOnTcpMessage, Tpdu>
                     case CALLING_TSAP:
                         TsapParameter tsap = (TsapParameter) parameter;
                         out.writeByte(tsap.getDeviceGroup().getCode());
-                        out.writeByte((byte)
-                            ((tsap.getRackNumber() << 4) | (tsap.getSlotNumber())));
+                        out.writeByte((byte) ((tsap.getRackNumber() << 4) | (tsap.getSlotNumber() & 0x0F)));
                         break;
                     case CHECKSUM:
                         ChecksumParameter checksum = (ChecksumParameter) parameter;
@@ -392,9 +391,9 @@ public class IsoTPProtocol extends MessageToMessageCodec<IsoOnTcpMessage, Tpdu>
 
     private Parameter parseCallParameter(ByteBuf out, ParameterCode parameterCode) {
         DeviceGroup deviceGroup = DeviceGroup.valueOf(out.readByte());
-        byte tmp = out.readByte();
-        byte rackId = (byte) ((tmp & 0xF0) >> 4);
-        byte slotId = (byte) (tmp & 0x0F);
+        byte rackAndSlot = out.readByte();
+        byte rackId = (byte) ((rackAndSlot & 0xF0) >> 4);
+        byte slotId = (byte) (rackAndSlot & 0x0F);
         switch (parameterCode) {
             case CALLING_TSAP:
                 return new CallingTsapParameter(deviceGroup, rackId, slotId);

-- 
To stop receiving notification emails like this one, please contact
"commits@plc4x.apache.org" <co...@plc4x.apache.org>.

[incubator-plc4x] 05/06: add more encode and decode parameter tests

Posted by jm...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jmclean pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-plc4x.git

commit bd85953d70eef96cd53002fd4ce44e40432e7df0
Author: Justin Mclean <jm...@apache.org>
AuthorDate: Mon Jan 8 14:16:41 2018 +1100

    add more encode and decode parameter tests
---
 .../plc4x/java/isotp/netty/IsoTPProtocolTest.java  | 266 ++++++++++++++++++++-
 1 file changed, 259 insertions(+), 7 deletions(-)

diff --git a/plc4j/protocols/s7/src/test/java/org/apache/plc4x/java/isotp/netty/IsoTPProtocolTest.java b/plc4j/protocols/s7/src/test/java/org/apache/plc4x/java/isotp/netty/IsoTPProtocolTest.java
index 55f3529..7950e26 100644
--- a/plc4j/protocols/s7/src/test/java/org/apache/plc4x/java/isotp/netty/IsoTPProtocolTest.java
+++ b/plc4j/protocols/s7/src/test/java/org/apache/plc4x/java/isotp/netty/IsoTPProtocolTest.java
@@ -23,9 +23,7 @@ import io.netty.buffer.Unpooled;
 import io.netty.channel.ChannelHandlerContext;
 import org.apache.plc4x.java.isoontcp.netty.model.IsoOnTcpMessage;
 import org.apache.plc4x.java.isotp.netty.model.IsoTPMessage;
-import org.apache.plc4x.java.isotp.netty.model.params.CallingTsapParameter;
-import org.apache.plc4x.java.isotp.netty.model.params.Parameter;
-import org.apache.plc4x.java.isotp.netty.model.params.TsapParameter;
+import org.apache.plc4x.java.isotp.netty.model.params.*;
 import org.apache.plc4x.java.isotp.netty.model.tpdus.*;
 import org.apache.plc4x.java.isotp.netty.model.types.*;
 import org.apache.plc4x.java.netty.NettyTestBase;
@@ -342,7 +340,7 @@ public class IsoTPProtocolTest {
 
     @Test
     @Tag("fast")
-    public void encodeCallingParameters() throws Exception {
+    public void encodeCallingParameter() throws Exception {
         ChannelHandlerContext ctx = new MockChannelHandlerContext();
         ByteBuf buf = Unpooled.buffer();
         ArrayList<Parameter> parmameters = new ArrayList<>();
@@ -372,16 +370,100 @@ public class IsoTPProtocolTest {
 
     @Test
     @Tag("fast")
+    public void encodeChecksumParameter() throws Exception {
+        ChannelHandlerContext ctx = new MockChannelHandlerContext();
+        ByteBuf buf = Unpooled.buffer();
+        ArrayList<Parameter> parmameters = new ArrayList<>();
+        ChecksumParameter checksumParameter = new ChecksumParameter((byte)0x77);
+        parmameters.add(checksumParameter);
+        ErrorTpdu tpdu = new ErrorTpdu((short)0x1, RejectCause.REASON_NOT_SPECIFIED, parmameters, buf);
+        ArrayList<Object> out = new ArrayList<>();
+
+        isoTPProtocol.encode(ctx, tpdu, out);
+
+        assertTrue(out.size() == 1, "Message not decoded");
+
+        ByteBuf userData = ((IsoOnTcpMessage)out.get(0)).getUserData();
+
+        assertTrue(userData.writerIndex() == 8, "Incorrect message length");
+        assertTrue(userData.readByte() == (byte)0x7, "Incorrect header length");
+        assertTrue(userData.readByte() == TpduCode.TPDU_ERROR.getCode(), "Incorrect Tpdu code");
+        assertTrue(userData.readShort() == (short)0x1, "Incorrect destination reference code");
+        assertTrue(userData.readByte() == RejectCause.REASON_NOT_SPECIFIED.getCode(), "Incorrect reject cause code");
+        assertTrue(userData.readByte() == ParameterCode.CHECKSUM.getCode(), "Incorrect parameter code");
+        assertTrue(userData.readByte() == (byte)0x1, "Incorrect parameter length");
+        assertTrue(userData.readByte() == 0x77, "Incorrect checksum");
+    }
+
+    @Test
+    @Tag("fast")
+    public void encodeAditionalInformationParameter() throws Exception {
+        ChannelHandlerContext ctx = new MockChannelHandlerContext();
+        ByteBuf buf = Unpooled.buffer();
+        ArrayList<Parameter> parmameters = new ArrayList<>();
+        byte[] data = {'O','p','p','s'};
+        DisconnectAdditionalInformationParameter informationParameter = new DisconnectAdditionalInformationParameter(data);
+        parmameters.add(informationParameter);
+        ErrorTpdu tpdu = new ErrorTpdu((short)0x1, RejectCause.REASON_NOT_SPECIFIED, parmameters, buf);
+        ArrayList<Object> out = new ArrayList<>();
+
+        isoTPProtocol.encode(ctx, tpdu, out);
+
+        assertTrue(out.size() == 1, "Message not decoded");
+
+        ByteBuf userData = ((IsoOnTcpMessage)out.get(0)).getUserData();
+
+        assertTrue(userData.writerIndex() == 11, "Incorrect message length");
+        assertTrue(userData.readByte() == (byte)0xA, "Incorrect header length");
+        assertTrue(userData.readByte() == TpduCode.TPDU_ERROR.getCode(), "Incorrect Tpdu code");
+        assertTrue(userData.readShort() == (short)0x1, "Incorrect destination reference code");
+        assertTrue(userData.readByte() == RejectCause.REASON_NOT_SPECIFIED.getCode(), "Incorrect reject cause code");
+        assertTrue(userData.readByte() == ParameterCode.DISCONNECT_ADDITIONAL_INFORMATION.getCode(), "Incorrect parameter code");
+        assertTrue(userData.readByte() == (byte)0x4, "Incorrect parameter length");
+        assertTrue(userData.readByte() == 'O', "Incorrect data");
+        assertTrue(userData.readByte() == 'p', "Incorrect data");
+        assertTrue(userData.readByte() == 'p', "Incorrect data");
+        assertTrue(userData.readByte() == 's', "Incorrect data");
+    }
+
+    @Test
+    @Tag("fast")
+    public void encodeSizeParameter() throws Exception {
+        ChannelHandlerContext ctx = new MockChannelHandlerContext();
+        ByteBuf buf = Unpooled.buffer();
+        ArrayList<Parameter> parmameters = new ArrayList<>();
+        TpduSizeParameter sizeParameter = new TpduSizeParameter(TpduSize.SIZE_512);
+        parmameters.add(sizeParameter);
+        ErrorTpdu tpdu = new ErrorTpdu((short)0x1, RejectCause.REASON_NOT_SPECIFIED, parmameters, buf);
+        ArrayList<Object> out = new ArrayList<>();
+
+        isoTPProtocol.encode(ctx, tpdu, out);
+
+        assertTrue(out.size() == 1, "Message not decoded");
+
+        ByteBuf userData = ((IsoOnTcpMessage)out.get(0)).getUserData();
+
+        assertTrue(userData.writerIndex() == 8, "Incorrect message length");
+        assertTrue(userData.readByte() == (byte)0x7, "Incorrect header length");
+        assertTrue(userData.readByte() == TpduCode.TPDU_ERROR.getCode(), "Incorrect Tpdu code");
+        assertTrue(userData.readShort() == (short)0x1, "Incorrect destination reference code");
+        assertTrue(userData.readByte() == RejectCause.REASON_NOT_SPECIFIED.getCode(), "Incorrect reject cause code");
+        assertTrue(userData.readByte() == ParameterCode.TPDU_SIZE.getCode(), "Incorrect parameter code");
+        assertTrue(userData.readByte() == (byte)0x1, "Incorrect parameter length");
+        assertTrue(userData.readByte() == TpduSize.SIZE_512.getCode(), "Incorrect tdpu size");
+    }
+
+    @Test
+    @Tag("fast")
     public void decodeError() throws Exception {
         ChannelHandlerContext ctx = new MockChannelHandlerContext();
         ByteBuf buf = Unpooled.buffer();
         ArrayList<Object> out = new ArrayList<>();
 
-        buf.writeByte(0x6); // header length
+        buf.writeByte(0x4); // header length
         buf.writeByte(TpduCode.TPDU_ERROR.getCode());
         buf.writeShort(0x01); // destination reference
-        buf.writeShort(0x02); // source reference
-        buf.writeByte(ProtocolClass.CLASS_0.getCode());
+        buf.writeByte(RejectCause.REASON_NOT_SPECIFIED.getCode());
         IsoOnTcpMessage in = new IsoOnTcpMessage(buf);
 
         isoTPProtocol.decode(ctx, in, out);
@@ -426,4 +508,174 @@ public class IsoTPProtocolTest {
         isoTPProtocol.decode(ctx, null, out);
         assertTrue(out.size() == 0, "Message decoded when blank message passed");
     }
+
+    @Test
+    @Tag("fast")
+    public void decodeCallingParameter() throws Exception {
+        ChannelHandlerContext ctx = new MockChannelHandlerContext();
+        ByteBuf buf = Unpooled.buffer();
+        ArrayList<Object> out = new ArrayList<>();
+
+        buf.writeByte(0x8); // header length
+        buf.writeByte(TpduCode.TPDU_ERROR.getCode());
+        buf.writeShort(0x01); // destination reference
+        buf.writeByte(RejectCause.REASON_NOT_SPECIFIED.getCode()); // reject clause
+        buf.writeByte(ParameterCode.CALLING_TSAP.getCode());
+        buf.writeByte(0x2); // parameter length
+        buf.writeByte(DeviceGroup.PG_OR_PC.getCode());
+        buf.writeByte((byte) ((0x1 << 4) | 0x7));
+        IsoOnTcpMessage in = new IsoOnTcpMessage(buf);
+
+        isoTPProtocol.decode(ctx, in, out);
+
+        assertTrue(out.size() == 1, "Message not decoded");
+
+        ErrorTpdu errorTpdu = (ErrorTpdu) ((IsoTPMessage)out.get(0)).getTpdu();
+
+        assertTrue(errorTpdu.getTpduCode() == TpduCode.TPDU_ERROR, "Message code not correct");
+        assertTrue(errorTpdu.getDestinationReference() == (short) 0x1, "Message destination reference not correct");
+        assertTrue(errorTpdu.getRejectCause() == RejectCause.REASON_NOT_SPECIFIED, "Message reject cause not correct");
+        assertTrue(errorTpdu.getParameters().size() == 1, "Incorrect number of parameters");
+        CallingTsapParameter parameter = (CallingTsapParameter) errorTpdu.getParameters().get(0);
+        assertTrue(parameter.getType() == ParameterCode.CALLING_TSAP, "Parameter type incorrect");
+        assertTrue(parameter.getDeviceGroup() == DeviceGroup.PG_OR_PC, "Device group incorrect");
+        assertTrue(parameter.getRackNumber() == 0x1, "Rack number incorrect");
+        assertTrue(parameter.getSlotNumber() == 0x7, "Slot number incorrect");
+    }
+
+    @Test
+    @Tag("fast")
+    public void decodeCalledParameter() throws Exception {
+        ChannelHandlerContext ctx = new MockChannelHandlerContext();
+        ByteBuf buf = Unpooled.buffer();
+        ArrayList<Object> out = new ArrayList<>();
+
+        buf.writeByte(0x8); // header length
+        buf.writeByte(TpduCode.TPDU_ERROR.getCode());
+        buf.writeShort(0x01); // destination reference
+        buf.writeByte(RejectCause.REASON_NOT_SPECIFIED.getCode()); // reject clause
+        buf.writeByte(ParameterCode.CALLED_TSAP.getCode());
+        buf.writeByte(0x2); // parameter length
+        buf.writeByte(DeviceGroup.PG_OR_PC.getCode());
+        buf.writeByte((byte) ((0x2 << 4) | 0x3));
+        IsoOnTcpMessage in = new IsoOnTcpMessage(buf);
+
+        isoTPProtocol.decode(ctx, in, out);
+
+        assertTrue(out.size() == 1, "Message not decoded");
+
+        ErrorTpdu errorTpdu = (ErrorTpdu) ((IsoTPMessage)out.get(0)).getTpdu();
+
+        assertTrue(errorTpdu.getTpduCode() == TpduCode.TPDU_ERROR, "Message code not correct");
+        assertTrue(errorTpdu.getDestinationReference() == (short) 0x1, "Message destination reference not correct");
+        assertTrue(errorTpdu.getRejectCause() == RejectCause.REASON_NOT_SPECIFIED, "Message reject cause not correct");
+        assertTrue(errorTpdu.getParameters().size() == 1, "Incorrect number of parameters");
+        CalledTsapParameter parameter = (CalledTsapParameter) errorTpdu.getParameters().get(0);
+        assertTrue(parameter.getType() == ParameterCode.CALLED_TSAP, "Parameter type incorrect");
+        assertTrue(parameter.getDeviceGroup() == DeviceGroup.PG_OR_PC, "Device group incorrect");
+        assertTrue(parameter.getRackNumber() == 0x2, "Rack number incorrect");
+        assertTrue(parameter.getSlotNumber() == 0x3, "Slot number incorrect");
+    }
+
+    @Test
+    @Tag("fast")
+    public void decodeChecksumParameter() throws Exception {
+        ChannelHandlerContext ctx = new MockChannelHandlerContext();
+        ByteBuf buf = Unpooled.buffer();
+        ArrayList<Object> out = new ArrayList<>();
+
+        buf.writeByte(0x8); // header length
+        buf.writeByte(TpduCode.TPDU_ERROR.getCode());
+        buf.writeShort(0x01); // destination reference
+        buf.writeByte(RejectCause.REASON_NOT_SPECIFIED.getCode()); // reject clause
+        buf.writeByte(ParameterCode.CHECKSUM.getCode());
+        buf.writeByte(0x1); // parameter length
+        buf.writeByte(0x33); // checksum
+        IsoOnTcpMessage in = new IsoOnTcpMessage(buf);
+
+        isoTPProtocol.decode(ctx, in, out);
+
+        assertTrue(out.size() == 1, "Message not decoded");
+
+        ErrorTpdu errorTpdu = (ErrorTpdu) ((IsoTPMessage)out.get(0)).getTpdu();
+
+        assertTrue(errorTpdu.getTpduCode() == TpduCode.TPDU_ERROR, "Message code not correct");
+        assertTrue(errorTpdu.getDestinationReference() == (short) 0x1, "Message destination reference not correct");
+        assertTrue(errorTpdu.getRejectCause() == RejectCause.REASON_NOT_SPECIFIED, "Message reject cause not correct");
+        assertTrue(errorTpdu.getParameters().size() == 1, "Incorrect number of parameters");
+        ChecksumParameter parameter = (ChecksumParameter) errorTpdu.getParameters().get(0);
+        assertTrue(parameter.getType() == ParameterCode.CHECKSUM, "Parameter type incorrect");
+        assertTrue(parameter.getChecksum() == 0x33, "Checksum incorrect");
+    }
+
+    @Test
+    @Tag("fast")
+    public void decodeSizeParameter() throws Exception {
+        ChannelHandlerContext ctx = new MockChannelHandlerContext();
+        ByteBuf buf = Unpooled.buffer();
+        ArrayList<Object> out = new ArrayList<>();
+
+        buf.writeByte(0x8); // header length
+        buf.writeByte(TpduCode.TPDU_ERROR.getCode());
+        buf.writeShort(0x01); // destination reference
+        buf.writeByte(RejectCause.REASON_NOT_SPECIFIED.getCode()); // reject clause
+        buf.writeByte(ParameterCode.TPDU_SIZE.getCode());
+        buf.writeByte(0x1); // parameter length
+        buf.writeByte(TpduSize.SIZE_256.getCode());
+        IsoOnTcpMessage in = new IsoOnTcpMessage(buf);
+
+        isoTPProtocol.decode(ctx, in, out);
+
+        assertTrue(out.size() == 1, "Message not decoded");
+
+        ErrorTpdu errorTpdu = (ErrorTpdu) ((IsoTPMessage)out.get(0)).getTpdu();
+
+        assertTrue(errorTpdu.getTpduCode() == TpduCode.TPDU_ERROR, "Message code not correct");
+        assertTrue(errorTpdu.getDestinationReference() == (short) 0x1, "Message destination reference not correct");
+        assertTrue(errorTpdu.getRejectCause() == RejectCause.REASON_NOT_SPECIFIED, "Message reject cause not correct");
+        assertTrue(errorTpdu.getParameters().size() == 1, "Incorrect number of parameters");
+        TpduSizeParameter parameter = (TpduSizeParameter) errorTpdu.getParameters().get(0);
+        assertTrue(parameter.getType() == ParameterCode.TPDU_SIZE, "Parameter type incorrect");
+        assertTrue(parameter.getTpduSize() == TpduSize.SIZE_256, "Size incorrect");
+    }
+
+    @Test
+    @Tag("fast")
+    public void decodeAdditionalInformationParameter() throws Exception {
+        ChannelHandlerContext ctx = new MockChannelHandlerContext();
+        ByteBuf buf = Unpooled.buffer();
+        ArrayList<Object> out = new ArrayList<>();
+
+        buf.writeByte(0x8); // header length
+        buf.writeByte(TpduCode.TPDU_ERROR.getCode());
+        buf.writeShort(0x01); // destination reference
+        buf.writeByte(RejectCause.REASON_NOT_SPECIFIED.getCode()); // reject clause
+        buf.writeByte(ParameterCode.DISCONNECT_ADDITIONAL_INFORMATION.getCode());
+        buf.writeByte(0x5); // parameter length
+        buf.writeByte('E');
+        buf.writeByte('r');
+        buf.writeByte('r');
+        buf.writeByte('o');
+        buf.writeByte('r');
+        IsoOnTcpMessage in = new IsoOnTcpMessage(buf);
+
+        isoTPProtocol.decode(ctx, in, out);
+
+        assertTrue(out.size() == 1, "Message not decoded");
+
+        ErrorTpdu errorTpdu = (ErrorTpdu) ((IsoTPMessage)out.get(0)).getTpdu();
+
+        assertTrue(errorTpdu.getTpduCode() == TpduCode.TPDU_ERROR, "Message code not correct");
+        assertTrue(errorTpdu.getDestinationReference() == (short) 0x1, "Message destination reference not correct");
+        assertTrue(errorTpdu.getRejectCause() == RejectCause.REASON_NOT_SPECIFIED, "Message reject cause not correct");
+        assertTrue(errorTpdu.getParameters().size() == 1, "Incorrect number of parameters");
+        DisconnectAdditionalInformationParameter parameter = (DisconnectAdditionalInformationParameter) errorTpdu.getParameters().get(0);
+        assertTrue(parameter.getType() == ParameterCode.DISCONNECT_ADDITIONAL_INFORMATION, "Parameter type incorrect");
+        byte[] data = parameter.getData();
+        assertTrue(data[0] == 'E', "Data incorrect");
+        assertTrue(data[1] == 'r', "Data incorrect");
+        assertTrue(data[2] == 'r', "Data incorrect");
+        assertTrue(data[3] == 'o', "Data incorrect");
+        assertTrue(data[4] == 'r', "Data incorrect");
+    }
 }

-- 
To stop receiving notification emails like this one, please contact
"commits@plc4x.apache.org" <co...@plc4x.apache.org>.

[incubator-plc4x] 04/06: no real need to calculate size/length every time through the loop

Posted by jm...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jmclean pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-plc4x.git

commit 5ad5d2d69327b13ae843d4c98c506e9c477b704b
Author: Justin Mclean <jm...@apache.org>
AuthorDate: Mon Jan 8 14:16:02 2018 +1100

    no real need to calculate size/length every time through the loop
---
 .../plc4x/java/s7/netty/Plc4XS7Protocol.java       | 34 ++++++++++++----------
 1 file changed, 19 insertions(+), 15 deletions(-)

diff --git a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/Plc4XS7Protocol.java b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/Plc4XS7Protocol.java
index a0520c8..d50486a 100644
--- a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/Plc4XS7Protocol.java
+++ b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/Plc4XS7Protocol.java
@@ -88,7 +88,7 @@ public class Plc4XS7Protocol extends MessageToMessageCodec<S7Message, PlcRequest
             requests.put(s7ReadRequest.getTpduReference(), msg);
 
             out.add(s7ReadRequest);
-        } else if(msg.getRequest() instanceof PlcWriteRequest) {
+        } else if (msg.getRequest() instanceof PlcWriteRequest) {
             List<VarParameterItem> parameterItems = new LinkedList<>();
             List<VarPayloadItem> payloadItems = new LinkedList<>();
 
@@ -154,7 +154,8 @@ public class Plc4XS7Protocol extends MessageToMessageCodec<S7Message, PlcRequest
                             "The number of requested items doesn't match the number of returned items");
                     }
                     List<VarPayloadItem> payloadItems = payload.getPayloadItems();
-                    for (int i = 0; i < payloadItems.size(); i++) {
+                    final int noPayLoadItems = payloadItems.size();
+                    for (int i = 0; i < noPayLoadItems; i++) {
                         VarPayloadItem payloadItem = payloadItems.get(i);
 
                         // Get the request item for this payload item
@@ -192,7 +193,8 @@ public class Plc4XS7Protocol extends MessageToMessageCodec<S7Message, PlcRequest
                             "The number of requested items doesn't match the number of returned items");
                     }
                     List<VarPayloadItem> payloadItems = payload.getPayloadItems();
-                    for (int i = 0; i < payloadItems.size(); i++) {
+                    final int noPayLoadItems = payloadItems.size();
+                    for (int i = 0; i < noPayLoadItems; i++) {
                         VarPayloadItem payloadItem = payloadItems.get(i);
 
                         // Get the request item for this payload item
@@ -286,32 +288,33 @@ public class Plc4XS7Protocol extends MessageToMessageCodec<S7Message, PlcRequest
     }
 
     private byte[] encodeData(Object[] values) {
-        if(values.length == 0) {
+        final int length = values.length;
+        if (length == 0) {
             return new byte[]{};
         }
         byte[] result = null;
         Class valueType = values[0].getClass();
         if (valueType == Boolean.class) {
             // TODO: Check if this is true and the result is not Math.ceil(values.lenght / 8)
-            result = new byte[values.length];
-            for(int i = 0; i < values.length; i++) {
+            result = new byte[length];
+            for (int i = 0; i < length; i++) {
                 result[i] = (byte) (((Boolean) values[i]) ? 0x01 : 0x00);
             }
         } else if (valueType == Byte[].class) {
-            result = new byte[values.length];
-            for(int i = 0; i < values.length; i++) {
+            result = new byte[length];
+            for (int i = 0; i < length; i++) {
                 result[i] = (byte) values[i];
             }
         } else if (valueType == Short.class) {
-            result = new byte[values.length * 2];
-            for(int i = 0; i < values.length; i++) {
+            result = new byte[length * 2];
+            for (int i = 0; i < length; i++) {
                 short intValue = (short) values[i];
                 result[i * 2] = (byte) ((intValue & 0xff00) >> 8);
                 result[(i * 2) + 1] = (byte) (intValue & 0xff);
             }
         } else if (valueType == Integer.class) {
-            result = new byte[values.length * 4];
-            for(int i = 0; i < values.length; i++) {
+            result = new byte[length * 4];
+            for (int i = 0; i < length; i++) {
                 int intValue = (int) values[i];
                 result[i * 4] = (byte) ((intValue & 0xff000000) >> 24);
                 result[(i * 4) + 1] = (byte) ((intValue & 0x00ff0000) >> 16);
@@ -321,8 +324,8 @@ public class Plc4XS7Protocol extends MessageToMessageCodec<S7Message, PlcRequest
         } else if (valueType == Calendar.class) {
             result = null;
         } else if (valueType == Float.class) {
-            result = new byte[values.length * 4];
-            for(int i = 0; i < values.length; i++) {
+            result = new byte[length * 4];
+            for (int i = 0; i < length; i++) {
                 float floatValue = (float) values[i];
                 int intValue = Float.floatToIntBits(floatValue);
                 result[i * 4] = (byte) ((intValue & 0xff000000) >> 24);
@@ -359,8 +362,9 @@ public class Plc4XS7Protocol extends MessageToMessageCodec<S7Message, PlcRequest
     private List<Object> decodeData(Class<?> datatype, byte[] s7Data) throws PlcProtocolException {
         List<Object> result = new LinkedList<>();
         int i = 0;
+        final int length = s7Data.length;
         
-        while (i < s7Data.length) {
+        while (i < length) {
             if (datatype == Boolean.class) {
                 result.add((s7Data[i] & 0x01) == 0x01);
                 i+=1;

-- 
To stop receiving notification emails like this one, please contact
"commits@plc4x.apache.org" <co...@plc4x.apache.org>.