[automation] Binding actions cannot be configured by UIs #1745
Comments
|
@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? |
|
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 (a22476c) 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. |
Definitely, things that don't work should not be visible. |
|
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. |
|
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. |
|
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. |
|
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 |
|
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... |
|
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 |
|
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 |
|
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 |
|
Given that #2810 has been merged, is the missing part only a matter of integration in UI, or is there still anything needed from core side? |
|
@kaikreuzer Don‘t move it - the core side is not finished yet. |
Yes, #2810 only gives access to the Thing actions of a Thing and allows to invoke them, but the input parameters are still difficult to handle. Example: "inputs": [
{
"name": "from",
"type": "java.time.LocalTime",
"label": "Begin Timestamp",
"description": "The beginning of the time range",
"required": false,
"tags": [],
"reference": "",
"defaultValue": ""
},
{
"name": "until",
"type": "java.time.LocalTime",
"label": "End Timestamp",
"description": "The (inclusive) end of the time range",
"required": false,
"tags": [],
"reference": "",
"defaultValue": ""
}
],To show input fields for this, I would need to an extra processing of these definitions inside the UI — meanwhile the rest is using config descriptions. When comparing this with how config descriptions look (e.g. https:/openhab/openhab-webui/blob/217fc060ee0a976acc9e046a3dd8ed5f3d7470da/bundles/org.openhab.ui/web/src/assets/definitions/persistence.js#L23), there are many similarities, so it should be possible to modify the API to return config descriptions for the inputs. |
|
This issue has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/incorporating-matter/127907/162 |
The main difficulty would be the conversion of input parameter types. Are current input types only Java primitive data types? And what to do with the output type ? Ignore it ? We could also decide to build two groups of config parameters, one for input parameters and one for the output parameter ? Edit: we could keep by default the current format in the REST response and have a new one based on ConfigDescription triggered by a new API parameter. Like that we keep backward compatibility. WDYT ? |
Yeah, that‘s the difficulty — current input types can in fact be defined by any string value and are specified in some cases, but usually they default to the canonical name of the input‘s class, see:
Yes — when executing an action, the output map will be serialised to a JSON object (basically key-value pairs), and I guess the values of the output map are just made
I am not sure if there is actual need to keep backward compatibility compatibility here … I think HABApp might use this data, but I don‘t think anything in the official openHAB distro does use this. Where to get the config descriptions from? I have several options in mind:
WDYT? |
|
I am currently on vacation in 🇫🇷, will be back in action next week, so I will leave it to you. |
|
This is not exactly related to our current discussion but when I run this API (GET actions/{thingUID}), it looks like outputs field is always empty. Then the API returns I guess we should have one output ? |
|
Yes, looks like a bug. |
|
The "required" parameter of the @ActionInput annotation is not clear for me. Does it mean the value provided should not be null ? It is what I guess but in this case why do we have this in certain bindings, that "required = true" but with @nullable ? @florian-h05 : if my understanding is correct, that means if you get a config description parameter with required set to false, then the user is not forced to fill the parameter but when calling the action, you will have to provide null as value. |
I don’t think that I have to actually pass null as value through the API, I would think null is passed if the param is missing. But that is something we can easily try out … |
|
I am not succeeding to invoke an action with the API. Here is what I provide when I invoke the action with the UI: |
|
Looks like it works with Boolean and String types but not with Integer ! Another failing basic example: Called like that: Leads ot exception I guess that 123 is put in an Object that is not java.lang.Integer. |
…params Related to openhab#1745 Signed-off-by: Laurent Garnier <[email protected]>
I don't know why but my provided value 123 is stored in a java.lang.Double as 123.0. Our current code that build the method arguments before invoking the method should very probably apply type casting. |
Tested, you're right. In case a parameter is not provided, null is passed as parameter to the method. So, regarding invokation oif the action through the REST API, it works well for the following parameter types: boolean, Boolean, double, Double, String. Of course, for boolean and double, if you do not provide any value or if you provide null as value, there is an expected exception as null is considered. I have an idea how to fix that. |
IMO we should cast the double received from the API to the required type then, theoretically this is a lossy conversion but since the double has an integer value we don’t have to worry about loosing precision. |
API GET /actions/{thingUID} now returns the input parameters also as a list of configuration description parameters.
It is provided only when all input parameters have a type than can be mapped to the type of a configuration description parameter.
It will be used in particular by Main UI to expose actions.
Also enhance the POST API (execute a thing action) in order to be more flexible regarding the type of each provided argument value and to map the value to the expected data type.
Related to openhab#1745
Signed-off-by: Laurent Garnier <[email protected]>
API GET /actions/{thingUID} now returns the input parameters also as a list of configuration description parameters.
It is provided only when all input parameters have a type than can be mapped to the type of a configuration description parameter.
It will be used in particular by Main UI to expose actions.
Also enhance the POST API (execute a thing action) in order to be more flexible regarding the type of each provided argument value and to map the value to the expected data type.
Related to openhab#1745
Signed-off-by: Laurent Garnier <[email protected]>
It is now done. String value is also accepted for numbers. |
|
@florian-h05 : for QuantityType, should I simply consider type DECIMAL but with unit and unitLabel provided ? |
|
@lolodomo Can you please first merge my PR lolodomo#3 before doing further changes? I have just resolved conflicts. |
|
@florian-h05 : do not forget you will have to display the action result in UI. Is the current output fine for you? We could imagine a new parameter to request the output as a simple string. WDYT? PS: I will merge your changes this evening. |
I would think it already is a JS object (this is what a Map gets serialised to) of stringified Java objects. Is this not the case?
Let me know if there are new conflicts that have to be resolved then. |
I would rather provide the Dimension and introduce a new context „quantity“ for TEXT input. The UI already includes a list of dimensions with their units to be used by the unit field of Items, so the data is already there … |
Ok but if the map contains a "result" entry, just display its value (and not the full map). |
|
Can you please provide me an example output? |
|
For an action returning an Integer, the result is the following: The UI should just show: 5544 The UI should just show: Here is the result. The UI should just show an empty string. |
|
If I try to return a java.time.LocalDateTime, I get this log: Current code only accepts Boolean, String, Integer, Float and Double as output types. |
That sounds good, because these are super easy to serialise for the API! |
|
I have to check if all our actions return one of these 5 types. |
|
Here are the types found in existing thing action inputs that will be supported by UI:
And those that will not be supported:
|
|
I am counting 21 existing thing actions that cannot be run properly due to an unsupported output type:
They should fail here: @jlaur: as you are concerned with energidataservice, does it mean that these actions can be used inside a rule but not through the REST API ? |
All actions are working correctly for both DSL and JavaScript, but I have not tested any of them through REST API since I have not been aware of this really. |
Fixes openhab#1745 Return config description parameters for the ActionInputs of ThingActions for the REST GET /action/{thingUID} and REST GET /module-types endpoints. The config description parameters are only provided if all input parameters have a type that can be mapped to a config description parameter (String, boolean, Boolean, byte, Byte, short, Short, int, Integer, long, Long, float, Float, double, Double, Number, DecimalType, QuantityType<?>, LocalDateTime, LocalDate, LocalTime, ZonedDateTime, Date, Instant and Duration). Enhance the REST POST /actions/{thingUID}/{actionUid} endpoint (allows invoking Thing actions via REST) and the AnnotationActionHandler (allows invoking Thing actions from UI-rules) in order to be more flexible regarding the type of each provided argument value and to map the value to the expected data type. Number and string values will be accepted as inputs and the expected data type will be created from this value. This will be used by the UI's Thing page and rule editor to allow invoking Thing actions through the UI or adding them to UI-bases rules. Signed-off-by: Laurent Garnier <[email protected]> Signed-off-by: Florian Hotze <[email protected]>
This is a follow-up issue of eclipse-archived/smarthome#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 (openhab/openhab-addons#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.Internal automation actions like
sayorplaySounddefine next to the inputs a list of configuration descriptions matching their inputs.Conclusions of eclipse-archived/smarthome#6602 are:
Question is how to proceed? Should we hide all
ThingActionsin the near future? Or would it be an option to introduce a way to add configuration descriptions toThingActions(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:
and a rule like:
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?
The text was updated successfully, but these errors were encountered: