Sitemap: Add parameters to control slider behavior

[mueller-ma]

Let's try the new issue template (#3427):

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

In openHAB two different behavior exist for sending updates:

1. Send the update when the mouse click/finger touch is released.
2. Send the update while the slider is still changed.

I tested these sliders:

• Basic UI: Option 1
• Config page of Dimmer items in Main UI: Option 2
• Android app: Recently changed from 1 to 2
• iOS app has a setting to set option 1 or 2 globally

Option 2 is good for things that can change quickly, e.g. brightness or volume, but not for rollershutters as they cannot move so fast. This option may also cause issues when too many updates are sent in a short time, e.g. here: https://github.com/openhab/openhab-android/issues/3241

Describe the solution you'd like

Add two new parameters to the Slider, Color and Setpoint widgets, similar to those in the oh-slider-card widget in Main UI:

value: ""
config:
  releaseOnly: false             # Send while sliding
  commandInterval: 300         # But only every 300 (ms?)

The Color widget may have a slider for brightness and the Setpoint widget has a bottom sheet with a slider on Android.

Describe alternatives you've considered

Additional context

https://community.openhab.org/t/define-slider-behavior/144714

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

[mueller-ma]

@openhab/core-maintainers Can you comment in this issue? The current behavior of the sliders in the Android app is causing issues for some users and I want to fix it the right way. IMO the right way are two new parameters in the sitemap syntax, similar to the slider parameters in Main UI.

[kaikreuzer]

Adding a releaseOnly optional parameter might indeed be the most flexible solution. If it is not given, I would say that option 2 should be the default. I wouldn't necessarily add commandInterval - in the past we used 300ms everywhere where this was needed and I think this is a suitable value - no real need to make this configurable (and change the code everywhere to take the configuration into account).

[lolodomo]

If default is option 2, the most impacted UI would then be Basic UI and this will require someone to implement the change. As a side note, the behaviour of the slider is not working well in Basic UI, sometimes the new value when you release the mouse is just not considered. Regarding the setpoint widget, having a slider was not really in the original way this widget was defined. Maybe a specific option has to be introduced for that widget to let the user choose between a slider and the up/down buttons ? I have to check how it now renders in Android app. But adding option(s) about sliding to the setpoint widget also means enhancing the setpoint widget in all UIs to add the (optional) slider. Or we assume that this will be used only by the Android app ?

[maniac103]

For setpoints (and roller shutters), it's only a detail view, the up/down buttons are the default. https://github.com/openhab/openhab-android/issues/3247 has a screenshot of it. For that reason, I'd keep those two items aside in the discussion here and imply releaseOnly for them in said detail view. There's also a third option: imply behavior from widget and/or item type, like I suggested in https://github.com/openhab/openhab-android/pull/3244 Maybe that kind of logic could also be used if the user didn't give a value. We'll need it in the Android app in any case when dealing with older servers.

[mueller-ma]

For that reason, I'd keep those two items aside in the discussion here and imply releaseOnly for them in said detail view.

That seems like a good compromise 👍

Regarding the setpoint widget, having a slider was not really in the original way this widget was defined.

The detail view of the Setpoint widget has been implemented in the Android app years ago and AFAIK users really like it. Maybe enhance Basic UI, but that's a bit off-topic here.

There's also a third option: imply behavior from widget and/or item type

As you said I think it's a good idea to use this as fallback.

[lolodomo]

I can easily enhance the sitemap syntax to add this new parameter. I suppose we also need to enhance the REST API response? I am just asking myself if the default should not be rather mode 1 (send the update when the mouse click/finger touch is released). Remember that for items in a group, when opening the group, you will get the default without any way to adjust it. So the default should be a behaviour that will work for all use cases. And you mentioned use cases for which mode 2 is inappropriate.

[mueller-ma]

Yes, the REST API needs to be enhanced as well. I don't have a strong opinion on the default setting.

[maniac103]

So the default should be a behaviour that will work for all use cases.

How about my option 3 mentioned above?

[lolodomo]

releaseOnly will just be a boolean. It will be set to true if "releaseOnly" is present in the element properties and to false if not. So the default would then be behaviour 2 and users should adjust their sitemaps if they want behaviour 1. For the default slider created automatically for groups or when using "Default" element (most critical case as users cannot customise it), I would suggest to keep behaviour 1 as it is OK in all situations while behaviour 2 is not. Is it ok for you ?

I am going to create the core PR enhancing the sitemap syntax and the REST API.

[lolodomo]

@maniac103 @mueller-ma : did you already start something in Android app for this new option ? I am going to start for Basic UI after fixing the current slider misbehaviour.

[mueller-ma]

No, I haven't started implementing it. Is this already supported by the Main UI Sitemap generator?

[lolodomo]

Is this already supported by the Main UI Sitemap generator?

I just searched and did not find any PR for that

[lolodomo]

@mherwege : any chance that you add this new attribute in the sitemap generator ?

[mherwege]

@mherwege : any chance that you add this new attribute in the sitemap generator ?

Yes, I will.

[mherwege]

I forgot about it, but I did already create the PR for it in the sitemap creator: https://github.com/openhab/openhab-webui/issues/2345. It has been merged a while already.

[lolodomo]

I wouldn't necessarily add commandInterval - in the past we used 300ms everywhere where this was needed and I think this is a suitable value

@kaikreuzer : in my proposed implementation for Basic UI, I reused the existing parameter sendFrequency as command interval and considered 200 ms as default value (the value which was there before). @mueller-ma : what was your choice in the Android app ? Did you consider this parameter or hardcode an interval ?

I will be in favour of using this parameter sendFrequency but in case we decide to not use it, I would suggest to remove it from the sitemap documentation.

[maniac103]

Did you consider this parameter or hardcode an interval ?

We hard coded it (to 200ms). I don't see a compelling reason for making this configurable - do you?

[lolodomo]

I don't see a compelling reason for making this configurable - do you?

Maybe some devices do not accept commands at such frequency?

But if you all think it is useless, I will also keep the hardcoded value of 200ms in Basic UI and remove this parameter from the syntax.

[kaikreuzer]

I think the 200ms is fine and I do not remember that anybody ever asked to have it configurable. So I'd go the simple way and simply leave it at this default and remove the parameter from the syntax.

[lolodomo]

I already removed the usage of this parameter from my PR.

[lolodomo]

Remains documentation to update.

[mueller-ma]

https://github.com/openhab/openhab-docs/pull/2287

[lolodomo]

@openhab/core-maintainers : I believe we can now close this feature request as implemented. Only support in iOS app is not yet done.