You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@plc4x.apache.org by sr...@apache.org on 2018/10/18 15:01:22 UTC
[incubator-plc4x] branch master updated: [ADS] fix wrong
implementation of type bounds
This is an automated email from the ASF dual-hosted git repository.
sruehl pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-plc4x.git
The following commit(s) were added to refs/heads/master by this push:
new f00e35d [ADS] fix wrong implementation of type bounds
f00e35d is described below
commit f00e35d5e3d5ed88be2e9d57a86449d7c0ac7d1f
Author: Sebastian Rühl <sr...@apache.org>
AuthorDate: Thu Oct 18 17:00:37 2018 +0200
[ADS] fix wrong implementation of type bounds
---
.../apache/plc4x/java/ads/model/AdsDataType.java | 15 +++-
.../plc4x/java/ads/model/AdsPlcFieldHandler.java | 84 ++++++++++++++--------
.../plc4x/java/ads/protocol/Plc4x2AdsProtocol.java | 3 +-
.../ads/protocol/util/LittleEndianEncoder.java | 12 +---
.../java/ads/protocol/Plc4x2AdsProtocolTest.java | 8 +--
plc4j/protocols/ads/src/test/resources/logback.xml | 2 +-
6 files changed, 78 insertions(+), 46 deletions(-)
diff --git a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/model/AdsDataType.java b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/model/AdsDataType.java
index e6cf4c2..87925a8 100644
--- a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/model/AdsDataType.java
+++ b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/model/AdsDataType.java
@@ -174,7 +174,7 @@ public enum AdsDataType {
* If no size specification is given, the default size of 80 characters will be used: Memory use [Bytes] = 80 + 1 Byte for string terminated Null character;
* If string size specification is given: Memory use [Bytes] = String Size + 1 Byte for string terminated Null character);
*/
- STRING(81),
+ STRING(81 * 8),
/**
* TIME
* Duration time. The most siginificant digit is one millisecond. The data type is handled internally like DWORD.
@@ -510,7 +510,7 @@ public enum AdsDataType {
this.upperBound = upperBound;
this.typeName = name();
this.memoryUse = memoryUse;
- this.targetByteSize = this.memoryUse * 8;
+ this.targetByteSize = this.memoryUse / 8;
}
public String getTypeName() {
@@ -536,4 +536,15 @@ public enum AdsDataType {
public boolean withinBounds(double other) {
return other >= lowerBound && other <= upperBound;
}
+
+ @Override
+ public String toString() {
+ return "AdsDataType{" +
+ "typeName='" + typeName + '\'' +
+ ", lowerBound=" + lowerBound +
+ ", upperBound=" + upperBound +
+ ", memoryUse=" + memoryUse +
+ ", targetByteSize=" + targetByteSize +
+ "} " + super.toString();
+ }
}
diff --git a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/model/AdsPlcFieldHandler.java b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/model/AdsPlcFieldHandler.java
index bb9be24..9d1ec6a 100644
--- a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/model/AdsPlcFieldHandler.java
+++ b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/model/AdsPlcFieldHandler.java
@@ -721,28 +721,28 @@ public class AdsPlcFieldHandler extends DefaultPlcFieldHandler {
Class<? extends FieldItem> fieldType;
switch (adsField.getAdsDataType()) {
case BYTE:
- fieldType = DefaultLongFieldItem.class;
+ fieldType = DefaultByteFieldItem.class;
break;
case WORD:
- fieldType = DefaultLongFieldItem.class;
+ fieldType = DefaultByteArrayFieldItem.class;
break;
case DWORD:
- fieldType = DefaultLongFieldItem.class;
+ fieldType = DefaultByteArrayFieldItem.class;
break;
case SINT:
- fieldType = DefaultLongFieldItem.class;
+ fieldType = DefaultIntegerFieldItem.class;
break;
case USINT:
fieldType = DefaultLongFieldItem.class;
break;
case INT:
- fieldType = DefaultLongFieldItem.class;
+ fieldType = DefaultShortFieldItem.class;
break;
case UINT:
- fieldType = DefaultLongFieldItem.class;
+ fieldType = DefaultIntegerFieldItem.class;
break;
case DINT:
- fieldType = DefaultLongFieldItem.class;
+ fieldType = DefaultIntegerFieldItem.class;
break;
case UDINT:
fieldType = DefaultLongFieldItem.class;
@@ -754,7 +754,7 @@ public class AdsPlcFieldHandler extends DefaultPlcFieldHandler {
fieldType = DefaultBigIntegerFieldItem.class;
break;
case INT32:
- fieldType = DefaultLongFieldItem.class;
+ fieldType = DefaultIntegerFieldItem.class;
break;
case INT64:
fieldType = DefaultLongFieldItem.class;
@@ -820,39 +820,67 @@ public class AdsPlcFieldHandler extends DefaultPlcFieldHandler {
AdsField adsField = (AdsField) field;
BigDecimal minValue = BigDecimal.valueOf(adsField.getAdsDataType().getLowerBound());
BigDecimal maxValue = BigDecimal.valueOf(adsField.getAdsDataType().getUpperBound());
+ Class<? extends FieldItem> fieldType;
switch (adsField.getAdsDataType()) {
case REAL:
+ fieldType = DefaultFloatFieldItem.class;
break;
case LREAL:
+ fieldType = DefaultDoubleFieldItem.class;
break;
default:
throw new IllegalArgumentException(
"Cannot assign floating point values to " + adsField.getAdsDataType().name() + " fields.");
}
- Double[] floatingPointValues = new Double[values.length];
- for (int i = 0; i < values.length; i++) {
- if (values[i] instanceof Float) {
- floatingPointValues[i] = ((Float) values[i]).doubleValue();
- } else if (values[i] instanceof Double) {
- floatingPointValues[i] = (Double) values[i];
- } else {
- throw new IllegalArgumentException(
- "Value of type " + values[i].getClass().getName() +
- " is not assignable to " + adsField.getAdsDataType().name() + " fields.");
- }
+ if (fieldType == DefaultDoubleFieldItem.class) {
+ Double[] floatingPointValues = new Double[values.length];
+ for (int i = 0; i < values.length; i++) {
+ if (values[i] instanceof Float) {
+ floatingPointValues[i] = ((Float) values[i]).doubleValue();
+ } else if (values[i] instanceof Double) {
+ floatingPointValues[i] = (Double) values[i];
+ } else {
+ throw new IllegalArgumentException(
+ "Value of type " + values[i].getClass().getName() +
+ " is not assignable to " + adsField.getAdsDataType().name() + " fields.");
+ }
- if (minValue.compareTo(new BigDecimal(floatingPointValues[i])) > 0) {
- throw new IllegalArgumentException(
- "Value of " + floatingPointValues[i] + " exceeds allowed minimum for type "
- + adsField.getAdsDataType().name() + " (min " + minValue.toString() + ")");
+ if (minValue.compareTo(new BigDecimal(floatingPointValues[i])) > 0) {
+ throw new IllegalArgumentException(
+ "Value of " + floatingPointValues[i] + " exceeds allowed minimum for type "
+ + adsField.getAdsDataType().name() + " (min " + minValue.toString() + ")");
+ }
+ if (maxValue.compareTo(new BigDecimal(floatingPointValues[i])) < 0) {
+ throw new IllegalArgumentException(
+ "Value of " + floatingPointValues[i] + " exceeds allowed maximum for type "
+ + adsField.getAdsDataType().name() + " (max " + maxValue.toString() + ")");
+ }
}
- if (maxValue.compareTo(new BigDecimal(floatingPointValues[i])) < 0) {
- throw new IllegalArgumentException(
- "Value of " + floatingPointValues[i] + " exceeds allowed maximum for type "
- + adsField.getAdsDataType().name() + " (max " + maxValue.toString() + ")");
+ return new DefaultDoubleFieldItem(floatingPointValues);
+ } else {
+ Float[] floatingPointValues = new Float[values.length];
+ for (int i = 0; i < values.length; i++) {
+ if (values[i] instanceof Float) {
+ floatingPointValues[i] = (Float) values[i];
+ } else {
+ throw new IllegalArgumentException(
+ "Value of type " + values[i].getClass().getName() +
+ " is not assignable to " + adsField.getAdsDataType().name() + " fields.");
+ }
+
+ if (minValue.compareTo(new BigDecimal(floatingPointValues[i])) > 0) {
+ throw new IllegalArgumentException(
+ "Value of " + floatingPointValues[i] + " exceeds allowed minimum for type "
+ + adsField.getAdsDataType().name() + " (min " + minValue.toString() + ")");
+ }
+ if (maxValue.compareTo(new BigDecimal(floatingPointValues[i])) < 0) {
+ throw new IllegalArgumentException(
+ "Value of " + floatingPointValues[i] + " exceeds allowed maximum for type "
+ + adsField.getAdsDataType().name() + " (max " + maxValue.toString() + ")");
+ }
}
+ return new DefaultFloatFieldItem(floatingPointValues);
}
- return new DefaultDoubleFieldItem(floatingPointValues);
}
private FieldItem internalEncodeString(PlcField field, Object[] values) {
diff --git a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/protocol/Plc4x2AdsProtocol.java b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/protocol/Plc4x2AdsProtocol.java
index 1ba5cd9..fc67896 100644
--- a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/protocol/Plc4x2AdsProtocol.java
+++ b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/protocol/Plc4x2AdsProtocol.java
@@ -159,7 +159,8 @@ public class Plc4x2AdsProtocol extends MessageToMessageCodec<AmsPacket, PlcReque
int bytesToBeWritten = bytes.length;
int maxTheoreticalSize = directAdsField.getAdsDataType().getTargetByteSize() * directAdsField.getNumberOfElements();
if (bytesToBeWritten > maxTheoreticalSize) {
- throw new PlcProtocolPayloadTooBigException("Ads", maxTheoreticalSize, bytesToBeWritten, values);
+ LOGGER.debug("Requested AdsDatatype {} is exceeded by number of bytes {}. Limit {}.", directAdsField.getAdsDataType(), bytesToBeWritten, maxTheoreticalSize);
+ throw new PlcProtocolPayloadTooBigException("ADS", maxTheoreticalSize, bytesToBeWritten, values);
}
Data data = Data.of(bytes);
AmsPacket amsPacket = AdsWriteRequest.of(targetAmsNetId, targetAmsPort, sourceAmsNetId, sourceAmsPort, invokeId, indexGroup, indexOffset, data);
diff --git a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/protocol/util/LittleEndianEncoder.java b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/protocol/util/LittleEndianEncoder.java
index 44b1247..de41485 100644
--- a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/protocol/util/LittleEndianEncoder.java
+++ b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/protocol/util/LittleEndianEncoder.java
@@ -191,16 +191,12 @@ public class LittleEndianEncoder {
return localTimeStream
.map(localTime -> ChronoUnit.MILLIS.between(LocalTime.of(0, 0), localTime))
.peek(value -> checkBound(adsDataType, value))
+ .map(Long::intValue)
.map(time -> new byte[]{
(byte) (time & 0x00000000_000000ffL),
(byte) ((time & 0x00000000_0000ff00L) >> 8),
(byte) ((time & 0x00000000_00ff0000L) >> 16),
(byte) ((time & 0x00000000_ff000000L) >> 24),
-
- (byte) ((time & 0x000000ff_00000000L) >> 32),
- (byte) ((time & 0x0000ff00_00000000L) >> 40),
- (byte) ((time & 0x00ff0000_00000000L) >> 48),
- (byte) ((time & 0xff000000_00000000L) >> 56),
});
}
@@ -210,16 +206,12 @@ public class LittleEndianEncoder {
.map(localDate -> localDate.atTime(0, 0).toInstant(ZoneOffset.UTC))
.map(Instant::getEpochSecond)
.peek(value -> checkBound(adsDataType, value))
+ .map(Long::intValue)
.map(time -> new byte[]{
(byte) (time & 0x00000000_000000ffL),
(byte) ((time & 0x00000000_0000ff00L) >> 8),
(byte) ((time & 0x00000000_00ff0000L) >> 16),
(byte) ((time & 0x00000000_ff000000L) >> 24),
-
- (byte) ((time & 0x000000ff_00000000L) >> 32),
- (byte) ((time & 0x0000ff00_00000000L) >> 40),
- (byte) ((time & 0x00ff0000_00000000L) >> 48),
- (byte) ((time & 0xff000000_00000000L) >> 56),
});
}
diff --git a/plc4j/protocols/ads/src/test/java/org/apache/plc4x/java/ads/protocol/Plc4x2AdsProtocolTest.java b/plc4j/protocols/ads/src/test/java/org/apache/plc4x/java/ads/protocol/Plc4x2AdsProtocolTest.java
index 2cdd449..4f38bd4 100644
--- a/plc4j/protocols/ads/src/test/java/org/apache/plc4x/java/ads/protocol/Plc4x2AdsProtocolTest.java
+++ b/plc4j/protocols/ads/src/test/java/org/apache/plc4x/java/ads/protocol/Plc4x2AdsProtocolTest.java
@@ -111,14 +111,14 @@ public class Plc4x2AdsProtocolTest {
ImmutablePair.of(
new PlcRequestContainer<>(
(InternalPlcRequest) new DefaultPlcWriteRequest.Builder(null, new AdsPlcFieldHandler()) // TODO: remove null
- .addItem(RandomStringUtils.randomAscii(10), "1/1:" + pair.adsDataType, pair.getValue())
+ .addItem(RandomStringUtils.randomAscii(10), "1/1:" + pair.adsDataType.name(), pair.getValue())
.build(), new CompletableFuture<>()),
AdsWriteResponse.of(targetAmsNetId, targetAmsPort, sourceAmsNetId, sourceAmsPort, invokeId, Result.of(0))
),
ImmutablePair.of(
new PlcRequestContainer<>(
(InternalPlcRequest) new DefaultPlcReadRequest.Builder(null, new AdsPlcFieldHandler()) // TODO: remove null
- .addItem(RandomStringUtils.randomAscii(10), "1/1:" + pair.adsDataType)
+ .addItem(RandomStringUtils.randomAscii(10), "1/1:" + pair.adsDataType.name())
.build(), new CompletableFuture<>()),
AdsReadResponse.of(targetAmsNetId, targetAmsPort, sourceAmsNetId, sourceAmsPort, invokeId, Result.of(0), Data.of(pair.getByteRepresentation()))
)
@@ -145,7 +145,7 @@ public class Plc4x2AdsProtocolTest {
dataTypeMap.put(LocalDateTime.class, AdsDataType.DATE_AND_TIME);
dataTypeMap.put(byte[].class, AdsDataType.BYTE);
dataTypeMap.put(Byte[].class, AdsDataType.BYTE);
- return new AdsDataTypePair(dataTypePair, dataTypeMap.getOrDefault(dataTypePair.getDataTypeClass(), AdsDataType.BYTE));
+ return new AdsDataTypePair(dataTypePair, dataTypeMap.get(dataTypePair.getDataTypeClass()));
}
private static class AdsDataTypePair extends Plc4XSupportedDataTypes.DataTypePair {
@@ -154,7 +154,7 @@ public class Plc4x2AdsProtocolTest {
private AdsDataTypePair(Plc4XSupportedDataTypes.DataTypePair dataTypePair, AdsDataType adsDataType) {
super(dataTypePair.getDataTypePair());
- this.adsDataType = adsDataType;
+ this.adsDataType = Objects.requireNonNull(adsDataType);
}
}
diff --git a/plc4j/protocols/ads/src/test/resources/logback.xml b/plc4j/protocols/ads/src/test/resources/logback.xml
index dd243bd..5bfdbdc 100644
--- a/plc4j/protocols/ads/src/test/resources/logback.xml
+++ b/plc4j/protocols/ads/src/test/resources/logback.xml
@@ -31,7 +31,7 @@
</encoder>
</appender>
- <root level="info">
+ <root level="debug">
<appender-ref ref="STDOUT" />
</root>