search

eclipse-archived / smarthome

Eclipse SmartHome™ project
https://www.eclipse.org/smarthome/
Eclipse Public License 2.0
862 stars 783 forks source link

DSL specified properties are not handled correctly #2215

Open cdjackson opened 7 years ago

cdjackson commented 7 years ago

When we add properties using the DiscoveryResultBuilder.withProperties method, we can add properties and configuration and when the thing is instantiated from the inbox, (if I remember correctly!) the framework uses the configDescription to decide if the entry is a property or a parameter and moves them into the appropriate place.

This is not the case if things are defined in the DSL - in this case it seems that all properties are defined as configuration. It therefore seems to be impossible to add properties (or correct me if I'm wrong ;) ).

kaikreuzer commented 7 years ago

If it is as you describe, this sounds like a bug...

sjsf commented 7 years ago

correct me if I'm wrong ;)

no, you are right: PersistentInbox.java:197

There indeed only the configuration gets synchronized. However, if we would synchronize the properties too, then these get persisted as well. In #1682 we intentionally made properties be something transient, i.e. they cannot be changed in a persistent way anymore after a Thing was created. If we now synchronize them as well, we open up a backdoor again for bindings to permanently alter properties. Not through the ThingHandler, but via the DiscoveryService. That would be okay from my side, considering what I suggested in https://github.com/eclipse/smarthome/issues/1977#issuecomment-250448150.

@kaikreuzer any objections to fixing this bug by including the properties in the synchronization?

kaikreuzer commented 7 years ago

@sjka Hm, the issue is about DSL-defined Things, so what does this have to do with the Inbox and its synchronization?

sjsf commented 7 years ago

I was mislead by @cdjackson's introduction of the problem, I guess. You are right, the GenericThingProvider indeed does not set any properties, everything ends up as configuration. We indeed should add the same logic there...

sjsf commented 7 years ago

okayokay, working on it... ;-)

cdjackson commented 7 years ago

I'm not sure why these two issues came in at the same time, but thanks :)