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

Add Attribute Data On Callback functionality #98

Open
nm17 opened this issue Aug 30, 2024 · 8 comments
Open

Add Attribute Data On Callback functionality #98

nm17 opened this issue Aug 30, 2024 · 8 comments

Comments

@nm17
Copy link

nm17 commented Aug 30, 2024

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
Copy link
Author

nm17 commented Aug 30, 2024

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 nm17 changed the title Add an Attribute Data On Callback functionality Add Attribute Data On Callback functionality Aug 30, 2024
@lulf
Copy link
Member

lulf commented Aug 30, 2024

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.

@nm17
Copy link
Author

nm17 commented Aug 30, 2024

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?

@lulf
Copy link
Member

lulf commented Aug 30, 2024

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

@lulf
Copy link
Member

lulf commented Aug 30, 2024

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
Copy link
Member

lulf commented Aug 30, 2024

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
Copy link
Member

lulf commented Aug 30, 2024

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

@nm17
Copy link
Author

nm17 commented Aug 30, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants