[hue] Fix reconnection, parallel commands, trigger channels, and light level formula (#15169)

* [hue] post merge tweaks

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>

* [hue] abandon internal restart

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>

* [hue] remove externalRestartScheduled flag

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>

* [hue] serialize PUT calls

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>

* [hue] GET requests shall not activate trigger channels

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>

* [hue] fix LightLevel formula

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>

* [hue] fix Button DTO null error

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>

---------

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
(cherry picked from commit ae36108cbd)
This commit is contained in:
Andrew Fiddian-Green 2023-07-27 13:55:17 +01:00 committed by Fabian Wolter
parent 974847fc0a
commit 91c008a06c
7 changed files with 50 additions and 124 deletions

View File

@ -69,8 +69,8 @@ Device things support some of the following channels:
| dynamics | Number:Time | Sets the duration of dynamic transitions between light states. (Advanced) |
| alert | String | Allows setting an alert on a light e.g. flashing them. (Advanced) |
| effect | String | Allows setting an effect on a light e.g. 'candle' effect. (Advanced) |
| button-last-event | Number | Informs which button was last pressed in the device. (Trigger Channel) |
| rotary-steps | Number | Informs about the number of rotary steps of the last rotary dial movement. (Trigger Channel) |
| button-last-event | (String) | Informs which button was last pressed in the device. (Trigger Channel) |
| rotary-steps | (String) | Informs about the number of rotary steps of the last rotary dial movement. (Trigger Channel) |
| motion | Switch | Shows if motion has been detected by the sensor. (Read Only) |
| motion-enabled | Switch | Supports enabling / disabling the motion sensor. (Advanced) |
| light-level | Number:Illuminance | Shows the current light level measured by the sensor. (Read Only) |

View File

@ -416,7 +416,6 @@ public class Clip2Bridge implements Closeable {
private static final int CHECK_ALIVE_SECONDS = 300;
private static final int REQUEST_INTERVAL_MILLISECS = 50;
private static final int MAX_CONCURRENT_STREAMS = 3;
private static final int RESTART_AFTER_SECONDS = 5;
private static final ResourceReference BRIDGE = new ResourceReference().setType(ResourceType.BRIDGE);
@ -463,15 +462,12 @@ public class Clip2Bridge implements Closeable {
private final Semaphore streamMutex = new Semaphore(MAX_CONCURRENT_STREAMS, true);
private boolean closing;
private boolean internalRestartScheduled;
private boolean externalRestartScheduled;
private State onlineState = State.CLOSED;
private Optional<Instant> lastRequestTime = Optional.empty();
private Instant sessionExpireTime = Instant.MAX;
private @Nullable Session http2Session;
private @Nullable Future<?> checkAliveTask;
private @Nullable Future<?> internalRestartTask;
private Map<Integer, Future<?>> fatalErrorTasks = new ConcurrentHashMap<>();
/**
@ -481,14 +477,20 @@ public class Clip2Bridge implements Closeable {
* @param bridgeHandler the bridge handler.
* @param hostName the host name (ip address) of the Hue bridge
* @param applicationKey the application key.
* @throws ApiException if unable to open Jetty HTTP/2 client.
*/
public Clip2Bridge(HttpClientFactory httpClientFactory, Clip2BridgeHandler bridgeHandler, String hostName,
String applicationKey) {
String applicationKey) throws ApiException {
LOGGER.debug("Clip2Bridge()");
httpClient = httpClientFactory.getCommonHttpClient();
http2Client = httpClientFactory.createHttp2Client("hue-clip2", httpClient.getSslContextFactory());
http2Client.setConnectTimeout(Clip2Bridge.TIMEOUT_SECONDS * 1000);
http2Client.setIdleTimeout(-1);
try {
http2Client.start();
} catch (Exception e) {
throw new ApiException("Error starting HTTP/2 client", e);
}
this.bridgeHandler = bridgeHandler;
this.hostName = hostName;
this.applicationKey = applicationKey;
@ -541,9 +543,11 @@ public class Clip2Bridge implements Closeable {
@Override
public void close() {
closing = true;
externalRestartScheduled = false;
internalRestartScheduled = false;
close2();
try {
http2Client.stop();
} catch (Exception e) {
}
}
/**
@ -552,26 +556,15 @@ public class Clip2Bridge implements Closeable {
private void close2() {
synchronized (this) {
LOGGER.debug("close2()");
boolean notifyHandler = onlineState == State.ACTIVE && !internalRestartScheduled
&& !externalRestartScheduled && !closing;
boolean notifyHandler = onlineState == State.ACTIVE && !closing;
onlineState = State.CLOSED;
synchronized (fatalErrorTasks) {
fatalErrorTasks.values().forEach(task -> cancelTask(task, true));
fatalErrorTasks.clear();
}
if (!internalRestartScheduled) {
// don't close the task if a restart is current
cancelTask(internalRestartTask, true);
internalRestartTask = null;
}
cancelTask(checkAliveTask, true);
checkAliveTask = null;
closeSession();
try {
http2Client.stop();
} catch (Exception e) {
// ignore
}
if (notifyHandler) {
bridgeHandler.onConnectionOffline();
}
@ -584,7 +577,7 @@ public class Clip2Bridge implements Closeable {
private void closeSession() {
LOGGER.debug("closeSession()");
Session session = http2Session;
if (Objects.nonNull(session)) {
if (Objects.nonNull(session) && !session.isClosed()) {
session.close(0, null, Callback.NOOP);
}
http2Session = null;
@ -599,24 +592,13 @@ public class Clip2Bridge implements Closeable {
* @param cause the exception that caused the error.
*/
private synchronized void fatalError(Object listener, Http2Exception cause) {
if (externalRestartScheduled || internalRestartScheduled || onlineState == State.CLOSED || closing) {
if (onlineState == State.CLOSED || closing) {
return;
}
String causeId = listener.getClass().getSimpleName();
if (listener instanceof ContentStreamListenerAdapter) {
// on GET / PUT requests the caller handles errors and closes the stream; the session is still OK
LOGGER.debug("fatalError() {} {} ignoring", causeId, cause.error);
} else if (cause.error == Http2Error.GO_AWAY) {
LOGGER.debug("fatalError() {} {} scheduling reconnect", causeId, cause.error);
// schedule task to open again
internalRestartScheduled = true;
cancelTask(internalRestartTask, false);
internalRestartTask = bridgeHandler.getScheduler().schedule(
() -> internalRestart(onlineState == State.ACTIVE), RESTART_AFTER_SECONDS, TimeUnit.SECONDS);
// force close immediately to be clean when internalRestart() starts
close2();
} else {
if (LOGGER.isDebugEnabled()) {
LOGGER.debug("fatalError() {} {} closing", causeId, cause.error, cause);
@ -659,7 +641,6 @@ public class Clip2Bridge implements Closeable {
* @throws InterruptedException
*/
public Resources getResources(ResourceReference reference) throws ApiException, InterruptedException {
sleepDuringRestart();
if (onlineState == State.CLOSED) {
throw new ApiException("getResources() offline");
}
@ -735,30 +716,6 @@ public class Clip2Bridge implements Closeable {
return Objects.isNull(id) || id.isEmpty() ? url : url + "/" + id;
}
/**
* Restart the session.
*
* @param active boolean that selects whether to restart in active or passive mode.
*/
private void internalRestart(boolean active) {
try {
openPassive();
if (active) {
openActive();
}
internalRestartScheduled = false;
} catch (ApiException e) {
if (LOGGER.isDebugEnabled()) {
LOGGER.debug("internalRestart() failed", e);
} else {
LOGGER.warn("Scheduled reconnection task failed.");
}
internalRestartScheduled = false;
close2();
} catch (InterruptedException e) {
}
}
/**
* The event stream calls this method when it has received text data. It parses the text as JSON into a list of
* Event entries, converts the list of events to a list of resources, and forwards that list to the bridge
@ -892,11 +849,6 @@ public class Clip2Bridge implements Closeable {
synchronized (this) {
LOGGER.debug("openPassive()");
onlineState = State.CLOSED;
try {
http2Client.start();
} catch (Exception e) {
throw new ApiException("Error starting HTTP/2 client", e);
}
openSession();
openCheckAliveTask();
onlineState = State.PASSIVE;
@ -965,14 +917,14 @@ public class Clip2Bridge implements Closeable {
}
/**
* Use an HTTP/2 PUT command to send a resource to the server.
* Use an HTTP/2 PUT command to send a resource to the server. Note: the Hue bridge server can sometimes get
* confused by parallel PUT commands, so use 'synchronized' to prevent that.
*
* @param resource the resource to put.
* @throws ApiException if something fails.
* @throws InterruptedException
*/
public void putResource(Resource resource) throws ApiException, InterruptedException {
sleepDuringRestart();
public synchronized void putResource(Resource resource) throws ApiException, InterruptedException {
if (onlineState == State.CLOSED) {
return;
}
@ -1067,32 +1019,6 @@ public class Clip2Bridge implements Closeable {
throw new HttpUnauthorizedException("Application key registration failed");
}
public void setExternalRestartScheduled() {
externalRestartScheduled = true;
internalRestartScheduled = false;
cancelTask(internalRestartTask, false);
internalRestartTask = null;
close2();
}
/**
* Sleep the caller during any period when the connection is restarting.
*
* @throws ApiException if anything failed.
* @throws InterruptedException
*/
private void sleepDuringRestart() throws ApiException, InterruptedException {
Future<?> restartTask = this.internalRestartTask;
if (Objects.nonNull(restartTask)) {
try {
restartTask.get(RESTART_AFTER_SECONDS * 2, TimeUnit.SECONDS);
} catch (ExecutionException | TimeoutException e) {
throw new ApiException("sleepDuringRestart() error", e);
}
}
internalRestartScheduled = false;
}
/**
* Test the Hue Bridge connection state by attempting to connect and trying to execute a basic command that requires
* authentication.

View File

@ -12,10 +12,11 @@
*/
package org.openhab.binding.hue.internal.dto.clip2;
import java.util.Objects;
import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.openhab.binding.hue.internal.dto.clip2.enums.ButtonEventType;
import org.openhab.core.library.types.StringType;
import org.openhab.core.types.State;
import com.google.gson.annotations.SerializedName;
@ -26,17 +27,17 @@ import com.google.gson.annotations.SerializedName;
*/
@NonNullByDefault
public class Button {
private @NonNullByDefault({}) @SerializedName("last_event") String lastEvent;
private @Nullable @SerializedName("last_event") String lastEvent;
/**
* @return the last button event as an enum.
* @throws IllegalArgumentException if lastEvent is bad.
* @throws IllegalArgumentException if lastEvent is null or bad.
*/
public ButtonEventType getLastEvent() throws IllegalArgumentException {
return ButtonEventType.valueOf(lastEvent.toUpperCase());
}
public State getLastEventState() {
return new StringType(getLastEvent().name());
String lastEvent = this.lastEvent;
if (Objects.nonNull(lastEvent)) {
return ButtonEventType.valueOf(lastEvent.toUpperCase());
}
throw new IllegalArgumentException("lastEvent field is null");
}
}

View File

@ -40,16 +40,15 @@ public class LightLevel {
}
/**
* Raw sensor light level is '10000 * log10(lux) + 1' so apply the inverse formula to convert to Lux.
* Raw sensor light level formula is '10000 * log10(lux + 1)' so apply the inverse formula to convert back to Lux.
* NOTE: the Philips/Signify API documentation quotes the formula as '10000 * log10(lux) + 1', however this code
* author thinks that that formula is wrong since zero Lux would cause a log10(0) negative infinity overflow!
*
* @return a QuantityType with light level in Lux, or UNDEF.
*/
public State getLightLevelState() {
if (lightLevelValid) {
double rawLightLevel = lightLevel;
if (rawLightLevel > 1f) {
return new QuantityType<>(Math.pow(10f, (rawLightLevel - 1f) / 10000f), Units.LUX);
}
return new QuantityType<>(Math.pow(10f, (double) lightLevel / 10000f) - 1f, Units.LUX);
}
return UnDefType.UNDEF;
}

View File

@ -196,11 +196,6 @@ public class Resource {
return UnDefType.NULL;
}
public State getButtonLastEventState() {
Button button = this.button;
return Objects.nonNull(button) ? button.getLastEventState() : UnDefType.NULL;
}
public List<ResourceReference> getChildren() {
List<ResourceReference> children = this.children;
return Objects.nonNull(children) ? children : List.of();

View File

@ -486,7 +486,15 @@ public class Clip2BridgeHandler extends BaseBridgeHandler {
String applicationKey = config.applicationKey;
applicationKey = Objects.nonNull(applicationKey) ? applicationKey : "";
clip2Bridge = new Clip2Bridge(httpClientFactory, this, ipAddress, applicationKey);
try {
clip2Bridge = new Clip2Bridge(httpClientFactory, this, ipAddress, applicationKey);
} catch (ApiException e) {
logger.trace("initializeAssets() communication error on '{}'", ipAddress, e);
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
"@text/offline.api2.comm-error.exception [\"" + e.getMessage() + "\"]");
return;
}
assetsLoaded = true;
}
@ -499,14 +507,9 @@ public class Clip2BridgeHandler extends BaseBridgeHandler {
*/
public void onConnectionOffline() {
if (assetsLoaded) {
try {
getClip2Bridge().setExternalRestartScheduled();
cancelTask(checkConnectionTask, false);
checkConnectionTask = scheduler.schedule(() -> checkConnection(), RECONNECT_DELAY_SECONDS,
TimeUnit.SECONDS);
} catch (AssetNotLoadedException e) {
// should never occur
}
cancelTask(checkConnectionTask, false);
checkConnectionTask = scheduler.schedule(() -> checkConnection(), RECONNECT_DELAY_SECONDS,
TimeUnit.SECONDS);
}
}

View File

@ -783,9 +783,10 @@ public class Clip2ThingHandler extends BaseThingHandler {
if (fullUpdate) {
addSupportedChannel(CHANNEL_2_BUTTON_LAST_EVENT);
controlIds.put(resource.getId(), resource.getControlId());
} else {
State buttonState = resource.getButtonEventState(controlIds);
updateState(CHANNEL_2_BUTTON_LAST_EVENT, buttonState, fullUpdate);
}
State buttonState = resource.getButtonEventState(controlIds);
updateState(CHANNEL_2_BUTTON_LAST_EVENT, buttonState, fullUpdate);
break;
case DEVICE_POWER:
@ -828,8 +829,9 @@ public class Clip2ThingHandler extends BaseThingHandler {
case RELATIVE_ROTARY:
if (fullUpdate) {
addSupportedChannel(CHANNEL_2_ROTARY_STEPS);
} else {
updateState(CHANNEL_2_ROTARY_STEPS, resource.getRotaryStepsState(), fullUpdate);
}
updateState(CHANNEL_2_ROTARY_STEPS, resource.getRotaryStepsState(), fullUpdate);
break;
case TEMPERATURE: