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

Rollershutter slider has incorrect value #4358

Open
mueller-ma opened this issue Aug 13, 2024 · 9 comments
Open

Rollershutter slider has incorrect value #4358

mueller-ma opened this issue Aug 13, 2024 · 9 comments
Labels
bug An unexpected problem or unintended behavior of the Core

Comments

@mueller-ma
Copy link
Member

Actual behaviour

I have a group Item with the following config:
grafik

And a Sitemap widget configured as Switch item=MyGroup

When the average is let's say 50, I expect the slider in the bottom sheet to be at 50, but it's either at 0 (when the actual value is 0) or 100 otherwise.

Here's what gets passed to Widget.updateFromEvent():
rollershutter
The state for the widget is ON, which translates to the numeric value 100. But the item state is 3.00000, which is correct.

Is there any metadata I need to set? Or should the client make a special handling for Rollershutter group switches?

Expected behaviour

The slider has the correct value.

@mueller-ma mueller-ma added the bug An unexpected problem or unintended behavior of the Core label Aug 13, 2024
@maniac103
Copy link
Contributor

I'd say that the actual issue is not within the client, but the widget state being ON. Why that is I'm not sure though...

@mueller-ma
Copy link
Member Author

Because it's a switch, I guess.

@maniac103
Copy link
Contributor

Good point. Any reason for not making this a Rollershutter widget?

@mueller-ma
Copy link
Member Author

There's no explicit Rollershutter widget: https://www.openhab.org/docs/ui/sitemaps.html#element-types

The client handles Switch with Rollershutter items different: https://github.com/openhab/openhab-android/blob/main/mobile/src/main/java/org/openhab/habdroid/ui/WidgetAdapter.kt#L376

@maniac103
Copy link
Contributor

Oh, I completely forgot about that :-/ Anyways, the reason why widget state is preferred over item state is outlined in the commit that introduced it: openhab/openhab-android@68cff1f ... AFAICT, that rationale also applies to roller shutters, doesn't it?

@mueller-ma
Copy link
Member Author

In this special case I'd prefer the Item state over the Widget state. As the Widget state is only ON/OFF, there cannot be any UoM or number formatting applied.

@maniac103
Copy link
Contributor

I understand and agree for this specific special case, but the question is: how do you intend to separate this special case from all others properly?

@maniac103
Copy link
Contributor

maniac103 commented Aug 15, 2024

Thinking about it more, I think this needs to be fixed in core, since otherwise there'll be inconsistencies between the various clients (and special case code tends to become hard to maintain).
It's probably as easy as changing the check here to also cover groups of type Rollershutter. Something like this should work, I think (untested):

diff --git a/bundles/org.openhab.core.ui/src/main/java/org/openhab/core/ui/internal/items/ItemUIRegistryImpl.java b/bundles/org.openhab.core.ui/src/main/java/org/openhab/core/ui/internal/items/ItemUIRegistryImpl.jav+a
index f5eccaf2b..cb4764d8c 100644
--- a/bundles/org.openhab.core.ui/src/main/java/org/openhab/core/ui/internal/items/ItemUIRegistryImpl.java
+++ b/bundles/org.openhab.core.ui/src/main/java/org/openhab/core/ui/internal/items/ItemUIRegistryImpl.java
@@ -733,19 +733,22 @@ public class ItemUIRegistryImpl implements ItemUIRegistry {
             itemState = convertStateToWidgetUnit(quantityTypeState, w);
         }
 
-        if (w instanceof Switch && i instanceof RollershutterItem) {
-            // RollerShutter are represented as Switch in a Sitemap but need a PercentType state
-            returnState = itemState.as(PercentType.class);
-        } else if (w instanceof Slider) {
+        if (w instanceof Slider) {
             if (i.getAcceptedDataTypes().contains(PercentType.class)) {
                 returnState = itemState.as(PercentType.class);
             } else if (!(itemState instanceof QuantityType<?>)) {
                 returnState = itemState.as(DecimalType.class);
             }
         } else if (w instanceof Switch sw) {
-            StateDescription stateDescr = i.getStateDescription();
-            if (sw.getMappings().isEmpty() && (stateDescr == null || stateDescr.getOptions().isEmpty())) {
-                returnState = itemState.as(OnOffType.class);
+            if (i instanceof RollershutterItem
+                    || (i instanceof GroupItem gi && gi.getBaseItem() instanceof RollershutterItem)) {
+                // RollerShutter are represented as Switch in a Sitemap but need a PercentType state
+                returnState = itemState.as(PercentType.class);
+            } else {
+                StateDescription stateDescr = i.getStateDescription();
+                if (sw.getMappings().isEmpty() && (stateDescr == null || stateDescr.getOptions().isEmpty())) {
+                    returnState = itemState.as(OnOffType.class);
+                }
             }
         }

@mueller-ma
Copy link
Member Author

I also think it should be fixed at core. @openhab/core-maintainers can you move this issue to the core repo?

@kaikreuzer kaikreuzer transferred this issue from openhab/openhab-android Aug 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An unexpected problem or unintended behavior of the Core
Projects
None yet
Development

No branches or pull requests

2 participants