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

Items: Fix createItem does not set metadata & further improvement #109

Closed florian-h05 closed 2 years ago

florian-h05 commented 2 years ago

Items: Fix createItem does not set metadata & improvements with Item configuration & feature addition

Description

Breaking Changes

addItem & replaceItem now use the itemConfig object.

Add a note to https://github.com/openhab/openhab-distro/blob/main/distributions/openhab/src/main/resources/bin/update.lst. Update https://github.com/openhab/openhab-webui/blob/main/bundles/org.openhab.ui/web/src/assets/openhab-js-tern-defs.json for code-completion in WebUI.

Testing

From the README:

items.replaceItem({
   type: 'Switch',
   name: 'Hallway_Light',
   label: 'Hallway Light',
   category: 'light',
   groups: ['Hallway', 'Light'],
   tags: ['Lightbulb'],
   channels: {
     'binding:thing:device:hallway#light': {},
     'binding:thing:device:livingroom#light': { 
       profile: 'system:follow' 
     }
   },
   metadata: {
     expire: '10m,command=OFF',
     stateDescription: {
       config: {
         pattern: '%d%%',
         min: 0,
         max: 100,
         step: 1,
         options: '1=Red, 2=Green, 3=Blue'
       }
     }
   }
 });
florian-h05 commented 2 years ago

REMINDER: After this PR gets merged, update https://github.com/openhab/openhab-webui/blob/main/bundles/org.openhab.ui/web/src/assets/openhab-js-tern-defs.json#L156 for the breaking changes.

janwo commented 2 years ago

Looks good :) Is it also possible to set configurations in metadata like the following use case?

Number nTest "Test Number Item" {
    stateDescription=""[
        pattern="%d%%",
        min="0",
        max="100",
        step="1",
        options="1=Red, 2=Green, 3=Blue"
    ]
}
florian-h05 commented 2 years ago

@janwo

Currently this is not possible, but I am working on a solution for this now

florian-h05 commented 2 years ago

@janwo

Got it working now, you should try the following code snippet:

var stateDescription = new Map()
  .set('pattern', '%d%%')
  .set('min', '0')
  .set('max', '100')
  .set('step', '1')
  .set('options', '1=Red, 2=Green, 3=Blue');

var metadata = new Map();
metadata.set('stateDescription', ['', stateDescription]);

items.replaceItem({
  type: 'Number',
  name: 'nTest',
  label: 'Test Number Item',
  metadata: metadata
});

@janwo Could you please review my PR?

jpg0 commented 2 years ago

@florian-h05 is it possible to use object notion too, such as:

items.replaceItem({
  type: 'Number',
  name: 'nTest',
  label: 'Test Number Item',
  metadata: {
    stateDescription: {
      pattern: '%d%%',
      min: 0,
      max: 100,
      step: 1,
      options: '1=Red, 2=Green, 3=Blue' //even this could be an object
    }
  }
});

I would think this to be more idiomatic JS.

florian-h05 commented 2 years ago

@florian-h05 is it possible to use object notion too, such as …

@jpg0 Currently not, I was also thinking about that way. You are right with your argument, that is also easier to read.

Should I change it to use that way?

jpg0 commented 2 years ago

Personally I prefer the object notation style, and that is already how the primary config object is created, so I'd say yes, change it. (You can most likely code it so that both work, if you want to.)

florian-h05 commented 2 years ago

Okay, then I will change it to the object notation style. I will not code to support both ways because that would blow up the README and overwhelm some readers.

florian-h05 commented 2 years ago

@jpg0 Finished it, ready for review and merging.

You have to modify your code snippet a little bit:


items.replaceItem({
  type: 'Number',
  name: 'nTest',
  label: 'Test Number Item',
  metadata: {
    stateDescription: {
      config: { // need to wrap configuration in extra object, because e.g. for expire you may use value & config
        // those key:value pairs are translated to Map because Java Metadata() requires Map
        pattern: '%d%%',
        min: 0,
        max: 100,
        step: 1,
        options: '1=Red, 2=Green, 3=Blue' // I have not tested if this could be an object, but I do not see any reason why it should not
      }
    }
  }
});
jpg0 commented 2 years ago

@florian-h05 FYI the createItem function exists because I used it to create items from the script, then register them via a custom ItemProvider (also created from the script). This way the items are not managed (they exist only in memory), which means that they are tied to the lifecycle of the script - e.g. if the script is unloaded, the items disappear. Managed items are created and persist until they are explicitly deleted (or replaced), regardless of the script.

Saying this, I have no problem with you removing the createItem function; I no longer use it personally.

florian-h05 commented 2 years ago

@jpg0 Thank you for that explanation.

Saying this, I have no problem with you removing the createItem function; I no longer use it personally.

Okay, in fact I do not remove it (it is used by addItem), I just removed it from the docs.

florian-h05 commented 2 years ago

@jpg0 Can you merge or do you want to wait for @digitaldan?

jhuizingh commented 2 years ago

This looks great! How long until it gets into the officially released jsscripting binding? I'm trying to decide whether I should just wait for that or install it manually from the main branch.

florian-h05 commented 2 years ago

This looks great! How long until it gets into the officially released jsscripting binding? I'm trying to decide whether I should just wait for that or install it manually from the main branch.

I think that the next version of JS Scripting and this library will be released together with openHAB 3.3.0 Stable this summer, so I would recommend installing from GitHub with: npm i openhab/openhab-js (installs from this repo's main).

digitaldan commented 2 years ago

I think its probably worthwhile to push a openhab-JS release to NPM, and update the binding with the new version so milestone and nightlies get our updated changes