You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@plc4x.apache.org by cd...@apache.org on 2018/11/29 09:29:17 UTC

[incubator-plc4x] 01/02: PLC4X-76 - When receiving responses with more than 512 byte, the IsoOnTcp protocol doesn't work

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

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

commit dcca499a4d3537aeb08dd55b93c2cc1d3505f45d
Author: Christofer Dutz <ch...@c-ware.de>
AuthorDate: Thu Nov 29 10:28:36 2018 +0100

    PLC4X-76 - When receiving responses with more than 512 byte, the IsoOnTcp protocol doesn't work
    
    - Refactored the layer directly on TCP for S7 to work with chunked messages.
---
 .../java/s7/connection/S7PlcConnectionIT.java      |  19 +++++++
 .../s7/connection/s7-read-large-response.pcapng    | Bin 0 -> 1040 bytes
 .../plc4x/java/base/PlcByteToMessageCodec.java     |  59 +++++++++++++++++++++
 .../java/isoontcp/protocol/IsoOnTcpProtocol.java   |  25 ++++-----
 4 files changed, 91 insertions(+), 12 deletions(-)

diff --git a/plc4j/drivers/s7/src/test/java/org/apache/plc4x/java/s7/connection/S7PlcConnectionIT.java b/plc4j/drivers/s7/src/test/java/org/apache/plc4x/java/s7/connection/S7PlcConnectionIT.java
index 00d16bd..831d0d3 100644
--- a/plc4j/drivers/s7/src/test/java/org/apache/plc4x/java/s7/connection/S7PlcConnectionIT.java
+++ b/plc4j/drivers/s7/src/test/java/org/apache/plc4x/java/s7/connection/S7PlcConnectionIT.java
@@ -28,6 +28,7 @@ import org.junit.After;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
+import org.junit.jupiter.api.Disabled;
 import org.junit.rules.Timeout;
 
 import java.util.concurrent.CompletableFuture;
@@ -88,6 +89,24 @@ public class S7PlcConnectionIT {
         SUT.close();
     }
 
+    /**
+     * In case of a slow network connection Netty tends to call the IsoOnTcpProtocol decode method
+     * prior to reading the full packet. Therefor we usually check if enough bytes have been read.
+     * If not we give up and wait for Netty to call again.
+     *
+     * In case of a fast connection with large response sizes it seems that Netty splits up the
+     * responses into 512 byte chunks.
+     */
+    @Test
+    @Disabled
+    public void readLargeResponse() throws Exception {
+        SUT.connect();
+        EmbeddedChannel channel = (EmbeddedChannel) SUT.getChannel();
+        assertThat("No outbound messages should exist.", channel.outboundMessages().size(), equalTo(0));
+
+        SUT.sendPcapFile("org/apache/plc4x/java/s7/connection/s7-read-large-response.pcapng");
+    }
+
     @Test
     public void write() throws Exception {
         SUT.connect();
diff --git a/plc4j/drivers/s7/src/test/resources/org/apache/plc4x/java/s7/connection/s7-read-large-response.pcapng b/plc4j/drivers/s7/src/test/resources/org/apache/plc4x/java/s7/connection/s7-read-large-response.pcapng
new file mode 100644
index 0000000..c4cbcf7
Binary files /dev/null and b/plc4j/drivers/s7/src/test/resources/org/apache/plc4x/java/s7/connection/s7-read-large-response.pcapng differ
diff --git a/plc4j/protocols/driver-bases/base/src/main/java/org/apache/plc4x/java/base/PlcByteToMessageCodec.java b/plc4j/protocols/driver-bases/base/src/main/java/org/apache/plc4x/java/base/PlcByteToMessageCodec.java
new file mode 100644
index 0000000..a97ef2c
--- /dev/null
+++ b/plc4j/protocols/driver-bases/base/src/main/java/org/apache/plc4x/java/base/PlcByteToMessageCodec.java
@@ -0,0 +1,59 @@
+/*
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements.  See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership.  The ASF licenses this file
+ to you 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.apache.plc4x.java.base;
+
+import io.netty.channel.ChannelHandler;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.ByteToMessageCodec;
+import org.apache.commons.lang3.reflect.FieldUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.reflect.Field;
+
+public abstract class PlcByteToMessageCodec<OUTBOUND_IN>
+    extends ByteToMessageCodec<OUTBOUND_IN> {
+
+    private static final Logger logger = LoggerFactory.getLogger(PlcByteToMessageCodec.class);
+
+    private volatile ChannelHandler prevChannelHandler = null;
+
+    public PlcByteToMessageCodec() {
+    }
+
+    public PlcByteToMessageCodec(Class<? extends OUTBOUND_IN> outboundMessageType) {
+        super(outboundMessageType);
+    }
+
+    protected ChannelHandler getPrevChannelHandler(ChannelHandlerContext ctx) {
+        if(prevChannelHandler == null) {
+            try {
+                Field prevField = FieldUtils.getField(ctx.getClass(), "prev", true);
+                if(prevField != null) {
+                    ChannelHandlerContext prevContext = (ChannelHandlerContext) prevField.get(ctx);
+                    prevChannelHandler = prevContext.handler();
+                }
+            } catch(Exception e) {
+                logger.error("Error accessing field 'prev'", e);
+            }
+        }
+        return prevChannelHandler;
+    }
+
+}
diff --git a/plc4j/protocols/iso-on-tcp/src/main/java/org/apache/plc4x/java/isoontcp/protocol/IsoOnTcpProtocol.java b/plc4j/protocols/iso-on-tcp/src/main/java/org/apache/plc4x/java/isoontcp/protocol/IsoOnTcpProtocol.java
index 32648fa..afd6146 100644
--- a/plc4j/protocols/iso-on-tcp/src/main/java/org/apache/plc4x/java/isoontcp/protocol/IsoOnTcpProtocol.java
+++ b/plc4j/protocols/iso-on-tcp/src/main/java/org/apache/plc4x/java/isoontcp/protocol/IsoOnTcpProtocol.java
@@ -20,17 +20,16 @@ package org.apache.plc4x.java.isoontcp.protocol;
 
 import io.netty.buffer.ByteBuf;
 import io.netty.buffer.ByteBufUtil;
-import io.netty.buffer.Unpooled;
 import io.netty.channel.ChannelHandlerContext;
 import org.apache.plc4x.java.api.exceptions.PlcProtocolException;
-import org.apache.plc4x.java.base.PlcMessageToMessageCodec;
+import org.apache.plc4x.java.base.PlcByteToMessageCodec;
 import org.apache.plc4x.java.isoontcp.protocol.model.IsoOnTcpMessage;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.util.List;
 
-public class IsoOnTcpProtocol extends PlcMessageToMessageCodec<ByteBuf, IsoOnTcpMessage> {
+public class IsoOnTcpProtocol extends PlcByteToMessageCodec<IsoOnTcpMessage> {
 
     static final byte ISO_ON_TCP_MAGIC_NUMBER = 0x03;
 
@@ -41,7 +40,7 @@ public class IsoOnTcpProtocol extends PlcMessageToMessageCodec<ByteBuf, IsoOnTcp
     ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 
     @Override
-    protected void encode(ChannelHandlerContext ctx, IsoOnTcpMessage in, List<Object> out) {
+    protected void encode(ChannelHandlerContext ctx, IsoOnTcpMessage in, ByteBuf out) throws Exception {
         logger.debug("ISO on TCP Message sent");
         // At this point of processing all higher levels have already serialized their payload.
         // This data is passed to the lower levels in form of an IoBuffer.
@@ -49,22 +48,19 @@ public class IsoOnTcpProtocol extends PlcMessageToMessageCodec<ByteBuf, IsoOnTcp
 
         int packetSize = userData.readableBytes() + 4;
 
-        ByteBuf buf = Unpooled.buffer();
         // Version (is always constant 0x03)
-        buf.writeByte(ISO_ON_TCP_MAGIC_NUMBER);
+        out.writeByte(ISO_ON_TCP_MAGIC_NUMBER);
         // Reserved (is always constant 0x00)
-        buf.writeByte((byte) 0x00);
+        out.writeByte((byte) 0x00);
         // Packet length (including ISOonTCP header)
         // ("remaining" returns the number of bytes left to read in this buffer.
         // It is usually set to a read position of 0 and a limit at the end.
         // So in general remaining is equivalent to a non-existing
         // "userData.size()" method.)
-        buf.writeShort((short) packetSize);
+        out.writeShort((short) packetSize);
 
         // Output the payload.
-        buf.writeBytes(userData);
-
-        out.add(buf);
+        out.writeBytes(userData);
     }
 
     ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
@@ -79,7 +75,9 @@ public class IsoOnTcpProtocol extends PlcMessageToMessageCodec<ByteBuf, IsoOnTcp
         // If at least 4 bytes are readable, peek into them (without changing the read position)
         // and get the packet length. Only if the available amount of readable bytes is larger or
         // equal to this, continue processing the rest.
-        if(in.readableBytes() >= 4) {
+        /*if(chunkedResponse != null) {
+            chunkedResponse.writeBytes(in);
+        } else*/ if(in.readableBytes() >= 4) {
             logger.debug("ISO on TCP Message received");
             // The ISO on TCP protocol is really simple and in this case the buffer length
             // will take care of the higher levels not reading more than is in the packet.
@@ -101,6 +99,9 @@ public class IsoOnTcpProtocol extends PlcMessageToMessageCodec<ByteBuf, IsoOnTcp
                 // Simply place the current buffer to the output ... the next handler will continue.
                 ByteBuf payload = in.readBytes(packetLength - 4);
                 out.add(new IsoOnTcpMessage(payload));
+            /*} else {
+                chunkedResponse = Unpooled.buffer(packetLength);
+                chunkedResponse.writeBytes(in, packetLength);*/
             }
         }
     }