eclipse-archived / smarthome

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

Documentation and base implementation of handleConfigurationUpdate should be improved #1035

Open kaikreuzer opened 8 years ago

kaikreuzer commented 8 years ago

The example code for handleConfigurationUpdate does a

dispose();
updateConfiguration(configuration);
initialize();

This is imho not the way that implementations should do it (see my https://github.com/openhab/openhab2-addons/pull/650#discussion_r52759852). If they have no better logic, they should simply use the super implementation in BaseThingHandler.

Within BaseThingHandler.updateConfiguration, callback.thingUpdated() is called - I wonder why this isn't instead only calling callback.configurationUpdated()?

kaikreuzer commented 8 years ago

@sbussweiler Could you have a look at this? Am I missing something regarding the base implementation?

kaikreuzer commented 8 years ago

@sbussweiler Ping :-)

sbussweiler commented 8 years ago

We will clarify the impact offline ;-)

sbussweiler commented 8 years ago

As we discussed, BaseThingHandler.updateConfiguration() should also call callback.configurationUpdated(). I will update the implementation and the documentation.

markusjw commented 7 years ago

What if a binding programmer wants to override handleConfigurationUpdate(), but mainly use the suggested code of BaseThingHandler? The variable callback is private. What should an overriding method do to fire all necessary notifications?