Possible Classic UI Dimmer / Slider Bug

[mgbowman]

After updating to 1.7.0, any Slider attached to any Dimmer does not receive the INCREASE / DECREASE commands from the browser.

I've tested with both Safari 8.0.4 (10600.4.10.7) and Chrome 41.0.2272.76 (64-bit).

It appears the XHR http://openhab.local:8080/CMD is not getting triggered from the browser.

This issue does not appear for me in 1.6.1

[teichsta]

@sja could you have a look please?

[mgbowman]

I've dug a little deeper and it appears the issue was introduced when @sja refactored repeatedRequest in https://github.com/openhab/openhab/commit/386d44737b3633b3d123e388d488ba77f06b57cd

-           function requestWithTimer(request, frequency) {
-               if(repeat_on) {
-                   looping=1;
-                   ChangeState(request);
-                   var fctCall = "requestWithTimer('" + request + "'," + frequency + ")";
-                   setTimeout(fctCall, 300);
-               }
-           }
-           
-           function repeatedRequest(request, frequency, switchFlag) {
-               switchSupport = switchFlag;
-               repeat_on=1;
-               if(switchSupport) {
-                   var fctCall = "requestWithTimer('" + request + "'," + frequency + ")";
-                   setTimeout(fctCall, frequency);
-               } else {
-                   requestWithTimer(request, frequency);
-               }
-           }
+      OH.repeatedRequest = function repeatedRequest(request, frequency, switchFlag) {
+        var state = OH.dimmer;
+        state.switchSupport = !!switchFlag;
+        state.resetTimer();
+        var changeStateFn = function () {
+          state.alreadyChangedState = true;
+          OH.changeState(request);
+        };
+        state.repeatInterval = setInterval(changeStateFn, frequency);
+      };
+
+      OH.stopRepeatedRequest = function stopRepeatedRequest(request) {
+        var state = OH.dimmer;
+        state.resetTimer();
+        if (state.switchSupport && !state.alreadyChangedState) {
+          // Switch completely ON or OFF because user lift finger/mouse before timeout.
+          OH.changeState(request);
+        }
+        state.alreadyChangedState = false;
+      };

In the old code, there was a call to ChangeState(request) [now OH.changeState(request)] before the timeout, which is missing in the refactored code.

This was accounted for by checking if !state.alreadyChangedState in stopRepeatedRequest but this only happens for Sliders that have switchSupport

I believe the simple fix for this would be to change if (state.switchSupport && !state.alreadyChangedState) to if (!state.alreadyChangedState)

I'm not sure if this would affect any Sliders that have switchSupport enabled but I don't think so as the OH.changeState(request) would still fire as the state would not have changed by the time the user lifts their finger/mouse before the timeout.

[Testato]

Same issue here, If you use Android OH App the slider work. After used the App the browser classicUI also work

[mgbowman]

Just a quick follow-up, if you click and hold the mouse in the browser (for at least 200ms), the event will fire as normal. It's just when you click and release within 200ms the event doesn't fire.

[sja]

I'm not sure how the UI should behave.

All useful combinations are in demo.sitemap, which is my testbed:

Frame label="Percent-based Widgets" {
    Slider item=DimmedLight switchSupport
    Colorpicker item=RGBLight icon="slider"
    ...
    Slider item=DemoBlinds
}

First: All the above controls are handled by repeatedRequest/stopRepeatedRequest.

1. The DimmedLight should be send a ON/OFF, if I short press (<200ms) the up/down btn.
2. The DimmedLight should be send an INCREASE/DECREASE, if long press (>200ms) the up/down btn. This repeats until pressing the button stops.
3. The DemoBlinds (switchSupport = false) should ... do what? Fo me this is a Rollershutter control. OpenHAB renders this as INCREASE/DECREASE for touchstart/mousedown and ON/OFF for touchend/mouseup. What does that mean for a blind?

Behaviour 1 is implemented by delaying the request for 200ms. If before that time a touchend/mouseup event occurs, the flag alreadyChangedState is false, no request was sent and the request from touchend/mouseup is sent.

Behaviour 2 occurs 200ms after touchstart/mousedown are elapsed. Then the first request is made, alreadyChangedState is set to true and an interval is set to repeat that request every 300ms (or what value frequency has). If then a touchend/mouseup event occurs, nothing happens because alreadyChangedState was set to true.

My question: What is the intended actin or behaviour for case 3?

[kaikreuzer]

For 2: It should repeatedly (!) send INCREASE/DECREASE every 300ms as long as the button is pressed.

For 3: Blinds should not send ON/OFF/INCREASE/DECREASE at all, but UP/DOWN/STOP/MOVE. On press, it should send UP/DOWN If the button is pressed longer than 200/300ms, it should send a STOP upon release.

[sja]

For 2: Yes, I meant that. I corrected the text. For 1: What should MOVE do? I can imagine UP, DOWN and STOP.

[sja]

@mgbowman You wrote "After updating to 1.7.0, any Slider attached to any Dimmer does not receive the INCREASE / DECREASE commands from the browser."

So for me, a Dimmer in ClassicUI should only send INCREASE/DECREASE in case of a long press. For ON/OFF you have to add switchSupport attribute.

[kaikreuzer]

For 1: What should MOVE do?

Not relevant for the UI. In general it is the opposite of STOP. So having stopped, a MOVE should simply make it move again in the direction it did before the STOP. I guess MOVE is the least used command in openHAB ;-)

[udo1toni]

Plese don't forget: for knx there is another way of relative dimming as well (most common today): Start Dimming with an INCREASE/DECREASE (not repeated!) and stop Dimming with STOP. See Details in https://github.com/openhab/openhab/issues/635

[kaikreuzer]

Yes, but the specification about the UI must not be in any way specific to any binding. So #635 must be handled within the binding, not within the UI.

[mgbowman]

@sja I should have been more clear. I was used to repeatedly clicking the up / down arrows in my browser to change the volume on my AVR.

Reading your comments, what you did makes sense. Assuming switchSupport, send ON / OFF when it's < 200ms 'click' and repeatedly send INCREASE / DECREASE for > 200ms long press.

With that said, assuming no switchSupport, any user that clicks < 200ms, the UI does nothing. Maybe this will confuse users especially since in 1.6.1, the event was fired. It confused me which led to this issue.

[PelicanHu]

I've reported the same for MiLight slider: https://github.com/openhab/openhab/issues/2807

[00daniel00]

Hi, in the last version from openhab (October 10 2015) I tried to use the new slider in the ClassicUI on Chrome 45.0.2454.101 m, but every went crazy in the interface, the slider didn't respond and after some time other items disappear from the interface, in the browser console an error keep popping up "cannot set property 'innerhtml' of undefined" related to Logic.js:1850, so I commented that line in Logic.js and every worked fine. There is some problem there but I cant dig in more. Hope it help somehow.

[kaikreuzer]

Btw, @teichsta, is there a slider in the ClassicUI by now? This hasn't been ported to ESH yet, right? Is there an issue to track?

[sja]

@00daniel00 Can you show me the sitemap you're using this? On line 1743, it does not seem to find the correct value element. I haven't seen that on review, it seems not very fault tolerant. Maybe your nesting is wrong, but I'm faster able to fix this if I know your environment to reproduce it.

@kaikreuzer In my mind you participated on this decision, because was a new feature for OH1 and classicui. But I may be wrong on that.

[kaikreuzer]

@kaikreuzer In my mind you participated on this decision, because was a new feature for OH1 and classicui. But I may be wrong on that.

Yes, I remember that there was a discussion, but I cannot remember that there was a decision not to port it to ESH, correct? Therefore I am wondering, how and why it got lost...

[00daniel00]

@sja Sure, Im running openhab in a RPi1. The slider is used for a zwave dimmer. Here is the sitemap piece of that particular item:

    Group item=SP_Habitacion  {
        Switch item=Luz1_SP_Habitacion
        Switch item=Luz2_SP_Habitacion
        Slider item=Luz3_SP_Habitacion
    }

And the item definition:

Dimmer Luz3_SP_Habitacion "Dimmer"  <dimmer> (SP_Habitacion,gInterruptores,gApagar) { zwave="20:command=switch_multilevel"}

PD: If you need the full files I could mail them to you as is was not possible for me to attached them here, I dont have write permission to this repository.

[z00tv]

I noticed the same issue exists in the classic ui on the 1.8 snapshot. I quickly setup 5 dimmers, none of them work on safari or firefox.

[tankman316]

I'm having the same issue. I setup a brand new install of 1.7 and everything worked. I tried the latest 1.8 snapshot and none of the dimmers work. I can't make the slider do anything in the UI and I'm not sure how to check if anything is firing on the backend.

[kaikreuzer]

Btw, @teichsta, is there a slider in the ClassicUI by now? This hasn't been ported to ESH yet, right? Is there an issue to track?

As it still hasn't been ported to ESH and it does not seem to work in OH1, I would actually suggest to revert this change as time is two short for the 1.8 release.

[udo1toni]

Slider works in Classic UI (at least for me, just using the nightly). I have no Buttons but a Slider to set Dimming value (very nice and needs no increase/decrease command, because the value is set absolutely)

[sja]

@kaikreuzer I started the PR for the slider to ESH https://github.com/eclipse/smarthome/pull/778

[kaikreuzer]

Thanks! Does "started" mean that you will soon provide another follow-up PR?

[sja]

No, "started" means, started the process of review & merge. :-) Do you expect any other changes?

About this Issue: If we have a normal slider now, we won't have those timing problems anymore, because we always send absolute values, right?

[kaikreuzer]

Well, you are mentioning "so ClassicUI needs some more sync from OH1", so it seems there is work left to be done. Remember that the ClassicUI will disappear from this repo after the 1.8 release, so any code changes should by then be ported to ESH or they will be lost.

If we have a normal slider now, we won't have those timing problems anymore, because we always send absolute values, right?

You are right, the original issue only applies to the old button solution - if this is replaced, the problems cannot appear anymore. Btw, what is done for rollershutters? I actually preferred the buttons over a slider for them....

[sja]

Yes, there were some refactorings which will be lost. Won't be dramatic, but cool to keep that. So: If the time has come to remove Classic UI from here, don't see this here as blocker. I can access ti from git history if needed.

[kaikreuzer]

Well, I think the chances that nobody will remember to do so are pretty high. So we will end up with older / more buggy code in ESH, while throwing away the better version of it. And the time to remove the Classic UI is there already - the 1.8 release is out very soon so it is time to act here.

[z00tv]

As an outsider - classic UI is the best this project has - all other options except the apps are poor - sorry, UI development is not this projects strong suit.

On Dec 24, 2015, at 6:22 AM, Sebastian Janzen notifications@github.com wrote:

Yes, there were some refactorings which will be lost. Won't be dramatic, but cool to keep that. So: If the time has come to remove Classic UI from here, don't see this here as blocker. I can access ti from git history if needed.

— Reply to this email directly or view it on GitHub.

[sja]

Ok, the next free time will be spent to get some love here. It should be between 25. and 31.12.

Von meinem Telefon gesendet

Am 25.12.2015 um 02:04 schrieb z00tv notifications@github.com:

As an outsider - classic UI is the best this project has - all other options except the apps are poor - sorry, UI development is not this projects strong suit.

On Dec 24, 2015, at 6:22 AM, Sebastian Janzen notifications@github.com wrote:

Yes, there were some refactorings which will be lost. Won't be dramatic, but cool to keep that. So: If the time has come to remove Classic UI from here, don't see this here as blocker. I can access ti from git history if needed.

— Reply to this email directly or view it on GitHub.

— Reply to this email directly or view it on GitHub.

[kaikreuzer]

:+1:

[ldemeyer]

Since a week now I started learning OpenHAB. I ran into a problem that is related to this post I think. This issue is also affecting me when using the android app. I made a rollershutter item and on the webpage, I got the 2 up-down arrows (round ones) and that was fine but on the android app I got a slider. The up/down on the webpage sent out the UP and DOWN commands, BUT the app sent the % slider value. So this item became unusable. I think the app is now out of sync with the website version in this respect. I do apologise if I am wrong here because as I said I started just a week ago. It just made the learning curve a LOT steeper and I learned a lot in the proces.

[buzink]

For me the slider in the (iPhone) app does something, but it is not suited to change volume (increases unexpectedly to very loud upon touching the slider). The up/down arrows in Classic UI in Chrome, Firefox and Safari look more promising, but do nothing (also not if I keep the mouse button down longer than 200 ms). I use the slider as shown in an example on the Frontier Silicon Radio Binding:

item: Number RadioVolDimmer "Radio Volume [%d %%]" (gRadio) { frontiersiliconradio="radio:VOLUME" }

sitemap: Slider visibility=[RadioPower==ON] item=RadioVolDimmer

[watou]

Possibly related user report on the forum.

[watou]

Another possibly related user report (flagged by @robnielsen).

[robnielsen]

I replace Logic.min.js with Logic.js and here's the stack trace:

TypeError: Cannot set property 'innerHTML' of undefined at Slider.that.displayValue (https://192.168.1.2:8443/classicui/WebApp/Action/Logic.min.js:1850:27) at new Slider (https://192.168.1.2:8443/classicui/WebApp/Action/Logic.min.js:1854:8) at InitSlider (https://192.168.1.2:8443/classicui/WebApp/Action/Logic.min.js:1876:5) at InitForms (https://192.168.1.2:8443/classicui/WebApp/Action/Logic.min.js:1904:3) at ShowAsync (https://192.168.1.2:8443/classicui/WebApp/Action/Logic.min.js:1496:4) at https://192.168.1.2:8443/classicui/WebApp/Action/Logic.min.js:1386:12 at __callback (https://192.168.1.2:8443/classicui/WebApp/Action/Logic.min.js:1559:10) at XMLHttpRequest.c (https://192.168.1.2:8443/classicui/WebApp/Action/Logic.min.js:292:26)

It appears that valueEl is undefined in the function:

that.displayValue = function (value) {
    that.handle.style.left = value + '%';
    that.progress.style.width = value + '%';
     that.valueEl.innerHTML = value + ' ' + '%';
}

Which is defined earlier in the function Slider:

that.valueEl = that.opt.parentNode.querySelectorAll("span")[4];

[robnielsen]

Here's what that.opt.parentNode.querySelectorAll("span") contains:

NodeList[4] 0: span.options.optionsSlider 1: span.iSliderContainer 2: span.iSliderProgress 3: span.iSlider length: 4 proto: NodeList

When inspecting it with chrome's debugger. The array contains 4 elements, not 5.

[sja]

@robnielsen Thanks for your research. Yes, it should the iSlider, so index 3. Will fix that.

[teichsta]

fixed through #3796