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

Fix Documentation of Transform Values - specifically Script Transformations #2348

Closed
wants to merge 1 commit into from

Conversation

vchrizz
Copy link

@vchrizz vchrizz commented Aug 11, 2024

Transformations given as example for JavaScript JS(|"String has " + input.length + " characters") result in following error in OpenHAB 4.2.1 log:

Configuration error for channel 'mqtt:topic:...' java.lang.IllegalArgumentException: The transformation pattern must consist of the type and the pattern separated by a colon

Using format JS:|"String has " + input.length + " characters" the transformation works as expected.

Transformation as given `JS(|"String has " + input.length + " characters")` gives following error in OpenHAB 4.2.1 log:

Configuration error for channel '...' java.lang.IllegalArgumentException: The transformation pattern must consist of the type and the pattern separated by a colon

Using format `JS:|"String has " + input.length + " characters"` the transformation works as expected.


Signed-off-by: Christoph <github-mail@chil.at>
Copy link

netlify bot commented Aug 11, 2024

Thanks for your pull request to the openHAB documentation! The result can be previewed at the URL below (this comment and the preview will be updated if you add more commits).

Built without sensitive environment variables

Name Link
🔨 Latest commit 7f2344b
🔍 Latest deploy log https://app.netlify.com/sites/openhab-docs-preview/deploys/66b86994ad1e3000083190fc
😎 Deploy Preview https://deploy-preview-2348--openhab-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@jimtng
Copy link
Contributor

jimtng commented Aug 11, 2024

This isn't necessarily an error. There are two syntaxes when it comes to transformations. When using it in sitemaps / labels, you'd use the parentheses, i.e. JS(....) but when using it in bindings such as MQTT, you'd use the colon i.e. JS:...... So the documentation is not incorrect.

Perhaps instead of changing it, it could be clarified further about the difference in the syntaxes and when it can be applied. If there's already such a document, perhaps a simple link to it would suffice?

@vchrizz
Copy link
Author

vchrizz commented Aug 11, 2024

I'm new to OpenHAB, if JS(...) is possible in other places, it definetly needs more clarification about "Transform Values" because in the UI the description for "Incoming Value Transformations" states the example JSONPATH:$.device.status.temperature which is confusing if you read the documentation, want to use JavaScript and see another syntax for this exact case which leads to a not functioning generic mqtt thing and an error message in the log. Then searching (in forums, etc.) for information how to correctly transform values leads you to conversations about scripting solutions but I could not find a single statement how the script (neither a file nor an inline script) should be defined in the first place. So new users like me fall back to read the documentation over and over again, even comparing documentation of older versions to newer versions where in old OpenHAB versions JavaScript Transformation Add-On is required (which does not exist any more in recent versions and this confuses readers even further). Maybe some hint, that in newer versions (from which version exactly?) the Add-On "JavaScript Scripting" alone is required for JavaScript Transformations would be also helpful.

@vchrizz vchrizz changed the title Fix Inline Script Transformations Fix Documentation of Inline Script Transformations Aug 11, 2024
@vchrizz vchrizz changed the title Fix Documentation of Inline Script Transformations Fix Documentation of Transform Values specifically Script Transformations Aug 11, 2024
@vchrizz vchrizz changed the title Fix Documentation of Transform Values specifically Script Transformations Fix Documentation of Transform Values - specifically Script Transformations Aug 11, 2024
@jimtng
Copy link
Contributor

jimtng commented Aug 11, 2024

I've just read the documentation and it is actually mentioned here including examples where it uses the () syntax:
https://next.openhab.org/docs/configuration/transformations.html#usage

Then on point number 3:

Bindings
Transformations can sometimes be used in binding add-ons. For example, transforming an openHAB ON command into "action=powerup" for sending to a device. If, and how, this use may be available is described in individual binding documentation.

So for example, in MQTT Binding documentation, it gives you the syntax for invoking transformations:
https://next.openhab.org/addons/bindings/mqtt.generic/#incoming-value-transformation

Admittedly having two syntaxes isn't ideal and I too have got them confused in the past.

How should we make it even clearer to avoid confusion for new users?

@vchrizz
Copy link
Author

vchrizz commented Aug 11, 2024

Sorry if I'm mixing up topics. Funny, I have read that usage you mentioned several times but I have also read that general advice not to use textual configuration so I kind of ignored the usage in textual configuration and went on searching for the correct syntax in the web UI. ;)
The second link you mentioned is interesting, I skimmed that too but this page does not mention javascript at all so I was unsure.
What do you think about another example for JavaScript as file and as inline in the MQTT Binding documentation and a Link from Script Transformation to MQTT Binding documentation informing about different syntax in different situations?

@stefan-hoehn
Copy link
Contributor

@florian-h05 can you confirm that this is correct for jsscripting?

@florian-h05
Copy link
Contributor

@jimtng said everything that has to be said. The changes in this PR aren't correct, the examples are for Item state transformations (to be used in the label part of .items files or the state description metadata).
However, as already said, the syntax from the changes is used for channel transformations by the MQTT binding.
I haven't seen that syntax anywhere else, the script profile works differently and does not have that confusing syntax.

From my POV the docs in openhab-docs are complete and correct.

I have just checked the linked MQTT docs and the channel add dialog for MQTT things, and there is docs for the MQTT-specific syntax both in the README and the UI:

image

@florian-h05
Copy link
Contributor

florian-h05 commented Aug 12, 2024

What do you think about another example for JavaScript as file and as inline in the MQTT Binding documentation and a Link from Script Transformation to MQTT Binding documentation informing about different syntax in different situations?

Additional JS examples inside these docs are fine, but I am not sure if we want to link to the MQTT docs there - having links from the docs repo content to addons repo content is now always a good idea.

The JS Scripting readme also includes a section about Item state transformations: https://next.openhab.org/addons/automation/jsscripting/#js-transformation

@rkoshak
Copy link
Contributor

rkoshak commented Aug 12, 2024

the syntax from the changes is used for channel transformations by the MQTT binding.
I haven't seen that syntax anywhere else

HTTP and at least one other add-on (which I can't find at the moment but I remember being surprised to see it there, I tried to search for "∩" but nothing relevant came up) also use this syntax. Basically those add-ons that allow transformation chaining use the : instead of ( ) syntax.

the channel add dialog for MQTT things

The docs for HTTP are not quite as good as those for MQTT

image

@vchrizz
Copy link
Author

vchrizz commented Aug 12, 2024

If you think the docs are fine, I apologize for the inconvenience of my PR. For me it took quite some time to get it sorted, that I can simply use JS:input/1000 within "Incoming Value Transformations" of a MQTT channel for such very simple calculations. The channel add dialog for MQTT things shows how to use String extraction on a JSON example but not a JavaScript example which is indeed possible. All other solutions I found while searching were not that simple. As said, I was reading the docs but it just wasn't that obvious to me that I may use JS:|<javascript> to transform values. I just wanted to save this time for other Newcomers like me. I'm fine if you close this PR, I finally found a simple solution that can simply be done in the UI without creating proxy items or writing scripts or similar.

@jimtng
Copy link
Contributor

jimtng commented Aug 16, 2024

@vchrizz out of curiosity, why did you need input/1000? If this channel has a unit, you could use UoM to perform this for you for free, without needing any transformations.

@openhab-bot
Copy link
Collaborator

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/binding-transformations/157861/1

@jimtng
Copy link
Contributor

jimtng commented Aug 18, 2024

FYI openhab/openhab-core#4352

Although this is currently only applicable to the http binding, eventually this will be adopted by all bindings with transformations, so they will all support the parentheses syntax.

@florian-h05
Copy link
Contributor

florian-h05 commented Aug 18, 2024

From my POV the docs in the docs repo are fine, and if Jims work is adopted by all bindings offering transformations, this will reduce overall complexity.

The docs for HTTP are not quite as good as those for MQTT

In that case I think we should improve HTTP docs and a script example inside the MQTT docs would be nice as well.

@vchrizz Thanks for your PR — even if it doesn‘t get merged, it put our attention on this topic, which is great!

@jimtng
Copy link
Contributor

jimtng commented Aug 18, 2024

In that case I think we should improve HTTP docs

openhab/openhab-addons#17288

and a script example inside the MQTT docs

After http binding I'll work on MQTT binding. I'll add that in a separate PR later on if I remember!

@vchrizz
Copy link
Author

vchrizz commented Aug 18, 2024

@vchrizz out of curiosity, why did you need input/1000? If this channel has a unit, you could use UoM to perform this for you for free, without needing any transformations.

I was following some tutorial (which seems to be outdated or incorrect or...), that had me creating some things/channels/items that led to log-messages like [WARN ] [penhab.core.library.items.NumberItem] - Failed to update item 'Shellyem3_foo_emeter0_total' because '1.2 W' could not be converted to the item unit 'kWh' and log-messages like [WARN ] [penhab.core.library.items.NumberItem] - Unit 'W' could not be parsed to a known unit. Keeping old unit 'kWh' for item 'Shellyem3_foo_emeter0_total', that led to debugging and fixing this issue with the Transformation, that led to trying to understand what is going on, that led to this PR.
Thank you for the hint with UoM - Units of Measurement. It seems I still had some 'Things' and/or 'items' created not correctly; after re-creating Things/channels/items and paying attention to set the units correctly in the first place (not editing channels/items afterwards to change/fix units), I could also create the items without Transformations and have them store the correct value with correct unit by using UoM.

@florian-h05
Copy link
Contributor

Ref https://community.openhab.org/t/syntax-for-binding-transformations/157861?u=florian-h05 for progress of the binding transformation changes.

@florian-h05
Copy link
Contributor

florian-h05 commented Sep 7, 2024

With all related PRs (see linked community post) merged now, I think we can close this PR.

@vchrizz
Copy link
Author

vchrizz commented Sep 7, 2024

Thank you!

@vchrizz vchrizz closed this Sep 7, 2024
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.

6 participants