Verbose debug logging and status handling of the CoordinatorHandler

[triller-telekom]

Without this PR the coordinator reacts immediately on changes from the
underlying zss library. However this leads to correct but problematic status
changes, for example: ONLINE -> OFFLINE -> ONLINE

This happens during initialization where the status from the zss library
reports: UNINITIALISED -> ONLINE -> INILIALISING -> OFFLINE -> ONLINE -> ONLINE

The problem with these changes are that the children of such a bridge handler
react immediately with bridgeStatusChanged() on whatever state the bridge
has.

Inside the bridgeStatusChanged() a child ThingHandler might also want
to initialize the network inside the zss library and this will cause
unexpected results.

Signed-off-by: Stefan Triller stefan.triller@telekom.de

[cdjackson]

I would suggest that it's better to solve this issue in the library rather than adding the flag here - what do you think? In any case, I will look at the library before merging this as it doesn't look like the best notification sequence.

[cdjackson]

@triller-telekom please can you advise what dongle you are using.

[cdjackson]

I've updated the framework to better handle transitions. It restricts some transitions, and hold off state transitions until the dongle initialisation is complete. It ought to resolve this at source, thus negating the need for this PR.

zsmartsystems/com.zsmartsystems.zigbee#432

If this resolves the issue I'd suggest to revise this to remove the coordinator transitions, and keep the other fixes/debug logging etc.

[triller-telekom]

I already thought about fixing this this in ZSS, but didn't think it through all the way you did. Since you knew the code better you have done a larger refactoring in the ZigbeeNetworkManager and I just tried it and it works as expected. Thank you!

So your fix is even better and I have removed it from this PR. This PR now only contains that:

• I do not set the coordinator to ONLINE if it already is ONLINE (just to be 100% sure since it happened in the past and we do not want the bridgeStatusChanged() method of child things to be called).
• a child thing is set to UNKNOWN instead of OFFLINE while its scheduled init job still runs.
• a couple of debug messages to see whats happening during initialization.

@cdjackson Please have a look again. Thanks.

[cdjackson]

Thanks for the update @triller-telekom - just one question, although depending on the answer I might have further thoughts ;)

[cdjackson]

As a matter of interest, why does this "confuse child things"? Do children get the bridgeStatusChanged callback? If so, I just wonder if this transition change code shouldn't be put into the framework so that children are only notified of bridge changes?

Or am I missing something else?

[triller-telekom]

Do children get the bridgeStatusChanged callback?

Yes I have observed this. My bridge (ember coordinator) was ONLINE and ZSS reported ONLINE again and this has triggered the bridgeStatusChanged of the child things.

But I think its fine for ZSS to report ONLINE if it already is ONLINE, because then the user of the ZSS library (in this case the bridgehandler) can decide what to do with this information. I.e. the bridge might have missed a transition to ONLINE before and only get the second call with ONLINE. In such a case it can decide on its own if it accepts the second ONLINE call.

WDYT?

[cdjackson]

I think its fine for ZSS to report ONLINE if it already is ONLINE

With the updates I made a few days ago, ZSS should not report ONLINE if it is already ONLINE - only changes are reported.

Anyway, that wasn't really where I was coming from, and I just realised I used the term "framework" to mean ESH, but I think you interpreted it to be the ZSS framework...

My point was that shouldn't we be able to use the updateStatus method and set the same status without "confusing child things"? ie the framework should not call bridgeStatusChanged if, well, the bridge status did not change (right?). So my point is that shouldn't this check that you have added be added to ESH framework so that it doesn't call bridgeStatusChanged, and therefore doesn't confuse the child things if the bridge status didn't really change?

[triller-telekom]

You are right, this check already exists:

https://github.com/eclipse/smarthome/blob/master/bundles/core/org.eclipse.smarthome.core.thing/src/main/java/org/eclipse/smarthome/core/thing/internal/ThingManagerImpl.java#L231

So, sorry about the confusion. What I had observed was probably related to the oscillation of the bridge status, which you have fixed in ZSS now.

I have removed the check from this PR.

[cdjackson]

Thanks.

[cdjackson]

LGTM