openhab / openhab-js

openHAB JavaScript Library for JavaScript Scripting Automation
https://www.openhab.org/addons/automation/jsscripting/
Eclipse Public License 2.0
38 stars 31 forks source link

[docs] Transformation action is missing #100

Closed rkoshak closed 1 year ago

rkoshak commented 2 years ago

All the core Actions are imported in actions.js except for Transformation. We should include all the core Actions.

I'll submit a PR in the next couple of days but is someone wants to run with this please do. It's only one line of code and a few lines of docs strings.

Your Environment

digitaldan commented 2 years ago

So we dynamically add all the actions that bundles expose here https://github.com/openhab/openhab-js/blob/d1b3cd5f87e967b59d8c612efc38bc83b7b3eb81/actions.js#L26 , this is how things like myopenHAB notification actions get added. I believe Transformation is being loaded here as well. Since this is a bundle that i think is always provided with openHAB its probably worth adding documentation even though we dynamically load it. wdyt?

rkoshak commented 2 years ago

So we dynamically add all the actions that bundles expose here

True, but Transformation is a core Action like Log and ScriptExecution and Exec.

https://www.openhab.org/javadoc/latest/org/openhab/core/transform/actions/transformation

I would have expected it to work the same as the other core actions.

But indeed, it appears to work so maybe we just need to document it? I just ran the following in scratchpad and it worked.

console.log(actions.Transformation.transform('MAP', 'en.map', 'ON'));

But there is no mention of Transformation in any of the docs we have right now. Why would it be that Transformation is loaded like that but the other actions are not? That's weird.

Anyway, since it's a core Action maybe all that's required is to add a section to the README to discuss it.

digitaldan commented 2 years ago

But there is no mention of Transformation in any of the docs we have right now. Why would it be that Transformation is loaded like that but the other actions are not? That's weird.

The transformation bundle exports its action like add-ons do, where the actions we explicitly load and document are located in https://github.com/openhab/openhab-core/tree/main/bundles/org.openhab.core.model.script/src/org/openhab/core/model/script/actions

I'm guessing this package was assumed to be the source of truth for core actions, which is why it's not documented.

technically any bundle, core or addon can register an action which we will load. I think we should probably document the popular ones., trying to keep up with all add-on provided actions could be tricky.

rkoshak commented 2 years ago

Yes, I wouldn't want to try to keep up with all the add-ons. But it's probably important to keep up with core. Since some of the core actions are documented when it's not mentioned it can lead some (like I was) to the conclusion that it's not included.

I would say let's add a section to the README mentioning that bindings and core can register their own actions and to look at the add-on readme for details. Then proceed to discuss those that openHAB core registers and maybe the broadcast actions too. That should give users enough information to know where to look for more details when needed. Sound reasonable?

digitaldan commented 2 years ago

Sounds good, we actually do have a little blurb about this on the readme, but its small and easily missed:

https://github.com/openhab/openhab-js/blob/main/README.md#actions

digitaldan commented 2 years ago

looks like we are also missing docs for persistence actions

https://github.com/openhab/openhab-core/blob/92917946d40c908181143f6c8335f40752bd3b69/bundles/org.openhab.core.persistence/src/main/java/org/openhab/core/persistence/extensions/PersistenceExtensions.java

digitaldan commented 2 years ago

although we cover this functionality in our Item class I believe, but technically there is a actions.PersistenceExtensions exported. Probably not worth documenting separately ?

rkoshak commented 2 years ago

That's a good question. We probably want users using the methods on Item instead of calling the PersistenceExtensions actions directly. It might cause confusion to show two ways of doing it in the docs.

florian-h05 commented 1 year ago

Anyway, since it's a core Action maybe all that's required is to add a section to the README to discuss it.

Transformation actions should be documented as they are available.

technically there is a actions.PersistenceExtensions exported. Probably not worth documenting separately ?

I wouldn't document actions whose functionality is implemented in our JS classes, so we should not document is separately.