Skip to content

Commit 5d99a7f

Browse files
wbornwborn
authored
[dsmr] Use ThingHandlerService for discovery (openhab#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 <[email protected]>
1 parent fdada9a commit 5d99a7f

File tree

6 files changed

+105
-126
lines changed

6 files changed

+105
-126
lines changed

‎bundles/org.openhab.binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/DSMRHandlerFactory.java

+7-74
Original file line numberDiff line numberDiff line change
@@ -14,28 +14,19 @@
1414

1515
import static org.openhab.binding.dsmr.internal.DSMRBindingConstants.*;
1616

17-
import java.util.HashMap;
18-
import java.util.Hashtable;
19-
import java.util.Map;
20-
2117
import org.eclipse.jdt.annotation.NonNullByDefault;
2218
import org.eclipse.jdt.annotation.Nullable;
23-
import org.openhab.binding.dsmr.internal.discovery.DSMRMeterDiscoveryService;
2419
import org.openhab.binding.dsmr.internal.handler.DSMRBridgeHandler;
2520
import org.openhab.binding.dsmr.internal.handler.DSMRMeterHandler;
2621
import org.openhab.binding.dsmr.internal.meter.DSMRMeterType;
27-
import org.openhab.core.config.discovery.DiscoveryService;
28-
import org.openhab.core.i18n.LocaleProvider;
29-
import org.openhab.core.i18n.TranslationProvider;
3022
import org.openhab.core.io.transport.serial.SerialPortManager;
3123
import org.openhab.core.thing.Bridge;
3224
import org.openhab.core.thing.Thing;
3325
import org.openhab.core.thing.ThingTypeUID;
34-
import org.openhab.core.thing.ThingUID;
3526
import org.openhab.core.thing.binding.BaseThingHandlerFactory;
3627
import org.openhab.core.thing.binding.ThingHandler;
3728
import org.openhab.core.thing.binding.ThingHandlerFactory;
38-
import org.osgi.framework.ServiceRegistration;
29+
import org.osgi.service.component.annotations.Activate;
3930
import org.osgi.service.component.annotations.Component;
4031
import org.osgi.service.component.annotations.Reference;
4132
import org.slf4j.Logger;
@@ -53,11 +44,12 @@ public class DSMRHandlerFactory extends BaseThingHandlerFactory {
5344

5445
private final Logger logger = LoggerFactory.getLogger(DSMRHandlerFactory.class);
5546

56-
private final Map<ThingUID, ServiceRegistration<?>> discoveryServiceRegs = new HashMap<>();
47+
private final SerialPortManager serialPortManager;
5748

58-
private @NonNullByDefault({}) SerialPortManager serialPortManager;
59-
private @NonNullByDefault({}) LocaleProvider localeProvider;
60-
private @NonNullByDefault({}) TranslationProvider i18nProvider;
49+
@Activate
50+
public DSMRHandlerFactory(@Reference SerialPortManager serialPortManager) {
51+
this.serialPortManager = serialPortManager;
52+
}
6153

6254
/**
6355
* Returns if the specified ThingTypeUID is supported by this handler.
@@ -100,70 +92,11 @@ public boolean supportsThingType(ThingTypeUID thingTypeUID) {
10092
logger.debug("Searching for thingTypeUID {}", thingTypeUID);
10193

10294
if (THING_TYPE_DSMR_BRIDGE.equals(thingTypeUID) || THING_TYPE_SMARTY_BRIDGE.equals(thingTypeUID)) {
103-
DSMRBridgeHandler handler = new DSMRBridgeHandler((Bridge) thing, serialPortManager);
104-
registerDiscoveryService(handler);
105-
return handler;
95+
return new DSMRBridgeHandler((Bridge) thing, serialPortManager);
10696
} else if (DSMRMeterType.METER_THING_TYPES.contains(thingTypeUID)) {
10797
return new DSMRMeterHandler(thing);
10898
}
10999

110100
return null;
111101
}
112-
113-
/**
114-
* Registers a meter discovery service for the bridge handler.
115-
*
116-
* @param bridgeHandler handler to register service for
117-
*/
118-
private synchronized void registerDiscoveryService(DSMRBridgeHandler bridgeHandler) {
119-
DSMRMeterDiscoveryService discoveryService = new DSMRMeterDiscoveryService(bridgeHandler);
120-
121-
discoveryService.setLocaleProvider(localeProvider);
122-
discoveryService.setTranslationProvider(i18nProvider);
123-
this.discoveryServiceRegs.put(bridgeHandler.getThing().getUID(),
124-
bundleContext.registerService(DiscoveryService.class.getName(), discoveryService, new Hashtable<>()));
125-
}
126-
127-
@Override
128-
protected synchronized void removeHandler(ThingHandler thingHandler) {
129-
if (thingHandler instanceof DSMRBridgeHandler) {
130-
ServiceRegistration<?> serviceReg = this.discoveryServiceRegs.remove(thingHandler.getThing().getUID());
131-
if (serviceReg != null) {
132-
DSMRMeterDiscoveryService service = (DSMRMeterDiscoveryService) getBundleContext()
133-
.getService(serviceReg.getReference());
134-
serviceReg.unregister();
135-
if (service != null) {
136-
service.unsetLocaleProvider();
137-
service.unsetTranslationProvider();
138-
}
139-
}
140-
}
141-
}
142-
143-
@Reference
144-
protected void setSerialPortManager(final SerialPortManager serialPortManager) {
145-
this.serialPortManager = serialPortManager;
146-
}
147-
148-
protected void unsetSerialPortManager(final SerialPortManager serialPortManager) {
149-
this.serialPortManager = null;
150-
}
151-
152-
@Reference
153-
protected void setLocaleProvider(final LocaleProvider localeProvider) {
154-
this.localeProvider = localeProvider;
155-
}
156-
157-
protected void unsetLocaleProvider(final LocaleProvider localeProvider) {
158-
this.localeProvider = null;
159-
}
160-
161-
@Reference
162-
protected void setTranslationProvider(TranslationProvider i18nProvider) {
163-
this.i18nProvider = i18nProvider;
164-
}
165-
166-
protected void unsetTranslationProvider(TranslationProvider i18nProvider) {
167-
this.i18nProvider = null;
168-
}
169102
}

‎bundles/org.openhab.binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/discovery/DSMRBridgeDiscoveryService.java

+10-28
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import org.openhab.core.io.transport.serial.SerialPortManager;
3939
import org.openhab.core.thing.ThingTypeUID;
4040
import org.openhab.core.thing.ThingUID;
41+
import org.osgi.service.component.annotations.Activate;
4142
import org.osgi.service.component.annotations.Component;
4243
import org.osgi.service.component.annotations.Reference;
4344
import org.slf4j.Logger;
@@ -74,7 +75,7 @@ public class DSMRBridgeDiscoveryService extends DSMRDiscoveryService implements
7475
/**
7576
* Serial Port Manager.
7677
*/
77-
private @NonNullByDefault({}) SerialPortManager serialPortManager;
78+
private final SerialPortManager serialPortManager;
7879

7980
/**
8081
* DSMR Device that is scanned when discovery process in progress.
@@ -91,6 +92,14 @@ public class DSMRBridgeDiscoveryService extends DSMRDiscoveryService implements
9192
*/
9293
private boolean scanning;
9394

95+
@Activate
96+
public DSMRBridgeDiscoveryService(final @Reference TranslationProvider i18nProvider,
97+
final @Reference LocaleProvider localeProvider, final @Reference SerialPortManager serialPortManager) {
98+
this.i18nProvider = i18nProvider;
99+
this.localeProvider = localeProvider;
100+
this.serialPortManager = serialPortManager;
101+
}
102+
94103
/**
95104
* Starts a new discovery scan.
96105
*
@@ -203,31 +212,4 @@ public void handleErrorEvent(DSMRConnectorErrorEvent portEvent) {
203212
logger.debug("[{}] Error on port during discovery: {}", currentScannedPortName, portEvent);
204213
stopSerialPortScan();
205214
}
206-
207-
@Reference
208-
protected void setSerialPortManager(final SerialPortManager serialPortManager) {
209-
this.serialPortManager = serialPortManager;
210-
}
211-
212-
protected void unsetSerialPortManager(final SerialPortManager serialPortManager) {
213-
this.serialPortManager = null;
214-
}
215-
216-
@Reference
217-
protected void setLocaleProvider(final LocaleProvider localeProvider) {
218-
this.localeProvider = localeProvider;
219-
}
220-
221-
protected void unsetLocaleProvider(final LocaleProvider localeProvider) {
222-
this.localeProvider = null;
223-
}
224-
225-
@Reference
226-
protected void setTranslationProvider(TranslationProvider i18nProvider) {
227-
this.i18nProvider = i18nProvider;
228-
}
229-
230-
protected void unsetTranslationProvider(TranslationProvider i18nProvider) {
231-
this.i18nProvider = null;
232-
}
233215
}

‎bundles/org.openhab.binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/discovery/DSMRI18nProviderTracker.java

+50
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/**
2+
* Copyright (c) 2010-2021 Contributors to the openHAB project
3+
*
4+
* See the NOTICE file(s) distributed with this work for additional
5+
* information.
6+
*
7+
* This program and the accompanying materials are made available under the
8+
* terms of the Eclipse Public License 2.0 which is available at
9+
* http://www.eclipse.org/legal/epl-2.0
10+
*
11+
* SPDX-License-Identifier: EPL-2.0
12+
*/
13+
package org.openhab.binding.dsmr.internal.discovery;
14+
15+
import org.eclipse.jdt.annotation.NonNullByDefault;
16+
import org.eclipse.jdt.annotation.Nullable;
17+
import org.openhab.core.i18n.LocaleProvider;
18+
import org.openhab.core.i18n.TranslationProvider;
19+
import org.openhab.core.thing.binding.ThingHandlerService;
20+
import org.osgi.service.component.annotations.Activate;
21+
import org.osgi.service.component.annotations.Component;
22+
import org.osgi.service.component.annotations.Deactivate;
23+
import org.osgi.service.component.annotations.Reference;
24+
25+
/**
26+
* Tracks the i18n provider dependencies of the {@link DSMRMeterDiscoveryService}. The {@link DSMRMeterDiscoveryService}
27+
* is a {@link ThingHandlerService} so its i18n dependencies cannot be injected using service component annotations.
28+
*
29+
* @author Wouter Born - Initial contribution
30+
*/
31+
@Component
32+
@NonNullByDefault
33+
public class DSMRI18nProviderTracker {
34+
35+
static @Nullable TranslationProvider i18nProvider;
36+
static @Nullable LocaleProvider localeProvider;
37+
38+
@Activate
39+
public DSMRI18nProviderTracker(final @Reference TranslationProvider i18nProvider,
40+
final @Reference LocaleProvider localeProvider) {
41+
DSMRI18nProviderTracker.i18nProvider = i18nProvider;
42+
DSMRI18nProviderTracker.localeProvider = localeProvider;
43+
}
44+
45+
@Deactivate
46+
public void deactivate() {
47+
i18nProvider = null;
48+
localeProvider = null;
49+
}
50+
}

‎bundles/org.openhab.binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/discovery/DSMRMeterDiscoveryService.java

+26-22
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import java.util.stream.Collectors;
2222

2323
import org.eclipse.jdt.annotation.NonNullByDefault;
24+
import org.eclipse.jdt.annotation.Nullable;
2425
import org.openhab.binding.dsmr.internal.device.cosem.CosemObject;
2526
import org.openhab.binding.dsmr.internal.device.cosem.CosemObjectType;
2627
import org.openhab.binding.dsmr.internal.device.p1telegram.P1Telegram;
@@ -29,10 +30,10 @@
2930
import org.openhab.binding.dsmr.internal.handler.DSMRMeterHandler;
3031
import org.openhab.binding.dsmr.internal.meter.DSMRMeterDescriptor;
3132
import org.openhab.binding.dsmr.internal.meter.DSMRMeterType;
32-
import org.openhab.core.i18n.LocaleProvider;
33-
import org.openhab.core.i18n.TranslationProvider;
3433
import org.openhab.core.library.types.StringType;
3534
import org.openhab.core.thing.Thing;
35+
import org.openhab.core.thing.binding.ThingHandler;
36+
import org.openhab.core.thing.binding.ThingHandlerService;
3637
import org.slf4j.Logger;
3738
import org.slf4j.LoggerFactory;
3839

@@ -43,22 +44,41 @@
4344
* @author Hilbrand Bouwkamp - Refactored code to detect meters during actual discovery phase.
4445
*/
4546
@NonNullByDefault
46-
public class DSMRMeterDiscoveryService extends DSMRDiscoveryService implements P1TelegramListener {
47+
public class DSMRMeterDiscoveryService extends DSMRDiscoveryService implements P1TelegramListener, ThingHandlerService {
4748

4849
private final Logger logger = LoggerFactory.getLogger(DSMRMeterDiscoveryService.class);
4950

5051
/**
5152
* The {@link DSMRBridgeHandler} instance
5253
*/
53-
private final DSMRBridgeHandler dsmrBridgeHandler;
54+
private @NonNullByDefault({}) DSMRBridgeHandler dsmrBridgeHandler;
5455

5556
/**
5657
* Constructs a new {@link DSMRMeterDiscoveryService} attached to the give bridge handler.
5758
*
5859
* @param dsmrBridgeHandler The bridge handler this discovery service is attached to
5960
*/
60-
public DSMRMeterDiscoveryService(DSMRBridgeHandler dsmrBridgeHandler) {
61-
this.dsmrBridgeHandler = dsmrBridgeHandler;
61+
public DSMRMeterDiscoveryService() {
62+
super();
63+
this.i18nProvider = DSMRI18nProviderTracker.i18nProvider;
64+
this.localeProvider = DSMRI18nProviderTracker.localeProvider;
65+
}
66+
67+
@Override
68+
public void deactivate() {
69+
super.deactivate();
70+
}
71+
72+
@Override
73+
public @Nullable ThingHandler getThingHandler() {
74+
return dsmrBridgeHandler;
75+
}
76+
77+
@Override
78+
public void setThingHandler(ThingHandler handler) {
79+
if (handler instanceof DSMRBridgeHandler) {
80+
dsmrBridgeHandler = (DSMRBridgeHandler) handler;
81+
}
6282
}
6383

6484
@Override
@@ -179,20 +199,4 @@ protected void reportConfigurationValidationResults(List<DSMRMeterType> invalidC
179199
invalidConfigured.stream().map(m -> m.name()).collect(Collectors.joining(", ")),
180200
unconfiguredMeters.stream().map(m -> m.name()).collect(Collectors.joining(", ")));
181201
}
182-
183-
public void setLocaleProvider(final LocaleProvider localeProvider) {
184-
this.localeProvider = localeProvider;
185-
}
186-
187-
public void unsetLocaleProvider() {
188-
this.localeProvider = null;
189-
}
190-
191-
public void setTranslationProvider(TranslationProvider i18nProvider) {
192-
this.i18nProvider = i18nProvider;
193-
}
194-
195-
public void unsetTranslationProvider() {
196-
this.i18nProvider = null;
197-
}
198202
}

‎bundles/org.openhab.binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/handler/DSMRBridgeHandler.java

+8
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import static org.openhab.binding.dsmr.internal.DSMRBindingConstants.THING_TYPE_SMARTY_BRIDGE;
1616

1717
import java.util.ArrayList;
18+
import java.util.Collection;
1819
import java.util.List;
1920
import java.util.concurrent.ScheduledFuture;
2021
import java.util.concurrent.TimeUnit;
@@ -31,12 +32,14 @@
3132
import org.openhab.binding.dsmr.internal.device.connector.DSMRSerialSettings;
3233
import org.openhab.binding.dsmr.internal.device.p1telegram.P1Telegram;
3334
import org.openhab.binding.dsmr.internal.device.p1telegram.P1TelegramListener;
35+
import org.openhab.binding.dsmr.internal.discovery.DSMRMeterDiscoveryService;
3436
import org.openhab.core.io.transport.serial.SerialPortManager;
3537
import org.openhab.core.thing.Bridge;
3638
import org.openhab.core.thing.ChannelUID;
3739
import org.openhab.core.thing.ThingStatus;
3840
import org.openhab.core.thing.ThingStatusDetail;
3941
import org.openhab.core.thing.binding.BaseBridgeHandler;
42+
import org.openhab.core.thing.binding.ThingHandlerService;
4043
import org.openhab.core.types.Command;
4144
import org.slf4j.Logger;
4245
import org.slf4j.LoggerFactory;
@@ -114,6 +117,11 @@ public DSMRBridgeHandler(Bridge bridge, SerialPortManager serialPortManager) {
114117
smartyMeter = THING_TYPE_SMARTY_BRIDGE.equals(bridge.getThingTypeUID());
115118
}
116119

120+
@Override
121+
public Collection<Class<? extends ThingHandlerService>> getServices() {
122+
return List.of(DSMRMeterDiscoveryService.class);
123+
}
124+
117125
/**
118126
* The {@link DSMRBridgeHandler} does not support handling commands.
119127
*

‎bundles/org.openhab.binding.dsmr/src/test/java/org/openhab/binding/dsmr/internal/discovery/DSMRMeterDiscoveryServiceTest.java

+4-2
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ public void testInvalidConfiguredMeters() {
6666
P1Telegram expected = TelegramReaderUtil.readTelegram(EXPECTED_CONFIGURED_TELEGRAM, TelegramState.OK);
6767
AtomicReference<List<DSMRMeterType>> invalidConfiguredRef = new AtomicReference<>();
6868
AtomicReference<List<DSMRMeterType>> unconfiguredRef = new AtomicReference<>();
69-
DSMRMeterDiscoveryService service = new DSMRMeterDiscoveryService(bridge) {
69+
DSMRMeterDiscoveryService service = new DSMRMeterDiscoveryService() {
7070
@Override
7171
protected void reportConfigurationValidationResults(List<DSMRMeterType> invalidConfigured,
7272
List<DSMRMeterType> unconfiguredMeters) {
@@ -75,6 +75,7 @@ protected void reportConfigurationValidationResults(List<DSMRMeterType> invalidC
7575
unconfiguredRef.set(unconfiguredMeters);
7676
}
7777
};
78+
service.setThingHandler(bridge);
7879

7980
// Mock the invalid configuration by reading a telegram that is valid for a meter that is a subset of the
8081
// expected meter.
@@ -110,13 +111,14 @@ protected void reportConfigurationValidationResults(List<DSMRMeterType> invalidC
110111
public void testUnregisteredMeters() {
111112
P1Telegram telegram = TelegramReaderUtil.readTelegram(UNREGISTERED_METER_TELEGRAM, TelegramState.OK);
112113
AtomicBoolean unregisteredMeter = new AtomicBoolean(false);
113-
DSMRMeterDiscoveryService service = new DSMRMeterDiscoveryService(bridge) {
114+
DSMRMeterDiscoveryService service = new DSMRMeterDiscoveryService() {
114115
@Override
115116
protected void reportUnregisteredMeters() {
116117
super.reportUnregisteredMeters();
117118
unregisteredMeter.set(true);
118119
}
119120
};
121+
service.setThingHandler(bridge);
120122
when(bridge.getThing().getUID()).thenReturn(new ThingUID("dsmr:dsmrBridge:22e5393c"));
121123
when(bridge.getThing().getThings()).thenReturn(Collections.emptyList());
122124

0 commit comments

Comments
 (0)