Add color_temp_command_template for mqtt light component#5105
Conversation
|
@jonfink, thanks for your PR! By analyzing the history of the files in this pull request, we identified @robbiet480, @pvizeli and @fabaff to be potential reviewers. |
There was a problem hiding this comment.
line too long (107 > 79 characters)
There was a problem hiding this comment.
line too long (111 > 79 characters)
There was a problem hiding this comment.
line too long (102 > 79 characters)
There was a problem hiding this comment.
line too long (80 > 79 characters)
There was a problem hiding this comment.
line too long (84 > 79 characters)
jonfink
force-pushed
the
mqtt_light_color_temp
branch
from
24773da to
3b6e228
Compare
Documentation to support home-assistant/core#5105
There was a problem hiding this comment.
This will become color_temp_command_COMMAND ?
There was a problem hiding this comment.
The dictionary key will be color_temp_command to go alongside the existing dictionary key of color_temp.
Honestly, I wasn't sure how best to handle this as it seemed more far-reaching than desired to make a change in the mqtt light component that required an addition to the homeassistant.const module. If that is the standard practice, then CONF_COLOR_TEMP_COMMAND could be added to homeassistant/const.py...
There was a problem hiding this comment.
Just use CONF_COLOR_TEMP_COMMAND as key here.
|
What needs to be done here to get this approved? @jonfink how can I help? |
Home assistant internal unit for color temperature is Mired but some systems (e.g., SmartThings through mqtt bridge) use Kelvin. Conversion between these types can be handled with the value template concept (used previously to unpack json values). Added a "color_temp_command_template" configuration option for MQTT lights that specifies a function to be applied to the HA-internal color temperature value to get a light-compatible value.
jonfink
force-pushed
the
mqtt_light_color_temp
branch
from
66012e9 to
8b88d77
Compare
|
I think I've addressed all of the above comments with the exception of the dictionary keys used to instantiate the MqttLight class |
There was a problem hiding this comment.
Just use CONF_COLOR_TEMP_COMMAND as key here.
| self._topic[CONF_COLOR_TEMP_COMMAND_TOPIC] is not None: | ||
| color_temp = int(kwargs[ATTR_COLOR_TEMP]) | ||
| color_temp = int(float(( | ||
| self.templates[CONF_COLOR_TEMP+"_COMMAND"]( |
There was a problem hiding this comment.
Use CONF_COLOR_TEMP_COMMAND as key here.
There was a problem hiding this comment.
Hey @MartinHjelmare -- doesn't CONF_COLOR_TEMP_COMMAND have to be added to https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/const.py ? Is that the change you are calling for?
There was a problem hiding this comment.
No, you've already added it to homeassistant/components/light/mqtt.py which is fine. Constants that are only used in one module don't need to be in const.py. Just use the constant now in the two places where it's needed.
|
How do the collaborators feel about this approach vs using a configuration option and doing the conversion from mired to kelvin there? I ask because, someone (me) is going to want to add RGB templating next, and you can't quite do RGB -> hue/sat very well in a template (as far as I know, the math would be too complex). So why not just add two more variables, color_temp_mode (similar to flux component: https://home-assistant.io/components/switch.flux/), and color_mode (rgb vs xy vs hue/sat) ? I think really, the only reason @jonfink is adding the support for this, is so we can convert HA's default color_temp (mired) to kelvin. I can't see any other use for templating out the command other than that conversion. |
|
@jjensn I like that approach 👍 |
|
@jonfink any updates on this? |
|
This PR seems to have gone stale. Closing it. You can reopen it when you're ready to finish it. |
|
Any chance on resurrecting this? Would be handy to control osram bulbs via HA->ST. |
10 participants
Description:
Home assistant internal unit for color temperature is Mired but some
systems (e.g., SmartThings through mqtt bridge) use Kelvin. Conversion
between these types can be handled with the value template
concept (used previously to unpack json values).
Added a "color_temp_command_template" configuration option for MQTT
lights that specifies a function to be applied to the HA-internal
color temperature value to get a light-compatible value.
Related issue (if applicable):
n/a
Pull request in home-assistant.github.io with documentation (if applicable):
home-assistant/home-assistant.io#1673
Checklist:
If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
toxrun successfully. Your PR cannot be merged unless tests passREQUIREMENTSvariable (example).requirements_all.txtby runningscript/gen_requirements_all.py..coveragerc.If the code does not interact with devices:
n/a code interacts with mqtt devices