Fix ThingManager is missing config description during normalization

[J-N-K]

Reported on the forum. In some cases the ThingManageImpl tried to initialize the thing before all config descriptions are loaded.

This can happen because the OH-INF/config directory is processed in parallel to the thing descriptions. In case the handler factory is added and the thing description parser sets the corresponding ReadyMarker before the config descriptions are completly parsed this results in a warning because the config description cannot be found during normalization of the thing or channel configuration.

Signed-off-by: Jan N. Klug github@klug.nrw

[openhab-bot]

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

https://community.openhab.org/t/openhab-3-4-milestone-discussion/138093/126

[splatch]

I am not sure approach in this PR is valid because relying on registering config descriptors in thing manager builds functionality which should be provided by config layer itself. Maybe a new ConfigDescriptionManager or ConfigDescriptorRegistry could help here?

Just an idea:
OH-INF/config -> ConfigDescriptorProvider -> ConfigDescriptorRegistry -> ConfigDescriptorManager -> ConfigDescriptorListener -> ThingManagerImpl

With this approach each step has its own responsibility and ThingManager can be free of internals of config descriptor discovery.

[J-N-K]

The problem with that approach is that the ThingManagerImpl can't know which config descriptions are needed (while that may be possible to check for the thing itself, the config descriptions for all channels of that thing can't be easily checked in advance (e.g. in the reported case on the forum the missing description is http:channel-config). Iterating over all channels, getting the config description URI and checking if they are available in the config description registry seems to be quite expensive compared to the solution we have now.

All config descriptions are available, when the bundle has been processed, so this is a sort of short-cut. The same approach is used for the thing descriptions themself, so I merely extended what is already there.

[splatch]

Thanks for explaining detail of issue. Still I don't think that tracking directly appearing of config descriptors within thing manager itself is fully justified. A basic fact that thing manager tracks thing types which might be provided by OH-INF entries, does not mean that it should keep eye on configs too.
Imperative approach taken now makes logic less clear. Making kind of a dependency set for things would not only simplify code, cause you then could just rely on ie. thingDependencies.satisfied() call, it would also let cover various kinds of requirements to activate thing under one interface.
Keep in mind that config descriptors both for thing types and channel types can be provisioned through ConfigDescriptorProvider. Tracking OH-INF entries might be incomplete to cover all bindings.

[J-N-K]

While it is incomplete it is at least better than what we have now.

I did check if an alternative approach is possible and that requires a BIG refactoring. It is quite easy to check if all dependencies are met (we can just check for the thing and all of it's channels if the type is present in the respective registry and if a config description uri is present and the config description itself is present in the registry).

What makes it difficult is that we need a trigger to check if a thing can be initialized. We currently use two of these: a ready marker when a bundle has finished loading thing types and the addition of a thing handler factory. This PR adds a third one - a ready marker when a bundle has finished loading config descriptions.

If we don't use the ready markers anymore, we would need to be notified of a change in ConfigDescriptionRegistry, ThingTypeRegistry and ChannelTypeRegistry. Unfortunately none of these implements change listeners.

[openhab-bot]

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

https://community.openhab.org/t/openhab-3-4-release-discussion/142257/131

[J-N-K]

Closing, will be superseded by another solution that also tries to solve #1924.