From 843ec092dcb745d1c6aac1109dbb3ad1d93f93bb Mon Sep 17 00:00:00 2001 From: J-N-K Date: Fri, 11 Dec 2020 03:21:12 +0100 Subject: [PATCH] [deconz] improve initialization (#9321) Signed-off-by: Jan N. Klug --- .../openhab/binding/deconz/internal/Util.java | 3 - .../discovery/ThingDiscoveryService.java | 31 ++--- .../deconz/internal/dto/BridgeFullState.java | 15 +++ .../handler/DeconzBaseThingHandler.java | 70 ++++------ .../internal/handler/DeconzBridgeHandler.java | 126 ++++++++---------- .../internal/handler/GroupThingHandler.java | 20 +-- .../internal/handler/LightThingHandler.java | 57 ++++---- .../handler/SensorBaseThingHandler.java | 38 ++---- .../netutils/WebSocketConnection.java | 9 +- .../deconz/internal/types/ResourceType.java | 27 +++- .../openhab/binding/deconz/DeconzTest.java | 7 +- 11 files changed, 176 insertions(+), 227 deletions(-) diff --git a/bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/Util.java b/bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/Util.java index 35713268b..d276e0684 100644 --- a/bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/Util.java +++ b/bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/Util.java @@ -25,8 +25,6 @@ import java.util.stream.Stream; import org.eclipse.jdt.annotation.NonNullByDefault; import org.openhab.core.library.types.DateTimeType; import org.openhab.core.library.types.PercentType; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** * The {@link Util} class defines common utility methods @@ -35,7 +33,6 @@ import org.slf4j.LoggerFactory; */ @NonNullByDefault public class Util { - private static final Logger LOGGER = LoggerFactory.getLogger(Util.class); public static String buildUrl(String host, int port, String... urlParts) { StringBuilder url = new StringBuilder(); diff --git a/bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/discovery/ThingDiscoveryService.java b/bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/discovery/ThingDiscoveryService.java index e95fdd0e7..3e3bd8dba 100644 --- a/bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/discovery/ThingDiscoveryService.java +++ b/bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/discovery/ThingDiscoveryService.java @@ -25,7 +25,6 @@ import java.util.stream.Stream; import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; import org.openhab.binding.deconz.internal.Util; -import org.openhab.binding.deconz.internal.dto.BridgeFullState; import org.openhab.binding.deconz.internal.dto.GroupMessage; import org.openhab.binding.deconz.internal.dto.LightMessage; import org.openhab.binding.deconz.internal.dto.SensorMessage; @@ -70,10 +69,19 @@ public class ThingDiscoveryService extends AbstractDiscoveryService implements D } @Override - protected void startScan() { + public void startScan() { final DeconzBridgeHandler handler = this.handler; if (handler != null) { - handler.requestFullState(false); + handler.getBridgeFullState().thenAccept(fullState -> { + stopScan(); + removeOlderResults(getTimestampOfLastScan()); + fullState.ifPresent(state -> { + state.sensors.forEach(this::addSensor); + state.lights.forEach(this::addLight); + state.groups.forEach(this::addGroup); + }); + + }); } } @@ -282,7 +290,6 @@ public class ThingDiscoveryService extends AbstractDiscoveryService implements D public void setThingHandler(@Nullable ThingHandler handler) { if (handler instanceof DeconzBridgeHandler) { this.handler = (DeconzBridgeHandler) handler; - ((DeconzBridgeHandler) handler).setDiscoveryService(this); this.bridgeUID = handler.getThing().getUID(); } } @@ -301,20 +308,4 @@ public class ThingDiscoveryService extends AbstractDiscoveryService implements D public void deactivate() { super.deactivate(); } - - /** - * Call this method when a full bridge state request has been performed and either the fullState - * are known or a failure happened. - * - * @param fullState The fullState or null. - */ - public void stateRequestFinished(final @Nullable BridgeFullState fullState) { - stopScan(); - removeOlderResults(getTimestampOfLastScan()); - if (fullState != null) { - fullState.sensors.forEach(this::addSensor); - fullState.lights.forEach(this::addLight); - fullState.groups.forEach(this::addGroup); - } - } } diff --git a/bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/dto/BridgeFullState.java b/bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/dto/BridgeFullState.java index 2aa258502..897945442 100644 --- a/bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/dto/BridgeFullState.java +++ b/bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/dto/BridgeFullState.java @@ -16,6 +16,8 @@ import java.util.Collections; import java.util.Map; import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; +import org.openhab.binding.deconz.internal.types.ResourceType; /** * http://dresden-elektronik.github.io/deconz-rest-doc/configuration/ @@ -42,4 +44,17 @@ public class BridgeFullState { public Map sensors = Collections.emptyMap(); public Map lights = Collections.emptyMap(); public Map groups = Collections.emptyMap(); + + public @Nullable DeconzBaseMessage getMessage(ResourceType resourceType, String id) { + switch (resourceType) { + case LIGHTS: + return lights.get(id); + case SENSORS: + return sensors.get(id); + case GROUPS: + return groups.get(id); + default: + return null; + } + } } diff --git a/bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/handler/DeconzBaseThingHandler.java b/bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/handler/DeconzBaseThingHandler.java index 2a3ae3e34..3834ccbec 100644 --- a/bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/handler/DeconzBaseThingHandler.java +++ b/bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/handler/DeconzBaseThingHandler.java @@ -14,11 +14,8 @@ package org.openhab.binding.deconz.internal.handler; import static org.openhab.binding.deconz.internal.Util.buildUrl; -import java.net.SocketTimeoutException; -import java.util.concurrent.CompletionException; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; import java.util.function.Consumer; import org.eclipse.jdt.annotation.NonNullByDefault; @@ -46,8 +43,7 @@ import com.google.gson.Gson; * @author Jan N. Klug - Refactored to abstract class */ @NonNullByDefault -public abstract class DeconzBaseThingHandler extends BaseThingHandler - implements WebSocketMessageListener { +public abstract class DeconzBaseThingHandler extends BaseThingHandler implements WebSocketMessageListener { private final Logger logger = LoggerFactory.getLogger(DeconzBaseThingHandler.class); protected final ResourceType resourceType; protected ThingConfig config = new ThingConfig(); @@ -88,6 +84,15 @@ public abstract class DeconzBaseThingHandler extend } } + private @Nullable DeconzBridgeHandler getBridgeHandler() { + Bridge bridge = getBridge(); + if (bridge == null) { + updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.BRIDGE_OFFLINE); + return null; + } + return (DeconzBridgeHandler) bridge.getHandler(); + } + @Override public void bridgeStatusChanged(ThingStatusInfo bridgeStatusInfo) { if (config.id.isEmpty()) { @@ -98,12 +103,7 @@ public abstract class DeconzBaseThingHandler extend if (bridgeStatusInfo.getStatus() == ThingStatus.ONLINE) { // the bridge is ONLINE, we can communicate with the gateway, so we update the connection parameters and // register the listener - Bridge bridge = getBridge(); - if (bridge == null) { - updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.BRIDGE_OFFLINE); - return; - } - DeconzBridgeHandler bridgeHandler = (DeconzBridgeHandler) bridge.getHandler(); + DeconzBridgeHandler bridgeHandler = getBridgeHandler(); if (bridgeHandler == null) { updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.BRIDGE_OFFLINE); return; @@ -129,14 +129,6 @@ public abstract class DeconzBaseThingHandler extend } } - /** - * parse the initial state response message - * - * @param r AsyncHttpClient.Result with the state response result - * @return a message of the correct type - */ - protected abstract @Nullable T parseStateResponse(AsyncHttpClient.Result r); - /** * processes a newly received (initial) state response * @@ -144,36 +136,26 @@ public abstract class DeconzBaseThingHandler extend * * @param stateResponse */ - protected abstract void processStateResponse(@Nullable T stateResponse); + protected abstract void processStateResponse(DeconzBaseMessage stateResponse); /** * Perform a request to the REST API for retrieving the full light state with all data and configuration. */ - protected void requestState(Consumer<@Nullable T> processor) { - AsyncHttpClient asyncHttpClient = http; - if (asyncHttpClient == null) { - return; + protected void requestState(Consumer processor) { + DeconzBridgeHandler bridgeHandler = getBridgeHandler(); + if (bridgeHandler != null) { + bridgeHandler.getBridgeFullState() + .thenAccept(f -> f.map(s -> s.getMessage(resourceType, config.id)).ifPresentOrElse(message -> { + logger.trace("{} processing {}", thing.getUID(), message); + processor.accept(message); + }, () -> { + if (initializationJob != null) { + stopInitializationJob(); + initializationJob = scheduler.schedule(() -> requestState(this::processStateResponse), 10, + TimeUnit.SECONDS); + } + })); } - - String url = buildUrl(bridgeConfig.host, bridgeConfig.httpPort, bridgeConfig.apikey, - resourceType.getIdentifier(), config.id); - logger.trace("Requesting URL for initial data: {}", url); - - // Get initial data - asyncHttpClient.get(url, bridgeConfig.timeout).thenApply(this::parseStateResponse).exceptionally(e -> { - if (e instanceof SocketTimeoutException || e instanceof TimeoutException - || e instanceof CompletionException) { - logger.debug("Get new state failed: ", e); - } else { - updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, e.getMessage()); - } - - stopInitializationJob(); - initializationJob = scheduler.schedule(() -> requestState(this::processStateResponse), 10, - TimeUnit.SECONDS); - - return null; - }).thenAccept(processor); } /** diff --git a/bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/handler/DeconzBridgeHandler.java b/bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/handler/DeconzBridgeHandler.java index 60573e838..a489f6a0e 100644 --- a/bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/handler/DeconzBridgeHandler.java +++ b/bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/handler/DeconzBridgeHandler.java @@ -16,10 +16,7 @@ import static org.openhab.binding.deconz.internal.BindingConstants.*; import static org.openhab.binding.deconz.internal.Util.buildUrl; import java.net.SocketTimeoutException; -import java.util.Collection; -import java.util.Collections; -import java.util.Map; -import java.util.Set; +import java.util.*; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; import java.util.concurrent.ScheduledFuture; @@ -34,6 +31,7 @@ import org.openhab.binding.deconz.internal.dto.BridgeFullState; import org.openhab.binding.deconz.internal.netutils.AsyncHttpClient; import org.openhab.binding.deconz.internal.netutils.WebSocketConnection; import org.openhab.binding.deconz.internal.netutils.WebSocketConnectionListener; +import org.openhab.core.cache.ExpiringCacheAsync; import org.openhab.core.config.core.Configuration; import org.openhab.core.io.net.http.WebSocketFactory; import org.openhab.core.thing.Bridge; @@ -64,7 +62,6 @@ public class DeconzBridgeHandler extends BaseBridgeHandler implements WebSocketC public static final Set SUPPORTED_THING_TYPES = Collections.singleton(BRIDGE_TYPE); private final Logger logger = LoggerFactory.getLogger(DeconzBridgeHandler.class); - private @Nullable ThingDiscoveryService thingDiscoveryService; private final WebSocketConnection websocket; private final AsyncHttpClient http; private DeconzBridgeConfig config = new DeconzBridgeConfig(); @@ -75,6 +72,8 @@ public class DeconzBridgeHandler extends BaseBridgeHandler implements WebSocketC private boolean ignoreConfigurationUpdate; private boolean websocketReconnect = false; + private final ExpiringCacheAsync> fullStateCache = new ExpiringCacheAsync<>(1000); + /** The poll frequency for the API Key verification */ private static final int POLL_FREQUENCY_SEC = 10; @@ -130,7 +129,7 @@ public class DeconzBridgeHandler extends BaseBridgeHandler implements WebSocketC stopTimer(); scheduledFuture = scheduler.schedule(() -> requestApiKey(), POLL_FREQUENCY_SEC, TimeUnit.SECONDS); } else if (r.getResponseCode() == 200) { - ApiKeyMessage[] response = gson.fromJson(r.getBody(), ApiKeyMessage[].class); + ApiKeyMessage[] response = Objects.requireNonNull(gson.fromJson(r.getBody(), ApiKeyMessage[].class)); if (response.length == 0) { throw new IllegalStateException("Authorisation request response is empty"); } @@ -139,65 +138,65 @@ public class DeconzBridgeHandler extends BaseBridgeHandler implements WebSocketC configuration.put(CONFIG_APIKEY, config.apikey); updateConfiguration(configuration); updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_PENDING, "Waiting for configuration"); - requestFullState(true); + initializeBridgeState(); } else { throw new IllegalStateException("Unknown status code for authorisation request"); } } /** - * Parses the response message to the REST API for retrieving the full bridge state with all sensors and switches - * and configuration. + * get the full state of the bridge from the cache * - * @param r The response + * @return a CompletableFuture that returns an Optional of the bridge full state */ - private @Nullable BridgeFullState parseBridgeFullStateResponse(AsyncHttpClient.Result r) { - if (r.getResponseCode() == 403) { - return null; - } else if (r.getResponseCode() == 200) { - return gson.fromJson(r.getBody(), BridgeFullState.class); - } else { - throw new IllegalStateException("Unknown status code for full state request"); + public CompletableFuture> getBridgeFullState() { + return fullStateCache.getValue(this::refreshFullStateCache); + } + + /** + * refresh the full bridge state (used for initial processing and state-lookup) + * + * @return Completable future with an Optional of the BridgeFullState + */ + private CompletableFuture> refreshFullStateCache() { + logger.trace("{} starts refreshing the fullStateCache", thing.getUID()); + if (config.apikey == null) { + return CompletableFuture.completedFuture(Optional.empty()); } + String url = buildUrl(config.getHostWithoutPort(), config.httpPort, config.apikey); + return http.get(url, config.timeout).thenApply(r -> { + if (r.getResponseCode() == 403) { + return Optional.ofNullable((BridgeFullState) null); + } else if (r.getResponseCode() == 200) { + return Optional.ofNullable(gson.fromJson(r.getBody(), BridgeFullState.class)); + } else { + throw new IllegalStateException("Unknown status code for full state request"); + } + }).handle((v, t) -> { + if (t == null) { + return v; + } else if (t instanceof SocketTimeoutException || t instanceof TimeoutException + || t instanceof CompletionException) { + logger.debug("Get full state failed", t); + } else if (t != null) { + updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, t.getMessage()); + } + return Optional.empty(); + }); } /** * Perform a request to the REST API for retrieving the full bridge state with all sensors and switches * and configuration. */ - public void requestFullState(boolean isInitialRequest) { - if (config.apikey == null) { - return; - } - String url = buildUrl(config.getHostWithoutPort(), config.httpPort, config.apikey); - http.get(url, config.timeout).thenApply(this::parseBridgeFullStateResponse).exceptionally(e -> { - if (e instanceof SocketTimeoutException || e instanceof TimeoutException - || e instanceof CompletionException) { - logger.debug("Get full state failed", e); - } else { - updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, e.getMessage()); - } - return null; - }).whenComplete((value, error) -> { - final ThingDiscoveryService thingDiscoveryService = this.thingDiscoveryService; - if (thingDiscoveryService != null) { - // Hand over sensors to discovery service - thingDiscoveryService.stateRequestFinished(value); - } - }).thenAccept(fullState -> { - if (fullState == null) { - if (isInitialRequest) { - scheduledFuture = scheduler.schedule(() -> requestFullState(true), POLL_FREQUENCY_SEC, - TimeUnit.SECONDS); - } - return; - } - if (fullState.config.name.isEmpty()) { + public void initializeBridgeState() { + getBridgeFullState().thenAccept(fullState -> fullState.ifPresentOrElse(state -> { + if (state.config.name.isEmpty()) { updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.NONE, "You are connected to a HUE bridge, not a deCONZ software!"); return; } - if (fullState.config.websocketport == 0) { + if (state.config.websocketport == 0) { updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.NONE, "deCONZ software too old. No websocket support!"); return; @@ -205,35 +204,37 @@ public class DeconzBridgeHandler extends BaseBridgeHandler implements WebSocketC // Add some information about the bridge Map editProperties = editProperties(); - editProperties.put("apiversion", fullState.config.apiversion); - editProperties.put("swversion", fullState.config.swversion); - editProperties.put("fwversion", fullState.config.fwversion); - editProperties.put("uuid", fullState.config.uuid); - editProperties.put("zigbeechannel", String.valueOf(fullState.config.zigbeechannel)); - editProperties.put("ipaddress", fullState.config.ipaddress); + editProperties.put("apiversion", state.config.apiversion); + editProperties.put("swversion", state.config.swversion); + editProperties.put("fwversion", state.config.fwversion); + editProperties.put("uuid", state.config.uuid); + editProperties.put("zigbeechannel", String.valueOf(state.config.zigbeechannel)); + editProperties.put("ipaddress", state.config.ipaddress); ignoreConfigurationUpdate = true; updateProperties(editProperties); ignoreConfigurationUpdate = false; // Use requested websocket port if no specific port is given - websocketPort = config.port == 0 ? fullState.config.websocketport : config.port; + websocketPort = config.port == 0 ? state.config.websocketport : config.port; websocketReconnect = true; startWebsocket(); - }).exceptionally(e -> { + }, () -> { + // initial response was empty, re-trying in POLL_FREQUENCY_SEC seconds + scheduledFuture = scheduler.schedule(this::initializeBridgeState, POLL_FREQUENCY_SEC, TimeUnit.SECONDS); + })).exceptionally(e -> { if (e != null) { updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.NONE, e.getMessage()); } else { updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.NONE); } - logger.warn("Full state parsing failed", e); + logger.warn("Initial full state parsing failed", e); return null; }); } /** * Starts the websocket connection. - * - * {@link #requestFullState} need to be called first to obtain the websocket port. + * {@link #initializeBridgeState} need to be called first to obtain the websocket port. */ private void startWebsocket() { if (websocket.isConnected() || websocketPort == 0 || websocketReconnect == false) { @@ -269,7 +270,7 @@ public class DeconzBridgeHandler extends BaseBridgeHandler implements WebSocketC if (config.apikey == null) { requestApiKey(); } else { - requestFullState(true); + initializeBridgeState(); } } @@ -324,13 +325,4 @@ public class DeconzBridgeHandler extends BaseBridgeHandler implements WebSocketC public DeconzBridgeConfig getBridgeConfig() { return config; } - - /** - * Called by the {@link ThingDiscoveryService}. Informs the bridge handler about the service. - * - * @param thingDiscoveryService The service - */ - public void setDiscoveryService(ThingDiscoveryService thingDiscoveryService) { - this.thingDiscoveryService = thingDiscoveryService; - } } diff --git a/bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/handler/GroupThingHandler.java b/bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/handler/GroupThingHandler.java index dad89c31a..52930db7c 100644 --- a/bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/handler/GroupThingHandler.java +++ b/bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/handler/GroupThingHandler.java @@ -17,13 +17,11 @@ import static org.openhab.binding.deconz.internal.BindingConstants.*; import java.util.Set; import org.eclipse.jdt.annotation.NonNullByDefault; -import org.eclipse.jdt.annotation.Nullable; import org.openhab.binding.deconz.internal.Util; import org.openhab.binding.deconz.internal.dto.DeconzBaseMessage; import org.openhab.binding.deconz.internal.dto.GroupAction; import org.openhab.binding.deconz.internal.dto.GroupMessage; import org.openhab.binding.deconz.internal.dto.GroupState; -import org.openhab.binding.deconz.internal.netutils.AsyncHttpClient; import org.openhab.binding.deconz.internal.types.ResourceType; import org.openhab.core.library.types.DecimalType; import org.openhab.core.library.types.HSBType; @@ -55,7 +53,7 @@ import com.google.gson.Gson; * @author Jan N. Klug - Initial contribution */ @NonNullByDefault -public class GroupThingHandler extends DeconzBaseThingHandler { +public class GroupThingHandler extends DeconzBaseThingHandler { public static final Set SUPPORTED_THING_TYPE_UIDS = Set.of(THING_TYPE_LIGHTGROUP); private final Logger logger = LoggerFactory.getLogger(GroupThingHandler.class); @@ -128,21 +126,7 @@ public class GroupThingHandler extends DeconzBaseThingHandler { } @Override - protected @Nullable GroupMessage parseStateResponse(AsyncHttpClient.Result r) { - if (r.getResponseCode() == 403) { - return null; - } else if (r.getResponseCode() == 200) { - return gson.fromJson(r.getBody(), GroupMessage.class); - } else { - throw new IllegalStateException("Unknown status code " + r.getResponseCode() + " for full state request"); - } - } - - @Override - protected void processStateResponse(@Nullable GroupMessage stateResponse) { - if (stateResponse == null) { - return; - } + protected void processStateResponse(DeconzBaseMessage stateResponse) { messageReceived(config.id, stateResponse); } diff --git a/bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/handler/LightThingHandler.java b/bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/handler/LightThingHandler.java index 19ad0f71b..6995955c5 100644 --- a/bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/handler/LightThingHandler.java +++ b/bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/handler/LightThingHandler.java @@ -27,7 +27,6 @@ import org.openhab.binding.deconz.internal.Util; import org.openhab.binding.deconz.internal.dto.DeconzBaseMessage; import org.openhab.binding.deconz.internal.dto.LightMessage; import org.openhab.binding.deconz.internal.dto.LightState; -import org.openhab.binding.deconz.internal.netutils.AsyncHttpClient; import org.openhab.binding.deconz.internal.types.ResourceType; import org.openhab.core.library.types.*; import org.openhab.core.thing.ChannelUID; @@ -58,7 +57,7 @@ import com.google.gson.Gson; * @author Jan N. Klug - Initial contribution */ @NonNullByDefault -public class LightThingHandler extends DeconzBaseThingHandler { +public class LightThingHandler extends DeconzBaseThingHandler { public static final Set SUPPORTED_THING_TYPE_UIDS = Set.of(THING_TYPE_COLOR_TEMPERATURE_LIGHT, THING_TYPE_DIMMABLE_LIGHT, THING_TYPE_COLOR_LIGHT, THING_TYPE_EXTENDED_COLOR_LIGHT, THING_TYPE_ONOFF_LIGHT, THING_TYPE_WINDOW_COVERING, THING_TYPE_WARNING_DEVICE, THING_TYPE_DOORLOCK); @@ -263,41 +262,31 @@ public class LightThingHandler extends DeconzBaseThingHandler { } @Override - protected @Nullable LightMessage parseStateResponse(AsyncHttpClient.Result r) { - if (r.getResponseCode() == 403) { - return null; - } else if (r.getResponseCode() == 200) { - LightMessage lightMessage = Objects.requireNonNull(gson.fromJson(r.getBody(), LightMessage.class)); - if (needsPropertyUpdate) { - // if we did not receive an ctmin/ctmax, then we probably don't need it - needsPropertyUpdate = false; - - Integer ctmax = lightMessage.ctmax; - Integer ctmin = lightMessage.ctmin; - if (ctmin != null && ctmax != null) { - Map properties = new HashMap<>(thing.getProperties()); - properties.put(PROPERTY_CT_MAX, - Integer.toString(Util.constrainToRange(ctmax, ZCL_CT_MIN, ZCL_CT_MAX))); - properties.put(PROPERTY_CT_MIN, - Integer.toString(Util.constrainToRange(ctmin, ZCL_CT_MIN, ZCL_CT_MAX))); - updateProperties(properties); - } - } - return lightMessage; - } else { - throw new IllegalStateException("Unknown status code " + r.getResponseCode() + " for full state request"); - } - } - - @Override - protected void processStateResponse(@Nullable LightMessage stateResponse) { - if (stateResponse == null) { + protected void processStateResponse(DeconzBaseMessage stateResponse) { + if (!(stateResponse instanceof LightMessage)) { return; } - if (stateResponse.state.effect != null) { - checkAndUpdateEffectChannels(stateResponse); + + LightMessage lightMessage = (LightMessage) stateResponse; + if (needsPropertyUpdate) { + // if we did not receive an ctmin/ctmax, then we probably don't need it + needsPropertyUpdate = false; + + Integer ctmax = lightMessage.ctmax; + Integer ctmin = lightMessage.ctmin; + if (ctmin != null && ctmax != null) { + Map properties = new HashMap<>(thing.getProperties()); + properties.put(PROPERTY_CT_MAX, Integer.toString(Util.constrainToRange(ctmax, ZCL_CT_MIN, ZCL_CT_MAX))); + properties.put(PROPERTY_CT_MIN, Integer.toString(Util.constrainToRange(ctmin, ZCL_CT_MIN, ZCL_CT_MAX))); + updateProperties(properties); + } } - messageReceived(config.id, stateResponse); + + if (lightMessage.state.effect != null) { + checkAndUpdateEffectChannels(lightMessage); + } + + messageReceived(config.id, lightMessage); } private enum EffectLightModel { diff --git a/bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/handler/SensorBaseThingHandler.java b/bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/handler/SensorBaseThingHandler.java index d21b60a9b..275eec94f 100644 --- a/bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/handler/SensorBaseThingHandler.java +++ b/bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/handler/SensorBaseThingHandler.java @@ -28,7 +28,6 @@ import org.openhab.binding.deconz.internal.dto.DeconzBaseMessage; import org.openhab.binding.deconz.internal.dto.SensorConfig; import org.openhab.binding.deconz.internal.dto.SensorMessage; import org.openhab.binding.deconz.internal.dto.SensorState; -import org.openhab.binding.deconz.internal.netutils.AsyncHttpClient; import org.openhab.binding.deconz.internal.types.ResourceType; import org.openhab.core.library.types.DecimalType; import org.openhab.core.library.types.OnOffType; @@ -63,7 +62,7 @@ import com.google.gson.Gson; * @author Lukas Agethen - Refactored to provide better extensibility */ @NonNullByDefault -public abstract class SensorBaseThingHandler extends DeconzBaseThingHandler { +public abstract class SensorBaseThingHandler extends DeconzBaseThingHandler { private final Logger logger = LoggerFactory.getLogger(SensorBaseThingHandler.class); /** * The sensor state. Contains all possible fields for all supported sensors and switches @@ -106,25 +105,15 @@ public abstract class SensorBaseThingHandler extends DeconzBaseThingHandler editProperties = editProperties(); - editProperties.put(Thing.PROPERTY_FIRMWARE_VERSION, stateResponse.swversion); - editProperties.put(Thing.PROPERTY_MODEL_ID, stateResponse.modelid); - editProperties.put(UNIQUE_ID, stateResponse.uniqueid); + editProperties.put(Thing.PROPERTY_FIRMWARE_VERSION, sensorMessage.swversion); + editProperties.put(Thing.PROPERTY_MODEL_ID, sensorMessage.modelid); + editProperties.put(UNIQUE_ID, sensorMessage.uniqueid); ignoreConfigurationUpdate = true; updateProperties(editProperties); @@ -166,7 +155,7 @@ public abstract class SensorBaseThingHandler extends DeconzBaseThingHandler 0) { createChannel(CHANNEL_LAST_SEEN, ChannelKind.STATE); updateState(CHANNEL_LAST_SEEN, Util.convertTimestampToDateTime(lastSeen)); @@ -179,10 +168,7 @@ public abstract class SensorBaseThingHandler extends DeconzBaseThingHandler> EXPECTED_MESSAGE_TYPES = Map.of( - ResourceType.GROUPS, GroupMessage.class, ResourceType.LIGHTS, LightMessage.class, ResourceType.SENSORS, - SensorMessage.class); - private final Logger logger = LoggerFactory.getLogger(WebSocketConnection.class); private final WebSocketClient client; @@ -130,7 +123,7 @@ public class WebSocketConnection { return; } - Class expectedMessageType = EXPECTED_MESSAGE_TYPES.get(changedMessage.r); + Class expectedMessageType = changedMessage.r.getExpectedMessageType(); if (expectedMessageType == null) { logger.warn("BUG! Could not get expected message type for resource type {}. Please report this incident.", changedMessage.r); diff --git a/bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/types/ResourceType.java b/bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/types/ResourceType.java index 02ddf78c4..fb5a9c177 100644 --- a/bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/types/ResourceType.java +++ b/bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/types/ResourceType.java @@ -17,6 +17,11 @@ import java.util.Map; import java.util.stream.Collectors; import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; +import org.openhab.binding.deconz.internal.dto.DeconzBaseMessage; +import org.openhab.binding.deconz.internal.dto.GroupMessage; +import org.openhab.binding.deconz.internal.dto.LightMessage; +import org.openhab.binding.deconz.internal.dto.SensorMessage; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -27,10 +32,10 @@ import org.slf4j.LoggerFactory; */ @NonNullByDefault public enum ResourceType { - GROUPS("groups", "action"), - LIGHTS("lights", "state"), - SENSORS("sensors", "config"), - UNKNOWN("", ""); + GROUPS("groups", "action", GroupMessage.class), + LIGHTS("lights", "state", LightMessage.class), + SENSORS("sensors", "config", SensorMessage.class), + UNKNOWN("", "", null); private static final Map MAPPING = Arrays.stream(ResourceType.values()) .collect(Collectors.toMap(v -> v.identifier, v -> v)); @@ -38,10 +43,13 @@ public enum ResourceType { private String identifier; private String commandUrl; + private @Nullable Class expectedMessageType; - ResourceType(String identifier, String commandUrl) { + ResourceType(String identifier, String commandUrl, + @Nullable Class expectedMessageType) { this.identifier = identifier; this.commandUrl = commandUrl; + this.expectedMessageType = expectedMessageType; } /** @@ -62,6 +70,15 @@ public enum ResourceType { return commandUrl; } + /** + * get the expected message type for this resource type + * + * @return + */ + public @Nullable Class getExpectedMessageType() { + return expectedMessageType; + } + /** * get the resource type from a string * diff --git a/bundles/org.openhab.binding.deconz/src/test/java/org/openhab/binding/deconz/DeconzTest.java b/bundles/org.openhab.binding.deconz/src/test/java/org/openhab/binding/deconz/DeconzTest.java index eb9aa34f7..2bffe3410 100644 --- a/bundles/org.openhab.binding.deconz/src/test/java/org/openhab/binding/deconz/DeconzTest.java +++ b/bundles/org.openhab.binding.deconz/src/test/java/org/openhab/binding/deconz/DeconzTest.java @@ -20,6 +20,8 @@ import java.io.IOException; import java.nio.charset.StandardCharsets; import java.time.ZoneId; import java.time.ZonedDateTime; +import java.util.Optional; +import java.util.concurrent.CompletableFuture; import org.eclipse.jdt.annotation.NonNullByDefault; import org.junit.jupiter.api.BeforeEach; @@ -78,11 +80,12 @@ public class DeconzTest { assertEquals(6, bridgeFullState.lights.size()); assertEquals(9, bridgeFullState.sensors.size()); + Mockito.doAnswer(answer -> CompletableFuture.completedFuture(Optional.of(bridgeFullState))).when(bridgeHandler) + .getBridgeFullState(); ThingDiscoveryService discoveryService = new ThingDiscoveryService(); discoveryService.setThingHandler(bridgeHandler); discoveryService.addDiscoveryListener(discoveryListener); - - discoveryService.stateRequestFinished(bridgeFullState); + discoveryService.startScan(); Mockito.verify(discoveryListener, times(20)).thingDiscovered(any(), any()); }