openhab / openhab-js

openHAB JavaScript Library for JavaScript Scripting Automation
https://www.openhab.org/addons/automation/jsscripting/
Eclipse Public License 2.0
38 stars 31 forks source link

event data payload differs from UI to file-bases rules #111

Closed florian-h05 closed 2 years ago

florian-h05 commented 2 years ago

As already noticed in some PRs (https://github.com/openhab/openhab-js/pull/106, https://github.com/openhab/openhab-js/pull/110), the event object differs from UI-based to file-based rules.

In file-based rules the event data is "formatted" by getTriggeredData(input).

A solution to unify the event object of both would be to simply expose the above mentioned function and to advise the user to add the following on top of his UI rules:

if (typeof event !== 'undefined') event = require('openhab').rules.getTriggeredData(new Map().set('event', event));

Is there any way to put automatically add content to UI scripts?

WDYT @digitaldan @jpg0 ?

jpg0 commented 2 years ago

Whilst that would technically work in this case, my fear is that due to there being different implementations triggering the rules, this is only a hack rather than a proper solution. It won't fix all differences (e.g. can you use const in UI rules yet?), and may also break if the current wrapping code changes.

As for whether it's possible to inject this transparently, sure, it's possible.

florian-h05 commented 2 years ago

my fear is that due to there being different implementations triggering the rules, this is only a hack rather than a proper solution.

So you would not advise to inject or add that to the top of MainUI scripts?

By the way, I found one more difference between UI and file based: UI based scripts do not reload on changes in node_modules. You have to disable/enable them to reload dependencies.

ssalonen commented 2 years ago

e.g. can you use const in UI rules yet?

yeah to my knowledge const rules are not supported since context is 'reused'. That particular limitation can be hacked around by wrapping with IIFE, (function () { .. })() to my knowledge. However, that might suffer from this context differences ? (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/this)

jpg0 commented 2 years ago

So you would not advise to inject or add that to the top of MainUI scripts?

Probably not, no. The real* problem here is that the script execution environment differs (e.g. the const problem, dependency reloading, etc), and this is the thing that needs fixing. Injecting scripts in one environment doesn't really fix the underlying problem; it may bring some things a little closer, but also makes the whole system even more complex.

*The real problem may actually be the semantic meaning of file-based and UI-based scripts differ. File-based scripts are run at startup and reload time (only) and register things that they want, like rules and triggers, somewhat like a system-based plugin may do. UI-based scripts are executed based on a specific action that occurs in OH, but their execution is typically ephemeral and stateless. I'm not sure that we've spent the time thinking about how these two things interact and complement each other - we are kind-of squinting and saying that File and UI doesn't matter and they are the same thing, but they're probably not.

ssalonen commented 2 years ago

Does the workaround in the first post work for thing status update triggers?

florian-h05 commented 2 years ago

Closing as we won‘t fix this here as this is related to openHAB Core.