search

openhab / openhab-core

Core framework of openHAB
https://www.openhab.org/
Eclipse Public License 2.0
932 stars 429 forks source link

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

Open sheilbronn opened 1 month ago

sheilbronn commented 1 month ago

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-bot commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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.