Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecating handle #888

Open
dlech opened this issue Jul 14, 2022 · 6 comments
Open

Deprecating handle #888

dlech opened this issue Jul 14, 2022 · 6 comments
Labels
enhancement New feature or request Opinions Appreciated Please add an opinion on your desired resolution on this issue!

Comments

@dlech
Copy link
Collaborator

dlech commented Jul 14, 2022

In #883 we realized that Android has an solution for differentiating services/characteristics that isn't compatible with the current Bleak notion of an attribute handle. The reason we expose the handle property in Bleak to begin with is to differentiate between two services or characteristics that have the same UUID. To get the value, we are using the attribute handle of the service/characteristic. However, this value is not the same cross-platform - it can be off-by-one between OSes. On macOS, we are using a private ObjC property to get the value (which could disappear some day and break Bleak). On BlueZ we have to do some string scraping to get the value from the D-Bus object path. And as we have just realized, on Android, there isn't a way to get the value at all.

But Android actually has IMHO a rather elegant solution. Each service/characteristic is identified by two pieces of information, the UUID and an "instance id" that is essentially a 0 based index of the occurrences of that UUID. We could use this idea in Bleak by allowing a tuple of a UUID and instance id as the char_specifier argument.

This seems much nicer that the existing situation. Currently, I think many users are just hard-coding the handle value, but as mentioned already, this doesn't work cross-platform since the value varies by OS, and it also breaks if the device has a firmware upgrade that rearranges the GATT attributes. The current way to avoid these pitfalls is to iterate all of the characteristics to find the one you want and then pass the characteristic instance itself as the char_specifier. But we could avoid the iteration step by allowing the new form of the argument as describe above.

If we deprecate the use of handles, then we don't have to come up with a hack to fix #883 and we can just not implement handles on Android. (And there seems to be a decent amount of support for #759 which is going in the same direction of not using handle).

@dlech dlech added enhancement New feature or request Opinions Appreciated Please add an opinion on your desired resolution on this issue! labels Jul 14, 2022
@dlech
Copy link
Collaborator Author

dlech commented Aug 2, 2022

According to a book I just read, using the 0x2901 "Characteristic User Description" is another recommended way to uniquely identify a characteristic. Also, in some cases, the service probably needs to be part of the characteristic "path".

@jackjansen
Copy link
Contributor

I think that completely hiding handles (if the backend supports them) is the wrong way to go, because really handles are the common low-level addressing mechanism for attributes and therefore for all higher level abstractions (services, characteristics, values, ...).

But steering users away from using handles if they don't have to is a good idea. For example, by not allow a bare integer handle as parameter to all types of calls, but introducing a BleakGATTAttribute type that encapsulates the handle, and allowing that in stead.

I don't think it should be a superclass of BleakGATTCharacteristic and friends, because then allowing it as parameter to, say, read_gatt_char would not catch the programmer error of passing a BleakGATTService in stead of a BleakGATTCharacteristic.

But implementation-wise most of the read_gatt_* methods in most backends could be implemented with all the common code in a read_gatt_attribute after getting the BleakGATTAttribute from their argument.

@dlech
Copy link
Collaborator Author

dlech commented Aug 14, 2022

Here is my view of the bigger picture. BLE operations are primarily on characteristics, very rarely on descriptors and never on services. So what we really should be focused on is characteristics. So right now, Bleak is really simple to use if each characteristic is unique, in other words, each characteristic has a different UUID. But when there is more than one characteristic with the same UUID, then it is not so simple. You have to call get_services() to get the list of services, then iterate the list find the one you want, then get the characteristics for that service, then iterate the list of characteristics to find the one you want. Then get the handle of that characteristic.

So what I am hoping to achieve here is a way to get the desired characteristic in one line of code in a way that works cross-platfrom and in a way that least chance of breaking if the low-level attributes are reordered (i.e. after a firmware update on the remote device). Since services, characteristics and descriptors have a tree-like relationship, it seems like we should be able to come up with some sort of "tree path" specifier. Then we could have a find_characteristic(self, path) method or just pass the path specifier directly to the existing *_gatt_char() methods.

The simplest case for the path to a characteristic would just be the service UUID and the characteristic UUID.

char_path = ("1111", "2222")

But what if that service has more than one instance of the same characteristic UUID. As suggested already, is is recommended that the Characteristic User Description Descriptor should be used in this case, so it would be nice if we could support that. However, there is no guarantee that devices will actually implement this, so a zero-based index of the nth instance of this characteristic like on Android seems like a reasonable fallback.

char_path = ("1111", ("2222", "user description"))
char_path = ("1111", ("2222", 0))

It is also possible for there to be multiple instances of the same service. There are no service descriptors, so indexed addressing as described above seems like the way to go. However, this wouldn't handle cases like "I want the instance of service 1111 with optional characteristic 2222" where there are two services with UUID 1111 in an unknown order where one service has a characteristic with UUID 2222 and the other service does not.

char_path = (("1111", 0), "2222")

Another case that Bleak doesn't currently handle (an no one has requested support for it, so maybe isn't used in practice?) is included services. This is where a service can have another service as a "child" in the tree (e.g. a battery service may have an included temperature service that would be used to get the battery temperature).

char_path = ("3333", ("1111", "2222"))

I should probably go back and check my references to make sure, but I think this covers most cases.

To really simplify things for users we could print out these paths in the service explorer example, then users would just have to copy and paste that into their script and things would magically just work.

#939, #724, #659 show that there is room for such an improvement here.

@jackjansen
Copy link
Contributor

@dlech I fully agree with the characteristics being the main abstraction, and accessing those should be as simple as possible.

But the fact that there's the lower-level GATT (or ATT, I keep getting mixed up the the BT technology terms) abstraction of Attributes and Handles means that somehow exposing these in the API (optionally) enables implementing stuff that would otherwise be impossible.

But, indeed, the preferred way of accessing a Characteristic should never be with a handle.

(And I wasn't aware that it is allowed to have multiple instances of the same service, nor that is was possible to have nested services. That's really nasty, presuming it really happens in the real world.)

@dlech
Copy link
Collaborator Author

dlech commented Aug 15, 2022

enables implementing stuff that would otherwise be impossible.

What specifically would be enabled that currently is not possible in Bleak?

@jackjansen
Copy link
Contributor

Hmm, good point. I've used accessing by handle only to debug issues in my BLE server implementation (where it allowed me to determine that I had accidentally attached the descriptors to my characteristic twice). But that's hardly a common use case.

Ok, point taken. Allowing people never to have to use integer handles is much more important.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Opinions Appreciated Please add an opinion on your desired resolution on this issue!
Projects
None yet
Development

No branches or pull requests

2 participants