trouble icon indicating copy to clipboard operation
trouble copied to clipboard

Add Attribute Data On Callback functionality

Open nm17 opened this issue 1 year ago • 8 comments

Current problem

Current GATT API requires the user to create a buffer for data that can't be changed afterwards by the outside code easily and requires the buffer to live as much as the device does ('d).

This limits the possible returned values from GATT to a fixed length, without modifying the Attribute Table, which isn't good developer experience.

My proposed solution

Add two variants for AttributeData which allow read and write functionality, containing the props and the callback, which processes either reads or writes. I'm not exactly sure about the parameters and return type for the callback, I will think about that later. Any suggestions are welcome.

The questions for now

  • Can I assign myself on something like this? I would like to attempt this myself.
  • Will this conflict with any other features which are currently in the works?
  • Is this even something that the devs of trouble would like to see implemented?

nm17 avatar Aug 30 '24 15:08 nm17

Should this variant be done by:

  • Adding a new variant in AttributeData?
    This option is easier to do, but will add another variant, which might be non ideal, since this enum is public and doesn't have #[non_exhaustive], which will break API for those who match against it (?)
  • Somehow modifying the existing Data and DataReadOnly variants?
    This one is harder, but will leave the amount of variants the same, without breaking compatibility.

Honestly, either way is fine IMHO, since I don't think many people are matching on AttributeData either way.

nm17 avatar Aug 30 '24 16:08 nm17

Thanks for raising the issue and sharing your ideas. I think storing the callback per attribute is not going to work generically without an allocator? But what could work is providing a callback to the gatt server perhaps, and then pass the handle and 'buffer' to the callback.

Regarding breaking APIs I wouldn't worry about it at this point, we don't have any release on crates.io yet.

lulf avatar Aug 30 '24 17:08 lulf

I guess anything that allocates will be off limits for this repo? Or can I add a feature flag for this and put the additional AttributeData variant and functions that use it under it?

nm17 avatar Aug 30 '24 17:08 nm17

Yes, I would like to avoid alloc. Especially if we can get almost similar functionality with just a slight inconvenience in the api.

lulf avatar Aug 30 '24 17:08 lulf

Maybe you could look at the gatt api for nrf softdevice for inspiration. It allows using heapless Vec types as attribute types, which allows for not always copying all the data.

lulf avatar Aug 30 '24 17:08 lulf

I'll throw another idea into the mix :)

What if it was possible to make trait out of the AttributeTable functionality, so that you could potentially replace that one with a type that only worked using callbacks. This way you could potentially run a gatt server that didn't require specifying an AttributeTable up front, and AttributeTable could be one implementation of such a trait. Then you could potentially provide your own alloc-based implementation that had the more convenient experience.

I'm not sure if it's feasible or not, and would be a lot of work, but sounds like it could have some nice benefits.

lulf avatar Aug 30 '24 18:08 lulf

Also, regarding your first idea, it might be possible to have that closure per attribute, if you look at how it's done in bleps https://github.com/bjoernQ/bleps/blob/main/example/src/main.rs#L88

lulf avatar Aug 30 '24 18:08 lulf

What if it was possible to make trait out of the AttributeTable functionality, so that you could potentially replace that one with a type that only worked using callbacks

That would be great, honestly. That would provide a lot of flexibility for me. I'll see what I can do

nm17 avatar Aug 30 '24 20:08 nm17

I think we have settled on the async connection gatt event approach now

lulf avatar Jan 07 '25 09:01 lulf