Fix iOS Bluetooth read operation suspension issue

[rafaelfrancisco-dev]

Description

This pull request addresses an issue where Bluetooth read operations would suspend indefinitely on iOS devices due to an overly strict characteristic comparison in our event handling logic.

Problem

In our characteristic event handling, we were performing an identity comparison between two DiscoveredCharacteristic objects. On iOS, the system creates new instances of characteristic objects during operations, resulting in different object references despite representing the same logical Bluetooth characteristic. This identity check prevented read operations from completing successfully. This issue may be related to improper implementation of the library code on our side, but the current fix resolves the problem without negatively impacting existing functionality.

Solution

Removed the redundant identity comparison condition, allowing the code to properly compare characteristics by their UUIDs rather than by object reference. This ensures that characteristics are matched based on their logical identity rather than memory location.

[twyatt]

First off, thanks for the thorough explanation and PR!

Unfortunately, I think this will adversely effect the Android implementation, which already does a similar comparison.

Ultimately, we should lean on the overridden equality check, but fix it to properly compare by their UUIDs (as you mentioned).

Unfortunately, unlike Android (which provides an instance ID for characteristics), Core Bluetooth exposes no such API, so if two different characteristics exist with the same UUID (I admit, this is very unlikely, but it does not violate the spec) then our equality check should properly see them as not equal. At this point, aside from object reference, I'm not sure how this can be accomplished w/ Core Bluetooth.

It is an option that we could allow Apple platforms to have a "degraded" implementation, but I was really hoping to avoid that (and keep the platforms aligned) — perhaps it would make sense to have the equality check use UUIDs as well as the characteristic properties? 🤷 It's not perfect, but Core Bluetooth hasn't given us many other options.

[twyatt]

I just realized that on Apple platforms, instance IDs can be artificially generated during service discovery process.

Ok, I am running into some challenges with the instance ID approach. I'll look into it further in the coming days and keep you updated.

[twyatt]

On iOS, the system creates new instances of characteristic objects during operations, resulting in different object references despite representing the same logical Bluetooth characteristic.

@rafaelfrancisco-dev can you elaborate on this?

It seems that some libraries perform Equatable equality checks, while others perform identity comparison. So, it is unclear to me what the correct and/or better approach is.

It is possible either approach should work? Without knowing the implementation of Equatable for CBCharacteristic, we may never know. But being that both approaches are applied in libraries would hint that the CBCharacteristic objects provided to the didUpdateValueForCharacteristic are the same as those found in the discovered services. It is unclear to me why the current Kable code is not working properly. I'm more than happy to change/improve it, but I'm having trouble understanding what exactly we need to change it to.

Simply comparing UUIDs seems insufficient to me, since 2 characteristics can technically have the same UUID (which is why Android provides instanceId). I'd like to find a solution that can properly differentiate 2 different characteristics with the same UUID — if possible.

[rafaelfrancisco-dev]

On iOS, the system creates new instances of characteristic objects during operations, resulting in different object references despite representing the same logical Bluetooth characteristic.

To give you context, this problem started by our need to connect to a device that has two Device Information services (there is an issue outside our control where we can't override the default service already being advertised by the used hardware and opted to create a new service with the same exact UUID's, this, while an ugly solution, doesn't seem to be explicitly forbidden by the BLE spec).

So to get the correct service we iterate the discovered services and characteristics and get the last one matching the UUID for the characteristic we need at the moment.

val discoveredServices = services
    .filterNotNull()
    .first()

val infoServiceUuid = Uuid.service("device_information")
val modelCharUuid = Uuid.characteristic("model_number_string")

val modelCharacteristics = discoveredServices.flatMap { service ->
    service.characteristics.filter { char ->
        char.serviceUuid == infoServiceUuid && char.characteristicUuid == modelCharUuid
    }
}
val lastModelChar = modelCharacteristics.lastOrNull()
    ?: throw IllegalStateException("No readable characteristic found for $modelCharUuid")

val model = read(lastModelChar)

On Android devices this code snippet works perfectly, on iOS however the read would suspend indefinitely due to equality check failing on the when statement I deleted on this PR:

eventCharacteristic is DiscoveredCharacteristic && characteristic is DiscoveredCharacteristic ->
             eventCharacteristic == characteristic

The eventCharacteristic is a DiscoveredCharacteristic but the equality check would fail in the eventCharacteristic == characteristic check, which I can only surmise it fails due to being a new instance (since when we compare by UUID's the check is valid).

[twyatt]

Thanks for the clarification. Makes sense. I'll follow up soon.

[twyatt]

Apologies, I fell ill and am just now feeling better. I'll take another look at this in the next day or two.

[rafaelfrancisco-dev]

No need to apologize, take your time. In the meantime we've been using the fork, so there is no rush at all.

[davertay-j]

Simply comparing UUIDs seems insufficient to me, since 2 characteristics can technically have the same UUID (which is why Android provides instanceId). I'd like to find a solution that can properly differentiate 2 different characteristics with the same UUID — if possible.

As iOS may return new instances on each delegate response we cannot use object identity here, and we also cannot use solely UUID as some devices have duplicates. I'm wondering if the solution is to uniqify by enumeration which is the approach we settled on in Topaz browser

[twyatt]

No need to apologize, take your time. In the meantime we've been using the fork, so there is no rush at all.

@rafaelfrancisco-dev Thanks for being understanding/patient. ❤️

I've looked into this a lot more, and discussed with my colleagues — being that Core Bluetooth provides neither instance IDs nor source code; it is difficult to know the best approach to workaround Core Bluetooth's limitations.

We have some ideas of possible workarounds¹ but I'd really like to perform some thorough testing prior to committing to workarounds. As such, I plan to get #51 in a usable state (which I'm actively working on) before being able to resume work on this issue.

Footnotes

1. The current workaround idea is to artificially create instance IDs upon service discovery — special care will have to be taken to invalidate discovered references if service discovery happens again after connection is established or upon reconnecting. ↩

[fgroeger]

Is there any update on this? We currently run into the same issue that our code works perfectly fine on Android, but on iOS the observing does not work. We are now also using a fork for now, but we'd like to use the official implementation as soon as possible again. Another idea which came to our mind was to offer a method to observe on all characteristics, then the filtering could be done on the caller side - what do you think about that?

We also would like to support for this topic, but we want to clarify with you first what your preferred solution would look like.

[twyatt]

@fgroeger I understand that using a fork can be cumbersome. I had hoped to get to this sooner, but work projects have taken much of my time and I haven't been able to give as much time to Kable as I would like.

Hopefully I'll find some time soon, but in the mean time, I've created #1017 to provide a workaround (forceCharacteristicEqualityByUuid peripheral builder option).

When set to true, it will behave as this PR is proposing. Hopefully that will allow users that are affected by this a workable solution (while not breaking other consumers) until a proper fix can be implemented.

[fgroeger]

Great! That will help us probably. I will test it in our scenario and let you know. Thanks for the quick response!

[fgroeger]

It works fine for us, thank you! 🚀

[rafaelfrancisco-dev]

This solution also works on my end.
Thanks for your work !