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

API to build advertising data #25

Open
kuon opened this issue Sep 15, 2020 · 8 comments
Open

API to build advertising data #25

kuon opened this issue Sep 15, 2020 · 8 comments
Labels
enhancement New feature or request

Comments

@kuon
Copy link

kuon commented Sep 15, 2020

Right now, to build data packets, we have to do something like:

    #[rustfmt::skip]
    let adv_data = &[
        0x02, 0x01, raw::BLE_GAP_ADV_FLAGS_LE_ONLY_GENERAL_DISC_MODE as u8,
        0x03, 0x03, 0x09, 0x18,
        0x0a, 0x09, b'H', b'e', b'l', b'l', b'o', b'R', b'T', b'I', b'C',
    ];
    #[rustfmt::skip]
    let scan_data = &[
        0x03, 0x03, 0x09, 0x18,
    ];

We need an API to build those array without having to fiddle with bytes.

I'd like to help, but I could not find the reference documentation for those data packet.

I guessed that adv_data is an array of length(1byte), type(1byte), data(length - 1 bytes). But I am not very familiar with BLE and found dozen of different documentation with no real reference for this.

@kuon
Copy link
Author

kuon commented Sep 15, 2020

I'm trying to gather as much reference as possible:

@kuon
Copy link
Author

kuon commented Sep 22, 2020

I'd like to tackle this issue, but I need to know a few things:

  • Is a builder pattern OK, or do you prefer a plain rust struct that will be transformed into data?
  • what AD types and GATT services should we support with the "simple API"
  • GATT services are also used when creating gatt server, so I guess it would be better to have their definition at only one place

@Dirbaio Dirbaio changed the title API to build datas API to build advertising data Sep 22, 2020
@Dirbaio
Copy link
Member

Dirbaio commented Sep 22, 2020

A good starting point would be what the Nordic SDK supports:

https://github.com/akiles/nrf5_sdk/blob/master/components/ble/common/ble_advdata.h#L146
https://github.com/akiles/nrf5_sdk/blob/master/components/ble/common/ble_advdata.c#L509

Also, Rubble already has support for this in Rust

https://jonas-schievink.github.io/rubble/rubble/link/ad_structure/enum.AdStructure.html

For builder pattern vs struct: I'd prefer a plain struct that you then encode to bytes (like rubble and the Nordic sdk). There are not that many fields to justify a builder IMO. Also using a builder opens interesting questions, for example: what if you set the UUID list 2 times? Does it use the last one? Do they get concatenated? If it's a regular Rust struct you don't have these issues.

About GATT services: they're a separate concern from advertising data. In advertising data you supply a list of UUIDs that may or may not match the list of GATT services you support after the connection is established.

@kuon
Copy link
Author

kuon commented Sep 23, 2020

Yeah I guess a builder isn't justified. Especially if we don't use builders elsewhere (like for config). We have to be consistent.

I think this library is low level enough to use struct/enum directly.

I will use nordic SDK as starting point and use a rubble like implementation.

@kuon
Copy link
Author

kuon commented Sep 27, 2020

I started working on it, and I came up with that:

pub enum DeviceName<'a> {
    Short(&'a str),
    Long(&'a str),
}

// maybe implement something like rubble name()
pub struct CompanyId(u16);

pub struct ManufacturerData<'a> {
    company_identifier: CompanyId,
    data: &'a [u8],
}

pub struct RawData<'a> (u8, &'a [u8]);

pub struct ServiceData<'a> (u16, &'a [u8]);

pub struct Flags(u8);

pub enum Uuid {
    U16(u16),
    U32(u32),
    U128([u8;4]),
}

pub struct AdvertData<'a> {
    flags: Flags,
    name: Option<&'a DeviceName<'a>>,
    service: Option<&'a [ServiceData<'a>]>,
    manufacturer: Option<&'a ManufacturerData<'a>>,
    uuids_more_available: Option<&'a [Uuid]>,
    uuids_complete: Option<&'a [Uuid]>,
    uuids_solicited: Option<&'a [Uuid]>,
    raw: Option<&'a [RawData<'a>]>
}

pub struct ScanData<'a> {
    name: Option<&'a DeviceName<'a>>,
    service: Option<&'a [ServiceData<'a>]>,
    manufacturer: Option<&'a ManufacturerData<'a>>,
    uuids_more_available: Option<&'a [Uuid]>,
    uuids_complete: Option<&'a [Uuid]>,
    uuids_solicited: Option<&'a [Uuid]>,
    raw: Option<&'a [RawData<'a>]>
}

Rubble uses a list of enums, but the problem with the list of enum is that we can specify the same thing multiple times, so the API is not fully safe. Also, scan response data cannot include flags, that's why I duplicated the structure. But we could also imagine passing a is_scan_data bool on the to_bytes method and specify that to get either advert or scan data from the struct.

I am also wondering if we should include something like rubble bitflags for flags.

The above structure is the closest I could come to represent the underlying data the best, they are a bit verbose, but with Default it should be ok. They are also unfinished, I need to include appearance and a few other things.

Also, rubble has a list of appearances in the gatt, which can be included in the advert data, do you think we should include a list like that?

Finally, in the nordic SDK, to include appearance (or name), you only pass a bool and the SDK gets the appearance (or name) from gap data, but I think it's better to have a complete data struct and not rely on existing gap data.

@kuon
Copy link
Author

kuon commented Nov 11, 2020

I'd like to tackle this issue, @Dirbaio what do you think?

@richard-uk1
Copy link

@Dirbaio what are you thoughts on overhead of helpers to construct packets? I'm not sure that they would end up as zero-cost abstractions. If not, they could be compile-time macros. Or when you're able to create &'static [u8] in const fns that would be the best solution.

@Dirbaio
Copy link
Member

Dirbaio commented Aug 22, 2021

Doing it at runtime is probably fine, that's what the C nrf5-sdk does. Also it makes it possible to make stuff like device names dynamic which is nice.

Getting it to work wth today's const fn limitations will be quite hard though, I'd just focus on runtime building.

It can be "translate to/from struct" like the one above, or a "builder" where you add data and it writes it to a buffer as you go. I have the suspicion the second will be better for code size.

@alexmoon alexmoon added the enhancement New feature or request label Sep 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants