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 7ef7f00 [ADS] fixed a bunch of sonar issues 7ef7f00 is described below commit 7ef7f00527e1b3ef68699ab916b7ff3a33f503fd Author: Sebastian Rühl <sru...@apache.org> AuthorDate: Thu Oct 18 19:25:10 2018 +0200 [ADS] fixed a bunch of sonar issues --- .../java/ads/api/commands/UnknownCommand.java | 2 +- .../java/ads/api/commands/types/IndexGroup.java | 8 +++++ .../java/ads/api/commands/types/IndexOffset.java | 4 +++ .../ads/api/commands/types/TransmissionMode.java | 4 +++ .../plc4x/java/ads/api/generic/AmsPacket.java | 8 +++-- .../plc4x/java/ads/api/generic/types/AmsPort.java | 8 ++--- .../ads/api/serial/AmsSerialAcknowledgeFrame.java | 28 ++++++++++++---- .../plc4x/java/ads/api/serial/AmsSerialFrame.java | 32 +++++++++++++----- .../java/ads/api/serial/AmsSerialResetFrame.java | 28 ++++++++++++---- .../plc4x/java/ads/api/tcp/AmsTCPPacket.java | 12 +++++-- .../java/ads/connection/AdsTcpPlcConnection.java | 28 +++++++++++++--- .../org/apache/plc4x/java/ads/model/AdsField.java | 1 + .../plc4x/java/ads/model/DirectAdsField.java | 3 +- .../plc4x/java/ads/model/SymbolicAdsField.java | 3 +- .../java/ads/protocol/Ads2PayloadProtocol.java | 39 +++++++++++----------- .../java/ads/protocol/Payload2SerialProtocol.java | 2 ++ .../plc4x/java/ads/protocol/Plc4x2AdsProtocol.java | 4 +-- .../exception/AdsProtocolOverflowException.java} | 15 ++++++--- .../plc4x/java/ads/protocol/util/DigestUtil.java | 10 +++--- .../ads/protocol/util/LittleEndianDecoder.java | 2 +- .../protocol/util/SingleMessageRateLimiter.java | 10 +++--- 21 files changed, 177 insertions(+), 74 deletions(-) diff --git a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/api/commands/UnknownCommand.java b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/api/commands/UnknownCommand.java index df395dc..44bf79c 100644 --- a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/api/commands/UnknownCommand.java +++ b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/api/commands/UnknownCommand.java @@ -32,7 +32,7 @@ import static java.util.Objects.requireNonNull; @AdsCommandType(Command.UNKNOWN) public class UnknownCommand extends AmsPacket { - private transient final ByteBuf remainingBytes; + private final transient ByteBuf remainingBytes; private UnknownCommand(AmsHeader amsHeader, ByteBuf remainingBytes) { super(amsHeader); diff --git a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/api/commands/types/IndexGroup.java b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/api/commands/types/IndexGroup.java index 1a84ebf..c52a498 100644 --- a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/api/commands/types/IndexGroup.java +++ b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/api/commands/types/IndexGroup.java @@ -85,6 +85,10 @@ public class IndexGroup extends UnsignedIntLEByteValue { public static final IndexGroup ADSIGRP_DEVICE_DATA = IndexGroup.of(0xF100); public static final IndexGroup ADSIOFFS_DEVDATA_ADSSTATE = IndexGroup.of(0x0000); public static final IndexGroup ADSIOFFS_DEVDATA_DEVSTATE = IndexGroup.of(0x0002); + + private ReservedGroups() { + // Container class + } } public static final class SystemServiceGroups { @@ -111,5 +115,9 @@ public class IndexGroup extends UnsignedIntLEByteValue { public static final IndexGroup SYSTEMSERVICE_TIMESERVICES = IndexGroup.of(400); public static final IndexGroup SYSTEMSERVICE_STARTPROCESS = IndexGroup.of(500); public static final IndexGroup SYSTEMSERVICE_CHANGENETID = IndexGroup.of(600); + + private SystemServiceGroups() { + // Container class + } } } diff --git a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/api/commands/types/IndexOffset.java b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/api/commands/types/IndexOffset.java index 943c41c..e307f80 100644 --- a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/api/commands/types/IndexOffset.java +++ b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/api/commands/types/IndexOffset.java @@ -65,5 +65,9 @@ public class IndexOffset extends UnsignedIntLEByteValue { public static final IndexOffset TIMESERVICE_SYSTEMTIMES = IndexOffset.of(2); public static final IndexOffset TIMESERVICE_RTCTIMEDIFF = IndexOffset.of(3); public static final IndexOffset TIMESERVICE_ADJUSTTIMETORTC = IndexOffset.of(4); + + private SystemServiceOffsets() { + // Container class + } } } diff --git a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/api/commands/types/TransmissionMode.java b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/api/commands/types/TransmissionMode.java index 64ae165..83b1147 100644 --- a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/api/commands/types/TransmissionMode.java +++ b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/api/commands/types/TransmissionMode.java @@ -69,5 +69,9 @@ public class TransmissionMode extends UnsignedIntLEByteValue { public static final TransmissionMode ADSTRANS_SERVERONCHA2 = TransmissionMode.of(6); public static final TransmissionMode ADSTRANS_CLIENT1REQ = TransmissionMode.of(10); public static final TransmissionMode ADSTRANS_MAXMODES = TransmissionMode.of(Integer.MAX_VALUE); + + private DefinedValues() { + // Container class + } } } diff --git a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/api/generic/AmsPacket.java b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/api/generic/AmsPacket.java index 51e10ca..b8a70fc 100644 --- a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/api/generic/AmsPacket.java +++ b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/api/generic/AmsPacket.java @@ -66,8 +66,12 @@ public abstract class AmsPacket implements ByteReadable { @Override public boolean equals(Object o) { - if (this == o) return true; - if (!(o instanceof AmsPacket)) return false; + if (this == o) { + return true; + } + if (!(o instanceof AmsPacket)) { + return false; + } AmsPacket amsPacket = (AmsPacket) o; diff --git a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/api/generic/types/AmsPort.java b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/api/generic/types/AmsPort.java index 5ae3123..061e301 100644 --- a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/api/generic/types/AmsPort.java +++ b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/api/generic/types/AmsPort.java @@ -69,10 +69,6 @@ public class AmsPort extends UnsignedShortLEByteValue { public static class ReservedPorts { - private ReservedPorts() { - // Container class - } - /** * Logger (only NT-Log) */ @@ -128,5 +124,9 @@ public class AmsPort extends UnsignedShortLEByteValue { * Scope */ public static final AmsPort scope = AmsPort.of(900); + + private ReservedPorts() { + // Container class + } } } diff --git a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/api/serial/AmsSerialAcknowledgeFrame.java b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/api/serial/AmsSerialAcknowledgeFrame.java index 16911b8..d3dfc77 100644 --- a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/api/serial/AmsSerialAcknowledgeFrame.java +++ b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/api/serial/AmsSerialAcknowledgeFrame.java @@ -121,16 +121,30 @@ public class AmsSerialAcknowledgeFrame implements ByteReadable { @Override public boolean equals(Object o) { - if (this == o) return true; - if (!(o instanceof AmsSerialAcknowledgeFrame)) return false; + if (this == o) { + return true; + } + if (!(o instanceof AmsSerialAcknowledgeFrame)) { + return false; + } AmsSerialAcknowledgeFrame that = (AmsSerialAcknowledgeFrame) o; - if (!magicCookie.equals(that.magicCookie)) return false; - if (!transmitterAddress.equals(that.transmitterAddress)) return false; - if (!receiverAddress.equals(that.receiverAddress)) return false; - if (!fragmentNumber.equals(that.fragmentNumber)) return false; - if (!userDataLength.equals(that.userDataLength)) return false; + if (!magicCookie.equals(that.magicCookie)) { + return false; + } + if (!transmitterAddress.equals(that.transmitterAddress)) { + return false; + } + if (!receiverAddress.equals(that.receiverAddress)) { + return false; + } + if (!fragmentNumber.equals(that.fragmentNumber)) { + return false; + } + if (!userDataLength.equals(that.userDataLength)) { + return false; + } return crc.equals(that.crc); } diff --git a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/api/serial/AmsSerialFrame.java b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/api/serial/AmsSerialFrame.java index a0f1feb..8ca399e 100644 --- a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/api/serial/AmsSerialFrame.java +++ b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/api/serial/AmsSerialFrame.java @@ -142,17 +142,33 @@ public class AmsSerialFrame implements ByteReadable { @Override public boolean equals(Object o) { - if (this == o) return true; - if (!(o instanceof AmsSerialFrame)) return false; + if (this == o) { + return true; + } + if (!(o instanceof AmsSerialFrame)) { + return false; + } AmsSerialFrame that = (AmsSerialFrame) o; - if (!magicCookie.equals(that.magicCookie)) return false; - if (!transmitterAddress.equals(that.transmitterAddress)) return false; - if (!receiverAddress.equals(that.receiverAddress)) return false; - if (!fragmentNumber.equals(that.fragmentNumber)) return false; - if (!userDataLength.equals(that.userDataLength)) return false; - if (!userData.equals(that.userData)) return false; + if (!magicCookie.equals(that.magicCookie)) { + return false; + } + if (!transmitterAddress.equals(that.transmitterAddress)) { + return false; + } + if (!receiverAddress.equals(that.receiverAddress)) { + return false; + } + if (!fragmentNumber.equals(that.fragmentNumber)) { + return false; + } + if (!userDataLength.equals(that.userDataLength)) { + return false; + } + if (!userData.equals(that.userData)) { + return false; + } return crc.equals(that.crc); } diff --git a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/api/serial/AmsSerialResetFrame.java b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/api/serial/AmsSerialResetFrame.java index 4c8ab65..342ba0e 100644 --- a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/api/serial/AmsSerialResetFrame.java +++ b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/api/serial/AmsSerialResetFrame.java @@ -133,16 +133,30 @@ public class AmsSerialResetFrame implements ByteReadable { @Override public boolean equals(Object o) { - if (this == o) return true; - if (!(o instanceof AmsSerialResetFrame)) return false; + if (this == o) { + return true; + } + if (!(o instanceof AmsSerialResetFrame)) { + return false; + } AmsSerialResetFrame that = (AmsSerialResetFrame) o; - if (!magicCookie.equals(that.magicCookie)) return false; - if (!transmitterAddress.equals(that.transmitterAddress)) return false; - if (!receiverAddress.equals(that.receiverAddress)) return false; - if (!fragmentNumber.equals(that.fragmentNumber)) return false; - if (!userDataLength.equals(that.userDataLength)) return false; + if (!magicCookie.equals(that.magicCookie)) { + return false; + } + if (!transmitterAddress.equals(that.transmitterAddress)) { + return false; + } + if (!receiverAddress.equals(that.receiverAddress)) { + return false; + } + if (!fragmentNumber.equals(that.fragmentNumber)) { + return false; + } + if (!userDataLength.equals(that.userDataLength)) { + return false; + } return crc.equals(that.crc); } diff --git a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/api/tcp/AmsTCPPacket.java b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/api/tcp/AmsTCPPacket.java index 5c34f79..b2a5592 100644 --- a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/api/tcp/AmsTCPPacket.java +++ b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/api/tcp/AmsTCPPacket.java @@ -62,12 +62,18 @@ public class AmsTCPPacket implements ByteReadable { @Override public boolean equals(Object o) { - if (this == o) return true; - if (!(o instanceof AmsTCPPacket)) return false; + if (this == o) { + return true; + } + if (!(o instanceof AmsTCPPacket)) { + return false; + } AmsTCPPacket that = (AmsTCPPacket) o; - if (!amsTcpHeader.equals(that.amsTcpHeader)) return false; + if (!amsTcpHeader.equals(that.amsTcpHeader)) { + return false; + } return userData.equals(that.userData); } diff --git a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/connection/AdsTcpPlcConnection.java b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/connection/AdsTcpPlcConnection.java index d4b2364..3a78dd8 100644 --- a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/connection/AdsTcpPlcConnection.java +++ b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/connection/AdsTcpPlcConnection.java @@ -56,6 +56,9 @@ import java.net.UnknownHostException; import java.time.temporal.ChronoUnit; import java.util.*; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Consumer; import java.util.stream.Collectors; @@ -115,7 +118,7 @@ public class AdsTcpPlcConnection extends AdsAbstractPlcConnection implements Plc pipeline.addLast(new Payload2TcpProtocol()); pipeline.addLast(new Ads2PayloadProtocol()); pipeline.addLast(new Plc4x2AdsProtocol(targetAmsNetId, targetAmsPort, sourceAmsNetId, sourceAmsPort, fieldMapping)); - pipeline.addLast(new SingleItemToSingleRequestProtocol(AdsTcpPlcConnection.this, AdsTcpPlcConnection.this, timer)); // TODO: remove nulls; implement correctly + pipeline.addLast(new SingleItemToSingleRequestProtocol(AdsTcpPlcConnection.this, AdsTcpPlcConnection.this, timer)); } }; } @@ -200,7 +203,7 @@ public class AdsTcpPlcConnection extends AdsAbstractPlcConnection implements Plc Invoke.NONE, indexGroup, indexOffset, - Length.of(adsDataType.getTargetByteSize() * numberOfElements), + Length.of(adsDataType.getTargetByteSize() * (long) numberOfElements), transmissionMode, MaxDelay.of(0), CycleTime.of(cycleTime) @@ -223,7 +226,7 @@ public class AdsTcpPlcConnection extends AdsAbstractPlcConnection implements Plc .stream() .collect(Collectors.toMap( fieldName -> fieldName, - __ -> Pair.of(PlcResponseCode.OK, adsSubscriptionHandle) + ignored -> Pair.of(PlcResponseCode.OK, adsSubscriptionHandle) )); future.complete(new DefaultPlcSubscriptionResponse(internalPlcSubscriptionRequest, responseItems)); @@ -324,7 +327,24 @@ public class AdsTcpPlcConnection extends AdsAbstractPlcConnection implements Plc @Override public void close() throws PlcConnectionException { - // TODO: unregister all consumers. + try { + consumerRegistrations.values().forEach(getChannel().pipeline().get(Plc4x2AdsProtocol.class)::removeConsumer); + List<PlcSubscriptionHandle> collect = consumerRegistrations.keySet().stream() + .map(InternalPlcConsumerRegistration::getAssociatedHandles) + .flatMap(Collection::stream) + .map(PlcSubscriptionHandle.class::cast) + .collect(Collectors.toList()); + + PlcUnsubscriptionRequest plcUnsubscriptionRequest = new DefaultPlcUnsubscriptionRequest.Builder(this).addHandles(collect).build(); + unsubscribe(plcUnsubscriptionRequest).get(5, TimeUnit.SECONDS); + + consumerRegistrations.clear(); + } catch (InterruptedException e) { + LOGGER.warn("Exception while closing", e); + Thread.currentThread().interrupt(); + } catch (RuntimeException | ExecutionException | TimeoutException e) { + LOGGER.warn("Exception while closing", e); + } super.close(); } } diff --git a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/model/AdsField.java b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/model/AdsField.java index cab080b..d4938e6 100644 --- a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/model/AdsField.java +++ b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/model/AdsField.java @@ -20,6 +20,7 @@ package org.apache.plc4x.java.ads.model; import org.apache.plc4x.java.api.model.PlcField; +@FunctionalInterface public interface AdsField extends PlcField { AdsDataType getAdsDataType(); } diff --git a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/model/DirectAdsField.java b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/model/DirectAdsField.java index 70211de..27bc45f 100644 --- a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/model/DirectAdsField.java +++ b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/model/DirectAdsField.java @@ -57,7 +57,7 @@ public class DirectAdsField implements AdsField { return new DirectAdsField(indexGroup, indexOffset, adsDataType, numberOfElements); } - public static DirectAdsField of(String address) throws PlcInvalidFieldException { + public static DirectAdsField of(String address) { Matcher matcher = RESOURCE_ADDRESS_PATTERN.matcher(address); if (!matcher.matches()) { throw new PlcInvalidFieldException(address, RESOURCE_ADDRESS_PATTERN, "{indexGroup}/{indexOffset}:{adsDataType}([numberOfElements])?"); @@ -104,6 +104,7 @@ public class DirectAdsField implements AdsField { return indexOffset; } + @Override public AdsDataType getAdsDataType() { return adsDataType; } diff --git a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/model/SymbolicAdsField.java b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/model/SymbolicAdsField.java index bd94d7b..52f9720 100644 --- a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/model/SymbolicAdsField.java +++ b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/model/SymbolicAdsField.java @@ -46,7 +46,7 @@ public class SymbolicAdsField implements AdsField { } } - public static SymbolicAdsField of(String address) throws PlcInvalidFieldException { + public static SymbolicAdsField of(String address) { Matcher matcher = SYMBOLIC_ADDRESS_PATTERN.matcher(address); if (!matcher.matches()) { throw new PlcInvalidFieldException(address, SYMBOLIC_ADDRESS_PATTERN, "{address}"); @@ -70,6 +70,7 @@ public class SymbolicAdsField implements AdsField { return symbolicAddress; } + @Override public AdsDataType getAdsDataType() { return adsDataType; } diff --git a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/protocol/Ads2PayloadProtocol.java b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/protocol/Ads2PayloadProtocol.java index d0d8468..7d75019 100644 --- a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/protocol/Ads2PayloadProtocol.java +++ b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/protocol/Ads2PayloadProtocol.java @@ -28,6 +28,7 @@ import org.apache.plc4x.java.ads.api.commands.types.*; import org.apache.plc4x.java.ads.api.generic.AmsHeader; import org.apache.plc4x.java.ads.api.generic.AmsPacket; import org.apache.plc4x.java.ads.api.generic.types.*; +import org.apache.plc4x.java.ads.protocol.exception.AdsProtocolOverflowException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -91,7 +92,7 @@ public class Ads2PayloadProtocol extends MessageToMessageCodec<ByteBuf, AmsPacke LOGGER.debug("Correlated packet received {}", correlatedAmsPacket); } if (dataLength.getAsLong() > Integer.MAX_VALUE) { - throw new IllegalStateException("Overflow in datalength: " + dataLength.getAsLong()); + throw new AdsProtocolOverflowException(Integer.class, dataLength.getAsLong()); } ByteBuf commandBuffer = byteBuf.readBytes((int) dataLength.getAsLong()); AmsHeader amsHeader = AmsHeader.of(targetAmsNetId, targetAmsPort, sourceAmsNetId, sourceAmsPort, commandId, stateId, dataLength, errorCode, invoke); @@ -169,10 +170,10 @@ public class Ads2PayloadProtocol extends MessageToMessageCodec<ByteBuf, AmsPacke Result result = Result.of(commandBuffer); Length length = Length.of(commandBuffer); if (length.getAsLong() > Integer.MAX_VALUE) { - throw new IllegalStateException("Overflow in datalength: " + length.getAsLong()); + throw new AdsProtocolOverflowException(Integer.class, length.getAsLong()); } if (length.getAsLong() > ADS_READ_COMMAND_MAX_BYTES) { - throw new IllegalStateException("Overflow of ADS_READ_COMMAND_MAX_BYTES: " + ADS_READ_COMMAND_MAX_BYTES + ". Actual " + length.getAsLong() + "bytes."); + throw new AdsProtocolOverflowException("ADS_READ_COMMAND_MAX_BYTES", ADS_READ_COMMAND_MAX_BYTES, length.getAsLong()); } byte[] dataToRead = new byte[(int) length.getAsLong()]; commandBuffer.readBytes(dataToRead); @@ -189,10 +190,10 @@ public class Ads2PayloadProtocol extends MessageToMessageCodec<ByteBuf, AmsPacke IndexOffset indexOffset = IndexOffset.of(commandBuffer); Length length = Length.of(commandBuffer); if (length.getAsLong() > Integer.MAX_VALUE) { - throw new IllegalStateException("Overflow in datalength: " + length.getAsLong()); + throw new AdsProtocolOverflowException(Integer.class, length.getAsLong()); } if (length.getAsLong() > ADS_WRITE_COMMAND_MAX_BYTES) { - throw new IllegalStateException("Overflow of ADS_WRITE_COMMAND_MAX_BYTES: " + ADS_WRITE_COMMAND_MAX_BYTES + ". Actual " + length.getAsLong() + "bytes."); + throw new AdsProtocolOverflowException("ADS_WRITE_COMMAND_MAX_BYTES", ADS_WRITE_COMMAND_MAX_BYTES, length.getAsLong()); } byte[] dataToRead = new byte[(int) length.getAsLong()]; commandBuffer.readBytes(dataToRead); @@ -226,10 +227,10 @@ public class Ads2PayloadProtocol extends MessageToMessageCodec<ByteBuf, AmsPacke DeviceState deviceState = DeviceState.of(commandBuffer); Length length = Length.of(commandBuffer); if (length.getAsLong() > Integer.MAX_VALUE) { - throw new IllegalStateException("Overflow in datalength: " + length.getAsLong()); + throw new AdsProtocolOverflowException(Integer.class, length.getAsLong()); } if (length.getAsLong() > ADS_WRITE_CONTROL_COMMAND_MAX_BYTES) { - throw new IllegalStateException("Overflow of ADS_WRITE_CONTROL_COMMAND_MAX_BYTES: " + ADS_WRITE_CONTROL_COMMAND_MAX_BYTES + ". Actual " + length.getAsLong() + "bytes."); + throw new AdsProtocolOverflowException("ADS_WRITE_CONTROL_COMMAND_MAX_BYTES", ADS_WRITE_CONTROL_COMMAND_MAX_BYTES, length.getAsLong()); } byte[] dataToRead = new byte[(int) length.getAsLong()]; commandBuffer.readBytes(dataToRead); @@ -278,17 +279,17 @@ public class Ads2PayloadProtocol extends MessageToMessageCodec<ByteBuf, AmsPacke if (stateId.isRequest()) { Length length = Length.of(commandBuffer); if (length.getAsLong() > Integer.MAX_VALUE) { - throw new IllegalStateException("Overflow in datalength: " + length.getAsLong()); + throw new AdsProtocolOverflowException(Integer.class, length.getAsLong()); } Stamps stamps = Stamps.of(commandBuffer); if (stamps.getAsLong() > Integer.MAX_VALUE) { - throw new IllegalStateException("Overflow in datalength: " + stamps.getAsLong()); + throw new AdsProtocolOverflowException(Integer.class, stamps.getAsLong()); } // Note: the length includes the 4 Bytes of stamps which we read already so we substract. ByteBuf adsDeviceNotificationBuffer = commandBuffer.readBytes((int) length.getAsLong() - Stamps.NUM_BYTES); List<AdsStampHeader> adsStampHeaders = new ArrayList<>((int) stamps.getAsLong()); if (stamps.getAsLong() > MAX_NUM_STAMPS) { - throw new IllegalStateException("Overflow of MAX_NUM_STAMPS: " + MAX_NUM_STAMPS + ". Actual " + stamps.getAsLong()); + throw new AdsProtocolOverflowException("MAX_NUM_STAMPS", MAX_NUM_STAMPS, length.getAsLong()); } for (int i = 1; i <= stamps.getAsLong(); i++) { AdsStampHeader adsStampHeader = handleStampHeader(adsDeviceNotificationBuffer); @@ -307,7 +308,7 @@ public class Ads2PayloadProtocol extends MessageToMessageCodec<ByteBuf, AmsPacke List<AdsNotificationSample> adsNotificationSamples = new LinkedList<>(); if (samples.getAsLong() > MAX_NUM_SAMPLES) { - throw new IllegalStateException("Overflow of MAX_NUM_SAMPLES: " + MAX_NUM_SAMPLES + ". Actual " + samples.getAsLong()); + throw new AdsProtocolOverflowException("MAX_NUM_SAMPLES", MAX_NUM_SAMPLES, samples.getAsLong()); } for (int i = 1; i <= samples.getAsLong(); i++) { AdsNotificationSample adsNotificationSample = handleAdsNotificartionSample(adsDeviceNotificationBuffer); @@ -321,10 +322,10 @@ public class Ads2PayloadProtocol extends MessageToMessageCodec<ByteBuf, AmsPacke NotificationHandle notificationHandle = NotificationHandle.of(adsDeviceNotificationBuffer); SampleSize sampleSize = SampleSize.of(adsDeviceNotificationBuffer); if (sampleSize.getAsLong() > Integer.MAX_VALUE) { - throw new IllegalStateException("Overflow in datalength: " + sampleSize.getAsLong()); + throw new AdsProtocolOverflowException(Integer.class, sampleSize.getAsLong()); } if (sampleSize.getAsLong() > ADS_NOTIFICATION_SAMPLE_MAX_BYTES) { - throw new IllegalStateException("Overflow of ADS_NOTIFICATION_SAMPLE_MAX_BYTES: " + ADS_NOTIFICATION_SAMPLE_MAX_BYTES + ". Actual " + sampleSize.getAsLong() + "bytes."); + throw new AdsProtocolOverflowException("ADS_NOTIFICATION_SAMPLE_MAX_BYTES", ADS_NOTIFICATION_SAMPLE_MAX_BYTES, sampleSize.getAsLong()); } // TODO: do we need a special marker class for: Notice: If your handle becomes invalid, one notification without data will be send once as advice. byte[] dataToRead = new byte[(int) sampleSize.getAsLong()]; @@ -340,17 +341,17 @@ public class Ads2PayloadProtocol extends MessageToMessageCodec<ByteBuf, AmsPacke IndexOffset indexOffset = IndexOffset.of(commandBuffer); ReadLength readLength = ReadLength.of(commandBuffer); if (readLength.getAsLong() > Integer.MAX_VALUE) { - throw new IllegalStateException("Overflow in datalength: " + readLength.getAsLong()); + throw new AdsProtocolOverflowException(Integer.class, readLength.getAsLong()); } WriteLength writeLength = WriteLength.of(commandBuffer); if (writeLength.getAsLong() > Integer.MAX_VALUE) { - throw new IllegalStateException("Overflow in datalength: " + writeLength.getAsLong()); + throw new AdsProtocolOverflowException(Integer.class, writeLength.getAsLong()); } if (readLength.getAsLong() + writeLength.getAsLong() > Integer.MAX_VALUE) { - throw new IllegalStateException("Overflow in datalength: " + readLength.getAsLong() + writeLength.getAsLong()); + throw new AdsProtocolOverflowException(Integer.class, readLength.getAsLong() + writeLength.getAsLong()); } if (readLength.getAsLong() > ADS_READ_WRITE_COMMAND_REQUEST_MAX_BYTES) { - throw new IllegalStateException("Overflow of ADS_READ_WRITE_COMMAND_REQUEST_MAX_BYTES: " + ADS_READ_WRITE_COMMAND_REQUEST_MAX_BYTES + ". Actual " + readLength.getAsLong() + "bytes."); + throw new AdsProtocolOverflowException("ADS_READ_WRITE_COMMAND_REQUEST_MAX_BYTES", ADS_READ_WRITE_COMMAND_REQUEST_MAX_BYTES, readLength.getAsLong()); } byte[] dataToRead = new byte[(int) readLength.getAsLong()]; commandBuffer.readBytes(dataToRead); @@ -360,10 +361,10 @@ public class Ads2PayloadProtocol extends MessageToMessageCodec<ByteBuf, AmsPacke Result result = Result.of(commandBuffer); Length length = Length.of(commandBuffer); if (length.getAsLong() > Integer.MAX_VALUE) { - throw new IllegalStateException("Overflow in datalength: " + length.getAsLong()); + throw new AdsProtocolOverflowException(Integer.class, length.getAsLong()); } if (length.getAsLong() > ADS_READ_WRITE_COMMAND_RESPONSE_MAX_BYTES) { - throw new IllegalStateException("Overflow of ADS_READ_WRITE_COMMAND_RESPONSE_MAX_BYTES: " + ADS_READ_WRITE_COMMAND_RESPONSE_MAX_BYTES + ". Actual " + length.getAsLong() + "bytes."); + throw new AdsProtocolOverflowException("ADS_READ_WRITE_COMMAND_RESPONSE_MAX_BYTES", ADS_READ_WRITE_COMMAND_RESPONSE_MAX_BYTES, length.getAsLong()); } byte[] dataToRead = new byte[(int) length.getAsLong()]; commandBuffer.readBytes(dataToRead); diff --git a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/protocol/Payload2SerialProtocol.java b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/protocol/Payload2SerialProtocol.java index 41dfe4f..54cba1b 100644 --- a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/protocol/Payload2SerialProtocol.java +++ b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/protocol/Payload2SerialProtocol.java @@ -148,6 +148,8 @@ public class Payload2SerialProtocol extends MessageToMessageCodec<ByteBuf, ByteB LOGGER.debug("Ams Serial Reset Frame received {}", amsSerialResetFrame); ReferenceCountUtil.release(byteBuf); break; + default: + throw new PlcProtocolException("Unknown type: " + magicCookie); } CRC calculatedCrc = CRC.of(DigestUtil.calculateCrc16(magicCookie, transmitterAddress, receiverAddress, fragmentNumber, userDataLength, userData)); if (!crc.equals(calculatedCrc)) { 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 fc67896..c67f455 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 @@ -283,7 +283,7 @@ public class Plc4x2AdsProtocol extends MessageToMessageCodec<AmsPacket, PlcReque .stream() .collect(Collectors.toMap( fieldName -> fieldName, - __ -> responseCode + ignore -> responseCode )); return new DefaultPlcWriteResponse(plcWriteRequest, responseItems); } @@ -304,7 +304,7 @@ public class Plc4x2AdsProtocol extends MessageToMessageCodec<AmsPacket, PlcReque .stream() .collect(Collectors.toMap( fieldName -> fieldName, - __ -> Pair.of(responseCode, fieldItem) + ignore -> Pair.of(responseCode, fieldItem) )); return new DefaultPlcReadResponse(plcReadRequest, responseItems); diff --git a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/model/AdsField.java b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/protocol/exception/AdsProtocolOverflowException.java similarity index 58% copy from plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/model/AdsField.java copy to plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/protocol/exception/AdsProtocolOverflowException.java index cab080b..10f330a 100644 --- a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/model/AdsField.java +++ b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/protocol/exception/AdsProtocolOverflowException.java @@ -16,10 +16,17 @@ specific language governing permissions and limitations under the License. */ -package org.apache.plc4x.java.ads.model; -import org.apache.plc4x.java.api.model.PlcField; +package org.apache.plc4x.java.ads.protocol.exception; -public interface AdsField extends PlcField { - AdsDataType getAdsDataType(); +import org.apache.plc4x.java.api.exceptions.PlcRuntimeException; + +public class AdsProtocolOverflowException extends PlcRuntimeException { + public AdsProtocolOverflowException(Class<?> clazz, long length) { + super("Overflow in datatype " + clazz + " length: " + length); + } + + public AdsProtocolOverflowException(String constantName, long expectedLength, long actualLength) { + super("Overflow of " + constantName + ": " + expectedLength + ". Actual " + actualLength + "bytes."); + } } diff --git a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/protocol/util/DigestUtil.java b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/protocol/util/DigestUtil.java index 6305a31..9971ebb 100644 --- a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/protocol/util/DigestUtil.java +++ b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/protocol/util/DigestUtil.java @@ -33,7 +33,7 @@ public class DigestUtil { CRC16.isReflectOut(), CRC16.getFinalXor()); - private static CRC crc16 = new CRC(CRC16_ADS); + private static CRC crc = new CRC(CRC16_ADS); private DigestUtil() { // Utility class @@ -43,16 +43,16 @@ public class DigestUtil { if (byteReadables.length == 1) { return calculateCrc16(byteReadables[0].getBytes()); } - long currentCrcValue = crc16.init(); + long currentCrcValue = crc.init(); for (ByteReadable byteReadable : byteReadables) { - currentCrcValue = crc16.update(currentCrcValue, byteReadable.getBytes()); + currentCrcValue = crc.update(currentCrcValue, byteReadable.getBytes()); } - short finalCrc = crc16.finalCRC16(currentCrcValue); + short finalCrc = crc.finalCRC16(currentCrcValue); return Short.toUnsignedInt(Short.reverseBytes(finalCrc)); } public static int calculateCrc16(byte[] bytes) { - short finalCrc = (short) crc16.calculateCRC(bytes); + short finalCrc = (short) crc.calculateCRC(bytes); return Short.toUnsignedInt(Short.reverseBytes(finalCrc)); } diff --git a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/protocol/util/LittleEndianDecoder.java b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/protocol/util/LittleEndianDecoder.java index e09b508..235760e 100644 --- a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/protocol/util/LittleEndianDecoder.java +++ b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/protocol/util/LittleEndianDecoder.java @@ -42,7 +42,7 @@ public class LittleEndianDecoder { } @SuppressWarnings("unchecked") - public static FieldItem<?> decodeData(AdsDataType adsDataType, byte[] adsData) { + public static FieldItem decodeData(AdsDataType adsDataType, byte[] adsData) { ByteBuf wrappedBuffer = Unpooled.wrappedBuffer(adsData); switch (adsDataType) { case BIT: { diff --git a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/protocol/util/SingleMessageRateLimiter.java b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/protocol/util/SingleMessageRateLimiter.java index 7b159a5..1f33c3b 100644 --- a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/protocol/util/SingleMessageRateLimiter.java +++ b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/protocol/util/SingleMessageRateLimiter.java @@ -63,7 +63,7 @@ public class SingleMessageRateLimiter extends ChannelDuplexHandler { if (!messagesQueue.isEmpty() && messageOnTheWay.compareAndSet(false, true)) { ToSend pop = messagesQueue.pop(); LOGGER.debug("Sending {}", pop); - pop.channelHandlerContext.writeAndFlush(pop.toSend, pop.promise); + pop.channelHandlerContext.writeAndFlush(pop.objectToSend, pop.promise); LOGGER.debug("Send {}", pop); } }, 100, 10, TimeUnit.MILLISECONDS); @@ -103,12 +103,12 @@ public class SingleMessageRateLimiter extends ChannelDuplexHandler { private static final class ToSend { final ChannelHandlerContext channelHandlerContext; - final Object toSend; + final Object objectToSend; final ChannelPromise promise; - private ToSend(ChannelHandlerContext channelHandlerContext, Object toSend, ChannelPromise promise) { + private ToSend(ChannelHandlerContext channelHandlerContext, Object objectToSend, ChannelPromise promise) { this.channelHandlerContext = channelHandlerContext; - this.toSend = toSend; + this.objectToSend = objectToSend; this.promise = promise; } @@ -116,7 +116,7 @@ public class SingleMessageRateLimiter extends ChannelDuplexHandler { public String toString() { return "ToSend{" + "channelHandlerContext=" + channelHandlerContext + - ", toSend=" + toSend + + ", objectToSend=" + objectToSend + ", promise=" + promise + '}'; }