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

Hyphen in template cannot be used in sitemap mapping #2701

Closed
timbms opened this issue Aug 2, 2024 · 12 comments · Fixed by #2720
Closed

Hyphen in template cannot be used in sitemap mapping #2701

timbms opened this issue Aug 2, 2024 · 12 comments · Fixed by #2720
Assignees
Labels
bug Something isn't working main ui Main UI

Comments

@timbms
Copy link

timbms commented Aug 2, 2024

Expected Behavior

I am using Fritz!Box Templates to switch between operation modes of the Fritz radiator controller.
I am setting up a sitemap. I would like to configure the mappings in the UI and be able to see the resulting sitemap code under the "Code" tab.

Current Behavior:

The "Code" tab hangs because the hyphen is violating the expected token syntax.

Possible Solution

A fix in openhab-addons or more tolerance in sitemap parsing.

Steps to Reproduce (for Bugs)

  1. Setup mapping with hyphen
Screenshot 2024-08-02 at 20 07 08 2. View sitemap in "Code" tab Screenshot 2024-08-02 at 20 07 38

Your Environment

  • Version used: 4.2.0 - Release build
  • Operating System and version :
openHAB Distribution Version Information
----------------------------------------
build-no        : Release Build
online-repo     : https://openhab.jfrog.io/openhab/libs-release

Repository        Version
----------------------------------------
openhab-distro  : 4.2.0
openhab-core    : 4.2.0
openhab-addons  : 4.2.0
karaf           : 4.4.6
@timbms timbms added the bug Something isn't working label Aug 2, 2024
@lsiepel
Copy link

lsiepel commented Aug 10, 2024

This seems to be a core or ui issue. (I think the latter) who can transfer this issue? @openhab/webui-maintainers

@florian-h05
Copy link
Contributor

It’s a UI issue, thanks for the ping.

@florian-h05 florian-h05 transferred this issue from openhab/openhab-addons Aug 10, 2024
@florian-h05 florian-h05 changed the title [avmfritz] Hyphen in template cannot be used in sitemap mapping Hyphen in template cannot be used in sitemap mapping Aug 10, 2024
@florian-h05 florian-h05 added the main ui Main UI label Aug 10, 2024
@florian-h05
Copy link
Contributor

florian-h05 commented Aug 10, 2024

@mherwege Can you please take a look?
I think the Nearley definitions need to be changed.

@mherwege
Copy link
Contributor

I will have a look. Please assign to me.

@mherwege
Copy link
Contributor

mherwege commented Aug 12, 2024

The syntax cmd=label=icon in the Nearly parser is exactly what is in the original syntax here: https://github.com/openhab/openhab-core/blob/5eedd273baefc7868c31d80cff513e22cde60425/bundles/org.openhab.core.model.sitemap/src/org/openhab/core/model/sitemap/Sitemap.xtext#L219

The thing is, cmd can be Number, can an ID but then it has to comply with the ID syntax rules, which is no spaces, no -, starting with a character, or it can be a String. For a string, it has to be enclosed in double quotes.

I tried with something like: "test-command"=test and that works perfectly fine.

I suspect your syntax will also not work in a .sitemap file.

So I propose to close this, as this is a configuration issue.

@timbms
Copy link
Author

timbms commented Aug 12, 2024

@mherwege: The sitemap defined with mappings is and was working without a flaw. The issue became apparent only when I switched over to code view. It took me quite a while to establish what was wrong. If this is not a bug, wouldn't it be even better to prevent illegal definition in the Design view.
Apparently things got already better with openHAB 4.2.1: Error is not overlaid on editor, making it hard to read. Editor remains scrollable even in the case of an illegal value. Still one more issue is apparent: The offending code is wrongly quoted. The offending code truly is at line 56 col 107. However, the lines shown below are not the line 54-56:
Screenshot 2024-08-12 at 11 26 19

@mherwege
Copy link
Contributor

switched over to code view

And that is exactly what I did in my test above. If the string does not contain a hyphen, the double quotes are not required. If it does, you need to put it in double quotes to make it work. And that is the error it tells you in the code view.
The hyphen without double quote is not valid syntax, also not when defining it in a sitemap file. If it works, you got lucky. I could put in an extra check in the UI to warn for it, but it will not make a difference in that your configuration syntax is not correct.

@timbms
Copy link
Author

timbms commented Aug 12, 2024

I understand that the configuration is not correct, but it would be great if one would understand the root cause for the syntax error. As things improved already, probably with 4.2.1, it would be even better if the offending lines would be properly shown.

@mherwege
Copy link
Contributor

if the offending lines would be properly shown

I have not been able to do that consistently. The grammer parser generates errors, but these are not always at exactly where it happens.
A better approach is probably to check it in the UI, to make sure there are quotes around it when creating a mapping in the UI. I will look into that.

@timbms
Copy link
Author

timbms commented Aug 12, 2024

Here the grammar parser reports the right location, but the output shows the wrong lines, here just the end of the file.

@mherwege
Copy link
Contributor

We have little control over how Nearly creates that error message unfortunately. Therefore, when I have time I will look into putting checks in the UI to avoid the problem in the first place when creating it from the UI.

@mherwege
Copy link
Contributor

Also see kach/nearley#634

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working main ui Main UI
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants