[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 <github@maindrain.net>

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<ThingUID, ServiceRegistration<?>> 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;
-    }
 }

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;
-    }
 }

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;
+    }
+}

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;
-    }
 }

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<Class<? extends ThingHandlerService>> getServices() {
+        return List.of(DSMRMeterDiscoveryService.class);
+    }
+
     /**
      * The {@link DSMRBridgeHandler} does not support handling commands.
      *

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<List<DSMRMeterType>> invalidConfiguredRef = new AtomicReference<>();
         AtomicReference<List<DSMRMeterType>> unconfiguredRef = new AtomicReference<>();
-        DSMRMeterDiscoveryService service = new DSMRMeterDiscoveryService(bridge) {
+        DSMRMeterDiscoveryService service = new DSMRMeterDiscoveryService() {
             @Override
             protected void reportConfigurationValidationResults(List<DSMRMeterType> invalidConfigured,
                     List<DSMRMeterType> 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());