*v3: Removed Change-Id lines from patch descriptions.
*v2: Cancel ongoing CCC writes in bt_gatt_client_free.
*v1: Make bt_gatt_register_notify cancellable via bt_gatt_unregister_notify.
notify_client_free now unregisters its notify id, which will cancel any
pending CCC write, as well as unregister a registered notify callback.
This patch sets brings support for enabling notifications while in GATT
client-role. The changes that are introduced are:
1. Implemented the StartNotify and StopNotify methods of the
GattCharacteristic1 interface. This are internally tied to
bt_gatt_client_register_notify and bt_gatt_client_unregister_notify.
These also manage notification sessions on a per dbus sender basis.
2. The exported GATT API objects are not removed in the event of a disconnect,
if the devices are bonded. All notification sessions are also persisted and
automatically re-enabled on reconnection. Also, adding new notification
sessions via StartNotify is allowed during disconnects.
Arman Uguray (5):
shared/gatt: Make register_notify cancellable
core: gatt: Implement GattCharacteristic1.StartNotify
core: gatt: Implement GattCharacteristic1.StopNotify
core: device: Don't check ready in service_removed
core: gatt: Keep objects over disconnects
src/device.c | 22 ++-
src/gatt-client.c | 435 +++++++++++++++++++++++++++++++++++++++++++++--
src/shared/gatt-client.c | 129 ++++++++------
src/shared/gatt-client.h | 7 +-
tools/btgatt-client.c | 17 +-
5 files changed, 530 insertions(+), 80 deletions(-)
--
2.2.0.rc0.207.ga3a616c
This patch changes the way that the life-time for exported GATT API
objects are managed, so that they are kept alive in the case of a
disconnection if the devices are bonded. In this case, all API method
calls return an error except for StartNotify/StopNotify.
All notification sessions that were initiated during and in-between
connections are maintained and a notification callback is registered
for each session immediately on reconnection.
---
src/device.c | 5 ++
src/gatt-client.c | 222 +++++++++++++++++++++++++++++++++++++++++++++---------
2 files changed, 191 insertions(+), 36 deletions(-)
diff --git a/src/device.c b/src/device.c
index 29a8f23..059e3a4 100644
--- a/src/device.c
+++ b/src/device.c
@@ -529,6 +529,11 @@ static void gatt_client_cleanup(struct btd_device *device)
bt_gatt_client_unref(device->client);
device->client = NULL;
+ /*
+ * TODO: Once GATT over BR/EDR is properly supported, we should check
+ * the bonding state for the correct bearer based on the transport over
+ * which GATT is being done.
+ */
if (!device->le_state.bonded)
gatt_db_clear(device->db);
}
diff --git a/src/gatt-client.c b/src/gatt-client.c
index e157ac2..f03a278 100644
--- a/src/gatt-client.c
+++ b/src/gatt-client.c
@@ -58,6 +58,7 @@ struct btd_gatt_client {
struct bt_gatt_client *gatt;
struct queue *services;
+ struct queue *all_notify_clients;
};
struct service {
@@ -387,6 +388,9 @@ static DBusMessage *descriptor_read_value(DBusConnection *conn,
struct bt_gatt_client *gatt = desc->chrc->service->client->gatt;
struct async_dbus_op *op;
+ if (!gatt)
+ return btd_error_failed(msg, "Not connected");
+
if (desc->read_id)
return btd_error_in_progress(msg);
@@ -521,6 +525,9 @@ static DBusMessage *descriptor_write_value(DBusConnection *conn,
uint8_t *value = NULL;
size_t value_len = 0;
+ if (!gatt)
+ return btd_error_failed(msg, "Not connected");
+
if (desc->write_id)
return btd_error_in_progress(msg);
@@ -815,6 +822,9 @@ static DBusMessage *characteristic_read_value(DBusConnection *conn,
struct bt_gatt_client *gatt = chrc->service->client->gatt;
struct async_dbus_op *op;
+ if (!gatt)
+ return btd_error_failed(msg, "Not connected");
+
if (chrc->read_id)
return btd_error_in_progress(msg);
@@ -859,6 +869,9 @@ static DBusMessage *characteristic_write_value(DBusConnection *conn,
size_t value_len = 0;
bool supported = false;
+ if (!gatt)
+ return btd_error_failed(msg, "Not connected");
+
if (chrc->write_id)
return btd_error_in_progress(msg);
@@ -992,12 +1005,11 @@ static void notify_client_disconnect(DBusConnection *conn, void *user_data)
{
struct notify_client *client = user_data;
struct characteristic *chrc = client->chrc;
- struct bt_gatt_client *gatt = chrc->service->client->gatt;
DBG("owner %s", client->owner);
queue_remove(chrc->notify_clients, client);
- bt_gatt_client_unregister_notify(gatt, client->notify_id);
+ queue_remove(chrc->service->client->all_notify_clients, client);
update_notifying(chrc);
@@ -1057,34 +1069,41 @@ static void notify_cb(uint16_t value_handle, const uint8_t *value,
write_characteristic_cb, chrc);
}
-static void register_notify_cb(uint16_t att_ecode, void *user_data)
+static DBusMessage *create_notify_reply(struct async_dbus_op *op,
+ bool success, uint8_t att_ecode)
{
- struct async_dbus_op *op = user_data;
- struct notify_client *client = op->data;
- struct characteristic *chrc = client->chrc;
- DBusMessage *reply;
+ DBusMessage *reply = NULL;
- /* Make sure that an interim disconnect did not remove the client */
- if (!queue_find(chrc->notify_clients, NULL, client)) {
- bt_gatt_client_unregister_notify(chrc->service->client->gatt,
- client->notify_id);
- notify_client_unref(client);
+ if (!op->msg)
+ return NULL;
+ if (success)
+ reply = g_dbus_create_reply(op->msg, DBUS_TYPE_INVALID);
+ else if (att_ecode)
+ reply = create_gatt_dbus_error(op->msg, att_ecode);
+ else
reply = btd_error_failed(op->msg,
"Characteristic not available");
- goto done;
- }
- /*
- * Drop the reference count that we added when registering the callback.
- */
- notify_client_unref(client);
+ if (!reply)
+ error("Failed to construct D-Bus message reply");
+
+ return reply;
+}
+
+static void register_notify_cb(uint16_t att_ecode, void *user_data)
+{
+ struct async_dbus_op *op = user_data;
+ struct notify_client *client = op->data;
+ struct characteristic *chrc = client->chrc;
+ DBusMessage *reply;
if (att_ecode) {
queue_remove(chrc->notify_clients, client);
+ queue_remove(chrc->service->client->all_notify_clients, client);
notify_client_free(client);
- reply = create_gatt_dbus_error(op->msg, att_ecode);
+ reply = create_notify_reply(op, false, att_ecode);
goto done;
}
@@ -1096,7 +1115,7 @@ static void register_notify_cb(uint16_t att_ecode, void *user_data)
"Notifying");
}
- reply = g_dbus_create_reply(op->msg, DBUS_TYPE_INVALID);
+ reply = create_notify_reply(op, true, 0);
done:
if (reply)
@@ -1129,20 +1148,35 @@ static DBusMessage *characteristic_start_notify(DBusConnection *conn,
if (!client)
return btd_error_failed(msg, "Failed allocate notify session");
- op = new0(struct async_dbus_op, 1);
- if (!op) {
- notify_client_unref(client);
- return btd_error_failed(msg, "Failed to initialize request");
- }
+ queue_push_tail(chrc->notify_clients, client);
+ queue_push_tail(chrc->service->client->all_notify_clients, client);
/*
- * Add to the ref count so that a disconnect during register won't free
- * the client instance.
+ * If the device is currently not connected, return success. We will
+ * automatically try and register all clients when a GATT client becomes
+ * ready.
*/
- op->data = notify_client_ref(client);
- op->msg = dbus_message_ref(msg);
+ if (!gatt) {
+ DBusMessage *reply;
- queue_push_tail(chrc->notify_clients, client);
+ reply = g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
+ if (reply)
+ return reply;
+
+ /*
+ * Clean up and respond with an error instead of timing out to
+ * avoid any ambiguities.
+ */
+ error("Failed to construct D-Bus message reply");
+ goto fail;
+ }
+
+ op = new0(struct async_dbus_op, 1);
+ if (!op)
+ goto fail;
+
+ op->data = client;
+ op->msg = dbus_message_ref(msg);
client->notify_id = bt_gatt_client_register_notify(gatt,
chrc->value_handle,
@@ -1151,9 +1185,12 @@ static DBusMessage *characteristic_start_notify(DBusConnection *conn,
if (client->notify_id)
return NULL;
- queue_remove(chrc->notify_clients, client);
async_dbus_op_free(op);
+fail:
+ queue_remove(chrc->notify_clients, client);
+ queue_remove(chrc->service->client->all_notify_clients, client);
+
/* Directly free the client */
notify_client_free(client);
@@ -1176,6 +1213,7 @@ static DBusMessage *characteristic_stop_notify(DBusConnection *conn,
if (!client)
return btd_error_failed(msg, "No notify session started");
+ queue_remove(chrc->service->client->all_notify_clients, client);
bt_gatt_client_unregister_notify(gatt, client->notify_id);
update_notifying(chrc);
@@ -1680,6 +1718,13 @@ struct btd_gatt_client *btd_gatt_client_new(struct btd_device *device)
return NULL;
}
+ client->all_notify_clients = queue_new();
+ if (!client->all_notify_clients) {
+ queue_destroy(client->services, NULL);
+ free(client);
+ return NULL;
+ }
+
client->device = device;
ba2str(device_get_address(device), client->devaddr);
@@ -1694,11 +1739,44 @@ void btd_gatt_client_destroy(struct btd_gatt_client *client)
return;
queue_destroy(client->services, unregister_service);
+ queue_destroy(client->all_notify_clients, NULL);
bt_gatt_client_unref(client->gatt);
gatt_db_unref(client->db);
free(client);
}
+static void register_notify(void *data, void *user_data)
+{
+ struct notify_client *notify_client = data;
+ struct btd_gatt_client *client = user_data;
+ struct async_dbus_op *op;
+
+ DBG("Re-register subscribed notification client");
+
+ op = new0(struct async_dbus_op, 1);
+ if (!op)
+ goto fail;
+
+ op->data = notify_client;
+
+ notify_client->notify_id = bt_gatt_client_register_notify(client->gatt,
+ notify_client->chrc->value_handle,
+ register_notify_cb, notify_cb,
+ op, async_dbus_op_free);
+ if (notify_client->notify_id)
+ return;
+
+ async_dbus_op_free(op);
+
+fail:
+ DBG("Failed to re-register notification client");
+
+ queue_remove(notify_client->chrc->notify_clients, client);
+ queue_remove(client->all_notify_clients, client);
+
+ notify_client_free(notify_client);
+}
+
void btd_gatt_client_ready(struct btd_gatt_client *client)
{
struct bt_gatt_client *gatt;
@@ -1717,7 +1795,19 @@ void btd_gatt_client_ready(struct btd_gatt_client *client)
client->ready = true;
- create_services(client);
+ DBG("GATT client ready");
+
+ if (queue_isempty(client->services)) {
+ DBG("Exporting services");
+ create_services(client);
+ return;
+ }
+
+ /*
+ * Services have already been created before. Re-enable notifications
+ * for any pre-registered notification sessions.
+ */
+ queue_foreach(client->all_notify_clients, register_notify, client);
}
void btd_gatt_client_service_added(struct btd_gatt_client *client,
@@ -1755,18 +1845,78 @@ void btd_gatt_client_service_removed(struct btd_gatt_client *client,
unregister_service);
}
+static void clear_notify_id(void *data, void *user_data)
+{
+ struct notify_client *client = data;
+
+ client->notify_id = 0;
+}
+
+static void cancel_desc_ops(void *data, void *user_data)
+{
+ struct descriptor *desc = data;
+ struct bt_gatt_client *gatt = user_data;
+
+ if (desc->read_id) {
+ bt_gatt_client_cancel(gatt, desc->read_id);
+ desc->read_id = 0;
+ }
+
+ if (desc->write_id) {
+ bt_gatt_client_cancel(gatt, desc->write_id);
+ desc->write_id = 0;
+ }
+}
+
+static void cancel_chrc_ops(void *data, void *user_data)
+{
+ struct characteristic *chrc = data;
+ struct bt_gatt_client *gatt = user_data;
+
+ if (chrc->read_id) {
+ bt_gatt_client_cancel(gatt, chrc->read_id);
+ chrc->read_id = 0;
+ }
+
+ if (chrc->write_id) {
+ bt_gatt_client_cancel(gatt, chrc->write_id);
+ chrc->write_id = 0;
+ }
+
+ queue_foreach(chrc->descs, cancel_desc_ops, user_data);
+}
+
+static void cancel_ops(void *data, void *user_data)
+{
+ struct service *service = data;
+
+ queue_foreach(service->chrcs, cancel_chrc_ops, user_data);
+}
+
void btd_gatt_client_disconnected(struct btd_gatt_client *client)
{
- if (!client)
+ if (!client || !client->gatt)
return;
- DBG("Device disconnected. Cleaning up");
+ DBG("Device disconnected. Cleaning up.");
/*
* Remove all services. We'll recreate them when a new bt_gatt_client
- * becomes ready.
+ * becomes ready. Leave the services around if the device is bonded.
+ * TODO: Once GATT over BR/EDR is properly supported, we should pass the
+ * correct bdaddr_type based on the transport over which GATT is being
+ * done.
*/
- queue_remove_all(client->services, NULL, NULL, unregister_service);
+ if (!device_is_bonded(client->device, BDADDR_LE_PUBLIC)) {
+ DBG("Device not bonded. Removing exported services.");
+ queue_remove_all(client->services, NULL, NULL,
+ unregister_service);
+ } else {
+ DBG("Device is bonded. Keeping exported services up.");
+ queue_foreach(client->all_notify_clients, clear_notify_id,
+ NULL);
+ queue_foreach(client->services, cancel_ops, client->gatt);
+ }
bt_gatt_client_unref(client->gatt);
client->gatt = NULL;
--
2.2.0.rc0.207.ga3a616c
shared/gatt-client clears a given gatt-db if there's an error during its
init sequence, even if the given gatt-db was previously populated (e.g.
from a cache). This is to make sure that the database contents are at no
point invalid.
This patch removes a check for bt_gatt_client_is_ready and the
corresponding early-return from btd_device's service_removed handler, so
that other layers can be notified of invalidated gatt_db_attribute
pointers.
---
src/device.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/src/device.c b/src/device.c
index 7c8ae74..29a8f23 100644
--- a/src/device.c
+++ b/src/device.c
@@ -2927,8 +2927,21 @@ static void gatt_service_removed(struct gatt_db_attribute *attr,
bt_uuid_t uuid;
char uuid_str[MAX_LEN_UUID_STR];
- if (!bt_gatt_client_is_ready(device->client))
- return;
+ /*
+ * NOTE: shared/gatt-client clears the database in case of failure. This
+ * triggers the service_removed callback for all affected services.
+ * Hence, this function will be called in the following cases:
+ *
+ * 1. When a GATT service gets removed due to "Service Changed".
+ *
+ * 2. When a GATT service gets removed when the database get cleared
+ * upon disconnection with a non-bonded device.
+ *
+ * 3. When a GATT service gets removed when the database get cleared
+ * by shared/gatt-client when its initialization procedure fails,
+ * e.g. due to an ATT protocol error or an unexpected disconnect.
+ * In this case the gatt-client will not be ready.
+ */
gatt_db_attribute_get_service_data(attr, &start, &end, NULL, &uuid);
--
2.2.0.rc0.207.ga3a616c
This patch implements the StopNotify method of
org.bluez.GattCharacteristic1.
---
src/gatt-client.c | 21 +++++++++++++++++++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/src/gatt-client.c b/src/gatt-client.c
index 4f5022a..e157ac2 100644
--- a/src/gatt-client.c
+++ b/src/gatt-client.c
@@ -1163,8 +1163,25 @@ static DBusMessage *characteristic_start_notify(DBusConnection *conn,
static DBusMessage *characteristic_stop_notify(DBusConnection *conn,
DBusMessage *msg, void *user_data)
{
- /* TODO: Implement */
- return btd_error_failed(msg, "Not implemented");
+ struct characteristic *chrc = user_data;
+ struct bt_gatt_client *gatt = chrc->service->client->gatt;
+ const char *sender = dbus_message_get_sender(msg);
+ struct notify_client *client;
+
+ if (!chrc->notifying)
+ return btd_error_failed(msg, "Not notifying");
+
+ client = queue_remove_if(chrc->notify_clients, match_notify_sender,
+ (void *) sender);
+ if (!client)
+ return btd_error_failed(msg, "No notify session started");
+
+ bt_gatt_client_unregister_notify(gatt, client->notify_id);
+ update_notifying(chrc);
+
+ notify_client_unref(client);
+
+ return dbus_message_new_method_return(msg);
}
static void append_desc_path(void *data, void *user_data)
--
2.2.0.rc0.207.ga3a616c
This patch implements the StartNotify method of
org.bluez.GattCharacteristic1. Each call to StartNotify assigns a
session to the D-Bus sender which internally registers a unique
notify handler with the bt_gatt_client. Each received notification
or indication causes a PropertiesChanged signal to be emitted for the
"Value" property.
The notify handler gets automatically unregistered when sender
disconnects from D-Bus.
---
src/gatt-client.c | 254 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 245 insertions(+), 9 deletions(-)
diff --git a/src/gatt-client.c b/src/gatt-client.c
index cb8ddf6..4f5022a 100644
--- a/src/gatt-client.c
+++ b/src/gatt-client.c
@@ -88,6 +88,9 @@ struct characteristic {
unsigned int write_id;
struct queue *descs;
+
+ bool notifying;
+ struct queue *notify_clients;
};
struct descriptor {
@@ -231,7 +234,9 @@ static void async_dbus_op_free(void *data)
{
struct async_dbus_op *op = data;
- dbus_message_unref(op->msg);
+ if (op->msg)
+ dbus_message_unref(op->msg);
+
free(op);
}
@@ -689,12 +694,8 @@ static gboolean characteristic_value_exists(const GDBusPropertyTable *property,
static gboolean characteristic_get_notifying(const GDBusPropertyTable *property,
DBusMessageIter *iter, void *data)
{
- dbus_bool_t notifying = FALSE;
-
- /*
- * TODO: Return the correct value here once StartNotify and StopNotify
- * methods are implemented.
- */
+ struct characteristic *chrc = data;
+ dbus_bool_t notifying = chrc->notifying ? TRUE : FALSE;
dbus_message_iter_append_basic(iter, DBUS_TYPE_BOOLEAN, ¬ifying);
@@ -925,11 +926,238 @@ fail:
return btd_error_not_supported(msg);
}
+struct notify_client {
+ struct characteristic *chrc;
+ int ref_count;
+ char *owner;
+ guint watch;
+ unsigned int notify_id;
+};
+
+static void notify_client_free(struct notify_client *client)
+{
+ DBG("owner %s", client->owner);
+
+ g_dbus_remove_watch(btd_get_dbus_connection(), client->watch);
+ bt_gatt_client_unregister_notify(client->chrc->service->client->gatt,
+ client->notify_id);
+ free(client->owner);
+ free(client);
+}
+
+static void notify_client_unref(void *data)
+{
+ struct notify_client *client = data;
+
+ DBG("owner %s", client->owner);
+
+ if (__sync_sub_and_fetch(&client->ref_count, 1))
+ return;
+
+ notify_client_free(client);
+}
+
+static struct notify_client *notify_client_ref(struct notify_client *client)
+{
+ DBG("owner %s", client->owner);
+
+ __sync_fetch_and_add(&client->ref_count, 1);
+
+ return client;
+}
+
+static bool match_notifying(const void *a, const void *b)
+{
+ const struct notify_client *client = a;
+
+ return !!client->notify_id;
+}
+
+static void update_notifying(struct characteristic *chrc)
+{
+ if (!chrc->notifying)
+ return;
+
+ if (queue_find(chrc->notify_clients, match_notifying, NULL))
+ return;
+
+ chrc->notifying = false;
+
+ g_dbus_emit_property_changed(btd_get_dbus_connection(), chrc->path,
+ GATT_CHARACTERISTIC_IFACE,
+ "Notifying");
+}
+
+static void notify_client_disconnect(DBusConnection *conn, void *user_data)
+{
+ struct notify_client *client = user_data;
+ struct characteristic *chrc = client->chrc;
+ struct bt_gatt_client *gatt = chrc->service->client->gatt;
+
+ DBG("owner %s", client->owner);
+
+ queue_remove(chrc->notify_clients, client);
+ bt_gatt_client_unregister_notify(gatt, client->notify_id);
+
+ update_notifying(chrc);
+
+ notify_client_unref(client);
+}
+
+static struct notify_client *notify_client_create(struct characteristic *chrc,
+ const char *owner)
+{
+ struct notify_client *client;
+
+ client = new0(struct notify_client, 1);
+ if (!client)
+ return NULL;
+
+ client->chrc = chrc;
+ client->owner = strdup(owner);
+ if (!client->owner) {
+ free(client);
+ return NULL;
+ }
+
+ client->watch = g_dbus_add_disconnect_watch(btd_get_dbus_connection(),
+ owner, notify_client_disconnect,
+ client, NULL);
+ if (!client->watch) {
+ free(client->owner);
+ free(client);
+ return NULL;
+ }
+
+ return notify_client_ref(client);
+}
+
+static bool match_notify_sender(const void *a, const void *b)
+{
+ const struct notify_client *client = a;
+ const char *sender = b;
+
+ return strcmp(client->owner, sender) == 0;
+}
+
+static void notify_cb(uint16_t value_handle, const uint8_t *value,
+ uint16_t length, void *user_data)
+{
+ struct async_dbus_op *op = user_data;
+ struct notify_client *client = op->data;
+ struct characteristic *chrc = client->chrc;
+
+ /*
+ * Even if the value didn't change, we want to send a PropertiesChanged
+ * signal so that we propagate the notification/indication to
+ * applications.
+ */
+ gatt_db_attribute_reset(chrc->attr);
+ gatt_db_attribute_write(chrc->attr, 0, value, length, 0, NULL,
+ write_characteristic_cb, chrc);
+}
+
+static void register_notify_cb(uint16_t att_ecode, void *user_data)
+{
+ struct async_dbus_op *op = user_data;
+ struct notify_client *client = op->data;
+ struct characteristic *chrc = client->chrc;
+ DBusMessage *reply;
+
+ /* Make sure that an interim disconnect did not remove the client */
+ if (!queue_find(chrc->notify_clients, NULL, client)) {
+ bt_gatt_client_unregister_notify(chrc->service->client->gatt,
+ client->notify_id);
+ notify_client_unref(client);
+
+ reply = btd_error_failed(op->msg,
+ "Characteristic not available");
+ goto done;
+ }
+
+ /*
+ * Drop the reference count that we added when registering the callback.
+ */
+ notify_client_unref(client);
+
+ if (att_ecode) {
+ queue_remove(chrc->notify_clients, client);
+ notify_client_free(client);
+
+ reply = create_gatt_dbus_error(op->msg, att_ecode);
+
+ goto done;
+ }
+
+ if (!chrc->notifying) {
+ chrc->notifying = true;
+ g_dbus_emit_property_changed(btd_get_dbus_connection(),
+ chrc->path, GATT_CHARACTERISTIC_IFACE,
+ "Notifying");
+ }
+
+ reply = g_dbus_create_reply(op->msg, DBUS_TYPE_INVALID);
+
+done:
+ if (reply)
+ g_dbus_send_message(btd_get_dbus_connection(), reply);
+ else
+ error("Failed to construct D-Bus message reply");
+}
+
static DBusMessage *characteristic_start_notify(DBusConnection *conn,
DBusMessage *msg, void *user_data)
{
- /* TODO: Implement */
- return btd_error_failed(msg, "Not implemented");
+ struct characteristic *chrc = user_data;
+ struct bt_gatt_client *gatt = chrc->service->client->gatt;
+ const char *sender = dbus_message_get_sender(msg);
+ struct async_dbus_op *op;
+ struct notify_client *client;
+
+ if (!(chrc->props & BT_GATT_CHRC_PROP_NOTIFY ||
+ chrc->props & BT_GATT_CHRC_PROP_INDICATE))
+ return btd_error_not_supported(msg);
+
+ /* Each client can only have one active notify session. */
+ client = queue_find(chrc->notify_clients, match_notify_sender, sender);
+ if (client)
+ return client->notify_id ?
+ btd_error_failed(msg, "Already notifying") :
+ btd_error_in_progress(msg);
+
+ client = notify_client_create(chrc, sender);
+ if (!client)
+ return btd_error_failed(msg, "Failed allocate notify session");
+
+ op = new0(struct async_dbus_op, 1);
+ if (!op) {
+ notify_client_unref(client);
+ return btd_error_failed(msg, "Failed to initialize request");
+ }
+
+ /*
+ * Add to the ref count so that a disconnect during register won't free
+ * the client instance.
+ */
+ op->data = notify_client_ref(client);
+ op->msg = dbus_message_ref(msg);
+
+ queue_push_tail(chrc->notify_clients, client);
+
+ client->notify_id = bt_gatt_client_register_notify(gatt,
+ chrc->value_handle,
+ register_notify_cb, notify_cb,
+ op, async_dbus_op_free);
+ if (client->notify_id)
+ return NULL;
+
+ queue_remove(chrc->notify_clients, client);
+ async_dbus_op_free(op);
+
+ /* Directly free the client */
+ notify_client_free(client);
+
+ return btd_error_failed(msg, "Failed to register notify session");
}
static DBusMessage *characteristic_stop_notify(DBusConnection *conn,
@@ -1022,6 +1250,13 @@ static struct characteristic *characteristic_create(
return NULL;
}
+ chrc->notify_clients = queue_new();
+ if (!chrc->notify_clients) {
+ queue_destroy(chrc->descs, NULL);
+ free(chrc);
+ return NULL;
+ }
+
chrc->service = service;
gatt_db_attribute_get_char_data(attr, &chrc->handle,
@@ -1064,6 +1299,7 @@ static void unregister_characteristic(void *data)
if (chrc->write_id)
bt_gatt_client_cancel(gatt, chrc->write_id);
+ queue_remove_all(chrc->notify_clients, NULL, NULL, notify_client_unref);
queue_remove_all(chrc->descs, NULL, NULL, unregister_descriptor);
g_dbus_unregister_interface(btd_get_dbus_connection(), chrc->path,
--
2.2.0.rc0.207.ga3a616c
This patch makes CCC writes via bt_gatt_client_register_notify
cancellable. The following changes have been introduced:
1. bt_gatt_client_register_notify now returns the id immediately
instead of returning it in a callback. The callback is still
used to communicate ATT protocol errors.
2. A notify callback is immediately registered, so that if the
remote end sends any ATT notifications/indications, the caller
will start receiving them right away.
---
src/shared/gatt-client.c | 129 ++++++++++++++++++++++++++++-------------------
src/shared/gatt-client.h | 7 ++-
tools/btgatt-client.c | 17 ++++---
3 files changed, 91 insertions(+), 62 deletions(-)
diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index 04fb4cb..cbc5382 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -168,9 +168,10 @@ struct notify_data {
struct bt_gatt_client *client;
bool invalid;
unsigned int id;
+ unsigned int att_id;
int ref_count;
struct notify_chrc *chrc;
- bt_gatt_client_notify_id_callback_t callback;
+ bt_gatt_client_register_callback_t callback;
bt_gatt_client_notify_callback_t notify;
void *user_data;
bt_gatt_client_destroy_func_t destroy;
@@ -1011,22 +1012,20 @@ struct service_changed_op {
uint16_t end_handle;
};
-static void service_changed_reregister_cb(unsigned int id, uint16_t att_ecode,
- void *user_data)
+static void service_changed_reregister_cb(uint16_t att_ecode, void *user_data)
{
struct bt_gatt_client *client = user_data;
- if (!id || att_ecode) {
+ if (!att_ecode) {
util_debug(client->debug_callback, client->debug_data,
- "Failed to register handler for \"Service Changed\"");
+ "Re-registered handler for \"Service Changed\" after "
+ "change in GATT service");
return;
}
- client->svc_chngd_ind_id = id;
-
util_debug(client->debug_callback, client->debug_data,
- "Re-registered handler for \"Service Changed\" after change in "
- "GATT service");
+ "Failed to register handler for \"Service Changed\"");
+ client->svc_chngd_ind_id = 0;
}
static void process_service_changed(struct bt_gatt_client *client,
@@ -1090,11 +1089,12 @@ static void service_changed_complete(struct discovery_op *op, bool success,
/* The GATT service was modified. Re-register the handler for
* indications from the "Service Changed" characteristic.
*/
- if (bt_gatt_client_register_notify(client,
+ client->svc_chngd_ind_id = bt_gatt_client_register_notify(client,
gatt_db_attribute_get_handle(attr),
service_changed_reregister_cb,
service_changed_cb,
- client, NULL))
+ client, NULL);
+ if (client->svc_chngd_ind_id)
return;
util_debug(client->debug_callback, client->debug_data,
@@ -1184,24 +1184,24 @@ static void service_changed_cb(uint16_t value_handle, const uint8_t *value,
queue_push_tail(client->svc_chngd_queue, op);
}
-static void service_changed_register_cb(unsigned int id, uint16_t att_ecode,
- void *user_data)
+static void service_changed_register_cb(uint16_t att_ecode, void *user_data)
{
bool success;
struct bt_gatt_client *client = user_data;
- if (!id || att_ecode) {
+ if (att_ecode) {
util_debug(client->debug_callback, client->debug_data,
"Failed to register handler for \"Service Changed\"");
success = false;
+ client->svc_chngd_ind_id = 0;
goto done;
}
- client->svc_chngd_ind_id = id;
client->ready = true;
success = true;
util_debug(client->debug_callback, client->debug_data,
- "Registered handler for \"Service Changed\": %u", id);
+ "Registered handler for \"Service Changed\": %u",
+ client->svc_chngd_ind_id);
done:
notify_client_ready(client, success, att_ecode);
@@ -1211,7 +1211,6 @@ static void init_complete(struct discovery_op *op, bool success,
uint8_t att_ecode)
{
struct bt_gatt_client *client = op->client;
- bool registered;
struct gatt_db_attribute *attr = NULL;
bt_uuid_t uuid;
@@ -1235,14 +1234,14 @@ static void init_complete(struct discovery_op *op, bool success,
* the handler using the existing framework.
*/
client->ready = true;
- registered = bt_gatt_client_register_notify(client,
+ client->svc_chngd_ind_id = bt_gatt_client_register_notify(client,
gatt_db_attribute_get_handle(attr),
service_changed_register_cb,
service_changed_cb,
client, NULL);
client->ready = false;
- if (registered)
+ if (client->svc_chngd_ind_id)
return;
util_debug(client->debug_callback, client->debug_data,
@@ -1301,24 +1300,15 @@ static void complete_notify_request(void *data)
/* Increment the per-characteristic ref count of notify handlers */
__sync_fetch_and_add(¬ify_data->chrc->notify_count, 1);
- /* Add the handler to the bt_gatt_client's general list */
- queue_push_tail(notify_data->client->notify_list,
- notify_data_ref(notify_data));
-
- /* Assign an ID to the handler and notify the caller that it was
- * successfully registered.
- */
- if (notify_data->client->next_reg_id < 1)
- notify_data->client->next_reg_id = 1;
-
- notify_data->id = notify_data->client->next_reg_id++;
- notify_data->callback(notify_data->id, 0, notify_data->user_data);
+ notify_data->att_id = 0;
+ notify_data->callback(0, notify_data->user_data);
}
static bool notify_data_write_ccc(struct notify_data *notify_data, bool enable,
bt_att_response_func_t callback)
{
uint8_t pdu[4];
+ unsigned int att_id;
assert(notify_data->chrc->ccc_handle);
memset(pdu, 0, sizeof(pdu));
@@ -1338,13 +1328,13 @@ static bool notify_data_write_ccc(struct notify_data *notify_data, bool enable,
return false;
}
- notify_data->chrc->ccc_write_id = bt_att_send(notify_data->client->att,
- BT_ATT_OP_WRITE_REQ,
- pdu, sizeof(pdu),
- callback,
- notify_data, notify_data_unref);
+ att_id = bt_att_send(notify_data->client->att, BT_ATT_OP_WRITE_REQ,
+ pdu, sizeof(pdu), callback,
+ notify_data_ref(notify_data),
+ notify_data_unref);
+ notify_data->chrc->ccc_write_id = notify_data->att_id = att_id;
- return !!notify_data->chrc->ccc_write_id;
+ return !!att_id;
}
static uint8_t process_error(const void *pdu, uint16_t length)
@@ -1377,7 +1367,8 @@ static void enable_ccc_callback(uint8_t opcode, const void *pdu,
* the next one in the queue. If there was an error sending the
* write request, then just move on to the next queued entry.
*/
- notify_data->callback(0, att_ecode, notify_data->user_data);
+ queue_remove(notify_data->client->notify_list, notify_data);
+ notify_data->callback(att_ecode, notify_data->user_data);
while ((notify_data = queue_pop_head(
notify_data->chrc->reg_notify_queue))) {
@@ -1426,6 +1417,16 @@ static void complete_unregister_notify(void *data)
{
struct notify_data *notify_data = data;
+ /*
+ * If a procedure to enable the CCC is still pending, then cancel it and
+ * return.
+ */
+ if (notify_data->att_id) {
+ bt_att_cancel(notify_data->client->att, notify_data->att_id);
+ notify_data_unref(notify_data);
+ return;
+ }
+
if (__sync_sub_and_fetch(¬ify_data->chrc->notify_count, 1)) {
notify_data_unref(notify_data);
return;
@@ -1449,6 +1450,10 @@ static void notify_handler(void *data, void *user_data)
if (pdu_data->length > 2)
value = pdu_data->pdu + 2;
+ /*
+ * Even if the notify data has a pending ATT request to write to the
+ * CCC, there is really no reason not to notify the handlers.
+ */
if (notify_data->notify)
notify_data->notify(value_handle, value, pdu_data->length - 2,
notify_data->user_data);
@@ -1475,10 +1480,22 @@ static void notify_cb(uint8_t opcode, const void *pdu, uint16_t length,
bt_gatt_client_unref(client);
}
+static void notify_data_cleanup(void *data)
+{
+ struct notify_data *notify_data = data;
+
+ if (notify_data->att_id)
+ bt_att_cancel(notify_data->client->att, notify_data->att_id);
+
+ notify_data_unref(notify_data);
+}
+
static void bt_gatt_client_free(struct bt_gatt_client *client)
{
bt_gatt_client_cancel_all(client);
+ queue_destroy(client->notify_list, notify_data_cleanup);
+
if (client->ready_destroy)
client->ready_destroy(client->ready_data);
@@ -1496,7 +1513,6 @@ static void bt_gatt_client_free(struct bt_gatt_client *client)
queue_destroy(client->svc_chngd_queue, free);
queue_destroy(client->long_write_queue, request_unref);
- queue_destroy(client->notify_list, notify_data_unref);
queue_destroy(client->notify_chrcs, notify_chrc_free);
queue_destroy(client->pending_requests, request_unref);
@@ -2569,9 +2585,9 @@ static bool match_notify_chrc_value_handle(const void *a, const void *b)
return chrc->value_handle == value_handle;
}
-bool bt_gatt_client_register_notify(struct bt_gatt_client *client,
+unsigned int bt_gatt_client_register_notify(struct bt_gatt_client *client,
uint16_t chrc_value_handle,
- bt_gatt_client_notify_id_callback_t callback,
+ bt_gatt_client_register_callback_t callback,
bt_gatt_client_notify_callback_t notify,
void *user_data,
bt_gatt_client_destroy_func_t destroy)
@@ -2580,10 +2596,10 @@ bool bt_gatt_client_register_notify(struct bt_gatt_client *client,
struct notify_chrc *chrc = NULL;
if (!client || !client->db || !chrc_value_handle || !callback)
- return false;
+ return 0;
if (!bt_gatt_client_is_ready(client) || client->in_svc_chngd)
- return false;
+ return 0;
/* Check if a characteristic ref count has been started already */
chrc = queue_find(client->notify_chrcs, match_notify_chrc_value_handle,
@@ -2596,17 +2612,16 @@ bool bt_gatt_client_register_notify(struct bt_gatt_client *client,
*/
chrc = notify_chrc_create(client, chrc_value_handle);
if (!chrc)
- return false;
-
+ return 0;
}
/* Fail if we've hit the maximum allowed notify sessions */
if (chrc->notify_count == INT_MAX)
- return false;
+ return 0;
notify_data = new0(struct notify_data, 1);
if (!notify_data)
- return false;
+ return 0;
notify_data->client = client;
notify_data->ref_count = 1;
@@ -2616,13 +2631,24 @@ bool bt_gatt_client_register_notify(struct bt_gatt_client *client,
notify_data->user_data = user_data;
notify_data->destroy = destroy;
+ /* Add the handler to the bt_gatt_client's general list */
+ queue_push_tail(client->notify_list, notify_data);
+
+ /* Assign an ID to the handler and notify the caller that it was
+ * successfully registered.
+ */
+ if (client->next_reg_id < 1)
+ client->next_reg_id = 1;
+
+ notify_data->id = client->next_reg_id++;
+
/*
* If a write to the CCC descriptor is in progress, then queue this
* request.
*/
if (chrc->ccc_write_id) {
queue_push_tail(chrc->reg_notify_queue, notify_data);
- return true;
+ return notify_data->id;
}
/*
@@ -2630,16 +2656,17 @@ bool bt_gatt_client_register_notify(struct bt_gatt_client *client,
*/
if (chrc->notify_count > 0) {
complete_notify_request(notify_data);
- return true;
+ return notify_data->id;
}
/* Write to the CCC descriptor */
if (!notify_data_write_ccc(notify_data, true, enable_ccc_callback)) {
+ queue_remove(client->notify_list, notify_data);
free(notify_data);
- return false;
+ return 0;
}
- return true;
+ return notify_data->id;
}
bool bt_gatt_client_unregister_notify(struct bt_gatt_client *client,
diff --git a/src/shared/gatt-client.h b/src/shared/gatt-client.h
index 9a00feb..b84fa13 100644
--- a/src/shared/gatt-client.h
+++ b/src/shared/gatt-client.h
@@ -49,8 +49,7 @@ typedef void (*bt_gatt_client_write_long_callback_t)(bool success,
typedef void (*bt_gatt_client_notify_callback_t)(uint16_t value_handle,
const uint8_t *value, uint16_t length,
void *user_data);
-typedef void (*bt_gatt_client_notify_id_callback_t)(unsigned int id,
- uint16_t att_ecode,
+typedef void (*bt_gatt_client_register_callback_t)(uint16_t att_ecode,
void *user_data);
typedef void (*bt_gatt_client_service_changed_callback_t)(uint16_t start_handle,
uint16_t end_handle,
@@ -110,9 +109,9 @@ unsigned int bt_gatt_client_write_long_value(struct bt_gatt_client *client,
void *user_data,
bt_gatt_client_destroy_func_t destroy);
-bool bt_gatt_client_register_notify(struct bt_gatt_client *client,
+unsigned int bt_gatt_client_register_notify(struct bt_gatt_client *client,
uint16_t chrc_value_handle,
- bt_gatt_client_notify_id_callback_t callback,
+ bt_gatt_client_register_callback_t callback,
bt_gatt_client_notify_callback_t notify,
void *user_data,
bt_gatt_client_destroy_func_t destroy);
diff --git a/tools/btgatt-client.c b/tools/btgatt-client.c
index 62c4d3e..8bda89b 100644
--- a/tools/btgatt-client.c
+++ b/tools/btgatt-client.c
@@ -856,16 +856,15 @@ static void notify_cb(uint16_t value_handle, const uint8_t *value,
PRLOG("\n");
}
-static void register_notify_cb(unsigned int id, uint16_t att_ecode,
- void *user_data)
+static void register_notify_cb(uint16_t att_ecode, void *user_data)
{
- if (!id) {
+ if (att_ecode) {
PRLOG("Failed to register notify handler "
"- error code: 0x%02x\n", att_ecode);
return;
}
- PRLOG("Registered notify handler with id: %u\n", id);
+ PRLOG("Registered notify handler!");
}
static void cmd_register_notify(struct client *cli, char *cmd_str)
@@ -873,6 +872,7 @@ static void cmd_register_notify(struct client *cli, char *cmd_str)
char *argv[2];
int argc = 0;
uint16_t value_handle;
+ unsigned int id;
char *endptr = NULL;
if (!bt_gatt_client_is_ready(cli->gatt)) {
@@ -891,12 +891,15 @@ static void cmd_register_notify(struct client *cli, char *cmd_str)
return;
}
- if (!bt_gatt_client_register_notify(cli->gatt, value_handle,
+ id = bt_gatt_client_register_notify(cli->gatt, value_handle,
register_notify_cb,
- notify_cb, NULL, NULL))
+ notify_cb, NULL, NULL);
+ if (!id) {
printf("Failed to register notify handler\n");
+ return;
+ }
- printf("\n");
+ PRLOG("Registering notify handler with id: %u\n", id);
}
static void unregister_notify_usage(void)
--
2.2.0.rc0.207.ga3a616c