[freebox] Use ThingHandlerService for discovery (#9088)

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

bundles/org.openhab.binding.freebox/src/main/java/org/openhab/binding/freebox/internal/FreeboxDataListener.java

@@ -13,8 +13,9 @@
 package org.openhab.binding.freebox.internal;
 
 import java.util.List;
-import java.util.Map;
 
+import org.eclipse.jdt.annotation.NonNullByDefault;
+import org.eclipse.jdt.annotation.Nullable;
 import org.openhab.binding.freebox.internal.api.model.FreeboxAirMediaReceiver;
 import org.openhab.binding.freebox.internal.api.model.FreeboxLanHost;
 import org.openhab.core.thing.ThingUID;
@@ -27,15 +28,9 @@ import org.openhab.core.thing.ThingUID;
  * @author Laurent Garnier - add discovery configuration
  * @author Laurent Garnier - use new internal classes
  */
+@NonNullByDefault
 public interface FreeboxDataListener {
 
-    /**
-     * Update the discovery configuration.
-     *
-     * @param configProperties the configuration
-     */
-    public void applyConfig(Map<String, Object> configProperties);
-
     /**
      * This method is called just after the bridge thing handler fetched new data
      * from the Freebox server.
@@ -44,6 +39,6 @@ public interface FreeboxDataListener {
      * @param lanHosts the LAN data received from the Freebox server.
      * @param airPlayDevices the list of AirPlay devices received from the Freebox server.
      */
-    public void onDataFetched(ThingUID bridge, List<FreeboxLanHost> lanHosts,
+    public void onDataFetched(ThingUID bridge, @Nullable List<FreeboxLanHost> lanHosts,
-            List<FreeboxAirMediaReceiver> airPlayDevices);
+            @Nullable List<FreeboxAirMediaReceiver> airPlayDevices);
 }

bundles/org.openhab.binding.freebox/src/main/java/org/openhab/binding/freebox/internal/FreeboxHandlerFactory.java

@@ -13,7 +13,6 @@
 package org.openhab.binding.freebox.internal;
 
 import java.util.Dictionary;
-import java.util.HashMap;
 import java.util.Hashtable;
 import java.util.Map;
 import java.util.Set;
@@ -23,13 +22,11 @@ import java.util.stream.Stream;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
-import org.openhab.binding.freebox.internal.discovery.FreeboxDiscoveryService;
 import org.openhab.binding.freebox.internal.handler.FreeboxHandler;
 import org.openhab.binding.freebox.internal.handler.FreeboxThingHandler;
 import org.openhab.core.audio.AudioHTTPServer;
 import org.openhab.core.audio.AudioSink;
 import org.openhab.core.config.core.Configuration;
-import org.openhab.core.config.discovery.DiscoveryService;
 import org.openhab.core.i18n.TimeZoneProvider;
 import org.openhab.core.net.HttpServiceUtil;
 import org.openhab.core.net.NetworkAddressService;
@@ -66,7 +63,6 @@ public class FreeboxHandlerFactory extends BaseThingHandlerFactory {
 
     private final Logger logger = LoggerFactory.getLogger(FreeboxHandlerFactory.class);
 
-    private final Map<ThingUID, ServiceRegistration<?>> discoveryServiceRegs = new HashMap<>();
     private final Map<ThingUID, ServiceRegistration<AudioSink>> audioSinkRegistrations = new ConcurrentHashMap<>();
 
     private final AudioHTTPServer audioHTTPServer;
@@ -120,9 +116,7 @@ public class FreeboxHandlerFactory extends BaseThingHandlerFactory {
         ThingTypeUID thingTypeUID = thing.getThingTypeUID();
 
         if (thingTypeUID.equals(FreeboxBindingConstants.FREEBOX_BRIDGE_TYPE_SERVER)) {
-            FreeboxHandler handler = new FreeboxHandler((Bridge) thing);
+            return new FreeboxHandler((Bridge) thing);
-            registerDiscoveryService(handler);
-            return handler;
         } else if (FreeboxBindingConstants.SUPPORTED_THING_TYPES_UIDS.contains(thingTypeUID)) {
             FreeboxThingHandler handler = new FreeboxThingHandler(thing, timeZoneProvider);
             if (FreeboxBindingConstants.FREEBOX_THING_TYPE_AIRPLAY.equals(thingTypeUID)) {
@@ -136,33 +130,11 @@ public class FreeboxHandlerFactory extends BaseThingHandlerFactory {
 
     @Override
     protected void removeHandler(ThingHandler thingHandler) {
-        if (thingHandler instanceof FreeboxHandler) {
+        if (thingHandler instanceof FreeboxThingHandler) {
-            unregisterDiscoveryService(thingHandler.getThing());
-        } else if (thingHandler instanceof FreeboxThingHandler) {
             unregisterAudioSink(thingHandler.getThing());
         }
     }
 
-    private synchronized void registerDiscoveryService(FreeboxHandler bridgeHandler) {
-        FreeboxDiscoveryService discoveryService = new FreeboxDiscoveryService(bridgeHandler);
-        discoveryService.activate(null);
-        discoveryServiceRegs.put(bridgeHandler.getThing().getUID(),
-                bundleContext.registerService(DiscoveryService.class.getName(), discoveryService, new Hashtable<>()));
-    }
-
-    private synchronized void unregisterDiscoveryService(Thing thing) {
-        ServiceRegistration<?> serviceReg = discoveryServiceRegs.remove(thing.getUID());
-        if (serviceReg != null) {
-            // remove discovery service, if bridge handler is removed
-            FreeboxDiscoveryService service = (FreeboxDiscoveryService) bundleContext
-                    .getService(serviceReg.getReference());
-            serviceReg.unregister();
-            if (service != null) {
-                service.deactivate();
-            }
-        }
-    }
-
     private synchronized void registerAudioSink(FreeboxThingHandler thingHandler) {
         String callbackUrl = createCallbackUrl();
         FreeboxAirPlayAudioSink audioSink = new FreeboxAirPlayAudioSink(thingHandler, audioHTTPServer, callbackUrl);

bundles/org.openhab.binding.freebox/src/main/java/org/openhab/binding/freebox/internal/discovery/FreeboxDiscoveryService.java

@@ -16,6 +16,8 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
+import org.eclipse.jdt.annotation.NonNullByDefault;
+import org.eclipse.jdt.annotation.Nullable;
 import org.openhab.binding.freebox.internal.FreeboxBindingConstants;
 import org.openhab.binding.freebox.internal.FreeboxDataListener;
 import org.openhab.binding.freebox.internal.api.FreeboxException;
@@ -27,12 +29,15 @@ import org.openhab.binding.freebox.internal.config.FreeboxNetDeviceConfiguration
 import org.openhab.binding.freebox.internal.config.FreeboxNetInterfaceConfiguration;
 import org.openhab.binding.freebox.internal.config.FreeboxServerConfiguration;
 import org.openhab.binding.freebox.internal.handler.FreeboxHandler;
+import org.openhab.core.config.core.Configuration;
 import org.openhab.core.config.discovery.AbstractDiscoveryService;
 import org.openhab.core.config.discovery.DiscoveryResult;
 import org.openhab.core.config.discovery.DiscoveryResultBuilder;
 import org.openhab.core.thing.Thing;
 import org.openhab.core.thing.ThingStatus;
 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;
 
@@ -43,8 +48,11 @@ import org.slf4j.LoggerFactory;
  * @author Laurent Garnier - Initial contribution
  * @author Laurent Garnier - add discovery settings
  * @author Laurent Garnier - use new internal API manager
+ * @author Laurent Garnier - use ThingHandlerService
  */
-public class FreeboxDiscoveryService extends AbstractDiscoveryService implements FreeboxDataListener {
+@NonNullByDefault
+public class FreeboxDiscoveryService extends AbstractDiscoveryService
+        implements FreeboxDataListener, ThingHandlerService {
 
     private final Logger logger = LoggerFactory.getLogger(FreeboxDiscoveryService.class);
 
@@ -52,7 +60,7 @@ public class FreeboxDiscoveryService extends AbstractDiscoveryService implements
 
     private static final String PHONE_ID = "wired";
 
-    private FreeboxHandler bridgeHandler;
+    private @Nullable FreeboxHandler bridgeHandler;
     private boolean discoverPhone;
     private boolean discoverNetDevice;
     private boolean discoverNetInterface;
@@ -61,9 +69,8 @@ public class FreeboxDiscoveryService extends AbstractDiscoveryService implements
     /**
      * Creates a FreeboxDiscoveryService with background discovery disabled.
      */
-    public FreeboxDiscoveryService(FreeboxHandler freeboxBridgeHandler) {
+    public FreeboxDiscoveryService() {
         super(FreeboxBindingConstants.SUPPORTED_THING_TYPES_UIDS, SEARCH_TIME, false);
-        this.bridgeHandler = freeboxBridgeHandler;
         this.discoverPhone = true;
         this.discoverNetDevice = true;
         this.discoverNetInterface = true;
@@ -71,52 +78,58 @@ public class FreeboxDiscoveryService extends AbstractDiscoveryService implements
     }
 
     @Override
-    public void activate(Map<String, Object> configProperties) {
+    public void setThingHandler(ThingHandler handler) {
-        super.activate(configProperties);
+        if (handler instanceof FreeboxHandler) {
-        applyConfig(configProperties);
+            bridgeHandler = (FreeboxHandler) handler;
-        bridgeHandler.registerDataListener(this);
+        }
+    }
+
+    @Override
+    public @Nullable ThingHandler getThingHandler() {
+        return bridgeHandler;
+    }
+
+    @Override
+    public void activate() {
+        super.activate(null);
+        FreeboxHandler handler = bridgeHandler;
+        if (handler != null) {
+            Configuration config = handler.getThing().getConfiguration();
+            Object property = config.get(FreeboxServerConfiguration.DISCOVER_PHONE);
+            discoverPhone = property != null ? ((Boolean) property).booleanValue() : true;
+            property = config.get(FreeboxServerConfiguration.DISCOVER_NET_DEVICE);
+            discoverNetDevice = property != null ? ((Boolean) property).booleanValue() : true;
+            property = config.get(FreeboxServerConfiguration.DISCOVER_NET_INTERFACE);
+            discoverNetInterface = property != null ? ((Boolean) property).booleanValue() : true;
+            property = config.get(FreeboxServerConfiguration.DISCOVER_AIRPLAY_RECEIVER);
+            discoverAirPlayReceiver = property != null ? ((Boolean) property).booleanValue() : true;
+            logger.debug("Freebox discovery - discoverPhone : {}", discoverPhone);
+            logger.debug("Freebox discovery - discoverNetDevice : {}", discoverNetDevice);
+            logger.debug("Freebox discovery - discoverNetInterface : {}", discoverNetInterface);
+            logger.debug("Freebox discovery - discoverAirPlayReceiver : {}", discoverAirPlayReceiver);
+
+            handler.registerDataListener(this);
+        }
     }
 
     @Override
     public void deactivate() {
-        bridgeHandler.unregisterDataListener(this);
+        FreeboxHandler handler = bridgeHandler;
-        super.deactivate();
+        if (handler != null) {
-    }
+            handler.unregisterDataListener(this);
-
-    @Override
-    public void applyConfig(Map<String, Object> configProperties) {
-        if (configProperties != null) {
-            Object property = configProperties.get(FreeboxServerConfiguration.DISCOVER_PHONE);
-            if (property != null) {
-                discoverPhone = ((Boolean) property).booleanValue();
-            }
-            property = configProperties.get(FreeboxServerConfiguration.DISCOVER_NET_DEVICE);
-            if (property != null) {
-                discoverNetDevice = ((Boolean) property).booleanValue();
-            }
-            property = configProperties.get(FreeboxServerConfiguration.DISCOVER_NET_INTERFACE);
-            if (property != null) {
-                discoverNetInterface = ((Boolean) property).booleanValue();
-            }
-            property = configProperties.get(FreeboxServerConfiguration.DISCOVER_AIRPLAY_RECEIVER);
-            if (property != null) {
-                discoverAirPlayReceiver = ((Boolean) property).booleanValue();
-            }
         }
-        logger.debug("Freebox discovery - discoverPhone : {}", discoverPhone);
+        super.deactivate();
-        logger.debug("Freebox discovery - discoverNetDevice : {}", discoverNetDevice);
-        logger.debug("Freebox discovery - discoverNetInterface : {}", discoverNetInterface);
-        logger.debug("Freebox discovery - discoverAirPlayReceiver : {}", discoverAirPlayReceiver);
     }
 
     @Override
     protected void startScan() {
         logger.debug("Starting Freebox discovery scan");
-        if (bridgeHandler.getThing().getStatus() == ThingStatus.ONLINE) {
+        FreeboxHandler handler = bridgeHandler;
+        if (handler != null && handler.getThing().getStatus() == ThingStatus.ONLINE) {
             try {
-                List<FreeboxLanHost> lanHosts = bridgeHandler.getApiManager().getLanHosts();
+                List<FreeboxLanHost> lanHosts = handler.getApiManager().getLanHosts();
-                List<FreeboxAirMediaReceiver> airPlayDevices = bridgeHandler.getApiManager().getAirMediaReceivers();
+                List<FreeboxAirMediaReceiver> airPlayDevices = handler.getApiManager().getAirMediaReceivers();
-                onDataFetched(bridgeHandler.getThing().getUID(), lanHosts, airPlayDevices);
+                onDataFetched(handler.getThing().getUID(), lanHosts, airPlayDevices);
             } catch (FreeboxException e) {
                 logger.warn("Error while requesting data for things discovery", e);
             }
@@ -124,12 +137,8 @@ public class FreeboxDiscoveryService extends AbstractDiscoveryService implements
     }
 
     @Override
-    public void onDataFetched(ThingUID bridge, List<FreeboxLanHost> lanHosts,
+    public void onDataFetched(ThingUID bridge, @Nullable List<FreeboxLanHost> lanHosts,
-            List<FreeboxAirMediaReceiver> airPlayDevices) {
+            @Nullable List<FreeboxAirMediaReceiver> airPlayDevices) {
-        if (bridge == null) {
-            return;
-        }
-
         ThingUID thingUID;
         DiscoveryResult discoveryResult;
 

bundles/org.openhab.binding.freebox/src/main/java/org/openhab/binding/freebox/internal/handler/FreeboxHandler.java

@@ -15,7 +15,8 @@ package org.openhab.binding.freebox.internal.handler;
 import static org.openhab.binding.freebox.internal.FreeboxBindingConstants.*;
 
 import java.util.ArrayList;
-import java.util.HashMap;
+import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.CopyOnWriteArrayList;
@@ -33,6 +34,7 @@ import org.openhab.binding.freebox.internal.api.model.FreeboxLcdConfig;
 import org.openhab.binding.freebox.internal.api.model.FreeboxSambaConfig;
 import org.openhab.binding.freebox.internal.api.model.FreeboxSystemConfig;
 import org.openhab.binding.freebox.internal.config.FreeboxServerConfiguration;
+import org.openhab.binding.freebox.internal.discovery.FreeboxDiscoveryService;
 import org.openhab.core.config.core.Configuration;
 import org.openhab.core.library.types.DecimalType;
 import org.openhab.core.library.types.IncreaseDecreaseType;
@@ -48,6 +50,7 @@ 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.ThingHandler;
+import org.openhab.core.thing.binding.ThingHandlerService;
 import org.openhab.core.types.Command;
 import org.openhab.core.types.RefreshType;
 import org.osgi.framework.Bundle;
@@ -87,6 +90,11 @@ public class FreeboxHandler extends BaseBridgeHandler {
         uptime = -1;
     }
 
+    @Override
+    public Collection<Class<? extends ThingHandlerService>> getServices() {
+        return Collections.singleton(FreeboxDiscoveryService.class);
+    }
+
     @Override
     public void handleCommand(ChannelUID channelUID, Command command) {
         if (command instanceof RefreshType) {
@@ -140,17 +148,6 @@ public class FreeboxHandler extends BaseBridgeHandler {
 
         configuration = getConfigAs(FreeboxServerConfiguration.class);
 
-        // Update the discovery configuration
-        Map<String, Object> configDiscovery = new HashMap<>();
-        configDiscovery.put(FreeboxServerConfiguration.DISCOVER_PHONE, configuration.discoverPhone);
-        configDiscovery.put(FreeboxServerConfiguration.DISCOVER_NET_DEVICE, configuration.discoverNetDevice);
-        configDiscovery.put(FreeboxServerConfiguration.DISCOVER_NET_INTERFACE, configuration.discoverNetInterface);
-        configDiscovery.put(FreeboxServerConfiguration.DISCOVER_AIRPLAY_RECEIVER,
-                configuration.discoverAirPlayReceiver);
-        for (FreeboxDataListener dataListener : dataListeners) {
-            dataListener.applyConfig(configDiscovery);
-        }
-
         if (configuration.fqdn != null && !configuration.fqdn.isEmpty()) {
             if (configuration.appToken == null || configuration.appToken.isEmpty()) {
                 updateStatus(ThingStatus.UNKNOWN, ThingStatusDetail.CONFIGURATION_PENDING,