fix: Fixed issues with most discoverers, that didn't cancel timout ti…

…mers and hereby prevented the application from closing.
apache · Sep 5, 2024 · 7b88ac7 · 7b88ac7
1 parent 8863512
commit 7b88ac7
Show file tree

Hide file tree

Showing 12 changed files with 30 additions and 52 deletions.
diff --git a/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/discovery/AdsPlcDiscoverer.java b/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/discovery/AdsPlcDiscoverer.java
@@ -205,6 +205,8 @@ public CompletableFuture<PlcDiscoveryResponse> discoverWithHandler(PlcDiscoveryR
             public void run() {
                 PlcDiscoveryResponse response =
                     new DefaultPlcDiscoveryResponse(discoveryRequest, PlcResponseCode.OK, new ArrayList<>(values));
+                timer.cancel();
+                timer.purge();
                 future.complete(response);
             }
         }, 5000L);

diff --git a/.../drivers/eip/src/main/java/org/apache/plc4x/java/eip/base/discovery/EipPlcDiscoverer.java b/.../drivers/eip/src/main/java/org/apache/plc4x/java/eip/base/discovery/EipPlcDiscoverer.java
@@ -162,6 +162,8 @@ public CompletableFuture<PlcDiscoveryResponse> discoverWithHandler(PlcDiscoveryR
             public void run() {
                 PlcDiscoveryResponse response =
                     new DefaultPlcDiscoveryResponse(discoveryRequest, PlcResponseCode.OK, new ArrayList<>(values));
+                timer.cancel();
+                timer.purge();
                 future.complete(response);
             }
         }, 5000L);

diff --git a/plc4j/drivers/eip/src/test/java/org/apache/plc4x/java/eip/base/ManualEipDiscovery.java b/plc4j/drivers/eip/src/test/java/org/apache/plc4x/java/eip/base/ManualEipDiscovery.java
@@ -19,13 +19,13 @@
 
 package org.apache.plc4x.java.eip.base;
 
-import org.apache.plc4x.java.api.messages.PlcDiscoveryResponse;
-
 public class ManualEipDiscovery {
 
     public static void main(String[] args) throws Exception {
-        PlcDiscoveryResponse discoveryResponse = new EIPDriver().discoveryRequestBuilder().addQuery("all", "*").build().executeWithHandler(discoveryItem -> System.out.println(discoveryItem.getConnectionUrl())).get();
-        System.out.println(discoveryResponse);
+        new EIPDriver().discoveryRequestBuilder().addQuery("all", "*")
+            .build()
+            .executeWithHandler(discoveryItem -> System.out.println("Found new device: " + discoveryItem.getConnectionUrl() + " (" + discoveryItem.getName() + ")"))
+            .get();
     }
 
 }
diff --git a/...nxnetip/src/main/java/org/apache/plc4x/java/knxnetip/discovery/KnxNetIpPlcDiscoverer.java b/...nxnetip/src/main/java/org/apache/plc4x/java/knxnetip/discovery/KnxNetIpPlcDiscoverer.java
@@ -53,12 +53,10 @@
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
-import java.util.Queue;
 import java.util.Timer;
 import java.util.TimerTask;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.ConcurrentLinkedQueue;
 import java.util.concurrent.TimeUnit;
 
 public class KnxNetIpPlcDiscoverer implements PlcDiscoverer {
@@ -182,6 +180,8 @@ public CompletableFuture<PlcDiscoveryResponse> discoverWithHandler(PlcDiscoveryR
             public void run() {
                 PlcDiscoveryResponse response =
                     new DefaultPlcDiscoveryResponse(discoveryRequest, PlcResponseCode.OK, new ArrayList<>(values.values()));
+                timer.cancel();
+                timer.purge();
                 future.complete(response);
             }
         }, 5000L);

diff --git a/...rivers/knxnetip/src/test/java/org/apache/plc4x/java/knxnetip/ManualKnxNetIpDiscovery.java b/...rivers/knxnetip/src/test/java/org/apache/plc4x/java/knxnetip/ManualKnxNetIpDiscovery.java
@@ -19,13 +19,13 @@
 
 package org.apache.plc4x.java.knxnetip;
 
-import org.apache.plc4x.java.api.messages.PlcDiscoveryResponse;
-
 public class ManualKnxNetIpDiscovery {
 
     public static void main(String[] args) throws Exception {
-        PlcDiscoveryResponse discoveryResponse = new KnxNetIpDriver().discoveryRequestBuilder().addQuery("all", "*").build().execute().get();
-        System.out.println(discoveryResponse);
+        new KnxNetIpDriver().discoveryRequestBuilder().addQuery("all", "*")
+            .build()
+            .executeWithHandler(discoveryItem -> System.out.println("Found new device: " + discoveryItem.getConnectionUrl() + " (" + discoveryItem.getName() + ")"))
+            .get();
     }
 
 }
diff --git a/...ers/profinet-ng/src/main/java/org/apache/plc4x/java/profinet/channel/ProfinetChannel.java b/...ers/profinet-ng/src/main/java/org/apache/plc4x/java/profinet/channel/ProfinetChannel.java
@@ -106,8 +106,6 @@ public PacketListener createListener() {
                     // Check if it's a PROFINET packet
                     if (payload.getHeader().getDstPort().value() == -30572 || payload.getHeader().getDstPort().value() == -15536 || payload.getHeader().getDstPort().value() == -15535) {
                         isPnPacket = true;
-                    } else {
-                        System.out.println("UDP Packet from port: " + payload.getHeader().getSrcPort().value());
                     }
                 }
 

diff --git a/...rofinet-ng/src/main/java/org/apache/plc4x/java/profinet/discovery/ProfinetDiscoverer.java b/...rofinet-ng/src/main/java/org/apache/plc4x/java/profinet/discovery/ProfinetDiscoverer.java
@@ -63,7 +63,6 @@ public class ProfinetDiscoverer implements PlcDiscoverer {
     private static final MacAddress PROFINET_BROADCAST_MAC_ADDRESS = new MacAddress(new byte[]{0x01, 0x0E, (byte) 0xCF, 0x00, 0x00, 0x00});
     final private ProfinetChannel channel;
     final List<PlcDiscoveryItem> values = new ArrayList<>();
-    final Set<Timer> periodicTimers = new HashSet<>();
     private final Logger logger = LoggerFactory.getLogger(ProfinetDiscoverer.class);
     private PlcDiscoveryItemHandler handler;
 
@@ -128,10 +127,8 @@ public void run() {
                         logger.error("Error occurred while closing handle");
                     }
                 }
-                for (Timer timer : periodicTimers) {
-                    timer.cancel();
-                    timer.purge();
-                }
+                timer.cancel();
+                timer.purge();
                 future.complete(response);
             }
         }, delay);

diff --git a/...ofinet-ng/src/test/java/org/apache/plc4x/java/profinet/ManualProfinetIoDiscoveryTest.java b/...ofinet-ng/src/test/java/org/apache/plc4x/java/profinet/ManualProfinetIoDiscoveryTest.java
@@ -20,27 +20,15 @@
 
 import org.apache.plc4x.java.DefaultPlcDriverManager;
 import org.apache.plc4x.java.api.PlcDriver;
-import org.apache.plc4x.java.api.messages.PlcDiscoveryItem;
-import org.apache.plc4x.java.api.messages.PlcDiscoveryResponse;
-
-import java.util.Map;
-import java.util.TreeMap;
 
 public class ManualProfinetIoDiscoveryTest {
 
     public static void main(String[] args) throws Exception {
         final PlcDriver profinetDriver = new DefaultPlcDriverManager().getDriver("profinet");
-        final PlcDiscoveryResponse plcDiscoveryResponse = profinetDriver.discoveryRequestBuilder().build().execute().get();
-        // As we can reach some devices from multiple network devices, aggregate them by connection url
-        Map<String, PlcDiscoveryItem> items = new TreeMap<>();
-        for (PlcDiscoveryItem responseValue : plcDiscoveryResponse.getValues()) {
-            items.put(responseValue.getConnectionUrl(), responseValue);
-        }
-        // Output the aggregated values.
-        for (Map.Entry<String, PlcDiscoveryItem> stringPlcDiscoveryItemEntry : items.entrySet()) {
-            PlcDiscoveryItem responseValue = stringPlcDiscoveryItemEntry.getValue();
-            System.out.println(responseValue.getName() + ":  " + responseValue.getConnectionUrl());
-        }
+        profinetDriver.discoveryRequestBuilder()
+            .build()
+            .executeWithHandler(discoveryItem -> System.out.println("Found new device: " + discoveryItem.getConnectionUrl() + " (" + discoveryItem.getName() + ")"))
+            .get();
     }
 
 }
diff --git a/.../profinet/src/test/java/org/apache/plc4x/java/profinet/ManualProfinetIoDiscoveryTest.java b/.../profinet/src/test/java/org/apache/plc4x/java/profinet/ManualProfinetIoDiscoveryTest.java
@@ -20,17 +20,15 @@
 
 import org.apache.plc4x.java.DefaultPlcDriverManager;
 import org.apache.plc4x.java.api.PlcDriver;
-import org.apache.plc4x.java.api.messages.PlcDiscoveryItem;
-import org.apache.plc4x.java.api.messages.PlcDiscoveryResponse;
 
 public class ManualProfinetIoDiscoveryTest {
 
     public static void main(String[] args) throws Exception {
         final PlcDriver profinetDriver = new DefaultPlcDriverManager().getDriver("profinet");
-        final PlcDiscoveryResponse plcDiscoveryResponse = profinetDriver.discoveryRequestBuilder().build().execute().get();
-        for (PlcDiscoveryItem responseValue : plcDiscoveryResponse.getValues()) {
-            System.out.println(responseValue.getConnectionUrl());
-        }
+        profinetDriver.discoveryRequestBuilder()
+            .build()
+            .executeWithHandler(discoveryItem -> System.out.println("Found new device: " + discoveryItem.getConnectionUrl() + " (" + discoveryItem.getName() + ")"))
+            .get();
     }
 
 }
diff --git a/...rivers/s7/src/main/java/org/apache/plc4x/java/s7/readwrite/discovery/ProfinetChannel.java b/...rivers/s7/src/main/java/org/apache/plc4x/java/s7/readwrite/discovery/ProfinetChannel.java
@@ -130,8 +130,6 @@ public PacketListener createListener() {
                     // Check if it's a PROFINET packet
                     if (payload.getHeader().getDstPort().value() == -30572 || payload.getHeader().getDstPort().value() == -15536 || payload.getHeader().getDstPort().value() == -15535) {
                         isPnPacket = true;
-                    } else {
-                        System.out.println("UDP Packet from port: " + payload.getHeader().getSrcPort().value());
                     }
                 }
 

diff --git a/...rivers/s7/src/main/java/org/apache/plc4x/java/s7/readwrite/discovery/S7PlcDiscoverer.java b/...rivers/s7/src/main/java/org/apache/plc4x/java/s7/readwrite/discovery/S7PlcDiscoverer.java
@@ -61,10 +61,8 @@
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 import java.util.Timer;
 import java.util.TimerTask;
 import java.util.concurrent.CompletableFuture;
@@ -91,7 +89,6 @@ public class S7PlcDiscoverer implements PlcDiscoverer {
 
     final private ProfinetChannel channel;
     final List<PlcDiscoveryItem> values = new ArrayList<>();
-    final Set<Timer> periodicTimers = new HashSet<>();
     private PlcDiscoveryItemHandler handler;
 
     public S7PlcDiscoverer(ProfinetChannel channel) {
@@ -166,10 +163,8 @@ public void run() {
                         logger.error("Error occurred while closing handle");
                     }
                 }
-                for (Timer timer : periodicTimers) {
-                    timer.cancel();
-                    timer.purge();
-                }
+                timer.cancel();
+                timer.purge();
                 future.complete(response);
             }
         }, delay);

diff --git a/plc4j/drivers/s7/src/test/java/org/apache/plc4x/java/s7/readwrite/ManualS7Discovery.java b/plc4j/drivers/s7/src/test/java/org/apache/plc4x/java/s7/readwrite/ManualS7Discovery.java
@@ -19,13 +19,13 @@
 
 package org.apache.plc4x.java.s7.readwrite;
 
-import org.apache.plc4x.java.api.messages.PlcDiscoveryResponse;
-
 public class ManualS7Discovery {
 
     public static void main(String[] args) throws Exception {
-        PlcDiscoveryResponse discoveryResponse = new S7Driver().discoveryRequestBuilder().addQuery("all", "*").build().execute().get();
-        System.out.println(discoveryResponse);
+        new S7Driver().discoveryRequestBuilder().addQuery("all", "*")
+            .build()
+            .executeWithHandler(discoveryItem -> System.out.println("Found new device: " + discoveryItem.getConnectionUrl() + " (" + discoveryItem.getName() + ")"))
+            .get();
     }
 
 }