Closed klopyrev closed 11 months ago
Sure, I can do that. I'll build this into the RGB lights though; adding COLOR_MODE_HS
to DmxWhite
really just transforms it into what we call an rgbww
light. So I'll just add letters to rgb
, rgbw
and rgbww
to output hue and saturation.
What letter would you like for this? h
is already taken sadly. :(
What about u for hue and U for saturation?
There's another problem using rgbww, then. There is no conversion between cold and warm white, and color temperature. That is, this line is not correct. Instead, rgbww_to_color_temperature
needs to be called.
Added in v0.1.13, please test and report back.
rgbww_to_color_temperature
converts cold/warm into the Kelvin scale, but we need a [0. 255] scale. The coldest temperature 0
, means the warm white LED is also 0
, so they're fully related.
This is my configuration:
- channel: 1
name: color_temp_light
type: color_temp
min_temp: 1750K
max_temp: 9910K
channel_setup:
- d
- t
- 0
- 0
- 0
- channel: 6
name: rgbww_light
type: rgbww
min_temp: 1750K
max_temp: 9910K
channel_setup:
- d
- t
- 0
- u
- U
Here's the test script:
alias: Test light
sequence:
- service: light.turn_on
data:
kelvin: 3000
brightness_pct: 50
target:
entity_id:
- light.color_temp_light
- light.rgbww_light
mode: single
The resulting Art-Net packets (in decimal) are:
128 39 00 00 00
128 65 00 00 00
The second number for each channel is the color temperature value. My setup uses the min_temp and max_temp setting such that 0 = 1750K and 255 = 9910K. So, each step = (9910 - 1750) / 255 = 32. 3000K corresponds to (3000 - 1750) / 32 = 39. Thus, the color_temp
light is getting the right value, and the rgbww
light is getting the wrong value.
This is debug that prints the arguments for each of the async_turn_on
calls:
DmxRGBWW.async_turn_on({'brightness': 128, 'rgbww_color': (0, 0, 0, 65, 63)})
DmxWhite.async_turn_on({'color_temp': 333, 'color_temp_kelvin': 3000, 'brightness': 128})
The conversion happens here.
Following the calculation of color_temperature_to_rgbww(3000, 128, 1750, 9910)
:
max_mireds = color_temperature_kelvin_to_mired(min_kelvin) = 571
min_mireds = color_temperature_kelvin_to_mired(max_kelvin) = 100
temperature = color_temperature_kelvin_to_mired(temperature) = 333
mired_range = max_mireds - min_mireds = 471
cold = ((max_mireds - temperature) / mired_range) * brightness = 64.67940552016985
warm = brightness - cold = 63.32059447983015
return (0, 0, 0, round(cold), round(warm))
This is notably different from what is actually needed for the color temperature channel (at least for my LED):
temperature_range = max_kelvin - min_kelvin = 8160
color_temp_channel = ((temperature - min_kelvin) / temperature_range) * 255 = 39.0625
return round(color_temp_channel)
Using the formula from the DMX profiles PDF I linked to earlier for my bulb with DMX-Value = 39
:
CCT = 1750 + 32*DMX-Value = 2998
Thus, using the value of cold_white
for color temperature is not quite right (at least for my bulb). The problems are:
Thanks for writing that all up! Managed to fix it in v0.1.14, please test. :)
Just finished testing your changes. Findings:
t
and T
are swapped now, to the extent that the comment is no longer correct. When color_temp_kelvin
is equal to min_kelvin
, t
gets a value of 255. The comment says 255 = cold. However, low color temperature is warm, not cold.async_turn_on
method gets these arguments: {'brightness': 5, 'rgbww_color': (0, 0, 0, 1, 4)}
. There is simply not enough precision to compute the right color temperature value. The more robust approach would be to specify color temperature in the list of supported color modes. That will result in the color temperature being passed without any lossy conversion.One more thought: I think overall the most robust approach for this library is to avoid doing any conversion at all. Let all conversion happen in LightEntity library, instead.
Let the user specify the supported color mode from LightEntity (or multiple). Add a mapping from letter (like in your current design), to a method to extract a specific value from the kwargs.
E.g. for my light, I would specify supported color modes of COLOR_TEMP and HS. I can specify each value in a script and the Dmx library gets exactly what I specify. The only thing it needs to do is to convert into the range 0-255 for DMX.
Alright both should be fixed in v0.1.15. Thanks for testing so thoroughly! My staging environment isn't linked to physical lights, so didn't notice temperature being inverted.
I fully agree with your last thought, the architecture has grown a bit organically and isn't designed for what it's all doing now. Though I think some mapping will always be necessary (e.g. service call does HS, but light is defined in RGB), a layer of indirection and a more holistic approach in general towards the lights would much better. My dissatisfaction with the code is also the reason why I haven't submitted it to HACS' default repositories yet.
In my platform refactor project to include additional entities beyond just lights, I am rewriting it from the ground up. Party because I have to, party because of the above.
Wow, thanks for these updates. You're a pretty fast coder.
I'll look through the changes later today. I do need to get actual work done today.
My idea about not doing any conversions is to let the conversions happen in the LightEntity library. If the light wants RGB, the user would configure the DMX light library specifying RGB color mode. Then, if the user specifies an HS value in a script, then the LightEntity library will convert HS to RGB. Similarly, if the light wants HS, the user would configure the DMX light library specifying HS color mode. Then, if the user specify an RGB value in a script, then the LightEntity library will convert RGB to HS.
There are two benefits to this: 1) LightEntity becomes a single source of truth for conversions, instead of some code duplicated in the two libraries. 2) The DMX library becomes much simpler and less prone to bugs.
Hi Breina. The color temperature logic looks good now.
Thank you very much for adding these features. I'm overall impressed with how quickly you got to it. I don't have a whole lot of experience with open source, and I'm surprised that you are supporting this library very well.
I'm looking forward to the next versions. I do still recommend removing all color conversion logic from your library. An LED driver shouldn't do its own color conversion if it doesn't need to.
I'm using these professional film lights for home lighting. They have really high CRI and can support a huge number of colors.
The supported DMX profiles are listed here.
I'd like to use the
D CCT GM HUE SAT
profile, which supports expressing color temperature and hue + saturation. That way, I can set the color temperature of the light, or set a specific color.Playing around with your code, if I add
COLOR_MODE_HS
to_supported_color_modes
inDmxWhite
, then in the dashboard home assistant UI, it allows me to specify the color either using color temperature, or hue and saturation. Depending on which one I use in the dashboard UI, I find theasync_turn_on
function being called either with one of these two sets of parameters:{'hs_color': (248.0, 69.0)}
{'color_temp': 127, 'color_temp_kelvin': 7842}
Thus, this functionality is already supported by home assistant. I'd be nice to support it in artnet_led.
I was able to get the light to work with this config, but I can't specify colors: