Add ColorUtil.xyToDuv #4401
Add ColorUtil.xyToDuv #4401
Conversation
Signed-off-by: Cody Cutrer <[email protected]>
bundles/org.openhab.core/src/main/java/org/openhab/core/util/ColorUtil.java
Outdated
Show resolved
Hide resolved
bundles/org.openhab.core/src/main/java/org/openhab/core/util/ColorUtil.java
Outdated
Show resolved
Hide resolved
and only accept xy, not xyY, to match xyToKelvin (the function that is most closely related to this one) Signed-off-by: Cody Cutrer <[email protected]>
bundles/org.openhab.core/src/main/java/org/openhab/core/util/ColorUtil.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Cody Cutrer <[email protected]>
There was a problem hiding this comment.
Looking again at this PR, I am asking myself why it is necessary to use BigDecimal for the computations - especially using MathContext.DECIMAL128 which ensures that at least 34 digits are used.
I did a quick rewrite to double, and the tests still passed.
So is there a reason to use the computationally much more demanding BigDecimals? Are you aware of any instability caused by special input values, etc.?
I added my implementation as comments, please feel free to resolve those conversations without change in case you have a good reason to go for the BigDecimals.
bundles/org.openhab.core/src/main/java/org/openhab/core/util/ColorUtil.java
Outdated
Show resolved
Hide resolved
bundles/org.openhab.core/src/main/java/org/openhab/core/util/ColorUtil.java
Outdated
Show resolved
Hide resolved
bundles/org.openhab.core/src/main/java/org/openhab/core/util/ColorUtil.java
Outdated
Show resolved
Hide resolved
|
Nope, no good reason. I suspect the original code started out as float (single precision), and there were accuracy issues, and so it jumped all the way to BigDecimal when double would have been plenty sufficient. |
Then I would prefer the implementation based on double. It has way better performance and is easier to read. Maybe you could remove all unused BigDecimal constants as well. |
Signed-off-by: Cody Cutrer <[email protected]>
|
@ccutrer for the esp light bulb you may want to look at my recently merged PR in the Zigbee binding. |
This adds a new helper method to calculate the Duv (Delta u, v) of a color. The code is taken almost literally from the mqtt.espmilighthub binding, but changed slightly to fit in with other functions in ColorUtil. I intend to use this from espmilighthub, but also the HomeKit add-on. HomeKit doesn't allow linking both HSB and Color Temperature characteristics to a single accessory, even though the iOS Home App will present a Color Temperature picker for any HSB light. So I want to allow linking devices that support both RGB+CCT to HomeKit, and if the Duv is within threshold, send a command to the Color Temperature channel instead.
I also think it might be useful to add a profile to apply to a Color channel link that does the same thing - if a color is close enough to "white", send a command to the color temperature item instead. This would cover all other use cases, such as BasicUI and MainUI.
This can also be useful for user rules dealing with RGB+CCT light.