Skip to content
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

[Sitemap] Widget for color temperature #3891

Closed
5 of 6 tasks
mueller-ma opened this issue Nov 26, 2023 · 92 comments
Closed
5 of 6 tasks

[Sitemap] Widget for color temperature #3891

mueller-ma opened this issue Nov 26, 2023 · 92 comments
Labels

Comments

@mueller-ma
Copy link
Member

mueller-ma commented Nov 26, 2023

Is your feature request related to a problem? Please describe.

For some time I created two sliders for lightbulbs that can change their color temperature. One for brightness, one for color temperature.
Recently I acquired a Nanoleaf Elements light (LEDs with brightness and color temperature settings). The binding allows to use a color picker to control both brightness and color temperature settings. All colors expect in the range cold white to warm white are ignored by the binding, but I can control both with just one widget.

Describe the solution you'd like

Add a new option to the Color widget (e.g. temperatureOnly) that limits the color selection from cold white to warm white.

Example from the Ikea Tradfri app:
tradfri

Describe alternatives you've considered

Additional context

Coordination between maintainers

Notify maintainers of other UIs:
@openhab/webui-maintainers
@openhab/android-maintainers
@openhab/ios-maintainers

Checklist for implementation:

  • Core
  • Basic UI
  • Main UI
  • Android app
  • iOS app
  • Docs
@lolodomo
Copy link
Contributor

For a color item, we have already the colorpicker widget to select the color, the slider widget to select its brightness and the switch widget to turn it on or off.

For color temperature, we could imagine either a new widget or an option to the existing colorpicker widget. In both cases, we need to implement a new UI in all apps. It could be a simple slider ? As a min and max values are probably required, a new widget similar to slider widget would be better I believe. We could name it colortemperature.

@lolodomo
Copy link
Contributor

To be thought how an UI can then build the command to be sent to the server.

@mueller-ma
Copy link
Member Author

For a color item, we have already the colorpicker widget to select the color

The colorpicker can also control the brightness, which is very nice.

When making the color temperature a new option for the colorpicker, we solve two issues:

  1. The state is transmitted as HSB and the binding then has to parse brightness and color temperature from that single state.
  2. Clients without updates show the current color picker which is still useful. It also shows unsupported colors, but better than nothing.

@maniac103
Copy link
Contributor

The state is transmitted as HSB and the binding then has to parse brightness and color temperature from that single state.

Color temperature channels are either Dimmers or Numbers though, not Colors, so I don't think that idea will fly.

For controlling color temperature channels, the client would need to differentiate between dimmer (relative) or absolute channels (by item type):

  • Dimmer items have a slider range of 0..100; we'd need to show some generic gradient for those. Commands sent are ordinary percentages.
  • Number items should have min and max in their state description, so we could calculate the precise colors for the gradient. Commands sent equal those of normal number sliders

@mueller-ma
Copy link
Member Author

Ok, so then it's required to add two items to one widget, like ColorTemperature brightnessItem=foo temperatureItem=bar. This is probably the first widget with more than one item. Is this an argument against it?

@andrewfg
Copy link
Contributor

You could consider to do it like the Philips Hue App -- see screenshots below..

image

image

@lolodomo
Copy link
Contributor

lolodomo commented Oct 20, 2024

We have two system channel types for color temperature. The first expects a Dimmer item:

public static final ChannelType SYSTEM_COLOR_TEMPERATURE = ChannelTypeBuilder

The second expects a Number:Temperature item:
public static final ChannelType SYSTEM_COLOR_TEMPERATURE_ABS = ChannelTypeBuilder

For both of them, it looks natural to use a Slider widget.

But I agree that for the second we could imagine a new dedicated widget showing the rendering of white, I imagine it as a slider + an image showing a gradient of temperatures but each app could implement it as it prefers. The idea would be to handle a kelvin (or mireds) value.
Are you ok with the idea of a new sitemap element for that ?
For Baisc UI, I need to find an image showing a color gradient between 1000 and 10000 K. The widget will look similar to the color widget but with a rectangle instead of a circle.
Something like:
https://upload.wikimedia.org/wikipedia/commons/e/e9/Color_temperature_black_body_800-12200K.svg

@lolodomo
Copy link
Contributor

For this new sitemap element, I imagine in Basic UI the same rendering as for a Slider element but with an additional button allowing to open a popup to pick a color temperature.

@andrewfg
Copy link
Contributor

@andrewfg : do you know mathematics to convert a kelvin temperature into a RGB value ?

In OH core ColorUtil we have public methods for converting everything to everything, and back. For your specific case of Kelvin to RGB one would use kelvinToXY() => xyToHSB() => hsbToRGB() ..

@andrewfg
Copy link
Contributor

andrewfg commented Oct 20, 2024

PS apropos this issue in general: As the OP has remarked, most light bindings have two ways of setting the Color Temperature -- namely

  • via a precise QuantityType:Temperature in Kelvin (or its inverse in Mirek/Mired), and
  • via a 0..100% slider.

It is important to note that in the latter case the respective 0% and 100% points relate to the minimum and maximum Mirek capabilities of the actual lamp being controlled. So, in other words, if you have two lamps from different manufacturers with different colour temperature capabilities, then any particular slider position (say 42%) could relate to a totally different colour temperature.

Therefore for the purposes of this PR I would strongly suggest to focus only on a UI element that produces an absolute QuantityType:Temperature value. i.e. forget about the 0..100% slider stuff..

@lolodomo
Copy link
Contributor

Yes, that is also my idea.

I prefer a separate widget that could also be considered when Default widget is set and the item is of type Number with temperature dimension and has colorTemperature tag.

I would name it Colortemperaturepicker.

@mueller-ma : is it also ok for you?

I can implement the core part quickly, it is only few new lines of code.

@lolodomo
Copy link
Contributor

lolodomo commented Oct 20, 2024

Reading the first messages, I realize that this will provide only color temperature control, not brightness control in the same widget.
As brightness and color temperature are not part of the same item, we have no choice.
Having a widget linked to 2 different items would be a massive change.
Or we should rethink the Color item to embed optionally color temperature but that would be another big change.

@lolodomo
Copy link
Contributor

But having two widgets, one for brightness and one for colour temperature is acceptable I believe.

@andrewfg
Copy link
Contributor

^
Think of the example of the Hue App (see below) it supports 4 widgets -- namely On/Off, Brightness / Color Picker / Colour Temperature Picker. (Not to mention the other Hue specific widgets for Effects etc.)

Screenshot_20241020_173206_Hue

@mueller-ma
Copy link
Member Author

The issue is mostly above having one widget for brightness and temperature. As I suggested in my first post, the color picker could be limited to temperatures of white.

@andrewfg
Copy link
Contributor

andrewfg commented Oct 20, 2024

@lolodomo also when you implement something, please remember that the widget output QuantityType:Temperature should NOT produce a linear output in Kelvin. This is because the human eye does not perceive a linear Kelvin transition as a linear visual transition. That is the reason why the inverse Kelvin Mirek unit was invented. The human eye DOES perceive a linear Mirek transition as a linear visual effect.

@lolodomo
Copy link
Contributor

I understand that my proposal does not respond to your need!
What would be your concrete proposal that we can implement in OH considering the existing constraints?

@lolodomo
Copy link
Contributor

That is the reason why the inverse Kelvin Mirek unit was invented. The human eye DOES perceive a linear Mirek transition as a linear visual effect.

If this is better, we can consider mired instead of Kelvin.

@lolodomo
Copy link
Contributor

Think of the example of the Hue App (see below) it supports 4 widgets -- namely On/Off, Brightness / Color Picker / Colour Temperature Picker.

So that's fine to have different widgets?

@maniac103
Copy link
Contributor

As I suggested in my first post, the color picker could be limited to temperatures of white.

I don't see how that would work (in terms of syntax) though. A color picker for color and brightness needs only one item (as brightness is part of color), a color picker for color temperature would need two items (color temperature and brightness). How should core and/or main UI validate that properly?

@lolodomo
Copy link
Contributor

The hue example shows clearly different "widgets", we could have the same approach in openHAB. We already have these existing widgets:

  1. Switch
  2. Slider for brightness
  3. Colour picker + Slider for Brightness => this was possible because color and brightness are combined in the same HSB state.

My proposal is to add a fourth widget for colour temperature (but not combined with brightness)..

@andrewfg
Copy link
Contributor

if this is better, we can consider mired instead of Kelvin.

You must definitely do this. It must produce QuantityType:Temperature where the linear transition from one end of the widget control to the other produces a linear output in Mirek. Typically the lowest Mirek is 153 (aka 6500 Kelvin) and the highest Mirek is 500 (aka 2000 Kelvin)

add a fourth widget for colour temperature (but not combined with brightness)..

Makes sense.

@lolodomo
Copy link
Contributor

@andrewfg : I will have more difficulty to find an image with a gradient in mired instead of Kelvin.

@andrewfg
Copy link
Contributor

more difficulty to find an image with a gradient in mired instead of Kelvin.

Umm. That is not really a reason to do it wrong. And you could use the gradient that the OP copied from Hue above. IMHO the Hue engineers do really have a solid understanding.

@lolodomo
Copy link
Contributor

I found this one but the values looks wrong, like negative.
https://commons.wikimedia.org/wiki/File:Color_temperature_black_body_mired.svg

@andrewfg
Copy link
Contributor

^
@lolodomo if you want, I can try to (mis) use the ColorUtils methods to programmatically produce a table of RGB values that match the proper gradient. Advantage of doing it that way would be getting a perfect match between UI and OH Java code. (Always assuming your display has proper color calibration). I would give you a table with 101 RGB values matching the Mirek scale.

PS you also need to decide the min/max range of the scale. It seems that max Mirek 500 (2000 K) is fairly non controversial. And in OH bindings (resp. actual lamps) min Mirek 153 (6500 K) is common. But others seem to prefer min Mirek 100 (10000 K). The latter would perhaps be more future proof for future much bluer lamps, but would overflow for lamps like Hue or Zigbee ones.

@andrewfg
Copy link
Contributor

the values looks wrong

Agreed. ):

@andrewfg
Copy link
Contributor

I will create some gradients tomorrow. What image format do you prefer (e.g. png etc.) and what width (in pixels)? Note pixel height is trivial since each row is the same..

@lolodomo
Copy link
Contributor

lolodomo commented Oct 20, 2024

Looking at the current color picker in Basic UI, I see the gradient for brightness dynamically adjusted with the selected colour. I assume it is computed, I have to check how. Maybe I could replace it with a computed gradient of mired values.

If not possible, we could have an image with a linear gradient between 100 and 500 mired. Same width as the colorwheel in the colour picker (to be checked). Colorwheel image is in PNG format so let's go with same format. I will reject selection of temperature outside the min/max of the current light. If the user clicks < min in the image, I will adjust to min. Similar for max.

But if I could build dynamically the gradient, that would be even better.

lolodomo added a commit to lolodomo/openhab-webui that referenced this issue Nov 9, 2024
lolodomo added a commit to lolodomo/openhab-webui that referenced this issue Nov 9, 2024
kaikreuzer pushed a commit to openhab/openhab-webui that referenced this issue Nov 9, 2024
@lolodomo
Copy link
Contributor

lolodomo commented Nov 10, 2024

Feature is now implemented in Basic UI and will be available in 4.3 M3.
I will update sitemap documentation.

@andrewfg
Copy link
Contributor

I will update sitemap documentation.

@lolodomo gentle reminder to add this (doc) to 4.3 RC .. and also add to the 'What's new in 4.3' announcement..

@mueller-ma
Copy link
Member Author

@maniac103 Can you share the code from your prototype for the Android app? A draft PR would be fine for me, so I can have a look at it and maybe finish the new slider.

@maniac103
Copy link
Contributor

Can you share the code from your prototype for the Android app?

Sure: openhab/openhab-android#3830

I think it should work, but it's completely untested at the moment.

lolodomo added a commit to lolodomo/openhab-docs that referenced this issue Nov 24, 2024
Also include a small change of the Colorpicker element.

Related to openhab/openhab-core#3891
Related to openhab/openhab-webui#2873

Signed-off-by: Laurent Garnier <[email protected]>
stefan-hoehn pushed a commit to openhab/openhab-docs that referenced this issue Nov 29, 2024
* [sitemap] New Colortemperaturepicker element

Also include a small change of the Colorpicker element.

Related to openhab/openhab-core#3891
Related to openhab/openhab-webui#2873

Signed-off-by: Laurent Garnier <[email protected]>

* Review comments

Signed-off-by: Laurent Garnier <[email protected]>

---------

Signed-off-by: Laurent Garnier <[email protected]>
florian-h05 pushed a commit to openhab/openhab-webui that referenced this issue Nov 30, 2024
Closes #2852.

Refs openhab/openhab-core#4420.
Related to openhab/openhab-core#3891.

This implements configuring a color temperature picker in the sitemap builder UI.

It also does some visualisation improvements of names and labels (by
defaults shows item label in treeview, analogous to model treeview).

---------

Also-by: Florian Hotze <[email protected]>
Signed-off-by: Mark Herwege <[email protected]>
@florian-h05
Copy link
Contributor

I just merged the Main UI PR 👍
@mueller-ma Can you please update the to-do list on top?

@lolodomo
Copy link
Contributor

Documentation was also updated (merged).
I believe this feature request can now be closed.
Remains only the implementation in Android and iOS apps.

@maniac103
Copy link
Contributor

Implementation for Android also is 90% done. I just need to do some final cleanup there.

@lolodomo
Copy link
Contributor

lolodomo commented Dec 1, 2024

I discovered a case where determining of unit of min/max in state description does not work !
Imagine an item linked to a channel that defines a state description with min/max/pattern in mirek like for example "%.0f mirek" as pattern.
If the user defines a label for this item with a pattern in Kelvin like for example "Lamp [%:0f K]", the final state description retrieved by UI has min/max in mirek but pattern is `"%:0f K" so considering unit in pattern to get unit of min/max is wrong in this case.

I believe we really need the ranegUnit proposed by @lsiepel to avoid any error.

@lolodomo
Copy link
Contributor

lolodomo commented Dec 1, 2024

Until we have the rangeUnit in state description, I believe it would be safer for color temperature to consider min/max values themselves instead of the unit in pattern. If values are <= 1000, we could assume it is mirek. If they are >= 1000, they are Kelvin. WDYT ?

@lsiepel
Copy link
Contributor

lsiepel commented Dec 1, 2024

@lolodomo refers to this issue: #4432

@maniac103
Copy link
Contributor

Is this an actual issue (IOW, are there bindings that use Mirek) or (at least currently) an academic one?

@andrewfg
Copy link
Contributor

andrewfg commented Dec 1, 2024

There is at least one binding that uses mirek. However we have a work around for it for the state description for purposes of interpreting min/max -- albeit not for purposes of UI

@lolodomo
Copy link
Contributor

lolodomo commented Dec 1, 2024

Is this an actual issue (IOW, are there bindings that use Mirek) or (at least currently) an academic one?

Mirek is used in one binding (don't remember which one) of our distribution + the new matter binding.
The problem is not specific to mirek, we have the same issue if state description is in Kelvin but the user sets a pattern with mirek on the item that overrides pattern from channel..
To be clear, the logic to guess the unit of min/max from the pattern in state description is not fully reliable.
Note that there is no problem if the pattern is set on sitemap element.

@lolodomo
Copy link
Contributor

lolodomo commented Dec 3, 2024

PR openhab/openhab-webui#2894 is fixing unit determination from state description.

@lolodomo
Copy link
Contributor

lolodomo commented Dec 8, 2024

It is now also implemented in Android app. I believe we can close this feature without waiting for an implementation in iOS app.

@lsiepel
Copy link
Contributor

lsiepel commented Dec 8, 2024

It is now also implemented in Android app. I believe we can close this feature without waiting for an implementation in iOS app.

Is there a issue in the iOS repo that you can link?

@lolodomo
Copy link
Contributor

lolodomo commented Dec 8, 2024

There is no issue but I guess this is not yet implemented. I have not checked.

@mueller-ma
Copy link
Member Author

openhab/openhab-ios#855

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants