2011-02-21 21:30:39

by Anderson Lizardo

[permalink] [raw]
Subject: [PATCH 1/5] Add read/write callbacks to attribute server

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



2011-02-24 19:10:22

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH v3 5/5] Implement ATT handle indication

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

2011-02-23 15:14:17

by Anderson Lizardo

[permalink] [raw]
Subject: [PATCH v3 1/5] Add read/write callbacks to attribute server

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


2011-02-23 15:14:20

by Anderson Lizardo

[permalink] [raw]
Subject: [PATCH v3 4/5] Implement server-side ATT handle notification

From: Andre Dieb Martins <[email protected]>

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


2011-02-23 15:14:18

by Anderson Lizardo

[permalink] [raw]
Subject: [PATCH v3 2/5] Initial Client Characteristic Configuration implementation

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


2011-02-23 15:14:21

by Anderson Lizardo

[permalink] [raw]
Subject: [PATCH v3 5/5] Implement ATT handle indication

From: Elvis Pfützenreuter <[email protected]>

---
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


2011-02-23 15:14:19

by Anderson Lizardo

[permalink] [raw]
Subject: [PATCH v3 3/5] Check properties before setting client configs

From: Andre Dieb Martins <[email protected]>

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


2011-02-23 14:28:11

by Anderson Lizardo

[permalink] [raw]
Subject: Re: [PATCHv2 1/5] Add read/write callbacks to attribute server

Hi Johan,

On Wed, Feb 23, 2011 at 12:21 AM, Johan Hedberg <[email protected]> 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

2011-02-23 14:29:15

by Anderson Lizardo

[permalink] [raw]
Subject: Re: [PATCHv2 2/5] Initial Client Characteristic Configuration implementation

Hi Johan,

On Wed, Feb 23, 2011 at 12:26 AM, Johan Hedberg <[email protected]> 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

2011-02-23 03:26:08

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCHv2 2/5] Initial Client Characteristic Configuration implementation

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

2011-02-23 03:21:54

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCHv2 1/5] Add read/write callbacks to attribute server

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

2011-02-22 21:01:27

by Anderson Lizardo

[permalink] [raw]
Subject: [PATCHv2 1/5] Add read/write callbacks to attribute server

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


2011-02-22 21:01:31

by Anderson Lizardo

[permalink] [raw]
Subject: [PATCHv2 5/5] Implement ATT handle indication

From: Elvis Pfützenreuter <[email protected]>

---
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


2011-02-22 21:01:30

by Anderson Lizardo

[permalink] [raw]
Subject: [PATCHv2 4/5] Implement server-side ATT handle notification

From: Andre Dieb Martins <[email protected]>

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


2011-02-22 21:01:29

by Anderson Lizardo

[permalink] [raw]
Subject: [PATCHv2 3/5] Check permissions before setting client configs

From: Andre Dieb Martins <[email protected]>

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


2011-02-22 21:01:28

by Anderson Lizardo

[permalink] [raw]
Subject: [PATCHv2 2/5] Initial Client Characteristic Configuration implementation

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


2011-02-21 21:30:43

by Anderson Lizardo

[permalink] [raw]
Subject: [PATCH 5/5] Implement ATT handle indication

From: Elvis Pfützenreuter <[email protected]>

---
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


2011-02-21 21:30:41

by Anderson Lizardo

[permalink] [raw]
Subject: [PATCH 3/5] Check permissions before setting client configs

From: Andre Dieb Martins <[email protected]>

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


2011-02-21 21:30:42

by Anderson Lizardo

[permalink] [raw]
Subject: [PATCH 4/5] Implement server-side ATT handle notification

From: Andre Dieb Martins <[email protected]>

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


2011-02-21 21:30:40

by Anderson Lizardo

[permalink] [raw]
Subject: [PATCH 2/5] Initial Client Characteristic Configuration implementation

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


2011-03-07 20:52:24

by Peter Dons Tychsen

[permalink] [raw]
Subject: Re: [PATCHv2 2/5] Initial Client Characteristic Configuration implementation

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