batch notifications when possible

[ccutrer]

I've been running this for several days on my home installation, and it's definitely improved responsiveness when asking Siri to do something that involves multiple accessories.

[andylintner]

I like the idea here. I'm going to leave this pending until I have some time to dig into the implementation - should get to it in the next few days.

[ccutrer]

👍 it's definitely a bit more complex than the other PRs. One thing that could probably use another pass is the thread safety. Like I said, I'm still a Java greenie, so I wasn't sure here. I used a ConcurrentMap for the pendingNotifications because that's what the other fields are. But if my understanding of synchronized it correct, all accesses to member fields are already protected by that, so we don't need to use that thread safe data structure. It made me wonder if this class started without using synchronized, and then ran in to problems, so additional thread safety measures were added without simplifying other code (see also the double-check on the set when adding a subscription).

[yfre]

as first: batch is very useful and actually required by HAP spec.

but im wondering whether we should reply with "EVENT" to a subscription immediately (as batch or not).

currently we have following sequence of messages (recorded from home app on mac os):

Receive GET /characteristics?id=1958124085.9
Send HTTP/1.1 200 OK. {"characteristics":[{"value":false,"aid":1958124085,"iid":9}]}

Receive PUT /characteristics {"characteristics":[{"aid":1958124085,"iid":9,"ev":true}]}
Send EVENT/1.0 200 OK{"characteristics":[{"aid":1958124085,"iid":9,"value":false}]}
Send HTTP/1.1 204 No Content

so,
it as first for value via characteristics and then subscriber to the same characteristic and we replay firt with EVENT and then with No Content.

according to HAP specification, we should first send "no Content" and
"When the value changes, the accessory sends .. EVENT". i.e. only in case of changes.

[ccutrer]

This is an interesting point. Technically there's actually a race condition with how the controller gets the current value, then registers for notifications (the value could change between the initial GET, and the notification registration). While the probability is small, it goes up as the number of devices exposed on your bridge goes up, both from more devices, and for it taking longer to process these requests and responses. I read the spec when it says "When the value changes, the accessory sends .. EVENT" as emphasizing not to send EVENTs on a timer or something, but only on actual changes. Not necessarily that you shouldn't send an initial EVENT with the current state of the item.

I do agree that the first EVENT for a subscription shouldn't be sent until after we respond 204 to the request to register it.