[powermax] Use ThingHandlerService for discovery (#9083)

Signed-off-by: Laurent Garnier <lg.hc@free.fr>

bundles/org.openhab.binding.powermax/src/main/java/org/openhab/binding/powermax/internal/PowermaxHandlerFactory.java

@@ -14,17 +14,11 @@ package org.openhab.binding.powermax.internal;
 
 import static org.openhab.binding.powermax.internal.PowermaxBindingConstants.*;
 
-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.powermax.internal.discovery.PowermaxDiscoveryService;
 import org.openhab.binding.powermax.internal.handler.PowermaxBridgeHandler;
 import org.openhab.binding.powermax.internal.handler.PowermaxThingHandler;
 import org.openhab.core.config.core.Configuration;
-import org.openhab.core.config.discovery.DiscoveryService;
 import org.openhab.core.i18n.TimeZoneProvider;
 import org.openhab.core.io.transport.serial.SerialPortManager;
 import org.openhab.core.thing.Bridge;
@@ -34,7 +28,6 @@ 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;
@@ -49,8 +42,6 @@ import org.osgi.service.component.annotations.Reference;
 @Component(service = ThingHandlerFactory.class, configurationPid = "binding.powermax")
 public class PowermaxHandlerFactory extends BaseThingHandlerFactory {
 
-    private final Map<ThingUID, ServiceRegistration<?>> discoveryServiceRegs = new HashMap<>();
-
     private final SerialPortManager serialPortManager;
     private final TimeZoneProvider timeZoneProvider;
 
@@ -89,40 +80,11 @@ public class PowermaxHandlerFactory extends BaseThingHandlerFactory {
         ThingTypeUID thingTypeUID = thing.getThingTypeUID();
 
         if (SUPPORTED_BRIDGE_TYPES_UIDS.contains(thingTypeUID)) {
-            PowermaxBridgeHandler handler = new PowermaxBridgeHandler((Bridge) thing, serialPortManager);
+            return new PowermaxBridgeHandler((Bridge) thing, serialPortManager);
-            registerDiscoveryService(handler);
-            return handler;
         } else if (SUPPORTED_THING_TYPES_UIDS.contains(thingTypeUID)) {
             return new PowermaxThingHandler(thing, timeZoneProvider);
         }
 
         return null;
     }
-
-    @Override
-    protected void removeHandler(ThingHandler thingHandler) {
-        if (thingHandler instanceof PowermaxBridgeHandler) {
-            // remove discovery service, if bridge handler is removed
-            unregisterDiscoveryService((PowermaxBridgeHandler) thingHandler);
-        }
-    }
-
-    private synchronized void registerDiscoveryService(PowermaxBridgeHandler bridgeHandler) {
-        PowermaxDiscoveryService discoveryService = new PowermaxDiscoveryService(bridgeHandler);
-        discoveryService.activate();
-        discoveryServiceRegs.put(bridgeHandler.getThing().getUID(),
-                bundleContext.registerService(DiscoveryService.class.getName(), discoveryService, new Hashtable<>()));
-    }
-
-    private synchronized void unregisterDiscoveryService(PowermaxBridgeHandler bridgeHandler) {
-        ServiceRegistration<?> serviceReg = discoveryServiceRegs.remove(bridgeHandler.getThing().getUID());
-        if (serviceReg != null) {
-            PowermaxDiscoveryService service = (PowermaxDiscoveryService) bundleContext
-                    .getService(serviceReg.getReference());
-            serviceReg.unregister();
-            if (service != null) {
-                service.deactivate();
-            }
-        }
-    }
 }

bundles/org.openhab.binding.powermax/src/main/java/org/openhab/binding/powermax/internal/discovery/PowermaxDiscoveryService.java

@@ -16,6 +16,8 @@ import java.util.Date;
 import java.util.HashMap;
 import java.util.Map;
 
+import org.eclipse.jdt.annotation.NonNullByDefault;
+import org.eclipse.jdt.annotation.Nullable;
 import org.openhab.binding.powermax.internal.PowermaxBindingConstants;
 import org.openhab.binding.powermax.internal.config.PowermaxX10Configuration;
 import org.openhab.binding.powermax.internal.config.PowermaxZoneConfiguration;
@@ -31,6 +33,7 @@ import org.openhab.core.config.discovery.DiscoveryResultBuilder;
 import org.openhab.core.thing.Thing;
 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;
 
@@ -39,28 +42,47 @@ import org.slf4j.LoggerFactory;
  * all enrolled zones and X10 devices
  *
  * @author Laurent Garnier - Initial contribution
+ * @author Laurent Garnier - Use ThingHandlerService
  */
-public class PowermaxDiscoveryService extends AbstractDiscoveryService implements PowermaxPanelSettingsListener {
+@NonNullByDefault
+public class PowermaxDiscoveryService extends AbstractDiscoveryService
+        implements PowermaxPanelSettingsListener, ThingHandlerService {
 
     private final Logger logger = LoggerFactory.getLogger(PowermaxDiscoveryService.class);
 
     private static final int SEARCH_TIME = 5;
 
-    private PowermaxBridgeHandler bridgeHandler;
+    private @Nullable PowermaxBridgeHandler bridgeHandler;
 
     /**
      * Creates a PowermaxDiscoveryService with background discovery disabled.
      */
-    public PowermaxDiscoveryService(PowermaxBridgeHandler bridgeHandler) {
+    public PowermaxDiscoveryService() {
         super(PowermaxBindingConstants.SUPPORTED_THING_TYPES_UIDS, SEARCH_TIME, true);
-        this.bridgeHandler = bridgeHandler;
+    }
+
+    @Override
+    public void setThingHandler(ThingHandler handler) {
+        if (handler instanceof PowermaxBridgeHandler) {
+            bridgeHandler = (PowermaxBridgeHandler) handler;
+        }
+    }
+
+    @Override
+    public @Nullable ThingHandler getThingHandler() {
+        return bridgeHandler;
     }
 
     /**
      * Activates the Discovery Service.
      */
+    @Override
     public void activate() {
-        bridgeHandler.registerPanelSettingsListener(this);
+        super.activate(null);
+        PowermaxBridgeHandler handler = bridgeHandler;
+        if (handler != null) {
+            handler.registerPanelSettingsListener(this);
+        }
     }
 
     /**
@@ -68,30 +90,38 @@ public class PowermaxDiscoveryService extends AbstractDiscoveryService implement
      */
     @Override
     public void deactivate() {
-        bridgeHandler.unregisterPanelSettingsListener(this);
+        PowermaxBridgeHandler handler = bridgeHandler;
+        if (handler != null) {
+            handler.unregisterPanelSettingsListener(this);
+        }
+        super.deactivate();
     }
 
     @Override
     protected void startScan() {
         logger.debug("Updating discovered things (new scan)");
-        updateFromSettings(bridgeHandler.getPanelSettings());
+        PowermaxBridgeHandler handler = bridgeHandler;
+        if (handler != null) {
+            updateFromSettings(handler.getPanelSettings());
+        }
     }
 
     @Override
-    public void onPanelSettingsUpdated(PowermaxPanelSettings settings) {
+    public void onPanelSettingsUpdated(@Nullable PowermaxPanelSettings settings) {
         logger.debug("Updating discovered things (global settings updated)");
         updateFromSettings(settings);
     }
 
     @Override
-    public void onZoneSettingsUpdated(int zoneNumber, PowermaxPanelSettings settings) {
+    public void onZoneSettingsUpdated(int zoneNumber, @Nullable PowermaxPanelSettings settings) {
         logger.debug("Updating discovered things (zone {} updated)", zoneNumber);
         PowermaxZoneSettings zoneSettings = (settings == null) ? null : settings.getZoneSettings(zoneNumber);
         updateFromZoneSettings(zoneNumber, zoneSettings);
     }
 
-    private void updateFromSettings(PowermaxPanelSettings settings) {
+    private void updateFromSettings(@Nullable PowermaxPanelSettings settings) {
-        if (settings != null) {
+        PowermaxBridgeHandler handler = bridgeHandler;
+        if (handler != null && settings != null) {
             long beforeUpdate = new Date().getTime();
 
             for (int i = 1; i <= settings.getNbZones(); i++) {
@@ -105,14 +135,15 @@ public class PowermaxDiscoveryService extends AbstractDiscoveryService implement
             }
 
             // Remove not updated discovered things
-            removeOlderResults(beforeUpdate, bridgeHandler.getThing().getUID());
+            removeOlderResults(beforeUpdate, handler.getThing().getUID());
         }
     }
 
-    private void updateFromZoneSettings(int zoneNumber, PowermaxZoneSettings zoneSettings) {
+    private void updateFromZoneSettings(int zoneNumber, @Nullable PowermaxZoneSettings zoneSettings) {
-        if (zoneSettings != null) {
+        PowermaxBridgeHandler handler = bridgeHandler;
+        if (handler != null && zoneSettings != null) {
             // Prevent for adding already known zone
-            for (Thing thing : bridgeHandler.getThing().getThings()) {
+            for (Thing thing : handler.getThing().getThings()) {
                 ThingHandler thingHandler = thing.getHandler();
                 if (thing.getThingTypeUID().equals(PowermaxBindingConstants.THING_TYPE_ZONE)
                         && thingHandler instanceof PowermaxThingHandler) {
@@ -123,7 +154,7 @@ public class PowermaxDiscoveryService extends AbstractDiscoveryService implement
                 }
             }
 
-            ThingUID bridgeUID = bridgeHandler.getThing().getUID();
+            ThingUID bridgeUID = handler.getThing().getUID();
             ThingUID thingUID = new ThingUID(PowermaxBindingConstants.THING_TYPE_ZONE, bridgeUID,
                     String.valueOf(zoneNumber));
             String sensorType = zoneSettings.getSensorType();
@@ -144,10 +175,11 @@ public class PowermaxDiscoveryService extends AbstractDiscoveryService implement
         }
     }
 
-    private void updateFromDeviceSettings(int deviceNumber, PowermaxX10Settings deviceSettings) {
+    private void updateFromDeviceSettings(int deviceNumber, @Nullable PowermaxX10Settings deviceSettings) {
-        if (deviceSettings != null && deviceSettings.isEnabled()) {
+        PowermaxBridgeHandler handler = bridgeHandler;
+        if (handler != null && deviceSettings != null && deviceSettings.isEnabled()) {
             // Prevent for adding already known X10 device
-            for (Thing thing : bridgeHandler.getThing().getThings()) {
+            for (Thing thing : handler.getThing().getThings()) {
                 ThingHandler thingHandler = thing.getHandler();
                 if (thing.getThingTypeUID().equals(PowermaxBindingConstants.THING_TYPE_X10)
                         && thingHandler instanceof PowermaxThingHandler) {
@@ -158,7 +190,7 @@ public class PowermaxDiscoveryService extends AbstractDiscoveryService implement
                 }
             }
 
-            ThingUID bridgeUID = bridgeHandler.getThing().getUID();
+            ThingUID bridgeUID = handler.getThing().getUID();
             ThingUID thingUID = new ThingUID(PowermaxBindingConstants.THING_TYPE_X10, bridgeUID,
                     String.valueOf(deviceNumber));
             String name = (deviceSettings.getName() != null) ? deviceSettings.getName()

bundles/org.openhab.binding.powermax/src/main/java/org/openhab/binding/powermax/internal/handler/PowermaxBridgeHandler.java

@@ -14,6 +14,8 @@ package org.openhab.binding.powermax.internal.handler;
 
 import static org.openhab.binding.powermax.internal.PowermaxBindingConstants.*;
 
+import java.util.Collection;
+import java.util.Collections;
 import java.util.EventObject;
 import java.util.List;
 import java.util.Map;
@@ -23,6 +25,7 @@ import java.util.concurrent.TimeUnit;
 
 import org.openhab.binding.powermax.internal.config.PowermaxIpConfiguration;
 import org.openhab.binding.powermax.internal.config.PowermaxSerialConfiguration;
+import org.openhab.binding.powermax.internal.discovery.PowermaxDiscoveryService;
 import org.openhab.binding.powermax.internal.message.PowermaxCommManager;
 import org.openhab.binding.powermax.internal.state.PowermaxArmMode;
 import org.openhab.binding.powermax.internal.state.PowermaxPanelSettings;
@@ -40,6 +43,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.openhab.core.types.RefreshType;
 import org.slf4j.Logger;
@@ -95,6 +99,11 @@ public class PowermaxBridgeHandler extends BaseBridgeHandler implements Powermax
         this.serialPortManager = serialPortManager;
     }
 
+    @Override
+    public Collection<Class<? extends ThingHandlerService>> getServices() {
+        return Collections.singleton(PowermaxDiscoveryService.class);
+    }
+
     public PowermaxState getCurrentState() {
         return currentState;
     }

bundles/org.openhab.binding.powermax/src/main/java/org/openhab/binding/powermax/internal/handler/PowermaxThingHandler.java

@@ -17,6 +17,7 @@ import static org.openhab.binding.powermax.internal.PowermaxBindingConstants.*;
 import java.time.Instant;
 import java.time.ZonedDateTime;
 
+import org.eclipse.jdt.annotation.Nullable;
 import org.openhab.binding.powermax.internal.config.PowermaxX10Configuration;
 import org.openhab.binding.powermax.internal.config.PowermaxZoneConfiguration;
 import org.openhab.binding.powermax.internal.state.PowermaxPanelSettings;
@@ -202,7 +203,7 @@ public class PowermaxThingHandler extends BaseThingHandler implements PowermaxPa
     }
 
     @Override
-    public void onPanelSettingsUpdated(PowermaxPanelSettings settings) {
+    public void onPanelSettingsUpdated(@Nullable PowermaxPanelSettings settings) {
         if (getThing().getThingTypeUID().equals(THING_TYPE_ZONE)) {
             PowermaxZoneConfiguration config = getConfigAs(PowermaxZoneConfiguration.class);
             onZoneSettingsUpdated(config.zoneNumber, settings);
@@ -234,7 +235,7 @@ public class PowermaxThingHandler extends BaseThingHandler implements PowermaxPa
     }
 
     @Override
-    public void onZoneSettingsUpdated(int zoneNumber, PowermaxPanelSettings settings) {
+    public void onZoneSettingsUpdated(int zoneNumber, @Nullable PowermaxPanelSettings settings) {
         if (getThing().getThingTypeUID().equals(THING_TYPE_ZONE)) {
             PowermaxZoneConfiguration config = getConfigAs(PowermaxZoneConfiguration.class);
             if (zoneNumber == config.zoneNumber) {

bundles/org.openhab.binding.powermax/src/main/java/org/openhab/binding/powermax/internal/state/PowermaxPanelSettingsListener.java

@@ -12,12 +12,16 @@
  */
 package org.openhab.binding.powermax.internal.state;
 
+import org.eclipse.jdt.annotation.NonNullByDefault;
+import org.eclipse.jdt.annotation.Nullable;
+
 /**
  * The {@link PowermaxPanelSettingsListener} is a listener for updated
  * alarm panel settings
  *
  * @author Laurent Garnier - Initial contribution
  */
+@NonNullByDefault
 public interface PowermaxPanelSettingsListener {
 
     /**
@@ -26,7 +30,7 @@ public interface PowermaxPanelSettingsListener {
      *
      * @param settings the updated alarm panel settings or null if the panel settings are unknown
      */
-    public void onPanelSettingsUpdated(PowermaxPanelSettings settings);
+    public void onPanelSettingsUpdated(@Nullable PowermaxPanelSettings settings);
 
     /**
      * This method is called when the bridge thing handler identifies
@@ -35,5 +39,5 @@ public interface PowermaxPanelSettingsListener {
      * @param zoneNumber the zone number
      * @param settings the updated alarm panel settings or null if the panel settings are unknown
      */
-    public void onZoneSettingsUpdated(int zoneNumber, PowermaxPanelSettings settings);
+    public void onZoneSettingsUpdated(int zoneNumber, @Nullable PowermaxPanelSettings settings);
 }