embassy-rs / trouble

A Rust Host BLE stack with a future goal of qualification.
Apache License 2.0
117 stars 22 forks source link

Expand get and set methods to allow getting and setting all attributes #155

Open petekubiak opened 1 week ago

petekubiak commented 1 week ago

This issue stems from this comment in #151. Currently the get and set methods on the gatt_server macro-generated type assume that they would only be called on characteristics and not other types of attribute. This is because they are based on the methods of the same names on the AttributeTable. In order to allow getting and setting of other types of attribute types, these methods need to be changed to accept some sort of AttributeHandle type.

Here is a suggestion of what this might look like.

lulf commented 1 week ago

I've got nothing against the ability to read/write to handles that are not characteristics, but out of curiosity: what is the use case for using get and set on non-characteristic handles? (I wasn't aware that you could even have attribute handles that were not a characteristic)

petekubiak commented 1 week ago

To be honest I wasn't aware of this either, but in the linked comment @alexmoon said "you also need to be able to get and set descriptor values and the like"

alexmoon commented 1 week ago

Attributes include:

I believe from the client's perspective all of those are read-only except for Characteristic Value and Descriptor Value. Descriptor Value includes things like the Client Characteristic Configuration Descriptor (for enabling notifications/indications) but can also include arbitrary additional descriptors depending on the service (for example HID defines several descriptor values). So at the very least, the attribute table needs to include support for Descriptor Values.

The other GATT attributes may need to be changed at runtime by the server in certain circumstances. For example, a device may need to enable or disable a service depending on some configuration. And applications need to be able to send Service Changed indications to clients to indicate that things have changed. We probably don't need to go as far as putting everything in the attribute table, but we should keep in mind that the data is not necessarily static.

lulf commented 1 week ago

Ok, so out of those, I think only the service attributes are the ones not modifiable right now, so I agree it's good to fix this generically.