[Network] Added param to differentiate between mac and IP WOL Request (#11387)

* Added possiblity to send WOL Requests to configured Hostname, also removed unnecessary unit from Timeout Annotation in WakeOnLanPacketSenderTest
* Introduced parameter to decide whether to send WOL via IP or MAC
* Added two Methods for user clarity, marked old method as deprecated, adjusted README
* Updated internal methods to use explicit calls as well, added deprecated method call to README

Signed-off-by: Jonathan Saxen <jonathan@saxen.info>

diff --git a/bundles/org.openhab.binding.network/README.md b/bundles/org.openhab.binding.network/README.md
index 4ebe399de095e2710878f1783978fe0b1b2bd7f7..71c6ac9c998dc707aa7c666f234cfe7dae64f4b8 100644 (file)
--- a/bundles/org.openhab.binding.network/README.md
+++ b/bundles/org.openhab.binding.network/README.md
@@ -250,6 +250,11 @@ if (actions === null) {
     logInfo("actions", "Actions not found, check thing ID")
     return
 } else {
-    actions.sendWakeOnLanPacket()
+    // Send via MAC address
+    actions.sendWakeOnLanPacketViaMac()
+    actions.sendWakeOnLanPacket() // deprecated
+    
+    // Send via IP address
+    actions.sendWakeOnLanPacketViaIp()
 }
 ```

diff --git a/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/WakeOnLanPacketSender.java b/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/WakeOnLanPacketSender.java
index 4e9da808cda49f1b0f0a79a6cf34c93229a3f712..6c9fc2ff1f25f7bda4f8e2da25f7c058957729d9 100644 (file)
--- a/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/WakeOnLanPacketSender.java
+++ b/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/WakeOnLanPacketSender.java
@@ -54,21 +54,28 @@ public class WakeOnLanPacketSender {
     @Nullable
     private final Integer port;
 
-    private byte @Nullable [] magicPacket;
-    private final Consumer<byte[]> magicPacketSender;
+    private final Consumer<byte[]> magicPacketMacSender;
+    private final Consumer<byte[]> magicPacketIpSender;
 
     public WakeOnLanPacketSender(String macAddress, @Nullable String hostname, @Nullable Integer port) {
+        logger.debug("initialized WOL Packet Sender (mac: {}, hostname: {}, port: {}", macAddress, hostname, port);
         this.macAddress = macAddress;
         this.hostname = hostname;
         this.port = port;
-        this.magicPacketSender = this::sendMagicPacket;
+        this.magicPacketMacSender = this::sendMagicPacketViaMac;
+        this.magicPacketIpSender = this::sendMagicPacketViaIp;
     }
 
+    /**
+     * Used for testing only.
+     */
     public WakeOnLanPacketSender(String macAddress) {
+        logger.debug("initialized WOL Packet Sender (mac: {}", macAddress);
         this.macAddress = macAddress;
         this.hostname = null;
         this.port = null;
-        this.magicPacketSender = this::sendMagicPacket;
+        this.magicPacketMacSender = this::sendMagicPacketViaMac;
+        this.magicPacketIpSender = this::sendMagicPacketViaIp;
     }
 
     /**
@@ -78,17 +85,28 @@ public class WakeOnLanPacketSender {
         this.macAddress = macAddress;
         this.hostname = null;
         this.port = null;
-        this.magicPacketSender = magicPacketSender;
+        this.magicPacketMacSender = magicPacketSender;
+        this.magicPacketIpSender = this::sendMagicPacketViaIp;
     }
 
-    public void sendPacket() {
-        byte[] localMagicPacket = magicPacket;
-        if (localMagicPacket == null) {
-            localMagicPacket = createMagicPacket(createMacBytes(macAddress));
-            magicPacket = localMagicPacket;
-        }
+    public void sendWakeOnLanPacketViaMac() {
+        byte[] magicPacket = createMagicPacket();
+        this.magicPacketMacSender.accept(magicPacket);
+    }
 
-        magicPacketSender.accept(localMagicPacket);
+    public void sendWakeOnLanPacketViaIp() {
+        byte[] magicPacket = createMagicPacket();
+        this.magicPacketIpSender.accept(magicPacket);
+    }
+
+    private byte[] createMagicPacket() {
+        byte[] macBytes = createMacBytes(this.macAddress);
+        byte[] magicPacket = new byte[MAGIC_PACKET_BYTE_SIZE];
+        Arrays.fill(magicPacket, 0, PREFIX_BYTE_SIZE, (byte) 0xff);
+        for (int i = PREFIX_BYTE_SIZE; i < MAGIC_PACKET_BYTE_SIZE; i += MAC_BYTE_SIZE) {
+            System.arraycopy(macBytes, 0, magicPacket, i, macBytes.length);
+        }
+        return magicPacket;
     }
 
     private byte[] createMacBytes(String macAddress) {
@@ -102,23 +120,24 @@ public class WakeOnLanPacketSender {
         return HexUtils.hexToBytes(hexString);
     }
 
-    private byte[] createMagicPacket(byte[] macBytes) {
-        byte[] bytes = new byte[MAGIC_PACKET_BYTE_SIZE];
-        Arrays.fill(bytes, 0, PREFIX_BYTE_SIZE, (byte) 0xff);
-        for (int i = PREFIX_BYTE_SIZE; i < MAGIC_PACKET_BYTE_SIZE; i += MAC_BYTE_SIZE) {
-            System.arraycopy(macBytes, 0, bytes, i, macBytes.length);
+    private void sendMagicPacketViaMac(byte[] magicPacket) {
+        try (DatagramSocket socket = new DatagramSocket()) {
+            logger.debug("Sending Wake-on-LAN Packet via Broadcast");
+            broadcastMagicPacket(magicPacket, socket);
+        } catch (SocketException e) {
+            logger.error("Failed to open Wake-on-LAN datagram socket", e);
         }
-        return bytes;
     }
 
-    private void sendMagicPacket(byte[] magicPacket) {
+    private void sendMagicPacketViaIp(byte[] magicPacket) {
         try (DatagramSocket socket = new DatagramSocket()) {
-            if (StringUtils.isEmpty(hostname)) {
-                broadcastMagicPacket(magicPacket, socket);
-            } else {
+            if (!StringUtils.isEmpty(this.hostname)) {
+                logger.debug("Sending Wake-on-LAN Packet via IP Address");
                 SocketAddress socketAddress = new InetSocketAddress(this.hostname,
                         Objects.requireNonNullElse(this.port, WOL_UDP_PORT));
                 sendMagicPacketToIp(magicPacket, socket, socketAddress);
+            } else {
+                throw new IllegalStateException("Hostname is not set!");
             }
         } catch (SocketException e) {
             logger.error("Failed to open Wake-on-LAN datagram socket", e);
@@ -131,14 +150,14 @@ public class WakeOnLanPacketSender {
                 DatagramPacket packet = new DatagramPacket(magicPacket, MAGIC_PACKET_BYTE_SIZE, broadcastAddress,
                         WOL_UDP_PORT);
                 socket.send(packet);
-                logger.debug("Wake-on-LAN packet sent (MAC address: {}, broadcast address: {})", macAddress,
+                logger.debug("Wake-on-LAN packet sent (MAC address: {}, broadcast address: {})", this.macAddress,
                         broadcastAddress.getHostAddress());
             } catch (IOException e) {
-                logger.debug("Failed to send Wake-on-LAN packet (MAC address: {}, broadcast address: {})", macAddress,
-                        broadcastAddress.getHostAddress(), e);
+                logger.error("Failed to send Wake-on-LAN packet (MAC address: {}, broadcast address: {})",
+                        this.macAddress, broadcastAddress.getHostAddress(), e);
             }
         });
-        logger.info("Wake-on-LAN packets sent (MAC address: {})", macAddress);
+        logger.info("Wake-on-LAN packets sent (MAC address: {})", this.macAddress);
     }
 
     private void sendMagicPacketToIp(byte[] magicPacket, DatagramSocket socket, SocketAddress ip) {
@@ -146,9 +165,9 @@ public class WakeOnLanPacketSender {
         try {
             socket.send(packet);
         } catch (IOException e) {
-            logger.debug("Failed to send Wake-on-LAN packet (MAC address: {}, address: {})", macAddress, ip, e);
+            logger.error("Failed to send Wake-on-LAN packet (IP address: {})", ip, e);
         }
-        logger.info("Wake-on-LAN packets sent (MAC address: {}, IP address: {})", macAddress, ip);
+        logger.info("Wake-on-LAN packets sent (IP address: {})", ip);
     }
 
     private Stream<InetAddress> broadcastAddressStream() {
@@ -156,7 +175,7 @@ public class WakeOnLanPacketSender {
             try {
                 return InetAddress.getByName(address);
             } catch (UnknownHostException e) {
-                logger.debug("Failed to get broadcast address '{}' by name", address, e);
+                logger.error("Failed to get broadcast address '{}' by name", address, e);
                 return null;
             }
         }).filter(Objects::nonNull);

diff --git a/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/action/NetworkActions.java b/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/action/NetworkActions.java
index e0e0964174a18d6ac3b824850602fc4cc536d594..d9cab7f5bf809707bf3b70772676a76bfb26e42a 100644 (file)
--- a/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/action/NetworkActions.java
+++ b/bundles/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/action/NetworkActions.java
@@ -47,17 +47,36 @@ public class NetworkActions implements ThingActions {
         return handler;
     }
 
+    /**
+     * @deprecated Use sendWakeOnLanPacketViaMac or sendWakeOnLanPacketViaIp instead.
+     */
+    @Deprecated
     @RuleAction(label = "send a WoL packet", description = "Send a Wake-on-LAN packet to wake the device.")
     public void sendWakeOnLanPacket() {
+        sendWakeOnLanPacketViaMac();
+    }
+
+    @RuleAction(label = "send a WoL packet", description = "Send a Wake-on-LAN packet to wake the device via Mac.")
+    public void sendWakeOnLanPacketViaMac() {
+        NetworkHandler localHandler = handler;
+        if (localHandler != null) {
+            localHandler.sendWakeOnLanPacketViaMac();
+        } else {
+            logger.warn("Failed to send Wake-on-LAN packet (handler null)");
+        }
+    }
+
+    @RuleAction(label = "send a WoL packet", description = "Send a Wake-on-LAN packet to wake the device via IP.")
+    public void sendWakeOnLanPacketViaIp() {
         NetworkHandler localHandler = handler;
         if (localHandler != null) {
-            localHandler.sendWakeOnLanPacket();
+            localHandler.sendWakeOnLanPacketViaIp();
         } else {
             logger.warn("Failed to send Wake-on-LAN packet (handler null)");
         }
     }
 
     public static void sendWakeOnLanPacket(ThingActions actions) {
-        ((NetworkActions) actions).sendWakeOnLanPacket();
+        ((NetworkActions) actions).sendWakeOnLanPacketViaMac();
     }
 }

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 f27fe42033c70c215fa18b6a4ddcf453abe0d29f..d38131c9a26a9accd64c4bcf557c737515024d6e 100644 (file)
--- 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
@@ -242,11 +242,16 @@ public class NetworkHandler extends BaseThingHandler
         return Collections.singletonList(NetworkActions.class);
     }
 
-    public void sendWakeOnLanPacket() {
+    public void sendWakeOnLanPacketViaIp() {
+        // Hostname can't be null
+        wakeOnLanPacketSender.sendWakeOnLanPacketViaIp();
+    }
+
+    public void sendWakeOnLanPacketViaMac() {
         if (handlerConfiguration.macAddress.isEmpty()) {
             throw new IllegalStateException(
                     "Cannot send WoL packet because the 'macAddress' is not configured for " + thing.getUID());
         }
-        wakeOnLanPacketSender.sendPacket();
+        wakeOnLanPacketSender.sendWakeOnLanPacketViaMac();
     }
 }

diff --git a/bundles/org.openhab.binding.network/src/test/java/org/openhab/binding/network/internal/WakeOnLanPacketSenderTest.java b/bundles/org.openhab.binding.network/src/test/java/org/openhab/binding/network/internal/WakeOnLanPacketSenderTest.java
index 517521d4464bd254210d67f40fe8b263492f5047..60693d45426b70027e8aa545674f1a43e12544d9 100644 (file)
--- a/bundles/org.openhab.binding.network/src/test/java/org/openhab/binding/network/internal/WakeOnLanPacketSenderTest.java
+++ b/bundles/org.openhab.binding.network/src/test/java/org/openhab/binding/network/internal/WakeOnLanPacketSenderTest.java
@@ -53,7 +53,7 @@ public class WakeOnLanPacketSenderTest {
         WakeOnLanPacketSender sender = new WakeOnLanPacketSender("6f:70:65:6e:48:41",
                 bytes -> System.arraycopy(bytes, 0, actualPacket, 0, bytes.length));
 
-        sender.sendPacket();
+        sender.sendWakeOnLanPacketViaMac();
 
         assertValidMagicPacket(HexUtils.hexToBytes("6f:70:65:6e:48:41", ":"), actualPacket);
     }
@@ -65,7 +65,7 @@ public class WakeOnLanPacketSenderTest {
         WakeOnLanPacketSender sender = new WakeOnLanPacketSender("6F-70-65-6E-48-41",
                 bytes -> System.arraycopy(bytes, 0, actualPacket, 0, bytes.length));
 
-        sender.sendPacket();
+        sender.sendWakeOnLanPacketViaMac();
 
         assertValidMagicPacket(HexUtils.hexToBytes("6F-70-65-6E-48-41", "-"), actualPacket);
     }
@@ -77,7 +77,7 @@ public class WakeOnLanPacketSenderTest {
         WakeOnLanPacketSender sender = new WakeOnLanPacketSender("6f70656e4841",
                 bytes -> System.arraycopy(bytes, 0, actualPacket, 0, bytes.length));
 
-        sender.sendPacket();
+        sender.sendWakeOnLanPacketViaMac();
 
         assertValidMagicPacket(HexUtils.hexToBytes("6f70656e4841"), actualPacket);
     }
@@ -93,13 +93,13 @@ public class WakeOnLanPacketSenderTest {
     }
 
     @Test
-    public void sendWithHostnameNullAndPortNull() throws IOException, InterruptedException {
-        sendWOLTest(null, null);
+    public void sendWithHostnameNullAndPortNull() {
+        assertThrows(IllegalStateException.class, () -> sendWOLTest(null, null));
     }
 
     @Test
-    public void sendWithHostnameNull() throws IOException, InterruptedException {
-        sendWOLTest(null, 4444);
+    public void sendWithHostnameNull() {
+        assertThrows(IllegalStateException.class, () -> sendWOLTest(null, 4444));
     }
 
     private void sendWOLTest(String hostname, Integer port) throws InterruptedException, IOException {
@@ -112,36 +112,41 @@ public class WakeOnLanPacketSenderTest {
             Thread.sleep(100);
         }
 
-        WakeOnLanPacketSender sender = new WakeOnLanPacketSender("6f70656e4841", hostname, port);
-        sender.sendPacket();
+        try {
+            WakeOnLanPacketSender sender = new WakeOnLanPacketSender("6f70656e4841", hostname, port);
+            sender.sendWakeOnLanPacketViaIp();
 
-        // This Test is only applicable for IP Requests
-        if (hostname != null && port != null) {
-            socket.receive(datagramPacket);
-        }
-
-        socket.close();
+            // This Test is only applicable for IP Requests
+            if (hostname != null && port != null) {
+                socket.receive(datagramPacket);
+            }
 
-        Assertions.assertTrue(datagramPacket.getData().length > 0);
+            Assertions.assertTrue(datagramPacket.getData().length > 0);
+        } finally {
+            socket.close();
+        }
     }
 
     @Test
     public void sendWithEmptyMacAddressThrowsException() {
-        assertThrows(IllegalStateException.class, () -> new WakeOnLanPacketSender("").sendPacket());
+        assertThrows(IllegalStateException.class, () -> new WakeOnLanPacketSender("").sendWakeOnLanPacketViaMac());
     }
 
     @Test
     public void sendWithTooShortMacAddressThrowsException() {
-        assertThrows(IllegalStateException.class, () -> new WakeOnLanPacketSender("6f:70:65:6e:48").sendPacket());
+        assertThrows(IllegalStateException.class,
+                () -> new WakeOnLanPacketSender("6f:70:65:6e:48").sendWakeOnLanPacketViaMac());
     }
 
     @Test
     public void sendWithTooLongMacAddressThrowsException() {
-        assertThrows(IllegalStateException.class, () -> new WakeOnLanPacketSender("6f:70:65:6e:48:41:42").sendPacket());
+        assertThrows(IllegalStateException.class,
+                () -> new WakeOnLanPacketSender("6f:70:65:6e:48:41:42").sendWakeOnLanPacketViaMac());
     }
 
     @Test
     public void sendWithUnsupportedSeparatorInMacAddressThrowsException() {
-        assertThrows(IllegalStateException.class, () -> new WakeOnLanPacketSender("6f=70=65=6e=48=41").sendPacket());
+        assertThrows(IllegalStateException.class,
+                () -> new WakeOnLanPacketSender("6f=70=65=6e=48=41").sendWakeOnLanPacketViaMac());
     }
 }