Skip to content

Commit

Permalink
[plugwise] Use ThingHandlerService for discovery (openhab#9041)
Browse files Browse the repository at this point in the history 
This simplifies the PlugwiseHandlerFactory code so it no longer needs to register and keep track of a discovery service for each bridge.

Also contains a few other improvements:

* fix lastRequest timestamp not updated when sending messages to discover node properties
* use java.time Duration and Instant
* use List.of, Set.of
* remove redundant null suppression because of EEAs

Signed-off-by: Wouter Born <[email protected]>
  • Loading branch information
@wborn
wborn authored Nov 16, 2020
1 parent 4333f78 commit 1a113f6
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 73 deletions.
10 changes: 2 additions & 8 deletions ...lugwise/src/main/java/org/openhab/binding/plugwise/internal/PlugwiseBindingConstants.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,7 @@
*/
package org.openhab.binding.plugwise.internal;

import static java.util.stream.Collectors.*;

import java.util.Collections;
import java.util.Set;
import java.util.stream.Stream;

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.openhab.core.thing.ThingTypeUID;
Expand Down Expand Up @@ -65,8 +61,6 @@ public class PlugwiseBindingConstants {
public static final ThingTypeUID THING_TYPE_STICK = new ThingTypeUID(BINDING_ID, "stick");
public static final ThingTypeUID THING_TYPE_SWITCH = new ThingTypeUID(BINDING_ID, "switch");

public static final Set<ThingTypeUID> SUPPORTED_THING_TYPES_UIDS = Stream
.of(THING_TYPE_CIRCLE, THING_TYPE_CIRCLE_PLUS, THING_TYPE_SCAN, THING_TYPE_SENSE, THING_TYPE_STEALTH,
THING_TYPE_STICK, THING_TYPE_SWITCH)
.collect(collectingAndThen(toSet(), Collections::unmodifiableSet));
public static final Set<ThingTypeUID> SUPPORTED_THING_TYPES_UIDS = Set.of(THING_TYPE_CIRCLE, THING_TYPE_CIRCLE_PLUS,
THING_TYPE_SCAN, THING_TYPE_SENSE, THING_TYPE_STEALTH, THING_TYPE_STICK, THING_TYPE_SWITCH);
}
32 changes: 1 addition & 31 deletions ....plugwise/src/main/java/org/openhab/binding/plugwise/internal/PlugwiseHandlerFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,27 +14,20 @@

import static org.openhab.binding.plugwise.internal.PlugwiseBindingConstants.*;

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.plugwise.internal.handler.PlugwiseRelayDeviceHandler;
import org.openhab.binding.plugwise.internal.handler.PlugwiseScanHandler;
import org.openhab.binding.plugwise.internal.handler.PlugwiseSenseHandler;
import org.openhab.binding.plugwise.internal.handler.PlugwiseStickHandler;
import org.openhab.binding.plugwise.internal.handler.PlugwiseSwitchHandler;
import org.openhab.core.config.discovery.DiscoveryService;
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;
Expand All @@ -48,8 +41,6 @@
@Component(service = ThingHandlerFactory.class, configurationPid = "binding.plugwise")
public class PlugwiseHandlerFactory extends BaseThingHandlerFactory {

private final Map<ThingUID, ServiceRegistration<?>> discoveryServiceRegistrations = new HashMap<>();

private final SerialPortManager serialPortManager;

@Activate
Expand All @@ -67,9 +58,7 @@ public boolean supportsThingType(ThingTypeUID thingTypeUID) {
ThingTypeUID thingTypeUID = thing.getThingTypeUID();

if (thingTypeUID.equals(THING_TYPE_STICK)) {
PlugwiseStickHandler handler = new PlugwiseStickHandler((Bridge) thing, serialPortManager);
registerDiscoveryService(handler);
return handler;
return new PlugwiseStickHandler((Bridge) thing, serialPortManager);
} else if (thingTypeUID.equals(THING_TYPE_CIRCLE) || thingTypeUID.equals(THING_TYPE_CIRCLE_PLUS)
|| thingTypeUID.equals(THING_TYPE_STEALTH)) {
return new PlugwiseRelayDeviceHandler(thing);
Expand All @@ -83,23 +72,4 @@ public boolean supportsThingType(ThingTypeUID thingTypeUID) {

return null;
}

private void registerDiscoveryService(PlugwiseStickHandler handler) {
PlugwiseThingDiscoveryService discoveryService = new PlugwiseThingDiscoveryService(handler);
discoveryService.activate();
this.discoveryServiceRegistrations.put(handler.getThing().getUID(),
bundleContext.registerService(DiscoveryService.class.getName(), discoveryService, new Hashtable<>()));
}

@Override
protected void removeHandler(ThingHandler thingHandler) {
ServiceRegistration<?> registration = this.discoveryServiceRegistrations.get(thingHandler.getThing().getUID());
if (registration != null) {
PlugwiseThingDiscoveryService discoveryService = (PlugwiseThingDiscoveryService) bundleContext
.getService(registration.getReference());
discoveryService.deactivate();
registration.unregister();
discoveryServiceRegistrations.remove(thingHandler.getThing().getUID());
}
}
}
61 changes: 41 additions & 20 deletions ...se/src/main/java/org/openhab/binding/plugwise/internal/PlugwiseThingDiscoveryService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
import static java.util.stream.Collectors.*;
import static org.openhab.binding.plugwise.internal.PlugwiseBindingConstants.*;

import java.time.Duration;
import java.time.Instant;
import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
Expand Down Expand Up @@ -44,6 +46,8 @@
import org.openhab.core.thing.ThingStatus;
import org.openhab.core.thing.ThingTypeUID;
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;

Expand All @@ -56,21 +60,21 @@
*/
@NonNullByDefault
public class PlugwiseThingDiscoveryService extends AbstractDiscoveryService
implements PlugwiseMessageListener, PlugwiseStickStatusListener {
implements PlugwiseMessageListener, PlugwiseStickStatusListener, ThingHandlerService {

private static class CurrentRoleCall {
private boolean isRoleCalling;
private int currentNodeID;
private int attempts;
private long lastRequestMillis;
private Instant lastRequest = Instant.MIN;
}

private static class DiscoveredNode {
private final MACAddress macAddress;
private final Map<String, String> properties = new HashMap<>();
private DeviceType deviceType = DeviceType.UNKNOWN;
private int attempts;
private long lastRequestMillis;
private Instant lastRequest = Instant.MIN;

public DiscoveredNode(MACAddress macAddress) {
this.macAddress = macAddress;
Expand All @@ -87,27 +91,25 @@ public boolean isDataComplete() {

private static final int MIN_NODE_ID = 0;
private static final int MAX_NODE_ID = 63;
private static final int DISCOVERY_INTERVAL = 180;

private static final int WATCH_INTERVAL = 1;

private static final int MESSAGE_TIMEOUT = 15;
private static final int MESSAGE_RETRY_ATTEMPTS = 5;

private static final Duration DISCOVERY_INTERVAL = Duration.ofMinutes(3);
private static final Duration MESSAGE_TIMEOUT = Duration.ofSeconds(15);
private static final Duration WATCH_INTERVAL = Duration.ofSeconds(1);

private final Logger logger = LoggerFactory.getLogger(PlugwiseThingDiscoveryService.class);

private final PlugwiseStickHandler stickHandler;
private @NonNullByDefault({}) PlugwiseStickHandler stickHandler;

private @Nullable ScheduledFuture<?> discoveryJob;
private @Nullable ScheduledFuture<?> watchJob;
private CurrentRoleCall currentRoleCall = new CurrentRoleCall();

private final Map<MACAddress, DiscoveredNode> discoveredNodes = new ConcurrentHashMap<>();

public PlugwiseThingDiscoveryService(PlugwiseStickHandler stickHandler) throws IllegalArgumentException {
public PlugwiseThingDiscoveryService() throws IllegalArgumentException {
super(DISCOVERED_THING_TYPES_UIDS, 1, true);
this.stickHandler = stickHandler;
this.stickHandler.addStickStatusListener(this);
}

@Override
Expand All @@ -118,6 +120,7 @@ public synchronized void abortScan() {
stopDiscoveryWatchJob();
}

@Override
public void activate() {
super.activate(new HashMap<>());
}
Expand All @@ -138,7 +141,7 @@ private void createDiscoveryResult(DiscoveredNode node) {
}

@Override
protected void deactivate() {
public void deactivate() {
super.deactivate();
stickHandler.removeMessageListener(this);
stickHandler.removeStickStatusListener(this);
Expand All @@ -147,8 +150,10 @@ protected void deactivate() {
private void discoverNewNodeDetails(MACAddress macAddress) {
if (!isAlreadyDiscovered(macAddress)) {
logger.debug("Discovered new node ({})", macAddress);
discoveredNodes.put(macAddress, new DiscoveredNode(macAddress));
DiscoveredNode node = new DiscoveredNode(macAddress);
discoveredNodes.put(macAddress, node);
updateInformation(macAddress);
node.lastRequest = Instant.now();
} else {
logger.debug("Already discovered node ({})", macAddress);
}
Expand Down Expand Up @@ -184,6 +189,11 @@ private ThingStatus getStickStatus() {
return stickHandler.getThing().getStatus();
}

@Override
public @Nullable ThingHandler getThingHandler() {
return stickHandler;
}

private void handleAnnounceAwakeRequest(AnnounceAwakeRequestMessage message) {
discoverNewNodeDetails(message.getMACAddress());
}
Expand Down Expand Up @@ -267,7 +277,7 @@ private void roleCall(int nodeID) {
currentRoleCall.attempts++;
}
currentRoleCall.currentNodeID = nodeID;
currentRoleCall.lastRequestMillis = System.currentTimeMillis();
currentRoleCall.lastRequest = Instant.now();
} else {
logger.warn("Invalid node ID for role call: {}", nodeID);
}
Expand All @@ -277,6 +287,14 @@ private void sendMessage(Message message) {
stickHandler.sendMessage(message, PlugwiseMessagePriority.UPDATE_AND_DISCOVERY);
}

@Override
public void setThingHandler(ThingHandler handler) {
if (handler instanceof PlugwiseStickHandler) {
stickHandler = (PlugwiseStickHandler) handler;
stickHandler.addStickStatusListener(this);
}
}

@Override
protected void startBackgroundDiscovery() {
logger.debug("Starting Plugwise device background discovery");
Expand All @@ -288,7 +306,8 @@ protected void startBackgroundDiscovery() {

ScheduledFuture<?> localDiscoveryJob = discoveryJob;
if (localDiscoveryJob == null || localDiscoveryJob.isCancelled()) {
discoveryJob = scheduler.scheduleWithFixedDelay(discoveryRunnable, 0, DISCOVERY_INTERVAL, TimeUnit.SECONDS);
discoveryJob = scheduler.scheduleWithFixedDelay(discoveryRunnable, 0, DISCOVERY_INTERVAL.toMillis(),
TimeUnit.MILLISECONDS);
}
}

Expand All @@ -297,7 +316,8 @@ private void startDiscoveryWatchJob() {

Runnable watchRunnable = () -> {
if (currentRoleCall.isRoleCalling) {
if ((System.currentTimeMillis() - currentRoleCall.lastRequestMillis) > (MESSAGE_TIMEOUT * 1000)
Duration durationSinceLastRequest = Duration.between(currentRoleCall.lastRequest, Instant.now());
if (durationSinceLastRequest.compareTo(MESSAGE_TIMEOUT) > 0
&& currentRoleCall.attempts < MESSAGE_RETRY_ATTEMPTS) {
logger.debug("Resending timed out role call message for node with ID {} on Circle+ ({})",
currentRoleCall.currentNodeID, getCirclePlusMAC());
Expand All @@ -313,10 +333,11 @@ private void startDiscoveryWatchJob() {
while (it.hasNext()) {
Entry<MACAddress, DiscoveredNode> entry = it.next();
DiscoveredNode node = entry.getValue();
if (System.currentTimeMillis() - node.lastRequestMillis > (MESSAGE_TIMEOUT * 1000)
&& node.attempts < MESSAGE_RETRY_ATTEMPTS) {
Duration durationSinceLastRequest = Duration.between(node.lastRequest, Instant.now());
if (durationSinceLastRequest.compareTo(MESSAGE_TIMEOUT) > 0 && node.attempts < MESSAGE_RETRY_ATTEMPTS) {
logger.debug("Resending timed out information request message to node ({})", node.macAddress);
updateInformation(node.macAddress);
node.lastRequest = Instant.now();
node.attempts++;
} else if (node.attempts >= MESSAGE_RETRY_ATTEMPTS) {
logger.debug("Giving up on information request for node ({})", node.macAddress);
Expand All @@ -332,8 +353,8 @@ private void startDiscoveryWatchJob() {

ScheduledFuture<?> localWatchJob = watchJob;
if (localWatchJob == null || localWatchJob.isCancelled()) {
watchJob = scheduler.scheduleWithFixedDelay(watchRunnable, WATCH_INTERVAL, WATCH_INTERVAL,
TimeUnit.SECONDS);
watchJob = scheduler.scheduleWithFixedDelay(watchRunnable, WATCH_INTERVAL.toMillis(),
WATCH_INTERVAL.toMillis(), TimeUnit.MILLISECONDS);
}
}

Expand Down
13 changes: 6 additions & 7 deletions ...b.binding.plugwise/src/main/java/org/openhab/binding/plugwise/internal/PlugwiseUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,17 +43,17 @@ private PlugwiseUtils() {
}

public static DeviceType getDeviceType(ThingTypeUID uid) {
if (uid.equals(THING_TYPE_CIRCLE)) {
if (THING_TYPE_CIRCLE.equals(uid)) {
return CIRCLE;
} else if (uid.equals(THING_TYPE_CIRCLE_PLUS)) {
} else if (THING_TYPE_CIRCLE_PLUS.equals(uid)) {
return CIRCLE_PLUS;
} else if (uid.equals(THING_TYPE_SCAN)) {
} else if (THING_TYPE_SCAN.equals(uid)) {
return SCAN;
} else if (uid.equals(THING_TYPE_SENSE)) {
} else if (THING_TYPE_SENSE.equals(uid)) {
return SENSE;
} else if (uid.equals(THING_TYPE_STEALTH)) {
} else if (THING_TYPE_STEALTH.equals(uid)) {
return STEALTH;
} else if (uid.equals(THING_TYPE_SWITCH)) {
} else if (THING_TYPE_SWITCH.equals(uid)) {
return SWITCH;
} else {
return UNKNOWN;
Expand Down Expand Up @@ -106,7 +106,6 @@ public static String upperUnderscoreToLowerCamel(String text) {
return upperCamel.substring(0, 1).toLowerCase() + upperCamel.substring(1);
}

@SuppressWarnings("null")
public static boolean updateProperties(Map<String, String> properties, InformationResponseMessage message) {
boolean update = false;

Expand Down
9 changes: 2 additions & 7 deletions ...c/main/java/org/openhab/binding/plugwise/internal/handler/PlugwiseRelayDeviceHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,13 @@
*/
package org.openhab.binding.plugwise.internal.handler;

import static java.util.stream.Collectors.*;
import static org.openhab.binding.plugwise.internal.PlugwiseBindingConstants.*;
import static org.openhab.core.thing.ThingStatus.*;

import java.time.Duration;
import java.time.LocalDateTime;
import java.util.Collections;
import java.util.List;
import java.util.concurrent.TimeUnit;
import java.util.stream.Stream;

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
Expand Down Expand Up @@ -222,10 +219,8 @@ public boolean shouldBeScheduled() {
}
};

private final List<PlugwiseDeviceTask> recurringTasks = Stream
.of(clockUpdateTask, currentPowerUpdateTask, energyUpdateTask, informationUpdateTask,
realTimeClockUpdateTask, setClockTask)
.collect(collectingAndThen(toList(), Collections::unmodifiableList));
private final List<PlugwiseDeviceTask> recurringTasks = List.of(clockUpdateTask, currentPowerUpdateTask,
energyUpdateTask, informationUpdateTask, realTimeClockUpdateTask, setClockTask);

private final Logger logger = LoggerFactory.getLogger(PlugwiseRelayDeviceHandler.class);
private final DeviceType deviceType;
Expand Down
8 changes: 8 additions & 0 deletions ...ise/src/main/java/org/openhab/binding/plugwise/internal/handler/PlugwiseStickHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import java.io.IOException;
import java.time.Duration;
import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.concurrent.CopyOnWriteArrayList;
Expand All @@ -28,6 +29,7 @@
import org.openhab.binding.plugwise.internal.PlugwiseDeviceTask;
import org.openhab.binding.plugwise.internal.PlugwiseInitializationException;
import org.openhab.binding.plugwise.internal.PlugwiseMessagePriority;
import org.openhab.binding.plugwise.internal.PlugwiseThingDiscoveryService;
import org.openhab.binding.plugwise.internal.PlugwiseUtils;
import org.openhab.binding.plugwise.internal.config.PlugwiseStickConfig;
import org.openhab.binding.plugwise.internal.listener.PlugwiseMessageListener;
Expand All @@ -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.ThingHandlerService;
import org.openhab.core.types.Command;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -122,6 +125,11 @@ public void dispose() {
return circlePlusMAC;
}

@Override
public Collection<Class<? extends ThingHandlerService>> getServices() {
return List.of(PlugwiseThingDiscoveryService.class);
}

public @Nullable MACAddress getStickMAC() {
return stickMAC;
}
Expand Down

0 comments on commit 1a113f6

Please sign in to comment.