[plugwise] Use ThingHandlerService for discovery (#9041)

This simplifies the PlugwiseHandlerFactory code so it no longer needs to register and keep track of a discovery service for each bridge.

Also contains a few other improvements:

* fix lastRequest timestamp not updated when sending messages to discover node properties
* use java.time Duration and Instant
* use List.of, Set.of
* remove redundant null suppression because of EEAs

Signed-off-by: Wouter Born <github@maindrain.net>

diff --git a/bundles/org.openhab.binding.plugwise/src/main/java/org/openhab/binding/plugwise/internal/PlugwiseBindingConstants.java b/bundles/org.openhab.binding.plugwise/src/main/java/org/openhab/binding/plugwise/internal/PlugwiseBindingConstants.java
index 274e527861f619014490146b7388c7bade50e235..278c17bf046d3ed442013e5f04807ede3ec9d533 100644 (file)
--- a/bundles/org.openhab.binding.plugwise/src/main/java/org/openhab/binding/plugwise/internal/PlugwiseBindingConstants.java
+++ b/bundles/org.openhab.binding.plugwise/src/main/java/org/openhab/binding/plugwise/internal/PlugwiseBindingConstants.java
@@ -12,11 +12,7 @@
  */
 package org.openhab.binding.plugwise.internal;
 
-import static java.util.stream.Collectors.*;
-
-import java.util.Collections;
 import java.util.Set;
-import java.util.stream.Stream;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.openhab.core.thing.ThingTypeUID;
@@ -65,8 +61,6 @@ public class PlugwiseBindingConstants {
     public static final ThingTypeUID THING_TYPE_STICK = new ThingTypeUID(BINDING_ID, "stick");
     public static final ThingTypeUID THING_TYPE_SWITCH = new ThingTypeUID(BINDING_ID, "switch");
 
-    public static final Set<ThingTypeUID> SUPPORTED_THING_TYPES_UIDS = Stream
-            .of(THING_TYPE_CIRCLE, THING_TYPE_CIRCLE_PLUS, THING_TYPE_SCAN, THING_TYPE_SENSE, THING_TYPE_STEALTH,
-                    THING_TYPE_STICK, THING_TYPE_SWITCH)
-            .collect(collectingAndThen(toSet(), Collections::unmodifiableSet));
+    public static final Set<ThingTypeUID> SUPPORTED_THING_TYPES_UIDS = Set.of(THING_TYPE_CIRCLE, THING_TYPE_CIRCLE_PLUS,
+            THING_TYPE_SCAN, THING_TYPE_SENSE, THING_TYPE_STEALTH, THING_TYPE_STICK, THING_TYPE_SWITCH);
 }

diff --git a/bundles/org.openhab.binding.plugwise/src/main/java/org/openhab/binding/plugwise/internal/PlugwiseHandlerFactory.java b/bundles/org.openhab.binding.plugwise/src/main/java/org/openhab/binding/plugwise/internal/PlugwiseHandlerFactory.java
index bd4670565fd970514c1ac168d0219b9f75f3d310..8b52be589d3dfca438c039bd0f88db9627d41881 100644 (file)
--- a/bundles/org.openhab.binding.plugwise/src/main/java/org/openhab/binding/plugwise/internal/PlugwiseHandlerFactory.java
+++ b/bundles/org.openhab.binding.plugwise/src/main/java/org/openhab/binding/plugwise/internal/PlugwiseHandlerFactory.java
@@ -14,10 +14,6 @@ package org.openhab.binding.plugwise.internal;
 
 import static org.openhab.binding.plugwise.internal.PlugwiseBindingConstants.*;
 
-import java.util.HashMap;
-import java.util.Hashtable;
-import java.util.Map;
-
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
 import org.openhab.binding.plugwise.internal.handler.PlugwiseRelayDeviceHandler;
@@ -25,16 +21,13 @@ import org.openhab.binding.plugwise.internal.handler.PlugwiseScanHandler;
 import org.openhab.binding.plugwise.internal.handler.PlugwiseSenseHandler;
 import org.openhab.binding.plugwise.internal.handler.PlugwiseStickHandler;
 import org.openhab.binding.plugwise.internal.handler.PlugwiseSwitchHandler;
-import org.openhab.core.config.discovery.DiscoveryService;
 import org.openhab.core.io.transport.serial.SerialPortManager;
 import org.openhab.core.thing.Bridge;
 import org.openhab.core.thing.Thing;
 import org.openhab.core.thing.ThingTypeUID;
-import org.openhab.core.thing.ThingUID;
 import org.openhab.core.thing.binding.BaseThingHandlerFactory;
 import org.openhab.core.thing.binding.ThingHandler;
 import org.openhab.core.thing.binding.ThingHandlerFactory;
-import org.osgi.framework.ServiceRegistration;
 import org.osgi.service.component.annotations.Activate;
 import org.osgi.service.component.annotations.Component;
 import org.osgi.service.component.annotations.Reference;
@@ -48,8 +41,6 @@ import org.osgi.service.component.annotations.Reference;
 @Component(service = ThingHandlerFactory.class, configurationPid = "binding.plugwise")
 public class PlugwiseHandlerFactory extends BaseThingHandlerFactory {
 
-    private final Map<ThingUID, ServiceRegistration<?>> discoveryServiceRegistrations = new HashMap<>();
-
     private final SerialPortManager serialPortManager;
 
     @Activate
@@ -67,9 +58,7 @@ public class PlugwiseHandlerFactory extends BaseThingHandlerFactory {
         ThingTypeUID thingTypeUID = thing.getThingTypeUID();
 
         if (thingTypeUID.equals(THING_TYPE_STICK)) {
-            PlugwiseStickHandler handler = new PlugwiseStickHandler((Bridge) thing, serialPortManager);
-            registerDiscoveryService(handler);
-            return handler;
+            return new PlugwiseStickHandler((Bridge) thing, serialPortManager);
         } else if (thingTypeUID.equals(THING_TYPE_CIRCLE) || thingTypeUID.equals(THING_TYPE_CIRCLE_PLUS)
                 || thingTypeUID.equals(THING_TYPE_STEALTH)) {
             return new PlugwiseRelayDeviceHandler(thing);
@@ -83,23 +72,4 @@ public class PlugwiseHandlerFactory extends BaseThingHandlerFactory {
 
         return null;
     }
-
-    private void registerDiscoveryService(PlugwiseStickHandler handler) {
-        PlugwiseThingDiscoveryService discoveryService = new PlugwiseThingDiscoveryService(handler);
-        discoveryService.activate();
-        this.discoveryServiceRegistrations.put(handler.getThing().getUID(),
-                bundleContext.registerService(DiscoveryService.class.getName(), discoveryService, new Hashtable<>()));
-    }
-
-    @Override
-    protected void removeHandler(ThingHandler thingHandler) {
-        ServiceRegistration<?> registration = this.discoveryServiceRegistrations.get(thingHandler.getThing().getUID());
-        if (registration != null) {
-            PlugwiseThingDiscoveryService discoveryService = (PlugwiseThingDiscoveryService) bundleContext
-                    .getService(registration.getReference());
-            discoveryService.deactivate();
-            registration.unregister();
-            discoveryServiceRegistrations.remove(thingHandler.getThing().getUID());
-        }
-    }
 }

diff --git a/bundles/org.openhab.binding.plugwise/src/main/java/org/openhab/binding/plugwise/internal/PlugwiseThingDiscoveryService.java b/bundles/org.openhab.binding.plugwise/src/main/java/org/openhab/binding/plugwise/internal/PlugwiseThingDiscoveryService.java
index 4bcd9937c8a781525b5413de9c8b59dc736e3dd6..4907150b90877abe23d4e42dffee170e4686da0a 100644 (file)
--- a/bundles/org.openhab.binding.plugwise/src/main/java/org/openhab/binding/plugwise/internal/PlugwiseThingDiscoveryService.java
+++ b/bundles/org.openhab.binding.plugwise/src/main/java/org/openhab/binding/plugwise/internal/PlugwiseThingDiscoveryService.java
@@ -15,6 +15,8 @@ package org.openhab.binding.plugwise.internal;
 import static java.util.stream.Collectors.*;
 import static org.openhab.binding.plugwise.internal.PlugwiseBindingConstants.*;
 
+import java.time.Duration;
+import java.time.Instant;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Iterator;
@@ -44,6 +46,8 @@ import org.openhab.core.thing.Thing;
 import org.openhab.core.thing.ThingStatus;
 import org.openhab.core.thing.ThingTypeUID;
 import org.openhab.core.thing.ThingUID;
+import org.openhab.core.thing.binding.ThingHandler;
+import org.openhab.core.thing.binding.ThingHandlerService;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -56,13 +60,13 @@ import org.slf4j.LoggerFactory;
  */
 @NonNullByDefault
 public class PlugwiseThingDiscoveryService extends AbstractDiscoveryService
-        implements PlugwiseMessageListener, PlugwiseStickStatusListener {
+        implements PlugwiseMessageListener, PlugwiseStickStatusListener, ThingHandlerService {
 
     private static class CurrentRoleCall {
         private boolean isRoleCalling;
         private int currentNodeID;
         private int attempts;
-        private long lastRequestMillis;
+        private Instant lastRequest = Instant.MIN;
     }
 
     private static class DiscoveredNode {
@@ -70,7 +74,7 @@ public class PlugwiseThingDiscoveryService extends AbstractDiscoveryService
         private final Map<String, String> properties = new HashMap<>();
         private DeviceType deviceType = DeviceType.UNKNOWN;
         private int attempts;
-        private long lastRequestMillis;
+        private Instant lastRequest = Instant.MIN;
 
         public DiscoveredNode(MACAddress macAddress) {
             this.macAddress = macAddress;
@@ -87,16 +91,16 @@ public class PlugwiseThingDiscoveryService extends AbstractDiscoveryService
 
     private static final int MIN_NODE_ID = 0;
     private static final int MAX_NODE_ID = 63;
-    private static final int DISCOVERY_INTERVAL = 180;
 
-    private static final int WATCH_INTERVAL = 1;
-
-    private static final int MESSAGE_TIMEOUT = 15;
     private static final int MESSAGE_RETRY_ATTEMPTS = 5;
 
+    private static final Duration DISCOVERY_INTERVAL = Duration.ofMinutes(3);
+    private static final Duration MESSAGE_TIMEOUT = Duration.ofSeconds(15);
+    private static final Duration WATCH_INTERVAL = Duration.ofSeconds(1);
+
     private final Logger logger = LoggerFactory.getLogger(PlugwiseThingDiscoveryService.class);
 
-    private final PlugwiseStickHandler stickHandler;
+    private @NonNullByDefault({}) PlugwiseStickHandler stickHandler;
 
     private @Nullable ScheduledFuture<?> discoveryJob;
     private @Nullable ScheduledFuture<?> watchJob;
@@ -104,10 +108,8 @@ public class PlugwiseThingDiscoveryService extends AbstractDiscoveryService
 
     private final Map<MACAddress, DiscoveredNode> discoveredNodes = new ConcurrentHashMap<>();
 
-    public PlugwiseThingDiscoveryService(PlugwiseStickHandler stickHandler) throws IllegalArgumentException {
+    public PlugwiseThingDiscoveryService() throws IllegalArgumentException {
         super(DISCOVERED_THING_TYPES_UIDS, 1, true);
-        this.stickHandler = stickHandler;
-        this.stickHandler.addStickStatusListener(this);
     }
 
     @Override
@@ -118,6 +120,7 @@ public class PlugwiseThingDiscoveryService extends AbstractDiscoveryService
         stopDiscoveryWatchJob();
     }
 
+    @Override
     public void activate() {
         super.activate(new HashMap<>());
     }
@@ -138,7 +141,7 @@ public class PlugwiseThingDiscoveryService extends AbstractDiscoveryService
     }
 
     @Override
-    protected void deactivate() {
+    public void deactivate() {
         super.deactivate();
         stickHandler.removeMessageListener(this);
         stickHandler.removeStickStatusListener(this);
@@ -147,8 +150,10 @@ public class PlugwiseThingDiscoveryService extends AbstractDiscoveryService
     private void discoverNewNodeDetails(MACAddress macAddress) {
         if (!isAlreadyDiscovered(macAddress)) {
             logger.debug("Discovered new node ({})", macAddress);
-            discoveredNodes.put(macAddress, new DiscoveredNode(macAddress));
+            DiscoveredNode node = new DiscoveredNode(macAddress);
+            discoveredNodes.put(macAddress, node);
             updateInformation(macAddress);
+            node.lastRequest = Instant.now();
         } else {
             logger.debug("Already discovered node ({})", macAddress);
         }
@@ -184,6 +189,11 @@ public class PlugwiseThingDiscoveryService extends AbstractDiscoveryService
         return stickHandler.getThing().getStatus();
     }
 
+    @Override
+    public @Nullable ThingHandler getThingHandler() {
+        return stickHandler;
+    }
+
     private void handleAnnounceAwakeRequest(AnnounceAwakeRequestMessage message) {
         discoverNewNodeDetails(message.getMACAddress());
     }
@@ -267,7 +277,7 @@ public class PlugwiseThingDiscoveryService extends AbstractDiscoveryService
                 currentRoleCall.attempts++;
             }
             currentRoleCall.currentNodeID = nodeID;
-            currentRoleCall.lastRequestMillis = System.currentTimeMillis();
+            currentRoleCall.lastRequest = Instant.now();
         } else {
             logger.warn("Invalid node ID for role call: {}", nodeID);
         }
@@ -277,6 +287,14 @@ public class PlugwiseThingDiscoveryService extends AbstractDiscoveryService
         stickHandler.sendMessage(message, PlugwiseMessagePriority.UPDATE_AND_DISCOVERY);
     }
 
+    @Override
+    public void setThingHandler(ThingHandler handler) {
+        if (handler instanceof PlugwiseStickHandler) {
+            stickHandler = (PlugwiseStickHandler) handler;
+            stickHandler.addStickStatusListener(this);
+        }
+    }
+
     @Override
     protected void startBackgroundDiscovery() {
         logger.debug("Starting Plugwise device background discovery");
@@ -288,7 +306,8 @@ public class PlugwiseThingDiscoveryService extends AbstractDiscoveryService
 
         ScheduledFuture<?> localDiscoveryJob = discoveryJob;
         if (localDiscoveryJob == null || localDiscoveryJob.isCancelled()) {
-            discoveryJob = scheduler.scheduleWithFixedDelay(discoveryRunnable, 0, DISCOVERY_INTERVAL, TimeUnit.SECONDS);
+            discoveryJob = scheduler.scheduleWithFixedDelay(discoveryRunnable, 0, DISCOVERY_INTERVAL.toMillis(),
+                    TimeUnit.MILLISECONDS);
         }
     }
 
@@ -297,7 +316,8 @@ public class PlugwiseThingDiscoveryService extends AbstractDiscoveryService
 
         Runnable watchRunnable = () -> {
             if (currentRoleCall.isRoleCalling) {
-                if ((System.currentTimeMillis() - currentRoleCall.lastRequestMillis) > (MESSAGE_TIMEOUT * 1000)
+                Duration durationSinceLastRequest = Duration.between(currentRoleCall.lastRequest, Instant.now());
+                if (durationSinceLastRequest.compareTo(MESSAGE_TIMEOUT) > 0
                         && currentRoleCall.attempts < MESSAGE_RETRY_ATTEMPTS) {
                     logger.debug("Resending timed out role call message for node with ID {} on Circle+ ({})",
                             currentRoleCall.currentNodeID, getCirclePlusMAC());
@@ -313,10 +333,11 @@ public class PlugwiseThingDiscoveryService extends AbstractDiscoveryService
             while (it.hasNext()) {
                 Entry<MACAddress, DiscoveredNode> entry = it.next();
                 DiscoveredNode node = entry.getValue();
-                if (System.currentTimeMillis() - node.lastRequestMillis > (MESSAGE_TIMEOUT * 1000)
-                        && node.attempts < MESSAGE_RETRY_ATTEMPTS) {
+                Duration durationSinceLastRequest = Duration.between(node.lastRequest, Instant.now());
+                if (durationSinceLastRequest.compareTo(MESSAGE_TIMEOUT) > 0 && node.attempts < MESSAGE_RETRY_ATTEMPTS) {
                     logger.debug("Resending timed out information request message to node ({})", node.macAddress);
                     updateInformation(node.macAddress);
+                    node.lastRequest = Instant.now();
                     node.attempts++;
                 } else if (node.attempts >= MESSAGE_RETRY_ATTEMPTS) {
                     logger.debug("Giving up on information request for node ({})", node.macAddress);
@@ -332,8 +353,8 @@ public class PlugwiseThingDiscoveryService extends AbstractDiscoveryService
 
         ScheduledFuture<?> localWatchJob = watchJob;
         if (localWatchJob == null || localWatchJob.isCancelled()) {
-            watchJob = scheduler.scheduleWithFixedDelay(watchRunnable, WATCH_INTERVAL, WATCH_INTERVAL,
-                    TimeUnit.SECONDS);
+            watchJob = scheduler.scheduleWithFixedDelay(watchRunnable, WATCH_INTERVAL.toMillis(),
+                    WATCH_INTERVAL.toMillis(), TimeUnit.MILLISECONDS);
         }
     }
 

diff --git a/bundles/org.openhab.binding.plugwise/src/main/java/org/openhab/binding/plugwise/internal/PlugwiseUtils.java b/bundles/org.openhab.binding.plugwise/src/main/java/org/openhab/binding/plugwise/internal/PlugwiseUtils.java
index ea459fc6c7bcfa6f781e35df28ca505270c1312b..076a22b1463a54736e384ad736724fad63683d22 100644 (file)
--- a/bundles/org.openhab.binding.plugwise/src/main/java/org/openhab/binding/plugwise/internal/PlugwiseUtils.java
+++ b/bundles/org.openhab.binding.plugwise/src/main/java/org/openhab/binding/plugwise/internal/PlugwiseUtils.java
@@ -43,17 +43,17 @@ public final class PlugwiseUtils {
     }
 
     public static DeviceType getDeviceType(ThingTypeUID uid) {
-        if (uid.equals(THING_TYPE_CIRCLE)) {
+        if (THING_TYPE_CIRCLE.equals(uid)) {
             return CIRCLE;
-        } else if (uid.equals(THING_TYPE_CIRCLE_PLUS)) {
+        } else if (THING_TYPE_CIRCLE_PLUS.equals(uid)) {
             return CIRCLE_PLUS;
-        } else if (uid.equals(THING_TYPE_SCAN)) {
+        } else if (THING_TYPE_SCAN.equals(uid)) {
             return SCAN;
-        } else if (uid.equals(THING_TYPE_SENSE)) {
+        } else if (THING_TYPE_SENSE.equals(uid)) {
             return SENSE;
-        } else if (uid.equals(THING_TYPE_STEALTH)) {
+        } else if (THING_TYPE_STEALTH.equals(uid)) {
             return STEALTH;
-        } else if (uid.equals(THING_TYPE_SWITCH)) {
+        } else if (THING_TYPE_SWITCH.equals(uid)) {
             return SWITCH;
         } else {
             return UNKNOWN;
@@ -106,7 +106,6 @@ public final class PlugwiseUtils {
         return upperCamel.substring(0, 1).toLowerCase() + upperCamel.substring(1);
     }
 
-    @SuppressWarnings("null")
     public static boolean updateProperties(Map<String, String> properties, InformationResponseMessage message) {
         boolean update = false;
 

diff --git a/bundles/org.openhab.binding.plugwise/src/main/java/org/openhab/binding/plugwise/internal/handler/PlugwiseRelayDeviceHandler.java b/bundles/org.openhab.binding.plugwise/src/main/java/org/openhab/binding/plugwise/internal/handler/PlugwiseRelayDeviceHandler.java
index 57ff70f0baa4028151d15727ba9ae0fda878b7e4..907e98d058dc550f18ba91e0bfe7ccfdca858aba 100644 (file)
--- a/bundles/org.openhab.binding.plugwise/src/main/java/org/openhab/binding/plugwise/internal/handler/PlugwiseRelayDeviceHandler.java
+++ b/bundles/org.openhab.binding.plugwise/src/main/java/org/openhab/binding/plugwise/internal/handler/PlugwiseRelayDeviceHandler.java
@@ -12,16 +12,13 @@
  */
 package org.openhab.binding.plugwise.internal.handler;
 
-import static java.util.stream.Collectors.*;
 import static org.openhab.binding.plugwise.internal.PlugwiseBindingConstants.*;
 import static org.openhab.core.thing.ThingStatus.*;
 
 import java.time.Duration;
 import java.time.LocalDateTime;
-import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
-import java.util.stream.Stream;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
@@ -222,10 +219,8 @@ public class PlugwiseRelayDeviceHandler extends AbstractPlugwiseThingHandler {
         }
     };
 
-    private final List<PlugwiseDeviceTask> recurringTasks = Stream
-            .of(clockUpdateTask, currentPowerUpdateTask, energyUpdateTask, informationUpdateTask,
-                    realTimeClockUpdateTask, setClockTask)
-            .collect(collectingAndThen(toList(), Collections::unmodifiableList));
+    private final List<PlugwiseDeviceTask> recurringTasks = List.of(clockUpdateTask, currentPowerUpdateTask,
+            energyUpdateTask, informationUpdateTask, realTimeClockUpdateTask, setClockTask);
 
     private final Logger logger = LoggerFactory.getLogger(PlugwiseRelayDeviceHandler.class);
     private final DeviceType deviceType;

diff --git a/bundles/org.openhab.binding.plugwise/src/main/java/org/openhab/binding/plugwise/internal/handler/PlugwiseStickHandler.java b/bundles/org.openhab.binding.plugwise/src/main/java/org/openhab/binding/plugwise/internal/handler/PlugwiseStickHandler.java
index f51766be084e42f39cf6a162d53eb87af8719448..9de91cca0a70d1f579b4cfd7c0377d11ab20503e 100644 (file)
--- a/bundles/org.openhab.binding.plugwise/src/main/java/org/openhab/binding/plugwise/internal/handler/PlugwiseStickHandler.java
+++ b/bundles/org.openhab.binding.plugwise/src/main/java/org/openhab/binding/plugwise/internal/handler/PlugwiseStickHandler.java
@@ -18,6 +18,7 @@ import static org.openhab.core.thing.ThingStatus.*;
 
 import java.io.IOException;
 import java.time.Duration;
+import java.util.Collection;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.CopyOnWriteArrayList;
@@ -28,6 +29,7 @@ import org.openhab.binding.plugwise.internal.PlugwiseCommunicationHandler;
 import org.openhab.binding.plugwise.internal.PlugwiseDeviceTask;
 import org.openhab.binding.plugwise.internal.PlugwiseInitializationException;
 import org.openhab.binding.plugwise.internal.PlugwiseMessagePriority;
+import org.openhab.binding.plugwise.internal.PlugwiseThingDiscoveryService;
 import org.openhab.binding.plugwise.internal.PlugwiseUtils;
 import org.openhab.binding.plugwise.internal.config.PlugwiseStickConfig;
 import org.openhab.binding.plugwise.internal.listener.PlugwiseMessageListener;
@@ -48,6 +50,7 @@ import org.openhab.core.thing.Thing;
 import org.openhab.core.thing.ThingStatus;
 import org.openhab.core.thing.ThingStatusDetail;
 import org.openhab.core.thing.binding.BaseBridgeHandler;
+import org.openhab.core.thing.binding.ThingHandlerService;
 import org.openhab.core.types.Command;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -122,6 +125,11 @@ public class PlugwiseStickHandler extends BaseBridgeHandler implements PlugwiseM
         return circlePlusMAC;
     }
 
+    @Override
+    public Collection<Class<? extends ThingHandlerService>> getServices() {
+        return List.of(PlugwiseThingDiscoveryService.class);
+    }
+
     public @Nullable MACAddress getStickMAC() {
         return stickMAC;
     }