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

Parameters injected to JS transformations are not resetted across further invocations of the transformation script #4414

Open
sheilbronn opened this issue Oct 13, 2024 · 4 comments
Labels
bug An unexpected problem or unintended behavior of the Core

Comments

@sheilbronn
Copy link

sheilbronn commented Oct 13, 2024

Background

(I'm on Openhab 4.3.0 M2 on a Raspberry Pi 4. This is the first issue I filed for Openhab, pls bear with me for any mistakes. My Openhabian system is manually upgraded to bullseye, although this is not supported... But I thought this issue still might be worth to get investigated since it is easily reproducible...)

Problem: When passing adding additional parameters to toItemScript as with ?fig=2 in the following snippet...

Number:Temperature Refoss_P11d_Temp "Temperature" { channel="mqtt:topic:openhab:refossp11d:temperature" [profile="transform:JS",toItemScript="significant.js?fig=2"], expire="30m" }
Number:ElectricCurrent Refoss_P11d_Current "Current [%.1f %unit%]"  { channel="mqtt:topic:openhab:refossp11d:current" [profile="transform:JS",toItemScript="significant.js"], expire="30m"}

this parameter is shared across all invocations of significant.js, i.e. fig is always 2 when significant.js is called, even when it is not explicitly given as in my second item above.
This is rather surprising behavior, and I was wondering whether it's a bug or a feature in M2 (or before?)

PS: Update: It seems that one has to manually set the injected variable to undefined at the end of the transformation script in order to have reset before the next invocation - IMHO this is not an easily expected behavior that at least should be well documented:

fig = undefined; // reset fig to undefined for the next invocation

As suggested in the forum by Florian H. I'm also filing an issue here:

Expected Behavior

I'd expect any parameters that are NOT passed specifically (e.g. "?param1=val1&param2=val2") always be set to undefined....

Current Behavior

When passing adding additional parameters (e.g. "significant.js?param1=val&param2=val") to a JS transformation script via toItemScript() - as with ?fig=2 above - these parameters are shared across all invocations of significant.js, i.e. fig is always 2 when significant.js is called, even when it is not explicitly given as in my second item above

Possible Solution

Injected parameters should be implicitly reset to undefined before further invocations of the transformation script

Steps to Reproduce (for Bugs)

Here's is a JS transformation to reproduce:

// return the input value unchanged, logging arg1 and arg2 if defined
(function(i) {
    // console.log("donothing.js: started");
    var strarg1 = "" ;
    var strarg2 = "" ;
    if (typeof arg1 !== 'undefined') { 
        strarg1 = "    arg1=" + arg1.toString() + " (" + typeof arg1 + ")";
    }
    if (typeof arg2 !== 'undefined') { 
        strarg2 = "    arg2=" + arg2.toString() + " (" + typeof arg2 + ")";
    }
    console.log("donothing.js: i=" + i.toString() + strarg1  + strarg2);
    arg1 = "UNCHANGED";
    arg2 = "UNCHANGED";
    return i;
})(input)

Use this as sample transformation as in
Number:Energy Refoss_TotalYesterday "Total Yesterday [%d %unit%]" { channel="mqtt:topic:openhab:refossp11b:yesterday" [profile="transform:JS",toItemScript="donothing.js?arg1=ONE"], expire="90m" }
Number:Energy Refoss_TotalToday "Total Today [%d %unit%]" { channel="mqtt:topic:openhab:refossp11b:today" [profile="transform:JS",toItemScript="donothing.js?arg2=TWO"], expire="90m" }

will give output like this, showing the behavior:

2024-10-13 14:14:11.963 [INFO ] [org.openhab.automation.script       ] - donothing.js: i=0.0    arg1=ONE (string)    arg2=UNCHANGED (string)
2024-10-13 14:14:11.968 [INFO ] [org.openhab.automation.script       ] - donothing.js: i=0.0    arg1=UNCHANGED (string)    arg2=a (string)
2024-10-13 14:31:04.997 [INFO ] [org.openhab.automation.script       ] - donothing.js: i=0.0    arg1=ONE (string)    arg2=UNCHANGED (string)
2024-10-13 14:31:05.003 [INFO ] [org.openhab.automation.script       ] - donothing.js: i=0.0    arg1=UNCHANGED (string)    arg2=a (string)
2024-10-13 14:41:09.819 [INFO ] [org.openhab.automation.script       ] - donothing.js: i=0.0    arg1=ONE (string)    arg2=UNCHANGED (string)
2024-10-13 14:41:09.826 [INFO ] [org.openhab.automation.script       ] - donothing.js: i=0.0    arg1=UNCHANGED (string)    arg2=a (string)
2024-10-13 15:17:07.687 [INFO ] [org.openhab.automation.script       ] - donothing.js: i=0.0    arg1=ONE (string)    arg2=UNCHANGED (string)
2024-10-13 15:17:07.693 [INFO ] [org.openhab.automation.script       ] - donothing.js: i=0.0    arg1=UNCHANGED (string)    arg2=a (string)

Context/Rationale

Background: The significant.js script mentioned initially is my transformation script for rounding incoming values from sensors to a more sensible, reduced number of significant figures/digits depending on the unit of the measurement. [ADDED: This would cause fewer updates for the values actually fed into the receiving item.] I want to use fig= to override my unit specific default, e.g. 2 for temperatures.)

Your Environment

  • OpenHab 4.3 M2 (and maybe before)
  • Addons.cfg:
[automation = jsscripting
binding = astro,network,mqtt,tr064,mail,avmfritz,dwdunwetter,exec,amazonechocontrol,amazondashbutton,bluetooth,homeconnect,hpprinter,samsungtv,openweathermap,dwdpollenflug,weathercompany,telegram,mybmw,lgwebos
transformation = jinja,jsonpath,map,regex,basicprofiles,scale,exec
](url)
  • Version on "Raspbian GNU/Linux 12 (bookworm)" (manually upgraded from 11, although not officially supported!)":
$ java --version
openjdk 17.0.12 2024-07-16
OpenJDK Runtime Environment (build 17.0.12+7-Raspbian-2deb12u1rpt1)
OpenJDK Client VM (build 17.0.12+7-Raspbian-2deb12u1rpt1, mixed mode, emulated-client)

@sheilbronn sheilbronn added the bug An unexpected problem or unintended behavior of the Core label Oct 13, 2024
@openhab-bot
Copy link
Collaborator

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

https://community.openhab.org/t/openhab-4-3-milestone-discussion/158139/48

@rkoshak
Copy link

rkoshak commented Oct 15, 2024

As an explanation for the current behavior, the transformation gets loaded and instantiated as an Object. On each call to the transformation, the same Object is reused. That means any variables, constants, and such will be preserved from one call to the next.

The same is true for Script actions.

One gotcha with transformations is that the same transformation "Object" is reused across all usages of the transformation. So if you use it on two different Things, for example, you can have variables set from one Thing when running the transformation on the second Thing.

Note, for JS Scripting, I'm pretty sure there is a lock in place to prevent two Things (for example) from using the transformation at the same time. A Multi-threaded exception would be thrown if that were to happen.

I do like the idea that arguments passed into the transformation as arguments be reset at the end of the transformation automatically. Otherwise one cannot support optional arguments.

Finally:

rounding incoming values from sensors to a more sensible, reduced number of significant figures/digits depending on the unit of the measurement.

are you familiar with the round profile?

@sheilbronn
Copy link
Author

Finally:

rounding incoming values from sensors to a more sensible, reduced number of significant figures/digits depending on the unit of the measurement.

are you familiar with the round profile?

Yes, I am familiar with proper rounding. But in dealing with incoming measurements I prefer reducing the item "flickering" using the concept of significant figures, e.g. in my case default=2 for temperatures, default=5 for athmospheric pressures and 3 for Watts. My above mentioned transformation script significant.js takes care of this and I wanted the additional parameter fig to help me overrule my defaults...
But your comment actually brings me to the suggestion of adding a basic profile called "basic-profiles:significant" with these semantics. @rkoshak: What do you think?

@rkoshak
Copy link

rkoshak commented Oct 17, 2024

But your comment actually brings me to the suggestion of adding a basic profile called "basic-profiles:significant" with these semantics. @rkoshak: What do you think?

I think it's probable niche but overall I don't have a problem with it. I see no reason why that wouldn't be accepted in a PR.

That doesn't really close this issue though which still needs to be addressed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An unexpected problem or unintended behavior of the Core
Projects
None yet
Development

No branches or pull requests

3 participants