[homekit] quick restart on network changes

[yfre]

homekit binding makes currently a full restart on network changes like IP address changes.
the full restart includes removing all openhab items from the list of homekit accessories and adding them again. basically, the start/stop methods for binding start/stop were re-used

this could lead in some case to config reset in home all, like reported here
https://community.openhab.org/t/homekit-no-response-error-on-all-openhab-items/118200/172

this PR reduces the stop logic to minimum and especially skip the removal of homekit accessories on stop.

the changes are following:

• slight rework on "start" function in order to remember the address the HomeKit was bound to
• replace call of the standard "stop" function copy the few needed step to onChange function
• add check to onChange function whether the removed address is the address HomeKit was bound to

Signed-off-by: Eugen Freiter freiter@gmx.de

[openhab-bot]

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

https://community.openhab.org/t/homekit-no-response-error-on-all-openhab-items/118200/175

[jlaur]

Regression of #11720, looking at this might ease review.

[Sail-Away]

Some additional hints: in my logs I see that the reaction to short network interruptions is somewhat random. One event either causes no restart, one restart or two consecutive restarts. Looks like, depending on the actual timing, the network change listener either doesn't catch the event at all, or just one added or removed interface event, or both the removed and added events.

So would it make sense to add some debouncing?

Also, what I don't understand: why would we restart the bridge at all if the interface has just been removed, or if a different, unused network interface changes? Shouldn't a restart only be necessary if the used interface actually changes to a different address? However, this would make it necessary to evaluate the reoved and added, so it's not as straight forward as the current solution.

[yfre]

Some additional hints: in my logs I see that the reaction to short network interruptions is somewhat random. One event either causes no restart, one restart or two consecutive restarts. Looks like, depending on the actual timing, the network change listener either doesn't catch the event at all, or just one added or removed interface event, or both the removed and added events.

So would it make sense to add some debouncing?
not sure about this behaviour. my idea was to keep it the logic small and simple so that we can quickly restart on every event.

Also, what I don't understand: why would we restart the bridge at all if the interface has just been removed, or if a different, unused network interface changes? Shouldn't a restart only be necessary if the used interface actually changes to a different address? However, this would make it necessary to evaluate the reoved and added, so it's not as straight forward as the current solution.

good idea. it just one check which does not make it too complex but maybe reduce number of restart. added to PR

[lolodomo]

LGTM

[Sail-Away]

Here's my first feedback after serveral days of testing (with a slightly modified code, as I also wanted to catch the added interfaces in the log; code can be found here). Really seems to be an improvement enhancing stability, had no issues so far.

Stopping and starting of the bridge now is skipped when an (anyway unused IPv6) interface gets removed or added, which happens quite often. Number of actual restarts thus is considerably reduced.

Actual restarts in case of a network interruption seem to behave well now, though accessories still get removed from the accessory list. This is due to the fact that on removing the interface all connections to subscribing devices get terminated, and in this case the SubscriptionManager unsubscribes all characteristics. However, as the restart of the bridge only is triggered by a second event when the network interface is added again, and this typically happens in the order of several seconds later, nothing gets messed up. To illustrate this, I copied a few lines from the log:
homekit_excerpt.log

Still I think that onChanged at least theoretically could be entered a second time before finishing the first time, as the network changes appear as separate events. So I wonder if it would make sense to lock onChanged (define it as synchronized).

One other small hint. I would change the log message
logger.trace("restarting HomeKit bridge on network interface changes.");
into something like
logger.trace("HomeKit bridge reacting on network interface changes.");,
as a call of onChanged now not necessarily leads to a restart.

[yfre]

@Sail-Away thank you a lot for testing and detailed feedback. it good to hear PR improves behaviour.

valid point with overlapping restarts and synchronized.
i will open new pull request for this.

[yfre]

@Sail-Away
here we go
#12205