Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from
Apr 23, 2022

Conversation

florian-h05
Copy link
Contributor

@florian-h05 florian-h05 commented Apr 11, 2022

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'
       }
     }
   }
 });

Fixes openhab#108.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
And `addItem` & `replaceItem` as they use `createItem` under the hood.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Use them in `items/managed.js` for `addItem`.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Adapt to breaking changes from 2a2b18c.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
@florian-h05
Copy link
Contributor Author

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
Copy link

janwo commented Apr 16, 2022

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
Copy link
Contributor Author

@janwo

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

@florian-h05 florian-h05 changed the title Items: Fix createItem does not set metadata & further improvemts [WIP] Items: Fix createItem does not set metadata & further improvement Apr 16, 2022
@florian-h05
Copy link
Contributor Author

@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?

@florian-h05 florian-h05 changed the title [WIP] Items: Fix createItem does not set metadata & further improvement Items: Fix createItem does not set metadata & further improvement Apr 16, 2022
@jpg0
Copy link
Collaborator

jpg0 commented Apr 16, 2022

@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
Copy link
Contributor Author

@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
Copy link
Collaborator

jpg0 commented Apr 16, 2022

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
Copy link
Contributor Author

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 florian-h05 force-pushed the fix-createitem-metadata branch from 211ce05 to e39ef52 Compare April 16, 2022 21:48
@florian-h05
Copy link
Contributor Author

florian-h05 commented Apr 16, 2022

@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
      }
    }
  }
});

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
@florian-h05 florian-h05 force-pushed the fix-createitem-metadata branch from e39ef52 to d418b53 Compare April 16, 2022 22:47
Copy link

@janwo janwo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me :)

@@ -242,10 +243,9 @@ See [openhab-js : items](https://openhab.github.io/openhab-js/items.html) for fu
* .getItem(name, nullIfMissing) ⇒ <code>Item</code>
* .getItems() ⇒ <code>Array.&lt;Item&gt;</code>
* .getItemsByTag(...tagNames) ⇒ <code>Array.&lt;Item&gt;</code>
* .createItem(itemName, [itemType], [category], [groups], [label], [tags], [giBaseType], [groupFunction], [itemMetadata])
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why removing it?

Copy link
Contributor Author

@florian-h05 florian-h05 Apr 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function only creates an Item object but does not create a new Item in openHAB.
I think there is no case where normal scripters should have a use for that and like in the linked issue, createItem only confuses.

@jpg0 jpg0 self-requested a review April 17, 2022 14:46
Copy link
Collaborator

@jpg0 jpg0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jpg0
Copy link
Collaborator

jpg0 commented Apr 17, 2022

@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
Copy link
Contributor Author

@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
Copy link
Contributor Author

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

@jpg0 jpg0 merged commit 394fb99 into openhab:main Apr 23, 2022
@florian-h05 florian-h05 deleted the fix-createitem-metadata branch April 23, 2022 12:42
@jhuizingh
Copy link

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
Copy link
Contributor Author

florian-h05 commented Apr 23, 2022

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
Copy link
Contributor

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

florian-h05 added a commit to florian-h05/openhab-distro that referenced this pull request May 14, 2022
This adds two warnings for two breaking changes from the openhab-js library included in the JS Scripting Automation Add-On:
* openhab/openhab-js#67
* openhab/openhab-js#109

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
wborn pushed a commit to openhab/openhab-distro that referenced this pull request May 26, 2022
This adds two warnings for two breaking changes from the openhab-js library included in the JS Scripting Automation Add-On:
* openhab/openhab-js#67
* openhab/openhab-js#109

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
@florian-h05 florian-h05 added bug Something isn't working enhancement New feature or request labels May 27, 2022
@florian-h05 florian-h05 added the breaking change API breaking changes label Jun 9, 2022
florian-h05 added a commit to florian-h05/openhab-webui that referenced this pull request Jun 19, 2022
This updates `addItem(...)` and `updateItem(...)` for the breaking
changes from openhab/openhab-js#109.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
ghys pushed a commit to openhab/openhab-webui that referenced this pull request Jun 24, 2022
This updates `addItem(...)` and `updateItem(...)` for the breaking
changes from openhab/openhab-js#109.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Also-by: Yannick Schaus <github@schaus.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change API breaking changes bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can not create item metadata config using openhab-js
5 participants