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 2019/01/16 20:42:18 UTC
[incubator-plc4x] branch develop updated: S7: changed byteLength
and blockNumber from short to int
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
The following commit(s) were added to refs/heads/develop by this push:
new 980c069 S7: changed byteLength and blockNumber from short to int
new 7031908 Merge pull request #44 from timbo2k/too_short_range_for_offset
980c069 is described below
commit 980c0695e3500df60b2fbe159aef6821de0d5e32
Author: Tim Mitsch <ti...@tmbeng.com>
AuthorDate: Wed Jan 16 21:05:24 2019 +0100
S7: changed byteLength and blockNumber from short to int
---
.../org/apache/plc4x/java/s7/model/S7Field.java | 54 ++++++++++++++++++----
.../model/params/items/S7AnyVarParameterItem.java | 10 ++--
.../strategies/DefaultS7MessageProcessor.java | 12 ++---
.../java/org/apache/plc4x/java/issues/PLC4X56.java | 44 +++++++++++++++++-
.../src/test/resources/example_with_strings.yml | 14 +++---
5 files changed, 104 insertions(+), 30 deletions(-)
diff --git a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/model/S7Field.java b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/model/S7Field.java
index a460d6a..8f7e555 100644
--- a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/model/S7Field.java
+++ b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/model/S7Field.java
@@ -28,10 +28,13 @@ import java.util.regex.Pattern;
public class S7Field implements PlcField {
+ //byteOffset theoretically can reach up to 2097151 ... see checkByteOffset() below --> 7digits
private static final Pattern ADDRESS_PATTERN =
- Pattern.compile("^%(?<memoryArea>.)(?<transferSizeCode>[XBWD]?)(?<byteOffset>\\d{1,5})(.(?<bitOffset>[0-7]))?:(?<dataType>[a-zA-Z_]+)(\\[(?<numElements>\\d+)])?");
+ Pattern.compile("^%(?<memoryArea>.)(?<transferSizeCode>[XBWD]?)(?<byteOffset>\\d{1,7})(.(?<bitOffset>[0-7]))?:(?<dataType>[a-zA-Z_]+)(\\[(?<numElements>\\d+)])?");
+
+ //blockNumber usually has its max hat around 64000 --> 5digits
private static final Pattern DATA_BLOCK_ADDRESS_PATTERN =
- Pattern.compile("^%DB(?<blockNumber>\\d{1,4}).DB(?<transferSizeCode>[XBWD]?)(?<byteOffset>\\d{1,5})(.(?<bitOffset>[0-7]))?:(?<dataType>[a-zA-Z_]+)(\\[(?<numElements>\\d+)])?");
+ Pattern.compile("^%DB(?<blockNumber>\\d{1,5}).DB(?<transferSizeCode>[XBWD]?)(?<byteOffset>\\d{1,7})(.(?<bitOffset>[0-7]))?:(?<dataType>[a-zA-Z_]+)(\\[(?<numElements>\\d+)])?");
private static final String DATA_TYPE = "dataType";
private static final String TRANSFER_SIZE_CODE = "transferSizeCode";
@@ -43,12 +46,12 @@ public class S7Field implements PlcField {
private final TransportSize dataType;
private final MemoryArea memoryArea;
- private final short blockNumber;
- private final short byteOffset;
+ private final int blockNumber;
+ private final int byteOffset;
private final short bitOffset;
private final int numElements;
- private S7Field(TransportSize dataType, MemoryArea memoryArea, short blockNumber, short byteOffset, short bitOffset, int numElements) {
+ private S7Field(TransportSize dataType, MemoryArea memoryArea, int blockNumber, int byteOffset, short bitOffset, int numElements) {
this.dataType = dataType;
this.memoryArea = memoryArea;
this.blockNumber = blockNumber;
@@ -65,11 +68,11 @@ public class S7Field implements PlcField {
return memoryArea;
}
- public short getBlockNumber() {
+ public int getBlockNumber() {
return blockNumber;
}
- public short getByteOffset() {
+ public int getByteOffset() {
return byteOffset;
}
@@ -92,8 +95,11 @@ public class S7Field implements PlcField {
TransportSize dataType = TransportSize.valueOf(matcher.group(DATA_TYPE));
MemoryArea memoryArea = MemoryArea.DATA_BLOCKS;
String transferSizeCode = matcher.group(TRANSFER_SIZE_CODE);
- short blockNumber = Short.parseShort(matcher.group(BLOCK_NUMBER));
- short byteOffset = Short.parseShort(matcher.group(BYTE_OFFSET));
+
+ int blockNumber = checkDatablockNumber(Integer.parseInt(matcher.group(BLOCK_NUMBER)));
+
+ int byteOffset = checkByteOffset(Integer.parseInt(matcher.group(BYTE_OFFSET)));
+
short bitOffset = 0;
if(matcher.group(BIT_OFFSET) != null) {
bitOffset = Short.parseShort(matcher.group(BIT_OFFSET));
@@ -116,7 +122,9 @@ public class S7Field implements PlcField {
TransportSize dataType = TransportSize.valueOf(matcher.group(DATA_TYPE));
MemoryArea memoryArea = MemoryArea.valueOfShortName(matcher.group(MEMORY_AREA));
String transferSizeCode = matcher.group(TRANSFER_SIZE_CODE);
- short byteOffset = Short.parseShort(matcher.group(BYTE_OFFSET));
+
+ int byteOffset = checkByteOffset(Integer.parseInt(matcher.group(BYTE_OFFSET)));
+
short bitOffset = 0;
if(matcher.group(BIT_OFFSET) != null) {
bitOffset = Short.parseShort(matcher.group(BIT_OFFSET));
@@ -139,6 +147,32 @@ public class S7Field implements PlcField {
}
/**
+ * checks if DatablockNumber of S7Field is in valid range
+ * @param blockNumber given DatablockNumber
+ * @return given blockNumber if Ok, throws PlcInvalidFieldException otherwise
+ */
+ private static int checkDatablockNumber(int blockNumber){
+ //ToDo check the value or add reference - limit eventually depending on active S7 --> make a case selection
+ if(blockNumber>64000 || blockNumber<1){
+ throw new PlcInvalidFieldException("Datablock numbers larger than 64000 or smaller than 1 are not supported.");
+ }
+ return blockNumber;
+ }
+
+ /**
+ * checks if ByteOffset from S7Field is in valid range
+ * @param byteOffset given byteOffset
+ * @return given byteOffset if Ok, throws PlcInvalidFieldException otherwise
+ */
+ private static int checkByteOffset(int byteOffset){
+ //ToDo check the value or add reference
+ if(byteOffset>2097151 || byteOffset<0){
+ throw new PlcInvalidFieldException("ByteOffset must be smaller than 2097151 and positive.");
+ }
+ return byteOffset;
+ }
+
+ /**
* correct the storage of "array"-like variables like STRING
* @param numElements auto-detected numElements (1 if no numElements in brackets has been given, x if a specific number has been given)
* @param dataType detected Transport-Size that represents the data-type
diff --git a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/model/params/items/S7AnyVarParameterItem.java b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/model/params/items/S7AnyVarParameterItem.java
index b1849f8..e9bc87c 100644
--- a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/model/params/items/S7AnyVarParameterItem.java
+++ b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/model/params/items/S7AnyVarParameterItem.java
@@ -44,11 +44,11 @@ public class S7AnyVarParameterItem implements VarParameterItem {
private final MemoryArea memoryArea;
private final TransportSize dataType;
private final int numElements;
- private final short dataBlockNumber;
- private final short byteOffset;
+ private final int dataBlockNumber;
+ private final int byteOffset;
private final byte bitOffset;
- public S7AnyVarParameterItem(SpecificationType specificationType, MemoryArea memoryArea, TransportSize dataType, int numElements, short dataBlockNumber, short byteOffset, byte bitOffset) {
+ public S7AnyVarParameterItem(SpecificationType specificationType, MemoryArea memoryArea, TransportSize dataType, int numElements, int dataBlockNumber, int byteOffset, byte bitOffset) {
this.specificationType = specificationType;
this.memoryArea = memoryArea;
this.dataType = dataType;
@@ -79,11 +79,11 @@ public class S7AnyVarParameterItem implements VarParameterItem {
return numElements;
}
- public short getDataBlockNumber() {
+ public int getDataBlockNumber() {
return dataBlockNumber;
}
- public short getByteOffset() {
+ public int getByteOffset() {
return byteOffset;
}
diff --git a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/strategies/DefaultS7MessageProcessor.java b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/strategies/DefaultS7MessageProcessor.java
index 439199a..41db08a 100644
--- a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/strategies/DefaultS7MessageProcessor.java
+++ b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/strategies/DefaultS7MessageProcessor.java
@@ -141,7 +141,7 @@ public class DefaultS7MessageProcessor implements S7MessageProcessor {
(double) maxResponseSize / (double) s7AnyVarParameterItem.getDataType().getSizeInBytes());
int sizeMaxNumElementInBytes = maxNumElements * s7AnyVarParameterItem.getDataType().getSizeInBytes();
int remainingNumElements = s7AnyVarParameterItem.getNumElements();
- short curByteOffset = s7AnyVarParameterItem.getByteOffset();
+ int curByteOffset = s7AnyVarParameterItem.getByteOffset();
while(remainingNumElements > 0) {
int numCurElements = Math.min(remainingNumElements, maxNumElements);
@@ -218,7 +218,7 @@ public class DefaultS7MessageProcessor implements S7MessageProcessor {
if (varParameterItem instanceof S7AnyVarParameterItem) {
S7AnyVarParameterItem s7AnyVarParameterItem = (S7AnyVarParameterItem) varParameterItem;
- short byteOffset = s7AnyVarParameterItem.getByteOffset();
+ int byteOffset = s7AnyVarParameterItem.getByteOffset();
if (s7AnyVarParameterItem.getDataType() == TransportSize.BOOL) {
processBooleanWriteVarParameter(request, varParameter, varPayload, s7AnyVarParameterItem,
varPayloadItem, byteOffset, compositeRequestMessage);
@@ -235,8 +235,8 @@ public class DefaultS7MessageProcessor implements S7MessageProcessor {
private void processBooleanWriteVarParameter(S7RequestMessage request, VarParameter varParameter, VarPayload varPayload,
S7AnyVarParameterItem s7AnyVarParameterItem, VarPayloadItem varPayloadItem,
- short byteOffset, S7CompositeRequestMessage compositeRequestMessage) {
- short curParameterByteOffset = byteOffset;
+ int byteOffset, S7CompositeRequestMessage compositeRequestMessage) {
+ int curParameterByteOffset = byteOffset;
byte curParameterBitOffset = s7AnyVarParameterItem.getBitOffset();
byte curPayloadBitOffset = 0;
for (int i = 0; i < s7AnyVarParameterItem.getNumElements(); i++) {
@@ -288,8 +288,8 @@ public class DefaultS7MessageProcessor implements S7MessageProcessor {
private void processNonBooleanWriteVarParameter(S7RequestMessage request, VarParameter varParameter, VarPayload varPayload,
S7AnyVarParameterItem s7AnyVarParameterItem, VarPayloadItem varPayloadItem,
- short byteOffset, S7CompositeRequestMessage compositeRequestMessage) {
- short curByteOffset = byteOffset;
+ int byteOffset, S7CompositeRequestMessage compositeRequestMessage) {
+ int curByteOffset = byteOffset;
int payloadPosition = 0;
for (int i = 0; i < s7AnyVarParameterItem.getNumElements(); i++) {
int elementSize = s7AnyVarParameterItem.getDataType().getSizeInBytes();
diff --git a/plc4j/protocols/s7/src/test/java/org/apache/plc4x/java/issues/PLC4X56.java b/plc4j/protocols/s7/src/test/java/org/apache/plc4x/java/issues/PLC4X56.java
index d5de122..15d7a42 100644
--- a/plc4j/protocols/s7/src/test/java/org/apache/plc4x/java/issues/PLC4X56.java
+++ b/plc4j/protocols/s7/src/test/java/org/apache/plc4x/java/issues/PLC4X56.java
@@ -19,10 +19,14 @@
package org.apache.plc4x.java.issues;
+import org.apache.plc4x.java.api.exceptions.PlcInvalidFieldException;
import org.apache.plc4x.java.s7.model.S7Field;
import org.apache.plc4x.java.s7.netty.model.types.MemoryArea;
import org.apache.plc4x.java.s7.netty.model.types.TransportSize;
+import org.junit.Assert;
+import org.junit.Rule;
import org.junit.jupiter.api.Test;
+import org.junit.rules.ExpectedException;
import static org.hamcrest.core.IsEqual.equalTo;
import static org.junit.Assert.assertThat;
@@ -33,11 +37,47 @@ public class PLC4X56 {
void name() {
S7Field field = S7Field.of("%DB56.DBB100:SINT[25]");
assertThat(field.getMemoryArea(), equalTo(MemoryArea.DATA_BLOCKS));
- assertThat(field.getBlockNumber(), equalTo((short) 56));
- assertThat(field.getByteOffset(), equalTo((short) 100));
+ assertThat(field.getBlockNumber(), equalTo(56));
+ assertThat(field.getByteOffset(), equalTo(100));
assertThat(field.getBitOffset(), equalTo((short) 0));
assertThat(field.getDataType(), equalTo(TransportSize.SINT));
assertThat(field.getNumElements(), equalTo(25));
}
+ @Test
+ public void invalidBlockLengthThrowsException() throws Exception {
+ try {
+ S7Field field = S7Field.of("%DB2000.DBB8000000:SINT[25]");
+ Assert.fail();
+ }
+ catch (PlcInvalidFieldException e){
+ e.printStackTrace();
+ // exception thrown --> Test Ok
+ }
+ }
+
+ @Test
+ public void invalidBlockNumber1ThrowsException() throws Exception {
+ try {
+ S7Field field = S7Field.of("%DB0.DBB800:SINT[25]");
+ Assert.fail();
+ }
+ catch (PlcInvalidFieldException e){
+ e.printStackTrace();
+ // exception thrown --> Test Ok
+ }
+ }
+
+ @Test
+ public void invalidBlockNumber2ThrowsException() throws Exception {
+ try {
+ S7Field field = S7Field.of("%DB80000.DBB800:SINT[25]");
+ Assert.fail();
+ }
+ catch (PlcInvalidFieldException e){
+ e.printStackTrace();
+ // exception thrown --> Test Ok
+ }
+ }
+
}
diff --git a/plc4j/utils/scraper/src/test/resources/example_with_strings.yml b/plc4j/utils/scraper/src/test/resources/example_with_strings.yml
index 37309f0..dce108a 100644
--- a/plc4j/utils/scraper/src/test/resources/example_with_strings.yml
+++ b/plc4j/utils/scraper/src/test/resources/example_with_strings.yml
@@ -26,13 +26,13 @@ jobs:
sources:
- S7_TIM
fields:
- test1: '%DB810:DBW0:UINT'
- test2: '%DB810:DBX4:STRING'
- test3: '%DB810:DBX264:STRING'
- test4: '%DB810:DBX524:STRING'
- test5: '%DB810:DBX784:STRING'
- test6: '%DB810:DBX1044:STRING'
- test7: '%DB810:DBD0:REAL'
+ test1: '%DB810:DBW0:INT'
+ test2: '%DB810:DBX6:STRING'
+ test3: '%DB810:DBX266:STRING'
+ test4: '%DB810:DBX526:STRING'
+ test5: '%DB810:DBX786:STRING'
+ test6: '%DB810:DBX46806:STRING'
+ test7: '%DB810:DBD2:REAL'
test8: '%DB811:DBX12:STRING'
test9: '%DB811:DBX280:STRING'
test10: '%DB811:DBB1000:BYTE[8]'