Skip to content

Commit 67394e2

Browse files
authored
Fix precision-related issues (openhab#20917)
Resolves openhab#20912 Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
1 parent 8212a6f commit 67394e2

8 files changed

Lines changed: 147 additions & 14 deletions

File tree

bundles/org.openhab.io.hueemulation/src/main/java/org/openhab/io/hueemulation/internal/StateUtils.java

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
*/
1313
package org.openhab.io.hueemulation.internal;
1414

15+
import java.math.BigDecimal;
16+
import java.math.RoundingMode;
1517
import java.util.ArrayList;
1618
import java.util.List;
1719
import java.util.Map;
@@ -48,6 +50,9 @@
4850
@NonNullByDefault
4951
public class StateUtils {
5052

53+
private static final BigDecimal BIG_DECIMAL_HUNDRED = BigDecimal.valueOf(100);
54+
private static final BigDecimal BIG_DECIMAL_HUE_BRI_MAX = BigDecimal.valueOf(HueStateBulb.MAX_BRI);
55+
5156
/**
5257
* Compute the hue state from a given item state and a device type.
5358
*
@@ -65,7 +70,8 @@ public static AbstractHueState colorStateFromItemState(State itemState, @Nullabl
6570
if (itemState instanceof HSBType hsbState) {
6671
state = new HueStateColorBulb(hsbState);
6772
} else if (itemState instanceof PercentType percentState) {
68-
state = new HueStateColorBulb(percentState, percentState.intValue() > 0);
73+
state = new HueStateColorBulb(percentState,
74+
percentState.toBigDecimal().compareTo(BigDecimal.ZERO) > 0);
6975
} else if (itemState instanceof OnOffType onOffState) {
7076
state = new HueStateColorBulb(onOffState == OnOffType.ON);
7177
} else {
@@ -76,9 +82,9 @@ public static AbstractHueState colorStateFromItemState(State itemState, @Nullabl
7682
case WhiteTemperatureType:
7783
if (itemState instanceof HSBType hsbState) {
7884
PercentType brightness = hsbState.getBrightness();
79-
state = new HueStateBulb(brightness, brightness.intValue() > 0);
85+
state = new HueStateBulb(brightness, brightness.toBigDecimal().compareTo(BigDecimal.ZERO) > 0);
8086
} else if (itemState instanceof PercentType percentState) {
81-
state = new HueStateBulb(percentState, percentState.intValue() > 0);
87+
state = new HueStateBulb(percentState, percentState.toBigDecimal().compareTo(BigDecimal.ZERO) > 0);
8288
} else if (itemState instanceof OnOffType onOffState) {
8389
state = new HueStateBulb(onOffState == OnOffType.ON);
8490
} else {
@@ -170,7 +176,7 @@ public static Command commandByItemState(State state) throws IllegalStateExcepti
170176
if (newState.bri != null) {
171177
try {
172178
state.as(HueStateBulb.class).bri = newState.bri;
173-
command = new PercentType((int) (newState.bri * 100.0 / HueStateBulb.MAX_BRI + 0.5));
179+
command = percentTypeFromHueBrightness(newState.bri);
174180
successApplied.put("bri", newState.bri);
175181
} catch (ClassCastException e) {
176182
errorApplied.add("bri");
@@ -183,7 +189,7 @@ public static Command commandByItemState(State state) throws IllegalStateExcepti
183189
if (newBri < 0 || newBri > HueStateBulb.MAX_BRI) {
184190
throw new IllegalArgumentException();
185191
}
186-
command = new PercentType((int) (newBri * 100.0 / HueStateBulb.MAX_BRI + 0.5));
192+
command = percentTypeFromHueBrightness(newBri);
187193
successApplied.put("bri", newState.bri);
188194
} catch (ClassCastException e) {
189195
errorApplied.add("bri_inc");
@@ -349,6 +355,28 @@ public static Command commandByItemState(State state) throws IllegalStateExcepti
349355
return command;
350356
}
351357

358+
/**
359+
* Converts a Hue brightness value (0-254) to a {@link PercentType} with two decimal places.
360+
*
361+
* @param brightness the Hue brightness value
362+
* @return the corresponding PercentType
363+
*/
364+
public static PercentType percentTypeFromHueBrightness(int brightness) {
365+
return new PercentType(BigDecimal.valueOf(brightness).multiply(BIG_DECIMAL_HUNDRED)
366+
.divide(BIG_DECIMAL_HUE_BRI_MAX, 2, RoundingMode.HALF_UP));
367+
}
368+
369+
/**
370+
* Converts a {@link PercentType} to a Hue brightness value (1-254).
371+
*
372+
* @param percentValue the PercentType value
373+
* @return the corresponding Hue brightness value
374+
*/
375+
public static int hueBrightnessFromPercentType(PercentType percentValue) {
376+
return Math.max(1, percentValue.toBigDecimal().multiply(BIG_DECIMAL_HUE_BRI_MAX)
377+
.divide(BIG_DECIMAL_HUNDRED, 0, RoundingMode.HALF_UP).intValue());
378+
}
379+
352380
public static @Nullable DeviceType determineTargetType(ConfigStore cs, Item element) {
353381
String category = element.getCategory();
354382
String type = element.getType();

bundles/org.openhab.io.hueemulation/src/main/java/org/openhab/io/hueemulation/internal/dto/HueStateBulb.java

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@
1212
*/
1313
package org.openhab.io.hueemulation.internal.dto;
1414

15+
import org.eclipse.jdt.annotation.NonNullByDefault;
1516
import org.openhab.core.library.types.PercentType;
17+
import org.openhab.io.hueemulation.internal.StateUtils;
1618

1719
/**
1820
* Hue API state object
@@ -21,9 +23,9 @@
2123
* @author David Graeff - "extended color light bulbs" support
2224
*
2325
*/
26+
@NonNullByDefault
2427
public class HueStateBulb extends HueStatePlug {
25-
// https://github.com/openhab/openhab-addons/issues/2881
26-
// Apparently the maximum brightness is 254
28+
2729
public static final int MAX_BRI = 254;
2830
public int bri = 0;
2931

@@ -47,16 +49,15 @@ public HueStateBulb(boolean on) {
4749
*/
4850
public HueStateBulb(PercentType brightness, boolean on) {
4951
super(on);
50-
this.bri = Math.max(1, (int) (brightness.intValue() * MAX_BRI / 100.0 + 0.5));
52+
this.bri = StateUtils.hueBrightnessFromPercentType(brightness);
5153
}
5254

5355
public PercentType toBrightnessType() {
54-
int bri = this.bri * 100 / MAX_BRI;
55-
56-
if (!this.on) {
57-
bri = 0;
56+
if (this.on) {
57+
return StateUtils.percentTypeFromHueBrightness(this.bri);
58+
} else {
59+
return PercentType.ZERO;
5860
}
59-
return new PercentType(bri);
6061
}
6162

6263
@Override

bundles/org.openhab.io.hueemulation/src/main/java/org/openhab/io/hueemulation/internal/dto/HueStateColorBulb.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
*/
1313
package org.openhab.io.hueemulation.internal.dto;
1414

15+
import org.eclipse.jdt.annotation.NonNullByDefault;
1516
import org.openhab.core.library.types.DecimalType;
1617
import org.openhab.core.library.types.HSBType;
1718
import org.openhab.core.library.types.PercentType;
@@ -24,6 +25,7 @@
2425
* @author Florian Lentz - added xy support
2526
*
2627
*/
28+
@NonNullByDefault
2729
public class HueStateColorBulb extends HueStateBulb {
2830
public static final int MAX_HUE = 65535; // For extended color light bulbs
2931
public int hue = 0;
@@ -71,7 +73,7 @@ public HueStateColorBulb(PercentType brightness, boolean on) {
7173
* @param hsb Color information. Sets the hue state to "on" if brightness is > 0.
7274
*/
7375
public HueStateColorBulb(HSBType hsb) {
74-
super(hsb.getBrightness(), hsb.getBrightness().intValue() > 0);
76+
super(hsb.getBrightness(), hsb.getBrightness().compareTo(DecimalType.ZERO) > 0);
7577
this.hue = (int) (hsb.getHue().intValue() * MAX_HUE / 360.0 + 0.5);
7678
this.sat = (int) (hsb.getSaturation().intValue() * MAX_SAT / 100.0 + 0.5);
7779
colormode = this.sat > 0 ? ColorMode.hs : ColorMode.ct;

bundles/org.openhab.io.hueemulation/src/main/java/org/openhab/io/hueemulation/internal/dto/HueStatePlug.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
*/
1313
package org.openhab.io.hueemulation.internal.dto;
1414

15+
import org.eclipse.jdt.annotation.NonNullByDefault;
1516
import org.openhab.core.library.types.OnOffType;
1617

1718
/**
@@ -20,6 +21,7 @@
2021
* @author David Graeff - Initial contribution
2122
*
2223
*/
24+
@NonNullByDefault
2325
public class HueStatePlug extends AbstractHueState {
2426
public boolean on;
2527

bundles/org.openhab.io.hueemulation/src/main/java/org/openhab/io/hueemulation/internal/dto/changerequest/HueStateChange.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,4 +35,11 @@ public class HueStateChange {
3535
public Integer sat_inc;
3636
public List<Double> xy_inc;
3737
public Integer ct_inc;
38+
39+
@Override
40+
public String toString() {
41+
return "on: " + on + ", bri: " + bri + ", hue: " + hue + ", sat: " + sat + ", effect: " + effect + ", ct: " + ct
42+
+ ", alert: " + alert + ", xy: " + xy + ", transitiontime: " + transitiontime + ", bri_inc: " + bri_inc
43+
+ ", hue_inc: " + hue_inc + ", sat_inc: " + sat_inc + ", xy_inc: " + xy_inc + ", ct_inc: " + ct_inc;
44+
}
3845
}

bundles/org.openhab.io.hueemulation/src/main/java/org/openhab/io/hueemulation/internal/rest/LightsAndGroups.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -447,10 +447,16 @@ public Response setLightStateApi(@Context UriInfo uri, //
447447
"Invalid request: No state change data received!");
448448
}
449449

450+
logger.debug("Received state change for light {}: {}", id, newState);
451+
450452
hueDevice.state = StateUtils.colorStateFromItemState(hueDevice.item.getState(), hueDevice.deviceType);
451453

452454
String itemUID = hueDevice.item.getUID();
453455
List<HueResponse> responses = new ArrayList<>();
456+
if (newState.on == Boolean.TRUE && newState.bri != null && newState.bri == 0) {
457+
// The bulb should be on, but the brightness is 0. Skip the brightness change.
458+
newState.bri = null;
459+
}
454460
Command command = StateUtils.computeCommandByState(responses, "/lights/" + id + "/state", hueDevice.state,
455461
newState);
456462

bundles/org.openhab.io.hueemulation/src/test/java/org/openhab/io/hueemulation/internal/rest/LightsAndGroupsTests.java

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -332,6 +332,25 @@ public void changeOnAndBriValues() throws Exception {
332332
assertThat(stateColorBulb.bri, is(200));
333333
}
334334

335+
@Test
336+
public void changeOnAndInvalidBriValues() throws Exception {
337+
HueLightEntry hueLightEntry = cs.ds.lights.get("2");
338+
assertNotNull(hueLightEntry);
339+
HueStateColorBulb stateColorBulb = assertInstanceOf(HueStateColorBulb.class, hueLightEntry.state);
340+
assertThat(stateColorBulb.on, is(false));
341+
assertThat(stateColorBulb.bri, is(1));
342+
343+
String body = "{'on':true,'bri':0}";
344+
ContentResponse response = commonSetup.sendPut("/testuser/lights/2/state", body);
345+
assertThat(response.getStatus(), is(200));
346+
assertThat(response.getContentAsString(), containsString("success"));
347+
hueLightEntry = cs.ds.lights.get("2");
348+
assertNotNull(hueLightEntry);
349+
stateColorBulb = assertInstanceOf(HueStateColorBulb.class, hueLightEntry.state);
350+
assertThat(stateColorBulb.on, is(true));
351+
assertThat(stateColorBulb.bri, is(1));
352+
}
353+
335354
@Test
336355
public void changeHueSatValues() throws Exception {
337356
HueLightEntry hueLightEntry = cs.ds.lights.get("2");
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
/*
2+
* Copyright (c) 2010-2026 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.io.hueemulation.internal.rest;
14+
15+
import static org.hamcrest.CoreMatchers.*;
16+
import static org.hamcrest.MatcherAssert.assertThat;
17+
18+
import java.util.stream.Stream;
19+
20+
import org.eclipse.jdt.annotation.NonNullByDefault;
21+
import org.junit.jupiter.params.ParameterizedTest;
22+
import org.junit.jupiter.params.provider.Arguments;
23+
import org.junit.jupiter.params.provider.MethodSource;
24+
import org.openhab.core.library.types.PercentType;
25+
import org.openhab.io.hueemulation.internal.StateUtils;
26+
27+
/**
28+
* Tests for {@link StateUtils}.
29+
*
30+
* @author Jacob Laursen - Initial contribution
31+
*/
32+
@NonNullByDefault
33+
public class StateUtilsTests {
34+
35+
@ParameterizedTest
36+
@MethodSource("provideTestCasesForPercentTypeFromHueBrightness")
37+
void percentTypeFromHueBrightness(int bri, PercentType expectedPercent) {
38+
assertThat(StateUtils.percentTypeFromHueBrightness(bri), is(equalTo(expectedPercent)));
39+
}
40+
41+
@ParameterizedTest
42+
@MethodSource("provideTestCasesForHueBrightnessFromPercentType")
43+
void hueBrightnessFromPercentType(PercentType percentValue, int expectedBri) {
44+
assertThat(StateUtils.hueBrightnessFromPercentType(percentValue), is(equalTo(expectedBri)));
45+
}
46+
47+
private static Stream<Arguments> provideTestCasesForPercentTypeFromHueBrightness() {
48+
return Stream.of( //
49+
Arguments.of(0, PercentType.ZERO), //
50+
Arguments.of(1, PercentType.valueOf("0.39")), //
51+
Arguments.of(2, PercentType.valueOf("0.79")), //
52+
Arguments.of(3, PercentType.valueOf("1.18")), //
53+
Arguments.of(127, new PercentType(50)), //
54+
Arguments.of(253, PercentType.valueOf("99.61")), //
55+
Arguments.of(254, PercentType.HUNDRED) //
56+
);
57+
}
58+
59+
private static Stream<Arguments> provideTestCasesForHueBrightnessFromPercentType() {
60+
return Stream.of( //
61+
Arguments.of(PercentType.ZERO, 1), //
62+
Arguments.of(PercentType.valueOf("0.5"), 1), //
63+
Arguments.of(new PercentType(1), 3), //
64+
Arguments.of(new PercentType(50), 127), //
65+
Arguments.of(PercentType.HUNDRED, 254) //
66+
);
67+
}
68+
}

0 commit comments

Comments
 (0)