Skip to content
This repository has been archived by the owner on May 7, 2020. It is now read-only.

[Tradfri] Fix color light bug when brightness is less than 2% #4344

Merged
merged 1 commit into from Sep 28, 2017
Merged

[Tradfri] Fix color light bug when brightness is less than 2% #4344

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
38 changes: 35 additions & 3 deletions ...i.test/src/test/java/org/eclipse/smarthome/binding/tradfri/internal/TradfriColorTest.java
View file
Open in desktop
Expand Up @@ -22,7 +22,7 @@ public class TradfriColorTest {

@Test
public void testFromCieKnownGood1() {
TradfriColor color = TradfriColor.fromCie(29577, 12294, 254);
TradfriColor color = TradfriColor.fromCie(29577, 12294, 354);
assertNotNull(color);
assertEquals(254, (int) color.rgbR);
assertEquals(2, (int) color.rgbG);
Expand All @@ -41,15 +41,31 @@ public void testFromCieKnownGood2() {
TradfriColor color = TradfriColor.fromCie(19983, 37417, 84);
assertNotNull(color);
assertEquals(30, (int) color.rgbR);
assertEquals(84, (int) color.rgbG);
assertEquals(86, (int) color.rgbG);
assertEquals(7, (int) color.rgbB);
assertEquals(19983, (int) color.xyX);
assertEquals(37417, (int) color.xyY);
assertEquals(84, (int) color.brightness);
assertNotNull(color.hsbType);
assertEquals(102, color.hsbType.getHue().intValue());
assertEquals(89, color.hsbType.getSaturation().intValue());
assertEquals(33, color.hsbType.getBrightness().intValue());
assertEquals(34, color.hsbType.getBrightness().intValue());
}

@Test
public void testFromCieKnownGood3() {
TradfriColor color = TradfriColor.fromCie(19983, 37417, 1);
assertNotNull(color);
assertEquals(0, (int) color.rgbR);
assertEquals(2, (int) color.rgbG);
assertEquals(0, (int) color.rgbB);
assertEquals(19983, (int) color.xyX);
assertEquals(37417, (int) color.xyY);
assertEquals(1, (int) color.brightness);
assertNotNull(color.hsbType);
assertEquals(120, color.hsbType.getHue().intValue());
assertEquals(100, color.hsbType.getSaturation().intValue());
assertEquals(1, color.hsbType.getBrightness().intValue());
}

@Test
Expand All @@ -67,6 +83,22 @@ public void testFromHSBTypeKnownGood1() {
assertEquals(100, color.hsbType.getSaturation().intValue());
assertEquals(100, color.hsbType.getBrightness().intValue());
}

@Test
public void testFromHSBTypeKnownGood2() {
TradfriColor color = TradfriColor.fromHSBType(new HSBType("0,100,1"));
assertNotNull(color);
assertEquals(2, (int) color.rgbR);
assertEquals(0, (int) color.rgbG);
assertEquals(0, (int) color.rgbB);
assertEquals(45914, (int) color.xyX);
assertEquals(19615, (int) color.xyY);
assertEquals(2, (int) color.brightness);
assertNotNull(color.hsbType);
assertEquals(0, color.hsbType.getHue().intValue());
assertEquals(100, color.hsbType.getSaturation().intValue());
assertEquals(1, color.hsbType.getBrightness().intValue());
}

@Test
public void testConversionReverse() {
Expand Down
14 changes: 6 additions & 8 deletions ...dfri/src/main/java/org/eclipse/smarthome/binding/tradfri/handler/TradfriLightHandler.java
View file
Open in desktop
Expand Up @@ -346,16 +346,14 @@ public LightData setBrightness(PercentType brightness) {
}

public PercentType getBrightness() {
PercentType result = null;

JsonElement dimmer = attributes.get(DIMMER);
if (dimmer != null) {
int b = dimmer.getAsInt();
if (b == 1) {
return new PercentType(1);
}
return new PercentType((int) Math.round(b / 2.54));
} else {
return null;
}
result = TradfriColor.xyBrightnessToPercentType(dimmer.getAsInt());
}

return result;
}

public boolean getReachabilityStatus() {
Expand Down
20 changes: 18 additions & 2 deletions ...ng.tradfri/src/main/java/org/eclipse/smarthome/binding/tradfri/internal/TradfriColor.java
View file
Open in desktop
Expand Up @@ -245,8 +245,8 @@ private static HSBType constructHsbTypeFromRgbWithBrightnessPercent(int rgbR, in
int xyBrightness) {
// construct HSBType from RGB values
HSBType hsbFullBright = HSBType.fromRGB(rgbR, rgbG, rgbB);
// get hue and saturation from HSBType and construct new HSBType based on these values with the given brightbess
PercentType brightnessPercent = new PercentType((int) (xyBrightness / 2.54));
// get hue and saturation from HSBType and construct new HSBType based on these values with the given brightness
PercentType brightnessPercent = xyBrightnessToPercentType(xyBrightness);
HSBType hsb = new HSBType(hsbFullBright.getHue(), hsbFullBright.getSaturation(), brightnessPercent);
return hsb;
}
Expand Down Expand Up @@ -279,5 +279,21 @@ public static PercentType calculateColorTemperature(int xyX, int xyY) {
}
return new PercentType((int) Math.round(value * 100.0));
}


/**
* Converts the xyBrightness value to PercentType
*
* @param xyBrightness xy brightness level 0 to 254
* @return {@link PercentType} with brightness level (0 = light is off, 1 = lowest, 100 = highest)
*/
public static PercentType xyBrightnessToPercentType(int xyBrightness) {
if (xyBrightness > 254) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

out of curiosity: why are values > 254 a problem? All we have to do is adopt the 2.54 in line 296 and we could also deal with 255 or greater. wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well - my understanding for a PercentType is to be between 0 and 100. So - if xyBrightness > 254 then xyBrightness / 2.54 will return value > 100 and it will throw IllegalArgumentException

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I get that. All we have to adopt is the divisor (2.54) to a value that matches our upper bound (254). In case we move the upper bound to 255 the divisor should be adopted to 2.55 to match the PercentType. I just wanted to know where the 254 originates and why there are possible values grater then 254.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this comes from the IKEA API (not not really sure)

May be @SJKA or @kaikreuzer can give more info

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the range 0 to 254 is defined for Zigbee ZLL brightness and the IKEA gateway adopts this.

xyBrightness = 254;
} else if (xyBrightness < 0) {
xyBrightness = 0;
}
return new PercentType((int) Math.ceil(xyBrightness / 2.54));
}

}