You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@plc4x.apache.org by hu...@apache.org on 2021/01/10 10:17:48 UTC
[plc4x] 02/02: Tidied up disconnect logic
This is an automated email from the ASF dual-hosted git repository.
hutcheb pushed a commit to branch bug/close_connection
in repository https://gitbox.apache.org/repos/asf/plc4x.git
commit ba961c509868d0b1d9a572d4e3a18598af613bdc
Author: hutcheb <be...@gmail.com>
AuthorDate: Sun Jan 10 05:14:32 2021 -0500
Tidied up disconnect logic
Added system property to override option to wait for disconnection to occur.
Added reminder methods for each driver to explicitly override await disconnect option.
---
.../java/org/apache/plc4x/java/abeth/AbEthDriver.java | 9 +++++++++
.../java/org/apache/plc4x/java/ads/ADSPlcDriver.java | 9 +++++++++
.../apache/plc4x/java/canopen/CANOpenPlcDriver.java | 9 +++++++++
.../apache/plc4x/java/eip/readwrite/EIPDriver.java | 9 +++++++++
.../plc4x/java/firmata/readwrite/FirmataDriver.java | 9 +++++++++
.../apache/plc4x/java/knxnetip/KnxNetIpDriver.java | 3 +++
.../org/apache/plc4x/java/modbus/ModbusDriver.java | 9 +++++++++
.../org/apache/plc4x/java/s7/readwrite/S7Driver.java | 9 +++++++++
.../spi/connection/DefaultNettyPlcConnection.java | 19 ++++++++++++++-----
.../java/spi/connection/GeneratedDriverBase.java | 13 +++++++++++++
.../plc4x/test/driver/DriverTestsuiteRunner.java | 3 +++
11 files changed, 96 insertions(+), 5 deletions(-)
diff --git a/plc4j/drivers/ab-eth/src/main/java/org/apache/plc4x/java/abeth/AbEthDriver.java b/plc4j/drivers/ab-eth/src/main/java/org/apache/plc4x/java/abeth/AbEthDriver.java
index 0a98a0b..d12b791 100644
--- a/plc4j/drivers/ab-eth/src/main/java/org/apache/plc4x/java/abeth/AbEthDriver.java
+++ b/plc4j/drivers/ab-eth/src/main/java/org/apache/plc4x/java/abeth/AbEthDriver.java
@@ -70,6 +70,15 @@ public class AbEthDriver extends GeneratedDriverBase<CIPEncapsulationPacket> {
return new IEC61131ValueHandler();
}
+ /**
+ * This protocol doesn't have a disconnect procedure, so there is no need to wait for a login to finish.
+ * @return false
+ */
+ @Override
+ protected boolean awaitDisconnectComplete() {
+ return false;
+ }
+
@Override
protected ProtocolStackConfigurer<CIPEncapsulationPacket> getStackConfigurer() {
return SingleProtocolStackConfigurer.builder(CIPEncapsulationPacket.class, CIPEncapsulationPacketIO.class)
diff --git a/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/ADSPlcDriver.java b/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/ADSPlcDriver.java
index fd5f25c..9161ce4 100644
--- a/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/ADSPlcDriver.java
+++ b/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/ADSPlcDriver.java
@@ -83,6 +83,15 @@ public class ADSPlcDriver extends GeneratedDriverBase<AmsTCPPacket> {
return new IEC61131ValueHandler();
}
+ /**
+ * This protocol doesn't have a disconnect procedure, so there is no need to wait for a login to finish.
+ * @return false
+ */
+ @Override
+ protected boolean awaitDisconnectComplete() {
+ return false;
+ }
+
@Override
protected ProtocolStackConfigurer<AmsTCPPacket> getStackConfigurer() {
return SingleProtocolStackConfigurer.builder(AmsTCPPacket.class, AmsTCPPacketIO.class)
diff --git a/plc4j/drivers/canopen/src/main/java/org/apache/plc4x/java/canopen/CANOpenPlcDriver.java b/plc4j/drivers/canopen/src/main/java/org/apache/plc4x/java/canopen/CANOpenPlcDriver.java
index 6db7d94..c82d8f2 100644
--- a/plc4j/drivers/canopen/src/main/java/org/apache/plc4x/java/canopen/CANOpenPlcDriver.java
+++ b/plc4j/drivers/canopen/src/main/java/org/apache/plc4x/java/canopen/CANOpenPlcDriver.java
@@ -96,6 +96,15 @@ public class CANOpenPlcDriver extends GeneratedDriverBase<CANOpenFrame> {
};
}
+ /**
+ * This protocol doesn't have a disconnect procedure, so there is no need to wait for a login to finish.
+ * @return false
+ */
+ @Override
+ protected boolean awaitDisconnectComplete() {
+ return false;
+ }
+
@Override
protected BaseOptimizer getOptimizer() {
return new SingleFieldOptimizer();
diff --git a/plc4j/drivers/eip/src/main/java/org/apache/plc4x/java/eip/readwrite/EIPDriver.java b/plc4j/drivers/eip/src/main/java/org/apache/plc4x/java/eip/readwrite/EIPDriver.java
index 799f332..3fcb65d 100644
--- a/plc4j/drivers/eip/src/main/java/org/apache/plc4x/java/eip/readwrite/EIPDriver.java
+++ b/plc4j/drivers/eip/src/main/java/org/apache/plc4x/java/eip/readwrite/EIPDriver.java
@@ -62,6 +62,15 @@ public class EIPDriver extends GeneratedDriverBase<EipPacket> {
return new IEC61131ValueHandler();
}
+ /**
+ * This protocol doesn't have a disconnect procedure, so there is no need to wait for a login to finish.
+ * @return false
+ */
+ @Override
+ protected boolean awaitDisconnectComplete() {
+ return false;
+ }
+
@Override
protected String getDefaultTransport() {
return "tcp";
diff --git a/plc4j/drivers/firmata/src/main/java/org/apache/plc4x/java/firmata/readwrite/FirmataDriver.java b/plc4j/drivers/firmata/src/main/java/org/apache/plc4x/java/firmata/readwrite/FirmataDriver.java
index f5a396a..923d7cc 100644
--- a/plc4j/drivers/firmata/src/main/java/org/apache/plc4x/java/firmata/readwrite/FirmataDriver.java
+++ b/plc4j/drivers/firmata/src/main/java/org/apache/plc4x/java/firmata/readwrite/FirmataDriver.java
@@ -79,6 +79,15 @@ public class FirmataDriver extends GeneratedDriverBase<FirmataMessage> {
return new IEC61131ValueHandler();
}
+ /**
+ * This protocol doesn't have a disconnect procedure, so there is no need to wait for a login to finish.
+ * @return false
+ */
+ @Override
+ protected boolean awaitDisconnectComplete() {
+ return false;
+ }
+
@Override
protected ProtocolStackConfigurer<FirmataMessage> getStackConfigurer() {
return SingleProtocolStackConfigurer.builder(FirmataMessage.class, FirmataMessageIO.class)
diff --git a/plc4j/drivers/knxnetip/src/main/java/org/apache/plc4x/java/knxnetip/KnxNetIpDriver.java b/plc4j/drivers/knxnetip/src/main/java/org/apache/plc4x/java/knxnetip/KnxNetIpDriver.java
index d0c8994..e1a5348 100644
--- a/plc4j/drivers/knxnetip/src/main/java/org/apache/plc4x/java/knxnetip/KnxNetIpDriver.java
+++ b/plc4j/drivers/knxnetip/src/main/java/org/apache/plc4x/java/knxnetip/KnxNetIpDriver.java
@@ -93,6 +93,9 @@ public class KnxNetIpDriver extends GeneratedDriverBase<KnxNetIpMessage> {
}
@Override
+ protected boolean awaitDisconnectComplete() { return true; }
+
+ @Override
protected ProtocolStackConfigurer<KnxNetIpMessage> getStackConfigurer() {
return SingleProtocolStackConfigurer.builder(KnxNetIpMessage.class, KnxNetIpMessageIO.class)
.withProtocol(KnxNetIpProtocolLogic.class)
diff --git a/plc4j/drivers/modbus/src/main/java/org/apache/plc4x/java/modbus/ModbusDriver.java b/plc4j/drivers/modbus/src/main/java/org/apache/plc4x/java/modbus/ModbusDriver.java
index d833543..673ca18 100644
--- a/plc4j/drivers/modbus/src/main/java/org/apache/plc4x/java/modbus/ModbusDriver.java
+++ b/plc4j/drivers/modbus/src/main/java/org/apache/plc4x/java/modbus/ModbusDriver.java
@@ -67,6 +67,15 @@ public class ModbusDriver extends GeneratedDriverBase<ModbusTcpADU> {
return false;
}
+ /**
+ * This protocol doesn't have a disconnect procedure, so there is no need to wait for a login to finish.
+ * @return false
+ */
+ @Override
+ protected boolean awaitDisconnectComplete() {
+ return false;
+ }
+
@Override
protected boolean canRead() {
return true;
diff --git a/plc4j/drivers/s7/src/main/java/org/apache/plc4x/java/s7/readwrite/S7Driver.java b/plc4j/drivers/s7/src/main/java/org/apache/plc4x/java/s7/readwrite/S7Driver.java
index 26b39c6..dd90664 100644
--- a/plc4j/drivers/s7/src/main/java/org/apache/plc4x/java/s7/readwrite/S7Driver.java
+++ b/plc4j/drivers/s7/src/main/java/org/apache/plc4x/java/s7/readwrite/S7Driver.java
@@ -86,6 +86,15 @@ public class S7Driver extends GeneratedDriverBase<TPKTPacket> {
return new IEC61131ValueHandler();
}
+ /**
+ * This protocol doesn't have a disconnect procedure, so there is no need to wait for a login to finish.
+ * @return false
+ */
+ @Override
+ protected boolean awaitDisconnectComplete() {
+ return false;
+ }
+
@Override
protected ProtocolStackConfigurer<TPKTPacket> getStackConfigurer() {
return SingleProtocolStackConfigurer.builder(TPKTPacket.class, TPKTPacketIO.class)
diff --git a/plc4j/spi/src/main/java/org/apache/plc4x/java/spi/connection/DefaultNettyPlcConnection.java b/plc4j/spi/src/main/java/org/apache/plc4x/java/spi/connection/DefaultNettyPlcConnection.java
index 1fc70ad..ce8d355 100644
--- a/plc4j/spi/src/main/java/org/apache/plc4x/java/spi/connection/DefaultNettyPlcConnection.java
+++ b/plc4j/spi/src/main/java/org/apache/plc4x/java/spi/connection/DefaultNettyPlcConnection.java
@@ -42,13 +42,15 @@ public class DefaultNettyPlcConnection extends AbstractPlcConnection implements
*/
// TODO: maybe find a way to make this configurable per jvm
protected final static Timer timer = new HashedWheelTimer();
+ protected final static long DEFAULT_DISCONNECT_WAIT_TIME = 10000L;
private static final Logger logger = LoggerFactory.getLogger(DefaultNettyPlcConnection.class);
protected final Configuration configuration;
protected final ChannelFactory channelFactory;
protected final boolean awaitSessionSetupComplete;
+ protected final boolean awaitSessionDisconnectComplete;
protected final ProtocolStackConfigurer stackConfigurer;
- private final CompletableFuture<Void> sessionDisconnectCompleteFuture = new CompletableFuture<>();
+ protected final CompletableFuture<Void> sessionDisconnectCompleteFuture = new CompletableFuture<>();
protected Channel channel;
protected boolean connected;
@@ -56,11 +58,12 @@ public class DefaultNettyPlcConnection extends AbstractPlcConnection implements
public DefaultNettyPlcConnection(boolean canRead, boolean canWrite, boolean canSubscribe,
PlcFieldHandler fieldHandler, PlcValueHandler valueHandler, Configuration configuration,
ChannelFactory channelFactory, boolean awaitSessionSetupComplete,
- ProtocolStackConfigurer stackConfigurer, BaseOptimizer optimizer) {
+ boolean awaitSessionDisconnectComplete, ProtocolStackConfigurer stackConfigurer, BaseOptimizer optimizer) {
super(canRead, canWrite, canSubscribe, fieldHandler, valueHandler, optimizer);
this.configuration = configuration;
this.channelFactory = channelFactory;
this.awaitSessionSetupComplete = awaitSessionSetupComplete;
+ this.awaitSessionDisconnectComplete = awaitSessionDisconnectComplete;
this.stackConfigurer = stackConfigurer;
this.connected = false;
@@ -109,17 +112,23 @@ public class DefaultNettyPlcConnection extends AbstractPlcConnection implements
@Override
public void close() throws PlcConnectionException {
- // TODO call protocols close method
channel.pipeline().fireUserEventTriggered(new DisconnectEvent());
try {
- sessionDisconnectCompleteFuture.get(10000L, TimeUnit.MILLISECONDS);
+ if (awaitSessionDisconnectComplete) {
+ sessionDisconnectCompleteFuture.get(DEFAULT_DISCONNECT_WAIT_TIME, TimeUnit.MILLISECONDS);
+ }
} catch (Exception e) {
- //Do Nothing
+ logger.error("Timeout while trying to close connection");
}
channel.pipeline().fireUserEventTriggered(new CloseConnectionEvent());
channel.close().awaitUninterruptibly();
+
+ if (!sessionDisconnectCompleteFuture.isDone()) {
+ sessionDisconnectCompleteFuture.complete(null );
+ }
+
channel = null;
connected = false;
}
diff --git a/plc4j/spi/src/main/java/org/apache/plc4x/java/spi/connection/GeneratedDriverBase.java b/plc4j/spi/src/main/java/org/apache/plc4x/java/spi/connection/GeneratedDriverBase.java
index fe43179..6c58d71 100644
--- a/plc4j/spi/src/main/java/org/apache/plc4x/java/spi/connection/GeneratedDriverBase.java
+++ b/plc4j/spi/src/main/java/org/apache/plc4x/java/spi/connection/GeneratedDriverBase.java
@@ -40,6 +40,8 @@ public abstract class GeneratedDriverBase<BASE_PACKET extends Message> implement
public static final String PROPERTY_PLC4X_FORCE_AWAIT_SETUP_COMPLETE = "PLC4X_FORCE_AWAIT_SETUP_COMPLETE";
+ public static final String PROPERTY_PLC4X_FORCE_AWAIT_DISCONNECT_COMPLETE = "PLC4X_FORCE_AWAIT_DISCONNECT_COMPLETE";
+
private static final Pattern URI_PATTERN = Pattern.compile(
"^(?<protocolCode>[a-z0-9\\-]*)(:(?<transportCode>[a-z0-9]*))?://(?<transportConfig>[^?]*)(\\?(?<paramString>.*))?");
@@ -61,6 +63,10 @@ public abstract class GeneratedDriverBase<BASE_PACKET extends Message> implement
return true;
}
+ protected boolean awaitDisconnectComplete() {
+ return false;
+ }
+
protected BaseOptimizer getOptimizer() {
return null;
}
@@ -138,6 +144,12 @@ public abstract class GeneratedDriverBase<BASE_PACKET extends Message> implement
awaitSetupComplete = Boolean.parseBoolean(System.getProperty(PROPERTY_PLC4X_FORCE_AWAIT_SETUP_COMPLETE));
}
+ // Make the "await disconnect complete" overridable via system property.
+ boolean awaitDisconnectComplete = awaitDisconnectComplete();
+ if(System.getProperty(PROPERTY_PLC4X_FORCE_AWAIT_DISCONNECT_COMPLETE) != null) {
+ awaitDisconnectComplete = Boolean.parseBoolean(System.getProperty(PROPERTY_PLC4X_FORCE_AWAIT_DISCONNECT_COMPLETE));
+ }
+
return new DefaultNettyPlcConnection(
canRead(), canWrite(), canSubscribe(),
getFieldHandler(),
@@ -145,6 +157,7 @@ public abstract class GeneratedDriverBase<BASE_PACKET extends Message> implement
configuration,
channelFactory,
awaitSetupComplete,
+ awaitDisconnectComplete,
getStackConfigurer(),
getOptimizer());
}
diff --git a/plc4j/utils/test-utils/src/main/java/org/apache/plc4x/test/driver/DriverTestsuiteRunner.java b/plc4j/utils/test-utils/src/main/java/org/apache/plc4x/test/driver/DriverTestsuiteRunner.java
index 3638517..5258718 100644
--- a/plc4j/utils/test-utils/src/main/java/org/apache/plc4x/test/driver/DriverTestsuiteRunner.java
+++ b/plc4j/utils/test-utils/src/main/java/org/apache/plc4x/test/driver/DriverTestsuiteRunner.java
@@ -154,6 +154,9 @@ public class DriverTestsuiteRunner {
// Force the driver to not wait for the connection before returning the connection.
System.setProperty(GeneratedDriverBase.PROPERTY_PLC4X_FORCE_AWAIT_SETUP_COMPLETE, "false");
+ // Force the driver to not wait for the disconnection before returning closing the channel.
+ System.setProperty(GeneratedDriverBase.PROPERTY_PLC4X_FORCE_AWAIT_DISCONNECT_COMPLETE, "false");
+
TimeUnit.MILLISECONDS.sleep(200);
return new DriverTestsuite(testsuiteName, driverName, driverParameters, setupSteps, teardownSteps, testcases, bigEndian);