From 22eebc797a9810966a26ad0442de872e76bdef8d Mon Sep 17 00:00:00 2001 From: Christoph Weitkamp Date: Sun, 9 May 2021 20:10:45 +0200 Subject: [PATCH] [hue] Fixed ColorTemperature set to UNDEF (#10609) * Fixed ColorTemperature set to UNDEF Signed-off-by: Christoph Weitkamp * Fixed SAT findings Signed-off-by: Christoph Weitkamp * Fixed warning during unit tests Signed-off-by: Christoph Weitkamp * Changed color temperature handling in GroupHandler Signed-off-by: Christoph Weitkamp --- .../binding/hue/internal/ApiVersion.java | 5 +-- .../openhab/binding/hue/internal/Config.java | 4 +- .../openhab/binding/hue/internal/Group.java | 2 +- .../binding/hue/internal/HueBridge.java | 2 +- .../openhab/binding/hue/internal/State.java | 7 +-- .../internal/handler/HueBridgeHandler.java | 14 +++--- .../hue/internal/handler/HueGroupHandler.java | 38 +++++----------- .../hue/internal/handler/HueLightHandler.java | 45 ++++++------------- .../internal/handler/HueLightHandlerTest.java | 2 + 9 files changed, 41 insertions(+), 78 deletions(-) diff --git a/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/ApiVersion.java b/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/ApiVersion.java index 5075f9533..b338827b6 100644 --- a/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/ApiVersion.java +++ b/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/ApiVersion.java @@ -121,10 +121,7 @@ public class ApiVersion { if (micro != other.micro) { return false; } - if (minor != other.minor) { - return false; - } - return true; + return minor == other.minor; } @Override diff --git a/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/Config.java b/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/Config.java index ab587ad19..81c8c2885 100644 --- a/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/Config.java +++ b/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/Config.java @@ -132,7 +132,7 @@ public class Config { * @return ip address of proxy or null */ public String getProxyAddress() { - return proxyaddress.equals("none") ? null : proxyaddress; + return "none".equals(proxyaddress) ? null : proxyaddress; } /** @@ -141,7 +141,7 @@ public class Config { * @return port of proxy or null */ public Integer getProxyPort() { - return proxyaddress.equals("none") ? null : proxyport; + return "none".equals(proxyaddress) ? null : proxyport; } /** diff --git a/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/Group.java b/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/Group.java index 4808eb422..5235d383d 100644 --- a/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/Group.java +++ b/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/Group.java @@ -58,7 +58,7 @@ public class Group { * @return modifiability of group */ public boolean isModifiable() { - return !id.equals("0"); + return !"0".equals(id); } /** diff --git a/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/HueBridge.java b/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/HueBridge.java index 14b25f8cf..7b5b70c1c 100644 --- a/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/HueBridge.java +++ b/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/HueBridge.java @@ -840,7 +840,7 @@ public class HueBridge { @Override protected Result doNetwork(String address, String requestMethod, @Nullable String body) throws IOException { // GET requests cannot be scheduled, so will continue working normally for convenience - if (requestMethod.equals("GET")) { + if ("GET".equals(requestMethod)) { return super.doNetwork(address, requestMethod, body); } else { String extractedAddress = Util.quickMatch("^http://[^/]+(.+)$", address); diff --git a/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/State.java b/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/State.java index 7ce101bdc..1155691fc 100644 --- a/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/State.java +++ b/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/State.java @@ -51,7 +51,7 @@ public class State { HS, /** - * Color temperature in mirek + * Color temperature in mired */ CT } @@ -287,9 +287,6 @@ public class State { if (sat != other.sat) { return false; } - if (!Arrays.equals(xy, other.xy)) { - return false; - } - return true; + return Arrays.equals(xy, other.xy); } } diff --git a/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/handler/HueBridgeHandler.java b/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/handler/HueBridgeHandler.java index 5b3891340..3ee5fc01e 100644 --- a/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/handler/HueBridgeHandler.java +++ b/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/handler/HueBridgeHandler.java @@ -164,14 +164,12 @@ public class HueBridgeHandler extends ConfigStatusBridgeHandler implements HueCl } catch (IOException e) { return false; } catch (ApiException e) { - if (e.getMessage().contains("SocketTimeout") || e.getMessage().contains("ConnectException") - || e.getMessage().contains("SocketException") - || e.getMessage().contains("NoRouteToHostException")) { - return false; - } else { - // this seems to be only an authentication issue - return true; - } + String message = e.getMessage(); + return message != null && // + !message.contains("SocketTimeout") && // + !message.contains("ConnectException") && // + !message.contains("SocketException") && // + !message.contains("NoRouteToHostException"); } return true; } diff --git a/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/handler/HueGroupHandler.java b/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/handler/HueGroupHandler.java index 27cdb5f0a..efa228f2f 100644 --- a/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/handler/HueGroupHandler.java +++ b/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/handler/HueGroupHandler.java @@ -28,7 +28,6 @@ import org.eclipse.jdt.annotation.Nullable; import org.openhab.binding.hue.internal.FullGroup; import org.openhab.binding.hue.internal.Scene; import org.openhab.binding.hue.internal.State; -import org.openhab.binding.hue.internal.State.ColorMode; import org.openhab.binding.hue.internal.StateUpdate; import org.openhab.binding.hue.internal.dto.ColorTemperature; import org.openhab.core.library.types.DecimalType; @@ -48,7 +47,6 @@ import org.openhab.core.thing.binding.BaseThingHandler; import org.openhab.core.thing.binding.ThingHandler; import org.openhab.core.types.Command; import org.openhab.core.types.StateOption; -import org.openhab.core.types.UnDefType; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -325,13 +323,13 @@ public class HueGroupHandler extends BaseThingHandler implements GroupStatusList private @Nullable Integer getCurrentColorTemp(@Nullable State groupState) { Integer colorTemp = lastSentColorTemp; if (colorTemp == null && groupState != null) { - colorTemp = groupState.getColorTemperature(); + return groupState.getColorTemperature(); } return colorTemp; } private @Nullable StateUpdate convertBrightnessChangeToStateUpdate(IncreaseDecreaseType command, FullGroup group) { - Integer currentBrightness = getCurrentBrightness(group); + Integer currentBrightness = getCurrentBrightness(group.getState()); if (currentBrightness == null) { return null; } @@ -339,15 +337,11 @@ public class HueGroupHandler extends BaseThingHandler implements GroupStatusList return createBrightnessStateUpdate(currentBrightness, newBrightness); } - private @Nullable Integer getCurrentBrightness(FullGroup group) { - if (lastSentBrightness != null) { - return lastSentBrightness; + private @Nullable Integer getCurrentBrightness(@Nullable State groupState) { + if (lastSentBrightness == null && groupState != null) { + return groupState.isOn() ? groupState.getBrightness() : 0; } - State currentState = group.getState(); - if (currentState == null) { - return null; - } - return currentState.isOn() ? currentState.getBrightness() : 0; + return lastSentBrightness; } private StateUpdate createBrightnessStateUpdate(int currentBrightness, int newBrightness) { @@ -406,24 +400,16 @@ public class HueGroupHandler extends BaseThingHandler implements GroupStatusList } updateState(CHANNEL_COLOR, hsbType); - ColorMode colorMode = state.getColorMode(); - if (ColorMode.CT.equals(colorMode)) { - updateState(CHANNEL_COLORTEMPERATURE, - LightStateConverter.toColorTemperaturePercentType(state, colorTemperatureCapabilties)); - updateState(CHANNEL_COLORTEMPERATURE_ABS, LightStateConverter.toColorTemperature(state)); - } else { - updateState(CHANNEL_COLORTEMPERATURE, UnDefType.UNDEF); - updateState(CHANNEL_COLORTEMPERATURE_ABS, UnDefType.UNDEF); - } - - PercentType brightnessPercentType = LightStateConverter.toBrightnessPercentType(state); - if (!state.isOn()) { - brightnessPercentType = PercentType.ZERO; - } + PercentType brightnessPercentType = state.isOn() ? LightStateConverter.toBrightnessPercentType(state) + : PercentType.ZERO; updateState(CHANNEL_BRIGHTNESS, brightnessPercentType); updateState(CHANNEL_SWITCH, OnOffType.from(state.isOn())); + updateState(CHANNEL_COLORTEMPERATURE, + LightStateConverter.toColorTemperaturePercentType(state, colorTemperatureCapabilties)); + updateState(CHANNEL_COLORTEMPERATURE_ABS, LightStateConverter.toColorTemperature(state)); + return true; } diff --git a/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/handler/HueLightHandler.java b/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/handler/HueLightHandler.java index fcd6d37f4..fd727666b 100644 --- a/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/handler/HueLightHandler.java +++ b/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/handler/HueLightHandler.java @@ -28,7 +28,6 @@ import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; import org.openhab.binding.hue.internal.FullLight; import org.openhab.binding.hue.internal.State; -import org.openhab.binding.hue.internal.State.ColorMode; import org.openhab.binding.hue.internal.StateUpdate; import org.openhab.binding.hue.internal.action.LightActions; import org.openhab.binding.hue.internal.dto.Capabilities; @@ -52,7 +51,6 @@ import org.openhab.core.thing.binding.ThingHandlerService; import org.openhab.core.types.Command; import org.openhab.core.types.StateDescription; import org.openhab.core.types.StateDescriptionFragmentBuilder; -import org.openhab.core.types.UnDefType; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -290,7 +288,6 @@ public class HueLightHandler extends BaseThingHandler implements LightStatusList } break; case CHANNEL_SWITCH: - logger.trace("CHANNEL_SWITCH handling command {}", command); if (command instanceof OnOffType) { lightState = LightStateConverter.toOnOffLightState((OnOffType) command); if (isOsramPar16) { @@ -391,31 +388,25 @@ public class HueLightHandler extends BaseThingHandler implements LightStatusList private @Nullable Integer getCurrentColorTemp(@Nullable State lightState) { Integer colorTemp = lastSentColorTemp; if (colorTemp == null && lightState != null) { - colorTemp = lightState.getColorTemperature(); + return lightState.getColorTemperature(); } return colorTemp; } private @Nullable StateUpdate convertBrightnessChangeToStateUpdate(IncreaseDecreaseType command, FullLight light) { - StateUpdate stateUpdate = null; Integer currentBrightness = getCurrentBrightness(light.getState()); - if (currentBrightness != null) { - int newBrightness = LightStateConverter.toAdjustedBrightness(command, currentBrightness); - stateUpdate = createBrightnessStateUpdate(currentBrightness, newBrightness); + if (currentBrightness == null) { + return null; } - return stateUpdate; + int newBrightness = LightStateConverter.toAdjustedBrightness(command, currentBrightness); + return createBrightnessStateUpdate(currentBrightness, newBrightness); } private @Nullable Integer getCurrentBrightness(@Nullable State lightState) { - Integer brightness = lastSentBrightness; - if (brightness == null && lightState != null) { - if (!lightState.isOn()) { - brightness = 0; - } else { - brightness = lightState.getBrightness(); - } + if (lastSentBrightness == null && lightState != null) { + return lightState.isOn() ? lightState.getBrightness() : 0; } - return brightness; + return lastSentBrightness; } private StateUpdate createBrightnessStateUpdate(int currentBrightness, int newBrightness) { @@ -503,24 +494,16 @@ public class HueLightHandler extends BaseThingHandler implements LightStatusList } updateState(CHANNEL_COLOR, hsbType); - ColorMode colorMode = state.getColorMode(); - if (ColorMode.CT.equals(colorMode)) { - updateState(CHANNEL_COLORTEMPERATURE, - LightStateConverter.toColorTemperaturePercentType(state, colorTemperatureCapabilties)); - updateState(CHANNEL_COLORTEMPERATURE_ABS, LightStateConverter.toColorTemperature(state)); - } else { - updateState(CHANNEL_COLORTEMPERATURE, UnDefType.UNDEF); - updateState(CHANNEL_COLORTEMPERATURE_ABS, UnDefType.UNDEF); - } - - PercentType brightnessPercentType = LightStateConverter.toBrightnessPercentType(state); - if (!state.isOn()) { - brightnessPercentType = PercentType.ZERO; - } + PercentType brightnessPercentType = state.isOn() ? LightStateConverter.toBrightnessPercentType(state) + : PercentType.ZERO; updateState(CHANNEL_BRIGHTNESS, brightnessPercentType); updateState(CHANNEL_SWITCH, OnOffType.from(state.isOn())); + updateState(CHANNEL_COLORTEMPERATURE, + LightStateConverter.toColorTemperaturePercentType(state, colorTemperatureCapabilties)); + updateState(CHANNEL_COLORTEMPERATURE_ABS, LightStateConverter.toColorTemperature(state)); + StringType stringType = LightStateConverter.toAlertStringType(state); if (!"NULL".equals(stringType.toString())) { updateState(CHANNEL_ALERT, stringType); diff --git a/bundles/org.openhab.binding.hue/src/test/java/org/openhab/binding/hue/internal/handler/HueLightHandlerTest.java b/bundles/org.openhab.binding.hue/src/test/java/org/openhab/binding/hue/internal/handler/HueLightHandlerTest.java index 58adabcb2..7f2398af4 100644 --- a/bundles/org.openhab.binding.hue/src/test/java/org/openhab/binding/hue/internal/handler/HueLightHandlerTest.java +++ b/bundles/org.openhab.binding.hue/src/test/java/org/openhab/binding/hue/internal/handler/HueLightHandlerTest.java @@ -38,6 +38,7 @@ import org.openhab.core.thing.ChannelUID; import org.openhab.core.thing.Thing; import org.openhab.core.thing.ThingStatus; import org.openhab.core.thing.ThingUID; +import org.openhab.core.thing.binding.ThingHandlerCallback; import org.openhab.core.thing.i18n.ChannelTypeI18nLocalizationService; import org.openhab.core.types.Command; @@ -411,6 +412,7 @@ public class HueLightHandlerTest { return mockBridge; } }; + hueLightHandler.setCallback(mock(ThingHandlerCallback.class)); hueLightHandler.initialize(); verify(mockThing).setProperty(eq(Thing.PROPERTY_MODEL_ID), eq(expectedModel));