From f9e38cbf2f6482aee53c620ced8c3197767758ad Mon Sep 17 00:00:00 2001 From: lolodomo Date: Tue, 24 Nov 2020 19:42:00 +0100 Subject: [PATCH] [freebox] Use ThingHandlerService for discovery (#9088) Signed-off-by: Laurent Garnier --- .../freebox/internal/FreeboxDataListener.java | 15 +-- .../internal/FreeboxHandlerFactory.java | 32 +----- .../discovery/FreeboxDiscoveryService.java | 99 ++++++++++--------- .../internal/handler/FreeboxHandler.java | 21 ++-- 4 files changed, 70 insertions(+), 97 deletions(-) diff --git a/bundles/org.openhab.binding.freebox/src/main/java/org/openhab/binding/freebox/internal/FreeboxDataListener.java b/bundles/org.openhab.binding.freebox/src/main/java/org/openhab/binding/freebox/internal/FreeboxDataListener.java index 694c9f785bd..d32bcecfd42 100644 --- a/bundles/org.openhab.binding.freebox/src/main/java/org/openhab/binding/freebox/internal/FreeboxDataListener.java +++ b/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 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 lanHosts, - List airPlayDevices); + public void onDataFetched(ThingUID bridge, @Nullable List lanHosts, + @Nullable List airPlayDevices); } diff --git a/bundles/org.openhab.binding.freebox/src/main/java/org/openhab/binding/freebox/internal/FreeboxHandlerFactory.java b/bundles/org.openhab.binding.freebox/src/main/java/org/openhab/binding/freebox/internal/FreeboxHandlerFactory.java index d2d6e884c4a..4e17d85a8fc 100644 --- a/bundles/org.openhab.binding.freebox/src/main/java/org/openhab/binding/freebox/internal/FreeboxHandlerFactory.java +++ b/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> discoveryServiceRegs = new HashMap<>(); private final Map> 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); - registerDiscoveryService(handler); - return handler; + return new FreeboxHandler((Bridge) thing); } 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) { - unregisterDiscoveryService(thingHandler.getThing()); - } else if (thingHandler instanceof FreeboxThingHandler) { + 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); diff --git a/bundles/org.openhab.binding.freebox/src/main/java/org/openhab/binding/freebox/internal/discovery/FreeboxDiscoveryService.java b/bundles/org.openhab.binding.freebox/src/main/java/org/openhab/binding/freebox/internal/discovery/FreeboxDiscoveryService.java index 41a0d2532f4..75b13aab48a 100644 --- a/bundles/org.openhab.binding.freebox/src/main/java/org/openhab/binding/freebox/internal/discovery/FreeboxDiscoveryService.java +++ b/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 configProperties) { - super.activate(configProperties); - applyConfig(configProperties); - bridgeHandler.registerDataListener(this); + public void setThingHandler(ThingHandler handler) { + if (handler instanceof FreeboxHandler) { + bridgeHandler = (FreeboxHandler) handler; + } + } + + @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); - super.deactivate(); - } - - @Override - public void applyConfig(Map 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(); - } + FreeboxHandler handler = bridgeHandler; + if (handler != null) { + handler.unregisterDataListener(this); } - logger.debug("Freebox discovery - discoverPhone : {}", discoverPhone); - logger.debug("Freebox discovery - discoverNetDevice : {}", discoverNetDevice); - logger.debug("Freebox discovery - discoverNetInterface : {}", discoverNetInterface); - logger.debug("Freebox discovery - discoverAirPlayReceiver : {}", discoverAirPlayReceiver); + super.deactivate(); } @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 lanHosts = bridgeHandler.getApiManager().getLanHosts(); - List airPlayDevices = bridgeHandler.getApiManager().getAirMediaReceivers(); - onDataFetched(bridgeHandler.getThing().getUID(), lanHosts, airPlayDevices); + List lanHosts = handler.getApiManager().getLanHosts(); + List airPlayDevices = handler.getApiManager().getAirMediaReceivers(); + 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 lanHosts, - List airPlayDevices) { - if (bridge == null) { - return; - } - + public void onDataFetched(ThingUID bridge, @Nullable List lanHosts, + @Nullable List airPlayDevices) { ThingUID thingUID; DiscoveryResult discoveryResult; diff --git a/bundles/org.openhab.binding.freebox/src/main/java/org/openhab/binding/freebox/internal/handler/FreeboxHandler.java b/bundles/org.openhab.binding.freebox/src/main/java/org/openhab/binding/freebox/internal/handler/FreeboxHandler.java index 554b4edc220..4ace7dd1c6c 100644 --- a/bundles/org.openhab.binding.freebox/src/main/java/org/openhab/binding/freebox/internal/handler/FreeboxHandler.java +++ b/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> 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 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,