GenericCronTriggerHandler does not give back context information

[lukicsl]

I am referring to the closed issue in JSR223/Jython: Class Decoration - CronTrigger - inputs map is empty

I am not familiar with the code, but it would be enough if just the information which is put out in the log output would somehow be packed into the map:

re.logger.debug("The trigger '{}' of rule '{}' is triggered.", trigger.getId(), ruleUID);

In the module GenericCronTriggerHandler.java nothing is passed back.

If I have multiple Cron triggers inside one rule, it would be good to know which particular trigger was fired. Today this is not possible as no context information is passed.

[5iver]

I hope to enhance this functionality for this and other handlers in OH 3.0... https://github.com/orgs/openhab-scripters/projects/2#card-21955025.

[lukicsl]

I hope to enhance this functionality for this and other handlers in OH 3.0... https://github.com/orgs/openhab-scripters/projects/2#card-21955025.

BTW StartupTrigger is not empty.

[5iver]

BTW StartupTrigger is not empty.

StartupTrigger currently only exists in Jython. But when I implement it in OHC, the event will not be null :wink:.

[J-N-K]

The problem with the GenericCronTrigger (and other time-related triggers) is that no event exists that could be passed. The reason is that those triggers directly trigger the rule instead if creating an event and triggering the rule over the event bus.

I do see the benefit of getting that information, though. @openhab/core-maintainers Do you think we should create a sort of "virtual event" for these triggers?

Type: org.openhab.core.automation.GenericCronTriggerEvent Topic: openhab/cron/triggered Payload: { "cronExpression" : "..." }

[kaikreuzer]

Don't we have the information about the rule itself at hand? I'd assume that this should be enough in such cases - if a rule has multiple cron triggers, it should be clear from the timestamp, which of it has fired.

[lukicsl]

Don't we have the information about the rule itself at hand? I'd assume that this should be enough in such cases - if a rule has multiple cron triggers, it should be clear from the timestamp, which of it has fired.

When a „regular“ rule trigger is defined (e.g. channel event) you ca supply a trigger name, which is part of the trigger conf data ( optional). When such a trigger is fired you get that info as part of the event data structure. This is good as I used to create a class which cofigures multiple triggers and a sigle event handling function. In this function I can do action based on different triggers. Though when a cron is triggered the event structure does not contain the trigger name. This exceptional case can be handled so far. This though limits to having just one cron trigger per class. It would be good if the supplied trigger name is delivered back for cron triggers too.

[J-N-K]

That seems to be a different request. You would like to get the label of the trigger (if present) in the rule context. This is easy code-wise, but we had a discussion https://github.com/openhab/openhab-core/issues/1796 about polluting the scope. In the light of that discussion I'm not convinced that injecting a new trigger variable is really a good idea. I would like to hear what @jpg0 (JS), @boc-tothefuture (JRuby) and @wborn (Groovy) think about it.

So if any agreement has been reached on how to present that information, I can implement it.

[jpg0]

My view has always been the same, which is that we should leave the execution context for scripts as close as possible to their standard (non openHAB) environment. It's interesting to read #1796 now, because I also had a very bad time porting the ES5 OH libraries to ES6+ because of all the things injected into the context which subtly break execution (e.g. I believe event was a good one for JS). It is because of this that I moved all the magic injection into explicit imports for JS (although the magic was later re-added as default, but can be disabled). The ability to add new symbols (both in cases like this, and even for add-ons to do when they are installed) only seems to exacerbate this symbol clashing problem. Therefore my vote is not to add this as an injected symbol, but rather think more about how context can be provided explicitly, ideally cross-language.

[rkoshak]

I two would like to second the idea of finding some sort of cross-language solution. However, don't forget about UI rules. There are already discontinuities between the two environments even within one language which are disconcerting. For example, in JS Scripting in a text based rule you get to provide what the "event" Object gets called (it's the argument passed to the function that is called when the rule triggers) but you get a JavaScript Event Object which renames all the stuff inside the event to match what's used in Rules DSL.

But in the UI we get that same old JSR223 event Object which is always called event and the members have the JSR223 names (e.g. event.itemState as opposed to myeventname.state).

So my request would be to not forget about UI rules users. Fixing this for just text based rules is only going to lead to much confusion.

I'll also mention that in the UI there are non-cron based time triggers, you can manually trigger a rule through the UI, and a rule can call another rule. In all of these cases there is no event Object and in a couple of cases there is no triggering event really (manual and rules that call other rules).

[jpg0]

@rkoshak I think this points at a larger problem which keeps coming up - we are talking about file and UI 'rules' being interchangable, however a file rule and a script rule are semantically different concepts. A UI rule is a snippet of code which is attached to some trigger to be executed at some point. A file rule is broader than this - it's a piece of code that runs at startup (or modification etc) and explicitly creates rules and attaches code to them. It's kind-of like the UI script plus the UI itself which sets it up and wraps it with a trigger (and maybe condition etc). This is not even getting into how file scripts can even provide items, channels, and provide whatever Identifiable<T>'s to the system.

My view is that we should be explicit about these distinct concepts (maybe even give them distinct names), and treat one (UI rules) as a subset of the other. We already have a number of problems in JS-land because we try to say they are the same but there are distinct and fundamental differences (like the execution engine differs, script environments are reused so you cannot use const in UI rules). The sooner we embrace the fact that they are different but nested, the sooner we can plot out a sensible path to consistency.

[rkoshak]

I will never argue that they are different. My concerns are as follows (these are generic concerns, not concerns with what you've said):

1. With the exception of Yannick, I think I'm the only person active on GitHub who actually uses and supports others in using UI rules. This results in ideas, approaches, and decisions being made that have sometimes huge impact on the usability of UI rules, mostly out of ignorance. And since our least technically capable users are the ones most likely to be using UI rules, these are the user who are least able to cope with these impacts. That's why I chimed in here and have had other similar discussions. When all you know are text based rules, things that seem like a really good idea, even a recognized best practice, can become a nightmare in UI rules.

2. The further apart that UI rules and text based rules diverge the more work it will be to document them and to help users on the forum when they have problems. There is a real cost imposed on both devs, documenters, and end users when the two diverge. And since most of the devs and maintainers don't use or test in UI rules, there will be cases where an innocent change in text based rules causes problems in UI rules, or more likely simply doesn't even get documented. As a case in point, that JSR223 event Object that we get in UI rules isn't really documented anywhere in the OH docs. And were it to be added to the docs, where? It probably doesn't belong in the JS Scripting readme because the add-on has nothing to do with it. Maybe on the JSR223 page, but who among the non-technical readers will know to look there?

[jpg0]

So whilst I am saying that they are different, I am not saying that they share nothing. What I would like to see is, for a file based script looking like this:

code
code
begin rule def
rule parameters (triggers, conditions etc)
begin execution code
code
code
code
end execution code
end rule def
code
code

The section within the execution code boundaries would be identical for UI-based scripts, and the execution would also be the same. But this is certainly a subset of the file based scripts, there would be no explicit setup of the rule for the UI based script. (That is unless we want a 2 types of script in the UI, which I would be happy with as long as they had different names.) This way there would absolutely be shared docs, but there would be different docs for the parts outside the execution code (either UI docs about how to configure things in the UI, or script specific docs about how to handle openHAB objects like rules in file based scripts).

[rkoshak]

I think we are on the same page up to a point. Where problems occur is in the details.

For example, let's go back to the whole discussion that caused the introduction of the property on the JS Scripting add-on to automatically import the library.

Clearly, best practice in a file based rule would be not to do that. Keep the namespace empty and only import stuff in a way that lets the user define the name it gets imported as. This perfectly fits your pattern and all around sounds like a great idea. But, as we saw, in practice that leads to situations where up to 50% of all the code written in a UI rules could be the same old imports over and over and over again.

I completely agree the stuff between begin execution code and end execution code should be the same. That would be fantastic actually. But, if we only cater to the file based rules that could impose a pretty significant burden on UI based rules. So all I want to make sure is that any approach that heads down this path to please consider the impact to UI rules users.

[jpg0]

The way I would suggesting tackling the problem you raise is like I mentioned back then: to have static code "fixed" around the UI code. In JS, in the UI, above the start of the execution, there would be something like:

const {x,y,z} = require(...)
function onRuleTriggered(event) {

custom code goes here

}

This way the custom code can get whatever imports we decide, and file based scripts can copy this (or not, if they like).

I didn't push this hard back then even though I'm not a fan of the automatic imports, but over time we are seeing more cracks form between the two approaches because we are trying to converge two things which cannot be the same.

[J-N-K]

With the new DateTimeTrigger it would be useful to know which item triggered the rule. I would therefore "adjust" my proposal of the virtual (because they are not send over the bus) events:

Type: org.openhab.core.automation.TimerEvent Topic: openhab/timer/{triggerUID}/triggered Payload: JSON representation of the configuration, so either { "cronExpression" : "..." } or { "itemName": "..." } or similar.

The good thing is, that we don't need to inject anything new and the injected event stays compatible with what it was before, an instance of Event.