2012-09-11 07:38:11

by Ganir, Chen

[permalink] [raw]
Subject: [PATCH 00/10] Implement Generic battery and LE Battery client

From: Chen Ganir <[email protected]>

This patch set replaces previous patch sets which implemented the LE battery
GATT Client. This patch set implements a generic device battery D-Bus interface
that can be used to get remote device battery status using D-Bus. In addition,
This patch set also implements the GATT Battery client, which uses the generic
device battery to expose LE device battery status.

see doc/battery-api.txt and doc/device-api.txt for more information.


Chen Ganir (10):
battery: Add generic device battery documentation
battery: Implement Generic device battery
battery: Add GATT Battery Client Service skeleton
battery: Add client connection logic
battery: Discover Characteristic Descriptors
battery: Get Battery ID
battery: Add Battery to device
battery: Read Battery level characteristic
battery: Add support for notifications
battery: Support persistent battery level

Makefile.am | 9 +-
doc/battery-api.txt | 33 +++
doc/device-api.txt | 5 +
lib/uuid.h | 3 +
profiles/battery/battery.c | 590 ++++++++++++++++++++++++++++++++++++++++++++
profiles/battery/battery.h | 24 ++
profiles/battery/main.c | 52 ++++
profiles/battery/manager.c | 61 +++++
profiles/battery/manager.h | 24 ++
src/device.c | 184 ++++++++++++++
src/device.h | 15 ++
test/test-device | 13 +
12 files changed, 1011 insertions(+), 2 deletions(-)
create mode 100644 doc/battery-api.txt
create mode 100644 profiles/battery/battery.c
create mode 100644 profiles/battery/battery.h
create mode 100644 profiles/battery/main.c
create mode 100644 profiles/battery/manager.c
create mode 100644 profiles/battery/manager.h

--
1.7.9.5



2012-09-13 11:32:04

by Ganir, Chen

[permalink] [raw]
Subject: Re: [PATCH 02/10] battery: Implement Generic device battery

Anderson,

On 09/12/2012 01:57 PM, Anderson Lizardo wrote:
> Hi Chen,
>
> On Wed, Sep 12, 2012 at 6:30 AM, Chen Ganir <[email protected]> wrote:
>>> Both BtIO and GAttrib intend to mimic GLib-like libraries where this is
>>> the convention. The plan is not to let this spread to the entire code
>>> base (particularly with the likely move to libell in the long run), so
>>> please don't use CamelCase.
>>>
>> Ok. So for the battery options i'll simply change it to battery_option. What
>> about the convention for defining a function pointer ?
>
> See the result of:
>
> grep -r 'typedef.*(' *
>
> on your bluez tree. As Johan mentioned, you should ignore the (few)
> occurrences of camel case typedef declarations.
>
> Regards,
>
Thanks.

--
BR,
Chen Ganir


2012-09-13 11:27:09

by Ganir, Chen

[permalink] [raw]
Subject: Re: [PATCH 09/10] battery: Add support for notifications

Joao,

On 09/12/2012 07:58 AM, Chen Ganir wrote:
> Joao,
> On 09/12/2012 01:08 AM, Joao Paulo Rechi Vita wrote:
>> On Tue, Sep 11, 2012 at 4:38 AM, <[email protected]> wrote:
>>> From: Chen Ganir <[email protected]>
>>>
>>> Add support for emitting PropertyChanged when a battery level
>>> characteristic notification is sent from the peer device.
>>> ---
>>> profiles/battery/battery.c | 95
>>> +++++++++++++++++++++++++++++++++++++++++++-
>>> 1 file changed, 93 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/profiles/battery/battery.c b/profiles/battery/battery.c
>>> index a93b352..95f548e 100644
>>> --- a/profiles/battery/battery.c
>>> +++ b/profiles/battery/battery.c
>>> @@ -43,6 +43,7 @@ struct battery {
>>> GAttrib *attrib; /* GATT connection */
>>> guint attioid; /* Att watcher id */
>>> struct att_range *svc_range; /* Battery range */
>>> + guint attnotid; /* Att notifications
>>> id */
>>> GSList *chars; /* Characteristics */
>>> };
>>>
>>> @@ -56,6 +57,7 @@ struct characteristic {
>>> uint8_t ns; /* Battery Namespace */
>>> uint16_t description; /* Battery
>>> description */
>>> uint8_t level;
>>> + gboolean canNotify;
>>> };
>>>
>>> struct descriptor {
>>> @@ -86,6 +88,14 @@ static void char_free(gpointer user_data)
>>> g_free(c);
>>> }
>>>
>>> +static gint cmp_char_val_handle(gconstpointer a, gconstpointer b)
>>> +{
>>> + const struct characteristic *ch = a;
>>> + const uint16_t *handle = b;
>>> +
>>> + return ch->attr.value_handle - *handle;
>>> +}
>>> +
>>> static void battery_free(gpointer user_data)
>>> {
>>> struct battery *batt = user_data;
>>> @@ -99,6 +109,10 @@ static void battery_free(gpointer user_data)
>>> if (batt->attrib != NULL)
>>> g_attrib_unref(batt->attrib);
>>>
>>> + if (batt->attrib != NULL) {
>>> + g_attrib_unregister(batt->attrib, batt->attnotid);
>>> + g_attrib_unref(batt->attrib);
>>> + }
>>
>> You're using the same condition twice here (batt->attrib != NULL).
>> Looks like a copy & paste error. Make sure you check for attnotid
>> before checking attrib, and that you don't unref attrib twice.
>>
> Correct. I'll fix this and check for attnotid.
>
>>> btd_device_unref(batt->dev);
>>> g_free(batt->svc_range);
>>> g_free(batt);
>>> @@ -140,6 +154,18 @@ static void process_batteryservice_char(struct
>>> characteristic *ch)
>>> }
>>> }
>>>
>>> +static void batterylevel_enable_notify_cb(guint8 status, const
>>> guint8 *pdu,
>>> + guint16 len, gpointer
>>> user_data)
>>> +{
>>> + struct characteristic *ch = (struct characteristic *)user_data;
>>> +
>>> + if (status != 0) {
>>> + error("Could not enable batt level notification.");
>>> + ch->canNotify = FALSE;
>>> + process_batteryservice_char(ch);
>>
>> I don't think is necessary to call process_batteryservice_char() here,
>> since you already called it during the characteristics discovery.
>>
> I'll remove the unnecessary call here.
>
>
After checking this again, this call should not be removed. The reason
for that is simple - if a characteristic can not be notified, and we
read its initial value from file, we need to make sure we refresh the
battery level from the peer device automatically (since we will not get
notified when level changes).


>>> + }
>>> +}
>>> +
>>> static gint device_battery_cmp(gconstpointer a, gconstpointer b)
>>> {
>>> const struct characteristic *ch = a;
>>> @@ -178,6 +204,19 @@ static void batterylevel_refresh_cb(struct
>>> device_battery *batt)
>>> process_batteryservice_char(ch);
>>> }
>>>
>>> +static void enable_battery_notification(struct characteristic *ch,
>>> +
>>> uint16_t handle)
>>> +{
>>> + uint8_t atval[2];
>>> + uint16_t val;
>>> +
>>> + val = GATT_CLIENT_CHARAC_CFG_NOTIF_BIT;
>>> +
>>> + att_put_u16(val, atval);
>>> + gatt_write_char(ch->batt->attrib, handle, atval, 2,
>>> + batterylevel_enable_notify_cb, ch);
>>> +}
>>> +
>>> static void batterylevel_presentation_format_desc_cb(guint8 status,
>>> const guint8 *pdu,
>>> guint16 len,
>>> gpointer user_data)
>>> @@ -207,13 +246,20 @@ static void
>>> batterylevel_presentation_format_desc_cb(guint8 status,
>>> desc->ch->description = att_get_u16(&value[5]);
>>> }
>>>
>>> -
>>> static void process_batterylevel_desc(struct descriptor *desc)
>>> {
>>> struct characteristic *ch = desc->ch;
>>> char uuidstr[MAX_LEN_UUID_STR];
>>> bt_uuid_t btuuid;
>>>
>>> + bt_uuid16_create(&btuuid, GATT_CLIENT_CHARAC_CFG_UUID);
>>> +
>>> + if (bt_uuid_cmp(&desc->uuid, &btuuid) == 0 &&
>>> g_strcmp0(ch->attr.uuid,
>>> + BATTERY_LEVEL_UUID)
>>> == 0) {
>>> + enable_battery_notification(ch, desc->handle);
>>> + return;
>>> + }
>>> +
>>> bt_uuid16_create(&btuuid, GATT_CHARAC_FMT_UUID);
>>>
>>> if (bt_uuid_cmp(&desc->uuid, &btuuid) == 0) {
>>> @@ -319,12 +365,54 @@ static void configure_battery_cb(GSList
>>> *characteristics, guint8 status,
>>> }
>>> }
>>>
>>> +static void proc_batterylevel(struct characteristic *c, const
>>> uint8_t *pdu,
>>> + uint16_t len,
>>> gboolean final)
>>> +{
>>> + if (!pdu) {
>>> + error("Battery level notification: Invalid pdu length");
>>> + return;
>>> + }
>>> +
>>> + c->level = pdu[1];
>>> +
>>> + btd_device_set_battery_opt(c->devbatt, BATTERY_OPT_LEVEL,
>>> c->level,
>>> +
>>> BATTERY_OPT_INVALID);
>>> +}
>>> +
>>> +static void notif_handler(const uint8_t *pdu, uint16_t len, gpointer
>>> user_data)
>>> +{
>>> + struct battery *batt = user_data;
>>> + struct characteristic *ch;
>>> + uint16_t handle;
>>> + GSList *l;
>>> +
>>> + if (len < 3) {
>>> + error("notif_handler: Bad pdu received");
>>> + return;
>>> + }
>>> +
>>> + handle = att_get_u16(&pdu[1]);
>>> + l = g_slist_find_custom(batt->chars, &handle,
>>> cmp_char_val_handle);
>>> + if (l == NULL) {
>>> + error("notif_handler: Unexpected handle 0x%04x",
>>> handle);
>>> + return;
>>> + }
>>> +
>>> + ch = l->data;
>>> + if (g_strcmp0(ch->attr.uuid, BATTERY_LEVEL_UUID) == 0) {
>>> + proc_batterylevel(ch, pdu, len, FALSE);
>>> + }
>>> +}
>>> +
>>> static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
>>> {
>>> struct battery *batt = user_data;
>>>
>>> batt->attrib = g_attrib_ref(attrib);
>>>
>>> + batt->attnotid = g_attrib_register(batt->attrib,
>>> ATT_OP_HANDLE_NOTIFY,
>>> + notif_handler, batt,
>>> NULL);
>>> +
>>> if (batt->chars == NULL) {
>>> gatt_discover_char(batt->attrib,
>>> batt->svc_range->start,
>>> batt->svc_range->end, NULL,
>>> @@ -333,7 +421,8 @@ static void attio_connected_cb(GAttrib *attrib,
>>> gpointer user_data)
>>> GSList *l;
>>> for (l = batt->chars; l; l = l->next) {
>>> struct characteristic *c = l->data;
>>> - process_batteryservice_char(c);
>>> + if (!c->canNotify)
>>> + process_batteryservice_char(c);
>>> }
>>> }
>>> }
>>> @@ -342,6 +431,8 @@ static void attio_disconnected_cb(gpointer
>>> user_data)
>>> {
>>> struct battery *batt = user_data;
>>>
>>> + g_attrib_unregister(batt->attrib, batt->attnotid);
>>> + batt->attnotid = 0;
>>> g_attrib_unref(batt->attrib);
>>> batt->attrib = NULL;
>>> }
>>> --
>>> 1.7.9.5
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe
>>> linux-bluetooth" in
>>> the body of a message to [email protected]
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>>
>>
>
> Thanks,
>


--
BR,
Chen Ganir


2012-09-12 10:57:20

by Anderson Lizardo

[permalink] [raw]
Subject: Re: [PATCH 02/10] battery: Implement Generic device battery

Hi Chen,

On Wed, Sep 12, 2012 at 6:30 AM, Chen Ganir <[email protected]> wrote:
>> Both BtIO and GAttrib intend to mimic GLib-like libraries where this is
>> the convention. The plan is not to let this spread to the entire code
>> base (particularly with the likely move to libell in the long run), so
>> please don't use CamelCase.
>>
> Ok. So for the battery options i'll simply change it to battery_option. What
> about the convention for defining a function pointer ?

See the result of:

grep -r 'typedef.*(' *

on your bluez tree. As Johan mentioned, you should ignore the (few)
occurrences of camel case typedef declarations.

Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

2012-09-12 10:30:39

by Ganir, Chen

[permalink] [raw]
Subject: Re: [PATCH 02/10] battery: Implement Generic device battery

Johan,

On 09/12/2012 11:45 AM, Johan Hedberg wrote:
> Hi Chen,
>
> On Wed, Sep 12, 2012, Chen Ganir wrote:
>>> Since this symbol is being exported, shouldn't it be prefixed with
>>> btd_ as well?
>>>
>> I will rename it to struct btd_battery. Is that ok ?
>
> Yes.
>
Thanks.

>>
>>>> +typedef void (*RefreshBattFunc) (struct device_battery *batt);
>>>
>>> I don't think we use CamelCase for anything other than D-Bus method names.
>>>
>> We use it in gattrib.h for function pointers. What is the correct
>> convention for function pointers ? same as in adapter_ops ?
>>
>>>> +
>>>> +typedef enum {
>>>> + BATTERY_OPT_INVALID = 0,
>>>> + BATTERY_OPT_LEVEL,
>>>> + BATTERY_OPT_REFRESH_FUNC,
>>>> +} BatteryOption;
>>>
>>> Fix CamelCase usage here and on uses of this type as well.
>>>
>> btio.h also uses this convention. What should be the correct convention ?
>
> Both BtIO and GAttrib intend to mimic GLib-like libraries where this is
> the convention. The plan is not to let this spread to the entire code
> base (particularly with the likely move to libell in the long run), so
> please don't use CamelCase.
>
Ok. So for the battery options i'll simply change it to battery_option.
What about the convention for defining a function pointer ?


> Johan
>


--
BR,
Chen Ganir
Texas Instruments

2012-09-12 08:45:37

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH 02/10] battery: Implement Generic device battery

Hi Chen,

On Wed, Sep 12, 2012, Chen Ganir wrote:
> >Since this symbol is being exported, shouldn't it be prefixed with
> >btd_ as well?
> >
> I will rename it to struct btd_battery. Is that ok ?

Yes.

>
> >>+typedef void (*RefreshBattFunc) (struct device_battery *batt);
> >
> >I don't think we use CamelCase for anything other than D-Bus method names.
> >
> We use it in gattrib.h for function pointers. What is the correct
> convention for function pointers ? same as in adapter_ops ?
>
> >>+
> >>+typedef enum {
> >>+ BATTERY_OPT_INVALID = 0,
> >>+ BATTERY_OPT_LEVEL,
> >>+ BATTERY_OPT_REFRESH_FUNC,
> >>+} BatteryOption;
> >
> >Fix CamelCase usage here and on uses of this type as well.
> >
> btio.h also uses this convention. What should be the correct convention ?

Both BtIO and GAttrib intend to mimic GLib-like libraries where this is
the convention. The plan is not to let this spread to the entire code
base (particularly with the likely move to libell in the long run), so
please don't use CamelCase.

Johan

2012-09-12 04:58:34

by Ganir, Chen

[permalink] [raw]
Subject: Re: [PATCH 09/10] battery: Add support for notifications

Joao,
On 09/12/2012 01:08 AM, Joao Paulo Rechi Vita wrote:
> On Tue, Sep 11, 2012 at 4:38 AM, <[email protected]> wrote:
>> From: Chen Ganir <[email protected]>
>>
>> Add support for emitting PropertyChanged when a battery level
>> characteristic notification is sent from the peer device.
>> ---
>> profiles/battery/battery.c | 95 +++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 93 insertions(+), 2 deletions(-)
>>
>> diff --git a/profiles/battery/battery.c b/profiles/battery/battery.c
>> index a93b352..95f548e 100644
>> --- a/profiles/battery/battery.c
>> +++ b/profiles/battery/battery.c
>> @@ -43,6 +43,7 @@ struct battery {
>> GAttrib *attrib; /* GATT connection */
>> guint attioid; /* Att watcher id */
>> struct att_range *svc_range; /* Battery range */
>> + guint attnotid; /* Att notifications id */
>> GSList *chars; /* Characteristics */
>> };
>>
>> @@ -56,6 +57,7 @@ struct characteristic {
>> uint8_t ns; /* Battery Namespace */
>> uint16_t description; /* Battery description */
>> uint8_t level;
>> + gboolean canNotify;
>> };
>>
>> struct descriptor {
>> @@ -86,6 +88,14 @@ static void char_free(gpointer user_data)
>> g_free(c);
>> }
>>
>> +static gint cmp_char_val_handle(gconstpointer a, gconstpointer b)
>> +{
>> + const struct characteristic *ch = a;
>> + const uint16_t *handle = b;
>> +
>> + return ch->attr.value_handle - *handle;
>> +}
>> +
>> static void battery_free(gpointer user_data)
>> {
>> struct battery *batt = user_data;
>> @@ -99,6 +109,10 @@ static void battery_free(gpointer user_data)
>> if (batt->attrib != NULL)
>> g_attrib_unref(batt->attrib);
>>
>> + if (batt->attrib != NULL) {
>> + g_attrib_unregister(batt->attrib, batt->attnotid);
>> + g_attrib_unref(batt->attrib);
>> + }
>
> You're using the same condition twice here (batt->attrib != NULL).
> Looks like a copy & paste error. Make sure you check for attnotid
> before checking attrib, and that you don't unref attrib twice.
>
Correct. I'll fix this and check for attnotid.

>> btd_device_unref(batt->dev);
>> g_free(batt->svc_range);
>> g_free(batt);
>> @@ -140,6 +154,18 @@ static void process_batteryservice_char(struct characteristic *ch)
>> }
>> }
>>
>> +static void batterylevel_enable_notify_cb(guint8 status, const guint8 *pdu,
>> + guint16 len, gpointer user_data)
>> +{
>> + struct characteristic *ch = (struct characteristic *)user_data;
>> +
>> + if (status != 0) {
>> + error("Could not enable batt level notification.");
>> + ch->canNotify = FALSE;
>> + process_batteryservice_char(ch);
>
> I don't think is necessary to call process_batteryservice_char() here,
> since you already called it during the characteristics discovery.
>
I'll remove the unnecessary call here.


>> + }
>> +}
>> +
>> static gint device_battery_cmp(gconstpointer a, gconstpointer b)
>> {
>> const struct characteristic *ch = a;
>> @@ -178,6 +204,19 @@ static void batterylevel_refresh_cb(struct device_battery *batt)
>> process_batteryservice_char(ch);
>> }
>>
>> +static void enable_battery_notification(struct characteristic *ch,
>> + uint16_t handle)
>> +{
>> + uint8_t atval[2];
>> + uint16_t val;
>> +
>> + val = GATT_CLIENT_CHARAC_CFG_NOTIF_BIT;
>> +
>> + att_put_u16(val, atval);
>> + gatt_write_char(ch->batt->attrib, handle, atval, 2,
>> + batterylevel_enable_notify_cb, ch);
>> +}
>> +
>> static void batterylevel_presentation_format_desc_cb(guint8 status,
>> const guint8 *pdu, guint16 len,
>> gpointer user_data)
>> @@ -207,13 +246,20 @@ static void batterylevel_presentation_format_desc_cb(guint8 status,
>> desc->ch->description = att_get_u16(&value[5]);
>> }
>>
>> -
>> static void process_batterylevel_desc(struct descriptor *desc)
>> {
>> struct characteristic *ch = desc->ch;
>> char uuidstr[MAX_LEN_UUID_STR];
>> bt_uuid_t btuuid;
>>
>> + bt_uuid16_create(&btuuid, GATT_CLIENT_CHARAC_CFG_UUID);
>> +
>> + if (bt_uuid_cmp(&desc->uuid, &btuuid) == 0 && g_strcmp0(ch->attr.uuid,
>> + BATTERY_LEVEL_UUID) == 0) {
>> + enable_battery_notification(ch, desc->handle);
>> + return;
>> + }
>> +
>> bt_uuid16_create(&btuuid, GATT_CHARAC_FMT_UUID);
>>
>> if (bt_uuid_cmp(&desc->uuid, &btuuid) == 0) {
>> @@ -319,12 +365,54 @@ static void configure_battery_cb(GSList *characteristics, guint8 status,
>> }
>> }
>>
>> +static void proc_batterylevel(struct characteristic *c, const uint8_t *pdu,
>> + uint16_t len, gboolean final)
>> +{
>> + if (!pdu) {
>> + error("Battery level notification: Invalid pdu length");
>> + return;
>> + }
>> +
>> + c->level = pdu[1];
>> +
>> + btd_device_set_battery_opt(c->devbatt, BATTERY_OPT_LEVEL, c->level,
>> + BATTERY_OPT_INVALID);
>> +}
>> +
>> +static void notif_handler(const uint8_t *pdu, uint16_t len, gpointer user_data)
>> +{
>> + struct battery *batt = user_data;
>> + struct characteristic *ch;
>> + uint16_t handle;
>> + GSList *l;
>> +
>> + if (len < 3) {
>> + error("notif_handler: Bad pdu received");
>> + return;
>> + }
>> +
>> + handle = att_get_u16(&pdu[1]);
>> + l = g_slist_find_custom(batt->chars, &handle, cmp_char_val_handle);
>> + if (l == NULL) {
>> + error("notif_handler: Unexpected handle 0x%04x", handle);
>> + return;
>> + }
>> +
>> + ch = l->data;
>> + if (g_strcmp0(ch->attr.uuid, BATTERY_LEVEL_UUID) == 0) {
>> + proc_batterylevel(ch, pdu, len, FALSE);
>> + }
>> +}
>> +
>> static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
>> {
>> struct battery *batt = user_data;
>>
>> batt->attrib = g_attrib_ref(attrib);
>>
>> + batt->attnotid = g_attrib_register(batt->attrib, ATT_OP_HANDLE_NOTIFY,
>> + notif_handler, batt, NULL);
>> +
>> if (batt->chars == NULL) {
>> gatt_discover_char(batt->attrib, batt->svc_range->start,
>> batt->svc_range->end, NULL,
>> @@ -333,7 +421,8 @@ static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
>> GSList *l;
>> for (l = batt->chars; l; l = l->next) {
>> struct characteristic *c = l->data;
>> - process_batteryservice_char(c);
>> + if (!c->canNotify)
>> + process_batteryservice_char(c);
>> }
>> }
>> }
>> @@ -342,6 +431,8 @@ static void attio_disconnected_cb(gpointer user_data)
>> {
>> struct battery *batt = user_data;
>>
>> + g_attrib_unregister(batt->attrib, batt->attnotid);
>> + batt->attnotid = 0;
>> g_attrib_unref(batt->attrib);
>> batt->attrib = NULL;
>> }
>> --
>> 1.7.9.5
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>

Thanks,

--
BR,
Chen Ganir


2012-09-12 04:55:59

by Ganir, Chen

[permalink] [raw]
Subject: Re: [PATCH 08/10] battery: Read Battery level characteristic

On 09/12/2012 12:50 AM, Joao Paulo Rechi Vita wrote:
> On Tue, Sep 11, 2012 at 4:38 AM, <[email protected]> wrote:
>> From: Chen Ganir <[email protected]>
>>
>> Implement support for reading the battery level characteristic on
>> connection establishment.
>> ---
>> profiles/battery/battery.c | 87 +++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 85 insertions(+), 2 deletions(-)
>>
>> diff --git a/profiles/battery/battery.c b/profiles/battery/battery.c
>> index 31f2297..a93b352 100644
>> --- a/profiles/battery/battery.c
>> +++ b/profiles/battery/battery.c
>> @@ -55,6 +55,7 @@ struct characteristic {
>> GSList *desc; /* Descriptors */
>> uint8_t ns; /* Battery Namespace */
>> uint16_t description; /* Battery description */
>> + uint8_t level;
>
> Alignment problem here.
>
Thanks.


>> };
>>
>> struct descriptor {
>> @@ -103,6 +104,80 @@ static void battery_free(gpointer user_data)
>> g_free(batt);
>> }
>>
>> +static void read_batterylevel_cb(guint8 status, const guint8 *pdu, guint16 len,
>> + gpointer user_data)
>> +{
>> + struct characteristic *ch = user_data;
>> + uint8_t value[ATT_MAX_MTU];
>> + int vlen;
>> +
>> + if (status != 0) {
>> + error("Failed to read Battery Level:%s", att_ecode2str(status));
>> + return;
>> + }
>> +
>> + vlen = dec_read_resp(pdu, len, value, sizeof(value));
>> + if (!vlen) {
>> + error("Failed to read Battery Level: Protocol error\n");
>> + return;
>> + }
>> +
>> + if (vlen < 1) {
>> + error("Failed to read Battery Level: Wrong pdu len");
>> + return;
>> + }
>> +
>> + ch->level = value[0];
>> + btd_device_set_battery_opt(ch->devbatt, BATTERY_OPT_LEVEL, ch->level,
>> + BATTERY_OPT_INVALID);
>> +}
>> +
>> +static void process_batteryservice_char(struct characteristic *ch)
>> +{
>> + if (g_strcmp0(ch->attr.uuid, BATTERY_LEVEL_UUID) == 0) {
>> + gatt_read_char(ch->batt->attrib, ch->attr.value_handle, 0,
>> + read_batterylevel_cb, ch);
>> + }
>> +}
>> +
>> +static gint device_battery_cmp(gconstpointer a, gconstpointer b)
>> +{
>> + const struct characteristic *ch = a;
>> + const struct device_battery *batt = b;
>> +
>> + if (batt == ch->devbatt)
>> + return 0;
>> +
>> + return -1;
>> +}
>> +
>> +static struct characteristic *find_battery_char(struct device_battery *db)
>> +{
>> + GSList *l, *b;
>> +
>> + for (l = servers; l != NULL; l = g_slist_next(l)) {
>> + struct battery *batt = l->data;
>> +
>> + b = g_slist_find_custom(batt->chars, db, device_battery_cmp);
>> + if (!b)
>> + return NULL;
>> +
>> + return b->data;
>> + }
>> +
>> + return NULL;
>> +}
>> +
>> +static void batterylevel_refresh_cb(struct device_battery *batt)
>> +{
>> + struct characteristic *ch;
>> +
>> + ch = find_battery_char(batt);
>> +
>> + if (ch)
>> + process_batteryservice_char(ch);
>> +}
>> +
>> static void batterylevel_presentation_format_desc_cb(guint8 status,
>> const guint8 *pdu, guint16 len,
>> gpointer user_data)
>> @@ -151,7 +226,6 @@ static void process_batterylevel_desc(struct descriptor *desc)
>> DBG("Ignored descriptor %s characteristic %s", uuidstr, ch->attr.uuid);
>> }
>>
>> -
>> static void discover_desc_cb(guint8 status, const guint8 *pdu, guint16 len,
>> gpointer user_data)
>> {
>> @@ -192,8 +266,11 @@ static void discover_desc_cb(guint8 status, const guint8 *pdu, guint16 len,
>>
>> update_char:
>> ch->devbatt = btd_device_add_battery(ch->batt->dev);
>> -}
>> + btd_device_set_battery_opt(ch->devbatt, BATTERY_OPT_REFRESH_FUNC,
>> + batterylevel_refresh_cb, BATTERY_OPT_INVALID);
>>
>> + process_batteryservice_char(ch);
>
> There two call also should be part of configure_battery_cb() instead
> of discover_desc_cb().
>
True. Once i move the btd_device_add_battery call, i'll move those as well.

>> +}
>>
>> static void configure_battery_cb(GSList *characteristics, guint8 status,
>>
>> @@ -252,6 +329,12 @@ static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
>> gatt_discover_char(batt->attrib, batt->svc_range->start,
>> batt->svc_range->end, NULL,
>> configure_battery_cb, batt);
>> + } else {
>> + GSList *l;
>> + for (l = batt->chars; l; l = l->next) {
>> + struct characteristic *c = l->data;
>> + process_batteryservice_char(c);
>> + }
>> }
>> }
>>
>> --
>> 1.7.9.5
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>

Thanks,


--
BR,
Chen Ganir


2012-09-12 04:54:52

by Ganir, Chen

[permalink] [raw]
Subject: Re: [PATCH 07/10] battery: Add Battery to device

On 09/12/2012 12:40 AM, Joao Paulo Rechi Vita wrote:
> On Tue, Sep 11, 2012 at 4:38 AM, <[email protected]> wrote:
>> From: Chen Ganir <[email protected]>
>>
>> Add/Remove battery from device
>> ---
>> profiles/battery/battery.c | 14 ++++++++++----
>> 1 file changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/profiles/battery/battery.c b/profiles/battery/battery.c
>> index d1e0b6e..31f2297 100644
>> --- a/profiles/battery/battery.c
>> +++ b/profiles/battery/battery.c
>> @@ -49,8 +49,9 @@ struct battery {
>> static GSList *servers;
>>
>> struct characteristic {
>> - struct gatt_char attr; /* Characteristic */
>> - struct battery *batt; /* Parent Battery Service */
>> + struct device_battery *devbatt; /* device_battery pointer */
>> + struct gatt_char attr; /* Characteristic */
>> + struct battery *batt; /* Parent Battery Service */
>
> Just a minor pick here: the comment alignment of attr and batt should
> have been fixed on the previous commit, to keep it consistent along
> the series.
>
You are correct. That should have been like that, but i missed it. Will
fix it.

>> GSList *desc; /* Descriptors */
>> uint8_t ns; /* Battery Namespace */
>> uint16_t description; /* Battery description */
>> @@ -79,6 +80,8 @@ static void char_free(gpointer user_data)
>>
>> g_slist_free_full(c->desc, g_free);
>>
>> + btd_device_remove_battery(c->devbatt);
>> +
>> g_free(c);
>> }
>>
>> @@ -160,12 +163,12 @@ static void discover_desc_cb(guint8 status, const guint8 *pdu, guint16 len,
>> if (status != 0) {
>> error("Discover all characteristic descriptors failed [%s]: %s",
>> ch->attr.uuid, att_ecode2str(status));
>> - return;
>> + goto update_char;
>> }
>>
>> list = dec_find_info_resp(pdu, len, &format);
>> if (list == NULL)
>> - return;
>> + goto update_char;
>>
>> for (i = 0; i < list->num; i++) {
>> struct descriptor *desc;
>> @@ -186,6 +189,9 @@ static void discover_desc_cb(guint8 status, const guint8 *pdu, guint16 len,
>> }
>>
>> att_data_list_free(list);
>> +
>> +update_char:
>> + ch->devbatt = btd_device_add_battery(ch->batt->dev);
>
> If I understood correctly, adding a battery to the device is a
> consequence of finding the "Battery Level" characteristic, and not
> related to the descriptors discovery. So the call to
> btd_device_add_battery() should be done inside the
> configure_battery_cb() instead of the discover_desc_cb(). This way
> you'll not need all these goto's.
>
You are correct. It can be done there.

>> }
>>
>>
>> --
>> 1.7.9.5
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>

Thanks,

--
BR,
Chen Ganir


2012-09-12 04:49:30

by Ganir, Chen

[permalink] [raw]
Subject: Re: [PATCH 05/10] battery: Discover Characteristic Descriptors

Joao,

On 09/11/2012 11:52 PM, Joao Paulo Rechi Vita wrote:
> On Tue, Sep 11, 2012 at 4:38 AM, <[email protected]> wrote:
>> From: Chen Ganir <[email protected]>
>>
>> Discover all characteristic descriptors, and build a descriptor
>> list. Presentation Format Descriptor and Client Characteristic
>> Configuration descriptors are searched.
>> ---
>> profiles/battery/battery.c | 73 +++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 72 insertions(+), 1 deletion(-)
>>
>> diff --git a/profiles/battery/battery.c b/profiles/battery/battery.c
>> index 37cbce4..3a24a7b 100644
>> --- a/profiles/battery/battery.c
>> +++ b/profiles/battery/battery.c
>> @@ -51,6 +51,13 @@ static GSList *servers;
>> struct characteristic {
>> struct gatt_char attr; /* Characteristic */
>> struct battery *batt; /* Parent Battery Service */
>> + GSList *desc; /* Descriptors */
>
> Wrong alignment here.
>
Thanks.

>> +};
>> +
>> +struct descriptor {
>> + struct characteristic *ch; /* Parent Characteristic */
>> + uint16_t handle; /* Descriptor Handle */
>> + bt_uuid_t uuid; /* UUID */
>> };
>>
>> static gint cmp_device(gconstpointer a, gconstpointer b)
>> @@ -64,12 +71,21 @@ static gint cmp_device(gconstpointer a, gconstpointer b)
>> return -1;
>> }
>>
>> +static void char_free(gpointer user_data)
>> +{
>> + struct characteristic *c = user_data;
>> +
>> + g_slist_free_full(c->desc, g_free);
>> +
>> + g_free(c);
>> +}
>> +
>> static void battery_free(gpointer user_data)
>> {
>> struct battery *batt = user_data;
>>
>> if (batt->chars != NULL)
>> - g_slist_free_full(batt->chars, g_free);
>> + g_slist_free_full(batt->chars, char_free);
>>
>> if (batt->attioid > 0)
>> btd_device_remove_attio_callback(batt->dev, batt->attioid);
>> @@ -82,7 +98,47 @@ static void battery_free(gpointer user_data)
>> g_free(batt);
>> }
>>
>> +static void discover_desc_cb(guint8 status, const guint8 *pdu, guint16 len,
>> + gpointer user_data)
>> +{
>> + struct characteristic *ch = user_data;
>> + struct att_data_list *list;
>> + uint8_t format;
>> + int i;
>> +
>> + if (status != 0) {
>> + error("Discover all characteristic descriptors failed [%s]: %s",
>> + ch->attr.uuid, att_ecode2str(status));
>> + return;
>> + }
>> +
>> + list = dec_find_info_resp(pdu, len, &format);
>> + if (list == NULL)
>> + return;
>> +
>> + for (i = 0; i < list->num; i++) {
>> + struct descriptor *desc;
>> + uint8_t *value;
>> +
>> + value = list->data[i];
>> + desc = g_new0(struct descriptor, 1);
>> + desc->handle = att_get_u16(value);
>> + desc->ch = ch;
>> +
>> + if (format == 0x01)
>> + desc->uuid = att_get_uuid16(&value[2]);
>> + else
>> + desc->uuid = att_get_uuid128(&value[2]);
>> +
>> + ch->desc = g_slist_append(ch->desc, desc);
>> + }
>> +
>> + att_data_list_free(list);
>> +}
>> +
>> +
>
> Remove extra blank line here.
>
>> static void configure_battery_cb(GSList *characteristics, guint8 status,
>> +
>
> Remove extra blank line here.
>
Ok, will remove both of them.

>> gpointer user_data)
>> {
>> struct battery *batt = user_data;
>> @@ -97,6 +153,7 @@ static void configure_battery_cb(GSList *characteristics, guint8 status,
>> for (l = characteristics; l; l = l->next) {
>> struct gatt_char *c = l->data;
>> struct characteristic *ch;
>> + uint16_t start, end;
>>
>> ch = g_new0(struct characteristic, 1);
>> ch->attr.handle = c->handle;
>> @@ -106,6 +163,20 @@ static void configure_battery_cb(GSList *characteristics, guint8 status,
>> ch->batt = batt;
>>
>> batt->chars = g_slist_append(batt->chars, ch);
>> +
>> + start = c->value_handle + 1;
>> +
>> + if (l->next != NULL) {
>> + struct gatt_char *c = l->next->data;
>> + if (start == c->handle)
>> + continue;
>> + end = c->handle - 1;
>> + } else if (c->value_handle != batt->svc_range->end)
>> + end = batt->svc_range->end;
>> + else
>> + continue;
>> +
>> + gatt_find_info(batt->attrib, start, end, discover_desc_cb, ch);
>> }
>> }
>>
>> --
>> 1.7.9.5
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>

Thanks,



--
BR,
Chen Ganir


2012-09-12 04:48:22

by Ganir, Chen

[permalink] [raw]
Subject: Re: [PATCH 02/10] battery: Implement Generic device battery

Joao,

On 09/11/2012 09:27 PM, Joao Paulo Rechi Vita wrote:
> On Tue, Sep 11, 2012 at 4:38 AM, <[email protected]> wrote:
>> From: Chen Ganir <[email protected]>
>>
>> Add implementation for the generic battery in bluetooth device.
>> This patch adds new D-Bus interface for adding/removing/changing
>> peer device battery status, allowing management of remote device
>> batteries.
>> ---
>> src/device.c | 184 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> src/device.h | 15 +++++
>> test/test-device | 13 ++++
>> 3 files changed, 212 insertions(+)
>>
>> diff --git a/src/device.c b/src/device.c
>> index 02ef35e..ce4d467 100644
>> --- a/src/device.c
>> +++ b/src/device.c
>> @@ -166,6 +166,7 @@ struct btd_device {
>>
>> gboolean authorizing;
>> gint ref;
>> + GSList *batteries;
>>
>> GIOChannel *att_io;
>> guint cleanup_id;
>> @@ -180,6 +181,26 @@ static uint16_t uuid_list[] = {
>> 0
>> };
>>
>> +struct device_battery {
>> + DBusConnection *conn;
>> + uint16_t level;
>> + char *path;
>> + struct btd_device *device;
>> + RefreshBattFunc refresh_func;
>> +};
>> +
>> +static void battery_free(gpointer user_data)
>> +{
>> + struct device_battery *b = user_data;
>> +
>> + g_dbus_unregister_interface(b->conn, b->path, BATTERY_INTERFACE);
>> + dbus_connection_unref(b->conn);
>> + btd_device_unref(b->device);
>> + g_free(b->path);
>> + g_free(b);
>> +
>> +}
>> +
>> static void browse_request_free(struct browse_req *req)
>> {
>> if (req->listener_id)
>> @@ -259,6 +280,7 @@ static void device_free(gpointer user_data)
>> g_slist_free_full(device->primaries, g_free);
>> g_slist_free_full(device->attios, g_free);
>> g_slist_free_full(device->attios_offline, g_free);
>> + g_slist_free_full(device->batteries, battery_free);
>>
>> attio_cleanup(device);
>>
>> @@ -433,6 +455,15 @@ static DBusMessage *get_properties(DBusConnection *conn,
>> ptr = adapter_get_path(adapter);
>> dict_append_entry(&dict, "Adapter", DBUS_TYPE_OBJECT_PATH, &ptr);
>>
>> + /* Batteries */
>> + str = g_new0(char *, g_slist_length(device->batteries) + 1);
>> + for (i = 0, l = device->batteries; l; l = l->next, i++) {
>> + struct device_battery *b = l->data;
>> + str[i] = b->path;
>> + }
>> + dict_append_array(&dict, "Batteries", DBUS_TYPE_OBJECT_PATH, &str, i);
>> + g_free(str);
>> +
>> dbus_message_iter_close_container(&iter, &dict);
>>
>> return reply;
>> @@ -3263,3 +3294,156 @@ void btd_profile_disconnected(struct btd_profile *profile,
>> {
>> device->conns = g_slist_remove(device->conns, profile);
>> }
>> +
>> +static void batteries_changed(struct btd_device *device)
>> +{
>> + DBusConnection *conn = get_dbus_connection();
>> + char **batteries;
>> + GSList *l;
>> + int i;
>> +
>> + batteries = g_new0(char *, g_slist_length(device->batteries) + 1);
>> + for (i = 0, l = device->batteries; l; l = l->next, i++) {
>> + struct device_battery *batt = l->data;
>> + batteries[i] = batt->path;
>> + }
>> +
>> + emit_array_property_changed(conn, device->path, DEVICE_INTERFACE,
>> + "Batteries", DBUS_TYPE_OBJECT_PATH,
>> + &batteries, i);
>> +
>> + g_free(batteries);
>> +}
>> +
>> +static DBusMessage *refresh_batt_level(DBusConnection *conn,
>> + DBusMessage *msg, void *data)
>> +{
>> + struct device_battery *b = data;
>> +
>> + if (!b->refresh_func)
>> + return btd_error_not_supported(msg);
>> +
>> + b->refresh_func(b);
>> +
>> + return dbus_message_new_method_return(msg);
>> +}
>> +
>> +static DBusMessage *get_batt_properties(DBusConnection *conn,
>> + DBusMessage *msg, void *data)
>> +{
>> + struct device_battery *b = data;
>> + DBusMessageIter iter;
>> + DBusMessageIter dict;
>> + DBusMessage *reply;
>> +
>> + reply = dbus_message_new_method_return(msg);
>> + if (reply == NULL)
>> + return NULL;
>> +
>> + dbus_message_iter_init_append(reply, &iter);
>> +
>> + dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
>> + DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
>> + DBUS_TYPE_STRING_AS_STRING DBUS_TYPE_VARIANT_AS_STRING
>> + DBUS_DICT_ENTRY_END_CHAR_AS_STRING, &dict);
>> +
>> + dict_append_entry(&dict, "Level", DBUS_TYPE_UINT16, &b->level);
>> +
>> + dbus_message_iter_close_container(&iter, &dict);
>> +
>> + return reply;
>> +}
>> +
>> +static GDBusMethodTable battery_methods[] = {
>> + { GDBUS_METHOD("GetProperties",
>> + NULL, GDBUS_ARGS({ "properties", "a{sv}" }),
>> + get_batt_properties) },
>> + { GDBUS_METHOD("Refresh",
>> + NULL, NULL,
>> + refresh_batt_level) },
>> + { }
>> +};
>> +
>> +static GDBusSignalTable battery_signals[] = {
>> + { GDBUS_SIGNAL("PropertyChanged",
>> + GDBUS_ARGS({ "name", "s" }, { "value", "v" })) },
>> + { }
>> +};
>> +
>> +struct device_battery *btd_device_add_battery(struct btd_device *device)
>> +{
>> + struct device_battery *batt;
>> + DBusConnection *conn = get_dbus_connection();
>> +
>> + batt = g_new0(struct device_battery, 1);
>> + batt->path = g_strdup_printf("%s/BATT%04X", device->path,
>> + g_slist_length(device->batteries));
>> + batt->conn = dbus_connection_ref(conn);
>> +
>> + if (!g_dbus_register_interface(batt->conn, batt->path,
>> + BATTERY_INTERFACE, battery_methods, battery_signals,
>> + NULL, batt, NULL)) {
>> + error("D-Bus register interface %s failed", BATTERY_INTERFACE);
>> + dbus_connection_unref(batt->conn);
>> + g_free(batt->path);
>> + g_free(batt);
>> + return NULL;
>> + }
>> +
>> + batt->device = btd_device_ref(device);
>> + device->batteries = g_slist_append(device->batteries, batt);
>> + batteries_changed(device);
>> +
>> + return batt;
>> +}
>> +
>> +void btd_device_remove_battery(struct device_battery *batt)
>> +{
>> + struct btd_device *dev = batt->device;
>> +
>> + dev->batteries = g_slist_remove(dev->batteries, batt);
>> +
>> + battery_free(batt);
>> +
>> + batteries_changed(dev);
>> +}
>> +
>> +gboolean btd_device_set_battery_opt(struct device_battery *batt,
>> + BatteryOption opt1, ...)
>> +{
>> + va_list args;
>> + BatteryOption opt = opt1;
>> + int level;
>> +
>> + if (!batt)
>> + return FALSE;
>> +
>> + va_start(args, opt1);
>> +
>> + while (opt != BATTERY_OPT_INVALID) {
>> + switch (opt) {
>> + case BATTERY_OPT_LEVEL:
>> + level = va_arg(args, int);
>> + if (level != batt->level) {
>> + batt->level = level;
>> + emit_property_changed(batt->conn, batt->path,
>> + BATTERY_INTERFACE, "Level",
>> + DBUS_TYPE_UINT16, &level);
>> + }
>> + break;
>> + case BATTERY_OPT_REFRESH_FUNC:
>> + batt->refresh_func = va_arg(args, RefreshBattFunc);
>> + break;
>> + default:
>> + error("Unknown option %d", opt);
>> + return FALSE;
>> + }
>> +
>> + opt = va_arg(args, int);
>> + }
>> +
>> + va_end(args);
>> +
>> + return TRUE;
>> +
>> +}
>
> Please remove this empty line also.
>
Thanks, missed that as well.


>> diff --git a/src/device.h b/src/device.h
>> index f1d95c6..66eb5e8 100644
>> --- a/src/device.h
>> +++ b/src/device.h
>> @@ -23,8 +23,10 @@
>> */
>>
>> #define DEVICE_INTERFACE "org.bluez.Device"
>> +#define BATTERY_INTERFACE "org.bluez.Battery"
>>
>> struct btd_device;
>> +struct device_battery;
>
> Since this symbol is being exported, shouldn't it be prefixed with btd_ as well?
>
I will rename it to struct btd_battery. Is that ok ?


>>
>> typedef enum {
>> AUTH_TYPE_PINCODE,
>> @@ -54,6 +56,14 @@ struct btd_profile {
>> void (*adapter_remove) (struct btd_adapter *adapter);
>> };
>>
>> +typedef void (*RefreshBattFunc) (struct device_battery *batt);
>
> I don't think we use CamelCase for anything other than D-Bus method names.
>
We use it in gattrib.h for function pointers. What is the correct
convention for function pointers ? same as in adapter_ops ?

>> +
>> +typedef enum {
>> + BATTERY_OPT_INVALID = 0,
>> + BATTERY_OPT_LEVEL,
>> + BATTERY_OPT_REFRESH_FUNC,
>> +} BatteryOption;
>
> Fix CamelCase usage here and on uses of this type as well.
>
btio.h also uses this convention. What should be the correct convention ?



>> +
>> void btd_profile_foreach(void (*func)(struct btd_profile *p, void *data),
>> void *data);
>>
>> @@ -158,3 +168,8 @@ int device_unblock(DBusConnection *conn, struct btd_device *device,
>> void device_set_pnpid(struct btd_device *device, uint8_t vendor_id_src,
>> uint16_t vendor_id, uint16_t product_id,
>> uint16_t product_ver);
>> +
>> +struct device_battery *btd_device_add_battery(struct btd_device *device);
>> +void btd_device_remove_battery(struct device_battery *batt);
>> +gboolean btd_device_set_battery_opt(struct device_battery *batt,
>> + BatteryOption opt1, ...);
>> diff --git a/test/test-device b/test/test-device
>> index 63a96d3..7edb7b8 100755
>> --- a/test/test-device
>> +++ b/test/test-device
>> @@ -37,6 +37,7 @@ if (len(args) < 1):
>> print("")
>> print(" list")
>> print(" services <address>")
>> + print(" batteries <address>")
>> print(" create <address>")
>> print(" remove <address|path>")
>> print(" disconnect <address>")
>> @@ -205,5 +206,17 @@ if (args[0] == "services"):
>> print(path)
>> sys.exit(0)
>>
>> +if (args[0] == "batteries"):
>> + if (len(args) < 2):
>> + print("Need address parameter")
>> + else:
>> + path = adapter.FindDevice(args[1])
>> + device = dbus.Interface(bus.get_object("org.bluez", path),
>> + "org.bluez.Device")
>> + properties = device.GetProperties()
>> + for path in properties["Batteries"]:
>> + print(path)
>> + sys.exit(0)
>> +
>> print("Unknown command")
>> sys.exit(1)
>> --
>> 1.7.9.5
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>

Thanks,


--
BR,
Chen Ganir


2012-09-12 04:45:16

by Ganir, Chen

[permalink] [raw]
Subject: Re: [PATCH 01/10] battery: Add generic device battery documentation

Joao,

On 09/11/2012 06:34 PM, Joao Paulo Rechi Vita wrote:
> Hello Chen,
>
> On Tue, Sep 11, 2012 at 4:38 AM, <[email protected]> wrote:
>> From: Chen Ganir <[email protected]>
>>
>> Add documentation for the generic battery D-Bus interface.
>> ---
>> doc/battery-api.txt | 33 +++++++++++++++++++++++++++++++++
>> doc/device-api.txt | 5 +++++
>> 2 files changed, 38 insertions(+)
>> create mode 100644 doc/battery-api.txt
>>
>> diff --git a/doc/battery-api.txt b/doc/battery-api.txt
>> new file mode 100644
>> index 0000000..d5ef5ed
>> --- /dev/null
>> +++ b/doc/battery-api.txt
>> @@ -0,0 +1,33 @@
>> +BlueZ D-Bus Battery API description
>> +****************************************
>> +
>> + Texas Instruments, Inc. <[email protected]>
>> +
>> +Device Battery hierarchy
>> +=====================================
>> +
>> +Service org.bluez
>> +Interface org.bluez.Battery
>> +Object path [variable prefix]/{hci0,..}/dev_XX_XX_XX_XX_XX_XX/BATTYYYY
>> +YYYY is numeric value between 0 and 9999.
>> +
>> +Methods dict GetProperties()
>> +
>> + Returns all properties for the interface. See the
>> + Properties section for the available properties.
>> +
>> + void Refresh()
>> +
>> + Refresh the batterty level. If the battery level changed, the
>> + PropertyChanged signal will be sent with the new value.
>> +
>> +Signals PropertyChanged(string name, variant value)
>> +
>> + This signal indicates a changed value of the given
>> + property.
>> +
>> +Properties uint16 Level [readonly]
>> +
>> + Battery level (0-100).
>> +
>> +
>
> Can you please remove these two blank lines on the end of the file?
>
Ok.

>> diff --git a/doc/device-api.txt b/doc/device-api.txt
>> index 1f0dc96..c98d539 100644
>> --- a/doc/device-api.txt
>> +++ b/doc/device-api.txt
>> @@ -179,3 +179,8 @@ Properties string Address [readonly]
>> Note that this property can exhibit false-positives
>> in the case of Bluetooth 2.1 (or newer) devices that
>> have disabled Extended Inquiry Response support.
>> +
>> + array{object} Batteries [readonly]
>> +
>> + List of device battery object paths that represents the available
>> + batteries on the remote device.
>> --
>> 1.7.9.5
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>

Thanks for the review.

--
BR,
Chen Ganir


2012-09-11 22:08:47

by Joao Paulo Rechi Vita

[permalink] [raw]
Subject: Re: [PATCH 09/10] battery: Add support for notifications

On Tue, Sep 11, 2012 at 4:38 AM, <[email protected]> wrote:
> From: Chen Ganir <[email protected]>
>
> Add support for emitting PropertyChanged when a battery level
> characteristic notification is sent from the peer device.
> ---
> profiles/battery/battery.c | 95 +++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 93 insertions(+), 2 deletions(-)
>
> diff --git a/profiles/battery/battery.c b/profiles/battery/battery.c
> index a93b352..95f548e 100644
> --- a/profiles/battery/battery.c
> +++ b/profiles/battery/battery.c
> @@ -43,6 +43,7 @@ struct battery {
> GAttrib *attrib; /* GATT connection */
> guint attioid; /* Att watcher id */
> struct att_range *svc_range; /* Battery range */
> + guint attnotid; /* Att notifications id */
> GSList *chars; /* Characteristics */
> };
>
> @@ -56,6 +57,7 @@ struct characteristic {
> uint8_t ns; /* Battery Namespace */
> uint16_t description; /* Battery description */
> uint8_t level;
> + gboolean canNotify;
> };
>
> struct descriptor {
> @@ -86,6 +88,14 @@ static void char_free(gpointer user_data)
> g_free(c);
> }
>
> +static gint cmp_char_val_handle(gconstpointer a, gconstpointer b)
> +{
> + const struct characteristic *ch = a;
> + const uint16_t *handle = b;
> +
> + return ch->attr.value_handle - *handle;
> +}
> +
> static void battery_free(gpointer user_data)
> {
> struct battery *batt = user_data;
> @@ -99,6 +109,10 @@ static void battery_free(gpointer user_data)
> if (batt->attrib != NULL)
> g_attrib_unref(batt->attrib);
>
> + if (batt->attrib != NULL) {
> + g_attrib_unregister(batt->attrib, batt->attnotid);
> + g_attrib_unref(batt->attrib);
> + }

You're using the same condition twice here (batt->attrib != NULL).
Looks like a copy & paste error. Make sure you check for attnotid
before checking attrib, and that you don't unref attrib twice.

> btd_device_unref(batt->dev);
> g_free(batt->svc_range);
> g_free(batt);
> @@ -140,6 +154,18 @@ static void process_batteryservice_char(struct characteristic *ch)
> }
> }
>
> +static void batterylevel_enable_notify_cb(guint8 status, const guint8 *pdu,
> + guint16 len, gpointer user_data)
> +{
> + struct characteristic *ch = (struct characteristic *)user_data;
> +
> + if (status != 0) {
> + error("Could not enable batt level notification.");
> + ch->canNotify = FALSE;
> + process_batteryservice_char(ch);

I don't think is necessary to call process_batteryservice_char() here,
since you already called it during the characteristics discovery.

> + }
> +}
> +
> static gint device_battery_cmp(gconstpointer a, gconstpointer b)
> {
> const struct characteristic *ch = a;
> @@ -178,6 +204,19 @@ static void batterylevel_refresh_cb(struct device_battery *batt)
> process_batteryservice_char(ch);
> }
>
> +static void enable_battery_notification(struct characteristic *ch,
> + uint16_t handle)
> +{
> + uint8_t atval[2];
> + uint16_t val;
> +
> + val = GATT_CLIENT_CHARAC_CFG_NOTIF_BIT;
> +
> + att_put_u16(val, atval);
> + gatt_write_char(ch->batt->attrib, handle, atval, 2,
> + batterylevel_enable_notify_cb, ch);
> +}
> +
> static void batterylevel_presentation_format_desc_cb(guint8 status,
> const guint8 *pdu, guint16 len,
> gpointer user_data)
> @@ -207,13 +246,20 @@ static void batterylevel_presentation_format_desc_cb(guint8 status,
> desc->ch->description = att_get_u16(&value[5]);
> }
>
> -
> static void process_batterylevel_desc(struct descriptor *desc)
> {
> struct characteristic *ch = desc->ch;
> char uuidstr[MAX_LEN_UUID_STR];
> bt_uuid_t btuuid;
>
> + bt_uuid16_create(&btuuid, GATT_CLIENT_CHARAC_CFG_UUID);
> +
> + if (bt_uuid_cmp(&desc->uuid, &btuuid) == 0 && g_strcmp0(ch->attr.uuid,
> + BATTERY_LEVEL_UUID) == 0) {
> + enable_battery_notification(ch, desc->handle);
> + return;
> + }
> +
> bt_uuid16_create(&btuuid, GATT_CHARAC_FMT_UUID);
>
> if (bt_uuid_cmp(&desc->uuid, &btuuid) == 0) {
> @@ -319,12 +365,54 @@ static void configure_battery_cb(GSList *characteristics, guint8 status,
> }
> }
>
> +static void proc_batterylevel(struct characteristic *c, const uint8_t *pdu,
> + uint16_t len, gboolean final)
> +{
> + if (!pdu) {
> + error("Battery level notification: Invalid pdu length");
> + return;
> + }
> +
> + c->level = pdu[1];
> +
> + btd_device_set_battery_opt(c->devbatt, BATTERY_OPT_LEVEL, c->level,
> + BATTERY_OPT_INVALID);
> +}
> +
> +static void notif_handler(const uint8_t *pdu, uint16_t len, gpointer user_data)
> +{
> + struct battery *batt = user_data;
> + struct characteristic *ch;
> + uint16_t handle;
> + GSList *l;
> +
> + if (len < 3) {
> + error("notif_handler: Bad pdu received");
> + return;
> + }
> +
> + handle = att_get_u16(&pdu[1]);
> + l = g_slist_find_custom(batt->chars, &handle, cmp_char_val_handle);
> + if (l == NULL) {
> + error("notif_handler: Unexpected handle 0x%04x", handle);
> + return;
> + }
> +
> + ch = l->data;
> + if (g_strcmp0(ch->attr.uuid, BATTERY_LEVEL_UUID) == 0) {
> + proc_batterylevel(ch, pdu, len, FALSE);
> + }
> +}
> +
> static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
> {
> struct battery *batt = user_data;
>
> batt->attrib = g_attrib_ref(attrib);
>
> + batt->attnotid = g_attrib_register(batt->attrib, ATT_OP_HANDLE_NOTIFY,
> + notif_handler, batt, NULL);
> +
> if (batt->chars == NULL) {
> gatt_discover_char(batt->attrib, batt->svc_range->start,
> batt->svc_range->end, NULL,
> @@ -333,7 +421,8 @@ static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
> GSList *l;
> for (l = batt->chars; l; l = l->next) {
> struct characteristic *c = l->data;
> - process_batteryservice_char(c);
> + if (!c->canNotify)
> + process_batteryservice_char(c);
> }
> }
> }
> @@ -342,6 +431,8 @@ static void attio_disconnected_cb(gpointer user_data)
> {
> struct battery *batt = user_data;
>
> + g_attrib_unregister(batt->attrib, batt->attnotid);
> + batt->attnotid = 0;
> g_attrib_unref(batt->attrib);
> batt->attrib = NULL;
> }
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html



--
João Paulo Rechi Vita
Openbossa Labs - INdT

2012-09-11 21:50:10

by Joao Paulo Rechi Vita

[permalink] [raw]
Subject: Re: [PATCH 08/10] battery: Read Battery level characteristic

On Tue, Sep 11, 2012 at 4:38 AM, <[email protected]> wrote:
> From: Chen Ganir <[email protected]>
>
> Implement support for reading the battery level characteristic on
> connection establishment.
> ---
> profiles/battery/battery.c | 87 +++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 85 insertions(+), 2 deletions(-)
>
> diff --git a/profiles/battery/battery.c b/profiles/battery/battery.c
> index 31f2297..a93b352 100644
> --- a/profiles/battery/battery.c
> +++ b/profiles/battery/battery.c
> @@ -55,6 +55,7 @@ struct characteristic {
> GSList *desc; /* Descriptors */
> uint8_t ns; /* Battery Namespace */
> uint16_t description; /* Battery description */
> + uint8_t level;

Alignment problem here.

> };
>
> struct descriptor {
> @@ -103,6 +104,80 @@ static void battery_free(gpointer user_data)
> g_free(batt);
> }
>
> +static void read_batterylevel_cb(guint8 status, const guint8 *pdu, guint16 len,
> + gpointer user_data)
> +{
> + struct characteristic *ch = user_data;
> + uint8_t value[ATT_MAX_MTU];
> + int vlen;
> +
> + if (status != 0) {
> + error("Failed to read Battery Level:%s", att_ecode2str(status));
> + return;
> + }
> +
> + vlen = dec_read_resp(pdu, len, value, sizeof(value));
> + if (!vlen) {
> + error("Failed to read Battery Level: Protocol error\n");
> + return;
> + }
> +
> + if (vlen < 1) {
> + error("Failed to read Battery Level: Wrong pdu len");
> + return;
> + }
> +
> + ch->level = value[0];
> + btd_device_set_battery_opt(ch->devbatt, BATTERY_OPT_LEVEL, ch->level,
> + BATTERY_OPT_INVALID);
> +}
> +
> +static void process_batteryservice_char(struct characteristic *ch)
> +{
> + if (g_strcmp0(ch->attr.uuid, BATTERY_LEVEL_UUID) == 0) {
> + gatt_read_char(ch->batt->attrib, ch->attr.value_handle, 0,
> + read_batterylevel_cb, ch);
> + }
> +}
> +
> +static gint device_battery_cmp(gconstpointer a, gconstpointer b)
> +{
> + const struct characteristic *ch = a;
> + const struct device_battery *batt = b;
> +
> + if (batt == ch->devbatt)
> + return 0;
> +
> + return -1;
> +}
> +
> +static struct characteristic *find_battery_char(struct device_battery *db)
> +{
> + GSList *l, *b;
> +
> + for (l = servers; l != NULL; l = g_slist_next(l)) {
> + struct battery *batt = l->data;
> +
> + b = g_slist_find_custom(batt->chars, db, device_battery_cmp);
> + if (!b)
> + return NULL;
> +
> + return b->data;
> + }
> +
> + return NULL;
> +}
> +
> +static void batterylevel_refresh_cb(struct device_battery *batt)
> +{
> + struct characteristic *ch;
> +
> + ch = find_battery_char(batt);
> +
> + if (ch)
> + process_batteryservice_char(ch);
> +}
> +
> static void batterylevel_presentation_format_desc_cb(guint8 status,
> const guint8 *pdu, guint16 len,
> gpointer user_data)
> @@ -151,7 +226,6 @@ static void process_batterylevel_desc(struct descriptor *desc)
> DBG("Ignored descriptor %s characteristic %s", uuidstr, ch->attr.uuid);
> }
>
> -
> static void discover_desc_cb(guint8 status, const guint8 *pdu, guint16 len,
> gpointer user_data)
> {
> @@ -192,8 +266,11 @@ static void discover_desc_cb(guint8 status, const guint8 *pdu, guint16 len,
>
> update_char:
> ch->devbatt = btd_device_add_battery(ch->batt->dev);
> -}
> + btd_device_set_battery_opt(ch->devbatt, BATTERY_OPT_REFRESH_FUNC,
> + batterylevel_refresh_cb, BATTERY_OPT_INVALID);
>
> + process_batteryservice_char(ch);

There two call also should be part of configure_battery_cb() instead
of discover_desc_cb().

> +}
>
> static void configure_battery_cb(GSList *characteristics, guint8 status,
>
> @@ -252,6 +329,12 @@ static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
> gatt_discover_char(batt->attrib, batt->svc_range->start,
> batt->svc_range->end, NULL,
> configure_battery_cb, batt);
> + } else {
> + GSList *l;
> + for (l = batt->chars; l; l = l->next) {
> + struct characteristic *c = l->data;
> + process_batteryservice_char(c);
> + }
> }
> }
>
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html



--
João Paulo Rechi Vita
Openbossa Labs - INdT

2012-09-11 21:40:39

by Joao Paulo Rechi Vita

[permalink] [raw]
Subject: Re: [PATCH 07/10] battery: Add Battery to device

On Tue, Sep 11, 2012 at 4:38 AM, <[email protected]> wrote:
> From: Chen Ganir <[email protected]>
>
> Add/Remove battery from device
> ---
> profiles/battery/battery.c | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/profiles/battery/battery.c b/profiles/battery/battery.c
> index d1e0b6e..31f2297 100644
> --- a/profiles/battery/battery.c
> +++ b/profiles/battery/battery.c
> @@ -49,8 +49,9 @@ struct battery {
> static GSList *servers;
>
> struct characteristic {
> - struct gatt_char attr; /* Characteristic */
> - struct battery *batt; /* Parent Battery Service */
> + struct device_battery *devbatt; /* device_battery pointer */
> + struct gatt_char attr; /* Characteristic */
> + struct battery *batt; /* Parent Battery Service */

Just a minor pick here: the comment alignment of attr and batt should
have been fixed on the previous commit, to keep it consistent along
the series.

> GSList *desc; /* Descriptors */
> uint8_t ns; /* Battery Namespace */
> uint16_t description; /* Battery description */
> @@ -79,6 +80,8 @@ static void char_free(gpointer user_data)
>
> g_slist_free_full(c->desc, g_free);
>
> + btd_device_remove_battery(c->devbatt);
> +
> g_free(c);
> }
>
> @@ -160,12 +163,12 @@ static void discover_desc_cb(guint8 status, const guint8 *pdu, guint16 len,
> if (status != 0) {
> error("Discover all characteristic descriptors failed [%s]: %s",
> ch->attr.uuid, att_ecode2str(status));
> - return;
> + goto update_char;
> }
>
> list = dec_find_info_resp(pdu, len, &format);
> if (list == NULL)
> - return;
> + goto update_char;
>
> for (i = 0; i < list->num; i++) {
> struct descriptor *desc;
> @@ -186,6 +189,9 @@ static void discover_desc_cb(guint8 status, const guint8 *pdu, guint16 len,
> }
>
> att_data_list_free(list);
> +
> +update_char:
> + ch->devbatt = btd_device_add_battery(ch->batt->dev);

If I understood correctly, adding a battery to the device is a
consequence of finding the "Battery Level" characteristic, and not
related to the descriptors discovery. So the call to
btd_device_add_battery() should be done inside the
configure_battery_cb() instead of the discover_desc_cb(). This way
you'll not need all these goto's.

> }
>
>
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html



--
João Paulo Rechi Vita
Openbossa Labs - INdT

2012-09-11 20:52:13

by Joao Paulo Rechi Vita

[permalink] [raw]
Subject: Re: [PATCH 05/10] battery: Discover Characteristic Descriptors

On Tue, Sep 11, 2012 at 4:38 AM, <[email protected]> wrote:
> From: Chen Ganir <[email protected]>
>
> Discover all characteristic descriptors, and build a descriptor
> list. Presentation Format Descriptor and Client Characteristic
> Configuration descriptors are searched.
> ---
> profiles/battery/battery.c | 73 +++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 72 insertions(+), 1 deletion(-)
>
> diff --git a/profiles/battery/battery.c b/profiles/battery/battery.c
> index 37cbce4..3a24a7b 100644
> --- a/profiles/battery/battery.c
> +++ b/profiles/battery/battery.c
> @@ -51,6 +51,13 @@ static GSList *servers;
> struct characteristic {
> struct gatt_char attr; /* Characteristic */
> struct battery *batt; /* Parent Battery Service */
> + GSList *desc; /* Descriptors */

Wrong alignment here.

> +};
> +
> +struct descriptor {
> + struct characteristic *ch; /* Parent Characteristic */
> + uint16_t handle; /* Descriptor Handle */
> + bt_uuid_t uuid; /* UUID */
> };
>
> static gint cmp_device(gconstpointer a, gconstpointer b)
> @@ -64,12 +71,21 @@ static gint cmp_device(gconstpointer a, gconstpointer b)
> return -1;
> }
>
> +static void char_free(gpointer user_data)
> +{
> + struct characteristic *c = user_data;
> +
> + g_slist_free_full(c->desc, g_free);
> +
> + g_free(c);
> +}
> +
> static void battery_free(gpointer user_data)
> {
> struct battery *batt = user_data;
>
> if (batt->chars != NULL)
> - g_slist_free_full(batt->chars, g_free);
> + g_slist_free_full(batt->chars, char_free);
>
> if (batt->attioid > 0)
> btd_device_remove_attio_callback(batt->dev, batt->attioid);
> @@ -82,7 +98,47 @@ static void battery_free(gpointer user_data)
> g_free(batt);
> }
>
> +static void discover_desc_cb(guint8 status, const guint8 *pdu, guint16 len,
> + gpointer user_data)
> +{
> + struct characteristic *ch = user_data;
> + struct att_data_list *list;
> + uint8_t format;
> + int i;
> +
> + if (status != 0) {
> + error("Discover all characteristic descriptors failed [%s]: %s",
> + ch->attr.uuid, att_ecode2str(status));
> + return;
> + }
> +
> + list = dec_find_info_resp(pdu, len, &format);
> + if (list == NULL)
> + return;
> +
> + for (i = 0; i < list->num; i++) {
> + struct descriptor *desc;
> + uint8_t *value;
> +
> + value = list->data[i];
> + desc = g_new0(struct descriptor, 1);
> + desc->handle = att_get_u16(value);
> + desc->ch = ch;
> +
> + if (format == 0x01)
> + desc->uuid = att_get_uuid16(&value[2]);
> + else
> + desc->uuid = att_get_uuid128(&value[2]);
> +
> + ch->desc = g_slist_append(ch->desc, desc);
> + }
> +
> + att_data_list_free(list);
> +}
> +
> +

Remove extra blank line here.

> static void configure_battery_cb(GSList *characteristics, guint8 status,
> +

Remove extra blank line here.

> gpointer user_data)
> {
> struct battery *batt = user_data;
> @@ -97,6 +153,7 @@ static void configure_battery_cb(GSList *characteristics, guint8 status,
> for (l = characteristics; l; l = l->next) {
> struct gatt_char *c = l->data;
> struct characteristic *ch;
> + uint16_t start, end;
>
> ch = g_new0(struct characteristic, 1);
> ch->attr.handle = c->handle;
> @@ -106,6 +163,20 @@ static void configure_battery_cb(GSList *characteristics, guint8 status,
> ch->batt = batt;
>
> batt->chars = g_slist_append(batt->chars, ch);
> +
> + start = c->value_handle + 1;
> +
> + if (l->next != NULL) {
> + struct gatt_char *c = l->next->data;
> + if (start == c->handle)
> + continue;
> + end = c->handle - 1;
> + } else if (c->value_handle != batt->svc_range->end)
> + end = batt->svc_range->end;
> + else
> + continue;
> +
> + gatt_find_info(batt->attrib, start, end, discover_desc_cb, ch);
> }
> }
>
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html



--
João Paulo Rechi Vita
Openbossa Labs - INdT

2012-09-11 18:27:01

by Joao Paulo Rechi Vita

[permalink] [raw]
Subject: Re: [PATCH 02/10] battery: Implement Generic device battery

On Tue, Sep 11, 2012 at 4:38 AM, <[email protected]> wrote:
> From: Chen Ganir <[email protected]>
>
> Add implementation for the generic battery in bluetooth device.
> This patch adds new D-Bus interface for adding/removing/changing
> peer device battery status, allowing management of remote device
> batteries.
> ---
> src/device.c | 184 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> src/device.h | 15 +++++
> test/test-device | 13 ++++
> 3 files changed, 212 insertions(+)
>
> diff --git a/src/device.c b/src/device.c
> index 02ef35e..ce4d467 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -166,6 +166,7 @@ struct btd_device {
>
> gboolean authorizing;
> gint ref;
> + GSList *batteries;
>
> GIOChannel *att_io;
> guint cleanup_id;
> @@ -180,6 +181,26 @@ static uint16_t uuid_list[] = {
> 0
> };
>
> +struct device_battery {
> + DBusConnection *conn;
> + uint16_t level;
> + char *path;
> + struct btd_device *device;
> + RefreshBattFunc refresh_func;
> +};
> +
> +static void battery_free(gpointer user_data)
> +{
> + struct device_battery *b = user_data;
> +
> + g_dbus_unregister_interface(b->conn, b->path, BATTERY_INTERFACE);
> + dbus_connection_unref(b->conn);
> + btd_device_unref(b->device);
> + g_free(b->path);
> + g_free(b);
> +
> +}
> +
> static void browse_request_free(struct browse_req *req)
> {
> if (req->listener_id)
> @@ -259,6 +280,7 @@ static void device_free(gpointer user_data)
> g_slist_free_full(device->primaries, g_free);
> g_slist_free_full(device->attios, g_free);
> g_slist_free_full(device->attios_offline, g_free);
> + g_slist_free_full(device->batteries, battery_free);
>
> attio_cleanup(device);
>
> @@ -433,6 +455,15 @@ static DBusMessage *get_properties(DBusConnection *conn,
> ptr = adapter_get_path(adapter);
> dict_append_entry(&dict, "Adapter", DBUS_TYPE_OBJECT_PATH, &ptr);
>
> + /* Batteries */
> + str = g_new0(char *, g_slist_length(device->batteries) + 1);
> + for (i = 0, l = device->batteries; l; l = l->next, i++) {
> + struct device_battery *b = l->data;
> + str[i] = b->path;
> + }
> + dict_append_array(&dict, "Batteries", DBUS_TYPE_OBJECT_PATH, &str, i);
> + g_free(str);
> +
> dbus_message_iter_close_container(&iter, &dict);
>
> return reply;
> @@ -3263,3 +3294,156 @@ void btd_profile_disconnected(struct btd_profile *profile,
> {
> device->conns = g_slist_remove(device->conns, profile);
> }
> +
> +static void batteries_changed(struct btd_device *device)
> +{
> + DBusConnection *conn = get_dbus_connection();
> + char **batteries;
> + GSList *l;
> + int i;
> +
> + batteries = g_new0(char *, g_slist_length(device->batteries) + 1);
> + for (i = 0, l = device->batteries; l; l = l->next, i++) {
> + struct device_battery *batt = l->data;
> + batteries[i] = batt->path;
> + }
> +
> + emit_array_property_changed(conn, device->path, DEVICE_INTERFACE,
> + "Batteries", DBUS_TYPE_OBJECT_PATH,
> + &batteries, i);
> +
> + g_free(batteries);
> +}
> +
> +static DBusMessage *refresh_batt_level(DBusConnection *conn,
> + DBusMessage *msg, void *data)
> +{
> + struct device_battery *b = data;
> +
> + if (!b->refresh_func)
> + return btd_error_not_supported(msg);
> +
> + b->refresh_func(b);
> +
> + return dbus_message_new_method_return(msg);
> +}
> +
> +static DBusMessage *get_batt_properties(DBusConnection *conn,
> + DBusMessage *msg, void *data)
> +{
> + struct device_battery *b = data;
> + DBusMessageIter iter;
> + DBusMessageIter dict;
> + DBusMessage *reply;
> +
> + reply = dbus_message_new_method_return(msg);
> + if (reply == NULL)
> + return NULL;
> +
> + dbus_message_iter_init_append(reply, &iter);
> +
> + dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
> + DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
> + DBUS_TYPE_STRING_AS_STRING DBUS_TYPE_VARIANT_AS_STRING
> + DBUS_DICT_ENTRY_END_CHAR_AS_STRING, &dict);
> +
> + dict_append_entry(&dict, "Level", DBUS_TYPE_UINT16, &b->level);
> +
> + dbus_message_iter_close_container(&iter, &dict);
> +
> + return reply;
> +}
> +
> +static GDBusMethodTable battery_methods[] = {
> + { GDBUS_METHOD("GetProperties",
> + NULL, GDBUS_ARGS({ "properties", "a{sv}" }),
> + get_batt_properties) },
> + { GDBUS_METHOD("Refresh",
> + NULL, NULL,
> + refresh_batt_level) },
> + { }
> +};
> +
> +static GDBusSignalTable battery_signals[] = {
> + { GDBUS_SIGNAL("PropertyChanged",
> + GDBUS_ARGS({ "name", "s" }, { "value", "v" })) },
> + { }
> +};
> +
> +struct device_battery *btd_device_add_battery(struct btd_device *device)
> +{
> + struct device_battery *batt;
> + DBusConnection *conn = get_dbus_connection();
> +
> + batt = g_new0(struct device_battery, 1);
> + batt->path = g_strdup_printf("%s/BATT%04X", device->path,
> + g_slist_length(device->batteries));
> + batt->conn = dbus_connection_ref(conn);
> +
> + if (!g_dbus_register_interface(batt->conn, batt->path,
> + BATTERY_INTERFACE, battery_methods, battery_signals,
> + NULL, batt, NULL)) {
> + error("D-Bus register interface %s failed", BATTERY_INTERFACE);
> + dbus_connection_unref(batt->conn);
> + g_free(batt->path);
> + g_free(batt);
> + return NULL;
> + }
> +
> + batt->device = btd_device_ref(device);
> + device->batteries = g_slist_append(device->batteries, batt);
> + batteries_changed(device);
> +
> + return batt;
> +}
> +
> +void btd_device_remove_battery(struct device_battery *batt)
> +{
> + struct btd_device *dev = batt->device;
> +
> + dev->batteries = g_slist_remove(dev->batteries, batt);
> +
> + battery_free(batt);
> +
> + batteries_changed(dev);
> +}
> +
> +gboolean btd_device_set_battery_opt(struct device_battery *batt,
> + BatteryOption opt1, ...)
> +{
> + va_list args;
> + BatteryOption opt = opt1;
> + int level;
> +
> + if (!batt)
> + return FALSE;
> +
> + va_start(args, opt1);
> +
> + while (opt != BATTERY_OPT_INVALID) {
> + switch (opt) {
> + case BATTERY_OPT_LEVEL:
> + level = va_arg(args, int);
> + if (level != batt->level) {
> + batt->level = level;
> + emit_property_changed(batt->conn, batt->path,
> + BATTERY_INTERFACE, "Level",
> + DBUS_TYPE_UINT16, &level);
> + }
> + break;
> + case BATTERY_OPT_REFRESH_FUNC:
> + batt->refresh_func = va_arg(args, RefreshBattFunc);
> + break;
> + default:
> + error("Unknown option %d", opt);
> + return FALSE;
> + }
> +
> + opt = va_arg(args, int);
> + }
> +
> + va_end(args);
> +
> + return TRUE;
> +
> +}

Please remove this empty line also.

> diff --git a/src/device.h b/src/device.h
> index f1d95c6..66eb5e8 100644
> --- a/src/device.h
> +++ b/src/device.h
> @@ -23,8 +23,10 @@
> */
>
> #define DEVICE_INTERFACE "org.bluez.Device"
> +#define BATTERY_INTERFACE "org.bluez.Battery"
>
> struct btd_device;
> +struct device_battery;

Since this symbol is being exported, shouldn't it be prefixed with btd_ as well?

>
> typedef enum {
> AUTH_TYPE_PINCODE,
> @@ -54,6 +56,14 @@ struct btd_profile {
> void (*adapter_remove) (struct btd_adapter *adapter);
> };
>
> +typedef void (*RefreshBattFunc) (struct device_battery *batt);

I don't think we use CamelCase for anything other than D-Bus method names.

> +
> +typedef enum {
> + BATTERY_OPT_INVALID = 0,
> + BATTERY_OPT_LEVEL,
> + BATTERY_OPT_REFRESH_FUNC,
> +} BatteryOption;

Fix CamelCase usage here and on uses of this type as well.

> +
> void btd_profile_foreach(void (*func)(struct btd_profile *p, void *data),
> void *data);
>
> @@ -158,3 +168,8 @@ int device_unblock(DBusConnection *conn, struct btd_device *device,
> void device_set_pnpid(struct btd_device *device, uint8_t vendor_id_src,
> uint16_t vendor_id, uint16_t product_id,
> uint16_t product_ver);
> +
> +struct device_battery *btd_device_add_battery(struct btd_device *device);
> +void btd_device_remove_battery(struct device_battery *batt);
> +gboolean btd_device_set_battery_opt(struct device_battery *batt,
> + BatteryOption opt1, ...);
> diff --git a/test/test-device b/test/test-device
> index 63a96d3..7edb7b8 100755
> --- a/test/test-device
> +++ b/test/test-device
> @@ -37,6 +37,7 @@ if (len(args) < 1):
> print("")
> print(" list")
> print(" services <address>")
> + print(" batteries <address>")
> print(" create <address>")
> print(" remove <address|path>")
> print(" disconnect <address>")
> @@ -205,5 +206,17 @@ if (args[0] == "services"):
> print(path)
> sys.exit(0)
>
> +if (args[0] == "batteries"):
> + if (len(args) < 2):
> + print("Need address parameter")
> + else:
> + path = adapter.FindDevice(args[1])
> + device = dbus.Interface(bus.get_object("org.bluez", path),
> + "org.bluez.Device")
> + properties = device.GetProperties()
> + for path in properties["Batteries"]:
> + print(path)
> + sys.exit(0)
> +
> print("Unknown command")
> sys.exit(1)
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html



--
João Paulo Rechi Vita
Openbossa Labs - INdT

2012-09-11 15:34:24

by Joao Paulo Rechi Vita

[permalink] [raw]
Subject: Re: [PATCH 01/10] battery: Add generic device battery documentation

Hello Chen,

On Tue, Sep 11, 2012 at 4:38 AM, <[email protected]> wrote:
> From: Chen Ganir <[email protected]>
>
> Add documentation for the generic battery D-Bus interface.
> ---
> doc/battery-api.txt | 33 +++++++++++++++++++++++++++++++++
> doc/device-api.txt | 5 +++++
> 2 files changed, 38 insertions(+)
> create mode 100644 doc/battery-api.txt
>
> diff --git a/doc/battery-api.txt b/doc/battery-api.txt
> new file mode 100644
> index 0000000..d5ef5ed
> --- /dev/null
> +++ b/doc/battery-api.txt
> @@ -0,0 +1,33 @@
> +BlueZ D-Bus Battery API description
> +****************************************
> +
> + Texas Instruments, Inc. <[email protected]>
> +
> +Device Battery hierarchy
> +=====================================
> +
> +Service org.bluez
> +Interface org.bluez.Battery
> +Object path [variable prefix]/{hci0,..}/dev_XX_XX_XX_XX_XX_XX/BATTYYYY
> +YYYY is numeric value between 0 and 9999.
> +
> +Methods dict GetProperties()
> +
> + Returns all properties for the interface. See the
> + Properties section for the available properties.
> +
> + void Refresh()
> +
> + Refresh the batterty level. If the battery level changed, the
> + PropertyChanged signal will be sent with the new value.
> +
> +Signals PropertyChanged(string name, variant value)
> +
> + This signal indicates a changed value of the given
> + property.
> +
> +Properties uint16 Level [readonly]
> +
> + Battery level (0-100).
> +
> +

Can you please remove these two blank lines on the end of the file?

> diff --git a/doc/device-api.txt b/doc/device-api.txt
> index 1f0dc96..c98d539 100644
> --- a/doc/device-api.txt
> +++ b/doc/device-api.txt
> @@ -179,3 +179,8 @@ Properties string Address [readonly]
> Note that this property can exhibit false-positives
> in the case of Bluetooth 2.1 (or newer) devices that
> have disabled Extended Inquiry Response support.
> +
> + array{object} Batteries [readonly]
> +
> + List of device battery object paths that represents the available
> + batteries on the remote device.
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html



--
João Paulo Rechi Vita
Openbossa Labs - INdT

2012-09-11 07:38:14

by Ganir, Chen

[permalink] [raw]
Subject: [PATCH 03/10] battery: Add GATT Battery Client Service skeleton

From: Chen Ganir <[email protected]>

Add support for the Battery Service Gatt Client side. Implement
the basic skeleton.
---
Makefile.am | 9 ++++-
lib/uuid.h | 2 +
profiles/battery/battery.c | 89 ++++++++++++++++++++++++++++++++++++++++++++
profiles/battery/battery.h | 24 ++++++++++++
profiles/battery/main.c | 52 ++++++++++++++++++++++++++
profiles/battery/manager.c | 61 ++++++++++++++++++++++++++++++
profiles/battery/manager.h | 24 ++++++++++++
7 files changed, 259 insertions(+), 2 deletions(-)
create mode 100644 profiles/battery/battery.c
create mode 100644 profiles/battery/battery.h
create mode 100644 profiles/battery/main.c
create mode 100644 profiles/battery/manager.c
create mode 100644 profiles/battery/manager.h

diff --git a/Makefile.am b/Makefile.am
index 4977a05..af5d8a4 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -212,7 +212,7 @@ endif

if GATTMODULES
builtin_modules += thermometer alert time gatt_example proximity deviceinfo \
- gatt
+ gatt battery
builtin_sources += profiles/thermometer/main.c \
profiles/thermometer/manager.h \
profiles/thermometer/manager.c \
@@ -241,7 +241,12 @@ builtin_sources += profiles/thermometer/main.c \
profiles/deviceinfo/deviceinfo.c \
profiles/gatt/main.c profiles/gatt/manager.h \
profiles/gatt/manager.c profiles/gatt/gas.h \
- profiles/gatt/gas.c
+ profiles/gatt/gas.c \
+ profiles/battery/main.c \
+ profiles/battery/manager.c \
+ profiles/battery/manager.h \
+ profiles/battery/battery.c \
+ profiles/battery/battery.h
endif

builtin_modules += formfactor
diff --git a/lib/uuid.h b/lib/uuid.h
index aa6efdf..58ad0b3 100644
--- a/lib/uuid.h
+++ b/lib/uuid.h
@@ -56,6 +56,8 @@ extern "C" {
#define PNPID_UUID "00002a50-0000-1000-8000-00805f9b34fb"
#define DEVICE_INFORMATION_UUID "0000180a-0000-1000-8000-00805f9b34fb"

+#define BATTERY_SERVICE_UUID "0000180f-0000-1000-8000-00805f9b34fb"
+
#define GATT_UUID "00001801-0000-1000-8000-00805f9b34fb"
#define IMMEDIATE_ALERT_UUID "00001802-0000-1000-8000-00805f9b34fb"
#define LINK_LOSS_UUID "00001803-0000-1000-8000-00805f9b34fb"
diff --git a/profiles/battery/battery.c b/profiles/battery/battery.c
new file mode 100644
index 0000000..7702e39
--- /dev/null
+++ b/profiles/battery/battery.c
@@ -0,0 +1,89 @@
+/*
+ *
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2012 Texas Instruments, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <glib.h>
+#include <bluetooth/uuid.h>
+#include <stdbool.h>
+
+#include "adapter.h"
+#include "device.h"
+#include "att.h"
+#include "gattrib.h"
+#include "gatt.h"
+#include "battery.h"
+
+struct battery {
+ struct btd_device *dev; /* Device reference */
+};
+
+static GSList *servers;
+
+static gint cmp_device(gconstpointer a, gconstpointer b)
+{
+ const struct battery *batt = a;
+ const struct btd_device *dev = b;
+
+ if (dev == batt->dev)
+ return 0;
+
+ return -1;
+}
+
+static void battery_free(gpointer user_data)
+{
+ struct battery *batt = user_data;
+
+ btd_device_unref(batt->dev);
+ g_free(batt);
+}
+
+
+int battery_register(struct btd_device *device)
+{
+ struct battery *batt;
+
+ batt = g_new0(struct battery, 1);
+ batt->dev = btd_device_ref(device);
+
+ servers = g_slist_prepend(servers, batt);
+
+ return 0;
+}
+
+void battery_unregister(struct btd_device *device)
+{
+ struct battery *batt;
+ GSList *l;
+
+ l = g_slist_find_custom(servers, device, cmp_device);
+ if (l == NULL)
+ return;
+
+ batt = l->data;
+ servers = g_slist_remove(servers, batt);
+
+ battery_free(batt);
+}
diff --git a/profiles/battery/battery.h b/profiles/battery/battery.h
new file mode 100644
index 0000000..9933343
--- /dev/null
+++ b/profiles/battery/battery.h
@@ -0,0 +1,24 @@
+/*
+ *
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2012 Texas Instruments, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ */
+
+int battery_register(struct btd_device *device);
+void battery_unregister(struct btd_device *device);
diff --git a/profiles/battery/main.c b/profiles/battery/main.c
new file mode 100644
index 0000000..d4a23c9
--- /dev/null
+++ b/profiles/battery/main.c
@@ -0,0 +1,52 @@
+/*
+ *
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2012 Texas Instruments, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdint.h>
+#include <glib.h>
+#include <errno.h>
+
+#include "hcid.h"
+#include "plugin.h"
+#include "manager.h"
+#include "log.h"
+
+static int battery_init(void)
+{
+ if (!main_opts.gatt_enabled) {
+ error("GATT is disabled");
+ return -ENOTSUP;
+ }
+
+ return battery_manager_init();
+}
+
+static void battery_exit(void)
+{
+ battery_manager_exit();
+}
+
+BLUETOOTH_PLUGIN_DEFINE(battery, VERSION, BLUETOOTH_PLUGIN_PRIORITY_DEFAULT,
+ battery_init, battery_exit)
diff --git a/profiles/battery/manager.c b/profiles/battery/manager.c
new file mode 100644
index 0000000..72878b2
--- /dev/null
+++ b/profiles/battery/manager.c
@@ -0,0 +1,61 @@
+/*
+ *
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2012 Texas Instruments, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ */
+
+#include <glib.h>
+#include <errno.h>
+#include <bluetooth/uuid.h>
+#include <stdbool.h>
+
+#include "adapter.h"
+#include "device.h"
+#include "att.h"
+#include "gattrib.h"
+#include "gatt.h"
+#include "battery.h"
+#include "manager.h"
+
+static int battery_driver_probe(struct btd_device *device, GSList *uuids)
+{
+ return battery_register(device);
+}
+
+static void battery_driver_remove(struct btd_device *device)
+{
+ battery_unregister(device);
+}
+
+static struct btd_profile battery_profile = {
+ .name = "battery",
+ .remote_uuids = BTD_UUIDS(BATTERY_SERVICE_UUID),
+ .device_probe = battery_driver_probe,
+ .device_remove = battery_driver_remove
+};
+
+int battery_manager_init(void)
+{
+ return btd_profile_register(&battery_profile);
+}
+
+void battery_manager_exit(void)
+{
+ btd_profile_unregister(&battery_profile);
+}
diff --git a/profiles/battery/manager.h b/profiles/battery/manager.h
new file mode 100644
index 0000000..b2c849f
--- /dev/null
+++ b/profiles/battery/manager.h
@@ -0,0 +1,24 @@
+/*
+ *
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2012 Texas Instruments, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ */
+
+int battery_manager_init(void);
+void battery_manager_exit(void);
--
1.7.9.5


2012-09-11 07:38:18

by Ganir, Chen

[permalink] [raw]
Subject: [PATCH 07/10] battery: Add Battery to device

From: Chen Ganir <[email protected]>

Add/Remove battery from device
---
profiles/battery/battery.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/profiles/battery/battery.c b/profiles/battery/battery.c
index d1e0b6e..31f2297 100644
--- a/profiles/battery/battery.c
+++ b/profiles/battery/battery.c
@@ -49,8 +49,9 @@ struct battery {
static GSList *servers;

struct characteristic {
- struct gatt_char attr; /* Characteristic */
- struct battery *batt; /* Parent Battery Service */
+ struct device_battery *devbatt; /* device_battery pointer */
+ struct gatt_char attr; /* Characteristic */
+ struct battery *batt; /* Parent Battery Service */
GSList *desc; /* Descriptors */
uint8_t ns; /* Battery Namespace */
uint16_t description; /* Battery description */
@@ -79,6 +80,8 @@ static void char_free(gpointer user_data)

g_slist_free_full(c->desc, g_free);

+ btd_device_remove_battery(c->devbatt);
+
g_free(c);
}

@@ -160,12 +163,12 @@ static void discover_desc_cb(guint8 status, const guint8 *pdu, guint16 len,
if (status != 0) {
error("Discover all characteristic descriptors failed [%s]: %s",
ch->attr.uuid, att_ecode2str(status));
- return;
+ goto update_char;
}

list = dec_find_info_resp(pdu, len, &format);
if (list == NULL)
- return;
+ goto update_char;

for (i = 0; i < list->num; i++) {
struct descriptor *desc;
@@ -186,6 +189,9 @@ static void discover_desc_cb(guint8 status, const guint8 *pdu, guint16 len,
}

att_data_list_free(list);
+
+update_char:
+ ch->devbatt = btd_device_add_battery(ch->batt->dev);
}


--
1.7.9.5


2012-09-11 07:38:20

by Ganir, Chen

[permalink] [raw]
Subject: [PATCH 09/10] battery: Add support for notifications

From: Chen Ganir <[email protected]>

Add support for emitting PropertyChanged when a battery level
characteristic notification is sent from the peer device.
---
profiles/battery/battery.c | 95 +++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 93 insertions(+), 2 deletions(-)

diff --git a/profiles/battery/battery.c b/profiles/battery/battery.c
index a93b352..95f548e 100644
--- a/profiles/battery/battery.c
+++ b/profiles/battery/battery.c
@@ -43,6 +43,7 @@ struct battery {
GAttrib *attrib; /* GATT connection */
guint attioid; /* Att watcher id */
struct att_range *svc_range; /* Battery range */
+ guint attnotid; /* Att notifications id */
GSList *chars; /* Characteristics */
};

@@ -56,6 +57,7 @@ struct characteristic {
uint8_t ns; /* Battery Namespace */
uint16_t description; /* Battery description */
uint8_t level;
+ gboolean canNotify;
};

struct descriptor {
@@ -86,6 +88,14 @@ static void char_free(gpointer user_data)
g_free(c);
}

+static gint cmp_char_val_handle(gconstpointer a, gconstpointer b)
+{
+ const struct characteristic *ch = a;
+ const uint16_t *handle = b;
+
+ return ch->attr.value_handle - *handle;
+}
+
static void battery_free(gpointer user_data)
{
struct battery *batt = user_data;
@@ -99,6 +109,10 @@ static void battery_free(gpointer user_data)
if (batt->attrib != NULL)
g_attrib_unref(batt->attrib);

+ if (batt->attrib != NULL) {
+ g_attrib_unregister(batt->attrib, batt->attnotid);
+ g_attrib_unref(batt->attrib);
+ }
btd_device_unref(batt->dev);
g_free(batt->svc_range);
g_free(batt);
@@ -140,6 +154,18 @@ static void process_batteryservice_char(struct characteristic *ch)
}
}

+static void batterylevel_enable_notify_cb(guint8 status, const guint8 *pdu,
+ guint16 len, gpointer user_data)
+{
+ struct characteristic *ch = (struct characteristic *)user_data;
+
+ if (status != 0) {
+ error("Could not enable batt level notification.");
+ ch->canNotify = FALSE;
+ process_batteryservice_char(ch);
+ }
+}
+
static gint device_battery_cmp(gconstpointer a, gconstpointer b)
{
const struct characteristic *ch = a;
@@ -178,6 +204,19 @@ static void batterylevel_refresh_cb(struct device_battery *batt)
process_batteryservice_char(ch);
}

+static void enable_battery_notification(struct characteristic *ch,
+ uint16_t handle)
+{
+ uint8_t atval[2];
+ uint16_t val;
+
+ val = GATT_CLIENT_CHARAC_CFG_NOTIF_BIT;
+
+ att_put_u16(val, atval);
+ gatt_write_char(ch->batt->attrib, handle, atval, 2,
+ batterylevel_enable_notify_cb, ch);
+}
+
static void batterylevel_presentation_format_desc_cb(guint8 status,
const guint8 *pdu, guint16 len,
gpointer user_data)
@@ -207,13 +246,20 @@ static void batterylevel_presentation_format_desc_cb(guint8 status,
desc->ch->description = att_get_u16(&value[5]);
}

-
static void process_batterylevel_desc(struct descriptor *desc)
{
struct characteristic *ch = desc->ch;
char uuidstr[MAX_LEN_UUID_STR];
bt_uuid_t btuuid;

+ bt_uuid16_create(&btuuid, GATT_CLIENT_CHARAC_CFG_UUID);
+
+ if (bt_uuid_cmp(&desc->uuid, &btuuid) == 0 && g_strcmp0(ch->attr.uuid,
+ BATTERY_LEVEL_UUID) == 0) {
+ enable_battery_notification(ch, desc->handle);
+ return;
+ }
+
bt_uuid16_create(&btuuid, GATT_CHARAC_FMT_UUID);

if (bt_uuid_cmp(&desc->uuid, &btuuid) == 0) {
@@ -319,12 +365,54 @@ static void configure_battery_cb(GSList *characteristics, guint8 status,
}
}

+static void proc_batterylevel(struct characteristic *c, const uint8_t *pdu,
+ uint16_t len, gboolean final)
+{
+ if (!pdu) {
+ error("Battery level notification: Invalid pdu length");
+ return;
+ }
+
+ c->level = pdu[1];
+
+ btd_device_set_battery_opt(c->devbatt, BATTERY_OPT_LEVEL, c->level,
+ BATTERY_OPT_INVALID);
+}
+
+static void notif_handler(const uint8_t *pdu, uint16_t len, gpointer user_data)
+{
+ struct battery *batt = user_data;
+ struct characteristic *ch;
+ uint16_t handle;
+ GSList *l;
+
+ if (len < 3) {
+ error("notif_handler: Bad pdu received");
+ return;
+ }
+
+ handle = att_get_u16(&pdu[1]);
+ l = g_slist_find_custom(batt->chars, &handle, cmp_char_val_handle);
+ if (l == NULL) {
+ error("notif_handler: Unexpected handle 0x%04x", handle);
+ return;
+ }
+
+ ch = l->data;
+ if (g_strcmp0(ch->attr.uuid, BATTERY_LEVEL_UUID) == 0) {
+ proc_batterylevel(ch, pdu, len, FALSE);
+ }
+}
+
static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
{
struct battery *batt = user_data;

batt->attrib = g_attrib_ref(attrib);

+ batt->attnotid = g_attrib_register(batt->attrib, ATT_OP_HANDLE_NOTIFY,
+ notif_handler, batt, NULL);
+
if (batt->chars == NULL) {
gatt_discover_char(batt->attrib, batt->svc_range->start,
batt->svc_range->end, NULL,
@@ -333,7 +421,8 @@ static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
GSList *l;
for (l = batt->chars; l; l = l->next) {
struct characteristic *c = l->data;
- process_batteryservice_char(c);
+ if (!c->canNotify)
+ process_batteryservice_char(c);
}
}
}
@@ -342,6 +431,8 @@ static void attio_disconnected_cb(gpointer user_data)
{
struct battery *batt = user_data;

+ g_attrib_unregister(batt->attrib, batt->attnotid);
+ batt->attnotid = 0;
g_attrib_unref(batt->attrib);
batt->attrib = NULL;
}
--
1.7.9.5


2012-09-11 07:38:15

by Ganir, Chen

[permalink] [raw]
Subject: [PATCH 04/10] battery: Add client connection logic

From: Chen Ganir <[email protected]>

Add connection logic to the Battery Plugin. When the driver is
loaded, it will request a connection to the remote device and
release the connection request when destroyed.
---
profiles/battery/battery.c | 92 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 92 insertions(+)

diff --git a/profiles/battery/battery.c b/profiles/battery/battery.c
index 7702e39..37cbce4 100644
--- a/profiles/battery/battery.c
+++ b/profiles/battery/battery.c
@@ -30,17 +30,29 @@

#include "adapter.h"
#include "device.h"
+#include "gattrib.h"
+#include "attio.h"
#include "att.h"
#include "gattrib.h"
#include "gatt.h"
#include "battery.h"
+#include "log.h"

struct battery {
struct btd_device *dev; /* Device reference */
+ GAttrib *attrib; /* GATT connection */
+ guint attioid; /* Att watcher id */
+ struct att_range *svc_range; /* Battery range */
+ GSList *chars; /* Characteristics */
};

static GSList *servers;

+struct characteristic {
+ struct gatt_char attr; /* Characteristic */
+ struct battery *batt; /* Parent Battery Service */
+};
+
static gint cmp_device(gconstpointer a, gconstpointer b)
{
const struct battery *batt = a;
@@ -56,20 +68,100 @@ static void battery_free(gpointer user_data)
{
struct battery *batt = user_data;

+ if (batt->chars != NULL)
+ g_slist_free_full(batt->chars, g_free);
+
+ if (batt->attioid > 0)
+ btd_device_remove_attio_callback(batt->dev, batt->attioid);
+
+ if (batt->attrib != NULL)
+ g_attrib_unref(batt->attrib);
+
btd_device_unref(batt->dev);
+ g_free(batt->svc_range);
g_free(batt);
}

+static void configure_battery_cb(GSList *characteristics, guint8 status,
+ gpointer user_data)
+{
+ struct battery *batt = user_data;
+ GSList *l;
+
+ if (status != 0) {
+ error("Discover Battery characteristics: %s",
+ att_ecode2str(status));
+ return;
+ }
+
+ for (l = characteristics; l; l = l->next) {
+ struct gatt_char *c = l->data;
+ struct characteristic *ch;
+
+ ch = g_new0(struct characteristic, 1);
+ ch->attr.handle = c->handle;
+ ch->attr.properties = c->properties;
+ ch->attr.value_handle = c->value_handle;
+ memcpy(ch->attr.uuid, c->uuid, MAX_LEN_UUID_STR + 1);
+ ch->batt = batt;
+
+ batt->chars = g_slist_append(batt->chars, ch);
+ }
+}
+
+static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
+{
+ struct battery *batt = user_data;
+
+ batt->attrib = g_attrib_ref(attrib);
+
+ if (batt->chars == NULL) {
+ gatt_discover_char(batt->attrib, batt->svc_range->start,
+ batt->svc_range->end, NULL,
+ configure_battery_cb, batt);
+ }
+}
+
+static void attio_disconnected_cb(gpointer user_data)
+{
+ struct battery *batt = user_data;
+
+ g_attrib_unref(batt->attrib);
+ batt->attrib = NULL;
+}
+
+static gint primary_uuid_cmp(gconstpointer a, gconstpointer b)
+{
+ const struct gatt_primary *prim = a;
+ const char *uuid = b;
+
+ return g_strcmp0(prim->uuid, uuid);
+}

int battery_register(struct btd_device *device)
{
struct battery *batt;
+ struct gatt_primary *prim;
+ GSList *primaries, *l;
+
+ primaries = btd_device_get_primaries(device);
+
+ l = g_slist_find_custom(primaries, BATTERY_SERVICE_UUID,
+ primary_uuid_cmp);
+ prim = l->data;

batt = g_new0(struct battery, 1);
batt->dev = btd_device_ref(device);

+ batt->svc_range = g_new0(struct att_range, 1);
+ batt->svc_range->start = prim->range.start;
+ batt->svc_range->end = prim->range.end;
+
servers = g_slist_prepend(servers, batt);

+ batt->attioid = btd_device_add_attio_callback(device,
+ attio_connected_cb, attio_disconnected_cb,
+ batt);
return 0;
}

--
1.7.9.5


2012-09-11 07:38:16

by Ganir, Chen

[permalink] [raw]
Subject: [PATCH 05/10] battery: Discover Characteristic Descriptors

From: Chen Ganir <[email protected]>

Discover all characteristic descriptors, and build a descriptor
list. Presentation Format Descriptor and Client Characteristic
Configuration descriptors are searched.
---
profiles/battery/battery.c | 73 +++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 72 insertions(+), 1 deletion(-)

diff --git a/profiles/battery/battery.c b/profiles/battery/battery.c
index 37cbce4..3a24a7b 100644
--- a/profiles/battery/battery.c
+++ b/profiles/battery/battery.c
@@ -51,6 +51,13 @@ static GSList *servers;
struct characteristic {
struct gatt_char attr; /* Characteristic */
struct battery *batt; /* Parent Battery Service */
+ GSList *desc; /* Descriptors */
+};
+
+struct descriptor {
+ struct characteristic *ch; /* Parent Characteristic */
+ uint16_t handle; /* Descriptor Handle */
+ bt_uuid_t uuid; /* UUID */
};

static gint cmp_device(gconstpointer a, gconstpointer b)
@@ -64,12 +71,21 @@ static gint cmp_device(gconstpointer a, gconstpointer b)
return -1;
}

+static void char_free(gpointer user_data)
+{
+ struct characteristic *c = user_data;
+
+ g_slist_free_full(c->desc, g_free);
+
+ g_free(c);
+}
+
static void battery_free(gpointer user_data)
{
struct battery *batt = user_data;

if (batt->chars != NULL)
- g_slist_free_full(batt->chars, g_free);
+ g_slist_free_full(batt->chars, char_free);

if (batt->attioid > 0)
btd_device_remove_attio_callback(batt->dev, batt->attioid);
@@ -82,7 +98,47 @@ static void battery_free(gpointer user_data)
g_free(batt);
}

+static void discover_desc_cb(guint8 status, const guint8 *pdu, guint16 len,
+ gpointer user_data)
+{
+ struct characteristic *ch = user_data;
+ struct att_data_list *list;
+ uint8_t format;
+ int i;
+
+ if (status != 0) {
+ error("Discover all characteristic descriptors failed [%s]: %s",
+ ch->attr.uuid, att_ecode2str(status));
+ return;
+ }
+
+ list = dec_find_info_resp(pdu, len, &format);
+ if (list == NULL)
+ return;
+
+ for (i = 0; i < list->num; i++) {
+ struct descriptor *desc;
+ uint8_t *value;
+
+ value = list->data[i];
+ desc = g_new0(struct descriptor, 1);
+ desc->handle = att_get_u16(value);
+ desc->ch = ch;
+
+ if (format == 0x01)
+ desc->uuid = att_get_uuid16(&value[2]);
+ else
+ desc->uuid = att_get_uuid128(&value[2]);
+
+ ch->desc = g_slist_append(ch->desc, desc);
+ }
+
+ att_data_list_free(list);
+}
+
+
static void configure_battery_cb(GSList *characteristics, guint8 status,
+
gpointer user_data)
{
struct battery *batt = user_data;
@@ -97,6 +153,7 @@ static void configure_battery_cb(GSList *characteristics, guint8 status,
for (l = characteristics; l; l = l->next) {
struct gatt_char *c = l->data;
struct characteristic *ch;
+ uint16_t start, end;

ch = g_new0(struct characteristic, 1);
ch->attr.handle = c->handle;
@@ -106,6 +163,20 @@ static void configure_battery_cb(GSList *characteristics, guint8 status,
ch->batt = batt;

batt->chars = g_slist_append(batt->chars, ch);
+
+ start = c->value_handle + 1;
+
+ if (l->next != NULL) {
+ struct gatt_char *c = l->next->data;
+ if (start == c->handle)
+ continue;
+ end = c->handle - 1;
+ } else if (c->value_handle != batt->svc_range->end)
+ end = batt->svc_range->end;
+ else
+ continue;
+
+ gatt_find_info(batt->attrib, start, end, discover_desc_cb, ch);
}
}

--
1.7.9.5


2012-09-11 07:38:19

by Ganir, Chen

[permalink] [raw]
Subject: [PATCH 08/10] battery: Read Battery level characteristic

From: Chen Ganir <[email protected]>

Implement support for reading the battery level characteristic on
connection establishment.
---
profiles/battery/battery.c | 87 +++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 85 insertions(+), 2 deletions(-)

diff --git a/profiles/battery/battery.c b/profiles/battery/battery.c
index 31f2297..a93b352 100644
--- a/profiles/battery/battery.c
+++ b/profiles/battery/battery.c
@@ -55,6 +55,7 @@ struct characteristic {
GSList *desc; /* Descriptors */
uint8_t ns; /* Battery Namespace */
uint16_t description; /* Battery description */
+ uint8_t level;
};

struct descriptor {
@@ -103,6 +104,80 @@ static void battery_free(gpointer user_data)
g_free(batt);
}

+static void read_batterylevel_cb(guint8 status, const guint8 *pdu, guint16 len,
+ gpointer user_data)
+{
+ struct characteristic *ch = user_data;
+ uint8_t value[ATT_MAX_MTU];
+ int vlen;
+
+ if (status != 0) {
+ error("Failed to read Battery Level:%s", att_ecode2str(status));
+ return;
+ }
+
+ vlen = dec_read_resp(pdu, len, value, sizeof(value));
+ if (!vlen) {
+ error("Failed to read Battery Level: Protocol error\n");
+ return;
+ }
+
+ if (vlen < 1) {
+ error("Failed to read Battery Level: Wrong pdu len");
+ return;
+ }
+
+ ch->level = value[0];
+ btd_device_set_battery_opt(ch->devbatt, BATTERY_OPT_LEVEL, ch->level,
+ BATTERY_OPT_INVALID);
+}
+
+static void process_batteryservice_char(struct characteristic *ch)
+{
+ if (g_strcmp0(ch->attr.uuid, BATTERY_LEVEL_UUID) == 0) {
+ gatt_read_char(ch->batt->attrib, ch->attr.value_handle, 0,
+ read_batterylevel_cb, ch);
+ }
+}
+
+static gint device_battery_cmp(gconstpointer a, gconstpointer b)
+{
+ const struct characteristic *ch = a;
+ const struct device_battery *batt = b;
+
+ if (batt == ch->devbatt)
+ return 0;
+
+ return -1;
+}
+
+static struct characteristic *find_battery_char(struct device_battery *db)
+{
+ GSList *l, *b;
+
+ for (l = servers; l != NULL; l = g_slist_next(l)) {
+ struct battery *batt = l->data;
+
+ b = g_slist_find_custom(batt->chars, db, device_battery_cmp);
+ if (!b)
+ return NULL;
+
+ return b->data;
+ }
+
+ return NULL;
+}
+
+static void batterylevel_refresh_cb(struct device_battery *batt)
+{
+ struct characteristic *ch;
+
+ ch = find_battery_char(batt);
+
+ if (ch)
+ process_batteryservice_char(ch);
+}
+
static void batterylevel_presentation_format_desc_cb(guint8 status,
const guint8 *pdu, guint16 len,
gpointer user_data)
@@ -151,7 +226,6 @@ static void process_batterylevel_desc(struct descriptor *desc)
DBG("Ignored descriptor %s characteristic %s", uuidstr, ch->attr.uuid);
}

-
static void discover_desc_cb(guint8 status, const guint8 *pdu, guint16 len,
gpointer user_data)
{
@@ -192,8 +266,11 @@ static void discover_desc_cb(guint8 status, const guint8 *pdu, guint16 len,

update_char:
ch->devbatt = btd_device_add_battery(ch->batt->dev);
-}
+ btd_device_set_battery_opt(ch->devbatt, BATTERY_OPT_REFRESH_FUNC,
+ batterylevel_refresh_cb, BATTERY_OPT_INVALID);

+ process_batteryservice_char(ch);
+}

static void configure_battery_cb(GSList *characteristics, guint8 status,

@@ -252,6 +329,12 @@ static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
gatt_discover_char(batt->attrib, batt->svc_range->start,
batt->svc_range->end, NULL,
configure_battery_cb, batt);
+ } else {
+ GSList *l;
+ for (l = batt->chars; l; l = l->next) {
+ struct characteristic *c = l->data;
+ process_batteryservice_char(c);
+ }
}
}

--
1.7.9.5


2012-09-11 07:38:17

by Ganir, Chen

[permalink] [raw]
Subject: [PATCH 06/10] battery: Get Battery ID

From: Chen Ganir <[email protected]>

Read the battery level format characteristic descriptor to get the
unique namespace and description values.
---
lib/uuid.h | 1 +
profiles/battery/battery.c | 100 ++++++++++++++++++++++++++++++++++----------
2 files changed, 79 insertions(+), 22 deletions(-)

diff --git a/lib/uuid.h b/lib/uuid.h
index 58ad0b3..5c1b3ff 100644
--- a/lib/uuid.h
+++ b/lib/uuid.h
@@ -57,6 +57,7 @@ extern "C" {
#define DEVICE_INFORMATION_UUID "0000180a-0000-1000-8000-00805f9b34fb"

#define BATTERY_SERVICE_UUID "0000180f-0000-1000-8000-00805f9b34fb"
+#define BATTERY_LEVEL_UUID "00002a19-0000-1000-8000-00805f9b34fb"

#define GATT_UUID "00001801-0000-1000-8000-00805f9b34fb"
#define IMMEDIATE_ALERT_UUID "00001802-0000-1000-8000-00805f9b34fb"
diff --git a/profiles/battery/battery.c b/profiles/battery/battery.c
index 3a24a7b..d1e0b6e 100644
--- a/profiles/battery/battery.c
+++ b/profiles/battery/battery.c
@@ -52,6 +52,8 @@ struct characteristic {
struct gatt_char attr; /* Characteristic */
struct battery *batt; /* Parent Battery Service */
GSList *desc; /* Descriptors */
+ uint8_t ns; /* Battery Namespace */
+ uint16_t description; /* Battery description */
};

struct descriptor {
@@ -98,6 +100,55 @@ static void battery_free(gpointer user_data)
g_free(batt);
}

+static void batterylevel_presentation_format_desc_cb(guint8 status,
+ const guint8 *pdu, guint16 len,
+ gpointer user_data)
+{
+ struct descriptor *desc = user_data;
+ uint8_t value[ATT_MAX_MTU];
+ int vlen;
+
+ if (status != 0) {
+ error("Presentation Format desc read failed: %s",
+ att_ecode2str(status));
+ return;
+ }
+
+ vlen = dec_read_resp(pdu, len, value, sizeof(value));
+ if (!vlen) {
+ error("Presentation Format desc read failed: Protocol error\n");
+ return;
+ }
+
+ if (vlen < 7) {
+ error("Presentation Format desc read failed: Invalid range");
+ return;
+ }
+
+ desc->ch->ns = value[4];
+ desc->ch->description = att_get_u16(&value[5]);
+}
+
+
+static void process_batterylevel_desc(struct descriptor *desc)
+{
+ struct characteristic *ch = desc->ch;
+ char uuidstr[MAX_LEN_UUID_STR];
+ bt_uuid_t btuuid;
+
+ bt_uuid16_create(&btuuid, GATT_CHARAC_FMT_UUID);
+
+ if (bt_uuid_cmp(&desc->uuid, &btuuid) == 0) {
+ gatt_read_char(ch->batt->attrib, desc->handle, 0,
+ batterylevel_presentation_format_desc_cb, desc);
+ return;
+ }
+
+ bt_uuid_to_string(&desc->uuid, uuidstr, MAX_LEN_UUID_STR);
+ DBG("Ignored descriptor %s characteristic %s", uuidstr, ch->attr.uuid);
+}
+
+
static void discover_desc_cb(guint8 status, const guint8 *pdu, guint16 len,
gpointer user_data)
{
@@ -131,6 +182,7 @@ static void discover_desc_cb(guint8 status, const guint8 *pdu, guint16 len,
desc->uuid = att_get_uuid128(&value[2]);

ch->desc = g_slist_append(ch->desc, desc);
+ process_batterylevel_desc(desc);
}

att_data_list_free(list);
@@ -152,31 +204,35 @@ static void configure_battery_cb(GSList *characteristics, guint8 status,

for (l = characteristics; l; l = l->next) {
struct gatt_char *c = l->data;
- struct characteristic *ch;
- uint16_t start, end;
-
- ch = g_new0(struct characteristic, 1);
- ch->attr.handle = c->handle;
- ch->attr.properties = c->properties;
- ch->attr.value_handle = c->value_handle;
- memcpy(ch->attr.uuid, c->uuid, MAX_LEN_UUID_STR + 1);
- ch->batt = batt;

- batt->chars = g_slist_append(batt->chars, ch);
-
- start = c->value_handle + 1;
-
- if (l->next != NULL) {
- struct gatt_char *c = l->next->data;
- if (start == c->handle)
+ if (g_strcmp0(c->uuid, BATTERY_LEVEL_UUID) == 0) {
+ struct characteristic *ch;
+ uint16_t start, end;
+
+ ch = g_new0(struct characteristic, 1);
+ ch->attr.handle = c->handle;
+ ch->attr.properties = c->properties;
+ ch->attr.value_handle = c->value_handle;
+ memcpy(ch->attr.uuid, c->uuid, MAX_LEN_UUID_STR + 1);
+ ch->batt = batt;
+
+ batt->chars = g_slist_append(batt->chars, ch);
+
+ start = c->value_handle + 1;
+
+ if (l->next != NULL) {
+ struct gatt_char *c = l->next->data;
+ if (start == c->handle)
+ continue;
+ end = c->handle - 1;
+ } else if (c->value_handle != batt->svc_range->end)
+ end = batt->svc_range->end;
+ else
continue;
- end = c->handle - 1;
- } else if (c->value_handle != batt->svc_range->end)
- end = batt->svc_range->end;
- else
- continue;

- gatt_find_info(batt->attrib, start, end, discover_desc_cb, ch);
+ gatt_find_info(batt->attrib, start, end,
+ discover_desc_cb, ch);
+ }
}
}

--
1.7.9.5


2012-09-11 07:38:13

by Ganir, Chen

[permalink] [raw]
Subject: [PATCH 02/10] battery: Implement Generic device battery

From: Chen Ganir <[email protected]>

Add implementation for the generic battery in bluetooth device.
This patch adds new D-Bus interface for adding/removing/changing
peer device battery status, allowing management of remote device
batteries.
---
src/device.c | 184 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
src/device.h | 15 +++++
test/test-device | 13 ++++
3 files changed, 212 insertions(+)

diff --git a/src/device.c b/src/device.c
index 02ef35e..ce4d467 100644
--- a/src/device.c
+++ b/src/device.c
@@ -166,6 +166,7 @@ struct btd_device {

gboolean authorizing;
gint ref;
+ GSList *batteries;

GIOChannel *att_io;
guint cleanup_id;
@@ -180,6 +181,26 @@ static uint16_t uuid_list[] = {
0
};

+struct device_battery {
+ DBusConnection *conn;
+ uint16_t level;
+ char *path;
+ struct btd_device *device;
+ RefreshBattFunc refresh_func;
+};
+
+static void battery_free(gpointer user_data)
+{
+ struct device_battery *b = user_data;
+
+ g_dbus_unregister_interface(b->conn, b->path, BATTERY_INTERFACE);
+ dbus_connection_unref(b->conn);
+ btd_device_unref(b->device);
+ g_free(b->path);
+ g_free(b);
+
+}
+
static void browse_request_free(struct browse_req *req)
{
if (req->listener_id)
@@ -259,6 +280,7 @@ static void device_free(gpointer user_data)
g_slist_free_full(device->primaries, g_free);
g_slist_free_full(device->attios, g_free);
g_slist_free_full(device->attios_offline, g_free);
+ g_slist_free_full(device->batteries, battery_free);

attio_cleanup(device);

@@ -433,6 +455,15 @@ static DBusMessage *get_properties(DBusConnection *conn,
ptr = adapter_get_path(adapter);
dict_append_entry(&dict, "Adapter", DBUS_TYPE_OBJECT_PATH, &ptr);

+ /* Batteries */
+ str = g_new0(char *, g_slist_length(device->batteries) + 1);
+ for (i = 0, l = device->batteries; l; l = l->next, i++) {
+ struct device_battery *b = l->data;
+ str[i] = b->path;
+ }
+ dict_append_array(&dict, "Batteries", DBUS_TYPE_OBJECT_PATH, &str, i);
+ g_free(str);
+
dbus_message_iter_close_container(&iter, &dict);

return reply;
@@ -3263,3 +3294,156 @@ void btd_profile_disconnected(struct btd_profile *profile,
{
device->conns = g_slist_remove(device->conns, profile);
}
+
+static void batteries_changed(struct btd_device *device)
+{
+ DBusConnection *conn = get_dbus_connection();
+ char **batteries;
+ GSList *l;
+ int i;
+
+ batteries = g_new0(char *, g_slist_length(device->batteries) + 1);
+ for (i = 0, l = device->batteries; l; l = l->next, i++) {
+ struct device_battery *batt = l->data;
+ batteries[i] = batt->path;
+ }
+
+ emit_array_property_changed(conn, device->path, DEVICE_INTERFACE,
+ "Batteries", DBUS_TYPE_OBJECT_PATH,
+ &batteries, i);
+
+ g_free(batteries);
+}
+
+static DBusMessage *refresh_batt_level(DBusConnection *conn,
+ DBusMessage *msg, void *data)
+{
+ struct device_battery *b = data;
+
+ if (!b->refresh_func)
+ return btd_error_not_supported(msg);
+
+ b->refresh_func(b);
+
+ return dbus_message_new_method_return(msg);
+}
+
+static DBusMessage *get_batt_properties(DBusConnection *conn,
+ DBusMessage *msg, void *data)
+{
+ struct device_battery *b = data;
+ DBusMessageIter iter;
+ DBusMessageIter dict;
+ DBusMessage *reply;
+
+ reply = dbus_message_new_method_return(msg);
+ if (reply == NULL)
+ return NULL;
+
+ dbus_message_iter_init_append(reply, &iter);
+
+ dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
+ DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
+ DBUS_TYPE_STRING_AS_STRING DBUS_TYPE_VARIANT_AS_STRING
+ DBUS_DICT_ENTRY_END_CHAR_AS_STRING, &dict);
+
+ dict_append_entry(&dict, "Level", DBUS_TYPE_UINT16, &b->level);
+
+ dbus_message_iter_close_container(&iter, &dict);
+
+ return reply;
+}
+
+static GDBusMethodTable battery_methods[] = {
+ { GDBUS_METHOD("GetProperties",
+ NULL, GDBUS_ARGS({ "properties", "a{sv}" }),
+ get_batt_properties) },
+ { GDBUS_METHOD("Refresh",
+ NULL, NULL,
+ refresh_batt_level) },
+ { }
+};
+
+static GDBusSignalTable battery_signals[] = {
+ { GDBUS_SIGNAL("PropertyChanged",
+ GDBUS_ARGS({ "name", "s" }, { "value", "v" })) },
+ { }
+};
+
+struct device_battery *btd_device_add_battery(struct btd_device *device)
+{
+ struct device_battery *batt;
+ DBusConnection *conn = get_dbus_connection();
+
+ batt = g_new0(struct device_battery, 1);
+ batt->path = g_strdup_printf("%s/BATT%04X", device->path,
+ g_slist_length(device->batteries));
+ batt->conn = dbus_connection_ref(conn);
+
+ if (!g_dbus_register_interface(batt->conn, batt->path,
+ BATTERY_INTERFACE, battery_methods, battery_signals,
+ NULL, batt, NULL)) {
+ error("D-Bus register interface %s failed", BATTERY_INTERFACE);
+ dbus_connection_unref(batt->conn);
+ g_free(batt->path);
+ g_free(batt);
+ return NULL;
+ }
+
+ batt->device = btd_device_ref(device);
+ device->batteries = g_slist_append(device->batteries, batt);
+ batteries_changed(device);
+
+ return batt;
+}
+
+void btd_device_remove_battery(struct device_battery *batt)
+{
+ struct btd_device *dev = batt->device;
+
+ dev->batteries = g_slist_remove(dev->batteries, batt);
+
+ battery_free(batt);
+
+ batteries_changed(dev);
+}
+
+gboolean btd_device_set_battery_opt(struct device_battery *batt,
+ BatteryOption opt1, ...)
+{
+ va_list args;
+ BatteryOption opt = opt1;
+ int level;
+
+ if (!batt)
+ return FALSE;
+
+ va_start(args, opt1);
+
+ while (opt != BATTERY_OPT_INVALID) {
+ switch (opt) {
+ case BATTERY_OPT_LEVEL:
+ level = va_arg(args, int);
+ if (level != batt->level) {
+ batt->level = level;
+ emit_property_changed(batt->conn, batt->path,
+ BATTERY_INTERFACE, "Level",
+ DBUS_TYPE_UINT16, &level);
+ }
+ break;
+ case BATTERY_OPT_REFRESH_FUNC:
+ batt->refresh_func = va_arg(args, RefreshBattFunc);
+ break;
+ default:
+ error("Unknown option %d", opt);
+ return FALSE;
+ }
+
+ opt = va_arg(args, int);
+ }
+
+ va_end(args);
+
+ return TRUE;
+
+}
diff --git a/src/device.h b/src/device.h
index f1d95c6..66eb5e8 100644
--- a/src/device.h
+++ b/src/device.h
@@ -23,8 +23,10 @@
*/

#define DEVICE_INTERFACE "org.bluez.Device"
+#define BATTERY_INTERFACE "org.bluez.Battery"

struct btd_device;
+struct device_battery;

typedef enum {
AUTH_TYPE_PINCODE,
@@ -54,6 +56,14 @@ struct btd_profile {
void (*adapter_remove) (struct btd_adapter *adapter);
};

+typedef void (*RefreshBattFunc) (struct device_battery *batt);
+
+typedef enum {
+ BATTERY_OPT_INVALID = 0,
+ BATTERY_OPT_LEVEL,
+ BATTERY_OPT_REFRESH_FUNC,
+} BatteryOption;
+
void btd_profile_foreach(void (*func)(struct btd_profile *p, void *data),
void *data);

@@ -158,3 +168,8 @@ int device_unblock(DBusConnection *conn, struct btd_device *device,
void device_set_pnpid(struct btd_device *device, uint8_t vendor_id_src,
uint16_t vendor_id, uint16_t product_id,
uint16_t product_ver);
+
+struct device_battery *btd_device_add_battery(struct btd_device *device);
+void btd_device_remove_battery(struct device_battery *batt);
+gboolean btd_device_set_battery_opt(struct device_battery *batt,
+ BatteryOption opt1, ...);
diff --git a/test/test-device b/test/test-device
index 63a96d3..7edb7b8 100755
--- a/test/test-device
+++ b/test/test-device
@@ -37,6 +37,7 @@ if (len(args) < 1):
print("")
print(" list")
print(" services <address>")
+ print(" batteries <address>")
print(" create <address>")
print(" remove <address|path>")
print(" disconnect <address>")
@@ -205,5 +206,17 @@ if (args[0] == "services"):
print(path)
sys.exit(0)

+if (args[0] == "batteries"):
+ if (len(args) < 2):
+ print("Need address parameter")
+ else:
+ path = adapter.FindDevice(args[1])
+ device = dbus.Interface(bus.get_object("org.bluez", path),
+ "org.bluez.Device")
+ properties = device.GetProperties()
+ for path in properties["Batteries"]:
+ print(path)
+ sys.exit(0)
+
print("Unknown command")
sys.exit(1)
--
1.7.9.5


2012-09-11 07:38:12

by Ganir, Chen

[permalink] [raw]
Subject: [PATCH 01/10] battery: Add generic device battery documentation

From: Chen Ganir <[email protected]>

Add documentation for the generic battery D-Bus interface.
---
doc/battery-api.txt | 33 +++++++++++++++++++++++++++++++++
doc/device-api.txt | 5 +++++
2 files changed, 38 insertions(+)
create mode 100644 doc/battery-api.txt

diff --git a/doc/battery-api.txt b/doc/battery-api.txt
new file mode 100644
index 0000000..d5ef5ed
--- /dev/null
+++ b/doc/battery-api.txt
@@ -0,0 +1,33 @@
+BlueZ D-Bus Battery API description
+****************************************
+
+ Texas Instruments, Inc. <[email protected]>
+
+Device Battery hierarchy
+=====================================
+
+Service org.bluez
+Interface org.bluez.Battery
+Object path [variable prefix]/{hci0,..}/dev_XX_XX_XX_XX_XX_XX/BATTYYYY
+YYYY is numeric value between 0 and 9999.
+
+Methods dict GetProperties()
+
+ Returns all properties for the interface. See the
+ Properties section for the available properties.
+
+ void Refresh()
+
+ Refresh the batterty level. If the battery level changed, the
+ PropertyChanged signal will be sent with the new value.
+
+Signals PropertyChanged(string name, variant value)
+
+ This signal indicates a changed value of the given
+ property.
+
+Properties uint16 Level [readonly]
+
+ Battery level (0-100).
+
+
diff --git a/doc/device-api.txt b/doc/device-api.txt
index 1f0dc96..c98d539 100644
--- a/doc/device-api.txt
+++ b/doc/device-api.txt
@@ -179,3 +179,8 @@ Properties string Address [readonly]
Note that this property can exhibit false-positives
in the case of Bluetooth 2.1 (or newer) devices that
have disabled Extended Inquiry Response support.
+
+ array{object} Batteries [readonly]
+
+ List of device battery object paths that represents the available
+ batteries on the remote device.
--
1.7.9.5