[incubator-plc4x] branch master updated: Avoid some exceptions when disconnecting from S7 PLCs

[cd...@apache.org]

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 5363805  Avoid some exceptions when disconnecting from S7 PLCs
5363805 is described below

commit 53638056588ef50ef9a462a1d3cbbafa94fda7fd
Author: Christofer Dutz <ch...@c-ware.de>
AuthorDate: Thu Aug 2 15:00:36 2018 +0200

    Avoid some exceptions when disconnecting from S7 PLCs
---
 .../plc4x/java/s7/connection/S7PlcConnection.java       | 17 +++++++++++------
 .../plc4x/java/s7/connection/S7PlcConnectionIT.java     | 10 +++++++++-
 2 files changed, 20 insertions(+), 7 deletions(-)

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 33c0335..fd8c98d 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
@@ -200,13 +200,18 @@ public class S7PlcConnection extends AbstractPlcConnection implements PlcReader,
             DisconnectRequestTpdu disconnectRequest = new DisconnectRequestTpdu(
                 (short) 0x0000, (short) 0x000F, DisconnectReason.NORMAL, Collections.emptyList(),
                 null);
-            ChannelFuture sendDisconnectRequestFuture = channel.writeAndFlush(disconnectRequest);
-            sendDisconnectRequestFuture.addListener((ChannelFutureListener) future -> {
-                // Close the session itself.
-                channel.closeFuture().await();
+            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();
+
+            // 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) {
                 channel.eventLoop().parent().shutdownGracefully();
-            });
-            sendDisconnectRequestFuture.awaitUninterruptibly();
+            }
         }
         super.close();
     }
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 6cbfd16..c9c4b94 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
@@ -65,11 +65,19 @@ public class S7PlcConnectionIT {
     }
 
     @Test
-    public void connect() {
+    public void testConnect() {
         assertThat(s7PlcConnection, notNullValue());
         assertThat("The connection should be 'connected'", s7PlcConnection.isConnected(), is( true) );
     }
 
+    @Test
+    public void testDisconnect() {
+        assertThat(s7PlcConnection, notNullValue());
+        assertThat("The connection should be 'connected'", s7PlcConnection.isConnected(), is( true) );
+        s7PlcConnection.close();
+        assertThat("The connection should be 'connected'", s7PlcConnection.isConnected(), is( false) );
+    }
+
     // TODO more tests for connect, close, read and write
 
 }