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

MultiLevelSensor gets confused with multiple LevelProperties with different units #1148

Closed
freaktechnik opened this issue Jun 28, 2018 · 15 comments
Milestone

Comments

@freaktechnik
Copy link
Contributor

If a MultiLevelSensor thing has multiple LevelProperty properties, it adapts the last updated value as its current main value. It does however not adapt the unit of the updated property, so the center of the thing will contain a number with the wrong unit.

@mrstegeman
Copy link
Contributor

Yeah, I've actually been trying to figure out what to do in cases like this. The same problem occurs on a smart plug with 2 outlets. A workable solution is to only set one of them as a LevelProperty, and just leave the @type off of the others. The MultiLevelSensor icon will at least know what to use then. The others will just be shown as generic number inputs.

@freaktechnik
Copy link
Contributor Author

Best case scenario the user can choose what property is displayed as primary value. I'm thinking of the weather station here, where some may want to quickly see the temperature, while other may want to see CO2 concentration or humidity etc.

@mrstegeman
Copy link
Contributor

As the capability system matures, we should start to gain things like TemperatureProperty, HumidityProperty, etc. When that happens, our selection of high-level icons will grow as well.

I want to keep this issue open, though, for cases where there will continue to be 2 of the exact same property type.

@benfrancis
Copy link
Member

Hmm, I didn't think of that.

For the time being can we get the UI to just pick one of the LevelPropertys, probably the first one, and only display that property's value changes in the main Things screen?

I agree in this use case it would be nice if the user could choose, but in the meantime we should at least not switch between different properties and display the wrong unit.

It would definitely be preferable that you don't have to leave off semantic annotations just to tell our UI which value to show.

@mrstegeman
Copy link
Contributor

@benfrancis The problem with picking the "first one" is that it's a JSON object, so "first" is indeterminate. We could try to sort the property names, but that is not necessarily what the developer intended either.

What @dhylands and I decided to do in the case of the dual-outlet smart plug was to set one to the OnOffProperty and the other as BooleanProperty. The 2 will have different property details, but at least the capability icon will be consistent.

@mrstegeman
Copy link
Contributor

The same thing also occurs when you have a smart plug with a light on it. It will need to have an OnOffProperty for both the SmartPlug and the Light, and if you include OnOffSwitch, things get even worse.

@rzr
Copy link
Contributor

rzr commented Aug 1, 2018

Unsure if related but I have observed UI issues when a thing contains 2 properties of same types (ie onoff switch)

@mrstegeman
Copy link
Contributor

Yes, that's the exact same issue.

@rzr
Copy link
Contributor

rzr commented Aug 1, 2018

I spent some time to figure out the problem, but I am not sure I got it yet (maybe using the same node ID in widget's template) but I can share a sample code to reproduce the issue, I'll ping you on IRC.

I've a possible workaround to share.

rzr added a commit to TizenTeam/gateway that referenced this issue Aug 2, 2018
Previously item ids were duplicated,
the observed consequence is that any click on widgets
is seen as click on 1st declared element.

For example, a thing with 2 switches is displayed correctly,
and click events are also triggered but will be "redirected"
to 1st switch only, this is not desired.

Change-Id: I27ef75a95889c37224aa7da6b0fbf8e2ad57ba65
Bug: WebThingsIO#1148
Signed-off-by: Philippe Coval <p.coval@samsung.com>
@rzr
Copy link
Contributor

rzr commented Aug 2, 2018

Feedback welcome I only made the fix for switch prop then this can be replicated to other components if solution is acceptable.

ps: Gz for @mrstegeman .fork() !

rzr added a commit to TizenTeam/gateway that referenced this issue Aug 2, 2018
Previously item ids were duplicated,
the observed consequence is that any click on widgets
is seen as click on 1st declared element.

For example, a thing with 2 switches is displayed correctly,
and click events are also triggered but will be "redirected"
to 1st switch only, this is not desired.

Change-Id: I27ef75a95889c37224aa7da6b0fbf8e2ad57ba65
Bug: WebThingsIO#1148
Origin: https://github.com/tizenteam/gateway
Signed-off-by: Philippe Coval <p.coval@samsung.com>
rzr added a commit to TizenTeam/gateway that referenced this issue Aug 2, 2018
Previously item ids were duplicated,
the observed consequence is that any click on widgets
is seen as click on 1st declared element.

For example, a thing with 2 switches is displayed correctly,
and click events are also triggered
but event will be "redirected" to 1st switch only,
this is not desired.

This change fix it for boolean related widget,
and once solution merged,
it can be easly replicated to others WoT types (level, text etc).

Change-Id: I27ef75a95889c37224aa7da6b0fbf8e2ad57ba65
Bug: WebThingsIO#1148
Forwarded: WebThingsIO#1249
Origin: https://github.com/tizenteam/gateway
Signed-off-by: Philippe Coval <p.coval@samsung.com>
rzr added a commit to TizenTeam/gateway that referenced this issue Aug 2, 2018
Previously item ids were duplicated,
the observed consequence is that any click on widgets
is seen as click on 1st declared element.

For example, a thing with 2 switches is displayed correctly,
and click events are also triggered
but event will be "redirected" to 1st switch only,
this is not desired.

This change fix it for boolean related widget,
and once solution merged,
it can be easly replicated to others WoT types (level, text etc).

Change-Id: I27ef75a95889c37224aa7da6b0fbf8e2ad57ba65
Bug: WebThingsIO#1148
Forwarded: WebThingsIO#1249
Origin: https://github.com/tizenteam/gateway
Signed-off-by: Philippe Coval <p.coval@samsung.com>
rzr added a commit to TizenTeam/gateway that referenced this issue Aug 2, 2018
Previously item ids were duplicated,
the observed consequence is that any click on widgets
is seen as click on 1st declared element.

For example, a thing with 2 switches is displayed correctly,
and click events are also triggered
but event will be "redirected" to 1st switch only,
this is not desired.

This change fix it for boolean related widget,
and once solution merged,
it can be easly replicated to others WoT types (level, text etc).

Change-Id: I27ef75a95889c37224aa7da6b0fbf8e2ad57ba65
Bug: WebThingsIO#1148
Forwarded: WebThingsIO#1249
Origin: https://github.com/tizenteam/gateway
Signed-off-by: Philippe Coval <p.coval@samsung.com>
rzr added a commit to TizenTeam/gateway that referenced this issue Aug 3, 2018
Previously item ids were duplicated,
the observed consequence is that any click on widgets
is seen as click on 1st declared element.

For example, a thing with 2 switches is displayed correctly,
and click events are also triggered
but event will be "redirected" to 1st switch only,
this is not desired.

It was tested for boolean, number related components.

Change-Id: I27ef75a95889c37224aa7da6b0fbf8e2ad57ba65
Bug: WebThingsIO#1148
Forwarded: WebThingsIO#1249
Origin: https://github.com/tizenteam/gateway
Signed-off-by: Philippe Coval <p.coval@samsung.com>
rzr added a commit to TizenTeam/gateway that referenced this issue Aug 3, 2018
Previously UI item's ids were duplicated,
the observed consequence is that any click on widgets
is catched by 1st declared element.

For example, a thing with 2 switches is displayed correctly,
and click events are also triggered
but event will be "redirected" to 1st switch only,
this is not desired.

It was tested for boolean, number related components.

Change-Id: I27ef75a95889c37224aa7da6b0fbf8e2ad57ba65
Bug: WebThingsIO#1148
Forwarded: WebThingsIO#1249
Origin: https://github.com/tizenteam/gateway
Signed-off-by: Philippe Coval <p.coval@samsung.com>
rzr added a commit to TizenTeam/gateway that referenced this issue Aug 3, 2018
Previously UI item's ids were duplicated,
the observed consequence is that any click on widgets
is catched by 1st declared element.

For example, a thing with 2 switches is displayed correctly,
and click events are also triggered
but event will be "redirected" to 1st switch only,
this is not desired.

It was tested for boolean, number related components.

Slider not fully updated, to keep passing tests,
more refactoring to come in that part (grep '#slide')

Change-Id: I27ef75a95889c37224aa7da6b0fbf8e2ad57ba65
Bug: WebThingsIO#1148
Forwarded: WebThingsIO#1249
Origin: https://github.com/tizenteam/gateway
Signed-off-by: Philippe Coval <p.coval@samsung.com>
rzr added a commit to TizenTeam/gateway that referenced this issue Aug 3, 2018
Previously UI item's ids were duplicated,
the observed consequence is that any click on widgets
is catched by 1st declared element.

For example, a thing with 2 switches is displayed correctly,
and click events are also triggered
but event will be "redirected" to 1st switch only,
this is not desired.

It was tested for boolean, number related components.

Slider not fully updated, to keep passing tests,
more refactoring to come in that part (grep '#slider')

Change-Id: I27ef75a95889c37224aa7da6b0fbf8e2ad57ba65
Bug: WebThingsIO#1148
Forwarded: WebThingsIO#1249
Origin: https://github.com/tizenteam/gateway
Signed-off-by: Philippe Coval <p.coval@samsung.com>
rzr added a commit to TizenTeam/gateway that referenced this issue Aug 4, 2018
Previously UI item's ids were duplicated,
the observed consequence is that any click on widgets
is catched by 1st declared element.

For example, a thing with 2 switches is displayed correctly,
and click events are also triggered
but event will be "redirected" to 1st switch only,
this is not desired.

It was tested for boolean, number related components.

Slider not fully updated, to keep passing tests,
more refactoring to come in that part (grep '#slider')

Change-Id: I27ef75a95889c37224aa7da6b0fbf8e2ad57ba65
Bug: WebThingsIO#1148
Forwarded: WebThingsIO#1249
Origin: https://github.com/tizenteam/gateway
Signed-off-by: Philippe Coval <p.coval@samsung.com>
rzr added a commit to TizenTeam/gateway that referenced this issue Aug 4, 2018
To avoid clash LevelProperty's id are renamed to slide-level-${count}

It was tested on ARTIK05x thing (2 switches, 2 booleans, 2 levels)

Change-Id: If530737168af14eb8f6f1e9e230e04a432490045
Bug: WebThingsIO#1148
Forwarded: WebThingsIO#1249
Origin: https://github.com/tizenteam/gateway
Signed-off-by: Philippe Coval <p.coval@samsung.com>
@rzr
Copy link
Contributor

rzr commented Aug 4, 2018

Can you please try linked patches from:
https://github.com/TizenTeam/gateway/tree/sandbox/rzr/devel/master

It worked for me on this "in progress" thing:

ARTIK05x

rzr added a commit to TizenTeam/gateway that referenced this issue Aug 10, 2018
Previously UI item's ids were duplicated,
the observed consequence is that any click on widgets
is catched by 1st declared element.

For example, a thing with 2 switches is displayed correctly,
and click events are also triggered
but event will be "redirected" to 1st switch only,
this is not desired.

It was tested for boolean, number related components.

Slider not fully updated, to keep passing tests,
more refactoring to come in that part (grep '#slider')

Change-Id: I27ef75a95889c37224aa7da6b0fbf8e2ad57ba65
Bug: WebThingsIO#1148
Forwarded: WebThingsIO#1249
Origin: https://github.com/tizenteam/gateway
Signed-off-by: Philippe Coval <p.coval@samsung.com>
rzr added a commit to TizenTeam/gateway that referenced this issue Aug 10, 2018
To avoid clash LevelProperty's id are renamed to slide-level-${count}

It was tested on ARTIK05x thing (2 switches, 2 booleans, 2 levels)

Change-Id: If530737168af14eb8f6f1e9e230e04a432490045
Bug: WebThingsIO#1148
Forwarded: WebThingsIO#1249
Origin: https://github.com/tizenteam/gateway
Signed-off-by: Philippe Coval <p.coval@samsung.com>
@rzr
Copy link
Contributor

rzr commented Aug 21, 2018

I confirm this UI issue is affecting latest release,

I can share a code for ARTIK or RPi to reproduce and verify pending change at:

#1249

artik-720

@freaktechnik
Copy link
Contributor Author

I think you found an adjacent issue, what I was initially referencing is the state the gateway chooses for the central thing blob for BinarySensors or MultiLevelSensors, not necessarily the actual individual properties not working individually, which is the even bigger issue, I guess.

rzr added a commit to TizenTeam/gateway that referenced this issue Aug 21, 2018
Previously UI item's ids were duplicated,
the observed consequence is that any click on widgets
is catched by 1st declared element.

For example, a thing with 2 switches is displayed correctly,
and click events are also triggered
but event will be "redirected" to 1st switch only,
this is not desired.

It was tested for boolean, number related components.

Slider not fully updated, to keep passing tests,
more refactoring to come in that part (grep '#slider')

Change-Id: I27ef75a95889c37224aa7da6b0fbf8e2ad57ba65
Bug: WebThingsIO#1148
Forwarded: WebThingsIO#1249
Origin: https://github.com/tizenteam/gateway
Signed-off-by: Philippe Coval <p.coval@samsung.com>
@rzr
Copy link
Contributor

rzr commented Aug 21, 2018

@mrstegeman said { Yes, that's the exact same issue. }

May I ask you @freaktechnik to test my patch and report if it does not make situation worse ;)

@freaktechnik
Copy link
Contributor Author

May I ask you @freaktechnik to test my patch and report if it does not make situation worse ;)

I think it fixes it even, but it's hard to reproduce for me, without going through the paces and building a custom virtual thing that happens to trigger this. I had encountered this randomly in the wild...

rzr added a commit to TizenTeam/gateway that referenced this issue Aug 21, 2018
Previously UI item's ids were duplicated,
the observed consequence is that any click on widgets
is catched by 1st declared element.

For example, a thing with 2 switches is displayed correctly,
and click events are also triggered
but event will be "redirected" to 1st switch only,
this is not desired.

It was tested for boolean, number related components.

Slider not fully updated, to keep passing tests,
more refactoring to come in that part (grep '#slider')

Change-Id: I27ef75a95889c37224aa7da6b0fbf8e2ad57ba65
Bug: WebThingsIO#1148
Forwarded: WebThingsIO#1249
Origin: https://github.com/tizenteam/gateway
Signed-off-by: Philippe Coval <p.coval@samsung.com>
@mrstegeman mrstegeman added review and removed backlog labels Aug 31, 2018
rzr added a commit to TizenTeam/gateway that referenced this issue Sep 11, 2018
Previously UI item's ids were duplicated,
the observed consequence is that any click on widgets
is catched by 1st declared element.

For example, a thing with 2 switches is displayed correctly,
and click events are also triggered
but event will be "redirected" to 1st switch only,
this is not desired.

The change is only fixing boolean widgets, others to come later.

Note: It was suggested to use nested inputs,
but unfortunately CSS can't traverse (to input)
and get back to parent (label), so "for" facility and id must be used.

Change-Id: I27ef75a95889c37224aa7da6b0fbf8e2ad57ba65
Bug: WebThingsIO#1148
Forwarded: WebThingsIO#1249
Origin: https://github.com/tizenteam/gateway
Signed-off-by: Philippe Coval <p.coval@samsung.com>
rzr added a commit to TizenTeam/gateway that referenced this issue Sep 11, 2018
Previously UI item's ids were duplicated,
the observed consequence is that any click on widgets
is catched by 1st declared element.

For example, a thing with 2 switches is displayed correctly,
and click events are also triggered
but event will be "redirected" to 1st switch only,
this is not desired.

The change is only fixing boolean widgets, others to come later.

Note: It was suggested to use nested inputs,
but unfortunately CSS can't traverse (to input)
and get back to parent (label), so "for" facility and id must be used.

Change-Id: I27ef75a95889c37224aa7da6b0fbf8e2ad57ba65
Bug: WebThingsIO#1148
Forwarded: WebThingsIO#1249
Origin: https://github.com/tizenteam/gateway
Signed-off-by: Philippe Coval <p.coval@samsung.com>
mrstegeman pushed a commit that referenced this issue Sep 19, 2018
* ui: Fix boolean and switch widgets (if multiple)

Previously UI item's ids were duplicated,
the observed consequence is that any click on widgets
is catched by 1st declared element.

For example, a thing with 2 switches is displayed correctly,
and click events are also triggered
but event will be "redirected" to 1st switch only,
this is not desired.

The change is only fixing boolean widgets, others to come later.

Note: It was suggested to use nested inputs,
but unfortunately CSS can't traverse (to input)
and get back to parent (label), so "for" facility and id must be used.

Change-Id: I27ef75a95889c37224aa7da6b0fbf8e2ad57ba65
Bug: #1148
Forwarded: #1249
Origin: https://github.com/tizenteam/gateway
Signed-off-by: Philippe Coval <p.coval@samsung.com>

* Update switch.js
@ghost ghost removed the review label Sep 19, 2018
rzr added a commit to TizenTeam/gateway that referenced this issue Sep 24, 2018
In case of multiple property of same types,
UI events were forwarded to 1st instance of widget.

This change is a follow up of the switch change:

Bug: WebThingsIO#1148
Relate-to: WebThingsIO#1249
Change-Id: I2018092168af14eb8f6f1e9e230e04a432490045
Signed-off-by: Philippe Coval <p.coval@samsung.com>
rzr added a commit to TizenTeam/gateway that referenced this issue Sep 24, 2018
In case of multiple property of same types,
UI events were forwarded to 1st instance of widget.

This change is a follow up of the switch change:

Origin: WebThingsIO#1370
Bug: WebThingsIO#1148
Relate-to: WebThingsIO#1249
Change-Id: I2018092168af14eb8f6f1e9e230e04a432490045
Signed-off-by: Philippe Coval <p.coval@samsung.com>
mrstegeman pushed a commit that referenced this issue Sep 25, 2018
* ui: Select by class instead of (multiple) id

In case of multiple property of same types,
UI events were forwarded to 1st instance of widget.

This change is a follow up of the switch change:

Origin: #1370
Bug: #1148
Relate-to: #1249
Change-Id: I2018092168af14eb8f6f1e9e230e04a432490045
Signed-off-by: Philippe Coval <p.coval@samsung.com>

* ui: Prevent duplicated id

This change has no real effect,
only for making code more uniform,

Note that slider id has been renamed to slider-level
to remove ambiguity with slider,
and counter suffix adjusted.

Forwarded: #1370
Change-Id: I64fe6a78a8ae9d300bab58561a7b0eec7bfb09db
Signed-off-by: Philippe Coval <p.coval@samsung.com>
@mrstegeman mrstegeman added this to the 0.6 milestone Jan 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants