[network] Adressing issue #11503 (#12316)

Signed-off-by: clinique <gael@lhopital.org>

bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/NetworkBindingConfiguration.java

@@ -29,11 +29,11 @@ import org.openhab.binding.network.internal.utils.NetworkUtils.ArpPingUtilEnum;
 @NonNullByDefault
 public class NetworkBindingConfiguration {
 
-    public Boolean allowSystemPings = true;
+    public boolean allowSystemPings = true;
-    public Boolean allowDHCPlisten = true;
+    public boolean allowDHCPlisten = true;
     public BigDecimal cacheDeviceStateTimeInMS = BigDecimal.valueOf(2000);
     public String arpPingToolPath = "arping";
-    public @NonNullByDefault({}) ArpPingUtilEnum arpPingUtilMethod;
+    public ArpPingUtilEnum arpPingUtilMethod = ArpPingUtilEnum.DISABLED;
     // For backwards compatibility reasons, the default is to use the ping method execution time as latency value
     public boolean preferResponseTimeAsLatency = false;
 

bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/NetworkBindingConfigurationListener.java

@@ -12,11 +12,14 @@
  */
 package org.openhab.binding.network.internal;
 
+import org.eclipse.jdt.annotation.NonNullByDefault;
+
 /**
  * Listener for binding configuration changes.
  *
  * @author Andreas Hirsch - Initial contribution
  */
+@NonNullByDefault
 public interface NetworkBindingConfigurationListener {
 
     void bindingConfigurationChanged();

bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/PresenceDetection.java

@@ -57,10 +57,9 @@ public class PresenceDetection implements IPRequestReceivedCallback {
 
     /// Configuration variables
     private boolean useDHCPsniffing = false;
-    private String arpPingState = "Disabled";
     private String ipPingState = "Disabled";
     protected String arpPingUtilPath = "";
-    protected ArpPingUtilEnum arpPingMethod = ArpPingUtilEnum.UNKNOWN_TOOL;
+    protected ArpPingUtilEnum arpPingMethod = ArpPingUtilEnum.DISABLED;
     protected @Nullable IpPingMethodEnum pingMethod = null;
     private boolean iosDevice;
     private Set<Integer> tcpPorts = new HashSet<>();
@@ -184,40 +183,13 @@ public class PresenceDetection implements IPRequestReceivedCallback {
      * it will be disabled as well.
      *
      * @param enable Enable or disable ARP ping
-     * @param arpPingUtilPath c
+     * @param destinationAddress target ip address
      */
     private void setUseArpPing(boolean enable, @Nullable InetAddress destinationAddress) {
         if (!enable || arpPingUtilPath.isEmpty()) {
-            arpPingState = "Disabled";
+            arpPingMethod = ArpPingUtilEnum.DISABLED;
-            arpPingMethod = ArpPingUtilEnum.UNKNOWN_TOOL;
+        } else if (!(destinationAddress instanceof Inet4Address)) {
-            return;
+            arpPingMethod = ArpPingUtilEnum.DISABLED_INVALID_IP;
-        } else if (destinationAddress == null || !(destinationAddress instanceof Inet4Address)) {
-            arpPingState = "Destination is not a valid IPv4 address";
-            arpPingMethod = ArpPingUtilEnum.UNKNOWN_TOOL;
-            return;
-        }
-
-        switch (arpPingMethod) {
-            case UNKNOWN_TOOL: {
-                arpPingState = "Unknown arping tool";
-                break;
-            }
-            case THOMAS_HABERT_ARPING: {
-                arpPingState = "Arping tool by Thomas Habets";
-                break;
-            }
-            case THOMAS_HABERT_ARPING_WITHOUT_TIMEOUT: {
-                arpPingState = "Arping tool by Thomas Habets (old version)";
-                break;
-            }
-            case ELI_FULKERSON_ARP_PING_FOR_WINDOWS: {
-                arpPingState = "Eli Fulkerson ARPing tool for Windows";
-                break;
-            }
-            case IPUTILS_ARPING: {
-                arpPingState = "Ipuitls Arping";
-                break;
-            }
         }
     }
 
@@ -234,7 +206,7 @@ public class PresenceDetection implements IPRequestReceivedCallback {
     }
 
     public String getArpPingState() {
-        return arpPingState;
+        return arpPingMethod.description;
     }
 
     public String getIPPingState() {
@@ -318,7 +290,7 @@ public class PresenceDetection implements IPRequestReceivedCallback {
         if (pingMethod != null) {
             detectionChecks += 1;
         }
-        if (arpPingMethod != ArpPingUtilEnum.UNKNOWN_TOOL) {
+        if (arpPingMethod.canProceed) {
             interfaceNames = networkUtils.getInterfaceNames();
             detectionChecks += interfaceNames.size();
         }

bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/SpeedTestConfiguration.java

@@ -12,25 +12,37 @@
  */
 package org.openhab.binding.network.internal;
 
+import org.eclipse.jdt.annotation.NonNullByDefault;
+import org.eclipse.jdt.annotation.Nullable;
+
 /**
  * The {@link SpeedTestConfiguration} is the class used to match the
  * thing configuration.
  *
  * @author Gaël L'hopital - Initial contribution
  */
+@NonNullByDefault
 public class SpeedTestConfiguration {
-    public Integer refreshInterval = 20;
+    public int refreshInterval = 20;
-    public Integer initialDelay = 5;
+    public int initialDelay = 5;
-    public Integer uploadSize = 1000000;
+    public int uploadSize = 1000000;
-    public Integer maxTimeout = 3;
+    public int maxTimeout = 3;
-    private String url;
+    private @Nullable String url;
-    private String fileName;
+    private @Nullable String fileName;
 
-    public String getUploadURL() {
+    public @Nullable String getUploadURL() {
-        return url + (url.endsWith("/") ? "" : "/");
+        String localUrl = url;
+        if (localUrl != null) {
+            localUrl += localUrl.endsWith("/") ? "" : "/";
+        }
+        return localUrl;
     }
 
-    public String getDownloadURL() {
+    public @Nullable String getDownloadURL() {
-        return getUploadURL() + fileName;
+        String result = getUploadURL();
+        if (result != null && fileName != null) {
+            result += fileName;
+        }
+        return result;
     }
 }

bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/WakeOnLanPacketSender.java

@@ -13,13 +13,18 @@
 package org.openhab.binding.network.internal;
 
 import java.io.IOException;
-import java.net.*;
+import java.net.DatagramPacket;
+import java.net.DatagramSocket;
+import java.net.InetAddress;
+import java.net.InetSocketAddress;
+import java.net.SocketAddress;
+import java.net.SocketException;
+import java.net.UnknownHostException;
 import java.util.Arrays;
 import java.util.Objects;
 import java.util.function.Consumer;
 import java.util.stream.Stream;
 
-import org.apache.commons.lang3.StringUtils;
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
 import org.openhab.core.net.NetUtil;
@@ -48,11 +53,8 @@ public class WakeOnLanPacketSender {
 
     private final String macAddress;
 
-    @Nullable
+    private final @Nullable String hostname;
-    private final String hostname;
+    private final @Nullable Integer port;
-
-    @Nullable
-    private final Integer port;
 
     private final Consumer<byte[]> magicPacketMacSender;
     private final Consumer<byte[]> magicPacketIpSender;
@@ -131,10 +133,10 @@ public class WakeOnLanPacketSender {
 
     private void sendMagicPacketViaIp(byte[] magicPacket) {
         try (DatagramSocket socket = new DatagramSocket()) {
-            if (!StringUtils.isEmpty(this.hostname)) {
+            if (hostname != null && !hostname.isBlank()) {
                 logger.debug("Sending Wake-on-LAN Packet via IP Address");
-                SocketAddress socketAddress = new InetSocketAddress(this.hostname,
+                SocketAddress socketAddress = new InetSocketAddress(hostname,
-                        Objects.requireNonNullElse(this.port, WOL_UDP_PORT));
+                        Objects.requireNonNullElse(port, WOL_UDP_PORT));
                 sendMagicPacketToIp(magicPacket, socket, socketAddress);
             } else {
                 throw new IllegalStateException("Hostname is not set!");

bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/dhcp/DHCPPacketListenerServer.java

@@ -70,7 +70,7 @@ public class DHCPPacketListenerServer extends Thread {
 
         Byte dhcpMessageType = request.getDHCPMessageType();
 
-        if (dhcpMessageType != DHCPPacket.DHCPREQUEST) {
+        if (dhcpMessageType == null || dhcpMessageType != DHCPPacket.DHCPREQUEST) {
             return; // skipping non DHCPREQUEST message types
         }
 

bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/handler/SpeedTestHandler.java

@@ -69,7 +69,8 @@ public class SpeedTestHandler extends BaseThingHandler implements ISpeedTestList
     }
 
     private synchronized void startSpeedTest() {
-        if (speedTestSocket == null) {
+        String url = configuration.getDownloadURL();
+        if (speedTestSocket == null && url != null) {
             logger.debug("Network speedtest started");
             final SpeedTestSocket socket = new SpeedTestSocket(1500);
             speedTestSocket = socket;
@@ -78,7 +79,7 @@ public class SpeedTestHandler extends BaseThingHandler implements ISpeedTestList
             updateState(CHANNEL_TEST_START, new DateTimeType());
             updateState(CHANNEL_TEST_END, UnDefType.NULL);
             updateProgress(new QuantityType<>(0, Units.PERCENT));
-            socket.startDownload(configuration.getDownloadURL());
+            socket.startDownload(url);
         } else {
             logger.info("A speedtest is already in progress, will retry on next refresh");
         }
@@ -109,7 +110,8 @@ public class SpeedTestHandler extends BaseThingHandler implements ISpeedTestList
                 switch (testReport.getSpeedTestMode()) {
                     case DOWNLOAD:
                         updateState(CHANNEL_RATE_DOWN, quantity);
-                        if (speedTestSocket != null && configuration != null) {
+                        String url = configuration.getUploadURL();
+                        if (speedTestSocket != null && url != null) {
                             speedTestSocket.startUpload(configuration.getUploadURL(), configuration.uploadSize);
                         }
                         break;

bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/utils/LatencyParser.java

@@ -16,6 +16,7 @@ import java.util.Optional;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
+import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -24,9 +25,10 @@ import org.slf4j.LoggerFactory;
  *
  * @author Andreas Hirsch - Initial contribution
  */
+@NonNullByDefault
 public class LatencyParser {
 
-    private static Pattern LATENCY_PATTERN = Pattern.compile(".*time=(.*) ?ms");
+    private static final Pattern LATENCY_PATTERN = Pattern.compile(".*time=(.*) ?ms");
     private final Logger logger = LoggerFactory.getLogger(LatencyParser.class);
 
     // This is how the input looks like on Mac and Linux:

bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/utils/NetworkUtils.java

@@ -203,19 +203,19 @@ public class NetworkUtils {
     public ArpPingUtilEnum determineNativeARPpingMethod(String arpToolPath) {
         String result = ExecUtil.executeCommandLineAndWaitResponse(Duration.ofMillis(100), arpToolPath, "--help");
         if (result == null || result.isBlank()) {
-            return ArpPingUtilEnum.UNKNOWN_TOOL;
+            return ArpPingUtilEnum.DISABLED_UNKNOWN_TOOL;
         } else if (result.contains("Thomas Habets")) {
             if (result.matches("(?s)(.*)w sec Specify a timeout(.*)")) {
                 return ArpPingUtilEnum.THOMAS_HABERT_ARPING;
             } else {
                 return ArpPingUtilEnum.THOMAS_HABERT_ARPING_WITHOUT_TIMEOUT;
             }
-        } else if (result.contains("-w timeout")) {
+        } else if (result.contains("-w timeout") || result.contains("-w <timeout>")) {
             return ArpPingUtilEnum.IPUTILS_ARPING;
         } else if (result.contains("Usage: arp-ping.exe")) {
             return ArpPingUtilEnum.ELI_FULKERSON_ARP_PING_FOR_WINDOWS;
         }
-        return ArpPingUtilEnum.UNKNOWN_TOOL;
+        return ArpPingUtilEnum.DISABLED_UNKNOWN_TOOL;
     }
 
     public enum IpPingMethodEnum {
@@ -292,11 +292,21 @@ public class NetworkUtils {
     }
 
     public enum ArpPingUtilEnum {
-        UNKNOWN_TOOL,
+        DISABLED("Disabled", false),
-        IPUTILS_ARPING,
+        DISABLED_INVALID_IP("Destination is not a valid IPv4 address", false),
-        THOMAS_HABERT_ARPING,
+        DISABLED_UNKNOWN_TOOL("Unknown arping tool", false),
-        THOMAS_HABERT_ARPING_WITHOUT_TIMEOUT,
+        IPUTILS_ARPING("Iputils Arping", true),
-        ELI_FULKERSON_ARP_PING_FOR_WINDOWS
+        THOMAS_HABERT_ARPING("Arping tool by Thomas Habets", true),
+        THOMAS_HABERT_ARPING_WITHOUT_TIMEOUT("Arping tool by Thomas Habets (old version)", true),
+        ELI_FULKERSON_ARP_PING_FOR_WINDOWS("Eli Fulkerson ARPing tool for Windows", true);
+
+        public final String description;
+        public final boolean canProceed;
+
+        ArpPingUtilEnum(String description, boolean canProceed) {
+            this.description = description;
+            this.canProceed = canProceed;
+        }
     }
 
     /**
@@ -317,7 +327,7 @@ public class NetworkUtils {
             String interfaceName, String ipV4address, int timeoutInMS) throws IOException, InterruptedException {
         double execStartTimeInMS = System.currentTimeMillis();
 
-        if (arpUtilPath == null || arpingTool == null || arpingTool == ArpPingUtilEnum.UNKNOWN_TOOL) {
+        if (arpUtilPath == null || arpingTool == null || !arpingTool.canProceed) {
             return Optional.empty();
         }
         Process proc;

bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/utils/PingResult.java

@@ -14,15 +14,18 @@ package org.openhab.binding.network.internal.utils;
 
 import java.util.Optional;
 
+import org.eclipse.jdt.annotation.NonNullByDefault;
+
 /**
  * Information about the ping result.
  *
  * @author Andreas Hirsch - Initial contribution
  */
+@NonNullByDefault
 public class PingResult {
 
     private boolean success;
-    private Double responseTimeInMS;
+    private Optional<Double> responseTimeInMS = Optional.empty();
     private double executionTimeInMS;
 
     /**
@@ -46,14 +49,14 @@ public class PingResult {
      *         by ping command is not available.
      */
     public Optional<Double> getResponseTimeInMS() {
-        return responseTimeInMS == null ? Optional.empty() : Optional.of(responseTimeInMS);
+        return responseTimeInMS;
     }
 
     /**
      * @param responseTimeInMS Response time in ms which was returned by the ping command.
      */
     public void setResponseTimeInMS(double responseTimeInMS) {
-        this.responseTimeInMS = responseTimeInMS;
+        this.responseTimeInMS = Optional.of(responseTimeInMS);
     }
 
     @Override

bundles/org.openhab.binding.network/src/main/resources/OH-INF/thing/thing-types.xml

@@ -207,16 +207,19 @@
 		<description>States whether a device is online or offline</description>
 		<state readOnly="true"></state>
 	</channel-type>
+
 	<channel-type id="latency">
 		<item-type>Number:Time</item-type>
 		<label>Latency</label>
 		<description>States the latency time</description>
 		<state readOnly="true" pattern="%d %unit%"></state>
 	</channel-type>
+
 	<channel-type id="lastseen">
 		<item-type>DateTime</item-type>
 		<label>Last Seen</label>
 		<description>States the last seen date/time</description>
+		<category>time</category>
 		<state readOnly="true"></state>
 	</channel-type>
 </thing:thing-descriptions>

bundles/org.openhab.binding.network/src/test/java/org/openhab/binding/network/internal/PresenceDetectionTest.java

@@ -178,8 +178,8 @@ public class PresenceDetectionTest {
 
     @Test
     public void reuseValueTests() throws InterruptedException, IOException {
-        final long START_TIME = 1000L;
+        final long startTime = 1000L;
-        when(subject.cache.getCurrentNanoTime()).thenReturn(TimeUnit.MILLISECONDS.toNanos(START_TIME));
+        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);
@@ -188,19 +188,19 @@ public class PresenceDetectionTest {
         assertThat(v.getLowestLatency(), is(19.0));
 
         // Advance in time but not expire the cache (1ms left)
-        final long ALMOST_EXPIRE = START_TIME + CACHETIME - 1;
+        final long almostExpire = startTime + CACHETIME - 1;
-        when(subject.cache.getCurrentNanoTime()).thenReturn(TimeUnit.MILLISECONDS.toNanos(ALMOST_EXPIRE));
+        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(ALMOST_EXPIRE + CACHETIME)));
+                is(TimeUnit.MILLISECONDS.toNanos(almostExpire + CACHETIME)));
 
         // Cache expire. A new PresenceDetectionValue instance will be returned
         when(subject.cache.getCurrentNanoTime())
-                .thenReturn(TimeUnit.MILLISECONDS.toNanos(ALMOST_EXPIRE + CACHETIME + CACHETIME + 1));
+                .thenReturn(TimeUnit.MILLISECONDS.toNanos(almostExpire + CACHETIME + CACHETIME + 1));
         v2 = subject.updateReachableValue(PresenceDetectionType.ICMP_PING, 25);
         assertNotEquals(v, v2);
         assertThat(v2.getLowestLatency(), is(25.0));

bundles/org.openhab.binding.network/src/test/java/org/openhab/binding/network/internal/WakeOnLanPacketSenderTest.java

@@ -22,6 +22,8 @@ import java.net.DatagramPacket;
 import java.net.DatagramSocket;
 import java.util.Arrays;
 
+import org.eclipse.jdt.annotation.NonNullByDefault;
+import org.eclipse.jdt.annotation.Nullable;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.Timeout;
@@ -33,6 +35,7 @@ import org.openhab.core.util.HexUtils;
  * @author Wouter Born - Initial contribution
  */
 @Timeout(value = 10)
+@NonNullByDefault
 public class WakeOnLanPacketSenderTest {
 
     private void assertValidMagicPacket(byte[] macBytes, byte[] packet) {
@@ -102,7 +105,8 @@ public class WakeOnLanPacketSenderTest {
         assertThrows(IllegalStateException.class, () -> sendWOLTest(null, 4444));
     }
 
-    private void sendWOLTest(String hostname, Integer port) throws InterruptedException, IOException {
+    private void sendWOLTest(@Nullable String hostname, @Nullable Integer port)
+            throws InterruptedException, IOException {
         DatagramSocket socket = new DatagramSocket(4444);
 
         byte[] buf = new byte[256];

bundles/org.openhab.binding.network/src/test/java/org/openhab/binding/network/internal/dhcp/DHCPTest.java

@@ -22,6 +22,7 @@ import java.io.IOException;
 import java.net.InetAddress;
 import java.net.SocketException;
 
+import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.junit.jupiter.api.Test;
 import org.openhab.binding.network.internal.dhcp.DHCPPacket.BadPacketException;
 
@@ -30,6 +31,7 @@ import org.openhab.binding.network.internal.dhcp.DHCPPacket.BadPacketException;
  *
  * @author David Graeff - Initial contribution
  */
+@NonNullByDefault
 public class DHCPTest {
     @Test
     public void testService() throws SocketException {

bundles/org.openhab.binding.network/src/test/java/org/openhab/binding/network/internal/toberemoved/cache/ExpiringCacheAsyncTest.java

@@ -17,6 +17,7 @@ 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;
@@ -26,6 +27,7 @@ import org.openhab.binding.network.internal.toberemoved.cache.ExpiringCacheAsync
  *
  * @author David Graeff - Initial contribution
  */
+@NonNullByDefault
 public class ExpiringCacheAsyncTest {
     @Test
     public void testConstructorWrongCacheTime() {

bundles/org.openhab.binding.network/src/test/java/org/openhab/binding/network/internal/toberemoved/cache/ExpiringCacheHelper.java

@@ -12,11 +12,14 @@
  */
 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
+ * @author David Graeff - Initial Contribution
  */
+@NonNullByDefault
 public class ExpiringCacheHelper {
     public static long expireTime(@SuppressWarnings("rawtypes") ExpiringCacheAsync cache) {
         return cache.expiresAt;

bundles/org.openhab.binding.network/src/test/java/org/openhab/binding/network/internal/utils/LatencyParserTest.java

@@ -16,6 +16,7 @@ import static org.junit.jupiter.api.Assertions.*;
 
 import java.util.Optional;
 
+import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.junit.jupiter.api.Test;
 
 /**
@@ -23,6 +24,7 @@ import org.junit.jupiter.api.Test;
  *
  * @author Andreas Hirsch - Initial contribution
  */
+@NonNullByDefault
 public class LatencyParserTest {
 
     @Test