ConfigStatusThingHandler behavior

[Nadahar]

First of all, I'd like to say that you have created a sort of "red tape loop" when it comes to potential issues with OH. I have repeatedly tried to discuss such matters on the forum, only to be told that "the developers mostly doesn't read this" and that the proper place to discuss this is in a GitHub issue. But, when I get here, the issue templates leads me back to the forum unless I have a very concrete bug to report (or a feature request).

It makes me feel that there's no place where I can bring up things that I find odd or strange without being 100% sure that it's in fact a bug that should be fixed. As such, I've deleted the template for this issue, as it doesn't fit this case at all. I'm not at all sure that this is a bug.

I'm working on a binding, and it's very closed to finished. So, I'm looking for potential problems in behavior, logs etc. just to "make sure". This has led me to wonder why ConfigStatusThingHandler does what it does, as it seems to me that it posts double "updates" when configuration is changed. My issue is with this part:

openhab-core/bundles/org.openhab.core.thing/src/main/java/org/openhab/core/thing/binding/ConfigStatusThingHandler.java

Lines 63 to 77 in 297d54e

	@Override
	public void handleConfigurationUpdate(Map<String, Object> configurationParameters) {
	super.handleConfigurationUpdate(configurationParameters);
	if (configStatusCallback != null) {
	configStatusCallback.configUpdated(new ThingConfigStatusSource(getThing().getUID().getAsString()));
	}
	}
	@Override
	protected void updateConfiguration(Configuration configuration) {
	super.updateConfiguration(configuration);
	if (configStatusCallback != null) {
	configStatusCallback.configUpdated(new ThingConfigStatusSource(getThing().getUID().getAsString()));
	}
	}

Both handleConfigurationUpdate() and updateConfiguration() calls configStatusCallback.configUpdated(), but handleConfigurationUpdate() also calls updateConfiguration() if the configuration has actually changed, at least in BaseThingHandler. Since super() is called before configStatusCallback.configuUpdated(), it will never be called if validateConfigurationParameters() throws an exception, so that's not a viable path for it to be "useful" either. In short, I can't find why it is useful to call configStatusCallback.configuUpdated() from handleConfigurationUpdate(), but it's hard to be sure that I have considered all scenarios.

I've tried to study how this came to be. In the original commit, configStatusCallback.configuUpdated() was only called from handleConfigurationUpdate(): eclipse-archived/smarthome@ab04004. This was later modified in eclipse-archived/smarthome@8cd40b6 to also be called from updateConfiguration() presumably because it wasn't always called when needed. The question is if the call to configStatusCallback.configuUpdated() should have been moved instead of duplicated.

If the call is redundant, it certainly isn't a major issue, but it might be causing some issues before I've found examples in several bindings where they have overridden the behavior to disable the call from handleConfigurationUpdate() under certain circumstances to avoid some kind of loop condition. So, it might have been causing some headache for binding developers. Regardless, the main motivation for me to file this issue is that I just can't make sense of it, important or not.