-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add min/max color temperature capabilities to channel state descriptions #17638
Comments
@lolodomo for info.. |
Having now studied the code of the bindings mentioned above, the following is my proposed list of actions: AVMFritzSupports fixed min/max of 2700 K / 6500 K. AmazonEchoSupports fixed min/max of 1000 K / 10000 K. DaliSupports fixed min/max of 1000 K / 20000 K. DeconzSupports dynamic min/max in Kelvin. GoveeSupports fixed min/max of 2000 K / 9000 K. HueSupports dynamic min/max in Kelvin. LifxSupports dynamic min/max in Kelvin. MQTT:EspMiLightBulbSupports dynamic min/max in Mirek!! Line 64 in abe8199
NanoLeafSupports dynamic min/max in Kelvin. ZigbeeSupports dynamic min/max in Kelvin. |
What is the difference between mired and mirek ? |
This is not yet clear for me too. There is also the choice of unit done for the item. What happens if the unit is set to Kelvin but state description is in aligned to mired. Here are all the conditions: to consider
|
Actually they are identical. Originally Mired was defined ("micro reciprocal degree"). However the term "degree" is not properly defined. Therefore, subsequently, and properly, the SI committee defined the unit as Mirek ("micro reciprocal kelvin"). The problem is that many people are stuck in the past with Mired, whereas we should look to the future proper SI unit. |
I own two models of hue lights. None of my things expose a color temperature channel. I will give you the models so that you can confirm this is expected. I have a Matter bulb with color temperature but te current channel is currently only a 0%-100%. So not ideal test but of course I can set a state description on any virtual item. |
@andrewfg : I plan to simply use the unit in pattern of the state description to infer the unit of min)max of the state description. We could also list all bindings having a relative color temperature channel to potentially extend them with an absolute color temperature channel. WDYT? |
First is model RB_265 from vendor INNR. Normal that I have not colour temperature channel? |
Apart from the EspMiLightBulb, all the other ones seem to have state min/max in Kelvin. So there are two options:
|
I would not say "normal" but it is certainly possible.
Yes, I think this is indeed class 2. above
I think this should be class 4. above. As I recal it, you are using Hue Bridge v1 (round one). So this can't support Hue API v2. So the lamps cannot declare their capabilities dynamically, Which may mean that the CT channel is no available in your case. Or?? |
@ccutrer I wonder if you can please give your thoughts about potentially changing the state description of EspMiLightBulb from Mired to Kelvin as mentioned in this comment => #17638 (comment) -- whereby the proposed change would be as follows. I guess this could be considered as a breaking change, but perhaps not a very major one? => WDYT?
|
Option 1 is the preferred. I see no reason to force using Kelvin. |
Ok. (Just a tip to remember that min and max become reversed when Mired is inverted into Kelvin). |
I highly lean towards option 1. Besides ESPMilightHub, many (most?) things coming in via MQTT use mired, not Kelvin. In particular, Home Assistant explicitly specifies color temperatures are in mireds only. It would be non-trivial to plumb a change of units all the way through from device discovery to the mqtt.generic value wrappers - we rely on core to do exactly that when we get a command, or post updates! And if you're using a Homie device that supports color temperature (in mired), it would be even more special casing, since Homie has no higher level concept of lighting or color temperature - just a number that just happens to have a particular unit. |
@ccutrer thanks for your feedback.
I am not sure that this is really relevant. ?? In OH we have UoM QuantityTypes with (to be specific) conversion both ways Mirek <=> Kelvin. However I would ignore this comment, since it certainly does not relate to this Issue here.
In OH many bindings produce such bare numbers. However IMHO that does not prevent using UoM since the "particular unit" is known at the time of writing code. But again I would ignore this comment, since it certainly does not relate to this Issue here. Conclusion: I shall not touch EspMiLight. And @lolodomo will handle the state description min/max values based on the unit provided in the state description pattern. |
When doing decisions here, please don't think only of what BasicUI can do, but make sure the REST API response contains enough details to also make other clients work. I'm thinking of cases like min/max in Mirek in state description, item value in Kelvin and min/max specified without unit on the widget level. Since state description and item units can differ, it's not clear to me yet what unit to assume for the sitemap widget. |
It's relevant because it's a binding that provides a state description with mireks, and not in your list of bindings doing so.
It's not a bare number. It's a quantity type, with a unit provided by the end device, and that unit could be either mirek or kelvin. If the color temperature picker can't handle mirek, it would exclude using it with any of these devices.
👍
I'm also a little confused here. Currently, I have items with an item unit of mired, with a (final - because I'm explicitly setting it) state description using K because in my sitemaps that's more familiar. I most of my color temp items are linked with channels that provide it in mired, but I think a couple use Kelvin - but I don't really care because the item ends up as described above. I leave the item in mired because it's easier for doing math with in my rules that automatically adjust color temperature throughout the day. But someone else might prefer keeping it in Kelvin, and only converting to mired to do the math. BUT... this whole paragraph is more relevant to the root issue of openhab/openhab-core#3891 (because it's about items), and less relevant to what state descriptions bindings provide (which is only a guidance to when the item is created). |
@ccutrer / @maniac103 for the avoidance of doubt, the issue is the following: A typical binding with a colour temperature absolute channel (a channel of type Number:Temperature or just plain Number) would have a channel type description as shown below. There is NO PROBLEM concerning the main live value of the channel because it is a QuantityType:Temperature which carries a UoM and can therefore be presented in Mirek or Kelvin and cast both ways. Our problem is with the minimum and maximum colour temperature values for the light in question. Which we need to set the two ends of the colour gradient in the color temperature picker widget. The current plan is to get those from the
There are a number of uses cases as follows:
|
@ccutrer in reply to your above..
Hopefully my post above makes it clearer.
Aha!! I though you were referring to the Home Assistant platform (in contrast to OH). But I now realise you are talking about the mqtt:homeassistant sub- binding. => I will look at that further and give more comments if any.
I now realise you are talking about the mqtt:homie sub- binding. => I will look at that further and give more comments if any.
I guess this is true. But it is still important to set the right guidance IMHO.. |
Somewhat relevant to this discussion: openhab/openhab-core#4079. I hadn't yet seen this, and perhaps need to go add it to my bindings? |
Indeed! The UnitHint would indeed be an alternate source for the UoM for the |
@ccutrer here is my further feedback on the other two mqtt sub- bindings: EDIT: Oops! I should have looked further in the code .. I see that there is already an MqttChannelStateDescriptionProvider in mqtt:generic .. which I believe does indeed do what we want here. Please ignore the prior version of this post |
Have seen contributors struggle with similar but more common cases where temperature is used (fahrenheit vs celsius) and the min/max (and what about step) is not having a unit set. The state pattern is not usable in those cases as it usually is set to I'd like to have some @openhab/core-maintainers to join the discussion as this still feels a bit hackish. and maybe there are idea's from core on how to solve this. |
Apropos UnitHint: It is currently implemented at the level of the ChannelType/StateChannelTypeBuilder and NOT at the level of StateDescription/StateDescriptionFragment/StateDescriptionFragmentBuilder. As we are here discussing the state description and various providers thereof, then unfortunately UnitHint seems to be located in the wrong place. Or?? |
The color temperature channel just allows choosing one of predefined color temperature. Choosing a precise temperature value is not relevant for this binding. |
@maniac103 : I have not checked but I believe the item state description is part of the API response, no ? |
Or allow UoM for the min/max/step values. |
@lolodomo It does contain it, I'm just unsure how to mix'n'match the response values. For reference, here's a sitemap JSON response for a {
"widgetId": "010102",
"type": "Slider",
"visibility": true,
"label": "Decke Farbtemperatur abs [- K]",
"labelSource": "SITEMAP_WIDGET",
"icon": "colorlight",
"staticIcon": false,
"pattern": "%.0f K",
"unit": "K",
"mappings": [
],
"switchSupport": false,
"releaseOnly": false,
"sendFrequency": 0,
"minValue": 2000.0,
"maxValue": 6000.0,
"step": 100.0,
"item": {
"link": "http://<ip>:8080/rest/items/BathroomCeilingLightAbsColorTemperature1",
"state": "NULL",
"stateDescription": {
"minimum": 1000,
"maximum": 10000,
"pattern": "%.0f K",
"readOnly": false,
"options": [
]
},
"unitSymbol": "K",
"type": "Number:Temperature",
"name": "BathroomCeilingLightAbsColorTemperature1",
"label": "Farbtemperatur",
"category": "ColorLight",
"tags": [
"Control",
"ColorTemperature"
],
"groupNames": [
"BathroomCeilingLight"
]
},
"widgets": [
]
}, You see 2 sets of min/max values (widget vs. item), 2 patterns (I wasn't aware of a pattern in Thinking about it, to make things simple from a user's POV and for ease of documentation, I'd prefer if we'd settle on Kelvin at least for state description. That way, specification of the sitemap element becomes free of surprises. |
Yes. I think that would be a good way forward. A new attribute would not break any legacy code.
These xml attributes are of type "decimal" rather than "string". So adding a UoM to them would risk breaking a lot of legacy code. |
Two things:
Therefore for color temperature state descriptions there are just two allowed unit designations -- If so, the MQTT:EspMiLight code here is in fact wrong. And presumably the analog code in MQTT:HomeAssistant and MQTT:Homie is also wrong. => @ccutrer do you agree? And if so, would you like me to add such corrections to this PR?
|
Our documentation is mentioning "mired" as symbol: My tests were done with unit=""mired" set on an item and I got no warning. I also see unit tests using "mired" as symbol. I will log Units.MIRED.getSymbol(). |
With a simple basic editor, I am not sure how you enter such an exponent! I will prefer keeping "mired". |
It is confusing. EDIT 1: but it seems to confirm the "mired" is in fact OK. EDIT 2 and QuantityType.valueOf("123 /MK") too!!
|
^
|
Hmm. Two things:
@maniac103 perhaps we should consider adding the following line here?
|
I didn't actually try this in code.
That sounds like a good idea, since |
While |
^ |
@andrew: one improvement would be to set unit hint to color temperature channel types. I will try to add it to the system channel type. Can you have a look for channel types defined in bindings ? |
Good idea. Will do. |
I have updated your initial message in which you listed bindings that still require investigation. I am almost sure that tradfri or Yeelight require something for example. As explained, the Sony projector requires no change. |
Not necessarily. Some bindings have only a dimensionless CT setting in percent (Number:Dimensionless / Dimmer) or even a low/medium/high setting (Number 1/2/3 etc.) .. and as I recall the Ikea lights are indeed of that latter class. However I will look again at all of them.. |
Discussed in openhab/openhab-addons#17638. Signed-off-by: Laurent Garnier <[email protected]>
For Epson projector also, the value is 0, 1, 2, 3, 4, 5, 6, 7, 8 or 9. Not directly a Kelvin or mirek value. |
Same for Samsung TV binding, the channel is not a value in Kelvin or mirek. |
Re-closing this issue since it is now tracked in #17754 |
Several bindings which support colour temperature absolute channels (i.e. channels for a colour temperature in Kelvin) should have the absolute minimum and maximum Kelvin colour temperature capabilities added to their state descriptions. This should be done:
See the discussion here openhab/openhab-core#3891
The following bindings are foreseen to have this update:
Depends on openhab/openhab-core#4420
Depends on openhab/openhab-core#4429
Note: there are several other bindings which have a more rudimentary control over colour temperature -- eith on a simple 0..100% scale or in steps low/medium/high or similiar. I am noting these below, but currently no action is forseen:
The text was updated successfully, but these errors were encountered: