You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mynewt.apache.org by Andrzej Kaczmarek <an...@codecoup.pl> on 2017/01/13 13:58:19 UTC

[RFC] Refactor UUID handling in Nimble

Hi,

I've refactored Nimble code to use dedicated type for UUID handling instead
of generic byte-array. Full diff is available here (it is split into
several patches for now to separate changes in stack and apps):

https://github.com/andrzej-kaczmarek/incubator-mynewt-core/c
ompare/develop...andrzej-kaczmarek:nimble/uuid

Start by looking at ble_uuid.h which defines new structures and helpers for
UUIDs (or see below where I included essential part):

https://github.com/andrzej-kaczmarek/incubator-mynewt-core/b
lob/1f571c7aeb22eb4e2ae96ca6d01d612afb338fd4/net/nimble/host
/include/host/ble_uuid.h

Now, let me explain what it is all about:

Current implementation handles all UUIDs as long 128-bit values so they are
passed like this (byte-array) in API and stored internally. This creates
overhead for handling short 16-bit UUIDs in terms of both memory and
processing time because:
- short values are stored as long ones anyway,
- they have to be converted back and forth since short UUID cannot be put
as long one in ATT PDU,
- there is no generic code to handle the above so similar patterns are
repeated over and over again.

With new approach there is dedicated type to pass UUID which contains also
its type so this information is available immediately - this is used e.g.
BlueZ and Zephyr.

enum {
    BLE_UUID_TYPE_16 = 16,
    BLE_UUID_TYPE_32 = 32,
    BLE_UUID_TYPE_128 = 128,
};

/* Generic UUID type, to be used only as a pointer */
typedef struct {
    uint8_t type;
} ble_uuid_t;

typedef struct {
    ble_uuid_t u;
    uint16_t value;
} ble_uuid16_t;

typedef struct {
    ble_uuid_t u;
    uint32_t value;
} ble_uuid32_t;

typedef struct {
    ble_uuid_t u;
    uint8_t value[16];
} ble_uuid128_t;

/* Universal UUID type, to be used for any-UUID static allocation */
typedef union {
    ble_uuid_t u;
    ble_uuid16_t u16;
    ble_uuid32_t u32;
    ble_uuid128_t u128;
} ble_uuid_any_t;

This particular approach to handle UUIDs comes from Zephyr which uses neat
trick to have common type for any UUID yet it allows to store 16-bit values
using less memory than for 128-bit values. There are dedicated types for
16- and 128-bit UUIDs so memory for an UUID can be allocated, but the API
uses "stub" type which defines only UUID type and is used as a pointer to
an actual UUID variable. You can use helpers to get UUID value or compare
them quickly without need to convert between 16- and 128-bit values. See
following examples how this works (or see sample apps):

// 16-bit UUID value with initialization
ble_uuid16_t uuid16 = BLE_UUID16_INIT(0x2801);
// 128-bit UUID value with initialization
ble_uuid128_t uuid128 = BLE_UUID128_INIT(0x00, 0x11, ... 0xFF);

// "stub" type is used to pass any type of UUID via API
ble_uuid_t *uuid1 = &uuid16.u;
ble_uuid_t *uuid2 = &uuid128.u;

// comparing UUIDs is really easy now
if (ble_uuid_cmp(uuid1, uuid2) == 0) { /* equal */ )

// since in most cases we use 16-bit UUIDs, it's convenient to easily
retrieve 16-bit value (or 0 if UUID is not 16-bit)
uint16_t u16 = ble_uuid_u16(uuid1);

// the above is equivalent of
u16 = uuid1->type == BLE_UUID_TYPE_16 ? BLE_UUID16(uuid1)->value : 0;

// we can easily define UUID inline as well, e.g. when defining service
static const struct ble_gatt_svc_def ble_svc_gap_defs[] = {
    {
        .type = BLE_GATT_SVC_TYPE_PRIMARY,
        .uuid = BLE_UUID16_DECLARE(BLE_SVC_GAP_UUID16),
(...)

// finally, if we need to store any UUID there is an union defined to help
ble_uuid_any_t uuid;
uuid.u16 = uuid16;

*IMPORTANT NOTE*:
There are two major differences between old and new API:
- Shen defining and registering services, in old API UUID value is copied
to attribute structure so it does not matter where UUID passed as parameter
is defined. In new API only pointer to UUID is stored thus you cannot e.g.
define value on stack since it will be destroyed.
- You need to make sure that 16-bit UUIDs are defined using helpers
dedicated for 16-bit UUIDs only, i.e. you should not create 128-bit UUID
with Bluetooth base since it will not be detected as 16-bit UUID.

Finally, there are of course some savings in both flash and RAM:

before:
<     136     286 *fill*
<   30738    5027 apps_bletiny.a
<   53633    3411 net_nimble_host.a
<     880     290 net_nimble_host_services_ans.a
<     496     289 net_nimble_host_services_gap.a
<     306      86 net_nimble_host_services_gatt.a
<  160884       2784      15828     179496      2bd28
/home/andk/devel/mynewt/bletiny/bin/targets/bletiny/
app/apps/bletiny/bletiny.elf

after:
>     143     364 *fill*
>   30470    5053 apps_bletiny.a
>   53409    3415 net_nimble_host.a
>     880     218 net_nimble_host_services_ans.a
>     496     217 net_nimble_host_services_gap.a
>     306      62 net_nimble_host_services_gatt.a
>  160396       2724      15828     178948      2bb04
/home/andk/devel/mynewt/bletiny/bin/targets/bletiny/
app/apps/bletiny/bletiny.elf

Comments are welcome! :-)

BR,
Andrzej

Re: [RFC] Refactor UUID handling in Nimble

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

I squashed code into a single commit (so bisect is not broken) and created
a pull request: https://github.com/apache/incubator-mynewt-core/pull/160
If there are comments about the code I think we can continue there.

BR,
Andrzej



On Fri, Jan 13, 2017 at 11:13 PM, Christopher Collins <cc...@apache.org>
wrote:

> Hi Andrzej,
>
> That all sounds good to me.
>
> Thanks,
> Chris
>
> On Fri, Jan 13, 2017 at 02:58:19PM +0100, Andrzej Kaczmarek wrote:
> > Hi,
> >
> > I've refactored Nimble code to use dedicated type for UUID handling
> instead
> > of generic byte-array. Full diff is available here (it is split into
> > several patches for now to separate changes in stack and apps):
> >
> > https://github.com/andrzej-kaczmarek/incubator-mynewt-core/c
> > ompare/develop...andrzej-kaczmarek:nimble/uuid
> >
> > Start by looking at ble_uuid.h which defines new structures and helpers
> for
> > UUIDs (or see below where I included essential part):
> >
> > https://github.com/andrzej-kaczmarek/incubator-mynewt-core/b
> > lob/1f571c7aeb22eb4e2ae96ca6d01d612afb338fd4/net/nimble/host
> > /include/host/ble_uuid.h
> >
> > Now, let me explain what it is all about:
> >
> > Current implementation handles all UUIDs as long 128-bit values so they
> are
> > passed like this (byte-array) in API and stored internally. This creates
> > overhead for handling short 16-bit UUIDs in terms of both memory and
> > processing time because:
> > - short values are stored as long ones anyway,
> > - they have to be converted back and forth since short UUID cannot be put
> > as long one in ATT PDU,
> > - there is no generic code to handle the above so similar patterns are
> > repeated over and over again.
> >
> > With new approach there is dedicated type to pass UUID which contains
> also
> > its type so this information is available immediately - this is used e.g.
> > BlueZ and Zephyr.
> >
> > enum {
> >     BLE_UUID_TYPE_16 = 16,
> >     BLE_UUID_TYPE_32 = 32,
> >     BLE_UUID_TYPE_128 = 128,
> > };
> >
> > /* Generic UUID type, to be used only as a pointer */
> > typedef struct {
> >     uint8_t type;
> > } ble_uuid_t;
> >
> > typedef struct {
> >     ble_uuid_t u;
> >     uint16_t value;
> > } ble_uuid16_t;
> >
> > typedef struct {
> >     ble_uuid_t u;
> >     uint32_t value;
> > } ble_uuid32_t;
> >
> > typedef struct {
> >     ble_uuid_t u;
> >     uint8_t value[16];
> > } ble_uuid128_t;
> >
> > /* Universal UUID type, to be used for any-UUID static allocation */
> > typedef union {
> >     ble_uuid_t u;
> >     ble_uuid16_t u16;
> >     ble_uuid32_t u32;
> >     ble_uuid128_t u128;
> > } ble_uuid_any_t;
> >
> > This particular approach to handle UUIDs comes from Zephyr which uses
> neat
> > trick to have common type for any UUID yet it allows to store 16-bit
> values
> > using less memory than for 128-bit values. There are dedicated types for
> > 16- and 128-bit UUIDs so memory for an UUID can be allocated, but the API
> > uses "stub" type which defines only UUID type and is used as a pointer to
> > an actual UUID variable. You can use helpers to get UUID value or compare
> > them quickly without need to convert between 16- and 128-bit values. See
> > following examples how this works (or see sample apps):
> >
> > // 16-bit UUID value with initialization
> > ble_uuid16_t uuid16 = BLE_UUID16_INIT(0x2801);
> > // 128-bit UUID value with initialization
> > ble_uuid128_t uuid128 = BLE_UUID128_INIT(0x00, 0x11, ... 0xFF);
> >
> > // "stub" type is used to pass any type of UUID via API
> > ble_uuid_t *uuid1 = &uuid16.u;
> > ble_uuid_t *uuid2 = &uuid128.u;
> >
> > // comparing UUIDs is really easy now
> > if (ble_uuid_cmp(uuid1, uuid2) == 0) { /* equal */ )
> >
> > // since in most cases we use 16-bit UUIDs, it's convenient to easily
> > retrieve 16-bit value (or 0 if UUID is not 16-bit)
> > uint16_t u16 = ble_uuid_u16(uuid1);
> >
> > // the above is equivalent of
> > u16 = uuid1->type == BLE_UUID_TYPE_16 ? BLE_UUID16(uuid1)->value : 0;
> >
> > // we can easily define UUID inline as well, e.g. when defining service
> > static const struct ble_gatt_svc_def ble_svc_gap_defs[] = {
> >     {
> >         .type = BLE_GATT_SVC_TYPE_PRIMARY,
> >         .uuid = BLE_UUID16_DECLARE(BLE_SVC_GAP_UUID16),
> > (...)
> >
> > // finally, if we need to store any UUID there is an union defined to
> help
> > ble_uuid_any_t uuid;
> > uuid.u16 = uuid16;
> >
> > *IMPORTANT NOTE*:
> > There are two major differences between old and new API:
> > - Shen defining and registering services, in old API UUID value is copied
> > to attribute structure so it does not matter where UUID passed as
> parameter
> > is defined. In new API only pointer to UUID is stored thus you cannot
> e.g.
> > define value on stack since it will be destroyed.
> > - You need to make sure that 16-bit UUIDs are defined using helpers
> > dedicated for 16-bit UUIDs only, i.e. you should not create 128-bit UUID
> > with Bluetooth base since it will not be detected as 16-bit UUID.
> >
> > Finally, there are of course some savings in both flash and RAM:
> >
> > before:
> > <     136     286 *fill*
> > <   30738    5027 apps_bletiny.a
> > <   53633    3411 net_nimble_host.a
> > <     880     290 net_nimble_host_services_ans.a
> > <     496     289 net_nimble_host_services_gap.a
> > <     306      86 net_nimble_host_services_gatt.a
> > <  160884       2784      15828     179496      2bd28
> > /home/andk/devel/mynewt/bletiny/bin/targets/bletiny/
> > app/apps/bletiny/bletiny.elf
> >
> > after:
> > >     143     364 *fill*
> > >   30470    5053 apps_bletiny.a
> > >   53409    3415 net_nimble_host.a
> > >     880     218 net_nimble_host_services_ans.a
> > >     496     217 net_nimble_host_services_gap.a
> > >     306      62 net_nimble_host_services_gatt.a
> > >  160396       2724      15828     178948      2bb04
> > /home/andk/devel/mynewt/bletiny/bin/targets/bletiny/
> > app/apps/bletiny/bletiny.elf
> >
> > Comments are welcome! :-)
> >
> > BR,
> > Andrzej
>

Re: [RFC] Refactor UUID handling in Nimble

Posted by Christopher Collins <cc...@apache.org>.
Hi Andrzej,

That all sounds good to me.

Thanks,
Chris

On Fri, Jan 13, 2017 at 02:58:19PM +0100, Andrzej Kaczmarek wrote:
> Hi,
> 
> I've refactored Nimble code to use dedicated type for UUID handling instead
> of generic byte-array. Full diff is available here (it is split into
> several patches for now to separate changes in stack and apps):
> 
> https://github.com/andrzej-kaczmarek/incubator-mynewt-core/c
> ompare/develop...andrzej-kaczmarek:nimble/uuid
> 
> Start by looking at ble_uuid.h which defines new structures and helpers for
> UUIDs (or see below where I included essential part):
> 
> https://github.com/andrzej-kaczmarek/incubator-mynewt-core/b
> lob/1f571c7aeb22eb4e2ae96ca6d01d612afb338fd4/net/nimble/host
> /include/host/ble_uuid.h
> 
> Now, let me explain what it is all about:
> 
> Current implementation handles all UUIDs as long 128-bit values so they are
> passed like this (byte-array) in API and stored internally. This creates
> overhead for handling short 16-bit UUIDs in terms of both memory and
> processing time because:
> - short values are stored as long ones anyway,
> - they have to be converted back and forth since short UUID cannot be put
> as long one in ATT PDU,
> - there is no generic code to handle the above so similar patterns are
> repeated over and over again.
> 
> With new approach there is dedicated type to pass UUID which contains also
> its type so this information is available immediately - this is used e.g.
> BlueZ and Zephyr.
> 
> enum {
>     BLE_UUID_TYPE_16 = 16,
>     BLE_UUID_TYPE_32 = 32,
>     BLE_UUID_TYPE_128 = 128,
> };
> 
> /* Generic UUID type, to be used only as a pointer */
> typedef struct {
>     uint8_t type;
> } ble_uuid_t;
> 
> typedef struct {
>     ble_uuid_t u;
>     uint16_t value;
> } ble_uuid16_t;
> 
> typedef struct {
>     ble_uuid_t u;
>     uint32_t value;
> } ble_uuid32_t;
> 
> typedef struct {
>     ble_uuid_t u;
>     uint8_t value[16];
> } ble_uuid128_t;
> 
> /* Universal UUID type, to be used for any-UUID static allocation */
> typedef union {
>     ble_uuid_t u;
>     ble_uuid16_t u16;
>     ble_uuid32_t u32;
>     ble_uuid128_t u128;
> } ble_uuid_any_t;
> 
> This particular approach to handle UUIDs comes from Zephyr which uses neat
> trick to have common type for any UUID yet it allows to store 16-bit values
> using less memory than for 128-bit values. There are dedicated types for
> 16- and 128-bit UUIDs so memory for an UUID can be allocated, but the API
> uses "stub" type which defines only UUID type and is used as a pointer to
> an actual UUID variable. You can use helpers to get UUID value or compare
> them quickly without need to convert between 16- and 128-bit values. See
> following examples how this works (or see sample apps):
> 
> // 16-bit UUID value with initialization
> ble_uuid16_t uuid16 = BLE_UUID16_INIT(0x2801);
> // 128-bit UUID value with initialization
> ble_uuid128_t uuid128 = BLE_UUID128_INIT(0x00, 0x11, ... 0xFF);
> 
> // "stub" type is used to pass any type of UUID via API
> ble_uuid_t *uuid1 = &uuid16.u;
> ble_uuid_t *uuid2 = &uuid128.u;
> 
> // comparing UUIDs is really easy now
> if (ble_uuid_cmp(uuid1, uuid2) == 0) { /* equal */ )
> 
> // since in most cases we use 16-bit UUIDs, it's convenient to easily
> retrieve 16-bit value (or 0 if UUID is not 16-bit)
> uint16_t u16 = ble_uuid_u16(uuid1);
> 
> // the above is equivalent of
> u16 = uuid1->type == BLE_UUID_TYPE_16 ? BLE_UUID16(uuid1)->value : 0;
> 
> // we can easily define UUID inline as well, e.g. when defining service
> static const struct ble_gatt_svc_def ble_svc_gap_defs[] = {
>     {
>         .type = BLE_GATT_SVC_TYPE_PRIMARY,
>         .uuid = BLE_UUID16_DECLARE(BLE_SVC_GAP_UUID16),
> (...)
> 
> // finally, if we need to store any UUID there is an union defined to help
> ble_uuid_any_t uuid;
> uuid.u16 = uuid16;
> 
> *IMPORTANT NOTE*:
> There are two major differences between old and new API:
> - Shen defining and registering services, in old API UUID value is copied
> to attribute structure so it does not matter where UUID passed as parameter
> is defined. In new API only pointer to UUID is stored thus you cannot e.g.
> define value on stack since it will be destroyed.
> - You need to make sure that 16-bit UUIDs are defined using helpers
> dedicated for 16-bit UUIDs only, i.e. you should not create 128-bit UUID
> with Bluetooth base since it will not be detected as 16-bit UUID.
> 
> Finally, there are of course some savings in both flash and RAM:
> 
> before:
> <     136     286 *fill*
> <   30738    5027 apps_bletiny.a
> <   53633    3411 net_nimble_host.a
> <     880     290 net_nimble_host_services_ans.a
> <     496     289 net_nimble_host_services_gap.a
> <     306      86 net_nimble_host_services_gatt.a
> <  160884       2784      15828     179496      2bd28
> /home/andk/devel/mynewt/bletiny/bin/targets/bletiny/
> app/apps/bletiny/bletiny.elf
> 
> after:
> >     143     364 *fill*
> >   30470    5053 apps_bletiny.a
> >   53409    3415 net_nimble_host.a
> >     880     218 net_nimble_host_services_ans.a
> >     496     217 net_nimble_host_services_gap.a
> >     306      62 net_nimble_host_services_gatt.a
> >  160396       2724      15828     178948      2bb04
> /home/andk/devel/mynewt/bletiny/bin/targets/bletiny/
> app/apps/bletiny/bletiny.elf
> 
> Comments are welcome! :-)
> 
> BR,
> Andrzej