From 50033bfb56c2e56962677ff9f5c145e92bd95591 Mon Sep 17 00:00:00 2001 From: Andrew Fiddian-Green Date: Thu, 2 Jan 2025 17:01:21 +0000 Subject: [PATCH 1/6] New Upnp device finder service Signed-off-by: Andrew Fiddian-Green --- .../io/transport/upnp/UpnpDeviceFinder.java | 158 ++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 bundles/org.openhab.core.io.transport.upnp/src/main/java/org/openhab/core/io/transport/upnp/UpnpDeviceFinder.java diff --git a/bundles/org.openhab.core.io.transport.upnp/src/main/java/org/openhab/core/io/transport/upnp/UpnpDeviceFinder.java b/bundles/org.openhab.core.io.transport.upnp/src/main/java/org/openhab/core/io/transport/upnp/UpnpDeviceFinder.java new file mode 100644 index 0000000000..6c12673ab2 --- /dev/null +++ b/bundles/org.openhab.core.io.transport.upnp/src/main/java/org/openhab/core/io/transport/upnp/UpnpDeviceFinder.java @@ -0,0 +1,158 @@ +package org.openhab.core.io.transport.upnp; + +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.Future; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; + +import org.jupnp.UpnpService; +import org.jupnp.model.message.header.RootDeviceHeader; +import org.jupnp.model.message.header.UDNHeader; +import org.jupnp.model.meta.LocalDevice; +import org.jupnp.model.meta.RemoteDevice; +import org.jupnp.model.types.UDN; +import org.jupnp.registry.Registry; +import org.jupnp.registry.RegistryListener; +import org.openhab.core.common.ThreadPoolManager; +import org.osgi.service.component.annotations.Activate; +import org.osgi.service.component.annotations.Component; +import org.osgi.service.component.annotations.Deactivate; +import org.osgi.service.component.annotations.Reference; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Component service that can be used to keep mis-behaving UPnP devices stored in the registry + * by sending a targeted M-SEARCH message for their UDN before their maxAge expires. + * + * @author Andrew Fiddian-Green - Initial Contribution + */ +@Component +public class UpnpDeviceFinder implements RegistryListener { + + private static final String DEVICE_FINDER_THREADPOOL = "upnpDeviceFinder"; + private static final int SEARCH_SCHEDULE_SECONDS = 60; + + private final Logger logger = LoggerFactory.getLogger(UpnpDeviceFinder.class); + private final ScheduledExecutorService scheduler = ThreadPoolManager.getScheduledPool(DEVICE_FINDER_THREADPOOL); + private final Map> subscriptions = new ConcurrentHashMap<>(); + + private final UpnpService upnpService; + + @Activate + public UpnpDeviceFinder(final @Reference UpnpService upnpService) { + this.upnpService = upnpService; + } + + @Activate + public void activate() { + upnpService.getRegistry().addListener(this); + upnpService.getControlPoint().search(); + upnpService.getControlPoint().search(new RootDeviceHeader()); + } + + @Deactivate + public void deactivate() { + upnpService.getRegistry().removeListener(this); + subscriptions.values().forEach(task -> task.cancel(false)); + subscriptions.clear(); + } + + /** + * Cancel scheduled search (if any) for the given UDN. + */ + private void cancelSearch(UDN udn) { + Future task = subscriptions.get(udn); + if (task != null) { + task.cancel(false); + } + } + + /** + * Schedule a search for the given device at a future time based on its maxAge. + */ + private void scheduleSearch(RemoteDevice device) { + UDN udn = device.getIdentity().getUdn(); + int delaySeconds = Math.max(SEARCH_SCHEDULE_SECONDS, + device.getIdentity().getMaxAgeSeconds() - SEARCH_SCHEDULE_SECONDS); + logger.debug("Scheduling search for {} in {} seconds", udn, delaySeconds); + cancelSearch(udn); + subscriptions.put(udn, scheduler.schedule(() -> { + logger.debug("Executing search for {}", udn); + upnpService.getControlPoint().search(new UDNHeader(udn)); + }, delaySeconds, TimeUnit.SECONDS)); + } + + /** + * Add the given UPnP device UDN to the subscriptions list and execute an initial immediate search. + */ + public void addUDN(UDN udn) { + if (!subscriptions.containsKey(udn)) { + logger.debug("Added subscription for {}", udn); + subscriptions.put(udn, scheduler.submit(() -> upnpService.getControlPoint().search(new UDNHeader(udn)))); + } + } + + /** + * Remove the given UPnP device UDN from the subscriptions list and cancel any pending search. + */ + public void removeUDN(UDN udn) { + subscriptions.remove(udn); + cancelSearch(udn); + logger.debug("Removed subscription for {}", udn); + } + + @Override + public void afterShutdown() { + } + + @Override + public void beforeShutdown(Registry registry) { + subscriptions.values().forEach(task -> task.cancel(false)); + subscriptions.clear(); + } + + @Override + public void localDeviceAdded(Registry registry, LocalDevice device) { + } + + @Override + public void localDeviceRemoved(Registry registry, LocalDevice device) { + } + + @Override + public void remoteDeviceAdded(Registry registry, RemoteDevice device) { + if (subscriptions.containsKey(device.getIdentity().getUdn())) { + scheduleSearch(device); + } + } + + @Override + public void remoteDeviceDiscoveryFailed(Registry registry, RemoteDevice device, Exception e) { + if (subscriptions.containsKey(device.getIdentity().getUdn())) { + logger.warn("Discovery of {} failed with exception {}", device.getIdentity().getUdn(), e.getMessage()); + } + } + + @Override + public void remoteDeviceDiscoveryStarted(Registry registry, RemoteDevice device) { + } + + @Override + public void remoteDeviceRemoved(Registry registry, RemoteDevice device) { + if (subscriptions.containsKey(device.getIdentity().getUdn())) { + UDN udn = device.getIdentity().getUdn(); + logger.warn("Device {} removed unexpectedly from registry; Executing search", udn); + cancelSearch(udn); + subscriptions.put(udn, scheduler.submit(() -> upnpService.getControlPoint().search(new UDNHeader(udn)))); + } + } + + @Override + public void remoteDeviceUpdated(Registry registry, RemoteDevice device) { + if (subscriptions.containsKey(device.getIdentity().getUdn())) { + scheduleSearch(device); + } + } +} From 3ad82c6b218ba808e45ce0878338d68f3ed441e5 Mon Sep 17 00:00:00 2001 From: Andrew Fiddian-Green Date: Thu, 2 Jan 2025 17:03:53 +0000 Subject: [PATCH 2/6] Immediate activation Signed-off-by: Andrew Fiddian-Green --- .../org/openhab/core/io/transport/upnp/UpnpDeviceFinder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundles/org.openhab.core.io.transport.upnp/src/main/java/org/openhab/core/io/transport/upnp/UpnpDeviceFinder.java b/bundles/org.openhab.core.io.transport.upnp/src/main/java/org/openhab/core/io/transport/upnp/UpnpDeviceFinder.java index 6c12673ab2..b8c00ff457 100644 --- a/bundles/org.openhab.core.io.transport.upnp/src/main/java/org/openhab/core/io/transport/upnp/UpnpDeviceFinder.java +++ b/bundles/org.openhab.core.io.transport.upnp/src/main/java/org/openhab/core/io/transport/upnp/UpnpDeviceFinder.java @@ -28,7 +28,7 @@ import org.slf4j.LoggerFactory; * * @author Andrew Fiddian-Green - Initial Contribution */ -@Component +@Component(immediate = true) public class UpnpDeviceFinder implements RegistryListener { private static final String DEVICE_FINDER_THREADPOOL = "upnpDeviceFinder"; From b95c3e581af305774219dd82417d9783dd4b093b Mon Sep 17 00:00:00 2001 From: Andrew Fiddian-Green Date: Thu, 2 Jan 2025 17:23:48 +0000 Subject: [PATCH 3/6] Add copyright header Signed-off-by: Andrew Fiddian-Green --- .../core/io/transport/upnp/UpnpDeviceFinder.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/bundles/org.openhab.core.io.transport.upnp/src/main/java/org/openhab/core/io/transport/upnp/UpnpDeviceFinder.java b/bundles/org.openhab.core.io.transport.upnp/src/main/java/org/openhab/core/io/transport/upnp/UpnpDeviceFinder.java index b8c00ff457..47c4caeb8b 100644 --- a/bundles/org.openhab.core.io.transport.upnp/src/main/java/org/openhab/core/io/transport/upnp/UpnpDeviceFinder.java +++ b/bundles/org.openhab.core.io.transport.upnp/src/main/java/org/openhab/core/io/transport/upnp/UpnpDeviceFinder.java @@ -1,3 +1,15 @@ +/** + * Copyright (c) 2010-2025 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.core.io.transport.upnp; import java.util.Map; From fb8b76b815e2726cbe18273a8ee497876bcbdcb2 Mon Sep 17 00:00:00 2001 From: Andrew Fiddian-Green Date: Fri, 3 Jan 2025 18:46:24 +0000 Subject: [PATCH 4/6] Retry the searches Signed-off-by: Andrew Fiddian-Green --- .../io/transport/upnp/UpnpDeviceFinder.java | 51 +++++++++++++------ 1 file changed, 36 insertions(+), 15 deletions(-) diff --git a/bundles/org.openhab.core.io.transport.upnp/src/main/java/org/openhab/core/io/transport/upnp/UpnpDeviceFinder.java b/bundles/org.openhab.core.io.transport.upnp/src/main/java/org/openhab/core/io/transport/upnp/UpnpDeviceFinder.java index 47c4caeb8b..17c5033efa 100644 --- a/bundles/org.openhab.core.io.transport.upnp/src/main/java/org/openhab/core/io/transport/upnp/UpnpDeviceFinder.java +++ b/bundles/org.openhab.core.io.transport.upnp/src/main/java/org/openhab/core/io/transport/upnp/UpnpDeviceFinder.java @@ -16,6 +16,7 @@ import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.Future; import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; import org.jupnp.UpnpService; @@ -45,10 +46,12 @@ public class UpnpDeviceFinder implements RegistryListener { private static final String DEVICE_FINDER_THREADPOOL = "upnpDeviceFinder"; private static final int SEARCH_SCHEDULE_SECONDS = 60; + private static final int SEARCH_RETRY_MAX = 5; + private static final int SEARCH_RETRY_INTERVAL = 5000; private final Logger logger = LoggerFactory.getLogger(UpnpDeviceFinder.class); private final ScheduledExecutorService scheduler = ThreadPoolManager.getScheduledPool(DEVICE_FINDER_THREADPOOL); - private final Map> subscriptions = new ConcurrentHashMap<>(); + private final Map> subscriptions = new ConcurrentHashMap<>(); private final UpnpService upnpService; @@ -73,27 +76,46 @@ public class UpnpDeviceFinder implements RegistryListener { /** * Cancel scheduled search (if any) for the given UDN. + * Interrupts the executeSearch() method below. */ private void cancelSearch(UDN udn) { Future task = subscriptions.get(udn); if (task != null) { - task.cancel(false); + task.cancel(true); } } + /** + * Execute the search for SEARCH_RETRY_MAX attempts at SEARCH_RETRY_INTERVAL. + * Can be interrupted by the cancelSearch() method above. + */ + private void executeSearch(UDN udn) { + logger.debug("Executing search for {}", udn); + for (int i = 0; i < SEARCH_RETRY_MAX; i++) { + upnpService.getControlPoint().search(new UDNHeader(udn)); + try { + Thread.sleep(SEARCH_RETRY_INTERVAL); + } catch (InterruptedException e) { + break; + } + } + } + + /** + * Schedule a search for the given device after the given delay. + */ + private void scheduleSearch(UDN udn, int delaySeconds) { + cancelSearch(udn); + logger.debug("Scheduling search for {} in {} seconds", udn, delaySeconds); + subscriptions.put(udn, scheduler.schedule(() -> executeSearch(udn), delaySeconds, TimeUnit.SECONDS)); + } + /** * Schedule a search for the given device at a future time based on its maxAge. */ private void scheduleSearch(RemoteDevice device) { - UDN udn = device.getIdentity().getUdn(); - int delaySeconds = Math.max(SEARCH_SCHEDULE_SECONDS, - device.getIdentity().getMaxAgeSeconds() - SEARCH_SCHEDULE_SECONDS); - logger.debug("Scheduling search for {} in {} seconds", udn, delaySeconds); - cancelSearch(udn); - subscriptions.put(udn, scheduler.schedule(() -> { - logger.debug("Executing search for {}", udn); - upnpService.getControlPoint().search(new UDNHeader(udn)); - }, delaySeconds, TimeUnit.SECONDS)); + scheduleSearch(device.getIdentity().getUdn(), + Math.max(SEARCH_SCHEDULE_SECONDS, device.getIdentity().getMaxAgeSeconds() - SEARCH_SCHEDULE_SECONDS)); } /** @@ -102,7 +124,7 @@ public class UpnpDeviceFinder implements RegistryListener { public void addUDN(UDN udn) { if (!subscriptions.containsKey(udn)) { logger.debug("Added subscription for {}", udn); - subscriptions.put(udn, scheduler.submit(() -> upnpService.getControlPoint().search(new UDNHeader(udn)))); + scheduleSearch(udn, 0); } } @@ -155,9 +177,8 @@ public class UpnpDeviceFinder implements RegistryListener { public void remoteDeviceRemoved(Registry registry, RemoteDevice device) { if (subscriptions.containsKey(device.getIdentity().getUdn())) { UDN udn = device.getIdentity().getUdn(); - logger.warn("Device {} removed unexpectedly from registry; Executing search", udn); - cancelSearch(udn); - subscriptions.put(udn, scheduler.submit(() -> upnpService.getControlPoint().search(new UDNHeader(udn)))); + logger.warn("Device {} removed unexpectedly from registry", udn); + scheduleSearch(udn, 0); } } From 52cea5e9c22876bcfe8078ca35b3dbe0f758396b Mon Sep 17 00:00:00 2001 From: Andrew Fiddian-Green Date: Sat, 4 Jan 2025 12:40:47 +0000 Subject: [PATCH 5/6] Quality improvements and JavaDoc Signed-off-by: Andrew Fiddian-Green --- .../io/transport/upnp/UpnpDeviceFinder.java | 58 ++++++++++--------- 1 file changed, 32 insertions(+), 26 deletions(-) diff --git a/bundles/org.openhab.core.io.transport.upnp/src/main/java/org/openhab/core/io/transport/upnp/UpnpDeviceFinder.java b/bundles/org.openhab.core.io.transport.upnp/src/main/java/org/openhab/core/io/transport/upnp/UpnpDeviceFinder.java index 17c5033efa..fa545d25af 100644 --- a/bundles/org.openhab.core.io.transport.upnp/src/main/java/org/openhab/core/io/transport/upnp/UpnpDeviceFinder.java +++ b/bundles/org.openhab.core.io.transport.upnp/src/main/java/org/openhab/core/io/transport/upnp/UpnpDeviceFinder.java @@ -14,7 +14,6 @@ package org.openhab.core.io.transport.upnp; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.Future; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; @@ -36,8 +35,16 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** - * Component service that can be used to keep mis-behaving UPnP devices stored in the registry - * by sending a targeted M-SEARCH message for their UDN before their maxAge expires. + * The {@link UpnpDeviceFinder} is a component service that can be used to keep 'mis-behaving' UPnP devices + * 'alive' in the registry by sending a targeted M-SEARCH 'ping' message for their UDN before their maxAge + * expires. + *

+ * Typically such a 'mis-behaving' device may fail to send regular NOTIFY alive messages, or fail to send + * them in time. This component substitutes for such lack by sending a targeted M-SEARCH message 60 seconds + * before the device's maxAge would normally expire. + *

+ * For a component to use this service it must consume a final @Reference to this class in an @Activate method, + * and call 'addUDN()' to start processing the given device, and respectively call 'removeUDN()') to stop it. * * @author Andrew Fiddian-Green - Initial Contribution */ @@ -45,6 +52,7 @@ import org.slf4j.LoggerFactory; public class UpnpDeviceFinder implements RegistryListener { private static final String DEVICE_FINDER_THREADPOOL = "upnpDeviceFinder"; + private static final int SEARCH_SCHEDULE_SECONDS = 60; private static final int SEARCH_RETRY_MAX = 5; private static final int SEARCH_RETRY_INTERVAL = 5000; @@ -76,10 +84,10 @@ public class UpnpDeviceFinder implements RegistryListener { /** * Cancel scheduled search (if any) for the given UDN. - * Interrupts the executeSearch() method below. + * May interrupt the executeSearch() method below. */ private void cancelSearch(UDN udn) { - Future task = subscriptions.get(udn); + ScheduledFuture task = subscriptions.get(udn); if (task != null) { task.cancel(true); } @@ -87,7 +95,7 @@ public class UpnpDeviceFinder implements RegistryListener { /** * Execute the search for SEARCH_RETRY_MAX attempts at SEARCH_RETRY_INTERVAL. - * Can be interrupted by the cancelSearch() method above. + * May be interrupted by the cancelSearch() method above. */ private void executeSearch(UDN udn) { logger.debug("Executing search for {}", udn); @@ -95,8 +103,8 @@ public class UpnpDeviceFinder implements RegistryListener { upnpService.getControlPoint().search(new UDNHeader(udn)); try { Thread.sleep(SEARCH_RETRY_INTERVAL); - } catch (InterruptedException e) { - break; + } catch (InterruptedException cancelled) { + return; } } } @@ -147,21 +155,27 @@ public class UpnpDeviceFinder implements RegistryListener { subscriptions.clear(); } - @Override - public void localDeviceAdded(Registry registry, LocalDevice device) { - } - - @Override - public void localDeviceRemoved(Registry registry, LocalDevice device) { - } - @Override public void remoteDeviceAdded(Registry registry, RemoteDevice device) { + remoteDeviceUpdated(registry, device); + } + + @Override + public void remoteDeviceUpdated(Registry registry, RemoteDevice device) { if (subscriptions.containsKey(device.getIdentity().getUdn())) { scheduleSearch(device); } } + @Override + public void remoteDeviceRemoved(Registry registry, RemoteDevice device) { + if (subscriptions.containsKey(device.getIdentity().getUdn())) { + UDN udn = device.getIdentity().getUdn(); + logger.warn("Device {} removed unexpectedly from registry", udn); + scheduleSearch(udn, 0); + } + } + @Override public void remoteDeviceDiscoveryFailed(Registry registry, RemoteDevice device, Exception e) { if (subscriptions.containsKey(device.getIdentity().getUdn())) { @@ -174,18 +188,10 @@ public class UpnpDeviceFinder implements RegistryListener { } @Override - public void remoteDeviceRemoved(Registry registry, RemoteDevice device) { - if (subscriptions.containsKey(device.getIdentity().getUdn())) { - UDN udn = device.getIdentity().getUdn(); - logger.warn("Device {} removed unexpectedly from registry", udn); - scheduleSearch(udn, 0); - } + public void localDeviceAdded(Registry registry, LocalDevice device) { } @Override - public void remoteDeviceUpdated(Registry registry, RemoteDevice device) { - if (subscriptions.containsKey(device.getIdentity().getUdn())) { - scheduleSearch(device); - } + public void localDeviceRemoved(Registry registry, LocalDevice device) { } } From 7ca682338a5e31eef172d1591f9e8625329a28b3 Mon Sep 17 00:00:00 2001 From: Andrew Fiddian-Green Date: Sat, 4 Jan 2025 13:07:10 +0000 Subject: [PATCH 6/6] Fix sequencing bug Signed-off-by: Andrew Fiddian-Green --- .../org/openhab/core/io/transport/upnp/UpnpDeviceFinder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundles/org.openhab.core.io.transport.upnp/src/main/java/org/openhab/core/io/transport/upnp/UpnpDeviceFinder.java b/bundles/org.openhab.core.io.transport.upnp/src/main/java/org/openhab/core/io/transport/upnp/UpnpDeviceFinder.java index fa545d25af..fc9a9fa7a6 100644 --- a/bundles/org.openhab.core.io.transport.upnp/src/main/java/org/openhab/core/io/transport/upnp/UpnpDeviceFinder.java +++ b/bundles/org.openhab.core.io.transport.upnp/src/main/java/org/openhab/core/io/transport/upnp/UpnpDeviceFinder.java @@ -140,8 +140,8 @@ public class UpnpDeviceFinder implements RegistryListener { * Remove the given UPnP device UDN from the subscriptions list and cancel any pending search. */ public void removeUDN(UDN udn) { - subscriptions.remove(udn); cancelSearch(udn); + subscriptions.remove(udn); logger.debug("Removed subscription for {}", udn); }