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] Add widget similar to a remote control #3441

Closed
6 tasks
mueller-ma opened this issue Mar 10, 2023 · 34 comments
Closed
6 tasks

[Sitemap] Add widget similar to a remote control #3441

mueller-ma opened this issue Mar 10, 2023 · 34 comments
Labels
enhancement An enhancement or new feature of the Core sitemap

Comments

@mueller-ma
Copy link
Member

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

I'd like to control my TV via Sitemap. For common functions, like opening an app, I can an extra widget (e.g. a Selection with a few pre-selected apps). But doing this for all functions is way to much work. Therefore I need buttons for up, down, left, right, ok and back.

Describe the solution you'd like

Add a new Sitemap widget (maybe called RemoteControl) that contains a dpad. While at it, other buttons can be added as well, but make the visibility of all buttons configurable.

Example: RemoteControl item=SomeStringItem showDpad=true showBack=true showHome=true showVolume=true showVoice=true will result in something similar to the Android TV app: https://www.xda-developers.com/google-home-d-pad-controls-android-tv/

Pressing a button will result a string to be send as command, e.g. DPAD_UP or VOLUME_MUTE. Maybe also add a special item type, like it's done for Player.

Describe alternatives you've considered

  • Use a webview with a dpad.
    • Disadvantage: Cannot be themed well and requires one html file per item.
  • Add a switch with mappings.
    • Disadvantage: The buttons are in one line, so "up" isn't above "ok" as it should be.

signal-2023-03-10-092739_003

Additional context

Coordination between maintainers

I'd implement this in the Android app and docs.

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

It is something relatively specific that will require a lot of work for a limited usage.

For a TV remote control for example, other basic controls would be expected, like changing channel.

@maniac103
Copy link
Contributor

It's also that there - unlike all other controls like player - isn't an item type which directly accepts the commands sent by the proposed control.

@wborn wborn added enhancement An enhancement or new feature of the Core sitemap labels Mar 11, 2023
@mueller-ma
Copy link
Member Author

It is something relatively specific that will require a lot of work for a limited usage.

I see. What about making it more generic by adding only the D-Pad?

@jimtng
Copy link
Contributor

jimtng commented Mar 19, 2023

It would also be great if we could specify additional buttons around it so as not to waste the vertical space. On my LG remote, I have Home and back buttons on the left that I use a lot. These can be made configurable (or hidden). To the right could be volume up/down?

@Cossey
Copy link
Contributor

Cossey commented Apr 18, 2023

I was thinking that this is one of the things missing from sitemaps, for the exact same reason (one way settable state like sending remote commands or maybe a traditional style numpad).

However, could we consider something a little more generic like a "button grid" where you would have a grid of buttons. Then you could specify the icon or text for each button in a mappings property. If the mapping for a button number does not exist/empty, then that particular button in the grid is "hidden" (like the visibility:hidden css instead of the display:none). When pressing a button, it should set the value of the item to the button index, which then you can process via a script.

This should help make it more generic. Because I could see someone making a keypad or a more detailed remote set of buttons if they wanted to. We should let the user choose.

Maybe if the label is empty, it is hidden from the UI so that only the buttongrid is shown.

syntax idea for a directional pad like above

ButtonGrid item=TVRemote_Control label="TV" mappings=[2="icon_up", 4="icon_left", 5="icon_enter", 6="icon_right", 8="icon_down"] size=[3, 3]

@mueller-ma
Copy link
Member Author

In general I like the idea of @Cossey. It gives a good amount of flexibility to the user. However I'd change two things:

When pressing a button, it should set the value of the item to the button index, which then you can process via a script.

I'd rather convert the button index to the value on the client, so the rule/binding is independent of the representation.

mappings=[2="icon_up [...]]

Using icons here is probably a good idea. Maybe we could use icons as label for mapping in a regular switch as well.

I suggest a syntax like:

ButtonGrid item=TVRemote_Control label="TV" mappings=[2="UP"="icon_up", 4="LEFT="icon_left", ...] size=[3, 3] icons=true
Switch item=... label="..." mappings=["UP"="icon_up", "LEFT"="icon_left"]  icons=true

icons=true means that the value label is interpreted as icon name. 2="UP"="icon_up" means that the button on index 2 uses the icon_up icon and send "UP" when getting pressed.

@lolodomo
Copy link
Contributor

We can try to implement that in 4.1.
I will answer soon but I think we have to avoid a mismatch with existing mappings.
Not sure we really need a new element. Maybe the switch element with mappings is sufficient if we add a grid property to define number of buttons per line.
Once clarified, I will them implement the core part.

@Cossey
Copy link
Contributor

Cossey commented Jul 17, 2023

Sounds awesome. Agreed that it should fit within the existing mappings; but hopefully still offer a little flexibility. Problem with switch when you have two or more mappings it is stateful. With a single mapping it shows as a stateless button. Maybe this could be extended with:

  • An option for stateless buttons, so this effectively applies to normal switch mappings too
  • grid for buttons (cells) per line and how many lines
  • a way to denote a grid cell being blank and be considered a spacing cell

Even if this would be for buttons per a single line only, which still maintains backwards compatibility; if we have to use multiple switch lines for more rows that wouldn't be too bad and might be easier to keep consistent re layout.

However with that approach, it might be nice to be able to forgo the spacing on the left for an icon and label when they are both set to an empty string, i know that probably makes it more work.

@lolodomo
Copy link
Contributor

lolodomo commented Sep 16, 2023

I would like to check if there is still an interest amongst @openhab/android-maintainers and @openhab/ios-maintainers ?

My proposal is to add a new sitemap element (Buttongrid) but keep a common syntax to define mappings for Switch/Selection/Buttongrid.
In addition to the request for the new sitemap element, there was an additional request, that is to be able to use icons as mappings for buttons.
The current syntax is: mappings=[ value1=label1, "value 2"="label 2" ]
The proposed enhanced syntax would be: mappings=[ value1=label1, "value 2"="label 2", value3=label3=icon3, "value 4"="label 4"="icon4" ]
So the icon is optional and a label is required in any case. I think that is important in case a UI does not support icons or in case a UI does not support a certain icon source. The UI will be able to use the label in case it is not able to render the requested icon.
We also have to agree on the fact that the icon should be used only when the UI renders a button. For the Selection element for example, the labels will be used rather.

Then we come back to the new Buttongrid element.
Here is the syntax I propose:

Buttongrid item=item label="label" icon=icon mappings=[value1="Label 1"=icon, , , value4="label 4"=icon] columns=2

So the mappings define the buttons in the order left to right and top to bottom. The "cols" field defines how many buttons you should have on each line.
And finally we introduce another change for mappings, the way to define an empty mapping. In my example, you will have no button at top right and bottom left, so only one button at top left and one at bottom right.

WDYT ?

@mueller-ma
Copy link
Member Author

Sounds good to me. I suggest to implement this as two features:

  1. Allow an icon as third mapping and support them in Switch widgets.
  2. The Buttongrid.

@lolodomo
Copy link
Contributor

lolodomo commented Sep 17, 2023

Yes, that was also my intention to separate in two features. And I will provide a PR in the core framework for the first one very soon (it is easy).

Regarding the Buttongrid, we should also discuss a thing that is also applicable to the Switch element: that is the limit in the number of buttons per line. If I correctly remember, this is limited to 4 buttons in Basic UI with an automatic failback to a selection in case the mappings contains more than 4 items. Considering how I plan to implement it in Basic UI using the grid layout of Material Design Lite, I will be constrained by max 4 columns on a phone, 8 on a tablet and 12 on a desktop.
Will it be acceptable to define that the max is 4 columns (4 buttons on the same line) ?

@lolodomo
Copy link
Contributor

lolodomo commented Sep 17, 2023

Will it be acceptable to define that the max is 4 columns (4 buttons on the same line) ?

In basic UI, I could probably go further 4 by adding several buttons in the same cell but I will then have no control on the proper alignment.
By the way, what do do if the user requests 10 columns (10 buttons per line) while we know it is just impossible to render on a phone ?

lolodomo added a commit to lolodomo/openhab-core that referenced this issue Sep 23, 2023
When set by the user, the icon can be used by UIs for switch element with mappings to render a button with the icon rather than the label.

Related to openhab#3441

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
lolodomo added a commit to lolodomo/openhab-webui that referenced this issue Sep 23, 2023
@lolodomo
Copy link
Contributor

The first step (using icon instead of label for buttons) is now implemented in core framework and in Basic UI. Not yet merged.

lolodomo added a commit to lolodomo/openhab-webui that referenced this issue Sep 24, 2023
Related to openhab/openhab-core#3441

Depends on openhab/openhab-core#3809

Icons will not be used if icons are nbot enabled (app setting).

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
lolodomo added a commit to lolodomo/openhab-core that referenced this issue Sep 24, 2023
Related to openhab#3441

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
@lolodomo
Copy link
Contributor

I created a PR to introduce the new Buttongrid element. Remains the problem to offer the ability to define an empty cell. I am not sure the way I rpoposed is doable.

@lolodomo
Copy link
Contributor

lolodomo commented Sep 24, 2023

@mueller-ma @maniac103 : I am asking myself a lot of questions about how we will render the buttongrid element in Basic UI and Android app.
My questions:

  1. Do we consider a limit in the number of columns ?
  2. What width to use for the buttons ? Max width to fill the space (width of the screen/window) ? Min width adjusted to its content (text or icon) ? Fixed width for each button ?
  3. Spacing between buttons
  4. Should we consider vertical alignment for the buttons ?

To summarize, how do you imagine the rendering ?

@lolodomo
Copy link
Contributor

lolodomo commented Sep 24, 2023

I created a PR to introduce the new Buttongrid element. Remains the problem to offer the ability to define an empty cell. I am not sure the way I rpoposed is doable.

Maybe we should simply come back to @Cossey original idea to add a button index in the mappings. We could then name the field "buttons" rather than "mappings".
Either

buttons=[1=value1="Label 1"=icon, 4=value4="label 4"=icon]

or even better

buttons=[1:value1="Label 1"=icon, 4:value4="label 4"=icon]

And maybe there is a control to add to check that the index is >=1 but has a max. Imagine a user enters 123456789 as index, this would create a BIG grid :)

@mueller-ma
Copy link
Member Author

Do we consider a limit in the number of columns ?

Not sure. Maybe we add a small limit (e.g. 5) now and see if it causes issues for users.

What width to use for the buttons ? Max width to fill the space (width of the screen/window) ? Min width adjusted to its content (text or icon) ? Fixed width for each button ?

I'd make them fixed width: screen width / number of cols

Spacing between buttons

I'd keep it similar to the current spacing between the buttons of a Roller Shutter item (up, down, stop).

@lolodomo
Copy link
Contributor

I'd make them fixed width: screen width / number of cols

So that means potentially very large buttons (horizontally) if the user requests only one or two columns for example.
The fixed length will help to get a proper vertical alignment.
I am fine with that, I can have a similar approach.

lolodomo added a commit to lolodomo/openhab-core that referenced this issue Sep 24, 2023
Related to openhab#3441

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
@lolodomo
Copy link
Contributor

PR for Buttongrid is now finished and ready for review.

kaikreuzer pushed a commit that referenced this issue Oct 1, 2023
Related to #3441

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
@kaikreuzer
Copy link
Member

@lolodomo Shall we hence close this issue here?

kaikreuzer pushed a commit to openhab/openhab-webui that referenced this issue Oct 3, 2023
Related to openhab/openhab-core#3441

Depends on openhab/openhab-core#3809

Icons will not be used if icons are not enabled (app setting).

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
@lolodomo
Copy link
Contributor

lolodomo commented Oct 3, 2023

@lolodomo Shall we hence close this issue here?

Yes probably and then create a change request in the repo of each UI. It was done in android repo already. One should be created in webui repo for Buttongrid in BasicUI.

@mueller-ma
Copy link
Member Author

One should be created in webui repo for Buttongrid in BasicUI.

and one for expanding the sitemap editor in Main UI.

lolodomo added a commit to lolodomo/openhab-webui that referenced this issue Dec 2, 2023
Related to openhab/openhab-core#3441

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
lolodomo added a commit to lolodomo/openhab-webui that referenced this issue Dec 2, 2023
Related to openhab/openhab-core#3441

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
lolodomo added a commit to lolodomo/openhab-webui that referenced this issue Dec 2, 2023
Related to openhab/openhab-core#3441

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
lolodomo added a commit to lolodomo/openhab-webui that referenced this issue Dec 2, 2023
Related to openhab/openhab-core#3441

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
lolodomo added a commit to lolodomo/openhab-webui that referenced this issue Dec 2, 2023
Related to openhab/openhab-core#3441

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
lolodomo added a commit to lolodomo/openhab-webui that referenced this issue Dec 3, 2023
Related to openhab/openhab-core#3441

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
lolodomo added a commit to lolodomo/openhab-webui that referenced this issue Dec 3, 2023
Related to openhab/openhab-core#3441

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
@lolodomo
Copy link
Contributor

lolodomo commented Dec 3, 2023

Playing with the Buttongrid widget, I realized that the solution with the index for each button + the columns parameter is really not ideal for example when you decide to add a new column to your grid. In that case, you have almost to update all the indexes.

Finally, I think we should replace the current index for each button by a couple line / column. In that case, we even do not need anymore the columns parameter.

So for a grid 2x2 with one button at top left and one button a bottom right, instead of:

buttons=[1:value1="Label 1"=icon, 4:value4="label 4"=icon] columns=2

I propose:

buttons=[1,1:value1="Label 1"=icon, 2,2:value4="label 4"=icon]

@mueller-ma : WDYT ?
We have to decide very quickly (before OH 4.1 is released). I can certainly change the sitemap syntax very easily and quickly.

@maniac103
Copy link
Contributor

@lolodomo Sounds good to me (at least). Some remarks:

  • Total grid size would be maximum of specified coordinates?
  • Do you want to require specifying coordinates for every button?
  • What about the REST response? Kill off columns from Widget response and replace position by coordinate (x/y pair) in Mapping response?

@lolodomo
Copy link
Contributor

lolodomo commented Dec 4, 2023

* Total grid size would be maximum of specified coordinates?

Yes

* Do you want to require specifying coordinates for every button?

No change here, yes.

* What about the REST response? Kill off `columns` from `Widget` response and replace `position` by `coordinate` (x/y pair) in `Mapping` response?

Kill off columns from Widget response and replace position by line + column in Mapping response.

I will try to submit the change this evening.

@mherwege
Copy link
Contributor

mherwege commented Dec 4, 2023

buttons=[1,1:value1="Label 1"=icon, 2,2:value4="label 4"=icon]

@lolodomo Aren't you running into problems using comma both for separating the position in the grid and the buttons?

@lolodomo
Copy link
Contributor

lolodomo commented Dec 4, 2023

@lolodomo Aren't you running into problems using comma both for separating the position in the grid and the buttons?

In fact, it works but I agree with you that it is not ideal. I am going to replace the comma by a point : line.column:

@lolodomo
Copy link
Contributor

lolodomo commented Dec 4, 2023

Arghhh ... does not work if I use "." or "_" or "x" as separator !

Works with 1:3:TEST="Test"

@maniac103
Copy link
Contributor

Alternatives: 1x3 or 1|3 ... the former could probably be read as a size, though.

@mherwege
Copy link
Contributor

mherwege commented Dec 4, 2023

On the sitemap builder, I have commas working, but I doubt it is ideal.
The parsers (both in the UI nearly and xtext) interpret the position as numbers, and these can have dots. Changing that in the parser creates risk of side effects (it tries to identify numbers first).
I can also work with :.
Please confirm what you implement, so I can adapt to that.

@lolodomo
Copy link
Contributor

lolodomo commented Dec 4, 2023

I tried 1x3 but my syntax failed. 1 x 3 works. I don't understand. The interpret tries to interpret 1x3 ad a INT !
But as 1:3 is working, I propose to go with that.

lolodomo added a commit to lolodomo/openhab-webui that referenced this issue Dec 4, 2023
Related to openhab/openhab-core#3441

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
lolodomo added a commit to lolodomo/openhab-webui that referenced this issue Dec 6, 2023
Related to openhab/openhab-core#3441

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
lolodomo added a commit to lolodomo/openhab-webui that referenced this issue Dec 6, 2023
Related to openhab/openhab-core#3441

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
lolodomo added a commit to lolodomo/openhab-webui that referenced this issue Dec 6, 2023
Support grids containing from 1 to 12 columns.
Only the first 8 columns are rendered on tablet.
Only the first 4 columns are rendered on phone.

Related to openhab/openhab-core#3441

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
lolodomo added a commit to lolodomo/openhab-webui that referenced this issue Dec 6, 2023
Support grids containing from 1 to 12 columns.
Only the first 8 columns are rendered on tablet.
Only the first 4 columns are rendered on phone.

Related to openhab/openhab-core#3441

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
kaikreuzer pushed a commit to openhab/openhab-webui that referenced this issue Dec 12, 2023
Support grids containing from 1 to 12 columns.
Only the first 8 columns are rendered on tablet.
Only the first 4 columns are rendered on phone.

Related to openhab/openhab-core#3441

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
@lolodomo
Copy link
Contributor

The new Butotngrid element is now ready (core, Basic UI, sitemap generator in Main UI, documentation).
Documentation PR is submitted but not yet merged.

@openhab/android-maintainers
@openhab/ios-maintainers
Remains only to add support in the Android and iOS apps.

@kaikreuzer : maybe you can close this issue ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature of the Core sitemap
Projects
None yet
Development

No branches or pull requests

9 participants