* [PATCH BlueZ 00/15] core: Use shared/gatt for GATT client role
@ 2014-12-09 0:40 Arman Uguray
2014-12-09 0:40 ` [PATCH BlueZ 01/15] attrib/gattrib: Add g_attrib_get_att Arman Uguray
` (15 more replies)
0 siblings, 16 replies; 22+ messages in thread
From: Arman Uguray @ 2014-12-09 0:40 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Arman Uguray
This patch set integrates shared/gatt into the core bluetoothd for GATT
client role. This set makes the following changes:
- Made small fixes and added getters to support legacy code paths.
- Removed explicit primary and included service discovery logic within
src/device in favor of initializing a bt_gatt_client. This includes
deferring service/profile registration until the bt_gatt_client is ready,
and proper handling for "Service Changed" events using gatt_db's service
added/removed callbacks. When new services are added during a connection,
the profiles are explicitly probed. The code still creates and maintains
instances of struct gatt_primary to support legacy plugins.
- Introduced gatt-callbacks.h. This defines callbacks that are invoked when
services are removed, the gatt-client becomes ready, and when the connection
is closed. These callbacks are similar and parallel to the existing ones
defined in attio.h and provide an alternate solution for shared/gatt.
gatt-callbacks.h is not meant to be the long-term solution and these may
perhaps be better served by new GATT specific functions added to the
btd_profile/btd_service framework.
- Rewrote the built-in GAP/GATT profile. It no longer handles the GATT
service and does no MTU exchange (which was racy and did not belong in a
profile in the first place) as these are handled by shared/gatt-client.
Renamed the profile to profiles/gap as it only handles the remote GAP
service and added handling for the "Device Name" characteristic. The profile
now uses gatt-callbacks instead of attio and bt_gatt_client/gatt_db instead
of GAttrib.
Arman Uguray (15):
attrib/gattrib: Add g_attrib_get_att.
shared/att: Add bt_att_get_fd.
attrib: Check if attrib is NULL in functions
shared/att: cancel_all before calling disconnect cb
shared/gatt-db: Make accessors work on const ptr
shared/gatt-db: Add UUID arg to foreach_service
core: device: Use bt_att_register_disconnect.
core: device: Use shared/gatt-client for GATT.
core: Rename device_attach_attrib
core: Use gatt_db service callbacks
core: Introduce gatt-callbacks
profiles/gatt: Don't handle GATT service.
profiles/gatt: Rename profile to gap.
profiles/gap: Rewrite using bt_gatt_client.
profiles/gap: Add Google copyright.
Makefile.am | 1 +
Makefile.plugins | 4 +-
attrib/gattrib.c | 31 ++-
attrib/gattrib.h | 3 +
profiles/gap/gas.c | 321 +++++++++++++++++++++++++++
profiles/gatt/gas.c | 457 --------------------------------------
src/attrib-server.c | 2 +-
src/device.c | 591 +++++++++++++++++++++++++++++++++++--------------
src/device.h | 2 +-
src/gatt-callbacks.h | 36 +++
src/shared/att-types.h | 3 +-
src/shared/att.c | 12 +-
src/shared/att.h | 1 +
src/shared/gatt-db.c | 41 ++--
src/shared/gatt-db.h | 28 ++-
tools/btgatt-client.c | 9 +-
tools/btgatt-server.c | 2 +-
unit/test-gatt.c | 4 +-
18 files changed, 885 insertions(+), 663 deletions(-)
create mode 100644 profiles/gap/gas.c
delete mode 100644 profiles/gatt/gas.c
create mode 100644 src/gatt-callbacks.h
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH BlueZ 01/15] attrib/gattrib: Add g_attrib_get_att.
2014-12-09 0:40 [PATCH BlueZ 00/15] core: Use shared/gatt for GATT client role Arman Uguray
@ 2014-12-09 0:40 ` Arman Uguray
2014-12-09 0:40 ` [PATCH BlueZ 02/15] shared/att: Add bt_att_get_fd Arman Uguray
` (14 subsequent siblings)
15 siblings, 0 replies; 22+ messages in thread
From: Arman Uguray @ 2014-12-09 0:40 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Arman Uguray
Added the g_attrib_get_att function which returns the underlying bt_att
structure associated with a GAttrib.
---
attrib/gattrib.c | 8 ++++++++
attrib/gattrib.h | 3 +++
2 files changed, 11 insertions(+)
diff --git a/attrib/gattrib.c b/attrib/gattrib.c
index cfa3b78..f3b1366 100644
--- a/attrib/gattrib.c
+++ b/attrib/gattrib.c
@@ -169,6 +169,14 @@ GIOChannel *g_attrib_get_channel(GAttrib *attrib)
return attrib->io;
}
+struct bt_att *g_attrib_get_att(GAttrib *attrib)
+{
+ if (!attrib)
+ return NULL;
+
+ return attrib->att;
+}
+
gboolean g_attrib_set_destroy_function(GAttrib *attrib, GDestroyNotify destroy,
gpointer user_data)
{
diff --git a/attrib/gattrib.h b/attrib/gattrib.h
index 2ed57c1..374bac2 100644
--- a/attrib/gattrib.h
+++ b/attrib/gattrib.h
@@ -31,6 +31,7 @@ extern "C" {
#define GATTRIB_ALL_REQS 0xFE
#define GATTRIB_ALL_HANDLES 0x0000
+struct bt_att; /* Forward declaration for compatibility */
struct _GAttrib;
typedef struct _GAttrib GAttrib;
@@ -47,6 +48,8 @@ void g_attrib_unref(GAttrib *attrib);
GIOChannel *g_attrib_get_channel(GAttrib *attrib);
+struct bt_att *g_attrib_get_att(GAttrib *attrib);
+
gboolean g_attrib_set_destroy_function(GAttrib *attrib,
GDestroyNotify destroy, gpointer user_data);
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH BlueZ 02/15] shared/att: Add bt_att_get_fd.
2014-12-09 0:40 [PATCH BlueZ 00/15] core: Use shared/gatt for GATT client role Arman Uguray
2014-12-09 0:40 ` [PATCH BlueZ 01/15] attrib/gattrib: Add g_attrib_get_att Arman Uguray
@ 2014-12-09 0:40 ` Arman Uguray
2014-12-09 12:50 ` Luiz Augusto von Dentz
2014-12-09 0:40 ` [PATCH BlueZ 03/15] attrib: Check if attrib is NULL in functions Arman Uguray
` (13 subsequent siblings)
15 siblings, 1 reply; 22+ messages in thread
From: Arman Uguray @ 2014-12-09 0:40 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Arman Uguray
This patch adds a getter for a bt_att structure's underlying connection
file descriptor.
---
src/shared/att.c | 8 ++++++++
src/shared/att.h | 1 +
2 files changed, 9 insertions(+)
diff --git a/src/shared/att.c b/src/shared/att.c
index ee425d8..9511bb2 100644
--- a/src/shared/att.c
+++ b/src/shared/att.c
@@ -922,6 +922,14 @@ void bt_att_unref(struct bt_att *att)
free(att);
}
+int bt_att_get_fd(struct bt_att *att)
+{
+ if (!att)
+ return -EINVAL;
+
+ return att->fd;
+}
+
bool bt_att_set_close_on_unref(struct bt_att *att, bool do_close)
{
if (!att || !att->io)
diff --git a/src/shared/att.h b/src/shared/att.h
index 99b5a5b..b946b18 100644
--- a/src/shared/att.h
+++ b/src/shared/att.h
@@ -33,6 +33,7 @@ struct bt_att *bt_att_new(int fd);
struct bt_att *bt_att_ref(struct bt_att *att);
void bt_att_unref(struct bt_att *att);
+int bt_att_get_fd(struct bt_att *att);
bool bt_att_set_close_on_unref(struct bt_att *att, bool do_close);
typedef void (*bt_att_response_func_t)(uint8_t opcode, const void *pdu,
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH BlueZ 03/15] attrib: Check if attrib is NULL in functions
2014-12-09 0:40 [PATCH BlueZ 00/15] core: Use shared/gatt for GATT client role Arman Uguray
2014-12-09 0:40 ` [PATCH BlueZ 01/15] attrib/gattrib: Add g_attrib_get_att Arman Uguray
2014-12-09 0:40 ` [PATCH BlueZ 02/15] shared/att: Add bt_att_get_fd Arman Uguray
@ 2014-12-09 0:40 ` Arman Uguray
2014-12-09 0:40 ` [PATCH BlueZ 04/15] shared/att: cancel_all before calling disconnect cb Arman Uguray
` (12 subsequent siblings)
15 siblings, 0 replies; 22+ messages in thread
From: Arman Uguray @ 2014-12-09 0:40 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Arman Uguray
This patch adds an early return to attrib/gattrib functions if the
attrib argument is NULL.
---
attrib/gattrib.c | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/attrib/gattrib.c b/attrib/gattrib.c
index f3b1366..b76d0a4 100644
--- a/attrib/gattrib.c
+++ b/attrib/gattrib.c
@@ -266,6 +266,9 @@ guint g_attrib_send(GAttrib *attrib, guint id, const guint8 *pdu, guint16 len,
bt_att_response_func_t response_cb = NULL;
bt_att_destroy_func_t destroy_cb = NULL;
+ if (!attrib)
+ return 0;
+
if (!pdu || !len)
return 0;
@@ -288,11 +291,17 @@ guint g_attrib_send(GAttrib *attrib, guint id, const guint8 *pdu, guint16 len,
gboolean g_attrib_cancel(GAttrib *attrib, guint id)
{
+ if (!attrib)
+ return FALSE;
+
return bt_att_cancel(attrib->att, id);
}
gboolean g_attrib_cancel_all(GAttrib *attrib)
{
+ if (!attrib)
+ return FALSE;
+
return bt_att_cancel_all(attrib->att);
}
@@ -302,6 +311,9 @@ guint g_attrib_register(GAttrib *attrib, guint8 opcode, guint16 handle,
{
struct attrib_callbacks *cb = NULL;
+ if (!attrib)
+ return 0;
+
if (func || notify) {
cb = new0(struct attrib_callbacks, 1);
if (!cb)
@@ -323,7 +335,7 @@ guint g_attrib_register(GAttrib *attrib, guint8 opcode, guint16 handle,
uint8_t *g_attrib_get_buffer(GAttrib *attrib, size_t *len)
{
- if (!len)
+ if (!attrib || !len)
return NULL;
*len = attrib->buflen;
@@ -332,6 +344,9 @@ uint8_t *g_attrib_get_buffer(GAttrib *attrib, size_t *len)
gboolean g_attrib_set_mtu(GAttrib *attrib, int mtu)
{
+ if (!attrib)
+ return FALSE;
+
/*
* Clients of this expect a buffer to use.
*
@@ -349,10 +364,16 @@ gboolean g_attrib_set_mtu(GAttrib *attrib, int mtu)
gboolean g_attrib_unregister(GAttrib *attrib, guint id)
{
+ if (!attrib)
+ return FALSE;
+
return bt_att_unregister(attrib->att, id);
}
gboolean g_attrib_unregister_all(GAttrib *attrib)
{
+ if (!attrib)
+ return false;
+
return bt_att_unregister_all(attrib->att);
}
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH BlueZ 04/15] shared/att: cancel_all before calling disconnect cb
2014-12-09 0:40 [PATCH BlueZ 00/15] core: Use shared/gatt for GATT client role Arman Uguray
` (2 preceding siblings ...)
2014-12-09 0:40 ` [PATCH BlueZ 03/15] attrib: Check if attrib is NULL in functions Arman Uguray
@ 2014-12-09 0:40 ` Arman Uguray
2014-12-09 0:40 ` [PATCH BlueZ 05/15] shared/gatt-db: Make accessors work on const ptr Arman Uguray
` (11 subsequent siblings)
15 siblings, 0 replies; 22+ messages in thread
From: Arman Uguray @ 2014-12-09 0:40 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Arman Uguray
Moved the call to bt_att_cancel_all to before the call to the registered
disconnect callbacks in bt_att's internal disconnect handler to make
sure that all affected user_data is destroyed. This is to prevent cases
of invalid access, where a user_data destroy function refers to data
that the upper layer might free in the disconnect callback.
---
src/shared/att.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/shared/att.c b/src/shared/att.c
index 9511bb2..fc8c598 100644
--- a/src/shared/att.c
+++ b/src/shared/att.c
@@ -554,6 +554,8 @@ static bool disconnect_cb(struct io *io, void *user_data)
util_debug(att->debug_callback, att->debug_data,
"Physical link disconnected");
+ bt_att_cancel_all(att);
+
bt_att_ref(att);
att->in_disconn = true;
queue_foreach(att->disconn_list, disconn_handler, NULL);
@@ -565,9 +567,7 @@ static bool disconnect_cb(struct io *io, void *user_data)
att->need_disconn_cleanup = false;
}
- bt_att_cancel_all(att);
bt_att_unregister_all(att);
-
bt_att_unref(att);
return false;
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH BlueZ 05/15] shared/gatt-db: Make accessors work on const ptr
2014-12-09 0:40 [PATCH BlueZ 00/15] core: Use shared/gatt for GATT client role Arman Uguray
` (3 preceding siblings ...)
2014-12-09 0:40 ` [PATCH BlueZ 04/15] shared/att: cancel_all before calling disconnect cb Arman Uguray
@ 2014-12-09 0:40 ` Arman Uguray
2014-12-09 0:40 ` [PATCH BlueZ 06/15] shared/gatt-db: Add UUID arg to foreach_service Arman Uguray
` (10 subsequent siblings)
15 siblings, 0 replies; 22+ messages in thread
From: Arman Uguray @ 2014-12-09 0:40 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Arman Uguray
Made gatt_db_attribute accessor functions accept a const pointer.
---
src/shared/gatt-db.c | 22 ++++++++++++----------
src/shared/gatt-db.h | 22 ++++++++++++----------
2 files changed, 24 insertions(+), 20 deletions(-)
diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
index 37ec946..379b4ad 100644
--- a/src/shared/gatt-db.c
+++ b/src/shared/gatt-db.c
@@ -1151,7 +1151,8 @@ struct gatt_db_attribute *gatt_db_get_attribute(struct gatt_db *db,
return service->attributes[handle - service_handle];
}
-const bt_uuid_t *gatt_db_attribute_get_type(struct gatt_db_attribute *attrib)
+const bt_uuid_t *gatt_db_attribute_get_type(
+ const struct gatt_db_attribute *attrib)
{
if (!attrib)
return NULL;
@@ -1159,7 +1160,7 @@ const bt_uuid_t *gatt_db_attribute_get_type(struct gatt_db_attribute *attrib)
return &attrib->uuid;
}
-uint16_t gatt_db_attribute_get_handle(struct gatt_db_attribute *attrib)
+uint16_t gatt_db_attribute_get_handle(const struct gatt_db_attribute *attrib)
{
if (!attrib)
return 0;
@@ -1167,7 +1168,7 @@ uint16_t gatt_db_attribute_get_handle(struct gatt_db_attribute *attrib)
return attrib->handle;
}
-bool gatt_db_attribute_get_service_uuid(struct gatt_db_attribute *attrib,
+bool gatt_db_attribute_get_service_uuid(const struct gatt_db_attribute *attrib,
bt_uuid_t *uuid)
{
struct gatt_db_service *service;
@@ -1198,9 +1199,10 @@ bool gatt_db_attribute_get_service_uuid(struct gatt_db_attribute *attrib,
return false;
}
-bool gatt_db_attribute_get_service_handles(struct gatt_db_attribute *attrib,
- uint16_t *start_handle,
- uint16_t *end_handle)
+bool gatt_db_attribute_get_service_handles(
+ const struct gatt_db_attribute *attrib,
+ uint16_t *start_handle,
+ uint16_t *end_handle)
{
struct gatt_db_service *service;
@@ -1214,7 +1216,7 @@ bool gatt_db_attribute_get_service_handles(struct gatt_db_attribute *attrib,
return true;
}
-bool gatt_db_attribute_get_service_data(struct gatt_db_attribute *attrib,
+bool gatt_db_attribute_get_service_data(const struct gatt_db_attribute *attrib,
uint16_t *start_handle,
uint16_t *end_handle,
bool *primary,
@@ -1244,7 +1246,7 @@ bool gatt_db_attribute_get_service_data(struct gatt_db_attribute *attrib,
return le_to_uuid(decl->value, decl->value_len, uuid);
}
-bool gatt_db_attribute_get_char_data(struct gatt_db_attribute *attrib,
+bool gatt_db_attribute_get_char_data(const struct gatt_db_attribute *attrib,
uint16_t *handle,
uint16_t *value_handle,
uint8_t *properties,
@@ -1281,7 +1283,7 @@ bool gatt_db_attribute_get_char_data(struct gatt_db_attribute *attrib,
return le_to_uuid(attrib->value + 3, attrib->value_len - 3, uuid);
}
-bool gatt_db_attribute_get_incl_data(struct gatt_db_attribute *attrib,
+bool gatt_db_attribute_get_incl_data(const struct gatt_db_attribute *attrib,
uint16_t *handle,
uint16_t *start_handle,
uint16_t *end_handle)
@@ -1317,7 +1319,7 @@ bool gatt_db_attribute_get_incl_data(struct gatt_db_attribute *attrib,
return true;
}
-bool gatt_db_attribute_get_permissions(struct gatt_db_attribute *attrib,
+bool gatt_db_attribute_get_permissions(const struct gatt_db_attribute *attrib,
uint32_t *permissions)
{
if (!attrib || !permissions)
diff --git a/src/shared/gatt-db.h b/src/shared/gatt-db.h
index 987ccf4..b9f12e3 100644
--- a/src/shared/gatt-db.h
+++ b/src/shared/gatt-db.h
@@ -139,35 +139,37 @@ bool gatt_db_unregister(struct gatt_db *db, unsigned int id);
struct gatt_db_attribute *gatt_db_get_attribute(struct gatt_db *db,
uint16_t handle);
-const bt_uuid_t *gatt_db_attribute_get_type(struct gatt_db_attribute *attrib);
+const bt_uuid_t *gatt_db_attribute_get_type(
+ const struct gatt_db_attribute *attrib);
-uint16_t gatt_db_attribute_get_handle(struct gatt_db_attribute *attrib);
+uint16_t gatt_db_attribute_get_handle(const struct gatt_db_attribute *attrib);
-bool gatt_db_attribute_get_service_uuid(struct gatt_db_attribute *attrib,
+bool gatt_db_attribute_get_service_uuid(const struct gatt_db_attribute *attrib,
bt_uuid_t *uuid);
-bool gatt_db_attribute_get_service_handles(struct gatt_db_attribute *attrib,
- uint16_t *start_handle,
- uint16_t *end_handle);
+bool gatt_db_attribute_get_service_handles(
+ const struct gatt_db_attribute *attrib,
+ uint16_t *start_handle,
+ uint16_t *end_handle);
-bool gatt_db_attribute_get_service_data(struct gatt_db_attribute *attrib,
+bool gatt_db_attribute_get_service_data(const struct gatt_db_attribute *attrib,
uint16_t *start_handle,
uint16_t *end_handle,
bool *primary,
bt_uuid_t *uuid);
-bool gatt_db_attribute_get_char_data(struct gatt_db_attribute *attrib,
+bool gatt_db_attribute_get_char_data(const struct gatt_db_attribute *attrib,
uint16_t *handle,
uint16_t *value_handle,
uint8_t *properties,
bt_uuid_t *uuid);
-bool gatt_db_attribute_get_incl_data(struct gatt_db_attribute *attrib,
+bool gatt_db_attribute_get_incl_data(const struct gatt_db_attribute *attrib,
uint16_t *handle,
uint16_t *start_handle,
uint16_t *end_handle);
-bool gatt_db_attribute_get_permissions(struct gatt_db_attribute *attrib,
+bool gatt_db_attribute_get_permissions(const struct gatt_db_attribute *attrib,
uint32_t *permissions);
typedef void (*gatt_db_attribute_read_t) (struct gatt_db_attribute *attrib,
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH BlueZ 06/15] shared/gatt-db: Add UUID arg to foreach_service
2014-12-09 0:40 [PATCH BlueZ 00/15] core: Use shared/gatt for GATT client role Arman Uguray
` (4 preceding siblings ...)
2014-12-09 0:40 ` [PATCH BlueZ 05/15] shared/gatt-db: Make accessors work on const ptr Arman Uguray
@ 2014-12-09 0:40 ` Arman Uguray
2014-12-09 0:40 ` [PATCH BlueZ 07/15] core: device: Use bt_att_register_disconnect Arman Uguray
` (9 subsequent siblings)
15 siblings, 0 replies; 22+ messages in thread
From: Arman Uguray @ 2014-12-09 0:40 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Arman Uguray
This patch adds the "uuid" argument to gatt_db_foreach_service, which
invokes the callback for a service only if "uuid" is NULL or if it
matches the GATT service UUID.
---
src/shared/gatt-db.c | 19 ++++++++++++++++---
src/shared/gatt-db.h | 6 ++++--
tools/btgatt-client.c | 9 ++++-----
tools/btgatt-server.c | 2 +-
unit/test-gatt.c | 4 ++--
5 files changed, 27 insertions(+), 13 deletions(-)
diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
index 379b4ad..98fb8a0 100644
--- a/src/shared/gatt-db.c
+++ b/src/shared/gatt-db.c
@@ -996,14 +996,17 @@ void gatt_db_find_information(struct gatt_db *db, uint16_t start_handle,
queue_foreach(db->services, find_information, &data);
}
-void gatt_db_foreach_service(struct gatt_db *db, gatt_db_attribute_cb_t func,
- void *user_data)
+void gatt_db_foreach_service(struct gatt_db *db, const bt_uuid_t *uuid,
+ gatt_db_attribute_cb_t func,
+ void *user_data)
{
- gatt_db_foreach_service_in_range(db, func, user_data, 0x0001, 0xffff);
+ gatt_db_foreach_service_in_range(db, uuid, func, user_data, 0x0001,
+ 0xffff);
}
struct foreach_data {
gatt_db_attribute_cb_t func;
+ const bt_uuid_t *uuid;
void *user_data;
uint16_t start, end;
};
@@ -1013,16 +1016,25 @@ static void foreach_service_in_range(void *data, void *user_data)
struct gatt_db_service *service = data;
struct foreach_data *foreach_data = user_data;
uint16_t svc_start;
+ bt_uuid_t uuid;
svc_start = get_handle_at_index(service, 0);
if (svc_start > foreach_data->end || svc_start < foreach_data->start)
return;
+ if (foreach_data->uuid) {
+ gatt_db_attribute_get_service_uuid(service->attributes[0],
+ &uuid);
+ if (bt_uuid_cmp(&uuid, foreach_data->uuid))
+ return;
+ }
+
foreach_data->func(service->attributes[0], foreach_data->user_data);
}
void gatt_db_foreach_service_in_range(struct gatt_db *db,
+ const bt_uuid_t *uuid,
gatt_db_attribute_cb_t func,
void *user_data,
uint16_t start_handle,
@@ -1034,6 +1046,7 @@ void gatt_db_foreach_service_in_range(struct gatt_db *db,
return;
data.func = func;
+ data.uuid = uuid;
data.user_data = user_data;
data.start = start_handle;
data.end = end_handle;
diff --git a/src/shared/gatt-db.h b/src/shared/gatt-db.h
index b9f12e3..e5fe6bb 100644
--- a/src/shared/gatt-db.h
+++ b/src/shared/gatt-db.h
@@ -105,9 +105,11 @@ void gatt_db_find_information(struct gatt_db *db, uint16_t start_handle,
typedef void (*gatt_db_attribute_cb_t)(struct gatt_db_attribute *attrib,
void *user_data);
-void gatt_db_foreach_service(struct gatt_db *db, gatt_db_attribute_cb_t func,
- void *user_data);
+void gatt_db_foreach_service(struct gatt_db *db, const bt_uuid_t *uuid,
+ gatt_db_attribute_cb_t func,
+ void *user_data);
void gatt_db_foreach_service_in_range(struct gatt_db *db,
+ const bt_uuid_t *uuid,
gatt_db_attribute_cb_t func,
void *user_data,
uint16_t start_handle,
diff --git a/tools/btgatt-client.c b/tools/btgatt-client.c
index fe94ae8..015142d 100644
--- a/tools/btgatt-client.c
+++ b/tools/btgatt-client.c
@@ -288,15 +288,14 @@ static void print_services(struct client *cli)
{
printf("\n");
- gatt_db_foreach_service(cli->db, print_service, cli);
+ gatt_db_foreach_service(cli->db, NULL, print_service, cli);
}
static void print_services_by_uuid(struct client *cli, const bt_uuid_t *uuid)
{
printf("\n");
- /* TODO: Filter by UUID */
- gatt_db_foreach_service(cli->db, print_service, cli);
+ gatt_db_foreach_service(cli->db, uuid, print_service, cli);
}
static void print_services_by_handle(struct client *cli, uint16_t handle)
@@ -304,7 +303,7 @@ static void print_services_by_handle(struct client *cli, uint16_t handle)
printf("\n");
/* TODO: Filter by handle */
- gatt_db_foreach_service(cli->db, print_service, cli);
+ gatt_db_foreach_service(cli->db, NULL, print_service, cli);
}
static void ready_cb(bool success, uint8_t att_ecode, void *user_data)
@@ -331,7 +330,7 @@ static void service_changed_cb(uint16_t start_handle, uint16_t end_handle,
printf("\nService Changed handled - start: 0x%04x end: 0x%04x\n",
start_handle, end_handle);
- gatt_db_foreach_service_in_range(cli->db, print_service, cli,
+ gatt_db_foreach_service_in_range(cli->db, NULL, print_service, cli,
start_handle, end_handle);
print_prompt();
}
diff --git a/tools/btgatt-server.c b/tools/btgatt-server.c
index a2a0ec9..1a9b9fb 100644
--- a/tools/btgatt-server.c
+++ b/tools/btgatt-server.c
@@ -969,7 +969,7 @@ static void print_service(struct gatt_db_attribute *attr, void *user_data)
static void cmd_services(struct server *server, char *cmd_str)
{
- gatt_db_foreach_service(server->db, print_service, server);
+ gatt_db_foreach_service(server->db, NULL, print_service, server);
}
static void cmd_help(struct server *server, char *cmd_str);
diff --git a/unit/test-gatt.c b/unit/test-gatt.c
index 3ce3d80..2f3f26a 100644
--- a/unit/test-gatt.c
+++ b/unit/test-gatt.c
@@ -414,7 +414,7 @@ static void match_services(struct gatt_db_attribute *client_serv_attr,
serv_test_data.match = client_serv_attr;
serv_test_data.found = false;
- gatt_db_foreach_service(source_db,
+ gatt_db_foreach_service(source_db, NULL,
find_matching_service, &serv_test_data);
g_assert(serv_test_data.found);
@@ -434,7 +434,7 @@ static void client_ready_cb(bool success, uint8_t att_ecode, void *user_data)
g_assert(context->client);
g_assert(context->client_db);
- gatt_db_foreach_service(context->client_db, match_services,
+ gatt_db_foreach_service(context->client_db, NULL, match_services,
context->data->source_db);
if (context->data->step) {
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH BlueZ 07/15] core: device: Use bt_att_register_disconnect.
2014-12-09 0:40 [PATCH BlueZ 00/15] core: Use shared/gatt for GATT client role Arman Uguray
` (5 preceding siblings ...)
2014-12-09 0:40 ` [PATCH BlueZ 06/15] shared/gatt-db: Add UUID arg to foreach_service Arman Uguray
@ 2014-12-09 0:40 ` Arman Uguray
2014-12-09 8:07 ` Johan Hedberg
2014-12-09 0:40 ` [PATCH BlueZ 08/15] core: device: Use shared/gatt-client for GATT Arman Uguray
` (8 subsequent siblings)
15 siblings, 1 reply; 22+ messages in thread
From: Arman Uguray @ 2014-12-09 0:40 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Arman Uguray
btd_device is now notified of ATT channel disconnections by registering
a disconnect handler with bt_att instead of setting up an io watch with
the GIOChannel.
---
src/device.c | 34 +++++++++++++++++++++-------------
1 file changed, 21 insertions(+), 13 deletions(-)
diff --git a/src/device.c b/src/device.c
index e9630ed..b31e98d 100644
--- a/src/device.c
+++ b/src/device.c
@@ -46,6 +46,8 @@
#include "log.h"
#include "src/shared/util.h"
+#include "src/shared/att.h"
+#include "src/shared/gatt-client.h"
#include "btio/btio.h"
#include "lib/uuid.h"
#include "lib/mgmt.h"
@@ -203,6 +205,9 @@ struct btd_device {
GSList *attios_offline;
guint attachid; /* Attrib server attach */
+ struct bt_att *att; /* The new ATT transport */
+ unsigned int att_disconn_id;
+
struct bearer_state bredr_state;
struct bearer_state le_state;
@@ -221,7 +226,6 @@ struct btd_device {
int8_t rssi;
GIOChannel *att_io;
- guint cleanup_id;
guint store_id;
};
@@ -466,10 +470,9 @@ static void attio_cleanup(struct btd_device *device)
device->attachid = 0;
}
- if (device->cleanup_id) {
- g_source_remove(device->cleanup_id);
- device->cleanup_id = 0;
- }
+ if (device->att_disconn_id)
+ bt_att_unregister_disconnect(device->att,
+ device->att_disconn_id);
if (device->att_io) {
g_io_channel_shutdown(device->att_io, FALSE, NULL);
@@ -477,6 +480,11 @@ static void attio_cleanup(struct btd_device *device)
device->att_io = NULL;
}
+ if (device->att) {
+ bt_att_unref(device->att);
+ device->att = NULL;
+ }
+
if (device->attrib) {
GAttrib *attrib = device->attrib;
device->attrib = NULL;
@@ -3401,8 +3409,7 @@ static void attio_disconnected(gpointer data, gpointer user_data)
attio->dcfunc(attio->user_data);
}
-static gboolean attrib_disconnected_cb(GIOChannel *io, GIOCondition cond,
- gpointer user_data)
+static void att_disconnected_cb(void *user_data)
{
struct btd_device *device = user_data;
int sock, err = 0;
@@ -3413,7 +3420,7 @@ static gboolean attrib_disconnected_cb(GIOChannel *io, GIOCondition cond,
if (device->browse)
goto done;
- sock = g_io_channel_unix_get_fd(io);
+ sock = bt_att_get_fd(device->att);
len = sizeof(err);
getsockopt(sock, SOL_SOCKET, SO_ERROR, &err, &len);
@@ -3436,8 +3443,6 @@ static gboolean attrib_disconnected_cb(GIOChannel *io, GIOCondition cond,
done:
attio_cleanup(device);
-
- return FALSE;
}
static void register_all_services(struct browse_req *req, GSList *services)
@@ -3656,7 +3661,7 @@ bool device_attach_attrib(struct btd_device *dev, GIOChannel *io)
}
if (cid == ATT_CID)
- mtu = ATT_DEFAULT_LE_MTU;
+ mtu = BT_ATT_DEFAULT_LE_MTU;
attrib = g_attrib_new(io, mtu);
if (!attrib) {
@@ -3672,8 +3677,11 @@ bool device_attach_attrib(struct btd_device *dev, GIOChannel *io)
}
dev->attrib = attrib;
- dev->cleanup_id = g_io_add_watch(io, G_IO_HUP,
- attrib_disconnected_cb, dev);
+ dev->att = bt_att_ref(g_attrib_get_att(attrib));
+
+ dev->att_disconn_id = bt_att_register_disconnect(dev->att,
+ att_disconnected_cb, dev, NULL);
+ bt_att_set_close_on_unref(dev->att, true);
/*
* Remove the device from the connect_list and give the passive
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH BlueZ 08/15] core: device: Use shared/gatt-client for GATT.
2014-12-09 0:40 [PATCH BlueZ 00/15] core: Use shared/gatt for GATT client role Arman Uguray
` (6 preceding siblings ...)
2014-12-09 0:40 ` [PATCH BlueZ 07/15] core: device: Use bt_att_register_disconnect Arman Uguray
@ 2014-12-09 0:40 ` Arman Uguray
2014-12-09 0:40 ` [PATCH BlueZ 09/15] core: Rename device_attach_attrib Arman Uguray
` (7 subsequent siblings)
15 siblings, 0 replies; 22+ messages in thread
From: Arman Uguray @ 2014-12-09 0:40 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Arman Uguray
This patch changes the GATT service discovery code path with the
following:
- As part for device_attach_attrib, a bt_gatt_client structure is
created that performs MTU exchange and service discovery, caches the
services in an internal list, and handles the remote GATT service.
- The device_browse_primary code path obtains service information
by iterating through bt_gatt_client's services, instead of directly
initiating primary and secondary service discovery using attrib/gatt.
If the bt_gatt_client isn't ready at the time, the code defers
registration of services and profile probing until the ready handler
is called.
- The attio connected callbacks are invoked from bt_gatt_client's
ready handler instead of device_attach_attrib.
---
src/device.c | 310 ++++++++++++++++++++++++++-----------------------
src/shared/att-types.h | 3 +-
2 files changed, 168 insertions(+), 145 deletions(-)
diff --git a/src/device.c b/src/device.c
index b31e98d..dcc03af 100644
--- a/src/device.c
+++ b/src/device.c
@@ -45,11 +45,13 @@
#include "log.h"
+#include "lib/uuid.h"
#include "src/shared/util.h"
#include "src/shared/att.h"
+#include "src/shared/queue.h"
+#include "src/shared/gatt-db.h"
#include "src/shared/gatt-client.h"
#include "btio/btio.h"
-#include "lib/uuid.h"
#include "lib/mgmt.h"
#include "attrib/att.h"
#include "hcid.h"
@@ -74,6 +76,10 @@
#define DISCONNECT_TIMER 2
#define DISCOVERY_TIMER 1
+#ifndef MIN
+#define MIN(a, b) ((a) < (b) ? (a) : (b))
+#endif
+
static DBusConnection *dbus_conn = NULL;
unsigned service_state_cb_id;
@@ -205,9 +211,18 @@ struct btd_device {
GSList *attios_offline;
guint attachid; /* Attrib server attach */
- struct bt_att *att; /* The new ATT transport */
+ struct bt_att *att; /* The new ATT transport */
+ uint16_t att_mtu; /* The ATT MTU */
unsigned int att_disconn_id;
+ /*
+ * TODO: For now, device creates and owns the client-role gatt_db, but
+ * this needs to be persisted in a more central place so that proper
+ * attribute cache support can be built.
+ */
+ struct gatt_db *client_db; /* GATT client cache */
+ struct bt_gatt_client *gatt_client; /* GATT client implementation */
+
struct bearer_state bredr_state;
struct bearer_state le_state;
@@ -236,7 +251,7 @@ static const uint16_t uuid_list[] = {
0
};
-static int device_browse_primary(struct btd_device *device, DBusMessage *msg);
+static int device_browse_gatt(struct btd_device *device, DBusMessage *msg);
static int device_browse_sdp(struct btd_device *device, DBusMessage *msg);
static struct bearer_state *get_state(struct btd_device *dev,
@@ -463,6 +478,21 @@ static void browse_request_free(struct browse_req *req)
g_free(req);
}
+static void gatt_client_cleanup(struct btd_device *device)
+{
+ if (!device->gatt_client)
+ return;
+
+ bt_gatt_client_set_service_changed(device->gatt_client, NULL, NULL,
+ NULL);
+ bt_gatt_client_set_ready_handler(device->gatt_client, NULL, NULL, NULL);
+ bt_gatt_client_unref(device->gatt_client);
+ device->gatt_client = NULL;
+
+ if (!device->le_state.bonded)
+ gatt_db_clear(device->client_db);
+}
+
static void attio_cleanup(struct btd_device *device)
{
if (device->attachid) {
@@ -480,6 +510,8 @@ static void attio_cleanup(struct btd_device *device)
device->att_io = NULL;
}
+ gatt_client_cleanup(device);
+
if (device->att) {
bt_att_unref(device->att);
device->att = NULL;
@@ -530,6 +562,8 @@ static void device_free(gpointer user_data)
attio_cleanup(device);
+ gatt_db_unref(device->client_db);
+
if (device->tmp_records)
sdp_list_free(device->tmp_records,
(sdp_free_func_t) sdp_record_free);
@@ -1450,7 +1484,7 @@ resolve_services:
if (bdaddr_type == BDADDR_BREDR)
err = device_browse_sdp(dev, msg);
else
- err = device_browse_primary(dev, msg);
+ err = device_browse_gatt(dev, msg);
if (err < 0)
return btd_error_failed(msg, strerror(-err));
@@ -2344,6 +2378,12 @@ static struct btd_device *device_new(struct btd_adapter *adapter,
if (device == NULL)
return NULL;
+ device->client_db = gatt_db_new();
+ if (!device->client_db) {
+ g_free(device);
+ return NULL;
+ }
+
address_up = g_ascii_strup(address, -1);
device->path = g_strdup_printf("%s/dev_%s", adapter_path, address_up);
g_strdelimit(device->path, ":", '_');
@@ -3162,7 +3202,7 @@ static int primary_cmp(gconstpointer a, gconstpointer b)
return memcmp(a, b, sizeof(struct gatt_primary));
}
-static void update_gatt_services(struct browse_req *req, GSList *current,
+static void update_gatt_uuids(struct browse_req *req, GSList *current,
GSList *found)
{
GSList *l, *lmatch;
@@ -3445,41 +3485,6 @@ done:
attio_cleanup(device);
}
-static void register_all_services(struct browse_req *req, GSList *services)
-{
- struct btd_device *device = req->device;
-
- btd_device_set_temporary(device, FALSE);
-
- update_gatt_services(req, device->primaries, services);
- g_slist_free_full(device->primaries, g_free);
- device->primaries = NULL;
-
- device_register_primaries(device, services, -1);
-
- device_probe_profiles(device, req->profiles_added);
-
- if (device->attios == NULL && device->attios_offline == NULL)
- attio_cleanup(device);
-
- g_dbus_emit_property_changed(dbus_conn, device->path,
- DEVICE_INTERFACE, "UUIDs");
-
- device_svc_resolved(device, device->bdaddr_type, 0);
-
- store_services(device);
-
- browse_request_free(req);
-}
-
-static int service_by_range_cmp(gconstpointer a, gconstpointer b)
-{
- const struct gatt_primary *prim = a;
- const struct att_range *range = b;
-
- return memcmp(&prim->range, range, sizeof(*range));
-}
-
static void send_le_browse_response(struct browse_req *req)
{
struct btd_device *dev = req->device;
@@ -3510,122 +3515,132 @@ static void send_le_browse_response(struct browse_req *req)
g_dbus_send_reply(dbus_conn, msg, DBUS_TYPE_INVALID);
}
-static void find_included_cb(uint8_t status, GSList *includes, void *user_data)
+static void add_primary(struct gatt_db_attribute *attr, void *user_data)
{
- struct included_search *search = user_data;
- struct btd_device *device = search->req->device;
struct gatt_primary *prim;
- GSList *l;
+ GSList **services = user_data;
+ bt_uuid_t uuid;
- DBG("status %u", status);
+ prim = g_new0(struct gatt_primary, 1);
+ if (!prim) {
+ DBG("Failed to allocate gatt_primary structure");
+ return;
+ }
- if (device->attrib == NULL || status) {
- struct browse_req *req = device->browse;
+ gatt_db_attribute_get_service_handles(attr, &prim->range.start,
+ &prim->range.end);
+ gatt_db_attribute_get_service_uuid(attr, &uuid);
+ bt_uuid_to_string(&uuid, prim->uuid, sizeof(prim->uuid));
- if (status)
- error("Find included services failed: %s (%d)",
- att_ecode2str(status), status);
- else
- error("Disconnected while doing included discovery");
+ *services = g_slist_append(*services, prim);
+}
- if (!req)
- goto complete;
+static void register_gatt_services(struct browse_req *req)
+{
+ struct btd_device *device = req->device;
+ GSList *services = NULL;
- send_le_browse_response(req);
- device->browse = NULL;
- browse_request_free(req);
+ if (!bt_gatt_client_is_ready(device->gatt_client))
+ return;
- goto complete;
- }
+ /*
+ * TODO: Remove the primaries list entirely once all profiles use
+ * shared/gatt.
+ */
+ gatt_db_foreach_service(device->client_db, NULL, add_primary,
+ &services);
- if (includes == NULL)
- goto next;
+ btd_device_set_temporary(device, FALSE);
- for (l = includes; l; l = l->next) {
- struct gatt_included *incl = l->data;
+ update_gatt_uuids(req, device->primaries, services);
+ g_slist_free_full(device->primaries, g_free);
+ device->primaries = NULL;
- if (g_slist_find_custom(search->services, &incl->range,
- service_by_range_cmp))
- continue;
+ device_register_primaries(device, services, -1);
- prim = g_new0(struct gatt_primary, 1);
- memcpy(prim->uuid, incl->uuid, sizeof(prim->uuid));
- memcpy(&prim->range, &incl->range, sizeof(prim->range));
+ device_probe_profiles(device, req->profiles_added);
- search->services = g_slist_append(search->services, prim);
- }
+ if (device->attios == NULL && device->attios_offline == NULL)
+ attio_cleanup(device);
-next:
- search->current = search->current->next;
- if (search->current == NULL) {
- register_all_services(search->req, search->services);
- search->services = NULL;
- goto complete;
- }
+ device_svc_resolved(device, device->bdaddr_type, 0);
- prim = search->current->data;
- gatt_find_included(device->attrib, prim->range.start, prim->range.end,
- find_included_cb, search);
- return;
+ store_services(device);
-complete:
- g_slist_free_full(search->services, g_free);
- g_free(search);
+ browse_request_free(req);
}
-static void find_included_services(struct browse_req *req, GSList *services)
+static void gatt_client_ready_cb(bool success, uint8_t att_ecode,
+ void *user_data)
{
- struct btd_device *device = req->device;
- struct included_search *search;
- struct gatt_primary *prim;
- GSList *l;
+ struct btd_device *device = user_data;
+
+ DBG("gatt-client ready -- status: %s, ATT error: %u",
+ success ? "success" : "failed",
+ att_ecode);
- DBG("service count %u", g_slist_length(services));
+ if (!success) {
+ if (device->browse) {
+ struct browse_req *req = device->browse;
+
+ send_le_browse_response(req);
+ device->browse = NULL;
+ browse_request_free(req);
+ }
- if (services == NULL) {
- DBG("No services found");
- register_all_services(req, NULL);
return;
}
- search = g_new0(struct included_search, 1);
- search->req = req;
-
- /* We have to completely duplicate the data in order to have a
- * clearly defined responsibility of freeing regardless of
- * failure or success. Otherwise memory leaks are inevitable.
- */
- for (l = services; l; l = g_slist_next(l)) {
- struct gatt_primary *dup;
+ device->att_mtu = bt_att_get_mtu(device->att);
- dup = g_memdup(l->data, sizeof(struct gatt_primary));
+ DBG("gatt-client exchanged MTU: %u", device->att_mtu);
- search->services = g_slist_append(search->services, dup);
- }
+ if (device->browse)
+ register_gatt_services(device->browse);
- search->current = search->services;
+ /*
+ * TODO: Change attio callbacks to accept a gatt-client instead of a
+ * GAttrib.
+ */
+ g_slist_foreach(device->attios, attio_connected, device->attrib);
+}
- prim = search->current->data;
- gatt_find_included(device->attrib, prim->range.start, prim->range.end,
- find_included_cb, search);
+static void gatt_client_service_changed(uint16_t start_handle,
+ uint16_t end_handle,
+ void *user_data)
+{
+ DBG("gatt-client: Service Changed: start 0x%04x, end: 0x%04x",
+ start_handle, end_handle);
}
-static void primary_cb(uint8_t status, GSList *services, void *user_data)
+static void gatt_client_init(struct btd_device *device)
{
- struct browse_req *req = user_data;
+ gatt_client_cleanup(device);
- DBG("status %u", status);
+ device->gatt_client = bt_gatt_client_new(device->client_db, device->att,
+ device->att_mtu);
+ if (!device->gatt_client) {
+ DBG("Failed to initialize gatt-client");
+ return;
+ }
- if (status) {
- struct btd_device *device = req->device;
+ if (!bt_gatt_client_set_ready_handler(device->gatt_client,
+ gatt_client_ready_cb,
+ device, NULL)) {
+ DBG("Failed to set ready handler for gatt-client");
+ gatt_client_cleanup(device);
+ return;
+ }
- send_le_browse_response(req);
- device->browse = NULL;
- browse_request_free(req);
+ if (!bt_gatt_client_set_service_changed(device->gatt_client,
+ gatt_client_service_changed,
+ device, NULL)) {
+ DBG("Failed to set service changed handler for gatt-client");
+ gatt_client_cleanup(device);
return;
}
- find_included_services(req, services);
+ DBG("gatt-client created");
}
bool device_attach_attrib(struct btd_device *dev, GIOChannel *io)
@@ -3660,10 +3675,8 @@ bool device_attach_attrib(struct btd_device *dev, GIOChannel *io)
}
}
- if (cid == ATT_CID)
- mtu = BT_ATT_DEFAULT_LE_MTU;
-
- attrib = g_attrib_new(io, mtu);
+ dev->att_mtu = MIN(mtu, BT_ATT_MAX_LE_MTU);
+ attrib = g_attrib_new(io, dev->att_mtu);
if (!attrib) {
error("Unable to create new GAttrib instance");
return false;
@@ -3678,11 +3691,12 @@ bool device_attach_attrib(struct btd_device *dev, GIOChannel *io)
dev->attrib = attrib;
dev->att = bt_att_ref(g_attrib_get_att(attrib));
-
dev->att_disconn_id = bt_att_register_disconnect(dev->att,
att_disconnected_cb, dev, NULL);
bt_att_set_close_on_unref(dev->att, true);
+ gatt_client_init(dev);
+
/*
* Remove the device from the connect_list and give the passive
* scanning another chance to be restarted in case there are
@@ -3690,8 +3704,6 @@ bool device_attach_attrib(struct btd_device *dev, GIOChannel *io)
*/
adapter_connect_list_remove(dev->adapter, dev);
- g_slist_foreach(dev->attios, attio_connected, dev->attrib);
-
return true;
}
@@ -3742,7 +3754,7 @@ done:
if (device->connect) {
if (!device->le_state.svc_resolved)
- device_browse_primary(device, NULL);
+ device_browse_gatt(device, NULL);
if (err < 0)
reply = btd_error_failed(device->connect,
@@ -3845,16 +3857,32 @@ static void att_browse_error_cb(const GError *gerr, gpointer user_data)
browse_request_free(req);
}
+static void browse_gatt_client(struct browse_req *req)
+{
+ struct btd_device *device = req->device;
+
+ if (!device->gatt_client) {
+ DBG("No gatt-client currently attached");
+ return;
+ }
+
+ /*
+ * If gatt-client is ready, then register all services. Otherwise, this
+ * will be deferred until client becomes ready.
+ */
+ if (bt_gatt_client_is_ready(device->gatt_client))
+ register_gatt_services(req);
+}
+
static void att_browse_cb(gpointer user_data)
{
struct att_callbacks *attcb = user_data;
struct btd_device *device = attcb->user_data;
- gatt_discover_primary(device->attrib, NULL, primary_cb,
- device->browse);
+ browse_gatt_client(device->browse);
}
-static int device_browse_primary(struct btd_device *device, DBusMessage *msg)
+static int device_browse_gatt(struct btd_device *device, DBusMessage *msg)
{
struct btd_adapter *adapter = device->adapter;
struct att_callbacks *attcb;
@@ -3869,7 +3897,7 @@ static int device_browse_primary(struct btd_device *device, DBusMessage *msg)
device->browse = req;
if (device->attrib) {
- gatt_discover_primary(device->attrib, NULL, primary_cb, req);
+ browse_gatt_client(device->browse);
goto done;
}
@@ -3986,7 +4014,7 @@ int device_discover_services(struct btd_device *device)
if (device->bredr)
err = device_browse_sdp(device, NULL);
else
- err = device_browse_primary(device, NULL);
+ err = device_browse_gatt(device, NULL);
if (err == 0 && device->discov_timer) {
g_source_remove(device->discov_timer);
@@ -4132,7 +4160,7 @@ static gboolean start_discovery(gpointer user_data)
if (device->bredr)
device_browse_sdp(device, NULL);
else
- device_browse_primary(device, NULL);
+ device_browse_gatt(device, NULL);
device->discov_timer = 0;
@@ -4269,7 +4297,7 @@ void device_bonding_complete(struct btd_device *device, uint8_t bdaddr_type,
if (bdaddr_type == BDADDR_BREDR)
device_browse_sdp(device, bonding->msg);
else
- device_browse_primary(device, bonding->msg);
+ device_browse_gatt(device, bonding->msg);
bonding_request_free(bonding);
} else if (!state->svc_resolved) {
@@ -4742,16 +4770,10 @@ GSList *btd_device_get_primaries(struct btd_device *device)
void btd_device_gatt_set_service_changed(struct btd_device *device,
uint16_t start, uint16_t end)
{
- GSList *l;
-
- for (l = device->primaries; l; l = g_slist_next(l)) {
- struct gatt_primary *prim = l->data;
-
- if (start <= prim->range.end && end >= prim->range.start)
- prim->changed = TRUE;
- }
-
- device_browse_primary(device, NULL);
+ /*
+ * TODO: Remove this function and handle service changed via
+ * gatt-client.
+ */
}
void btd_device_add_uuid(struct btd_device *device, const char *uuid)
diff --git a/src/shared/att-types.h b/src/shared/att-types.h
index 3d8829a..8b6d537 100644
--- a/src/shared/att-types.h
+++ b/src/shared/att-types.h
@@ -27,7 +27,8 @@
#define __packed __attribute__((packed))
#endif
-#define BT_ATT_DEFAULT_LE_MTU 23
+#define BT_ATT_DEFAULT_LE_MTU 23
+#define BT_ATT_MAX_LE_MTU 517
/* ATT protocol opcodes */
#define BT_ATT_OP_ERROR_RSP 0x01
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH BlueZ 09/15] core: Rename device_attach_attrib
2014-12-09 0:40 [PATCH BlueZ 00/15] core: Use shared/gatt for GATT client role Arman Uguray
` (7 preceding siblings ...)
2014-12-09 0:40 ` [PATCH BlueZ 08/15] core: device: Use shared/gatt-client for GATT Arman Uguray
@ 2014-12-09 0:40 ` Arman Uguray
2014-12-09 0:40 ` [PATCH BlueZ 10/15] core: Use gatt_db service callbacks Arman Uguray
` (6 subsequent siblings)
15 siblings, 0 replies; 22+ messages in thread
From: Arman Uguray @ 2014-12-09 0:40 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Arman Uguray
This patch renames the device_attach_attrib function to
device_attach_att_transport to remove ambiguities arise from the word
"attrib".
---
src/attrib-server.c | 2 +-
src/device.c | 4 ++--
src/device.h | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/attrib-server.c b/src/attrib-server.c
index 6571577..e73850d 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -1300,7 +1300,7 @@ static void connect_event(GIOChannel *io, GError *gerr, void *user_data)
if (!device)
return;
- device_attach_attrib(device, io);
+ device_attach_att_transport(device, io);
}
static gboolean register_core_services(struct gatt_server *server)
diff --git a/src/device.c b/src/device.c
index dcc03af..09df701 100644
--- a/src/device.c
+++ b/src/device.c
@@ -3643,7 +3643,7 @@ static void gatt_client_init(struct btd_device *device)
DBG("gatt-client created");
}
-bool device_attach_attrib(struct btd_device *dev, GIOChannel *io)
+bool device_attach_att_transport(struct btd_device *dev, GIOChannel *io)
{
GError *gerr = NULL;
GAttrib *attrib;
@@ -3728,7 +3728,7 @@ static void att_connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
goto done;
}
- if (!device_attach_attrib(device, io))
+ if (!device_attach_att_transport(device, io))
goto done;
if (attcb->success)
diff --git a/src/device.h b/src/device.h
index 2e0473e..10c4951 100644
--- a/src/device.h
+++ b/src/device.h
@@ -69,7 +69,7 @@ struct gatt_primary *btd_device_get_primary(struct btd_device *device,
GSList *btd_device_get_primaries(struct btd_device *device);
void btd_device_gatt_set_service_changed(struct btd_device *device,
uint16_t start, uint16_t end);
-bool device_attach_attrib(struct btd_device *dev, GIOChannel *io);
+bool device_attach_att_transport(struct btd_device *dev, GIOChannel *io);
void btd_device_add_uuid(struct btd_device *device, const char *uuid);
void device_add_eir_uuids(struct btd_device *dev, GSList *uuids);
void device_probe_profile(gpointer a, gpointer b);
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH BlueZ 10/15] core: Use gatt_db service callbacks
2014-12-09 0:40 [PATCH BlueZ 00/15] core: Use shared/gatt for GATT client role Arman Uguray
` (8 preceding siblings ...)
2014-12-09 0:40 ` [PATCH BlueZ 09/15] core: Rename device_attach_attrib Arman Uguray
@ 2014-12-09 0:40 ` Arman Uguray
2014-12-09 0:40 ` [PATCH BlueZ 11/15] core: Introduce gatt-callbacks Arman Uguray
` (5 subsequent siblings)
15 siblings, 0 replies; 22+ messages in thread
From: Arman Uguray @ 2014-12-09 0:40 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Arman Uguray
This patch integrates the gatt_db service callbacks into btd_device.
Based on the event, the device objects UUIDs list is updated and
profiles are probed for new services.
There is currently no mechanism to tell a profile that a service has
been removed, however profiles can use the gatt_db callbacks to make
necessary updates in the future.
---
src/device.c | 143 ++++++++++++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 117 insertions(+), 26 deletions(-)
diff --git a/src/device.c b/src/device.c
index 09df701..0bfac1e 100644
--- a/src/device.c
+++ b/src/device.c
@@ -2365,6 +2365,119 @@ static void load_att_info(struct btd_device *device, const char *local,
free(prim_uuid);
}
+static void device_register_primaries(struct btd_device *device,
+ GSList *prim_list, int psm)
+{
+ device->primaries = g_slist_concat(device->primaries, prim_list);
+}
+
+static void add_primary(struct gatt_db_attribute *attr, void *user_data)
+{
+ GSList **new_services = user_data;
+ struct gatt_primary *prim;
+ bt_uuid_t uuid;
+
+ prim = g_new0(struct gatt_primary, 1);
+ if (!prim) {
+ DBG("Failed to allocate gatt_primary structure");
+ return;
+ }
+
+ gatt_db_attribute_get_service_handles(attr, &prim->range.start,
+ &prim->range.end);
+ gatt_db_attribute_get_service_uuid(attr, &uuid);
+ bt_uuid_to_string(&uuid, prim->uuid, sizeof(prim->uuid));
+
+ *new_services = g_slist_append(*new_services, prim);
+}
+
+static void store_services(struct btd_device *device);
+
+static void gatt_service_added(struct gatt_db_attribute *attr, void *user_data)
+{
+ struct btd_device *device = user_data;
+ struct gatt_primary *prim;
+ GSList *new_service = NULL;
+ GSList *profiles_added = NULL;
+ uint16_t start, end;
+
+ if (!bt_gatt_client_is_ready(device->gatt_client))
+ return;
+
+ gatt_db_attribute_get_service_handles(attr, &start, &end);
+
+ DBG("GATT service added - start: 0x%04x, end: 0x%04x", start, end);
+
+ /*
+ * TODO: Remove the primaries list entirely once all profiles use
+ * shared/gatt.
+ */
+ add_primary(attr, &new_service);
+ if (!new_service)
+ return;
+
+ device_register_primaries(device, new_service, -1);
+
+ prim = new_service->data;
+ profiles_added = g_slist_append(profiles_added, g_strdup(prim->uuid));
+
+ device_probe_profiles(device, profiles_added);
+
+ store_services(device);
+
+ g_slist_free_full(profiles_added, g_free);
+}
+
+static gint prim_attr_cmp(gconstpointer a, gconstpointer b)
+{
+ const struct gatt_primary *prim = a;
+ const struct gatt_db_attribute *attr = b;
+ uint16_t start, end;
+
+ gatt_db_attribute_get_service_handles(attr, &start, &end);
+
+ return !(prim->range.start == start && prim->range.end == end);
+}
+
+static void gatt_service_removed(struct gatt_db_attribute *attr,
+ void *user_data)
+{
+ struct btd_device *device = user_data;
+ GSList *l;
+ struct gatt_primary *prim;
+ uint16_t start, end;
+
+ if (!bt_gatt_client_is_ready(device->gatt_client))
+ return;
+
+ gatt_db_attribute_get_service_handles(attr, &start, &end);
+
+ DBG("GATT service removed - start: 0x%04x, end: 0x%04x", start, end);
+
+ /* Remove the corresponding gatt_primary */
+ l = g_slist_find_custom(device->primaries, attr, prim_attr_cmp);
+ if (!l)
+ return;
+
+ prim = l->data;
+ device->primaries = g_slist_delete_link(device->primaries, l);
+
+ /* Remove the corresponding UUIDs entry */
+ l = g_slist_find_custom(device->uuids, prim->uuid, bt_uuid_strcmp);
+ device->uuids = g_slist_delete_link(device->uuids, l);
+ g_free(prim);
+
+ store_services(device);
+
+ /*
+ * TODO: Notify the profiles somehow. It may be sufficient for each
+ * profile to register a service_removed handler.
+ */
+
+ g_dbus_emit_property_changed(dbus_conn, device->path,
+ DEVICE_INTERFACE, "UUIDs");
+}
+
static struct btd_device *device_new(struct btd_adapter *adapter,
const char *address)
{
@@ -2405,6 +2518,10 @@ static struct btd_device *device_new(struct btd_adapter *adapter,
device->adapter = adapter;
device->temporary = TRUE;
+ gatt_db_register(device->client_db, gatt_service_added,
+ gatt_service_removed,
+ device, NULL);
+
return btd_device_ref(device);
}
@@ -3264,12 +3381,6 @@ static GSList *device_services_from_record(struct btd_device *device,
return prim_list;
}
-static void device_register_primaries(struct btd_device *device,
- GSList *prim_list, int psm)
-{
- device->primaries = g_slist_concat(device->primaries, prim_list);
-}
-
static void search_cb(sdp_list_t *recs, int err, gpointer user_data)
{
struct browse_req *req = user_data;
@@ -3515,26 +3626,6 @@ static void send_le_browse_response(struct browse_req *req)
g_dbus_send_reply(dbus_conn, msg, DBUS_TYPE_INVALID);
}
-static void add_primary(struct gatt_db_attribute *attr, void *user_data)
-{
- struct gatt_primary *prim;
- GSList **services = user_data;
- bt_uuid_t uuid;
-
- prim = g_new0(struct gatt_primary, 1);
- if (!prim) {
- DBG("Failed to allocate gatt_primary structure");
- return;
- }
-
- gatt_db_attribute_get_service_handles(attr, &prim->range.start,
- &prim->range.end);
- gatt_db_attribute_get_service_uuid(attr, &uuid);
- bt_uuid_to_string(&uuid, prim->uuid, sizeof(prim->uuid));
-
- *services = g_slist_append(*services, prim);
-}
-
static void register_gatt_services(struct browse_req *req)
{
struct btd_device *device = req->device;
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH BlueZ 11/15] core: Introduce gatt-callbacks
2014-12-09 0:40 [PATCH BlueZ 00/15] core: Use shared/gatt for GATT client role Arman Uguray
` (9 preceding siblings ...)
2014-12-09 0:40 ` [PATCH BlueZ 10/15] core: Use gatt_db service callbacks Arman Uguray
@ 2014-12-09 0:40 ` Arman Uguray
2014-12-09 0:40 ` [PATCH BlueZ 12/15] profiles/gatt: Don't handle GATT service Arman Uguray
` (4 subsequent siblings)
15 siblings, 0 replies; 22+ messages in thread
From: Arman Uguray @ 2014-12-09 0:40 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Arman Uguray
This patch introduces src/gatt-callbacks.h. This defines API functions
to get notified of bt_gatt_client related events, such as ready,
service removed, and disconnects. These callbacks are parallel to the
existing attio.h functions and will provide a temporary alternative
during the transition from attrib/ to shared/gatt.
---
Makefile.am | 1 +
src/device.c | 162 +++++++++++++++++++++++++++++++++++++++++++++++----
src/gatt-callbacks.h | 36 ++++++++++++
3 files changed, 188 insertions(+), 11 deletions(-)
create mode 100644 src/gatt-callbacks.h
diff --git a/Makefile.am b/Makefile.am
index c506122..ad2c336 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -182,6 +182,7 @@ src_bluetoothd_SOURCES = $(builtin_sources) \
src/gatt-dbus.h src/gatt-dbus.c \
src/gatt.h src/gatt.c \
src/device.h src/device.c src/attio.h \
+ src/gatt-callbacks.h \
src/dbus-common.c src/dbus-common.h \
src/eir.h src/eir.c
src_bluetoothd_LDADD = lib/libbluetooth-internal.la \
diff --git a/src/device.c b/src/device.c
index 0bfac1e..4b42580 100644
--- a/src/device.c
+++ b/src/device.c
@@ -59,6 +59,7 @@
#include "attrib/gattrib.h"
#include "attio.h"
#include "device.h"
+#include "gatt-callbacks.h"
#include "profile.h"
#include "service.h"
#include "dbus-common.h"
@@ -145,6 +146,14 @@ struct attio_data {
gpointer user_data;
};
+struct gatt_cb_data {
+ unsigned int id;
+ btd_gatt_client_ready_t ready_func;
+ btd_gatt_service_removed_t svc_removed_func;
+ btd_gatt_disconnect_t disconn_func;
+ void *user_data;
+};
+
struct svc_callback {
unsigned int id;
guint idle_id;
@@ -222,6 +231,8 @@ struct btd_device {
*/
struct gatt_db *client_db; /* GATT client cache */
struct bt_gatt_client *gatt_client; /* GATT client implementation */
+ GSList *gatt_callbacks;
+ unsigned int next_gatt_cb_id;
struct bearer_state bredr_state;
struct bearer_state le_state;
@@ -559,6 +570,7 @@ static void device_free(gpointer user_data)
g_slist_free_full(device->attios, g_free);
g_slist_free_full(device->attios_offline, g_free);
g_slist_free_full(device->svc_callbacks, svc_dev_remove);
+ g_slist_free_full(device->gatt_callbacks, g_free);
attio_cleanup(device);
@@ -2439,6 +2451,21 @@ static gint prim_attr_cmp(gconstpointer a, gconstpointer b)
return !(prim->range.start == start && prim->range.end == end);
}
+struct svc_removed_data {
+ struct gatt_db_attribute *attr;
+ struct bt_gatt_client *client;
+};
+
+static void notify_gatt_service_removed(gpointer data, gpointer user_data)
+{
+ struct gatt_cb_data *cb = data;
+ struct svc_removed_data *rm_data = user_data;
+
+ if (cb->svc_removed_func)
+ cb->svc_removed_func(rm_data->client, rm_data->attr,
+ cb->user_data);
+}
+
static void gatt_service_removed(struct gatt_db_attribute *attr,
void *user_data)
{
@@ -2446,6 +2473,7 @@ static void gatt_service_removed(struct gatt_db_attribute *attr,
GSList *l;
struct gatt_primary *prim;
uint16_t start, end;
+ struct svc_removed_data data;
if (!bt_gatt_client_is_ready(device->gatt_client))
return;
@@ -2469,13 +2497,19 @@ static void gatt_service_removed(struct gatt_db_attribute *attr,
store_services(device);
- /*
- * TODO: Notify the profiles somehow. It may be sufficient for each
- * profile to register a service_removed handler.
- */
-
g_dbus_emit_property_changed(dbus_conn, device->path,
DEVICE_INTERFACE, "UUIDs");
+
+ data.attr = attr;
+ data.client = device->gatt_client;
+
+ g_slist_foreach(device->gatt_callbacks, notify_gatt_service_removed,
+ &data);
+
+ /*
+ * TODO: Remove profiles matching this UUID if no other GATT services
+ * with the same UUID exist.
+ */
}
static struct btd_device *device_new(struct btd_adapter *adapter,
@@ -3560,6 +3594,16 @@ static void attio_disconnected(gpointer data, gpointer user_data)
attio->dcfunc(attio->user_data);
}
+static void gatt_disconnected(gpointer data, gpointer user_data)
+{
+ struct gatt_cb_data *gatt_data = data;
+
+ DBG("");
+
+ if (gatt_data->disconn_func)
+ gatt_data->disconn_func(gatt_data->user_data);
+}
+
static void att_disconnected_cb(void *user_data)
{
struct btd_device *device = user_data;
@@ -3578,6 +3622,7 @@ static void att_disconnected_cb(void *user_data)
DBG("%s (%d)", strerror(err), err);
g_slist_foreach(device->attios, attio_disconnected, NULL);
+ g_slist_foreach(device->gatt_callbacks, gatt_disconnected, NULL);
if (!device_get_auto_connect(device)) {
DBG("Automatic connection disabled");
@@ -3651,7 +3696,15 @@ static void register_gatt_services(struct browse_req *req)
device_probe_profiles(device, req->profiles_added);
- if (device->attios == NULL && device->attios_offline == NULL)
+ /*
+ * TODO: This check seems unnecessary. We may not always want to cleanup
+ * the connection since there will always be built-in plugins who want
+ * to interact with remote GATT services. Even if we didn't have those,
+ * the GATT D-Bus API will need to interact with these, so we should
+ * later remove this check entirely.
+ */
+ if (device->attios == NULL && device->attios_offline == NULL &&
+ device->gatt_callbacks == NULL)
attio_cleanup(device);
device_svc_resolved(device, device->bdaddr_type, 0);
@@ -3661,6 +3714,18 @@ static void register_gatt_services(struct browse_req *req)
browse_request_free(req);
}
+static void notify_gatt_client_ready(gpointer data, gpointer user_data)
+{
+ struct gatt_cb_data *gatt_data = data;
+ struct btd_device *device = user_data;
+
+ DBG("");
+
+ if (gatt_data->ready_func)
+ gatt_data->ready_func(device->gatt_client, device->client_db,
+ gatt_data->user_data);
+}
+
static void gatt_client_ready_cb(bool success, uint8_t att_ecode,
void *user_data)
{
@@ -3689,11 +3754,9 @@ static void gatt_client_ready_cb(bool success, uint8_t att_ecode,
if (device->browse)
register_gatt_services(device->browse);
- /*
- * TODO: Change attio callbacks to accept a gatt-client instead of a
- * GAttrib.
- */
g_slist_foreach(device->attios, attio_connected, device->attrib);
+ g_slist_foreach(device->gatt_callbacks, notify_gatt_client_ready,
+ device);
}
static void gatt_client_service_changed(uint16_t start_handle,
@@ -5080,7 +5143,8 @@ gboolean btd_device_remove_attio_callback(struct btd_device *device, guint id)
g_free(attio);
- if (device->attios != NULL || device->attios_offline != NULL)
+ if (device->attios != NULL || device->attios_offline != NULL ||
+ device->gatt_callbacks != NULL)
return TRUE;
attio_cleanup(device);
@@ -5088,6 +5152,82 @@ gboolean btd_device_remove_attio_callback(struct btd_device *device, guint id)
return TRUE;
}
+unsigned int btd_device_add_gatt_callbacks(struct btd_device *device,
+ btd_gatt_client_ready_t ready_func,
+ btd_gatt_service_removed_t service_removed_func,
+ btd_gatt_disconnect_t disconnect_func,
+ void *user_data)
+{
+ struct gatt_cb_data *gatt_data;
+
+ gatt_data = new0(struct gatt_cb_data, 1);
+ if (!gatt_data)
+ return 0;
+
+ if (device->next_gatt_cb_id < 1)
+ device->next_gatt_cb_id = 1;
+
+ device_set_auto_connect(device, TRUE);
+
+ gatt_data->id = device->next_gatt_cb_id++;
+ gatt_data->ready_func = ready_func;
+ gatt_data->svc_removed_func = service_removed_func;
+ gatt_data->disconn_func = disconnect_func;
+ gatt_data->user_data = user_data;
+
+ /*
+ * TODO: The connection might be incoming from attrib-server (see
+ * btd_device_add_attio_callback). I don't think this is a good place to
+ * attach the GAttrib to the device. We should come up with a more
+ * unified flow for attaching the GAttrib, bt_att, and bt_gatt_client
+ * for incoming and outgoing connections.
+ */
+ device->gatt_callbacks = g_slist_append(device->gatt_callbacks,
+ gatt_data);
+
+ if (ready_func && bt_gatt_client_is_ready(device->gatt_client))
+ ready_func(device->gatt_client, device->client_db, user_data);
+
+ return gatt_data->id;
+}
+
+static int gatt_cb_id_cmp(gconstpointer a, gconstpointer b)
+{
+ const struct gatt_cb_data *gatt_data = a;
+ guint id = GPOINTER_TO_UINT(b);
+
+ return gatt_data->id - id;
+}
+
+bool btd_device_remove_gatt_callbacks(struct btd_device *device,
+ unsigned int id)
+{
+ struct gatt_cb_data *gatt_data;
+ GSList *l;
+
+ l = g_slist_find_custom(device->gatt_callbacks, GUINT_TO_POINTER(id),
+ gatt_cb_id_cmp);
+ if (!l)
+ return false;
+
+ gatt_data = l->data;
+ device->gatt_callbacks = g_slist_remove(device->gatt_callbacks,
+ gatt_data);
+ free(gatt_data);
+
+ /*
+ * TODO: Consider removing this check and avoiding cleanup as it seems
+ * unnecessary.
+ */
+ if (device->attios != NULL || device->attios_offline != NULL ||
+ device->gatt_callbacks != NULL)
+ return true;
+
+ attio_cleanup(device);
+
+ return true;
+}
+
void btd_device_set_pnpid(struct btd_device *device, uint16_t source,
uint16_t vendor, uint16_t product, uint16_t version)
{
diff --git a/src/gatt-callbacks.h b/src/gatt-callbacks.h
new file mode 100644
index 0000000..2ac1800
--- /dev/null
+++ b/src/gatt-callbacks.h
@@ -0,0 +1,36 @@
+/*
+ *
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2014 Google Inc.
+ *
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ */
+
+#include <stdint.h>
+#include <stdbool.h>
+
+typedef void (*btd_gatt_client_ready_t) (struct bt_gatt_client *client,
+ struct gatt_db *db,
+ void *user_data);
+typedef void (*btd_gatt_service_removed_t) (struct bt_gatt_client *client,
+ struct gatt_db_attribute *attrib,
+ void *user_data);
+typedef void (*btd_gatt_disconnect_t) (void *user_data);
+
+unsigned int btd_device_add_gatt_callbacks(struct btd_device *device,
+ btd_gatt_client_ready_t ready_func,
+ btd_gatt_service_removed_t service_removed_func,
+ btd_gatt_disconnect_t disconnect_func,
+ void *user_data);
+bool btd_device_remove_gatt_callbacks(struct btd_device *device,
+ unsigned int id);
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH BlueZ 12/15] profiles/gatt: Don't handle GATT service.
2014-12-09 0:40 [PATCH BlueZ 00/15] core: Use shared/gatt for GATT client role Arman Uguray
` (10 preceding siblings ...)
2014-12-09 0:40 ` [PATCH BlueZ 11/15] core: Introduce gatt-callbacks Arman Uguray
@ 2014-12-09 0:40 ` Arman Uguray
2014-12-09 0:40 ` [PATCH BlueZ 13/15] profiles/gatt: Rename profile to gap Arman Uguray
` (3 subsequent siblings)
15 siblings, 0 replies; 22+ messages in thread
From: Arman Uguray @ 2014-12-09 0:40 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Arman Uguray
ATT MTU exchange and handling of indications from the "Service Changed"
characteristic are now handled by shared/gatt-client, so this profile
should only deal with the GAP service.
---
profiles/gatt/gas.c | 251 ++--------------------------------------------------
1 file changed, 5 insertions(+), 246 deletions(-)
diff --git a/profiles/gatt/gas.c b/profiles/gatt/gas.c
index b51b4a8..9d5d31e 100644
--- a/profiles/gatt/gas.c
+++ b/profiles/gatt/gas.c
@@ -52,12 +52,8 @@
struct gas {
struct btd_device *device;
struct att_range gap; /* GAP Primary service range */
- struct att_range gatt; /* GATT Primary service range */
GAttrib *attrib;
guint attioid;
- guint changed_ind;
- uint16_t changed_handle;
- uint16_t mtu;
};
static GSList *devices = NULL;
@@ -80,68 +76,6 @@ static int cmp_device(gconstpointer a, gconstpointer b)
return (gas->device == device ? 0 : -1);
}
-static void write_ctp_handle(struct btd_device *device, uint16_t uuid,
- uint16_t handle)
-{
- char *filename, group[6], value[7];
- GKeyFile *key_file;
- char *data;
- gsize length = 0;
-
- filename = btd_device_get_storage_path(device, "gatt");
- if (!filename) {
- warn("Unable to get gatt storage path for device");
- return;
- }
-
- key_file = g_key_file_new();
- g_key_file_load_from_file(key_file, filename, 0, NULL);
-
- snprintf(group, sizeof(group), "%hu", uuid);
- snprintf(value, sizeof(value), "0x%4.4X", handle);
- g_key_file_set_string(key_file, group, "Value", value);
-
- data = g_key_file_to_data(key_file, &length, NULL);
- if (length > 0) {
- create_file(filename, S_IRUSR | S_IWUSR);
- g_file_set_contents(filename, data, length, NULL);
- }
-
- g_free(data);
- g_free(filename);
- g_key_file_free(key_file);
-}
-
-static int read_ctp_handle(struct btd_device *device, uint16_t uuid,
- uint16_t *value)
-{
- char *filename, group[6];
- GKeyFile *key_file;
- char *str;
- int err = 0;
-
- filename = btd_device_get_storage_path(device, "gatt");
- if (!filename) {
- warn("Unable to get gatt storage path for device");
- return -ENOENT;
- }
-
- snprintf(group, sizeof(group), "%hu", uuid);
-
- key_file = g_key_file_new();
- g_key_file_load_from_file(key_file, filename, 0, NULL);
-
- str = g_key_file_get_string(key_file, group, "Value", NULL);
- if (str == NULL || sscanf(str, "%hx", value) != 1)
- err = -ENOENT;
-
- g_free(str);
- g_free(filename);
- g_key_file_free(key_file);
-
- return err;
-}
-
static void gap_appearance_cb(guint8 status, const guint8 *pdu, guint16 plen,
gpointer user_data)
{
@@ -176,163 +110,12 @@ done:
att_data_list_free(list);
}
-static void indication_cb(const uint8_t *pdu, uint16_t len, gpointer user_data)
-{
- uint8_t bdaddr_type;
- struct gas *gas = user_data;
- uint16_t start, end, olen;
- size_t plen;
- uint8_t *opdu;
-
- if (len < 7) { /* 1-byte opcode + 2-byte handle + 4 range */
- error("Malformed ATT notification");
- return;
- }
-
- start = get_le16(&pdu[3]);
- end = get_le16(&pdu[5]);
-
- DBG("Service Changed start: 0x%04X end: 0x%04X", start, end);
-
- /* Confirming indication received */
- opdu = g_attrib_get_buffer(gas->attrib, &plen);
- olen = enc_confirmation(opdu, plen);
- g_attrib_send(gas->attrib, 0, opdu, olen, NULL, NULL, NULL);
-
- bdaddr_type = btd_device_get_bdaddr_type(gas->device);
- if (!device_is_bonded(gas->device, bdaddr_type)) {
- DBG("Ignoring Service Changed: device is not bonded");
- return;
- }
-
- btd_device_gatt_set_service_changed(gas->device, start, end);
-}
-
-static void ccc_written_cb(guint8 status, const guint8 *pdu, guint16 plen,
- gpointer user_data)
-{
- struct gas *gas = user_data;
-
- if (status) {
- error("Write Service Changed CCC failed: %s",
- att_ecode2str(status));
- return;
- }
-
- DBG("Service Changed indications enabled");
-
- gas->changed_ind = g_attrib_register(gas->attrib, ATT_OP_HANDLE_IND,
- gas->changed_handle,
- indication_cb, gas, NULL);
-
- write_ctp_handle(gas->device, GATT_CHARAC_SERVICE_CHANGED,
- gas->changed_handle);
-}
-
-static void write_ccc(GAttrib *attrib, uint16_t handle, gpointer user_data)
-{
- uint8_t value[2];
-
- put_le16(GATT_CLIENT_CHARAC_CFG_IND_BIT, value);
- gatt_write_char(attrib, handle, value, sizeof(value), ccc_written_cb,
- user_data);
-}
-
-static void discover_ccc_cb(uint8_t status, GSList *descs, void *user_data)
-{
- struct gas *gas = user_data;
- struct gatt_desc *desc;
-
- if (status != 0) {
- error("Discover Service Changed CCC failed: %s",
- att_ecode2str(status));
- return;
- }
-
- /* There will be only one descriptor on list and it will be CCC */
- desc = descs->data;
-
- DBG("CCC: 0x%04x", desc->handle);
- write_ccc(gas->attrib, desc->handle, user_data);
-}
-
-static void gatt_characteristic_cb(uint8_t status, GSList *characteristics,
- void *user_data)
-{
- struct gas *gas = user_data;
- struct gatt_char *chr;
- uint16_t start, end;
- bt_uuid_t uuid;
-
- if (status) {
- error("Discover Service Changed handle: %s", att_ecode2str(status));
- return;
- }
-
- chr = characteristics->data;
-
- start = chr->value_handle + 1;
- end = gas->gatt.end;
-
- if (start > end) {
- error("Inconsistent database: Service Changed CCC missing");
- return;
- }
-
- gas->changed_handle = chr->value_handle;
-
- bt_uuid16_create(&uuid, GATT_CLIENT_CHARAC_CFG_UUID);
-
- gatt_discover_desc(gas->attrib, start, end, &uuid, discover_ccc_cb,
- gas);
-}
-
-static void exchange_mtu_cb(guint8 status, const guint8 *pdu, guint16 plen,
- gpointer user_data)
-{
- struct gas *gas = user_data;
- uint16_t rmtu;
-
- if (status) {
- error("MTU exchange: %s", att_ecode2str(status));
- return;
- }
-
- if (!dec_mtu_resp(pdu, plen, &rmtu)) {
- error("MTU exchange: protocol error");
- return;
- }
-
- gas->mtu = MIN(rmtu, gas->mtu);
- if (g_attrib_set_mtu(gas->attrib, gas->mtu))
- DBG("MTU exchange succeeded: %d", gas->mtu);
- else
- DBG("MTU exchange failed");
-}
-
static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
{
struct gas *gas = user_data;
- GIOChannel *io;
- GError *gerr = NULL;
- uint16_t cid, imtu;
uint16_t app;
gas->attrib = g_attrib_ref(attrib);
- io = g_attrib_get_channel(attrib);
-
- if (bt_io_get(io, &gerr, BT_IO_OPT_IMTU, &imtu,
- BT_IO_OPT_CID, &cid, BT_IO_OPT_INVALID) &&
- cid == ATT_CID) {
- gatt_exchange_mtu(gas->attrib, imtu, exchange_mtu_cb, gas);
- gas->mtu = imtu;
- DBG("MTU Exchange: Requesting %d", imtu);
- }
-
- if (gerr) {
- error("Could not acquire att imtu and cid: %s", gerr->message);
- g_error_free(gerr);
- }
if (device_get_appearance(gas->device, &app) < 0) {
bt_uuid_t uuid;
@@ -345,43 +128,23 @@ static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
}
/* TODO: Read other GAP characteristics - See Core spec page 1739 */
-
- /*
- * When re-connecting <<Service Changed>> handle and characteristic
- * value doesn't need to read again: known information from the
- * previous interaction.
- */
- if (gas->changed_handle == 0) {
- bt_uuid_t uuid;
-
- bt_uuid16_create(&uuid, GATT_CHARAC_SERVICE_CHANGED);
-
- gatt_discover_char(gas->attrib, gas->gatt.start, gas->gatt.end,
- &uuid, gatt_characteristic_cb, gas);
- }
}
static void attio_disconnected_cb(gpointer user_data)
{
struct gas *gas = user_data;
- g_attrib_unregister(gas->attrib, gas->changed_ind);
- gas->changed_ind = 0;
-
g_attrib_unref(gas->attrib);
gas->attrib = NULL;
}
-static int gas_register(struct btd_device *device, struct att_range *gap,
- struct att_range *gatt)
+static int gas_register(struct btd_device *device, struct att_range *gap)
{
struct gas *gas;
gas = g_new0(struct gas, 1);
gas->gap.start = gap->start;
gas->gap.end = gap->end;
- gas->gatt.start = gatt->start;
- gas->gatt.end = gatt->end;
gas->device = btd_device_ref(device);
@@ -391,9 +154,6 @@ static int gas_register(struct btd_device *device, struct att_range *gap,
attio_connected_cb,
attio_disconnected_cb, gas);
- read_ctp_handle(gas->device, GATT_CHARAC_SERVICE_CHANGED,
- &gas->changed_handle);
-
return 0;
}
@@ -414,17 +174,16 @@ static void gas_unregister(struct btd_device *device)
static int gatt_driver_probe(struct btd_service *service)
{
struct btd_device *device = btd_service_get_device(service);
- struct gatt_primary *gap, *gatt;
+ struct gatt_primary *gap;
gap = btd_device_get_primary(device, GAP_UUID);
- gatt = btd_device_get_primary(device, GATT_UUID);
- if (gap == NULL || gatt == NULL) {
- error("GAP and GATT are mandatory");
+ if (gap == NULL) {
+ error("GAP service is mandatory");
return -EINVAL;
}
- return gas_register(device, &gap->range, &gatt->range);
+ return gas_register(device, &gap->range);
}
static void gatt_driver_remove(struct btd_service *service)
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH BlueZ 13/15] profiles/gatt: Rename profile to gap.
2014-12-09 0:40 [PATCH BlueZ 00/15] core: Use shared/gatt for GATT client role Arman Uguray
` (11 preceding siblings ...)
2014-12-09 0:40 ` [PATCH BlueZ 12/15] profiles/gatt: Don't handle GATT service Arman Uguray
@ 2014-12-09 0:40 ` Arman Uguray
2014-12-09 0:40 ` [PATCH BlueZ 14/15] profiles/gap: Rewrite using bt_gatt_client Arman Uguray
` (2 subsequent siblings)
15 siblings, 0 replies; 22+ messages in thread
From: Arman Uguray @ 2014-12-09 0:40 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Arman Uguray
Since this built in profile only handles the GAP service, this patch renames it
to gap.
---
Makefile.plugins | 4 +-
profiles/gap/gas.c | 213 +++++++++++++++++++++++++++++++++++++++++++++++++++
profiles/gatt/gas.c | 216 ----------------------------------------------------
3 files changed, 215 insertions(+), 218 deletions(-)
create mode 100644 profiles/gap/gas.c
delete mode 100644 profiles/gatt/gas.c
diff --git a/Makefile.plugins b/Makefile.plugins
index 0448b91..52b51c5 100644
--- a/Makefile.plugins
+++ b/Makefile.plugins
@@ -72,8 +72,8 @@ builtin_sources += profiles/health/mcap.h profiles/health/mcap.c \
profiles/health/hdp_util.h profiles/health/hdp_util.c
endif
-builtin_modules += gatt
-builtin_sources += profiles/gatt/gas.c
+builtin_modules += gap
+builtin_sources += profiles/gap/gas.c
builtin_modules += scanparam
builtin_sources += profiles/scanparam/scan.c
diff --git a/profiles/gap/gas.c b/profiles/gap/gas.c
new file mode 100644
index 0000000..a4028dd
--- /dev/null
+++ b/profiles/gap/gas.c
@@ -0,0 +1,213 @@
+/*
+ *
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2012 Instituto Nokia de Tecnologia - INdT
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdbool.h>
+#include <stdlib.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <errno.h>
+
+#include <glib.h>
+
+#include "btio/btio.h"
+#include "lib/uuid.h"
+#include "src/plugin.h"
+#include "src/adapter.h"
+#include "src/device.h"
+#include "src/profile.h"
+#include "src/service.h"
+#include "src/shared/util.h"
+#include "attrib/att.h"
+#include "attrib/gattrib.h"
+#include "src/attio.h"
+#include "attrib/gatt.h"
+#include "src/log.h"
+#include "src/textfile.h"
+
+/* Generic Attribute/Access Service */
+struct gas {
+ struct btd_device *device;
+ struct att_range gap; /* GAP Primary service range */
+ GAttrib *attrib;
+ guint attioid;
+};
+
+static GSList *devices;
+
+static void gas_free(struct gas *gas)
+{
+ if (gas->attioid)
+ btd_device_remove_attio_callback(gas->device, gas->attioid);
+
+ g_attrib_unref(gas->attrib);
+ btd_device_unref(gas->device);
+ g_free(gas);
+}
+
+static int cmp_device(gconstpointer a, gconstpointer b)
+{
+ const struct gas *gas = a;
+ const struct btd_device *device = b;
+
+ return gas->device == device ? 0 : -1;
+}
+
+static void gap_appearance_cb(guint8 status, const guint8 *pdu, guint16 plen,
+ gpointer user_data)
+{
+ struct gas *gas = user_data;
+ struct att_data_list *list = NULL;
+ uint16_t app;
+ uint8_t *atval;
+
+ if (status != 0) {
+ error("Read characteristics by UUID failed: %s",
+ att_ecode2str(status));
+ return;
+ }
+
+ list = dec_read_by_type_resp(pdu, plen);
+ if (list == NULL)
+ return;
+
+ if (list->len != 4) {
+ error("GAP Appearance value: invalid data");
+ goto done;
+ }
+
+ atval = list->data[0] + 2; /* skip handle value */
+ app = get_le16(atval);
+
+ DBG("GAP Appearance: 0x%04x", app);
+
+ device_set_appearance(gas->device, app);
+
+done:
+ att_data_list_free(list);
+}
+
+static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
+{
+ struct gas *gas = user_data;
+ uint16_t app;
+
+ gas->attrib = g_attrib_ref(attrib);
+
+ if (device_get_appearance(gas->device, &app) < 0) {
+ bt_uuid_t uuid;
+
+ bt_uuid16_create(&uuid, GATT_CHARAC_APPEARANCE);
+
+ gatt_read_char_by_uuid(gas->attrib, gas->gap.start,
+ gas->gap.end, &uuid,
+ gap_appearance_cb, gas);
+ }
+
+ /* TODO: Read other GAP characteristics - See Core spec page 1739 */
+}
+
+static void attio_disconnected_cb(gpointer user_data)
+{
+ struct gas *gas = user_data;
+
+ g_attrib_unref(gas->attrib);
+ gas->attrib = NULL;
+}
+
+static int gas_register(struct btd_device *device, struct att_range *gap)
+{
+ struct gas *gas;
+
+ gas = g_new0(struct gas, 1);
+ gas->gap.start = gap->start;
+ gas->gap.end = gap->end;
+
+ gas->device = btd_device_ref(device);
+
+ devices = g_slist_append(devices, gas);
+
+ gas->attioid = btd_device_add_attio_callback(device,
+ attio_connected_cb,
+ attio_disconnected_cb, gas);
+
+ return 0;
+}
+
+static void gas_unregister(struct btd_device *device)
+{
+ struct gas *gas;
+ GSList *l;
+
+ l = g_slist_find_custom(devices, device, cmp_device);
+ if (l == NULL)
+ return;
+
+ gas = l->data;
+ devices = g_slist_remove(devices, gas);
+ gas_free(gas);
+}
+
+static int gap_driver_probe(struct btd_service *service)
+{
+ struct btd_device *device = btd_service_get_device(service);
+ struct gatt_primary *gap;
+
+ gap = btd_device_get_primary(device, GAP_UUID);
+
+ if (gap == NULL) {
+ error("GAP service is mandatory");
+ return -EINVAL;
+ }
+
+ return gas_register(device, &gap->range);
+}
+
+static void gap_driver_remove(struct btd_service *service)
+{
+ struct btd_device *device = btd_service_get_device(service);
+
+ gas_unregister(device);
+}
+
+static struct btd_profile gap_profile = {
+ .name = "gap-profile",
+ .remote_uuid = GAP_UUID,
+ .device_probe = gap_driver_probe,
+ .device_remove = gap_driver_remove
+};
+
+static int gap_init(void)
+{
+ devices = NULL;
+
+ btd_profile_register(&gap_profile);
+
+ return 0;
+}
+
+static void gap_exit(void)
+{
+ btd_profile_unregister(&gap_profile);
+}
+
+BLUETOOTH_PLUGIN_DEFINE(gap, VERSION, BLUETOOTH_PLUGIN_PRIORITY_DEFAULT,
+ gap_init, gap_exit)
diff --git a/profiles/gatt/gas.c b/profiles/gatt/gas.c
deleted file mode 100644
index 9d5d31e..0000000
--- a/profiles/gatt/gas.c
+++ /dev/null
@@ -1,216 +0,0 @@
-/*
- *
- * BlueZ - Bluetooth protocol stack for Linux
- *
- * Copyright (C) 2012 Instituto Nokia de Tecnologia - INdT
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
- *
- */
-
-#ifdef HAVE_CONFIG_H
-#include <config.h>
-#endif
-
-#include <stdbool.h>
-#include <stdlib.h>
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <fcntl.h>
-#include <errno.h>
-
-#include <glib.h>
-
-#include "btio/btio.h"
-#include "lib/uuid.h"
-#include "src/plugin.h"
-#include "src/adapter.h"
-#include "src/device.h"
-#include "src/profile.h"
-#include "src/service.h"
-#include "src/shared/util.h"
-#include "attrib/att.h"
-#include "attrib/gattrib.h"
-#include "src/attio.h"
-#include "attrib/gatt.h"
-#include "src/log.h"
-#include "src/textfile.h"
-
-/* Generic Attribute/Access Service */
-struct gas {
- struct btd_device *device;
- struct att_range gap; /* GAP Primary service range */
- GAttrib *attrib;
- guint attioid;
-};
-
-static GSList *devices = NULL;
-
-static void gas_free(struct gas *gas)
-{
- if (gas->attioid)
- btd_device_remove_attio_callback(gas->device, gas->attioid);
-
- g_attrib_unref(gas->attrib);
- btd_device_unref(gas->device);
- g_free(gas);
-}
-
-static int cmp_device(gconstpointer a, gconstpointer b)
-{
- const struct gas *gas = a;
- const struct btd_device *device = b;
-
- return (gas->device == device ? 0 : -1);
-}
-
-static void gap_appearance_cb(guint8 status, const guint8 *pdu, guint16 plen,
- gpointer user_data)
-{
- struct gas *gas = user_data;
- struct att_data_list *list = NULL;
- uint16_t app;
- uint8_t *atval;
-
- if (status != 0) {
- error("Read characteristics by UUID failed: %s",
- att_ecode2str(status));
- return;
- }
-
- list = dec_read_by_type_resp(pdu, plen);
- if (list == NULL)
- return;
-
- if (list->len != 4) {
- error("GAP Appearance value: invalid data");
- goto done;
- }
-
- atval = list->data[0] + 2; /* skip handle value */
- app = get_le16(atval);
-
- DBG("GAP Appearance: 0x%04x", app);
-
- device_set_appearance(gas->device, app);
-
-done:
- att_data_list_free(list);
-}
-
-static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
-{
- struct gas *gas = user_data;
- uint16_t app;
-
- gas->attrib = g_attrib_ref(attrib);
-
- if (device_get_appearance(gas->device, &app) < 0) {
- bt_uuid_t uuid;
-
- bt_uuid16_create(&uuid, GATT_CHARAC_APPEARANCE);
-
- gatt_read_char_by_uuid(gas->attrib, gas->gap.start,
- gas->gap.end, &uuid,
- gap_appearance_cb, gas);
- }
-
- /* TODO: Read other GAP characteristics - See Core spec page 1739 */
-}
-
-static void attio_disconnected_cb(gpointer user_data)
-{
- struct gas *gas = user_data;
-
- g_attrib_unref(gas->attrib);
- gas->attrib = NULL;
-}
-
-static int gas_register(struct btd_device *device, struct att_range *gap)
-{
- struct gas *gas;
-
- gas = g_new0(struct gas, 1);
- gas->gap.start = gap->start;
- gas->gap.end = gap->end;
-
- gas->device = btd_device_ref(device);
-
- devices = g_slist_append(devices, gas);
-
- gas->attioid = btd_device_add_attio_callback(device,
- attio_connected_cb,
- attio_disconnected_cb, gas);
-
- return 0;
-}
-
-static void gas_unregister(struct btd_device *device)
-{
- struct gas *gas;
- GSList *l;
-
- l = g_slist_find_custom(devices, device, cmp_device);
- if (l == NULL)
- return;
-
- gas = l->data;
- devices = g_slist_remove(devices, gas);
- gas_free(gas);
-}
-
-static int gatt_driver_probe(struct btd_service *service)
-{
- struct btd_device *device = btd_service_get_device(service);
- struct gatt_primary *gap;
-
- gap = btd_device_get_primary(device, GAP_UUID);
-
- if (gap == NULL) {
- error("GAP service is mandatory");
- return -EINVAL;
- }
-
- return gas_register(device, &gap->range);
-}
-
-static void gatt_driver_remove(struct btd_service *service)
-{
- struct btd_device *device = btd_service_get_device(service);
-
- gas_unregister(device);
-}
-
-static struct btd_profile gatt_profile = {
- .name = "gap-gatt-profile",
- .remote_uuid = GATT_UUID,
- .device_probe = gatt_driver_probe,
- .device_remove = gatt_driver_remove
-};
-
-static int gatt_init(void)
-{
- btd_profile_register(&gatt_profile);
-
- return 0;
-}
-
-static void gatt_exit(void)
-{
- btd_profile_unregister(&gatt_profile);
-}
-
-BLUETOOTH_PLUGIN_DEFINE(gatt, VERSION, BLUETOOTH_PLUGIN_PRIORITY_DEFAULT,
- gatt_init, gatt_exit)
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH BlueZ 14/15] profiles/gap: Rewrite using bt_gatt_client.
2014-12-09 0:40 [PATCH BlueZ 00/15] core: Use shared/gatt for GATT client role Arman Uguray
` (12 preceding siblings ...)
2014-12-09 0:40 ` [PATCH BlueZ 13/15] profiles/gatt: Rename profile to gap Arman Uguray
@ 2014-12-09 0:40 ` Arman Uguray
2014-12-09 0:40 ` [PATCH BlueZ 15/15] profiles/gap: Add Google copyright Arman Uguray
2014-12-09 12:53 ` [PATCH BlueZ 00/15] core: Use shared/gatt for GATT client role Luiz Augusto von Dentz
15 siblings, 0 replies; 22+ messages in thread
From: Arman Uguray @ 2014-12-09 0:40 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Arman Uguray
This patch rewrites the GAP profile to use the shared GATT stack instead
of GAttrib. The profile now also handles the Device Name characteristic.
---
profiles/gap/gas.c | 225 +++++++++++++++++++++++++++++++++++++++--------------
1 file changed, 166 insertions(+), 59 deletions(-)
diff --git a/profiles/gap/gas.c b/profiles/gap/gas.c
index a4028dd..7b28437 100644
--- a/profiles/gap/gas.c
+++ b/profiles/gap/gas.c
@@ -19,6 +19,7 @@
#include <config.h>
#endif
+#include <ctype.h>
#include <stdbool.h>
#include <stdlib.h>
#include <sys/types.h>
@@ -36,30 +37,34 @@
#include "src/profile.h"
#include "src/service.h"
#include "src/shared/util.h"
-#include "attrib/att.h"
-#include "attrib/gattrib.h"
-#include "src/attio.h"
-#include "attrib/gatt.h"
#include "src/log.h"
#include "src/textfile.h"
+#include "src/shared/att.h"
+#include "src/shared/queue.h"
+#include "src/shared/gatt-db.h"
+#include "src/shared/gatt-client.h"
+#include "src/gatt-callbacks.h"
/* Generic Attribute/Access Service */
struct gas {
struct btd_device *device;
- struct att_range gap; /* GAP Primary service range */
- GAttrib *attrib;
- guint attioid;
+ struct gatt_db *db;
+ struct bt_gatt_client *client;
+ uint16_t start_handle, end_handle;
+ unsigned int gatt_cb_id;
};
static GSList *devices;
static void gas_free(struct gas *gas)
{
- if (gas->attioid)
- btd_device_remove_attio_callback(gas->device, gas->attioid);
+ if (gas->gatt_cb_id)
+ btd_device_remove_gatt_callbacks(gas->device,
+ gas->gatt_cb_id);
- g_attrib_unref(gas->attrib);
btd_device_unref(gas->device);
+ gatt_db_unref(gas->db);
+ bt_gatt_client_unref(gas->client);
g_free(gas);
}
@@ -71,83 +76,193 @@ static int cmp_device(gconstpointer a, gconstpointer b)
return gas->device == device ? 0 : -1;
}
-static void gap_appearance_cb(guint8 status, const guint8 *pdu, guint16 plen,
- gpointer user_data)
+static char *name2utf8(const uint8_t *name, uint8_t len)
+{
+ char utf8_name[HCI_MAX_NAME_LENGTH + 2];
+ int i;
+
+ if (g_utf8_validate((const char *) name, len, NULL))
+ return g_strndup((char *) name, len);
+
+ memset(utf8_name, 0, sizeof(utf8_name));
+ strncpy(utf8_name, (char *) name, len);
+
+ /* Assume ASCII, and replace all non-ASCII with spaces */
+ for (i = 0; utf8_name[i] != '\0'; i++) {
+ if (!isascii(utf8_name[i]))
+ utf8_name[i] = ' ';
+ }
+
+ /* Remove leading and trailing whitespace characters */
+ g_strstrip(utf8_name);
+
+ return g_strdup(utf8_name);
+}
+
+static void read_device_name_cb(bool success, uint8_t att_ecode,
+ const uint8_t *value, uint16_t length,
+ void *user_data)
{
struct gas *gas = user_data;
- struct att_data_list *list = NULL;
- uint16_t app;
- uint8_t *atval;
+ char *name = name2utf8(value, length);
- if (status != 0) {
- error("Read characteristics by UUID failed: %s",
- att_ecode2str(status));
+ DBG("GAP Device Name: %s", name);
+
+ btd_device_device_set_name(gas->device, name);
+}
+
+static void handle_device_name(struct gas *gas, uint16_t value_handle)
+{
+ if (!bt_gatt_client_read_long_value(gas->client, value_handle, 0,
+ read_device_name_cb, gas, NULL))
+ DBG("Failed to send request to read device name");
+}
+
+static void read_appearance_cb(bool success, uint8_t att_ecode,
+ const uint8_t *value, uint16_t length,
+ void *user_data)
+{
+ struct gas *gas = user_data;
+ uint16_t appearance;
+
+ if (!success) {
+ DBG("Reading appearance failed with ATT error: %u", att_ecode);
return;
}
- list = dec_read_by_type_resp(pdu, plen);
- if (list == NULL)
+ /* The appearance value is a 16-bit unsigned integer */
+ if (length != 2) {
+ DBG("Malformed appearance value");
return;
-
- if (list->len != 4) {
- error("GAP Appearance value: invalid data");
- goto done;
}
- atval = list->data[0] + 2; /* skip handle value */
- app = get_le16(atval);
+ appearance = get_le16(value);
- DBG("GAP Appearance: 0x%04x", app);
+ DBG("GAP Appearance: 0x%04x", appearance);
- device_set_appearance(gas->device, app);
+ device_set_appearance(gas->device, appearance);
+}
-done:
- att_data_list_free(list);
+static void handle_appearance(struct gas *gas, uint16_t value_handle)
+{
+ if (!bt_gatt_client_read_value(gas->client, value_handle,
+ read_appearance_cb, gas, NULL))
+ DBG("Failed to send request to read appearance");
}
-static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
+static bool uuid_cmp(uint16_t u16, const bt_uuid_t *uuid)
+{
+ bt_uuid_t lhs;
+
+ bt_uuid16_create(&lhs, u16);
+
+ return bt_uuid_cmp(&lhs, uuid) == 0;
+}
+
+static void handle_characteristic(struct gatt_db_attribute *attr,
+ void *user_data)
{
struct gas *gas = user_data;
- uint16_t app;
+ uint16_t value_handle;
+ bt_uuid_t uuid;
+
+ if (!gatt_db_attribute_get_char_data(attr, NULL, &value_handle, NULL,
+ &uuid)) {
+ error("Failed to obtain characteristic data");
+ return;
+ }
- gas->attrib = g_attrib_ref(attrib);
+ if (uuid_cmp(GATT_CHARAC_DEVICE_NAME, &uuid))
+ handle_device_name(gas, value_handle);
+ else if (uuid_cmp(GATT_CHARAC_APPEARANCE, &uuid))
+ handle_appearance(gas, value_handle);
+ else {
+ char uuid_str[MAX_LEN_UUID_STR];
- if (device_get_appearance(gas->device, &app) < 0) {
- bt_uuid_t uuid;
+ /* TODO: Support peripheral privacy feature */
- bt_uuid16_create(&uuid, GATT_CHARAC_APPEARANCE);
+ bt_uuid_to_string(&uuid, uuid_str, sizeof(uuid_str));
+ DBG("Unsupported characteristic: %s", uuid_str);
+ }
+}
+
+static void handle_service(struct gatt_db_attribute *attr, void *user_data)
+{
+ struct gas *gas = user_data;
- gatt_read_char_by_uuid(gas->attrib, gas->gap.start,
- gas->gap.end, &uuid,
- gap_appearance_cb, gas);
+ if (gas->start_handle) {
+ error("More than one GAP service exists on device");
+ return;
}
- /* TODO: Read other GAP characteristics - See Core spec page 1739 */
+ gatt_db_attribute_get_service_handles(attr, &gas->start_handle,
+ &gas->end_handle);
+ gatt_db_service_foreach_char(attr, handle_characteristic, gas);
}
-static void attio_disconnected_cb(gpointer user_data)
+static void handle_gap_service(struct gas *gas)
+{
+ bt_uuid_t uuid;
+
+ bt_string_to_uuid(&uuid, GAP_UUID);
+
+ gatt_db_foreach_service(gas->db, &uuid, handle_service, gas);
+}
+
+static void gatt_client_ready_cb(struct bt_gatt_client *client,
+ struct gatt_db *db, void *user_data)
{
struct gas *gas = user_data;
- g_attrib_unref(gas->attrib);
- gas->attrib = NULL;
+ gas->client = bt_gatt_client_ref(client);
+ gas->db = gatt_db_ref(db);
+
+ handle_gap_service(gas);
}
-static int gas_register(struct btd_device *device, struct att_range *gap)
+static void gatt_client_disconn_cb(void *user_data)
+{
+ struct gas *gas = user_data;
+
+ gatt_db_unref(gas->db);
+ bt_gatt_client_unref(gas->client);
+
+ gas->db = NULL;
+ gas->client = NULL;
+}
+
+static int gas_register(struct btd_device *device)
{
struct gas *gas;
+ GSList *l;
+
+ /*
+ * There can't be more than one instance of the GAP service on the same
+ * device.
+ */
+ l = g_slist_find_custom(devices, device, cmp_device);
+ if (l)
+ return 0;
gas = g_new0(struct gas, 1);
- gas->gap.start = gap->start;
- gas->gap.end = gap->end;
gas->device = btd_device_ref(device);
-
devices = g_slist_append(devices, gas);
- gas->attioid = btd_device_add_attio_callback(device,
- attio_connected_cb,
- attio_disconnected_cb, gas);
+ /*
+ * Simply register the callbacks. We will perform initialization during
+ * the ready callback once the gatt-client has been initialized for this
+ * device.
+ *
+ * Here we don't register the service removed handler since the
+ * GAP service is mandatory and should not get removed while the device
+ * is connected.
+ */
+ gas->gatt_cb_id = btd_device_add_gatt_callbacks(device,
+ gatt_client_ready_cb,
+ NULL,
+ gatt_client_disconn_cb,
+ gas);
return 0;
}
@@ -169,16 +284,8 @@ static void gas_unregister(struct btd_device *device)
static int gap_driver_probe(struct btd_service *service)
{
struct btd_device *device = btd_service_get_device(service);
- struct gatt_primary *gap;
-
- gap = btd_device_get_primary(device, GAP_UUID);
-
- if (gap == NULL) {
- error("GAP service is mandatory");
- return -EINVAL;
- }
- return gas_register(device, &gap->range);
+ return gas_register(device);
}
static void gap_driver_remove(struct btd_service *service)
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH BlueZ 15/15] profiles/gap: Add Google copyright.
2014-12-09 0:40 [PATCH BlueZ 00/15] core: Use shared/gatt for GATT client role Arman Uguray
` (13 preceding siblings ...)
2014-12-09 0:40 ` [PATCH BlueZ 14/15] profiles/gap: Rewrite using bt_gatt_client Arman Uguray
@ 2014-12-09 0:40 ` Arman Uguray
2014-12-09 12:53 ` [PATCH BlueZ 00/15] core: Use shared/gatt for GATT client role Luiz Augusto von Dentz
15 siblings, 0 replies; 22+ messages in thread
From: Arman Uguray @ 2014-12-09 0:40 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Arman Uguray
Added Google Inc. to the copyright comment since the profile has been
mostly rewritten.
---
profiles/gap/gas.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/profiles/gap/gas.c b/profiles/gap/gas.c
index 7b28437..60d729b 100644
--- a/profiles/gap/gas.c
+++ b/profiles/gap/gas.c
@@ -3,6 +3,7 @@
* BlueZ - Bluetooth protocol stack for Linux
*
* Copyright (C) 2012 Instituto Nokia de Tecnologia - INdT
+ * Copyright (C) 2014 Google Inc.
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH BlueZ 07/15] core: device: Use bt_att_register_disconnect.
2014-12-09 0:40 ` [PATCH BlueZ 07/15] core: device: Use bt_att_register_disconnect Arman Uguray
@ 2014-12-09 8:07 ` Johan Hedberg
2014-12-09 13:12 ` Arman Uguray
0 siblings, 1 reply; 22+ messages in thread
From: Johan Hedberg @ 2014-12-09 8:07 UTC (permalink / raw)
To: Arman Uguray; +Cc: linux-bluetooth
Hi Arman,
On Mon, Dec 08, 2014, Arman Uguray wrote:
> + dev->att = bt_att_ref(g_attrib_get_att(attrib));
I'd expect a function called "get" to return a new reference, so the
extra ref() shouldn't be needed.
Btw, I hope this is just a temporary function that you've introduced to
be able to do the conversions in smaller bits, and that it'll be removed
as soon as the conversions are done?
Johan
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH BlueZ 02/15] shared/att: Add bt_att_get_fd.
2014-12-09 0:40 ` [PATCH BlueZ 02/15] shared/att: Add bt_att_get_fd Arman Uguray
@ 2014-12-09 12:50 ` Luiz Augusto von Dentz
2014-12-09 13:04 ` Arman Uguray
0 siblings, 1 reply; 22+ messages in thread
From: Luiz Augusto von Dentz @ 2014-12-09 12:50 UTC (permalink / raw)
To: Arman Uguray; +Cc: linux-bluetooth@vger.kernel.org
Hi Arman,
On Tue, Dec 9, 2014 at 2:40 AM, Arman Uguray <armansito@chromium.org> wrote:
> This patch adds a getter for a bt_att structure's underlying connection
> file descriptor.
> ---
> src/shared/att.c | 8 ++++++++
> src/shared/att.h | 1 +
> 2 files changed, 9 insertions(+)
>
> diff --git a/src/shared/att.c b/src/shared/att.c
> index ee425d8..9511bb2 100644
> --- a/src/shared/att.c
> +++ b/src/shared/att.c
> @@ -922,6 +922,14 @@ void bt_att_unref(struct bt_att *att)
> free(att);
> }
>
> +int bt_att_get_fd(struct bt_att *att)
> +{
> + if (!att)
> + return -EINVAL;
> +
> + return att->fd;
> +}
> +
> bool bt_att_set_close_on_unref(struct bt_att *att, bool do_close)
> {
> if (!att || !att->io)
> diff --git a/src/shared/att.h b/src/shared/att.h
> index 99b5a5b..b946b18 100644
> --- a/src/shared/att.h
> +++ b/src/shared/att.h
> @@ -33,6 +33,7 @@ struct bt_att *bt_att_new(int fd);
> struct bt_att *bt_att_ref(struct bt_att *att);
> void bt_att_unref(struct bt_att *att);
>
> +int bt_att_get_fd(struct bt_att *att);
It is not clear why this would be necessary?
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH BlueZ 00/15] core: Use shared/gatt for GATT client role
2014-12-09 0:40 [PATCH BlueZ 00/15] core: Use shared/gatt for GATT client role Arman Uguray
` (14 preceding siblings ...)
2014-12-09 0:40 ` [PATCH BlueZ 15/15] profiles/gap: Add Google copyright Arman Uguray
@ 2014-12-09 12:53 ` Luiz Augusto von Dentz
15 siblings, 0 replies; 22+ messages in thread
From: Luiz Augusto von Dentz @ 2014-12-09 12:53 UTC (permalink / raw)
To: Arman Uguray; +Cc: linux-bluetooth@vger.kernel.org
Hi Arman,
On Tue, Dec 9, 2014 at 2:40 AM, Arman Uguray <armansito@chromium.org> wrote:
> This patch set integrates shared/gatt into the core bluetoothd for GATT
> client role. This set makes the following changes:
>
> - Made small fixes and added getters to support legacy code paths.
>
> - Removed explicit primary and included service discovery logic within
> src/device in favor of initializing a bt_gatt_client. This includes
> deferring service/profile registration until the bt_gatt_client is ready,
> and proper handling for "Service Changed" events using gatt_db's service
> added/removed callbacks. When new services are added during a connection,
> the profiles are explicitly probed. The code still creates and maintains
> instances of struct gatt_primary to support legacy plugins.
>
> - Introduced gatt-callbacks.h. This defines callbacks that are invoked when
> services are removed, the gatt-client becomes ready, and when the connection
> is closed. These callbacks are similar and parallel to the existing ones
> defined in attio.h and provide an alternate solution for shared/gatt.
> gatt-callbacks.h is not meant to be the long-term solution and these may
> perhaps be better served by new GATT specific functions added to the
> btd_profile/btd_service framework.
>
> - Rewrote the built-in GAP/GATT profile. It no longer handles the GATT
> service and does no MTU exchange (which was racy and did not belong in a
> profile in the first place) as these are handled by shared/gatt-client.
> Renamed the profile to profiles/gap as it only handles the remote GAP
> service and added handling for the "Device Name" characteristic. The profile
> now uses gatt-callbacks instead of attio and bt_gatt_client/gatt_db instead
> of GAttrib.
>
> Arman Uguray (15):
> attrib/gattrib: Add g_attrib_get_att.
> shared/att: Add bt_att_get_fd.
> attrib: Check if attrib is NULL in functions
> shared/att: cancel_all before calling disconnect cb
> shared/gatt-db: Make accessors work on const ptr
> shared/gatt-db: Add UUID arg to foreach_service
> core: device: Use bt_att_register_disconnect.
> core: device: Use shared/gatt-client for GATT.
> core: Rename device_attach_attrib
> core: Use gatt_db service callbacks
> core: Introduce gatt-callbacks
> profiles/gatt: Don't handle GATT service.
> profiles/gatt: Rename profile to gap.
> profiles/gap: Rewrite using bt_gatt_client.
> profiles/gap: Add Google copyright.
>
> Makefile.am | 1 +
> Makefile.plugins | 4 +-
> attrib/gattrib.c | 31 ++-
> attrib/gattrib.h | 3 +
> profiles/gap/gas.c | 321 +++++++++++++++++++++++++++
> profiles/gatt/gas.c | 457 --------------------------------------
> src/attrib-server.c | 2 +-
> src/device.c | 591 +++++++++++++++++++++++++++++++++++--------------
> src/device.h | 2 +-
> src/gatt-callbacks.h | 36 +++
> src/shared/att-types.h | 3 +-
> src/shared/att.c | 12 +-
> src/shared/att.h | 1 +
> src/shared/gatt-db.c | 41 ++--
> src/shared/gatt-db.h | 28 ++-
> tools/btgatt-client.c | 9 +-
> tools/btgatt-server.c | 2 +-
> unit/test-gatt.c | 4 +-
> 18 files changed, 885 insertions(+), 663 deletions(-)
> create mode 100644 profiles/gap/gas.c
> delete mode 100644 profiles/gatt/gas.c
> create mode 100644 src/gatt-callbacks.h
>
> --
> 2.2.0.rc0.207.ga3a616c
I went ahead and applied the fixes not related to core changes, please
rebase the rest and work in the comments.
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH BlueZ 02/15] shared/att: Add bt_att_get_fd.
2014-12-09 12:50 ` Luiz Augusto von Dentz
@ 2014-12-09 13:04 ` Arman Uguray
0 siblings, 0 replies; 22+ messages in thread
From: Arman Uguray @ 2014-12-09 13:04 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth@vger.kernel.org
Hi Luiz,
> On Tue, Dec 9, 2014 at 4:50 AM, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
> Hi Arman,
>
> On Tue, Dec 9, 2014 at 2:40 AM, Arman Uguray <armansito@chromium.org> wrote:
>> This patch adds a getter for a bt_att structure's underlying connection
>> file descriptor.
>> ---
>> src/shared/att.c | 8 ++++++++
>> src/shared/att.h | 1 +
>> 2 files changed, 9 insertions(+)
>>
>> diff --git a/src/shared/att.c b/src/shared/att.c
>> index ee425d8..9511bb2 100644
>> --- a/src/shared/att.c
>> +++ b/src/shared/att.c
>> @@ -922,6 +922,14 @@ void bt_att_unref(struct bt_att *att)
>> free(att);
>> }
>>
>> +int bt_att_get_fd(struct bt_att *att)
>> +{
>> + if (!att)
>> + return -EINVAL;
>> +
>> + return att->fd;
>> +}
>> +
>> bool bt_att_set_close_on_unref(struct bt_att *att, bool do_close)
>> {
>> if (!att || !att->io)
>> diff --git a/src/shared/att.h b/src/shared/att.h
>> index 99b5a5b..b946b18 100644
>> --- a/src/shared/att.h
>> +++ b/src/shared/att.h
>> @@ -33,6 +33,7 @@ struct bt_att *bt_att_new(int fd);
>> struct bt_att *bt_att_ref(struct bt_att *att);
>> void bt_att_unref(struct bt_att *att);
>>
>> +int bt_att_get_fd(struct bt_att *att);
>
> It is not clear why this would be necessary?
>
I added this to make one call site work where src/device uses it to
obtain a socket error in the event of a disconnect. I could probably
avoid this by getting the fd out of the GIOChannel which device is
still caching anyway.
-Arman
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH BlueZ 07/15] core: device: Use bt_att_register_disconnect.
2014-12-09 8:07 ` Johan Hedberg
@ 2014-12-09 13:12 ` Arman Uguray
2014-12-09 15:50 ` Johan Hedberg
0 siblings, 1 reply; 22+ messages in thread
From: Arman Uguray @ 2014-12-09 13:12 UTC (permalink / raw)
To: BlueZ development
Hi Johan,
> On Tue, Dec 9, 2014 at 12:07 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Arman,
>
> On Mon, Dec 08, 2014, Arman Uguray wrote:
>> + dev->att = bt_att_ref(g_attrib_get_att(attrib));
>
> I'd expect a function called "get" to return a new reference, so the
> extra ref() shouldn't be needed.
>
I see it mostly as a raw getter and adding ref semantics to it seems
confusing to me. Unless we call it g_attrib_get_att_ref or something
to that end.
> Btw, I hope this is just a temporary function that you've introduced to
> be able to do the conversions in smaller bits, and that it'll be removed
> as soon as the conversions are done?
>
Pretty much. Basically we have the old code work through the GAttrib
shim while the new code work directly through the bt_att that the
GAttrib is wrapping. Once all conversions are done, device won't need
to include any of the GAttrib code and this function can be removed.
Thanks,
Arman
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH BlueZ 07/15] core: device: Use bt_att_register_disconnect.
2014-12-09 13:12 ` Arman Uguray
@ 2014-12-09 15:50 ` Johan Hedberg
0 siblings, 0 replies; 22+ messages in thread
From: Johan Hedberg @ 2014-12-09 15:50 UTC (permalink / raw)
To: Arman Uguray; +Cc: BlueZ development
Hi Arman,
On Tue, Dec 09, 2014, Arman Uguray wrote:
> > On Tue, Dec 9, 2014 at 12:07 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> > Hi Arman,
> >
> > On Mon, Dec 08, 2014, Arman Uguray wrote:
> >> + dev->att = bt_att_ref(g_attrib_get_att(attrib));
> >
> > I'd expect a function called "get" to return a new reference, so the
> > extra ref() shouldn't be needed.
> >
>
> I see it mostly as a raw getter and adding ref semantics to it seems
> confusing to me. Unless we call it g_attrib_get_att_ref or something
> to that end.
Could be that this is also just because of me spending too much time
looking at kernel code. There pretty much everywhere foo_ref() is called
foo_get() and foo_unref() is called foo_put(). I thought this was the
common behavior for user space *get() functions too, but looking now I
could only find agent_get() following this practice. The most annoying
thing about your code was really the nested function calls so whatever
solution gets rid of that is a step towards the better.
Johan
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2014-12-09 15:50 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-12-09 0:40 [PATCH BlueZ 00/15] core: Use shared/gatt for GATT client role Arman Uguray
2014-12-09 0:40 ` [PATCH BlueZ 01/15] attrib/gattrib: Add g_attrib_get_att Arman Uguray
2014-12-09 0:40 ` [PATCH BlueZ 02/15] shared/att: Add bt_att_get_fd Arman Uguray
2014-12-09 12:50 ` Luiz Augusto von Dentz
2014-12-09 13:04 ` Arman Uguray
2014-12-09 0:40 ` [PATCH BlueZ 03/15] attrib: Check if attrib is NULL in functions Arman Uguray
2014-12-09 0:40 ` [PATCH BlueZ 04/15] shared/att: cancel_all before calling disconnect cb Arman Uguray
2014-12-09 0:40 ` [PATCH BlueZ 05/15] shared/gatt-db: Make accessors work on const ptr Arman Uguray
2014-12-09 0:40 ` [PATCH BlueZ 06/15] shared/gatt-db: Add UUID arg to foreach_service Arman Uguray
2014-12-09 0:40 ` [PATCH BlueZ 07/15] core: device: Use bt_att_register_disconnect Arman Uguray
2014-12-09 8:07 ` Johan Hedberg
2014-12-09 13:12 ` Arman Uguray
2014-12-09 15:50 ` Johan Hedberg
2014-12-09 0:40 ` [PATCH BlueZ 08/15] core: device: Use shared/gatt-client for GATT Arman Uguray
2014-12-09 0:40 ` [PATCH BlueZ 09/15] core: Rename device_attach_attrib Arman Uguray
2014-12-09 0:40 ` [PATCH BlueZ 10/15] core: Use gatt_db service callbacks Arman Uguray
2014-12-09 0:40 ` [PATCH BlueZ 11/15] core: Introduce gatt-callbacks Arman Uguray
2014-12-09 0:40 ` [PATCH BlueZ 12/15] profiles/gatt: Don't handle GATT service Arman Uguray
2014-12-09 0:40 ` [PATCH BlueZ 13/15] profiles/gatt: Rename profile to gap Arman Uguray
2014-12-09 0:40 ` [PATCH BlueZ 14/15] profiles/gap: Rewrite using bt_gatt_client Arman Uguray
2014-12-09 0:40 ` [PATCH BlueZ 15/15] profiles/gap: Add Google copyright Arman Uguray
2014-12-09 12:53 ` [PATCH BlueZ 00/15] core: Use shared/gatt for GATT client role Luiz Augusto von Dentz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).