openhab-core icon indicating copy to clipboard operation
openhab-core copied to clipboard

[automation] Binding actions cannot be configured by UIs

Open cweitkamp opened this issue 4 years ago • 10 comments

This is a follow-up issue of https://github.com/eclipse-archived/smarthome/issues/6602 as no solution has been implemented yet and we need to track it for OH3 release.

While implementing and testing my port of the Pushover add-on (https://github.com/openhab/openhab-addons/pull/8586) I again stumbled upon this missing feature. Currently it is not possible to configure inputs for Binding actions based on ThingActions. The annotated inputs are not visible in UIs.

image

    @RuleAction(label = "@text/sendHTMLMessageActionLabel", description = "@text/sendHTMLMessageActionDescription")
    public @ActionOutput(name = "sent", label = "@text/sendMessageActionOutputLabel", description = "@text/sendMessageActionOutputDescription", type = "java.lang.Boolean") Boolean sendHtmlMessage(
            @ActionInput(name = "message", label = "@text/sendMessageActionInputMessageLabel", description = "@text/sendMessageActionInputMessageDescription", type = "java.lang.String", required = true) String message,
            @ActionInput(name = "title", label = "@text/sendMessageActionInputTitleLabel", description = "@text/sendMessageActionInputTitleDescription", type = "java.lang.String") @Nullable String title) {
        logger.trace("ThingAction 'sendHtmlMessage' called with value(s): message='{}', title='{}'", message, title);
        return send(getDefaultPushoverMessageBuilder(message).withHtmlFormatting(), title);
    }

Internal automation actions like say or playSound define next to the inputs a list of configuration descriptions matching their inputs.

Conclusions of https://github.com/eclipse-archived/smarthome/issues/6602 are:

  • No short term solution to add support for input in the UIs. Proposal is to hide all ThingActions.
  • The is uncertainty about when to use inputs and when to use configuration descriptions (e.g. currently all existing core actions for NGRE defined them twice, an input and a related configuration).

Question is how to proceed? Should we hide all ThingActions in the near future? Or would it be an option to introduce a way to add configuration descriptions to ThingActions (e.g. by adding a new annotation for it)?

A previous private discussion between @kaikreuzer , @ghys , @openhab-5iver and myself resulted in the following brainstorming:

  • Displaying inputs along with config parameters should be doable to configure an action instance in a rule.

  • Apparently inputs have a type which is referenced in the API as a Java type (like java.lang.String or org.eclipse.smarthome.core.types.Command or org.eclipse.smarthome.core.events.Event), that’s a little bit different to the config parameters whose type is “TEXT” etc. Supporting anything other than primitive types in the UI could be tricky.

  • Given a module type like: image

    and a rule like: image

    I’m not sure how inputs are to be configured here, I suppose in the simplest case it’s something like below, with what the user configured in input boxes. But what if you want to use an output from another module?

"inputs": {
  "topic": "some/topic",
  "value": "somevalue"
}
  • It is also IMO not exactly clear which inputs are to be presented to the user and which are “technical” (for example “event” in GenericEventCondition or “input” in CompareCondition).
  • The ItemCommandAction module type has both a “command” input and a “command” config parameter described as “the default command to be sent if none is passed as an input value” - this would be confusing for users if both were displayed.

cweitkamp avatar Oct 21 '20 14:10 cweitkamp

@cweitkamp While this is quite a nasty issue that needs to be tackled, I feel that we won't be able to solve it for the 3.0 release - would you agree to hence remove it from the OH3 issue tracking?

kaikreuzer avatar Dec 05 '20 21:12 kaikreuzer

Yes, that is okay for me. But I would like to agree on a temporarily solution like suggested in #1878. We should maybe hide binding actions having an additional parameters in the UI to not tangle users.

I already tried a very rough approach (https://github.com/openhab/openhab-core/commit/a22476cf0ff75644bfb397f3b524c78519a8aaf6) which just maps all defined inputs to configuration parameters respectively. This makes them visible in the UI but does not work properly. And it does not feel right to do it that (simple) way.

cweitkamp avatar Dec 06 '20 08:12 cweitkamp

We should maybe hide binding actions having an additional parameters in the UI to not tangle users.

Definitely, things that don't work should not be visible.

ghys avatar Dec 06 '20 17:12 ghys

I agree. By default - as a temporarily solution in OHC - or by adding the visibility annotation in every add-on. I tend to the first solution because it will be less work and easier to revert after a final implementation.

cweitkamp avatar Dec 06 '20 21:12 cweitkamp

As you work on solving this issue going forward, I just want to mention that not all binding actions make sense and should never be in the list. For example, the Astro binding only provides actions that return a value. There is no way to actually use that value as an action so they shouldn't be there in the list.

rkoshak avatar Dec 08 '20 21:12 rkoshak

I wouldn't say so. Action results are put in the scope and should in theory be consumable by other actions in the rule that are later in the sequence.

kaikreuzer avatar Dec 08 '20 21:12 kaikreuzer

But, at least in my experimentation, the scope isn't shared between actions, even actions of the same rule. It's definitely not the case that the scope is shared between the conditions and the actions. I may have messed up my experiments though and came to the wrong conclusion.

Given the following rule that has one action that puts a variable into the context and another action that tries to log it out I always get "foo is undefined" in the logs. Is there some other way to put a variable into the scope?

triggers:
  - id: "2"
    configuration:
      itemName: aTestSwitch
    type: core.ItemCommandTrigger
conditions: []
actions:
  - inputs: {}
    id: "1"
    configuration:
      type: application/javascript
      script: this.foo = "Hello world!";
    type: script.ScriptAction
  - inputs: {}
    id: "3"
    configuration:
      type: application/javascript
      script: >-
        var logger =
        Java.type("org.slf4j.LoggerFactory").getLogger("org.openhab.model.script.Experiments");


        logger.info("foo is " + this.foo);
    type: script.ScriptAction
2020-12-08 14:48:21.199 [INFO ] [org.openhab.model.script.Experiments] - foo is undefined

rkoshak avatar Dec 08 '20 21:12 rkoshak

That's why I added "in theory" - haven't tried it myself yet and won't have to time to go deeper into that before the release...

kaikreuzer avatar Dec 08 '20 21:12 kaikreuzer

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

https://community.openhab.org/t/openhab3-mail-rules-actions-not-same-as-openhab2/129377/2

openhab-bot avatar Nov 29 '21 12:11 openhab-bot

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

https://community.openhab.org/t/ipcamera-new-ip-camera-binding/42771/2682

openhab-bot avatar Mar 19 '22 01:03 openhab-bot

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

https://community.openhab.org/t/openhab-4-0-wishlist/142388/293

openhab-bot avatar Jan 20 '23 13:01 openhab-bot