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

Chen Ganir <chen.ganir@xxxxxx> · Wed, 12 Sep 2012 07:58:34 +0300

Joao,
On 09/12/2012 01:08 AM, Joao Paulo Rechi Vita wrote:
On Tue, Sep 11, 2012 at 4:38 AM,  <chen.ganir@xxxxxx> wrote:
From: Chen Ganir <chen.ganir@xxxxxx>

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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




Thanks,

--
BR,
Chen Ganir

--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html





