architecture
architecture copied to clipboard
How to support changing light colors when the light is off
Context
Most lights don't have a color when they are off, and the flux component uses light.turn_on with the new properties only when the light is already on. Platforms such as lifx and my vantage infusion lighting component support a light.{lifx,vantage}_set_state service that works even when the light is off. That pattern seems common and is prevalent for lights that support changing colors when off, but isn't directly supported by the light platform, so it is architecturally weird to have another core component such as flux leverage a pattern of behavior of some light implementations.
See also this old (now closed) PR: https://github.com/home-assistant/core/pull/29380
Proposal
We migrate set_state as a core feature of light, and change the implementations to override that rather than using their own idiomatic light.platform_set_state service.
The base implementation remembers the extra attributes and passes them back when the light is ultimately turned on.
flux switches to using the new set_state method instead of conditioning on the light being on before calling turn_on with the color settings.
Consequences
Light color is correct when they turn on rather than having them be wrong until the next flux timeout.
Each light implementation has to ponder set_state, but the base implementation probably can manage them just fine. We probably get to remove/simplify code from each of the platforms that have a PLATFORM_set_state implementation already.
Thoughts?
Anybody have thoughts on this? It seems like a problem for all lights that have color or color temperature capabilities. I'm open to just hearing ways that folks mitigate this enough that it doesn't matter, but it's the place where my house's automation feels the most janky right now (that color temperature changes only kick in after a light has been on for up to a minute or so).
Ideas?
I'd love some guidance on this... I'm willing to do the work as long as I'm confident it'll benefit the broader community (I'm personally using my custom flux module that takes a set_state_service option to call the underlying set_state of the color-capable device). https://github.com/home-assistant/core/pull/29380
If set_state
would be added, people would also expect to be able to see always every attribute in the light state, even if it is not currently in use. So now we will need to also add attributes to indicate which other attributes are actually in use?
And if we add this to light, then we should also add it to switch, etc
And how will it work for integrations that don't support it, just raise? What if you target some that do and some don't support it?
The idea of the base implementation storing the attributes until the light is turned on won't work. What if the light is turned on outside of Home Assistant?
I think that this feature is a lot more complicated than it might look like from the surface.
Before diving into details on how this feature could work, step 1 is to gather data from integrations to see which ones will even support set_state. I think the last time someone looked there were not that many integrations that actually supported it.
These are all good questions, though I'm not as familiar with all of the useful state properties for things like a switch.
What if I reimagined the PR (https://github.com/home-assistant/core/pull/29380/files) so that instead of add set_state, it takes either a set of light entities (like it currently does) OR a script to which it passes the flux value.
In that reimagination, flux can be used for setting light values or for periodically calling a script with a value plucked from a curve defined by sunrise/sunset/other times.
That doesn't seem to break anything and gives folks the ability to use flux for its execution and computation logic without relying on it for the final step of control.
Thoughts?
The idea of the base implementation storing the attributes until the light is turned on won't work. What if the light is turned on outside of Home Assistant?
Just a thought, but what if it never is? There are people who control their lights exclusively or almost exclusively through Home Assistant. An implementation that stores pending attribute changes until the light is turned on by Home Assistant would work very well for them.
Naturally we can't assume that a light is never turned on outside of Home Assistant. Since Home Assistant has imperfect information when this occurs — it can see that the light changed and what those changes are, but doesn't know whether the user wants the light to remain in that state or whether they want any of the pending attribute changes to be applied — attempting to guess the desired behavior and somehow still apply the pending attribute changes doesn't look like an option. I don't think it would be unreasonable for Home Assistant to clear its pending attribute changes in this situation.
As a user, I would not expect Home Assistant to magically know what to do when I go around it. When this happens, the safest option and the one with the least potential for surprise is to do nothing. Assuming this behavior is clearly documented under a "known issues" section on the integration pages, I do believe this feature would make a useful addition to Home Assistant even with that caveat.
All that said, I do agree with this:
I think that this feature is a lot more complicated than it might look like from the surface.
What do you think?
@mickdekkers I don't think that we should add code to the core to make a specific use case (only control via HA) work better while making other use cases more confusing.
@gjbadros I guess forwarding all the data to a script can work and let that figure it out is 👍
The idea of the base implementation storing the attributes until the light is turned on won't work. What if the light is turned on outside of Home Assistant?
Actually, the feature already exists in Home Assistant. The light integration supports default turn-on values that must be configured within a light profile.
This proposal seems like just a dynamic way to set the already existing default values.
Interesting, yea, just allowing the flux component to create a dynamic custom profile with a given name may be perfect, then sets of lights could reference that. I like that approach (though haven't looked at the code for the turn-on profiles). I'm otherwise busy these days so my hass projects on the backburner for a bit, but if no one beats me to it (which they're welcome to do) I think that could be a nice extension to the way flux works.
Seems like there's overlap between this and #537
This architecture issue is old, stale, and possibly obsolete. Things changed a lot over the years. Additionally, we have been moving to discussions for these architectural discussions.
For that reason, I'm going to close this issue.
../Frenck