All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/5] Add read/write callbacks to attribute server
@ 2011-02-21 21:30 Anderson Lizardo
  2011-02-21 21:30 ` [PATCH 2/5] Initial Client Characteristic Configuration implementation Anderson Lizardo
                   ` (8 more replies)
  0 siblings, 9 replies; 21+ messages in thread
From: Anderson Lizardo @ 2011-02-21 21:30 UTC (permalink / raw
  To: linux-bluetooth; +Cc: Anderson Lizardo

These callbacks will allow profiles to act before an attribute is read
and after it is written, to e.g. update the attribute value to/from an
external source.

Note that by the time the callback is called, the necessary security
checks (attribute permissions, authentication and encryption) were
already performed by the core attribute server.

The callback can optionally return an ATT status code, which will be
sent to the client using an Error Response PDU.
---
 attrib/att.h        |    7 +++++++
 src/attrib-server.c |   29 ++++++++++++++++++++++++++---
 src/attrib-server.h |    4 ++--
 3 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/attrib/att.h b/attrib/att.h
index 7d9afeb..05ae606 100644
--- a/attrib/att.h
+++ b/attrib/att.h
@@ -118,11 +118,18 @@ enum {
 	ATT_NOT_PERMITTED,	/* Operation not permitted */
 };
 
+struct attribute;
+
+typedef uint8_t (*att_cb_t)(struct attribute *, gpointer);
+
 struct attribute {
 	uint16_t handle;
 	uuid_t uuid;
 	int read_reqs;
 	int write_reqs;
+	att_cb_t read_cb;
+	att_cb_t write_cb;
+	gpointer cb_user_data;
 	int len;
 	uint8_t data[0];
 };
diff --git a/src/attrib-server.c b/src/attrib-server.c
index 7ec0f56..28649f1 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -229,6 +229,10 @@ static uint16_t read_by_group(struct gatt_channel *channel, uint16_t start,
 
 		status = att_check_reqs(channel, ATT_OP_READ_BY_GROUP_REQ,
 								a->read_reqs);
+
+		if (status == 0x00 && a->read_cb)
+			status = a->read_cb(a, a->cb_user_data);
+
 		if (status) {
 			g_slist_foreach(groups, (GFunc) g_free, NULL);
 			g_slist_free(groups);
@@ -317,6 +321,10 @@ static uint16_t read_by_type(struct gatt_channel *channel, uint16_t start,
 
 		status = att_check_reqs(channel, ATT_OP_READ_BY_TYPE_REQ,
 								a->read_reqs);
+
+		if (status == 0x00 && a->read_cb)
+			status = a->read_cb(a, a->cb_user_data);
+
 		if (status) {
 			g_slist_free(types);
 			return enc_error_resp(ATT_OP_READ_BY_TYPE_REQ,
@@ -564,6 +572,10 @@ static uint16_t read_value(struct gatt_channel *channel, uint16_t handle,
 	a = l->data;
 
 	status = att_check_reqs(channel, ATT_OP_READ_REQ, a->read_reqs);
+
+	if (status == 0x00 && a->read_cb)
+		status = a->read_cb(a, a->cb_user_data);
+
 	if (status)
 		return enc_error_resp(ATT_OP_READ_REQ, handle, status, pdu,
 									len);
@@ -591,6 +603,10 @@ static uint16_t read_blob(struct gatt_channel *channel, uint16_t handle,
 					ATT_ECODE_INVALID_OFFSET, pdu, len);
 
 	status = att_check_reqs(channel, ATT_OP_READ_BLOB_REQ, a->read_reqs);
+
+	if (status == 0x00 && a->read_cb)
+		status = a->read_cb(a, a->cb_user_data);
+
 	if (status)
 		return enc_error_resp(ATT_OP_READ_BLOB_REQ, handle, status,
 								pdu, len);
@@ -623,6 +639,13 @@ static uint16_t write_value(struct gatt_channel *channel, uint16_t handle,
 	memcpy(&uuid, &a->uuid, sizeof(uuid_t));
 	attrib_db_update(handle, &uuid, value, vlen);
 
+	if (a->write_cb) {
+		status = a->write_cb(a, a->cb_user_data);
+		if (status)
+			return enc_error_resp(ATT_OP_WRITE_REQ, handle, status,
+								pdu, len);
+	}
+
 	return enc_write_resp(pdu, len);
 }
 
@@ -1013,8 +1036,8 @@ void attrib_free_sdp(uint32_t sdp_handle)
 	remove_record_from_server(sdp_handle);
 }
 
-int attrib_db_add(uint16_t handle, uuid_t *uuid, int read_reqs, int write_reqs,
-						const uint8_t *value, int len)
+struct attribute *attrib_db_add(uint16_t handle, uuid_t *uuid, int read_reqs,
+				int write_reqs, const uint8_t *value, int len)
 {
 	struct attribute *a;
 
@@ -1030,7 +1053,7 @@ int attrib_db_add(uint16_t handle, uuid_t *uuid, int read_reqs, int write_reqs,
 
 	database = g_slist_insert_sorted(database, a, attribute_cmp);
 
-	return 0;
+	return a;
 }
 
 int attrib_db_update(uint16_t handle, uuid_t *uuid, const uint8_t *value,
diff --git a/src/attrib-server.h b/src/attrib-server.h
index ecd695c..85f3bdb 100644
--- a/src/attrib-server.h
+++ b/src/attrib-server.h
@@ -25,8 +25,8 @@
 int attrib_server_init(void);
 void attrib_server_exit(void);
 
-int attrib_db_add(uint16_t handle, uuid_t *uuid, int read_reqs, int write_reqs,
-						const uint8_t *value, int len);
+struct attribute *attrib_db_add(uint16_t handle, uuid_t *uuid, int read_reqs,
+				int write_reqs, const uint8_t *value, int len);
 int attrib_db_update(uint16_t handle, uuid_t *uuid, const uint8_t *value,
 								int len);
 int attrib_db_del(uint16_t handle);
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 2/5] Initial Client Characteristic Configuration implementation
  2011-02-21 21:30 [PATCH 1/5] Add read/write callbacks to attribute server Anderson Lizardo
@ 2011-02-21 21:30 ` Anderson Lizardo
  2011-02-21 21:30 ` [PATCH 3/5] Check permissions before setting client configs Anderson Lizardo
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 21+ messages in thread
From: Anderson Lizardo @ 2011-02-21 21:30 UTC (permalink / raw
  To: linux-bluetooth; +Cc: Anderson Lizardo

This initial version creates a private copy of the Client Configuration
attribute for each client on the first write to that attribute. Further
reads/writes should use the client's private copy.

Two list of attributes are created: one for values to be indicated and
another for values to be notified. The actual notification/indication
sending is implemented in a separate commit.

This initial version also does not check for the characteristic
properties, which is implemented in a separate commit as well.
---
 src/attrib-server.c |  177 ++++++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 146 insertions(+), 31 deletions(-)

diff --git a/src/attrib-server.c b/src/attrib-server.c
index 28649f1..be71621 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -54,6 +54,7 @@ static GSList *database = NULL;
 struct gatt_channel {
 	bdaddr_t src;
 	bdaddr_t dst;
+	GSList *configs, *notify, *indicate;
 	GAttrib *attrib;
 	guint mtu;
 	guint id;
@@ -143,6 +144,22 @@ static sdp_record_t *server_record_new(uuid_t *uuid, uint16_t start, uint16_t en
 	return record;
 }
 
+static int handle_cmp(gconstpointer a, gconstpointer b)
+{
+	const struct attribute *attrib = a;
+	uint16_t handle = GPOINTER_TO_UINT(b);
+
+	return attrib->handle - handle;
+}
+
+static int attribute_cmp(gconstpointer a1, gconstpointer a2)
+{
+	const struct attribute *attrib1 = a1;
+	const struct attribute *attrib2 = a2;
+
+	return attrib1->handle - attrib2->handle;
+}
+
 static uint8_t att_check_reqs(struct gatt_channel *channel, uint8_t opcode,
 								int reqs)
 {
@@ -174,6 +191,91 @@ static uint8_t att_check_reqs(struct gatt_channel *channel, uint8_t opcode,
 	return 0;
 }
 
+static uint8_t client_set_notifications(struct attribute *attr,
+							gpointer user_data)
+{
+	struct gatt_channel *channel = user_data;
+	struct attribute *a, *last_chr_val = NULL;
+	uint16_t handle, cfg_val;
+	uuid_t uuid;
+	GSList *l;
+
+	cfg_val = att_get_u16(attr->data);
+
+	sdp_uuid16_create(&uuid, GATT_CHARAC_UUID);
+	for (l = database, handle = 0; l != NULL; l = l->next) {
+		a = l->data;
+
+		if (a->handle >= attr->handle)
+			break;
+
+		if (sdp_uuid_cmp(&a->uuid, &uuid) == 0) {
+			handle = att_get_u16(&a->data[1]);
+			continue;
+		}
+
+		if (handle && a->handle == handle)
+			last_chr_val = a;
+	}
+
+	if (last_chr_val == NULL)
+		return 0;
+
+	/* FIXME: Characteristic properties shall be checked for
+	 * Notification/Indication permissions. */
+
+	if (cfg_val & 0x0001)
+		channel->notify = g_slist_append(channel->notify, last_chr_val);
+	else
+		channel->notify = g_slist_remove(channel->notify, last_chr_val);
+
+	if (cfg_val & 0x0002)
+		channel->indicate = g_slist_append(channel->indicate,
+								last_chr_val);
+	else
+		channel->indicate = g_slist_remove(channel->indicate,
+								last_chr_val);
+
+	return 0;
+}
+
+static struct attribute *client_cfg_attribute(struct gatt_channel *channel,
+		struct attribute *orig_attr, const uint8_t *value, int vlen)
+{
+	uuid_t uuid;
+	GSList *l;
+	guint handle = orig_attr->handle;
+
+	sdp_uuid16_create(&uuid, GATT_CLIENT_CHARAC_CFG_UUID);
+	if (sdp_uuid_cmp(&orig_attr->uuid, &uuid) != 0)
+		return NULL;
+
+	/* Value is unchanged, not need to create a private copy yet */
+	if (vlen == orig_attr->len && memcmp(orig_attr->data, value, vlen) == 0)
+		return orig_attr;
+
+	l = g_slist_find_custom(channel->configs, GUINT_TO_POINTER(handle),
+								handle_cmp);
+	if (!l) {
+		struct attribute *a;
+
+		/* Create a private copy of the Client Characteristic
+		 * Configuration attribute */
+		a = g_malloc0(sizeof(struct attribute) + vlen);
+		memcpy(a, orig_attr, sizeof(struct attribute));
+		memcpy(a->data, value, vlen);
+		a->write_cb = client_set_notifications;
+		a->cb_user_data = channel;
+
+		channel->configs = g_slist_insert_sorted(channel->configs, a,
+								attribute_cmp);
+
+		return a;
+	}
+
+	return l->data;
+}
+
 static uint16_t read_by_group(struct gatt_channel *channel, uint16_t start,
 						uint16_t end, uuid_t *uuid,
 						uint8_t *pdu, int len)
@@ -202,6 +304,8 @@ static uint16_t read_by_group(struct gatt_channel *channel, uint16_t start,
 
 	last_handle = end;
 	for (l = database, groups = NULL; l; l = l->next) {
+		struct attribute *client_attr;
+
 		a = l->data;
 
 		if (a->handle < start)
@@ -230,6 +334,10 @@ static uint16_t read_by_group(struct gatt_channel *channel, uint16_t start,
 		status = att_check_reqs(channel, ATT_OP_READ_BY_GROUP_REQ,
 								a->read_reqs);
 
+		client_attr = client_cfg_attribute(channel, a, a->data, a->len);
+		if (client_attr)
+			a = client_attr;
+
 		if (status == 0x00 && a->read_cb)
 			status = a->read_cb(a, a->cb_user_data);
 
@@ -308,6 +416,8 @@ static uint16_t read_by_type(struct gatt_channel *channel, uint16_t start,
 					ATT_ECODE_INVALID_HANDLE, pdu, len);
 
 	for (l = database, length = 0, types = NULL; l; l = l->next) {
+		struct attribute *client_attr;
+
 		a = l->data;
 
 		if (a->handle < start)
@@ -322,6 +432,10 @@ static uint16_t read_by_type(struct gatt_channel *channel, uint16_t start,
 		status = att_check_reqs(channel, ATT_OP_READ_BY_TYPE_REQ,
 								a->read_reqs);
 
+		client_attr = client_cfg_attribute(channel, a, a->data, a->len);
+		if (client_attr)
+			a = client_attr;
+
 		if (status == 0x00 && a->read_cb)
 			status = a->read_cb(a, a->cb_user_data);
 
@@ -506,22 +620,6 @@ static int find_by_type(uint16_t start, uint16_t end, uuid_t *uuid,
 	return len;
 }
 
-static int handle_cmp(gconstpointer a, gconstpointer b)
-{
-	const struct attribute *attrib = a;
-	uint16_t handle = GPOINTER_TO_UINT(b);
-
-	return attrib->handle - handle;
-}
-
-static int attribute_cmp(gconstpointer a1, gconstpointer a2)
-{
-	const struct attribute *attrib1 = a1;
-	const struct attribute *attrib2 = a2;
-
-	return attrib1->handle - attrib2->handle;
-}
-
 static struct attribute *find_primary_range(uint16_t start, uint16_t *end)
 {
 	struct attribute *attrib;
@@ -559,7 +657,7 @@ static struct attribute *find_primary_range(uint16_t start, uint16_t *end)
 static uint16_t read_value(struct gatt_channel *channel, uint16_t handle,
 							uint8_t *pdu, int len)
 {
-	struct attribute *a;
+	struct attribute *a, *client_attr;
 	uint8_t status;
 	GSList *l;
 	guint h = handle;
@@ -573,6 +671,10 @@ static uint16_t read_value(struct gatt_channel *channel, uint16_t handle,
 
 	status = att_check_reqs(channel, ATT_OP_READ_REQ, a->read_reqs);
 
+	client_attr = client_cfg_attribute(channel, a, a->data, a->len);
+	if (client_attr)
+		a = client_attr;
+
 	if (status == 0x00 && a->read_cb)
 		status = a->read_cb(a, a->cb_user_data);
 
@@ -586,7 +688,7 @@ static uint16_t read_value(struct gatt_channel *channel, uint16_t handle,
 static uint16_t read_blob(struct gatt_channel *channel, uint16_t handle,
 					uint16_t offset, uint8_t *pdu, int len)
 {
-	struct attribute *a;
+	struct attribute *a, *client_attr;
 	uint8_t status;
 	GSList *l;
 	guint h = handle;
@@ -604,6 +706,10 @@ static uint16_t read_blob(struct gatt_channel *channel, uint16_t handle,
 
 	status = att_check_reqs(channel, ATT_OP_READ_BLOB_REQ, a->read_reqs);
 
+	client_attr = client_cfg_attribute(channel, a, a->data, a->len);
+	if (client_attr)
+		a = client_attr;
+
 	if (status == 0x00 && a->read_cb)
 		status = a->read_cb(a, a->cb_user_data);
 
@@ -618,11 +724,10 @@ static uint16_t write_value(struct gatt_channel *channel, uint16_t handle,
 						const uint8_t *value, int vlen,
 						uint8_t *pdu, int len)
 {
-	struct attribute *a;
+	struct attribute *a, *client_attr;
 	uint8_t status;
 	GSList *l;
 	guint h = handle;
-	uuid_t uuid;
 
 	l = g_slist_find_custom(database, GUINT_TO_POINTER(h), handle_cmp);
 	if (!l)
@@ -636,8 +741,11 @@ static uint16_t write_value(struct gatt_channel *channel, uint16_t handle,
 		return enc_error_resp(ATT_OP_WRITE_REQ, handle, status, pdu,
 									len);
 
-	memcpy(&uuid, &a->uuid, sizeof(uuid_t));
-	attrib_db_update(handle, &uuid, value, vlen);
+	client_attr = client_cfg_attribute(channel, a, value, vlen);
+	if (client_attr)
+		a = client_attr;
+	else
+		attrib_db_update(a->handle, &a->uuid, value, vlen);
 
 	if (a->write_cb) {
 		status = a->write_cb(a, a->cb_user_data);
@@ -646,6 +754,10 @@ static uint16_t write_value(struct gatt_channel *channel, uint16_t handle,
 								pdu, len);
 	}
 
+	DBG("Notifications: %d, indications: %d",
+					g_slist_length(channel->notify),
+					g_slist_length(channel->indicate));
+
 	return enc_write_resp(pdu, len);
 }
 
@@ -664,6 +776,11 @@ static void channel_disconnect(void *user_data)
 	g_attrib_unref(channel->attrib);
 	clients = g_slist_remove(clients, channel);
 
+	g_slist_free(channel->notify);
+	g_slist_free(channel->indicate);
+	g_slist_foreach(channel->configs, (GFunc) g_free, NULL);
+	g_slist_free(channel->configs);
+
 	g_free(channel);
 }
 
@@ -976,6 +1093,11 @@ void attrib_server_exit(void)
 	for (l = clients; l; l = l->next) {
 		struct gatt_channel *channel = l->data;
 
+		g_slist_free(channel->notify);
+		g_slist_free(channel->indicate);
+		g_slist_foreach(channel->configs, (GFunc) g_free, NULL);
+		g_slist_free(channel->configs);
+
 		g_attrib_unref(channel->attrib);
 		g_free(channel);
 	}
@@ -1073,18 +1195,11 @@ int attrib_db_update(uint16_t handle, uuid_t *uuid, const uint8_t *value,
 
 	l->data = a;
 	a->handle = handle;
-	memcpy(&a->uuid, uuid, sizeof(uuid_t));
+	if (uuid != &a->uuid)
+		memcpy(&a->uuid, uuid, sizeof(uuid_t));
 	a->len = len;
 	memcpy(a->data, value, len);
 
-	/*
-	 * <<Client/Server Characteristic Configuration>> descriptors are
-	 * not supported yet. If a given attribute changes, the attribute
-	 * server shall report the new values using the mechanism selected
-	 * by the client. Notification/Indication shall not be automatically
-	 * sent if the client didn't request them.
-	 */
-
 	return 0;
 }
 
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 3/5] Check permissions before setting client configs
  2011-02-21 21:30 [PATCH 1/5] Add read/write callbacks to attribute server Anderson Lizardo
  2011-02-21 21:30 ` [PATCH 2/5] Initial Client Characteristic Configuration implementation Anderson Lizardo
@ 2011-02-21 21:30 ` Anderson Lizardo
  2011-02-21 21:30 ` [PATCH 4/5] Implement server-side ATT handle notification Anderson Lizardo
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 21+ messages in thread
From: Anderson Lizardo @ 2011-02-21 21:30 UTC (permalink / raw
  To: linux-bluetooth; +Cc: Andre Dieb Martins

From: Andre Dieb Martins <andre.dieb@signove.com>

Only enable notification/indication if the descriptor allows it.
---
 src/attrib-server.c |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/src/attrib-server.c b/src/attrib-server.c
index be71621..5675a1b 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -197,19 +197,21 @@ static uint8_t client_set_notifications(struct attribute *attr,
 	struct gatt_channel *channel = user_data;
 	struct attribute *a, *last_chr_val = NULL;
 	uint16_t handle, cfg_val;
+	uint8_t perm;
 	uuid_t uuid;
 	GSList *l;
 
 	cfg_val = att_get_u16(attr->data);
 
 	sdp_uuid16_create(&uuid, GATT_CHARAC_UUID);
-	for (l = database, handle = 0; l != NULL; l = l->next) {
+	for (l = database, handle = 0, perm = 0; l != NULL; l = l->next) {
 		a = l->data;
 
 		if (a->handle >= attr->handle)
 			break;
 
 		if (sdp_uuid_cmp(&a->uuid, &uuid) == 0) {
+			perm = att_get_u8(&a->data[0]);
 			handle = att_get_u16(&a->data[1]);
 			continue;
 		}
@@ -221,8 +223,11 @@ static uint8_t client_set_notifications(struct attribute *attr,
 	if (last_chr_val == NULL)
 		return 0;
 
-	/* FIXME: Characteristic properties shall be checked for
-	 * Notification/Indication permissions. */
+	if ((cfg_val & 0x0001) && !(perm & ATT_CHAR_PROPER_NOTIFY))
+		return ATT_ECODE_WRITE_NOT_PERM;
+
+	if ((cfg_val & 0x0002) && !(perm & ATT_CHAR_PROPER_INDICATE))
+		return ATT_ECODE_WRITE_NOT_PERM;
 
 	if (cfg_val & 0x0001)
 		channel->notify = g_slist_append(channel->notify, last_chr_val);
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 4/5] Implement server-side ATT handle notification
  2011-02-21 21:30 [PATCH 1/5] Add read/write callbacks to attribute server Anderson Lizardo
  2011-02-21 21:30 ` [PATCH 2/5] Initial Client Characteristic Configuration implementation Anderson Lizardo
  2011-02-21 21:30 ` [PATCH 3/5] Check permissions before setting client configs Anderson Lizardo
@ 2011-02-21 21:30 ` Anderson Lizardo
  2011-02-21 21:30 ` [PATCH 5/5] Implement ATT handle indication Anderson Lizardo
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 21+ messages in thread
From: Anderson Lizardo @ 2011-02-21 21:30 UTC (permalink / raw
  To: linux-bluetooth; +Cc: Andre Dieb Martins

From: Andre Dieb Martins <andre.dieb@signove.com>

Adds an initial version of ATT handle notification. Notifications are sent to
interested clients (based on the Client Characteristic Configuration) always
when the attribute is updated.

This enables services to call db_attrib_update() and send notifications on their
own terms.
---
 src/attrib-server.c |   29 +++++++++++++++++++++++++++++
 1 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/src/attrib-server.c b/src/attrib-server.c
index 5675a1b..585e1f3 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -961,6 +961,33 @@ static void confirm_event(GIOChannel *io, void *user_data)
 	return;
 }
 
+static void attrib_notify_clients(struct attribute *attr)
+{
+	uint8_t pdu[ATT_MAX_MTU];
+	guint mtu, handle = attr->handle;
+	uint16_t len;
+	GSList *l;
+
+	for (l = clients, mtu = 0, len = 0; l; l = l->next) {
+		struct gatt_channel *channel = l->data;
+
+		mtu = channel->mtu;
+		if (mtu == 0)
+			continue;
+
+		/* Notification */
+		if (g_slist_find_custom(channel->notify,
+					GUINT_TO_POINTER(handle), handle_cmp)) {
+			len = enc_notification(attr, pdu, mtu);
+			if (len == 0)
+				return;
+
+			g_attrib_send(channel->attrib, 0, pdu[0], pdu, len,
+							NULL, NULL, NULL);
+		}
+	}
+}
+
 static gboolean register_core_services(void)
 {
 	uint8_t atval[256];
@@ -1205,6 +1232,8 @@ int attrib_db_update(uint16_t handle, uuid_t *uuid, const uint8_t *value,
 	a->len = len;
 	memcpy(a->data, value, len);
 
+	attrib_notify_clients(a);
+
 	return 0;
 }
 
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 5/5] Implement ATT handle indication
  2011-02-21 21:30 [PATCH 1/5] Add read/write callbacks to attribute server Anderson Lizardo
                   ` (2 preceding siblings ...)
  2011-02-21 21:30 ` [PATCH 4/5] Implement server-side ATT handle notification Anderson Lizardo
@ 2011-02-21 21:30 ` Anderson Lizardo
  2011-02-22 21:01 ` [PATCHv2 1/5] Add read/write callbacks to attribute server Anderson Lizardo
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 21+ messages in thread
From: Anderson Lizardo @ 2011-02-21 21:30 UTC (permalink / raw
  To: linux-bluetooth; +Cc: Elvis Pfützenreuter

From: Elvis Pfützenreuter <epx@signove.com>

---
 src/attrib-server.c |   40 +++++++++++++++++++++++++++++++++++++++-
 1 files changed, 39 insertions(+), 1 deletions(-)

diff --git a/src/attrib-server.c b/src/attrib-server.c
index 585e1f3..ccb4228 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -59,6 +59,8 @@ struct gatt_channel {
 	guint mtu;
 	guint id;
 	gboolean encrypted;
+	guint outstanding_indications;
+	time_t oldest_indication;
 };
 
 struct group_elem {
@@ -885,6 +887,10 @@ static void channel_handler(const uint8_t *ipdu, uint16_t len,
 		length = find_by_type(start, end, &uuid, value, vlen,
 							opdu, channel->mtu);
 		break;
+	case ATT_OP_HANDLE_CNF:
+		if (channel->outstanding_indications)
+			channel->outstanding_indications -= 1;
+		return;
 	case ATT_OP_READ_MULTI_REQ:
 	case ATT_OP_PREP_WRITE_REQ:
 	case ATT_OP_EXEC_WRITE_REQ:
@@ -964,10 +970,14 @@ static void confirm_event(GIOChannel *io, void *user_data)
 static void attrib_notify_clients(struct attribute *attr)
 {
 	uint8_t pdu[ATT_MAX_MTU];
+	uint8_t pdu_ind[ATT_MAX_MTU];
 	guint mtu, handle = attr->handle;
-	uint16_t len;
+	uint16_t len, len_ind;
+	time_t now;
 	GSList *l;
 
+	time(&now);
+
 	for (l = clients, mtu = 0, len = 0; l; l = l->next) {
 		struct gatt_channel *channel = l->data;
 
@@ -985,6 +995,34 @@ static void attrib_notify_clients(struct attribute *attr)
 			g_attrib_send(channel->attrib, 0, pdu[0], pdu, len,
 							NULL, NULL, NULL);
 		}
+
+		/* The outstanding_indications variable counts how many
+		 * unconfirmed indications have been sent. The oldest_indication
+		 * variable gives a small time frame to accomodate
+		 * almost-concomitant indications, sending both without forcing
+		 * a serialization.
+		 *
+		 * Indications are withheld once oldest_indication is older than
+		 * at least 1 second and there are pending confirmations. */
+
+		if (channel->outstanding_indications &&
+				(now - channel->oldest_indication) > 1)
+			continue;
+
+		/* Indication */
+		if (g_slist_find_custom(channel->indicate,
+				GUINT_TO_POINTER(handle), handle_cmp) != NULL) {
+			len_ind = enc_indication(attr, pdu_ind, mtu);
+			if (len_ind == 0)
+				return;
+
+			g_attrib_send(channel->attrib, 0, pdu_ind[0], pdu_ind,
+						len_ind, NULL, NULL, NULL);
+
+			channel->outstanding_indications += 1;
+			if (channel->outstanding_indications == 1)
+				channel->oldest_indication = now;
+		}
 	}
 }
 
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCHv2 1/5] Add read/write callbacks to attribute server
  2011-02-21 21:30 [PATCH 1/5] Add read/write callbacks to attribute server Anderson Lizardo
                   ` (3 preceding siblings ...)
  2011-02-21 21:30 ` [PATCH 5/5] Implement ATT handle indication Anderson Lizardo
@ 2011-02-22 21:01 ` Anderson Lizardo
  2011-02-23  3:21   ` Johan Hedberg
                     ` (5 more replies)
  2011-02-22 21:01 ` [PATCHv2 2/5] Initial Client Characteristic Configuration implementation Anderson Lizardo
                   ` (3 subsequent siblings)
  8 siblings, 6 replies; 21+ messages in thread
From: Anderson Lizardo @ 2011-02-22 21:01 UTC (permalink / raw
  To: linux-bluetooth; +Cc: Anderson Lizardo

These callbacks will allow profiles to act before an attribute is read
and after it is written, to e.g. update the attribute value to/from an
external source.

Note that by the time the callback is called, the necessary security
checks (attribute permissions, authentication and encryption) were
already performed by the core attribute server.

The callback can optionally return an ATT status code, which will be
sent to the client using an Error Response PDU.
---
 attrib/att.h        |    7 +++++++
 src/attrib-server.c |   29 ++++++++++++++++++++++++++---
 src/attrib-server.h |    4 ++--
 3 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/attrib/att.h b/attrib/att.h
index 7d9afeb..397898f 100644
--- a/attrib/att.h
+++ b/attrib/att.h
@@ -118,11 +118,18 @@ enum {
 	ATT_NOT_PERMITTED,	/* Operation not permitted */
 };
 
+struct attribute;
+
+typedef uint8_t (*att_cb_t)(struct attribute *a, gpointer user_data);
+
 struct attribute {
 	uint16_t handle;
 	uuid_t uuid;
 	int read_reqs;
 	int write_reqs;
+	att_cb_t read_cb;
+	att_cb_t write_cb;
+	gpointer cb_user_data;
 	int len;
 	uint8_t data[0];
 };
diff --git a/src/attrib-server.c b/src/attrib-server.c
index 7ec0f56..28649f1 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -229,6 +229,10 @@ static uint16_t read_by_group(struct gatt_channel *channel, uint16_t start,
 
 		status = att_check_reqs(channel, ATT_OP_READ_BY_GROUP_REQ,
 								a->read_reqs);
+
+		if (status == 0x00 && a->read_cb)
+			status = a->read_cb(a, a->cb_user_data);
+
 		if (status) {
 			g_slist_foreach(groups, (GFunc) g_free, NULL);
 			g_slist_free(groups);
@@ -317,6 +321,10 @@ static uint16_t read_by_type(struct gatt_channel *channel, uint16_t start,
 
 		status = att_check_reqs(channel, ATT_OP_READ_BY_TYPE_REQ,
 								a->read_reqs);
+
+		if (status == 0x00 && a->read_cb)
+			status = a->read_cb(a, a->cb_user_data);
+
 		if (status) {
 			g_slist_free(types);
 			return enc_error_resp(ATT_OP_READ_BY_TYPE_REQ,
@@ -564,6 +572,10 @@ static uint16_t read_value(struct gatt_channel *channel, uint16_t handle,
 	a = l->data;
 
 	status = att_check_reqs(channel, ATT_OP_READ_REQ, a->read_reqs);
+
+	if (status == 0x00 && a->read_cb)
+		status = a->read_cb(a, a->cb_user_data);
+
 	if (status)
 		return enc_error_resp(ATT_OP_READ_REQ, handle, status, pdu,
 									len);
@@ -591,6 +603,10 @@ static uint16_t read_blob(struct gatt_channel *channel, uint16_t handle,
 					ATT_ECODE_INVALID_OFFSET, pdu, len);
 
 	status = att_check_reqs(channel, ATT_OP_READ_BLOB_REQ, a->read_reqs);
+
+	if (status == 0x00 && a->read_cb)
+		status = a->read_cb(a, a->cb_user_data);
+
 	if (status)
 		return enc_error_resp(ATT_OP_READ_BLOB_REQ, handle, status,
 								pdu, len);
@@ -623,6 +639,13 @@ static uint16_t write_value(struct gatt_channel *channel, uint16_t handle,
 	memcpy(&uuid, &a->uuid, sizeof(uuid_t));
 	attrib_db_update(handle, &uuid, value, vlen);
 
+	if (a->write_cb) {
+		status = a->write_cb(a, a->cb_user_data);
+		if (status)
+			return enc_error_resp(ATT_OP_WRITE_REQ, handle, status,
+								pdu, len);
+	}
+
 	return enc_write_resp(pdu, len);
 }
 
@@ -1013,8 +1036,8 @@ void attrib_free_sdp(uint32_t sdp_handle)
 	remove_record_from_server(sdp_handle);
 }
 
-int attrib_db_add(uint16_t handle, uuid_t *uuid, int read_reqs, int write_reqs,
-						const uint8_t *value, int len)
+struct attribute *attrib_db_add(uint16_t handle, uuid_t *uuid, int read_reqs,
+				int write_reqs, const uint8_t *value, int len)
 {
 	struct attribute *a;
 
@@ -1030,7 +1053,7 @@ int attrib_db_add(uint16_t handle, uuid_t *uuid, int read_reqs, int write_reqs,
 
 	database = g_slist_insert_sorted(database, a, attribute_cmp);
 
-	return 0;
+	return a;
 }
 
 int attrib_db_update(uint16_t handle, uuid_t *uuid, const uint8_t *value,
diff --git a/src/attrib-server.h b/src/attrib-server.h
index ecd695c..85f3bdb 100644
--- a/src/attrib-server.h
+++ b/src/attrib-server.h
@@ -25,8 +25,8 @@
 int attrib_server_init(void);
 void attrib_server_exit(void);
 
-int attrib_db_add(uint16_t handle, uuid_t *uuid, int read_reqs, int write_reqs,
-						const uint8_t *value, int len);
+struct attribute *attrib_db_add(uint16_t handle, uuid_t *uuid, int read_reqs,
+				int write_reqs, const uint8_t *value, int len);
 int attrib_db_update(uint16_t handle, uuid_t *uuid, const uint8_t *value,
 								int len);
 int attrib_db_del(uint16_t handle);
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCHv2 2/5] Initial Client Characteristic Configuration implementation
  2011-02-21 21:30 [PATCH 1/5] Add read/write callbacks to attribute server Anderson Lizardo
                   ` (4 preceding siblings ...)
  2011-02-22 21:01 ` [PATCHv2 1/5] Add read/write callbacks to attribute server Anderson Lizardo
@ 2011-02-22 21:01 ` Anderson Lizardo
  2011-02-23  3:26   ` Johan Hedberg
  2011-02-22 21:01 ` [PATCHv2 3/5] Check permissions before setting client configs Anderson Lizardo
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 21+ messages in thread
From: Anderson Lizardo @ 2011-02-22 21:01 UTC (permalink / raw
  To: linux-bluetooth; +Cc: Anderson Lizardo

This initial version creates a private copy of the Client Configuration
attribute for each client on the first write to that attribute. Further
reads/writes should use the client's private copy.

Two list of attributes are created: one for values to be indicated and
another for values to be notified. The actual notification/indication
sending is implemented in a separate commit.

This initial version also does not check for the characteristic
properties, which is implemented in a separate commit as well.
---
 src/attrib-server.c |  180 ++++++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 149 insertions(+), 31 deletions(-)

diff --git a/src/attrib-server.c b/src/attrib-server.c
index 28649f1..8c74a5b 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -54,6 +54,9 @@ static GSList *database = NULL;
 struct gatt_channel {
 	bdaddr_t src;
 	bdaddr_t dst;
+	GSList *configs;
+	GSList *notify;
+	GSList *indicate;
 	GAttrib *attrib;
 	guint mtu;
 	guint id;
@@ -143,6 +146,22 @@ static sdp_record_t *server_record_new(uuid_t *uuid, uint16_t start, uint16_t en
 	return record;
 }
 
+static int handle_cmp(gconstpointer a, gconstpointer b)
+{
+	const struct attribute *attrib = a;
+	uint16_t handle = GPOINTER_TO_UINT(b);
+
+	return attrib->handle - handle;
+}
+
+static int attribute_cmp(gconstpointer a1, gconstpointer a2)
+{
+	const struct attribute *attrib1 = a1;
+	const struct attribute *attrib2 = a2;
+
+	return attrib1->handle - attrib2->handle;
+}
+
 static uint8_t att_check_reqs(struct gatt_channel *channel, uint8_t opcode,
 								int reqs)
 {
@@ -174,6 +193,92 @@ static uint8_t att_check_reqs(struct gatt_channel *channel, uint8_t opcode,
 	return 0;
 }
 
+static uint8_t client_set_notifications(struct attribute *attr,
+							gpointer user_data)
+{
+	struct gatt_channel *channel = user_data;
+	struct attribute *a, *last_chr_val = NULL;
+	uint16_t handle, cfg_val;
+	uuid_t uuid;
+	GSList *l;
+
+	cfg_val = att_get_u16(attr->data);
+
+	sdp_uuid16_create(&uuid, GATT_CHARAC_UUID);
+	for (l = database, handle = 0; l != NULL; l = l->next) {
+		a = l->data;
+
+		if (a->handle >= attr->handle)
+			break;
+
+		if (sdp_uuid_cmp(&a->uuid, &uuid) == 0) {
+			handle = att_get_u16(&a->data[1]);
+			continue;
+		}
+
+		if (handle && a->handle == handle)
+			last_chr_val = a;
+	}
+
+	if (last_chr_val == NULL)
+		return 0;
+
+	/* FIXME: Characteristic properties shall be checked for
+	 * Notification/Indication permissions. */
+
+	if (cfg_val & 0x0001)
+		channel->notify = g_slist_append(channel->notify, last_chr_val);
+	else
+		channel->notify = g_slist_remove(channel->notify, last_chr_val);
+
+	if (cfg_val & 0x0002)
+		channel->indicate = g_slist_append(channel->indicate,
+								last_chr_val);
+	else
+		channel->indicate = g_slist_remove(channel->indicate,
+								last_chr_val);
+
+	return 0;
+}
+
+static struct attribute *client_cfg_attribute(struct gatt_channel *channel,
+						struct attribute *orig_attr,
+						const uint8_t *value, int vlen)
+{
+	uuid_t uuid;
+	GSList *l;
+	guint handle = orig_attr->handle;
+
+	sdp_uuid16_create(&uuid, GATT_CLIENT_CHARAC_CFG_UUID);
+	if (sdp_uuid_cmp(&orig_attr->uuid, &uuid) != 0)
+		return NULL;
+
+	/* Value is unchanged, not need to create a private copy yet */
+	if (vlen == orig_attr->len && memcmp(orig_attr->data, value, vlen) == 0)
+		return orig_attr;
+
+	l = g_slist_find_custom(channel->configs, GUINT_TO_POINTER(handle),
+								handle_cmp);
+	if (!l) {
+		struct attribute *a;
+
+		/* Create a private copy of the Client Characteristic
+		 * Configuration attribute */
+		a = g_malloc0(sizeof(*a) + vlen);
+		memcpy(a, orig_attr, sizeof(*a));
+		memcpy(a->data, value, vlen);
+		a->write_cb = client_set_notifications;
+		a->cb_user_data = channel;
+
+		channel->configs = g_slist_insert_sorted(channel->configs, a,
+								attribute_cmp);
+
+		return a;
+	}
+
+	return l->data;
+}
+
 static uint16_t read_by_group(struct gatt_channel *channel, uint16_t start,
 						uint16_t end, uuid_t *uuid,
 						uint8_t *pdu, int len)
@@ -202,6 +307,8 @@ static uint16_t read_by_group(struct gatt_channel *channel, uint16_t start,
 
 	last_handle = end;
 	for (l = database, groups = NULL; l; l = l->next) {
+		struct attribute *client_attr;
+
 		a = l->data;
 
 		if (a->handle < start)
@@ -230,6 +337,10 @@ static uint16_t read_by_group(struct gatt_channel *channel, uint16_t start,
 		status = att_check_reqs(channel, ATT_OP_READ_BY_GROUP_REQ,
 								a->read_reqs);
 
+		client_attr = client_cfg_attribute(channel, a, a->data, a->len);
+		if (client_attr)
+			a = client_attr;
+
 		if (status == 0x00 && a->read_cb)
 			status = a->read_cb(a, a->cb_user_data);
 
@@ -308,6 +419,8 @@ static uint16_t read_by_type(struct gatt_channel *channel, uint16_t start,
 					ATT_ECODE_INVALID_HANDLE, pdu, len);
 
 	for (l = database, length = 0, types = NULL; l; l = l->next) {
+		struct attribute *client_attr;
+
 		a = l->data;
 
 		if (a->handle < start)
@@ -322,6 +435,10 @@ static uint16_t read_by_type(struct gatt_channel *channel, uint16_t start,
 		status = att_check_reqs(channel, ATT_OP_READ_BY_TYPE_REQ,
 								a->read_reqs);
 
+		client_attr = client_cfg_attribute(channel, a, a->data, a->len);
+		if (client_attr)
+			a = client_attr;
+
 		if (status == 0x00 && a->read_cb)
 			status = a->read_cb(a, a->cb_user_data);
 
@@ -506,22 +623,6 @@ static int find_by_type(uint16_t start, uint16_t end, uuid_t *uuid,
 	return len;
 }
 
-static int handle_cmp(gconstpointer a, gconstpointer b)
-{
-	const struct attribute *attrib = a;
-	uint16_t handle = GPOINTER_TO_UINT(b);
-
-	return attrib->handle - handle;
-}
-
-static int attribute_cmp(gconstpointer a1, gconstpointer a2)
-{
-	const struct attribute *attrib1 = a1;
-	const struct attribute *attrib2 = a2;
-
-	return attrib1->handle - attrib2->handle;
-}
-
 static struct attribute *find_primary_range(uint16_t start, uint16_t *end)
 {
 	struct attribute *attrib;
@@ -559,7 +660,7 @@ static struct attribute *find_primary_range(uint16_t start, uint16_t *end)
 static uint16_t read_value(struct gatt_channel *channel, uint16_t handle,
 							uint8_t *pdu, int len)
 {
-	struct attribute *a;
+	struct attribute *a, *client_attr;
 	uint8_t status;
 	GSList *l;
 	guint h = handle;
@@ -573,6 +674,10 @@ static uint16_t read_value(struct gatt_channel *channel, uint16_t handle,
 
 	status = att_check_reqs(channel, ATT_OP_READ_REQ, a->read_reqs);
 
+	client_attr = client_cfg_attribute(channel, a, a->data, a->len);
+	if (client_attr)
+		a = client_attr;
+
 	if (status == 0x00 && a->read_cb)
 		status = a->read_cb(a, a->cb_user_data);
 
@@ -586,7 +691,7 @@ static uint16_t read_value(struct gatt_channel *channel, uint16_t handle,
 static uint16_t read_blob(struct gatt_channel *channel, uint16_t handle,
 					uint16_t offset, uint8_t *pdu, int len)
 {
-	struct attribute *a;
+	struct attribute *a, *client_attr;
 	uint8_t status;
 	GSList *l;
 	guint h = handle;
@@ -604,6 +709,10 @@ static uint16_t read_blob(struct gatt_channel *channel, uint16_t handle,
 
 	status = att_check_reqs(channel, ATT_OP_READ_BLOB_REQ, a->read_reqs);
 
+	client_attr = client_cfg_attribute(channel, a, a->data, a->len);
+	if (client_attr)
+		a = client_attr;
+
 	if (status == 0x00 && a->read_cb)
 		status = a->read_cb(a, a->cb_user_data);
 
@@ -618,11 +727,10 @@ static uint16_t write_value(struct gatt_channel *channel, uint16_t handle,
 						const uint8_t *value, int vlen,
 						uint8_t *pdu, int len)
 {
-	struct attribute *a;
+	struct attribute *a, *client_attr;
 	uint8_t status;
 	GSList *l;
 	guint h = handle;
-	uuid_t uuid;
 
 	l = g_slist_find_custom(database, GUINT_TO_POINTER(h), handle_cmp);
 	if (!l)
@@ -636,8 +744,11 @@ static uint16_t write_value(struct gatt_channel *channel, uint16_t handle,
 		return enc_error_resp(ATT_OP_WRITE_REQ, handle, status, pdu,
 									len);
 
-	memcpy(&uuid, &a->uuid, sizeof(uuid_t));
-	attrib_db_update(handle, &uuid, value, vlen);
+	client_attr = client_cfg_attribute(channel, a, value, vlen);
+	if (client_attr)
+		a = client_attr;
+	else
+		attrib_db_update(a->handle, &a->uuid, value, vlen);
 
 	if (a->write_cb) {
 		status = a->write_cb(a, a->cb_user_data);
@@ -646,6 +757,10 @@ static uint16_t write_value(struct gatt_channel *channel, uint16_t handle,
 								pdu, len);
 	}
 
+	DBG("Notifications: %d, indications: %d",
+					g_slist_length(channel->notify),
+					g_slist_length(channel->indicate));
+
 	return enc_write_resp(pdu, len);
 }
 
@@ -664,6 +779,11 @@ static void channel_disconnect(void *user_data)
 	g_attrib_unref(channel->attrib);
 	clients = g_slist_remove(clients, channel);
 
+	g_slist_free(channel->notify);
+	g_slist_free(channel->indicate);
+	g_slist_foreach(channel->configs, (GFunc) g_free, NULL);
+	g_slist_free(channel->configs);
+
 	g_free(channel);
 }
 
@@ -976,6 +1096,11 @@ void attrib_server_exit(void)
 	for (l = clients; l; l = l->next) {
 		struct gatt_channel *channel = l->data;
 
+		g_slist_free(channel->notify);
+		g_slist_free(channel->indicate);
+		g_slist_foreach(channel->configs, (GFunc) g_free, NULL);
+		g_slist_free(channel->configs);
+
 		g_attrib_unref(channel->attrib);
 		g_free(channel);
 	}
@@ -1073,18 +1198,11 @@ int attrib_db_update(uint16_t handle, uuid_t *uuid, const uint8_t *value,
 
 	l->data = a;
 	a->handle = handle;
-	memcpy(&a->uuid, uuid, sizeof(uuid_t));
+	if (uuid != &a->uuid)
+		memcpy(&a->uuid, uuid, sizeof(uuid_t));
 	a->len = len;
 	memcpy(a->data, value, len);
 
-	/*
-	 * <<Client/Server Characteristic Configuration>> descriptors are
-	 * not supported yet. If a given attribute changes, the attribute
-	 * server shall report the new values using the mechanism selected
-	 * by the client. Notification/Indication shall not be automatically
-	 * sent if the client didn't request them.
-	 */
-
 	return 0;
 }
 
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCHv2 3/5] Check permissions before setting client configs
  2011-02-21 21:30 [PATCH 1/5] Add read/write callbacks to attribute server Anderson Lizardo
                   ` (5 preceding siblings ...)
  2011-02-22 21:01 ` [PATCHv2 2/5] Initial Client Characteristic Configuration implementation Anderson Lizardo
@ 2011-02-22 21:01 ` Anderson Lizardo
  2011-02-22 21:01 ` [PATCHv2 4/5] Implement server-side ATT handle notification Anderson Lizardo
  2011-02-22 21:01 ` [PATCHv2 5/5] Implement ATT handle indication Anderson Lizardo
  8 siblings, 0 replies; 21+ messages in thread
From: Anderson Lizardo @ 2011-02-22 21:01 UTC (permalink / raw
  To: linux-bluetooth; +Cc: Andre Dieb Martins

From: Andre Dieb Martins <andre.dieb@signove.com>

Only enable notification/indication if the descriptor allows it.
---
 src/attrib-server.c |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/src/attrib-server.c b/src/attrib-server.c
index 8c74a5b..aa24985 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -199,19 +199,21 @@ static uint8_t client_set_notifications(struct attribute *attr,
 	struct gatt_channel *channel = user_data;
 	struct attribute *a, *last_chr_val = NULL;
 	uint16_t handle, cfg_val;
+	uint8_t perm;
 	uuid_t uuid;
 	GSList *l;
 
 	cfg_val = att_get_u16(attr->data);
 
 	sdp_uuid16_create(&uuid, GATT_CHARAC_UUID);
-	for (l = database, handle = 0; l != NULL; l = l->next) {
+	for (l = database, handle = 0, perm = 0; l != NULL; l = l->next) {
 		a = l->data;
 
 		if (a->handle >= attr->handle)
 			break;
 
 		if (sdp_uuid_cmp(&a->uuid, &uuid) == 0) {
+			perm = att_get_u8(&a->data[0]);
 			handle = att_get_u16(&a->data[1]);
 			continue;
 		}
@@ -223,8 +225,11 @@ static uint8_t client_set_notifications(struct attribute *attr,
 	if (last_chr_val == NULL)
 		return 0;
 
-	/* FIXME: Characteristic properties shall be checked for
-	 * Notification/Indication permissions. */
+	if ((cfg_val & 0x0001) && !(perm & ATT_CHAR_PROPER_NOTIFY))
+		return ATT_ECODE_WRITE_NOT_PERM;
+
+	if ((cfg_val & 0x0002) && !(perm & ATT_CHAR_PROPER_INDICATE))
+		return ATT_ECODE_WRITE_NOT_PERM;
 
 	if (cfg_val & 0x0001)
 		channel->notify = g_slist_append(channel->notify, last_chr_val);
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCHv2 4/5] Implement server-side ATT handle notification
  2011-02-21 21:30 [PATCH 1/5] Add read/write callbacks to attribute server Anderson Lizardo
                   ` (6 preceding siblings ...)
  2011-02-22 21:01 ` [PATCHv2 3/5] Check permissions before setting client configs Anderson Lizardo
@ 2011-02-22 21:01 ` Anderson Lizardo
  2011-02-22 21:01 ` [PATCHv2 5/5] Implement ATT handle indication Anderson Lizardo
  8 siblings, 0 replies; 21+ messages in thread
From: Anderson Lizardo @ 2011-02-22 21:01 UTC (permalink / raw
  To: linux-bluetooth; +Cc: Andre Dieb Martins

From: Andre Dieb Martins <andre.dieb@signove.com>

Adds an initial version of ATT handle notification. Notifications are sent to
interested clients (based on the Client Characteristic Configuration) always
when the attribute is updated.

This enables services to call db_attrib_update() and send notifications on their
own terms.
---
 src/attrib-server.c |   26 ++++++++++++++++++++++++++
 1 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/src/attrib-server.c b/src/attrib-server.c
index aa24985..8fcfa89 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -964,6 +964,30 @@ static void confirm_event(GIOChannel *io, void *user_data)
 	return;
 }
 
+static void attrib_notify_clients(struct attribute *attr)
+{
+	guint handle = attr->handle;
+	GSList *l;
+
+	for (l = clients; l; l = l->next) {
+		struct gatt_channel *channel = l->data;
+
+		/* Notification */
+		if (g_slist_find_custom(channel->notify,
+					GUINT_TO_POINTER(handle), handle_cmp)) {
+			uint8_t pdu[ATT_MAX_MTU];
+			uint16_t len;
+
+			len = enc_notification(attr, pdu, channel->mtu);
+			if (len == 0)
+				continue;
+
+			g_attrib_send(channel->attrib, 0, pdu[0], pdu, len,
+							NULL, NULL, NULL);
+		}
+	}
+}
+
 static gboolean register_core_services(void)
 {
 	uint8_t atval[256];
@@ -1208,6 +1232,8 @@ int attrib_db_update(uint16_t handle, uuid_t *uuid, const uint8_t *value,
 	a->len = len;
 	memcpy(a->data, value, len);
 
+	attrib_notify_clients(a);
+
 	return 0;
 }
 
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCHv2 5/5] Implement ATT handle indication
  2011-02-21 21:30 [PATCH 1/5] Add read/write callbacks to attribute server Anderson Lizardo
                   ` (7 preceding siblings ...)
  2011-02-22 21:01 ` [PATCHv2 4/5] Implement server-side ATT handle notification Anderson Lizardo
@ 2011-02-22 21:01 ` Anderson Lizardo
  8 siblings, 0 replies; 21+ messages in thread
From: Anderson Lizardo @ 2011-02-22 21:01 UTC (permalink / raw
  To: linux-bluetooth; +Cc: Elvis Pfützenreuter

From: Elvis Pfützenreuter <epx@signove.com>

---
 src/attrib-server.c |   40 ++++++++++++++++++++++++++++++++++++++++
 1 files changed, 40 insertions(+), 0 deletions(-)

diff --git a/src/attrib-server.c b/src/attrib-server.c
index 8fcfa89..dc4d864 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -61,6 +61,8 @@ struct gatt_channel {
 	guint mtu;
 	guint id;
 	gboolean encrypted;
+	guint outstanding_indications;
+	time_t oldest_indication;
 };
 
 struct group_elem {
@@ -888,6 +890,10 @@ static void channel_handler(const uint8_t *ipdu, uint16_t len,
 		length = find_by_type(start, end, &uuid, value, vlen,
 							opdu, channel->mtu);
 		break;
+	case ATT_OP_HANDLE_CNF:
+		if (channel->outstanding_indications)
+			channel->outstanding_indications -= 1;
+		return;
 	case ATT_OP_READ_MULTI_REQ:
 	case ATT_OP_PREP_WRITE_REQ:
 	case ATT_OP_EXEC_WRITE_REQ:
@@ -967,8 +973,11 @@ static void confirm_event(GIOChannel *io, void *user_data)
 static void attrib_notify_clients(struct attribute *attr)
 {
 	guint handle = attr->handle;
+	time_t now;
 	GSList *l;
 
+	time(&now);
+
 	for (l = clients; l; l = l->next) {
 		struct gatt_channel *channel = l->data;
 
@@ -985,6 +994,37 @@ static void attrib_notify_clients(struct attribute *attr)
 			g_attrib_send(channel->attrib, 0, pdu[0], pdu, len,
 							NULL, NULL, NULL);
 		}
+
+		/* The outstanding_indications variable counts how many
+		 * unconfirmed indications have been sent. The oldest_indication
+		 * variable gives a small time frame to accomodate
+		 * almost-concomitant indications, sending both without forcing
+		 * a serialization.
+		 *
+		 * Indications are withheld once oldest_indication is older than
+		 * at least 1 second and there are pending confirmations. */
+
+		if (channel->outstanding_indications &&
+				(now - channel->oldest_indication) > 1)
+			continue;
+
+		/* Indication */
+		if (g_slist_find_custom(channel->indicate,
+				GUINT_TO_POINTER(handle), handle_cmp) != NULL) {
+			uint8_t pdu[ATT_MAX_MTU];
+			uint16_t len;
+
+			len = enc_indication(attr, pdu, channel->mtu);
+			if (len == 0)
+				continue;
+
+			g_attrib_send(channel->attrib, 0, pdu[0], pdu, len,
+							NULL, NULL, NULL);
+
+			channel->outstanding_indications += 1;
+			if (channel->outstanding_indications == 1)
+				channel->oldest_indication = now;
+		}
 	}
 }
 
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCHv2 1/5] Add read/write callbacks to attribute server
  2011-02-22 21:01 ` [PATCHv2 1/5] Add read/write callbacks to attribute server Anderson Lizardo
@ 2011-02-23  3:21   ` Johan Hedberg
  2011-02-23 14:28     ` Anderson Lizardo
  2011-02-23 15:14   ` [PATCH v3 " Anderson Lizardo
                     ` (4 subsequent siblings)
  5 siblings, 1 reply; 21+ messages in thread
From: Johan Hedberg @ 2011-02-23  3:21 UTC (permalink / raw
  To: Anderson Lizardo; +Cc: linux-bluetooth

Hi Anderson,

On Tue, Feb 22, 2011, Anderson Lizardo wrote:
> +struct attribute;
> +
> +typedef uint8_t (*att_cb_t)(struct attribute *a, gpointer user_data);
> +
>  struct attribute {
>  	uint16_t handle;
>  	uuid_t uuid;
>  	int read_reqs;
>  	int write_reqs;
> +	att_cb_t read_cb;
> +	att_cb_t write_cb;
> +	gpointer cb_user_data;
>  	int len;
>  	uint8_t data[0];
>  };

I'm not really a fan of the needed forward declaration here. I can't
find you using "att_cb_t" anywhere else in your patches, so how about
just having the full type of the callbacks inside the struct definition
and skip the typedef completely?

Johan

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCHv2 2/5] Initial Client Characteristic Configuration implementation
  2011-02-22 21:01 ` [PATCHv2 2/5] Initial Client Characteristic Configuration implementation Anderson Lizardo
@ 2011-02-23  3:26   ` Johan Hedberg
  2011-02-23 14:29     ` Anderson Lizardo
  2011-03-07 20:52     ` Peter Dons Tychsen
  0 siblings, 2 replies; 21+ messages in thread
From: Johan Hedberg @ 2011-02-23  3:26 UTC (permalink / raw
  To: Anderson Lizardo; +Cc: linux-bluetooth

Hi Lizardo,

On Tue, Feb 22, 2011, Anderson Lizardo wrote:
> +static uint8_t client_set_notifications(struct attribute *attr,
> +							gpointer user_data)
> +{
> +	struct gatt_channel *channel = user_data;
> +	struct attribute *a, *last_chr_val = NULL;
> +	uint16_t handle, cfg_val;
> +	uuid_t uuid;
> +	GSList *l;
> +
> +	cfg_val = att_get_u16(attr->data);
> +
> +	sdp_uuid16_create(&uuid, GATT_CHARAC_UUID);
> +	for (l = database, handle = 0; l != NULL; l = l->next) {
> +		a = l->data;

The variable "a" is only used inside the for-loop so it should be
declared inside it as well. I think you can move handle inside the loop
as well as long as you declare it static (so it only gets initialized to
0 on the first iteration).

Johan

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCHv2 1/5] Add read/write callbacks to attribute server
  2011-02-23  3:21   ` Johan Hedberg
@ 2011-02-23 14:28     ` Anderson Lizardo
  0 siblings, 0 replies; 21+ messages in thread
From: Anderson Lizardo @ 2011-02-23 14:28 UTC (permalink / raw
  To: Anderson Lizardo, linux-bluetooth; +Cc: Johan Hedberg

Hi Johan,

On Wed, Feb 23, 2011 at 12:21 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> On Tue, Feb 22, 2011, Anderson Lizardo wrote:
>> +struct attribute;
>> +
>> +typedef uint8_t (*att_cb_t)(struct attribute *a, gpointer user_data);
>> +
>>  struct attribute {
>>       uint16_t handle;
>>       uuid_t uuid;
>>       int read_reqs;
>>       int write_reqs;
>> +     att_cb_t read_cb;
>> +     att_cb_t write_cb;
>> +     gpointer cb_user_data;
>>       int len;
>>       uint8_t data[0];
>>  };
>
> I'm not really a fan of the needed forward declaration here. I can't
> find you using "att_cb_t" anywhere else in your patches, so how about
> just having the full type of the callbacks inside the struct definition
> and skip the typedef completely?

Sure, I'll drop the typedef and send a v3.

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCHv2 2/5] Initial Client Characteristic Configuration implementation
  2011-02-23  3:26   ` Johan Hedberg
@ 2011-02-23 14:29     ` Anderson Lizardo
  2011-03-07 20:52     ` Peter Dons Tychsen
  1 sibling, 0 replies; 21+ messages in thread
From: Anderson Lizardo @ 2011-02-23 14:29 UTC (permalink / raw
  To: Anderson Lizardo, linux-bluetooth; +Cc: Johan Hedberg

Hi Johan,

On Wed, Feb 23, 2011 at 12:26 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> On Tue, Feb 22, 2011, Anderson Lizardo wrote:
>> +static uint8_t client_set_notifications(struct attribute *attr,
>> +                                                     gpointer user_data)
>> +{
>> +     struct gatt_channel *channel = user_data;
>> +     struct attribute *a, *last_chr_val = NULL;
>> +     uint16_t handle, cfg_val;
>> +     uuid_t uuid;
>> +     GSList *l;
>> +
>> +     cfg_val = att_get_u16(attr->data);
>> +
>> +     sdp_uuid16_create(&uuid, GATT_CHARAC_UUID);
>> +     for (l = database, handle = 0; l != NULL; l = l->next) {
>> +             a = l->data;
>
> The variable "a" is only used inside the for-loop so it should be
> declared inside it as well. I think you can move handle inside the loop
> as well as long as you declare it static (so it only gets initialized to
> 0 on the first iteration).

Both changes will be incorporated on the v3.

>
> Johan
>

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v3 1/5] Add read/write callbacks to attribute server
  2011-02-22 21:01 ` [PATCHv2 1/5] Add read/write callbacks to attribute server Anderson Lizardo
  2011-02-23  3:21   ` Johan Hedberg
@ 2011-02-23 15:14   ` Anderson Lizardo
  2011-02-23 15:14   ` [PATCH v3 2/5] Initial Client Characteristic Configuration implementation Anderson Lizardo
                     ` (3 subsequent siblings)
  5 siblings, 0 replies; 21+ messages in thread
From: Anderson Lizardo @ 2011-02-23 15:14 UTC (permalink / raw
  To: linux-bluetooth; +Cc: Anderson Lizardo

These callbacks will allow profiles to act before an attribute is read
and after it is written, to e.g. update the attribute value to/from an
external source.

Note that by the time the callback is called, the necessary security
checks (attribute permissions, authentication and encryption) were
already performed by the core attribute server.

The callback can optionally return an ATT status code, which will be
sent to the client using an Error Response PDU.
---
 attrib/att.h        |    3 +++
 src/attrib-server.c |   29 ++++++++++++++++++++++++++---
 src/attrib-server.h |    4 ++--
 3 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/attrib/att.h b/attrib/att.h
index 7d9afeb..9b0b538 100644
--- a/attrib/att.h
+++ b/attrib/att.h
@@ -123,6 +123,9 @@ struct attribute {
 	uuid_t uuid;
 	int read_reqs;
 	int write_reqs;
+	uint8_t (*read_cb)(struct attribute *a, gpointer user_data);
+	uint8_t (*write_cb)(struct attribute *a, gpointer user_data);
+	gpointer cb_user_data;
 	int len;
 	uint8_t data[0];
 };
diff --git a/src/attrib-server.c b/src/attrib-server.c
index 7ec0f56..28649f1 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -229,6 +229,10 @@ static uint16_t read_by_group(struct gatt_channel *channel, uint16_t start,
 
 		status = att_check_reqs(channel, ATT_OP_READ_BY_GROUP_REQ,
 								a->read_reqs);
+
+		if (status == 0x00 && a->read_cb)
+			status = a->read_cb(a, a->cb_user_data);
+
 		if (status) {
 			g_slist_foreach(groups, (GFunc) g_free, NULL);
 			g_slist_free(groups);
@@ -317,6 +321,10 @@ static uint16_t read_by_type(struct gatt_channel *channel, uint16_t start,
 
 		status = att_check_reqs(channel, ATT_OP_READ_BY_TYPE_REQ,
 								a->read_reqs);
+
+		if (status == 0x00 && a->read_cb)
+			status = a->read_cb(a, a->cb_user_data);
+
 		if (status) {
 			g_slist_free(types);
 			return enc_error_resp(ATT_OP_READ_BY_TYPE_REQ,
@@ -564,6 +572,10 @@ static uint16_t read_value(struct gatt_channel *channel, uint16_t handle,
 	a = l->data;
 
 	status = att_check_reqs(channel, ATT_OP_READ_REQ, a->read_reqs);
+
+	if (status == 0x00 && a->read_cb)
+		status = a->read_cb(a, a->cb_user_data);
+
 	if (status)
 		return enc_error_resp(ATT_OP_READ_REQ, handle, status, pdu,
 									len);
@@ -591,6 +603,10 @@ static uint16_t read_blob(struct gatt_channel *channel, uint16_t handle,
 					ATT_ECODE_INVALID_OFFSET, pdu, len);
 
 	status = att_check_reqs(channel, ATT_OP_READ_BLOB_REQ, a->read_reqs);
+
+	if (status == 0x00 && a->read_cb)
+		status = a->read_cb(a, a->cb_user_data);
+
 	if (status)
 		return enc_error_resp(ATT_OP_READ_BLOB_REQ, handle, status,
 								pdu, len);
@@ -623,6 +639,13 @@ static uint16_t write_value(struct gatt_channel *channel, uint16_t handle,
 	memcpy(&uuid, &a->uuid, sizeof(uuid_t));
 	attrib_db_update(handle, &uuid, value, vlen);
 
+	if (a->write_cb) {
+		status = a->write_cb(a, a->cb_user_data);
+		if (status)
+			return enc_error_resp(ATT_OP_WRITE_REQ, handle, status,
+								pdu, len);
+	}
+
 	return enc_write_resp(pdu, len);
 }
 
@@ -1013,8 +1036,8 @@ void attrib_free_sdp(uint32_t sdp_handle)
 	remove_record_from_server(sdp_handle);
 }
 
-int attrib_db_add(uint16_t handle, uuid_t *uuid, int read_reqs, int write_reqs,
-						const uint8_t *value, int len)
+struct attribute *attrib_db_add(uint16_t handle, uuid_t *uuid, int read_reqs,
+				int write_reqs, const uint8_t *value, int len)
 {
 	struct attribute *a;
 
@@ -1030,7 +1053,7 @@ int attrib_db_add(uint16_t handle, uuid_t *uuid, int read_reqs, int write_reqs,
 
 	database = g_slist_insert_sorted(database, a, attribute_cmp);
 
-	return 0;
+	return a;
 }
 
 int attrib_db_update(uint16_t handle, uuid_t *uuid, const uint8_t *value,
diff --git a/src/attrib-server.h b/src/attrib-server.h
index ecd695c..85f3bdb 100644
--- a/src/attrib-server.h
+++ b/src/attrib-server.h
@@ -25,8 +25,8 @@
 int attrib_server_init(void);
 void attrib_server_exit(void);
 
-int attrib_db_add(uint16_t handle, uuid_t *uuid, int read_reqs, int write_reqs,
-						const uint8_t *value, int len);
+struct attribute *attrib_db_add(uint16_t handle, uuid_t *uuid, int read_reqs,
+				int write_reqs, const uint8_t *value, int len);
 int attrib_db_update(uint16_t handle, uuid_t *uuid, const uint8_t *value,
 								int len);
 int attrib_db_del(uint16_t handle);
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v3 2/5] Initial Client Characteristic Configuration implementation
  2011-02-22 21:01 ` [PATCHv2 1/5] Add read/write callbacks to attribute server Anderson Lizardo
  2011-02-23  3:21   ` Johan Hedberg
  2011-02-23 15:14   ` [PATCH v3 " Anderson Lizardo
@ 2011-02-23 15:14   ` Anderson Lizardo
  2011-02-23 15:14   ` [PATCH v3 3/5] Check properties before setting client configs Anderson Lizardo
                     ` (2 subsequent siblings)
  5 siblings, 0 replies; 21+ messages in thread
From: Anderson Lizardo @ 2011-02-23 15:14 UTC (permalink / raw
  To: linux-bluetooth; +Cc: Anderson Lizardo

This initial version creates a private copy of the Client Configuration
attribute for each client on the first write to that attribute. Further
reads/writes should use the client's private copy.

Two list of attributes are created: one for values to be indicated and
another for values to be notified. The actual notification/indication
sending is implemented in a separate commit.

This initial version also does not check for the characteristic
properties, which is implemented in a separate commit as well.
---
 src/attrib-server.c |  181 ++++++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 150 insertions(+), 31 deletions(-)

diff --git a/src/attrib-server.c b/src/attrib-server.c
index 28649f1..47ca5d9 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -54,6 +54,9 @@ static GSList *database = NULL;
 struct gatt_channel {
 	bdaddr_t src;
 	bdaddr_t dst;
+	GSList *configs;
+	GSList *notify;
+	GSList *indicate;
 	GAttrib *attrib;
 	guint mtu;
 	guint id;
@@ -143,6 +146,22 @@ static sdp_record_t *server_record_new(uuid_t *uuid, uint16_t start, uint16_t en
 	return record;
 }
 
+static int handle_cmp(gconstpointer a, gconstpointer b)
+{
+	const struct attribute *attrib = a;
+	uint16_t handle = GPOINTER_TO_UINT(b);
+
+	return attrib->handle - handle;
+}
+
+static int attribute_cmp(gconstpointer a1, gconstpointer a2)
+{
+	const struct attribute *attrib1 = a1;
+	const struct attribute *attrib2 = a2;
+
+	return attrib1->handle - attrib2->handle;
+}
+
 static uint8_t att_check_reqs(struct gatt_channel *channel, uint8_t opcode,
 								int reqs)
 {
@@ -174,6 +193,93 @@ static uint8_t att_check_reqs(struct gatt_channel *channel, uint8_t opcode,
 	return 0;
 }
 
+static uint8_t client_set_notifications(struct attribute *attr,
+							gpointer user_data)
+{
+	struct gatt_channel *channel = user_data;
+	struct attribute *last_chr_val = NULL;
+	uint16_t cfg_val;
+	uuid_t uuid;
+	GSList *l;
+
+	cfg_val = att_get_u16(attr->data);
+
+	sdp_uuid16_create(&uuid, GATT_CHARAC_UUID);
+	for (l = database; l != NULL; l = l->next) {
+		struct attribute *a = l->data;
+		static uint16_t handle = 0;
+
+		if (a->handle >= attr->handle)
+			break;
+
+		if (sdp_uuid_cmp(&a->uuid, &uuid) == 0) {
+			handle = att_get_u16(&a->data[1]);
+			continue;
+		}
+
+		if (handle && a->handle == handle)
+			last_chr_val = a;
+	}
+
+	if (last_chr_val == NULL)
+		return 0;
+
+	/* FIXME: Characteristic properties shall be checked for
+	 * Notification/Indication permissions. */
+
+	if (cfg_val & 0x0001)
+		channel->notify = g_slist_append(channel->notify, last_chr_val);
+	else
+		channel->notify = g_slist_remove(channel->notify, last_chr_val);
+
+	if (cfg_val & 0x0002)
+		channel->indicate = g_slist_append(channel->indicate,
+								last_chr_val);
+	else
+		channel->indicate = g_slist_remove(channel->indicate,
+								last_chr_val);
+
+	return 0;
+}
+
+static struct attribute *client_cfg_attribute(struct gatt_channel *channel,
+						struct attribute *orig_attr,
+						const uint8_t *value, int vlen)
+{
+	guint handle = orig_attr->handle;
+	uuid_t uuid;
+	GSList *l;
+
+	sdp_uuid16_create(&uuid, GATT_CLIENT_CHARAC_CFG_UUID);
+	if (sdp_uuid_cmp(&orig_attr->uuid, &uuid) != 0)
+		return NULL;
+
+	/* Value is unchanged, not need to create a private copy yet */
+	if (vlen == orig_attr->len && memcmp(orig_attr->data, value, vlen) == 0)
+		return orig_attr;
+
+	l = g_slist_find_custom(channel->configs, GUINT_TO_POINTER(handle),
+								handle_cmp);
+	if (!l) {
+		struct attribute *a;
+
+		/* Create a private copy of the Client Characteristic
+		 * Configuration attribute */
+		a = g_malloc0(sizeof(*a) + vlen);
+		memcpy(a, orig_attr, sizeof(*a));
+		memcpy(a->data, value, vlen);
+		a->write_cb = client_set_notifications;
+		a->cb_user_data = channel;
+
+		channel->configs = g_slist_insert_sorted(channel->configs, a,
+								attribute_cmp);
+
+		return a;
+	}
+
+	return l->data;
+}
+
 static uint16_t read_by_group(struct gatt_channel *channel, uint16_t start,
 						uint16_t end, uuid_t *uuid,
 						uint8_t *pdu, int len)
@@ -202,6 +308,8 @@ static uint16_t read_by_group(struct gatt_channel *channel, uint16_t start,
 
 	last_handle = end;
 	for (l = database, groups = NULL; l; l = l->next) {
+		struct attribute *client_attr;
+
 		a = l->data;
 
 		if (a->handle < start)
@@ -230,6 +338,10 @@ static uint16_t read_by_group(struct gatt_channel *channel, uint16_t start,
 		status = att_check_reqs(channel, ATT_OP_READ_BY_GROUP_REQ,
 								a->read_reqs);
 
+		client_attr = client_cfg_attribute(channel, a, a->data, a->len);
+		if (client_attr)
+			a = client_attr;
+
 		if (status == 0x00 && a->read_cb)
 			status = a->read_cb(a, a->cb_user_data);
 
@@ -308,6 +420,8 @@ static uint16_t read_by_type(struct gatt_channel *channel, uint16_t start,
 					ATT_ECODE_INVALID_HANDLE, pdu, len);
 
 	for (l = database, length = 0, types = NULL; l; l = l->next) {
+		struct attribute *client_attr;
+
 		a = l->data;
 
 		if (a->handle < start)
@@ -322,6 +436,10 @@ static uint16_t read_by_type(struct gatt_channel *channel, uint16_t start,
 		status = att_check_reqs(channel, ATT_OP_READ_BY_TYPE_REQ,
 								a->read_reqs);
 
+		client_attr = client_cfg_attribute(channel, a, a->data, a->len);
+		if (client_attr)
+			a = client_attr;
+
 		if (status == 0x00 && a->read_cb)
 			status = a->read_cb(a, a->cb_user_data);
 
@@ -506,22 +624,6 @@ static int find_by_type(uint16_t start, uint16_t end, uuid_t *uuid,
 	return len;
 }
 
-static int handle_cmp(gconstpointer a, gconstpointer b)
-{
-	const struct attribute *attrib = a;
-	uint16_t handle = GPOINTER_TO_UINT(b);
-
-	return attrib->handle - handle;
-}
-
-static int attribute_cmp(gconstpointer a1, gconstpointer a2)
-{
-	const struct attribute *attrib1 = a1;
-	const struct attribute *attrib2 = a2;
-
-	return attrib1->handle - attrib2->handle;
-}
-
 static struct attribute *find_primary_range(uint16_t start, uint16_t *end)
 {
 	struct attribute *attrib;
@@ -559,7 +661,7 @@ static struct attribute *find_primary_range(uint16_t start, uint16_t *end)
 static uint16_t read_value(struct gatt_channel *channel, uint16_t handle,
 							uint8_t *pdu, int len)
 {
-	struct attribute *a;
+	struct attribute *a, *client_attr;
 	uint8_t status;
 	GSList *l;
 	guint h = handle;
@@ -573,6 +675,10 @@ static uint16_t read_value(struct gatt_channel *channel, uint16_t handle,
 
 	status = att_check_reqs(channel, ATT_OP_READ_REQ, a->read_reqs);
 
+	client_attr = client_cfg_attribute(channel, a, a->data, a->len);
+	if (client_attr)
+		a = client_attr;
+
 	if (status == 0x00 && a->read_cb)
 		status = a->read_cb(a, a->cb_user_data);
 
@@ -586,7 +692,7 @@ static uint16_t read_value(struct gatt_channel *channel, uint16_t handle,
 static uint16_t read_blob(struct gatt_channel *channel, uint16_t handle,
 					uint16_t offset, uint8_t *pdu, int len)
 {
-	struct attribute *a;
+	struct attribute *a, *client_attr;
 	uint8_t status;
 	GSList *l;
 	guint h = handle;
@@ -604,6 +710,10 @@ static uint16_t read_blob(struct gatt_channel *channel, uint16_t handle,
 
 	status = att_check_reqs(channel, ATT_OP_READ_BLOB_REQ, a->read_reqs);
 
+	client_attr = client_cfg_attribute(channel, a, a->data, a->len);
+	if (client_attr)
+		a = client_attr;
+
 	if (status == 0x00 && a->read_cb)
 		status = a->read_cb(a, a->cb_user_data);
 
@@ -618,11 +728,10 @@ static uint16_t write_value(struct gatt_channel *channel, uint16_t handle,
 						const uint8_t *value, int vlen,
 						uint8_t *pdu, int len)
 {
-	struct attribute *a;
+	struct attribute *a, *client_attr;
 	uint8_t status;
 	GSList *l;
 	guint h = handle;
-	uuid_t uuid;
 
 	l = g_slist_find_custom(database, GUINT_TO_POINTER(h), handle_cmp);
 	if (!l)
@@ -636,8 +745,11 @@ static uint16_t write_value(struct gatt_channel *channel, uint16_t handle,
 		return enc_error_resp(ATT_OP_WRITE_REQ, handle, status, pdu,
 									len);
 
-	memcpy(&uuid, &a->uuid, sizeof(uuid_t));
-	attrib_db_update(handle, &uuid, value, vlen);
+	client_attr = client_cfg_attribute(channel, a, value, vlen);
+	if (client_attr)
+		a = client_attr;
+	else
+		attrib_db_update(a->handle, &a->uuid, value, vlen);
 
 	if (a->write_cb) {
 		status = a->write_cb(a, a->cb_user_data);
@@ -646,6 +758,10 @@ static uint16_t write_value(struct gatt_channel *channel, uint16_t handle,
 								pdu, len);
 	}
 
+	DBG("Notifications: %d, indications: %d",
+					g_slist_length(channel->notify),
+					g_slist_length(channel->indicate));
+
 	return enc_write_resp(pdu, len);
 }
 
@@ -664,6 +780,11 @@ static void channel_disconnect(void *user_data)
 	g_attrib_unref(channel->attrib);
 	clients = g_slist_remove(clients, channel);
 
+	g_slist_free(channel->notify);
+	g_slist_free(channel->indicate);
+	g_slist_foreach(channel->configs, (GFunc) g_free, NULL);
+	g_slist_free(channel->configs);
+
 	g_free(channel);
 }
 
@@ -976,6 +1097,11 @@ void attrib_server_exit(void)
 	for (l = clients; l; l = l->next) {
 		struct gatt_channel *channel = l->data;
 
+		g_slist_free(channel->notify);
+		g_slist_free(channel->indicate);
+		g_slist_foreach(channel->configs, (GFunc) g_free, NULL);
+		g_slist_free(channel->configs);
+
 		g_attrib_unref(channel->attrib);
 		g_free(channel);
 	}
@@ -1073,18 +1199,11 @@ int attrib_db_update(uint16_t handle, uuid_t *uuid, const uint8_t *value,
 
 	l->data = a;
 	a->handle = handle;
-	memcpy(&a->uuid, uuid, sizeof(uuid_t));
+	if (uuid != &a->uuid)
+		memcpy(&a->uuid, uuid, sizeof(uuid_t));
 	a->len = len;
 	memcpy(a->data, value, len);
 
-	/*
-	 * <<Client/Server Characteristic Configuration>> descriptors are
-	 * not supported yet. If a given attribute changes, the attribute
-	 * server shall report the new values using the mechanism selected
-	 * by the client. Notification/Indication shall not be automatically
-	 * sent if the client didn't request them.
-	 */
-
 	return 0;
 }
 
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v3 3/5] Check properties before setting client configs
  2011-02-22 21:01 ` [PATCHv2 1/5] Add read/write callbacks to attribute server Anderson Lizardo
                     ` (2 preceding siblings ...)
  2011-02-23 15:14   ` [PATCH v3 2/5] Initial Client Characteristic Configuration implementation Anderson Lizardo
@ 2011-02-23 15:14   ` Anderson Lizardo
  2011-02-23 15:14   ` [PATCH v3 4/5] Implement server-side ATT handle notification Anderson Lizardo
  2011-02-23 15:14   ` [PATCH v3 5/5] Implement ATT handle indication Anderson Lizardo
  5 siblings, 0 replies; 21+ messages in thread
From: Anderson Lizardo @ 2011-02-23 15:14 UTC (permalink / raw
  To: linux-bluetooth; +Cc: Andre Dieb Martins

From: Andre Dieb Martins <andre.dieb@signove.com>

Only enable notification/indication if the descriptor allows it.
---
 src/attrib-server.c |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/src/attrib-server.c b/src/attrib-server.c
index 47ca5d9..21da17e 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -199,13 +199,14 @@ static uint8_t client_set_notifications(struct attribute *attr,
 	struct gatt_channel *channel = user_data;
 	struct attribute *last_chr_val = NULL;
 	uint16_t cfg_val;
+	uint8_t props;
 	uuid_t uuid;
 	GSList *l;
 
 	cfg_val = att_get_u16(attr->data);
 
 	sdp_uuid16_create(&uuid, GATT_CHARAC_UUID);
-	for (l = database; l != NULL; l = l->next) {
+	for (l = database, props = 0; l != NULL; l = l->next) {
 		struct attribute *a = l->data;
 		static uint16_t handle = 0;
 
@@ -213,6 +214,7 @@ static uint8_t client_set_notifications(struct attribute *attr,
 			break;
 
 		if (sdp_uuid_cmp(&a->uuid, &uuid) == 0) {
+			props = att_get_u8(&a->data[0]);
 			handle = att_get_u16(&a->data[1]);
 			continue;
 		}
@@ -224,8 +226,11 @@ static uint8_t client_set_notifications(struct attribute *attr,
 	if (last_chr_val == NULL)
 		return 0;
 
-	/* FIXME: Characteristic properties shall be checked for
-	 * Notification/Indication permissions. */
+	if ((cfg_val & 0x0001) && !(props & ATT_CHAR_PROPER_NOTIFY))
+		return ATT_ECODE_WRITE_NOT_PERM;
+
+	if ((cfg_val & 0x0002) && !(props & ATT_CHAR_PROPER_INDICATE))
+		return ATT_ECODE_WRITE_NOT_PERM;
 
 	if (cfg_val & 0x0001)
 		channel->notify = g_slist_append(channel->notify, last_chr_val);
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v3 4/5] Implement server-side ATT handle notification
  2011-02-22 21:01 ` [PATCHv2 1/5] Add read/write callbacks to attribute server Anderson Lizardo
                     ` (3 preceding siblings ...)
  2011-02-23 15:14   ` [PATCH v3 3/5] Check properties before setting client configs Anderson Lizardo
@ 2011-02-23 15:14   ` Anderson Lizardo
  2011-02-23 15:14   ` [PATCH v3 5/5] Implement ATT handle indication Anderson Lizardo
  5 siblings, 0 replies; 21+ messages in thread
From: Anderson Lizardo @ 2011-02-23 15:14 UTC (permalink / raw
  To: linux-bluetooth; +Cc: Andre Dieb Martins

From: Andre Dieb Martins <andre.dieb@signove.com>

Adds an initial version of ATT handle notification. Notifications are sent to
interested clients (based on the Client Characteristic Configuration) always
when the attribute is updated.

This enables services to call db_attrib_update() and send notifications on their
own terms.
---
 src/attrib-server.c |   26 ++++++++++++++++++++++++++
 1 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/src/attrib-server.c b/src/attrib-server.c
index 21da17e..62c10f4 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -965,6 +965,30 @@ static void confirm_event(GIOChannel *io, void *user_data)
 	return;
 }
 
+static void attrib_notify_clients(struct attribute *attr)
+{
+	guint handle = attr->handle;
+	GSList *l;
+
+	for (l = clients; l; l = l->next) {
+		struct gatt_channel *channel = l->data;
+
+		/* Notification */
+		if (g_slist_find_custom(channel->notify,
+					GUINT_TO_POINTER(handle), handle_cmp)) {
+			uint8_t pdu[ATT_MAX_MTU];
+			uint16_t len;
+
+			len = enc_notification(attr, pdu, channel->mtu);
+			if (len == 0)
+				continue;
+
+			g_attrib_send(channel->attrib, 0, pdu[0], pdu, len,
+							NULL, NULL, NULL);
+		}
+	}
+}
+
 static gboolean register_core_services(void)
 {
 	uint8_t atval[256];
@@ -1209,6 +1233,8 @@ int attrib_db_update(uint16_t handle, uuid_t *uuid, const uint8_t *value,
 	a->len = len;
 	memcpy(a->data, value, len);
 
+	attrib_notify_clients(a);
+
 	return 0;
 }
 
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v3 5/5] Implement ATT handle indication
  2011-02-22 21:01 ` [PATCHv2 1/5] Add read/write callbacks to attribute server Anderson Lizardo
                     ` (4 preceding siblings ...)
  2011-02-23 15:14   ` [PATCH v3 4/5] Implement server-side ATT handle notification Anderson Lizardo
@ 2011-02-23 15:14   ` Anderson Lizardo
  2011-02-24 19:10     ` Johan Hedberg
  5 siblings, 1 reply; 21+ messages in thread
From: Anderson Lizardo @ 2011-02-23 15:14 UTC (permalink / raw
  To: linux-bluetooth; +Cc: Elvis Pfützenreuter

From: Elvis Pfützenreuter <epx@signove.com>

---
 src/attrib-server.c |   40 ++++++++++++++++++++++++++++++++++++++++
 1 files changed, 40 insertions(+), 0 deletions(-)

diff --git a/src/attrib-server.c b/src/attrib-server.c
index 62c10f4..a0c30b5 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -61,6 +61,8 @@ struct gatt_channel {
 	guint mtu;
 	guint id;
 	gboolean encrypted;
+	guint outstanding_indications;
+	time_t oldest_indication;
 };
 
 struct group_elem {
@@ -889,6 +891,10 @@ static void channel_handler(const uint8_t *ipdu, uint16_t len,
 		length = find_by_type(start, end, &uuid, value, vlen,
 							opdu, channel->mtu);
 		break;
+	case ATT_OP_HANDLE_CNF:
+		if (channel->outstanding_indications)
+			channel->outstanding_indications -= 1;
+		return;
 	case ATT_OP_READ_MULTI_REQ:
 	case ATT_OP_PREP_WRITE_REQ:
 	case ATT_OP_EXEC_WRITE_REQ:
@@ -968,8 +974,11 @@ static void confirm_event(GIOChannel *io, void *user_data)
 static void attrib_notify_clients(struct attribute *attr)
 {
 	guint handle = attr->handle;
+	time_t now;
 	GSList *l;
 
+	time(&now);
+
 	for (l = clients; l; l = l->next) {
 		struct gatt_channel *channel = l->data;
 
@@ -986,6 +995,37 @@ static void attrib_notify_clients(struct attribute *attr)
 			g_attrib_send(channel->attrib, 0, pdu[0], pdu, len,
 							NULL, NULL, NULL);
 		}
+
+		/* The outstanding_indications variable counts how many
+		 * unconfirmed indications have been sent. The oldest_indication
+		 * variable gives a small time frame to accomodate
+		 * almost-concomitant indications, sending both without forcing
+		 * a serialization.
+		 *
+		 * Indications are withheld once oldest_indication is older than
+		 * at least 1 second and there are pending confirmations. */
+
+		if (channel->outstanding_indications &&
+				(now - channel->oldest_indication) > 1)
+			continue;
+
+		/* Indication */
+		if (g_slist_find_custom(channel->indicate,
+				GUINT_TO_POINTER(handle), handle_cmp) != NULL) {
+			uint8_t pdu[ATT_MAX_MTU];
+			uint16_t len;
+
+			len = enc_indication(attr, pdu, channel->mtu);
+			if (len == 0)
+				continue;
+
+			g_attrib_send(channel->attrib, 0, pdu[0], pdu, len,
+							NULL, NULL, NULL);
+
+			channel->outstanding_indications += 1;
+			if (channel->outstanding_indications == 1)
+				channel->oldest_indication = now;
+		}
 	}
 }
 
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 5/5] Implement ATT handle indication
  2011-02-23 15:14   ` [PATCH v3 5/5] Implement ATT handle indication Anderson Lizardo
@ 2011-02-24 19:10     ` Johan Hedberg
  0 siblings, 0 replies; 21+ messages in thread
From: Johan Hedberg @ 2011-02-24 19:10 UTC (permalink / raw
  To: Anderson Lizardo; +Cc: linux-bluetooth, Elvis Pfützenreuter

Hi,

On Wed, Feb 23, 2011, Anderson Lizardo wrote:
> ---
>  src/attrib-server.c |   40 ++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 40 insertions(+), 0 deletions(-)

I've pushed patches 1-4, but as there was some debate about this one
I'll wait until there's an updated version.

Johan

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCHv2 2/5] Initial Client Characteristic Configuration implementation
  2011-02-23  3:26   ` Johan Hedberg
  2011-02-23 14:29     ` Anderson Lizardo
@ 2011-03-07 20:52     ` Peter Dons Tychsen
  1 sibling, 0 replies; 21+ messages in thread
From: Peter Dons Tychsen @ 2011-03-07 20:52 UTC (permalink / raw
  To: Johan Hedberg; +Cc: Anderson Lizardo, linux-bluetooth

Hi,

On Wed, 2011-02-23 at 00:26 -0300, Johan Hedberg wrote:
> On Tue, Feb 22, 2011, Anderson Lizardo wrote:
> > +static uint8_t client_set_notifications(struct attribute *attr,
> > +                                                     gpointer
> user_data)
> > +{
> > +     struct gatt_channel *channel = user_data;
> > +     struct attribute *a, *last_chr_val = NULL;
> > +     uint16_t handle, cfg_val;
> > +     uuid_t uuid;
> > +     GSList *l;
> > +
> > +     cfg_val = att_get_u16(attr->data);
> > +
> > +     sdp_uuid16_create(&uuid, GATT_CHARAC_UUID);
> > +     for (l = database, handle = 0; l != NULL; l = l->next) {
> > +             a = l->data;
> 
> The variable "a" is only used inside the for-loop so it should be
> declared inside it as well. I think you can move handle inside the
> loop
> as well as long as you declare it static (so it only gets initialized
> to
> 0 on the first iteration). 

Would that not be a waste? Declaring it static would place it in
global-space. IMHO local variables must never be static, unless there
really is a need for it. For embedded devices with less memory, to many
of such constructs would be considered a problem. 

Putting it inside or outside the loops makes no difference with most
compilers as the optimizer will figure it out anyway (to the code, not
the style), but putting static in front will change the output and will
lower the global memory available. And since the number of used
instructions most likely will be the same, putting static in front will
at best be a waste.

Thanks,

/pedro

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2011-03-07 20:52 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-02-21 21:30 [PATCH 1/5] Add read/write callbacks to attribute server Anderson Lizardo
2011-02-21 21:30 ` [PATCH 2/5] Initial Client Characteristic Configuration implementation Anderson Lizardo
2011-02-21 21:30 ` [PATCH 3/5] Check permissions before setting client configs Anderson Lizardo
2011-02-21 21:30 ` [PATCH 4/5] Implement server-side ATT handle notification Anderson Lizardo
2011-02-21 21:30 ` [PATCH 5/5] Implement ATT handle indication Anderson Lizardo
2011-02-22 21:01 ` [PATCHv2 1/5] Add read/write callbacks to attribute server Anderson Lizardo
2011-02-23  3:21   ` Johan Hedberg
2011-02-23 14:28     ` Anderson Lizardo
2011-02-23 15:14   ` [PATCH v3 " Anderson Lizardo
2011-02-23 15:14   ` [PATCH v3 2/5] Initial Client Characteristic Configuration implementation Anderson Lizardo
2011-02-23 15:14   ` [PATCH v3 3/5] Check properties before setting client configs Anderson Lizardo
2011-02-23 15:14   ` [PATCH v3 4/5] Implement server-side ATT handle notification Anderson Lizardo
2011-02-23 15:14   ` [PATCH v3 5/5] Implement ATT handle indication Anderson Lizardo
2011-02-24 19:10     ` Johan Hedberg
2011-02-22 21:01 ` [PATCHv2 2/5] Initial Client Characteristic Configuration implementation Anderson Lizardo
2011-02-23  3:26   ` Johan Hedberg
2011-02-23 14:29     ` Anderson Lizardo
2011-03-07 20:52     ` Peter Dons Tychsen
2011-02-22 21:01 ` [PATCHv2 3/5] Check permissions before setting client configs Anderson Lizardo
2011-02-22 21:01 ` [PATCHv2 4/5] Implement server-side ATT handle notification Anderson Lizardo
2011-02-22 21:01 ` [PATCHv2 5/5] Implement ATT handle indication Anderson Lizardo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.