2010-12-15 19:54:08

by Claudio Takahasi

Subject: [PATCH 1/5] Implement cancel primary discovery session

Extend bt_cancel_discovery function to cancel an ongoing Discover
All Primary Services procedure.
---
src/device.c | 11 +++----
src/glib-helper.c | 73 ++++++++++++++++++++++++++++++++++++++++++-----------
2 files changed, 63 insertions(+), 21 deletions(-)

diff --git a/src/device.c b/src/device.c
index cec2153..d20a6d4 100644
--- a/src/device.c
+++ b/src/device.c
@@ -186,8 +186,7 @@ static void browse_request_cancel(struct browse_req *req)

adapter_get_address(adapter, &src);

- if (device->type != DEVICE_TYPE_LE)
- bt_cancel_discovery(&src, &device->bdaddr);
+ bt_cancel_discovery(&src, &device->bdaddr);

device->browse = NULL;
browse_request_free(req);
@@ -1530,11 +1529,11 @@ static void primary_cb(GSList *services, int err, gpointer user_data)
goto done;
}

- services_changed(req->device);
- device_set_temporary(req->device, FALSE);
- device_probe_drivers(req->device, services);
+ services_changed(device);
+ device_set_temporary(device, FALSE);
+ device_probe_drivers(device, services);

- create_device_reply(req->device, req);
+ create_device_reply(device, req);

done:
device->browse = NULL;
diff --git a/src/glib-helper.c b/src/glib-helper.c
index 8181f4d..6505249 100644
--- a/src/glib-helper.c
+++ b/src/glib-helper.c
@@ -51,12 +51,15 @@ struct gattrib_context {
bdaddr_t src;
bdaddr_t dst;
GAttrib *attrib;
+ GIOChannel *io;
bt_primary_t cb;
bt_destroy_t destroy;
gpointer user_data;
GSList *uuids;
};

+static GSList *gattrib_list = NULL;
+
struct cached_sdp_session {
bdaddr_t src;
bdaddr_t dst;
@@ -68,12 +71,18 @@ static GSList *cached_sdp_sessions = NULL;

static void gattrib_context_free(struct gattrib_context *ctxt)
{
+ gattrib_list = g_slist_remove(gattrib_list, ctxt);
if (ctxt->destroy)
ctxt->destroy(ctxt->user_data);

g_slist_foreach(ctxt->uuids, (GFunc) g_free, NULL);
g_slist_free(ctxt->uuids);
g_attrib_unref(ctxt->attrib);
+ if (ctxt->io) {
+ g_io_channel_unref(ctxt->io);
+ g_io_channel_shutdown(ctxt->io, FALSE, NULL);
+ }
+
g_free(ctxt);
}

@@ -138,7 +147,6 @@ struct search_context {
bdaddr_t dst;
sdp_session_t *session;
bt_callback_t cb;
- bt_primary_t prim_cb;
bt_destroy_t destroy;
gpointer user_data;
uuid_t uuid;
@@ -373,21 +381,16 @@ static gint find_by_bdaddr(gconstpointer data, gconstpointer user_data)
bacmp(&ctxt->src, &search->src));
}

-int bt_cancel_discovery(const bdaddr_t *src, const bdaddr_t *dst)
+static gint gattrib_find_by_bdaddr(gconstpointer data, gconstpointer user_data)
{
- struct search_context search, *ctxt;
- GSList *match;
+ const struct gattrib_context *ctxt = data, *search = user_data;

- memset(&search, 0, sizeof(search));
- bacpy(&search.src, src);
- bacpy(&search.dst, dst);
-
- /* Ongoing SDP Discovery */
- match = g_slist_find_custom(context_list, &search, find_by_bdaddr);
- if (!match)
- return -ENODATA;
+ return (bacmp(&ctxt->dst, &search->dst) &&
+ bacmp(&ctxt->src, &search->src));
+}

- ctxt = match->data;
+static int cancel_sdp(struct search_context *ctxt)
+{
if (!ctxt->session)
return -ENOTCONN;

@@ -398,9 +401,48 @@ int bt_cancel_discovery(const bdaddr_t *src, const bdaddr_t *dst)
sdp_close(ctxt->session);

search_context_cleanup(ctxt);
+
+ return 0;
+}
+
+static int cancel_gattrib(struct gattrib_context *ctxt)
+{
+ if (ctxt->attrib)
+ g_attrib_cancel_all(ctxt->attrib);
+
+ gattrib_context_free(ctxt);
+
return 0;
}

+int bt_cancel_discovery(const bdaddr_t *src, const bdaddr_t *dst)
+{
+ struct search_context sdp_ctxt;
+ struct gattrib_context gatt_ctxt;
+ GSList *match;
+
+ memset(&sdp_ctxt, 0, sizeof(sdp_ctxt));
+ bacpy(&sdp_ctxt.src, src);
+ bacpy(&sdp_ctxt.dst, dst);
+
+ /* Ongoing SDP Discovery */
+ match = g_slist_find_custom(context_list, &sdp_ctxt, find_by_bdaddr);
+ if (match)
+ return cancel_sdp(match->data);
+
+ memset(&gatt_ctxt, 0, sizeof(gatt_ctxt));
+ bacpy(&gatt_ctxt.src, src);
+ bacpy(&gatt_ctxt.dst, dst);
+
+ /* Ongoing Discover All Primary Services */
+ match = g_slist_find_custom(gattrib_list, &gatt_ctxt,
+ gattrib_find_by_bdaddr);
+ if (match == NULL)
+ return -ENOTCONN;
+
+ return cancel_gattrib(match->data);
+}
+
static void primary_cb(guint8 status, const guint8 *pdu, guint16 plen,
gpointer user_data)
{
@@ -471,6 +513,7 @@ static void connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
return;
}

+ ctxt->attrib = g_attrib_new(io);
gatt_discover_primary(ctxt->attrib, 0x0001, 0xffff, NULL, primary_cb,
ctxt);
}
@@ -513,9 +556,9 @@ int bt_discover_primary(const bdaddr_t *src, const bdaddr_t *dst, int psm,
return -EIO;
}

- ctxt->attrib = g_attrib_new(io);
+ ctxt->io = io;

- g_io_channel_unref(io);
+ gattrib_list = g_slist_append(gattrib_list, ctxt);

return 0;
}
--
1.7.3.3

2010-12-23 22:37:17

by Johan Hedberg

Subject: Re: [PATCH v5 2/5] Change CreatePairedDevice to support LE devices

Hi Sheldon,

On Thu, Dec 23, 2010, Sheldon Demario wrote:
> CreatePairedDevice implements now the same behaviour of CreateDevice,
> triggering Discover All Primary Services when needed. SMP negotiation
> starts when the link is established. LE capable kernel is required to
> test this method properly.
>
> Limitation: For dual mode devices, Discover All Primary Services is not
> being executed after SDP search if GATT record is found.
> ---
> src/adapter.c | 102 +++++++++++++++++++++++++++++++++++++----------------
> src/device.c | 89 ++++++++++++++++++++++++----------------------
> src/device.h | 5 ++-
> src/glib-helper.c | 12 +++++-
> src/glib-helper.h | 1 +
> 5 files changed, 132 insertions(+), 77 deletions(-)

Pushed upstream. Thanks.

Johan

2010-12-23 21:29:50

by Sheldon Demario

Subject: [PATCH v5 2/5] Change CreatePairedDevice to support LE devices

CreatePairedDevice implements now the same behaviour of CreateDevice,
triggering Discover All Primary Services when needed. SMP negotiation
starts when the link is established. LE capable kernel is required to
test this method properly.

Limitation: For dual mode devices, Discover All Primary Services is not
being executed after SDP search if GATT record is found.
---
src/adapter.c | 102 +++++++++++++++++++++++++++++++++++++----------------
src/device.c | 89 ++++++++++++++++++++++++----------------------
src/device.h | 5 ++-
src/glib-helper.c | 12 +++++-
src/glib-helper.h | 1 +
5 files changed, 132 insertions(+), 77 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index 1913171..fddf0ad 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -1520,15 +1520,48 @@ static gboolean event_is_connectable(uint8_t type)
}
}

+static struct btd_device *create_device_internal(DBusConnection *conn,
+ struct btd_adapter *adapter,
+ const gchar *address,
+ gboolean force, int *err)
+{
+ struct remote_dev_info *dev, match;
+ struct btd_device *device;
+ device_type_t type;
+
+ memset(&match, 0, sizeof(struct remote_dev_info));
+ str2ba(address, &match.bdaddr);
+ match.name_status = NAME_ANY;
+
+ dev = adapter_search_found_devices(adapter, &match);
+ if (dev && dev->flags)
+ type = flags2type(dev->flags);
+ else
+ type = DEVICE_TYPE_BREDR;
+
+ if (!force && type == DEVICE_TYPE_LE &&
+ !event_is_connectable(dev->evt_type)) {
+ if (err)
+ *err = -ENOTCONN;
+
+ return NULL;
+ }
+
+ device = adapter_create_device(conn, adapter, address, type);
+ if (!device && err)
+ *err = -ENOMEM;
+
+ return device;
+}
+
static DBusMessage *create_device(DBusConnection *conn,
DBusMessage *msg, void *data)
{
struct btd_adapter *adapter = data;
struct btd_device *device;
- struct remote_dev_info *dev, match;
const gchar *address;
+ DBusMessage *reply;
int err;
- device_type_t type;

if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &address,
DBUS_TYPE_INVALID) == FALSE)
@@ -1545,41 +1578,36 @@ static DBusMessage *create_device(DBusConnection *conn,

DBG("%s", address);

- memset(&match, 0, sizeof(struct remote_dev_info));
- str2ba(address, &match.bdaddr);
- match.name_status = NAME_ANY;
+ device = create_device_internal(conn, adapter, address, TRUE, &err);
+ if (!device)
+ goto failed;

- dev = adapter_search_found_devices(adapter, &match);
- if (dev && dev->flags)
- type = flags2type(dev->flags);
+ if (device_get_type(device) != DEVICE_TYPE_LE)
+ err = device_browse_sdp(device, conn, msg, NULL, FALSE);
else
- type = DEVICE_TYPE_BREDR;
+ err = device_browse_primary(device, conn, msg, FALSE);

- device = adapter_create_device(conn, adapter, address, type);
- if (!device)
- return NULL;
+ if (err < 0) {
+ adapter_remove_device(conn, adapter, device, TRUE);
+ return btd_error_failed(msg, strerror(-err));
+ }

- if (type == DEVICE_TYPE_LE && !event_is_connectable(dev->evt_type)) {
+ return NULL;
+
+failed:
+ if (err == -ENOTCONN) {
/* Device is not connectable */
const char *path = device_get_path(device);
- DBusMessage *reply;

reply = dbus_message_new_method_return(msg);

dbus_message_append_args(reply,
- DBUS_TYPE_OBJECT_PATH, &path,
- DBUS_TYPE_INVALID);
-
- return reply;
- }
-
- err = device_browse(device, conn, msg, NULL, FALSE);
- if (err < 0) {
- adapter_remove_device(conn, adapter, device, TRUE);
- return btd_error_failed(msg, strerror(-err));
- }
+ DBUS_TYPE_OBJECT_PATH, &path,
+ DBUS_TYPE_INVALID);
+ } else
+ reply = btd_error_failed(msg, strerror(-err));

- return NULL;
+ return reply;
}

static uint8_t parse_io_capability(const char *capability)
@@ -1604,6 +1632,7 @@ static DBusMessage *create_paired_device(DBusConnection *conn,
struct btd_device *device;
const gchar *address, *agent_path, *capability, *sender;
uint8_t cap;
+ int err;

if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &address,
DBUS_TYPE_OBJECT_PATH, &agent_path,
@@ -1628,12 +1657,23 @@ static DBusMessage *create_paired_device(DBusConnection *conn,
if (cap == IO_CAPABILITY_INVALID)
return btd_error_invalid_args(msg);

- device = adapter_get_device(conn, adapter, address);
- if (!device)
- return btd_error_failed(msg,
- "Unable to create a new device object");
+ device = adapter_find_device(adapter, address);
+ if (!device) {
+ device = create_device_internal(conn, adapter, address,
+ FALSE, &err);
+ if (!device)
+ return btd_error_failed(msg, strerror(-err));
+ }
+
+ if (device_get_type(device) != DEVICE_TYPE_LE)
+ return device_create_bonding(device, conn, msg,
+ agent_path, cap);

- return device_create_bonding(device, conn, msg, agent_path, cap);
+ err = device_browse_primary(device, conn, msg, TRUE);
+ if (err < 0)
+ return btd_error_failed(msg, strerror(-err));
+
+ return NULL;
}

static gint device_path_cmp(struct btd_device *device, const gchar *path)
diff --git a/src/device.c b/src/device.c
index e093b82..c33b22b 100644
--- a/src/device.c
+++ b/src/device.c
@@ -588,7 +588,7 @@ static DBusMessage *discover_services(DBusConnection *conn,
return btd_error_invalid_args(msg);

if (strlen(pattern) == 0) {
- err = device_browse(device, conn, msg, NULL, FALSE);
+ err = device_browse_sdp(device, conn, msg, NULL, FALSE);
if (err < 0)
goto fail;
} else {
@@ -599,7 +599,7 @@ static DBusMessage *discover_services(DBusConnection *conn,

sdp_uuid128_to_uuid(&uuid);

- err = device_browse(device, conn, msg, &uuid, FALSE);
+ err = device_browse_sdp(device, conn, msg, &uuid, FALSE);
if (err < 0)
goto fail;
}
@@ -992,6 +992,11 @@ void device_get_name(struct btd_device *device, char *name, size_t len)
strncpy(name, device->name, len);
}

+device_type_t device_get_type(struct btd_device *device)
+{
+ return device->type;
+}
+
void device_remove_bonding(struct btd_device *device)
{
char filename[PATH_MAX + 1];
@@ -1599,41 +1604,62 @@ done:
browse_request_free(req);
}

-static struct browse_req *browse_primary(struct btd_device *device, int *err)
+int device_browse_primary(struct btd_device *device, DBusConnection *conn,
+ DBusMessage *msg, gboolean secure)
{
struct btd_adapter *adapter = device->adapter;
struct browse_req *req;
bdaddr_t src;
- int ret;
+ int err;
+
+ if (device->browse)
+ return -EBUSY;

req = g_new0(struct browse_req, 1);
req->device = btd_device_ref(device);

adapter_get_address(adapter, &src);

- ret = bt_discover_primary(&src, &device->bdaddr, -1, primary_cb, req,
- NULL);
-
- if (ret < 0) {
+ err = bt_discover_primary(&src, &device->bdaddr, -1, primary_cb, req,
+ secure, NULL);
+ if (err < 0) {
browse_request_free(req);
- if (err)
- *err = ret;
+ return err;
+ }

- return NULL;
+ if (conn == NULL)
+ conn = get_dbus_connection();
+
+ req->conn = dbus_connection_ref(conn);
+ device->browse = req;
+
+ if (msg) {
+ const char *sender = dbus_message_get_sender(msg);
+
+ req->msg = dbus_message_ref(msg);
+ /* Track the request owner to cancel it
+ * automatically if the owner exits */
+ req->listener_id = g_dbus_add_disconnect_watch(conn,
+ sender,
+ discover_services_req_exit,
+ req, NULL);
}

- return req;
+ return err;
}

-static struct browse_req *browse_sdp(struct btd_device *device, uuid_t *search,
- gboolean reverse, int *err)
+int device_browse_sdp(struct btd_device *device, DBusConnection *conn,
+ DBusMessage *msg, uuid_t *search, gboolean reverse)
{
struct btd_adapter *adapter = device->adapter;
struct browse_req *req;
bt_callback_t cb;
bdaddr_t src;
uuid_t uuid;
- int ret;
+ int err;
+
+ if (device->browse)
+ return -EBUSY;

adapter_get_address(adapter, &src);

@@ -1648,34 +1674,11 @@ static struct browse_req *browse_sdp(struct btd_device *device, uuid_t *search,
cb = browse_cb;
}

- ret = bt_search_service(&src, &device->bdaddr, &uuid, cb, req, NULL);
- if (ret < 0) {
+ err = bt_search_service(&src, &device->bdaddr, &uuid, cb, req, NULL);
+ if (err < 0) {
browse_request_free(req);
- if (err)
- *err = ret;
-
- return NULL;
- }
-
- return req;
-}
-
-int device_browse(struct btd_device *device, DBusConnection *conn,
- DBusMessage *msg, uuid_t *search, gboolean reverse)
-{
- struct browse_req *req;
- int err = 0;
-
- if (device->browse)
- return -EBUSY;
-
- if (device->type == DEVICE_TYPE_LE)
- req = browse_primary(device, &err);
- else
- req = browse_sdp(device, search, reverse, &err);
-
- if (req == NULL)
return err;
+ }

if (conn == NULL)
conn = get_dbus_connection();
@@ -1786,7 +1789,7 @@ static gboolean start_discovery(gpointer user_data)
{
struct btd_device *device = user_data;

- device_browse(device, NULL, NULL, NULL, TRUE);
+ device_browse_sdp(device, NULL, NULL, NULL, TRUE);

device->discov_timer = 0;

@@ -2123,7 +2126,7 @@ void device_bonding_complete(struct btd_device *device, uint8_t status)
device->discov_timer = 0;
}

- device_browse(device, bonding->conn, bonding->msg,
+ device_browse_sdp(device, bonding->conn, bonding->msg,
NULL, FALSE);

bonding_request_free(bonding);
diff --git a/src/device.h b/src/device.h
index 86721bf..5dea953 100644
--- a/src/device.h
+++ b/src/device.h
@@ -47,9 +47,12 @@ struct btd_device *device_create(DBusConnection *conn,
const gchar *address, device_type_t type);
void device_set_name(struct btd_device *device, const char *name);
void device_get_name(struct btd_device *device, char *name, size_t len);
+device_type_t device_get_type(struct btd_device *device);
void device_remove(struct btd_device *device, gboolean remove_stored);
gint device_address_cmp(struct btd_device *device, const gchar *address);
-int device_browse(struct btd_device *device, DBusConnection *conn,
+int device_browse_primary(struct btd_device *device, DBusConnection *conn,
+ DBusMessage *msg, gboolean secure);
+int device_browse_sdp(struct btd_device *device, DBusConnection *conn,
DBusMessage *msg, uuid_t *search, gboolean reverse);
void device_probe_drivers(struct btd_device *device, GSList *profiles);
const sdp_record_t *btd_device_get_record(struct btd_device *device,
diff --git a/src/glib-helper.c b/src/glib-helper.c
index 648dd62..01517a3 100644
--- a/src/glib-helper.c
+++ b/src/glib-helper.c
@@ -517,9 +517,11 @@ static void connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)

int bt_discover_primary(const bdaddr_t *src, const bdaddr_t *dst, int psm,
bt_primary_t cb, void *user_data,
+ gboolean secure,
bt_destroy_t destroy)
{
struct gattrib_context *ctxt;
+ BtIOSecLevel sec_level;
GIOChannel *io;

ctxt = g_try_new0(struct gattrib_context, 1);
@@ -532,19 +534,25 @@ int bt_discover_primary(const bdaddr_t *src, const bdaddr_t *dst, int psm,
ctxt->cb = cb;
ctxt->destroy = destroy;

+ if (secure == TRUE)
+ sec_level = BT_IO_SEC_HIGH;
+ else
+ sec_level = BT_IO_SEC_LOW;
+
+
if (psm < 0)
io = bt_io_connect(BT_IO_L2CAP, connect_cb, ctxt, NULL, NULL,
BT_IO_OPT_SOURCE_BDADDR, src,
BT_IO_OPT_DEST_BDADDR, dst,
BT_IO_OPT_CID, GATT_CID,
- BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_LOW,
+ BT_IO_OPT_SEC_LEVEL, sec_level,
BT_IO_OPT_INVALID);
else
io = bt_io_connect(BT_IO_L2CAP, connect_cb, ctxt, NULL, NULL,
BT_IO_OPT_SOURCE_BDADDR, src,
BT_IO_OPT_DEST_BDADDR, dst,
BT_IO_OPT_PSM, psm,
- BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_LOW,
+ BT_IO_OPT_SEC_LEVEL, sec_level,
BT_IO_OPT_INVALID);

if (io == NULL) {
diff --git a/src/glib-helper.h b/src/glib-helper.h
index ad81d7f..25fe276 100644
--- a/src/glib-helper.h
+++ b/src/glib-helper.h
@@ -32,6 +32,7 @@ int bt_cancel_discovery(const bdaddr_t *src, const bdaddr_t *dst);

int bt_discover_primary(const bdaddr_t *src, const bdaddr_t *dst, int psm,
bt_primary_t cb, void *user_data,
+ gboolean secure,
bt_destroy_t destroy);

gchar *bt_uuid2string(uuid_t *uuid);
--
1.7.3.2

2010-12-21 14:34:57

by Sheldon Demario

Subject: [PATCH v4 2/5] Change CreatePairedDevice to support LE devices

CreatePairedDevice implements now the same behaviour of CreateDevice,
triggering Discover All Primary Services when needed. SMP negotiation
starts when the link is established. LE capable kernel is required to
test this method properly.

Limitation: For dual mode devices, Discover All Primary Services is not
being executed after SDP search if GATT record is found.
---
src/adapter.c | 102 +++++++++++++++++++++++++++++++++++++----------------
src/device.c | 89 ++++++++++++++++++++++++----------------------
src/device.h | 5 ++-
src/glib-helper.c | 12 +++++-
src/glib-helper.h | 1 +
5 files changed, 132 insertions(+), 77 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index cf3566d..82f55ef 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -1510,15 +1510,48 @@ static gboolean event_is_connectable(uint8_t type)
}
}

+static struct btd_device *create_device_internal(DBusConnection *conn,
+ struct btd_adapter *adapter,
+ const gchar *address,
+ gboolean force, int *err)
+{
+ struct remote_dev_info *dev, match;
+ struct btd_device *device;
+ device_type_t type;
+
+ memset(&match, 0, sizeof(struct remote_dev_info));
+ str2ba(address, &match.bdaddr);
+ match.name_status = NAME_ANY;
+
+ dev = adapter_search_found_devices(adapter, &match);
+ if (dev && dev->flags)
+ type = flags2type(dev->flags);
+ else
+ type = DEVICE_TYPE_BREDR;
+
+ if (!force && type == DEVICE_TYPE_LE &&
+ !event_is_connectable(dev->evt_type)) {
+ if (err)
+ *err = -ENOTCONN;
+
+ return NULL;
+ }
+
+ device = adapter_create_device(conn, adapter, address, type);
+ if (!device && err)
+ *err = -ENOMEM;
+
+ return device;
+}
+
static DBusMessage *create_device(DBusConnection *conn,
DBusMessage *msg, void *data)
{
struct btd_adapter *adapter = data;
struct btd_device *device;
- struct remote_dev_info *dev, match;
const gchar *address;
+ DBusMessage *reply;
int err;
- device_type_t type;

if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &address,
DBUS_TYPE_INVALID) == FALSE)
@@ -1535,41 +1568,36 @@ static DBusMessage *create_device(DBusConnection *conn,

DBG("%s", address);

- memset(&match, 0, sizeof(struct remote_dev_info));
- str2ba(address, &match.bdaddr);
- match.name_status = NAME_ANY;
+ device = create_device_internal(conn, adapter, address, TRUE, &err);
+ if (!device)
+ goto failed;

- dev = adapter_search_found_devices(adapter, &match);
- if (dev && dev->flags)
- type = flags2type(dev->flags);
+ if (device_get_type(device) != DEVICE_TYPE_LE)
+ err = device_browse_sdp(device, conn, msg, NULL, FALSE);
else
- type = DEVICE_TYPE_BREDR;
+ err = device_browse_primary(device, conn, msg, FALSE);

- device = adapter_create_device(conn, adapter, address, type);
- if (!device)
- return NULL;
+ if (err < 0) {
+ adapter_remove_device(conn, adapter, device, TRUE);
+ return btd_error_failed(msg, strerror(-err));
+ }

- if (type == DEVICE_TYPE_LE && !event_is_connectable(dev->evt_type)) {
+ return NULL;
+
+failed:
+ if (err == -ENOTCONN) {
/* Device is not connectable */
const char *path = device_get_path(device);
- DBusMessage *reply;

reply = dbus_message_new_method_return(msg);

dbus_message_append_args(reply,
- DBUS_TYPE_OBJECT_PATH, &path,
- DBUS_TYPE_INVALID);
-
- return reply;
- }
-
- err = device_browse(device, conn, msg, NULL, FALSE);
- if (err < 0) {
- adapter_remove_device(conn, adapter, device, TRUE);
- return btd_error_failed(msg, strerror(-err));
- }
+ DBUS_TYPE_OBJECT_PATH, &path,
+ DBUS_TYPE_INVALID);
+ } else
+ reply = btd_error_failed(msg, strerror(-err));

- return NULL;
+ return reply;
}

static uint8_t parse_io_capability(const char *capability)
@@ -1594,6 +1622,7 @@ static DBusMessage *create_paired_device(DBusConnection *conn,
struct btd_device *device;
const gchar *address, *agent_path, *capability, *sender;
uint8_t cap;
+ int err;

if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &address,
DBUS_TYPE_OBJECT_PATH, &agent_path,
@@ -1618,12 +1647,23 @@ static DBusMessage *create_paired_device(DBusConnection *conn,
if (cap == IO_CAPABILITY_INVALID)
return btd_error_invalid_args(msg);

- device = adapter_get_device(conn, adapter, address);
- if (!device)
- return btd_error_failed(msg,
- "Unable to create a new device object");
+ device = adapter_find_device(adapter, address);
+ if (!device) {
+ device = create_device_internal(conn, adapter, address,
+ FALSE, &err);
+ if (!device)
+ return btd_error_failed(msg, strerror(-err));
+ }
+
+ if (device_get_type(device) != DEVICE_TYPE_LE)
+ return device_create_bonding(device, conn, msg,
+ agent_path, cap);

- return device_create_bonding(device, conn, msg, agent_path, cap);
+ err = device_browse_primary(device, conn, msg, TRUE);
+ if (err < 0)
+ return btd_error_failed(msg, strerror(-err));
+
+ return NULL;
}

static gint device_path_cmp(struct btd_device *device, const gchar *path)
diff --git a/src/device.c b/src/device.c
index bef8e71..87a6647 100644
--- a/src/device.c
+++ b/src/device.c
@@ -583,7 +583,7 @@ static DBusMessage *discover_services(DBusConnection *conn,
return btd_error_invalid_args(msg);

if (strlen(pattern) == 0) {
- err = device_browse(device, conn, msg, NULL, FALSE);
+ err = device_browse_sdp(device, conn, msg, NULL, FALSE);
if (err < 0)
goto fail;
} else {
@@ -594,7 +594,7 @@ static DBusMessage *discover_services(DBusConnection *conn,

sdp_uuid128_to_uuid(&uuid);

- err = device_browse(device, conn, msg, &uuid, FALSE);
+ err = device_browse_sdp(device, conn, msg, &uuid, FALSE);
if (err < 0)
goto fail;
}
@@ -985,6 +985,11 @@ void device_get_name(struct btd_device *device, char *name, size_t len)
strncpy(name, device->name, len);
}

+device_type_t device_get_type(struct btd_device *device)
+{
+ return device->type;
+}
+
void device_remove_bonding(struct btd_device *device)
{
char filename[PATH_MAX + 1];
@@ -1537,41 +1542,62 @@ done:
browse_request_free(req);
}

-static struct browse_req *browse_primary(struct btd_device *device, int *err)
+int device_browse_primary(struct btd_device *device, DBusConnection *conn,
+ DBusMessage *msg, gboolean secure)
{
struct btd_adapter *adapter = device->adapter;
struct browse_req *req;
bdaddr_t src;
- int ret;
+ int err;
+
+ if (device->browse)
+ return -EBUSY;

req = g_new0(struct browse_req, 1);
req->device = btd_device_ref(device);

adapter_get_address(adapter, &src);

- ret = bt_discover_primary(&src, &device->bdaddr, -1, primary_cb, req,
- NULL);
-
- if (ret < 0) {
+ err = bt_discover_primary(&src, &device->bdaddr, -1, primary_cb, req,
+ secure, NULL);
+ if (err < 0) {
browse_request_free(req);
- if (err)
- *err = ret;
+ return err;
+ }

- return NULL;
+ if (conn == NULL)
+ conn = get_dbus_connection();
+
+ req->conn = dbus_connection_ref(conn);
+ device->browse = req;
+
+ if (msg) {
+ const char *sender = dbus_message_get_sender(msg);
+
+ req->msg = dbus_message_ref(msg);
+ /* Track the request owner to cancel it
+ * automatically if the owner exits */
+ req->listener_id = g_dbus_add_disconnect_watch(conn,
+ sender,
+ discover_services_req_exit,
+ req, NULL);
}

- return req;
+ return err;
}

-static struct browse_req *browse_sdp(struct btd_device *device, uuid_t *search,
- gboolean reverse, int *err)
+int device_browse_sdp(struct btd_device *device, DBusConnection *conn,
+ DBusMessage *msg, uuid_t *search, gboolean reverse)
{
struct btd_adapter *adapter = device->adapter;
struct browse_req *req;
bt_callback_t cb;
bdaddr_t src;
uuid_t uuid;
- int ret;
+ int err;
+
+ if (device->browse)
+ return -EBUSY;

adapter_get_address(adapter, &src);

@@ -1586,34 +1612,11 @@ static struct browse_req *browse_sdp(struct btd_device *device, uuid_t *search,
cb = browse_cb;
}

- ret = bt_search_service(&src, &device->bdaddr, &uuid, cb, req, NULL);
- if (ret < 0) {
+ err = bt_search_service(&src, &device->bdaddr, &uuid, cb, req, NULL);
+ if (err < 0) {
browse_request_free(req);
- if (err)
- *err = ret;
-
- return NULL;
- }
-
- return req;
-}
-
-int device_browse(struct btd_device *device, DBusConnection *conn,
- DBusMessage *msg, uuid_t *search, gboolean reverse)
-{
- struct browse_req *req;
- int err = 0;
-
- if (device->browse)
- return -EBUSY;
-
- if (device->type == DEVICE_TYPE_LE)
- req = browse_primary(device, &err);
- else
- req = browse_sdp(device, search, reverse, &err);
-
- if (req == NULL)
return err;
+ }

if (conn == NULL)
conn = get_dbus_connection();
@@ -1716,7 +1719,7 @@ static gboolean start_discovery(gpointer user_data)
{
struct btd_device *device = user_data;

- device_browse(device, NULL, NULL, NULL, TRUE);
+ device_browse_sdp(device, NULL, NULL, NULL, TRUE);

device->discov_timer = 0;

@@ -2053,7 +2056,7 @@ void device_bonding_complete(struct btd_device *device, uint8_t status)
device->discov_timer = 0;
}

- device_browse(device, bonding->conn, bonding->msg,
+ device_browse_sdp(device, bonding->conn, bonding->msg,
NULL, FALSE);

bonding_request_free(bonding);
diff --git a/src/device.h b/src/device.h
index 74b968c..9c645df 100644
--- a/src/device.h
+++ b/src/device.h
@@ -46,9 +46,12 @@ struct btd_device *device_create(DBusConnection *conn,
const gchar *address, device_type_t type);
void device_set_name(struct btd_device *device, const char *name);
void device_get_name(struct btd_device *device, char *name, size_t len);
+device_type_t device_get_type(struct btd_device *device);
void device_remove(struct btd_device *device, gboolean remove_stored);
gint device_address_cmp(struct btd_device *device, const gchar *address);
-int device_browse(struct btd_device *device, DBusConnection *conn,
+int device_browse_primary(struct btd_device *device, DBusConnection *conn,
+ DBusMessage *msg, gboolean secure);
+int device_browse_sdp(struct btd_device *device, DBusConnection *conn,
DBusMessage *msg, uuid_t *search, gboolean reverse);
void device_probe_drivers(struct btd_device *device, GSList *profiles);
const sdp_record_t *btd_device_get_record(struct btd_device *device,
diff --git a/src/glib-helper.c b/src/glib-helper.c
index c440e60..459a21c 100644
--- a/src/glib-helper.c
+++ b/src/glib-helper.c
@@ -520,9 +520,11 @@ static void connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)

int bt_discover_primary(const bdaddr_t *src, const bdaddr_t *dst, int psm,
bt_primary_t cb, void *user_data,
+ gboolean secure,
bt_destroy_t destroy)
{
struct gattrib_context *ctxt;
+ BtIOSecLevel sec_level;
GIOChannel *io;
GError *gerr = NULL;

@@ -536,19 +538,25 @@ int bt_discover_primary(const bdaddr_t *src, const bdaddr_t *dst, int psm,
ctxt->cb = cb;
ctxt->destroy = destroy;

+ if (secure == TRUE)
+ sec_level = BT_IO_SEC_HIGH;
+ else
+ sec_level = BT_IO_SEC_LOW;
+
+
if (psm < 0)
io = bt_io_connect(BT_IO_L2CAP, connect_cb, ctxt, NULL, &gerr,
BT_IO_OPT_SOURCE_BDADDR, src,
BT_IO_OPT_DEST_BDADDR, dst,
BT_IO_OPT_CID, GATT_CID,
- BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_LOW,
+ BT_IO_OPT_SEC_LEVEL, sec_level,
BT_IO_OPT_INVALID);
else
io = bt_io_connect(BT_IO_L2CAP, connect_cb, ctxt, NULL, &gerr,
BT_IO_OPT_SOURCE_BDADDR, src,
BT_IO_OPT_DEST_BDADDR, dst,
BT_IO_OPT_PSM, psm,
- BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_LOW,
+ BT_IO_OPT_SEC_LEVEL, sec_level,
BT_IO_OPT_INVALID);

if (io == NULL) {
diff --git a/src/glib-helper.h b/src/glib-helper.h
index 10718bb..17739cf 100644
--- a/src/glib-helper.h
+++ b/src/glib-helper.h
@@ -35,6 +35,7 @@ int bt_cancel_discovery(const bdaddr_t *src, const bdaddr_t *dst);

int bt_discover_primary(const bdaddr_t *src, const bdaddr_t *dst, int psm,
bt_primary_t cb, void *user_data,
+ gboolean secure,
bt_destroy_t destroy);

gchar *bt_uuid2string(uuid_t *uuid);
--
1.7.3.2

2010-12-21 12:47:09

by Johan Hedberg

Subject: Re: [PATCH v3] Remove unneeded variable

Hi,

On Tue, Dec 21, 2010, Anderson Lizardo wrote:
> From: Claudio Takahasi <[email protected]>
>
> ---
> src/glib-helper.c | 5 ++---
> 1 files changed, 2 insertions(+), 3 deletions(-)

Pushed upstream. Thanks.

Johan

2010-12-21 12:03:54

by Anderson Lizardo

Subject: Re: [PATCH v2 5/5] Remove unneeded variable

On Mon, Dec 20, 2010 at 7:02 PM, Johan Hedberg <[email protected]> wrote:
> Hi Claudio,
>
> On Mon, Dec 20, 2010, Claudio Takahasi wrote:
>> ---
>> ?src/glib-helper.c | ? ?5 ++---
>> ?1 files changed, 2 insertions(+), 3 deletions(-)
>
> This patch seems fine but it doesn't apply (probably due to me not
> applying v2 2/5).

Just sent a rebased one that should apply to master branch.

Regards,
--
Anderson Lizardo
OpenBossa Labs - INdT
Manaus - Brazil

2010-12-21 12:02:48

by Anderson Lizardo

Subject: [PATCH v3] Remove unneeded variable

From: Claudio Takahasi <[email protected]>

---
src/glib-helper.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/glib-helper.c b/src/glib-helper.c
index 0233eb7..bf39a83 100644
--- a/src/glib-helper.c
+++ b/src/glib-helper.c
@@ -514,7 +514,6 @@ int bt_discover_primary(const bdaddr_t *src, const bdaddr_t *dst, int psm,
{
struct gattrib_context *ctxt;
GIOChannel *io;
- GError *gerr = NULL;

ctxt = g_try_new0(struct gattrib_context, 1);
if (ctxt == NULL)
@@ -527,14 +526,14 @@ int bt_discover_primary(const bdaddr_t *src, const bdaddr_t *dst, int psm,
ctxt->destroy = destroy;

if (psm < 0)
- io = bt_io_connect(BT_IO_L2CAP, connect_cb, ctxt, NULL, &gerr,
+ io = bt_io_connect(BT_IO_L2CAP, connect_cb, ctxt, NULL, NULL,
BT_IO_OPT_SOURCE_BDADDR, src,
BT_IO_OPT_DEST_BDADDR, dst,
BT_IO_OPT_CID, GATT_CID,
BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_LOW,
BT_IO_OPT_INVALID);
else
- io = bt_io_connect(BT_IO_L2CAP, connect_cb, ctxt, NULL, &gerr,
+ io = bt_io_connect(BT_IO_L2CAP, connect_cb, ctxt, NULL, NULL,
BT_IO_OPT_SOURCE_BDADDR, src,
BT_IO_OPT_DEST_BDADDR, dst,
BT_IO_OPT_PSM, psm,
--
1.7.0.4

2010-12-21 08:42:43

by Johan Hedberg

Subject: Re: [PATCH v3 2/5] Change CreatePairedDevice to support LE devices

Hi Sheldon,

On Mon, Dec 20, 2010, Sheldon Demario wrote:
> +static struct btd_device *create_device_internal(DBusConnection *conn,
> + struct btd_adapter *adapter,
> + const gchar *address,
> + gboolean secure, int *err)
> +{
> + struct remote_dev_info *dev, match;
> + struct btd_device *device;
> + device_type_t type;
> +
> + memset(&match, 0, sizeof(struct remote_dev_info));
> + str2ba(address, &match.bdaddr);
> + match.name_status = NAME_ANY;
> +
> + dev = adapter_search_found_devices(adapter, &match);
> + if (dev && dev->flags)
> + type = flags2type(dev->flags);
> + else
> + type = DEVICE_TYPE_BREDR;
> +
> + if (!secure && type == DEVICE_TYPE_LE &&
> + !event_is_connectable(dev->evt_type)) {

I think you got something mixed up here. I asked you to change the
sec_level_high variable occurences to secure. Here you've gone and
changed the force variable to secure (something that I didn't request).

I.e. please use "force" like before (since that describes its purpose
within the create_device_internal function more intuitively) and change
the following:

> +int device_browse_primary(struct btd_device *device, DBusConnection *conn,
> + DBusMessage *msg, gboolean sec_level_high)

> +int device_browse_primary(struct btd_device *device, DBusConnection *conn,
> + DBusMessage *msg, gboolean sec_level_high);

> int bt_discover_primary(const bdaddr_t *src, const bdaddr_t *dst, int psm,
> bt_primary_t cb, void *user_data,
> + gboolean sec_level_high,
> bt_destroy_t destroy)

> int bt_discover_primary(const bdaddr_t *src, const bdaddr_t *dst, int psm,
> bt_primary_t cb, void *user_data,
> + gboolean sec_level_high,
> bt_destroy_t destroy);

Johan

2010-12-20 23:57:14

by Sheldon Demario

Subject: [PATCH v3 2/5] Change CreatePairedDevice to support LE devices

CreatePairedDevice implements now the same behaviour of CreateDevice,
triggering Discover All Primary Services when needed. SMP negotiation
starts when the link is established. LE capable kernel is required to
test this method properly.

Limitation: For dual mode devices, Discover All Primary Services is not
being executed after SDP search if GATT record is found.
---
src/adapter.c | 102 +++++++++++++++++++++++++++++++++++++----------------
src/device.c | 89 ++++++++++++++++++++++++----------------------
src/device.h | 5 ++-
src/glib-helper.c | 12 +++++-
src/glib-helper.h | 1 +
5 files changed, 132 insertions(+), 77 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index cf3566d..c0fe4c0 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -1510,15 +1510,48 @@ static gboolean event_is_connectable(uint8_t type)
}
}

+static struct btd_device *create_device_internal(DBusConnection *conn,
+ struct btd_adapter *adapter,
+ const gchar *address,
+ gboolean secure, int *err)
+{
+ struct remote_dev_info *dev, match;
+ struct btd_device *device;
+ device_type_t type;
+
+ memset(&match, 0, sizeof(struct remote_dev_info));
+ str2ba(address, &match.bdaddr);
+ match.name_status = NAME_ANY;
+
+ dev = adapter_search_found_devices(adapter, &match);
+ if (dev && dev->flags)
+ type = flags2type(dev->flags);
+ else
+ type = DEVICE_TYPE_BREDR;
+
+ if (!secure && type == DEVICE_TYPE_LE &&
+ !event_is_connectable(dev->evt_type)) {
+ if (err)
+ *err = -ENOTCONN;
+
+ return NULL;
+ }
+
+ device = adapter_create_device(conn, adapter, address, type);
+ if (!device && err)
+ *err = -ENOMEM;
+
+ return device;
+}
+
static DBusMessage *create_device(DBusConnection *conn,
DBusMessage *msg, void *data)
{
struct btd_adapter *adapter = data;
struct btd_device *device;
- struct remote_dev_info *dev, match;
const gchar *address;
+ DBusMessage *reply;
int err;
- device_type_t type;

if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &address,
DBUS_TYPE_INVALID) == FALSE)
@@ -1535,41 +1568,36 @@ static DBusMessage *create_device(DBusConnection *conn,

DBG("%s", address);

- memset(&match, 0, sizeof(struct remote_dev_info));
- str2ba(address, &match.bdaddr);
- match.name_status = NAME_ANY;
+ device = create_device_internal(conn, adapter, address, TRUE, &err);
+ if (!device)
+ goto failed;

- dev = adapter_search_found_devices(adapter, &match);
- if (dev && dev->flags)
- type = flags2type(dev->flags);
+ if (device_get_type(device) != DEVICE_TYPE_LE)
+ err = device_browse_sdp(device, conn, msg, NULL, FALSE);
else
- type = DEVICE_TYPE_BREDR;
+ err = device_browse_primary(device, conn, msg, FALSE);

- device = adapter_create_device(conn, adapter, address, type);
- if (!device)
- return NULL;
+ if (err < 0) {
+ adapter_remove_device(conn, adapter, device, TRUE);
+ return btd_error_failed(msg, strerror(-err));
+ }

- if (type == DEVICE_TYPE_LE && !event_is_connectable(dev->evt_type)) {
+ return NULL;
+
+failed:
+ if (err == -ENOTCONN) {
/* Device is not connectable */
const char *path = device_get_path(device);
- DBusMessage *reply;

reply = dbus_message_new_method_return(msg);

dbus_message_append_args(reply,
- DBUS_TYPE_OBJECT_PATH, &path,
- DBUS_TYPE_INVALID);
-
- return reply;
- }
-
- err = device_browse(device, conn, msg, NULL, FALSE);
- if (err < 0) {
- adapter_remove_device(conn, adapter, device, TRUE);
- return btd_error_failed(msg, strerror(-err));
- }
+ DBUS_TYPE_OBJECT_PATH, &path,
+ DBUS_TYPE_INVALID);
+ } else
+ reply = btd_error_failed(msg, strerror(-err));

- return NULL;
+ return reply;
}

static uint8_t parse_io_capability(const char *capability)
@@ -1594,6 +1622,7 @@ static DBusMessage *create_paired_device(DBusConnection *conn,
struct btd_device *device;
const gchar *address, *agent_path, *capability, *sender;
uint8_t cap;
+ int err;

if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &address,
DBUS_TYPE_OBJECT_PATH, &agent_path,
@@ -1618,12 +1647,23 @@ static DBusMessage *create_paired_device(DBusConnection *conn,
if (cap == IO_CAPABILITY_INVALID)
return btd_error_invalid_args(msg);

- device = adapter_get_device(conn, adapter, address);
- if (!device)
- return btd_error_failed(msg,
- "Unable to create a new device object");
+ device = adapter_find_device(adapter, address);
+ if (!device) {
+ device = create_device_internal(conn, adapter, address,
+ FALSE, &err);
+ if (!device)
+ return btd_error_failed(msg, strerror(-err));
+ }
+
+ if (device_get_type(device) != DEVICE_TYPE_LE)
+ return device_create_bonding(device, conn, msg,
+ agent_path, cap);

- return device_create_bonding(device, conn, msg, agent_path, cap);
+ err = device_browse_primary(device, conn, msg, TRUE);
+ if (err < 0)
+ return btd_error_failed(msg, strerror(-err));
+
+ return NULL;
}

static gint device_path_cmp(struct btd_device *device, const gchar *path)
diff --git a/src/device.c b/src/device.c
index bef8e71..33b09eb 100644
--- a/src/device.c
+++ b/src/device.c
@@ -583,7 +583,7 @@ static DBusMessage *discover_services(DBusConnection *conn,
return btd_error_invalid_args(msg);

if (strlen(pattern) == 0) {
- err = device_browse(device, conn, msg, NULL, FALSE);
+ err = device_browse_sdp(device, conn, msg, NULL, FALSE);
if (err < 0)
goto fail;
} else {
@@ -594,7 +594,7 @@ static DBusMessage *discover_services(DBusConnection *conn,

sdp_uuid128_to_uuid(&uuid);

- err = device_browse(device, conn, msg, &uuid, FALSE);
+ err = device_browse_sdp(device, conn, msg, &uuid, FALSE);
if (err < 0)
goto fail;
}
@@ -985,6 +985,11 @@ void device_get_name(struct btd_device *device, char *name, size_t len)
strncpy(name, device->name, len);
}

+device_type_t device_get_type(struct btd_device *device)
+{
+ return device->type;
+}
+
void device_remove_bonding(struct btd_device *device)
{
char filename[PATH_MAX + 1];
@@ -1537,41 +1542,62 @@ done:
browse_request_free(req);
}

-static struct browse_req *browse_primary(struct btd_device *device, int *err)
+int device_browse_primary(struct btd_device *device, DBusConnection *conn,
+ DBusMessage *msg, gboolean sec_level_high)
{
struct btd_adapter *adapter = device->adapter;
struct browse_req *req;
bdaddr_t src;
- int ret;
+ int err;
+
+ if (device->browse)
+ return -EBUSY;

req = g_new0(struct browse_req, 1);
req->device = btd_device_ref(device);

adapter_get_address(adapter, &src);

- ret = bt_discover_primary(&src, &device->bdaddr, -1, primary_cb, req,
- NULL);
-
- if (ret < 0) {
+ err = bt_discover_primary(&src, &device->bdaddr, -1, primary_cb, req,
+ sec_level_high, NULL);
+ if (err < 0) {
browse_request_free(req);
- if (err)
- *err = ret;
+ return err;
+ }

- return NULL;
+ if (conn == NULL)
+ conn = get_dbus_connection();
+
+ req->conn = dbus_connection_ref(conn);
+ device->browse = req;
+
+ if (msg) {
+ const char *sender = dbus_message_get_sender(msg);
+
+ req->msg = dbus_message_ref(msg);
+ /* Track the request owner to cancel it
+ * automatically if the owner exits */
+ req->listener_id = g_dbus_add_disconnect_watch(conn,
+ sender,
+ discover_services_req_exit,
+ req, NULL);
}

- return req;
+ return err;
}

-static struct browse_req *browse_sdp(struct btd_device *device, uuid_t *search,
- gboolean reverse, int *err)
+int device_browse_sdp(struct btd_device *device, DBusConnection *conn,
+ DBusMessage *msg, uuid_t *search, gboolean reverse)
{
struct btd_adapter *adapter = device->adapter;
struct browse_req *req;
bt_callback_t cb;
bdaddr_t src;
uuid_t uuid;
- int ret;
+ int err;
+
+ if (device->browse)
+ return -EBUSY;

adapter_get_address(adapter, &src);

@@ -1586,34 +1612,11 @@ static struct browse_req *browse_sdp(struct btd_device *device, uuid_t *search,
cb = browse_cb;
}

- ret = bt_search_service(&src, &device->bdaddr, &uuid, cb, req, NULL);
- if (ret < 0) {
+ err = bt_search_service(&src, &device->bdaddr, &uuid, cb, req, NULL);
+ if (err < 0) {
browse_request_free(req);
- if (err)
- *err = ret;
-
- return NULL;
- }
-
- return req;
-}
-
-int device_browse(struct btd_device *device, DBusConnection *conn,
- DBusMessage *msg, uuid_t *search, gboolean reverse)
-{
- struct browse_req *req;
- int err = 0;
-
- if (device->browse)
- return -EBUSY;
-
- if (device->type == DEVICE_TYPE_LE)
- req = browse_primary(device, &err);
- else
- req = browse_sdp(device, search, reverse, &err);
-
- if (req == NULL)
return err;
+ }

if (conn == NULL)
conn = get_dbus_connection();
@@ -1716,7 +1719,7 @@ static gboolean start_discovery(gpointer user_data)
{
struct btd_device *device = user_data;

- device_browse(device, NULL, NULL, NULL, TRUE);
+ device_browse_sdp(device, NULL, NULL, NULL, TRUE);

device->discov_timer = 0;

@@ -2053,7 +2056,7 @@ void device_bonding_complete(struct btd_device *device, uint8_t status)
device->discov_timer = 0;
}

- device_browse(device, bonding->conn, bonding->msg,
+ device_browse_sdp(device, bonding->conn, bonding->msg,
NULL, FALSE);

bonding_request_free(bonding);
diff --git a/src/device.h b/src/device.h
index 74b968c..947de95 100644
--- a/src/device.h
+++ b/src/device.h
@@ -46,9 +46,12 @@ struct btd_device *device_create(DBusConnection *conn,
const gchar *address, device_type_t type);
void device_set_name(struct btd_device *device, const char *name);
void device_get_name(struct btd_device *device, char *name, size_t len);
+device_type_t device_get_type(struct btd_device *device);
void device_remove(struct btd_device *device, gboolean remove_stored);
gint device_address_cmp(struct btd_device *device, const gchar *address);
-int device_browse(struct btd_device *device, DBusConnection *conn,
+int device_browse_primary(struct btd_device *device, DBusConnection *conn,
+ DBusMessage *msg, gboolean sec_level_high);
+int device_browse_sdp(struct btd_device *device, DBusConnection *conn,
DBusMessage *msg, uuid_t *search, gboolean reverse);
void device_probe_drivers(struct btd_device *device, GSList *profiles);
const sdp_record_t *btd_device_get_record(struct btd_device *device,
diff --git a/src/glib-helper.c b/src/glib-helper.c
index c440e60..6ab288a 100644
--- a/src/glib-helper.c
+++ b/src/glib-helper.c
@@ -520,9 +520,11 @@ static void connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)

int bt_discover_primary(const bdaddr_t *src, const bdaddr_t *dst, int psm,
bt_primary_t cb, void *user_data,
+ gboolean sec_level_high,
bt_destroy_t destroy)
{
struct gattrib_context *ctxt;
+ BtIOSecLevel sec_level;
GIOChannel *io;
GError *gerr = NULL;

@@ -536,19 +538,25 @@ int bt_discover_primary(const bdaddr_t *src, const bdaddr_t *dst, int psm,
ctxt->cb = cb;
ctxt->destroy = destroy;

+ if (sec_level_high == TRUE)
+ sec_level = BT_IO_SEC_HIGH;
+ else
+ sec_level = BT_IO_SEC_LOW;
+
+
if (psm < 0)
io = bt_io_connect(BT_IO_L2CAP, connect_cb, ctxt, NULL, &gerr,
BT_IO_OPT_SOURCE_BDADDR, src,
BT_IO_OPT_DEST_BDADDR, dst,
BT_IO_OPT_CID, GATT_CID,
- BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_LOW,
+ BT_IO_OPT_SEC_LEVEL, sec_level,
BT_IO_OPT_INVALID);
else
io = bt_io_connect(BT_IO_L2CAP, connect_cb, ctxt, NULL, &gerr,
BT_IO_OPT_SOURCE_BDADDR, src,
BT_IO_OPT_DEST_BDADDR, dst,
BT_IO_OPT_PSM, psm,
- BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_LOW,
+ BT_IO_OPT_SEC_LEVEL, sec_level,
BT_IO_OPT_INVALID);

if (io == NULL) {
diff --git a/src/glib-helper.h b/src/glib-helper.h
index 10718bb..9cac915 100644
--- a/src/glib-helper.h
+++ b/src/glib-helper.h
@@ -35,6 +35,7 @@ int bt_cancel_discovery(const bdaddr_t *src, const bdaddr_t *dst);

int bt_discover_primary(const bdaddr_t *src, const bdaddr_t *dst, int psm,
bt_primary_t cb, void *user_data,
+ gboolean sec_level_high,
bt_destroy_t destroy);

gchar *bt_uuid2string(uuid_t *uuid);
--
1.7.3.2

2010-12-20 23:02:27

by Johan Hedberg

Subject: Re: [PATCH v2 5/5] Remove unneeded variable

Hi Claudio,

On Mon, Dec 20, 2010, Claudio Takahasi wrote:
> ---
> src/glib-helper.c | 5 ++---
> 1 files changed, 2 insertions(+), 3 deletions(-)

This patch seems fine but it doesn't apply (probably due to me not
applying v2 2/5).

Johan

2010-12-20 23:00:58

by Johan Hedberg

Subject: Re: [PATCH v2 4/5] Fix missing reply when create device is cancelled

Hi Claudio,

On Mon, Dec 20, 2010, Claudio Takahasi wrote:
> When CancelDeviceCreation is called or when the device is removed for
> any reason, the reply for the pending CreateDevice is not sent.
> ---
> src/device.c | 8 ++++++--
> 1 files changed, 6 insertions(+), 2 deletions(-)

Pushed upstream. Thanks.

Johan

2010-12-20 22:59:09

by Johan Hedberg

Subject: Re: [PATCH v2 2/5] Change CreatePairedDevice to support LE devices

Hi,

> +static struct btd_device *create_common(DBusConnection *conn,

I don't really like this name since it doesn't explain what the function
does in any way except that it creates "something". Could you call it
e.g. __create_device() or create_device_internal()?

> + if (type == DEVICE_TYPE_LE && !event_is_connectable(dev->evt_type)) {
> + if (force)
> + goto create;
> +
> + if (err)
> + *err = -ENOTCONN;
> +
> + return NULL;
> + }
> +
> +create:

You could avoid the label and goto by simply adding if (!force && type...
to the beginning of the if-statement.

> +int device_browse_primary(struct btd_device *device, DBusConnection *conn,
> + DBusMessage *msg, gboolean sec_level_high)

Could you call the gboolean parameter just "secure"?

Johan

2010-12-20 22:47:35

by Claudio Takahasi

Subject: [PATCH v2 4/5] Fix missing reply when create device is cancelled

When CancelDeviceCreation is called or when the device is removed for
any reason, the reply for the pending CreateDevice is not sent.
---
src/device.c | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/device.c b/src/device.c
index 33b09eb..b7c7ddf 100644
--- a/src/device.c
+++ b/src/device.c
@@ -727,8 +727,10 @@ void device_request_disconnect(struct btd_device *device, DBusMessage *msg)
if (device->bonding)
bonding_request_cancel(device->bonding);

- if (device->browse)
+ if (device->browse) {
+ discover_services_reply(device->browse, -ECANCELED, NULL);
browse_request_cancel(device->browse);
+ }

if (msg)
device->disconnects = g_slist_append(device->disconnects,
@@ -1040,8 +1042,10 @@ void device_remove(struct btd_device *device, gboolean remove_stored)
if (device->bonding)
device_cancel_bonding(device, HCI_OE_USER_ENDED_CONNECTION);

- if (device->browse)
+ if (device->browse) {
+ discover_services_reply(device->browse, -ECANCELED, NULL);
browse_request_cancel(device->browse);
+ }

if (device->handle)
do_disconnect(device);
--
1.7.3.3

2010-12-20 22:49:48

by Claudio Takahasi

Subject: [PATCH v2 5/5] Remove unneeded variable

---
src/glib-helper.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/glib-helper.c b/src/glib-helper.c
index 6ab288a..aa7f2c7 100644
--- a/src/glib-helper.c
+++ b/src/glib-helper.c
@@ -526,7 +526,6 @@ int bt_discover_primary(const bdaddr_t *src, const bdaddr_t *dst, int psm,
struct gattrib_context *ctxt;
BtIOSecLevel sec_level;
GIOChannel *io;
- GError *gerr = NULL;

ctxt = g_try_new0(struct gattrib_context, 1);
if (ctxt == NULL)
@@ -545,14 +544,14 @@ int bt_discover_primary(const bdaddr_t *src, const bdaddr_t *dst, int psm,


if (psm < 0)
- io = bt_io_connect(BT_IO_L2CAP, connect_cb, ctxt, NULL, &gerr,
+ io = bt_io_connect(BT_IO_L2CAP, connect_cb, ctxt, NULL, NULL,
BT_IO_OPT_SOURCE_BDADDR, src,
BT_IO_OPT_DEST_BDADDR, dst,
BT_IO_OPT_CID, GATT_CID,
BT_IO_OPT_SEC_LEVEL, sec_level,
BT_IO_OPT_INVALID);
else
- io = bt_io_connect(BT_IO_L2CAP, connect_cb, ctxt, NULL, &gerr,
+ io = bt_io_connect(BT_IO_L2CAP, connect_cb, ctxt, NULL, NULL,
BT_IO_OPT_SOURCE_BDADDR, src,
BT_IO_OPT_DEST_BDADDR, dst,
BT_IO_OPT_PSM, psm,
--
1.7.3.3

2010-12-20 22:45:38

by Claudio Takahasi

Subject: Re: [PATCH 3/5] Return an error if adapter_create_device fails

Hi Johan,

On Wed, Dec 15, 2010 at 4:54 PM, Claudio Takahasi
<[email protected]> wrote:
> This function can fail if the device object is already registered or if
> the system doesn't have enough memory. Issue detected due wrong device
> object reference counting that doesn't unregister the D-Bus object path
> when the device is removed.
> ---
>  src/adapter.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/src/adapter.c b/src/adapter.c
> index c1fddce..f71d063 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -1595,7 +1595,8 @@ static DBusMessage *create_device(DBusConnection *conn,
>
>        device = adapter_create_device(conn, adapter, address, type);
>        if (!device)
> -               return NULL;
> +               return btd_error_failed(msg,
> +                               "Unable to create a new device object");
>
>        if (type == DEVICE_TYPE_LE && !event_is_connectable(dev->evt_type)) {
>                /* Device is not connectable */
> @@ -1693,7 +1694,8 @@ static DBusMessage *create_paired_device(DBusConnection *conn,
>
>                device = adapter_create_device(conn, adapter, address, type);
>                if (!device)
> -                       return NULL;
> +                       return btd_error_failed(msg,
> +                               "Unable to create a new device object");
>        } else
>                type = device_get_type(device);
>
> --
> 1.7.3.3
>
>

This patch is not necessary anymore. Please ignore it!

Claudio

2010-12-20 22:43:41

by Claudio Takahasi

Subject: [PATCH v2 2/5] Change CreatePairedDevice to support LE devices

From: Sheldon Demario <[email protected]>

CreatePairedDevice implements now the same behaviour of CreateDevice,
triggering Discover All Primary Services when needed. SMP negotiation
starts when the link is established. LE capable kernel is required to
test this method properly.

Limitation: For dual mode devices, Discover All Primary Services is not
being executed after SDP search if GATT record is found.
---
src/adapter.c | 104 +++++++++++++++++++++++++++++++++++++----------------
src/device.c | 89 +++++++++++++++++++++++----------------------
src/device.h | 5 ++-
src/glib-helper.c | 12 +++++-
src/glib-helper.h | 1 +
5 files changed, 134 insertions(+), 77 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index cf3566d..047473b 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -1510,15 +1510,51 @@ static gboolean event_is_connectable(uint8_t type)
}
}

+static struct btd_device *create_common(DBusConnection *conn,
+ struct btd_adapter *adapter,
+ const gchar *address,
+ gboolean force, int *err)
+{
+ struct remote_dev_info *dev, match;
+ struct btd_device *device;
+ device_type_t type;
+
+ memset(&match, 0, sizeof(struct remote_dev_info));
+ str2ba(address, &match.bdaddr);
+ match.name_status = NAME_ANY;
+
+ dev = adapter_search_found_devices(adapter, &match);
+ if (dev && dev->flags)
+ type = flags2type(dev->flags);
+ else
+ type = DEVICE_TYPE_BREDR;
+
+ if (type == DEVICE_TYPE_LE && !event_is_connectable(dev->evt_type)) {
+ if (force)
+ goto create;
+
+ if (err)
+ *err = -ENOTCONN;
+
+ return NULL;
+ }
+
+create:
+ device = adapter_create_device(conn, adapter, address, type);
+ if (!device && err)
+ *err = -ENOMEM;
+
+ return device;
+}
+
static DBusMessage *create_device(DBusConnection *conn,
DBusMessage *msg, void *data)
{
struct btd_adapter *adapter = data;
struct btd_device *device;
- struct remote_dev_info *dev, match;
const gchar *address;
+ DBusMessage *reply;
int err;
- device_type_t type;

if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &address,
DBUS_TYPE_INVALID) == FALSE)
@@ -1535,41 +1571,36 @@ static DBusMessage *create_device(DBusConnection *conn,

DBG("%s", address);

- memset(&match, 0, sizeof(struct remote_dev_info));
- str2ba(address, &match.bdaddr);
- match.name_status = NAME_ANY;
+ device = create_common(conn, adapter, address, TRUE, &err);
+ if (!device)
+ goto failed;

- dev = adapter_search_found_devices(adapter, &match);
- if (dev && dev->flags)
- type = flags2type(dev->flags);
+ if (device_get_type(device) != DEVICE_TYPE_LE)
+ err = device_browse_sdp(device, conn, msg, NULL, FALSE);
else
- type = DEVICE_TYPE_BREDR;
+ err = device_browse_primary(device, conn, msg, FALSE);

- device = adapter_create_device(conn, adapter, address, type);
- if (!device)
- return NULL;
+ if (err < 0) {
+ adapter_remove_device(conn, adapter, device, TRUE);
+ return btd_error_failed(msg, strerror(-err));
+ }

- if (type == DEVICE_TYPE_LE && !event_is_connectable(dev->evt_type)) {
+ return NULL;
+
+failed:
+ if (err == -ENOTCONN) {
/* Device is not connectable */
const char *path = device_get_path(device);
- DBusMessage *reply;

reply = dbus_message_new_method_return(msg);

dbus_message_append_args(reply,
- DBUS_TYPE_OBJECT_PATH, &path,
- DBUS_TYPE_INVALID);
-
- return reply;
- }
-
- err = device_browse(device, conn, msg, NULL, FALSE);
- if (err < 0) {
- adapter_remove_device(conn, adapter, device, TRUE);
- return btd_error_failed(msg, strerror(-err));
- }
+ DBUS_TYPE_OBJECT_PATH, &path,
+ DBUS_TYPE_INVALID);
+ } else
+ reply = btd_error_failed(msg, strerror(-err));

- return NULL;
+ return reply;
}

static uint8_t parse_io_capability(const char *capability)
@@ -1594,6 +1625,7 @@ static DBusMessage *create_paired_device(DBusConnection *conn,
struct btd_device *device;
const gchar *address, *agent_path, *capability, *sender;
uint8_t cap;
+ int err;

if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &address,
DBUS_TYPE_OBJECT_PATH, &agent_path,
@@ -1618,12 +1650,22 @@ static DBusMessage *create_paired_device(DBusConnection *conn,
if (cap == IO_CAPABILITY_INVALID)
return btd_error_invalid_args(msg);

- device = adapter_get_device(conn, adapter, address);
- if (!device)
- return btd_error_failed(msg,
- "Unable to create a new device object");
+ device = adapter_find_device(adapter, address);
+ if (!device) {
+ device = create_common(conn, adapter, address, FALSE, &err);
+ if (!device)
+ return btd_error_failed(msg, strerror(-err));
+ }
+
+ if (device_get_type(device) != DEVICE_TYPE_LE)
+ return device_create_bonding(device, conn, msg,
+ agent_path, cap);

- return device_create_bonding(device, conn, msg, agent_path, cap);
+ err = device_browse_primary(device, conn, msg, TRUE);
+ if (err < 0)
+ return btd_error_failed(msg, strerror(-err));
+
+ return NULL;
}

static gint device_path_cmp(struct btd_device *device, const gchar *path)
diff --git a/src/device.c b/src/device.c
index bef8e71..33b09eb 100644
--- a/src/device.c
+++ b/src/device.c
@@ -583,7 +583,7 @@ static DBusMessage *discover_services(DBusConnection *conn,
return btd_error_invalid_args(msg);

if (strlen(pattern) == 0) {
- err = device_browse(device, conn, msg, NULL, FALSE);
+ err = device_browse_sdp(device, conn, msg, NULL, FALSE);
if (err < 0)
goto fail;
} else {
@@ -594,7 +594,7 @@ static DBusMessage *discover_services(DBusConnection *conn,

sdp_uuid128_to_uuid(&uuid);

- err = device_browse(device, conn, msg, &uuid, FALSE);
+ err = device_browse_sdp(device, conn, msg, &uuid, FALSE);
if (err < 0)
goto fail;
}
@@ -985,6 +985,11 @@ void device_get_name(struct btd_device *device, char *name, size_t len)
strncpy(name, device->name, len);
}

+device_type_t device_get_type(struct btd_device *device)
+{
+ return device->type;
+}
+
void device_remove_bonding(struct btd_device *device)
{
char filename[PATH_MAX + 1];
@@ -1537,41 +1542,62 @@ done:
browse_request_free(req);
}

-static struct browse_req *browse_primary(struct btd_device *device, int *err)
+int device_browse_primary(struct btd_device *device, DBusConnection *conn,
+ DBusMessage *msg, gboolean sec_level_high)
{
struct btd_adapter *adapter = device->adapter;
struct browse_req *req;
bdaddr_t src;
- int ret;
+ int err;
+
+ if (device->browse)
+ return -EBUSY;

req = g_new0(struct browse_req, 1);
req->device = btd_device_ref(device);

adapter_get_address(adapter, &src);

- ret = bt_discover_primary(&src, &device->bdaddr, -1, primary_cb, req,
- NULL);
-
- if (ret < 0) {
+ err = bt_discover_primary(&src, &device->bdaddr, -1, primary_cb, req,
+ sec_level_high, NULL);
+ if (err < 0) {
browse_request_free(req);
- if (err)
- *err = ret;
+ return err;
+ }

- return NULL;
+ if (conn == NULL)
+ conn = get_dbus_connection();
+
+ req->conn = dbus_connection_ref(conn);
+ device->browse = req;
+
+ if (msg) {
+ const char *sender = dbus_message_get_sender(msg);
+
+ req->msg = dbus_message_ref(msg);
+ /* Track the request owner to cancel it
+ * automatically if the owner exits */
+ req->listener_id = g_dbus_add_disconnect_watch(conn,
+ sender,
+ discover_services_req_exit,
+ req, NULL);
}

- return req;
+ return err;
}

-static struct browse_req *browse_sdp(struct btd_device *device, uuid_t *search,
- gboolean reverse, int *err)
+int device_browse_sdp(struct btd_device *device, DBusConnection *conn,
+ DBusMessage *msg, uuid_t *search, gboolean reverse)
{
struct btd_adapter *adapter = device->adapter;
struct browse_req *req;
bt_callback_t cb;
bdaddr_t src;
uuid_t uuid;
- int ret;
+ int err;
+
+ if (device->browse)
+ return -EBUSY;

adapter_get_address(adapter, &src);

@@ -1586,34 +1612,11 @@ static struct browse_req *browse_sdp(struct btd_device *device, uuid_t *search,
cb = browse_cb;
}

- ret = bt_search_service(&src, &device->bdaddr, &uuid, cb, req, NULL);
- if (ret < 0) {
+ err = bt_search_service(&src, &device->bdaddr, &uuid, cb, req, NULL);
+ if (err < 0) {
browse_request_free(req);
- if (err)
- *err = ret;
-
- return NULL;
- }
-
- return req;
-}
-
-int device_browse(struct btd_device *device, DBusConnection *conn,
- DBusMessage *msg, uuid_t *search, gboolean reverse)
-{
- struct browse_req *req;
- int err = 0;
-
- if (device->browse)
- return -EBUSY;
-
- if (device->type == DEVICE_TYPE_LE)
- req = browse_primary(device, &err);
- else
- req = browse_sdp(device, search, reverse, &err);
-
- if (req == NULL)
return err;
+ }

if (conn == NULL)
conn = get_dbus_connection();
@@ -1716,7 +1719,7 @@ static gboolean start_discovery(gpointer user_data)
{
struct btd_device *device = user_data;

- device_browse(device, NULL, NULL, NULL, TRUE);
+ device_browse_sdp(device, NULL, NULL, NULL, TRUE);

device->discov_timer = 0;

@@ -2053,7 +2056,7 @@ void device_bonding_complete(struct btd_device *device, uint8_t status)
device->discov_timer = 0;
}

- device_browse(device, bonding->conn, bonding->msg,
+ device_browse_sdp(device, bonding->conn, bonding->msg,
NULL, FALSE);

bonding_request_free(bonding);
diff --git a/src/device.h b/src/device.h
index 74b968c..947de95 100644
--- a/src/device.h
+++ b/src/device.h
@@ -46,9 +46,12 @@ struct btd_device *device_create(DBusConnection *conn,
const gchar *address, device_type_t type);
void device_set_name(struct btd_device *device, const char *name);
void device_get_name(struct btd_device *device, char *name, size_t len);
+device_type_t device_get_type(struct btd_device *device);
void device_remove(struct btd_device *device, gboolean remove_stored);
gint device_address_cmp(struct btd_device *device, const gchar *address);
-int device_browse(struct btd_device *device, DBusConnection *conn,
+int device_browse_primary(struct btd_device *device, DBusConnection *conn,
+ DBusMessage *msg, gboolean sec_level_high);
+int device_browse_sdp(struct btd_device *device, DBusConnection *conn,
DBusMessage *msg, uuid_t *search, gboolean reverse);
void device_probe_drivers(struct btd_device *device, GSList *profiles);
const sdp_record_t *btd_device_get_record(struct btd_device *device,
diff --git a/src/glib-helper.c b/src/glib-helper.c
index c440e60..6ab288a 100644
--- a/src/glib-helper.c
+++ b/src/glib-helper.c
@@ -520,9 +520,11 @@ static void connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)

int bt_discover_primary(const bdaddr_t *src, const bdaddr_t *dst, int psm,
bt_primary_t cb, void *user_data,
+ gboolean sec_level_high,
bt_destroy_t destroy)
{
struct gattrib_context *ctxt;
+ BtIOSecLevel sec_level;
GIOChannel *io;
GError *gerr = NULL;

@@ -536,19 +538,25 @@ int bt_discover_primary(const bdaddr_t *src, const bdaddr_t *dst, int psm,
ctxt->cb = cb;
ctxt->destroy = destroy;

+ if (sec_level_high == TRUE)
+ sec_level = BT_IO_SEC_HIGH;
+ else
+ sec_level = BT_IO_SEC_LOW;
+
+
if (psm < 0)
io = bt_io_connect(BT_IO_L2CAP, connect_cb, ctxt, NULL, &gerr,
BT_IO_OPT_SOURCE_BDADDR, src,
BT_IO_OPT_DEST_BDADDR, dst,
BT_IO_OPT_CID, GATT_CID,
- BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_LOW,
+ BT_IO_OPT_SEC_LEVEL, sec_level,
BT_IO_OPT_INVALID);
else
io = bt_io_connect(BT_IO_L2CAP, connect_cb, ctxt, NULL, &gerr,
BT_IO_OPT_SOURCE_BDADDR, src,
BT_IO_OPT_DEST_BDADDR, dst,
BT_IO_OPT_PSM, psm,
- BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_LOW,
+ BT_IO_OPT_SEC_LEVEL, sec_level,
BT_IO_OPT_INVALID);

if (io == NULL) {
diff --git a/src/glib-helper.h b/src/glib-helper.h
index 10718bb..9cac915 100644
--- a/src/glib-helper.h
+++ b/src/glib-helper.h
@@ -35,6 +35,7 @@ int bt_cancel_discovery(const bdaddr_t *src, const bdaddr_t *dst);

int bt_discover_primary(const bdaddr_t *src, const bdaddr_t *dst, int psm,
bt_primary_t cb, void *user_data,
+ gboolean sec_level_high,
bt_destroy_t destroy);

gchar *bt_uuid2string(uuid_t *uuid);
--
1.7.3.3

2010-12-20 14:03:40

by Claudio Takahasi

Subject: Re: [PATCH 2/5] Change CreatePairedDevice to support LE devices

Hi Brian,

On Thu, Dec 16, 2010 at 7:42 PM, Brian Gix <[email protected]> wrote:
> Hi Claudio,
>
> On Wed, 2010-12-15 at 16:54 -0300, Claudio Takahasi wrote:
>> From: Sheldon Demario <[email protected]>
>>
>> CreatePairedDevice implements now the same behaviour of CreateDevice,
>> triggering Discover All Primary Services when needed. SMP negotiation
>> starts when the link is established. LE capable kernel is required to
>> test this method properly.
>
> What is your plan for handling single mode LE (remote) devices which
> have privacy enabled (random addresses). This impacts connection
> establishment, and SM pairing, as the remote device's addr type must
> be known at that time, and handled differently.

At the moment, we don't have a clear picture how we will address
random address. For this first phase I was considering to support
public address only.

The kernel doesn't have persistent storage of the key, we will
implement it using the new kernel management interface.
We discussed the possibility of translate/resolve the address in the
kernel, but we didn't reach a conclusion yet.

Regards,
Claudio.

2010-12-16 22:42:47

by Brian Gix

Subject: Re: [PATCH 2/5] Change CreatePairedDevice to support LE devices

Hi Claudio,

On Wed, 2010-12-15 at 16:54 -0300, Claudio Takahasi wrote:
> From: Sheldon Demario <[email protected]>
>
> CreatePairedDevice implements now the same behaviour of CreateDevice,
> triggering Discover All Primary Services when needed. SMP negotiation
> starts when the link is established. LE capable kernel is required to
> test this method properly.

What is your plan for handling single mode LE (remote) devices which
have privacy enabled (random addresses). This impacts connection
establishment, and SM pairing, as the remote device's addr type must
be known at that time, and handled differently.

>
> Limitation: For dual mode devices, Discover All Primary Services is not
> being executed after SDP search if GATT record is found.
> ---
> src/adapter.c | 46 ++++++++++++++++++++++++---
> src/device.c | 89 +++++++++++++++++++++++++++-------------------------
> src/device.h | 7 +++-
> src/glib-helper.c | 5 ++-
> src/glib-helper.h | 3 ++
> 5 files changed, 98 insertions(+), 52 deletions(-)
>
> diff --git a/src/adapter.c b/src/adapter.c
> index 2ff59a0..c1fddce 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -1611,7 +1611,11 @@ static DBusMessage *create_device(DBusConnection *conn,
> return reply;
> }
>
> - err = device_browse(device, conn, msg, NULL, FALSE);
> + if (type != DEVICE_TYPE_LE)
> + err = device_browse_sdp(device, conn, msg, NULL, FALSE);
> + else
> + err = device_browse_primary(device, conn, msg, BT_IO_SEC_LOW);
> +
> if (err < 0) {
> adapter_remove_device(conn, adapter, device, TRUE);
> return btd_error_failed(msg, strerror(-err));
> @@ -1642,6 +1646,8 @@ static DBusMessage *create_paired_device(DBusConnection *conn,
> struct btd_device *device;
> const gchar *address, *agent_path, *capability, *sender;
> uint8_t cap;
> + device_type_t type;
> + int err;
>
> if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &address,
> DBUS_TYPE_OBJECT_PATH, &agent_path,
> @@ -1666,12 +1672,40 @@ static DBusMessage *create_paired_device(DBusConnection *conn,
> if (cap == IO_CAPABILITY_INVALID)
> return btd_error_invalid_args(msg);
>
> - device = adapter_get_device(conn, adapter, address);
> - if (!device)
> - return btd_error_failed(msg,
> - "Unable to create a new device object");
> + device = adapter_find_device(adapter, address);
> + if (!device) {
> + struct remote_dev_info *dev, match;
> +
> + memset(&match, 0, sizeof(struct remote_dev_info));
> + str2ba(address, &match.bdaddr);
> + match.name_status = NAME_ANY;
> +
> + dev = adapter_search_found_devices(adapter, &match);
> + if (dev && dev->flags)
> + type = flags2type(dev->flags);
> + else
> + type = DEVICE_TYPE_BREDR;
> +
> + if (type == DEVICE_TYPE_LE &&
> + !event_is_connectable(dev->evt_type))
> + return btd_error_failed(msg,
> + "Device is not connectable");
> +
> + device = adapter_create_device(conn, adapter, address, type);
> + if (!device)
> + return NULL;
> + } else
> + type = device_get_type(device);
> +
> + if (type != DEVICE_TYPE_LE)
> + return device_create_bonding(device, conn, msg,
> + agent_path, cap);
>
> - return device_create_bonding(device, conn, msg, agent_path, cap);
> + err = device_browse_primary(device, conn, msg, BT_IO_SEC_HIGH);
> + if (err < 0)
> + return btd_error_failed(msg, strerror(-err));
> +
> + return NULL;
> }
>
> static gint device_path_cmp(struct btd_device *device, const gchar *path)
> diff --git a/src/device.c b/src/device.c
> index d20a6d4..cf3b146 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -586,7 +586,7 @@ static DBusMessage *discover_services(DBusConnection *conn,
> return btd_error_invalid_args(msg);
>
> if (strlen(pattern) == 0) {
> - err = device_browse(device, conn, msg, NULL, FALSE);
> + err = device_browse_sdp(device, conn, msg, NULL, FALSE);
> if (err < 0)
> goto fail;
> } else {
> @@ -597,7 +597,7 @@ static DBusMessage *discover_services(DBusConnection *conn,
>
> sdp_uuid128_to_uuid(&uuid);
>
> - err = device_browse(device, conn, msg, &uuid, FALSE);
> + err = device_browse_sdp(device, conn, msg, &uuid, FALSE);
> if (err < 0)
> goto fail;
> }
> @@ -988,6 +988,11 @@ void device_get_name(struct btd_device *device, char *name, size_t len)
> strncpy(name, device->name, len);
> }
>
> +device_type_t device_get_type(struct btd_device *device)
> +{
> + return device->type;
> +}
> +
> void device_remove_bonding(struct btd_device *device)
> {
> char filename[PATH_MAX + 1];
> @@ -1540,41 +1545,62 @@ done:
> browse_request_free(req);
> }
>
> -static struct browse_req *browse_primary(struct btd_device *device, int *err)
> +int device_browse_primary(struct btd_device *device, DBusConnection *conn,
> + DBusMessage *msg, BtIOSecLevel sec_level)
> {
> struct btd_adapter *adapter = device->adapter;
> struct browse_req *req;
> bdaddr_t src;
> - int ret;
> + int err;
> +
> + if (device->browse)
> + return -EBUSY;
>
> req = g_new0(struct browse_req, 1);
> req->device = btd_device_ref(device);
>
> adapter_get_address(adapter, &src);
>
> - ret = bt_discover_primary(&src, &device->bdaddr, -1, primary_cb, req,
> - NULL);
> -
> - if (ret < 0) {
> + err = bt_discover_primary(&src, &device->bdaddr, -1, primary_cb, req,
> + sec_level, NULL);
> + if (err < 0) {
> browse_request_free(req);
> - if (err)
> - *err = ret;
> + return err;
> + }
>
> - return NULL;
> + if (conn == NULL)
> + conn = get_dbus_connection();
> +
> + req->conn = dbus_connection_ref(conn);
> + device->browse = req;
> +
> + if (msg) {
> + const char *sender = dbus_message_get_sender(msg);
> +
> + req->msg = dbus_message_ref(msg);
> + /* Track the request owner to cancel it
> + * automatically if the owner exits */
> + req->listener_id = g_dbus_add_disconnect_watch(conn,
> + sender,
> + discover_services_req_exit,
> + req, NULL);
> }
>
> - return req;
> + return err;
> }
>
> -static struct browse_req *browse_sdp(struct btd_device *device, uuid_t *search,
> - gboolean reverse, int *err)
> +int device_browse_sdp(struct btd_device *device, DBusConnection *conn,
> + DBusMessage *msg, uuid_t *search, gboolean reverse)
> {
> struct btd_adapter *adapter = device->adapter;
> struct browse_req *req;
> bt_callback_t cb;
> bdaddr_t src;
> uuid_t uuid;
> - int ret;
> + int err;
> +
> + if (device->browse)
> + return -EBUSY;
>
> adapter_get_address(adapter, &src);
>
> @@ -1589,34 +1615,11 @@ static struct browse_req *browse_sdp(struct btd_device *device, uuid_t *search,
> cb = browse_cb;
> }
>
> - ret = bt_search_service(&src, &device->bdaddr, &uuid, cb, req, NULL);
> - if (ret < 0) {
> + err = bt_search_service(&src, &device->bdaddr, &uuid, cb, req, NULL);
> + if (err < 0) {
> browse_request_free(req);
> - if (err)
> - *err = ret;
> -
> - return NULL;
> - }
> -
> - return req;
> -}
> -
> -int device_browse(struct btd_device *device, DBusConnection *conn,
> - DBusMessage *msg, uuid_t *search, gboolean reverse)
> -{
> - struct browse_req *req;
> - int err = 0;
> -
> - if (device->browse)
> - return -EBUSY;
> -
> - if (device->type == DEVICE_TYPE_LE)
> - req = browse_primary(device, &err);
> - else
> - req = browse_sdp(device, search, reverse, &err);
> -
> - if (req == NULL)
> return err;
> + }
>
> if (conn == NULL)
> conn = get_dbus_connection();
> @@ -1719,7 +1722,7 @@ static gboolean start_discovery(gpointer user_data)
> {
> struct btd_device *device = user_data;
>
> - device_browse(device, NULL, NULL, NULL, TRUE);
> + device_browse_sdp(device, NULL, NULL, NULL, TRUE);
>
> device->discov_timer = 0;
>
> @@ -2051,7 +2054,7 @@ void device_bonding_complete(struct btd_device *device, uint8_t status)
> device->discov_timer = 0;
> }
>
> - device_browse(device, bonding->conn, bonding->msg,
> + device_browse_sdp(device, bonding->conn, bonding->msg,
> NULL, FALSE);
>
> bonding_request_free(bonding);
> diff --git a/src/device.h b/src/device.h
> index 784e931..cafa529 100644
> --- a/src/device.h
> +++ b/src/device.h
> @@ -24,6 +24,8 @@
>
> #define DEVICE_INTERFACE "org.bluez.Device"
>
> +#include "btio.h"
> +
> struct btd_device;
>
> typedef enum {
> @@ -46,9 +48,12 @@ struct btd_device *device_create(DBusConnection *conn,
> const gchar *address, device_type_t type);
> void device_set_name(struct btd_device *device, const char *name);
> void device_get_name(struct btd_device *device, char *name, size_t len);
> +device_type_t device_get_type(struct btd_device *device);
> void device_remove(struct btd_device *device, gboolean remove_stored);
> gint device_address_cmp(struct btd_device *device, const gchar *address);
> -int device_browse(struct btd_device *device, DBusConnection *conn,
> +int device_browse_primary(struct btd_device *device, DBusConnection *conn,
> + DBusMessage *msg, BtIOSecLevel sec_level);
> +int device_browse_sdp(struct btd_device *device, DBusConnection *conn,
> DBusMessage *msg, uuid_t *search, gboolean reverse);
> void device_probe_drivers(struct btd_device *device, GSList *profiles);
> const sdp_record_t *btd_device_get_record(struct btd_device *device,
> diff --git a/src/glib-helper.c b/src/glib-helper.c
> index 6505249..edc46d8 100644
> --- a/src/glib-helper.c
> +++ b/src/glib-helper.c
> @@ -520,6 +520,7 @@ static void connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
>
> int bt_discover_primary(const bdaddr_t *src, const bdaddr_t *dst, int psm,
> bt_primary_t cb, void *user_data,
> + BtIOSecLevel sec_level,
> bt_destroy_t destroy)
> {
> struct gattrib_context *ctxt;
> @@ -541,14 +542,14 @@ int bt_discover_primary(const bdaddr_t *src, const bdaddr_t *dst, int psm,
> BT_IO_OPT_SOURCE_BDADDR, src,
> BT_IO_OPT_DEST_BDADDR, dst,
> BT_IO_OPT_CID, GATT_CID,
> - BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_LOW,
> + BT_IO_OPT_SEC_LEVEL, sec_level,
> BT_IO_OPT_INVALID);
> else
> io = bt_io_connect(BT_IO_L2CAP, connect_cb, ctxt, NULL, &gerr,
> BT_IO_OPT_SOURCE_BDADDR, src,
> BT_IO_OPT_DEST_BDADDR, dst,
> BT_IO_OPT_PSM, psm,
> - BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_LOW,
> + BT_IO_OPT_SEC_LEVEL, sec_level,
> BT_IO_OPT_INVALID);
>
> if (io == NULL) {
> diff --git a/src/glib-helper.h b/src/glib-helper.h
> index 5bb20a6..a16de6c 100644
> --- a/src/glib-helper.h
> +++ b/src/glib-helper.h
> @@ -21,6 +21,8 @@
> *
> */
>
> +#include "btio.h"
> +
> typedef void (*bt_callback_t) (sdp_list_t *recs, int err, gpointer user_data);
> typedef void (*bt_primary_t) (GSList *l, int err, gpointer user_data);
> typedef void (*bt_destroy_t) (gpointer user_data);
> @@ -35,6 +37,7 @@ int bt_cancel_discovery(const bdaddr_t *src, const bdaddr_t *dst);
>
> int bt_discover_primary(const bdaddr_t *src, const bdaddr_t *dst, int psm,
> bt_primary_t cb, void *user_data,
> + BtIOSecLevel sec_level,
> bt_destroy_t destroy);
>
> gchar *bt_uuid2string(uuid_t *uuid);

2010-12-16 21:01:11

by Johan Hedberg

Subject: Re: [PATCH 2/5] Change CreatePairedDevice to support LE devices

Hi Claudio,

On Thu, Dec 16, 2010, Claudio Takahasi wrote:
> Currently, the purpose are service search functions and UUIDs utility functions.
> glib-helper was originally created to implement some utility functions
> to manage connections and sdp search abstractions.
> Connection functions were moved/removed when btio was created. I have
> two suggestions to try cleanup the code:
> 1. keep only sdp functions that use GLib types and rename the file to
> gsdp or other convenient name
> 2. Or create a btd_device_search/cancel functions(moving them to
> device.c) and try to remove glib-helper from the source tree, in the
> worst case keep only functions to manipulate UUIDs
>
> The bt_discover_primary can be moved to gatt.c if we split the
> discover cancel function.
> bt_discover_services() can be removed, there isn't reference in code.
>
> Which approach do you prefer? Any other suggestion?

Considering that around 90% of the file is SDP stuff right now I think
option 1 sounds better. I.e. rename the file to something SDP related as
well as create some more appropriate namespace for its public functions
than bt_*

Johan

2010-12-16 17:16:26

by Claudio Takahasi

Subject: Re: [PATCH 2/5] Change CreatePairedDevice to support LE devices

Hi Johan,

On Thu, Dec 16, 2010 at 6:24 AM, Johan Hedberg <[email protected]> wrote:
> Hi,
>
> On Wed, Dec 15, 2010, Claudio Takahasi wrote:
>> CreatePairedDevice implements now the same behaviour of CreateDevice,
>> triggering Discover All Primary Services when needed. SMP negotiation
>> starts when the link is established. LE capable kernel is required to
>> test this method properly.
>>
>> Limitation: For dual mode devices, Discover All Primary Services is not
>> being executed after SDP search if GATT record is found.
>> ---
>>  src/adapter.c     |   46 ++++++++++++++++++++++++---
>>  src/device.c      |   89 +++++++++++++++++++++++++++-------------------------
>>  src/device.h      |    7 +++-
>>  src/glib-helper.c |    5 ++-
>>  src/glib-helper.h |    3 ++
>>  5 files changed, 98 insertions(+), 52 deletions(-)
>
> Couple of issue here:
>
>> @@ -1642,6 +1646,8 @@ static DBusMessage *create_paired_device(DBusConnection *conn,
>>       struct btd_device *device;
>>       const gchar *address, *agent_path, *capability, *sender;
>>       uint8_t cap;
>> +     device_type_t type;
>> +     int err;
>>
>>       if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &address,
>>                                       DBUS_TYPE_OBJECT_PATH, &agent_path,
>> @@ -1666,12 +1672,40 @@ static DBusMessage *create_paired_device(DBusConnection *conn,
>>       if (cap == IO_CAPABILITY_INVALID)
>>               return btd_error_invalid_args(msg);
>>
>> -     device = adapter_get_device(conn, adapter, address);
>> -     if (!device)
>> -             return btd_error_failed(msg,
>> -                             "Unable to create a new device object");
>> +     device = adapter_find_device(adapter, address);
>> +     if (!device) {
>> +             struct remote_dev_info *dev, match;
>> +
>> +             memset(&match, 0, sizeof(struct remote_dev_info));
>> +             str2ba(address, &match.bdaddr);
>> +             match.name_status = NAME_ANY;
>> +
>> +             dev = adapter_search_found_devices(adapter, &match);
>> +             if (dev && dev->flags)
>> +                     type = flags2type(dev->flags);
>> +             else
>> +                     type = DEVICE_TYPE_BREDR;
>> +
>> +             if (type == DEVICE_TYPE_LE &&
>> +                                     !event_is_connectable(dev->evt_type))
>> +                     return btd_error_failed(msg,
>> +                                     "Device is not connectable");
>> +
>> +             device = adapter_create_device(conn, adapter, address, type);
>> +             if (!device)
>> +                     return NULL;
>> +     } else
>> +             type = device_get_type(device);
>> +
>> +     if (type != DEVICE_TYPE_LE)
>> +             return device_create_bonding(device, conn, msg,
>> +                                                     agent_path, cap);
>>
>> -     return device_create_bonding(device, conn, msg, agent_path, cap);
>> +     err = device_browse_primary(device, conn, msg, BT_IO_SEC_HIGH);
>> +     if (err < 0)
>> +             return btd_error_failed(msg, strerror(-err));
>> +
>> +     return NULL;
>>  }
>
> I don't really like the way this makes the create_paired_device function
> quite long. Could you maybe refactor the if (!device) branch into a
> separate function?
ok. We will try. Maybe it will be possible to create a common function
to move shared code between CreateDevice and CreatePairedDevice.

>
>> diff --git a/src/device.h b/src/device.h
>> index 784e931..cafa529 100644
>> --- a/src/device.h
>> +++ b/src/device.h
>> @@ -24,6 +24,8 @@
>>
>>  #define DEVICE_INTERFACE     "org.bluez.Device"
>>
>> +#include "btio.h"
>> +
>
> Includes should be the first thing after the copyright/license comments
> in the file. However, to keep a clear visibility of potential circular
> dependencies Marcel has requested this kind of inclusion of an internal
> header file from within an internal header file to be avoided. Instead
> make sure you include btio.h from early enough in the respective .c
> file. You could also reconsider if you really need BtIOSecLevel here.
> Maybe a "gboolean secure" flag would be enough?
For now a boolean is enough, we can change it to boolean.

>
>> --- a/src/glib-helper.h
>> +++ b/src/glib-helper.h
>> @@ -21,6 +21,8 @@
>>   *
>>   */
>>
>> +#include "btio.h"
>> +
>
> Same here.
>
> I'm feeling a little bit ambivalent about your additions to
> glib-helper.c. It never had a clearly defined scope and I had been
> hoping to get rid of it completely. However now it seems you guys are
> constantly adding new stuff there. Is it really so that you can't find a
> more specific location for these functions? Could you describe in one or
> two sentences the purpose and scope that you think the glib-helper.c
> functions have?
>
> Johan
>

Currently, the purpose are service search functions and UUIDs utility functions.
glib-helper was originally created to implement some utility functions
to manage connections and sdp search abstractions.
Connection functions were moved/removed when btio was created. I have
two suggestions to try cleanup the code:
1. keep only sdp functions that use GLib types and rename the file to
gsdp or other convenient name
2. Or create a btd_device_search/cancel functions(moving them to
device.c) and try to remove glib-helper from the source tree, in the
worst case keep only functions to manipulate UUIDs

The bt_discover_primary can be moved to gatt.c if we split the
discover cancel function.
bt_discover_services() can be removed, there isn't reference in code.

Which approach do you prefer? Any other suggestion?

Regards,
Claudio

2010-12-16 09:24:32

by Johan Hedberg

Subject: Re: [PATCH 2/5] Change CreatePairedDevice to support LE devices

Hi,

On Wed, Dec 15, 2010, Claudio Takahasi wrote:
> CreatePairedDevice implements now the same behaviour of CreateDevice,
> triggering Discover All Primary Services when needed. SMP negotiation
> starts when the link is established. LE capable kernel is required to
> test this method properly.
>
> Limitation: For dual mode devices, Discover All Primary Services is not
> being executed after SDP search if GATT record is found.
> ---
> src/adapter.c | 46 ++++++++++++++++++++++++---
> src/device.c | 89 +++++++++++++++++++++++++++-------------------------
> src/device.h | 7 +++-
> src/glib-helper.c | 5 ++-
> src/glib-helper.h | 3 ++
> 5 files changed, 98 insertions(+), 52 deletions(-)

Couple of issue here:

> @@ -1642,6 +1646,8 @@ static DBusMessage *create_paired_device(DBusConnection *conn,
> struct btd_device *device;
> const gchar *address, *agent_path, *capability, *sender;
> uint8_t cap;
> + device_type_t type;
> + int err;
>
> if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &address,
> DBUS_TYPE_OBJECT_PATH, &agent_path,
> @@ -1666,12 +1672,40 @@ static DBusMessage *create_paired_device(DBusConnection *conn,
> if (cap == IO_CAPABILITY_INVALID)
> return btd_error_invalid_args(msg);
>
> - device = adapter_get_device(conn, adapter, address);
> - if (!device)
> - return btd_error_failed(msg,
> - "Unable to create a new device object");
> + device = adapter_find_device(adapter, address);
> + if (!device) {
> + struct remote_dev_info *dev, match;
> +
> + memset(&match, 0, sizeof(struct remote_dev_info));
> + str2ba(address, &match.bdaddr);
> + match.name_status = NAME_ANY;
> +
> + dev = adapter_search_found_devices(adapter, &match);
> + if (dev && dev->flags)
> + type = flags2type(dev->flags);
> + else
> + type = DEVICE_TYPE_BREDR;
> +
> + if (type == DEVICE_TYPE_LE &&
> + !event_is_connectable(dev->evt_type))
> + return btd_error_failed(msg,
> + "Device is not connectable");
> +
> + device = adapter_create_device(conn, adapter, address, type);
> + if (!device)
> + return NULL;
> + } else
> + type = device_get_type(device);
> +
> + if (type != DEVICE_TYPE_LE)
> + return device_create_bonding(device, conn, msg,
> + agent_path, cap);
>
> - return device_create_bonding(device, conn, msg, agent_path, cap);
> + err = device_browse_primary(device, conn, msg, BT_IO_SEC_HIGH);
> + if (err < 0)
> + return btd_error_failed(msg, strerror(-err));
> +
> + return NULL;
> }

I don't really like the way this makes the create_paired_device function
quite long. Could you maybe refactor the if (!device) branch into a
separate function?

> diff --git a/src/device.h b/src/device.h
> index 784e931..cafa529 100644
> --- a/src/device.h
> +++ b/src/device.h
> @@ -24,6 +24,8 @@
>
> #define DEVICE_INTERFACE "org.bluez.Device"
>
> +#include "btio.h"
> +

Includes should be the first thing after the copyright/license comments
in the file. However, to keep a clear visibility of potential circular
dependencies Marcel has requested this kind of inclusion of an internal
header file from within an internal header file to be avoided. Instead
make sure you include btio.h from early enough in the respective .c
file. You could also reconsider if you really need BtIOSecLevel here.
Maybe a "gboolean secure" flag would be enough?

> --- a/src/glib-helper.h
> +++ b/src/glib-helper.h
> @@ -21,6 +21,8 @@
> *
> */
>
> +#include "btio.h"
> +

Same here.

I'm feeling a little bit ambivalent about your additions to
glib-helper.c. It never had a clearly defined scope and I had been
hoping to get rid of it completely. However now it seems you guys are
constantly adding new stuff there. Is it really so that you can't find a
more specific location for these functions? Could you describe in one or
two sentences the purpose and scope that you think the glib-helper.c
functions have?

Johan

2010-12-16 09:15:26

by Johan Hedberg

Subject: Re: [PATCH 1/5] Implement cancel primary discovery session

Hi Claudio,

On Wed, Dec 15, 2010, Claudio Takahasi wrote:
> Extend bt_cancel_discovery function to cancel an ongoing Discover
> All Primary Services procedure.
> ---
> src/device.c | 11 +++----
> src/glib-helper.c | 73 ++++++++++++++++++++++++++++++++++++++++++-----------
> 2 files changed, 63 insertions(+), 21 deletions(-)

Thanks. This patch has been pushed upstream.

Johan

2010-12-15 19:54:12

by Claudio Takahasi

Subject: [PATCH 5/5] Remove unneeded variable

---
src/glib-helper.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/glib-helper.c b/src/glib-helper.c
index edc46d8..8cf29ae 100644
--- a/src/glib-helper.c
+++ b/src/glib-helper.c
@@ -525,7 +525,6 @@ int bt_discover_primary(const bdaddr_t *src, const bdaddr_t *dst, int psm,
{
struct gattrib_context *ctxt;
GIOChannel *io;
- GError *gerr = NULL;

ctxt = g_try_new0(struct gattrib_context, 1);
if (ctxt == NULL)
@@ -538,14 +537,14 @@ int bt_discover_primary(const bdaddr_t *src, const bdaddr_t *dst, int psm,
ctxt->destroy = destroy;

if (psm < 0)
- io = bt_io_connect(BT_IO_L2CAP, connect_cb, ctxt, NULL, &gerr,
+ io = bt_io_connect(BT_IO_L2CAP, connect_cb, ctxt, NULL, NULL,
BT_IO_OPT_SOURCE_BDADDR, src,
BT_IO_OPT_DEST_BDADDR, dst,
BT_IO_OPT_CID, GATT_CID,
BT_IO_OPT_SEC_LEVEL, sec_level,
BT_IO_OPT_INVALID);
else
- io = bt_io_connect(BT_IO_L2CAP, connect_cb, ctxt, NULL, &gerr,
+ io = bt_io_connect(BT_IO_L2CAP, connect_cb, ctxt, NULL, NULL,
BT_IO_OPT_SOURCE_BDADDR, src,
BT_IO_OPT_DEST_BDADDR, dst,
BT_IO_OPT_PSM, psm,
--
1.7.3.3

2010-12-15 19:54:11

by Claudio Takahasi

Subject: [PATCH 4/5] Fix missing reply when create device is cancelled

When CancelDeviceCreation is called or when the device is removed for
any reason, the reply for the pending CreateDevice is not sent.
---
src/device.c | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/device.c b/src/device.c
index cf3b146..a881c4c 100644
--- a/src/device.c
+++ b/src/device.c
@@ -730,8 +730,10 @@ void device_request_disconnect(struct btd_device *device, DBusMessage *msg)
if (device->bonding)
bonding_request_cancel(device->bonding);

- if (device->browse)
+ if (device->browse) {
+ discover_services_reply(device->browse, -ECANCELED, NULL);
browse_request_cancel(device->browse);
+ }

if (msg)
device->disconnects = g_slist_append(device->disconnects,
@@ -1043,8 +1045,10 @@ void device_remove(struct btd_device *device, gboolean remove_stored)
if (device->bonding)
device_cancel_bonding(device, HCI_OE_USER_ENDED_CONNECTION);

- if (device->browse)
+ if (device->browse) {
+ discover_services_reply(device->browse, -ECANCELED, NULL);
browse_request_cancel(device->browse);
+ }

if (device->handle)
do_disconnect(device);
--
1.7.3.3

2010-12-15 19:54:10

by Claudio Takahasi

Subject: [PATCH 3/5] Return an error if adapter_create_device fails

This function can fail if the device object is already registered or if
the system doesn't have enough memory. Issue detected due wrong device
object reference counting that doesn't unregister the D-Bus object path
when the device is removed.
---
src/adapter.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index c1fddce..f71d063 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -1595,7 +1595,8 @@ static DBusMessage *create_device(DBusConnection *conn,

device = adapter_create_device(conn, adapter, address, type);
if (!device)
- return NULL;
+ return btd_error_failed(msg,
+ "Unable to create a new device object");

if (type == DEVICE_TYPE_LE && !event_is_connectable(dev->evt_type)) {
/* Device is not connectable */
@@ -1693,7 +1694,8 @@ static DBusMessage *create_paired_device(DBusConnection *conn,

device = adapter_create_device(conn, adapter, address, type);
if (!device)
- return NULL;
+ return btd_error_failed(msg,
+ "Unable to create a new device object");
} else
type = device_get_type(device);

--
1.7.3.3

2010-12-15 19:54:09

by Claudio Takahasi

Subject: [PATCH 2/5] Change CreatePairedDevice to support LE devices

From: Sheldon Demario <[email protected]>

CreatePairedDevice implements now the same behaviour of CreateDevice,
triggering Discover All Primary Services when needed. SMP negotiation
starts when the link is established. LE capable kernel is required to
test this method properly.

Limitation: For dual mode devices, Discover All Primary Services is not
being executed after SDP search if GATT record is found.
---
src/adapter.c | 46 ++++++++++++++++++++++++---
src/device.c | 89 +++++++++++++++++++++++++++-------------------------
src/device.h | 7 +++-
src/glib-helper.c | 5 ++-
src/glib-helper.h | 3 ++
5 files changed, 98 insertions(+), 52 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index 2ff59a0..c1fddce 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -1611,7 +1611,11 @@ static DBusMessage *create_device(DBusConnection *conn,
return reply;
}

- err = device_browse(device, conn, msg, NULL, FALSE);
+ if (type != DEVICE_TYPE_LE)
+ err = device_browse_sdp(device, conn, msg, NULL, FALSE);
+ else
+ err = device_browse_primary(device, conn, msg, BT_IO_SEC_LOW);
+
if (err < 0) {
adapter_remove_device(conn, adapter, device, TRUE);
return btd_error_failed(msg, strerror(-err));
@@ -1642,6 +1646,8 @@ static DBusMessage *create_paired_device(DBusConnection *conn,
struct btd_device *device;
const gchar *address, *agent_path, *capability, *sender;
uint8_t cap;
+ device_type_t type;
+ int err;

if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &address,
DBUS_TYPE_OBJECT_PATH, &agent_path,
@@ -1666,12 +1672,40 @@ static DBusMessage *create_paired_device(DBusConnection *conn,
if (cap == IO_CAPABILITY_INVALID)
return btd_error_invalid_args(msg);

- device = adapter_get_device(conn, adapter, address);
- if (!device)
- return btd_error_failed(msg,
- "Unable to create a new device object");
+ device = adapter_find_device(adapter, address);
+ if (!device) {
+ struct remote_dev_info *dev, match;
+
+ memset(&match, 0, sizeof(struct remote_dev_info));
+ str2ba(address, &match.bdaddr);
+ match.name_status = NAME_ANY;
+
+ dev = adapter_search_found_devices(adapter, &match);
+ if (dev && dev->flags)
+ type = flags2type(dev->flags);
+ else
+ type = DEVICE_TYPE_BREDR;
+
+ if (type == DEVICE_TYPE_LE &&
+ !event_is_connectable(dev->evt_type))
+ return btd_error_failed(msg,
+ "Device is not connectable");
+
+ device = adapter_create_device(conn, adapter, address, type);
+ if (!device)
+ return NULL;
+ } else
+ type = device_get_type(device);
+
+ if (type != DEVICE_TYPE_LE)
+ return device_create_bonding(device, conn, msg,
+ agent_path, cap);

- return device_create_bonding(device, conn, msg, agent_path, cap);
+ err = device_browse_primary(device, conn, msg, BT_IO_SEC_HIGH);
+ if (err < 0)
+ return btd_error_failed(msg, strerror(-err));
+
+ return NULL;
}

static gint device_path_cmp(struct btd_device *device, const gchar *path)
diff --git a/src/device.c b/src/device.c
index d20a6d4..cf3b146 100644
--- a/src/device.c
+++ b/src/device.c
@@ -586,7 +586,7 @@ static DBusMessage *discover_services(DBusConnection *conn,
return btd_error_invalid_args(msg);

if (strlen(pattern) == 0) {
- err = device_browse(device, conn, msg, NULL, FALSE);
+ err = device_browse_sdp(device, conn, msg, NULL, FALSE);
if (err < 0)
goto fail;
} else {
@@ -597,7 +597,7 @@ static DBusMessage *discover_services(DBusConnection *conn,

sdp_uuid128_to_uuid(&uuid);

- err = device_browse(device, conn, msg, &uuid, FALSE);
+ err = device_browse_sdp(device, conn, msg, &uuid, FALSE);
if (err < 0)
goto fail;
}
@@ -988,6 +988,11 @@ void device_get_name(struct btd_device *device, char *name, size_t len)
strncpy(name, device->name, len);
}

+device_type_t device_get_type(struct btd_device *device)
+{
+ return device->type;
+}
+
void device_remove_bonding(struct btd_device *device)
{
char filename[PATH_MAX + 1];
@@ -1540,41 +1545,62 @@ done:
browse_request_free(req);
}

-static struct browse_req *browse_primary(struct btd_device *device, int *err)
+int device_browse_primary(struct btd_device *device, DBusConnection *conn,
+ DBusMessage *msg, BtIOSecLevel sec_level)
{
struct btd_adapter *adapter = device->adapter;
struct browse_req *req;
bdaddr_t src;
- int ret;
+ int err;
+
+ if (device->browse)
+ return -EBUSY;

req = g_new0(struct browse_req, 1);
req->device = btd_device_ref(device);

adapter_get_address(adapter, &src);

- ret = bt_discover_primary(&src, &device->bdaddr, -1, primary_cb, req,
- NULL);
-
- if (ret < 0) {
+ err = bt_discover_primary(&src, &device->bdaddr, -1, primary_cb, req,
+ sec_level, NULL);
+ if (err < 0) {
browse_request_free(req);
- if (err)
- *err = ret;
+ return err;
+ }

- return NULL;
+ if (conn == NULL)
+ conn = get_dbus_connection();
+
+ req->conn = dbus_connection_ref(conn);
+ device->browse = req;
+
+ if (msg) {
+ const char *sender = dbus_message_get_sender(msg);
+
+ req->msg = dbus_message_ref(msg);
+ /* Track the request owner to cancel it
+ * automatically if the owner exits */
+ req->listener_id = g_dbus_add_disconnect_watch(conn,
+ sender,
+ discover_services_req_exit,
+ req, NULL);
}

- return req;
+ return err;
}

-static struct browse_req *browse_sdp(struct btd_device *device, uuid_t *search,
- gboolean reverse, int *err)
+int device_browse_sdp(struct btd_device *device, DBusConnection *conn,
+ DBusMessage *msg, uuid_t *search, gboolean reverse)
{
struct btd_adapter *adapter = device->adapter;
struct browse_req *req;
bt_callback_t cb;
bdaddr_t src;
uuid_t uuid;
- int ret;
+ int err;
+
+ if (device->browse)
+ return -EBUSY;

adapter_get_address(adapter, &src);

@@ -1589,34 +1615,11 @@ static struct browse_req *browse_sdp(struct btd_device *device, uuid_t *search,
cb = browse_cb;
}

- ret = bt_search_service(&src, &device->bdaddr, &uuid, cb, req, NULL);
- if (ret < 0) {
+ err = bt_search_service(&src, &device->bdaddr, &uuid, cb, req, NULL);
+ if (err < 0) {
browse_request_free(req);
- if (err)
- *err = ret;
-
- return NULL;
- }
-
- return req;
-}
-
-int device_browse(struct btd_device *device, DBusConnection *conn,
- DBusMessage *msg, uuid_t *search, gboolean reverse)
-{
- struct browse_req *req;
- int err = 0;
-
- if (device->browse)
- return -EBUSY;
-
- if (device->type == DEVICE_TYPE_LE)
- req = browse_primary(device, &err);
- else
- req = browse_sdp(device, search, reverse, &err);
-
- if (req == NULL)
return err;
+ }

if (conn == NULL)
conn = get_dbus_connection();
@@ -1719,7 +1722,7 @@ static gboolean start_discovery(gpointer user_data)
{
struct btd_device *device = user_data;

- device_browse(device, NULL, NULL, NULL, TRUE);
+ device_browse_sdp(device, NULL, NULL, NULL, TRUE);

device->discov_timer = 0;

@@ -2051,7 +2054,7 @@ void device_bonding_complete(struct btd_device *device, uint8_t status)
device->discov_timer = 0;
}

- device_browse(device, bonding->conn, bonding->msg,
+ device_browse_sdp(device, bonding->conn, bonding->msg,
NULL, FALSE);

bonding_request_free(bonding);
diff --git a/src/device.h b/src/device.h
index 784e931..cafa529 100644
--- a/src/device.h
+++ b/src/device.h
@@ -24,6 +24,8 @@

#define DEVICE_INTERFACE "org.bluez.Device"

+#include "btio.h"
+
struct btd_device;

typedef enum {
@@ -46,9 +48,12 @@ struct btd_device *device_create(DBusConnection *conn,
const gchar *address, device_type_t type);
void device_set_name(struct btd_device *device, const char *name);
void device_get_name(struct btd_device *device, char *name, size_t len);
+device_type_t device_get_type(struct btd_device *device);
void device_remove(struct btd_device *device, gboolean remove_stored);
gint device_address_cmp(struct btd_device *device, const gchar *address);
-int device_browse(struct btd_device *device, DBusConnection *conn,
+int device_browse_primary(struct btd_device *device, DBusConnection *conn,
+ DBusMessage *msg, BtIOSecLevel sec_level);
+int device_browse_sdp(struct btd_device *device, DBusConnection *conn,
DBusMessage *msg, uuid_t *search, gboolean reverse);
void device_probe_drivers(struct btd_device *device, GSList *profiles);
const sdp_record_t *btd_device_get_record(struct btd_device *device,
diff --git a/src/glib-helper.c b/src/glib-helper.c
index 6505249..edc46d8 100644
--- a/src/glib-helper.c
+++ b/src/glib-helper.c
@@ -520,6 +520,7 @@ static void connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)

int bt_discover_primary(const bdaddr_t *src, const bdaddr_t *dst, int psm,
bt_primary_t cb, void *user_data,
+ BtIOSecLevel sec_level,
bt_destroy_t destroy)
{
struct gattrib_context *ctxt;
@@ -541,14 +542,14 @@ int bt_discover_primary(const bdaddr_t *src, const bdaddr_t *dst, int psm,
BT_IO_OPT_SOURCE_BDADDR, src,
BT_IO_OPT_DEST_BDADDR, dst,
BT_IO_OPT_CID, GATT_CID,
- BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_LOW,
+ BT_IO_OPT_SEC_LEVEL, sec_level,
BT_IO_OPT_INVALID);
else
io = bt_io_connect(BT_IO_L2CAP, connect_cb, ctxt, NULL, &gerr,
BT_IO_OPT_SOURCE_BDADDR, src,
BT_IO_OPT_DEST_BDADDR, dst,
BT_IO_OPT_PSM, psm,
- BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_LOW,
+ BT_IO_OPT_SEC_LEVEL, sec_level,
BT_IO_OPT_INVALID);

if (io == NULL) {
diff --git a/src/glib-helper.h b/src/glib-helper.h
index 5bb20a6..a16de6c 100644
--- a/src/glib-helper.h
+++ b/src/glib-helper.h
@@ -21,6 +21,8 @@
*
*/

+#include "btio.h"
+
typedef void (*bt_callback_t) (sdp_list_t *recs, int err, gpointer user_data);
typedef void (*bt_primary_t) (GSList *l, int err, gpointer user_data);
typedef void (*bt_destroy_t) (gpointer user_data);
@@ -35,6 +37,7 @@ int bt_cancel_discovery(const bdaddr_t *src, const bdaddr_t *dst);

int bt_discover_primary(const bdaddr_t *src, const bdaddr_t *dst, int psm,
bt_primary_t cb, void *user_data,
+ BtIOSecLevel sec_level,
bt_destroy_t destroy);

gchar *bt_uuid2string(uuid_t *uuid);
--
1.7.3.3