You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mynewt.apache.org by Christopher Collins <cc...@apache.org> on 2017/01/24 19:45:33 UTC

NimBLE host advertising data API

Hello all,

I've mentioned this before - I really don't care for the advertising
data API that we ended up with:
http://mynewt.apache.org/latest/network/ble/ble_hs/ble_gap/functions/ble_gap_adv_set_fields/

I think we should change this API now before the 1.0 release.  I was
wondering what others think.

The current API is high-level and is relatively easy to use, but
requires a lot of code space and RAM.  I think a function which just
takes a raw byte buffer (or mbuf) would be much better.  Then, there
could be a helper function which converts an instance of `struct
ble_hs_adv_fields` to a raw byte buffer.

A simple peripheral that always advertises the same data shouldn't be
burdened with the ble_hs_adv_fields API.

There is actually a rationale as to why the API is the way it is today.
There are a few fields in the advertisement data that the host can be
configured to fill in automatically:
    * Flags (contains advertising type).
    * TX Power Level

I figured it would be safer if these values got calculated when
advertising is initiated.  This is impractical if the host were handed a
byte buffer rather than a series of fields.

Under the new proposal, the application would need to specify the
correct advertising type when building the byte buffer, and the tx power
level would be queried before the advertising procedure is actually
started.  I don't think this will be a problem in practice, and I think
the benefits in code size and RAM usage outweigh the API burden.

All thoughts welcome.

Thanks,
Chris

Re: NimBLE host advertising data API

Posted by Andrzej Kaczmarek <an...@codecoup.pl>.
Hi David,

On Tue, Jan 24, 2017 at 9:26 PM, David G. Simmons <sa...@mac.com> wrote:

> These fields are also still using unit16_t, etc. instead of the new
> bleu_uinit16_t for UUIDs in various places:
>
> struct ble_hs_adv_fields {
>     /*** 0x01 - Flags. */
>     uint8_t flags;
>     unsigned flags_is_present:1;
>
>     /*** 0x02,0x03 - 16-bit service class UUIDs. */
>     uint16_t *uuids16;
>     uint8_t num_uuids16;
>     unsigned uuids16_is_complete:1;
>
>     /*** 0x04,0x05 - 32-bit service class UUIDs. */
>     uint32_t *uuids32;
>     uint8_t num_uuids32;
>     unsigned uuids32_is_complete:1;
>
>     /*** 0x06,0x07 - 128-bit service class UUIDs. */
>     void *uuids128;
>     uint8_t num_uuids128;
>     unsigned uuids128_is_complete:1;
>
>
This was actually left on purpose.

The problem is that incoming adv reports are unpacked to this struct by the
host and there are internal buffers to store UUIDs which can then be
pointed here. Converting these fields to UUIDs would also require changing
these buffers and they would consume extra memory. With this adv API rework
we can also fix this properly - I guess adv data will just be passed to
application also as a raw buffer so there would be no need to unpack in the
host internally.

BR,
Andrzej

Re: NimBLE host advertising data API

Posted by "David G. Simmons" <sa...@mac.com>.
These fields are also still using unit16_t, etc. instead of the new bleu_uinit16_t for UUIDs in various places:

struct ble_hs_adv_fields {
    /*** 0x01 - Flags. */
    uint8_t flags;
    unsigned flags_is_present:1;

    /*** 0x02,0x03 - 16-bit service class UUIDs. */
    uint16_t *uuids16;
    uint8_t num_uuids16;
    unsigned uuids16_is_complete:1;

    /*** 0x04,0x05 - 32-bit service class UUIDs. */
    uint32_t *uuids32;
    uint8_t num_uuids32;
    unsigned uuids32_is_complete:1;

    /*** 0x06,0x07 - 128-bit service class UUIDs. */
    void *uuids128;
    uint8_t num_uuids128;
    unsigned uuids128_is_complete:1;


> On Jan 24, 2017, at 2:45 PM, Christopher Collins <cc...@apache.org> wrote:
> 
> The current API is high-level and is relatively easy to use, but
> requires a lot of code space and RAM.  I think a function which just
> takes a raw byte buffer (or mbuf) would be much better.  Then, there
> could be a helper function which converts an instance of `struct
> ble_hs_adv_fields` to a raw byte buffer.

--
David G. Simmons
(919) 534-5099
Web <https://davidgs.com/> • Blog <https://davidgs.com/davidgs_blog> • Linkedin <http://linkedin.com/in/davidgsimmons> • Twitter <http://twitter.com/TechEvangelist1> • GitHub <http://github.com/davidgs>
/** Message digitally signed for security and authenticity.
* If you cannot read the PGP.sig attachment, please go to
 * http://www.gnupg.com/ <http://www.gnupg.com/> Secure your email!!!
 * Public key available at keyserver.pgp.com <http://keyserver.pgp.com/>
**/
♺ This email uses 100% recycled electrons. Don't blow it by printing!

There are only 2 hard things in computer science: Cache invalidation, naming things, and off-by-one errors.



Re: NimBLE host advertising data API

Posted by Kevin Townsend <ke...@adafruit.com>.
+1 ... raw buffer plus helpers offers the best of both worlds imho.

Le mar. 24 janv. 2017 à 21:40, will sanfilippo <wi...@runtime.io> a écrit :

> I am not sure I have any intelligent comments on this, but that has never
> stopped me from commenting in the past, so…
>
>
>
> I think a byte buffer interface is fine as long as you have helper
> functions to create that buffer. Having folks have to figure out how to
> create an advertisement without any helper functions would be a bad idea
> (imho).
>
>
>
> I am not sure I completely understand your example re:Tx Power Level.
> Would this field still get added by the host or would there be a helper
> function that a developer could call to add the Tx Power Level field to the
> advertisement?
>
>
>
>
>
> > On Jan 24, 2017, at 11:45 AM, Christopher Collins <cc...@apache.org>
> wrote:
>
> >
>
> > Hello all,
>
> >
>
> > I've mentioned this before - I really don't care for the advertising
>
> > data API that we ended up with:
>
> >
> http://mynewt.apache.org/latest/network/ble/ble_hs/ble_gap/functions/ble_gap_adv_set_fields/
>
> >
>
> > I think we should change this API now before the 1.0 release.  I was
>
> > wondering what others think.
>
> >
>
> > The current API is high-level and is relatively easy to use, but
>
> > requires a lot of code space and RAM.  I think a function which just
>
> > takes a raw byte buffer (or mbuf) would be much better.  Then, there
>
> > could be a helper function which converts an instance of `struct
>
> > ble_hs_adv_fields` to a raw byte buffer.
>
> >
>
> > A simple peripheral that always advertises the same data shouldn't be
>
> > burdened with the ble_hs_adv_fields API.
>
> >
>
> > There is actually a rationale as to why the API is the way it is today.
>
> > There are a few fields in the advertisement data that the host can be
>
> > configured to fill in automatically:
>
> >    * Flags (contains advertising type).
>
> >    * TX Power Level
>
> >
>
> > I figured it would be safer if these values got calculated when
>
> > advertising is initiated.  This is impractical if the host were handed a
>
> > byte buffer rather than a series of fields.
>
> >
>
> > Under the new proposal, the application would need to specify the
>
> > correct advertising type when building the byte buffer, and the tx power
>
> > level would be queried before the advertising procedure is actually
>
> > started.  I don't think this will be a problem in practice, and I think
>
> > the benefits in code size and RAM usage outweigh the API burden.
>
> >
>
> > All thoughts welcome.
>
> >
>
> > Thanks,
>
> > Chris
>
>
>
>

Re: NimBLE host advertising data API

Posted by will sanfilippo <wi...@runtime.io>.
+1

Sounds good.

> On Jan 24, 2017, at 2:53 PM, Christopher Collins <cc...@apache.org> wrote:
> 
> On Tue, Jan 24, 2017 at 12:40:04PM -0800, will sanfilippo wrote:
>> I am not sure I have any intelligent comments on this, but that has never stopped me from commenting in the past, so…
> 
> No worries.  Thanks for the feedback!
> 
>> 
>> I think a byte buffer interface is fine as long as you have helper functions to create that buffer. Having folks have to figure out how to create an advertisement without any helper functions would be a bad idea (imho).
>> 
>> I am not sure I completely understand your example re:Tx Power Level. Would this field still get added by the host or would there be a helper function that a developer could call to add the Tx Power Level field to the advertisement?
> 
> The host wouldn't modify the advertising data at all.  If the
> application wants to advertise the tx power level, it would need to
> arrange for it to be written to the byte buffer.  If using the helper
> function, the application would write the correct value to the
> tx_pwr_lvl field in the ble_hs_adv_fields struct before converting the
> struct to a byte array.  The application would either "just know" the
> correct value, or it would query the host prior to building the
> advertising data buffer.
> 
> Chris


Re: NimBLE host advertising data API

Posted by Christopher Collins <cc...@apache.org>.
On Tue, Jan 24, 2017 at 12:40:04PM -0800, will sanfilippo wrote:
> I am not sure I have any intelligent comments on this, but that has never stopped me from commenting in the past, so\u2026

No worries.  Thanks for the feedback!

> 
> I think a byte buffer interface is fine as long as you have helper functions to create that buffer. Having folks have to figure out how to create an advertisement without any helper functions would be a bad idea (imho).
> 
> I am not sure I completely understand your example re:Tx Power Level. Would this field still get added by the host or would there be a helper function that a developer could call to add the Tx Power Level field to the advertisement?

The host wouldn't modify the advertising data at all.  If the
application wants to advertise the tx power level, it would need to
arrange for it to be written to the byte buffer.  If using the helper
function, the application would write the correct value to the
tx_pwr_lvl field in the ble_hs_adv_fields struct before converting the
struct to a byte array.  The application would either "just know" the
correct value, or it would query the host prior to building the
advertising data buffer.

Chris

Re: NimBLE host advertising data API

Posted by will sanfilippo <wi...@runtime.io>.
I am not sure I have any intelligent comments on this, but that has never stopped me from commenting in the past, so…

I think a byte buffer interface is fine as long as you have helper functions to create that buffer. Having folks have to figure out how to create an advertisement without any helper functions would be a bad idea (imho).

I am not sure I completely understand your example re:Tx Power Level. Would this field still get added by the host or would there be a helper function that a developer could call to add the Tx Power Level field to the advertisement?


> On Jan 24, 2017, at 11:45 AM, Christopher Collins <cc...@apache.org> wrote:
> 
> Hello all,
> 
> I've mentioned this before - I really don't care for the advertising
> data API that we ended up with:
> http://mynewt.apache.org/latest/network/ble/ble_hs/ble_gap/functions/ble_gap_adv_set_fields/
> 
> I think we should change this API now before the 1.0 release.  I was
> wondering what others think.
> 
> The current API is high-level and is relatively easy to use, but
> requires a lot of code space and RAM.  I think a function which just
> takes a raw byte buffer (or mbuf) would be much better.  Then, there
> could be a helper function which converts an instance of `struct
> ble_hs_adv_fields` to a raw byte buffer.
> 
> A simple peripheral that always advertises the same data shouldn't be
> burdened with the ble_hs_adv_fields API.
> 
> There is actually a rationale as to why the API is the way it is today.
> There are a few fields in the advertisement data that the host can be
> configured to fill in automatically:
>    * Flags (contains advertising type).
>    * TX Power Level
> 
> I figured it would be safer if these values got calculated when
> advertising is initiated.  This is impractical if the host were handed a
> byte buffer rather than a series of fields.
> 
> Under the new proposal, the application would need to specify the
> correct advertising type when building the byte buffer, and the tx power
> level would be queried before the advertising procedure is actually
> started.  I don't think this will be a problem in practice, and I think
> the benefits in code size and RAM usage outweigh the API burden.
> 
> All thoughts welcome.
> 
> Thanks,
> Chris


Re: NimBLE host advertising data API

Posted by Christopher Collins <cc...@apache.org>.
Thanks Andrzej.  That makes sense.

Chris

On Fri, Jan 27, 2017 at 04:57:48PM +0100, Andrzej Kaczmarek wrote:
> Hi Chris,
> 
> I created a pull request which also updates API to raw data for the
> opposite direction, i.e. from host to app. Application can still parse this
> to ble_hs_adv_fields since helper is now public, but has to do this
> explicitly. The advantage is that if application prefers to work on raw
> data (I added simple iterator to make this easier) some extra code and
> static variables are removed saving some flash.
> 
> There are also some extra fixes includes for advertising API I had pending
> in my tree.
> 
> Link: https://github.com/apache/incubator-mynewt-core/pull/169
> 
> BR,
> Andrzej
> 
> 
> 
> On Thu, Jan 26, 2017 at 4:16 AM, Christopher Collins <cc...@apache.org>
> wrote:
> 
> > Heads up: I have pushed this API change.
> >
> > The new functions are:
> >     int ble_gap_adv_set_data(const uint8_t *data, int data_len);
> >     int ble_gap_adv_rsp_set_data(const uint8_t *data, int data_len);
> >
> >     /* Converts a high-level set of fields to a byte buffer. */
> >     int ble_hs_adv_set_fields(const struct ble_hs_adv_fields *adv_fields,
> >                               uint8_t *dst, uint8_t *dst_len,
> >                               uint8_t max_len):
> >
> > The old functions are still around because they are convenient
> > (ble_gap_adv_set_data() and ble_gap_adv_rsp_set_data()); they won't get
> > included in the build if your app does not call them.
> >
> > If you use ble_hs_adv_set_fields() or the old functions, please be aware
> > of one more API change: you need to explicitly specify the flags value
> > that you want to include in advertisements.  Before this change, you
> > could specify a value of 0, and the flags field would get
> > auto-calculated by the host.
> >
> > E.g.,
> >
> > OLD API:
> >
> >     fields.flags_is_present = 1;
> >     fields.flags = 0;
> >
> > NEW API:
> >
> >     fields.flags = BLE_HS_ADV_F_DISC_GEN |
> >                    BLE_HS_ADV_F_BREDR_UNSUP;
> >
> > Thanks,
> > Chris
> >
> > On Tue, Jan 24, 2017 at 11:45:33AM -0800, Christopher Collins wrote:
> > > Hello all,
> > >
> > > I've mentioned this before - I really don't care for the advertising
> > > data API that we ended up with:
> > > http://mynewt.apache.org/latest/network/ble/ble_hs/ble_
> > gap/functions/ble_gap_adv_set_fields/
> > >
> > > I think we should change this API now before the 1.0 release.  I was
> > > wondering what others think.
> > >
> > > The current API is high-level and is relatively easy to use, but
> > > requires a lot of code space and RAM.  I think a function which just
> > > takes a raw byte buffer (or mbuf) would be much better.  Then, there
> > > could be a helper function which converts an instance of `struct
> > > ble_hs_adv_fields` to a raw byte buffer.
> > >
> > > A simple peripheral that always advertises the same data shouldn't be
> > > burdened with the ble_hs_adv_fields API.
> > >
> > > There is actually a rationale as to why the API is the way it is today.
> > > There are a few fields in the advertisement data that the host can be
> > > configured to fill in automatically:
> > >     * Flags (contains advertising type).
> > >     * TX Power Level
> > >
> > > I figured it would be safer if these values got calculated when
> > > advertising is initiated.  This is impractical if the host were handed a
> > > byte buffer rather than a series of fields.
> > >
> > > Under the new proposal, the application would need to specify the
> > > correct advertising type when building the byte buffer, and the tx power
> > > level would be queried before the advertising procedure is actually
> > > started.  I don't think this will be a problem in practice, and I think
> > > the benefits in code size and RAM usage outweigh the API burden.
> > >
> > > All thoughts welcome.
> > >
> > > Thanks,
> > > Chris
> >

Re: NimBLE host advertising data API

Posted by Andrzej Kaczmarek <an...@codecoup.pl>.
Hi Chris,

I created a pull request which also updates API to raw data for the
opposite direction, i.e. from host to app. Application can still parse this
to ble_hs_adv_fields since helper is now public, but has to do this
explicitly. The advantage is that if application prefers to work on raw
data (I added simple iterator to make this easier) some extra code and
static variables are removed saving some flash.

There are also some extra fixes includes for advertising API I had pending
in my tree.

Link: https://github.com/apache/incubator-mynewt-core/pull/169

BR,
Andrzej



On Thu, Jan 26, 2017 at 4:16 AM, Christopher Collins <cc...@apache.org>
wrote:

> Heads up: I have pushed this API change.
>
> The new functions are:
>     int ble_gap_adv_set_data(const uint8_t *data, int data_len);
>     int ble_gap_adv_rsp_set_data(const uint8_t *data, int data_len);
>
>     /* Converts a high-level set of fields to a byte buffer. */
>     int ble_hs_adv_set_fields(const struct ble_hs_adv_fields *adv_fields,
>                               uint8_t *dst, uint8_t *dst_len,
>                               uint8_t max_len):
>
> The old functions are still around because they are convenient
> (ble_gap_adv_set_data() and ble_gap_adv_rsp_set_data()); they won't get
> included in the build if your app does not call them.
>
> If you use ble_hs_adv_set_fields() or the old functions, please be aware
> of one more API change: you need to explicitly specify the flags value
> that you want to include in advertisements.  Before this change, you
> could specify a value of 0, and the flags field would get
> auto-calculated by the host.
>
> E.g.,
>
> OLD API:
>
>     fields.flags_is_present = 1;
>     fields.flags = 0;
>
> NEW API:
>
>     fields.flags = BLE_HS_ADV_F_DISC_GEN |
>                    BLE_HS_ADV_F_BREDR_UNSUP;
>
> Thanks,
> Chris
>
> On Tue, Jan 24, 2017 at 11:45:33AM -0800, Christopher Collins wrote:
> > Hello all,
> >
> > I've mentioned this before - I really don't care for the advertising
> > data API that we ended up with:
> > http://mynewt.apache.org/latest/network/ble/ble_hs/ble_
> gap/functions/ble_gap_adv_set_fields/
> >
> > I think we should change this API now before the 1.0 release.  I was
> > wondering what others think.
> >
> > The current API is high-level and is relatively easy to use, but
> > requires a lot of code space and RAM.  I think a function which just
> > takes a raw byte buffer (or mbuf) would be much better.  Then, there
> > could be a helper function which converts an instance of `struct
> > ble_hs_adv_fields` to a raw byte buffer.
> >
> > A simple peripheral that always advertises the same data shouldn't be
> > burdened with the ble_hs_adv_fields API.
> >
> > There is actually a rationale as to why the API is the way it is today.
> > There are a few fields in the advertisement data that the host can be
> > configured to fill in automatically:
> >     * Flags (contains advertising type).
> >     * TX Power Level
> >
> > I figured it would be safer if these values got calculated when
> > advertising is initiated.  This is impractical if the host were handed a
> > byte buffer rather than a series of fields.
> >
> > Under the new proposal, the application would need to specify the
> > correct advertising type when building the byte buffer, and the tx power
> > level would be queried before the advertising procedure is actually
> > started.  I don't think this will be a problem in practice, and I think
> > the benefits in code size and RAM usage outweigh the API burden.
> >
> > All thoughts welcome.
> >
> > Thanks,
> > Chris
>

Re: NimBLE host advertising data API

Posted by Christopher Collins <cc...@apache.org>.
Heads up: I have pushed this API change.

The new functions are:
    int ble_gap_adv_set_data(const uint8_t *data, int data_len);
    int ble_gap_adv_rsp_set_data(const uint8_t *data, int data_len);

    /* Converts a high-level set of fields to a byte buffer. */
    int ble_hs_adv_set_fields(const struct ble_hs_adv_fields *adv_fields,
                              uint8_t *dst, uint8_t *dst_len,
                              uint8_t max_len):

The old functions are still around because they are convenient
(ble_gap_adv_set_data() and ble_gap_adv_rsp_set_data()); they won't get
included in the build if your app does not call them.

If you use ble_hs_adv_set_fields() or the old functions, please be aware
of one more API change: you need to explicitly specify the flags value
that you want to include in advertisements.  Before this change, you
could specify a value of 0, and the flags field would get
auto-calculated by the host.

E.g.,

OLD API:

    fields.flags_is_present = 1;
    fields.flags = 0;

NEW API:

    fields.flags = BLE_HS_ADV_F_DISC_GEN |
                   BLE_HS_ADV_F_BREDR_UNSUP;

Thanks,
Chris

On Tue, Jan 24, 2017 at 11:45:33AM -0800, Christopher Collins wrote:
> Hello all,
> 
> I've mentioned this before - I really don't care for the advertising
> data API that we ended up with:
> http://mynewt.apache.org/latest/network/ble/ble_hs/ble_gap/functions/ble_gap_adv_set_fields/
> 
> I think we should change this API now before the 1.0 release.  I was
> wondering what others think.
> 
> The current API is high-level and is relatively easy to use, but
> requires a lot of code space and RAM.  I think a function which just
> takes a raw byte buffer (or mbuf) would be much better.  Then, there
> could be a helper function which converts an instance of `struct
> ble_hs_adv_fields` to a raw byte buffer.
> 
> A simple peripheral that always advertises the same data shouldn't be
> burdened with the ble_hs_adv_fields API.
> 
> There is actually a rationale as to why the API is the way it is today.
> There are a few fields in the advertisement data that the host can be
> configured to fill in automatically:
>     * Flags (contains advertising type).
>     * TX Power Level
> 
> I figured it would be safer if these values got calculated when
> advertising is initiated.  This is impractical if the host were handed a
> byte buffer rather than a series of fields.
> 
> Under the new proposal, the application would need to specify the
> correct advertising type when building the byte buffer, and the tx power
> level would be queried before the advertising procedure is actually
> started.  I don't think this will be a problem in practice, and I think
> the benefits in code size and RAM usage outweigh the API burden.
> 
> All thoughts welcome.
> 
> Thanks,
> Chris

Re: NimBLE host advertising data API

Posted by Andrzej Kaczmarek <an...@codecoup.pl>.
Hi Chris,

On Tue, Jan 24, 2017 at 8:45 PM, Christopher Collins <cc...@apache.org>
wrote:

> Hello all,
>
> I've mentioned this before - I really don't care for the advertising
> data API that we ended up with:
> http://mynewt.apache.org/latest/network/ble/ble_hs/ble_
> gap/functions/ble_gap_adv_set_fields/
>
> I think we should change this API now before the 1.0 release.  I was
> wondering what others think.
>
> The current API is high-level and is relatively easy to use, but
> requires a lot of code space and RAM.  I think a function which just
> takes a raw byte buffer (or mbuf) would be much better.  Then, there
> could be a helper function which converts an instance of `struct
> ble_hs_adv_fields` to a raw byte buffer.
>
> A simple peripheral that always advertises the same data shouldn't be
> burdened with the ble_hs_adv_fields API.
>

Raw buffer works for me.

Anyway, I also like approach used in Zephyr: the data passed via API is an
array of TLV structures which is then flattened by the host. Using few
macros you can create nice looking definition of adv data. The advantage
compared to ble_hs_adv_fields for me is smaller memory overhead since you
only define elements you need, while the adv struct always occupies quite a
lot of memory no matter how many tags are defined.


> There is actually a rationale as to why the API is the way it is today.
> There are a few fields in the advertisement data that the host can be
> configured to fill in automatically:
>     * Flags (contains advertising type).
>     * TX Power Level
>
> I figured it would be safer if these values got calculated when
> advertising is initiated.  This is impractical if the host were handed a
> byte buffer rather than a series of fields.
>
> Under the new proposal, the application would need to specify the
> correct advertising type when building the byte buffer, and the tx power
> level would be queried before the advertising procedure is actually
> started.  I don't think this will be a problem in practice, and I think
> the benefits in code size and RAM usage outweigh the API burden.
>

+1, let app have full control over adv data

BR,
Andrzej