This is an automated email from the ASF dual-hosted git repository.
cdutz 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 f80c9ef Made the disconnect for S7 respect the ISO TP protocol.
f80c9ef is described below
commit f80c9ef8be8d163e0702f77b028977d310978606
Author: Christofer Dutz <christofer.d...@c-ware.de>
AuthorDate: Fri Aug 3 09:27:08 2018 +0200
Made the disconnect for S7 respect the ISO TP protocol.
---
.../ads/connection/AdsAbstractPlcConnection.java | 3 +-
.../apache/plc4x/java/ads/adslib/AmsRouter.java | 6 +++-
.../base/connection/AbstractPlcConnection.java | 2 +-
plc4j/protocols/s7/pom.xml | 4 +++
.../plc4x/java/s7/connection/S7PlcConnection.java | 38 +++++++++++++++++++---
.../java/s7/connection/S7PlcConnectionIT.java | 4 +--
6 files changed, 47 insertions(+), 10 deletions(-)
diff --git
a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/connection/AdsAbstractPlcConnection.java
b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/connection/AdsAbstractPlcConnection.java
index cf1b8d9..125d012 100644
---
a/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/connection/AdsAbstractPlcConnection.java
+++
b/plc4j/protocols/ads/src/main/java/org/apache/plc4x/java/ads/connection/AdsAbstractPlcConnection.java
@@ -33,6 +33,7 @@ import org.apache.plc4x.java.ads.model.SymbolicAdsAddress;
import org.apache.plc4x.java.api.connection.PlcProprietarySender;
import org.apache.plc4x.java.api.connection.PlcReader;
import org.apache.plc4x.java.api.connection.PlcWriter;
+import org.apache.plc4x.java.api.exceptions.PlcConnectionException;
import org.apache.plc4x.java.api.exceptions.PlcRuntimeException;
import org.apache.plc4x.java.api.messages.*;
import org.apache.plc4x.java.api.messages.items.RequestItem;
@@ -189,7 +190,7 @@ public abstract class AdsAbstractPlcConnection extends
AbstractPlcConnection imp
}
@Override
- public void close() {
+ public void close() throws PlcConnectionException {
addressMapping.values().stream()
.parallel()
.map(adsAddress -> AdsWriteRequest.of(
diff --git
a/plc4j/protocols/ads/src/test/java/org/apache/plc4x/java/ads/adslib/AmsRouter.java
b/plc4j/protocols/ads/src/test/java/org/apache/plc4x/java/ads/adslib/AmsRouter.java
index ba0bf79..06786af 100644
---
a/plc4j/protocols/ads/src/test/java/org/apache/plc4x/java/ads/adslib/AmsRouter.java
+++
b/plc4j/protocols/ads/src/test/java/org/apache/plc4x/java/ads/adslib/AmsRouter.java
@@ -107,7 +107,11 @@ public class AmsRouter {
AdsTcpPlcConnection route = mapping.get(ams);
if (route != null) {
AdsTcpPlcConnection conn = route;
- conn.close();
+ try {
+ conn.close();
+ } catch (PlcConnectionException e) {
+ throw new RuntimeException(e);
+ }
MutableInt refCounter = refCounts.getOrDefault(conn, new
MutableInt());
if (0 == refCounter.decrementAndGet()) {
mapping.remove(ams);
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 83f1540..c679915 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
@@ -80,7 +80,7 @@ public abstract class AbstractPlcConnection implements
PlcConnection {
}
@Override
- public void close() {
+ public void close() throws PlcConnectionException {
channel = null;
connected = false;
}
diff --git a/plc4j/protocols/s7/pom.xml b/plc4j/protocols/s7/pom.xml
index 75dec71..9ea4e45 100644
--- a/plc4j/protocols/s7/pom.xml
+++ b/plc4j/protocols/s7/pom.xml
@@ -76,6 +76,10 @@
<groupId>org.apache.commons</groupId>
<artifactId>commons-lang3</artifactId>
</dependency>
+ <dependency>
+ <groupId>org.apache.commons</groupId>
+ <artifactId>commons-configuration2</artifactId>
+ </dependency>
<dependency>
<groupId>ch.qos.logback</groupId>
diff --git
a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/connection/S7PlcConnection.java
b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/connection/S7PlcConnection.java
index fd8c98d..9b80741 100644
---
a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/connection/S7PlcConnection.java
+++
b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/connection/S7PlcConnection.java
@@ -19,6 +19,8 @@ under the License.
package org.apache.plc4x.java.s7.connection;
import io.netty.channel.*;
+import org.apache.commons.configuration2.Configuration;
+import org.apache.commons.configuration2.SystemConfiguration;
import org.apache.commons.lang3.StringUtils;
import org.apache.plc4x.java.api.connection.PlcReader;
import org.apache.plc4x.java.api.connection.PlcWriter;
@@ -50,6 +52,9 @@ import org.slf4j.LoggerFactory;
import java.net.InetAddress;
import java.util.Collections;
import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
@@ -75,6 +80,10 @@ public class S7PlcConnection extends AbstractPlcConnection
implements PlcReader,
private static final int ISO_ON_TCP_PORT = 102;
+ // Fetch values from configuration
+ private static final Configuration CONF = new SystemConfiguration();
+ private static final long CLOSE_DEVICE_TIMEOUT_MS =
CONF.getLong("plc4x.s7connection.close.device,timeout", 1_000);
+
private static final Pattern S7_DATABLOCK_ADDRESS_PATTERN =
Pattern.compile("^DATA_BLOCKS/(?<blockNumber>\\d{1,4})/(?<byteOffset>\\d{1,4})");
private static final Pattern S7_ADDRESS_PATTERN =
@@ -194,19 +203,38 @@ public class S7PlcConnection extends
AbstractPlcConnection implements PlcReader,
}
@Override
- public void close() {
+ public void close() throws PlcConnectionException {
if ((channel != null) && channel.isOpen()) {
// Send the PLC a message that the connection is being closed.
DisconnectRequestTpdu disconnectRequest = new
DisconnectRequestTpdu(
(short) 0x0000, (short) 0x000F, DisconnectReason.NORMAL,
Collections.emptyList(),
null);
- channel.writeAndFlush(disconnectRequest).awaitUninterruptibly();
// In case of an ISO TP Class 0 connection, the remote is usually
expected to actively
- // close the connection. We are simplifying things by doing it
ourselves immediately.
- // TODO: It would probably be more spec-conform to wait for the
remote to disconnect and to only do it actively if it doesn't happen within a
certain time. But this solution is good enough for now.
- channel.close();
+ // close the connection. So we add a listener waiting for this to
happen.
+ CompletableFuture<Void> disconnectFuture = new
CompletableFuture<>();
+ channel.closeFuture().addListener(
+ (ChannelFutureListener) future ->
disconnectFuture.complete(null));
+
+ // Send the disconnect request.
+ channel.writeAndFlush(disconnectRequest);
+ // Wait for the configured time for the remote to close the
session.
+ try {
+ disconnectFuture.get(CLOSE_DEVICE_TIMEOUT_MS,
TimeUnit.MILLISECONDS);
+ }
+ // If the remote didn't close the connection within the given
time-frame, we have to take
+ // care of closing the connection.
+ catch (TimeoutException e) {
+ logger.info("Remote didn't close connection within the
configured timeout of " +
+ CLOSE_DEVICE_TIMEOUT_MS + "ms, shutting down actively.");
+ channel.close();
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ } catch (ExecutionException e) {
+ throw new PlcConnectionException(e);
+ }
+ // Do some additional cleanup operations ...
// In normal operation, the channels event loop has a parent,
however when running with
// the embedded channel for unit tests, parent is null.
if(channel.eventLoop().parent() != null) {
diff --git
a/plc4j/protocols/s7/src/test/java/org/apache/plc4x/java/s7/connection/S7PlcConnectionIT.java
b/plc4j/protocols/s7/src/test/java/org/apache/plc4x/java/s7/connection/S7PlcConnectionIT.java
index c9c4b94..324ee0a 100644
---
a/plc4j/protocols/s7/src/test/java/org/apache/plc4x/java/s7/connection/S7PlcConnectionIT.java
+++
b/plc4j/protocols/s7/src/test/java/org/apache/plc4x/java/s7/connection/S7PlcConnectionIT.java
@@ -56,7 +56,7 @@ public class S7PlcConnectionIT {
}
@After
- public void tearDown() {
+ public void tearDown() throws PlcConnectionException{
if(s7PlcConnection.isConnected()) {
s7PlcConnection.close();
}
@@ -71,7 +71,7 @@ public class S7PlcConnectionIT {
}
@Test
- public void testDisconnect() {
+ public void testDisconnect() throws PlcConnectionException {
assertThat(s7PlcConnection, notNullValue());
assertThat("The connection should be 'connected'",
s7PlcConnection.isConnected(), is( true) );
s7PlcConnection.close();
