Fix race condition in ANCS discovery by adding MaybeFinishDiscovery helper

[tituscmd]

Heyo!

This PR introduces a new helper method "MaybeFinishDiscovery" in AppleNotificationCenterClient to ensure that ANCS discovery is only marked complete once all all required handles are discovered and both CCCDs are successfully subscribed.

Previously, onServiceDiscovered was called too early (at the end of characteristic or descriptor discovery for example), which allowed iOS to send ANCS notifications before the client was fully initialized. I've replaced all those with the new helper method. I'm not sure if this is the best way of doing it, but it's been working so far and I have an uptime of 4 days right now.

With this change, onServiceDiscovered is only called once the ANCS client is actually ready. This prevents early notifications from causing errors due to incomplete discovery state.

[cyberneel]

This seems good to me, but I am wondering one thing.

Are all these characteristics and services found independently of each other, or are we guaranteed by BLE that they appear in some order where service is discovered last?

• If we are guaranteed service shows last, then I think it is fine.
• If we are NOT guaranteed these service shows last, then we should cache the service connection handle and act on it once all the descriptors are found.

This all depends on the order we are receiving things, and I don't know the answer to this question.

[cyberneel]

Basically, I am thinking, is there a possibility that we don't run the onServiceDiscovered call because we wait for the other things, and then we are never pinged about it from the device afterward, and don't end up connecting?

[tituscmd]

After digging around a bit, it seems that BLE doesn't guarantee any order; Services, characteristics, and descriptors are discovered via separate GATT procedures. My helper was merely checking that ANCS discovery didn't finish too early, which eliminated the crashes which were due to, well, early write operations.

In practice I definitely noticed that sometimes, even with the new helper, iOS notifications didn't seem to get sent over to my watch. That also points to the fact that there is no strict order.

Caching seems like a smart and logical way to go about this.

[cyberneel]

Alright, in that case, let's cache the handle for the service and just check each time we get a new handle, we see if that new information completes the data needed to connect.

So instead of ignoring the service due to other handles not being known, we check if each piece of information we receive was the last part needed on each handle receive call.

@tituscmd Do you think you could do this?

[tituscmd]

I'll definitely try! 😅

But I'll be busy for a few days, so it won't be very soon.

[cyberneel]

Yeah no worries.

If I have time before then, is there a way to edit this code without merging this first?

[tituscmd]

Yeah no worries.

If I have time before then, is there a way to edit this code without merging this first?

Yes, you can just fork my branch and then submit a pull request for it. Once I approve the PR, I’ll commit the changes, and this PR will be updated accordingly.

[cyberneel]

NOTE: Going to merge this for now, and I will then add the functionality I talk about here