[discovery.upnp] Devices may apply a grace period for removal from the Inbox (#2144)

* [discovery.upnp] delay removing devices from Inbox

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>

bundles/org.openhab.core.config.discovery.upnp/src/main/java/org/openhab/core/config/discovery/upnp/UpnpDiscoveryParticipant.java

@@ -62,4 +62,21 @@ public interface UpnpDiscoveryParticipant {
      */
     @Nullable
     ThingUID getThingUID(RemoteDevice device);
+
+    /**
+     * The JUPnP library strictly follows the UPnP specification insofar as if a device fails to send its next
+     * 'ssdp:alive' notification within its declared 'maxAge' period, it is immediately considered to be gone. But
+     * unfortunately some openHAB bindings handle devices that can sometimes be a bit late in sending their 'ssdp:alive'
+     * notifications even though they have not really gone offline, which means that such devices are repeatedly removed
+     * from, and (re)added to, the Inbox.
+     *
+     * To prevent this, a binding that implements a UpnpDiscoveryParticipant may OPTIONALLY implement this method to
+     * specify an additional delay period (grace period) to wait before the device is removed from the Inbox.
+     *
+     * @param device the UPnP device on the network
+     * @return the additional grace period delay in seconds before the device will be removed from the Inbox
+     */
+    default long getRemovalGracePeriodSeconds(RemoteDevice device) {
+        return 0;
+    }
 }

bundles/org.openhab.core.config.discovery.upnp/src/main/java/org/openhab/core/config/discovery/upnp/internal/UpnpDiscoveryService.java

@@ -17,11 +17,15 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.CopyOnWriteArraySet;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
 
 import org.jupnp.UpnpService;
 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.jupnp.transport.RouterException;
@@ -65,6 +69,11 @@ public class UpnpDiscoveryService extends AbstractDiscoveryService
 
     private UpnpService upnpService;
 
+    /*
+     * Map of scheduled tasks to remove devices from the Inbox
+     */
+    private Map<UDN, Future<?>> deviceRemovalTasks = new ConcurrentHashMap<>();
+
     @Override
     protected void activate(Map<String, Object> configProperties) {
         super.activate(configProperties);
@@ -157,6 +166,9 @@ public class UpnpDiscoveryService extends AbstractDiscoveryService
             try {
                 DiscoveryResult result = participant.createResult(device);
                 if (result != null) {
+                    if (participant.getRemovalGracePeriodSeconds(device) > 0) {
+                        cancelRemovalTask(device.getIdentity().getUdn());
+                    }
                     thingDiscovered(result);
                 }
             } catch (Exception e) {
@@ -165,13 +177,33 @@ public class UpnpDiscoveryService extends AbstractDiscoveryService
         }
     }
 
+    /*
+     * If the device has been scheduled to be removed, cancel its respective removal task
+     */
+    private void cancelRemovalTask(UDN udn) {
+        Future<?> deviceRemovalTask = deviceRemovalTasks.remove(udn);
+        if (deviceRemovalTask != null) {
+            deviceRemovalTask.cancel(false);
+        }
+    }
+
     @Override
     public void remoteDeviceRemoved(Registry registry, RemoteDevice device) {
         for (UpnpDiscoveryParticipant participant : participants) {
             try {
                 ThingUID thingUID = participant.getThingUID(device);
                 if (thingUID != null) {
-                    thingRemoved(thingUID);
+                    long gracePeriod = participant.getRemovalGracePeriodSeconds(device);
+                    if (gracePeriod <= 0) {
+                        thingRemoved(thingUID);
+                    } else {
+                        UDN udn = device.getIdentity().getUdn();
+                        cancelRemovalTask(udn);
+                        deviceRemovalTasks.put(udn, scheduler.schedule(() -> {
+                            thingRemoved(thingUID);
+                            cancelRemovalTask(udn);
+                        }, gracePeriod, TimeUnit.SECONDS));
+                    }
                 }
             } catch (Exception e) {
                 logger.error("Participant '{}' threw an exception", participant.getClass().getName(), e);