[BasicUI] HSB Colorpicker sets 50% brightness when color is “picked”

[Trickx]

Setup:

• OH 2.5.9
• Tasmota HSB driven LED bulb connected via MQTT

Issue I have an issue with a colorpicker (HSB). When I open the colorpicker, it shows correct color and brightness information. My Tasmota bulb sends JSON-formatted result strings via MQTT in case of an update. While JSONpath parsing of a switch works fine, the colorpicker jumps back to 50% brightness after setting a color in most cases (but not always). The command is send correctly.

It seems that the status update is not processed correctly. JSONpath parsing works fine when triggered externally as well as via the colorpicker:

2020-10-21 15:17:01.278 [ome.event.ItemCommandEvent] - Item ‘BTL_bulb_1_color’ received command 238,92,100
2020-10-21 15:17:01.327 [vent.ItemStateChangedEvent] - BTL_bulb_1_color changed from 359,100,100 to 238,92,100
2020-10-21 15:17:05.348 [ome.event.ItemCommandEvent] - Item ‘BTL_bulb_1_color’ received command 116,6,100
2020-10-21 15:17:05.427 [vent.ItemStateChangedEvent] - BTL_bulb_1_color changed from 238,92,100 to 116,6,100
2020-10-21 15:17:08.384 [ome.event.ItemCommandEvent] - Item ‘BTL_bulb_1_color’ received command 46,98,100
2020-10-21 15:17:08.424 [vent.ItemStateChangedEvent] - BTL_bulb_1_color changed from 116,6,100 to 46,98,100
2020-10-21 15:17:09.315 [ome.event.ItemCommandEvent] - Item ‘BTL_bulb_1_color’ received command 303,82,100
2020-10-21 15:17:09.391 [vent.ItemStateChangedEvent] - BTL_bulb_1_color changed from 46,98,100 to 303,82,100

As you can see, brightness was always set to 100, but the colorpicker shows 50%.

Additionally, when I change the color externally and re-open the colorpicker, a white color and brightness of 50% are shown. My interpretation is that the communication between the bulb item and the colorpicker is somehow buggy.

The setup works fine via the iOS app, so openHAB config should be ok. This issue has been discussed also at openHAB's community forum.

[kaikreuzer]

From the logs that you have provided, I'd think you are right that this is a bug in the colorpicker. @lolodomo As you have helped a lot in the past on the Basic UI, might you be interested in looking into this?

[lolodomo]

Yes, the color picker looks a little buggy. My current feeling is that it could be a HSB)RGB convention problem.

[Shituation]

Any news on this? is it there any way to use sliders to choose colors in sitemap in the meanwhile?

[lolodomo]

Noone proposed a contribution for that. And this is probably out of my main skills. HSB sliders like in MainUI would be better and more reliable. I am almost convinced that the source of the problem is the HSB <-> RGB conversion done with the current color picker. .

[andrewfg]

Hi @lolodomo I came across this issue during my work on the Philips Hue CLIP 2 PR and I am hoping you may be able to give me some advice concerning the 'color' channel for Hue CLIP 2.

In Hue CLIP 2 the lamp state parameters are represented by four JSON elements as follows..

.. where ..

• The 'OnOff' element is a boolean.
• The 'Dimmer' element is the brightness 0..100
• The 'ColorTemperature' element is the colour temperature in 'Mirek' (reciprocal Kelvin) units.
• The 'ColorXy' element is the colour hue as an XY pair on a colour coordinate plane.

My question specifically concerns the Hue full colour lights.

As you know, in OH we use the following channel types, state classes, and UI controls as follows..

• Channel type 'Color' <=> state class 'HSBType' <=> UI control 'Colorpicker'
• Channel type 'Dimmer' <=> state class 'Number:Dimensionless' <=> UI control 'Slider'

This does not match at all well with the Hue CLIP 2 JSON model, since the OH channel type 'Color' (HSBType) is a hybrid that combines the JSON 'Dimmer' and 'ColorXy' elements into a single state. And furthermore, it makes sense from a user perspective, to map the 'Dimmer' JSON element (also/separately) to an OH channel type 'Dimmer'; so we risk to have two OH channels 'cross-interfering' with the state of the lamps; i.e. OH 'Color' reads/writes both JSON 'Dimmer' and JSON 'ColorXy' elements, and/or OH 'Dimmer` reads/writes JSON 'Dimmer' element only..

=> WDYT ?

PS not to mention the further complication that, as this issue states, the BasicUI Colorpicker widget is apparently broken.

[lolodomo]

If I am not wrong, we define only one channel "Color" which can then be used to adjust color and brigthness through items linked to that unique channel. HSB is I believe the reference color system we use in openHAB. So I assume you will have to implement conversions. Your problem is certainly not specific to Basic UI, I assume you will encounter it in MainUI too. This is certainly something to discuss in your new hue PR.

[andrewfg]

@lolodomo many thanks for the feedback.

For the Hue CLIP 2 implementation I am inclined to support TWO channels - namely Color and Dimmer. The reason is that one could then combine the dimmer aspects of multiple colour, ambient, and monochrome lights into a single OH group Item so that all lamps in the group can be dimmed with one command. One would then (also) have a Color control for the subset of lights with colour support for changing the hue.

Anyway I will play around with it on my operative system for a while, and let you know the outcome.

Note: I will probably also have to push a fix for this Basic UI issue, since my operative system is based on BasicUI sitemaps...

[andrewfg]

^ I looked at the UI code, and I think the web stuff is way beyond my capabilities. @lolodomo you mentioned that your suspicion is due to the conversion RGB <=> HSB presumably in a java file (??); so as I can definitely do java, could you please point me towards what files I should look at? And I will do the rest..

[lolodomo]

No, it is in the JavaScript file.

[andrewfg]

in the JavaScript file.

What module is it in?

[lolodomo]

What module is it in?

There is only one: https://github.com/openhab/openhab-webui/blob/main/bundles/org.openhab.ui.basic/web-src/smarthome.js

In case the color widget used by MainUI works well, one idea would be to switch to it in Basic UI.

[andrewfg]

@lolodomo many thanks for the link. I made the following findings..

1. I tested the RGB <=> HSB conversion code in the JavaScript and it produces the same results as the equivalent code in our 'HSBType' java class (allowing for integer rounding effects). So I think that eliminates your suspicion about the cause of the bug.
2. I ran the JavaScript in the Chrome browser debugger and I can see that the problem occurs in the 'onSliderFinish()' function in the java script, and specifically the call in line 1227 (link below) to 'debounceProxy().finish()'. The code comments say "some browsers fire onchange while the slider handle is being moved. This is incorrect, according to the specs, but it's impossible to detect, so DebounceProxy is used"

https://github.com/openhab/openhab-webui/blob/587994b7ff52eac2d351a3bccd2aa0a42257f833/bundles/org.openhab.ui.basic/web-src/smarthome.js#L1227

Potential Bug Fix: perhaps modern browsers are no longer among the aforementioned 'some browsers' (??) so one potential fix for the OH bug could simply be to remove the lines marked REMOVE below that reference the 'debounceProxy' ..

=> WDYT ??

        // Some browsers fire onchange while the slider handle is being moved.
        // This is incorrect, according to the specs, but it's impossible to detect,
        // so DebounceProxy is used
        function onSliderChange() {
            _t.hsvValue.v = _t.slider.value / 100;
            _t.debounceProxy.call(); <== REMOVE
        }

        function onSliderFinish() {
            _t.hsvValue.v = _t.slider.value / 100;

            _t.debounceProxy.call(); <== REMOVE
            _t.debounceProxy.finish(); <== REMOVE
        }

[andrewfg]

^ @lolodomo I did some deeper debugging in Chrome, and actually I think my hypothesis above is wrong; and yours was/is indeed correct. My new hypothesis is that some parts of the HSB / RGB code takes and produces floats, whereas other parts are based on short integer. And the java script was never able to handle that..

[lolodomo]

Maybe the color widget used in MainUI is not doing any conversion to/from RGB ?

[ghys]

It doesn't. The color picker in Framework7 accepts HSB values (among others) in several formats and performs conversions automatically. https://v5.framework7.io/docs/color-picker#color-picker-value

It could be worthwhile to study how that works (it probably uses its own util functions): https://v5.framework7.io/docs/utils#colorhextorgb

[andrewfg]

Okay .. after about one day of debugging .. I think I may have figured out the cause of the problem..

When the color picker control refreshes itself, the JavaScript method extractValueFromLabel() gets called to extract the item's current state from the state presentation part of the item label. And that value is then passed to the setValue() method which assumes that the state presentation value is a comma separated HSB triplet like [11, 22, 33] and tries to create an HSB from that triplet. But the fact is that the item label does NOT contain an triplet (e.g. "Kitchen Bay Light Colour [66]"), which means that setValue() creates an invalid HSB. Normally the refresh code should then modify that HSB with the new brightness slider position value, and apply that new value to the color picker control state. However since the HSB is already invalid, it cannot be modified, and this forces the brightness slider to go to its default 50% position.

So I think in fact that the problem is not in the Basic UI JavaScript at all. But rather somewhere in the OH Core code relating to the formatting of the state presentation part of the item label for items of type color. => Can somebody please kindly check this for me?

[lolodomo]

The initial value is set there: https://github.com/openhab/openhab-webui/blob/main/bundles/org.openhab.ui.basic/src/main/java/org/openhab/ui/basic/internal/render/ColorpickerRenderer.java#L69

[andrewfg]

^ Hmm.

It seems that the bug only occurs if the item is created with a 'hard' state presentation part in its label..

1. label "Kitchen Bay Light Colour" - the bug does NOT occur
2. label "Kitchen Bay Light Colour [%s]" - the bug occurs

The reason is that this.updateWidget() (see below) calls extractValueFromLabel() which can return either null (case 1.) or some other (bad) value (case 2.) and that in turn causes widget.setValue() to use either the real proper state value or the other bad value, and in the latter case the bad value creates a bad HSB which makes the slider fail to its default 50% position.

this.updateWidget = function(widget, update) {
    var
        value = this.extractValueFromLabel(update.label), <= IF LABEL HAS A STATE PRESENTATION PART
..
    if (value === null) {
        value = update.state;
    }
    widget.setValue(smarthome.UI.escapeHtml(value), update.state, update.visibility); <= THEN SET_VALUE FAILS !!

https://github.com/openhab/openhab-webui/blob/f1db954f76743328ca790e8586b25747497d4cfe/bundles/org.openhab.ui.basic/web-src/smarthome.js#L1950

I am not sure how to fix this.

---

EDIT: but I think that at least the OH core should ensure that if the label is actually something like "Kitchen Bay Light Colour [%s]" then the %s should be properly populated with the item's actual HSB value e.g. [11,22,33] (or something like that) rather than merely a single number. Or ??

[lolodomo]

Did you see #1140 ?

What value is shown by the sitemap when you use %s? Does BasicUI even shows a value?

[andrewfg]

Did you see https://github.com/openhab/openhab-webui/issues/1140 ?

Not. Yet .. :)

What value is shown by the sitemap when you use %s ?

When I set the label with [%s] in the sitemap (code as follows) it displays as in the image underneath. It looks exactly the same when I set the label with [%s] in the Item definition.

Frame label="Kitchen" icon="light" {
    Colorpicker item=Kitchen_Bay_Light_Colour label="Aardvark [%s]"
    Text item=Kitchen_Bay_Light_Colour label="Alligator [%s]"
}

I would expect "hue,saturation,brightness".

Indeed that is what I expected. But as you can see the Colorpicker shows nothing; although the Text does show the brightness value. I think this could be because public class ColorItem extends DimmerItem .. but not very well ???

This would be, I think, a different issue.

I am not sure if it is a different issue, or in fact the ultimate cause of the original issue? It is certainly wrong that the state only shows "brightness" rather than "hue,saturation,brightness". And it is certainly true that if you create an HSB from "brightness" in the JavaScript, this creates a NaN HSB which causes the brightness slider to snap to 50%. And certainly removing the [%s] from the label eliminates, at least for me, the problem of the color picker snapping to 50%..

[lolodomo]

Looking at BasicUI code, that makes sense that no value is displayed, this is not a surprise at all. Nothing is done for that in the snippet.

[lolodomo]

Ah ok, you are using a text sitemap element to display a value.

[andrewfg]

^ I think the reason why %s returns "brightness" instead of "hue,saturation,brightness" is as follows..

• HSBType extends (via PercentType) from DecimalType
• the HSBType.brightness field is therefore in fact the root DecimalType.value field
• the HSBType.hue and HSBType.saturation fields are by contrast added locally in HSBType
• so when HSBType renders its state, it calls its parent DecimalType to render its state, which renders just the value (aka the brightness field)

[lolodomo]

I am afraid you are mixing up different unrelated things. For your wish to set a pattern for color, it may be because there is no format method in HSBType overriding the one in DecimalType? Edit: I think I can add this method with a test unit.

[andrewfg]

^ Sorry if I am mixing things. And I have no wishes. I have just been reporting my findings concerning this bug. My conclusion is that if the Item has %s in its label, the bug occurs. But if you remove the %s then the bug goes away (at least for me). That is all..

[lolodomo]

@Trickx : are you using pattern %s on your item or sitemap widget for your color item?

[lolodomo]

My conclusion is that if the Item has %s in its label, the bug occurs. But if you remove the %s then the bug goes away (at least for me). That is all..

Ok, we can add the support of pattern in label for color items. If it can help... But as I never used any pattern for colour, I can tell you that the colorpicker has a bug not related to that, see my other issue.

[lolodomo]

@andrewfg : can you provide a scenario to reproduce your bug without involving MQTT? Then I could reproduce it.

[andrewfg]

can you provide a scenario to reproduce your bug without involving MQTT?

I found it during testing of my PR for implementing CLIP 2 api support in the Hue binding. And therefore I am pretty sure that it is also applicable to the Hue CLIP 1 scenario. I am currently running the CLIP 2 on my operative system, but let me revert to CLIP 1 tomorrow, and I will post a test case for you based on that..

[lolodomo]

When opening the page, the color state is not taken from the label. Maybe the problem is when there is an update while the page is already opened? In that case, it could be either the content of the sitemap SSE event or more probably the bad handling of this event by the JavaScript (which must consider the state and not the state in the label - both are provided in the SSE event). I can check that in the JavaScript code.

[lolodomo]

or more probably the bad handling of this event by the JavaScript (which must consider the state and not the state in the label

Confirmed, I see that the JavaScript uses value parameter instead of itemState parameter (second parameter): https://github.com/openhab/openhab-webui/blob/main/bundles/org.openhab.ui.basic/web-src/smarthome.js#L1335 It makes no difference if you don't define a label pattern but of course it makes a difference if you define one. I should be able to test it and fix it.

But I am still not sure that is the original issue declared there.

[lolodomo]

Ok, I can reproduce your problem with a small change in demo items/sitemap. In fact, it happens even when you open the colorpicker.

sitemap demo label="Demo Sitemap" {
    Frame label="Demo Items" {
        Text item=DemoContact label="Contact [MAP(en.map):%s]"
        Text item=DemoString label="Message [%s]"
        Text item=DemoDateTime label="Date [%1$tA, %1$td.%1$tm.%1$tY]"
        Group item=DemoSwitchGroup icon=icon1
        Colorpicker item=DemoColor
        Colorpicker item=DemoColor label="demo color [%s]"
    }

First I use the sitemap element without label/pattern, open the color picker and sets a color. Then I reopen the color picker and the color is ok:

Then I open the second sitemap element, the one with label "demo color [%s]" and I got that:

[lolodomo]

The problem with label pattern is now fixed, see the PR.

Remains to enhance the pattern matching to display h,s,b rather than only the brightness. As previously mentioned, I have a good idea where it has to be enhanced (in core framework). I will try it.

[lolodomo]

@andrewfg : pattern support also added in core framework, look at the PR for screen captures.

[andrewfg]

@lolodomo many thanks for your efforts; I think these two changes have certainly improved the code.

still not sure that is the original issue declared there.

Yeah. Its hard to say. But I do suspect that parsing 'itemState', instead of 'value', may have a positive impact also in other use cases.

I don't know if it is relevant, or if it helps, but also during my testing yesterday I did notice some log entries like the following. However I cannot reproduce them today. So it may have just been some kind of Item cache artefact..

2022-11-18 18:24:39.158 [WARN ] [e.internal.SseItemStatesEventBuilder] - Attempting to send a state update of an item which doesn't exist: undefined
2022-11-18 18:26:03.413 [WARN ] [e.internal.SseItemStatesEventBuilder] - Attempting to send a state update of an item which doesn't exist: KitchenBayLight_Color
2022-11-18 18:27:27.956 [WARN ] [e.internal.SseItemStatesEventBuilder] - Attempting to send a state update of an item which doesn't exist: KitchenBayLight_Color

[lolodomo]

Yeah. Its hard to say. But I do suspect that parsing 'itemState', instead of 'value', may have a positive impact also in other use cases.

It will only if you have the strange idea to add a pattern to the label of the color item or colorpicker sitemap element. It should not be so common in practice.

But anyway, this is clearly better (proper) to use the itemState parameter in the JavaScript code.

[lolodomo]

What annoys me largely better is the bug in that color picker when you try to select a color around saturation 100% (limit of the circle). Look at my other issue. For me, there is clearly a bug in the RGB to HSB conversion inside the JavaScript code.

[andrewfg]

clearly a bug in the RGB to HSB conversion inside the JavaScript code.

I suppose the solution is, in the onMouseDown or onMouseUp methods, to do a mouse position hit test that it is within the inner circle, rather than the outer square. ?? There is probably some math trigonometry calculation needed. Which may be beyond my long dormant math skills..

---

EDIT: correct me if I am wrong, but the solution could be in the updateValue() method where the event.touches are evaluated? The current code looks like it is just checking for a touch within the square. But I imagine with some smart trigonometry, one could adjust the touch to be within the circle only. => WDYT?

https://github.com/openhab/openhab-webui/blob/f1db954f76743328ca790e8586b25747497d4cfe/bundles/org.openhab.ui.basic/web-src/smarthome.js#L1080

EDIT 2: actually it looks like the code in lines 1091..1103 is indeed doing a hit test. So maybe the problem is that this hit test is not enthusiastic enough to eliminate the edge cases on the circle (or perhaps over enthusiastic). The trigonometry is being done in integers and screen pixel resolution with possibly a quite wide mouse hotspot, so it may just be rounding errors.

=> I will do some debugging in Chrome to see if I can learn more...

[lolodomo]

EDIT 2: actually it looks like the code in lines 1091..1103 is indeed doing a hit test. So maybe the problem is that this hit test is not enthusiastic enough to eliminate the edge cases on the circle (or perhaps over enthusiastic). The trigonometry is being done in integers and screen pixel resolution with possibly a quite wide mouse hotspot, so it may just be rounding errors.

Yes, it looks like the mouse position is converted in HSB in a not perfect way, at least for the saturation.

I don't think it is the initial HSB to RGB conversion as the cursor is at the right place before you change the position with the mouse.

Please switch to the other issue #1140, I have added information.

=> I will do some debugging in Chrome to see if I can learn more...

Thank you if you can debug that in Chrome. I was asking myself if it could depend on the WEB browser, I am using Firefox.

[andrewfg]

@lolodomo as mentioned on the forum (post below) it seems that your three PRs have also solved this problem reported here. Many thanks. So I think you can close this Issue as well.

https://community.openhab.org/t/hsb-colorpicker-sets-50-brightness-when-color-is-picked/96698/29?u=andrewfg

[lolodomo]

as mentioned on the forum (post below) it seems that your three PRs have also solved this problem reported here.

Ok so I close the issue. @Trickx : feel free to reopen it if your problem is not solved in coming version OH 3.4M6.