Things should automatically inherit channels from their (current) thing type

[kaikreuzer]

I know that this has been discussed already, but I couldn't find an according issue for it, so I am entering one:

There is currently a mismatch in how Things are created that are read from a) the storage and b) from DSL.

In the DSL case, the Thing is constructed based on its type (and the channels declared within this type). Channels that are specified in the DSL are additionally added to it.

The ManagedThingProvider in contrast stores the Thing including all its channels and reads it back from the storage in the same way. As a result, channels that are newly introduced in a thing type do not end up on existing things. They have to be deleted and newly created in order to show the new features.

Imho we should change the ManagedThingProvider to work similar as the GenericThingProvider: It should only store what is not already defined on the thing type and always make sure that a loaded Thing has all channels that are defined on the current version of its type.

[sjsf]

Fully makes sense from my point of view. However, the implementation is going to be a little bit tricky as now the ManagedThingProvider will also have to wait until all XMLs have been processed, i.e. the ThingTypes are available. Just wanted to point out this limitation which we would newly introduce.

[kaikreuzer]

Alternatively, could we have a service that updates Things in the registry dynamically when a thing type appears? That way, it could be the same mechanism (and implementation) for Managed+Generic and we would not even block the creation of Things in case no type information is available.

[maggu2810]

Imho we should change the ManagedThingProvider to work similar as the GenericThingProvider: It should only store what is not already defined on the thing type and always make sure that a loaded Thing has all channels that are defined on the current version of its type.

Make sense for me, too.

is going to be a little bit tricky as now the ManagedThingProvider will also have to wait until all XMLs have been processed

Wouldn't it be make sense to wait for all the thing handling (and perhaps other stuff) until all XMLs of a binding has been read?
This would also solve stuff #1550

[cdjackson]

I've just seen this issue. This is a major concern with ZWave since thing types change reasonably often and there needs to be a way to update the definitions without deleting all the things and adding them back again each time.

In #4958 I proposed a different solution to this - to version the thing types and allow people to manually update (ie the UI tells them there is a new definition available, and they click a button in the UI to update to the latest version).

One major concern I would have with the solution proposed here is it will automatically break peoples systems if they update and their channels suddenly change names. Any rules, linked items etc will be broken and they probably won't understand why. I would suggest that some user input, or at least the option for people to disable the auto update, should be considered to avoid unhappy users.

I guess you could claim that if a user is upgrading their binding, then they should expect things like this. Maybe, but if they have a fully working system with 150 things, and they need to upgrade as they have purchased a new device, they probably won't expect to have lots of work to make their system run again.

[J-N-K]

I agree with @cdjackson, it should require user input to update the thing, at least if linked channels are removed or changed. Deleting or changing unlinked channels or adding new channels is safe, so this could be done in the background.

[kaikreuzer]

@cdjackson What you describe rather sounds like a binding that continuously breaks its own backward compatibility. I think this is something that should be addressed in the first place.

The normal case when upgrading should be that new channels are added and users want to see them supported - this is mainly what drove the creation of this issue and which would be solved nicely.
An upgrade should not break a users system and should not require any change in the Thing (apart from possible adding stuff), be it automatically or manually.

For users that do not want their thing definitions to be altered, they should anyhow best define them textually.

In general, you must consider that Things can be provided by different ThingProviders - they do not at all have to refer to any specific thing type and their type can even be dynamically changed. A "version" property simply doesn't fit in the there, imho.

[cdjackson]

What you describe rather sounds like a binding that continuously breaks its own backward compatibility

I'm not sure I understand what you mean. There is no breaking of backward compatibility.

Let me try and explain.

The ZWave binding supports many different channels - these are not changed. The channels link to ZWave functions - again, this doesn't change. So there's no backward compatibility that is changing - at least not in the software.

What does change though is the XML files. The XML files configure what channels are used for a particular thing, and how these channels are configured for the device to binding interaction. So, if we decide that the best way for a device to be configured is to use a specific channel, with a specific configuration, this is put in the XML. Maybe later it's decided that this needs to change for some reason, so the XML is updated.

For users that do not want their thing definitions to be altered, they should anyhow best define them textually.

Bu the thingType defines the thing - at least in ZWave. This is a feature we added a long time ago. Channels have properties, these properties are stored in the XML and this is the configuration I am specifically talking about here. The user can't configure this.

In general, you must consider that Things can be provided by different ThingProviders

Maybe this works for simple things and simple bindings...

I know in other bindings, the thing handler provides much of this functionality, and I think this is why you think that there is a backward compatibility issue. However, in the ZWave binding, much of this is configured in the thing files - otherwise we'd simply have hundreds of handlers to handle all the different versions and options that devices support.

[ivivanov-bg]

How about delegating this to the ThingManager - it anyway waits for the xmlThingTypes parsing, so when doing doRegisterHandler it can also migrate the thing type

[mgeramb]

Is there any progress in this discussion? I thinks it's not a good idea to let users recreate the things to get new features.
I would prefer a solution where things are automatically upgraded, because it's consistent with things defined in the files. And I do not see the sense of user input if a binding breaks its downward compatibility, because the old not supported channels will get broken in any way. So it's enough to remove the channels automatically and existing items get unlinked.

[J-N-K]

@mgeramb, silently removing the channel and it‘s link might result in unexpected and hard to find losses in functionality of the user installation. E.g. removing the rain probability channel of a weather forecast binding would leave the linked item in it‘s current state, this may be unnoticed for a long time.

[sjsf]

...as would a remaining channel which simply won't be fed by the new binding version anymore!

[J-N-K]

That’s sad and I don‘t argue against removing these channels. I‘m jut saying that the user should receive a clear notification.

[ivivanov-bg]

And what is the problem together with the channel to also remove the item (and the item-channel link) ?

[mgeramb]

@J-N-K normally it should not happen that a binding is not downward compatible, that‘s the bad thing in my opinion. I don‘t know how this should be handeled by the openHAB framework, because the configuration is broken in this case in anyway.
The more important discussion should be, how new channels could be added, because the recreating of things by the user is not the way how it should be for a good experience.

[J-N-K]

@mgeramb, silently ADDING channels is what I would expect, this just gives new functionality which is unused at that time. REmOVING channels may break installations, this should not happen without a big red sign notifying the user that something might go wrong.

[cdjackson]

There is also a "change" case - not just add and removing channels! Channels have properties, and configuration. If this changes then it will also break installations.

Again, consider the cases I wrote above for ZWave where channels are configured through the properties and not just hard coded in Java. Changing this configuration may have an impact on peoples configurations.

[ivivanov-bg]

Well - I think - when anything is changed on the thing configuration (either adding, removing or updating channels) - the one who makes the changes probably won't think like: "OK, I'm removing this channel for all new pairings, but someone might have old version so let's handle it anyway in the code - just in case"

So - let's look from this site - when thing declaration is changed - the already paired things should be handled either:

1. By the binding developer - doing what ever he things would be better (and this means on each initialization check if the thing is updated and if not - update it)
   or
2. By the framework using well defined principal and guaranteeing that when the handler is initializing - the thing will already be up to date with the latest thing type definition

[cdjackson]

This is why my proposal (in another issue I think) was to simply raise a flag in the UI to indicate there were changes, and allow the user to update the thing. Then at least it’s done consciously and if any changes occur, they at least know why - they don’t just suddenly end up with “strange” changes to their system.

[maggu2810]

I have done something similar in one of my non-public bindings.
Every thing type contains a property containing that version.
A thing contains its thing type version -- so the version of the thing type it reflects.
If the version if the thing type itself differs from the one of the thing there will be triggered a special logic.

[J-N-K]

How do we proceed here? When the OneWire binding was introduced I originally implemented something like the thing type version @maggu2810 mentioned. It was removed by review request because of this more general approach. What is the current idea here?

[kaikreuzer]

I still think that the most important change is the one I have described when creating this issue:

Imho we should change the ManagedThingProvider to work similar as the GenericThingProvider

Currently, we have a different behavior for DSL + Paper UI defined Things, which is a bad situation.
Having both acting the same way would be important.

To also cover exotic cases as the one for Z-Wave, we should imho have a separate discussion (and the solution might be as simple as adding a "persist" flag to channels in a thing type, so that those are written to the thing as if they were manually added).

So if there is any volunteer to adapt the ManagedThingProvider, feel free to come up with a PR.

[J-N-K]

So the idea is to mainly duplicate the thing creation by the GenericThingProvider (i.e. create the thing from the ThingHandlerFactory) and merge that with the thing provided by the StorageService?

[kaikreuzer]

Yes, I think that describes the idea correctly. Maybe not "duplicate" the code, but rather move it to a common place and use it from both places if possible.

[J-N-K]

There is a problem: when creating things from DSL, first a new thing is created, then additionally defined channels are added. Channels from the XML that are no longer present in newer XML versions are not created.

When things are created from the database there is no way to distinguish between user-added channels and channels removed from the thing.

I can therefore add new channels from the XML but not delete removed channels. Any idea?