From 5d99a7f524bc0c03b2836c8fcd940ed9812f79cb Mon Sep 17 00:00:00 2001 From: Wouter Born Date: Thu, 8 Apr 2021 22:49:14 +0200 Subject: [PATCH] [dsmr] Use ThingHandlerService for discovery (#9044) This simplifies the DSMRHandlerFactory code so it no longer needs to register and keep track of a discovery service for each bridge. Also contains a few other improvements: * more constructor injection * add a few missing @NonNullByDefault on test classes Signed-off-by: Wouter Born --- .../dsmr/internal/DSMRHandlerFactory.java | 81 ++----------------- .../discovery/DSMRBridgeDiscoveryService.java | 38 +++------ .../discovery/DSMRI18nProviderTracker.java | 50 ++++++++++++ .../discovery/DSMRMeterDiscoveryService.java | 48 ++++++----- .../internal/handler/DSMRBridgeHandler.java | 8 ++ .../DSMRMeterDiscoveryServiceTest.java | 6 +- 6 files changed, 105 insertions(+), 126 deletions(-) create mode 100644 bundles/org.openhab.binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/discovery/DSMRI18nProviderTracker.java diff --git a/bundles/org.openhab.binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/DSMRHandlerFactory.java b/bundles/org.openhab.binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/DSMRHandlerFactory.java index 26cfe9da267..0a3162174e5 100644 --- a/bundles/org.openhab.binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/DSMRHandlerFactory.java +++ b/bundles/org.openhab.binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/DSMRHandlerFactory.java @@ -14,28 +14,19 @@ package org.openhab.binding.dsmr.internal; import static org.openhab.binding.dsmr.internal.DSMRBindingConstants.*; -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.dsmr.internal.discovery.DSMRMeterDiscoveryService; import org.openhab.binding.dsmr.internal.handler.DSMRBridgeHandler; import org.openhab.binding.dsmr.internal.handler.DSMRMeterHandler; import org.openhab.binding.dsmr.internal.meter.DSMRMeterType; -import org.openhab.core.config.discovery.DiscoveryService; -import org.openhab.core.i18n.LocaleProvider; -import org.openhab.core.i18n.TranslationProvider; 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; import org.slf4j.Logger; @@ -53,11 +44,12 @@ public class DSMRHandlerFactory extends BaseThingHandlerFactory { private final Logger logger = LoggerFactory.getLogger(DSMRHandlerFactory.class); - private final Map> discoveryServiceRegs = new HashMap<>(); + private final SerialPortManager serialPortManager; - private @NonNullByDefault({}) SerialPortManager serialPortManager; - private @NonNullByDefault({}) LocaleProvider localeProvider; - private @NonNullByDefault({}) TranslationProvider i18nProvider; + @Activate + public DSMRHandlerFactory(@Reference SerialPortManager serialPortManager) { + this.serialPortManager = serialPortManager; + } /** * Returns if the specified ThingTypeUID is supported by this handler. @@ -100,70 +92,11 @@ public class DSMRHandlerFactory extends BaseThingHandlerFactory { logger.debug("Searching for thingTypeUID {}", thingTypeUID); if (THING_TYPE_DSMR_BRIDGE.equals(thingTypeUID) || THING_TYPE_SMARTY_BRIDGE.equals(thingTypeUID)) { - DSMRBridgeHandler handler = new DSMRBridgeHandler((Bridge) thing, serialPortManager); - registerDiscoveryService(handler); - return handler; + return new DSMRBridgeHandler((Bridge) thing, serialPortManager); } else if (DSMRMeterType.METER_THING_TYPES.contains(thingTypeUID)) { return new DSMRMeterHandler(thing); } return null; } - - /** - * Registers a meter discovery service for the bridge handler. - * - * @param bridgeHandler handler to register service for - */ - private synchronized void registerDiscoveryService(DSMRBridgeHandler bridgeHandler) { - DSMRMeterDiscoveryService discoveryService = new DSMRMeterDiscoveryService(bridgeHandler); - - discoveryService.setLocaleProvider(localeProvider); - discoveryService.setTranslationProvider(i18nProvider); - this.discoveryServiceRegs.put(bridgeHandler.getThing().getUID(), - bundleContext.registerService(DiscoveryService.class.getName(), discoveryService, new Hashtable<>())); - } - - @Override - protected synchronized void removeHandler(ThingHandler thingHandler) { - if (thingHandler instanceof DSMRBridgeHandler) { - ServiceRegistration serviceReg = this.discoveryServiceRegs.remove(thingHandler.getThing().getUID()); - if (serviceReg != null) { - DSMRMeterDiscoveryService service = (DSMRMeterDiscoveryService) getBundleContext() - .getService(serviceReg.getReference()); - serviceReg.unregister(); - if (service != null) { - service.unsetLocaleProvider(); - service.unsetTranslationProvider(); - } - } - } - } - - @Reference - protected void setSerialPortManager(final SerialPortManager serialPortManager) { - this.serialPortManager = serialPortManager; - } - - protected void unsetSerialPortManager(final SerialPortManager serialPortManager) { - this.serialPortManager = null; - } - - @Reference - protected void setLocaleProvider(final LocaleProvider localeProvider) { - this.localeProvider = localeProvider; - } - - protected void unsetLocaleProvider(final LocaleProvider localeProvider) { - this.localeProvider = null; - } - - @Reference - protected void setTranslationProvider(TranslationProvider i18nProvider) { - this.i18nProvider = i18nProvider; - } - - protected void unsetTranslationProvider(TranslationProvider i18nProvider) { - this.i18nProvider = null; - } } diff --git a/bundles/org.openhab.binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/discovery/DSMRBridgeDiscoveryService.java b/bundles/org.openhab.binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/discovery/DSMRBridgeDiscoveryService.java index 06bf01f8740..2c7f5baed16 100644 --- a/bundles/org.openhab.binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/discovery/DSMRBridgeDiscoveryService.java +++ b/bundles/org.openhab.binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/discovery/DSMRBridgeDiscoveryService.java @@ -38,6 +38,7 @@ import org.openhab.core.io.transport.serial.SerialPortIdentifier; import org.openhab.core.io.transport.serial.SerialPortManager; import org.openhab.core.thing.ThingTypeUID; import org.openhab.core.thing.ThingUID; +import org.osgi.service.component.annotations.Activate; import org.osgi.service.component.annotations.Component; import org.osgi.service.component.annotations.Reference; import org.slf4j.Logger; @@ -74,7 +75,7 @@ public class DSMRBridgeDiscoveryService extends DSMRDiscoveryService implements /** * Serial Port Manager. */ - private @NonNullByDefault({}) SerialPortManager serialPortManager; + private final SerialPortManager serialPortManager; /** * DSMR Device that is scanned when discovery process in progress. @@ -91,6 +92,14 @@ public class DSMRBridgeDiscoveryService extends DSMRDiscoveryService implements */ private boolean scanning; + @Activate + public DSMRBridgeDiscoveryService(final @Reference TranslationProvider i18nProvider, + final @Reference LocaleProvider localeProvider, final @Reference SerialPortManager serialPortManager) { + this.i18nProvider = i18nProvider; + this.localeProvider = localeProvider; + this.serialPortManager = serialPortManager; + } + /** * Starts a new discovery scan. * @@ -203,31 +212,4 @@ public class DSMRBridgeDiscoveryService extends DSMRDiscoveryService implements logger.debug("[{}] Error on port during discovery: {}", currentScannedPortName, portEvent); stopSerialPortScan(); } - - @Reference - protected void setSerialPortManager(final SerialPortManager serialPortManager) { - this.serialPortManager = serialPortManager; - } - - protected void unsetSerialPortManager(final SerialPortManager serialPortManager) { - this.serialPortManager = null; - } - - @Reference - protected void setLocaleProvider(final LocaleProvider localeProvider) { - this.localeProvider = localeProvider; - } - - protected void unsetLocaleProvider(final LocaleProvider localeProvider) { - this.localeProvider = null; - } - - @Reference - protected void setTranslationProvider(TranslationProvider i18nProvider) { - this.i18nProvider = i18nProvider; - } - - protected void unsetTranslationProvider(TranslationProvider i18nProvider) { - this.i18nProvider = null; - } } diff --git a/bundles/org.openhab.binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/discovery/DSMRI18nProviderTracker.java b/bundles/org.openhab.binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/discovery/DSMRI18nProviderTracker.java new file mode 100644 index 00000000000..b47f0c8616a --- /dev/null +++ b/bundles/org.openhab.binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/discovery/DSMRI18nProviderTracker.java @@ -0,0 +1,50 @@ +/** + * Copyright (c) 2010-2021 Contributors to the openHAB project + * + * See the NOTICE file(s) distributed with this work for additional + * information. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0 + * + * SPDX-License-Identifier: EPL-2.0 + */ +package org.openhab.binding.dsmr.internal.discovery; + +import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; +import org.openhab.core.i18n.LocaleProvider; +import org.openhab.core.i18n.TranslationProvider; +import org.openhab.core.thing.binding.ThingHandlerService; +import org.osgi.service.component.annotations.Activate; +import org.osgi.service.component.annotations.Component; +import org.osgi.service.component.annotations.Deactivate; +import org.osgi.service.component.annotations.Reference; + +/** + * Tracks the i18n provider dependencies of the {@link DSMRMeterDiscoveryService}. The {@link DSMRMeterDiscoveryService} + * is a {@link ThingHandlerService} so its i18n dependencies cannot be injected using service component annotations. + * + * @author Wouter Born - Initial contribution + */ +@Component +@NonNullByDefault +public class DSMRI18nProviderTracker { + + static @Nullable TranslationProvider i18nProvider; + static @Nullable LocaleProvider localeProvider; + + @Activate + public DSMRI18nProviderTracker(final @Reference TranslationProvider i18nProvider, + final @Reference LocaleProvider localeProvider) { + DSMRI18nProviderTracker.i18nProvider = i18nProvider; + DSMRI18nProviderTracker.localeProvider = localeProvider; + } + + @Deactivate + public void deactivate() { + i18nProvider = null; + localeProvider = null; + } +} diff --git a/bundles/org.openhab.binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/discovery/DSMRMeterDiscoveryService.java b/bundles/org.openhab.binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/discovery/DSMRMeterDiscoveryService.java index 8c3f6716ca5..a742eb1208f 100644 --- a/bundles/org.openhab.binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/discovery/DSMRMeterDiscoveryService.java +++ b/bundles/org.openhab.binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/discovery/DSMRMeterDiscoveryService.java @@ -21,6 +21,7 @@ import java.util.Set; import java.util.stream.Collectors; import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; import org.openhab.binding.dsmr.internal.device.cosem.CosemObject; import org.openhab.binding.dsmr.internal.device.cosem.CosemObjectType; import org.openhab.binding.dsmr.internal.device.p1telegram.P1Telegram; @@ -29,10 +30,10 @@ import org.openhab.binding.dsmr.internal.handler.DSMRBridgeHandler; import org.openhab.binding.dsmr.internal.handler.DSMRMeterHandler; import org.openhab.binding.dsmr.internal.meter.DSMRMeterDescriptor; import org.openhab.binding.dsmr.internal.meter.DSMRMeterType; -import org.openhab.core.i18n.LocaleProvider; -import org.openhab.core.i18n.TranslationProvider; import org.openhab.core.library.types.StringType; import org.openhab.core.thing.Thing; +import org.openhab.core.thing.binding.ThingHandler; +import org.openhab.core.thing.binding.ThingHandlerService; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -43,22 +44,41 @@ import org.slf4j.LoggerFactory; * @author Hilbrand Bouwkamp - Refactored code to detect meters during actual discovery phase. */ @NonNullByDefault -public class DSMRMeterDiscoveryService extends DSMRDiscoveryService implements P1TelegramListener { +public class DSMRMeterDiscoveryService extends DSMRDiscoveryService implements P1TelegramListener, ThingHandlerService { private final Logger logger = LoggerFactory.getLogger(DSMRMeterDiscoveryService.class); /** * The {@link DSMRBridgeHandler} instance */ - private final DSMRBridgeHandler dsmrBridgeHandler; + private @NonNullByDefault({}) DSMRBridgeHandler dsmrBridgeHandler; /** * Constructs a new {@link DSMRMeterDiscoveryService} attached to the give bridge handler. * * @param dsmrBridgeHandler The bridge handler this discovery service is attached to */ - public DSMRMeterDiscoveryService(DSMRBridgeHandler dsmrBridgeHandler) { - this.dsmrBridgeHandler = dsmrBridgeHandler; + public DSMRMeterDiscoveryService() { + super(); + this.i18nProvider = DSMRI18nProviderTracker.i18nProvider; + this.localeProvider = DSMRI18nProviderTracker.localeProvider; + } + + @Override + public void deactivate() { + super.deactivate(); + } + + @Override + public @Nullable ThingHandler getThingHandler() { + return dsmrBridgeHandler; + } + + @Override + public void setThingHandler(ThingHandler handler) { + if (handler instanceof DSMRBridgeHandler) { + dsmrBridgeHandler = (DSMRBridgeHandler) handler; + } } @Override @@ -179,20 +199,4 @@ public class DSMRMeterDiscoveryService extends DSMRDiscoveryService implements P invalidConfigured.stream().map(m -> m.name()).collect(Collectors.joining(", ")), unconfiguredMeters.stream().map(m -> m.name()).collect(Collectors.joining(", "))); } - - public void setLocaleProvider(final LocaleProvider localeProvider) { - this.localeProvider = localeProvider; - } - - public void unsetLocaleProvider() { - this.localeProvider = null; - } - - public void setTranslationProvider(TranslationProvider i18nProvider) { - this.i18nProvider = i18nProvider; - } - - public void unsetTranslationProvider() { - this.i18nProvider = null; - } } diff --git a/bundles/org.openhab.binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/handler/DSMRBridgeHandler.java b/bundles/org.openhab.binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/handler/DSMRBridgeHandler.java index cd969781045..fd1e757e157 100644 --- a/bundles/org.openhab.binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/handler/DSMRBridgeHandler.java +++ b/bundles/org.openhab.binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/handler/DSMRBridgeHandler.java @@ -15,6 +15,7 @@ package org.openhab.binding.dsmr.internal.handler; import static org.openhab.binding.dsmr.internal.DSMRBindingConstants.THING_TYPE_SMARTY_BRIDGE; import java.util.ArrayList; +import java.util.Collection; import java.util.List; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; @@ -31,12 +32,14 @@ import org.openhab.binding.dsmr.internal.device.connector.DSMRConnectorErrorEven import org.openhab.binding.dsmr.internal.device.connector.DSMRSerialSettings; import org.openhab.binding.dsmr.internal.device.p1telegram.P1Telegram; import org.openhab.binding.dsmr.internal.device.p1telegram.P1TelegramListener; +import org.openhab.binding.dsmr.internal.discovery.DSMRMeterDiscoveryService; import org.openhab.core.io.transport.serial.SerialPortManager; import org.openhab.core.thing.Bridge; import org.openhab.core.thing.ChannelUID; 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; @@ -114,6 +117,11 @@ public class DSMRBridgeHandler extends BaseBridgeHandler implements DSMREventLis smartyMeter = THING_TYPE_SMARTY_BRIDGE.equals(bridge.getThingTypeUID()); } + @Override + public Collection> getServices() { + return List.of(DSMRMeterDiscoveryService.class); + } + /** * The {@link DSMRBridgeHandler} does not support handling commands. * diff --git a/bundles/org.openhab.binding.dsmr/src/test/java/org/openhab/binding/dsmr/internal/discovery/DSMRMeterDiscoveryServiceTest.java b/bundles/org.openhab.binding.dsmr/src/test/java/org/openhab/binding/dsmr/internal/discovery/DSMRMeterDiscoveryServiceTest.java index 25344d1bbb3..0f36c6dafbb 100644 --- a/bundles/org.openhab.binding.dsmr/src/test/java/org/openhab/binding/dsmr/internal/discovery/DSMRMeterDiscoveryServiceTest.java +++ b/bundles/org.openhab.binding.dsmr/src/test/java/org/openhab/binding/dsmr/internal/discovery/DSMRMeterDiscoveryServiceTest.java @@ -66,7 +66,7 @@ public class DSMRMeterDiscoveryServiceTest { P1Telegram expected = TelegramReaderUtil.readTelegram(EXPECTED_CONFIGURED_TELEGRAM, TelegramState.OK); AtomicReference> invalidConfiguredRef = new AtomicReference<>(); AtomicReference> unconfiguredRef = new AtomicReference<>(); - DSMRMeterDiscoveryService service = new DSMRMeterDiscoveryService(bridge) { + DSMRMeterDiscoveryService service = new DSMRMeterDiscoveryService() { @Override protected void reportConfigurationValidationResults(List invalidConfigured, List unconfiguredMeters) { @@ -75,6 +75,7 @@ public class DSMRMeterDiscoveryServiceTest { unconfiguredRef.set(unconfiguredMeters); } }; + service.setThingHandler(bridge); // Mock the invalid configuration by reading a telegram that is valid for a meter that is a subset of the // expected meter. @@ -110,13 +111,14 @@ public class DSMRMeterDiscoveryServiceTest { public void testUnregisteredMeters() { P1Telegram telegram = TelegramReaderUtil.readTelegram(UNREGISTERED_METER_TELEGRAM, TelegramState.OK); AtomicBoolean unregisteredMeter = new AtomicBoolean(false); - DSMRMeterDiscoveryService service = new DSMRMeterDiscoveryService(bridge) { + DSMRMeterDiscoveryService service = new DSMRMeterDiscoveryService() { @Override protected void reportUnregisteredMeters() { super.reportUnregisteredMeters(); unregisteredMeter.set(true); } }; + service.setThingHandler(bridge); when(bridge.getThing().getUID()).thenReturn(new ThingUID("dsmr:dsmrBridge:22e5393c")); when(bridge.getThing().getThings()).thenReturn(Collections.emptyList());