Fix for accessory configuration on restart

[gbrooker]

Split out from PR #59

Prior to this fix, restarting a bridge incremented the configuration count each time, causing HomeKit to forget room settings.

Add didChangeAccessoryList to DeviceDelegate.

[Bouke]

Yeah this has been bugging me as well. However the solution proposed would assume that after reinstantiating the device, the configuration hasn't changed. This isn't true as well, one could've changed the accessories / services / characteristics.

I think we should generate and store a hash value of the current device setup. We can then compare the hash after instantiation, and only when it differs, update the configuration number. To achieve this, we could create a property that returns a hash value that is build off the cascading tree of elements. (device -> accessories* -> services* -> characteristics*).

What do you think?

[Bouke]

See also commit 30e8c26, where I implemented this idea. How does that work for you?

[gbrooker]

The stableHash is a great idea. I have modified accordingly.

The initial value for Configuration.number is now back to 0, as in the current master branch, because on the first ever Device creation, the configuration number will be incremented and a hash number determined by updatedConfiguration()

[gbrooker]

There seems to be a problem with travis, so the second check failed for an unknown reason.

[Bouke]

So with the other changes, this PR mostly adds the didChangeAccessoryList event, which is fired after calling addAccessories(). How is this event useful? As the consumer of this API, the event seems not that useful to me.

[gbrooker]

Yes, what is left in this PR essentially adds a new method to DeviceDelegate, to allow owners of the Device to monitor changes in the list of attached accessories. This is useful if a bridge has some form of auto-discovery, and an owner can use the delegate method to update its interface if the accessory list changes.

[Bouke]

So the autodiscovery is something that's done by an external factor, I think? Wouldn't it be clearer that this external factor signal the update to the UI itself?

[gbrooker]

The auto discovery is performed by a HAP device, independently from the top-level application.

It is useful for the owner Device, the top level app which created it, to monitor changes through the DeviceDelegate. A change to the list of accessories could for instance cause it to update its user interface.

[gbrooker]

I've rebased this PR on the current master, so is now compatible with the nio network stack

[gbrooker]

The tests are failing due some travis error - it seems one of the test subsystem dependencies has an expired key.

[Bouke]

Ok I'm on board with this change. So maybe we also want to be able to remove accessories from the device? Then we would get these two delegate methods (replacing didChangeAccessoryList):

• didAdd(_ accessory: Accessory)
• didRemove(_ accessory: Accessory)

What do you think?

[gbrooker]

Ok I'm on board with this change. So maybe we also want to be able to remove accessories from the device? Then we would get these two delegate methods (replacing didChangeAccessoryList):

• didAdd(_ accessory: Accessory)
• didRemove(_ accessory: Accessory)

What do you think?

I don't see any harm in splitting into two separate callbacks, however do you propose the function is called for every single accessory added or removed ? Perhaps it may be better to pass the array of accessories added or removed by Device.addAccessories() or .removeAccessories in a single call, to reduce the number of times the notification is sent.

For my own use, which is to update a UI representation, it doesn't matter what has changed, just that something has changed. I reconstruct a table view based on the entire accessory list when didChangeAccessoryList() is called. For this purpose, there is no benefit to split into two different callbacks, as the additional information provided is not used.

[gbrooker]

I have rebased on the latest master and added additional methods to the delegate to pass the list of devices added or removed for those who need it. I have kept the simple notification method, because there is no need to know the specific changes when updating the UI, the current list whatever it is is just redrawn.

Can we integrate into master please ?