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

[jsscripting] openhab-js integration #11656

Merged
merged 26 commits into from
Dec 13, 2021
Merged

Conversation

digitaldan
Copy link
Contributor

@digitaldan digitaldan commented Nov 28, 2021

This PR integrates @jpg0 excellent OHJ helper library, which is now part of https://github.com/openhab/openhab-js and provides an out of the box environment for users to write Javascript based rules without needing to access raw Java types. Scripts can now be written like

items.getItem("KitchenLight").sendCommand("ON");

without any other imports or logic

This pr also enables node.js style console logging (console.log, console.debug...) as well as common javascript setTimeout support .

Todo

  • Update documentation
  • Update common logging name for user scripts and add to openhab-core default log config (script.js right now)
  • Add a simpler path for user scripts (deprecate the jsr223 path)

Fixes openhab#11222

Signed-off-by: Dan Cunningham <dan@digitaldan.com>
Signed-off-by: Dan Cunningham <dan@digitaldan.com>
Signed-off-by: Dan Cunningham <dan@digitaldan.com>
Signed-off-by: Dan Cunningham <dan@digitaldan.com>
Signed-off-by: Dan Cunningham <dan@digitaldan.com>
Signed-off-by: Dan Cunningham <dan@digitaldan.com>
Signed-off-by: Dan Cunningham <dan@digitaldan.com>
Signed-off-by: Dan Cunningham <dan@digitaldan.com>
Signed-off-by: Dan Cunningham <dan@digitaldan.com>
Signed-off-by: Dan Cunningham <dan@digitaldan.com>
Signed-off-by: Dan Cunningham <dan@digitaldan.com>
Signed-off-by: Dan Cunningham <dan@digitaldan.com>
Signed-off-by: Dan Cunningham <dan@digitaldan.com>
Signed-off-by: Dan Cunningham <dan@digitaldan.com>
@digitaldan digitaldan added the work in progress A PR that is not yet ready to be merged label Nov 28, 2021
}
};

function setTimeout(cb, delay) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally we probably should also have a polyfill for setInterval!?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just added this in, also i changed both types of timers to support arguments to better confirm with standards.

globalThis.setTimeout = setTimeout;
globalThis.clearTimeout = clearTimeout;
globalThis.global = globalThis;
globalThis.process = { env: { NODE_ENV: '' } };

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I added this in my scripts, this was to prevent npm libraries from crashing when they access process.env.NODE_ENV. For which this is enough. But most often this is used to check if it equals 'production', in which case libraries often emit less/no debug logging, type checks, or do other shortcuts to optimize performance.

So there is a tradeoff here basically between DX and performance. I would tend to favor DX in our case, but should we want to prioritize perf, then this could be changed to:

Suggested change
globalThis.process = { env: { NODE_ENV: '' } };
globalThis.process = { env: { NODE_ENV: 'production' } };

Signed-off-by: Dan Cunningham <dan@digitaldan.com>
Signed-off-by: Dan Cunningham <dan@digitaldan.com>
Signed-off-by: Dan Cunningham <dan@digitaldan.com>
Signed-off-by: Dan Cunningham <dan@digitaldan.com>
@wborn wborn added the enhancement An enhancement or new feature for an existing add-on label Dec 5, 2021
… bit. This also includes an upcoming PR for cache support, which i will remove from here after it gets merged.

Signed-off-by: Dan Cunningham <dan@digitaldan.com>
Signed-off-by: Dan Cunningham <dan@digitaldan.com>
Signed-off-by: Dan Cunningham <dan@digitaldan.com>
Signed-off-by: Dan Cunningham <dan@digitaldan.com>
Signed-off-by: Dan Cunningham <dan@digitaldan.com>
Signed-off-by: Dan Cunningham <dan@digitaldan.com>
@digitaldan digitaldan removed the work in progress A PR that is not yet ready to be merged label Dec 13, 2021
@digitaldan
Copy link
Contributor Author

Hello everyone, this is no longer a WIP and is ready for review. All required core changes have been made.

There is still openhab/openhab-core#2595 that should be merged as well (but this PR does not depend on it).

Copy link
Member

@kaikreuzer kaikreuzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thank you!

*/
@Component(service = ScriptEngineFactory.class)
@Component(service = ScriptEngineFactory.class, configurationPid = "org.openhab.automation.jsscripting", property = Constants.SERVICE_PID
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our configuration pids follow the pattern org.openhab.<serviceid>.

…penhab/automation/jsscripting/internal/OpenhabGraalJSScriptEngine.java
@kaikreuzer kaikreuzer merged commit 4481ecf into openhab:main Dec 13, 2021
@kaikreuzer kaikreuzer added this to the 3.2 milestone Dec 13, 2021
NickWaterton pushed a commit to NickWaterton/openhab-addons that referenced this pull request Dec 30, 2021
Fixes openhab#11222

Signed-off-by: Dan Cunningham <dan@digitaldan.com>
Signed-off-by: Nick Waterton <n.waterton@outlook.com>
mischmidt83 pushed a commit to mischmidt83/openhab-addons that referenced this pull request Jan 9, 2022
Fixes openhab#11222

Signed-off-by: Dan Cunningham <dan@digitaldan.com>
Signed-off-by: Michael Schmidt <mi.schmidt.83@gmail.com>
nemerdaud pushed a commit to nemerdaud/openhab-addons that referenced this pull request Jan 28, 2022
Fixes openhab#11222

Signed-off-by: Dan Cunningham <dan@digitaldan.com>
volkmarnissen pushed a commit to volkmarnissen/openhab-addons that referenced this pull request Mar 3, 2022
Fixes openhab#11222

Signed-off-by: Dan Cunningham <dan@digitaldan.com>
marcfischerboschio pushed a commit to bosch-io/openhab-addons that referenced this pull request May 5, 2022
Fixes openhab#11222

Signed-off-by: Dan Cunningham <dan@digitaldan.com>
andan67 pushed a commit to andan67/openhab-addons that referenced this pull request Nov 6, 2022
Fixes openhab#11222

Signed-off-by: Dan Cunningham <dan@digitaldan.com>
andrasU pushed a commit to andrasU/openhab-addons that referenced this pull request Nov 12, 2022
Fixes openhab#11222

Signed-off-by: Dan Cunningham <dan@digitaldan.com>
Signed-off-by: Andras Uhrin <andras.uhrin@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature for an existing add-on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants