deviceplug / btleplug

Rust Cross-Platform Host-Side Bluetooth LE Access Library
Other
812 stars 150 forks source link

Winrt backend forgets service advertisements #189

Closed rib closed 3 years ago

rib commented 3 years ago

Describe the bug

On windows I'm currently seeing that services may be advertised via CentralEvent::ServicesAdvertisement events and briefly be listed as a service under the corresponding peripheral's properties but then the service list will usually be cleared shortly afterwards due to a further AdvertisementReceived that doesn't include any service IDs.

This behaviour is currently causing some problems for me while I periodically query the peripherals (while scanning) and check associated services to recognise heart rate monitors. Although I can recognise the devices if I catch the ServicesAdvertisement event it's a lot more practical for my use case if I can rely on the properties for a peripheral too.

Expected behavior Once a service has been advertised for a peripheral then btleplug should remember the association and not clear services if we get advertisements without any service uuids included.

My current understanding is that BluetoothLEAdvertisementReceivedEventArgs::ServiceUuids should just be interpreted as a convenient collection of all UUIDS under all data sections for the advertisement with a consistent 128bit format. I think it should generally be assumed to be incomplete - and so advertised service UUIDs should be merged into the peripheral properties instead of replacing the set of services associated with a peripheral.

This is what Microsoft's API docs say:

An IVector of GUID, representing the list of service UUIDs in 128-bit GUID format. This property aggregates all the 16-bit, 32-bit, and 128-bit service UUIDs into a single list.

(ref: https://docs.microsoft.com/en-us/uwp/api/windows.devices.bluetooth.advertisement.bluetoothleadvertisement.serviceuuids?view=winrt-20348)

also this is what the (supplemental) spec says:

The Service UUID data type is used to include a list of Service or Service Class UUIDs.

There are six data types defined for the three sizes of Service UUIDs that may be returned: • 16-bit Bluetooth Service UUIDs • 32-bit Bluetooth Service UUIDs • Global 128-bit Service UUIDs

Two Service UUID data types are assigned to each size of Service UUID. One Service UUID data type indicates that the Service UUID list is incomplete and the other indicates the Service UUID list is complete.

An extended inquiry response or advertising data packet shall not contain more than one instance for each Service UUID data size. If a device has no Service UUIDs of a certain size, 16, 32, or 128 bit, the corresponding field in the extended inquiry response or advertising data packet shall be marked as complete with no Service UUIDs. An omitted Service UUID data type shall be interpreted as an empty incomplete-list.

The Service UUID data types corresponding to 32-bit Service UUIDs shall not be sent in advertising data packets

(ref: https://www.bluetooth.com/specifications/specs/core-specification-supplement-10/)

In particular "An omitted Service UUID data type shall be interpreted as an empty incomplete-list."

My interpretation here is to consider that an empty set of ServiceUuids here may just be because this advertisement was for other data besides service uuids.

Even if ServiceUuids is not empty I guess it could still be incomplete unless all the data sections containing uuids also indicate that the list was complete.

Instead of having to manually check the raw data ourselves it seems like it might be reasonable to always treat the list of services as incomplete and always union with the set of services associated with a peripheral.

I don't know if it's ever possible for services to be removed from a device but if not then I guess it should be ok to always merge services IDs instead of replacing.

Btw, I'm happy to take a look at making a PR for this, I just figured I'd post an issue to keep track of this and check my interpretation here makes sense

qdot commented 3 years ago

@rib Yeah that sounds correct to me, I've seen similar when watching Service Advertisements in sniffers. Union'ing should be fine, so you can go ahead with the PR.

I think we've had maybe one or two devices where services appear after some sort of characteristic is sent (@blackspherefollower might remember), but that's such an outlier that I think we can ignore it until it's reported here.

blackspherefollower commented 3 years ago

I think there's some devices that only expose things after pairing.

I think that some of the other advertisement fields may suffer the same issue though (manufacture data?)

rib commented 3 years ago

okey, yeah, delays or conditions on when data and services get advertised sounds like something to expect.

what would be more awkward is if devices ever wanted to explicitly revoke services that they previously advertised. I'm guessing that's very unlikely / unheard of? Supporting that on Windows would mean having to pretty much ignore their ServiceUuids info and always manually parse the raw data and make sure that uuid lists marked as 'complete' can replace a set of services associated with a peripheral (and effectively remove ones not in the list)

blackspherefollower commented 3 years ago

I think the advertisement packets only contain one attribute a go, which is why the name wipes the services, etc on the Windows advertisements. In the unlikely case that services get removed, maybe it's safer to copy the service UUIDs if the property is populated (as opposed to taking the union)

rib commented 3 years ago

From what I've seen advertisement packets can contain multiple raw data sections - e.g. from testing earlier I was seeing groups of 2, 3 or 4 sections arrive together e.g. combining a list of 16bit uuids, plus appearance state, and a 'complete local name'.

On Windows it looks like the LocalName and ServiceUuids are just provided as a convenience over the raw data. Where the LocalName abstracts over a 'complete local name' or a 'shortened local name' and the ServicesUuids abstracts over 6 different raw data types for 16, 32 and 128 bit uid lists that are either explicitly complete or incomplete.

My impression atm is that the best default would be to always union because there are cases where ServiceUuids can be populated based on 'incomplete' lists of service uuids - which looks like a common case.

The corner case where it looks like replacement could potentially make sense is if ServiceUuids isn't empty and if you were to inspect the raw data and see that a 'complete' list of 16, 32 and 128bit services were provided - (and I guess the device would technically have to report all three since otherwise there's always the possibility that additional services could be added via the others).

Tbh, now I write that out I tend to think that fundamentally Bluetooth isn't designed to make it possible for a device to revoke an advertised service, considering how fiddly/contrived it would be to recognise unambiguously.

rib commented 3 years ago

I think this can be closed now that https://github.com/deviceplug/btleplug/pull/195 was merged