From: Chen Ganir <chen.ganir@ti.com>
To: Joao Paulo Rechi Vita <jprvita@openbossa.org>
Cc: <linux-bluetooth@vger.kernel.org>
Subject: Re: [PATCH 09/10] battery: Add support for notifications
Date: Wed, 12 Sep 2012 07:58:34 +0300 [thread overview]
Message-ID: <5050167A.8020400@ti.com> (raw)
In-Reply-To: <CAAngNMaMmfx1+PpzJLcRJnBSa6+1LPyw+i489VgtoyRkK9Wqfg@mail.gmail.com>
Joao,
On 09/12/2012 01:08 AM, Joao Paulo Rechi Vita wrote:
> On Tue, Sep 11, 2012 at 4:38 AM, <chen.ganir@ti.com> wrote:
>> From: Chen Ganir <chen.ganir@ti.com>
>>
>> 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 majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>
Thanks,
--
BR,
Chen Ganir
next prev parent reply other threads:[~2012-09-12 4:58 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-09-11 7:38 [PATCH 00/10] Implement Generic battery and LE Battery client chen.ganir
2012-09-11 7:38 ` [PATCH 01/10] battery: Add generic device battery documentation chen.ganir
2012-09-11 15:34 ` Joao Paulo Rechi Vita
2012-09-12 4:45 ` Chen Ganir
2012-09-11 7:38 ` [PATCH 02/10] battery: Implement Generic device battery chen.ganir
2012-09-11 18:27 ` Joao Paulo Rechi Vita
2012-09-12 4:48 ` Chen Ganir
2012-09-12 8:45 ` Johan Hedberg
2012-09-12 10:30 ` Chen Ganir
2012-09-12 10:57 ` Anderson Lizardo
2012-09-13 11:32 ` Chen Ganir
2012-09-11 7:38 ` [PATCH 03/10] battery: Add GATT Battery Client Service skeleton chen.ganir
2012-09-11 7:38 ` [PATCH 04/10] battery: Add client connection logic chen.ganir
2012-09-11 7:38 ` [PATCH 05/10] battery: Discover Characteristic Descriptors chen.ganir
2012-09-11 20:52 ` Joao Paulo Rechi Vita
2012-09-12 4:49 ` Chen Ganir
2012-09-11 7:38 ` [PATCH 06/10] battery: Get Battery ID chen.ganir
2012-09-11 7:38 ` [PATCH 07/10] battery: Add Battery to device chen.ganir
2012-09-11 21:40 ` Joao Paulo Rechi Vita
2012-09-12 4:54 ` Chen Ganir
2012-09-11 7:38 ` [PATCH 08/10] battery: Read Battery level characteristic chen.ganir
2012-09-11 21:50 ` Joao Paulo Rechi Vita
2012-09-12 4:55 ` Chen Ganir
2012-09-11 7:38 ` [PATCH 09/10] battery: Add support for notifications chen.ganir
2012-09-11 22:08 ` Joao Paulo Rechi Vita
2012-09-12 4:58 ` Chen Ganir [this message]
2012-09-13 11:27 ` Chen Ganir
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5050167A.8020400@ti.com \
--to=chen.ganir@ti.com \
--cc=jprvita@openbossa.org \
--cc=linux-bluetooth@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).