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

[RfC] Redundant channel information for managed things #3660

Open
jlaur opened this issue Jun 17, 2023 · 9 comments
Open

[RfC] Redundant channel information for managed things #3660

jlaur opened this issue Jun 17, 2023 · 9 comments
Labels
enhancement An enhancement or new feature of the Core

Comments

@jlaur
Copy link
Contributor

jlaur commented Jun 17, 2023

This is to be considered a discussion issue and a follow-up to #3330 (comment). I thought it would be more clean to have a distinct issue for tracking this.

Recently there were some discussions in this forum thread that triggered me to come back to this topic:
https://community.openhab.org/t/come-back-and-learn-how-to-use-file-based-configuration/147067

The issue (as I see it) is the difference between managed things:

{
  "danfossairunit:airunit:-1062731542": {
    "class": "org.openhab.core.thing.internal.ThingStorageEntity",
    "value": {
      "isBridge": false,
      "channels": [
        {
          "uid": "danfossairunit:airunit:-1062731542:main#current_time",
          "id": "main#current_time",
          "channelTypeUID": "danfossairunit:currentTime",
          "itemType": "DateTime",
          "kind": "STATE",
          "label": "Aktuel tid",
          "description": "Aktuel tid rapporteret af air unit\u0027en",
          "defaultTags": [],
          "properties": {},
          "configuration": {}
        }
      ],
      "label": "Danfoss Air Unit",
      "configuration": {
        "host": "ccm.local",
        "refreshInterval": 10,
        "updateUnchangedValuesEveryMillis": 60000
      },
      "properties": {
        "modelId": "w2/a2",
        "serialNumber": "xxx",
        "thingTypeVersion": "2",
        "vendor": "Danfoss"
      },
      "UID": "danfossairunit:airunit:-1062731542",
      "thingTypeUID": "danfossairunit:airunit"
    }
  },

[...]

and file-based things:

Thing danfossairunit:airunit:a2 "Danfoss HRV" [host="ccm.local", refreshInterval=10,
updateUnchangedValuesEveryMillis=60000]

For managed things there is some information stored which is not stored for file-based things:

  • Label
  • Description
  • Properties
  • Channels (including label, description, channel type UID, auto update policy and properties)

All this is redundant. In my experience redundancy usually will haunt you because of bugs in synchronization, i.e. failure to
properly maintain the redundant data.

This seems to be the case here also.

In 4.0 channel migration is partially solved by #3330 thanks to a massive effort by @J-N-K. With "partially" I mean no disrespect, but we still have at least these issues:

And this would have been avoided:

Example - migrated Thing:

image

Recreated Thing with correct channel order and new channels in configured language:

image

Additionally, even with this migration mechanism in place, contributors are required to provide upgrade instructions for maintaining managed things (they are not needed for file-based things). These instructions are redundant themselves, since actually all information is already available. Otherwise file-based things wouldn't work. They are also error-prone and need proper testing for each binding removing/updating or adding channels.

It seems also that there is a lot of complexity for being able to synchronize/maintain things.

Therefore, I'm wondering if managed things could be simplified by aligning with file-based things in the sense that the redundant information is removed from jsondb and instead created runtime - similar to how this works for file-based things. I'm suggesting/asking this only from a logical standpoint - I'm not familiar with the code.

When re-reading #3330 (comment), if I understand it correctly, this comment assumes that we have to deal with this redundancy. I understand this can be difficult, and just to be clear: I do not suggest to complicate it even further, but actually to completely remove everything from jsondb which is redundant.

The benefits would be:

  • Simplification - leading to fewer bugs.
  • Easier and less error-phone for contributors to add/update/remove channels.
  • Existing bugs/short-comings would be automatically resolved, for example the three mentioned above.
  • One benefit of using file-based things would be eliminated - the argument that managed things from time to time might need to be recreated to be fully correct would disappear.
  • Changing language would have effect on labels and descriptions (at least after a restart).

I don't know if this is feasible at all and/or complete nonsense, and maybe it has already been discussed in the past?

Cc @J-N-K, @lolodomo, @kaikreuzer - tagging you directly as you probably have some direct feedback on these thoughts.

@jlaur jlaur added the enhancement An enhancement or new feature of the Core label Jun 17, 2023
@jlaur jlaur changed the title Redundant channel information for managed things RfC: Redundant channel information for managed things Jun 17, 2023
@jlaur jlaur changed the title RfC: Redundant channel information for managed things [RfC] Redundant channel information for managed things Jun 17, 2023
@J-N-K
Copy link
Member

J-N-K commented Jun 18, 2023

The information is NOT redundant. You can change the label or description of a channel (I don't know how to do that in text files, I guess you can do it also there). So in general the label and the description should be preserved when changing something like Number to Number:Temperature because there could be a good reason why the user changed the label.

@jlaur
Copy link
Contributor Author

jlaur commented Jun 18, 2023

You can change the label or description of a channel

How do you do that?

@J-N-K
Copy link
Member

J-N-K commented Jun 18, 2023

Go to the "Things", select a thing, go to "Channels", select a channel. Click either on "Channel details" or "Configure channel" (depending on the channel). Change/add label or description, press "Done".

@jlaur
Copy link
Contributor Author

jlaur commented Jun 18, 2023

Go to the "Things", select a thing, go to "Channels", select a channel. Click either on "Channel details" or "Configure channel" (depending on the channel). Change/add label or description, press "Done".

Thanks, I didn't see this in 3.4, but it works like you described in 4.0. Is it a new feature? And do you know why it's possible to edit label and description for a channel? I really don't see the use, so probably I'm missing something.

@lolodomo
Copy link
Contributor

Same here, I do not see the use to modify a channel label/description.

@J-N-K
Copy link
Member

J-N-K commented Jun 19, 2023

For extensible channels it's absolutely necessary, because there inheriting from the channel-type would result in channels with different ids but the same label.

But there is also a use-case for other devices: Think of a two-channel unwire input, standard labels "Input 0", "Input 1" but my device has them labeled as "Input 1" and "Input 2", changing the channel-label then makes a lot of sense. On an 8-channel switch the channels might be labeled "Relais 0" to "Relais 7", adding a description what is actually linked to the channel might be a good idea to keep track of that.

@jlaur
Copy link
Contributor Author

jlaur commented Jun 19, 2023

For extensible channels it's absolutely necessary, because there inheriting from the channel-type would result in channels with different ids but the same label.

Good point, agreed, and this also corresponds to the behavior in .things files where you can at least provide label for such channels.

But there is also a use-case for other devices: Think of a two-channel unwire input, standard labels "Input 0", "Input 1" but my device has them labeled as "Input 1" and "Input 2", changing the channel-label then makes a lot of sense. On an 8-channel switch the channels might be labeled "Relais 0" to "Relais 7", adding a description what is actually linked to the channel might be a good idea to keep track of that.

Wouldn't it be sufficient to use proper item labels/descriptions for such cases? Renaming labels for channels provided by a binding seems to just obscure things. You also risk having the channel labels/descriptions overridden by an upgrade.

Anyway, this seems like a corner case that of course could be discussed. We still need channel label, at least for extensible channels, but what about the other data provided in the JSON mentioned - do we agree that it contains at least some redundancy, especially for channels, and that this has been - and still is - causing problems?

@openhab-bot
Copy link
Collaborator

This issue has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/thoughts-on-upgrading-bindings-to-use-new-uom-features-for-humidity/153615/10

@splatch
Copy link
Contributor

splatch commented Feb 4, 2024

The information is NOT redundant. You can change the label or description of a channel (I don't know how to do that in text files, I guess you can do it also there). So in general the label and the description should be preserved when changing something like Number to Number:Temperature because there could be a good reason why the user changed the label.

As far I know thing files provide only a way to specify channel label. They DO NOT have any way to provision description. The xtext/xbase syntax haven't been extended since a long time, I believe its mostly the same. Its still smells with ESH. ;-) Same thing applies to channel and thing properties - both elements will be read only. Thing defined in file can't be amended by handler through editThing() call, channel list can't be mutated.
I am not certain how important thing properties are. To my knowledge in some situations they are used as sort of workaround to pass information between discovery and acceptance of a thing (framework automatically maps matching properties to config parameters). They store sometimes serial number etc. which is displayed only for information purposes.

Nonetheless, these are not available when using xtext stuff.

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

No branches or pull requests

5 participants