openhab / openhab-webui

Web UIs of openHAB
Eclipse Public License 2.0
224 stars 242 forks source link

HabPannel Slider go to preview value #219

Open sifourquier opened 4 years ago

sifourquier commented 4 years ago

Hello I have a bug in opanhab 2.5.3-1 ine HabPannel Some time when i presse on a possition slider go to this possition but after he go back to preview position and send to serveur previous value not the new value. I have this bug on linux (Chrominum Firefox) and Android (Brave) I have do a video https://www.youtube.com/watch?v=JtR1WIm26do&feature=youtu.be first clic that work but after i presse 75 and he go back to 19 and we can watch in inspector he send 19 and same with 89 he go back to 19

The linked items are Number test "test" \<test> (DailyArchiving)

nahuellofeudo commented 4 years ago

I'm seeing the same issue in OpenHab 2.5.4-1 This same bug seems to have been reported multiple times in the past, here: https://community.openhab.org/t/slider-and-knob-issues/27233 and here: https://community.openhab.org/t/slider-returns-wrong-values/94295

For what it's worth, the issue of sliders bouncing back seems to be correlated with the following warning being logged in Chrome's Javascript console: [Violation] 'setTimeout' handler took ms Where is a number between 50 and around 100.

I can confirm that when the slider bounces back to the previous value, the UI only sends OpenHab the "old" value. For example, if my UI has a slider with values 1 and 2:

I click on 1 --> UI sends value 1 I click on 2 --> UI sends value 2 I click on 1 and UI bounces back to 2 --> UI sends value 2 again.

nahuellofeudo commented 4 years ago

There is a bug reported in the slider component's bug tracker that looks similar to this one: https://github.com/angular-slider/angularjs-slider/issues/346

ghys commented 4 years ago

That for investigating this issue @nahuellofeudo! It looks like HABPanel uses angularjs-slider v5.4.3 and the bug you've mentioned is claimed to be fixed in 5.8.0, I agree it looks related to this issue. The latest release is 7.0.0, it could be worthwhile to upgrade, it could take a while since we need to make sure it doesn't introduce any regressions, but in the other hand there are other problems with the slider that an upgrade might fix as well.

nahuellofeudo commented 4 years ago

I tried using Chrome's debugger to replace the slider code in vendor.js straight from the 5.8.0 release and I'm still seeing the problem. I'm not 100% sure I'm doing it right, though (setting a breakpoint on vendor.js line 556 and pasting the content of rzslider.min.js from [1] on top of the existing code).

[1] https://github.com/angular-slider/angularjs-slider/archive/5.8.0.zip

nahuellofeudo commented 4 years ago

This is weird: I changed slider.widget.js:updateValue() like this:

    function updateValue() {
        var value = getValue();

        $timeout(function () {
            vm.value = vm.slider.value = value;
            vm.ready = true;
            console.warn("Slider updateValue timeout: " + value);
        });
    }

(added a console.warn)

And I'm getting constant warnings about the two sliders I have in my panel. And I'm getting timeout warnings about BOTH sliders, even when I only touch one of them. The warnings keep going for a while after I touch my slider, and roughly 10 - 15 seconds later they stop. Here is a sample output:

Screenshot_20200809_024646

The slider I touched is the one that logs 19, and the one that logs 25 is a second slider that I didn't change since I loaded the panel.

nahuellofeudo commented 4 years ago

Quick update: the extra events seem to be caused by my mouse pointer just hovering around the panel. The timeouts appear even if I don't click on anything. Just moving the mouse around causes warnings to appear.

ghys commented 4 years ago

I'm not 100% sure I'm doing it right, though (setting a breakpoint on vendor.js line 556 and pasting the content of rzslider.min.js from [1] on top of the existing code).

I never did that, you might want to do something like this instead - the 2nd technique i.e. copy the code in conf/html and modify the file to make sure.

nahuellofeudo commented 4 years ago

The more I look into this issue, the more it looks like a race condition.

Some of the values I'm pushing to my dashboards change very frequently, like:

and for some reason, OpenHab insists on pushing to the UI items like number of Z-Wave frames, even though I'm not showing them anywhere. And when I set a breakpoint in my slider's Update() function, I see that the slider's update function (and consequently the timeount handler) is being called for events like "Memory available changed" or "CPU usage changed".

Screenshot: 2020-08-09_03-06

nahuellofeudo commented 4 years ago

Yup. The sliders' Update() function is being called for every single event that OpenHab pushes to the UI:

2020-08-09_03-19

ghys commented 4 years ago

This logic is due for an update in OH3 anyways, there's a new API to get state updates for a pre-configured set of items only (i.e. only those necessary for rendering), the new main UI already uses it and it's in the plans for HABPanel to use it as well. OH2 doesn't have this API so it's likely to change much.

That being said, it's not normal that you get updates when an unrelated item that isn't even shown is updated, that might be a bug (the item name should be part of the event and the slider should ignore it if it doesn't match its configured item), Thanks again for investigating anyways!

nahuellofeudo commented 4 years ago

It's great news that the logic of the UI will be improved in OH3! I can't wait to migrate my system to it. I looked at the other widgets' code and the Slider seems to be the only one that implements a timeout function, which could explain why it's the only one that shows this behavior.

For now, I'm going to change the code of slider.widget.js to remove the call to $timeout within updateValue() and I'll follow your instructions to deploy it across all my UIs to see what happens. I'll update this bug in a day or two with my results.

nahuellofeudo commented 4 years ago

Quick update on this issue: patching slider.widget.js to remove the call to $timeout definitely helped. My sliders don't jump back anymore. They still jump back and forth for a few milliseconds but they always stabilize on the value I choose. I'm going to try replacing the slider code with version 5.8.0 to see if it further improves things. @ghys would you like a PR for the change to UpdateValue()?

sifourquier commented 4 years ago

Thanks you for your help the try to fix bug. But can you say where are slider.widget.js I only found it in /var/lib/openhab2/tmp/mvn/org/openhab/ui/bundles/org.openhab.ui.habpanel/2.5.8/org.openhab.ui.habpanel-2.5.8.jar

but i thing is not the used file is only tmp file?

nahuellofeudo commented 4 years ago

@sifourquier that would be the file, yes.