scriptable automation

[smerschjohann]

This PR replaces #803, #806 and #813

It provides the ability to write Rules/ActionHandlers/Triggers directly from script files.

To accomplish this, this PR introduces:

1. A script loader that can load scripts from a directory and bundles

2. An import mechanism to allow bundles to provide script dependant classes for scripts. This means: scripts can import class instances via a default interface where the imported instances can have a reference to the scriptEngineId. This allows the unloading of Rules etc. if the script becomes unavailable. This extension mechanism is for example used to provide the following:

   ScriptExtension.importPreset("RuleSupport")
   ScriptExtension.importPreset("RuleSimple")
   

3. The RuleSupport and RuleSimple presets provide the ability for scripts to programmatically add and remove Rules and Modules to the ScriptEngine. By providing the ScriptedRuleProvider, these Rules will only be kept at runtime and will not be persisted on reboot except the Rule will be explicitly added by addPermanent. For an example script, see: distribution/smarthome/conf/scripts/test.py

[kaikreuzer]

Ok, here's some initial feedback/questions.

First of all, you (again) did a tremendous job - I easily got it all working with Jython, although I have never used Jython before in my life :-)

I must admit, that I do not yet fully understand the whole architecture. What would help me is, if your could briefly describe what the purpose of these bundles exactly is for you:

• module.script
• module.script.defaultscope
• module.script.extensions.rulesupport
• module.script.loader

It feels that

• the module.script.loader should rather be a parser.script
• parts of module.script.extensions.rulesupport should go into module.script as this bundle was meant to provide the general jsr223 support
• parts of module.script.extensions.rulesupport (the "types") should go into module.script.defaultscope as this bundle was meant to provide the services and classes that should be available throughout scripts.
• it would be nice to have additional, optional bundles like module.script.jython or module.script.groovy, which would include the libraries required for those languages, so that stuff like this wouldn't be necessary anymore.

Please also check the code again against the guidelines: I noticed specifically A.9 (considering #1568), B.4, B.6, C.1.

I'll go deeper into the code, once we have agreed on the bundle structure - thanks!

[smerschjohann]

What would help me is, if your could briefly describe what the purpose of these bundles exactly is for you

In general, I have divided the scripting functionality in multiple parts and made the scripts as modular as possible, by allowing them to only load the parts that they need.

module.script

This was meant as general basis to load script-engines using jsr223

module.script.defaultscope

These are types/classes/objects which are ALWAYS injected to the script engines scope and should therefore be kept small since by the introduction of the ScriptExtensions, the wanted type and object sets can be imported when required.

module.script.extensions.rulesupport

This is the script extension that allows to write rules using scripts. I moved it to an extension instead of importing them into the default scope by default to allow different extensions to coexist as well as to use the script engines without this support at all. It would for example not make much sense for the ScriptActionHandler, already introduced by you, to have these classes by default.

It further allows that the Openhab2 or other implementations to offer slightly different interfaces or even a Openhab 1 compatibility layer without interference to other scripts as each script can load the extensions it requires. Therefore, scripts can coexist with other scripts depending on another set of implementations.

This RuleSupport extension already consists of two presets: RuleSupport (always required if one wants to write rules) and SimpleRule (which tries to make writing rules easier).

module.script.loader
the module.script.loader should rather be a parser.script

This module allows the loading of scripts by bundle or from a filesystem directory. It is not named as parser as it really is not a parser but a loader ;) A parser would only "statically" parse a file and import it into a predefined structure etc. But in this context, the files are read, yes, but then passed on to the correct script engine and loaded into it. Therefore, I think the name parser does not fit here.

parts of module.script.extensions.rulesupport should go into module.script as this bundle was meant to provide the general jsr223 support

I wanted to divide the general jsr223 support (which is also used for simple ActionHandler triggering) from explicit rule writing support, therefore I moved those components to the newly introduced extension.

parts of module.script.extensions.rulesupport (the "types") should go into module.script.defaultscope as this bundle was meant to provide the services and classes that should be available throughout scripts.

Same here: a script explicitly import the wanted set of types e.g. by ScriptExtension.importPreset("RuleSupport") instead of always cluttering the default scope even if the script aims at something completely different.
Another preset could for example provide the possibility to implement bindings which do need a different set of types and could be imported by e.g. ScriptExtension.importPreset("BindingSupport")

it would be nice to have additional, optional bundles like module.script.jython or module.script.groovy, which would include the libraries required for those languages, so that stuff like this wouldn't be necessary anymore.

I agree, if the license is compatible with the the eclipse smarthome project, I can prepare a module for that

Please also check the code again against the guidelines: I noticed specifically A.9 (considering #1568), B.4, B.6, C.1.

Ok, I will do that. But why do you still require at most JavaSE 7? It is not even supported by Oracle anymore.

[kgoderis]

I will try to have a look as well before I leave on holidays, but certainly I will wait for this PR to be integrated in the repo before I start the refactoring of what I developed some time ago (see previous threads/discssions). My stuff is a bit outdated and I do not want to clutter things.

[kaikreuzer]

module.script.extensions.rulesupport

This is the script extension that allows to write rules using scripts.

What does "write" mean here exactly? And what is it concretely offering? It only consists out of internal classes, so it is no API for anybody. It only has one DS component called "loaderscriptscope" which from its name rather sounds application to module.script.loader.

two presets: RuleSupport (always required if one wants to write rules) and SimpleRule (which tries to make writing rules easier).

I don't yet understand what those "presets" are and how they are used. Do you have a pointer for me?

the module.script.loader should rather be a parser.script
This module allows the loading of scripts by bundle or from a filesystem directory.

Well, my point was that there already IS a loader for resources in bundles (from which you have duplicated a lot of code, which is in general not nice). This existing loader passes the content as-is to registered parsers, which is then supposed to spit out a set of rules. That's why I was wondering, if this existing infrastructure and API could be used here? After all, this is exactly their purpose.

I wanted to divide the general jsr223 support (which is also used for simple ActionHandler triggering) from explicit rule writing support, therefore I moved those components to the newly introduced extension.

What is the difference for "rule writing support"? Is it, that the whole chain of trigger/action/condition is executed within the script engine itself and thus the "normal" rule engine is not at all involved anymore? Does it have to be this way?

Let me challenge you with some requirements to make it maybe a bit clearer, why I ask those questions:

• I would like to be able to use Jython etc. as well in the existing ScriptAction/ScriptCondition and we should also add a ScriptTrigger.
• I would like to have Xbase module handlers and a DSL rule parser that can read the existing (old) *.rules files and creates a set of Rule instances from it.
• I would like to be able to create rule templates that contain scripts in any language
• I would like to be able to see the structure of a rule (trigger/condition/action) in the Paper UI, even if it comes through "rulesupport".
• I would like to define a new "rule" file format, as a successor of the *.rules files, that allow to reference other module types (available in the system, not scripted) and potentially other scripting languages than Xbase. Think of it as a "nicer" (easier writable) version of the JSON rule definitions.

For these reasons, I am trying to get your work into the existing framework as smoothly as possible - so that it isn't just some special optional feature, but that its power can be used in many different use cases.

Another preset could for example provide the possibility to implement bindings which do need a different set of types and could be imported by e.g. ScriptExtension.importPreset("BindingSupport")

Oh hell, so this is actually NOT about the automation component at all, but about a general integration of scripting code into the runtime? Then we should think of an even bigger refactoring to get some stuff out of the "automation" namespace.

I agree, if the license is compatible with the the eclipse smarthome project

Jython seems to have a pretty custom license, so I doubt that we will get that into ESH - but it should be fine to add such a bundle in OH2 then. Groovy instead is definitely fine in ESH, so we could have JS and Groovy support here and adding Jython in OH2.

But why do you still require at most JavaSE 7?

Because especially on embedded hardware even Java 5 is still used by many people. Nonetheless, we plan to move to Java 8 soon, but as it will require to switch the whole build, it would be good if your code still compiles with Java 7 for now - as it is just two lines using String.join, that should be fairly simple :-)

[kgoderis]

I would like to have Xbase module handlers and a DSL rule parser that can read the existing (old) *.rules files and creates a set of Rule instances from it.

@kaikreuzer That is exactly my (to be refreshed) PR is doing, with the the Xtext script running as an Action, and the various .rules trigger being mapped to a (new) set of Triggers

[kaikreuzer]

@kgoderis, right, looking forward to it :-)
So I would appreciate, if you could also have a look at this PR and what you think how this can be achieved best, having this goal in mind.

[kgoderis]

This is quite a chunk of code. I am not sure I have understood everything here, but here are some questions:

1. Is ScriptExtensionProvider not the same as ScriptScopeProvider? I understand that you do not want to clutter de DefaultScopeProvider, but you could create other ScopeProvider(s) to support the Scripts no? To that extend, should we not introduce an intermediate mechanism to manage the Contexts themselves, based on javax.script.SimpleScriptContext?
2. Like Kai I was wondering why you introduced a new resource loading mechanism?
3. Since .rules are in fact a kind of "decorated" scripts, should I then refactor the .rules contribution as an extension of the Scripted.... code? Or shall we keep it completely apart?
4. Why did you not turn ScriptEngineProvider into an OSGi service? It will prevent problems with those script engine that rely on the ContextClassLoader for finding resources (for example, this is the case for Ruby!)
5. for .rules it is necessary to be able to get the IEvaluationContext so that variables can be shared between rules. Not sure if this also necessary for Scripts, but maybe this need to be included
6. Will there be a ConsoleCommandExtension for scripts?

The rest I need to revisit later today

[smerschjohann]

@kaikreuzer:

What does "write" mean here exactly? And what is it concretely offering? It only consists out of internal classes, so it is no API for anybody.

the internal only relates to the OSGI model, it provides the functionality to the scripts which is not OSGI.

I don't yet understand what those "presets" are and how they are used. Do you have a pointer for me?

Yes, LoaderScriptExtension.java provides three different presets and they are used in the script like:

ScriptExtension.importPreset("RuleSupport")
ScriptExtension.importPreset("RuleSimple")

to load and instanciate the objects needed for this preset. This model is not currently possible with the default scope, as this does not allow to instantiate classes for a particular script. But using this feature, it is e.g. possible to unregister rules on script deletion/modification.

That's why I was wondering, if this existing infrastructure and API could be used here? After all, this is exactly their purpose.

hm, yeah by refactoring the original code, this can be possible. But some needed features were missing at the time of writing in december and I did not want to rewrite the other parts as well at that time. But yes, this would be a task of a first refactoring. Specifically, context information to implement unloading behavior was missing if I remember correctly.

What is the difference for "rule writing support"? Is it, that the whole chain of trigger/action/condition is executed within the script engine itself and thus the "normal" rule engine is not at all involved anymore?

No that is not the case, there is the SimpleRule, which makes writing rules easier if you don't want to expose the action part as a separate action. In this case the Action is "hidden" by a unique id using the ScriptedAction.

Nevertheless, as it can be seen in the example, it is possible to write actions that are normally registered to the automation engine and therefore are exposed as new action to the engine.
This is also the case with the other chain elements.

This would allow the user to first write SimpleRules, refactor them accordingly where it fits, bundle them in a OSGI bundle and provide them as a jar archive.

Oh hell, so this is actually NOT about the automation component at all, but about a general integration of scripting code into the runtime? Then we should think of an even bigger refactoring to get some stuff out of the "automation" namespace.

No reason to swear ;) Yes, it allows scripting everything with the right extensions or with additional effort on the script side. And you are right, it really does not have to be in the automation namespace ;)

Jython seems to have a pretty custom license, so I doubt that we will get that into ESH - but it should be fine to add such a bundle in OH2 then. Groovy instead is definitely fine in ESH, so we could have JS and Groovy support here and adding Jython in OH2.

Actually, I would not like that separation as it would mean that jython degrades to a lower "supported" level. Yes, it is custom. But on the other side, comparable to a BSD license, just by reading it, and also stated on the wikipedia page where is states it is FSF compliant. I guess it would be fine to try getting the approval from eclipse.

@kgoderis:

1. Is ScriptExtensionProvider not the same as ScriptScopeProvider

No, it provides an import mechanism, where on the Java side, the classes can be instantiated based on the unique id of the script.

1. Like Kai I was wondering why you introduced a new resource loading mechanism?

Because the current implementation did not fit nicely, after first general acceptance and refactoring, it can be exchanged quiet easily...

1. Since .rules are in fact a kind of "decorated" scripts, should I then refactor the .rules contribution as an extension of the Scripted.... code? Or shall we keep it completely apart?

Why is a rule a decorated script? Even the .rule part should contain at least triggers and actions. But yes, in my opinion, it would make sense to generalize them.

1. Why did you not turn ScriptEngineProvider into an OSGi service?

Patches are welcome.

1. for .rules it is necessary to be able to get the IEvaluationContext so that variables can be shared between rules.

This is not necessary for scripts as rules implemented in the same file will share the same context. But isn't this just an implementation detail of your rule to automation wrapper?

1. Will there be a ConsoleCommandExtension for scripts?

good idea, why not?

[kgoderis]

Since .rules are in fact a kind of "decorated" scripts, should I then refactor the .rules contribution as an extension of the Scripted.... code? Or shall we keep it completely apart?
Why is a rule a decorated script? Even the .rule part should contain at least triggers and actions. But yes, in my opinion, it would make sense to generalize them.

by decorated, I mean that from an Xtext point of view it is in essence a Script, with some DSL for triggers and so forth.

[kgoderis]

Is ScriptExtensionProvider not the same as ScriptScopeProvider
No, it provides an import mechanism, where on the Java side, the classes can be instantiated based on the unique id of the script.

mmm... would it not be more logical than to have a new class that extends ScriptScopeProvider, and that provides the class instantiation functionality?

[smerschjohann]

ScriptScopeProviders are meant to be always imported in the scripts, at least in the current state. This is the main fact, that I don't want here.

[kgoderis]

@smerschjohann @kaikreuzer Would it make sense to commit the PR to a new dev branch so that we can prepare aux. PR's on it?

[danchom]

I'm not in details in script engines and to be fair I don't understand fully this PR. For me the script handler has to support script conditions and script actions as it does at the moment. They gives possibility to write conditions base on the state of the system and to change the state of the system through the rule execution.
To have possibility to write scripts on different languages is good option for me, but I'm not sure how fat are these script engines and if they are applicable in embedded device scenario.
To manage scripts in system may be is good option too because to embed complex scripts into json files is not suitable. The script conditions/actions have to support references to external complex scripts.
But I don't understand the feature to write a new rules through the script. Is this some kind of AI is it? I understand that you want to have opportunity to convert scripts of old rule engine to the new one but why this complex task has to be done by the script? Isn't be easier to have a special rule provider, written in java, which will register old rules into new rule engine?
Anyway this PR does not change the automation core modules and if you think it is suitable for you feel free to merge it.

[smerschjohann]

actually, (some) script engines are not much more resource hungry than java itself, so that should not be the main issue here.

I think the steps for rule creation are small enough to be directly integrated. So I think the benefit to have it directly in the scripts is higher than wrapping it by a limited "Converted"RuleProvider. Yes, you might even think about implementing an AI, the possibilities should not be limited by an unnecessary mapping.

Nonetheless, as @kaikreuzer already said, it might be a good idea to look at the namespaces again to find the correct integration point for smarthome.

[kaikreuzer]

Nonetheless, as @kaikreuzer already said, it might be a good idea to look at the namespaces again to find the correct integration point for smarthome.

I'll try to come back to you on this soon again as I definitely want to move this forward as well. Thanks for your patience meanwhile!

[spacemanspiff2007]

Anyway this PR does not change the automation core modules and if you think it is suitable for you feel free to merge it.

Hi, what is the current state on the PR for JSR223 support?

[smerschjohann]

Well, I stopped working on it, since the feedback is low. This PR was working as is, but as the feedback, amongst other, suggested namespace changes, I waited for a full review as it can be quiet cumbersome to change only a single thing at a time. Especially, if the whole picture is not clear.

Since I have a few free days now, I am hopefully able to migrate my current openhab1 setup to openhab2 now. With this done, I can offer a test build with that feature enabled, if you are willing to give feedback on the general functionality of this feature.

[spacemanspiff2007]

This would be lovely. The Jython support is the only thing holding me back.
Thank you for you work @smerschjohann !

[kaikreuzer]

I really have to apologize @smerschjohann and I am glad that you are still patiently waiting and didn't give up yet.

I'll try to find 1-2 hours over christmas to go through my open questions again and summarize what I think should be refactored. Also note that there were quite some changes on the rule engine itself in the meantime, so this might also require some adaptions.

[kaikreuzer]

Ok, I will try to recap what should be done from what I understand from the discussion:

• refactor module.script.loader to module.parser.script and use the existing resource loading mechanism
• ScriptExtensionManager should be registered as a DS component (we by now use DS and no service trackers anymore)
• ScriptScopeProvider interface should be removed as we now have the ScriptExtensionProvider interface as a replacement
• DefaultScriptScopeProvider should be refactored to a CoreScriptExtensionProvider. I would suggest that ScriptExtensionProviders should also take a configuration via DS, so that their default presets can be configured externally (and thus can be different for different solutions). With this in place, I would then also suggest to completely remove the bundle module.script.defaultscope and have the CoreScriptExtensionProvider moved to module.script.
• Provide a module.script.groovy bundle that brings the required jars and makes additional installation by the user unnecessary
• RuleSupportActivator should be removed, DS to be used instead
• Wrt "further presets e.g. for binding dev": I'd suggest to keep the rulesupport bundle as is for now and have such discussions to extend the use of scripts outside of the rule engine to a later point. As nothing is exposed by this bundle, a future refactoring should always be easily possible.

Does this make sense to you, @smerschjohann?

[kaikreuzer]

Actually, I would not like that separation as it would mean that jython degrades to a lower "supported" level. Yes, it is custom. But on the other side, comparable to a BSD license, just by reading it, and also stated on the wikipedia page where is states it is FSF compliant. I guess it would be fine to try getting the approval from eclipse.

I have checked this. It is fine to use Jython as a works-with dependency, i.e. use it, if it is available on the users machine, but we would not be able to distribute the binary ourselves from Eclipse. Having this huge software IP cleared would be an immense effort and I doubt it is worth it. We could add such a module.script.jython bundle in the openhab-core repo, though. For the openHAB users, there would be no "lower supported level", it would be all the same as groovy. All other ESH-based solutions are also free to decide for some way to pre-install Jython, if this is of any interest to them.

[steve-bate]

@petrklus Yes, this is the PR. I've just added a link to this PR in the openhab2-jython README.md.

[lewie]

@steve-bate, @smerschjohann,

is there a way to access action services like mail or XMPP in scriptable automation?
In jsr223 OH1 for example we get a list with:
oh.getActions()

And for example how to access persistence extensions?
In jsr223 OH1 it is like:

PersistenceExtensions.changedSince(item,date);
PersistenceExtensions.persist(item);
...

Thanks

EDIT:
Got the Action Services now over: Java.type("org.eclipse.smarthome.model.script.ScriptServiceUtil").getActionServices()
;-)

[lewie]

Based on @steve-bate first Jython examples, realized a bit simple JavaScript Code. It is experimental, but it works nearly like in OH1 JSR223 binding.

My first greatest concern is to migrate existing JS from OH1 to ESH automation with this JS tests.

@smerschjohann, distinctions from JavaScript view:

• In jslib/triggersAndConditions.js tried to build a StartupTrigger in JS according to Steve's original, no luck:

13:39:07.601 [INFO ] [smarthome.event.RuleAddedEvent      ] - Rule 'SimpleRule__NashornJavaAdapter_9b9be3bb-b84b-4175-bd67-c4213071a8b3' has been added.
13:39:07.601 [INFO ] [smarthome.event.RuleStatusInfoEvent ] - SimpleRule__NashornJavaAdapter_9b9be3bb-b84b-4175-bd67-c4213071a8b3 updated: INITIALIZING
13:39:07.601 [INFO ] [smarthome.event.RuleStatusInfoEvent ] - SimpleRule__NashornJavaAdapter_9b9be3bb-b84b-4175-bd67-c4213071a8b3 updated: UNINITIALIZED (HANDLER_INITIALIZING_ERROR): Missing handler 'jsr223.StartupTrigger' for module '3771bec8-d83e-47e0-8a68-326fd4bc4ede'

• Doesn't it make sense, to provide ESH triggers via java? I miss StartupTrigger and ShutdownTrigger yet.
• Most main types are accessible directly, so org.eclipse.smarthome.core.types.UnDefType should too. It is a often occurring condition. Others like NextPreviousType, PlayPauseType, RawType, RewindFastforwardType, StringListType are nice to have, can be imported if needed.

From the user's point of view, it is great to work with new JSR223 on OH2, especially the access to much more classes as is possible in OH1. It goes far beyond the possibilities so far, it can be done many more prototyping things now. I'm looking forward to packable scripts.

[smerschjohann]

@kaikreuzer

Is dynamic loading a must and if so, what is your plan to do it?

Yes, I think it is a must. I have a look in to it, I think I am able to do it this week.

@lewie as you found a way already, I will not include that into the default namespace, neither as an extension. Might come in the future, but I think I shouldn't change any "minor" things right now.

@lewie and @steve-bate thank you for providing these examples which even contain other ideas, I did not even think about ;)

After I have made the last changes (hopefully this week), I would like to ask you two to test your scripts against the latest commit to verify that everything is still working or you can at least fix the incompatibilities that might arise due to the latest changes. I did not check it, but if I'm not mistaken, the jython examples are not working against the current head.

StartupTrigger etc. are not directly related to this PR, so separate PRs should either go to ESH or openhab 2.

[lewie]

@smerschjohann, of course, you're absolutely right, it's only trifles.
It is time that this PR is finally merged, now.

Thanks Simon for your second great JSR223 work!

[kaikreuzer]

@smerschjohann Good to hear that you'll continue working on this PR this week - we should be really close to a merge then!

StartupTrigger etc. are not directly related to this PR

This is absolutely right.
The reason why those do not exist yet is that I wanted to wait for #1896 before implementing them, to make sure that they function as expected (on the DSL rules, we have/had plenty of issues with the current "fuzzy" way of determining when the system is up).

[steve-bate]

@smerschjohann, I've been trying my scripts with the latest code. It seems like the code is working but some of my scripts are broken (as expected) because of naming changes. I'm working on fixing those, but I did see the following exception in the log and it seemed suspicious:

2017-04-08 09:19:56.114 [TRACE] [.m.s.i.ScriptEngineManagerImpl:131  ] - scriptUnloaded() not defined in script
2017-04-08 09:19:56.114 [INFO ] [.r.s.ScriptedAutomationManager:86   ] - removeAll added handlers
2017-04-08 09:19:56.117 [WARN ] [.c.c.registry.AbstractRegistry:149  ] - Could not remove element: nulljava.lang.NullPointerException: null
	at java.lang.String.replace(String.java:2240)
	at org.eclipse.smarthome.automation.events.RuleEventFactory.buildTopic(RuleEventFactory.java:167)
	at org.eclipse.smarthome.automation.events.RuleEventFactory.buildTopic(RuleEventFactory.java:171)
	at org.eclipse.smarthome.automation.events.RuleEventFactory.createRuleRemovedEvent(RuleEventFactory.java:146)
	at org.eclipse.smarthome.automation.core.internal.RuleRegistryImpl.postRuleRemovedEvent(RuleRegistryImpl.java:300)
	at org.eclipse.smarthome.automation.core.internal.RuleRegistryImpl.notifyListenersAboutRemovedElement(RuleRegistryImpl.java:362)
	at org.eclipse.smarthome.automation.core.internal.RuleRegistryImpl.notifyListenersAboutRemovedElement(RuleRegistryImpl.java:1)
	at org.eclipse.smarthome.core.common.registry.AbstractRegistry.removed(AbstractRegistry.java:147)
	at org.eclipse.smarthome.automation.module.script.rulesupport.shared.ScriptedRuleProvider.removeRule(ScriptedRuleProvider.java:59)
	at org.eclipse.smarthome.automation.module.script.rulesupport.shared.ScriptedRuleProvider.removeRule(ScriptedRuleProvider.java:54)
	at org.eclipse.smarthome.automation.module.script.rulesupport.shared.RuleSupportRuleRegistryDelegate.removeAllAddedByScript(RuleSupportRuleRegistryDelegate.java:111)

This happened when I removed the rule_decorators.py example script from the jsr223 directory.

[smerschjohann]

@steve-bate Sorry, I meant the changes including the changes I made this week. Could you check if this is still happening?

The latest change relies on #3218 to have support for dynamic module handlers.

@kaikreuzer I have changed the package of the FileWatcher and refactored that class a little bit as it had some conflicts with the latest head. I also added the ability to add custom handlers by updating the moduletypes accordingly. But due to some bugs in the head, I have opened another PR: #3218

Let's see if it can me merged now ;)

[kaikreuzer]

Thanks @smerschjohann, sounds good!
I couldn't easily see what changes were done exactly as you squashed the commits again - in such cases, having incremental commits can be useful to simplify the review process.
But assuming the moduletype update feature didn't mean any major structural changes, we should indeed be good now.
I am on holiday until Easter, so I would let these days pass to give @danchom and @maggu2810 a chance to have another look over it as well. If nobody vetoes, I would then create a CQ with the code at Easter and hand it over to the Eclipse IP team for approval.

[steve-bate]

Sorry, I meant the changes including the changes I made this week. Could you check if this is still happening?

@smerschjohann I reset my automation-scriptable branch to yours. Unfortunately, I'm not getting a clean build (issues with missing AbstractWatchQueueReader and BundleProcessorVetoManager interface changes) but it seems to work well enough to do some testing. I still see the previously reported exception. It seems to be related to a SimpleRule/Rule object having a null UID. The Rule default constructor has a comment that the UID will be set by the engine. Apparently it's not being set in some scenarios.

[smerschjohann]

@kaikreuzer hm, I still have the dev branch, but yes those changes were not that big. Mainly the scriptloading, scope variables and a slitly changed SimpleTriggerHandler.

@steve-bate actually I have those errors as well, but they don't come from any part of this code. Those problems seems to be in the current master. Nonetheless, they don't seem to influence this PR at all.

[smerschjohann]

I will have a look at that uid problem

[kaikreuzer]

I am on holiday until Easter, so I would let these days pass to give @danchom and @maggu2810 a chance to have another look over it as well.

Ok, so as there was no further feedback resp. any veto, I have created CQ 13331 for this PR and am waiting for approval through the Eclipse IP team - once we have it, we will finally merge this PR! So @smerschjohann please refrain from any changes on this branch from now on, thanks! You and @steve-bate might want to start with the preparation of some good documentation of this new feature!

[kaikreuzer]

Great news, everyone, the CQ has already been approved!
I'll now work on packaging this feature nicely on p2 and Karaf as well and make it available asap in the openHAB snapshot distro as well - so @smerschjohann & @steve-bate start typing the documentation and expect people to come up with questions soon!

[kaikreuzer]

FTR: #3330

[Marty56]

Great achievement! I am eager to try it!!
Does build 901 already contain this code?

[maggu2810]

@smerschjohann WRT #3568 I have a question about the methods of a ScriptExtensionProvider.

A provider could contain multiple presets.
A preset is a map between an "identifier" and an object (instance or class).
There are default presets that get always injected to an ScriptEngine and non-default presets which could be imported by "importPreset".

What about the getTypes and get function?
Let's assume a provider contains two presets: PA and PB
PA contains one entry: key "ea" with an Integer "1"
PB contains one entry: key "ea" with a String "2"

Looking at a current implementation of getTypes it returns the keys of the preset map(s) (the example uses only one preset).
So, what should be returned using the preset PA and PB? Does the type collection contain only "ea"?
What should be returned by get for the type "ea"? Should the Integer "1" or the String "2" be returned?