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

Extend sitemap syntax for switch to support press & release buttons #4183

Merged
merged 3 commits into from
May 5, 2024

Conversation

lolodomo
Copy link
Contributor

Mappings attribute for switch element now accepts one or two commands for each button. If only one command is provided, the button is a click button, the command is sent to the item when the button is clicked. If two commands are provided (separated by ":"), the button is a press & release button, the first command is sent to the item when the button is pressed and the second when the button is released.

Syntax example for a click button: Switch item=DemoSwitch mappings=[ ON="ON" ]
Syntax example for a press & release button: Switch item=DemoSwitch mappings=[ ON:OFF="ON" ]

Related to #3822

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

Mappings attribute for switch element now accepts one or two commands for each button.
If only one command is provided, the button is a click button, the command is sent to the item when the button is clicked.
If two commands are provided (separated by ":"), the button is a press & release button, the first command is sent to the item when the button is pressed and the second when the button is released.

Syntax example for a click button: Switch item=DemoSwitch mappings=[ ON="ON" ]
Syntax example for a press & release button: Switch item=DemoSwitch mappings=[ ON:OFF="ON" ]

Related to openhab#3822

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
@lolodomo lolodomo requested a review from a team as a code owner April 13, 2024 13:29
@mherwege
Copy link
Contributor

LGTM
I am OK with this. However, it does introduce an extra constraint on string commands. They are not allowed to contain : anymore. I don't expect this to be common, but it could be out there.

@lolodomo
Copy link
Contributor Author

However, it does introduce an extra constraint on string commands. They are not allowed to contain : anymore. I don't expect this to be common, but it could be out there.

Hopefully not. Simply use string in double quotes as commands like for example "val:On":"val:Off"="my button".

@lolodomo
Copy link
Contributor Author

Here is where is defined the syntax of a command. It can be a number (0 for example), an id (ON for example).or a string ("val:On" for example).

https://github.com/openhab/openhab-core/blob/main/bundles/org.openhab.core.model.sitemap/src/org/openhab/core/model/sitemap/Sitemap.xtext#L237

Even if I am sure, I will test it.

@lolodomo
Copy link
Contributor Author

Tested and working.
Please note that already today (without this change) you need to use double quotes around your command value in case it embeds a character that is not a number, a letter or a _. So double quotes were already required if your command value includes a :.

@lolodomo
Copy link
Contributor Author

lolodomo commented Apr 18, 2024

But my new code for the sitemap generator simply splitting on ":" is probably not sufficient. I will enhance it.

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
lolodomo added a commit to lolodomo/openhab-webui that referenced this pull request Apr 20, 2024
Related to openhab/openhab-core#3822

Depends on openhab/openhab-core#4183

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
lolodomo added a commit to lolodomo/openhab-webui that referenced this pull request Apr 20, 2024
Related to openhab/openhab-core#3822

Depends on openhab/openhab-core#4183

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
@@ -99,6 +99,11 @@ public class UIComponentSitemapProvider implements SitemapProvider, RegistryChan

private static final Pattern CONDITION_PATTERN = Pattern
.compile("(?<item>[A-Za-z]\\w*)?\\s*(?<condition>==|!=|<=|>=|<|>)?\\s*(?<sign>\\+|-)?(?<state>.+)");
private static final Pattern COMMANDS_PATTERN1 = Pattern.compile("^\"(?<cmd1>[^\"]*)\":\"(?<cmd2>[^\"]*)\"$");
Copy link
Contributor

@mherwege mherwege Apr 22, 2024

Choose a reason for hiding this comment

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

I think this single regex will do:
"^(?<cmd1>\"[^\"]*\"|[^\": ]*):(?<cmd2>.*)$"
You just need to strip the opening and closing double quotes from cmd1 and cmd2 after the match if any exist.

@mherwege
Copy link
Contributor

Mappings are also used in the Selection widget. As there is no differentiation in the parser, it will also create this extra field for the Selection widget. Should that be restricted (and different types of mappings created for each?). Or should it just be ignored for a Selection widget.

@lolodomo
Copy link
Contributor Author

lolodomo commented Apr 22, 2024

Or should it just be ignored for a Selection widget.

My idea was to ignore the optional release command in the case of Selection widget.

@mherwege
Copy link
Contributor

mherwege commented Apr 23, 2024

I am fighting a bit to make things work in the UI sitemap generator. The issue is that these mappings (and similarly the rules) are sent as one field to OH core. These do not contain quotes, so the splitting has always been a bit of an art.
The proper solution would be to bring the quotes into these strings and treat them properly. They are escaped in the JSON with . That is easy on the UI side, and makes things a lot simpler there. But I see them showing up in the sitemap REST API (where the mappings and rules are split into fields) with " as well then. So it would probably require changing the treatment in core in sync with this as well.

@lolodomo
Copy link
Contributor Author

lolodomo commented Apr 29, 2024

@mherwege : until a kind of refactoring is started in the sitemap generator to handle properly quotes, how can we proceed here to not being blocked ? As I understand, with the current implementation, I have to consider that the string will not contain quotes but then it is not possible to make a difference between a ":" being the separator between two commands and ":" being a part of a command.
Maybe as a temporary solution, I can come back to my initial proposal being to split on ":" to separate commands ?
Or would you like I choose another character as separator ?

@mherwege
Copy link
Contributor

@lolodomo I would keep the syntax. I have the refactoring on the generator almost done, but need some more testing to verify it does not break anything. I am short on time at the moment so things may be a bit slow on my side.

Utimately, I actually think it is a stupid thing to pass these configurations as such through the REST API, and not the fields it gets split into. That would avoid a lot of string manipulation in the generator UI as well as in this code. I may look into that in the future, but don't have time for that now, so we should leave that out.

@lolodomo
Copy link
Contributor Author

Ok, in that case, I am going to consider your REGEX simplification proposal.

@mherwege
Copy link
Contributor

mherwege commented Apr 30, 2024

@lolodomo I implemented a solution in the UI, but in doing that (and allowing quotes to be saved), I also needed to modify UIComponentSitemapProvider.java. This change should not have an impact beyond supporting quoted strings in the definition, but would be required for the UI code to work. It is important to consider that change when progressing with this PR. Without that PR, a sitemap UI would show quotes around the mappings when they are in the definition.

@lolodomo
Copy link
Contributor Author

@mherwege : would you like that I remove my changes in UIComponentSitemapProvider from this PR and then you can create another PR specific to the sitemap generator ?

@mherwege
Copy link
Contributor

mherwege commented Apr 30, 2024

I can include it in #4204, but it can then only be merged after your PR. I am fine with that.

@mherwege
Copy link
Contributor

Included in #4204, but not tested, as it will now depend on this PR.

Changes will be done in a separate PR.

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
@lolodomo
Copy link
Contributor Author

lolodomo commented Apr 30, 2024

@mherwege : change removed

@openhab/core-maintainers : please review & merge this ultra small PR (5 lines).

@holgerfriedrich
Copy link
Member

@J-N-K Could I ask you to take a look at this? I like the functionality, but might miss further implications of this PR....

Copy link
Member

@J-N-K J-N-K 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

@J-N-K J-N-K merged commit b40e6db into openhab:main May 5, 2024
5 checks passed
@J-N-K J-N-K added the enhancement An enhancement or new feature of the Core label May 5, 2024
@J-N-K J-N-K added the sitemap label May 5, 2024
@J-N-K J-N-K added this to the 4.2 milestone May 5, 2024
@lolodomo lolodomo deleted the press_release_button branch May 5, 2024 07:46
kaikreuzer pushed a commit to openhab/openhab-webui that referenced this pull request May 6, 2024
Related to openhab/openhab-core#3822

Depends on openhab/openhab-core#4183

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
lolodomo added a commit to lolodomo/openhab-docs that referenced this pull request May 7, 2024
florian-h05 pushed a commit to openhab/openhab-webui that referenced this pull request May 7, 2024
Related to openhab/openhab-core#4183.
Depends on openhab/openhab-core#4204.

This PR implements press/release button support.

Refactor code to allow proper quoting of arguments in mappings and conditions.
Quotes are now preserved, and therefore they need to be removed in core
at usage (openhab/openhab-core#4204).

---------

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>
lolodomo added a commit to lolodomo/openhab-docs that referenced this pull request May 7, 2024
stefan-hoehn pushed a commit to openhab/openhab-docs that referenced this pull request May 12, 2024
stefan-hoehn added a commit to openhab/openhab-docs that referenced this pull request May 19, 2024
* Apply logo for free@home binding

Apply logo for free@home binding

Signed-off-by: Andras Uhrin <andras.uhrin@gmail.com>

* [sitemap] New Slider parameter releaseOnly (#2287)

* [saicismart][renault] Add addon SVG logo for Renault and SAIG MG Cars (#2290)

* [saicismart] Add addon logo for MG Cars from https://de.wikipedia.org/wiki/Datei:Mg_logo.svg

Signed-off-by: dougculnane <doug@culnane.net>

* [saicismart] Use SVG not PNG for addon logo

Signed-off-by: dougculnane <doug@culnane.net>

* [renault] Use SVG not PNG for addon logo

Signed-off-by: dougculnane <doug@culnane.net>

---------

Signed-off-by: dougculnane <doug@culnane.net>

* Update persistence actions docs (#2289)

* update persistence actions docs

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* correction

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* document HistoricItem

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* review adjustment

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

---------

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

* Update link to update-description-1.0.0.xsd (#2275)

The link for the update-description-1.0.0.xsd is wrong.

Signed-off-by: M Valla <12682715+mvalla@users.noreply.github.com>

* Replace logo of free@home binding with SVG version

Replace logo of free@home binding with SVG version

Signed-off-by: Andras Uhrin <andras.uhrin@gmail.com>

* [sitemap] Precision for Default element leading to Slider element (#2294)

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

* [sitemap] Press & release feature for buttons in a switch element (#2293)

Related to openhab/openhab-core#4183 and openhab/openhab-core#3822

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

* [airgradient] Add logo for recently added binding (#2296)

* [airgradient] Add logo for recently added binding

* Add airgradient logo in SVG format

Signed-off-by: Jørgen Austvik <jaustvik@acm.org>

---------

Signed-off-by: Jørgen Austvik <jaustvik@acm.org>

* [multimedia] Add record and transcribe commands (#2295)

* [multimedia] Add record and transcribe commands

Signed-off-by: Miguel Álvarez <miguelwork92@gmail.com>

* apply pr review

Signed-off-by: Miguel Álvarez <miguelwork92@gmail.com>

* fix md errors

Signed-off-by: Stefan Höhn <mail@stefanhoehn.com>

---------

Signed-off-by: Miguel Álvarez <miguelwork92@gmail.com>
Signed-off-by: Stefan Höhn <mail@stefanhoehn.com>
Co-authored-by: stefan-hoehn <mail@stefanhoehn.com>

* Apply logo for free@home binding

Apply logo for free@home binding

Signed-off-by: Andras Uhrin <andras.uhrin@gmail.com>

* Replace logo of free@home binding with SVG version

Replace logo of free@home binding with SVG version

Signed-off-by: Andras Uhrin <andras.uhrin@gmail.com>

---------

Signed-off-by: Andras Uhrin <andras.uhrin@gmail.com>
Signed-off-by: dougculnane <doug@culnane.net>
Signed-off-by: Mark Herwege <mark.herwege@telenet.be>
Signed-off-by: M Valla <12682715+mvalla@users.noreply.github.com>
Signed-off-by: Laurent Garnier <lg.hc@free.fr>
Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
Signed-off-by: Miguel Álvarez <miguelwork92@gmail.com>
Signed-off-by: Stefan Höhn <mail@stefanhoehn.com>
Co-authored-by: stefan-hoehn <mail@stefanhoehn.com>
Co-authored-by: mueller-ma <mueller-ma@users.noreply.github.com>
Co-authored-by: Doug Culnane <32482395+dougculnane@users.noreply.github.com>
Co-authored-by: Mark Herwege <mherwege@users.noreply.github.com>
Co-authored-by: M Valla <12682715+mvalla@users.noreply.github.com>
Co-authored-by: lolodomo <lg.hc@free.fr>
Co-authored-by: Jørgen Austvik <jaustvik@acm.org>
Co-authored-by: GiviMAD <GiviMAD@users.noreply.github.com>
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 sitemap
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants