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/13 00:45:48 UTC

[incubator-plc4x] branch master updated (ef8c3d4 -> d88234a)

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 ef8c3d4  added one missing testcase for typed read response
     new e9d0fab  no need for empty constructor
     new d88234a  refactor to reduce complexity

The 2 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/s7/netty/Plc4XS7Protocol.java       | 441 ++++++++++++---------
 .../org/apache/plc4x/java/s7/netty/S7Protocol.java | 354 +++++++++--------
 .../netty/model/params/CpuServicesParameter.java   |   4 -
 3 files changed, 445 insertions(+), 354 deletions(-)

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

[incubator-plc4x] 01/02: no need for empty constructor

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 e9d0fab2cabc88d9182780bbcff58e01df8d5e28
Author: Justin Mclean <jm...@apache.org>
AuthorDate: Sat Jan 13 11:45:20 2018 +1100

    no need for empty constructor
---
 .../apache/plc4x/java/s7/netty/model/params/CpuServicesParameter.java | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/model/params/CpuServicesParameter.java b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/model/params/CpuServicesParameter.java
index dcd9680..7914179 100644
--- a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/model/params/CpuServicesParameter.java
+++ b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/model/params/CpuServicesParameter.java
@@ -22,10 +22,6 @@ import org.apache.plc4x.java.s7.netty.model.types.ParameterType;
 
 public class CpuServicesParameter implements S7Parameter {
 
-
-    public CpuServicesParameter() {
-    }
-
     @Override
     public ParameterType getType() {
         return ParameterType.CPU_SERVICES;

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

[incubator-plc4x] 02/02: refactor to reduce complexity

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 d88234a0af556cc3b19181dd3503ad5ced2ec6b5
Author: Justin Mclean <jm...@apache.org>
AuthorDate: Sat Jan 13 11:45:38 2018 +1100

    refactor to reduce complexity
---
 .../plc4x/java/s7/netty/Plc4XS7Protocol.java       | 441 ++++++++++++---------
 .../org/apache/plc4x/java/s7/netty/S7Protocol.java | 354 +++++++++--------
 2 files changed, 445 insertions(+), 350 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 e706405..f359a3a 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
@@ -61,171 +61,197 @@ public class Plc4XS7Protocol extends MessageToMessageCodec<S7Message, PlcRequest
 
     @Override
     protected void encode(ChannelHandlerContext ctx, PlcRequestContainer msg, List<Object> out) throws Exception {
-        if (msg.getRequest() instanceof PlcReadRequest) {
-            List<VarParameterItem> parameterItems = new LinkedList<>();
-
-            PlcReadRequest readRequest = (PlcReadRequest) msg.getRequest();
-            for (ReadRequestItem requestItem : readRequest.getRequestItems()) {
-                // Try to get the correct S7 transport size for the given data type.
-                // (Map PLC4X data type to S7 data type)
-                TransportSize transportSize = encodeTransportSize(requestItem.getDatatype());
-                if (transportSize == null) {
-                    throw new PlcException("Unknown transport size for datatype " + requestItem.getDatatype());
-                }
+        PlcRequest request = msg.getRequest();
+        if (request instanceof PlcReadRequest) {
+            encodeREadRequest(msg, out);
+        } else if (request instanceof PlcWriteRequest) {
+            encodeWriteRequest(msg, out);
+        }
+    }
 
-                // Depending on the address type, generate the corresponding type of request item.
-                VarParameterItem varParameterItem = encodeVarParameterItem(requestItem.getAddress(), transportSize, requestItem.getSize());
-                parameterItems.add(varParameterItem);
+    private void encodeWriteRequest(PlcRequestContainer msg, List<Object> out) throws PlcException {
+        List<VarParameterItem> parameterItems = new LinkedList<>();
+        List<VarPayloadItem> payloadItems = new LinkedList<>();
+
+        PlcWriteRequest writeRequest = (PlcWriteRequest) msg.getRequest();
+        for (WriteRequestItem requestItem : writeRequest.getRequestItems()) {
+            // Try to get the correct S7 transport size for the given data type.
+            // (Map PLC4X data type to S7 data type)
+            TransportSize transportSize = encodeTransportSize(requestItem.getDatatype());
+            if (transportSize == null) {
+                throw new PlcException("Unknown transport size for datatype " + requestItem.getDatatype());
             }
-            VarParameter readVarParameter = new VarParameter(ParameterType.READ_VAR, parameterItems);
-
-            // Assemble the request.
-            S7RequestMessage s7ReadRequest = new S7RequestMessage(MessageType.JOB,
-                (short) tpduGenerator.getAndIncrement(), Collections.singletonList(readVarParameter),
-                Collections.emptyList());
-
-            requests.put(s7ReadRequest.getTpduReference(), msg);
-
-            out.add(s7ReadRequest);
-        } else if (msg.getRequest() instanceof PlcWriteRequest) {
-            List<VarParameterItem> parameterItems = new LinkedList<>();
-            List<VarPayloadItem> payloadItems = new LinkedList<>();
-
-            PlcWriteRequest writeRequest = (PlcWriteRequest) msg.getRequest();
-            for (WriteRequestItem requestItem : writeRequest.getRequestItems()) {
-                // Try to get the correct S7 transport size for the given data type.
-                // (Map PLC4X data type to S7 data type)
-                TransportSize transportSize = encodeTransportSize(requestItem.getDatatype());
-                if (transportSize == null) {
-                    throw new PlcException("Unknown transport size for datatype " + requestItem.getDatatype());
-                }
 
-                // Depending on the address type, generate the corresponding type of request item.
-                VarParameterItem varParameterItem = encodeVarParameterItem(
-                    requestItem.getAddress(), transportSize, requestItem.getValues().size());
-                parameterItems.add(varParameterItem);
+            // Depending on the address type, generate the corresponding type of request item.
+            VarParameterItem varParameterItem = encodeVarParameterItem(
+                requestItem.getAddress(), transportSize, requestItem.getValues().size());
+            parameterItems.add(varParameterItem);
 
-                DataTransportSize dataTransportSize = encodeDataTransportSize(requestItem.getDatatype());
-                if (dataTransportSize == null) {
-                    throw new PlcException("Unknown data transport size for datatype " + requestItem.getDatatype());
-                }
+            DataTransportSize dataTransportSize = encodeDataTransportSize(requestItem.getDatatype());
+            if (dataTransportSize == null) {
+                throw new PlcException("Unknown data transport size for datatype " + requestItem.getDatatype());
+            }
 
-                VarPayloadItem varPayloadItem = new VarPayloadItem(
-                    DataTransportErrorCode.RESERVED, dataTransportSize, encodeData(requestItem.getValues().toArray()));
+            VarPayloadItem varPayloadItem = new VarPayloadItem(
+                DataTransportErrorCode.RESERVED, dataTransportSize, encodeData(requestItem.getValues().toArray()));
 
-                payloadItems.add(varPayloadItem);
-            }
-            VarParameter writeVarParameter = new VarParameter(ParameterType.WRITE_VAR, parameterItems);
-            VarPayload writeVarPayload = new VarPayload(ParameterType.WRITE_VAR, payloadItems);
+            payloadItems.add(varPayloadItem);
+        }
+        VarParameter writeVarParameter = new VarParameter(ParameterType.WRITE_VAR, parameterItems);
+        VarPayload writeVarPayload = new VarPayload(ParameterType.WRITE_VAR, payloadItems);
+
+        // Assemble the request.
+        S7RequestMessage s7WriteRequest = new S7RequestMessage(MessageType.JOB,
+            (short) tpduGenerator.getAndIncrement(), Collections.singletonList(writeVarParameter),
+            Collections.singletonList(writeVarPayload));
+
+        requests.put(s7WriteRequest.getTpduReference(), msg);
 
-            // Assemble the request.
-            S7RequestMessage s7WriteRequest = new S7RequestMessage(MessageType.JOB,
-                (short) tpduGenerator.getAndIncrement(), Collections.singletonList(writeVarParameter),
-                Collections.singletonList(writeVarPayload));
+        out.add(s7WriteRequest);
+    }
+
+    private void encodeREadRequest(PlcRequestContainer msg, List<Object> out) throws PlcException {
+        List<VarParameterItem> parameterItems = new LinkedList<>();
+
+        PlcReadRequest readRequest = (PlcReadRequest) msg.getRequest();
+        encodeParameterItems(parameterItems, readRequest);
+        VarParameter readVarParameter = new VarParameter(ParameterType.READ_VAR, parameterItems);
+
+        // Assemble the request.
+        S7RequestMessage s7ReadRequest = new S7RequestMessage(MessageType.JOB,
+            (short) tpduGenerator.getAndIncrement(), Collections.singletonList(readVarParameter),
+            Collections.emptyList());
+
+        requests.put(s7ReadRequest.getTpduReference(), msg);
 
-            requests.put(s7WriteRequest.getTpduReference(), msg);
+        out.add(s7ReadRequest);
+    }
+
+    private void encodeParameterItems(List<VarParameterItem> parameterItems, PlcReadRequest readRequest) throws PlcException {
+        for (ReadRequestItem requestItem : readRequest.getRequestItems()) {
+            // Try to get the correct S7 transport size for the given data type.
+            // (Map PLC4X data type to S7 data type)
+            TransportSize transportSize = encodeTransportSize(requestItem.getDatatype());
+            if (transportSize == null) {
+                throw new PlcException("Unknown transport size for datatype " + requestItem.getDatatype());
+            }
 
-            out.add(s7WriteRequest);
+            // Depending on the address type, generate the corresponding type of request item.
+            VarParameterItem varParameterItem = encodeVarParameterItem(requestItem.getAddress(), transportSize, requestItem.getSize());
+            parameterItems.add(varParameterItem);
         }
     }
 
     @SuppressWarnings("unchecked")
     @Override
     protected void decode(ChannelHandlerContext ctx, S7Message msg, List<Object> out) throws Exception {
-        if (msg instanceof S7ResponseMessage) {
-            S7ResponseMessage responseMessage = (S7ResponseMessage) msg;
-            short tpduReference = responseMessage.getTpduReference();
-            if (requests.containsKey(tpduReference)) {
-                PlcRequestContainer requestContainer = requests.remove(tpduReference);
-
-                PlcResponse response = null;
-
-                // Handle the response to a read request.
-                if (requestContainer.getRequest() instanceof PlcReadRequest) {
-                    PlcReadRequest plcReadRequest = (PlcReadRequest) requestContainer.getRequest();
-
-                    List<ReadResponseItem<?>> responseItems = new LinkedList<>();
-                    VarPayload payload = responseMessage.getPayload(VarPayload.class)
-                        .orElseThrow(() -> new PlcProtocolException("No VarPayload supplied"));
-                    // If the numbers of items don't match, we're in big trouble as the only
-                    // way to know how to interpret the responses is by aligning them with the
-                    // items from the request as this information is not returned by the PLC.
-                    if (plcReadRequest.getRequestItems().size() != payload.getPayloadItems().size()) {
-                        throw new PlcProtocolException(
-                            "The number of requested items doesn't match the number of returned items");
-                    }
-                    List<VarPayloadItem> payloadItems = payload.getPayloadItems();
-                    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
-                        ReadRequestItem requestItem = plcReadRequest.getRequestItems().get(i);
-
-                        ResponseCode responseCode = decodeResponseCode(payloadItem.getReturnCode());
-
-                        ReadResponseItem responseItem;
-                        // Something went wrong.
-                        if (responseCode != ResponseCode.OK) {
-                            responseItem = new ReadResponseItem<>(requestItem, responseCode, null);
-                        }
-                        // All Ok.
-                        else {
-                            byte[] data = payloadItem.getData();
-                            Class<?> datatype = requestItem.getDatatype();
-                            List<?> value = decodeData(datatype, data);
-                            responseItem = new ReadResponseItem(requestItem, responseCode, value);
-                        }
-                        responseItems.add(responseItem);
-                    }
-                    if (plcReadRequest instanceof TypeSafePlcReadRequest) {
-                        response = new TypeSafePlcReadResponse((TypeSafePlcReadRequest) plcReadRequest, responseItems);
-                    } else {
-                        response = new PlcReadResponse(plcReadRequest, responseItems);
-                    }
-                }
+        if (!(msg instanceof S7ResponseMessage)) {
+            return;
+        }
+        S7ResponseMessage responseMessage = (S7ResponseMessage) msg;
+        short tpduReference = responseMessage.getTpduReference();
+        if (requests.containsKey(tpduReference)) {
+            PlcRequestContainer requestContainer = requests.remove(tpduReference);
+            PlcRequest request = requestContainer.getRequest();
+            PlcResponse response = null;
+
+            // Handle the response to a read request.
+            if (request instanceof PlcReadRequest) {
+                response = decodeReadRequest(responseMessage, requestContainer);
+            }
+            else if (request instanceof PlcWriteRequest) {
+                response = decodeWriteRequest(responseMessage, requestContainer);
+            }
 
-                // Handle the response to a write request.
-                else if (requestContainer.getRequest() instanceof PlcWriteRequest) {
-                    PlcWriteRequest plcWriteRequest = (PlcWriteRequest) requestContainer.getRequest();
-                    List<WriteResponseItem<?>> responseItems = new LinkedList<>();
-                    VarPayload payload = responseMessage.getPayload(VarPayload.class)
-                        .orElseThrow(() -> new PlcProtocolException("No VarPayload supplied"));
-                    // If the numbers of items don't match, we're in big trouble as the only
-                    // way to know how to interpret the responses is by aligning them with the
-                    // items from the request as this information is not returned by the PLC.
-                    if (plcWriteRequest.getRequestItems().size() != payload.getPayloadItems().size()) {
-                        throw new PlcProtocolException(
-                            "The number of requested items doesn't match the number of returned items");
-                    }
-                    List<VarPayloadItem> payloadItems = payload.getPayloadItems();
-                    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
-                        WriteRequestItem requestItem = plcWriteRequest.getRequestItems().get(i);
-
-                        // A write response contains only the return code for every item.
-                        ResponseCode responseCode = decodeResponseCode(payloadItem.getReturnCode());
-
-                        WriteResponseItem responseItem = new WriteResponseItem(requestItem, responseCode);
-                        responseItems.add(responseItem);
-                    }
-
-                    if (plcWriteRequest instanceof TypeSafePlcWriteRequest) {
-                        response = new TypeSafePlcWriteResponse((TypeSafePlcWriteRequest) plcWriteRequest, responseItems);
-                    } else {
-                        response = new PlcWriteResponse(plcWriteRequest, responseItems);
-                    }
-                }
+            // Confirm the response being handled.
+            if (response != null) {
+                requestContainer.getResponseFuture().complete(response);
+            }
+        }
+    }
 
-                // Confirm the response being handled.
-                if (response != null) {
-                    requestContainer.getResponseFuture().complete(response);
-                }
+    @SuppressWarnings("unchecked")
+    private PlcResponse decodeWriteRequest(S7ResponseMessage responseMessage, PlcRequestContainer requestContainer) throws PlcProtocolException {
+        PlcResponse response;
+        PlcWriteRequest plcWriteRequest = (PlcWriteRequest) requestContainer.getRequest();
+        List<WriteResponseItem<?>> responseItems = new LinkedList<>();
+        VarPayload payload = responseMessage.getPayload(VarPayload.class)
+            .orElseThrow(() -> new PlcProtocolException("No VarPayload supplied"));
+        // If the numbers of items don't match, we're in big trouble as the only
+        // way to know how to interpret the responses is by aligning them with the
+        // items from the request as this information is not returned by the PLC.
+        if (plcWriteRequest.getRequestItems().size() != payload.getPayloadItems().size()) {
+            throw new PlcProtocolException(
+                "The number of requested items doesn't match the number of returned items");
+        }
+        List<VarPayloadItem> payloadItems = payload.getPayloadItems();
+        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
+            WriteRequestItem requestItem = plcWriteRequest.getRequestItems().get(i);
+
+            // A write response contains only the return code for every item.
+            ResponseCode responseCode = decodeResponseCode(payloadItem.getReturnCode());
+
+            WriteResponseItem responseItem = new WriteResponseItem(requestItem, responseCode);
+            responseItems.add(responseItem);
+        }
+
+        if (plcWriteRequest instanceof TypeSafePlcWriteRequest) {
+            response = new TypeSafePlcWriteResponse((TypeSafePlcWriteRequest) plcWriteRequest, responseItems);
+        } else {
+            response = new PlcWriteResponse(plcWriteRequest, responseItems);
+        }
+        return response;
+    }
+
+    @SuppressWarnings("unchecked")
+    private PlcResponse decodeReadRequest(S7ResponseMessage responseMessage, PlcRequestContainer requestContainer) throws PlcProtocolException {
+        PlcResponse response;
+        PlcReadRequest plcReadRequest = (PlcReadRequest) requestContainer.getRequest();
+
+        List<ReadResponseItem<?>> responseItems = new LinkedList<>();
+        VarPayload payload = responseMessage.getPayload(VarPayload.class)
+            .orElseThrow(() -> new PlcProtocolException("No VarPayload supplied"));
+        // If the numbers of items don't match, we're in big trouble as the only
+        // way to know how to interpret the responses is by aligning them with the
+        // items from the request as this information is not returned by the PLC.
+        if (plcReadRequest.getRequestItems().size() != payload.getPayloadItems().size()) {
+            throw new PlcProtocolException(
+                "The number of requested items doesn't match the number of returned items");
+        }
+        List<VarPayloadItem> payloadItems = payload.getPayloadItems();
+        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
+            ReadRequestItem requestItem = plcReadRequest.getRequestItems().get(i);
+
+            ResponseCode responseCode = decodeResponseCode(payloadItem.getReturnCode());
+
+            ReadResponseItem responseItem;
+            // Something went wrong.
+            if (responseCode != ResponseCode.OK) {
+                responseItem = new ReadResponseItem<>(requestItem, responseCode, null);
+            }
+            // All Ok.
+            else {
+                byte[] data = payloadItem.getData();
+                Class<?> datatype = requestItem.getDatatype();
+                List<?> value = decodeData(datatype, data);
+                responseItem = new ReadResponseItem(requestItem, responseCode, value);
             }
+            responseItems.add(responseItem);
         }
+        if (plcReadRequest instanceof TypeSafePlcReadRequest) {
+            response = new TypeSafePlcReadResponse((TypeSafePlcReadRequest) plcReadRequest, responseItems);
+        } else {
+            response = new PlcReadResponse(plcReadRequest, responseItems);
+        }
+        return response;
     }
 
     ////////////////////////////////////////////////////////////////////////////////
@@ -305,58 +331,93 @@ public class Plc4XS7Protocol extends MessageToMessageCodec<S7Message, PlcRequest
         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[length];
-            for (int i = 0; i < length; i++) {
-                result[i] = (byte) (((Boolean) values[i]) ? 0x01 : 0x00);
-            }
+            result = encodeBoolean(values, length);
         } else if (valueType == Byte.class) {
-            result = new byte[length];
-            for (int i = 0; i < length; i++) {
-                result[i] = (byte) values[i];
-            }
+            result = encodeByte(values, length);
         } else if (valueType == Short.class) {
-            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);
-            }
+            result = encodeShort(values, length);
         } else if (valueType == Integer.class) {
-            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);
-                result[(i * 4) + 2] = (byte) ((intValue & 0x0000ff00) >> 8);
-                result[(i * 4) + 3] = (byte) (intValue & 0xff);
-            }
+            result = encodeInteger(values, length);
         } else if (valueType == Calendar.class) {
             result = null;
         } else if (valueType == Float.class) {
-            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);
-                result[(i * 4) + 1] = (byte) ((intValue & 0x00ff0000) >> 16);
-                result[(i * 4) + 2] = (byte) ((intValue & 0x0000ff00) >> 8);
-                result[(i * 4) + 3] = (byte) (intValue & 0xff);
-            }
+            result = encodeFloat(values, length);
         } else if (valueType == String.class) {
-            int size = 0;
-            for (Object value : values) {
-                size = size + ((String) value).length();
-            }
-            result = new byte[size + length];
-            int j = 0;
-            for (Object value : values) {
-                String str = (String) value;
-                for (int i = 0; i < str.length(); i++) {
-                    result[j++] = (byte) str.charAt(i);
-                }
-                result[j++] = (byte) 0x0;
+            result = encodeString(values, length);
+        }
+        return result;
+    }
+
+    private byte[] encodeString(Object[] values, int length) {
+        byte[] result;
+        int size = 0;
+        for (Object value : values) {
+            size = size + ((String) value).length();
+        }
+        result = new byte[size + length];
+        int j = 0;
+        for (Object value : values) {
+            String str = (String) value;
+            for (int i = 0; i < str.length(); i++) {
+                result[j++] = (byte) str.charAt(i);
             }
+            result[j++] = (byte) 0x0;
+        }
+        return result;
+    }
+
+    private byte[] encodeFloat(Object[] values, int length) {
+        byte[] result;
+        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);
+            result[(i * 4) + 1] = (byte) ((intValue & 0x00ff0000) >> 16);
+            result[(i * 4) + 2] = (byte) ((intValue & 0x0000ff00) >> 8);
+            result[(i * 4) + 3] = (byte) (intValue & 0xff);
+        }
+        return result;
+    }
+
+    private byte[] encodeInteger(Object[] values, int length) {
+        byte[] result;
+        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);
+            result[(i * 4) + 2] = (byte) ((intValue & 0x0000ff00) >> 8);
+            result[(i * 4) + 3] = (byte) (intValue & 0xff);
+        }
+        return result;
+    }
+
+    private byte[] encodeShort(Object[] values, int length) {
+        byte[] result;
+        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);
+        }
+        return result;
+    }
+
+    private byte[] encodeByte(Object[] values, int length) {
+        byte[] result;
+        result = new byte[length];
+        for (int i = 0; i < length; i++) {
+            result[i] = (byte) values[i];
+        }
+        return result;
+    }
+
+    private byte[] encodeBoolean(Object[] values, int length) {
+        byte[] result;// TODO: Check if this is true and the result is not Math.ceil(values.lenght / 8)
+        result = new byte[length];
+        for (int i = 0; i < length; i++) {
+            result[i] = (byte) (((Boolean) values[i]) ? 0x01 : 0x00);
         }
         return result;
     }
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 ff7746c..d1cb2b6 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
@@ -78,27 +78,34 @@ public class S7Protocol extends MessageToMessageCodec<IsoTPMessage, S7Message> {
     }
 
     @Override
-    protected void encode(ChannelHandlerContext ctx, S7Message in, List<Object> out) throws Exception {
+    protected void encode(ChannelHandlerContext ctx, S7Message in, List<Object> out) {
         logger.debug("S7 Message sent");
 
         ByteBuf buf = Unpooled.buffer();
 
-        buf.writeByte(S7_PROTOCOL_MAGIC_NUMBER);
-        buf.writeByte(in.getMessageType().getCode());
-        // Reserved (is always constant 0x0000)
-        buf.writeShort((short) 0x0000);
-        // PDU Reference (Request Id, generated by the initiating node)
-        buf.writeShort(in.getTpduReference());
-        // S7 message parameters length
-        buf.writeShort(getParametersLength(in.getParameters()));
-        // Data field length
-        buf.writeShort(getPayloadsLength(in.getPayloads()));
-        if (in instanceof S7ResponseMessage) {
-            S7ResponseMessage s7ResponseMessage = (S7ResponseMessage) in;
-            buf.writeByte(s7ResponseMessage.getErrorClass());
-            buf.writeByte(s7ResponseMessage.getErrorCode());
+        encodeHeader(in, buf);
+        encodeParameters(in, buf);
+        encodePayloads(in, buf);
+
+        out.add(new DataTpdu(true, (byte) 1, Collections.emptyList(), buf));
+    }
+
+    private void encodePayloads(S7Message in, ByteBuf buf) {
+        for (S7Payload payload : in.getPayloads()) {
+            ParameterType parameterType = payload.getType();
+            if (parameterType == ParameterType.READ_VAR || parameterType == ParameterType.WRITE_VAR) {
+                VarPayload varPayload = (VarPayload) payload;
+                for (VarPayloadItem payloadItem : varPayload.getPayloadItems()) {
+                    buf.writeByte(payloadItem.getReturnCode().getCode());
+                    buf.writeByte(payloadItem.getDataTransportSize().getCode());
+                    buf.writeShort(payloadItem.getData().length);
+                    buf.writeBytes(payloadItem.getData());
+                }
+            }
         }
+    }
 
+    private void encodeParameters(S7Message in, ByteBuf buf) {
         for (S7Parameter s7Parameter : in.getParameters()) {
             buf.writeByte(s7Parameter.getType().getCode());
             switch (s7Parameter.getType()) {
@@ -111,26 +118,26 @@ public class S7Protocol extends MessageToMessageCodec<IsoTPMessage, S7Message> {
                     break;
                 default:
                     logger.error("writing this parameter type not implemented");
-                    return;
             }
         }
+    }
 
-        if (!in.getPayloads().isEmpty()) {
-            for (S7Payload payload : in.getPayloads()) {
-                ParameterType parameterType = payload.getType();
-                if (parameterType == ParameterType.READ_VAR || parameterType == ParameterType.WRITE_VAR) {
-                    VarPayload varPayload = (VarPayload) payload;
-                    for (VarPayloadItem payloadItem : varPayload.getPayloadItems()) {
-                        buf.writeByte(payloadItem.getReturnCode().getCode());
-                        buf.writeByte(payloadItem.getDataTransportSize().getCode());
-                        buf.writeShort(payloadItem.getData().length);
-                        buf.writeBytes(payloadItem.getData());
-                    }
-                }
-            }
+    private void encodeHeader(S7Message in, ByteBuf buf) {
+        buf.writeByte(S7_PROTOCOL_MAGIC_NUMBER);
+        buf.writeByte(in.getMessageType().getCode());
+        // Reserved (is always constant 0x0000)
+        buf.writeShort((short) 0x0000);
+        // PDU Reference (Request Id, generated by the initiating node)
+        buf.writeShort(in.getTpduReference());
+        // S7 message parameters length
+        buf.writeShort(getParametersLength(in.getParameters()));
+        // Data field length
+        buf.writeShort(getPayloadsLength(in.getPayloads()));
+        if (in instanceof S7ResponseMessage) {
+            S7ResponseMessage s7ResponseMessage = (S7ResponseMessage) in;
+            buf.writeByte(s7ResponseMessage.getErrorClass());
+            buf.writeByte(s7ResponseMessage.getErrorCode());
         }
-
-        out.add(new DataTpdu(true, (byte) 1, Collections.emptyList(), buf));
     }
 
     private void encodeSetupCommunication(ByteBuf buf, SetupCommunicationParameter s7Parameter) {
@@ -176,106 +183,123 @@ public class S7Protocol extends MessageToMessageCodec<IsoTPMessage, S7Message> {
     }
 
     @Override
-    protected void decode(ChannelHandlerContext ctx, IsoTPMessage in, List<Object> out) throws Exception {
+    protected void decode(ChannelHandlerContext ctx, IsoTPMessage in, List<Object> out) {
         if (logger.isTraceEnabled()) {
             logger.trace("Got Data: {}", ByteBufUtil.hexDump(in.getUserData()));
         }
         ByteBuf userData = in.getUserData();
-        if (userData.readableBytes() > 0) {
-            logger.debug("S7 Message received");
+        if (userData.readableBytes() == 0) {
+            return;
+        }
+        logger.debug("S7 Message received");
 
-            if (userData.readByte() != S7_PROTOCOL_MAGIC_NUMBER) {
-                logger.warn("Expecting S7 protocol magic number.");
-                logger.debug("Got Data: {}", ByteBufUtil.hexDump(userData));
-                return;
-            }
+        if (userData.readByte() != S7_PROTOCOL_MAGIC_NUMBER) {
+            logger.warn("Expecting S7 protocol magic number.");
+            logger.debug("Got Data: {}", ByteBufUtil.hexDump(userData));
+            return;
+        }
 
-            MessageType messageType = MessageType.valueOf(userData.readByte());
-            boolean isResponse = messageType == MessageType.ACK_DATA;
-            // Reserved (is always constant 0x0000)
-            userData.readShort();
-            short tpduReference = userData.readShort();
-            short headerParametersLength = userData.readShort();
-            short userDataLength = userData.readShort();
-            byte errorClass = 0;
-            byte errorCode = 0;
-            if (messageType == MessageType.ACK_DATA) {
-                errorClass = userData.readByte();
-                errorCode = userData.readByte();
+        MessageType messageType = MessageType.valueOf(userData.readByte());
+        boolean isResponse = messageType == MessageType.ACK_DATA;
+        userData.readShort();  // Reserved (is always constant 0x0000)
+        short tpduReference = userData.readShort();
+        short headerParametersLength = userData.readShort();
+        short userDataLength = userData.readShort();
+        byte errorClass = 0;
+        byte errorCode = 0;
+        if (isResponse) {
+            errorClass = userData.readByte();
+            errorCode = userData.readByte();
+        }
+
+        List<S7Parameter> s7Parameters = new LinkedList<>();
+        SetupCommunicationParameter setupCommunicationParameter = null;
+        VarParameter readWriteVarParameter = null;
+        int i = 0;
+
+        while (i < headerParametersLength) {
+            S7Parameter parameter = parseParameter(userData, isResponse, headerParametersLength - i);
+            s7Parameters.add(parameter);
+            if (parameter instanceof SetupCommunicationParameter) {
+                setupCommunicationParameter = (SetupCommunicationParameter) parameter;
             }
-            List<S7Parameter> s7Parameters = new LinkedList<>();
-            SetupCommunicationParameter setupCommunicationParameter = null;
-            VarParameter readWriteVarParameter = null;
-            int i = 0;
-
-            while (i < headerParametersLength) {
-                S7Parameter parameter = parseParameter(userData, isResponse, headerParametersLength - i);
-                s7Parameters.add(parameter);
-                if (parameter instanceof SetupCommunicationParameter) {
-                    setupCommunicationParameter = (SetupCommunicationParameter) parameter;
-                }
-                if (parameter instanceof VarParameter) {
-                    ParameterType paramType = parameter.getType();
-                    if (paramType == ParameterType.READ_VAR || paramType == ParameterType.WRITE_VAR) {
-                        readWriteVarParameter = (VarParameter) parameter;
-                    }
-                }
-                i += getParameterLength(parameter);
+            if (readWriteVarParameter == null)  {
+                readWriteVarParameter = decodeReadWriteParameter(parameter);
             }
-            List<S7Payload> s7Payloads = new LinkedList<>();
-            if(readWriteVarParameter != null) {
-                List<VarPayloadItem> payloadItems = new LinkedList<>();
-                i = 0;
-
-                while (i < userDataLength) {
-                    DataTransportErrorCode dataTransportErrorCode = DataTransportErrorCode.valueOf(userData.readByte());
-                    // This is a response to a WRITE_VAR request (It only contains the return code for every sent item.
-                    if ((readWriteVarParameter.getType() == ParameterType.WRITE_VAR) &&
-                        (messageType == MessageType.ACK_DATA)) {
-                        // Initialize a rudimentary payload (This is updated in the Plc4XS7Protocol class
-                        VarPayloadItem payload = new VarPayloadItem(dataTransportErrorCode, null, null);
-                        payloadItems.add(payload);
-                        i += 1;
-                    }
-                    // This is a response to a READ_VAR request.
-                    else if ((readWriteVarParameter.getType() == ParameterType.READ_VAR) &&
-                        (messageType == MessageType.ACK_DATA)) {
-                        DataTransportSize dataTransportSize = DataTransportSize.valueOf(userData.readByte());
-                        short length = (dataTransportSize.isSizeInBits()) ?
-                            (short) Math.ceil(userData.readShort() / 8.0) : userData.readShort();
-                        byte[] data = new byte[length];
-                        userData.readBytes(data);
-                        // Initialize a rudimentary payload (This is updated in the Plc4XS7Protocol class
-                        VarPayloadItem payload = new VarPayloadItem(dataTransportErrorCode, dataTransportSize, data);
-                        payloadItems.add(payload);
-                        i += getPayloadLength(payload);
-                    }
-                }
+            i += getParameterLength(parameter);
+        }
+
+        List<S7Payload> s7Payloads = decodePayloads(userData, isResponse, userDataLength, readWriteVarParameter);
 
-                VarPayload varPayload = new VarPayload(readWriteVarParameter.getType(), payloadItems);
-                s7Payloads.add(varPayload);
+        if (isResponse) {
+            setupCommunications(ctx, setupCommunicationParameter);
+            out.add(new S7ResponseMessage(messageType, tpduReference, s7Parameters, s7Payloads, errorClass, errorCode));
+        } else {
+            out.add(new S7RequestMessage(messageType, tpduReference, s7Parameters, s7Payloads));
+        }
+    }
+
+    private void setupCommunications(ChannelHandlerContext ctx, SetupCommunicationParameter setupCommunicationParameter) {
+        // If we got a SetupCommunicationParameter as part of the response
+        // we are currently in the process of establishing a connection with
+        // the PLC, so save some of the information in the session and tell
+        // the next layer to negotiate the connection parameters.
+        if (setupCommunicationParameter != null) {
+            maxAmqCaller = setupCommunicationParameter.getMaxAmqCaller();
+            maxAmqCallee = setupCommunicationParameter.getMaxAmqCallee();
+            pduSize = setupCommunicationParameter.getPduLength();
+
+            // Send an event that setup is complete.
+            ctx.channel().pipeline().fireUserEventTriggered(
+                new S7ConnectionEvent(S7ConnectionState.SETUP_COMPLETE));
+        }
+    }
+
+    private VarParameter decodeReadWriteParameter(S7Parameter parameter) {
+        VarParameter readWriteVarParameter = null;
+
+        if (parameter instanceof VarParameter) {
+            ParameterType paramType = parameter.getType();
+            if (paramType == ParameterType.READ_VAR || paramType == ParameterType.WRITE_VAR) {
+                readWriteVarParameter = (VarParameter) parameter;
             }
+        }
+        return readWriteVarParameter;
+    }
 
-            if (messageType == MessageType.ACK_DATA) {
-                // If we got a SetupCommunicationParameter as part of the response
-                // we are currently in the process of establishing a connection with
-                // the PLC, so save some of the information in the session and tell
-                // the next layer to negotiate the connection parameters.
-                if (setupCommunicationParameter != null) {
-                    maxAmqCaller = setupCommunicationParameter.getMaxAmqCaller();
-                    maxAmqCallee = setupCommunicationParameter.getMaxAmqCallee();
-                    pduSize = setupCommunicationParameter.getPduLength();
-
-                    // Send an event that setup is complete.
-                    ctx.channel().pipeline().fireUserEventTriggered(
-                        new S7ConnectionEvent(S7ConnectionState.SETUP_COMPLETE));
+    private List<S7Payload> decodePayloads(ByteBuf userData, boolean isResponse, short userDataLength, VarParameter readWriteVarParameter) {
+        int i = 0;
+        List<S7Payload> s7Payloads = new LinkedList<>();
+        if (readWriteVarParameter != null) {
+            List<VarPayloadItem> payloadItems = new LinkedList<>();
+
+            while (i < userDataLength) {
+                DataTransportErrorCode dataTransportErrorCode = DataTransportErrorCode.valueOf(userData.readByte());
+                // This is a response to a WRITE_VAR request (It only contains the return code for every sent item.
+                if ((readWriteVarParameter.getType() == ParameterType.WRITE_VAR) && isResponse) {
+                    // Initialize a rudimentary payload (This is updated in the Plc4XS7Protocol class
+                    VarPayloadItem payload = new VarPayloadItem(dataTransportErrorCode, null, null);
+                    payloadItems.add(payload);
+                    i += 1;
+                }
+                // This is a response to a READ_VAR request.
+                else if ((readWriteVarParameter.getType() == ParameterType.READ_VAR) && isResponse) {
+                    DataTransportSize dataTransportSize = DataTransportSize.valueOf(userData.readByte());
+                    short length = (dataTransportSize.isSizeInBits()) ?
+                        (short) Math.ceil(userData.readShort() / 8.0) : userData.readShort();
+                    byte[] data = new byte[length];
+                    userData.readBytes(data);
+                    // Initialize a rudimentary payload (This is updated in the Plc4XS7Protocol class
+                    VarPayloadItem payload = new VarPayloadItem(dataTransportErrorCode, dataTransportSize, data);
+                    payloadItems.add(payload);
+                    i += getPayloadLength(payload);
                 }
-                out.add(new S7ResponseMessage(
-                    messageType, tpduReference, s7Parameters, s7Payloads, errorClass, errorCode));
-            } else {
-                out.add(new S7RequestMessage(messageType, tpduReference, s7Parameters, s7Payloads));
             }
+
+            VarPayload varPayload = new VarPayload(readWriteVarParameter.getType(), payloadItems);
+            s7Payloads.add(varPayload);
         }
+        return s7Payloads;
     }
 
     private S7Parameter parseParameter(ByteBuf in, boolean isResponse, int restLength) {
@@ -294,40 +318,11 @@ public class S7Protocol extends MessageToMessageCodec<IsoTPMessage, S7Message> {
                 return null;
             case READ_VAR:
             case WRITE_VAR:
-                List<VarParameterItem> items = new LinkedList<>();
-                byte numItems = in.readByte();
-                if (!isResponse) {
-                    for (int i = 0; i < numItems; i++) {
-                        SpecificationType specificationType = SpecificationType.valueOf(in.readByte());
-                        // Length of the rest of this item.
-                        byte itemLength = in.readByte();
-                        if (itemLength != 0x0a) {
-                            logger.warn("Expecting a length of 10 here.");
-                            return null;
-                        }
-                        VariableAddressingMode variableAddressingMode = VariableAddressingMode.valueOf(in.readByte());
-                        if (variableAddressingMode == VariableAddressingMode.S7ANY) {
-                            TransportSize transportSize = TransportSize.valueOf(in.readByte());
-                            short length = in.readShort();
-                            short dbNumber = in.readShort();
-                            MemoryArea memoryArea = MemoryArea.valueOf(in.readByte());
-                            short byteAddress = (short) (in.readShort() << 5);
-                            byte tmp = in.readByte();
-                            // Only the least 3 bits are the bit address, the
-                            byte bitAddress = (byte) (tmp & 0x07);
-                            // Bits 4-8 belong to the byte address
-                            byteAddress = (short) (byteAddress | (tmp >> 3));
-                            S7AnyVarParameterItem item = new S7AnyVarParameterItem(
-                                    specificationType, memoryArea, transportSize,
-                                    length, dbNumber, byteAddress, bitAddress);
-                            items.add(item);
-                        } else {
-                            logger.error("Error parsing item type");
-                            return null;
-                        }
-                    }
+                List<VarParameterItem> varParamameter = null;
+                if (isResponse) {
+                    varParamameter = parseReadWriteVarParameter(in);
                 }
-                return new VarParameter(parameterType, items);
+                return new VarParameter(parameterType, varParamameter);
             case SETUP_COMMUNICATION:
                 // Reserved (is always constant 0x00)
                 in.readByte();
@@ -341,6 +336,43 @@ public class S7Protocol extends MessageToMessageCodec<IsoTPMessage, S7Message> {
         return null;
     }
 
+    private List<VarParameterItem> parseReadWriteVarParameter(ByteBuf in) {
+        List<VarParameterItem> items = new LinkedList<>();
+        byte numItems = in.readByte();
+
+        for (int i = 0; i < numItems; i++) {
+            SpecificationType specificationType = SpecificationType.valueOf(in.readByte());
+            // Length of the rest of this item.
+            byte itemLength = in.readByte();
+            if (itemLength != 0x0a) {
+                logger.warn("Expecting a length of 10 here.");
+                return items;
+            }
+            VariableAddressingMode variableAddressingMode = VariableAddressingMode.valueOf(in.readByte());
+            if (variableAddressingMode == VariableAddressingMode.S7ANY) {
+                TransportSize transportSize = TransportSize.valueOf(in.readByte());
+                short length = in.readShort();
+                short dbNumber = in.readShort();
+                MemoryArea memoryArea = MemoryArea.valueOf(in.readByte());
+                short byteAddress = (short) (in.readShort() << 5);
+                byte tmp = in.readByte();
+                // Only the least 3 bits are the bit address, the
+                byte bitAddress = (byte) (tmp & 0x07);
+                // Bits 4-8 belong to the byte address
+                byteAddress = (short) (byteAddress | (tmp >> 3));
+                S7AnyVarParameterItem item = new S7AnyVarParameterItem(
+                        specificationType, memoryArea, transportSize,
+                        length, dbNumber, byteAddress, bitAddress);
+                items.add(item);
+            } else {
+                logger.error("Error parsing item type");
+                return items;
+            }
+        }
+
+        return items;
+    }
+
     private short getParametersLength(List<S7Parameter> parameters) {
         short l = 0;
         if (parameters != null) {
@@ -353,13 +385,15 @@ public class S7Protocol extends MessageToMessageCodec<IsoTPMessage, S7Message> {
 
     private short getPayloadsLength(List<S7Payload> payloads) {
         short l = 0;
-        if (payloads != null) {
-            for (S7Payload payload : payloads) {
-                if(payload instanceof VarPayload) {
-                    VarPayload varPayload = (VarPayload) payload;
-                    for (VarPayloadItem payloadItem : varPayload.getPayloadItems()) {
-                        l += getPayloadLength(payloadItem);
-                    }
+        if (payloads == null) {
+            return 0;
+        }
+
+        for (S7Payload payload : payloads) {
+            if(payload instanceof VarPayload) {
+                VarPayload varPayload = (VarPayload) payload;
+                for (VarPayloadItem payloadItem : varPayload.getPayloadItems()) {
+                    l += getPayloadLength(payloadItem);
                 }
             }
         }
@@ -398,10 +432,10 @@ public class S7Protocol extends MessageToMessageCodec<IsoTPMessage, S7Message> {
     }
 
     private short getPayloadLength(VarPayloadItem payloadItem) {
-        if (payloadItem != null) {
-            return (short) (4 + payloadItem.getData().length);
+        if (payloadItem == null) {
+            return 0;
         }
-        return 0;
+        return (short) (4 + payloadItem.getData().length);
     }
 
 }

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