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

Add items, links and metadata elements for YAML configuration #4085

Closed
wants to merge 2 commits into from

Conversation

J-N-K
Copy link
Member

@J-N-K J-N-K commented Feb 11, 2024

Related to #3666

This adds the items, links and metadata elements. This allows to use YAML to configure everything that could be configured in .items files. Currently only read-only access is allowed.

@J-N-K J-N-K added the enhancement An enhancement or new feature of the Core label Feb 11, 2024
@J-N-K J-N-K requested a review from a team as a code owner February 11, 2024 20:21
@spacemanspiff2007
Copy link
Contributor

Thanks for working on the new config files.
I looked at the test cases and have some questions:

  1. Is the version mandatory? If yes it should not be.

  2. What's the idea behind readOnly?

  3. Why did you use a dedicated name field in a list for something that's a unique identifier instead of a mapping?
    E.g. why use this

items:
  - name: TestGroup1
    type: Group

over this?

items:
  TestGroup1:
    type: Group
  1. Why is it category instead of icon if it's used to display the icon in the UI?

  2. For items:
    Why is it tags but groupNames ? It's either tagNames and groupNames or tags and groups. It should be groups because it's clear that it's not a group definition.

  3. Why is metadata a special section in the file?
    The long time goal is to bind metadata to the item (maybe even OH5 - fingers crossed).
    Why is there no option to set item metadata directly on the item?

  4. Why do I have to set the metadata namespace, why is it not another mapping?

Why this

metadata:
  - namespace: ns1
    itemName: testItem
    value: Foo
    config:
      key: newValue

over this

metadata:
    itemName: testItem
    config:
      ns1:
        Foo:
          key: newValue
  1. Why is links a special section in the file?
    As a user I want to link things to an item so why is there no way to create the link directly on the item?

  2. Why use itemName and channleUID instead of item and channel

  3. Why is it

items:
    type: Group
    groupType: Switch

but

items:
    type: Number:Energy

I've spend quite some time on a sensible and user friendly suggestion. It seems that your suggestion is just yaml wrapper around the rest API or the internal openHAB API.
Have you missed the comment or was it a deliberate decision to not use the suggestions?
If so could you explain your reasoning?

@J-N-K
Copy link
Member Author

J-N-K commented Feb 12, 2024

If it's that discussion again, no.

@J-N-K J-N-K closed this Feb 12, 2024
@rkoshak
Copy link

rkoshak commented Feb 12, 2024

I'd really hate to see us never get this capability.

If there is anything I can do here to change you mind please let me know. I can police the thread, reviews, what ever.

@spacemanspiff2007, don't let perfection become the enemy of progress. Any support for a YAML way to support definition of these things is a huge improvement.

I don't see anything here that changes that assessment.

  1. Because if the intent is to be able to support multiple versions of the file format for backward compatibility you need to know the format version in the first place.

  2. Because all text file defined stuff is read only right now. We don't need write access to have parity with what we have now. That can wait.

  3. Because the ID for an Item has always been called "name". Consistency with everything else in OH is more important than being precise in this one place.

  4. That's what it's called in the UI. Again, consistency.

  5. I see this as a minor quibble

  6. Because it's currently a separate thing. It makes sense to model the YAML the way it's actually implemented. And while it's your long term goal to merge them with the Item, I don't know that it is a universally agreed upon goal.

  7. It's probably easier to map to the JSON that way?

  8. See 6

  9. I see this also as a minor quibble.

  10. Because that's how they are done now.

Frankly, isn't is better to get something out and working and then it can be refined once we get some actual user feedback? And if I understand the intent correctly, the purpose of the required version field, is so that in the future we can actually change everything you've brought up, if it were deemed appropriate, without breaking backwards compatibility. So it's not like we are stuck with this as it is first implemented.

From what I can tell, this is fantastic! I hope it's not all for naught.

@J-N-K
Copy link
Member Author

J-N-K commented Feb 12, 2024

  1. Future versions might have different syntax, to allow detecting "old" versions and automatically rewriting them (e.g. on upgrade) it is necessary to know which version we have. Technically we could assume that "no version = version 1" but it is much clearer to state the version explicitly.
  2. The readOnly element is optional. The YAML model repository code already allows adding elements to models (and saving these models), but there is no further support. This attribute defaults to true, so omitting it makes the model read-only (to be consistent with the way DSL models are handled now).
  3. The model repository expects a list of elements, therefore the name is needed. This is also consistent with metadata or links, because they don't have a unique identifier (e.g. for links the unique identifier is constructed as itemName -> channelUID. Always having all parts of the element as fields is an easy-to-understand concept.
  4. That's the way it is currently named. It is also consistent with the code-view in UI. The idea here is that you can easily copy and paste either to textual configuration or UI.
  5. see 4
  6. Because metadata and links are not part of items. It also makes sense because you might want to define the links together with things instead of item. In general: if you want it as easy as possible, you should use UI. If you want a more advanced approach, it make sense to get the concepts behind and be "closer to code".
  7. see 6
  8. see 4
  9. This might be fixed by additional mapping. I'll check that.

@J-N-K J-N-K reopened this Feb 12, 2024
@spacemanspiff2007
Copy link
Contributor

Textual configuration is openHABs unique selling point and if you look at the forums there are many users who choose openHAB deliberately do so because of the ability to configure (almost) everything in files.
The idea of a new file format is to provide something that is attractive so users will be intrinsically motivated to switch to the new format. That means if the new format is too hard to use or doesn't provide enough convenience people will not switch and we'll effectively break their installations once the old format is removed.
I'm grateful that @J-N-K is working on improving textual configuration but I fear that "it's new and it's yaml" will not be enough to convince textual users making it a frustration situation for both sides: For @J-N-K because he invested all the time and effort while the users are still unhappy and for users because they shall now switch to something which does not meet their requirements and/or is hard(er) to use.

be "closer to code"

No, configuration may never be close to code. It's always an abstraction made for humans while code is an abstraction made for machines. If it's close to code it will be impossible to change the code without breaking the configuration. That will result in either many required configuration changes or code that is effectively locked and can never be improved or changed.
There always has to be an additional layer of abstraction, otherwise it's just a machine-to-machine interface.


  1. but it is much clearer to state the version explicitly.

I get the idea but in my experience this has never properly worked out. If it's a backwards compatible change missing values will be created with a default value so the old configuration is still valid. If it's a backwards incompatible change (e.g. additional required field) it's impossible to create a valid configuration from the old file so the user has to modify the file anyway.
But I don't have strong feelings about it.

3. The model repository expects a list of elements, therefore the name is needed.

It's because you made it that way. We have a unique identifier (name) that may not be entered twice and the appropriate data structure for that in a yaml file is a mapping. It's also much easier to write in the file and less error prone. You can also skip the duplicate item name check if you do it that way. Why not pull the name from the map down after de serializing from yaml e.g. in post processing?

3. This is also consistent with metadata or links

But there you can have multiple entries for the same name, that's different.

  • That's the way it is currently named. It is also consistent with the code-view in UI. The idea here is that you can easily copy and paste either to textual configuration or UI.

  • see 4

But in UI it's an automatically generated text field and does not have to be entered by hand.
Why not fix UI or align it with text config? Proper naming is extremely important because it makes things much easier and more clear.
Especially groupNames, tags, itemName and channelUID is very inconsistent and should be groups, tags, item and channel.
UI also signaled that when (de-) serializing to/from yaml become a core feature they'll change the code tab to that.
So it's not necessary to align to the current naming scheme to UI.

6. Because metadata and links are not part of items. It also makes sense because you might want to define the links together with things instead of item.

You could also allow a channels node on a thing. That way it would work, too. The links is always between a thing and an item so it makes sense that it can be configured on one or/and the other. The user will never want to create a link that is floating nowhere, that just doesn't make sense. Intuitively most of the users will want to configure the link on the item because that's how things work now and that's how it makes the most sense ("how will this item receive the value - through this link").

As of metadata I don't care about the metadata node on the root level as long as there is a way to configure it on the item. 99.99% of users create metadata in an item context (it's item metadata after all) and the others misuse it as a persistence service or global variable to overcome scripting and rule limitations.
So for the majority of the users there should an easy way to do what they want to achieve. And that is creating metadata on an item.


With the current state of the PR this one liner (wrapped around for readability) becomes 18 lines of yaml code.
The item name is used 4 times in this file making it very error prone. Every time an item is renamed one has to find all the usage which is distributed all over the file. And there is no way to generate warnings or infos to show the user that he might missed something. The different sections introduce a break in the edit flow. First I add the item, then I have to scroll to the links section and add the link there, then further scroll down to the metadata section and edit there, then scroll back up to the items section and continue with the second item.

Dimmer light_temp "Light" (BedroomLights) ["Light"] { 
  channel="zwave:device:controller:node3:switch_binary",  autoupdate="false", 
  homekit="Lighting.ColorTemperature"[ minValue=50, maxValue=400 ]
}
items:
  - name: light_temp 
    type: Dimmer 
    label: Light
    groupNames:
      - BedroomLights
    tags:
      - Light

links:
  - itemName: light_temp 
    channelUID: zwave:device:controller:node3:switch_binary

metadata:
  - namespace: autoupdate
    itemName: light_temp 
    value: false
  - namespace: homekit
    itemName: light_temp 
    value: Lighting.ColorTemperature
    config:
      minValue: 50
      maxValue: 400

This would be a great intermediate file format to e.g. create items and things from json and it would be great to have something that works that way. There everything can be aligned to code and that's not a problem because it's something that will be machine generated anyway (since json is a machine-to-machine format). Additional tooling could then just create the jsons and openHAB will load them accordingly. On the other hand if it would be possible to set/reset the readOnly flag through the RestAPI this would also be possible and we wouldn't need another file format.

But the current state of the yaml format is not very human friendly and the configuration files are meant to be edited by humans.
In my suggestion I deliberately made the yaml in a way that it still can be written as a one-liner so even though it's more verbose it's still possible to align the values in one line which resembles the current file format making transition easy and still allowing compact files.
Especially the many unnecessary repetitions and the break in editing to have to scroll to the proper place will make this a pain.


Frankly, isn't is better to get something out and working and then it can be refined once we get some actual user feedback?

Just because I'm a more vocal user doesn't mean that I'm not an "actual user" or that my points are invalid or overblown.
I'm a heavy textual configuration user so I'm fully in the affected/target group for this change and because I use it so often I think I have a pretty good feeling what works well in config files and what doesn't.

There are also many other ways to get feedback, e.g. make a small pilot, make a questionnaire on the forums with different configuration snippets and different ways of configuring something. Testing with a final product is always the worst idea because changes will always break something for someone and it's a lot of effort for everyone involved, especially the developers.

@J-N-K
Copy link
Member Author

J-N-K commented Feb 13, 2024

I give up. Do it better. I don't need it. I'll now promote to remove textual configuration completely the next time XText blocks progress on other parts.

@J-N-K J-N-K closed this Feb 13, 2024
@J-N-K J-N-K deleted the yaml-item branch February 13, 2024 07:44
@rkoshak
Copy link

rkoshak commented Feb 13, 2024

There are also many other ways to get feedback, e.g. make a small pilot, make a questionnaire on the forums with different configuration snippets and different ways of configuring something. Testing with a final product is always the worst idea because changes will always break something for someone and it's a lot of effort for everyone involved, especially the developers.

So ultimately, we are at the position that if it doesn't come out fully formed like Athena from Zeus's forehead, you'd rather not have anything at all and you want to litigate each and every little detail. Well, congratulations, you've gotten your wish.

Note, this wasn't proposed as a complete replacement for the existing file formats right now. It was going to run in parallel at least until OH 5 (if not beyond). In fact it was "a small pilot" similar to the Experimental Rules Engine was in OH 2. But you killed it before it got out the gate.

The idea of a new file format is to provide something that is attractive so users will be intrinsically motivated to switch to the new format. That means if the new format is too hard to use or doesn't provide enough convenience people will not switch and we'll effectively break their installations once the old format is removed.

Unless someone else picks this up (which I frankly don't see happening) an alternative file format is dead. You've done no favors to the file config loving OH users.

And this completely ignores the benefit that would occur from having the contents of the Code tab in the UI match the contents of the YAML file, making going back and forth super easy and providing a migration path in both directions. That alone would be hugely attractive to many users even if the file format were obscenely obtuse and verbose. It could be XML and the fact they match would be hugely attractive.

This also completely ignores that without some change along these lines, there is no path to editing text file configs from the UI, something else that is hugely attractive.

I'm a user too. And I frankly was really looking forward to this. It was going to solve so many problems, or at least be the first step towards solving a bunch of problems. Now I see no path towards:

  • better support for rule templates and custom ui widgets
    • ability to include more than one per template
    • ability to include widgets and rules in the same template
    • ability to include Items along with the rules or UI widgets
  • make it easier to help users on the forum no matter how they've defined their configs (looks and works the same whether managed or file based)
  • creation of configuration modules that combine Items, Things, UI, etc that can be shared

I'm certainly not going to rage quite over this but closing the door on the above (and more) because it's not perfect right now seriously makes me consider stepping back from the platform. I don't see a path forward where OH can grow as a whole and the text file based users experience and managed config users will continue to drift apart to the point where the two are using two completely different platforms.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature of the Core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants