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

[metadata] Add support for configuration #189

Closed rkoshak closed 1 year ago

rkoshak commented 1 year ago

This has come up a number of times on the forum. There is some demand to support getting and modifying not just metadata values but also the configuration dict. This would add a few methods to the metadata namespace to support retrieving, adding, deleting and upserting metadata that also includes the configuration.

EDIT by @florian-h05: See https://community.openhab.org/t/set-statedescription-for-proxy-item-from-a-rule-js-scripting/141690/5.

florian-h05 commented 1 year ago

Looking at https://github.com/openhab/openhab-js/blob/main/metadata/metadata.js, passing a configuration object should actually be possible, have a look at what itemConfig is able to, especially for the stateDescription. For me, it seems like this is more an documentation issue than a missing capability of the API. I’ll try out later if it works like I think, and probably open a PR to get this into milestone 6. If you review, I’ll merge even though you are no official maintainer, but you have experience with JS.

rkoshak commented 1 year ago

Looking more closely at metadata.js, you are right, upsertValue, addValue, and updateValue do have a config argument that takes a Map. However, the function names are very misleading. A bigger concern though is it's missing a getter that returns the config. Also, updateValue basically throws away the config since createMetadata doesn't use the config. argument passed to it.

So it's a bit of both. It is missing from the docs but there is at least one bug and a missing capabilities.

florian-h05 commented 1 year ago

You are right, the implementation is a little bit „weird“. I’ll have a look at this, but I don’t think that it will make it into the 3.4.0 release because the feature freeze is coming on Sunday. But maybe we can declare this as bug fix??

jpg0 commented 1 year ago

FYI when I wrote this part of the library, I had almost no idea how openHAB configuration actually worked. I did it only to support some specific use-cases I had, so I'm not surprised that it's weird (and tbh I was confused about the distinction between metadata and configuration). I wouldn't be surprised if the best course of action would be to completely replace it!

florian-h05 commented 1 year ago

A small update how it’s going: I already figured out how to reimplement metadata, I just need to clean up the code and create a PR.