Support multiple characteristics with the same UUID on Apple#1218
Draft
twyatt wants to merge 4 commits into
Draft
Support multiple characteristics with the same UUID on Apple#1218twyatt wants to merge 4 commits into
twyatt wants to merge 4 commits into
Conversation
Core Bluetooth (unlike Android) does not provide instance ids for services/characteristics, so artificial instance ids are generated during service discovery (as positions within the cached service array). Discovered service/characteristic/descriptor equality is now based on UUIDs + instance ids (mirroring the Android implementation), rather than object reference. Characteristic change/error events are resolved (by object reference) against the cached discovered services, falling back to UUID-only association when the characteristic cannot be found. Fixes #1016 Co-authored-by: Travis Wyatt <travis.i.wyatt@gmail.com>
Co-authored-by: Travis Wyatt <travis.i.wyatt@gmail.com>
Co-authored-by: Travis Wyatt <travis.i.wyatt@gmail.com>
CBMutableService/CBMutableCharacteristic initializers are unavailable (prohibited) on watchOS, so the tests cannot compile in appleTest. Co-authored-by: Travis Wyatt <travis.i.wyatt@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #1016 (bug originally reported via #875).
Core Bluetooth — unlike Android — does not provide instance IDs for services/characteristics, so previously the Apple implementation fell back to object reference (
===) equality forDiscoveredService/DiscoveredCharacteristic/DiscoveredDescriptor. That prevented reliably distinguishing (and associating I/O events with) multiple characteristics that share the same UUID, causing operations (e.g. reads) to stall indefinitely (#875).Solution
Apple platforms only (Android already relies on system-provided instance IDs):
Connection.discoverServices), each service is assigned aninstanceIdequal to its position in the cached service array, and each characteristic aninstanceIdequal to its position within its parent service's characteristic array.PlatformDiscoveredService/PlatformDiscoveredCharacteristic/PlatformDiscoveredDescriptorequals/hashCodenow compare UUIDs + instance IDs (matching the Android implementation) instead of object references. Two different characteristics with the same UUID now compare as not equal, while re-wraps of the same underlying characteristic (e.g. across service re-discovery) compare as equal.PeripheralDelegatenow resolves theCBCharacteristicfromdidUpdateValueForCharacteristic(by object reference) to the correspondingPlatformDiscoveredCharacteristicfrom the most recent service discovery, soisAssociatedWithperforms an exact (instance-aware) match. If the characteristic cannot be found in the cached services (e.g. event received while a service re-discovery is in-flight), it falls back to UUID-only association (equivalent to theforceCharacteristicEqualityByUuidworkaround behavior) with a warning logged.ObservationEvent.CharacteristicChange/Errorhad theircharacteristictype widened fromPlatformDiscoveredCharacteristictoCharacteristic(internal API) to allow the UUID-only fallback; other platforms are unaffected.This also removes the force-unwraps of the weakly-referenced
CBCharacteristic.service/CBDescriptor.characteristicback-pointers inProfile.kt(parent wrappers are passed down during discovery instead).Notes
forceCharacteristicEqualityByUuid(@ObsoleteKableApi, annotated "will be removed after Support multiple characteristics with the same UUID #1016 is fixed") is intentionally left in place; it can be removed in a follow-up PR once this fix is validated.DiscoveredCharacteristicarguments by reference to the underlyingCBCharacteristic(obtaininProfile.kt), so no change was needed there; this PR fixes the event-association side (which previously relied on wrapper equality).Testing
ProfileTestscover instance ID assignment, equality of same-UUID characteristics (within a service and across same-UUID services), stability across re-wraps, andfindCharacteristicreference lookup (built onCBMutableService/CBMutableCharacteristic). Placed inmacosTest(rather thanappleTest) because theCBMutable*initializers are unavailable on watchOS.commonTestObservationEventTestscover UUID-based event association (isAssociatedWith).assemble+checkon macOS (includingmacosArm64TestandiosSimulatorArm64Test) pass, along with all other platform jobs. Also verified locally (Linux host):compileKotlinJvm,compileKotlinJs,compileKotlinWasmJs,compileAndroidMain,jvmTest,testAndroidHostTest,lintKotlin,jvmApiCheck, andklibApiCheck.