This is an automated email from the ASF dual-hosted git repository. cdutz pushed a commit to branch feature/alternate-PLC4X-108 in repository https://gitbox.apache.org/repos/asf/incubator-plc4x.git
commit 14c408bfb57210c07e8d978a879961699c917e48 Author: Christofer Dutz <[email protected]> AuthorDate: Mon Apr 1 16:41:55 2019 +0200 PLC4X-108 - Add ping() method to PlcConnection - Renamed the PlcConnection "parse" method to "prepareField" - Added a "ping" method to the PlcConnection (which returns a CompletableFuture<Void>) - Removed the getInetSocketAddress method --- .../org/apache/plc4x/java/api/PlcConnection.java | 11 ++++- .../PlcUnsupportedProtocolException.java | 36 ++++++++++++++ .../plc4x/java/s7/connection/S7PlcConnection.java | 4 +- .../java/s7/connection/S7PlcConnectionTests.java | 8 ++-- .../connection/SimulatedPlcConnection.java | 7 --- .../base/connection/AbstractPlcConnection.java | 55 ++++------------------ .../plc4x/java/base/connection/ChannelFactory.java | 8 ---- .../java/base/connection/NettyPlcConnection.java | 19 ++------ .../base/connection/AbstractPlcConnectionTest.java | 8 ---- .../base/connection/NettyPlcConnectionTest.java | 7 --- .../base/connection/RawSocketChannelFactory.java | 7 --- .../java/base/connection/SerialChannelFactory.java | 8 ---- .../base/connection/TcpSocketChannelFactory.java | 6 --- .../java/base/connection/TestChannelFactory.java | 7 --- .../dynamic/s7/connection/DynamicS7Connection.java | 10 +--- 15 files changed, 64 insertions(+), 137 deletions(-) diff --git a/plc4j/api/src/main/java/org/apache/plc4x/java/api/PlcConnection.java b/plc4j/api/src/main/java/org/apache/plc4x/java/api/PlcConnection.java index e7d9a83..3838594 100644 --- a/plc4j/api/src/main/java/org/apache/plc4x/java/api/PlcConnection.java +++ b/plc4j/api/src/main/java/org/apache/plc4x/java/api/PlcConnection.java @@ -29,6 +29,8 @@ import org.apache.plc4x.java.api.messages.PlcWriteRequest; import org.apache.plc4x.java.api.metadata.PlcConnectionMetadata; import org.apache.plc4x.java.api.model.PlcField; +import java.util.concurrent.CompletableFuture; + /** * Interface defining the most basic methods a PLC4X connection should support. * This generally handles the connection establishment itself and the parsing of @@ -60,7 +62,7 @@ public interface PlcConnection extends AutoCloseable { * * @throws PlcRuntimeException If the string cannot be parsed */ - default PlcField parse(String fieldQuery) throws PlcInvalidFieldException { + default PlcField prepareField(String fieldQuery) throws PlcInvalidFieldException { throw new PlcRuntimeException("Parse method is not implemented for this connection / driver"); } @@ -70,6 +72,13 @@ public interface PlcConnection extends AutoCloseable { PlcConnectionMetadata getMetadata(); /** + * Execute a ping query against a remote device to check the availability of the connection. + * + * @return CompletableFuture that is completed successfully (Void) or unsuccessfully with an PlcException. + */ + CompletableFuture<Void> ping(); + + /** * Obtain read request builder. * @throws PlcUnsupportedOperationException if the connection does not support reading */ diff --git a/plc4j/api/src/main/java/org/apache/plc4x/java/api/exceptions/PlcUnsupportedProtocolException.java b/plc4j/api/src/main/java/org/apache/plc4x/java/api/exceptions/PlcUnsupportedProtocolException.java new file mode 100644 index 0000000..f897d75 --- /dev/null +++ b/plc4j/api/src/main/java/org/apache/plc4x/java/api/exceptions/PlcUnsupportedProtocolException.java @@ -0,0 +1,36 @@ +/* + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, + software distributed under the License is distributed on an + "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + KIND, either express or implied. See the License for the + specific language governing permissions and limitations + under the License. + */ + +package org.apache.plc4x.java.api.exceptions; + +public class PlcUnsupportedProtocolException extends PlcConnectionException { + + public PlcUnsupportedProtocolException(String message) { + super(message); + } + + public PlcUnsupportedProtocolException(String message, Throwable cause) { + super(message, cause); + } + + public PlcUnsupportedProtocolException(Throwable cause) { + super(cause); + } + +} diff --git a/plc4j/drivers/s7/src/main/java/org/apache/plc4x/java/s7/connection/S7PlcConnection.java b/plc4j/drivers/s7/src/main/java/org/apache/plc4x/java/s7/connection/S7PlcConnection.java index e3e9aec..52ab5dd 100644 --- a/plc4j/drivers/s7/src/main/java/org/apache/plc4x/java/s7/connection/S7PlcConnection.java +++ b/plc4j/drivers/s7/src/main/java/org/apache/plc4x/java/s7/connection/S7PlcConnection.java @@ -25,7 +25,6 @@ import org.apache.commons.configuration2.SystemConfiguration; import org.apache.commons.lang3.StringUtils; import org.apache.plc4x.java.api.exceptions.PlcConnectionException; import org.apache.plc4x.java.api.exceptions.PlcInvalidFieldException; -import org.apache.plc4x.java.api.exceptions.PlcRuntimeException; import org.apache.plc4x.java.api.messages.PlcReadRequest; import org.apache.plc4x.java.api.messages.PlcReadResponse; import org.apache.plc4x.java.api.messages.PlcWriteRequest; @@ -204,7 +203,8 @@ public class S7PlcConnection extends NettyPlcConnection implements PlcReader, Pl channel.pipeline().fireUserEventTriggered(new ConnectEvent()); } - @Override public PlcField parse(String fieldQuery) throws PlcInvalidFieldException { + @Override + public PlcField prepareField(String fieldQuery) throws PlcInvalidFieldException { return S7Field.of(fieldQuery); } diff --git a/plc4j/drivers/s7/src/test/java/org/apache/plc4x/java/s7/connection/S7PlcConnectionTests.java b/plc4j/drivers/s7/src/test/java/org/apache/plc4x/java/s7/connection/S7PlcConnectionTests.java index c0d938d..cb28cb0 100644 --- a/plc4j/drivers/s7/src/test/java/org/apache/plc4x/java/s7/connection/S7PlcConnectionTests.java +++ b/plc4j/drivers/s7/src/test/java/org/apache/plc4x/java/s7/connection/S7PlcConnectionTests.java @@ -80,14 +80,14 @@ public class S7PlcConnectionTests { } @Test - public void parse() { - final PlcField field = SUT.parse("%DB1.DBX38.1:BOOL"); + public void prepareField() { + final PlcField field = SUT.prepareField("%DB1.DBX38.1:BOOL"); assertThat(field.getClass(), equalTo(S7Field.class)); assertThat(((S7Field) field).getDataType(), equalTo(TransportSize.BOOL)); } @Test(expected = PlcInvalidFieldException.class) - public void parseFails() { - SUT.parse("this is a bad field query"); + public void prepareFieldFails() { + SUT.prepareField("this is a bad field query"); } } \ No newline at end of file diff --git a/plc4j/drivers/simulated/src/main/java/org/apache/plc4x/java/simulated/connection/SimulatedPlcConnection.java b/plc4j/drivers/simulated/src/main/java/org/apache/plc4x/java/simulated/connection/SimulatedPlcConnection.java index 07fc20f..805d9af 100644 --- a/plc4j/drivers/simulated/src/main/java/org/apache/plc4x/java/simulated/connection/SimulatedPlcConnection.java +++ b/plc4j/drivers/simulated/src/main/java/org/apache/plc4x/java/simulated/connection/SimulatedPlcConnection.java @@ -29,7 +29,6 @@ import org.apache.plc4x.java.base.messages.*; import org.apache.plc4x.java.base.messages.items.BaseDefaultFieldItem; import org.apache.plc4x.java.base.model.*; -import java.net.InetSocketAddress; import java.time.Instant; import java.util.*; import java.util.concurrent.CompletableFuture; @@ -65,12 +64,6 @@ public class SimulatedPlcConnection extends AbstractPlcConnection implements Plc } @Override - protected Optional<InetSocketAddress> getInetSocketAddress() { - // IS okay, isConnected is overridden - return Optional.empty(); - } - - @Override public void close() { connected = false; } diff --git a/plc4j/protocols/driver-bases/base/src/main/java/org/apache/plc4x/java/base/connection/AbstractPlcConnection.java b/plc4j/protocols/driver-bases/base/src/main/java/org/apache/plc4x/java/base/connection/AbstractPlcConnection.java index ca08838..9f862d7 100644 --- a/plc4j/protocols/driver-bases/base/src/main/java/org/apache/plc4x/java/base/connection/AbstractPlcConnection.java +++ b/plc4j/protocols/driver-bases/base/src/main/java/org/apache/plc4x/java/base/connection/AbstractPlcConnection.java @@ -18,7 +18,6 @@ under the License. */ package org.apache.plc4x.java.base.connection; -import org.apache.commons.lang3.NotImplementedException; import org.apache.plc4x.java.api.PlcConnection; import org.apache.plc4x.java.api.exceptions.PlcUnsupportedOperationException; import org.apache.plc4x.java.api.messages.PlcReadRequest; @@ -27,13 +26,9 @@ import org.apache.plc4x.java.api.messages.PlcUnsubscriptionRequest; import org.apache.plc4x.java.api.messages.PlcWriteRequest; import org.apache.plc4x.java.api.metadata.PlcConnectionMetadata; import org.apache.plc4x.java.base.messages.InternalPlcMessage; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import java.net.InetSocketAddress; -import java.net.Socket; import java.util.Objects; -import java.util.Optional; +import java.util.concurrent.CompletableFuture; /** * Base class for implementing connections. @@ -43,16 +38,19 @@ import java.util.Optional; */ public abstract class AbstractPlcConnection implements PlcConnection, PlcConnectionMetadata { - private static final Logger LOGGER = LoggerFactory.getLogger(AbstractPlcConnection.class); - - private static final int PING_TIMEOUT_MS = 1_000; - @Override public PlcConnectionMetadata getMetadata() { return this; } @Override + public CompletableFuture<Void> ping() { + CompletableFuture<Void> future = new CompletableFuture<>(); + future.completeExceptionally(new PlcUnsupportedOperationException("The connection does not support pinging")); + return future; + } + + @Override public boolean canRead() { return false; } @@ -67,43 +65,6 @@ public abstract class AbstractPlcConnection implements PlcConnection, PlcConnect return false; } - /** - * The default implementation uses the {@link #ping(int)} method. - * Drivers that want to implement a more specific version have to overide it. - */ - @Override - public boolean isConnected() { - return ping(PING_TIMEOUT_MS); - } - - /** - * Method that sends a test-request or ping to the PLC to check if the PLC is still there. - * In most cases this method should only be used from the {@link #isConnected()} method. - * This method can only be used if ghe {@link #getInetSocketAddress()} returns a Socket Adress. - * Otherwise it throws a {@link NotImplementedException} to inform the user about that. - */ - protected boolean ping(int timeout) { - InetSocketAddress address = getInetSocketAddress() - .orElseThrow(() -> new NotImplementedException("Tries to check the connection with ping, " + - "but no Socket Address is given!")); - - try (Socket s = new Socket()) { - s.connect(address, timeout); - // TODO keep the adress for a (timely) next request??? - s.setReuseAddress(true); - return true; - } catch (Exception e) { - LOGGER.debug("Unable to ping PLC", e); - return false; - } - } - - /** - * Strategy Pattern method for the {@link #ping(int)} method. - * If a driver has no Inet Socket adress, it has to return an Empty Optional, never null. - */ - protected abstract Optional<InetSocketAddress> getInetSocketAddress(); - @Override public PlcReadRequest.Builder readRequestBuilder() { throw new PlcUnsupportedOperationException("The connection does not support reading"); diff --git a/plc4j/protocols/driver-bases/base/src/main/java/org/apache/plc4x/java/base/connection/ChannelFactory.java b/plc4j/protocols/driver-bases/base/src/main/java/org/apache/plc4x/java/base/connection/ChannelFactory.java index 55e245a..b3e3e6b 100644 --- a/plc4j/protocols/driver-bases/base/src/main/java/org/apache/plc4x/java/base/connection/ChannelFactory.java +++ b/plc4j/protocols/driver-bases/base/src/main/java/org/apache/plc4x/java/base/connection/ChannelFactory.java @@ -22,17 +22,9 @@ import io.netty.channel.Channel; import io.netty.channel.ChannelHandler; import org.apache.plc4x.java.api.exceptions.PlcConnectionException; -import java.net.InetSocketAddress; -import java.util.Optional; - public interface ChannelFactory { Channel createChannel(ChannelHandler channelHandler) throws PlcConnectionException; - /** - * If this Channel Facotry creates a Channel which has an ip / port it should return this - */ - Optional<InetSocketAddress> getInetSocketAddress(); - } diff --git a/plc4j/protocols/driver-bases/base/src/main/java/org/apache/plc4x/java/base/connection/NettyPlcConnection.java b/plc4j/protocols/driver-bases/base/src/main/java/org/apache/plc4x/java/base/connection/NettyPlcConnection.java index 9fc1223..42b370d 100644 --- a/plc4j/protocols/driver-bases/base/src/main/java/org/apache/plc4x/java/base/connection/NettyPlcConnection.java +++ b/plc4j/protocols/driver-bases/base/src/main/java/org/apache/plc4x/java/base/connection/NettyPlcConnection.java @@ -25,8 +25,6 @@ import io.netty.util.Timer; import org.apache.plc4x.java.api.exceptions.PlcConnectionException; import org.apache.plc4x.java.api.exceptions.PlcIoException; -import java.net.InetSocketAddress; -import java.util.Optional; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; @@ -97,23 +95,12 @@ public abstract class NettyPlcConnection extends AbstractPlcConnection { } /** - * If a InetSocketAdress is present, it uses the "ping" based default. - * Otherwise does the "old" approach to check nettys channel (e.g. serial). + * Check if the communication channel is active (channel.isActive()) and the driver for a given protocol + * has finished establishing the connection. */ @Override public boolean isConnected() { - // Use the "netty default" if no socket adress is present (like serial) - if (!getInetSocketAddress().isPresent()) { - return connected && channel.isActive(); - } else { - // TODO sr: Should we add a check like previously, if the channel is active? - return super.isConnected(); - } - } - - @Override - protected Optional<InetSocketAddress> getInetSocketAddress() { - return this.channelFactory.getInetSocketAddress(); + return connected && channel.isActive(); } public Channel getChannel() { diff --git a/plc4j/protocols/driver-bases/base/src/test/java/org/apache/plc4x/java/base/connection/AbstractPlcConnectionTest.java b/plc4j/protocols/driver-bases/base/src/test/java/org/apache/plc4x/java/base/connection/AbstractPlcConnectionTest.java index 0d2b098..2064709 100644 --- a/plc4j/protocols/driver-bases/base/src/test/java/org/apache/plc4x/java/base/connection/AbstractPlcConnectionTest.java +++ b/plc4j/protocols/driver-bases/base/src/test/java/org/apache/plc4x/java/base/connection/AbstractPlcConnectionTest.java @@ -26,9 +26,6 @@ import org.apache.plc4x.java.base.messages.PlcReader; import org.assertj.core.api.WithAssertions; import org.junit.jupiter.api.Test; -import java.net.InetSocketAddress; -import java.util.Optional; - import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.Mockito.mock; @@ -46,11 +43,6 @@ class AbstractPlcConnectionTest implements WithAssertions { } @Override - protected Optional<InetSocketAddress> getInetSocketAddress() { - return Optional.empty(); - } - - @Override public void close() { throw new NotImplementedException("not used"); } diff --git a/plc4j/protocols/driver-bases/base/src/test/java/org/apache/plc4x/java/base/connection/NettyPlcConnectionTest.java b/plc4j/protocols/driver-bases/base/src/test/java/org/apache/plc4x/java/base/connection/NettyPlcConnectionTest.java index 78269c3..8fe40ea 100644 --- a/plc4j/protocols/driver-bases/base/src/test/java/org/apache/plc4x/java/base/connection/NettyPlcConnectionTest.java +++ b/plc4j/protocols/driver-bases/base/src/test/java/org/apache/plc4x/java/base/connection/NettyPlcConnectionTest.java @@ -28,8 +28,6 @@ import org.apache.plc4x.java.base.events.ConnectEvent; import org.assertj.core.api.WithAssertions; import org.junit.jupiter.api.Test; -import java.net.InetSocketAddress; -import java.util.Optional; import java.util.concurrent.CompletableFuture; @@ -40,11 +38,6 @@ public class NettyPlcConnectionTest implements WithAssertions { public Channel createChannel(ChannelHandler channelHandler) throws PlcConnectionException { return new EmbeddedChannel(); } - - @Override - public Optional<InetSocketAddress> getInetSocketAddress() { - return Optional.empty(); - } }; NettyPlcConnection SUT = new NettyPlcConnection(channelFactory, true) { diff --git a/plc4j/protocols/driver-bases/raw-socket/src/main/java/org/apache/plc4x/java/base/connection/RawSocketChannelFactory.java b/plc4j/protocols/driver-bases/raw-socket/src/main/java/org/apache/plc4x/java/base/connection/RawSocketChannelFactory.java index fb77caf..a6fe95e 100644 --- a/plc4j/protocols/driver-bases/raw-socket/src/main/java/org/apache/plc4x/java/base/connection/RawSocketChannelFactory.java +++ b/plc4j/protocols/driver-bases/raw-socket/src/main/java/org/apache/plc4x/java/base/connection/RawSocketChannelFactory.java @@ -23,8 +23,6 @@ import io.netty.channel.ChannelHandler; import org.apache.plc4x.java.api.exceptions.PlcConnectionException; import java.net.InetAddress; -import java.net.InetSocketAddress; -import java.util.Optional; public class RawSocketChannelFactory implements ChannelFactory { @@ -39,11 +37,6 @@ public class RawSocketChannelFactory implements ChannelFactory { } @Override - public Optional<InetSocketAddress> getInetSocketAddress() { - return Optional.of(new InetSocketAddress(address, port)); - } - - @Override public Channel createChannel(ChannelHandler channelHandler) throws PlcConnectionException { /*try { diff --git a/plc4j/protocols/driver-bases/serial/src/main/java/org/apache/plc4x/java/base/connection/SerialChannelFactory.java b/plc4j/protocols/driver-bases/serial/src/main/java/org/apache/plc4x/java/base/connection/SerialChannelFactory.java index f95f618..cdfea8f 100644 --- a/plc4j/protocols/driver-bases/serial/src/main/java/org/apache/plc4x/java/base/connection/SerialChannelFactory.java +++ b/plc4j/protocols/driver-bases/serial/src/main/java/org/apache/plc4x/java/base/connection/SerialChannelFactory.java @@ -28,9 +28,6 @@ import io.netty.channel.jsc.JSerialCommDeviceAddress; import io.netty.channel.oio.OioEventLoopGroup; import org.apache.plc4x.java.api.exceptions.PlcConnectionException; -import java.net.InetSocketAddress; -import java.util.Optional; - public class SerialChannelFactory implements ChannelFactory { private final String serialPort; @@ -40,11 +37,6 @@ public class SerialChannelFactory implements ChannelFactory { } @Override - public Optional<InetSocketAddress> getInetSocketAddress() { - return Optional.empty(); - } - - @Override public Channel createChannel(ChannelHandler channelHandler) throws PlcConnectionException { JSerialCommDeviceAddress address = new JSerialCommDeviceAddress(serialPort); diff --git a/plc4j/protocols/driver-bases/tcp/src/main/java/org/apache/plc4x/java/base/connection/TcpSocketChannelFactory.java b/plc4j/protocols/driver-bases/tcp/src/main/java/org/apache/plc4x/java/base/connection/TcpSocketChannelFactory.java index cc77358..44e4835 100644 --- a/plc4j/protocols/driver-bases/tcp/src/main/java/org/apache/plc4x/java/base/connection/TcpSocketChannelFactory.java +++ b/plc4j/protocols/driver-bases/tcp/src/main/java/org/apache/plc4x/java/base/connection/TcpSocketChannelFactory.java @@ -28,8 +28,6 @@ import io.netty.channel.socket.nio.NioSocketChannel; import org.apache.plc4x.java.api.exceptions.PlcConnectionException; import java.net.InetAddress; -import java.net.InetSocketAddress; -import java.util.Optional; public class TcpSocketChannelFactory implements ChannelFactory { @@ -72,8 +70,4 @@ public class TcpSocketChannelFactory implements ChannelFactory { return port; } - @Override - public Optional<InetSocketAddress> getInetSocketAddress() { - return Optional.of(new InetSocketAddress(address, port)); - } } diff --git a/plc4j/protocols/driver-bases/test/src/main/java/org/apache/plc4x/java/base/connection/TestChannelFactory.java b/plc4j/protocols/driver-bases/test/src/main/java/org/apache/plc4x/java/base/connection/TestChannelFactory.java index ec27290..be58968 100644 --- a/plc4j/protocols/driver-bases/test/src/main/java/org/apache/plc4x/java/base/connection/TestChannelFactory.java +++ b/plc4j/protocols/driver-bases/test/src/main/java/org/apache/plc4x/java/base/connection/TestChannelFactory.java @@ -22,9 +22,6 @@ import io.netty.channel.Channel; import io.netty.channel.ChannelHandler; import io.netty.channel.embedded.EmbeddedChannel; -import java.net.InetSocketAddress; -import java.util.Optional; - public class TestChannelFactory implements ChannelFactory { private EmbeddedChannel channel; @@ -39,8 +36,4 @@ public class TestChannelFactory implements ChannelFactory { return channel; } - @Override - public Optional<InetSocketAddress> getInetSocketAddress() { - return Optional.empty(); - } } diff --git a/sandbox/dynamic-driver-s7/src/main/java/org/apache/plc4x/sandbox/java/dynamic/s7/connection/DynamicS7Connection.java b/sandbox/dynamic-driver-s7/src/main/java/org/apache/plc4x/sandbox/java/dynamic/s7/connection/DynamicS7Connection.java index ed1a595..2138358 100644 --- a/sandbox/dynamic-driver-s7/src/main/java/org/apache/plc4x/sandbox/java/dynamic/s7/connection/DynamicS7Connection.java +++ b/sandbox/dynamic-driver-s7/src/main/java/org/apache/plc4x/sandbox/java/dynamic/s7/connection/DynamicS7Connection.java @@ -39,12 +39,10 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.net.InetAddress; -import java.net.InetSocketAddress; import java.util.Arrays; import java.util.Collection; import java.util.HashMap; import java.util.Map; -import java.util.Optional; import java.util.concurrent.CompletableFuture; public class DynamicS7Connection extends DynamicDriverConnectionBase implements PlcReader { @@ -59,7 +57,6 @@ public class DynamicS7Connection extends DynamicDriverConnectionBase implements private short paramMaxAmqCaller; private short paramMaxAmqCallee; private String paramControllerType; - private int PORT; public DynamicS7Connection(InetAddress address, int rack, int slot, String params) { super("org/apache/plc4x/protocols/s7/protocol.scxml.xml", @@ -127,11 +124,6 @@ public class DynamicS7Connection extends DynamicDriverConnectionBase implements return "disconnect"; } - @Override protected Optional<InetSocketAddress> getInetSocketAddress() { - PORT = 102; - return Optional.of(new InetSocketAddress(address, PORT)); - } - @Override protected Collection<CustomAction> getAdditionalCustomActions() { return Arrays.asList( @@ -149,7 +141,7 @@ public class DynamicS7Connection extends DynamicDriverConnectionBase implements Map<String, Object> dataItems = new HashMap<>(); dataItems.put("hostname", address.getHostAddress()); - dataItems.put("port", Integer.toString(PORT)); + dataItems.put("port", "102"); dataItems.put("plcType", paramControllerType); dataItems.put("cotpLocalReference", "15");
