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 2020/11/18 16:00:28 UTC

[plc4x] branch feature/plc4go updated: - Fixed the default size calculation in ADS - Updated the testdata with a true full run

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

cdutz pushed a commit to branch feature/plc4go
in repository https://gitbox.apache.org/repos/asf/plc4x.git


The following commit(s) were added to refs/heads/feature/plc4go by this push:
     new 6a59b60  - Fixed the default size calculation in ADS - Updated the testdata with a true full run
6a59b60 is described below

commit 6a59b606cab20dc055493403dab96dffaaa83cbf
Author: Christofer Dutz <ch...@c-ware.de>
AuthorDate: Wed Nov 18 17:00:21 2020 +0100

    - Fixed the default size calculation in ADS
    - Updated the testdata with a true full run
---
 .../plc4x/java/ads/protocol/AdsProtocolLogic.java  |  19 +++++++++-----
 .../apache/plc4x/java/ads/utils/StaticHelper.java  |  10 ++++++++
 .../plc4x/protocol/ads/ManualAdsDriverTest.java    |   8 +++---
 .../org/apache/plc4x/test/manual/ManualTest.java   |  28 +++++++++++++--------
 .../protocols/ads/manual-test-capture.pcapng       | Bin 46792 -> 166372 bytes
 5 files changed, 44 insertions(+), 21 deletions(-)

diff --git a/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/protocol/AdsProtocolLogic.java b/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/protocol/AdsProtocolLogic.java
index 93990bf..d53f84d 100644
--- a/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/protocol/AdsProtocolLogic.java
+++ b/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/protocol/AdsProtocolLogic.java
@@ -234,15 +234,16 @@ public class AdsProtocolLogic extends Plc4xProtocolBase<AmsTCPPacket> implements
                 if (field.getAdsDataType() == AdsDataType.STRING) {
                     // If an explicit size is given with the string, use this, if not use 256
                     size = (field instanceof  AdsStringField) ?
-                        ((AdsStringField) field).getStringLength() + 1 : 81;
+                        ((AdsStringField) field).getStringLength() + 1 : 256;
                 } else if (field.getAdsDataType() == AdsDataType.WSTRING) {
                     // If an explicit size is given with the string, use this, if not use 512
                     size = (field instanceof  AdsStringField) ?
-                        ((long) ((AdsStringField) field).getStringLength() + 1) * 2: 162;
+                        ((long) ((AdsStringField) field).getStringLength() + 1) * 2: 512;
                 } else {
                     size = field.getAdsDataType().getNumBytes();
                 }
-                return (size * field.getNumberOfElements()) + 4;
+                // Status code + payload size
+                return 4 + (size * field.getNumberOfElements());
             }).sum();
 
         // With multi-requests, the index-group is fixed and the index offset indicates the number of elements.
@@ -272,8 +273,14 @@ public class AdsProtocolLogic extends Plc4xProtocolBase<AmsTCPPacket> implements
                     // Convert the response from the PLC into a PLC4X Response ...
                     future.complete(plcReadResponse);
                 } else {
-                    // TODO: Implement this correctly.
-                    future.completeExceptionally(new PlcException("Error"));
+                    switch (responseAdsData.getResult()) {
+                        case ADSERR_DEVICE_INVALIDSIZE:
+                            future.completeExceptionally(
+                                new PlcException("The parameter size was not correct (Internal error)"));
+                            break;
+                        default:
+                            future.completeExceptionally(new PlcException("Error"));
+                    }
                 }
                 // Finish the request-transaction.
                 transaction.endRequest();
@@ -336,7 +343,7 @@ public class AdsProtocolLogic extends Plc4xProtocolBase<AmsTCPPacket> implements
             if (field.getAdsDataType() == AdsDataType.STRING) {
                 strLen = (field instanceof AdsStringField) ? ((AdsStringField) field).getStringLength() : 256;
             } else if (field.getAdsDataType() == AdsDataType.WSTRING) {
-                strLen = (field instanceof AdsStringField) ? ((AdsStringField) field).getStringLength() : 512;
+                strLen = (field instanceof AdsStringField) ? ((AdsStringField) field).getStringLength() : 256;
             }
             final int stringLength = strLen;
             if (field.getNumberOfElements() == 1) {
diff --git a/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/utils/StaticHelper.java b/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/utils/StaticHelper.java
index 5622bac..61fd8e5 100644
--- a/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/utils/StaticHelper.java
+++ b/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/utils/StaticHelper.java
@@ -39,6 +39,11 @@ public class StaticHelper {
                     if (curByte != 0) {
                         bytes.add(curByte);
                     } else {
+                        // Gobble up the remaining data, which is not added to the string.
+                        i++;
+                        for(; (i < stringLength) && io.hasMore(8); i++) {
+                            io.readByte(8);
+                        }
                         break;
                     }
                 }
@@ -55,6 +60,11 @@ public class StaticHelper {
                         bytes.add((byte) (curShort >>> 8));
                         bytes.add((byte) (curShort & 0xFF));
                     } else {
+                        // Gobble up the remaining data, which is not added to the string.
+                        i++;
+                        for(; (i < stringLength) && io.hasMore(16); i++) {
+                            io.readShort(16);
+                        }
                         break;
                     }
                 }
diff --git a/plc4j/drivers/ads/src/test/java/org/apache/plc4x/protocol/ads/ManualAdsDriverTest.java b/plc4j/drivers/ads/src/test/java/org/apache/plc4x/protocol/ads/ManualAdsDriverTest.java
index 50cb24b..2df45f2 100644
--- a/plc4j/drivers/ads/src/test/java/org/apache/plc4x/protocol/ads/ManualAdsDriverTest.java
+++ b/plc4j/drivers/ads/src/test/java/org/apache/plc4x/protocol/ads/ManualAdsDriverTest.java
@@ -69,7 +69,7 @@ public class ManualAdsDriverTest extends ManualTest {
 
     public static void main(String[] args) throws Exception {
         ManualAdsDriverTest test = new ManualAdsDriverTest("ads:tcp://192.168.23.20?sourceAmsNetId=192.168.23.200.1.1&sourceAmsPort=65534&targetAmsNetId=192.168.23.20.1.1&targetAmsPort=851");
-        /*test.addTestCase("main.hurz_BOOL:BOOL", true);
+        test.addTestCase("main.hurz_BOOL:BOOL", true);
         test.addTestCase("main.hurz_BYTE:BYTE", Arrays.asList(false, false, true, false, true, false, true, false));
         test.addTestCase("main.hurz_WORD:WORD", Arrays.asList(true, false, true, false, false, true, false, true, true, false, true, true, true, false, false, false));
         test.addTestCase("main.hurz_DWORD:DWORD", Arrays.asList(true, true, true, true, true, true, false, false, true, true, false, true, true, true, true, false, true, false, false, false, true, false, false, false, true, false, true, true, true, false, false, false));
@@ -83,15 +83,15 @@ public class ManualAdsDriverTest extends ManualTest {
         test.addTestCase("main.hurz_ULINT:ULINT", 4242442424242424242L);
         test.addTestCase("main.hurz_REAL:REAL", 3.14159265359F);
         test.addTestCase("main.hurz_LREAL:LREAL", 2.71828182846D);
-        test.addTestCase("main.hurz_STRING:STRING", "hurz");*/
+        test.addTestCase("main.hurz_STRING:STRING", "hurz");
         test.addTestCase("main.hurz_WSTRING:WSTRING", "wolf");
-        /*test.addTestCase("main.hurz_TIME:TIME", "PT1.234S");
+        test.addTestCase("main.hurz_TIME:TIME", "PT1.234S");
         test.addTestCase("main.hurz_LTIME:LTIME", "PT24015H23M12.034002044S");
         test.addTestCase("main.hurz_DATE:DATE", "1978-03-28");
         test.addTestCase("main.hurz_TIME_OF_DAY:TIME_OF_DAY", "15:36:30.123");
         test.addTestCase("main.hurz_TOD:TOD", "16:17:18.123");
         test.addTestCase("main.hurz_DATE_AND_TIME:DATE_AND_TIME", "1996-05-06T15:36:30");
-        test.addTestCase("main.hurz_DT:DT", "1972-03-29T00:00");*/
+        test.addTestCase("main.hurz_DT:DT", "1972-03-29T00:00");
         test.run();
     }
 
diff --git a/plc4j/utils/test-utils/src/main/java/org/apache/plc4x/test/manual/ManualTest.java b/plc4j/utils/test-utils/src/main/java/org/apache/plc4x/test/manual/ManualTest.java
index 0c549d4..43081e3 100644
--- a/plc4j/utils/test-utils/src/main/java/org/apache/plc4x/test/manual/ManualTest.java
+++ b/plc4j/utils/test-utils/src/main/java/org/apache/plc4x/test/manual/ManualTest.java
@@ -40,8 +40,8 @@ public abstract class ManualTest {
         testCases = new ArrayList<>();
     }
 
-    public void addTestCase(String address, Object expectedValue) {
-        testCases.add(new TestCase(address, expectedValue));
+    public void addTestCase(String address, Object expectedReadValue) {
+        testCases.add(new TestCase(address, expectedReadValue, null));
     }
 
     public void run() throws Exception {
@@ -63,13 +63,13 @@ public abstract class ManualTest {
                 Assertions.assertNotNull(readResponse.getPlcValue(fieldName));
                 if(readResponse.getPlcValue(fieldName) instanceof PlcList) {
                     PlcList plcList = (PlcList) readResponse.getPlcValue(fieldName);
-                    List<Object> expectedValues = (List<Object>) testCase.expectedValue;
+                    List<Object> expectedValues = (List<Object>) testCase.expectedReadValue;
                     for (int j = 0; j < expectedValues.size(); j++) {
                         Assertions.assertEquals(expectedValues.get(j), plcList.getIndex(j).getObject());
                     }
                 } else {
                     Assertions.assertEquals(
-                        testCase.expectedValue.toString(), readResponse.getPlcValue(fieldName).getObject().toString());
+                        testCase.expectedReadValue.toString(), readResponse.getPlcValue(fieldName).getObject().toString());
                 }
             }
             System.out.println("Success");
@@ -107,13 +107,13 @@ public abstract class ManualTest {
                     Assertions.assertNotNull(readResponse.getPlcValue(fieldName));
                     if(readResponse.getPlcValue(fieldName) instanceof PlcList) {
                         PlcList plcList = (PlcList) readResponse.getPlcValue(fieldName);
-                        List<Object> expectedValues = (List<Object>) testCase.expectedValue;
+                        List<Object> expectedValues = (List<Object>) testCase.expectedReadValue;
                         for (int j = 0; j < expectedValues.size(); j++) {
                             Assertions.assertEquals(expectedValues.get(j), plcList.getIndex(j).getObject());
                         }
                     } else {
                         Assertions.assertEquals(
-                            testCase.expectedValue.toString(), readResponse.getPlcValue(fieldName).getObject().toString());
+                            testCase.expectedReadValue.toString(), readResponse.getPlcValue(fieldName).getObject().toString());
                     }
                 }
             }
@@ -123,19 +123,25 @@ public abstract class ManualTest {
 
     public static class TestCase {
         private final String address;
-        private final Object expectedValue;
+        private final Object expectedReadValue;
+        private final Object writeValue;
 
-        public TestCase(String address, Object expectedValue) {
+        public TestCase(String address, Object expectedReadValue, Object writeValue) {
             this.address = address;
-            this.expectedValue = expectedValue;
+            this.expectedReadValue = expectedReadValue;
+            this.writeValue = writeValue;
         }
 
         public String getAddress() {
             return address;
         }
 
-        public Object getExpectedValue() {
-            return expectedValue;
+        public Object getExpectedReadValue() {
+            return expectedReadValue;
+        }
+
+        public Object getWriteValue() {
+            return writeValue;
         }
     }
 
diff --git a/protocols/ads/src/test/resources/protocols/ads/manual-test-capture.pcapng b/protocols/ads/src/test/resources/protocols/ads/manual-test-capture.pcapng
index 89baf1e..4f30154 100644
Binary files a/protocols/ads/src/test/resources/protocols/ads/manual-test-capture.pcapng and b/protocols/ads/src/test/resources/protocols/ads/manual-test-capture.pcapng differ