[hue] Fix Hue color temperature regression when light is soft-off or unreachable#21018
[hue] Fix Hue color temperature regression when light is soft-off or unreachable#21018kaikreuzer wants to merge 1 commit into
Conversation
kaikreuzer
changed the title
kaikreuzer
force-pushed
the
kaikreuzer-hue-soft-off-regression
branch
from
83f950b to
0a8652a
Compare
…hable When a color-temperature command is sent to a light that is soft-off, unreachable, or experiences any other transient failure, the binding now: - Immediately reflects the commanded color temperature in the local state cache and item channel state - Remembers the pending mirek value - Merges it into the next plain ON command (switch/on-off channels only) - Clears the pending value once an ON or a successful direct CT command is processed This restores the previously supported behavior where color temperature set while a light is off takes effect when the light is next switched on. Fixes openhab#21011 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Kai Kreuzer <kai@openhab.org>
kaikreuzer
force-pushed
the
kaikreuzer-hue-soft-off-regression
branch
from
0a8652a to
ebae61d
Compare
|
@kaikreuzer I am not sure if I am fully in agreement with this approach. Until now the state of a light in OH is exactly what is received over the wire via SSE from the Hub. And you are proposing to interpolate a best guess buffer between a command sent, and an SSE not seen. i.e. by internally caching some assumed state. This breaks the model, and sets a bad precedent for other future horrors. So I plan to propose a better solution. However before I do it, I would like to better understand exactly what is your issue. So can you please post three things..
EDIT so in other words, I think that the correct state of an Item linked to CT channel after a PUT fails, should simply be whatever value it was commanded to. And it should only change again from that value when the next SSE event comes in to say something different. And if it is off, and/or offline then next SSE that comes in when it is once more on(line) will provide the new CT value directly. i.e. no internal caching. (please) |
|
PS so I am thinking that perhaps this PR should be simplified to just downgrading the log level.. E.g. if you send a CT command to a lamp which off, and then you turn it on (via the on/off only channel) does it come on at the desired CT. Or, if you change the brightness from ~0 to say 60 (via the dimming only channel) does it come on at the desired CT. PS do not try to turn it on via the HSB channel since that sends a munge of color xy, brightness, on/off state, and indeed the color xy may indeed then trash the CT. |
No, and that is the regression I would like to fix (note that before the v2 refactoring, this worked nicely). The use-case is simple: |
Haha. It certainly is not something that I would call a "refactoring". Clip2 is a whole new, and very different API, that totally replaces the whole Hue architecture. And in OH it is a total code base written from scratch. And it took me many weeks to write it. It is certainly not a refactoring nor even a "rewrite" (that would imply the same architecture). So the issue whatever it is, is certainly NOT a regression. Perhaps more an architectural fact of life. And or a request for new functionality. .. But anyway let me think about it.. |
@kaikreuzer can you please help me by running a few tests? A. BackgroundRefer to chart.. 
Internally within the Hue bridge Clip2 API, the state of a lamp is represented by four orthogonal state variables. This is a quantum step different than how the state used to be mapped in Clip1 API (via HSB). The orthogonal state variables are as follows:
These state variables are essentially orthogonal. And in OH these are directly mapped to the ... and then in OH we have the
Therefore when running tests, please always use the B. API Architecture (THEORY)Philips API architecture INTENTION is that the four orthogonal state variables are indeed orthogonal. So one can for example send new values of C. Your Observations (PRACTICE)@kaikreuzer if I understand you correctly, (please correct me if I am wrong), in your case the example cited above in B. DOES work as described. Please confirm. Yet setting a new value of D. Practical ConsiderationsAs you can see from the CIE chart there is a perfect mathematical congruence between CT and XY values. However mathematics inside the Hue bridge is not perfect. To be specific any color temperature between about 2000K and 6500K (depending on the model of the light) will ALWAYS produce a corresponding XY value. And any CT outside that range may not. But -- more importantly -- in the opposite direction any XY value will (unless we are extremely lucky) NOT produce a valid CT. I think Hue uses some algorithm where if XY is "close to" the locus then a CT is returned and otherwise not. However that "close to" algorithm is opaque to us. So to be specific: If you confirm your observations in C. above (OFF => CT => ON => CT' != CT). Can you please test in more detail:
In my opinion the states {1a} or {1c + 2a} are consistent with the Philips / Signify API architecture B. above. Whereas {1b} and {1c + 2b} are not. And indeed if {1b} or {1c + 2b} apply, then this is probably a regression and non compliance versus the API Architecture, rather than a regression caused by us. And @kaikreuzer if you have understood me so far, you may perhaps now understand my concern about making a hasty fix via this PR. ;) |
|
^ .. furthermore .. I do NOT see the problem that you cite (see my log below) neither the error log nor the failure to update the CT state. This was on a model LTW001 light. => So I think we need to fully understand the reasons before committing any fixes.. Note: the above is the log from a LTW001 on/off/dimming/ct-only light. And below is the log from a LCA006 on/off/dimming/full color light (it also has no errors). |
|
Also perhaps we need to go back to this forum thread. See in particular my then comment about not making a hack. https://community.openhab.org/t/philips-hue-clip-2-api-v2-discussion-thread/142111/369 @jlaur do you want to come back on this? |
|
And furthermore -- for the case that the light is offline due to hard power off -- please keep in mind that in the Hue app every light has a setting for "power on behaviour" i.e. [default, Last on, Stays off, Turns On, Custom]. So even if you have sent a CT value (or indeed any other value) to the device whilst it was hard powered off, the simple act of hard powering it on again will execute the "power on behaviour" and this will OVERRIDE whatever state had been sent to it from OH whilst it was physically off. See example below. |
|
Many thanks for your extensive efforts, @andrewfg!
Haha, yeah, ok, I didn't mean to belittle the effort - it was definitely a huge challenge and you did a fantastic job! Sorry that my reply took so long. I only now found the time for a debugging session and it even took me longer than expected. TL;DR: I tested many different lights and most of them behave quite differently. 🤦 I've created this rule for testing: As you can see in the logs, the CT at the end is 0 instead of the expected 100 (and the light itself if cold white) - and after 30 secs, the CT is updated to UNDEF:
This light behaves the same way:
This light works perfectly fine, although it also logs the INFO error and updates CT to 0 (although the light is turned on as warm white i.e. 100) - only 30 secs later it correctly updates CT to 100:
And this one works as expected (i.e. it is "warm white" at the end), while it logs the INFO error, sets CT to 0 and the end and to UNDEF after 30 seconds:
I agree that this PR is not making the situation any better, I'll hence close it. |
After reading that thread again, I'm starting to remember the complexity of this. I noticed the same issue as @kaikreuzer back then, but like you was not able to propose any "clean" solution. Reading about Kai's lights with different behavior, and the minimum dimming level from the forum thread, just one side comment: I was quite recently playing with this in context of #20912, and have one remaining issue with the Hue binding still not sorted out. For some of my lights, e.g. IKEA Trådfri, the minimum dimming level is not reported. When sending the command "0.4", the light doesn't actually turn on. This is a problem when forwarding commands from the Hue emulation. However, for other lights, such as the Hue Signe floor lamp I have in my office, such a command will actually turn on the lamp, although very dimmed (it reports dimming range 0.0%-100%). So compatiblity between devices seems like a topic on its own. I'll probably create an issue to track these findings as I need to come up with some kind of solution.
Please note that this only applies for original Hue lights, not other Zigbee lights controlled by the bridge. |
|
@kaikreuzer many thanks for the testing. I will analyze to see if there is any common conclusion. @jlaur thanks for the input about the Tradfri lights. I have some too so I will analyze. I thought we applied a default minimum dimming level for such, but it is evidently not working.. |
|
@kaikreuzer again many thanks for the logs. I had a quick look at your logs, and I have a few observations already. Maybe more to come..
|
|
^ .. apropos IKEA Tradfri: My own such lights have the following attributes and behaviour:
Ergo: this tends to reinforce the points and questions in @jlaur's and my prior posts above. But since my Tradfri's devices do not support CT, I cannot try @kaikreuzer 's tests. Although I may see if I can order an IKEA Tradfri with CT to play around with. In any case, I am inclined to force a default minimum dimming level of 1.0% on any device which does not support its own |
|
@jlaur some comments..
These two observations are consistent with the current logic.
The minimum dimming level is whatever value is reported by the device or 0.5% if the device does not report it. So your IKEA lamp follows logic branch 2. above and your Signe lamp follows logic branch 1. above.
I hear what you say. However it is obvious that Philips / Signify defines different minimum dimming levels on their devices for a distinct reason. (Just as they define different color gamuts on their different devices). So therefore -- per definition -- there is actually no compatiblity between devices that have different minimum levels .. they will track each other compatibly from dimming 100% down to their respective minimum dimming levels, but below that level the compatibility is indeed broken. This is a physical fact that depends only on the hardware of the two respective devices. We cannot dream to solve this via software.. *) note that this is a case where a 1. brightness command (with munging)1. dimming only and on-off-only commands (WITHOUT munging) |
|
@kaikreuzer some further thoughts apropos the 0.4% reported by all of your logs: It is not a clamping to the default min dim level COMMAND as I had earlier surmised (the default clamp level is 0.5). Actually 0.4 is what the device sends in its RESPONSE after the command is sent. So the sequence is
FACIT I really really need to see your above three test results again but at log TRACE; plus their respective values of |
|
Thanks @andrewfg, I'll do the tests and create the logs that you asked for. Hope to find some time for that in the coming days, so please be patient. |
@kaikreuzer perhaps to save time, try running the tests on my #21062 branch.. |
|
Hmm. @kaikreuzer while waiting for your logs, I bought a Lidl Livarno light (vendor: _TZ3000_49qchf10, modelId: TS0502A) which I think is from the same stable as some of your misbehaving lights. I noticed immediately that it does support a color temperature mirek schema (see below) which should potentially fix out of range HOWEVER using this value would actually be far far worse than wrong using the default schema. At least using the default schema does not crash the binding. So this leaves me in a difficult situation not knowing how to proceed... The light's packaging says the CT range is 2200K .. 6500K (which is 454Mk .. 153Mk) .. however as you can see its JSON is brain dead. (I suppose the 65535 may be a wrong statement of the K upper limit .. but the 0 value is completely and utterly brain dead). It even crashes my code with a divide by zero error. => @kaikreuzer and @jlaure I would appreciate you thoughts on this. |
|
Hi @andrewfg, thanks for following up on this and sorry for me falling quiet - I was actually busy with 5.2 preparations and now it is vacation time, so I probably won't find time for in-depth testing the next weeks and have to ask you for patience. Maybe the best is trying to figure out if reported values do not make any sense (such as the 0-65535 range) and simply ignore them and just go for sensible defaults. Or alternatively, do not support the CT-abs channel at all for such devices, which is probably the cleaner solution (assuming you can still do the CT 0-100% support without issues). |
Indeed. So I think that #21062 has fixed all of your issues. But obviously before we merge it, it would be good if you can test it and confirm that your issues are indeed fixed. For convenience, the JAR is in the first post of the PR so you can simply drop and test. |
|
Great - I'll test your PR next week! |
3 participants
When a color-temperature command is sent to a Hue light that is soft-off, the binding logs a spurious INFO message and the color temperature is lost:
The same issue occurs when the light is temporarily unreachable or any other transient failure prevents the command from taking effect.
Fixes #21011
Solution
When a color-temperature command is sent, the binding now:
pendingColorTemperatureMirek)The "succeeded with errors" log message is now emitted at DEBUG level instead of INFO, so soft-off and similar transient conditions no longer produce noise in the default log output.