ScriptProfile

[ccutrer]

Adds a profile that allows calling scripts (rules) in the system. The script can even cause the default action to happen or not.

This is my alternative approach to #3101.

[ccutrer]

I don't love having to catch an exception here for runNow to return outputs, but I was wary of changing the return type to Map<String, Object> in order to just return the new context directly, since it's technically an interface change.

[ccutrer]

In ruby, my "veto" has become:

script "shades_vetoing", name: "Shades Vetoing" do |callback:, item:, command:|
  next true if command == UP
  next true if command == REFRESH
  next true if item.tags.include?("AlwaysClose")

  shade_base_name = item.name[0..-20].sub("Black", "").sub("Roman", "")
  contact_item = items["#{shade_base_name}Window_Contact"]
  contact_item = DeckDoor_Contact if item.eql?(GreatDrapes_Rollershutter)
  contact_item ||= items["#{shade_base_name}Door_Contact"]
  next true unless contact_item.open?

  if item.tags.include?("DoubleHung")
    target = command == DOWN ? PercentType::HUNDRED : command
    if target > 45
      callback.handle_command(PercentType.new(45)) unless item.state == 45
      next false # veto
    end
  end
  false # veto
end

(compared to the previous incarnation at #3101 (comment))

[J-N-K]

Isn‘t this a duplicate of #2872?

[ccutrer]

Hmm. Not exactly. That one looks like it's referring to script engines, whereas this one refers to rules without triggers and with tag "Script" that Main UI refers to as Scripts.

[J-N-K]

I'm not sure if we should add a "third way" for scripting profiles. There is still the request for the "inline script", which I still do not see the need for, but should be discussed. I would think we should first come to an end in the discussion about #2872 and then decide which options for scripting we really need.

The VetoProfile could be much simpler, if you add a profile that takes an item name and a state (or maybe item name, state and operator, like we have in the conditions) and additionally an option to configure "veto commands", "veto updates", then you're done. This looks like about 100 loc and would be beneficial even for those that don't want to dive further into scripting.

[kaikreuzer]

@ccutrer It seems as if you have accidentally pushed a couple of unrelated commits to this PR, is that possible?

[ccutrer]

@ccutrer It seems as if you have accidentally pushed a couple of unrelated commits to this PR, is that possible?

Whoops, yup I accidentally pushed my "integration" branch which included my various PRs all in one. I've rebased and force-pushed just the two commits relevant to this PR.

[ccutrer]

Continuing from #2872 (comment) ....

This is more or less what I argued above as well, although as a ScriptProfile, I would prefer the "inline" version over #3135, but this could possibly be best discussed in #3135 then.

While inline is nice and quick for super basic things (and configuration directly in .items file if you use file based configuration), it wouldn't want to put much complexity in your script there. This PR re-uses the concept of "script" (a rule with no triggers, and tagged "Script") to give you a full editing UI that's already supported by the UI. It also enables re-usability of scripts, instead of having to duplicate the code among all items that are similarly configured. This means if you need to fix a bug, you only need to do it in one place. I feel like if you really want a relatively simple--but still customizable--profile, a transformation-based profile would fill that niche better.

[ccutrer]

The UI for configuring a script profile now looks like:

[J-N-K]

I'm very interested in any reason why this UI is better than the one rejected in #2872 which looks exactly the same.

[ccutrer]

Yes, they look the same. But this is referencing script objects, not transformation files. Scripts can be built completely within the UI, and there's no additional configuration for scripts:

[J-N-K]

There is also a PR for editing transformations (openhab/openhab-webui#1448). What additional configuration do you see for the SCRIPT transformation?

I also don't see where to configure the "additional named variables" you can pass into scripts (which you declared as benefit over using transformations in #2872 (comment))

[ccutrer]

There is also a PR for editing transformations (openhab/openhab-webui#1448).

Oooh, very nice! This has been a long time coming.

What additional configuration do you see for the SCRIPT transformation?

A script transformation needs the scripting language and the script file name. But maybe you're already bundling those up somewhere that the ScriptTransformationService no longer needs the script language given a script file name? And I know you're already presuming this profile will only work with the script transformation service, rather than any arbitrary transformation service. My last interaction with the ScriptTransformationService I had to name my files "*.script", even though it was actually Ruby.

I also don't see where to configure the "additional named variables" you can pass into scripts (which you declared as benefit over using transformations in #2872 (comment))

Maybe I didn't explain it well enough. Rule Actions can accept arbitrary inputs (as a Map<String, Object>). Scripting languages can fully take advantage of this. The ScriptProfile is setting up some of those inputs at

openhab-core/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/profiles/ScriptProfile.java

Line 145 in 6b014bc

var context = new HashMap<String, Object>();

- giving the script access to the callback in order to process (or not) the event however it would like, the command or state being passed to the profile, and (currently) the linked item. I could see passing through the entire ProfileContext object, and allowing arbitrary additional configuration on the profile. This is far more context, and far more flexible and extensible than the single string that can be passed to a transformation service.

In terms of potential changes, we could reduce the configuration of this profile to a single parameter -- instead of the four it has now. Just a single script id, and then pass to the script a string or some other event-type object telling it which method on StateProfile is being called.

[kaikreuzer]

While inline is nice and quick for super basic things
This PR re-uses the concept of "script". It also enables re-usability of scripts, instead of having to duplicate the code among all items that are similarly configured.

I agree with that. Re-using the "script" concept makes sense if these scripts are indeed used at various places. If they are specific to the created script profile, though, it rather brings overhead as you see them listed in the "general" section, although they are only meant for the dedicated script profile and nothing else.
Please also note that as mentioned here already, such inline scripts would also have proper UI support, that's already fully in place.

My assumption was that in 90% of the cases, one writes the script specifically for the profile and what is then reused is the profile, not the script. Would you say this assumption is incorrect?

[J-N-K]

I don't get your argument with the UI editor, neither here nor in the other PR. In both cases (here in the Script section, in the other PR in the Transformation section) the already existing script editor is used. The only thing that is missing (both here an in the other PR) is a quick-link from the profile configuration to "Add Transformation" (other PR) or "Add Script" (here).

[kaikreuzer]

The only thing that is missing

That was my point. The UI support for scripts is NOT missing for the in-line script solution. I only mentioned that because @ccutrer mentioned UI support as a feature for the "Re-use script concept" approach.

[J-N-K]

UI support is there, only the quick link is missing. That's simple and just needs a UI design idea, it's not a technical issue.

[ccutrer]

Soo... go whichever way you want with this. I no longer have a horse in the race. #2201 gave me an epiphany, and on the ruby side I've got a complete solution implemented without any changes to openhab-core necessary. You can see the example at https://ccutrer.github.io/openhab-jrubyscripting/OpenHAB/DSL.html#profile-class_method. #3151 will make my pure-Ruby approach slightly "nicer" (right now it's reaching in to a private field), but even that isn't strictly necessary.

[openhab-bot]

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/openhab-4-0-snapshot-discussion/142322/93

[wborn]

Can this be closed now that #3292 got merged?

[ccutrer]

Under mild protest ;) (I still feel that @J-N-K's implementation by using the script transformation is extremely limiting, but no longer relevant once I figured out that a script can provide its own profile(s) implemented however it wishes).