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 support for multiple AND conditions #3819

Merged
merged 1 commit into from
Oct 8, 2023

Conversation

lolodomo
Copy link
Contributor

@lolodomo lolodomo commented Oct 2, 2023

Concerns labelcolor, valuecolor and visibility

Also fix wrong positions of ")" in Sitemap.xtext

Closes #3058

Signed-off-by: Laurent Garnier [email protected]

Concerns labelcolor, valuecolor and visibility

Also fix wrong positions of ")" in Sitemap.xtext

Closes openhab#3058

Signed-off-by: Laurent Garnier <[email protected]>
@lolodomo lolodomo requested a review from a team as a code owner October 2, 2023 12:17
@lolodomo
Copy link
Contributor Author

lolodomo commented Oct 2, 2023

This feature is fully handled by the core framework and there is nothing to change in sitemap UIs. And the sitemap syntax remains fully backward compatible.

Copy link
Member

@kaikreuzer kaikreuzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks.

@kaikreuzer kaikreuzer merged commit e5518b9 into openhab:main Oct 8, 2023
2 checks passed
@kaikreuzer kaikreuzer added this to the 4.1 milestone Oct 8, 2023
@kaikreuzer kaikreuzer added the enhancement An enhancement or new feature of the Core label Oct 8, 2023
@lolodomo
Copy link
Contributor Author

lolodomo commented Oct 8, 2023

Thank you Kai.

@lolodomo lolodomo deleted the sitemap_AND_conditions branch October 8, 2023 07:50
@lolodomo
Copy link
Contributor Author

lolodomo commented Oct 8, 2023

@kaikreuzer @wborn : this requires a new build of core. I don't know if this requires a manual trigger or if the build will happen automatically during the night?

@wborn
Copy link
Member

wborn commented Oct 8, 2023

Yes it will automatically start a build at 3 AM (CEST).
Sometimes we trigger it manually when we are impatient. 🙂

@lolodomo
Copy link
Contributor Author

lolodomo commented Oct 8, 2023

Thank you for confirmation.

@mherwege
Copy link
Contributor

mherwege commented Oct 20, 2023

@lolodomo Before this PR, visibility rules always required an item and comparator in the condition (unlike color rules). As you now made it the same as for color rules, this is no longer required by the grammar. Can I assume this will correctly be picked up and interpreted by the UI's as well, or is this just for simplifying the grammar? There is a check in the UI sitemap editor for the presence of item and comparator, generating a warning if not present. If this is no longer required, I can remove it.

@lolodomo
Copy link
Contributor Author

lolodomo commented Oct 20, 2023

Can I assume this will correctly be picked up and interpreted by the UI's as well, or is this just for simplifying the grammar?

No problem with UIs, this Is in fact fully handled by the core framework.

I made the change for simplification and consistency but also because the additional constraint for visibility rules (compared to color rules) seems to have no valid reason.

If this is no longer required, I can remove it.

Yes

lolodomo added a commit to lolodomo/openhab-docs that referenced this pull request Nov 12, 2023
stefan-hoehn pushed a commit to openhab/openhab-docs that referenced this pull request Nov 19, 2023
* [sitemap] Combined conditions with AND

Related to openhab/openhab-core#3819

Signed-off-by: Laurent Garnier <[email protected]>

* Consider review comments

Signed-off-by: Laurent Garnier <[email protected]>

---------

Signed-off-by: Laurent Garnier <[email protected]>
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
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
4 participants