eclipse-archived / smarthome

Eclipse SmartHome™ project
https://www.eclipse.org/smarthome/
Eclipse Public License 2.0
862 stars 783 forks source link

[Automation] Rename the newly added actions service to thingActions #6703

Closed 5iver closed 5 years ago

5iver commented 5 years ago

This describes the action added in #6660 better for anyone using JSR223, and also does not step on the core.actions Jython module. Some more detail here.

Now, what exactly does this provide and how do you use it? Or is this being used to provide MQTTActions in the default scope? Will all new binding actions show up in the default scope?

Signed-off-by: Scott Rushworth openhab@5iver.com (github: openhab-5iver)

kaikreuzer commented 5 years ago

Currently, the actions instance essentially provides this method, which allows accessing an action that is related to a specific thing.

But the idea was to not clutter the scope with too many keywords in the future, so I wouldn't want a thingActions, serviceActions, 1xlegacyActions, etc., but rather have everything accessible through actions - i.e. pretty much the same idea as your python core.actions is doing.

We still need to define a clean scripting API, but I think that "actions" is going to be an important part of it, so I don't think changing it is a good idea. Could your python one be made obsolete, if the scope would directly give access to all different actions?

5iver commented 5 years ago

We still need to define a clean scripting API, but I think that "actions" is going to be an important part of it, so I don't think changing it is a good idea.

Understood. The method name threw me off a bit. I thought this was only going to be used for Thing actions.

Could your python one be made obsolete, if the scope would directly give access to all different actions?

Absolutely! Having all available actions show up in the scope would be a great addition. Adding Items directly into the scope would also be helpful.

To clarify a bit though, I'm using MQTT v2... am I understanding that MQTTActions showed up because the binding was installed? Your PR was pretty big, but it looked like this was what was happening.

After I got around the namespace issue, I found thatcore.actions also provided Things, so I could use Things.getStatusInfo, but I wasn't sure if I could have used actions (couldn't figure out how to do anything with it). All I saw was the get method, but this didn't look to be useful in scripts. The issue is that you need to import like...

from core.actions import Things

... to get it to work, or you were getting trumped in the namespace with actions. Hence the PR.

We still need to define a clean scripting API

Agreed... and you need a maintainer! I feel awkward saying that, because I could at least try, but my background is from the management side of product development, with relatively limited coding (my former engineers would get a chuckle if they saw me say differently). Although, it's become quite the hobby.

I've read volumes of code, but haven't gotten to the point of holding it a together my head. There have been some nice diagrams put into GH at times. Are these consolidated into any sort of design document? I've thought starting one for my own understanding of how the pieces all fit together.

pretty much the same idea as your python core.actions is doing.

I understand what it does, but I didn't write it! Pretty much the whole openhab2-Jython repo came from Steve Bate. I'm just tending to the code... updating when needed to get it compatible with API changes, with a couple features here and there..

kaikreuzer commented 5 years ago

am I understanding that MQTTActions showed up because the binding was installed?

Well the actions instance was added independently of the MQTT binding, but with the MQTT binding installed, a call to actions.get("scope", "thingUID") actually returns something.

I found thatcore.actions also provided Things, so I could use Things.getStatusInfo, but I wasn't sure if I could have used actions

That's a good point, which I haven't yet thought about - the "Things" class is currently the facade for DSL rules and it indeed provides the method getActions - so for JSR223 rules there are actually two paths to get hold of an actions instance: things.getActions(scope, uid) or actions.get(scope, uid). Which would be the better choice? When introducing the actions scope variable, my plan was to provide access to all kinds of actions through it - so having this for accessing thing actions seemed logical to me.

Agreed... and you need a maintainer!

You are more than welcome to take the lead on this! This is design & architecture work, so I don't think you'd need any extensive coding skills. Maybe one of the other heavy jsr223 users (like Steve) would be willing to help on the coding side of things... Let us make some plan on how to best move forward here.

kaikreuzer commented 5 years ago

Assuming we agree that thingActions is too specific to put it in the scope, let me close this PR for now.

openhab-bot commented 5 years ago

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

https://community.openhab.org/t/oh1-actions-oh2-actions/67564/2