From b4974b3ffdae3569fe8ddc4405c66390a97f59d6 Mon Sep 17 00:00:00 2001 From: Wouter Born Date: Thu, 11 Jan 2024 17:55:23 +0100 Subject: [PATCH] [network] Cleanup code (#16235) * Reuse ExpiringCacheAsync from Core * Use Duration and Instant * Replace Optional with null annotations * Cleanup JavaDocs * Improve logging * Add missing null annotations * Simplify code Signed-off-by: Wouter Born Signed-off-by: Ciprian Pascu --- .../internal/NetworkBindingConfiguration.java | 2 +- .../internal/NetworkBindingConstants.java | 14 +- .../network/internal/PresenceDetection.java | 548 ++++++++---------- .../internal/PresenceDetectionValue.java | 196 +++---- .../internal/dhcp/DHCPListenService.java | 20 +- .../dhcp/DHCPPacketListenerServer.java | 3 +- .../discovery/NetworkDiscoveryService.java | 71 +-- .../internal/handler/NetworkHandler.java | 41 +- .../internal/handler/SpeedTestHandler.java | 3 - .../toberemoved/cache/ExpiringCacheAsync.java | 143 ----- .../network/internal/utils/LatencyParser.java | 13 +- .../network/internal/utils/NetworkUtils.java | 177 +++--- .../network/internal/utils/PingResult.java | 39 +- .../internal/PresenceDetectionTest.java | 159 +++-- .../internal/PresenceDetectionValuesTest.java | 48 +- .../internal/discovery/DiscoveryTest.java | 15 +- .../internal/handler/NetworkHandlerTest.java | 31 +- .../cache/ExpiringCacheAsyncTest.java | 105 ---- .../cache/ExpiringCacheHelper.java | 27 - .../internal/utils/LatencyParserTest.java | 19 +- 20 files changed, 688 insertions(+), 986 deletions(-) delete mode 100644 bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/toberemoved/cache/ExpiringCacheAsync.java delete mode 100644 bundles/org.openhab.binding.network/src/test/java/org/openhab/binding/network/internal/toberemoved/cache/ExpiringCacheAsyncTest.java delete mode 100644 bundles/org.openhab.binding.network/src/test/java/org/openhab/binding/network/internal/toberemoved/cache/ExpiringCacheHelper.java diff --git a/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/NetworkBindingConfiguration.java b/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/NetworkBindingConfiguration.java index a1b92cc3bf5..c63eae9ff91 100644 --- a/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/NetworkBindingConfiguration.java +++ b/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/NetworkBindingConfiguration.java @@ -47,7 +47,7 @@ public class NetworkBindingConfiguration { this.preferResponseTimeAsLatency = newConfiguration.preferResponseTimeAsLatency; NetworkUtils networkUtils = new NetworkUtils(); - this.arpPingUtilMethod = networkUtils.determineNativeARPpingMethod(arpPingToolPath); + this.arpPingUtilMethod = networkUtils.determineNativeArpPingMethod(arpPingToolPath); notifyListeners(); } diff --git a/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/NetworkBindingConstants.java b/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/NetworkBindingConstants.java index 7cb73bb99f0..d31f0c77aeb 100644 --- a/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/NetworkBindingConstants.java +++ b/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/NetworkBindingConstants.java @@ -12,7 +12,6 @@ */ package org.openhab.binding.network.internal; -import java.util.HashSet; import java.util.Set; import org.eclipse.jdt.annotation.NonNullByDefault; @@ -36,10 +35,12 @@ public class NetworkBindingConstants { public static final ThingTypeUID SERVICE_DEVICE = new ThingTypeUID(BINDING_ID, "servicedevice"); public static final ThingTypeUID SPEEDTEST_DEVICE = new ThingTypeUID(BINDING_ID, "speedtest"); + public static final Set SUPPORTED_THING_TYPES_UIDS = Set.of(BACKWARDS_COMPATIBLE_DEVICE, PING_DEVICE, + SERVICE_DEVICE, SPEEDTEST_DEVICE); + // List of all Channel ids public static final String CHANNEL_ONLINE = "online"; public static final String CHANNEL_LATENCY = "latency"; - public static final String CHANNEL_DEPRECATED_TIME = "time"; public static final String CHANNEL_LASTSEEN = "lastseen"; public static final String CHANNEL_TEST_ISRUNNING = "isRunning"; public static final String CHANNEL_TEST_PROGRESS = "progress"; @@ -60,13 +61,4 @@ public class NetworkBindingConstants { public static final String PROPERTY_ICMP_STATE = "icmp_state"; public static final String PROPERTY_PRESENCE_DETECTION_TYPE = "presence_detection_type"; public static final String PROPERTY_IOS_WAKEUP = "uses_ios_wakeup"; - - public static final Set SUPPORTED_THING_TYPES_UIDS = new HashSet<>(); - - static { - SUPPORTED_THING_TYPES_UIDS.add(PING_DEVICE); - SUPPORTED_THING_TYPES_UIDS.add(SERVICE_DEVICE); - SUPPORTED_THING_TYPES_UIDS.add(BACKWARDS_COMPATIBLE_DEVICE); - SUPPORTED_THING_TYPES_UIDS.add(SPEEDTEST_DEVICE); - } } diff --git a/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/PresenceDetection.java b/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/PresenceDetection.java index 54d8f47e4dd..f6b4cd2b60e 100644 --- a/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/PresenceDetection.java +++ b/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/PresenceDetection.java @@ -12,13 +12,21 @@ */ package org.openhab.binding.network.internal; +import static org.openhab.binding.network.internal.PresenceDetectionType.*; + import java.io.IOException; import java.net.Inet4Address; import java.net.InetAddress; import java.net.SocketException; import java.net.UnknownHostException; +import java.time.Duration; +import java.time.Instant; +import java.util.ArrayList; import java.util.HashSet; +import java.util.List; import java.util.Set; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; @@ -31,26 +39,27 @@ import org.eclipse.jdt.annotation.Nullable; import org.openhab.binding.network.internal.dhcp.DHCPListenService; import org.openhab.binding.network.internal.dhcp.DHCPPacketListenerServer; import org.openhab.binding.network.internal.dhcp.IPRequestReceivedCallback; -import org.openhab.binding.network.internal.toberemoved.cache.ExpiringCacheAsync; import org.openhab.binding.network.internal.utils.NetworkUtils; import org.openhab.binding.network.internal.utils.NetworkUtils.ArpPingUtilEnum; import org.openhab.binding.network.internal.utils.NetworkUtils.IpPingMethodEnum; import org.openhab.binding.network.internal.utils.PingResult; import org.openhab.core.cache.ExpiringCache; +import org.openhab.core.cache.ExpiringCacheAsync; import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** - * The {@link PresenceDetection} handles the connection to the Device + * The {@link PresenceDetection} handles the connection to the Device. * * @author Marc Mettke - Initial contribution * @author David Gräff, 2017 - Rewritten * @author Jan N. Klug - refactored host name resolution + * @author Wouter Born - Reuse ExpiringCacheAsync from Core */ @NonNullByDefault public class PresenceDetection implements IPRequestReceivedCallback { - private static final int DESTINATION_TTL = 300 * 1000; // in ms, 300 s + private static final Duration DESTINATION_TTL = Duration.ofMinutes(5); NetworkUtils networkUtils = new NetworkUtils(); private final Logger logger = LoggerFactory.getLogger(PresenceDetection.class); @@ -64,32 +73,35 @@ public class PresenceDetection implements IPRequestReceivedCallback { private boolean iosDevice; private Set tcpPorts = new HashSet<>(); - private long refreshIntervalInMS = 60000; - private int timeoutInMS = 5000; - private long lastSeenInMS; + private Duration refreshInterval = Duration.ofMinutes(1); + private Duration timeout = Duration.ofSeconds(5); + private @Nullable Instant lastSeen; private @NonNullByDefault({}) String hostname; private @NonNullByDefault({}) ExpiringCache<@Nullable InetAddress> destination; - private @Nullable InetAddress cachedDestination = null; + private @Nullable InetAddress cachedDestination; private boolean preferResponseTimeAsLatency; - /// State variables (cannot be final because of test dependency injections) + // State variables (cannot be final because of test dependency injections) ExpiringCacheAsync cache; + private final PresenceDetectionListener updateListener; + private ScheduledExecutorService scheduledExecutorService; private Set networkInterfaceNames = Set.of(); private @Nullable ScheduledFuture refreshJob; - protected @Nullable ExecutorService executorService; + protected @Nullable ExecutorService detectionExecutorService; private String dhcpState = "off"; - private Integer currentCheck = 0; int detectionChecks; private String lastReachableNetworkInterfaceName = ""; - public PresenceDetection(final PresenceDetectionListener updateListener, int cacheDeviceStateTimeInMS) + public PresenceDetection(final PresenceDetectionListener updateListener, + ScheduledExecutorService scheduledExecutorService, Duration cacheDeviceStateTime) throws IllegalArgumentException { this.updateListener = updateListener; - cache = new ExpiringCacheAsync<>(cacheDeviceStateTimeInMS, () -> performPresenceDetection(false)); + this.scheduledExecutorService = scheduledExecutorService; + cache = new ExpiringCacheAsync<>(cacheDeviceStateTime); } public @Nullable String getHostname() { @@ -100,12 +112,12 @@ public class PresenceDetection implements IPRequestReceivedCallback { return tcpPorts; } - public long getRefreshInterval() { - return refreshIntervalInMS; + public Duration getRefreshInterval() { + return refreshInterval; } - public int getTimeout() { - return timeoutInMS; + public Duration getTimeout() { + return timeout; } public void setHostname(String hostname) { @@ -115,7 +127,8 @@ public class PresenceDetection implements IPRequestReceivedCallback { InetAddress destinationAddress = InetAddress.getByName(hostname); InetAddress cached = cachedDestination; if (!destinationAddress.equals(cached)) { - logger.trace("host name resolved to other address, (re-)setup presence detection"); + logger.trace("Hostname {} resolved to other address {}, (re-)setup presence detection", hostname, + destinationAddress); setUseArpPing(true, destinationAddress); if (useDHCPsniffing) { if (cached != null) { @@ -127,7 +140,7 @@ public class PresenceDetection implements IPRequestReceivedCallback { } return destinationAddress; } catch (UnknownHostException e) { - logger.trace("hostname resolution failed"); + logger.trace("Hostname resolution for {} failed", hostname); InetAddress cached = cachedDestination; if (cached != null) { disableDHCPListen(cached); @@ -150,12 +163,12 @@ public class PresenceDetection implements IPRequestReceivedCallback { this.useDHCPsniffing = enable; } - public void setRefreshInterval(long refreshInterval) { - this.refreshIntervalInMS = refreshInterval; + public void setRefreshInterval(Duration refreshInterval) { + this.refreshInterval = refreshInterval; } - public void setTimeout(int timeout) { - this.timeoutInMS = timeout; + public void setTimeout(Duration timeout) { + this.timeout = timeout; } public void setPreferResponseTimeAsLatency(boolean preferResponseTimeAsLatency) { @@ -163,11 +176,11 @@ public class PresenceDetection implements IPRequestReceivedCallback { } /** - * Sets the ping method. This method will perform a feature test. If SYSTEM_PING - * does not work on this system, JAVA_PING will be used instead. + * Sets the ping method. This method will perform a feature test. If {@link IpPingMethodEnum#SYSTEM_PING} + * does not work on this system, {@link IpPingMethodEnum#JAVA_PING} will be used instead. * - * @param useSystemPing Set to true to use a system ping method, false to use java ping and null to disable ICMP - * pings. + * @param useSystemPing Set to true to use a system ping method, false to use Java ping + * and null to disable ICMP pings. */ public void setUseIcmpPing(@Nullable Boolean useSystemPing) { if (useSystemPing == null) { @@ -201,9 +214,9 @@ public class PresenceDetection implements IPRequestReceivedCallback { } /** - * sets the path to arp ping + * Sets the path to ARP ping. * - * @param enable Enable or disable ARP ping + * @param enable enable or disable ARP ping * @param arpPingUtilPath enableDHCPListen(useDHCPsniffing); */ public void setUseArpPing(boolean enable, String arpPingUtilPath, ArpPingUtilEnum arpPingUtilMethod) { @@ -225,7 +238,7 @@ public class PresenceDetection implements IPRequestReceivedCallback { } /** - * Return true if the device presence detection is performed for an iOS device + * Return true if the device presence detection is performed for an iOS device * like iPhone or iPads. An additional port knock is performed before a ping. */ public boolean isIOSdevice() { @@ -233,7 +246,7 @@ public class PresenceDetection implements IPRequestReceivedCallback { } /** - * Set to true if the device presence detection should be performed for an iOS device + * Set to true if the device presence detection should be performed for an iOS device * like iPhone or iPads. An additional port knock is performed before a ping. */ public void setIOSDevice(boolean value) { @@ -241,58 +254,62 @@ public class PresenceDetection implements IPRequestReceivedCallback { } /** - * Return the last seen value in milliseconds based on {@link System#currentTimeMillis()} or 0 if not seen yet. + * Return the last seen value as an {@link Instant} or null if not yet seen. */ - public long getLastSeen() { - return lastSeenInMS; + public @Nullable Instant getLastSeen() { + return lastSeen; } /** - * Return asynchronously the value of the presence detection as a PresenceDetectionValue. + * Gets the presence detection value synchronously as a {@link PresenceDetectionValue}. + *

+ * The value is only updated if the cached value has not expired. + */ + public PresenceDetectionValue getValue() throws InterruptedException, ExecutionException { + return cache.getValue(this::performPresenceDetection).get(); + } + + /** + * Gets the presence detection value asynchronously as a {@link PresenceDetectionValue}. + *

+ * The value is only updated if the cached value has not expired. * - * @param callback A callback with the PresenceDetectionValue. The callback may + * @param callback a callback with the {@link PresenceDetectionValue}. The callback may * not happen immediately if the cached value expired, but as soon as a new * discovery took place. */ public void getValue(Consumer callback) { - cache.getValue(callback); + cache.getValue(this::performPresenceDetection).thenAccept(callback); } public ExecutorService getThreadsFor(int threadCount) { return Executors.newFixedThreadPool(threadCount); } + private void withDestinationAddress(Consumer consumer) { + InetAddress destinationAddress = destination.getValue(); + if (destinationAddress == null) { + logger.trace("The destinationAddress for {} is null", hostname); + } else { + consumer.accept(destinationAddress); + } + } + /** - * Perform a presence detection with ICMP-, ARP ping and - * TCP connection attempts simultaneously. A fixed thread pool will be created with as many - * thread as necessary to perform all tests at once. - * - * This is a NO-OP, if there is already an ongoing detection or if the cached value - * is not expired yet. + * Perform a presence detection with ICMP-, ARP ping and TCP connection attempts simultaneously. + * A fixed thread pool will be created with as many threads as necessary to perform all tests at once. * * Please be aware of the following restrictions: - * - ARP pings are only executed on IPv4 addresses. - * - Non system / Java pings are not recommended at all - * (not interruptible, useless TCP echo service fall back) + *

    + *
  • ARP pings are only executed on IPv4 addresses. + *
  • Non system / Java pings are not recommended at all (not interruptible, useless TCP echo service fall back) + *
* - * @param waitForDetectionToFinish If you want to synchronously wait for the result, set this to true - * @return Return true if a presence detection is performed and false otherwise. + * @return a {@link CompletableFuture} for obtaining the {@link PresenceDetectionValue} */ - public boolean performPresenceDetection(boolean waitForDetectionToFinish) { - if (executorService != null) { - logger.debug( - "There is already an ongoing presence discovery for {} and a new one was issued by the scheduler! TCP Port {}", - hostname, tcpPorts); - return false; - } - - if (!cache.isExpired()) { - return false; - } - + public CompletableFuture performPresenceDetection() { Set interfaceNames = null; - currentCheck = 0; detectionChecks = tcpPorts.size(); if (pingMethod != null) { detectionChecks += 1; @@ -308,295 +325,240 @@ public class PresenceDetection implements IPRequestReceivedCallback { detectionChecks += interfaceNames.size(); } + logger.trace("Performing {} presence detection checks for {}", detectionChecks, hostname); + + PresenceDetectionValue pdv = new PresenceDetectionValue(hostname, PresenceDetectionValue.UNREACHABLE); + if (detectionChecks == 0) { - return false; + return CompletableFuture.completedFuture(pdv); } - final ExecutorService executorService = getThreadsFor(detectionChecks); - this.executorService = executorService; + ExecutorService detectionExecutorService = getThreadsFor(detectionChecks); + this.detectionExecutorService = detectionExecutorService; + + List> completableFutures = new ArrayList<>(); for (Integer tcpPort : tcpPorts) { - executorService.execute(() -> { + completableFutures.add(CompletableFuture.runAsync(() -> { Thread.currentThread().setName("presenceDetectionTCP_" + hostname + " " + tcpPort); - performServicePing(tcpPort); - checkIfFinished(); - }); + performServicePing(pdv, tcpPort); + }, detectionExecutorService)); } // ARP ping for IPv4 addresses. Use single executor for Windows tool and // each own executor for each network interface for other tools if (arpPingMethod == ArpPingUtilEnum.ELI_FULKERSON_ARP_PING_FOR_WINDOWS) { - executorService.execute(() -> { + completableFutures.add(CompletableFuture.runAsync(() -> { Thread.currentThread().setName("presenceDetectionARP_" + hostname + " "); // arp-ping.exe tool capable of handling multiple interfaces by itself - performARPping(""); - checkIfFinished(); - }); + performArpPing(pdv, ""); + }, detectionExecutorService)); } else if (interfaceNames != null) { for (final String interfaceName : interfaceNames) { - executorService.execute(() -> { + completableFutures.add(CompletableFuture.runAsync(() -> { Thread.currentThread().setName("presenceDetectionARP_" + hostname + " " + interfaceName); - performARPping(interfaceName); - checkIfFinished(); - }); + performArpPing(pdv, interfaceName); + }, detectionExecutorService)); } } // ICMP ping if (pingMethod != null) { - executorService.execute(() -> { - if (pingMethod != IpPingMethodEnum.JAVA_PING) { - Thread.currentThread().setName("presenceDetectionICMP_" + hostname); - performSystemPing(); + completableFutures.add(CompletableFuture.runAsync(() -> { + Thread.currentThread().setName("presenceDetectionICMP_" + hostname); + if (pingMethod == IpPingMethodEnum.JAVA_PING) { + performJavaPing(pdv); } else { - performJavaPing(); + performSystemPing(pdv); } - checkIfFinished(); - }); + }, detectionExecutorService)); } - if (waitForDetectionToFinish) { - waitForPresenceDetection(); - } + return CompletableFuture.supplyAsync(() -> { + logger.debug("Waiting for {} detection futures for {} to complete", completableFutures.size(), hostname); + completableFutures.forEach(CompletableFuture::join); + logger.debug("All {} detection futures for {} have completed", completableFutures.size(), hostname); - return true; + if (!pdv.isReachable()) { + logger.debug("{} is unreachable, invalidating destination value", hostname); + destination.invalidateValue(); + } + + logger.debug("Sending listener final result: {}", pdv); + updateListener.finalDetectionResult(pdv); + + detectionExecutorService.shutdownNow(); + this.detectionExecutorService = null; + detectionChecks = 0; + + return pdv; + }, scheduledExecutorService); } /** - * Calls updateListener.finalDetectionResult() with a final result value. - * Safe to be called from different threads. After a call to this method, - * the presence detection process is finished and all threads are forcefully - * shut down. - */ - private synchronized void submitFinalResult() { - // Do nothing if we are not in a detection process - ExecutorService service = executorService; - if (service == null) { - return; - } - // Finish the detection process - service.shutdownNow(); - executorService = null; - detectionChecks = 0; - - PresenceDetectionValue v; - - // The cache will be expired by now if cache_time < timeoutInMS. But the device might be actually reachable. - // Therefore use lastSeenInMS here and not cache.isExpired() to determine if we got a ping response. - if (lastSeenInMS + timeoutInMS + 100 < System.currentTimeMillis()) { - // We haven't seen the device in the detection process - v = new PresenceDetectionValue(hostname, -1); - } else { - // Make the cache valid again and submit the value. - v = cache.getExpiredValue(); - } - cache.setValue(v); - - if (!v.isReachable()) { - // if target can't be reached, check if name resolution need to be updated - destination.invalidateValue(); - } - updateListener.finalDetectionResult(v); - } - - /** - * This method is called after each individual check and increases a check counter. - * If the counter equals the total checks,the final result is submitted. This will - * happen way before the "timeoutInMS", if all checks were successful. - * Thread safe. - */ - private synchronized void checkIfFinished() { - currentCheck += 1; - if (currentCheck < detectionChecks) { - return; - } - submitFinalResult(); - } - - /** - * Waits for the presence detection threads to finish. Returns immediately - * if no presence detection is performed right now. - */ - public void waitForPresenceDetection() { - ExecutorService service = executorService; - if (service == null) { - return; - } - try { - // We may get interrupted here by cancelRefreshJob(). - service.awaitTermination(timeoutInMS + 100, TimeUnit.MILLISECONDS); - submitFinalResult(); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); // Reset interrupt flag - service.shutdownNow(); - executorService = null; - } - } - - /** - * If the cached PresenceDetectionValue has not expired yet, the cached version - * is returned otherwise a new reachable PresenceDetectionValue is created with - * a latency of 0. + * Creates a new {@link PresenceDetectionValue} when a host is reachable. Also updates the {@link #lastSeen} + * value and sends a partial detection result to the {@link #updateListener}. + *

+ * It is safe to call this method from multiple threads. * - * It is safe to call this method from multiple threads. The returned PresenceDetectionValue - * might be still be altered in other threads though. - * - * @param type The detection type - * @return The non expired or a new instance of PresenceDetectionValue. + * @param type the detection type + * @param latency the latency */ - synchronized PresenceDetectionValue updateReachableValue(PresenceDetectionType type, double latency) { - lastSeenInMS = System.currentTimeMillis(); - PresenceDetectionValue v; - if (cache.isExpired()) { - v = new PresenceDetectionValue(hostname, 0); - } else { - v = cache.getExpiredValue(); - } - v.updateLatency(latency); - v.addType(type); - cache.setValue(v); - return v; + synchronized PresenceDetectionValue updateReachable(PresenceDetectionType type, Duration latency) { + PresenceDetectionValue pdv = new PresenceDetectionValue(hostname, latency); + updateReachable(pdv, type, latency); + return pdv; } - protected void performServicePing(int tcpPort) { + /** + * Updates the given {@link PresenceDetectionValue} when a host is reachable. Also updates the {@link #lastSeen} + * value and sends a partial detection result to the {@link #updateListener}. + *

+ * It is safe to call this method from multiple threads. + * + * @param pdv the {@link PresenceDetectionValue} to update + * @param type the detection type + * @param latency the latency + */ + synchronized void updateReachable(PresenceDetectionValue pdv, PresenceDetectionType type, Duration latency) { + updateReachable(pdv, type, latency, -1); + } + + synchronized void updateReachable(PresenceDetectionValue pdv, PresenceDetectionType type, Duration latency, + int tcpPort) { + lastSeen = Instant.now(); + pdv.addReachableDetectionType(type); + pdv.updateLatency(latency); + if (0 <= tcpPort) { + pdv.addReachableTcpPort(tcpPort); + } + logger.debug("Sending listener partial result: {}", pdv); + updateListener.partialDetectionResult(pdv); + } + + protected void performServicePing(PresenceDetectionValue pdv, int tcpPort) { logger.trace("Perform TCP presence detection for {} on port: {}", hostname, tcpPort); - try { - InetAddress destinationAddress = destination.getValue(); - if (destinationAddress != null) { - networkUtils.servicePing(destinationAddress.getHostAddress(), tcpPort, timeoutInMS).ifPresent(o -> { - if (o.isSuccess()) { - PresenceDetectionValue v = updateReachableValue(PresenceDetectionType.TCP_CONNECTION, - getLatency(o, preferResponseTimeAsLatency)); - v.addReachableTcpService(tcpPort); - updateListener.partialDetectionResult(v); - } - }); - } - } catch (IOException e) { - // This should not happen and might be a user configuration issue, we log a warning message therefore. - logger.warn("Could not create a socket connection", e); - } - } - /** - * Performs an "ARP ping" (ARP request) on the given interface. - * If it is an iOS device, the {@see NetworkUtils.wakeUpIOS()} method is - * called before performing the ARP ping. - * - * @param interfaceName The interface name. You can request a list of interface names - * from {@see NetworkUtils.getInterfaceNames()} for example. - */ - protected void performARPping(String interfaceName) { - try { - logger.trace("Perform ARP ping presence detection for {} on interface: {}", hostname, interfaceName); - InetAddress destinationAddress = destination.getValue(); - if (destinationAddress == null) { - return; - } - if (iosDevice) { - networkUtils.wakeUpIOS(destinationAddress); - Thread.sleep(50); - } - - networkUtils.nativeARPPing(arpPingMethod, arpPingUtilPath, interfaceName, - destinationAddress.getHostAddress(), timeoutInMS).ifPresent(o -> { - if (o.isSuccess()) { - PresenceDetectionValue v = updateReachableValue(PresenceDetectionType.ARP_PING, - getLatency(o, preferResponseTimeAsLatency)); - updateListener.partialDetectionResult(v); - lastReachableNetworkInterfaceName = interfaceName; - } else if (lastReachableNetworkInterfaceName.equals(interfaceName)) { - logger.trace("{} is no longer reachable on network interface: {}", hostname, interfaceName); - lastReachableNetworkInterfaceName = ""; - } - }); - } catch (IOException e) { - logger.trace("Failed to execute an arp ping for ip {}", hostname, e); - } catch (InterruptedException ignored) { - // This can be ignored, the thread will end anyway - } - } - - /** - * Performs a java ping. It is not recommended to use this, as it is not interruptible, - * and will not work on windows systems reliably and will fall back from ICMP pings to - * the TCP echo service on port 7 which barely no device or server supports nowadays. - * (https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/net/InetAddress.html#isReachable%28int%29) - */ - protected void performJavaPing() { - logger.trace("Perform java ping presence detection for {}", hostname); - - InetAddress destinationAddress = destination.getValue(); - if (destinationAddress == null) { - return; - } - - networkUtils.javaPing(timeoutInMS, destinationAddress).ifPresent(o -> { - if (o.isSuccess()) { - PresenceDetectionValue v = updateReachableValue(PresenceDetectionType.ICMP_PING, - getLatency(o, preferResponseTimeAsLatency)); - updateListener.partialDetectionResult(v); + withDestinationAddress(destinationAddress -> { + try { + PingResult pingResult = networkUtils.servicePing(destinationAddress.getHostAddress(), tcpPort, timeout); + if (pingResult.isSuccess()) { + updateReachable(pdv, TCP_CONNECTION, getLatency(pingResult), tcpPort); + } + } catch (IOException e) { + // This should not happen and might be a user configuration issue, we log a warning message therefore. + logger.warn("Could not create a socket connection", e); } }); } - protected void performSystemPing() { - try { - logger.trace("Perform native ping presence detection for {}", hostname); - InetAddress destinationAddress = destination.getValue(); - if (destinationAddress == null) { - return; - } + /** + * Performs an "ARP ping" (ARP request) on the given interface. + * If it is an iOS device, the {@link NetworkUtils#wakeUpIOS(InetAddress)} method is + * called before performing the ARP ping. + * + * @param pdv the {@link PresenceDetectionValue} to update + * @param interfaceName the interface name. You can request a list of interface names + * from {@link NetworkUtils#getInterfaceNames()} for example. + */ + protected void performArpPing(PresenceDetectionValue pdv, String interfaceName) { + logger.trace("Perform ARP ping presence detection for {} on interface: {}", hostname, interfaceName); - networkUtils.nativePing(pingMethod, destinationAddress.getHostAddress(), timeoutInMS).ifPresent(o -> { - if (o.isSuccess()) { - PresenceDetectionValue v = updateReachableValue(PresenceDetectionType.ICMP_PING, - getLatency(o, preferResponseTimeAsLatency)); - updateListener.partialDetectionResult(v); + withDestinationAddress(destinationAddress -> { + try { + if (iosDevice) { + networkUtils.wakeUpIOS(destinationAddress); + Thread.sleep(50); } - }); - } catch (IOException e) { - logger.trace("Failed to execute a native ping for ip {}", hostname, e); - } catch (InterruptedException e) { - // This can be ignored, the thread will end anyway - } + + PingResult pingResult = networkUtils.nativeArpPing(arpPingMethod, arpPingUtilPath, interfaceName, + destinationAddress.getHostAddress(), timeout); + if (pingResult != null) { + if (pingResult.isSuccess()) { + updateReachable(pdv, ARP_PING, getLatency(pingResult)); + lastReachableNetworkInterfaceName = interfaceName; + } else if (lastReachableNetworkInterfaceName.equals(interfaceName)) { + logger.trace("{} is no longer reachable on network interface: {}", hostname, interfaceName); + lastReachableNetworkInterfaceName = ""; + } + } + } catch (IOException e) { + logger.trace("Failed to execute an ARP ping for {}", hostname, e); + } catch (InterruptedException ignored) { + // This can be ignored, the thread will end anyway + } + }); } - private double getLatency(PingResult pingResult, boolean preferResponseTimeAsLatency) { - logger.debug("Getting latency from ping result {} using latency mode {}", pingResult, + /** + * Performs a Java ping. It is not recommended to use this, as it is not interruptible, + * and will not work on Windows systems reliably and will fall back from ICMP pings to + * the TCP echo service on port 7 which barely no device or server supports nowadays. + * + * @see InetAddress#isReachable(int) + */ + protected void performJavaPing(PresenceDetectionValue pdv) { + logger.trace("Perform Java ping presence detection for {}", hostname); + + withDestinationAddress(destinationAddress -> { + PingResult pingResult = networkUtils.javaPing(timeout, destinationAddress); + if (pingResult.isSuccess()) { + updateReachable(pdv, ICMP_PING, getLatency(pingResult)); + } + }); + } + + protected void performSystemPing(PresenceDetectionValue pdv) { + logger.trace("Perform native ping presence detection for {}", hostname); + + withDestinationAddress(destinationAddress -> { + try { + PingResult pingResult = networkUtils.nativePing(pingMethod, destinationAddress.getHostAddress(), + timeout); + if (pingResult != null && pingResult.isSuccess()) { + updateReachable(pdv, ICMP_PING, getLatency(pingResult)); + } + } catch (IOException e) { + logger.trace("Failed to execute a native ping for {}", hostname, e); + } catch (InterruptedException e) { + // This can be ignored, the thread will end anyway + } + }); + } + + private Duration getLatency(PingResult pingResult) { + logger.trace("Getting latency from ping result {} using latency mode {}", pingResult, preferResponseTimeAsLatency); - // Execution time is always set and this value is also the default. So lets use it first. - double latency = pingResult.getExecutionTimeInMS(); - - if (preferResponseTimeAsLatency && pingResult.getResponseTimeInMS().isPresent()) { - latency = pingResult.getResponseTimeInMS().get(); - } - - return latency; + Duration executionTime = pingResult.getExecutionTime(); + Duration responseTime = pingResult.getResponseTime(); + return preferResponseTimeAsLatency && responseTime != null ? responseTime : executionTime; } @Override public void dhcpRequestReceived(String ipAddress) { - PresenceDetectionValue v = updateReachableValue(PresenceDetectionType.DHCP_REQUEST, 0); - updateListener.partialDetectionResult(v); + updateReachable(DHCP_REQUEST, Duration.ZERO); } /** * Start/Restart a fixed scheduled runner to update the devices reach-ability state. - * - * @param scheduledExecutorService A scheduler to run pings periodically. */ - public void startAutomaticRefresh(ScheduledExecutorService scheduledExecutorService) { + public void startAutomaticRefresh() { ScheduledFuture future = refreshJob; if (future != null && !future.isDone()) { future.cancel(true); } - refreshJob = scheduledExecutorService.scheduleWithFixedDelay(() -> performPresenceDetection(true), 0, - refreshIntervalInMS, TimeUnit.MILLISECONDS); + refreshJob = scheduledExecutorService.scheduleWithFixedDelay(() -> { + try { + logger.debug("Refreshing {} reachability state", hostname); + getValue(); + } catch (InterruptedException | ExecutionException e) { + logger.debug("Failed to refresh {} presence detection", hostname, e); + } + }, 0, refreshInterval.toMillis(), TimeUnit.MILLISECONDS); } /** - * Return true if automatic refreshing is enabled. + * Return true if automatic refreshing is enabled. */ public boolean isAutomaticRefreshing() { return refreshJob != null; @@ -618,17 +580,17 @@ public class PresenceDetection implements IPRequestReceivedCallback { } /** - * Enables listing for dhcp packets to figure out if devices have entered the network. This does not work - * for iOS devices. The hostname of this network service object will be registered to the dhcp request packet + * Enables listening for DHCP packets to figure out if devices have entered the network. This does not work + * for iOS devices. The hostname of this network service object will be registered to the DHCP request packet * listener if enabled and unregistered otherwise. * - * @param destinationAddress the InetAddress to listen for. + * @param destinationAddress the {@link InetAddress} to listen for. */ private void enableDHCPListen(InetAddress destinationAddress) { try { DHCPPacketListenerServer listener = DHCPListenService.register(destinationAddress.getHostAddress(), this); dhcpState = String.format("Bound to port %d - %s", listener.getCurrentPort(), - (listener.usingPrivilegedPort() ? "Running normally" : "Port forwarding necessary !")); + (listener.usingPrivilegedPort() ? "Running normally" : "Port forwarding necessary!")); } catch (SocketException e) { dhcpState = String.format("Cannot use DHCP sniffing: %s", e.getMessage()); logger.warn("{}", dhcpState); diff --git a/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/PresenceDetectionValue.java b/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/PresenceDetectionValue.java index bf47262ec0d..bfec6937e40 100644 --- a/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/PresenceDetectionValue.java +++ b/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/PresenceDetectionValue.java @@ -12,6 +12,9 @@ */ package org.openhab.binding.network.internal; +import static org.openhab.binding.network.internal.utils.NetworkUtils.durationToMillis; + +import java.time.Duration; import java.util.ArrayList; import java.util.List; import java.util.Set; @@ -27,112 +30,26 @@ import org.eclipse.jdt.annotation.NonNullByDefault; */ @NonNullByDefault public class PresenceDetectionValue { - private double latency; - private boolean detectionIsFinished; - private final Set reachableByType = new TreeSet<>(); - private final List tcpServiceReachable = new ArrayList<>(); + + public static final Duration UNREACHABLE = Duration.ofMillis(-1); + private final String hostAddress; + private Duration latency; + + private final Set reachableDetectionTypes = new TreeSet<>(); + private final List reachableTcpPorts = new ArrayList<>(); /** - * Returns true if the target is reachable by any means. - */ - public boolean isReachable() { - return latency >= 0; - } - - /** - * Return the ping latency in ms or -1 if not reachable. Can be 0 if - * no specific latency is known but the device is still reachable. - */ - public double getLowestLatency() { - return latency; - } - - /** - * Return a string of comma separated successful presence detection types. - */ - public String getSuccessfulDetectionTypes() { - return reachableByType.stream().map(v -> v.name()).collect(Collectors.joining(", ")); - } - - /** - * Return the reachable tcp ports of the presence detection value. - * Thread safe. - */ - public List getReachableTCPports() { - synchronized (tcpServiceReachable) { - List copy = new ArrayList<>(); - copy.addAll(tcpServiceReachable); - return copy; - } - } - - /** - * Return true if the presence detection is fully completed (no running - * threads anymore). - */ - public boolean isFinished() { - return detectionIsFinished; - } - - ////// Package private methods ////// - - /** - * Create a new PresenceDetectionValue with an initial latency. + * Create a new {@link PresenceDetectionValue} with an initial latency. * * @param hostAddress The target IP - * @param latency The ping latency in ms. Can be <0 if the device is not reachable. + * @param latency The ping latency. Can be <0 if the device is not reachable. */ - PresenceDetectionValue(String hostAddress, double latency) { + PresenceDetectionValue(String hostAddress, Duration latency) { this.hostAddress = hostAddress; this.latency = latency; } - /** - * Add a successful PresenceDetectionType. - * - * @param type A PresenceDetectionType. - */ - void addType(PresenceDetectionType type) { - reachableByType.add(type); - } - - /** - * Called by {@see PresenceDetection} by all different means of presence detections. - * If the given latency is lower than the already stored one, the stored one will be overwritten. - * - * @param newLatency The new latency. - * @return Returns true if the latency was indeed lower and updated the stored one. - */ - boolean updateLatency(double newLatency) { - if (newLatency < 0) { - throw new IllegalArgumentException( - "Latency must be >=0. Create a new PresenceDetectionValue for a not reachable device!"); - } - if (newLatency > 0 && (latency == 0 || newLatency < latency)) { - latency = newLatency; - return true; - } - return false; - } - - /** - * Add a reachable tcp port to this presence detection result value object. - * Thread safe. - */ - void addReachableTcpService(int tcpPort) { - synchronized (tcpServiceReachable) { - tcpServiceReachable.add(tcpPort); - } - } - - /** - * Mark the result value as final. No modifications should occur after this call. - */ - void setDetectionIsFinished(boolean detectionIsFinished) { - this.detectionIsFinished = detectionIsFinished; - } - /** * Return the host address of the presence detection result object. */ @@ -140,18 +57,97 @@ public class PresenceDetectionValue { return hostAddress; } + /** + * Return the ping latency, {@value #UNREACHABLE} if not reachable. Can be 0 if + * no specific latency is known but the device is still reachable. + */ + public Duration getLowestLatency() { + return latency; + } + + /** + * Returns true if the target is reachable by any means. + */ + public boolean isReachable() { + return !UNREACHABLE.equals(latency); + } + + /** + * Called by {@see PresenceDetection} by all different means of presence detections. + * If the given latency is lower than the already stored one, the stored one will be overwritten. + * + * @param newLatency The new latency. + * @return Returns true if the latency was indeed lower and updated the stored one. + * @throws IllegalArgumentException when {@code newLatency} is negative + */ + boolean updateLatency(Duration newLatency) { + if (newLatency.isNegative()) { + throw new IllegalArgumentException( + "Latency must be >=0. Create a new PresenceDetectionValue for a not reachable device!"); + } else if (newLatency.isZero()) { + return false; + } else if (!isReachable() || latency.isZero() || newLatency.compareTo(latency) < 0) { + latency = newLatency; + return true; + } + return false; + } + + /** + * Add a successful {@link PresenceDetectionType}. + * + * @param type a {@link PresenceDetectionType}. + */ + void addReachableDetectionType(PresenceDetectionType type) { + reachableDetectionTypes.add(type); + } + /** * Return true if the target can be reached by ICMP or ARP pings. */ public boolean isPingReachable() { - return reachableByType.contains(PresenceDetectionType.ARP_PING) - || reachableByType.contains(PresenceDetectionType.ICMP_PING); + return reachableDetectionTypes.contains(PresenceDetectionType.ARP_PING) + || reachableDetectionTypes.contains(PresenceDetectionType.ICMP_PING); } /** * Return true if the target provides open TCP ports. */ - public boolean isTCPServiceReachable() { - return reachableByType.contains(PresenceDetectionType.TCP_CONNECTION); + public boolean isTcpServiceReachable() { + return reachableDetectionTypes.contains(PresenceDetectionType.TCP_CONNECTION); + } + + /** + * Return a string of comma-separated successful presence detection types. + */ + public String getSuccessfulDetectionTypes() { + return reachableDetectionTypes.stream().map(PresenceDetectionType::name).collect(Collectors.joining(", ")); + } + + /** + * Return the reachable TCP ports of the presence detection value. + * Thread safe. + */ + public List getReachableTcpPorts() { + synchronized (reachableTcpPorts) { + return new ArrayList<>(reachableTcpPorts); + } + } + + /** + * Add a reachable TCP port to this presence detection result value object. + * Thread safe. + */ + void addReachableTcpPort(int tcpPort) { + synchronized (reachableTcpPorts) { + reachableTcpPorts.add(tcpPort); + } + } + + @Override + public String toString() { + return "PresenceDetectionValue [hostAddress=" + hostAddress + ", latency=" + durationToMillis(latency) + + "ms, reachableDetectionTypes=" + reachableDetectionTypes + ", reachableTcpPorts=" + reachableTcpPorts + + "]"; } } diff --git a/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/dhcp/DHCPListenService.java b/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/dhcp/DHCPListenService.java index 4516b02a0c3..e0baabc80fa 100644 --- a/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/dhcp/DHCPListenService.java +++ b/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/dhcp/DHCPListenService.java @@ -34,34 +34,34 @@ import org.slf4j.LoggerFactory; @NonNullByDefault public class DHCPListenService { static @Nullable DHCPPacketListenerServer instance; - private static Map registeredListeners = new TreeMap<>(); - private static Logger logger = LoggerFactory.getLogger(DHCPListenService.class); + private static final Map REGISTERED_LISTENERS = new TreeMap<>(); + private static final Logger LOGGER = LoggerFactory.getLogger(DHCPListenService.class); public static synchronized DHCPPacketListenerServer register(String hostAddress, IPRequestReceivedCallback dhcpListener) throws SocketException { DHCPPacketListenerServer instance = DHCPListenService.instance; if (instance == null) { - instance = new DHCPPacketListenerServer((String ipAddress) -> { - IPRequestReceivedCallback listener = registeredListeners.get(ipAddress); + instance = new DHCPPacketListenerServer(ipAddress -> { + IPRequestReceivedCallback listener = REGISTERED_LISTENERS.get(ipAddress); if (listener != null) { listener.dhcpRequestReceived(ipAddress); } else { - logger.trace("DHCP request for unknown address: {}", ipAddress); + LOGGER.trace("DHCP request for unknown address: {}", ipAddress); } }); DHCPListenService.instance = instance; instance.start(); } - synchronized (registeredListeners) { - registeredListeners.put(hostAddress, dhcpListener); + synchronized (REGISTERED_LISTENERS) { + REGISTERED_LISTENERS.put(hostAddress, dhcpListener); } return instance; } public static void unregister(String hostAddress) { - synchronized (registeredListeners) { - registeredListeners.remove(hostAddress); - if (!registeredListeners.isEmpty()) { + synchronized (REGISTERED_LISTENERS) { + REGISTERED_LISTENERS.remove(hostAddress); + if (!REGISTERED_LISTENERS.isEmpty()) { return; } } diff --git a/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/dhcp/DHCPPacketListenerServer.java b/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/dhcp/DHCPPacketListenerServer.java index e339658becd..d541e1539ed 100644 --- a/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/dhcp/DHCPPacketListenerServer.java +++ b/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/dhcp/DHCPPacketListenerServer.java @@ -18,7 +18,6 @@ import java.net.DatagramSocket; import java.net.InetAddress; import java.net.InetSocketAddress; import java.net.SocketException; -import java.net.UnknownHostException; import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; @@ -64,7 +63,7 @@ public class DHCPPacketListenerServer extends Thread { } protected void receivePacket(DHCPPacket request, @Nullable InetAddress udpRemote) - throws BadPacketException, UnknownHostException, IOException { + throws BadPacketException, IOException { if (request.getOp() != DHCPPacket.BOOTREQUEST) { return; // skipping non BOOTREQUEST message types } diff --git a/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/discovery/NetworkDiscoveryService.java b/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/discovery/NetworkDiscoveryService.java index 7c4287c5be5..d2cc7de23bd 100644 --- a/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/discovery/NetworkDiscoveryService.java +++ b/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/discovery/NetworkDiscoveryService.java @@ -13,18 +13,18 @@ package org.openhab.binding.network.internal.discovery; import static org.openhab.binding.network.internal.NetworkBindingConstants.*; +import static org.openhab.binding.network.internal.utils.NetworkUtils.durationToMillis; -import java.util.Collections; +import java.time.Duration; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; -import java.util.stream.Collectors; -import java.util.stream.Stream; import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; @@ -56,7 +56,7 @@ import org.slf4j.LoggerFactory; @NonNullByDefault @Component(service = DiscoveryService.class, configurationPid = "discovery.network") public class NetworkDiscoveryService extends AbstractDiscoveryService implements PresenceDetectionListener { - static final int PING_TIMEOUT_IN_MS = 500; + static final Duration PING_TIMEOUT = Duration.ofMillis(500); static final int MAXIMUM_IPS_PER_INTERFACE = 255; private static final long DISCOVERY_RESULT_TTL = TimeUnit.MINUTES.toSeconds(10); private final Logger logger = LoggerFactory.getLogger(NetworkDiscoveryService.class); @@ -64,16 +64,16 @@ public class NetworkDiscoveryService extends AbstractDiscoveryService implements // TCP port 548 (Apple Filing Protocol (AFP)) // TCP port 554 (Windows share / Linux samba) // TCP port 1025 (Xbox / MS-RPC) - private Set tcpServicePorts = Collections - .unmodifiableSet(Stream.of(80, 548, 554, 1025).collect(Collectors.toSet())); + private Set tcpServicePorts = Set.of(80, 548, 554, 1025); private AtomicInteger scannedIPcount = new AtomicInteger(0); private @Nullable ExecutorService executorService = null; private final NetworkBindingConfiguration configuration = new NetworkBindingConfiguration(); private final NetworkUtils networkUtils = new NetworkUtils(); public NetworkDiscoveryService() { - super(SUPPORTED_THING_TYPES_UIDS, (int) Math.round( - new NetworkUtils().getNetworkIPs(MAXIMUM_IPS_PER_INTERFACE).size() * (PING_TIMEOUT_IN_MS / 1000.0)), + super(SUPPORTED_THING_TYPES_UIDS, + (int) Math.round(new NetworkUtils().getNetworkIPs(MAXIMUM_IPS_PER_INTERFACE).size() + * (durationToMillis(PING_TIMEOUT) / 1000.0)), false); } @@ -108,8 +108,8 @@ public class NetworkDiscoveryService extends AbstractDiscoveryService implements final String ip = value.getHostAddress(); if (value.isPingReachable()) { newPingDevice(ip); - } else if (value.isTCPServiceReachable()) { - List tcpServices = value.getReachableTCPports(); + } else if (value.isTcpServiceReachable()) { + List tcpServices = value.getReachableTcpPorts(); for (int port : tcpServices) { newServiceDevice(ip, port); } @@ -139,20 +139,24 @@ public class NetworkDiscoveryService extends AbstractDiscoveryService implements scannedIPcount.set(0); for (String ip : networkIPs) { - final PresenceDetection s = new PresenceDetection(this, 2000); - s.setHostname(ip); - s.setIOSDevice(true); - s.setUseDhcpSniffing(false); - s.setTimeout(PING_TIMEOUT_IN_MS); + final PresenceDetection pd = new PresenceDetection(this, scheduler, Duration.ofSeconds(2)); + pd.setHostname(ip); + pd.setIOSDevice(true); + pd.setUseDhcpSniffing(false); + pd.setTimeout(PING_TIMEOUT); // Ping devices - s.setUseIcmpPing(true); - s.setUseArpPing(true, configuration.arpPingToolPath, configuration.arpPingUtilMethod); + pd.setUseIcmpPing(true); + pd.setUseArpPing(true, configuration.arpPingToolPath, configuration.arpPingUtilMethod); // TCP devices - s.setServicePorts(tcpServicePorts); + pd.setServicePorts(tcpServicePorts); service.execute(() -> { Thread.currentThread().setName("Discovery thread " + ip); - s.performPresenceDetection(true); + try { + pd.getValue(); + } catch (ExecutionException | InterruptedException e) { + stopScan(); + } int count = scannedIPcount.incrementAndGet(); if (count == networkIPs.size()) { logger.trace("Scan of {} IPs successful", scannedIPcount); @@ -171,7 +175,7 @@ public class NetworkDiscoveryService extends AbstractDiscoveryService implements } try { - service.awaitTermination(PING_TIMEOUT_IN_MS, TimeUnit.MILLISECONDS); + service.awaitTermination(PING_TIMEOUT.toMillis(), TimeUnit.MILLISECONDS); } catch (InterruptedException e) { Thread.currentThread().interrupt(); // Reset interrupt flag } @@ -193,26 +197,16 @@ public class NetworkDiscoveryService extends AbstractDiscoveryService implements public void newServiceDevice(String ip, int tcpPort) { logger.trace("Found reachable service for device with IP address {} on port {}", ip, tcpPort); - String label; // TCP port 548 (Apple Filing Protocol (AFP)) // TCP port 554 (Windows share / Linux samba) // TCP port 1025 (Xbox / MS-RPC) - switch (tcpPort) { - case 80: - label = "Device providing a Webserver"; - break; - case 548: - label = "Device providing the Apple AFP Service"; - break; - case 554: - label = "Device providing Network/Samba Shares"; - break; - case 1025: - label = "Device providing Xbox/MS-RPC Capability"; - break; - default: - label = "Network Device"; - } + String label = switch (tcpPort) { + case 80 -> "Device providing a Webserver"; + case 548 -> "Device providing the Apple AFP Service"; + case 554 -> "Device providing Network/Samba Shares"; + case 1025 -> "Device providing Xbox/MS-RPC Capability"; + default -> "Network Device"; + }; label += " (" + ip + ":" + tcpPort + ")"; Map properties = new HashMap<>(); @@ -235,8 +229,7 @@ public class NetworkDiscoveryService extends AbstractDiscoveryService implements public void newPingDevice(String ip) { logger.trace("Found pingable network device with IP address {}", ip); - Map properties = new HashMap<>(); - properties.put(PARAMETER_HOSTNAME, ip); + Map properties = Map.of(PARAMETER_HOSTNAME, ip); thingDiscovered(DiscoveryResultBuilder.create(createPingUID(ip)).withTTL(DISCOVERY_RESULT_TTL) .withProperties(properties).withLabel("Network Device (" + ip + ")").build()); } diff --git a/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/handler/NetworkHandler.java b/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/handler/NetworkHandler.java index 10d7780fbb7..469be159e0f 100644 --- a/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/handler/NetworkHandler.java +++ b/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/handler/NetworkHandler.java @@ -13,7 +13,9 @@ package org.openhab.binding.network.internal.handler; import static org.openhab.binding.network.internal.NetworkBindingConstants.*; +import static org.openhab.binding.network.internal.utils.NetworkUtils.durationToMillis; +import java.time.Duration; import java.time.Instant; import java.time.ZonedDateTime; import java.util.Collection; @@ -33,7 +35,6 @@ import org.openhab.binding.network.internal.PresenceDetectionValue; import org.openhab.binding.network.internal.WakeOnLanPacketSender; import org.openhab.binding.network.internal.action.NetworkActions; import org.openhab.core.library.types.DateTimeType; -import org.openhab.core.library.types.DecimalType; import org.openhab.core.library.types.OnOffType; import org.openhab.core.library.types.QuantityType; import org.openhab.core.library.unit.MetricPrefix; @@ -95,18 +96,16 @@ public class NetworkHandler extends BaseThingHandler presenceDetection.getValue(value -> updateState(CHANNEL_ONLINE, OnOffType.from(value.isReachable()))); break; case CHANNEL_LATENCY: - case CHANNEL_DEPRECATED_TIME: presenceDetection.getValue(value -> { - updateState(CHANNEL_LATENCY, - new QuantityType<>(value.getLowestLatency(), MetricPrefix.MILLI(Units.SECOND))); - updateState(CHANNEL_DEPRECATED_TIME, new DecimalType(value.getLowestLatency())); + double latencyMs = durationToMillis(value.getLowestLatency()); + updateState(CHANNEL_LATENCY, new QuantityType<>(latencyMs, MetricPrefix.MILLI(Units.SECOND))); }); break; case CHANNEL_LASTSEEN: - if (presenceDetection.getLastSeen() > 0) { - Instant instant = Instant.ofEpochMilli(presenceDetection.getLastSeen()); + Instant lastSeen = presenceDetection.getLastSeen(); + if (lastSeen != null) { updateState(CHANNEL_LASTSEEN, new DateTimeType( - ZonedDateTime.ofInstant(instant, TimeZone.getDefault().toZoneId()).withFixedOffsetZone())); + ZonedDateTime.ofInstant(lastSeen, TimeZone.getDefault().toZoneId()).withFixedOffsetZone())); } else { updateState(CHANNEL_LASTSEEN, UnDefType.UNDEF); } @@ -128,28 +127,29 @@ public class NetworkHandler extends BaseThingHandler @Override public void partialDetectionResult(PresenceDetectionValue value) { + double latencyMs = durationToMillis(value.getLowestLatency()); updateState(CHANNEL_ONLINE, OnOffType.ON); - updateState(CHANNEL_LATENCY, new QuantityType<>(value.getLowestLatency(), MetricPrefix.MILLI(Units.SECOND))); - updateState(CHANNEL_DEPRECATED_TIME, new DecimalType(value.getLowestLatency())); + updateState(CHANNEL_LATENCY, new QuantityType<>(latencyMs, MetricPrefix.MILLI(Units.SECOND))); } @Override public void finalDetectionResult(PresenceDetectionValue value) { // We do not notify the framework immediately if a device presence detection failed and // the user configured retries to be > 1. - retryCounter = !value.isReachable() ? retryCounter + 1 : 0; + retryCounter = value.isReachable() ? 0 : retryCounter + 1; - if (retryCounter >= this.retries) { + if (retryCounter >= retries) { updateState(CHANNEL_ONLINE, OnOffType.OFF); updateState(CHANNEL_LATENCY, UnDefType.UNDEF); - updateState(CHANNEL_DEPRECATED_TIME, UnDefType.UNDEF); retryCounter = 0; } - if (value.isReachable()) { - Instant instant = Instant.ofEpochMilli(presenceDetection.getLastSeen()); + Instant lastSeen = presenceDetection.getLastSeen(); + if (value.isReachable() && lastSeen != null) { updateState(CHANNEL_LASTSEEN, new DateTimeType( - ZonedDateTime.ofInstant(instant, TimeZone.getDefault().toZoneId()).withFixedOffsetZone())); + ZonedDateTime.ofInstant(lastSeen, TimeZone.getDefault().toZoneId()).withFixedOffsetZone())); + } else if (!value.isReachable() && lastSeen == null) { + updateState(CHANNEL_LASTSEEN, UnDefType.UNDEF); } updateNetworkProperties(); @@ -196,14 +196,14 @@ public class NetworkHandler extends BaseThingHandler } this.retries = handlerConfiguration.retry.intValue(); - presenceDetection.setRefreshInterval(handlerConfiguration.refreshInterval.longValue()); - presenceDetection.setTimeout(handlerConfiguration.timeout.intValue()); + presenceDetection.setRefreshInterval(Duration.ofMillis(handlerConfiguration.refreshInterval)); + presenceDetection.setTimeout(Duration.ofMillis(handlerConfiguration.timeout)); wakeOnLanPacketSender = new WakeOnLanPacketSender(handlerConfiguration.macAddress, handlerConfiguration.hostname, handlerConfiguration.port, handlerConfiguration.networkInterfaceNames); updateStatus(ThingStatus.ONLINE); - presenceDetection.startAutomaticRefresh(scheduler); + presenceDetection.startAutomaticRefresh(); updateNetworkProperties(); } @@ -222,7 +222,8 @@ public class NetworkHandler extends BaseThingHandler // Create a new network service and apply all configurations. @Override public void initialize() { - initialize(new PresenceDetection(this, configuration.cacheDeviceStateTimeInMS.intValue())); + initialize(new PresenceDetection(this, scheduler, + Duration.ofMillis(configuration.cacheDeviceStateTimeInMS.intValue()))); } /** diff --git a/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/handler/SpeedTestHandler.java b/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/handler/SpeedTestHandler.java index 4b05979289a..f3bfb8b75cf 100644 --- a/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/handler/SpeedTestHandler.java +++ b/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/handler/SpeedTestHandler.java @@ -131,7 +131,6 @@ public class SpeedTestHandler extends BaseThingHandler implements ISpeedTestList if (SpeedTestError.UNSUPPORTED_PROTOCOL.equals(testError) || SpeedTestError.MALFORMED_URI.equals(testError)) { updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR, errorMessage); freeRefreshTask(); - return; } else if (SpeedTestError.SOCKET_TIMEOUT.equals(testError)) { timeouts--; if (timeouts <= 0) { @@ -141,12 +140,10 @@ public class SpeedTestHandler extends BaseThingHandler implements ISpeedTestList logger.warn("Speedtest timed out, {} attempts left. Message '{}'", timeouts, errorMessage); stopSpeedTest(); } - return; } else if (SpeedTestError.SOCKET_ERROR.equals(testError) || SpeedTestError.INVALID_HTTP_RESPONSE.equals(testError)) { updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, errorMessage); freeRefreshTask(); - return; } else { stopSpeedTest(); logger.warn("Speedtest failed: {}", errorMessage); diff --git a/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/toberemoved/cache/ExpiringCacheAsync.java b/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/toberemoved/cache/ExpiringCacheAsync.java deleted file mode 100644 index 992fe235d46..00000000000 --- a/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/toberemoved/cache/ExpiringCacheAsync.java +++ /dev/null @@ -1,143 +0,0 @@ -/** - * Copyright (c) 2010-2024 Contributors to the openHAB project - * - * See the NOTICE file(s) distributed with this work for additional - * information. - * - * This program and the accompanying materials are made available under the - * terms of the Eclipse Public License 2.0 which is available at - * http://www.eclipse.org/legal/epl-2.0 - * - * SPDX-License-Identifier: EPL-2.0 - */ -package org.openhab.binding.network.internal.toberemoved.cache; - -import java.util.LinkedList; -import java.util.List; -import java.util.concurrent.TimeUnit; -import java.util.function.Consumer; - -import org.eclipse.jdt.annotation.NonNullByDefault; -import org.eclipse.jdt.annotation.Nullable; - -/** - * Complementary class to {@link org.openhab.core.cache.ExpiringCache}, implementing an async variant - * of an expiring cache. Returns the cached value immediately to the callback if not expired yet, otherwise issue - * a fetch and notify callback implementors asynchronously. - * - * @author David Graeff - Initial contribution - * - * @param the type of the cached value - */ -@NonNullByDefault -public class ExpiringCacheAsync { - private final long expiry; - private ExpiringCacheUpdate cacheUpdater; - long expiresAt = 0; - private boolean refreshRequested = false; - private V value; - private final List> waitingCacheCallbacks = new LinkedList<>(); - - /** - * Implement the requestCacheUpdate method which will be called when the cache - * needs an updated value. Call {@see setValue} to update the cached value. - */ - public static interface ExpiringCacheUpdate { - void requestCacheUpdate(); - } - - /** - * Create a new instance. - * - * @param expiry the duration in milliseconds for how long the value stays valid. Must be greater than 0. - * @param cacheUpdater The cache will use this callback if a new value is needed. Must not be null. - * @throws IllegalArgumentException For an expire value {@literal <=0} or a null cacheUpdater. - */ - public ExpiringCacheAsync(long expiry, @Nullable ExpiringCacheUpdate cacheUpdater) throws IllegalArgumentException { - if (expiry <= 0) { - throw new IllegalArgumentException("Cache expire time must be greater than 0"); - } - if (cacheUpdater == null) { - throw new IllegalArgumentException("A cache updater is necessary"); - } - this.expiry = TimeUnit.MILLISECONDS.toNanos(expiry); - this.cacheUpdater = cacheUpdater; - } - - /** - * Returns the value - possibly from the cache, if it is still valid. - * - * @param callback callback to return the value - */ - public void getValue(Consumer callback) { - if (isExpired()) { - refreshValue(callback); - } else { - callback.accept(value); - } - } - - /** - * Invalidates the value in the cache. - */ - public void invalidateValue() { - expiresAt = 0; - } - - /** - * Updates the cached value with the given one. - * - * @param newValue The new value. All listeners, registered by getValueAsync() and refreshValue(), will be notified - * of the new value. - */ - public void setValue(V newValue) { - refreshRequested = false; - value = newValue; - expiresAt = getCurrentNanoTime() + expiry; - // Inform all callback handlers of the new value and clear the list - for (Consumer callback : waitingCacheCallbacks) { - callback.accept(value); - } - waitingCacheCallbacks.clear(); - } - - /** - * Returns an arbitrary time reference in nanoseconds. - * This is used for the cache to determine if a value has expired. - */ - public long getCurrentNanoTime() { - return System.nanoTime(); - } - - /** - * Refreshes and returns the value asynchronously. - * - * @return the new value - */ - private void refreshValue(Consumer callback) { - waitingCacheCallbacks.add(callback); - if (refreshRequested) { - return; - } - refreshRequested = true; - expiresAt = 0; - cacheUpdater.requestCacheUpdate(); - } - - /** - * Checks if the value is expired. - * - * @return true if the value is expired - */ - public boolean isExpired() { - return expiresAt < getCurrentNanoTime(); - } - - /** - * Return the raw value, no matter if it is already - * expired or still valid. - */ - public V getExpiredValue() { - return value; - } -} diff --git a/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/utils/LatencyParser.java b/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/utils/LatencyParser.java index f8222bd9a28..416d455edfd 100644 --- a/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/utils/LatencyParser.java +++ b/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/utils/LatencyParser.java @@ -12,11 +12,14 @@ */ package org.openhab.binding.network.internal.utils; -import java.util.Optional; +import static org.openhab.binding.network.internal.utils.NetworkUtils.millisToDuration; + +import java.time.Duration; import java.util.regex.Matcher; import java.util.regex.Pattern; import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -44,18 +47,18 @@ public class LatencyParser { * Examine a single ping command output line and try to extract the latency value if it is contained. * * @param inputLine Single output line of the ping command. - * @return Latency value provided by the ping command. Optional is empty if the provided line did not contain a + * @return Latency value provided by the ping command. null if the provided line did not contain a * latency value which matches the known patterns. */ - public Optional parseLatency(String inputLine) { + public @Nullable Duration parseLatency(String inputLine) { logger.debug("Parsing latency from input {}", inputLine); Matcher m = LATENCY_PATTERN.matcher(inputLine); if (m.find() && m.groupCount() == 1) { - return Optional.of(Double.parseDouble(m.group(1))); + return millisToDuration(Double.parseDouble(m.group(1))); } logger.debug("Did not find a latency value"); - return Optional.empty(); + return null; } } diff --git a/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/utils/NetworkUtils.java b/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/utils/NetworkUtils.java index 921d0c1cc9a..f86f0096418 100644 --- a/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/utils/NetworkUtils.java +++ b/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/utils/NetworkUtils.java @@ -25,17 +25,16 @@ import java.net.NetworkInterface; import java.net.NoRouteToHostException; import java.net.PortUnreachableException; import java.net.Socket; -import java.net.SocketAddress; import java.net.SocketException; import java.net.SocketTimeoutException; import java.net.UnknownHostException; import java.time.Duration; +import java.time.Instant; import java.util.ArrayList; import java.util.Enumeration; import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; -import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; @@ -54,6 +53,37 @@ import org.slf4j.LoggerFactory; */ @NonNullByDefault public class NetworkUtils { + + /** + * Nanos per millisecond. + */ + private static final long NANOS_PER_MILLI = 1000_000L; + + /** + * Converts a {@link Duration} to milliseconds. + *

+ * The result has a greater than millisecond precision compared to {@link Duration#toMillis()} which drops excess + * precision information. + * + * @param duration the {@link Duration} to be converted + * @return the equivalent milliseconds of the given {@link Duration} + */ + public static double durationToMillis(Duration duration) { + return (double) duration.toNanos() / NANOS_PER_MILLI; + } + + /** + * Converts a double representing milliseconds to a {@link Duration} instance. + *

+ * The result has a greater than millisecond precision compared to {@link Duration#ofMillis(long)}. + * + * @param millis the milliseconds to be converted + * @return a {@link Duration} instance representing the given milliseconds + */ + public static Duration millisToDuration(double millis) { + return Duration.ofNanos((long) (millis * NANOS_PER_MILLI)); + } + private final Logger logger = LoggerFactory.getLogger(NetworkUtils.class); private LatencyParser latencyParser = new LatencyParser(); @@ -93,7 +123,7 @@ public class NetworkUtils { result.add(InetAddress.getByAddress(segments).getHostAddress()); } } catch (UnknownHostException e) { - logger.debug("Could not build net ip address.", e); + logger.trace("Could not build net IP address.", e); } return result; } @@ -107,15 +137,14 @@ public class NetworkUtils { Set result = new HashSet<>(); try { - // For each interface ... for (Enumeration en = NetworkInterface.getNetworkInterfaces(); en.hasMoreElements();) { NetworkInterface networkInterface = en.nextElement(); if (!networkInterface.isLoopback()) { result.add(networkInterface.getName()); } } - } catch (SocketException ignored) { - // If we are not allowed to enumerate, we return an empty result set. + } catch (SocketException e) { + logger.trace("Could not get network interfaces", e); } return result; @@ -139,7 +168,7 @@ public class NetworkUtils { * @return Every single IP which can be assigned on the Networks the computer is connected to */ private Set getNetworkIPs(Set interfaceIPs, int maximumPerInterface) { - LinkedHashSet networkIPs = new LinkedHashSet<>(); + Set networkIPs = new LinkedHashSet<>(); short minCidrPrefixLength = 8; // historic Class A network, addresses = 16777214 if (maximumPerInterface != 0) { @@ -176,25 +205,24 @@ public class NetworkUtils { } /** - * Try to establish a tcp connection to the given port. Returns false if a timeout occurred - * or the connection was denied. + * Try to establish a TCP connection to the given port. * - * @param host The IP or hostname - * @param port The tcp port. Must be not 0. - * @param timeout Timeout in ms - * @return Ping result information. Optional is empty if ping command was not executed. - * @throws IOException + * @param host the IP or hostname + * @param port the TCP port. Must be not 0. + * @param timeout the timeout before the call aborts + * @return the {@link PingResult} of connecting to the given port + * @throws IOException if an error occurs during the connection */ - public Optional servicePing(String host, int port, int timeout) throws IOException { - double execStartTimeInMS = System.currentTimeMillis(); - - SocketAddress socketAddress = new InetSocketAddress(host, port); + public PingResult servicePing(String host, int port, Duration timeout) throws IOException { + Instant execStartTime = Instant.now(); + boolean success = false; try (Socket socket = new Socket()) { - socket.connect(socketAddress, timeout); - return Optional.of(new PingResult(true, System.currentTimeMillis() - execStartTimeInMS)); - } catch (ConnectException | SocketTimeoutException | NoRouteToHostException ignored) { - return Optional.of(new PingResult(false, System.currentTimeMillis() - execStartTimeInMS)); + socket.connect(new InetSocketAddress(host, port), (int) timeout.toMillis()); + success = true; + } catch (ConnectException | SocketTimeoutException | NoRouteToHostException e) { + logger.trace("Could not connect to {}:{}", host, port, e); } + return new PingResult(success, Duration.between(execStartTime, Instant.now())); } /** @@ -221,11 +249,12 @@ public class NetworkUtils { } try { - Optional pingResult = nativePing(method, "127.0.0.1", 1000); - if (pingResult.isPresent() && pingResult.get().isSuccess()) { + PingResult pingResult = nativePing(method, "127.0.0.1", Duration.ofSeconds(1)); + if (pingResult != null && pingResult.isSuccess()) { return method; } - } catch (IOException ignored) { + } catch (IOException e) { + logger.trace("Native ping to 127.0.0.1 failed", e); } catch (InterruptedException e) { Thread.currentThread().interrupt(); // Reset interrupt flag } @@ -233,11 +262,12 @@ public class NetworkUtils { } /** - * Return true if the external arp ping utility (arping) is available and executable on the given path. + * Return true if the external ARP ping utility (arping) is available and executable on the given path. */ - public ArpPingUtilEnum determineNativeARPpingMethod(String arpToolPath) { + public ArpPingUtilEnum determineNativeArpPingMethod(String arpToolPath) { String result = ExecUtil.executeCommandLineAndWaitResponse(Duration.ofMillis(100), arpToolPath, "--help"); if (result == null || result.isBlank()) { + logger.trace("The command did not return a response due to an error or timeout"); return ArpPingUtilEnum.DISABLED_UNKNOWN_TOOL; } else if (result.contains("Thomas Habets")) { if (result.matches("(?s)(.*)w sec Specify a timeout(.*)")) { @@ -249,8 +279,10 @@ public class NetworkUtils { return ArpPingUtilEnum.IPUTILS_ARPING; } else if (result.contains("Usage: arp-ping.exe")) { return ArpPingUtilEnum.ELI_FULKERSON_ARP_PING_FOR_WINDOWS; + } else { + logger.trace("The command output did not match any known output"); + return ArpPingUtilEnum.DISABLED_UNKNOWN_TOOL; } - return ArpPingUtilEnum.DISABLED_UNKNOWN_TOOL; } public enum IpPingMethodEnum { @@ -264,35 +296,36 @@ public class NetworkUtils { * Use the native ping utility of the operating system to detect device presence. * * @param hostname The DNS name, IPv4 or IPv6 address. Must not be null. - * @param timeoutInMS Timeout in milliseconds. Be aware that DNS resolution is not part of this timeout. - * @return Ping result information. Optional is empty if ping command was not executed. + * @param timeout the timeout before the call aborts. Be aware that DNS resolution is not part of this timeout. + * @return Ping result information. null if ping command was not executed. * @throws IOException The ping command could probably not be found */ - public Optional nativePing(@Nullable IpPingMethodEnum method, String hostname, int timeoutInMS) + public @Nullable PingResult nativePing(@Nullable IpPingMethodEnum method, String hostname, Duration timeout) throws IOException, InterruptedException { - double execStartTimeInMS = System.currentTimeMillis(); + Instant execStartTime = Instant.now(); Process proc; if (method == null) { - return Optional.empty(); + return null; } // Yes, all supported operating systems have their own ping utility with a different command line switch (method) { case IPUTILS_LINUX_PING: - proc = new ProcessBuilder("ping", "-w", String.valueOf(timeoutInMS / 1000), "-c", "1", hostname) + proc = new ProcessBuilder("ping", "-w", String.valueOf(timeout.toSeconds()), "-c", "1", hostname) .start(); break; case MAC_OS_PING: - proc = new ProcessBuilder("ping", "-t", String.valueOf(timeoutInMS / 1000), "-c", "1", hostname) + proc = new ProcessBuilder("ping", "-t", String.valueOf(timeout.toSeconds()), "-c", "1", hostname) .start(); break; case WINDOWS_PING: - proc = new ProcessBuilder("ping", "-w", String.valueOf(timeoutInMS), "-n", "1", hostname).start(); + proc = new ProcessBuilder("ping", "-w", String.valueOf(timeout.toMillis()), "-n", "1", hostname) + .start(); break; case JAVA_PING: default: - // We cannot estimate the command line for any other operating system and just return false - return Optional.empty(); + // We cannot estimate the command line for any other operating system and just return null + return null; } // The return code is 0 for a successful ping, 1 if device didn't @@ -303,7 +336,7 @@ public class NetworkUtils { int result = proc.waitFor(); if (result != 0) { - return Optional.of(new PingResult(false, System.currentTimeMillis() - execStartTimeInMS)); + return new PingResult(false, Duration.between(execStartTime, Instant.now())); } try (BufferedReader r = new BufferedReader(new InputStreamReader(proc.getInputStream()))) { @@ -315,14 +348,14 @@ public class NetworkUtils { // Because of the Windows issue, we need to check this. We assume that the ping was successful whenever // this specific string is contained in the output if (line.contains("TTL=") || line.contains("ttl=")) { - PingResult pingResult = new PingResult(true, System.currentTimeMillis() - execStartTimeInMS); - latencyParser.parseLatency(line).ifPresent(pingResult::setResponseTimeInMS); - return Optional.of(pingResult); + PingResult pingResult = new PingResult(true, Duration.between(execStartTime, Instant.now())); + pingResult.setResponseTime(latencyParser.parseLatency(line)); + return pingResult; } line = r.readLine(); } while (line != null); - return Optional.of(new PingResult(false, System.currentTimeMillis() - execStartTimeInMS)); + return new PingResult(false, Duration.between(execStartTime, Instant.now())); } } @@ -346,76 +379,78 @@ public class NetworkUtils { /** * Execute the arping tool to perform an ARP ping (only for IPv4 addresses). - * There exist two different arping utils with the same name unfortunatelly. - * * iputils arping which is sometimes preinstalled on fedora/ubuntu and the - * * https://github.com/ThomasHabets/arping which also works on Windows and MacOS. + * There exist two different arping utils with the same name unfortunately. + *

    + *
  • iputils arping which is sometimes preinstalled on Fedora/Ubuntu and the + *
  • https://github.com/ThomasHabets/arping which also works on Windows and macOS. + *
* * @param arpUtilPath The arping absolute path including filename. Example: "arping" or "/usr/bin/arping" or * "C:\something\arping.exe" or "arp-ping.exe" * @param interfaceName An interface name, on linux for example "wlp58s0", shown by ifconfig. Must not be null. * @param ipV4address The ipV4 address. Must not be null. - * @param timeoutInMS A timeout in milliseconds - * @return Ping result information. Optional is empty if ping command was not executed. + * @param timeout the timeout before the call aborts + * @return Ping result information. null if ping command was not executed. * @throws IOException The ping command could probably not be found */ - public Optional nativeARPPing(@Nullable ArpPingUtilEnum arpingTool, @Nullable String arpUtilPath, - String interfaceName, String ipV4address, int timeoutInMS) throws IOException, InterruptedException { - double execStartTimeInMS = System.currentTimeMillis(); - + public @Nullable PingResult nativeArpPing(@Nullable ArpPingUtilEnum arpingTool, @Nullable String arpUtilPath, + String interfaceName, String ipV4address, Duration timeout) throws IOException, InterruptedException { if (arpUtilPath == null || arpingTool == null || !arpingTool.canProceed) { - return Optional.empty(); + return null; } + Instant execStartTime = Instant.now(); Process proc; if (arpingTool == ArpPingUtilEnum.THOMAS_HABERT_ARPING_WITHOUT_TIMEOUT) { proc = new ProcessBuilder(arpUtilPath, "-c", "1", "-i", interfaceName, ipV4address).start(); } else if (arpingTool == ArpPingUtilEnum.THOMAS_HABERT_ARPING) { - proc = new ProcessBuilder(arpUtilPath, "-w", String.valueOf(timeoutInMS / 1000), "-C", "1", "-i", + proc = new ProcessBuilder(arpUtilPath, "-w", String.valueOf(timeout.toSeconds()), "-C", "1", "-i", interfaceName, ipV4address).start(); } else if (arpingTool == ArpPingUtilEnum.ELI_FULKERSON_ARP_PING_FOR_WINDOWS) { - proc = new ProcessBuilder(arpUtilPath, "-w", String.valueOf(timeoutInMS), "-x", ipV4address).start(); + proc = new ProcessBuilder(arpUtilPath, "-w", String.valueOf(timeout.toMillis()), "-x", ipV4address).start(); } else { - proc = new ProcessBuilder(arpUtilPath, "-w", String.valueOf(timeoutInMS / 1000), "-c", "1", "-I", + proc = new ProcessBuilder(arpUtilPath, "-w", String.valueOf(timeout.toSeconds()), "-c", "1", "-I", interfaceName, ipV4address).start(); } // The return code is 0 for a successful ping. 1 if device didn't respond and 2 if there is another error like // network interface not ready. - return Optional.of(new PingResult(proc.waitFor() == 0, System.currentTimeMillis() - execStartTimeInMS)); + return new PingResult(proc.waitFor() == 0, Duration.between(execStartTime, Instant.now())); } /** * Execute a Java ping. * - * @param timeoutInMS A timeout in milliseconds + * @param timeout the timeout before the call aborts * @param destinationAddress The address to check - * @return Ping result information. Optional is empty if ping command was not executed. + * @return Ping result information */ - public Optional javaPing(int timeoutInMS, InetAddress destinationAddress) { - double execStartTimeInMS = System.currentTimeMillis(); - + public PingResult javaPing(Duration timeout, InetAddress destinationAddress) { + Instant execStartTime = Instant.now(); + boolean success = false; try { - if (destinationAddress.isReachable(timeoutInMS)) { - return Optional.of(new PingResult(true, System.currentTimeMillis() - execStartTimeInMS)); - } else { - return Optional.of(new PingResult(false, System.currentTimeMillis() - execStartTimeInMS)); + if (destinationAddress.isReachable((int) timeout.toMillis())) { + success = true; } } catch (IOException e) { - return Optional.of(new PingResult(false, System.currentTimeMillis() - execStartTimeInMS)); + logger.trace("Could not connect to {}", destinationAddress, e); } + return new PingResult(success, Duration.between(execStartTime, Instant.now())); } /** * iOS devices are in a deep sleep mode, where they only listen to UDP traffic on port 5353 (Bonjour service * discovery). A packet on port 5353 will wake up the network stack to respond to ARP pings at least. * - * @throws IOException + * @throws IOException if an error occurs during the connection */ public void wakeUpIOS(InetAddress address) throws IOException { + int port = 5353; try (DatagramSocket s = new DatagramSocket()) { byte[] buffer = new byte[0]; - s.send(new DatagramPacket(buffer, buffer.length, address, 5353)); - } catch (PortUnreachableException ignored) { - // We ignore the port unreachable error + s.send(new DatagramPacket(buffer, buffer.length, address, port)); + logger.trace("Sent packet to {}:{} to wake up iOS device", address, port); + } catch (PortUnreachableException e) { + logger.trace("Unable to send packet to wake up iOS device at {}:{}", address, port, e); } } } diff --git a/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/utils/PingResult.java b/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/utils/PingResult.java index 9a0ba80bb5c..d14f1209447 100644 --- a/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/utils/PingResult.java +++ b/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/utils/PingResult.java @@ -12,9 +12,12 @@ */ package org.openhab.binding.network.internal.utils; -import java.util.Optional; +import static org.openhab.binding.network.internal.utils.NetworkUtils.durationToMillis; + +import java.time.Duration; import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; /** * Information about the ping result. @@ -25,16 +28,16 @@ import org.eclipse.jdt.annotation.NonNullByDefault; public class PingResult { private boolean success; - private Optional responseTimeInMS = Optional.empty(); - private double executionTimeInMS; + private @Nullable Duration responseTime; + private Duration executionTime; /** * @param success true if the device was reachable, false if not. - * @param executionTimeInMS Execution time of the ping command in ms. + * @param executionTime execution time of the ping command. */ - public PingResult(boolean success, double executionTimeInMS) { + public PingResult(boolean success, Duration executionTime) { this.success = success; - this.executionTimeInMS = executionTimeInMS; + this.executionTime = executionTime; } /** @@ -45,30 +48,32 @@ public class PingResult { } /** - * @return Response time in ms which was returned by the ping command. Optional is empty if response time provided + * @return response time which was returned by the ping command. null if response time provided * by ping command is not available. */ - public Optional getResponseTimeInMS() { - return responseTimeInMS; + public @Nullable Duration getResponseTime() { + return responseTime; } /** - * @param responseTimeInMS Response time in ms which was returned by the ping command. + * @param responseTime the response time which was returned by the ping command. */ - public void setResponseTimeInMS(double responseTimeInMS) { - this.responseTimeInMS = Optional.of(responseTimeInMS); + public void setResponseTime(@Nullable Duration responseTime) { + this.responseTime = responseTime; } @Override public String toString() { - return "PingResult{" + "success=" + success + ", responseTimeInMS=" + responseTimeInMS + ", executionTimeInMS=" - + executionTimeInMS + '}'; + Duration responseTime = this.responseTime; + String rt = responseTime == null ? "null" : durationToMillis(responseTime) + "ms"; + String et = durationToMillis(executionTime) + "ms"; + return "PingResult{success=" + success + ", responseTime=" + rt + ", executionTime=" + et + "}"; } /** - * @return Execution time of the ping command in ms. + * @return Execution time of the ping command. */ - public double getExecutionTimeInMS() { - return executionTimeInMS; + public Duration getExecutionTime() { + return executionTime; } } diff --git a/bundles/org.openhab.binding.network/src/test/java/org/openhab/binding/network/internal/PresenceDetectionTest.java b/bundles/org.openhab.binding.network/src/test/java/org/openhab/binding/network/internal/PresenceDetectionTest.java index 81073f6bcf3..bf9d09fc6a8 100644 --- a/bundles/org.openhab.binding.network/src/test/java/org/openhab/binding/network/internal/PresenceDetectionTest.java +++ b/bundles/org.openhab.binding.network/src/test/java/org/openhab/binding/network/internal/PresenceDetectionTest.java @@ -19,14 +19,13 @@ import static org.mockito.ArgumentMatchers.*; import static org.mockito.Mockito.*; import java.io.IOException; -import java.net.UnknownHostException; -import java.util.Optional; +import java.time.Duration; import java.util.Set; import java.util.concurrent.ExecutorService; -import java.util.concurrent.TimeUnit; +import java.util.concurrent.ScheduledExecutorService; import java.util.function.Consumer; -import org.junit.jupiter.api.AfterEach; +import org.eclipse.jdt.annotation.NonNullByDefault; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -35,8 +34,6 @@ import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import org.mockito.junit.jupiter.MockitoSettings; import org.mockito.quality.Strictness; -import org.openhab.binding.network.internal.toberemoved.cache.ExpiringCacheAsync; -import org.openhab.binding.network.internal.toberemoved.cache.ExpiringCacheHelper; import org.openhab.binding.network.internal.utils.NetworkUtils; import org.openhab.binding.network.internal.utils.NetworkUtils.ArpPingUtilEnum; import org.openhab.binding.network.internal.utils.NetworkUtils.IpPingMethodEnum; @@ -49,32 +46,30 @@ import org.openhab.binding.network.internal.utils.PingResult; */ @ExtendWith(MockitoExtension.class) @MockitoSettings(strictness = Strictness.LENIENT) +@NonNullByDefault public class PresenceDetectionTest { - private static final long CACHETIME = 2000L; - private PresenceDetection subject; + private @NonNullByDefault({}) PresenceDetection subject; - private @Mock Consumer callback; - private @Mock ExecutorService executorService; - private @Mock PresenceDetectionListener listener; - private @Mock NetworkUtils networkUtils; + private @Mock @NonNullByDefault({}) Consumer callback; + private @Mock @NonNullByDefault({}) ExecutorService detectionExecutorService; + private @Mock @NonNullByDefault({}) ScheduledExecutorService scheduledExecutorService; + private @Mock @NonNullByDefault({}) PresenceDetectionListener listener; + private @Mock @NonNullByDefault({}) NetworkUtils networkUtils; @BeforeEach - public void setUp() throws UnknownHostException { + public void setUp() { // Mock an interface when(networkUtils.getInterfaceNames()).thenReturn(Set.of("TESTinterface")); - doReturn(ArpPingUtilEnum.IPUTILS_ARPING).when(networkUtils).determineNativeARPpingMethod(anyString()); + doReturn(ArpPingUtilEnum.IPUTILS_ARPING).when(networkUtils).determineNativeArpPingMethod(anyString()); doReturn(IpPingMethodEnum.WINDOWS_PING).when(networkUtils).determinePingMethod(); - subject = spy(new PresenceDetection(listener, (int) CACHETIME)); + subject = spy(new PresenceDetection(listener, scheduledExecutorService, Duration.ofSeconds(2))); subject.networkUtils = networkUtils; - subject.cache = spy(new ExpiringCacheAsync<>(CACHETIME, () -> { - subject.performPresenceDetection(false); - })); // Set a useful configuration. The default presenceDetection is a no-op. subject.setHostname("127.0.0.1"); - subject.setTimeout(300); + subject.setTimeout(Duration.ofMillis(300)); subject.setUseDhcpSniffing(false); subject.setIOSDevice(true); subject.setServicePorts(Set.of(1010)); @@ -84,83 +79,103 @@ public class PresenceDetectionTest { assertThat(subject.pingMethod, is(IpPingMethodEnum.WINDOWS_PING)); } - @AfterEach - public void shutDown() { - subject.waitForPresenceDetection(); - } - // Depending on the amount of test methods an according amount of threads is spawned. // We will check if they spawn and return in time. @Test public void threadCountTest() { - assertNull(subject.executorService); + assertNull(subject.detectionExecutorService); - doNothing().when(subject).performARPping(any()); - doNothing().when(subject).performJavaPing(); - doNothing().when(subject).performSystemPing(); - doNothing().when(subject).performServicePing(anyInt()); + doNothing().when(subject).performArpPing(any(), any()); + doNothing().when(subject).performJavaPing(any()); + doNothing().when(subject).performSystemPing(any()); + doNothing().when(subject).performServicePing(any(), anyInt()); - subject.performPresenceDetection(false); + subject.getValue(callback -> { + }); // Thread count: ARP + ICMP + 1*TCP assertThat(subject.detectionChecks, is(3)); - assertNotNull(subject.executorService); + assertNotNull(subject.detectionExecutorService); + + // "Wait" for the presence detection to finish + ArgumentCaptor runnableCapture = ArgumentCaptor.forClass(Runnable.class); + verify(scheduledExecutorService, times(1)).execute(runnableCapture.capture()); + runnableCapture.getValue().run(); - subject.waitForPresenceDetection(); assertThat(subject.detectionChecks, is(0)); - assertNull(subject.executorService); + assertNull(subject.detectionExecutorService); } @Test public void partialAndFinalCallbackTests() throws InterruptedException, IOException { - doReturn(Optional.of(new PingResult(true, 10))).when(networkUtils).nativePing(eq(IpPingMethodEnum.WINDOWS_PING), - anyString(), anyInt()); - doReturn(Optional.of(new PingResult(true, 10))).when(networkUtils) - .nativeARPPing(eq(ArpPingUtilEnum.IPUTILS_ARPING), anyString(), anyString(), any(), anyInt()); - doReturn(Optional.of(new PingResult(true, 10))).when(networkUtils).servicePing(anyString(), anyInt(), anyInt()); + PingResult pingResult = new PingResult(true, Duration.ofMillis(10)); + doReturn(pingResult).when(networkUtils).nativePing(eq(IpPingMethodEnum.WINDOWS_PING), anyString(), any()); + doReturn(pingResult).when(networkUtils).nativeArpPing(eq(ArpPingUtilEnum.IPUTILS_ARPING), anyString(), + anyString(), any(), any()); + doReturn(pingResult).when(networkUtils).servicePing(anyString(), anyInt(), any()); - assertTrue(subject.performPresenceDetection(false)); - subject.waitForPresenceDetection(); + doReturn(detectionExecutorService).when(subject).getThreadsFor(anyInt()); - verify(subject, times(0)).performJavaPing(); - verify(subject).performSystemPing(); - verify(subject).performARPping(any()); - verify(subject).performServicePing(anyInt()); + subject.performPresenceDetection(); + + assertThat(subject.detectionChecks, is(3)); + + // Perform the different presence detection threads now + ArgumentCaptor capture = ArgumentCaptor.forClass(Runnable.class); + verify(detectionExecutorService, times(3)).execute(capture.capture()); + for (Runnable r : capture.getAllValues()) { + r.run(); + } + + // "Wait" for the presence detection to finish + ArgumentCaptor runnableCapture = ArgumentCaptor.forClass(Runnable.class); + verify(scheduledExecutorService, times(1)).execute(runnableCapture.capture()); + runnableCapture.getValue().run(); + + assertThat(subject.detectionChecks, is(0)); + + verify(subject, times(0)).performJavaPing(any()); + verify(subject).performSystemPing(any()); + verify(subject).performArpPing(any(), any()); + verify(subject).performServicePing(any(), anyInt()); verify(listener, times(3)).partialDetectionResult(any()); - ArgumentCaptor capture = ArgumentCaptor.forClass(PresenceDetectionValue.class); - verify(listener, times(1)).finalDetectionResult(capture.capture()); + ArgumentCaptor pdvCapture = ArgumentCaptor.forClass(PresenceDetectionValue.class); + verify(listener, times(1)).finalDetectionResult(pdvCapture.capture()); - assertThat(capture.getValue().getSuccessfulDetectionTypes(), is("ARP_PING, ICMP_PING, TCP_CONNECTION")); + assertThat(pdvCapture.getValue().getSuccessfulDetectionTypes(), is("ARP_PING, ICMP_PING, TCP_CONNECTION")); } @Test public void cacheTest() throws InterruptedException, IOException { - doReturn(Optional.of(new PingResult(true, 10))).when(networkUtils).nativePing(eq(IpPingMethodEnum.WINDOWS_PING), - anyString(), anyInt()); - doReturn(Optional.of(new PingResult(true, 10))).when(networkUtils) - .nativeARPPing(eq(ArpPingUtilEnum.IPUTILS_ARPING), anyString(), anyString(), any(), anyInt()); - doReturn(Optional.of(new PingResult(true, 10))).when(networkUtils).servicePing(anyString(), anyInt(), anyInt()); + PingResult pingResult = new PingResult(true, Duration.ofMillis(10)); + doReturn(pingResult).when(networkUtils).nativePing(eq(IpPingMethodEnum.WINDOWS_PING), anyString(), any()); + doReturn(pingResult).when(networkUtils).nativeArpPing(eq(ArpPingUtilEnum.IPUTILS_ARPING), anyString(), + anyString(), any(), any()); + doReturn(pingResult).when(networkUtils).servicePing(anyString(), anyInt(), any()); - doReturn(executorService).when(subject).getThreadsFor(anyInt()); + doReturn(detectionExecutorService).when(subject).getThreadsFor(anyInt()); // We expect no valid value assertTrue(subject.cache.isExpired()); // Get value will issue a PresenceDetection internally. subject.getValue(callback); - verify(subject).performPresenceDetection(eq(false)); - assertNotNull(subject.executorService); + verify(subject).performPresenceDetection(); + assertNotNull(subject.detectionExecutorService); // There should be no straight callback yet verify(callback, times(0)).accept(any()); // Perform the different presence detection threads now ArgumentCaptor capture = ArgumentCaptor.forClass(Runnable.class); - verify(executorService, times(3)).execute(capture.capture()); + verify(detectionExecutorService, times(3)).execute(capture.capture()); for (Runnable r : capture.getAllValues()) { r.run(); } + // "Wait" for the presence detection to finish - subject.waitForPresenceDetection(); + capture = ArgumentCaptor.forClass(Runnable.class); + verify(scheduledExecutorService, times(1)).execute(capture.capture()); + capture.getValue().run(); // Although there are multiple partial results and a final result, // the getValue() consumers get the fastest response possible, and only once. @@ -175,34 +190,4 @@ public class PresenceDetectionTest { subject.getValue(callback); verify(callback, times(2)).accept(any()); } - - @Test - public void reuseValueTests() throws InterruptedException, IOException { - final long startTime = 1000L; - when(subject.cache.getCurrentNanoTime()).thenReturn(TimeUnit.MILLISECONDS.toNanos(startTime)); - - // The PresenceDetectionValue.getLowestLatency() should return the smallest latency - PresenceDetectionValue v = subject.updateReachableValue(PresenceDetectionType.ICMP_PING, 20); - PresenceDetectionValue v2 = subject.updateReachableValue(PresenceDetectionType.ICMP_PING, 19); - assertEquals(v, v2); - assertThat(v.getLowestLatency(), is(19.0)); - - // Advance in time but not expire the cache (1ms left) - final long almostExpire = startTime + CACHETIME - 1; - when(subject.cache.getCurrentNanoTime()).thenReturn(TimeUnit.MILLISECONDS.toNanos(almostExpire)); - - // Updating should reset the expire timer of the cache - v2 = subject.updateReachableValue(PresenceDetectionType.ICMP_PING, 28); - assertEquals(v, v2); - assertThat(v2.getLowestLatency(), is(19.0)); - assertThat(ExpiringCacheHelper.expireTime(subject.cache), - is(TimeUnit.MILLISECONDS.toNanos(almostExpire + CACHETIME))); - - // Cache expire. A new PresenceDetectionValue instance will be returned - when(subject.cache.getCurrentNanoTime()) - .thenReturn(TimeUnit.MILLISECONDS.toNanos(almostExpire + CACHETIME + CACHETIME + 1)); - v2 = subject.updateReachableValue(PresenceDetectionType.ICMP_PING, 25); - assertNotEquals(v, v2); - assertThat(v2.getLowestLatency(), is(25.0)); - } } diff --git a/bundles/org.openhab.binding.network/src/test/java/org/openhab/binding/network/internal/PresenceDetectionValuesTest.java b/bundles/org.openhab.binding.network/src/test/java/org/openhab/binding/network/internal/PresenceDetectionValuesTest.java index 91515260fe2..240d5405160 100644 --- a/bundles/org.openhab.binding.network/src/test/java/org/openhab/binding/network/internal/PresenceDetectionValuesTest.java +++ b/bundles/org.openhab.binding.network/src/test/java/org/openhab/binding/network/internal/PresenceDetectionValuesTest.java @@ -16,6 +16,9 @@ import static org.hamcrest.CoreMatchers.*; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.jupiter.api.Assertions.*; +import java.time.Duration; + +import org.eclipse.jdt.annotation.NonNullByDefault; import org.junit.jupiter.api.Test; /** @@ -23,47 +26,40 @@ import org.junit.jupiter.api.Test; * * @author David Graeff - Initial contribution */ +@NonNullByDefault public class PresenceDetectionValuesTest { @Test public void updateLatencyTests() { - PresenceDetectionValue value = new PresenceDetectionValue("127.0.0.1", 10.0); - assertThat(value.getLowestLatency(), is(10.0)); - value.updateLatency(20.0); - assertThat(value.getLowestLatency(), is(10.0)); - value.updateLatency(0.0); - assertThat(value.getLowestLatency(), is(10.0)); - value.updateLatency(5.0); - assertThat(value.getLowestLatency(), is(5.0)); + PresenceDetectionValue value = new PresenceDetectionValue("127.0.0.1", Duration.ofMillis(10)); + assertThat(value.getLowestLatency(), is(Duration.ofMillis(10))); + value.updateLatency(Duration.ofMillis(20)); + assertThat(value.getLowestLatency(), is(Duration.ofMillis(10))); + value.updateLatency(Duration.ofMillis(0)); + assertThat(value.getLowestLatency(), is(Duration.ofMillis(10))); + value.updateLatency(Duration.ofMillis(5)); + assertThat(value.getLowestLatency(), is(Duration.ofMillis(5))); } @Test public void tcpTests() { - PresenceDetectionValue value = new PresenceDetectionValue("127.0.0.1", 10.0); - assertFalse(value.isTCPServiceReachable()); - value.addReachableTcpService(1010); - assertThat(value.getReachableTCPports(), hasItem(1010)); - value.addType(PresenceDetectionType.TCP_CONNECTION); - assertTrue(value.isTCPServiceReachable()); + PresenceDetectionValue value = new PresenceDetectionValue("127.0.0.1", Duration.ofMillis(10)); + assertFalse(value.isTcpServiceReachable()); + value.addReachableTcpPort(1010); + assertThat(value.getReachableTcpPorts(), hasItem(1010)); + value.addReachableDetectionType(PresenceDetectionType.TCP_CONNECTION); + assertTrue(value.isTcpServiceReachable()); assertThat(value.getSuccessfulDetectionTypes(), is("TCP_CONNECTION")); } - @Test - public void isFinishedTests() { - PresenceDetectionValue value = new PresenceDetectionValue("127.0.0.1", 10.0); - assertFalse(value.isFinished()); - value.setDetectionIsFinished(true); - assertTrue(value.isFinished()); - } - @Test public void pingTests() { - PresenceDetectionValue value = new PresenceDetectionValue("127.0.0.1", 10.0); + PresenceDetectionValue value = new PresenceDetectionValue("127.0.0.1", Duration.ofMillis(10)); assertFalse(value.isPingReachable()); - value.addType(PresenceDetectionType.ICMP_PING); + value.addReachableDetectionType(PresenceDetectionType.ICMP_PING); assertTrue(value.isPingReachable()); - value.addType(PresenceDetectionType.ARP_PING); - value.addType(PresenceDetectionType.TCP_CONNECTION); + value.addReachableDetectionType(PresenceDetectionType.ARP_PING); + value.addReachableDetectionType(PresenceDetectionType.TCP_CONNECTION); assertThat(value.getSuccessfulDetectionTypes(), is("ARP_PING, ICMP_PING, TCP_CONNECTION")); } } diff --git a/bundles/org.openhab.binding.network/src/test/java/org/openhab/binding/network/internal/discovery/DiscoveryTest.java b/bundles/org.openhab.binding.network/src/test/java/org/openhab/binding/network/internal/discovery/DiscoveryTest.java index 1abef4f09a5..d0fb5e772cb 100644 --- a/bundles/org.openhab.binding.network/src/test/java/org/openhab/binding/network/internal/discovery/DiscoveryTest.java +++ b/bundles/org.openhab.binding.network/src/test/java/org/openhab/binding/network/internal/discovery/DiscoveryTest.java @@ -17,8 +17,10 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.*; +import java.time.Duration; import java.util.List; +import org.eclipse.jdt.annotation.NonNullByDefault; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -39,16 +41,17 @@ import org.openhab.core.config.discovery.DiscoveryResult; */ @ExtendWith(MockitoExtension.class) @MockitoSettings(strictness = Strictness.LENIENT) +@NonNullByDefault public class DiscoveryTest { private final String ip = "127.0.0.1"; - private @Mock PresenceDetectionValue value; - private @Mock DiscoveryListener listener; + private @Mock @NonNullByDefault({}) PresenceDetectionValue value; + private @Mock @NonNullByDefault({}) DiscoveryListener listener; @BeforeEach public void setUp() { when(value.getHostAddress()).thenReturn(ip); - when(value.getLowestLatency()).thenReturn(10.0); + when(value.getLowestLatency()).thenReturn(Duration.ofMillis(10)); when(value.isReachable()).thenReturn(true); when(value.getSuccessfulDetectionTypes()).thenReturn("TESTMETHOD"); } @@ -62,7 +65,7 @@ public class DiscoveryTest { // Ping device when(value.isPingReachable()).thenReturn(true); - when(value.isTCPServiceReachable()).thenReturn(false); + when(value.isTcpServiceReachable()).thenReturn(false); d.partialDetectionResult(value); verify(listener).thingDiscovered(any(), result.capture()); DiscoveryResult dresult = result.getValue(); @@ -79,8 +82,8 @@ public class DiscoveryTest { // TCP device when(value.isPingReachable()).thenReturn(false); - when(value.isTCPServiceReachable()).thenReturn(true); - when(value.getReachableTCPports()).thenReturn(List.of(1010)); + when(value.isTcpServiceReachable()).thenReturn(true); + when(value.getReachableTcpPorts()).thenReturn(List.of(1010)); d.partialDetectionResult(value); verify(listener).thingDiscovered(any(), result.capture()); DiscoveryResult dresult = result.getValue(); diff --git a/bundles/org.openhab.binding.network/src/test/java/org/openhab/binding/network/internal/handler/NetworkHandlerTest.java b/bundles/org.openhab.binding.network/src/test/java/org/openhab/binding/network/internal/handler/NetworkHandlerTest.java index c3c59456de4..1b572d71d93 100644 --- a/bundles/org.openhab.binding.network/src/test/java/org/openhab/binding/network/internal/handler/NetworkHandlerTest.java +++ b/bundles/org.openhab.binding.network/src/test/java/org/openhab/binding/network/internal/handler/NetworkHandlerTest.java @@ -19,6 +19,11 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.*; +import java.time.Duration; +import java.time.Instant; +import java.util.concurrent.ScheduledExecutorService; + +import org.eclipse.jdt.annotation.NonNullByDefault; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -50,11 +55,13 @@ import org.openhab.core.thing.binding.ThingHandlerCallback; */ @ExtendWith(MockitoExtension.class) @MockitoSettings(strictness = Strictness.LENIENT) +@NonNullByDefault public class NetworkHandlerTest extends JavaTest { private ThingUID thingUID = new ThingUID("network", "ttype", "ping"); - private @Mock ThingHandlerCallback callback; - private @Mock Thing thing; + private @Mock @NonNullByDefault({}) ThingHandlerCallback callback; + private @Mock @NonNullByDefault({}) ScheduledExecutorService scheduledExecutorService; + private @Mock @NonNullByDefault({}) Thing thing; @BeforeEach public void setUp() { @@ -76,17 +83,18 @@ public class NetworkHandlerTest extends JavaTest { conf.put(NetworkBindingConstants.PARAMETER_TIMEOUT, 1234); return conf; }); - PresenceDetection presenceDetection = spy(new PresenceDetection(handler, 2000)); + PresenceDetection presenceDetection = spy( + new PresenceDetection(handler, scheduledExecutorService, Duration.ofSeconds(2))); // Mock start/stop automatic refresh - doNothing().when(presenceDetection).startAutomaticRefresh(any()); + doNothing().when(presenceDetection).startAutomaticRefresh(); doNothing().when(presenceDetection).stopAutomaticRefresh(); handler.initialize(presenceDetection); assertThat(handler.retries, is(10)); assertThat(presenceDetection.getHostname(), is("127.0.0.1")); assertThat(presenceDetection.getServicePorts().iterator().next(), is(8080)); - assertThat(presenceDetection.getRefreshInterval(), is(101010L)); - assertThat(presenceDetection.getTimeout(), is(1234)); + assertThat(presenceDetection.getRefreshInterval(), is(Duration.ofMillis(101010))); + assertThat(presenceDetection.getTimeout(), is(Duration.ofMillis(1234))); } @Test @@ -101,7 +109,7 @@ public class NetworkHandlerTest extends JavaTest { conf.put(NetworkBindingConstants.PARAMETER_HOSTNAME, "127.0.0.1"); return conf; }); - handler.initialize(new PresenceDetection(handler, 2000)); + handler.initialize(new PresenceDetection(handler, scheduledExecutorService, Duration.ofSeconds(2))); // Check that we are offline ArgumentCaptor statusInfoCaptor = ArgumentCaptor.forClass(ThingStatusInfo.class); verify(callback).statusUpdated(eq(thing), statusInfoCaptor.capture()); @@ -120,10 +128,12 @@ public class NetworkHandlerTest extends JavaTest { conf.put(NetworkBindingConstants.PARAMETER_HOSTNAME, "127.0.0.1"); return conf; }); - PresenceDetection presenceDetection = spy(new PresenceDetection(handler, 2000)); + PresenceDetection presenceDetection = spy( + new PresenceDetection(handler, scheduledExecutorService, Duration.ofSeconds(2))); // Mock start/stop automatic refresh - doNothing().when(presenceDetection).startAutomaticRefresh(any()); + doNothing().when(presenceDetection).startAutomaticRefresh(); doNothing().when(presenceDetection).stopAutomaticRefresh(); + doReturn(Instant.now()).when(presenceDetection).getLastSeen(); handler.initialize(presenceDetection); // Check that we are online @@ -133,7 +143,7 @@ public class NetworkHandlerTest extends JavaTest { // Mock result value PresenceDetectionValue value = mock(PresenceDetectionValue.class); - when(value.getLowestLatency()).thenReturn(10.0); + when(value.getLowestLatency()).thenReturn(Duration.ofMillis(10)); when(value.isReachable()).thenReturn(true); when(value.getSuccessfulDetectionTypes()).thenReturn("TESTMETHOD"); @@ -146,7 +156,6 @@ public class NetworkHandlerTest extends JavaTest { eq(new QuantityType<>("10.0 ms"))); // Final result affects the LASTSEEN channel - when(value.isFinished()).thenReturn(true); handler.finalDetectionResult(value); verify(callback).stateUpdated(eq(new ChannelUID(thingUID, NetworkBindingConstants.CHANNEL_LASTSEEN)), any()); } diff --git a/bundles/org.openhab.binding.network/src/test/java/org/openhab/binding/network/internal/toberemoved/cache/ExpiringCacheAsyncTest.java b/bundles/org.openhab.binding.network/src/test/java/org/openhab/binding/network/internal/toberemoved/cache/ExpiringCacheAsyncTest.java deleted file mode 100644 index f3eb12b763c..00000000000 --- a/bundles/org.openhab.binding.network/src/test/java/org/openhab/binding/network/internal/toberemoved/cache/ExpiringCacheAsyncTest.java +++ /dev/null @@ -1,105 +0,0 @@ -/** - * Copyright (c) 2010-2024 Contributors to the openHAB project - * - * See the NOTICE file(s) distributed with this work for additional - * information. - * - * This program and the accompanying materials are made available under the - * terms of the Eclipse Public License 2.0 which is available at - * http://www.eclipse.org/legal/epl-2.0 - * - * SPDX-License-Identifier: EPL-2.0 - */ -package org.openhab.binding.network.internal.toberemoved.cache; - -import static org.junit.jupiter.api.Assertions.*; -import static org.mockito.Mockito.*; - -import java.util.function.Consumer; - -import org.eclipse.jdt.annotation.NonNullByDefault; -import org.junit.jupiter.api.Test; -import org.mockito.ArgumentCaptor; -import org.openhab.binding.network.internal.toberemoved.cache.ExpiringCacheAsync.ExpiringCacheUpdate; - -/** - * Tests cases for {@see ExpiringAsyncCache} - * - * @author David Graeff - Initial contribution - */ -@NonNullByDefault -public class ExpiringCacheAsyncTest { - @Test - public void testConstructorWrongCacheTime() { - assertThrows(IllegalArgumentException.class, () -> - // Fail if cache time is <= 0 - new ExpiringCacheAsync<>(0, () -> { - })); - } - - @Test - public void testConstructorNoRefrehCommand() { - assertThrows(IllegalArgumentException.class, () -> new ExpiringCacheAsync<>(2000, null)); - } - - @Test - public void testFetchValue() { - ExpiringCacheUpdate u = mock(ExpiringCacheUpdate.class); - ExpiringCacheAsync t = new ExpiringCacheAsync<>(2000, u); - assertTrue(t.isExpired()); - // Request a value - @SuppressWarnings("unchecked") - Consumer consumer = mock(Consumer.class); - t.getValue(consumer); - // We expect a call to the updater object - verify(u).requestCacheUpdate(); - // Update the value now - t.setValue(10.0); - // The value should be valid - assertFalse(t.isExpired()); - // We expect a call to the consumer - ArgumentCaptor valueCaptor = ArgumentCaptor.forClass(Double.class); - verify(consumer).accept(valueCaptor.capture()); - assertEquals(10.0, valueCaptor.getValue(), 0); - } - - @Test - public void testExpiring() { - ExpiringCacheUpdate u = mock(ExpiringCacheUpdate.class); - @SuppressWarnings("unchecked") - Consumer consumer = mock(Consumer.class); - - ExpiringCacheAsync t = new ExpiringCacheAsync<>(100, u); - t.setValue(10.0); - assertFalse(t.isExpired()); - - // Request a value - t.getValue(consumer); - // There should be no call to update the cache - verify(u, times(0)).requestCacheUpdate(); - // Wait - try { - Thread.sleep(101); - } catch (InterruptedException ignored) { - return; - } - // Request a value two times - t.getValue(consumer); - t.getValue(consumer); - // There should be one call to update the cache - verify(u, times(1)).requestCacheUpdate(); - assertTrue(t.isExpired()); - } - - @Test - public void testFetchExpiredValue() { - ExpiringCacheUpdate u = mock(ExpiringCacheUpdate.class); - ExpiringCacheAsync t = new ExpiringCacheAsync<>(2000, u); - t.setValue(10.0); - // We should always be able to get the raw value, expired or not - assertEquals(10.0, t.getExpiredValue(), 0); - t.invalidateValue(); - assertTrue(t.isExpired()); - assertEquals(10.0, t.getExpiredValue(), 0); - } -} diff --git a/bundles/org.openhab.binding.network/src/test/java/org/openhab/binding/network/internal/toberemoved/cache/ExpiringCacheHelper.java b/bundles/org.openhab.binding.network/src/test/java/org/openhab/binding/network/internal/toberemoved/cache/ExpiringCacheHelper.java deleted file mode 100644 index 25c2abed4f0..00000000000 --- a/bundles/org.openhab.binding.network/src/test/java/org/openhab/binding/network/internal/toberemoved/cache/ExpiringCacheHelper.java +++ /dev/null @@ -1,27 +0,0 @@ -/** - * Copyright (c) 2010-2024 Contributors to the openHAB project - * - * See the NOTICE file(s) distributed with this work for additional - * information. - * - * This program and the accompanying materials are made available under the - * terms of the Eclipse Public License 2.0 which is available at - * http://www.eclipse.org/legal/epl-2.0 - * - * SPDX-License-Identifier: EPL-2.0 - */ -package org.openhab.binding.network.internal.toberemoved.cache; - -import org.eclipse.jdt.annotation.NonNullByDefault; - -/** - * Helper class to make the package private cacheUpdater field available for tests. - * - * @author David Graeff - Initial Contribution - */ -@NonNullByDefault -public class ExpiringCacheHelper { - public static long expireTime(@SuppressWarnings("rawtypes") ExpiringCacheAsync cache) { - return cache.expiresAt; - } -} diff --git a/bundles/org.openhab.binding.network/src/test/java/org/openhab/binding/network/internal/utils/LatencyParserTest.java b/bundles/org.openhab.binding.network/src/test/java/org/openhab/binding/network/internal/utils/LatencyParserTest.java index e394c6d4dec..cbc1bd7aa82 100644 --- a/bundles/org.openhab.binding.network/src/test/java/org/openhab/binding/network/internal/utils/LatencyParserTest.java +++ b/bundles/org.openhab.binding.network/src/test/java/org/openhab/binding/network/internal/utils/LatencyParserTest.java @@ -13,8 +13,9 @@ package org.openhab.binding.network.internal.utils; import static org.junit.jupiter.api.Assertions.*; +import static org.openhab.binding.network.internal.utils.NetworkUtils.durationToMillis; -import java.util.Optional; +import java.time.Duration; import org.eclipse.jdt.annotation.NonNullByDefault; import org.junit.jupiter.api.Test; @@ -34,11 +35,11 @@ public class LatencyParserTest { String input = "64 bytes from 192.168.1.1: icmp_seq=0 ttl=64 time=1.225 ms"; // Act - Optional resultLatency = latencyParser.parseLatency(input); + Duration resultLatency = latencyParser.parseLatency(input); // Assert - assertTrue(resultLatency.isPresent()); - assertEquals(1.225, resultLatency.get(), 0); + assertNotNull(resultLatency); + assertEquals(1.225, durationToMillis(resultLatency), 0); } @Test @@ -54,10 +55,10 @@ public class LatencyParserTest { for (String inputLine : inputLines) { // Act - Optional resultLatency = latencyParser.parseLatency(inputLine); + Duration resultLatency = latencyParser.parseLatency(inputLine); // Assert - assertFalse(resultLatency.isPresent()); + assertNull(resultLatency); } } @@ -68,10 +69,10 @@ public class LatencyParserTest { String input = "Reply from 192.168.178.207: bytes=32 time=2ms TTL=64"; // Act - Optional resultLatency = latencyParser.parseLatency(input); + Duration resultLatency = latencyParser.parseLatency(input); // Assert - assertTrue(resultLatency.isPresent()); - assertEquals(2, resultLatency.get(), 0); + assertNotNull(resultLatency); + assertEquals(2, durationToMillis(resultLatency), 0); } }