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: Render "Default" widgets and inside groups as text for read-only items #4559

Open
6 tasks
mueller-ma opened this issue Jan 15, 2025 · 12 comments
Open
6 tasks
Labels

Comments

@mueller-ma
Copy link
Member

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

I have some Switch items that are linked to a Thing channel and inform about some state. Switching this Item on or off is a no-op, e.g. "is the mop attached to the vacuum cleaner robot?" Switching it on doesn't attach the mop. For this reason bindings or myself add the metadata "readonly=true" to the Item.
When added to a Sitemap with a Default widget, a switch widget is shown in Basic UI and Android app. I haven't checked iOS. While this could be fixed by myself using Text instead, I don't have influence on the widget type when adding a group (Group item=GroupContainingTheReadOnlySwitch).

Describe the solution you'd like

In both cases render read-only Items as Text.

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
@mherwege
Copy link
Contributor

Why using a Switch Item type in the first place for this? You could use a String Item type, or a Contact Item type. I would argue Contact should be the right modelling, but I know that is also down to how the bindings implemented it. And there is no consistency there.

@maniac103
Copy link
Contributor

I would argue Contact should be the right modelling,

As I am the one who wrote the binding providing that info (I guess at least): I'm not sure about that statement. The problem with contact is that it maps to open/closed, and that kind of mapping doesn't fit well to true/false states. Same argument applies for String - a binary mapping makes much more sense for a binary state than a string.

The idea to make the Default widget a Text one for read-only switches totally makes sense to me.

@rkoshak
Copy link

rkoshak commented Jan 16, 2025

The problem with contact is that it maps to open/closed, and that kind of mapping doesn't fit well to true/false states.

I'm not sure I follow the logic. Both Contact and Switch are binary Items. Ignoring NULL and UNDEF they can only have two states, OPEN/CLOSED for Contact and ON/OFF for Switch. The only difference between Contact and Switch is you cannot send commands to a Contact. Contacts are supposed to be used for binary sensors where it makes no sense to command them.

OPEN/CLOSED maps to true/false no better and no worse than ON/OFF does.

As far as I can remember the intent was always that Contact is to be used where you have a binary sensor. But also as far as I can remember, binding authors have ignored that and always used Switch even for binary sensors (I'm looking at you Network 1.x binding which continues to use Switches to this day and which continues to confuse users to this day).

But if Contact can't be used to represent binary sensors because of the names of the states are OPEN/CLOSED, then we may as well get rid of it as a separate Item type. With the introduction of read only on the state description metadata having a separate Item type like this is a little redundant. Personally I'd vote for that because having them as separate types is confusing, especially since Contact is so rarely used, even in places where it's intended to be used.

But @mueller-ma, in the meantime, you should be able to use a profile to convert the ON/OFF to OPEN/CLOSED and link the Channel to a Contact. That might get you by until this issue gets resolved. That is unless you've already got a profile on this link in which case you may need to use a switch profile to combine what you have now with a mapping between ON/OPEN and OFF/CLOSED.

@maniac103
Copy link
Contributor

OPEN/CLOSED maps to true/false no better and no worse than ON/OFF does.

Let's say we want to express the 'mopping plate is attached' state. Is 'is attached' then mapped to OPEN or CLOSED?
The answer to that seems less intuitive to me than for ON/OFF...

@mherwege
Copy link
Contributor

That argument works in both directions. If I have a door that can be controlled (open or closed), I need a switch which only has ON or OFF. And don’t tell me I shoud use a rollershutter. This door doesn’t have a stop function.
So I agree there is a lot of semantic confusion possible, so why not make both controllable (with a default readOnly for contact items to keep backward compatibility) and make the default widget depend on the readOnly stateDescription. That’s already the case for numbers.

@maniac103
Copy link
Contributor

I agree, having all 4 variations of Contact/Switch + read only yes/no makes sense.

@rkoshak
Copy link

rkoshak commented Jan 17, 2025

Is 'is attached' then mapped to OPEN or CLOSED?

Either one would work, but I'd probably choose CLOSED. The OPEN/CLOSED isn't referencing a door or window state, it's referencing whether the circuit of the sensor is open (i.e. current is not flowing across the pins) or closed (i.e. current is flowing across the pins). For example, A closed reed sensor is one where the magnet is close enough to pull the little meta ball so it touches both pads “closing” the circuit.

so why not make both controllable (with a default readOnly for contact items to keep backward compatibility) and make the default widget depend on the readOnly stateDescription. That’s already the case for numbers.

I'd vote for eliminating Contact entirely. Binding authors only use it rarely, and as a result it's never been used consistently going back to OH 1.6. OH 5 is a time for breaking changes so we can be bold. Making Contact commandable just increases the confusion to me and does not make things any simpler. To ease the pain, maybe add some deprecation warnings, or something. Maybe create a new Binary Item type that Switch and Contact can map to (e.g. a Switch Item definition would become a Binary that accepts commands and a Contact would map to a Binary with the read only option set to true. Then we can add some deprecation warnings and the like.

@lolodomo
Copy link
Contributor

lolodomo commented Jan 26, 2025

I am asking myself why not rather handle the read-only feature on UI side? We could probably show the usual switch but making change impossible or ignored.

@mueller-ma
Copy link
Member Author

I'd avoid having a regular switch that looks like it could be switched, but the action is ignore. On Android the switch can be disabled (see below), but I still think displaying it as text is preferable. Bindings could also provide a text mapping, from my example above "attached" or "not attached".

Image

@mueller-ma
Copy link
Member Author

Thinking about your suggestion I think there are two similar cases:

  1. User defines a widget for a read-only item: In this case the widget should be read-only, e.g. like the switch in my post before.
  2. User defines a Default widget for a read-only item: In this case the best representation would be text, I think.

I prepare a PR for the first case for Android.

@lolodomo
Copy link
Contributor

lolodomo commented Jan 27, 2025

In HTML, we could simply add attribute "disabled" to the input element. But I should check the final rendering: identical or not? If identical, that would be perfect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants