Populate source when sending Item events via BusEvent #5122
Conversation
Signed-off-by: Florian Hotze <[email protected]>
| } | ||
| } | ||
|
|
||
| private String buildSource(@Nullable String source) { |
There was a problem hiding this comment.
@ccutrer @kaikreuzer Please let me know what you think about my approach here of using the provided source or defaulting to the package name.
The motiviation for not delegating was that in many cases helper libraries will take care of providing a source, e.g. org.openhab.automation.javascript.script.test.js or org.openhab.automation.jsscripting$file:test.js, and in that case IMO it is confusing and unneccassary to have something like org.openhab.automation.javascript.script.test.js=>org.openhab.core.automation.module.script.
BTW, what of the above sources do you prefer?
There was a problem hiding this comment.
yes, I think this is perfect. if a rule (or a helper library) sets a source, it should override the source completely, not just be the first delegation. my only ask would be do you think it would be possible to figure out the rule ID automatically here, and make that the actor? I know that might not work if it's in a timer or something, but it'd be nice for DSL rules that don't have a helper library. Which implies that my preference is for the source is <package>$<rule> for rules - org.openhab.automation.jsscripting$myrule (if possible to grab the full rule ID; if not then just file name is reasonable, though what would that be for non-file-based rules?).
There was a problem hiding this comment.
my only ask would be do you think it would be possible to figure out the rule ID automatically here, and make that the actor?
I see no way of doing that. The rule ID is injected into the script context somewhere else, and that context is "far away" here.
but it'd be nice for DSL rules that don't have a helper library.
If we assume that org.openhab.core.model.script.actions.BusEvent is ONLY used by rules DSL and all other environments use BusEvent from the ScriptAction script extension preset, we could at least hint to rules DSL; but I don't know how we would hint which rule.
Which implies that my preference is for the source is $ for rules - org.openhab.automation.jsscripting$myrule (if possible to grab the full rule ID; if not then just file name is reasonable, though what would that be for non-file-based rules?).
I also prefer $. The rule ID will be available and used for Script Actions and Script Conditions, for file based scripts the filename will be used. At least that will be my JS implementation.
|
LGTM as-is |
…EventImpl org.openhab.core.automation.module.script...ScriptBusEventImpl was a copy of org.openhab.core.model.script...BusEvent, which was removed in openhab#5122. In openhab#5122, a org.openhab.core.automation.module.script...BusEventImpl was added, which is already used by the org.openhab.core.model.script bundle. I think the ScriptBusEventImpl copy of the old BusEvent class was done to avoid a dependency org.openhab.core.automation.module.script => org.openhab.core.model.script. As we now have a BusEventImpl in the automation bundle, we can use it for the DefaultScriptScopeProvider and get rid of ScriptBusEventImpl. Signed-off-by: Florian Hotze <[email protected]>
…EventImpl (#5136) org.openhab.core.automation.module.script...ScriptBusEventImpl was a copy of org.openhab.core.model.script...BusEvent, which was removed in #5122. In #5122, a org.openhab.core.automation.module.script...BusEventImpl was added, which is already used by the org.openhab.core.model.script bundle. I think the ScriptBusEventImpl copy of the old BusEvent class was done to avoid a dependency org.openhab.core.automation.module.script => org.openhab.core.model.script. As we now have a BusEventImpl in the automation bundle, we can use it for the DefaultScriptScopeProvider and get rid of ScriptBusEventImpl. Signed-off-by: Florian Hotze <[email protected]>
Refs openhab/openhab-core#5122. Refs openhab/openhab-core#5136. Signed-off-by: Florian Hotze <[email protected]>
Refs openhab/openhab-core#5122. Refs openhab/openhab-core#5136. Signed-off-by: Florian Hotze <[email protected]>
Refs openhab/openhab-core#5122. Refs openhab/openhab-core#5136. Signed-off-by: Florian Hotze <[email protected]>
Refs openhab/openhab-core#5122. Refs openhab/openhab-core#5136. Signed-off-by: Florian Hotze <[email protected]>
3 participants
No description provided.