fix: scope BLE service-completeness check to services we actually implement - #116
Merged
Merged
Conversation
…lement The prior commit made the announced-vs-discovered comparison tolerant of same-family custom UUID drift, but a device can also advertise a service UUID that isn't part of any implemented sensor's family at all - e.g. Tacx NEO Bike Plus announces a bare "1000" fragment (not a real SIG service number, most likely a firmware/advertising-packet-size shorthand or scan-library artifact for its FE03xxxx custom service) alongside its two real, correctly-discovered standard services. That device never connected in production - confirmed not a transient registration race. checkAnnouncedServices() now filters the announced list down to services BleInterface.getSupportedServices() actually cares about (the same list already announced upfront to bindings like WebBluetooth's optionalServices, sourced from every registered sensor implementation's declared services) before checking completeness - an announced UUID no sensor of ours ever uses is simply irrelevant, not a completeness gap. Combined with the existing isSameServiceFamily() leniency, this still requires an exact (or same-family) match for any service we actually implement against - e.g. the MRK-R15-D829 rower's genuine FTMS registration race is unaffected, since FTMS is squarely in the expected-services set. DirectConnectInterface (WiFi/mDNS) picks up a trivial getSupportedServices() stub to satisfy the now-extended shared IBleInterface contract - it has no BLE-style completeness check of its own.
|
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.



Summary
[1818, 1826, 1000].1000is a bare, standalone entry that doesn't correspond to any real GATT service number in a range we recognize — most likely a firmware/advertising-packet-size shorthand or scan-library artifact for theFE031000-...custom service also visible in discovery (the digits line up exactly), but the device is a smart trainer, not something with a custom Tacx/Wahoo protocol we implement for it — nothing we implement ever needed that UUID in the first place.checkAnnouncedServices()(src/ble/base/peripheral.ts) now filters the announced list down to servicesBleInterface.getSupportedServices()(new method,src/ble/base/interface.ts) actually cares about — the same list already announced upfront to bindings like WebBluetooth'soptionalServices, sourced from every registered sensor implementation's declaredservices— before checking completeness. An announced UUID no sensor of ours ever uses is now simply irrelevant, not a completeness gap.isSameServiceFamily()leniency, this still requires an exact (or same-family) match for any service we actually implement against — the originalMRK-R15-D829rower FTMS registration race is unaffected, since FTMS is squarely in the expected-services set.DirectConnectInterface(WiFi/mDNS,src/direct-connect/base/interface.ts) gets a trivialgetSupportedServices()stub to satisfy the now-extended sharedIBleInterfacecontract — it has no BLE-style completeness check of its own.What changed
src/ble/types.ts:IBleInterface.getSupportedServices(): string[]added to the contract.src/ble/base/interface.ts:BleInterface.getSupportedServices()returns the cachedexpectedServicesfield (the same value last announced to bindings).src/ble/base/peripheral.ts:checkAnnouncedServices()filtersannouncedto entries matching (viaisSameServiceFamily) something ingetSupportedServices()before checking completeness.src/direct-connect/base/interface.ts: trivialgetSupportedServices()stub (returns[]).interface.unit.test.ts(new case for the getter),peripheral.unit.test.ts(NEO Bike Plus regression repro using the real production log UUIDs, plus an "expected custom UUID with a genuinely different base still fails" case isolating that scenario from the new filtering).Test plan
npm test— 1165 passed, 35 skipped (pre-existing skips, no change), 0 failednpx tsc -p tsconfig.esm.json --noEmit— clean