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

Bluetooth traits? #32

Open
gvcallen opened this issue Oct 28, 2022 · 23 comments
Open

Bluetooth traits? #32

gvcallen opened this issue Oct 28, 2022 · 23 comments

Comments

@gvcallen
Copy link

The docs at https://crates.io/crates/embedded-svc mention that this crate is for MCUs that typically have "WiFi or BLE support", however I find no traits related to Bluetooth or BLE specifically. Are there plans to add this support or are you looking for contributors regarding this?

@ivmarkov
Copy link
Collaborator

No Bluetooth traits yet. Main two reasons - the ESP-IDF BT implementation is not "officially" exposed yet in esp-idf-svc - and - I don't have a good grasp of what a generic Bluetooth traits should look like.

@ivmarkov
Copy link
Collaborator

ivmarkov commented Sep 7, 2023

Bluetooth (for now, the base driver and Classic BT on top but BLE is in the works too) is now exposed in esp-idf-svc mainline under the experimental feature flag. The exposed API is based on the ESP IDF Bluedroid one, and is a relatively literal translation of the espidf and Bluedroid callback model.

Still early for trait abstractions though.

JFYI ^^^

@jasta
Copy link

jasta commented Sep 26, 2023

This feels important for rs-matter as we'll need a generic way to talk about BLE to support BTP in a reusable way. If there's sufficient interest in helping get the reviews going, I am willing to throw my hat in here and try to get an MVP together. I'm currently troubleshooting my way through a basic hello word with rs-matter on my esp32c3...

@ivmarkov
Copy link
Collaborator

@jasta I think we should first try to pull the BTP stack in rs-matter in a "non-trait" way so to say. I.e. with a structure that has a hard dependency on - say - the esp32XX Bluedroid BLE stack that by the way needs love as it is unfinished.

Once we have the above implementation in rs-matter (obviously, behind a feature flag), we probably need another one - say - either something based on the Embassy BLE support (I think there is one?), or support for the Linux BLE stack. Only after that (I think) we would have enough examples to figure out what a useful set of BLE traits might look like.

At least I don't feel well-versed in the bluetooth stacks to start up-front with a set of traits. If you have apriori knowledge and experience with it - sure, you can try that way too. But otherwise it is probably safer to stick with the traitify-only-after you-have-a-few-implementations approach.

@jasta
Copy link

jasta commented Sep 27, 2023

I think I can skip a few steps there honestly. I do have a lot of prior BLE knowledge and I'm incorporating 3 separate implementation's APIs in order to come up with a proper higher level abstraction that should be easy to implement efficiently for everyone. Those impls are Android's BLE support, nimble, and BlueZ. I understand your point tho and I'll probably have to time box this work to make sure I'm not spinning my wheels.

@ivmarkov
Copy link
Collaborator

ivmarkov commented Sep 27, 2023

Well, if you are a BLE expert, just go ahead, propose some traits, get some feedback and there we have it! Even better. If you can do it for Classic BT, even better again. But still pls take a look at embassy, as their code might be async only, as they don't have a task scheduler, so we might have to provide async traits as well.

@jasta
Copy link

jasta commented Oct 1, 2023

I looked at their nRF BLE implementation and I think it will still work with non-async but non-blocking traits just fine. Most BLE APIs (all that I've seen actually) are event driven and callback-based. So the idea then is to make trait with event callbacks for all GATT server events and then an API for all GATT client events (requesting MTU exchange, notify/indicate writes initiated by the server, etc) that can be trivially implemented with an mpsc bounded channel. That is, the APIs can fail due to the queue being "busy" but are otherwise non blocking.

To keep this constrained and reasonably scoped I'm going to focus only on the BLE peripheral API, with the possibility of maturing/expanding scope as follows:

  1. Separate crates for each role to promote a la carte dependencies: ble-peripheral for starters (low power, usually embedded devices), then ble-central (laptop/phone/etc), and finally a bt-classic set of traits.
  2. Writing reference implementations for embedded (esp-idf) and desktop (maybe using an existing stack like btleplug). These would be separately shipped crates like ble-peripheral-espidf and ble-peripheral-btleplug.
  3. Try to get ble-peripheral moved into the embedded-rust community to better promote more adoption by various controllers.

I'm not saying I'm necessarily going to do all of the above, just that I'm laying things out with that progression in mind.

@ivmarkov
Copy link
Collaborator

ivmarkov commented Oct 1, 2023

If most APIs in the wild are event driven and callback-based, shouldn't we - in the spirit of being minimalistic with the traits and allow the user to use these however he/she sees fit - not impose any particular higher level pattern and just expose a plain, simple callback-based API?

@jasta
Copy link

jasta commented Oct 1, 2023

If most APIs in the wild are event driven and callback-based, shouldn't we - in the spirit of being minimalistic with the traits and allow the user to use these however he/she sees fit - not impose any particular higher level pattern and just expose a plain, simple callback-based API?

That is what I was trying to say above. The ble-peripheral crate will largely be one where you subscribe to GattServerEvent's and can issue a few commands out of band like request MTU, notify/indicate, and start/stop advertising. The implementation for most BLE stacks would be a simple 1:1 translation from its internal types to the trait types and a message queue of some kind to make sure the right thread calls the client commands (most stacks aren't fully thread safe and require client calls have a thread affinity). The latter I don't think can be avoided without the trait itself imposing this thread affinity which isn't technically true for every stack...

@ivmarkov
Copy link
Collaborator

ivmarkov commented Oct 1, 2023

If most APIs in the wild are event driven and callback-based, shouldn't we - in the spirit of being minimalistic with the traits and allow the user to use these however he/she sees fit - not impose any particular higher level pattern and just expose a plain, simple callback-based API?

That is what I was trying to say above. The ble-peripheral crate will largely be one where you subscribe to GattServerEvent's and can issue a few commands out of band like request MTU, notify/indicate, and start/stop advertising. The implementation for most BLE stacks would be a simple 1:1 translation from its internal types to the trait types and a message queue of some kind to make sure the right thread calls the client commands (most stacks aren't fully thread safe and require client calls have a thread affinity). The latter I don't think can be avoided without the trait itself imposing this thread affinity which isn't technically true for every stack...

OK so more or less what I have here for ESP IDF's BlueDroid Classic BT and unfinished here for BLE.
Except without any attempt at thread affinity - as I really did not find any info on behalf of what thread I'm allowed to call methods on the GATTS object (GAP / A2DP / AVRC / HFPC for classic BT).

@jasta
Copy link

jasta commented Oct 1, 2023

OK so more or less what I have here for ESP IDF's BlueDroid Classic BT and unfinished here for BLE.
Except without any attempt at thread affinity - as I really did not find any info on behalf of what thread I'm allowed to call methods on the GATTS object (GAP / A2DP / AVRC / HFPC for classic BT).

Exactly, I'm using this code as a guard rail to make sure esp-idf can also impl the traits I'm defining smoothly.

@jasta
Copy link

jasta commented Oct 7, 2023

Very early RFC is ready for eye balls, especially from you @ivmarkov as I really value your opinion on the code and future direction:

https://github.com/jasta/ble-peripheral/tree/main

There's a lot here I know, BLE is a pretty bloated / messy protocol IMO, but to the best of my knowledge this is a thorough and robust implementation that should handle all the major edge cases and should be implementable by just about any library. I focused on BlueR as the first use case for two reasons: 1) I can prototype faster on the host machine, and 2) esp-idf-svc support isn't complete enough to test all cases. Soon I'll be moving on to expanding and supporting esp-idf-svc as per project-chip/rs-matter#41 (comment). You can find the BlueR implementation kind of shoe horned into examples/bluer_echo_server but next up I'll be popping that out into its own crate and repo to highlight that this is really meant to be a trait-only project.

You can test the example trivially with a typical Linux host and the LightBlue app for Android/iOS.

@jasta
Copy link

jasta commented Oct 7, 2023

Might be relevant to rust-embedded/not-yet-awesome-embedded-rust#29 as well. I changed my mind on going with embedded-ble as a name because the traits apply equally well to embedded and not.

@ivmarkov
Copy link
Collaborator

ivmarkov commented Oct 8, 2023

Sorry for the delay here. Completely busy with the esp-idf-* crates and edge-executor ATM. Will try to spend time on it tmr.

@ivmarkov
Copy link
Collaborator

ivmarkov commented Oct 8, 2023

Maybe one early feedback I can provide still: the embedded community (especially the embassy one) is not too keen on using alloc where it can be avoided. Either by using heapless or lifetimes instead.

@jasta
Copy link

jasta commented Oct 8, 2023

Maybe one early feedback I can provide still: the embedded community (especially the embassy one) is not too keen on using alloc where it can be avoided. Either by using heapless or lifetimes instead.

I'm aware of this and the reasons behind it, but less so what are some good strategies to avoid this. Maybe I can make the whole GattService spec transitively generic over const N with a heapless Vec, but that's kind of awkward since the used memory becomes N^2 when the common case only needs N or 2N. Or I could use slice references but this is also very weird since you'll have to write a lot of boilerplate to make stack allocations that hold the actual heapless Vec's you'll use. I feel like this slice approach should be able to work nicely, I just haven't seen a good example of transitive slices used conveniently.

Specifically, this is easy and doesn't require any hard trade offs:

let x = [1, 2, 3];
do_stuff(&x);

But this now gets awkward and either uses alloc or wastes lots of space with heapless so the types are heterogenous:

let x = [vec![1], vec![2], vec![3, 3, 3]];
do_stuff(&x);

It actually feels like alloc is the right choice here because you'll waste less total space and this all happens at an early initialization period for the device most likely. Hmm.

@ivmarkov
Copy link
Collaborator

ivmarkov commented Oct 9, 2023

I'm not sure I understand your difficulties with lifetimes and slices. Let's take GattService and GattCharacteristic as an example. These are just metadata. As such, the GATT server does not need to modify them - just take them and use them for advertising. So, using covariant lifetimes (as these work for readonly data), you can simply do:

pub struct GattCharacteristic<'a> {
  pub uuid: UUID,
  pub properties: EnumSet<GattCharacteristicProperty>,
  pub permissions: EnumSet<GattCharacteristicPermission>,
  pub descriptors: &'a [GattDescriptor],
}

and then - more importantly - GattService:

pub struct GattService<'a> {
  pub uuid: UUID,
  pub service_type: GattServiceType,
  pub characteristics: &'a [GattCharacteristic<'a>],
}

If you are wondering, that's exactly how rs-matter metadata is modeled after I removed all allocations. And it works fine.

@jasta
Copy link

jasta commented Oct 9, 2023

Hmm I'll need to look again. I had the APIs that way at first but it was a terrible experience trying to actually build the structs because you had to set local bindings for each thing and define it bottoms up:

let descs = vec![GattDescriptor { ... }];
let chars = vec![GattCharacteristic { ..., descriptors: &descs }];
...

I'll look deeper into the rs-matter examples and see what I'm missing.

@ivmarkov
Copy link
Collaborator

ivmarkov commented Oct 9, 2023

Forget about vec!. It allocates. Here's how you should do it, and it works fine at least in 'static context.

@ivmarkov
Copy link
Collaborator

ivmarkov commented Oct 9, 2023

You can do it in non-static too, and yes, in that case you need to define the GATT characteristics first, and only after that the GATT service.

That would be an inconvenience only if you have to do it in a non-static context all the time, which I can't imagine to be the case. After all, and just like in rs-matter, you would be defining a concrete GATT service, for your concrete use case. Meaning, its characteristics would be hard-coded, ergo, these could be static or even const. Including GattService itself.

EDIT: A bit like this.

EDIT 2: To continue the analogy with rs-matter, I think here also we'll have two extreme case and nothing in-between:

  • Case 1 (most frequent) - the user is creating a concrete Matter device (or a GATT server, for your case). Since it is concrete, its characteristics (and the service def itself) can be encoded in a const 'static context;
  • Case 2: (much less frequent) - the user is building some sort of a "bridge". As in a bridge from something to Matter. In that case, what functionality (clusters, endpoints) the bridge supports is dynamic. But then, the fact that the bridge has to first create e.g. the cluster metadata instance and only after that e.g. the endpoint is no big deal (a bit like your case where the characteristics need to be created before the service so that they can be referenced in it). And for that case, you can still use - internally - either heapless or alloc, however the external contract - the traits and structures - would not be polluted by that.

@jasta
Copy link

jasta commented Oct 9, 2023

This is pure gold, thank you! I'm gonna play with this a bit more today but one question still nagging me is how static lifetime (and const) interact with these short lived structs that will be copied by the Peripheral impl and should otherwise get dropped after you call configure_gatt_server. If they're const I understand that won't happen and you'll just waste the memory. If they're &'static is there some kind of magic I'm not seeing to tell the compiler they're definitely valid until configure_gatt_server but not necessarily after without the local bindings? Without &'static (i.e. with a named &'a lifetime) I understand everything works fine if you use local bindings and declare the service bottom up.

@jasta
Copy link

jasta commented Oct 9, 2023

Err, I think I was doing something boneheaded when I first tried this. Rust has some magic I don't fully understand that allows this syntax:

struct Thing<'a, T> {
  value: T,
  others: &'a [Thing<'a, T>],
}

let things = [
  Thing { value: 1, others: &[
    Thing { value: 2, others: &[
      Thing { value: 3, others: &[] },
    ]}
  ]},
];

I had originally tried this with Vec instead of an array and it didn't work. With arrays, I think what's going on is that Rust is forced to conclude that stack storage must be used for the whole array itself. It then must set the lifetime of that array to '1, then when it captures Thing<'a> it creates the relationship that 1: 'a making it so that the array can only be dropped after the 'a lifetime ends. I updated the git repo with these changes so now alloc is not needed anywhere except for some Advertisement nice-to-have features. I'll flesh that all out next.

@dzervas
Copy link

dzervas commented Feb 23, 2024

@jasta first of all thanks for trying to create a BLE HAL - I was googling that and it led me here

About the lifetime/alloc/heapless problems: I was exactly there 2 days ago (what should I choose and why and where does my choice leave the user in terms of boilerplate) - try the Rust for Rustacians book (ISBN: 9781718501850). It's amazing and goes in depth for non-trivial problems.

I might be wrong but the reason the above code works is:

  • Thing is Sized as you're using it as Thing<'a, u8> in the snippet above and its size can be calculated at compile time (value is Sized as well since it's u8 and others is always usize since it's a reference)
  • since Thing with value 3 is sized, so it's 2 and 1 and an array of them
  • the Drop sequence when things goes out of scope is from the last element (so the only element) and from innermost to outermost

I don't see any problem with the above so the compiler is happy to do what you want. Sized is a blessing :) (again the only reason that the above makes sense to me is reading the first 2 chapters of the book. I still can't believe how much everything makes sense now - also I'm not affiliated with the book at all)

As for the implementation of ble-peripheral:

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

4 participants