You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@plc4x.apache.org by GitBox <gi...@apache.org> on 2018/10/02 07:50:52 UTC

[GitHub] JulianFeinauer closed pull request #25: PLC4X-57 Bugfix

JulianFeinauer closed pull request #25: PLC4X-57 Bugfix
URL: https://github.com/apache/incubator-plc4x/pull/25
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/examples/hello-plc4x/src/main/java/org/apache/plc4x/java/examples/helloplc4x/HelloPlc4x.java b/examples/hello-plc4x/src/main/java/org/apache/plc4x/java/examples/helloplc4x/HelloPlc4x.java
index bbf7c1be9..e0784349f 100644
--- a/examples/hello-plc4x/src/main/java/org/apache/plc4x/java/examples/helloplc4x/HelloPlc4x.java
+++ b/examples/hello-plc4x/src/main/java/org/apache/plc4x/java/examples/helloplc4x/HelloPlc4x.java
@@ -70,26 +70,7 @@ public static void main(String[] args) {
                 System.out.println("\nSynchronous request ...");
                 PlcReadResponse<?> syncResponse = plcReader.read(plcReadRequest).get();
                 // Simply iterating over the field names returned in the response.
-                for (String fieldName : syncResponse.getFieldNames()) {
-                    if(syncResponse.getResponseCode(fieldName) == PlcResponseCode.OK) {
-                        int numValues = syncResponse.getNumberOfValues(fieldName);
-                        // If it's just one element, output just one single line.
-                        if(numValues == 1) {
-                            System.out.println("Value[" + fieldName + "]: " + syncResponse.getObject(fieldName));
-                        }
-                        // If it's more than one element, output each in a single row.
-                        else {
-                            System.out.println("Value[" + fieldName + "]:");
-                            for(int i = 0; i < numValues; i++) {
-                                System.out.println(" - " + syncResponse.getObject(fieldName, i));
-                            }
-                        }
-                    }
-                    // Something went wrong, to output an error message instead.
-                    else {
-                        System.out.println("Error[" + fieldName + "]: " + syncResponse.getResponseCode(fieldName).name());
-                    }
-                }
+                printResponse(syncResponse);
 
                 //////////////////////////////////////////////////////////
                 // Read asynchronously ...
@@ -98,13 +79,7 @@ public static void main(String[] args) {
                 CompletableFuture<PlcReadResponse<?>> asyncResponse = plcReader.read(plcReadRequest);
                 asyncResponse.whenComplete((readResponse, throwable) -> {
                     if (readResponse != null) {
-                        for (String fieldName : syncResponse.getFieldNames()) {
-                            if (syncResponse.getResponseCode(fieldName) == PlcResponseCode.OK) {
-                                System.out.println("Value[" + fieldName + "]: " + syncResponse.getObject(fieldName));
-                            } else {
-                                System.out.println("Error[" + fieldName + "]: " + syncResponse.getResponseCode(fieldName).name());
-                            }
-                        }
+                        printResponse(syncResponse);
                     } else {
                         logger.error("An error occurred", throwable);
                     }
@@ -119,4 +94,27 @@ public static void main(String[] args) {
         }
     }
 
+    private static void printResponse(PlcReadResponse<?> syncResponse) {
+        for (String fieldName : syncResponse.getFieldNames()) {
+            if(syncResponse.getResponseCode(fieldName) == PlcResponseCode.OK) {
+                int numValues = syncResponse.getNumberOfValues(fieldName);
+                // If it's just one element, output just one single line.
+                if(numValues == 1) {
+                    System.out.println("Value[" + fieldName + "]: " + syncResponse.getObject(fieldName));
+                }
+                // If it's more than one element, output each in a single row.
+                else {
+                    System.out.println("Value[" + fieldName + "]:");
+                    for(int i = 0; i < numValues; i++) {
+                        System.out.println(" - " + syncResponse.getObject(fieldName, i));
+                    }
+                }
+            }
+            // Something went wrong, to output an error message instead.
+            else {
+                System.out.println("Error[" + fieldName + "]: " + syncResponse.getResponseCode(fieldName).name());
+            }
+        }
+    }
+
 }
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 83b7f3b53..e2963d371 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
@@ -21,12 +21,10 @@ Licensed to the Apache Software Foundation (ASF) under one
 import io.netty.buffer.ByteBuf;
 import io.netty.buffer.Unpooled;
 import io.netty.channel.ChannelHandlerContext;
+import org.apache.commons.lang3.ArrayUtils;
 import org.apache.commons.lang3.tuple.ImmutablePair;
 import org.apache.commons.lang3.tuple.Pair;
-import org.apache.plc4x.java.api.exceptions.PlcException;
-import org.apache.plc4x.java.api.exceptions.PlcIoException;
-import org.apache.plc4x.java.api.exceptions.PlcProtocolException;
-import org.apache.plc4x.java.api.exceptions.PlcProtocolPayloadTooBigException;
+import org.apache.plc4x.java.api.exceptions.*;
 import org.apache.plc4x.java.api.messages.PlcReadRequest;
 import org.apache.plc4x.java.api.messages.PlcRequest;
 import org.apache.plc4x.java.api.messages.PlcResponse;
@@ -51,11 +49,15 @@ Licensed to the Apache Software Foundation (ASF) under one
 import org.apache.plc4x.java.s7.netty.model.types.*;
 
 import java.io.IOException;
+import java.lang.reflect.Array;
 import java.math.BigInteger;
 import java.nio.ByteBuffer;
 import java.nio.charset.Charset;
 import java.util.*;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
 
 /**
  * This layer transforms between {@link PlcRequestContainer}s {@link S7Message}s.
@@ -420,22 +422,25 @@ private PlcResponse decodeReadResponse(S7ResponseMessage responseMessage, PlcReq
             FieldItem fieldItem = null;
             ByteBuf data = Unpooled.wrappedBuffer(payloadItem.getData());
             if (responseCode == PlcResponseCode.OK) {
+                // TODO 2018-09-27 jf: array returning only implemented for BOOL, BYTE, INTEGERS, FP
+                // not for CHARS & STRINGS and not for all other bit-strings except for BYTE
                 switch (field.getDataType()) {
                     // -----------------------------------------
                     // Bit
                     // -----------------------------------------
                     case BOOL: {
-                        byte byteValue = data.readByte();
-                        fieldItem = new S7BooleanFieldItem(field.getDataType(),byteValue != 0x00);
+                        Boolean[] booleans = readAllValues(Boolean.class, field, i -> data.readByte() != 0x00);
+                        fieldItem = new S7BooleanFieldItem(field.getDataType(),booleans);
                         break;
                     }
                     // -----------------------------------------
                     // Bit-strings
                     // -----------------------------------------
                     case BYTE: { // 1 byte
-                        BitSet bitSet = BitSet.valueOf(new byte[]{data.readByte()});
-                        Boolean[] booleanValues = new Boolean[8];
-                        for(int i = 0; i < 8; i++) {
+                        byte[] bytes = ArrayUtils.toPrimitive(readAllValues(Byte.class, field, i -> data.readByte()));
+                        BitSet bitSet = BitSet.valueOf(bytes);
+                        Boolean[] booleanValues = new Boolean[8 * bytes.length];
+                        for(int i = 0; i < 8 * bytes.length; i++) {
                             booleanValues[i] = bitSet.get(i);
                         }
                         fieldItem = new S7BooleanFieldItem(field.getDataType(),booleanValues);
@@ -474,59 +479,59 @@ private PlcResponse decodeReadResponse(S7ResponseMessage responseMessage, PlcReq
                     // -----------------------------------------
                     // 8 bit:
                     case SINT: {
-                        Long longValue = (long) data.readByte();
-                        fieldItem = new S7LongFieldItem(field.getDataType(), longValue);
+                        Long[] longs = readAllValues(Long.class, field, i -> (long)data.readByte());
+                        fieldItem = new S7LongFieldItem(field.getDataType(), longs);
                         break;
                     }
                     case USINT: {
-                        Long longValue = (long) data.readUnsignedByte();
-                        fieldItem = new S7LongFieldItem(field.getDataType(), longValue);
+                        Long[] longs = readAllValues(Long.class, field, i -> (long)data.readUnsignedByte());
+                        fieldItem = new S7LongFieldItem(field.getDataType(), longs);
                         break;
                     }
                     // 16 bit:
                     case INT: {
-                        Long longValue = (long) data.readShort();
-                        fieldItem = new S7LongFieldItem(field.getDataType(), longValue);
+                        Long[] longs = readAllValues(Long.class, field, i -> (long)data.readShort());
+                        fieldItem = new S7LongFieldItem(field.getDataType(), longs);
                         break;
                     }
                     case UINT: {
-                        Long longValue = (long) data.readUnsignedShort();
-                        fieldItem = new S7LongFieldItem(field.getDataType(), longValue);
+                        Long[] longs = readAllValues(Long.class, field, i -> (long)data.readUnsignedShort());
+                        fieldItem = new S7LongFieldItem(field.getDataType(), longs);
                         break;
                     }
                     // 32 bit:
                     case DINT: {
-                        Long longValue = (long) data.readInt();
-                        fieldItem = new S7LongFieldItem(field.getDataType(), longValue);
+                        Long[] longs = readAllValues(Long.class, field, i -> (long)data.readInt());
+                        fieldItem = new S7LongFieldItem(field.getDataType(), longs);
                         break;
                     }
                     case UDINT: {
-                        Long longValue = data.readUnsignedInt();
-                        fieldItem = new S7LongFieldItem(field.getDataType(), longValue);
+                        Long[] longs = readAllValues(Long.class, field, i -> data.readUnsignedInt());
+                        fieldItem = new S7LongFieldItem(field.getDataType(), longs);
                         break;
                     }
                     // 64 bit:
                     case LINT: {
-                        BigInteger bigIntegerValue = readSigned64BitInteger(data);
-                        fieldItem = new S7BigIntegerFieldItem(field.getDataType(), bigIntegerValue);
+                        BigInteger[] bigIntegers = readAllValues(BigInteger.class, field, i -> readSigned64BitInteger(data));
+                        fieldItem = new S7BigIntegerFieldItem(field.getDataType(), bigIntegers);
                         break;
                     }
                     case ULINT: {
-                        BigInteger bigIntegerValue = readUnsigned64BitInteger(data);
-                        fieldItem = new S7BigIntegerFieldItem(field.getDataType(), bigIntegerValue);
+                        BigInteger[] bigIntegers = readAllValues(BigInteger.class, field, i -> readUnsigned64BitInteger(data));
+                        fieldItem = new S7BigIntegerFieldItem(field.getDataType(), bigIntegers);
                         break;
                     }
                     // -----------------------------------------
                     // Floating point values
                     // -----------------------------------------
                     case REAL: {
-                        double doubleValue = data.readFloat();
-                        fieldItem = new S7FloatingPointFieldItem(field.getDataType(), doubleValue);
+                        Double[] doubles = readAllValues(Double.class, field, i -> (double)data.readFloat());
+                        fieldItem = new S7FloatingPointFieldItem(field.getDataType(), doubles);
                         break;
                     }
                     case LREAL: {
-                        double doubleValue = data.readDouble();
-                        fieldItem = new S7FloatingPointFieldItem(field.getDataType(), doubleValue);
+                        Double[] doubles = readAllValues(Double.class, field, i -> data.readDouble());
+                        fieldItem = new S7FloatingPointFieldItem(field.getDataType(), doubles);
                         break;
                     }
                     // -----------------------------------------
@@ -575,6 +580,17 @@ private PlcResponse decodeReadResponse(S7ResponseMessage responseMessage, PlcReq
         return new DefaultPlcReadResponse(plcReadRequest, values);
     }
 
+    private static <T> T[] readAllValues(Class<T> clazz, S7Field field, Function<Integer, T> extract) {
+        try {
+            return IntStream.rangeClosed(1, field.getNumElements())
+                .mapToObj(extract::apply)
+                .collect(Collectors.toList())
+                .toArray((T[])Array.newInstance(clazz, 0));
+        } catch (IndexOutOfBoundsException e) {
+            throw new PlcRuntimeException("To few bytes in the buffer to read requested type", e);
+        }
+    }
+
     @SuppressWarnings("unchecked")
     private PlcResponse decodeWriteResponse(S7ResponseMessage responseMessage, PlcRequestContainer requestContainer) throws PlcProtocolException {
         InternalPlcWriteRequest plcWriteRequest = (InternalPlcWriteRequest) requestContainer.getRequest();
diff --git a/plc4j/protocols/s7/src/test/java/org/apache/plc4x/java/s7/model/S7FieldTests.java b/plc4j/protocols/s7/src/test/java/org/apache/plc4x/java/s7/model/S7FieldTests.java
index d6c11d9ca..0f3545ee8 100644
--- a/plc4j/protocols/s7/src/test/java/org/apache/plc4x/java/s7/model/S7FieldTests.java
+++ b/plc4j/protocols/s7/src/test/java/org/apache/plc4x/java/s7/model/S7FieldTests.java
@@ -24,6 +24,7 @@ Licensed to the Apache Software Foundation (ASF) under one
 import org.apache.plc4x.java.s7.netty.model.types.TransportSize;
 import org.apache.plc4x.test.FastTests;
 import org.junit.experimental.categories.Category;
+import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.MethodSource;
@@ -32,6 +33,7 @@ Licensed to the Apache Software Foundation (ASF) under one
 
 import static org.hamcrest.core.IsEqual.equalTo;
 import static org.hamcrest.core.IsNull.notNullValue;
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.fail;
 
@@ -85,5 +87,10 @@ void testInvalidFieldQueryParsing(String fieldQuery) {
         }
     }
 
+    @Test
+    void checkGreedyNumFieldsParsing() {
+        S7Field field = S7Field.of("%DB56.DBB100:SINT[25]");
 
+        assertEquals(25, field.getNumElements());
+    }
 }
\ No newline at end of file


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services