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] AND operator accepted in any condition + added optional conditional rules for icon #3759

Closed
wants to merge 1 commit into from

Conversation

lolodomo
Copy link
Contributor

@lolodomo lolodomo commented Aug 14, 2023

Allow multiple conditions with AND operator in visibility/color/icon rules

Closes #3058

Also allow dynamic icon based on other item states.
Allow dynamic icon even with non OH icon sources.

Example: icon=[item1>0=temperature,==0=material:settings,f7:house]

Related to openhab/openhab-webui#1938

Unit tests added or extended to cover the new features.

Signed-off-by: Laurent Garnier lg.hc@free.fr

@lolodomo lolodomo requested a review from a team as a code owner August 14, 2023 12:13
@lolodomo lolodomo force-pushed the sitemap_dynamic_icon branch 2 times, most recently from 33bb38a to 202600f Compare August 14, 2023 12:19
@lolodomo
Copy link
Contributor Author

lolodomo commented Aug 14, 2023

@openhab/android-maintainers
@openhab/ios-maintainers

For sitemap UIs, the only change to consider is that SSE events will now have a field icon set when the icon is not defined as static in the sitemap element. This field was already existing but was never set by the server. So the UI should now consider it when reloading the icon of an element.

@lolodomo
Copy link
Contributor Author

I just realized that if icon rules are defined, the UI should then call the OH icon servlet without passing the item state. So the sitemap REST API should provide this information to UIs to allow distinguishing dynamic icon with rules from dynamic OH icon without rules. I will enhance that.

@maniac103
Copy link
Contributor

So the UI should now consider it when reloading the icon of an element.

FWIW, Android app already does that, see here

I just realized that if icon rules are defined, the UI should then call the OH icon servlet without passing the item state.

Why? Can't the servlet not figure that out by itself? It would be great if we made the icon logic not even more complex than it already is ;-)

@lolodomo
Copy link
Contributor Author

lolodomo commented Aug 14, 2023

Why? Can't the servlet not figure that out by itself? It would be great if we made the icon logic not even more complex than it already is ;-)

No because the icon servlet is fully independent of sitemap concept.
As I introduce a new dynamic way to do for icons, we have to be sure, if used, that UIs will not use the other dynamic way at the same time.
By the way, this is not something introduced by this enhancement but by the previous enhancement that added the staticIcon parameter.
I will now add another boolean "iconIgnoreState" to the sitemap REST API.
So "staticIcon" will tell you if you have to reload or not the icon when receiving updates.
"iconIgnoreState" will tell you if you have to ignore the state or not when reloading the icon.

@lolodomo lolodomo force-pushed the sitemap_dynamic_icon branch from 202600f to e54b9e2 Compare August 14, 2023 13:46
@lolodomo
Copy link
Contributor Author

I will now add another boolean "iconIgnoreState" to the sitemap REST API.

Done

@maniac103
Copy link
Contributor

So "staticIcon" will tell you if you have to reload or not the icon when receiving updates.
"iconIgnoreState" will tell you if you have to ignore the state or not when reloading the icon.

Are all 4 combinations of those 2 values valid? E.g. staticIcon=true combined with iconIgnoreState=false doesn't seem to make sense? Also I'm not sure if that second flag triggering a reload will work properly, since - as long as the URL stays the same - caching is likely to swallow reloads anyway (and, in the case of the Android app, we rely on that behavior to make sure icons aren't loaded over and over when scrolling through lists). IMO it would be better to just require the UI to select whether or not to pass state when fetching icons.

@lolodomo
Copy link
Contributor Author

lolodomo commented Aug 14, 2023

I am reverting the last change. The field "staticIcon" is sufficient.
The boolean "staticIcon" returned by the sitemap REST API will tell you if you have to use or not the item state when requesting the icon.
With this new change, when receiving SSE updates, either the icon field is filled and you must reload the icon, or it is null and you must not reload the icon.

@lolodomo lolodomo force-pushed the sitemap_dynamic_icon branch 5 times, most recently from 049fd57 to 733657f Compare August 14, 2023 20:52
lolodomo added a commit to lolodomo/openhab-webui that referenced this pull request Aug 14, 2023
Depends on openhab/openhab-core#3759

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
lolodomo added a commit to lolodomo/openhab-webui that referenced this pull request Aug 14, 2023
Depends on openhab/openhab-core#3759

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
lolodomo added a commit to lolodomo/openhab-webui that referenced this pull request Aug 14, 2023
Depends on openhab/openhab-core#3759 and openhab#1998

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
@lolodomo lolodomo force-pushed the sitemap_dynamic_icon branch from 733657f to 1be31f1 Compare August 14, 2023 23:07
lolodomo added a commit to lolodomo/openhab-webui that referenced this pull request Aug 14, 2023
Depends on openhab/openhab-core#3759 and openhab#1998

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
lolodomo added a commit to lolodomo/openhab-webui that referenced this pull request Aug 15, 2023
Depends on openhab/openhab-core#3759 and openhab#1998

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
lolodomo added a commit to lolodomo/openhab-webui that referenced this pull request Sep 9, 2023
Depends on openhab/openhab-core#3759 and openhab#1998

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
@lolodomo lolodomo force-pushed the sitemap_dynamic_icon branch from 1e78867 to b95bcb6 Compare September 27, 2023 06:23
@lolodomo lolodomo marked this pull request as draft September 27, 2023 06:24
@lolodomo lolodomo force-pushed the sitemap_dynamic_icon branch from b95bcb6 to fb0ab17 Compare September 27, 2023 07:10
@lolodomo lolodomo changed the title [sitemap] Extend icon parameter with optional conditional rules [sitemap] Added optional conditional rules for icon + AND operatior accepted in any condition Sep 27, 2023
@lolodomo lolodomo changed the title [sitemap] Added optional conditional rules for icon + AND operatior accepted in any condition [sitemap] Added optional conditional rules for icon + AND operator accepted in any condition Sep 27, 2023
@lolodomo lolodomo force-pushed the sitemap_dynamic_icon branch from fb0ab17 to ff068bd Compare September 28, 2023 06:40
@lolodomo lolodomo force-pushed the sitemap_dynamic_icon branch 2 times, most recently from a32abb8 to 53abb23 Compare September 30, 2023 08:46
@lolodomo lolodomo changed the title [sitemap] Added optional conditional rules for icon + AND operator accepted in any condition [sitemap] AND operator accepted in any condition + added optional conditional rules for icon Sep 30, 2023
@lolodomo lolodomo force-pushed the sitemap_dynamic_icon branch 3 times, most recently from 802d554 to fbbd35a Compare October 1, 2023 08:42
…ditional rules for icon

Allow multiple conditions with AND operator in visibility/color/icon rules

Closes openhab#3058

Also allow dynamic icon based on other item states.
Allow dynamic icon even with non OH icon sources.

Example: icon=[item1>0=temperature,==0=material:settings,f7:house]

Related to openhab/openhab-webui#1938

Unit tests added or extended to cover the new features.

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
@lolodomo lolodomo force-pushed the sitemap_dynamic_icon branch from fbbd35a to 10d85f4 Compare October 1, 2023 09:55
@lolodomo
Copy link
Contributor Author

lolodomo commented Oct 1, 2023

Regarding the ability to use multiple AND conditions, this is something fully handled by the core framework and there is nothing to change in sitemap UIs. And the sitemap syntax remains fully backward compatible.

@lolodomo lolodomo marked this pull request as ready for review October 1, 2023 10:45
@lolodomo
Copy link
Contributor Author

lolodomo commented Oct 2, 2023

I am going to extract the feature about AND conditions in a separate PR.

@lolodomo lolodomo marked this pull request as draft October 2, 2023 10:59
@lolodomo
Copy link
Contributor Author

lolodomo commented Oct 2, 2023

I close this PR. A new one will be opened.

@lolodomo lolodomo closed this Oct 2, 2023
@lolodomo lolodomo deleted the sitemap_dynamic_icon branch October 2, 2023 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Be able to connect multiple conditions with AND in sitemap's valuecolor
2 participants