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

[automation] Include ScriptExectution, PersistenceExtensions, etc. by default #2112

Closed
rkoshak opened this issue Jan 13, 2021 · 8 comments
Closed
Labels
automation enhancement An enhancement or new feature of the Core

Comments

@rkoshak
Copy link

rkoshak commented Jan 13, 2021

I suppose this is a feature request for the non-Rules DSL languages.

When writing rules there are a number of openHAB core actions that are commonly used by almost every user: createTimer, logging, executeCommandLine, say, Ephemeris actions, Persistence actions, HTTP actions, etc. Some of them, like the logging actions, would potentially be used in almost every rule.

Already there is a bunch of openHAB stuff included in the rule's scope: events, itemRegistry, event for Item triggered rules, the enum type States/Command (e.g. ON, CLOSED, INCREASE, etc). And in rules DSL a lot of these actions are included by default as well. I think that stuff is all defined in the default ScriptExtension, right?

Would it be possible to add the core extensions and actions that are not already in the default into the rule's scope without requiring the user to explicitly import them? If you can have the itemRegistry available without an import I figure the rest should be possible too.

Ideally I'd like to see them all including the MetadataRegistry and RulesRegistry (for advanced rules writers) but at a minimum I'd like to see the same set that is available in Rules DSL. I don't think it's a big deal if you have to reference the containing class (e.g. ScriptExectution.createTimer as opposed to just createTimer in Rules DSL). But needing to var ScriptExecution = Java.type("org,.openhab.blah.blah.blah.ScriptExecution"); in every rule that creates a timer seems unnecessary to impose on the user.

Currently, even with my knowledge of openHAB rules development, I have to have the JavaDocs open in one page so I can remember the package names and class names where all these actions are provided so I can import them.

Another viable approach could be to add the common ones to the script's scope and then offer some additional ScriptExtensions to include some of the more advanced stuff. There are already some RuleRegistry related ScriptExtensions, maybe add a similar ScriptExtension for MetadataRegistry, PersistenceExtensions (or is this already a ScriptExtension?), and the like.

@Confectrician
Copy link
Contributor

Related: #2056

@Cossey
Copy link
Contributor

Cossey commented Jan 18, 2021

I completely agree 1000%.

I moved from OH 2.5 to OH 3.0 recently. All my rules were written in DSL and then moving over to the new rule engine and rewriting them in javascript was very difficult for me, and I'm a programmer!

Something as simple as trying to send a push notification via the cloud connector eluded me for the first 8 hours. As the documentation is quite poor it wasn't obvious and spending time looking at threads about the non-DSL engine stuff took way to long. (The more comprehensive ones appear to be authored by @rkoshak).

Most of the common classes shouldn't need to be imported and should just "be there" like in DSL engine, even if you have to type the class name before the function now. I could compromise with scriptExtension if there were more presets to reflect real-world use cases. Like a preset for notifications and messaging.

After I port over all my rules, I think I might work on some of the existing documentation pages that are lacking automation equivalent examples.

In the end I settled with my own library.js I reference with load as well as a common.js that has convenience functions that apply across my rules.

@jpg0
Copy link
Contributor

jpg0 commented Jan 29, 2021

This is a great discussion to have, because for my use cases I 100% want the opposite!

I initially started with JS having all this stuff imported for me by default, and whilst it was fine for simple scripts, once they become more complex I repeatedly tripped over these 'defaults'. Pretty much every programming environment that I have used requires explicit importation of foreign symbols, because otherwise the 'magic' will bite you at some point. Examples where I have been bitten:

  • A name clash with something imported led to really weird behaviour because the type of the object was not what I was expecting
  • Something which I assumed to be undefined to denote that I had not yet set it was not
  • Imports changed in a version, introducing the above problems
  • Any of the above happen in 3rd party code that I import, where I don't have control over the variable names

I found the debugging these was a complete nightmare because I would spend ages trying to find my error, to discover that my code worked fine outside OH, but not inside, to finally realising that it was some 'magic' the OH had injected into my environment without me seeing it.

Saying this, I'm also sympathetic to those who want very simple scripts with minimal boilerplate. I therefore believe that the path forward should be to enable both use-cases. There are a few ways to do this:

  • Inject everything for scripts defined in the UI, but not on the filesystem
  • Some other toggle between script types, or (even better) define the imports outside the script itself
  • Put all extensions into a single, predefined namespace (so for example, oh.Exec rather than Exec) - this reduces clashes for my use-case, but does not eliminate them, it could be extended with a single import/include to get this oh root variable

Anyway, I wanted to call this out because (like pretty much every programming language), whilst this magic may be desirable in the very simple cases, it's certainly not once the things get more complex.

@rkoshak
Copy link
Author

rkoshak commented Jan 29, 2021

Inject everything for scripts defined in the UI, but not on the filesystem

I worry about this approach because we've already exceeded the "do it this way when X but do it that way when Y." Frankly, the area of rules development has already started to fracture to the point of incomprehensibility. This would just lead to one more case.

Put all extensions into a single, predefined namespace (so for example, oh.Exec rather than Exec) - this reduces clashes for my use-case, but does not eliminate them, it could be extended with a single import/include to get this oh root variable

I could get behind this approach, so long as it's consistent across all the languages (except Rules DSL for historic reasons).

whilst this magic may be desirable in the very simple cases, it's certainly not once the things get more complex.

We can explore this one a little bit though. Let's remember that we are working in the context of openHAB Rules. They are running on openHAB, triggered by events from openHAB, using data from openHAB, and interacting with openHAB. How pure of an environment is needed for 99.9% of the use cases? How likely is it that "things get more complex" to the vast majority of openHAB users? Why is it easier to call an action from a binding like publishMQTT than it is to call an action that is built into the core?

I ask because it seems like the argument is that usability for the vast majority of the developers of openHAB rules should be sacrificed to support a few rare use cases that would only be encountered by users like yourself who are doing things with with openHAB rules that the majority of openHAB users would likely not even comprehend, let alone try to replicate. Put another way, why must thousands of users have to import the logger every time because there is one JS library out there somewhere that uses logInfo too that maybe a few dozen people might want to use?

Perhaps I'm underestimating the number of users who would want to use third party libraries in their rules but I don't think so.

I'm not unsympathetic to the problem. But, based on my experience on the forum, it seems to be a pretty big cost to impose on our users for the benefit of very few. And there are alternative approaches to providing a pure environment for those users who want it like the HABApp approach.

And we already have all sorts of stuff injected: event, events, rules, actions, things, ir, items, and so on and so on. Why do I have to import ZonedDateTime and ScriptExecution when more than half of the rules most people write have to create Timers? Why do I have to import the logger?

@jpg0
Copy link
Contributor

jpg0 commented Jan 30, 2021

I don't disagree with your statement that the vast majority of users will want things predefined, because most scripts are trivial. Also, I'm not arguing here against adding a handful of extra things, I'm more concerned about the pattern how things are injected in general.

I can tell you exactly why I want to import the logger each time I write a script: because if I do not, any piece of code that I import that uses a variable called logger (which for 3rd party code is likely to be a lot) will break in weird ways. This is the same for the things that you mention; in fact event and events are used so prolifically in JS that the majority of 3rd party code will not work (I now remember that this was a particularly bad offender when trying to write JS code, as well as some of the injected unit definitions like ON, OFF, NULL etc). I already use a number of 3rd party libraries - such as colour manipulation (chroma.js) and displaying human-readable descriptions for cron expressions (cronstrue).

The way I see this is a conflict between the usage of 3rd party code versus reduction in boilerplate for scripts. If we force these things to be injected, then we diverge from the rest of the ecosystem for the language in question. This is why I'm thinking that the best option is to try to minimise boilerplate whilst retaining compatibility with the ecosystem. My preferred option would be to package everything from OH into a single root object which can be imported in a single line. This is how I have built the https://github.com/jpg0/ohj helper library:

let ohj = require('ohj');
//now I can use ohj.actions, ohj.rules, etc

To me, this represents the smallest set of boilerplate possible whilst retaining full compatibility. One of the options that I mentioned above was to inject only this require line, which would mean that only a clash on the name (ohj in this case) would cause problems. I do appreciate that we want to minimise bifurcation of rules configuration. The best way that I can think of to maintain full compatibility but make things easy for trivial scripts is to template the script blocks in the UI, such that they include a set of standard imports by default (we can even collapse them in the UI if we want). Most users will ignore this, but power-users have the option to delete the imports if they like.

Another angle I'd like to raise is the hope that over time the libraries that users are building on move up to higher level concepts. For example one possible angle is fluent statements such as the ones I use to configure my buttons https://github.com/jpg0/oh-config/blob/master/automation/jsr223/javascript/personal/buttons.js or schedules https://github.com/jpg0/oh-config/blob/master/automation/jsr223/javascript/personal/schedules.js. Other directions that allow wiring behaviour together in different ways (such as a dependent states) would be useful. All of these higher-level APIs would be in general be interacting with these (base-level) injected objects, and not the scripts themselves, which would mostly be an inconvenience for them.

Ultimately what I'm trying to do to preserve compatibility with the standard ecosystem, so that I (and others) can (a) write code as they would normally do and (b) use existing code.

@cweitkamp cweitkamp added automation enhancement An enhancement or new feature of the Core labels May 22, 2021
@J-N-K
Copy link
Member

J-N-K commented Mar 19, 2022

The discussion seems to have stopped a year ago. How would we like to proceed here? I tend to agree with @jpg0: Explicit imports are preferred.

@digitaldan
Copy link
Contributor

digitaldan commented Mar 19, 2022

We ended up making script injection (default requires) on by default (which exports the openhab-js library), but can be disabled as a configuration option for power users. Probably safe to close this one.

@rkoshak
Copy link
Author

rkoshak commented Mar 28, 2022

I think this is solved for JS Scripting but it might still be relevant to the other languages. But if no one from the other languages chime in I'm OK with closing it. It's probably not relevant for Java rules nor Groovy. I've no idea what jRuby does. It's relevant to Jython and Nashorn JS but am not sure how much effort is worth spending on those right now.

Explicit imports are preferred.

We had a long conversation about this on openhab-js. Explicit imports are fine and good when writing rules in text files because you can have lots of rules in the same file share the same imports. Three to five lines of code at the top of each file is no big deal.

In the UI it becomes very burdensome because you can't even have a set of imports for even the whole rule. It's only for each individual script. So if you have a Script Condition and Script Action, you might have to import the same stuff twice for the same rule. 30-50% of the code one would have to write in UI scripts ends up being imports.

So JS Scripting struck a compromise. The Helper Library comes with the add-on which, by itself, removes the need for a whole lot of imports since it wraps most of the Java stuff with JS. And there is an option to auto import the library or not based on the end user's choice. So users who are just doing light scripting for their rules in the UI are not burdened with a huge amount of imports just to do a one liner to send an alert or command an Item but power users who are using 3rd party npm libraries and such can also manage the imports as they need.

@J-N-K J-N-K closed this as completed Apr 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automation enhancement An enhancement or new feature of the Core
Projects
None yet
Development

No branches or pull requests

7 participants