Linux bluetooth development
 help / color / mirror / Atom feed
* [PATCHv2 1/7] Add support for Out of Band (OOB) association model.
From: Szymon Janc @ 2010-11-10 13:31 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1289395877-12384-1-git-send-email-szymon.janc@tieto.com>

---
 Makefile.am      |    3 +-
 lib/hci.h        |    3 ++
 plugins/hciops.c |   72 +++++++++++++++++++++++++++++++++++++++++---------
 src/adapter.c    |    7 ++++-
 src/adapter.h    |    3 ++
 src/device.c     |   76 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/device.h     |   12 ++++++++
 src/event.c      |   75 +++++++++++++++++++++++++++++++++++++++-------------
 src/event.h      |    3 +-
 src/oob.c        |   61 +++++++++++++++++++++++++++++++++++++++++++
 src/oob.h        |   47 +++++++++++++++++++++++++++++++++
 11 files changed, 326 insertions(+), 36 deletions(-)
 create mode 100644 src/oob.c
 create mode 100644 src/oob.h

diff --git a/Makefile.am b/Makefile.am
index da308a7..a61e754 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -238,7 +238,8 @@ src_bluetoothd_SOURCES = $(gdbus_sources) $(builtin_sources) \
 			src/adapter.h src/adapter.c \
 			src/device.h src/device.c \
 			src/dbus-common.c src/dbus-common.h \
-			src/event.h src/event.c
+			src/event.h src/event.c \
+			src/oob.c
 src_bluetoothd_LDADD = lib/libbluetooth.la @GLIB_LIBS@ @DBUS_LIBS@ \
 							@CAPNG_LIBS@ -ldl -lrt
 src_bluetoothd_LDFLAGS = -Wl,--export-dynamic \
diff --git a/lib/hci.h b/lib/hci.h
index 0cb120f..409abd9 100644
--- a/lib/hci.h
+++ b/lib/hci.h
@@ -524,6 +524,9 @@ typedef struct {
 
 #define OCF_REMOTE_OOB_DATA_NEG_REPLY	0x0033
 
+#define OOB_DATA_NOT_PRESENT	0x00
+#define OOB_DATA_PRESENT	0x01
+
 #define OCF_IO_CAPABILITY_NEG_REPLY	0x0034
 typedef struct {
 	bdaddr_t	bdaddr;
diff --git a/plugins/hciops.c b/plugins/hciops.c
index 829011a..88bcf15 100644
--- a/plugins/hciops.c
+++ b/plugins/hciops.c
@@ -3,6 +3,7 @@
  *  BlueZ - Bluetooth protocol stack for Linux
  *
  *  Copyright (C) 2004-2010  Marcel Holtmann <marcel@holtmann.org>
+ *  Copyright (C) 2010  ST-Ericsson SA
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -47,6 +48,7 @@
 #include "event.h"
 #include "device.h"
 #include "manager.h"
+#include "oob.h"
 
 #define HCI_REQ_TIMEOUT         5000
 
@@ -509,20 +511,41 @@ static void user_passkey_notify(int index, void *ptr)
 
 static void remote_oob_data_request(int index, void *ptr)
 {
-	hci_send_cmd(SK(index), OGF_LINK_CTL,
-				OCF_REMOTE_OOB_DATA_NEG_REPLY, 6, ptr);
+	bdaddr_t *dba = ptr;
+	struct btd_adapter *adapter;
+	struct btd_device *device;
+	char da[18];
+
+	ba2str(dba, da);
+	adapter = manager_find_adapter(&BDADDR(index));
+	device = adapter_find_device(adapter, da);
+
+	if (device_has_oob_data(device)) {
+		remote_oob_data_reply_cp cp;
+
+		bacpy(&cp.bdaddr, dba);
+		device_get_oob_data(device,cp.hash,cp.randomizer);
+
+		hci_send_cmd(SK(index), OGF_LINK_CTL, OCF_REMOTE_OOB_DATA_REPLY,
+				REMOTE_OOB_DATA_REPLY_CP_SIZE, &cp);
+	} else
+		hci_send_cmd(SK(index),
+				OGF_LINK_CTL, OCF_REMOTE_OOB_DATA_NEG_REPLY, 6,
+				ptr);
 }
 
 static void io_capa_request(int index, void *ptr)
 {
 	bdaddr_t *dba = ptr;
 	char sa[18], da[18];
-	uint8_t cap, auth;
 
 	ba2str(&BDADDR(index), sa); ba2str(dba, da);
 	info("io_capa_request (sba=%s, dba=%s)", sa, da);
 
-	if (btd_event_get_io_cap(&BDADDR(index), dba, &cap, &auth) < 0) {
+	/* If failed to establish IO capabilities then send negative reply
+	 * immediately. Positive reply will be sent when IO capabilities are
+	 * established. */
+	if (btd_event_request_io_cap(&BDADDR(index), dba)) {
 		io_capability_neg_reply_cp cp;
 		memset(&cp, 0, sizeof(cp));
 		bacpy(&cp.bdaddr, dba);
@@ -530,15 +553,6 @@ static void io_capa_request(int index, void *ptr)
 		hci_send_cmd(SK(index), OGF_LINK_CTL,
 					OCF_IO_CAPABILITY_NEG_REPLY,
 					IO_CAPABILITY_NEG_REPLY_CP_SIZE, &cp);
-	} else {
-		io_capability_reply_cp cp;
-		memset(&cp, 0, sizeof(cp));
-		bacpy(&cp.bdaddr, dba);
-		cp.capability = cap;
-		cp.oob_data = 0x00;
-		cp.authentication = auth;
-		hci_send_cmd(SK(index), OGF_LINK_CTL, OCF_IO_CAPABILITY_REPLY,
-					IO_CAPABILITY_REPLY_CP_SIZE, &cp);
 	}
 }
 
@@ -748,6 +762,15 @@ static void read_scan_complete(int index, uint8_t status, void *ptr)
 	adapter_mode_changed(adapter, rp->enable);
 }
 
+static void read_local_oob_data_complete(bdaddr_t *local, uint8_t status,
+		read_local_oob_data_rp *rp)
+{
+	if (status)
+		oob_local_data_updated(local, NULL, NULL);
+	else
+		oob_local_data_updated(local, rp->hash, rp->randomizer);
+}
+
 static inline void cmd_complete(int index, void *ptr)
 {
 	evt_cmd_complete *evt = ptr;
@@ -808,6 +831,10 @@ static inline void cmd_complete(int index, void *ptr)
 		ptr += sizeof(evt_cmd_complete);
 		adapter_update_tx_power(&BDADDR(index), status, ptr);
 		break;
+	case cmd_opcode_pack(OGF_HOST_CTL, OCF_READ_LOCAL_OOB_DATA):
+		ptr += sizeof(evt_cmd_complete);
+		read_local_oob_data_complete(&BDADDR(index), status, ptr);
+		break;
 	};
 }
 
@@ -2280,6 +2307,24 @@ static int hciops_get_remote_version(int index, uint16_t handle,
 	return 0;
 }
 
+static int hciops_read_local_oob_data(int index)
+{
+	int dd;
+	int err = 0;
+
+	dd = hci_open_dev(index);
+	if (dd < 0)
+		return -EIO;
+
+	err = hci_send_cmd(dd, OGF_HOST_CTL, OCF_READ_LOCAL_OOB_DATA, 0, 0);
+	if (err < 0)
+		err = -errno;
+
+	hci_close_dev(dd);
+
+	return err;
+}
+
 static struct btd_adapter_ops hci_ops = {
 	.setup = hciops_setup,
 	.cleanup = hciops_cleanup,
@@ -2326,6 +2371,7 @@ static struct btd_adapter_ops hci_ops = {
 	.write_le_host = hciops_write_le_host,
 	.get_remote_version = hciops_get_remote_version,
 	.encrypt_link = hciops_encrypt_link,
+	.read_local_oob_data = hciops_read_local_oob_data,
 };
 
 static int hciops_init(void)
diff --git a/src/adapter.c b/src/adapter.c
index 31014e5..39a6514 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -3265,7 +3265,7 @@ void adapter_remove_connection(struct btd_adapter *adapter,
 	/* clean pending HCI cmds */
 	device_get_address(device, &bdaddr);
 
-	if (device_is_authenticating(device))
+	if (device_is_authenticating(device) || device_has_oob_data(device))
 		device_cancel_authentication(device, TRUE);
 
 	if (device_is_temporary(device)) {
@@ -3738,3 +3738,8 @@ int btd_adapter_encrypt_link(struct btd_adapter *adapter, bdaddr_t *bdaddr,
 {
 	return adapter_ops->encrypt_link(adapter->dev_id, bdaddr, cb, user_data);
 }
+
+int btd_adapter_read_local_oob_data(struct btd_adapter *adapter)
+{
+	return adapter_ops->read_local_oob_data(adapter->dev_id);
+}
diff --git a/src/adapter.h b/src/adapter.h
index 8019cfc..cc62865 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -229,6 +229,7 @@ struct btd_adapter_ops {
 						gboolean delayed);
 	int (*encrypt_link) (int index, bdaddr_t *bdaddr, bt_hci_result_t cb,
 							gpointer user_data);
+	int (*read_local_oob_data) (int index);
 };
 
 int btd_register_adapter_ops(struct btd_adapter_ops *ops, gboolean priority);
@@ -289,3 +290,5 @@ int btd_adapter_get_remote_version(struct btd_adapter *adapter,
 
 int btd_adapter_encrypt_link(struct btd_adapter *adapter, bdaddr_t *bdaddr,
 				bt_hci_result_t cb, gpointer user_data);
+
+int btd_adapter_read_local_oob_data(struct btd_adapter *adapter);
diff --git a/src/device.c b/src/device.c
index 7c421e3..24fd44d 100644
--- a/src/device.c
+++ b/src/device.c
@@ -4,6 +4,7 @@
  *
  *  Copyright (C) 2006-2010  Nokia Corporation
  *  Copyright (C) 2004-2010  Marcel Holtmann <marcel@holtmann.org>
+ *  Copyright (C) 2010  ST-Ericsson SA
  *
  *
  *  This program is free software; you can redistribute it and/or modify
@@ -59,6 +60,7 @@
 #include "sdp-xml.h"
 #include "storage.h"
 #include "btio.h"
+#include "oob.h"
 
 #define DEFAULT_XML_BUF_SIZE	1024
 #define DISCONNECT_TIMER	2
@@ -132,6 +134,9 @@ struct btd_device {
 	uint8_t		cap;
 	uint8_t		auth;
 
+	uint8_t		local_cap;
+	uint8_t		local_auth;
+
 	uint16_t	handle;			/* Connection handle */
 
 	/* Whether were creating a security mode 3 connection */
@@ -149,6 +154,12 @@ struct btd_device {
 
 	gboolean	has_debug_key;
 	uint8_t		debug_key[16];
+
+	/* For OOB association model */
+	void (*oob_request_cb)(struct btd_device *device);
+	gboolean	has_oob_data;
+	uint8_t		hash[16];
+	uint8_t		randomizer[16];
 };
 
 static uint16_t uuid_list[] = {
@@ -829,6 +840,69 @@ static DBusMessage *disconnect(DBusConnection *conn, DBusMessage *msg,
 	return NULL;
 }
 
+void device_set_oob_data(struct btd_device *device, uint8_t *hash,
+		uint8_t *randomizer)
+{
+	if (!device)
+		return;
+
+	if (hash && randomizer) {
+		memcpy(device->hash, hash, 16);
+		memcpy(device->randomizer, randomizer, 16);
+		device->has_oob_data = TRUE;
+	}
+
+	if (device->oob_request_cb) {
+		device->oob_request_cb(device);
+		device->oob_request_cb = NULL;
+	}
+}
+
+gboolean device_get_oob_data(struct btd_device *device, uint8_t *hash,
+		uint8_t *randomizer)
+{
+	if (!device || !device->has_oob_data)
+		return FALSE;
+
+	memcpy(hash, device->hash, 16);
+	memcpy(randomizer, device->randomizer, 16);
+
+	return TRUE;
+}
+
+gboolean device_has_oob_data(struct btd_device *device)
+{
+	return device && device->has_oob_data;
+}
+
+gboolean device_request_oob_data(struct btd_device *device, void *cb)
+{
+	if (!device)
+		return FALSE;
+
+	device->oob_request_cb = cb;
+	return oob_request_remote_data(device);
+}
+
+void device_set_local_auth_cap(struct btd_device *device, uint8_t auth,
+		uint8_t cap)
+{
+	if (!device)
+		return;
+
+	device->local_auth = auth;
+	device->local_cap = cap;
+}
+void device_get_local_auth_cap(struct btd_device *device, uint8_t *auth,
+		uint8_t *cap)
+{
+	if (!device)
+		return;
+
+	*auth = device->local_auth;
+	*cap = device->local_cap;
+}
+
 static GDBusMethodTable device_methods[] = {
 	{ "GetProperties",	"",	"a{sv}",	get_properties	},
 	{ "SetProperty",	"sv",	"",		set_property	},
@@ -2264,6 +2338,8 @@ void device_cancel_authentication(struct btd_device *device, gboolean aborted)
 {
 	struct authentication_req *auth = device->authr;
 
+	device->has_oob_data = FALSE;
+
 	if (!auth)
 		return;
 
diff --git a/src/device.h b/src/device.h
index b570bd1..b62cdc5 100644
--- a/src/device.h
+++ b/src/device.h
@@ -4,6 +4,7 @@
  *
  *  Copyright (C) 2006-2010  Nokia Corporation
  *  Copyright (C) 2004-2010  Marcel Holtmann <marcel@holtmann.org>
+ *  Copyright (C) 2010  ST-Ericsson SA
  *
  *
  *  This program is free software; you can redistribute it and/or modify
@@ -89,6 +90,17 @@ void device_remove_connection(struct btd_device *device, DBusConnection *conn,
 gboolean device_has_connection(struct btd_device *device, uint16_t handle);
 void device_request_disconnect(struct btd_device *device, DBusMessage *msg);
 
+void device_set_oob_data(struct btd_device *device, uint8_t *hash,
+		uint8_t *randomizer);
+gboolean device_get_oob_data(struct btd_device *device, uint8_t *hash,
+		uint8_t *randomizer);
+gboolean device_has_oob_data(struct btd_device *device);
+gboolean device_request_oob_data(struct btd_device *device, void *cb);
+void device_set_local_auth_cap(struct btd_device *device, uint8_t auth,
+		uint8_t cap);
+void device_get_local_auth_cap(struct btd_device *device, uint8_t *auth,
+		uint8_t *cap);
+
 typedef void (*disconnect_watch) (struct btd_device *device, gboolean removal,
 					void *user_data);
 
diff --git a/src/event.c b/src/event.c
index e943c63..5a5a288 100644
--- a/src/event.c
+++ b/src/event.c
@@ -4,6 +4,7 @@
  *
  *  Copyright (C) 2006-2010  Nokia Corporation
  *  Copyright (C) 2004-2010  Marcel Holtmann <marcel@holtmann.org>
+ *  Copyright (C) 2010  ST-Ericsson SA
  *
  *
  *  This program is free software; you can redistribute it and/or modify
@@ -274,7 +275,7 @@ void btd_event_bonding_process_complete(bdaddr_t *local, bdaddr_t *peer,
 	if (!get_adapter_and_device(local, peer, &adapter, &device, TRUE))
 		return;
 
-	if (!device_is_authenticating(device)) {
+	if (!device_is_authenticating(device) && !device_has_oob_data(device)) {
 		/* This means that there was no pending PIN or SSP token
 		 * request from the controller, i.e. this is not a new
 		 * pairing */
@@ -756,26 +757,56 @@ void btd_event_returned_link_key(bdaddr_t *local, bdaddr_t *peer)
 	device_set_paired(device, TRUE);
 }
 
-int btd_event_get_io_cap(bdaddr_t *local, bdaddr_t *remote,
-						uint8_t *cap, uint8_t *auth)
+static void btd_event_io_cap_reply(struct btd_device *device)
+{
+	io_capability_reply_cp cp;
+	int dev;
+	struct btd_adapter *adapter = device_get_adapter(device);
+	uint16_t dev_id = adapter_get_dev_id(adapter);
+
+	dev = hci_open_dev(dev_id);
+	if (dev < 0) {
+		error("hci_open_dev(%d): %s (%d)", dev_id,
+		strerror(errno), errno);
+		return;
+	}
+
+	memset(&cp, 0, sizeof(cp));
+	device_get_address(device, &cp.bdaddr);
+	device_get_local_auth_cap(device, &cp.authentication, &cp.capability);
+	cp.oob_data = device_has_oob_data(device)
+			? OOB_DATA_PRESENT : OOB_DATA_NOT_PRESENT;
+
+	DBG("final capabilities reply is cap=0x%02x, auth=0x%02x, oob=0x%02x",
+	cp.capability, cp.authentication, cp.oob_data);
+
+	hci_send_cmd(dev, OGF_LINK_CTL, OCF_IO_CAPABILITY_REPLY,
+					IO_CAPABILITY_REPLY_CP_SIZE, &cp);
+
+	hci_close_dev(dev);
+}
+
+int btd_event_request_io_cap(bdaddr_t *local, bdaddr_t *remote)
 {
 	struct btd_adapter *adapter;
 	struct btd_device *device;
 	struct agent *agent = NULL;
 	uint8_t agent_cap;
 	int err;
+	uint8_t cap;
+	uint8_t auth;
 
 	if (!get_adapter_and_device(local, remote, &adapter, &device, TRUE))
 		return -ENODEV;
 
-	err = btd_adapter_get_auth_info(adapter, remote, auth);
+	err = btd_adapter_get_auth_info(adapter, remote, &auth);
 	if (err < 0)
 		return err;
 
-	DBG("initial authentication requirement is 0x%02x", *auth);
+	DBG("initial authentication requirement is 0x%02x", auth);
 
-	if (*auth == 0xff)
-		*auth = device_get_auth(device);
+	if (auth == 0xff)
+		auth = device_get_auth(device);
 
 	/* Check if the adapter is not pairable and if there isn't a bonding
 	 * in progress */
@@ -784,11 +815,11 @@ int btd_event_get_io_cap(bdaddr_t *local, bdaddr_t *remote,
 		if (device_get_auth(device) < 0x02) {
 			DBG("Allowing no bonding in non-bondable mode");
 			/* No input, no output */
-			*cap = 0x03;
+			cap = 0x03;
 			/* Kernel defaults to general bonding and so
 			 * overwrite for this special case. Otherwise
 			 * non-pairable test cases will fail. */
-			*auth = 0x00;
+			auth = 0x00;
 			goto done;
 		}
 		return -EPERM;
@@ -804,13 +835,13 @@ int btd_event_get_io_cap(bdaddr_t *local, bdaddr_t *remote,
 		}
 
 		/* No agent available, and no bonding case */
-		if (*auth == 0x00 || *auth == 0x04) {
+		if (auth == 0x00 || auth == 0x04) {
 			DBG("Allowing no bonding without agent");
 			/* No input, no output */
-			*cap = 0x03;
+			cap = 0x03;
 			/* If kernel defaults to general bonding, set it
 			 * back to no bonding */
-			*auth = 0x00;
+			auth = 0x00;
 			goto done;
 		}
 
@@ -820,7 +851,7 @@ int btd_event_get_io_cap(bdaddr_t *local, bdaddr_t *remote,
 
 	agent_cap = agent_get_io_capability(agent);
 
-	if (*auth == 0x00 || *auth == 0x04) {
+	if (auth == 0x00 || auth == 0x04) {
 		/* If remote requests dedicated bonding follow that lead */
 		if (device_get_auth(device) == 0x02 ||
 				device_get_auth(device) == 0x03) {
@@ -829,9 +860,9 @@ int btd_event_get_io_cap(bdaddr_t *local, bdaddr_t *remote,
 			 * then require it, otherwise don't */
 			if (device_get_cap(device) == 0x03 ||
 							agent_cap == 0x03)
-				*auth = 0x02;
+				auth = 0x02;
 			else
-				*auth = 0x03;
+				auth = 0x03;
 		}
 
 		/* If remote indicates no bonding then follow that. This
@@ -839,7 +870,7 @@ int btd_event_get_io_cap(bdaddr_t *local, bdaddr_t *remote,
 		 * as default. */
 		if (device_get_auth(device) == 0x00 ||
 					device_get_auth(device) == 0x01)
-			*auth = 0x00;
+			auth = 0x00;
 
 		/* If remote requires MITM then also require it, unless
 		 * our IO capability is NoInputNoOutput (so some
@@ -847,13 +878,19 @@ int btd_event_get_io_cap(bdaddr_t *local, bdaddr_t *remote,
 		if (device_get_auth(device) != 0xff &&
 					(device_get_auth(device) & 0x01) &&
 					agent_cap != 0x03)
-			*auth |= 0x01;
+			auth |= 0x01;
 	}
 
-	*cap = agent_get_io_capability(agent);
+	cap = agent_get_io_capability(agent);
 
 done:
-	DBG("final authentication requirement is 0x%02x", *auth);
+	DBG("final authentication requirement is 0x%02x", auth);
+
+	device_set_local_auth_cap(device, auth, cap);
+
+	/* If failed to request remote OOB data then reply immediately. */
+	if (!device_request_oob_data(device, btd_event_io_cap_reply))
+		btd_event_io_cap_reply(device);
 
 	return 0;
 }
diff --git a/src/event.h b/src/event.h
index 4a7b9c9..a3e7dda 100644
--- a/src/event.h
+++ b/src/event.h
@@ -36,8 +36,7 @@ void btd_event_le_set_scan_enable_complete(bdaddr_t *local, uint8_t status);
 void btd_event_write_simple_pairing_mode_complete(bdaddr_t *local);
 void btd_event_read_simple_pairing_mode_complete(bdaddr_t *local, void *ptr);
 void btd_event_returned_link_key(bdaddr_t *local, bdaddr_t *peer);
-int btd_event_get_io_cap(bdaddr_t *local, bdaddr_t *remote,
-						uint8_t *cap, uint8_t *auth);
+int btd_event_request_io_cap(bdaddr_t *local, bdaddr_t *remote);
 int btd_event_set_io_cap(bdaddr_t *local, bdaddr_t *remote,
 						uint8_t cap, uint8_t auth);
 int btd_event_user_confirm(bdaddr_t *sba, bdaddr_t *dba, uint32_t passkey);
diff --git a/src/oob.c b/src/oob.c
new file mode 100644
index 0000000..cc20c67
--- /dev/null
+++ b/src/oob.c
@@ -0,0 +1,61 @@
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2010  ST-Ericsson SA
+ *
+ *  Author: Szymon Janc <szymon.janc@tieto.com> for ST-Ericsson
+ *
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#include <glib.h>
+#include "manager.h"
+#include "adapter.h"
+#include "oob.h"
+
+static struct oob_plugin *active_plugin = NULL;
+
+void oob_activate_plugin(struct oob_plugin *plugin)
+{
+	if (!plugin || !plugin->local_data_updated|| !plugin->plugin_deactivated
+			|| !plugin->request_remote_data
+			|| active_plugin == plugin)
+		return;
+
+	if (active_plugin)
+		active_plugin->plugin_deactivated();
+
+	active_plugin = plugin;
+}
+
+void oob_deactivate_plugin(struct oob_plugin *plugin)
+{
+	if (active_plugin == plugin)
+		active_plugin = NULL;
+}
+
+gboolean oob_request_remote_data(struct btd_device *device)
+{
+	return active_plugin && active_plugin->request_remote_data(device);
+}
+
+void oob_local_data_updated(bdaddr_t *ba, uint8_t *hash, uint8_t *randomizer)
+{
+	if (active_plugin)
+		active_plugin->local_data_updated(ba, hash, randomizer);
+}
diff --git a/src/oob.h b/src/oob.h
new file mode 100644
index 0000000..b3e3623
--- /dev/null
+++ b/src/oob.h
@@ -0,0 +1,47 @@
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2010  ST-Ericsson SA
+ *
+ *  Author: Szymon Janc <szymon.janc@tieto.com> for ST-Ericsson
+ *
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+struct oob_plugin
+{
+	/* If request was successfully send this functions should return TRUE.
+	 * Function should not block for too long. */
+	gboolean (*request_remote_data)(struct btd_device *device);
+
+	/* Local OOB data updated. If corresponding HCI command failed, hash
+	 * and randomizer are NULL */
+	void (*local_data_updated)(bdaddr_t *ba, uint8_t *hash,
+			uint8_t *randomizer);
+
+	/* Plug-in was deactivated (called only for active plug-in). */
+	void (*plugin_deactivated)(void);
+};
+
+/* These functions are called by OOB plug-in. */
+void oob_activate_plugin(struct oob_plugin *plugin);
+void oob_deactivate_plugin(struct oob_plugin *plugin);
+
+/* These functions are called from stack to interact with OOB plug-in. */
+gboolean oob_request_remote_data(struct btd_device *device);
+void oob_local_data_updated(bdaddr_t *ba, uint8_t *hash, uint8_t *randomizer);
-- 
1.7.1


^ permalink raw reply related

* [PATCHv2 0/7]  Support for out of band association model
From: Szymon Janc @ 2010-11-10 13:31 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

Hello,

New version with some bugfixes/improvements and small changes in DBus API.
New feature is support for approval for incoming OOB pairing request.

Comments are welcome.

This email was sent on behalf of ST-Ericsson SA.

Szymon Janc (7):
  Add support for Out of Band (OOB) association model.
  Add DBus OOB plugin.
  Add DBus OOB API documentation.
  Add simple-oobprovider for testing.
  Add approval request for incoming pairing requests with OOB mechanism
  Update DBus OOB API with RequestApproval method.
  simple-agent - add RequestApproval method for OOB pairing

 Makefile.am             |   10 +-
 acinclude.m4            |    6 +
 doc/oob-api.txt         |   75 ++++++++++
 lib/hci.h               |    3 +
 plugins/dbusoob.c       |  353 +++++++++++++++++++++++++++++++++++++++++++++++
 plugins/hciops.c        |   96 +++++++++++--
 src/adapter.c           |   14 ++-
 src/adapter.h           |    6 +
 src/agent.c             |   59 ++++++++-
 src/agent.h             |    3 +
 src/device.c            |   98 +++++++++++++
 src/device.h            |   13 ++
 src/event.c             |  116 +++++++++++++---
 src/event.h             |    4 +-
 src/oob.c               |   61 ++++++++
 src/oob.h               |   47 +++++++
 test/simple-agent       |   10 ++
 test/simple-oobprovider |   61 ++++++++
 18 files changed, 997 insertions(+), 38 deletions(-)
 create mode 100644 doc/oob-api.txt
 create mode 100644 plugins/dbusoob.c
 create mode 100644 src/oob.c
 create mode 100644 src/oob.h
 create mode 100755 test/simple-oobprovider


^ permalink raw reply

* Re: [PATCH v2] Bluetooth: Automate remote name requests
From: Johan Hedberg @ 2010-11-10 13:17 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <1289367862.9615.243.camel@aeonflux>

Hi Marcel,

On Wed, Nov 10, 2010, Marcel Holtmann wrote:
> please split this patch into two. One creating the common function for
> the authentication requested and one for adding the name request.
> 
> My brain actually core dumps when following the logic and making sure
> that it is still correct. I pretty sure it is correct, but the whole
> patch is damn hard to review.

Yeah, sorry about this. I know the patch ended up being quite complex.
I'll resend it in three parts in a moment:

1. getting rid of multiple nested if-statements in the remote features
callbacks.
2. create a unified authentication request function
3. add the remote name request

Johan

^ permalink raw reply

* [PATCH 7/7] Changed number of fields selected via pbap queries
From: Bartosz Szatkowski @ 2010-11-10 13:15 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bartosz Szatkowski
In-Reply-To: <1289394930-6694-1-git-send-email-bulislaw@linux.com>

After fixing type handling in PBAP, there are more fields selected in each
query, contacts_other_query_from_uri needed to be adjusted.
---
 plugins/phonebook-tracker.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/plugins/phonebook-tracker.c b/plugins/phonebook-tracker.c
index f195f3f..672d59f 100644
--- a/plugins/phonebook-tracker.c
+++ b/plugins/phonebook-tracker.c
@@ -822,6 +822,7 @@
 	"SELECT \"\" \"\" \"\" \"\" \"\" \"\" \"\" \"\" \"\" \"\" \"\" "\
 	"\"\" \"\" \"\" \"\" \"\" \"\" \"\" \"\" \"\" \"\" \"\" \"\" "	\
 	"\"\" \"\" \"\" \"\" \"\" \"\" \"\" \"\" \"\" \"\" \"\" "	\
+	"\"\" \"\" \"\" \"\" \"\" \"\" \"\" \"\" \"\" "			\
 	"nco:phoneNumber(?t) \"NOTACALL\" \"false\" \"false\" <%s> "	\
 	"WHERE { "							\
 		"<%s> a nco:Contact . "					\
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH 6/7] Fix proper type handling in contact_query_from_uri
From: Bartosz Szatkowski @ 2010-11-10 13:15 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bartosz Szatkowski
In-Reply-To: <1289394930-6694-1-git-send-email-bulislaw@linux.com>

Previously all phone numbers, addresses and emails was considered to be "work".
Now there are three working types for emails and addresses: "work", "home",
"other" and four for phone numbers - these three as well as "cell".
---
 plugins/phonebook-tracker.c |   38 ++++++++++++++++++++++++++------------
 1 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/plugins/phonebook-tracker.c b/plugins/phonebook-tracker.c
index 616cc96..f195f3f 100644
--- a/plugins/phonebook-tracker.c
+++ b/plugins/phonebook-tracker.c
@@ -767,41 +767,55 @@
 	"} GROUP BY ?call ORDER BY DESC(nmo:receivedDate(?call))"
 
 #define CONTACTS_QUERY_FROM_URI						\
-	"SELECT ?v nco:fullname(<%s>) "					\
+	"SELECT nco:phoneNumber(?v) nco:fullname(<%s>) "		\
 	"nco:nameFamily(<%s>) nco:nameGiven(<%s>) "			\
 	"nco:nameAdditional(<%s>) nco:nameHonorificPrefix(<%s>) "	\
 	"nco:nameHonorificSuffix(<%s>) nco:emailAddress(?e) "		\
 	"nco:phoneNumber(?w) nco:pobox(?p) nco:extendedAddress(?p) "	\
 	"nco:streetAddress(?p) nco:locality(?p) nco:region(?p) "	\
-	"nco:postalcode(?p) nco:country(?p) ?f  nco:emailAddress(?ew)"	\
+	"nco:postalcode(?p) nco:country(?p) ?f nco:emailAddress(?ew) "	\
 	"nco:birthDate(<%s>) nco:nickname(<%s>) nco:url(<%s>) "		\
 	"nco:photo(<%s>) nco:fullname(?o) nco:department(?a) "		\
 	"nco:role(?a) nco:pobox(?pw) nco:extendedAddress(?pw) "		\
 	"nco:streetAddress(?pw) nco:locality(?pw) nco:region(?pw) "	\
 	"nco:postalcode(?pw) nco:country(?pw) nco:contactUID(<%s>) "	\
-	"nco:title(?a) nco:phoneNumber(?t) "				\
+	"nco:title(?a) ?t nco:pobox(?po) nco:extendedAddress(?po) "	\
+	"nco:streetAddress(?po) nco:locality(?po) nco:region(?po) "	\
+        "nco:postalcode(?po) nco:country(?po) nco:emailAddress(?eo) "	\
+	"?vc "								\
 	"\"NOTACALL\" \"false\" \"false\" <%s> "			\
 	"WHERE { "							\
-		"<%s> a nco:Contact . "					\
-	"OPTIONAL { <%s> nco:hasPhoneNumber ?h . 			\
-		OPTIONAL {"						\
+		"<%s> a nco:PersonContact . "				\
+	"OPTIONAL { <%s> nco:hasPhoneNumber ?h . "			\
+		"OPTIONAL {"						\
 		"?h a nco:FaxNumber ; "					\
 		"nco:phoneNumber ?f . "					\
 		"}"							\
 		"OPTIONAL {"						\
+		"?h a nco:CellPhoneNumber ; "				\
+		"nco:phoneNumber ?vc"					\
+		"}"							\
+		"OPTIONAL {"						\
 		"?h a nco:VoicePhoneNumber ; "				\
-		"nco:phoneNumber ?v"					\
+		"nco:phoneNumber ?t"					\
 		"}"							\
 	"}"								\
-	"OPTIONAL { <%s> nco:hasEmailAddress ?e . } "			\
-	"OPTIONAL { <%s> nco:hasPostalAddress ?p . } "			\
 	"OPTIONAL { "							\
 		"<%s> nco:hasAffiliation ?a . "				\
-		"OPTIONAL { ?a nco:hasPhoneNumber ?w . }" 		\
-		"OPTIONAL { ?a nco:hasEmailAddress ?ew . }"		\
-		"OPTIONAL { ?a nco:hasPostalAddress ?pw . } "		\
+		"OPTIONAL { ?a rdfs:label \"Work\" . "			\
+			"OPTIONAL { ?a nco:hasEmailAddress ?ew . } "	\
+			"OPTIONAL { ?a nco:hasPostalAddress ?pw . } "	\
+			"OPTIONAL { ?a nco:hasPhoneNumber ?w . } "	\
+		"}"							\
+		"OPTIONAL { ?a rdfs:label \"Home\" . "			\
+			"OPTIONAL { ?a nco:hasEmailAddress ?e . } "	\
+			"OPTIONAL { ?a nco:hasPostalAddress ?p . } "	\
+			"OPTIONAL { ?a nco:hasPhoneNumber ?v . } "	\
+		"}"							\
 		"OPTIONAL { ?a nco:org ?o . } "				\
 	"} "								\
+	"OPTIONAL { <%s> nco:hasPostalAddress ?po . } "			\
+	"OPTIONAL { <%s> nco:hasEmailAddress ?eo . } "			\
 	"}"
 
 #define CONTACTS_OTHER_QUERY_FROM_URI					\
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH 5/7] Fix proper type handling in combined_calls_query
From: Bartosz Szatkowski @ 2010-11-10 13:15 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bartosz Szatkowski
In-Reply-To: <1289394930-6694-1-git-send-email-bulislaw@linux.com>

Previously all phone numbers, addresses and emails was considered to be "work".
Now there are three working types for emails and addresses: "work", "home",
"other" and four for phone numbers - these three as well as "cell".
---
 plugins/phonebook-tracker.c |  152 +++++++++++++++++++++++++++++++++---------
 1 files changed, 119 insertions(+), 33 deletions(-)

diff --git a/plugins/phonebook-tracker.c b/plugins/phonebook-tracker.c
index c40eb63..616cc96 100644
--- a/plugins/phonebook-tracker.c
+++ b/plugins/phonebook-tracker.c
@@ -536,11 +536,11 @@
 	"} GROUP BY ?call ORDER BY DESC(nmo:sentDate(?call))"
 
 #define COMBINED_CALLS_QUERY						\
-	"SELECT nco:phoneNumber(?h) nco:fullname(?c) "			\
+	"SELECT ?h nco:fullname(?c) "					\
 	"nco:nameFamily(?c) nco:nameGiven(?c) "				\
 	"nco:nameAdditional(?c) nco:nameHonorificPrefix(?c) "		\
 	"nco:nameHonorificSuffix(?c) nco:emailAddress(?e) "		\
-	"nco:phoneNumber(?w) nco:pobox(?p) nco:extendedAddress(?p) "	\
+	"?w nco:pobox(?p) nco:extendedAddress(?p) "			\
 	"nco:streetAddress(?p) nco:locality(?p) nco:region(?p) "	\
 	"nco:postalcode(?p) nco:country(?p) \"\" nco:emailAddress(?ew) "\
 	"nco:birthDate(?c) nco:nickname(?c) nco:url(?c) "		\
@@ -548,7 +548,11 @@
 	"nco:role(?a) nco:pobox(?pw) nco:extendedAddress(?pw) "		\
 	"nco:streetAddress(?pw) nco:locality(?pw) nco:region(?pw) "	\
 	"nco:postalcode(?pw) nco:country(?pw) nco:contactUID(?c) "	\
-	"nco:title(?a) nco:phoneNumber(?t) nmo:receivedDate(?call) "	\
+	"nco:title(?a) nco:phoneNumber(?t) nco:pobox(?po) "		\
+	"nco:extendedAddress(?po) nco:streetAddress(?po) "		\
+	"nco:locality(?po) nco:region(?po) nco:postalcode(?po) "	\
+	"nco:country(?po) nco:emailAddress(?eo) ?vc "			\
+	"nmo:receivedDate(?call) "					\
 	"nmo:isSent(?call) nmo:isAnswered(?call) ?x "			\
 	"WHERE { "							\
 	"{ "								\
@@ -557,74 +561,156 @@
 		"?call a nmo:Call ; "					\
 		"nmo:to ?x ; "						\
 		"nmo:isSent true . "					\
-	"} UNION { "							\
-		"?x a nco:Contact . "					\
-		"?x nco:hasPhoneNumber ?h . "				\
-		"?call a nmo:Call ; "					\
-		"nmo:to ?x ; "						\
-		"nmo:isSent true . "					\
 		"?c a nco:PersonContact . "				\
-		"?c nco:hasPhoneNumber ?h . "				\
-		"OPTIONAL { ?c nco:hasEmailAddress ?e . } "		\
-		"OPTIONAL { ?c nco:hasPostalAddress ?p . } "		\
+		"?c nco:hasPhoneNumber ?t . "				\
+		"OPTIONAL { "						\
+			"?t a nco:CellPhoneNumber ; "			\
+				"nco:phoneNumber ?vc . "		\
+		"} "							\
+		"OPTIONAL { ?c nco:hasPostalAddress ?po . } "		\
+		"OPTIONAL { ?c nco:hasEmailAddress ?eo . } "		\
 		"OPTIONAL { "						\
 			"?c nco:hasAffiliation ?a . "			\
+			"OPTIONAL { "					\
+			"?a rdfs:label \"Work\" . "			\
 			"OPTIONAL { ?a nco:hasEmailAddress ?ew . } "	\
 			"OPTIONAL { ?a nco:hasPostalAddress ?pw . } "	\
+			"}"						\
+			"OPTIONAL { "					\
+			"?a rdfs:label \"Home\" . "			\
+			"OPTIONAL { ?a nco:hasEmailAddress ?e . } "	\
+			"OPTIONAL { ?a nco:hasPostalAddress ?p . } "	\
+			"} "						\
 			"OPTIONAL { ?a nco:org ?o . } "			\
 		"} "							\
 	"} UNION { "							\
 		"?x a nco:Contact . "					\
-		"?x nco:hasPhoneNumber ?w . "				\
+		"?x nco:hasPhoneNumber ?tmp . "				\
 		"?call a nmo:Call ; "					\
 		"nmo:to ?x ; "						\
 		"nmo:isSent true . "					\
 		"?c a nco:PersonContact . "				\
-		"OPTIONAL { ?c nco:hasEmailAddress ?e . } "		\
-		"OPTIONAL { ?c nco:hasPostalAddress ?p . } "		\
 		"?c nco:hasAffiliation ?a . "				\
-		"?a nco:hasPhoneNumber ?w . "				\
-		"OPTIONAL { ?a nco:hasEmailAddress ?ew . } "		\
-		"OPTIONAL { ?a nco:hasPostalAddress ?pw . } "		\
-		"OPTIONAL { ?a nco:org ?o . } "				\
+		"?a nco:hasPhoneNumber ?tmp . "				\
+		"OPTIONAL { "						\
+			"?a rdfs:label \"Work\" . "			\
+			"?tmp nco:phoneNumber ?w . "			\
+			"OPTIONAL { ?a nco:hasEmailAddress ?ew . } "	\
+			"OPTIONAL { ?a nco:hasPostalAddress ?pw . } "	\
+			"OPTIONAL { ?a nco:org ?o . } "			\
+			"{ "						\
+			"SELECT ?p ?e ?c WHERE { "			\
+			"?c nco:hasAffiliation ?b . "			\
+			"?b rdfs:label \"Home\" . "			\
+			"OPTIONAL {?b nco:hasEmailAddress ?e . } "	\
+			"OPTIONAL {?b nco:hasPostalAddress ?p . }} "	\
+			"} "						\
+		"}"							\
+		"OPTIONAL { "						\
+			"?a rdfs:label \"Home\" . "			\
+			"?tmp nco:phoneNumber ?h . "			\
+			"OPTIONAL { ?a nco:hasEmailAddress ?e . } "	\
+			"OPTIONAL { ?a nco:hasPostalAddress ?p . } "	\
+			"OPTIONAL { ?a nco:org ?o . } "			\
+			"{ "						\
+			"SELECT ?pw ?ew ?c WHERE { "			\
+			"?c nco:hasAffiliation ?b . "			\
+			"?b rdfs:label \"Work\" . "			\
+			"OPTIONAL {?b nco:hasEmailAddress ?ew . } "	\
+			"OPTIONAL {?b nco:hasPostalAddress ?pw . }} "	\
+			"} "						\
+		"}"							\
 	"} UNION { "							\
 		"?x a nco:Contact . "					\
 		"?x nco:hasPhoneNumber ?t . "				\
 		"?call a nmo:Call ; "					\
-		"nmo:from ?x ; "					\
-		"nmo:isSent false . "					\
+		"nmo:to ?x ; "						\
+		"nmo:isSent true . "					\
+		"OPTIONAL {?c a nco:PersonContact ; "			\
+			"nco:hasPhoneNumber ?t . } "			\
+		"OPTIONAL {?c a nco:PersonContact ; "			\
+			"nco:hasAffiliation ?a . "			\
+			"?a nco:hasPhoneNumber ?t . } "			\
+		"FILTER ( !bound(?c) && !bound(?a) ) . "		\
 	"} UNION { "							\
 		"?x a nco:Contact . "					\
-		"?x nco:hasPhoneNumber ?h . "				\
+		"?x nco:hasPhoneNumber ?t . "				\
 		"?call a nmo:Call ; "					\
 		"nmo:from ?x ; "					\
 		"nmo:isSent false . "					\
 		"?c a nco:PersonContact . "				\
-		"?c nco:hasPhoneNumber ?h . "				\
-		"OPTIONAL { ?c nco:hasEmailAddress ?e . } "		\
-		"OPTIONAL { ?c nco:hasPostalAddress ?p . } "		\
+		"?c nco:hasPhoneNumber ?t . "				\
+		"OPTIONAL { "						\
+			"?t a nco:CellPhoneNumber ; "			\
+				"nco:phoneNumber ?vc . "		\
+		"} "							\
+		"OPTIONAL { ?c nco:hasPostalAddress ?po . } "		\
+		"OPTIONAL { ?c nco:hasEmailAddress ?eo . } "		\
 		"OPTIONAL { "						\
 			"?c nco:hasAffiliation ?a . "			\
+			"OPTIONAL { "					\
+			"?a rdfs:label \"Work\" . "			\
 			"OPTIONAL { ?a nco:hasEmailAddress ?ew . } "	\
 			"OPTIONAL { ?a nco:hasPostalAddress ?pw . } "	\
+			"}"						\
+			"OPTIONAL { "					\
+			"?a rdfs:label \"Home\" . "			\
+			"OPTIONAL { ?a nco:hasEmailAddress ?e . } "	\
+			"OPTIONAL { ?a nco:hasPostalAddress ?p . } "	\
+			"} "						\
 			"OPTIONAL { ?a nco:org ?o . } "			\
 		"} "							\
 	"} UNION { "							\
 		"?x a nco:Contact . "					\
-		"?x nco:hasPhoneNumber ?w . "				\
+		"?x nco:hasPhoneNumber ?tmp . "				\
 		"?call a nmo:Call ; "					\
 		"nmo:from ?x ; "					\
 		"nmo:isSent false . "					\
 		"?c a nco:PersonContact . "				\
-		"OPTIONAL { ?c nco:hasEmailAddress ?e . } "		\
-		"OPTIONAL { ?c nco:hasPostalAddress ?p . } "		\
 		"?c nco:hasAffiliation ?a . "				\
-		"?a nco:hasPhoneNumber ?w . "				\
-		"OPTIONAL { ?a nco:hasEmailAddress ?ew . } "		\
-		"OPTIONAL { ?a nco:hasPostalAddress ?pw . } "		\
-		"OPTIONAL { ?a nco:org ?o . } "				\
+		"?a nco:hasPhoneNumber ?tmp . "				\
+		"OPTIONAL { "						\
+			"?a rdfs:label \"Work\" . "			\
+			"?tmp nco:phoneNumber ?w . "			\
+			"OPTIONAL { ?a nco:hasEmailAddress ?ew . } "	\
+			"OPTIONAL { ?a nco:hasPostalAddress ?pw . } "	\
+			"OPTIONAL { ?a nco:org ?o . } "			\
+			"{ "						\
+			"SELECT ?p ?e ?c WHERE { "			\
+			"?c nco:hasAffiliation ?b . "			\
+			"?b rdfs:label \"Home\" . "			\
+			"OPTIONAL {?b nco:hasEmailAddress ?e . } "	\
+			"OPTIONAL {?b nco:hasPostalAddress ?p . }} "	\
+			"} "						\
+		"}"							\
+		"OPTIONAL { "						\
+			"?a rdfs:label \"Home\" . "			\
+			"?tmp nco:phoneNumber ?h . "			\
+			"OPTIONAL { ?a nco:hasEmailAddress ?e . } "	\
+			"OPTIONAL { ?a nco:hasPostalAddress ?p . } "	\
+			"OPTIONAL { ?a nco:org ?o . } "			\
+			"{ "						\
+			"SELECT ?pw ?ew ?c WHERE { "			\
+			"?c nco:hasAffiliation ?b . "			\
+			"?b rdfs:label \"Work\" . "			\
+			"OPTIONAL {?b nco:hasEmailAddress ?ew . } "	\
+			"OPTIONAL {?b nco:hasPostalAddress ?pw . }} "	\
+			"} "						\
+		"}"							\
+	"} UNION { "							\
+		"?x a nco:Contact . "					\
+		"?x nco:hasPhoneNumber ?t . "				\
+		"?call a nmo:Call ; "					\
+		"nmo:from ?x ; "					\
+		"nmo:isSent false . "					\
+		"OPTIONAL {?c a nco:PersonContact ; "			\
+			"nco:hasPhoneNumber ?t . } "			\
+		"OPTIONAL {?c a nco:PersonContact ; "			\
+			"nco:hasAffiliation ?a . "			\
+			"?a nco:hasPhoneNumber ?t . } "			\
+		"FILTER ( !bound(?c) && !bound(?a) ) . "		\
 	"} "								\
-	"} GROUP BY ?call ORDER BY DESC(nmo:receivedDate(?call)) "
+	"} ORDER BY DESC(nmo:receivedDate(?call)) "
 
 #define COMBINED_CALLS_LIST						\
 	"SELECT ?c nco:nameFamily(?c) nco:nameGiven(?c) "		\
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH 4/7] Fix proper type handling in outgoing_calls_query
From: Bartosz Szatkowski @ 2010-11-10 13:15 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bartosz Szatkowski
In-Reply-To: <1289394930-6694-1-git-send-email-bulislaw@linux.com>

Previously all phone numbers, addresses and emails was considered to be "work".
Now there are three working types for emails and addresses: "work", "home",
"other" and four for phone numbers - these three as well as "cell".
---
 plugins/phonebook-tracker.c |   85 +++++++++++++++++++++++++++++++++----------
 1 files changed, 65 insertions(+), 20 deletions(-)

diff --git a/plugins/phonebook-tracker.c b/plugins/phonebook-tracker.c
index bd67699..c40eb63 100644
--- a/plugins/phonebook-tracker.c
+++ b/plugins/phonebook-tracker.c
@@ -404,19 +404,23 @@
 	"} GROUP BY ?call ORDER BY DESC(nmo:receivedDate(?call))"
 
 #define OUTGOING_CALLS_QUERY						\
-	"SELECT nco:phoneNumber(?h) nco:fullname(?c) "			\
+	"SELECT ?h nco:fullname(?c) "					\
 	"nco:nameFamily(?c) nco:nameGiven(?c) "				\
 	"nco:nameAdditional(?c) nco:nameHonorificPrefix(?c) "		\
 	"nco:nameHonorificSuffix(?c) nco:emailAddress(?e) "		\
-	"nco:phoneNumber(?w) nco:pobox(?p) nco:extendedAddress(?p) "	\
+	"?w nco:pobox(?p) nco:extendedAddress(?p) "			\
 	"nco:streetAddress(?p) nco:locality(?p) nco:region(?p) "	\
-	"nco:postalcode(?p) nco:country(?p) \"\" nco:emailAddress(?ew)"	\
+	"nco:postalcode(?p) nco:country(?p) \"\" nco:emailAddress(?ew) "\
 	"nco:birthDate(?c) nco:nickname(?c) nco:url(?c) "		\
 	"nco:photo(?c) nco:fullname(?o) nco:department(?a) "		\
 	"nco:role(?a) nco:pobox(?pw) nco:extendedAddress(?pw) "		\
 	"nco:streetAddress(?pw) nco:locality(?pw) nco:region(?pw) "	\
 	"nco:postalcode(?pw) nco:country(?pw) nco:contactUID(?c) "	\
-	"nco:title(?a) nco:phoneNumber(?t) nmo:receivedDate(?call) "	\
+	"nco:title(?a) nco:phoneNumber(?t) nco:pobox(?po) "		\
+	"nco:extendedAddress(?po) nco:streetAddress(?po) "		\
+	"nco:locality(?po) nco:region(?po) nco:postalcode(?po) "	\
+	"nco:country(?po) nco:emailAddress(?eo) ?vc "			\
+	"nmo:receivedDate(?call) "					\
 	"nmo:isSent(?call) nmo:isAnswered(?call) ?x "			\
 	"WHERE { "							\
 	"{ "								\
@@ -425,38 +429,79 @@
 		"?call a nmo:Call ; "					\
 		"nmo:to ?x ; "						\
 		"nmo:isSent true . "					\
+		"?c a nco:PersonContact . "				\
+		"?c nco:hasPhoneNumber ?t . "				\
+		"OPTIONAL { "						\
+			"?t a nco:CellPhoneNumber ; "			\
+				"nco:phoneNumber ?vc . "		\
+		"} "							\
+		"OPTIONAL { ?c nco:hasPostalAddress ?po . } "		\
+		"OPTIONAL { ?c nco:hasEmailAddress ?eo . } "		\
+		"OPTIONAL { "						\
+			"?c nco:hasAffiliation ?a . "			\
+			"OPTIONAL { "					\
+			"?a rdfs:label \"Work\" . "			\
+			"OPTIONAL { ?a nco:hasEmailAddress ?ew . } "	\
+			"OPTIONAL { ?a nco:hasPostalAddress ?pw . } "	\
+			"}"						\
+			"OPTIONAL { "					\
+			"?a rdfs:label \"Home\" . "			\
+			"OPTIONAL { ?a nco:hasEmailAddress ?e . } "	\
+			"OPTIONAL { ?a nco:hasPostalAddress ?p . } "	\
+			"} "						\
+			"OPTIONAL { ?a nco:org ?o . } "			\
+		"} "							\
 	"} UNION { "							\
 		"?x a nco:Contact . "					\
-		"?x nco:hasPhoneNumber ?h . "				\
+		"?x nco:hasPhoneNumber ?tmp . "				\
 		"?call a nmo:Call ; "					\
 		"nmo:to ?x ; "						\
 		"nmo:isSent true . "					\
 		"?c a nco:PersonContact . "				\
-		"?c nco:hasPhoneNumber ?h . "				\
-		"OPTIONAL { ?c nco:hasEmailAddress ?e . } "		\
-		"OPTIONAL { ?c nco:hasPostalAddress ?p . } "		\
+		"?c nco:hasAffiliation ?a . "				\
+		"?a nco:hasPhoneNumber ?tmp . "				\
 		"OPTIONAL { "						\
-			"?c nco:hasAffiliation ?a . "			\
+			"?a rdfs:label \"Work\" . "			\
+			"?tmp nco:phoneNumber ?w . "			\
 			"OPTIONAL { ?a nco:hasEmailAddress ?ew . } "	\
 			"OPTIONAL { ?a nco:hasPostalAddress ?pw . } "	\
 			"OPTIONAL { ?a nco:org ?o . } "			\
-		"} "							\
+			"{ "						\
+			"SELECT ?p ?e ?c WHERE { "			\
+			"?c nco:hasAffiliation ?b . "			\
+			"?b rdfs:label \"Home\" . "			\
+			"OPTIONAL {?b nco:hasEmailAddress ?e . } "	\
+			"OPTIONAL {?b nco:hasPostalAddress ?p . }} "	\
+			"} "						\
+		"}"							\
+		"OPTIONAL { "						\
+			"?a rdfs:label \"Home\" . "			\
+			"?tmp nco:phoneNumber ?h . "			\
+			"OPTIONAL { ?a nco:hasEmailAddress ?e . } "	\
+			"OPTIONAL { ?a nco:hasPostalAddress ?p . } "	\
+			"OPTIONAL { ?a nco:org ?o . } "			\
+			"{ "						\
+			"SELECT ?pw ?ew ?c WHERE { "			\
+			"?c nco:hasAffiliation ?b . "			\
+			"?b rdfs:label \"Work\" . "			\
+			"OPTIONAL {?b nco:hasEmailAddress ?ew . } "	\
+			"OPTIONAL {?b nco:hasPostalAddress ?pw . }} "	\
+			"} "						\
+		"}"							\
 	"} UNION { "							\
 		"?x a nco:Contact . "					\
-		"?x nco:hasPhoneNumber ?w . "				\
+		"?x nco:hasPhoneNumber ?t . "				\
 		"?call a nmo:Call ; "					\
 		"nmo:to ?x ; "						\
 		"nmo:isSent true . "					\
-		"?c a nco:PersonContact . "				\
-		"OPTIONAL { ?c nco:hasEmailAddress ?e . } "		\
-		"OPTIONAL { ?c nco:hasPostalAddress ?p . } "		\
-		"?c nco:hasAffiliation ?a . "				\
-		"?a nco:hasPhoneNumber ?w . "				\
-		"OPTIONAL { ?a nco:hasEmailAddress ?ew . } "		\
-		"OPTIONAL { ?a nco:hasPostalAddress ?pw . } "		\
-		"OPTIONAL { ?a nco:org ?o . } "				\
+		"OPTIONAL {?c a nco:PersonContact ; "			\
+			"nco:hasPhoneNumber ?t . } "			\
+		"OPTIONAL {?c a nco:PersonContact ; "			\
+			"nco:hasAffiliation ?a . "			\
+			"?a nco:hasPhoneNumber ?t . } "			\
+		"FILTER ( !bound(?c) && !bound(?a) ) . "		\
 	"} "								\
-	"} GROUP BY ?call ORDER BY DESC(nmo:sentDate(?call)) "
+	"} ORDER BY DESC(nmo:sentDate(?call)) "
 
 #define OUTGOING_CALLS_LIST						\
 	"SELECT ?c nco:nameFamily(?c) "					\
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH 3/7] Fix proper type handling in incoming_calls_query
From: Bartosz Szatkowski @ 2010-11-10 13:15 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bartosz Szatkowski
In-Reply-To: <1289394930-6694-1-git-send-email-bulislaw@linux.com>

Previously all phone numbers, addresses and emails was considered to be "work".
Now there are three working types for emails and addresses: "work", "home",
"other" and four for phone numbers - these three as well as "cell".
---
 plugins/phonebook-tracker.c |   83 +++++++++++++++++++++++++++++++++----------
 1 files changed, 64 insertions(+), 19 deletions(-)

diff --git a/plugins/phonebook-tracker.c b/plugins/phonebook-tracker.c
index e84c589..bd67699 100644
--- a/plugins/phonebook-tracker.c
+++ b/plugins/phonebook-tracker.c
@@ -266,11 +266,11 @@
 	"} GROUP BY ?call ORDER BY DESC(nmo:receivedDate(?call))"
 
 #define INCOMING_CALLS_QUERY						\
-	"SELECT nco:phoneNumber(?h) nco:fullname(?c) "			\
+	"SELECT ?h nco:fullname(?c) "					\
 	"nco:nameFamily(?c) nco:nameGiven(?c) "				\
 	"nco:nameAdditional(?c) nco:nameHonorificPrefix(?c) "		\
 	"nco:nameHonorificSuffix(?c) nco:emailAddress(?e) "		\
-	"nco:phoneNumber(?w) nco:pobox(?p) nco:extendedAddress(?p) "	\
+	"?w nco:pobox(?p) nco:extendedAddress(?p) "			\
 	"nco:streetAddress(?p) nco:locality(?p) nco:region(?p) "	\
 	"nco:postalcode(?p) nco:country(?p) \"\" nco:emailAddress(?ew) "\
 	"nco:birthDate(?c) nco:nickname(?c) nco:url(?c) "		\
@@ -278,7 +278,11 @@
 	"nco:role(?a) nco:pobox(?pw) nco:extendedAddress(?pw) "		\
 	"nco:streetAddress(?pw) nco:locality(?pw) nco:region(?pw) "	\
 	"nco:postalcode(?pw) nco:country(?pw) nco:contactUID(?c) "	\
-	"nco:title(?a) nco:phoneNumber(?t) nmo:receivedDate(?call) "	\
+	"nco:title(?a) nco:phoneNumber(?t) nco:pobox(?po) "		\
+	"nco:extendedAddress(?po) nco:streetAddress(?po) "		\
+	"nco:locality(?po) nco:region(?po) nco:postalcode(?po) "	\
+	"nco:country(?po) nco:emailAddress(?eo) ?vc "			\
+	"nmo:receivedDate(?call) "					\
 	"nmo:isSent(?call) nmo:isAnswered(?call) ?x "			\
 	"WHERE { "							\
 	"{ "								\
@@ -288,40 +292,81 @@
 		"nmo:from ?x ; "					\
 		"nmo:isSent false ; "					\
 		"nmo:isAnswered true . "				\
+		"?c a nco:PersonContact . "				\
+		"?c nco:hasPhoneNumber ?t . "				\
+		"OPTIONAL { "						\
+			"?t a nco:CellPhoneNumber ; "			\
+				"nco:phoneNumber ?vc . "		\
+		"} "							\
+		"OPTIONAL { ?c nco:hasPostalAddress ?po . } "		\
+		"OPTIONAL { ?c nco:hasEmailAddress ?eo . } "		\
+		"OPTIONAL { "						\
+			"?c nco:hasAffiliation ?a . "			\
+			"OPTIONAL { "					\
+			"?a rdfs:label \"Work\" . "			\
+			"OPTIONAL { ?a nco:hasEmailAddress ?ew . } "	\
+			"OPTIONAL { ?a nco:hasPostalAddress ?pw . } "	\
+			"}"						\
+			"OPTIONAL { "					\
+			"?a rdfs:label \"Home\" . "			\
+			"OPTIONAL { ?a nco:hasEmailAddress ?e . } "	\
+			"OPTIONAL { ?a nco:hasPostalAddress ?p . } "	\
+			"} "						\
+			"OPTIONAL { ?a nco:org ?o . } "			\
+		"} "							\
 	"} UNION { "							\
 		"?x a nco:Contact . "					\
-		"?x nco:hasPhoneNumber ?h . "				\
+		"?x nco:hasPhoneNumber ?tmp . "				\
 		"?call a nmo:Call ; "					\
 		"nmo:from ?x ; "					\
 		"nmo:isSent false ; "					\
 		"nmo:isAnswered true . "				\
 		"?c a nco:PersonContact . "				\
-		"?c nco:hasPhoneNumber ?h . "				\
-		"OPTIONAL { ?c nco:hasEmailAddress ?e . } "		\
-		"OPTIONAL { ?c nco:hasPostalAddress ?p . } "		\
+		"?c nco:hasAffiliation ?a . "				\
+		"?a nco:hasPhoneNumber ?tmp . "				\
 		"OPTIONAL { "						\
-			"?c nco:hasAffiliation ?a . "			\
+			"?a rdfs:label \"Work\" . "			\
+			"?tmp nco:phoneNumber ?w . "			\
 			"OPTIONAL { ?a nco:hasEmailAddress ?ew . } "	\
 			"OPTIONAL { ?a nco:hasPostalAddress ?pw . } "	\
 			"OPTIONAL { ?a nco:org ?o . } "			\
-		"} "							\
+			"{ "						\
+			"SELECT ?p ?e ?c WHERE { "			\
+			"?c nco:hasAffiliation ?b . "			\
+			"?b rdfs:label \"Home\" . "			\
+			"OPTIONAL {?b nco:hasEmailAddress ?e . } "	\
+			"OPTIONAL {?b nco:hasPostalAddress ?p . }} "	\
+			"} "						\
+		"}"							\
+		"OPTIONAL { "						\
+			"?a rdfs:label \"Home\" . "			\
+			"?tmp nco:phoneNumber ?h . "			\
+			"OPTIONAL { ?a nco:hasEmailAddress ?e . } "	\
+			"OPTIONAL { ?a nco:hasPostalAddress ?p . } "	\
+			"OPTIONAL { ?a nco:org ?o . } "			\
+			"{ "						\
+			"SELECT ?pw ?ew ?c WHERE { "			\
+			"?c nco:hasAffiliation ?b . "			\
+			"?b rdfs:label \"Work\" . "			\
+			"OPTIONAL {?b nco:hasEmailAddress ?ew . } "	\
+			"OPTIONAL {?b nco:hasPostalAddress ?pw . }} "	\
+			"} "						\
+		"}"							\
 	"} UNION { "							\
 		"?x a nco:Contact . "					\
-		"?x nco:hasPhoneNumber ?w . "				\
+		"?x nco:hasPhoneNumber ?t . "				\
 		"?call a nmo:Call ; "					\
 		"nmo:from ?x ; "					\
 		"nmo:isSent false ; "					\
 		"nmo:isAnswered true . "				\
-		"?c a nco:PersonContact . "				\
-		"OPTIONAL { ?c nco:hasEmailAddress ?e . } "		\
-		"OPTIONAL { ?c nco:hasPostalAddress ?p . } "		\
-		"?c nco:hasAffiliation ?a . "				\
-		"?a nco:hasPhoneNumber ?w . "				\
-		"OPTIONAL { ?a nco:hasEmailAddress ?ew . } "		\
-		"OPTIONAL { ?a nco:hasPostalAddress ?pw . } "		\
-		"OPTIONAL { ?a nco:org ?o . } "				\
+		"OPTIONAL {?c a nco:PersonContact ; "			\
+			"nco:hasPhoneNumber ?t . } "			\
+		"OPTIONAL {?c a nco:PersonContact ; "			\
+			"nco:hasAffiliation ?a . "			\
+			"?a nco:hasPhoneNumber ?t . } "			\
+		"FILTER ( !bound(?c) && !bound(?a) ) . "		\
 	"} "								\
-	"} GROUP BY ?call ORDER BY DESC(nmo:receivedDate(?call)) "
+	"} ORDER BY DESC(nmo:receivedDate(?call)) "
 
 #define INCOMING_CALLS_LIST						\
 	"SELECT ?c nco:nameFamily(?c) "					\
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH 2/7] Fix proper type handling in missed_calls_query
From: Bartosz Szatkowski @ 2010-11-10 13:15 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bartosz Szatkowski
In-Reply-To: <1289394930-6694-1-git-send-email-bulislaw@linux.com>

Previously all phone numbers, addresses and emails was considered to be "work".
Now there are three working types for emails and addresses: "work", "home",
"other" and four for phone numbers - these three as well as "cell".
---
 plugins/phonebook-tracker.c |   84 +++++++++++++++++++++++++++++++++----------
 1 files changed, 65 insertions(+), 19 deletions(-)

diff --git a/plugins/phonebook-tracker.c b/plugins/phonebook-tracker.c
index 83327e0..e84c589 100644
--- a/plugins/phonebook-tracker.c
+++ b/plugins/phonebook-tracker.c
@@ -127,11 +127,11 @@
 	"} GROUP BY ?c"
 
 #define MISSED_CALLS_QUERY						\
-	"SELECT nco:phoneNumber(?h) nco:fullname(?c) "			\
+	"SELECT ?h nco:fullname(?c) "					\
 	"nco:nameFamily(?c) nco:nameGiven(?c) "				\
 	"nco:nameAdditional(?c) nco:nameHonorificPrefix(?c) "		\
 	"nco:nameHonorificSuffix(?c) nco:emailAddress(?e) "		\
-	"nco:phoneNumber(?w) nco:pobox(?p) nco:extendedAddress(?p) "	\
+	"?w nco:pobox(?p) nco:extendedAddress(?p) "			\
 	"nco:streetAddress(?p) nco:locality(?p) nco:region(?p) "	\
 	"nco:postalcode(?p) nco:country(?p) \"\" nco:emailAddress(?ew) "\
 	"nco:birthDate(?c) nco:nickname(?c) nco:url(?c) "		\
@@ -139,7 +139,11 @@
 	"nco:role(?a) nco:pobox(?pw) nco:extendedAddress(?pw) "		\
 	"nco:streetAddress(?pw) nco:locality(?pw) nco:region(?pw) "	\
 	"nco:postalcode(?pw) nco:country(?pw) nco:contactUID(?c) "	\
-	"nco:title(?a) nco:phoneNumber(?t) nmo:receivedDate(?call) "	\
+	"nco:title(?a) nco:phoneNumber(?t) nco:pobox(?po) "		\
+	"nco:extendedAddress(?po) nco:streetAddress(?po) "		\
+	"nco:locality(?po) nco:region(?po) nco:postalcode(?po) "	\
+	"nco:country(?po) nco:emailAddress(?eo) ?vc "			\
+	"nmo:receivedDate(?call) "					\
 	"nmo:isSent(?call) nmo:isAnswered(?call) ?x "			\
 	"WHERE { "							\
 	"{ "								\
@@ -149,40 +153,82 @@
 		"nmo:from ?x ; "					\
 		"nmo:isSent false ; "					\
 		"nmo:isAnswered false . "				\
+		"?c a nco:PersonContact . "				\
+		"?c nco:hasPhoneNumber ?t . "				\
+		"OPTIONAL { "						\
+			"?t a nco:CellPhoneNumber ; "			\
+				"nco:phoneNumber ?vc . "		\
+		"} "							\
+		"OPTIONAL { ?c nco:hasPostalAddress ?po . } "		\
+		"OPTIONAL { ?c nco:hasEmailAddress ?eo . } "		\
+		"OPTIONAL { "						\
+			"?c nco:hasAffiliation ?a . "			\
+			"OPTIONAL { "					\
+			"?a rdfs:label \"Work\" . "			\
+			"OPTIONAL { ?a nco:hasEmailAddress ?ew . } "	\
+			"OPTIONAL { ?a nco:hasPostalAddress ?pw . } "	\
+			"}"						\
+			"OPTIONAL { "					\
+			"?a rdfs:label \"Home\" . "			\
+			"OPTIONAL { ?a nco:hasEmailAddress ?e . } "	\
+			"OPTIONAL { ?a nco:hasPostalAddress ?p . } "	\
+			"} "						\
+			"OPTIONAL { ?a nco:org ?o . } "			\
+		"} "							\
 	"} UNION { "							\
 		"?x a nco:Contact . "					\
-		"?x nco:hasPhoneNumber ?h . "				\
+		"?x nco:hasPhoneNumber ?tmp . "				\
 		"?call a nmo:Call ; "					\
 		"nmo:from ?x ; "					\
 		"nmo:isSent false ; "					\
 		"nmo:isAnswered false . "				\
 		"?c a nco:PersonContact . "				\
-		"?c nco:hasPhoneNumber ?h . "				\
-		"OPTIONAL { ?c nco:hasEmailAddress ?e . } "		\
-		"OPTIONAL { ?c nco:hasPostalAddress ?p . } "		\
+		"?c nco:hasAffiliation ?a . "				\
+		"?a nco:hasPhoneNumber ?tmp . "				\
 		"OPTIONAL { "						\
-			"?c nco:hasAffiliation ?a . "			\
+			"?a rdfs:label \"Work\" . "			\
+			"?tmp nco:phoneNumber ?w . "			\
 			"OPTIONAL { ?a nco:hasEmailAddress ?ew . } "	\
 			"OPTIONAL { ?a nco:hasPostalAddress ?pw . } "	\
 			"OPTIONAL { ?a nco:org ?o . } "			\
-		"} "							\
+			"{ "						\
+			"SELECT ?p ?e ?c WHERE { "			\
+			"?c nco:hasAffiliation ?b . "			\
+			"?b rdfs:label \"Home\" . "			\
+			"OPTIONAL {?b nco:hasEmailAddress ?e . } "	\
+			"OPTIONAL {?b nco:hasPostalAddress ?p . }} "	\
+			"} "						\
+		"}"							\
+		"OPTIONAL { "						\
+			"?a rdfs:label \"Home\" . "			\
+			"?tmp nco:phoneNumber ?h . "			\
+			"OPTIONAL { ?a nco:hasEmailAddress ?e . } "	\
+			"OPTIONAL { ?a nco:hasPostalAddress ?p . } "	\
+			"OPTIONAL { ?a nco:org ?o . } "			\
+			"{ "						\
+			"SELECT ?pw ?ew ?c WHERE { "			\
+			"?c nco:hasAffiliation ?b . "			\
+			"?b rdfs:label \"Work\" . "			\
+			"OPTIONAL {?b nco:hasEmailAddress ?ew . } "	\
+			"OPTIONAL {?b nco:hasPostalAddress ?pw . }} "	\
+			"} "						\
+		"}"							\
 	"} UNION { "							\
 		"?x a nco:Contact . "					\
-		"?x nco:hasPhoneNumber ?w . "				\
+		"?x nco:hasPhoneNumber ?t . "				\
 		"?call a nmo:Call ; "					\
 		"nmo:from ?x ; "					\
 		"nmo:isSent false ; "					\
 		"nmo:isAnswered false . "				\
-		"?c a nco:PersonContact . "				\
-		"OPTIONAL { ?c nco:hasEmailAddress ?e . } "		\
-		"OPTIONAL { ?c nco:hasPostalAddress ?p . } "		\
-		"?c nco:hasAffiliation ?a . "				\
-		"?a nco:hasPhoneNumber ?w . "				\
-		"OPTIONAL { ?a nco:hasEmailAddress ?ew . } "		\
-		"OPTIONAL { ?a nco:hasPostalAddress ?pw . } "		\
-		"OPTIONAL { ?a nco:org ?o . } "				\
+		"OPTIONAL {?c a nco:PersonContact ; "			\
+			"nco:hasPhoneNumber ?t . } "			\
+		"OPTIONAL {?c a nco:PersonContact ; "			\
+			"nco:hasAffiliation ?a . "			\
+			"?a nco:hasPhoneNumber ?t . } "			\
+		"FILTER ( !bound(?c) && !bound(?a) ) . "		\
 	"} "								\
-	"} GROUP BY ?call ORDER BY DESC(nmo:receivedDate(?call)) "
+	"} ORDER BY DESC(nmo:receivedDate(?call)) "
+
 
 #define MISSED_CALLS_LIST						\
 	"SELECT ?c nco:nameFamily(?c) "					\
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH 1/7] Fix proper type handling in contacts_query_all
From: Bartosz Szatkowski @ 2010-11-10 13:15 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bartosz Szatkowski

Previously all phone numbers, addresses and emails was considered to be "work".
Now there are three working types for emails and addresses: "work", "home",
"other" and four for phone numbers - these three as well as "cell".
---
 plugins/phonebook-tracker.c |   61 +++++++++++++++++++++++++++++++------------
 1 files changed, 44 insertions(+), 17 deletions(-)

diff --git a/plugins/phonebook-tracker.c b/plugins/phonebook-tracker.c
index 58f52ab..83327e0 100644
--- a/plugins/phonebook-tracker.c
+++ b/plugins/phonebook-tracker.c
@@ -43,8 +43,8 @@
 #define TRACKER_RESOURCES_INTERFACE "org.freedesktop.Tracker1.Resources"
 
 #define TRACKER_DEFAULT_CONTACT_ME "http://www.semanticdesktop.org/ontologies/2007/03/22/nco#default-contact-me"
-#define CONTACTS_ID_COL 38
-#define PULL_QUERY_COL_AMOUNT 39
+#define CONTACTS_ID_COL 47
+#define PULL_QUERY_COL_AMOUNT 48
 #define COUNT_QUERY_COL_AMOUNT 1
 #define COL_HOME_NUMBER 0
 #define COL_HOME_EMAIL 7
@@ -52,14 +52,16 @@
 #define COL_FAX_NUMBER 16
 #define COL_WORK_EMAIL 17
 #define COL_OTHER_NUMBER 34
-#define COL_DATE 35
-#define COL_SENT 36
-#define COL_ANSWERED 37
+#define COL_OTHER_EMAIL 42
+#define COL_CELL_NUMBER 43
+#define COL_DATE 44
+#define COL_SENT 45
+#define COL_ANSWERED 46
 #define ADDR_FIELD_AMOUNT 7
 #define CONTACT_ID_PREFIX "contact:"
 
 #define CONTACTS_QUERY_ALL						\
-	"SELECT ?v nco:fullname(?c) "					\
+	"SELECT nco:phoneNumber(?v) nco:fullname(?c) "			\
 	"nco:nameFamily(?c) nco:nameGiven(?c) "				\
 	"nco:nameAdditional(?c) nco:nameHonorificPrefix(?c) "		\
 	"nco:nameHonorificSuffix(?c) nco:emailAddress(?e) "		\
@@ -71,29 +73,43 @@
 	"nco:role(?a) nco:pobox(?pw) nco:extendedAddress(?pw) "		\
 	"nco:streetAddress(?pw) nco:locality(?pw) nco:region(?pw) "	\
 	"nco:postalcode(?pw) nco:country(?pw) nco:contactUID(?c) "	\
-	"nco:title(?a) nco:phoneNumber(?t) "				\
+	"nco:title(?a) ?t nco:pobox(?po) nco:extendedAddress(?po) "	\
+	"nco:streetAddress(?po) nco:locality(?po) nco:region(?po) "	\
+        "nco:postalcode(?po) nco:country(?po) nco:emailAddress(?eo) "	\
+	"?vc "								\
 	"\"NOTACALL\" \"false\" \"false\" ?c "				\
 	"WHERE { "							\
 		"?c a nco:PersonContact . "				\
-	"OPTIONAL { ?c nco:hasPhoneNumber ?h . 				\
-		OPTIONAL {"						\
+	"OPTIONAL { ?c nco:hasPhoneNumber ?h . "			\
+		"OPTIONAL {"						\
 		"?h a nco:FaxNumber ; "					\
 		"nco:phoneNumber ?f . "					\
 		"}"							\
 		"OPTIONAL {"						\
+		"?h a nco:CellPhoneNumber ; "				\
+		"nco:phoneNumber ?vc"					\
+		"}"							\
+		"OPTIONAL {"						\
 		"?h a nco:VoicePhoneNumber ; "				\
-		"nco:phoneNumber ?v"					\
+		"nco:phoneNumber ?t"					\
 		"}"							\
 	"}"								\
-	"OPTIONAL { ?c nco:hasEmailAddress ?e . } "			\
-	"OPTIONAL { ?c nco:hasPostalAddress ?p . } "			\
 	"OPTIONAL { "							\
 		"?c nco:hasAffiliation ?a . "				\
-		"OPTIONAL { ?a nco:hasPhoneNumber ?w . } " 		\
-		"OPTIONAL { ?a nco:hasEmailAddress ?ew . } "		\
-		"OPTIONAL { ?a nco:hasPostalAddress ?pw . } "		\
+		"OPTIONAL { ?a rdfs:label \"Work\" . "			\
+			"OPTIONAL { ?a nco:hasEmailAddress ?ew . } "	\
+			"OPTIONAL { ?a nco:hasPostalAddress ?pw . } "	\
+			"OPTIONAL { ?a nco:hasPhoneNumber ?w . } "	\
+		"}"							\
+		"OPTIONAL { ?a rdfs:label \"Home\" . "			\
+			"OPTIONAL { ?a nco:hasEmailAddress ?e . } "	\
+			"OPTIONAL { ?a nco:hasPostalAddress ?p . } "	\
+			"OPTIONAL { ?a nco:hasPhoneNumber ?v . } "	\
+		"}"							\
 		"OPTIONAL { ?a nco:org ?o . } "				\
 	"} "								\
+	"OPTIONAL { ?c nco:hasPostalAddress ?po . } "			\
+	"OPTIONAL { ?c nco:hasEmailAddress ?eo . } "			\
 	"}"
 
 #define CONTACTS_QUERY_ALL_LIST						\
@@ -1107,7 +1123,7 @@ static void pull_contacts(char **reply, int num_fields, void *user_data)
 	GString *vcards;
 	int last_index, i;
 	gboolean cdata_present = FALSE;
-	char *home_addr, *work_addr;
+	char *home_addr, *work_addr, *other_addr;
 
 	if (num_fields < 0) {
 		data->cb(NULL, 0, num_fields, 0, data->user_data);
@@ -1178,11 +1194,16 @@ add_numbers:
 	add_phone_number(contact, reply[COL_HOME_NUMBER], TEL_TYPE_HOME);
 	add_phone_number(contact, reply[COL_WORK_NUMBER], TEL_TYPE_WORK);
 	add_phone_number(contact, reply[COL_FAX_NUMBER], TEL_TYPE_FAX);
-	add_phone_number(contact, reply[COL_OTHER_NUMBER], TEL_TYPE_OTHER);
+	add_phone_number(contact, reply[COL_CELL_NUMBER], TEL_TYPE_MOBILE);
+	if ((g_strcmp0(reply[COL_OTHER_NUMBER], reply[COL_CELL_NUMBER]) != 0) &&
+			(g_strcmp0(reply[COL_OTHER_NUMBER], reply[COL_WORK_NUMBER]) != 0) &&
+			(g_strcmp0(reply[COL_OTHER_NUMBER], reply[COL_HOME_NUMBER]) != 0))
+		add_phone_number(contact, reply[COL_OTHER_NUMBER], TEL_TYPE_OTHER);
 
 	/* Adding emails */
 	add_email(contact, reply[COL_HOME_EMAIL], EMAIL_TYPE_HOME);
 	add_email(contact, reply[COL_WORK_EMAIL], EMAIL_TYPE_WORK);
+	add_email(contact, reply[COL_OTHER_EMAIL], EMAIL_TYPE_OTHER);
 
 	/* Adding addresses */
 	home_addr = g_strdup_printf("%s;%s;%s;%s;%s;%s;%s",
@@ -1193,11 +1214,17 @@ add_numbers:
 				reply[25], reply[26], reply[27], reply[28],
 				reply[29], reply[30], reply[31]);
 
+	other_addr = g_strdup_printf("%s;%s;%s;%s;%s;%s;%s",
+				reply[35], reply[36], reply[37], reply[38],
+				reply[39], reply[40], reply[41]);
+
 	add_address(contact, home_addr, ADDR_TYPE_HOME);
 	add_address(contact, work_addr, ADDR_TYPE_WORK);
+	add_address(contact, other_addr, ADDR_TYPE_OTHER);
 
 	g_free(home_addr);
 	g_free(work_addr);
+	g_free(other_addr);
 
 	/* Adding fields connected by nco:hasAffiliation - they may be in
 	 * separate replies */
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH v5] Bluetooth: btwilink driver
From: pavan_savoy @ 2010-11-10 13:07 UTC (permalink / raw)
  To: marcel, padovan; +Cc: linux-bluetooth, linux-kernel, Pavan Savoy

From: Pavan Savoy <pavan_savoy@ti.com>

Marcel,

Thanks for the comments...
This patch contains,
v5 comments :-
declaration and assiging of variables and private data fixed up.
proper casting.
removed redundant un-necessary checks in send_frame.
HCI_RUNNING fixes in terms of test_and_set/clear bit instead of set and
clear.
removed redundant checks for hdev, skb being NULL.
removed module_param of reset, also WiLink don't need HCI_RESET anyways.
removed ti_st_register_dev function and functionality moved to _probe.
module_init/exit function names fixed up.

stat byte counter increments and tx_complete is similar to hci_ldisc.
Also I have not implemented the flush routine, since the functionality
which needs to be done in flush routine is done in the underlying driver
which is the shared transport driver and moreover the btwilink driver by
itself doesn't maintains queue or data relevant to transport, so nothing
to do.

And Yes, I have verified this driver with multiple up/down reset on
hci0.
Also I generally test a2dp/ftp to verify large data transfers.

Please review and comment.

Thanks,
Pavan

v4 comments :-
module init now returns what platform_driver_register returns.
type casting of void* private data has been removed

v3 comments :-
Lizardo,
I have taken care of most of the comments you had.
Have re-wrote some of the code commenting you've mentioned.
Thanks for the comments,

The other few like -EPERM for platform driver registration is to keep
it similar to other drivers, type casting is maintained just to feel safe
and have style similar to other drivers.
BT_WILINK in Kconfig is similar to BT_MRVL.
I hope those aren't too critical.

-- patch description --

This is the bluetooth protocol driver for the TI WiLink7 chipsets.
Texas Instrument's WiLink chipsets combine wireless technologies
like BT, FM, GPS and WLAN onto a single chip.

This Bluetooth driver works on top of the TI_ST shared transport
line discipline driver which also allows other drivers like
FM V4L2 and GPS character driver to make use of the same UART interface.

Kconfig and Makefile modifications to enable the Bluetooth
driver for Texas Instrument's WiLink 7 chipset.

Signed-off-by: Pavan Savoy <pavan_savoy@ti.com>
---
 drivers/bluetooth/Kconfig    |   10 +
 drivers/bluetooth/Makefile   |    1 +
 drivers/bluetooth/btwilink.c |  379 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 390 insertions(+), 0 deletions(-)
 create mode 100644 drivers/bluetooth/btwilink.c

diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
index 02deef4..8e0de9a 100644
--- a/drivers/bluetooth/Kconfig
+++ b/drivers/bluetooth/Kconfig
@@ -219,4 +219,14 @@ config BT_ATH3K
 	  Say Y here to compile support for "Atheros firmware download driver"
 	  into the kernel or say M to compile it as module (ath3k).
 
+config BT_WILINK
+	tristate "Texas Instruments WiLink7 driver"
+	depends on TI_ST
+	help
+	  This enables the Bluetooth driver for Texas Instrument's BT/FM/GPS
+	  combo devices. This makes use of shared transport line discipline
+	  core driver to communicate with the BT core of the combo chip.
+
+	  Say Y here to compile support for Texas Instrument's WiLink7 driver
+	  into the kernel or say M to compile it as module.
 endmenu
diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
index 71bdf13..f4460f4 100644
--- a/drivers/bluetooth/Makefile
+++ b/drivers/bluetooth/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_BT_HCIBTSDIO)	+= btsdio.o
 obj-$(CONFIG_BT_ATH3K)		+= ath3k.o
 obj-$(CONFIG_BT_MRVL)		+= btmrvl.o
 obj-$(CONFIG_BT_MRVL_SDIO)	+= btmrvl_sdio.o
+obj-$(CONFIG_BT_WILINK)		+= btwilink.o
 
 btmrvl-y			:= btmrvl_main.o
 btmrvl-$(CONFIG_DEBUG_FS)	+= btmrvl_debugfs.o
diff --git a/drivers/bluetooth/btwilink.c b/drivers/bluetooth/btwilink.c
new file mode 100644
index 0000000..1b1c4bc
--- /dev/null
+++ b/drivers/bluetooth/btwilink.c
@@ -0,0 +1,379 @@
+/*
+ *  Texas Instrument's Bluetooth Driver For Shared Transport.
+ *
+ *  Bluetooth Driver acts as interface between HCI core and
+ *  TI Shared Transport Layer.
+ *
+ *  Copyright (C) 2009-2010 Texas Instruments
+ *  Author: Raja Mani <raja_mani@ti.com>
+ *	Pavan Savoy <pavan_savoy@ti.com>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ *
+ */
+
+#include <linux/platform_device.h>
+#include <net/bluetooth/bluetooth.h>
+#include <net/bluetooth/hci_core.h>
+
+#include <linux/ti_wilink_st.h>
+
+/* Bluetooth Driver Version */
+#define VERSION               "1.0"
+
+/* Number of seconds to wait for registration completion
+ * when ST returns PENDING status.
+ */
+#define BT_REGISTER_TIMEOUT   6000	/* 6 sec */
+
+/**
+ * struct ti_st - driver operation structure
+ * @hdev: hci device pointer which binds to bt driver
+ * @reg_status: ST registration callback status
+ * @st_write: write function provided by the ST driver
+ *	to be used by the driver during send_frame.
+ * @wait_reg_completion - completion sync between ti_st_open
+ *	and ti_st_registration_completion_cb.
+ */
+struct ti_st {
+	struct hci_dev *hdev;
+	char reg_status;
+	long (*st_write) (struct sk_buff *);
+	struct completion wait_reg_completion;
+};
+
+/* Increments HCI counters based on pocket ID (cmd,acl,sco) */
+static inline void ti_st_tx_complete(struct ti_st *hst, int pkt_type)
+{
+	struct hci_dev *hdev = hst->hdev;
+
+	/* Update HCI stat counters */
+	switch (pkt_type) {
+	case HCI_COMMAND_PKT:
+		hdev->stat.cmd_tx++;
+		break;
+
+	case HCI_ACLDATA_PKT:
+		hdev->stat.acl_tx++;
+		break;
+
+	case HCI_SCODATA_PKT:
+		hdev->stat.sco_tx++;
+		break;
+	}
+}
+
+/* ------- Interfaces to Shared Transport ------ */
+
+/* Called by ST layer to indicate protocol registration completion
+ * status.ti_st_open() function will wait for signal from this
+ * API when st_register() function returns ST_PENDING.
+ */
+static void st_registration_completion_cb(void *priv_data, char data)
+{
+	struct ti_st *lhst = priv_data;
+
+	/* Save registration status for use in ti_st_open() */
+	lhst->reg_status = data;
+	/* complete the wait in ti_st_open() */
+	complete(&lhst->wait_reg_completion);
+}
+
+/* Called by Shared Transport layer when receive data is
+ * available */
+static long st_receive(void *priv_data, struct sk_buff *skb)
+{
+	struct ti_st *lhst = priv_data;
+	int err;
+
+	if (!skb)
+		return -EFAULT;
+
+	if (!lhst) {
+		kfree_skb(skb);
+		return -EFAULT;
+	}
+
+	skb->dev = (void *) lhst->hdev;
+
+	/* Forward skb to HCI core layer */
+	err = hci_recv_frame(skb);
+	if (err < 0) {
+		BT_ERR("Unable to push skb to HCI core(%d)", err);
+		return err;
+	}
+
+	lhst->hdev->stat.byte_rx += skb->len;
+
+	return 0;
+}
+
+/* ------- Interfaces to HCI layer ------ */
+/* protocol structure registered with shared transport */
+static struct st_proto_s ti_st_proto = {
+	.type = ST_BT,
+	.recv = st_receive,
+	.reg_complete_cb = st_registration_completion_cb,
+};
+
+/* Called from HCI core to initialize the device */
+static int ti_st_open(struct hci_dev *hdev)
+{
+	unsigned long timeleft;
+	struct ti_st *hst;
+	int err;
+
+	BT_DBG("%s %p", hdev->name, hdev);
+
+	/* provide contexts for callbacks from ST */
+	hst = hdev->driver_data;
+	ti_st_proto.priv_data = hst;
+
+	err = st_register(&ti_st_proto);
+	if (err == -EINPROGRESS) {
+		/* Prepare wait-for-completion handler data structures.
+		 * Needed to synchronize this and
+		 * st_registration_completion_cb() functions.
+		 */
+		init_completion(&hst->wait_reg_completion);
+
+		/* Reset ST registration callback status flag , this value
+		 * will be updated in ti_st_registration_completion_cb()
+		 * function whenever it called from ST driver.
+		 */
+		hst->reg_status = -EINPROGRESS;
+
+		/* ST is busy with either protocol registration or firmware
+		 * download. Wait until the registration callback is called
+		 */
+		BT_DBG(" waiting for registration completion signal from ST");
+
+		timeleft = wait_for_completion_timeout
+			(&hst->wait_reg_completion,
+			 msecs_to_jiffies(BT_REGISTER_TIMEOUT));
+		if (!timeleft) {
+			BT_ERR("Timeout(%d sec),didn't get reg "
+					"completion signal from ST",
+					BT_REGISTER_TIMEOUT / 1000);
+			return -ETIMEDOUT;
+		}
+
+		/* Is ST registration callback called with ERROR status? */
+		if (hst->reg_status != 0) {
+			BT_ERR("ST registration completed with invalid "
+					"status %d", hst->reg_status);
+			return -EAGAIN;
+		}
+		err = 0;
+	} else if (err == -EPERM) {
+		BT_ERR("st_register failed %d", err);
+		return err;
+	}
+
+	/* ti_st_proto.write is filled up by the underlying shared
+	 * transport driver upon registration
+	 */
+	hst->st_write = ti_st_proto.write;
+	if (!hst->st_write) {
+		BT_ERR("undefined ST write function");
+
+		/* Undo registration with ST */
+		err = st_unregister(ST_BT);
+		if (err)
+			BT_ERR("st_unregister() failed with error %d", err);
+
+		hst->st_write = NULL;
+		return err;
+	}
+
+	/* Registration with ST layer is successful,
+	 * hardware is ready to accept commands from HCI core.
+	 */
+	if (test_and_set_bit(HCI_RUNNING, &hdev->flags)) {
+		clear_bit(HCI_RUNNING, &hdev->flags);
+		err = st_unregister(ST_BT);
+		if (err)
+			BT_ERR("st_unregister() failed with error %d", err);
+		hst->st_write = NULL;
+	}
+
+	return err;
+}
+
+/* Close device */
+static int ti_st_close(struct hci_dev *hdev)
+{
+	int err;
+	struct ti_st *hst = hdev->driver_data;
+
+	/* continue to unregister from transport */
+	err = st_unregister(ST_BT);
+	if (err)
+		BT_ERR("st_unregister() failed with error %d", err);
+
+	hst->st_write = NULL;
+
+	if (!test_and_clear_bit(HCI_RUNNING, &hdev->flags))
+		return 0;
+
+	return err;
+}
+
+static int ti_st_send_frame(struct sk_buff *skb)
+{
+	struct hci_dev *hdev;
+	struct ti_st *hst;
+	long len;
+
+	hdev = (struct hci_dev *)skb->dev;
+
+	if (!test_bit(HCI_RUNNING, &hdev->flags))
+		return -EBUSY;
+
+	hst = hdev->driver_data;
+
+	/* Prepend skb with frame type */
+	memcpy(skb_push(skb, 1), &bt_cb(skb)->pkt_type, 1);
+
+	BT_DBG(" %s: type %d len %d", hdev->name, bt_cb(skb)->pkt_type,
+			skb->len);
+
+	/* Insert skb to shared transport layer's transmit queue.
+	 * Freeing skb memory is taken care in shared transport layer,
+	 * so don't free skb memory here.
+	 */
+	len = hst->st_write(skb);
+	if (len < 0) {
+		kfree_skb(skb);
+		BT_ERR(" ST write failed (%ld)", len);
+		return -EAGAIN;
+	}
+
+	/* ST accepted our skb. So, Go ahead and do rest */
+	hdev->stat.byte_tx += len;
+	ti_st_tx_complete(hst, bt_cb(skb)->pkt_type);
+
+	return 0;
+}
+
+static void ti_st_destruct(struct hci_dev *hdev)
+{
+	BT_DBG("%s", hdev->name);
+
+	/* free ti_st memory */
+	kfree(hdev->driver_data);
+
+	return;
+}
+
+static int bt_ti_probe(struct platform_device *pdev)
+{
+	static struct ti_st *hst;
+	struct hci_dev *hdev;
+	int err;
+
+	hst = kzalloc(sizeof(struct ti_st), GFP_KERNEL);
+	if (!hst)
+		return -ENOMEM;
+
+	/* Expose "hciX" device to user space */
+	hdev = hci_alloc_dev();
+	if (!hdev)
+		return -ENOMEM;
+
+	BT_DBG("hdev %p", hdev);
+
+	hst->hdev = hdev;
+	hdev->bus = HCI_UART;
+	hdev->driver_data = hst;
+	hdev->open = ti_st_open;
+	hdev->close = ti_st_close;
+	hdev->flush = NULL;
+	hdev->send = ti_st_send_frame;
+	hdev->destruct = ti_st_destruct;
+	hdev->owner = THIS_MODULE;
+
+	err = hci_register_dev(hdev);
+	if (err < 0) {
+		BT_ERR("Can't register HCI device error %d", err);
+		hci_free_dev(hdev);
+		return err;
+	}
+
+	BT_DBG(" HCI device registered (hdev %p)", hdev);
+
+	dev_set_drvdata(&pdev->dev, hst);
+	return err;
+}
+
+static int bt_ti_remove(struct platform_device *pdev)
+{
+	struct hci_dev *hdev;
+	struct ti_st *hst = dev_get_drvdata(&pdev->dev);
+
+	if (!hst)
+		return -EFAULT;
+
+	hdev = hst->hdev;
+	ti_st_close(hdev);
+	hci_unregister_dev(hdev);
+
+	/* Free HCI device memory */
+	hci_free_dev(hdev);
+
+	/* Free driver data memory */
+	kfree(hst);
+
+	dev_set_drvdata(&pdev->dev, NULL);
+	return 0;
+}
+
+static struct platform_driver btwilink_driver = {
+	.probe = bt_ti_probe,
+	.remove = bt_ti_remove,
+	.driver = {
+		.name = "btwilink",
+		.owner = THIS_MODULE,
+	},
+};
+
+/* ------- Module Init/Exit interfaces ------ */
+static int __init btwilink_init(void)
+{
+	long ret;
+
+	BT_INFO(" Bluetooth Driver for TI WiLink - Version %s", VERSION);
+
+	ret = platform_driver_register(&btwilink_driver);
+	if (ret != 0) {
+		BT_ERR("btwilink platform driver registration failed");
+		return ret;
+	}
+	return 0;
+}
+
+static void __exit btwilink_exit(void)
+{
+	platform_driver_unregister(&btwilink_driver);
+}
+
+module_init(btwilink_init);
+module_exit(btwilink_exit);
+
+/* ------ Module Info ------ */
+
+MODULE_AUTHOR("Raja Mani <raja_mani@ti.com>");
+MODULE_DESCRIPTION("Bluetooth Driver for TI Shared Transport" VERSION);
+MODULE_VERSION(VERSION);
+MODULE_LICENSE("GPL");
-- 
1.5.6.3

^ permalink raw reply related

* Re: [PATCH] Fix segfault in HDP during device re-creation
From: Jose Antonio Santos Cadenas @ 2010-11-10 13:02 UTC (permalink / raw)
  To: Elvis Pfützenreuter; +Cc: linux-bluetooth
In-Reply-To: <BCBAC715-C7A9-415F-BE62-F843DE5A72D1@signove.com>

Hi Elvis,

2010/11/10 Elvis Pfützenreuter <epx@signove.com>:
>> Hi Elvis,
>>
>> 2010/11/10 Elvis Pfützenreuter <epx@signove.com>:
>>>>> ---
>>>>>  health/hdp.c |    1 +
>>>>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>>>>
>>>>> diff --git a/health/hdp.c b/health/hdp.c
>>>>> index 1eba8e1..d361b27 100644
>>>>> --- a/health/hdp.c
>>>>> +++ b/health/hdp.c
>>>>> @@ -259,6 +259,7 @@ static void device_unref_mcl(struct hdp_device *hdp_device)
>>>>>        if (!hdp_device->mcl)
>>>>>                return;
>>>>>
>>>>> +       mcap_close_mcl(hdp_device->mcl, FALSE);
>>>>>        mcap_mcl_unref(hdp_device->mcl);
>>>>>        hdp_device->mcl = NULL;
>>>>>        hdp_device->mcl_conn = FALSE;
>>>>> --
>>>>> 1.7.1
>>>>>
>>>>>
>>>>
>>>> Please Elvis, try this solution and tell us if it fix the segfault problem.
>>>
>>> Yes, it seems to have fixed the problem. And far cleaner :)
>>>
>>> I hadn't proposed a lookalike solution because this seems to disable MCL caching completely. HDP already calls mcap_close_mcl(cache=FALSE) when takes the initiative of closing the MCL; this patch takes care of remaining case.
>>
>> This function doesn't disable the caching in general, it just closes
>> this MCL without catching it, but caching is still active for all the
>> other MCL's. Additionally, nothing happens if mcap_close_mcl is called
>> more than once, so this patch takes care of this too.
>
> Indeed, MCL disconnection and hdp_device destruction are different moments. Still fulfilling the caffeine quota for this morning :P

Of course they are, but when a device is removed, is completely normal
that you (HDP) would like to delete the MCAP cache because you (HDP)
are forgetting this device so you won't have any structures for the
data channels nor other structs in HDP needed to manage the
reconnections of the data channels that could be cached. The way to
uncache an mcl in MCAP is to close it indicating that you don't want
to cache it. This is just what this patch to.

>
>>>
>>> So, perhaps it would be better to get rid of caching code in mcap.c?
>>
>> I can't understand this, can you explain this more?. As I see, MCAP
>> should do the catching because it holds all the information about the
>> channels that are connected and can cache it, HDP could not do this
>> without MCAP. More over, if in the future more profiles use MCAP they
>> may want to cache too.
>
> I do have the opinion that caching in mcap.c is more complicated than it could be.

My opinion is that caching in MCAP is the correct way because MCAP is
dealing with sending and receiving commands and need to retain certain
structures that are necessary for it, this structures do not concern
to HDP, only concerns to MCAP and should be MCAP who stores (caches)
them.

> I'd have used a single list with a CLOSED flag. Also, there is some confusion in nomenclature (mcap_uncache_mcl() recovers from the cache, while mcl_uncached_cb callback means that MCL has been invalidated). This is source of bugs like this one.

This is a different issue. Probably the nomenclature is not as clear
as it should be but I think that this is not the point here,
nevertheless we can change it too.

>
> At the very least, cached MCLs should have mcl->cb zeroed. if MCAP level caches MCLs, very well, but HDP level should be asked to set callbacks and user data again, as if it were a new MCL. (Actually, mcl_reconnected in hdp.c seems to do that; not sure why it did not avoid the particular bug that is under discussion.)

It is supposed that if HDP (or any other profile using MCAP) wants to
use cache, it should keep its variables all the time that it is
interested in caching them. If the profile using MCAP don't want to
keep its state should tell MCAP that it is not interested in caching
(as the patch in this thread does) because it is deleting its own
state.


Regards.

Jose

^ permalink raw reply

* Re: [RFC] Bluetooth Low energy support
From: Ville Tervo @ 2010-11-10 12:51 UTC (permalink / raw)
  To: ext Anderson Lizardo; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <AANLkTi=iCXfycMzHbgwS3jbvCxJvzVh97iSDRf4WhQeQ@mail.gmail.com>

On Wed, Nov 10, 2010 at 11:46:47AM +0100, ext Anderson Lizardo wrote:
> On Wed, Nov 10, 2010 at 2:20 AM, Ville Tervo <ville.tervo@nokia.com> wrote:
> > On Tue, Nov 09, 2010 at 05:25:10PM +0100, ext Anderson Lizardo wrote:
> >> While doing tests with your most recent trees (using devel HW from
> >> TI), I'm getting consistent panic on the following test:
> >>
> >> (dev1) hciconfig hciX leadv
> >> (dev2) hcitool -i hciX lecc <dev2_addr>
> >
> > Did you have server listening on dev 2 on ATT socket?
> 
> I had no bluetoothd running on any side (not sure if that was your question).

That was actually my question :) Thanks.

-- 
VIlle

^ permalink raw reply

* Re: [PATCH] Fix segfault in HDP during device re-creation
From: Elvis Pfützenreuter @ 2010-11-10 12:40 UTC (permalink / raw)
  To: Jose Antonio Santos Cadenas; +Cc: linux-bluetooth
In-Reply-To: <AANLkTim=3tZWao0Kzqq8nr4Z=dOrXL_aK76Ernx=UqQS@mail.gmail.com>

> Hi Elvis,
> 
> 2010/11/10 Elvis Pfützenreuter <epx@signove.com>:
>>>> ---
>>>>  health/hdp.c |    1 +
>>>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>>> 
>>>> diff --git a/health/hdp.c b/health/hdp.c
>>>> index 1eba8e1..d361b27 100644
>>>> --- a/health/hdp.c
>>>> +++ b/health/hdp.c
>>>> @@ -259,6 +259,7 @@ static void device_unref_mcl(struct hdp_device *hdp_device)
>>>>        if (!hdp_device->mcl)
>>>>                return;
>>>> 
>>>> +       mcap_close_mcl(hdp_device->mcl, FALSE);
>>>>        mcap_mcl_unref(hdp_device->mcl);
>>>>        hdp_device->mcl = NULL;
>>>>        hdp_device->mcl_conn = FALSE;
>>>> --
>>>> 1.7.1
>>>> 
>>>> 
>>> 
>>> Please Elvis, try this solution and tell us if it fix the segfault problem.
>> 
>> Yes, it seems to have fixed the problem. And far cleaner :)
>> 
>> I hadn't proposed a lookalike solution because this seems to disable MCL caching completely. HDP already calls mcap_close_mcl(cache=FALSE) when takes the initiative of closing the MCL; this patch takes care of remaining case.
> 
> This function doesn't disable the caching in general, it just closes
> this MCL without catching it, but caching is still active for all the
> other MCL's. Additionally, nothing happens if mcap_close_mcl is called
> more than once, so this patch takes care of this too.

Indeed, MCL disconnection and hdp_device destruction are different moments. Still fulfilling the caffeine quota for this morning :P

>> 
>> So, perhaps it would be better to get rid of caching code in mcap.c?
> 
> I can't understand this, can you explain this more?. As I see, MCAP
> should do the catching because it holds all the information about the
> channels that are connected and can cache it, HDP could not do this
> without MCAP. More over, if in the future more profiles use MCAP they
> may want to cache too.

I do have the opinion that caching in mcap.c is more complicated than it could be. I'd have used a single list with a CLOSED flag. Also, there is some confusion in nomenclature (mcap_uncache_mcl() recovers from the cache, while mcl_uncached_cb callback means that MCL has been invalidated). This is source of bugs like this one.

At the very least, cached MCLs should have mcl->cb zeroed. if MCAP level caches MCLs, very well, but HDP level should be asked to set callbacks and user data again, as if it were a new MCL. (Actually, mcl_reconnected in hdp.c seems to do that; not sure why it did not avoid the particular bug that is under discussion.)

^ permalink raw reply

* Re: [PATCH] Fix segfault in HDP during device re-creation
From: Jose Antonio Santos Cadenas @ 2010-11-10 11:49 UTC (permalink / raw)
  To: Elvis Pfützenreuter; +Cc: linux-bluetooth
In-Reply-To: <126814B4-25DA-4BA4-A0B8-0E5D57001EF7@signove.com>

Hi Elvis,

2010/11/10 Elvis Pfützenreuter <epx@signove.com>:
>>> ---
>>>  health/hdp.c |    1 +
>>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/health/hdp.c b/health/hdp.c
>>> index 1eba8e1..d361b27 100644
>>> --- a/health/hdp.c
>>> +++ b/health/hdp.c
>>> @@ -259,6 +259,7 @@ static void device_unref_mcl(struct hdp_device *hdp_device)
>>>        if (!hdp_device->mcl)
>>>                return;
>>>
>>> +       mcap_close_mcl(hdp_device->mcl, FALSE);
>>>        mcap_mcl_unref(hdp_device->mcl);
>>>        hdp_device->mcl = NULL;
>>>        hdp_device->mcl_conn = FALSE;
>>> --
>>> 1.7.1
>>>
>>>
>>
>> Please Elvis, try this solution and tell us if it fix the segfault problem.
>
> Yes, it seems to have fixed the problem. And far cleaner :)
>
> I hadn't proposed a lookalike solution because this seems to disable MCL caching completely. HDP already calls mcap_close_mcl(cache=FALSE) when takes the initiative of closing the MCL; this patch takes care of remaining case.

This function doesn't disable the caching in general, it just closes
this MCL without catching it, but caching is still active for all the
other MCL's. Additionally, nothing happens if mcap_close_mcl is called
more than once, so this patch takes care of this too.

>
> The only place hdp.c calls mcap_close_mcl(cache=TRUE) is when mcap_mcl_set_cb() fails, which seems "impossible", because it only depends on valid parameters to succeed.

This is because HDP wants to cache as long as possible.

>
> So, perhaps it would be better to get rid of caching code in mcap.c?

I can't understand this, can you explain this more?. As I see, MCAP
should do the catching because it holds all the information about the
channels that are connected and can cache it, HDP could not do this
without MCAP. More over, if in the future more profiles use MCAP they
may want to cache too.


Regards

^ permalink raw reply

* [PATCH] Fix pull phonebook reply if filter not set
From: Lukasz Pawlik @ 2010-11-10 11:43 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Lukasz Pawlik

According to the PBAP specification if filter is not set or is set to
0x00000000 in the application parameters header all attributes of the vCard
should be returned. Previously only mandatory attributes were returned in
phonebook pull reply. This patch fix this and now all currently supported
vCards attributes will be returned.
---
 plugins/vcard.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/plugins/vcard.c b/plugins/vcard.c
index 41f9fbd..3f69189 100644
--- a/plugins/vcard.c
+++ b/plugins/vcard.c
@@ -457,10 +457,16 @@ static void vcard_printf_end(GString *vcards)
 void phonebook_add_contact(GString *vcards, struct phonebook_contact *contact,
 					uint64_t filter, uint8_t format)
 {
-	if (format == FORMAT_VCARD30)
+	if (format == FORMAT_VCARD30 && filter)
 		filter |= (FILTER_VERSION | FILTER_FN | FILTER_N | FILTER_TEL);
-	else if (format == FORMAT_VCARD21)
+	else if (format == FORMAT_VCARD21 && filter)
 		filter |= (FILTER_VERSION | FILTER_N | FILTER_TEL);
+	else
+		filter = (FILTER_VERSION | FILTER_UID | FILTER_N | FILTER_FN |
+				FILTER_TEL | FILTER_EMAIL | FILTER_ADR |
+				FILTER_BDAY | FILTER_NICKNAME | FILTER_URL |
+				FILTER_PHOTO | FILTER_ORG | FILTER_ROLE |
+				FILTER_TITLE | FILTER_X_IRMC_CALL_DATETIME);
 
 	vcard_printf_begin(vcards, format);
 
-- 
1.7.0.4


^ permalink raw reply related

* Re: [PATCH] Fix segfault in HDP during device re-creation
From: Elvis Pfützenreuter @ 2010-11-10 11:36 UTC (permalink / raw)
  To: Jose Antonio Santos Cadenas; +Cc: linux-bluetooth
In-Reply-To: <AANLkTimoQRAHXJ65xa1-TMKGUfLujw8f9MhzxrxPXHmY@mail.gmail.com>

>> ---
>>  health/hdp.c |    1 +
>>  1 files changed, 1 insertions(+), 0 deletions(-)
>> 
>> diff --git a/health/hdp.c b/health/hdp.c
>> index 1eba8e1..d361b27 100644
>> --- a/health/hdp.c
>> +++ b/health/hdp.c
>> @@ -259,6 +259,7 @@ static void device_unref_mcl(struct hdp_device *hdp_device)
>>        if (!hdp_device->mcl)
>>                return;
>> 
>> +       mcap_close_mcl(hdp_device->mcl, FALSE);
>>        mcap_mcl_unref(hdp_device->mcl);
>>        hdp_device->mcl = NULL;
>>        hdp_device->mcl_conn = FALSE;
>> --
>> 1.7.1
>> 
>> 
> 
> Please Elvis, try this solution and tell us if it fix the segfault problem.

Yes, it seems to have fixed the problem. And far cleaner :)

I hadn't proposed a lookalike solution because this seems to disable MCL caching completely. HDP already calls mcap_close_mcl(cache=FALSE) when takes the initiative of closing the MCL; this patch takes care of remaining case.

The only place hdp.c calls mcap_close_mcl(cache=TRUE) is when mcap_mcl_set_cb() fails, which seems "impossible", because it only depends on valid parameters to succeed.

So, perhaps it would be better to get rid of caching code in mcap.c?

^ permalink raw reply

* RE: [PATCH 1/4] Sim Access Profile API
From: Waldemar.Rymarkiewicz @ 2010-11-10 11:12 UTC (permalink / raw)
  To: marcel; +Cc: linux-bluetooth, suraj, johan.hedberg, joakim.xj.ceder
In-Reply-To: <1289367190.9615.235.camel@aeonflux>

Hi Marcel,=20

>> +		void Disable()
>> +
>> +			Shudown SAP server and remove the SDP record.
>> +
>> +			Possible errors: org.bluez.Error.Failed
>
>I don't like this. If you have properties then just changing=20
>the property should be enough. So a SetProperty is more appropriate.

I see another option to get rid of 'Enabled' property and leave the methods=
. What would you say on that?

>> +
>> +		void Disconnect(boolean type)
>> +
>> +			Disconnect SAP client from the server.=20
>The 'type'
>> +			parameter indicates disconnection type.
>> +
>> +			True  - gracefull disconnection
>> +			False - immediate disconnection
>> +
>> +			Possible errors: org.bluez.Error.Failed
>
>I don't like this style of method names at all. Using method=20
>names like GracefulDisconnect and ImmediateDisconnect would be better.

That's fine.

>However I am not sure we should differentiate here at all. We=20
>should always to the graceful disconnect. What will the=20
>immediate disconnect bring us?

That's actually intended for testing only. One of PTS test cases expects th=
e tester to trigger immediate disconnect.
In practce, it is only used when connection to sim card is lost, but this i=
s obviously done internally.

Thanks,
/Waldek=

^ permalink raw reply

* Re: [RFC] Bluetooth Low energy support
From: Anderson Lizardo @ 2010-11-10 10:46 UTC (permalink / raw)
  To: Ville Tervo; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <20101110062030.GE20384@null>

On Wed, Nov 10, 2010 at 2:20 AM, Ville Tervo <ville.tervo@nokia.com> wrote:
> On Tue, Nov 09, 2010 at 05:25:10PM +0100, ext Anderson Lizardo wrote:
>> While doing tests with your most recent trees (using devel HW from
>> TI), I'm getting consistent panic on the following test:
>>
>> (dev1) hciconfig hciX leadv
>> (dev2) hcitool -i hciX lecc <dev2_addr>
>
> Did you have server listening on dev 2 on ATT socket?

I had no bluetoothd running on any side (not sure if that was your question).

>> I attached two logs. One is from dev1 machine (which has the oops),
>> the other is from the dev2 machine.
>
> I'll try to fix this today.

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

^ permalink raw reply

* Re: [PATCH] Fix segfault in HDP during device re-creation
From: Jose Antonio Santos Cadenas @ 2010-11-10  9:49 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Jose Antonio Santos Cadenas
In-Reply-To: <1289382461-10510-1-git-send-email-santoscadenas@gmail.com>

2010/11/10 Jose Antonio Santos Cadenas <santoscadenas@gmail.com>:
> ---
>  health/hdp.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/health/hdp.c b/health/hdp.c
> index 1eba8e1..d361b27 100644
> --- a/health/hdp.c
> +++ b/health/hdp.c
> @@ -259,6 +259,7 @@ static void device_unref_mcl(struct hdp_device *hdp_device)
>        if (!hdp_device->mcl)
>                return;
>
> +       mcap_close_mcl(hdp_device->mcl, FALSE);
>        mcap_mcl_unref(hdp_device->mcl);
>        hdp_device->mcl = NULL;
>        hdp_device->mcl_conn = FALSE;
> --
> 1.7.1
>
>

Please Elvis, try this solution and tell us if it fix the segfault problem.

Regards

^ permalink raw reply

* [PATCH] Fix segfault in HDP during device re-creation
From: Jose Antonio Santos Cadenas @ 2010-11-10  9:47 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Jose Antonio Santos Cadenas
In-Reply-To: <1289358915-6612-1-git-send-email-epx@signove.com>

---
 health/hdp.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/health/hdp.c b/health/hdp.c
index 1eba8e1..d361b27 100644
--- a/health/hdp.c
+++ b/health/hdp.c
@@ -259,6 +259,7 @@ static void device_unref_mcl(struct hdp_device *hdp_device)
 	if (!hdp_device->mcl)
 		return;
 
+	mcap_close_mcl(hdp_device->mcl, FALSE);
 	mcap_mcl_unref(hdp_device->mcl);
 	hdp_device->mcl = NULL;
 	hdp_device->mcl_conn = FALSE;
-- 
1.7.1


^ permalink raw reply related

* Re: Ubuntu 10.04 - BlueZ 4.60 - Console-only BlueZ setup
From: elroy @ 2010-11-10  8:11 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <930b6b5d0fcdfd10087260af984b94ac@libresoft.es>



José Antonio Santos Cadenas wrote:
> Hi Elroy,
> 
> 
> 
> Did you try simple-agent? It is a python scrypt in the bluez
> test/folder that doesn't require any gui. I don't if Ubuntu installs it,
> but if you download the source code from the web page or the git repo
> you will find it. Probably with this agent you will be able to pair your
> cell phone. 
> 
> 
>>
>>I have been documenting my progress so far, to try to aid others
>>following my path - this may be useful to elaborate on what I am doing
>>and have achieved so far.
>>
>>
>>Regards,
>>
>>Elroy Liddington.



Cheers Jose.


I'll add this gem to the page I'm documenting this on.

Looks like those other scripts would be useful too.


How would I be able to help in regards to knocking up a basic BlueZ MAN 
page etc so that others could at least be pointed in the right direction 
on where to go for help/docs?


Regards,

Elroy Liddington.


^ permalink raw reply

* Re: [PATCH] [RFC] Fix HDP-related segfault upon device recreation
From: Jose Antonio Santos Cadenas @ 2010-11-10  8:02 UTC (permalink / raw)
  To: Elvis Pfützenreuter; +Cc: linux-bluetooth
In-Reply-To: <1289358915-6612-1-git-send-email-epx@signove.com>

Hi Elvis,

2010/11/10 Elvis Pfützenreuter <epx@signove.com>:
> When device is removed and paired again, hdp_device is destroyed
> but a cached mcap_mcl may retain a pointer in mcl->cb->user_data.
> This patch ensures that such MCL is destroyed.
>
> There is probably a better solution to this e.g. changing cache
> strategy for accepted connections. A loop can be removed if 1:1
> relationship between hdp_device and MCL is guaranteed.
> ---
>  health/hdp.c      |    1 +
>  health/mcap.c     |   23 ++++++++++++++++++++++-
>  health/mcap_lib.h |    2 ++
>  3 files changed, 25 insertions(+), 1 deletions(-)
>
> diff --git a/health/hdp.c b/health/hdp.c
> index b141fe7..44ebe75 100644
> --- a/health/hdp.c
> +++ b/health/hdp.c
> @@ -277,6 +277,7 @@ static void free_health_device(struct hdp_device *device)
>        }
>
>        device_unref_mcl(device);
> +       mcap_invalidate_by_user_data(device->hdp_adapter->mi, device);
>
>        g_free(device);
>  }
> diff --git a/health/mcap.c b/health/mcap.c
> index cb7b74c..34ccdaa 100644
> --- a/health/mcap.c
> +++ b/health/mcap.c
> @@ -938,6 +938,27 @@ gboolean mcap_mcl_set_cb(struct mcap_mcl *mcl, gpointer user_data,
>        return TRUE;
>  }
>
> +static gint cmp_mcl_user_data(gconstpointer a, gconstpointer b)
> +{
> +       const struct mcap_mcl *mcl = a;
> +       gconstpointer user_data = b;
> +
> +       if (mcl->cb)
> +               if (mcl->cb->user_data == user_data)
> +                       return 0;
> +
> +       return 1;
> +}
> +
> +void mcap_invalidate_by_user_data(struct mcap_instance *mi, gpointer user_data)
> +{
> +       GSList *l;
> +
> +       while ((l = g_slist_find_custom(mi->cached, user_data,
> +                                               cmp_mcl_user_data)))
> +               mcap_close_mcl(l->data, FALSE);
> +}
> +
>  void mcap_mcl_get_addr(struct mcap_mcl *mcl, bdaddr_t *addr)
>  {
>        bacpy(addr, &mcl->addr);
> @@ -2143,4 +2164,4 @@ gboolean mcap_set_data_chan_mode(struct mcap_instance *mi, uint8_t mode,
>
>        return bt_io_set(mi->dcio, BT_IO_L2CAP, err, BT_IO_OPT_MODE, mode,
>                                                        BT_IO_OPT_INVALID);
> -}
> \ No newline at end of file
> +}
> diff --git a/health/mcap_lib.h b/health/mcap_lib.h
> index 7740623..cc10c17 100644
> --- a/health/mcap_lib.h
> +++ b/health/mcap_lib.h
> @@ -172,6 +172,8 @@ void mcap_mcl_get_addr(struct mcap_mcl *mcl, bdaddr_t *addr);
>  struct mcap_mcl *mcap_mcl_ref(struct mcap_mcl *mcl);
>  void mcap_mcl_unref(struct mcap_mcl *mcl);
>
> +void mcap_invalidate_by_user_data(struct mcap_instance *mi, gpointer user_data);
> +
>  /* CSP operations */
>
>  void mcap_enable_csp(struct mcap_instance *ms);

I'm not sure if this is the best way to face this issue. It seems that
this solution is a workaround to avoid the real problem. Let me have a
look and search for a better solution.

Regards.

^ permalink raw reply

* Re: [RFC] Bluetooth Low energy support
From: Ville Tervo @ 2010-11-10  6:20 UTC (permalink / raw)
  To: ext Anderson Lizardo; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <AANLkTina_HUT3SGEUMemVLrD4N3+Ni+xCsaC57whHDZG@mail.gmail.com>

Hi,

On Tue, Nov 09, 2010 at 05:25:10PM +0100, ext Anderson Lizardo wrote:
> Hi Ville,
> 
> On Mon, Oct 18, 2010 at 9:02 AM, Ville Tervo <ville.tervo@nokia.com> wrote:
> > Hi,
> >
> > Here is v2 of bluetooth low energy patch set.
> > Changes from previous version.
> 
> While doing tests with your most recent trees (using devel HW from
> TI), I'm getting consistent panic on the following test:
> 
> (dev1) hciconfig hciX leadv
> (dev2) hcitool -i hciX lecc <dev2_addr>

Did you have server listening on dev 2 on ATT socket?

> 
> I attached two logs. One is from dev1 machine (which has the oops),
> the other is from the dev2 machine.

I'll try to fix this today.

-- 
Ville

^ permalink raw reply

* Re: [PATCH v4] Bluetooth: btwilink driver
From: Marcel Holtmann @ 2010-11-10  6:08 UTC (permalink / raw)
  To: pavan_savoy; +Cc: padovan, linux-bluetooth, linux-kernel
In-Reply-To: <1287525975-17187-1-git-send-email-pavan_savoy@ti.com>

Hi Pavan,

> diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
> index 02deef4..8e0de9a 100644
> --- a/drivers/bluetooth/Kconfig
> +++ b/drivers/bluetooth/Kconfig
> @@ -219,4 +219,14 @@ config BT_ATH3K
>  	  Say Y here to compile support for "Atheros firmware download driver"
>  	  into the kernel or say M to compile it as module (ath3k).
>  
> +config BT_WILINK
> +	tristate "Texas Instruments WiLink7 driver"
> +	depends on TI_ST
> +	help
> +	  This enables the Bluetooth driver for Texas Instrument's BT/FM/GPS
> +	  combo devices. This makes use of shared transport line discipline
> +	  core driver to communicate with the BT core of the combo chip.
> +
> +	  Say Y here to compile support for Texas Instrument's WiLink7 driver
> +	  into the kernel or say M to compile it as module.
>  endmenu
> diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
> index 71bdf13..f4460f4 100644
> --- a/drivers/bluetooth/Makefile
> +++ b/drivers/bluetooth/Makefile
> @@ -18,6 +18,7 @@ obj-$(CONFIG_BT_HCIBTSDIO)	+= btsdio.o
>  obj-$(CONFIG_BT_ATH3K)		+= ath3k.o
>  obj-$(CONFIG_BT_MRVL)		+= btmrvl.o
>  obj-$(CONFIG_BT_MRVL_SDIO)	+= btmrvl_sdio.o
> +obj-$(CONFIG_BT_WILINK)		+= btwilink.o
>  
>  btmrvl-y			:= btmrvl_main.o
>  btmrvl-$(CONFIG_DEBUG_FS)	+= btmrvl_debugfs.o
> diff --git a/drivers/bluetooth/btwilink.c b/drivers/bluetooth/btwilink.c
> new file mode 100644
> index 0000000..218efd6
> --- /dev/null
> +++ b/drivers/bluetooth/btwilink.c
> @@ -0,0 +1,411 @@
> +/*
> + *  Texas Instrument's Bluetooth Driver For Shared Transport.
> + *
> + *  Bluetooth Driver acts as interface between HCI core and
> + *  TI Shared Transport Layer.
> + *
> + *  Copyright (C) 2009-2010 Texas Instruments
> + *  Author: Raja Mani <raja_mani@ti.com>
> + *	Pavan Savoy <pavan_savoy@ti.com>
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License version 2 as
> + *  published by the Free Software Foundation.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, write to the Free Software
> + *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
> + *
> + */
> +
> +#include <linux/platform_device.h>
> +#include <net/bluetooth/bluetooth.h>
> +#include <net/bluetooth/hci_core.h>
> +
> +#include <linux/ti_wilink_st.h>
> +
> +/* Bluetooth Driver Version */
> +#define VERSION               "1.0"
> +
> +/* Number of seconds to wait for registration completion
> + * when ST returns PENDING status.
> + */
> +#define BT_REGISTER_TIMEOUT   6000	/* 6 sec */
> +
> +/**
> + * struct ti_st - driver operation structure
> + * @hdev: hci device pointer which binds to bt driver
> + * @reg_status: ST registration callback status
> + * @st_write: write function provided by the ST driver
> + *	to be used by the driver during send_frame.
> + * @wait_reg_completion - completion sync between ti_st_open
> + *	and ti_st_registration_completion_cb.
> + */
> +struct ti_st {
> +	struct hci_dev *hdev;
> +	char reg_status;
> +	long (*st_write) (struct sk_buff *);
> +	struct completion wait_reg_completion;
> +};
> +
> +static int reset;
> +
> +/* Increments HCI counters based on pocket ID (cmd,acl,sco) */
> +static inline void ti_st_tx_complete(struct ti_st *hst, int pkt_type)
> +{
> +	struct hci_dev *hdev;
> +	hdev = hst->hdev;

please do this properly. Just write it like this:

	struct hci_dev *hdev = hst->hdev;
+
> +	/* Update HCI stat counters */
> +	switch (pkt_type) {
> +	case HCI_COMMAND_PKT:
> +		hdev->stat.cmd_tx++;
> +		break;
> +
> +	case HCI_ACLDATA_PKT:
> +		hdev->stat.acl_tx++;
> +		break;
> +
> +	case HCI_SCODATA_PKT:
> +		hdev->stat.sco_tx++;
> +		break;
> +	}
> +}
> +
> +/* ------- Interfaces to Shared Transport ------ */
> +
> +/* Called by ST layer to indicate protocol registration completion
> + * status.ti_st_open() function will wait for signal from this
> + * API when st_register() function returns ST_PENDING.
> + */
> +static void st_registration_completion_cb(void *priv_data, char data)
> +{
> +	struct ti_st *lhst = priv_data;
> +
> +	/* Save registration status for use in ti_st_open() */
> +	lhst->reg_status = data;
> +	/* complete the wait in ti_st_open() */
> +	complete(&lhst->wait_reg_completion);
> +}
> +
> +/* Called by Shared Transport layer when receive data is
> + * available */
> +static long st_receive(void *priv_data, struct sk_buff *skb)
> +{
> +	int err;
> +	struct ti_st *lhst = priv_data;

I really prefer if the variable with the assignment comes first.

> +	if (!skb)
> +		return -EFAULT;
> +
> +	if (!lhst) {
> +		kfree_skb(skb);
> +		return -EFAULT;
> +	}
> +
> +	skb->dev = (struct net_device *)lhst->hdev;

Don't do this cast. See the other drivers where we just use (void *)
cast.

> +	/* Forward skb to HCI core layer */
> +	err = hci_recv_frame(skb);
> +	if (err) {
> +		kfree_skb(skb);
> +		BT_ERR("Unable to push skb to HCI core(%d)", err);
> +		return err;
> +	}

So first of all, I prefer if you check like this:

	if (err < 0) {

And then second, you are double freeing the SKB here. The hci_recv_frame
will free the SKB in an error case.

> +
> +	lhst->hdev->stat.byte_rx += skb->len;
> +
> +	return 0;
> +}
> +
> +/* ------- Interfaces to HCI layer ------ */
> +/* protocol structure registered with shared transport */
> +static struct st_proto_s ti_st_proto = {
> +	.type = ST_BT,
> +	.recv = st_receive,
> +	.reg_complete_cb = st_registration_completion_cb,
> +	.priv_data = NULL,
> +};

Please don't bother with NULL assignment. It should not be needed.

> +/* Called from HCI core to initialize the device */
> +static int ti_st_open(struct hci_dev *hdev)
> +{
> +	unsigned long timeleft;
> +	struct ti_st *hst;
> +	int err;
> +
> +	BT_DBG("%s %p", hdev->name, hdev);
> +
> +	/* provide contexts for callbacks from ST */
> +	hst = hdev->driver_data;
> +	ti_st_proto.priv_data = hst;
> +
> +	err = st_register(&ti_st_proto);
> +	if (err == -EINPROGRESS) {
> +		/* Prepare wait-for-completion handler data structures.
> +		 * Needed to synchronize this and
> +		 * st_registration_completion_cb() functions.
> +		 */
> +		init_completion(&hst->wait_reg_completion);
> +
> +		/* Reset ST registration callback status flag , this value
> +		 * will be updated in ti_st_registration_completion_cb()
> +		 * function whenever it called from ST driver.
> +		 */
> +		hst->reg_status = -EINPROGRESS;
> +
> +		/* ST is busy with either protocol registration or firmware
> +		 * download. Wait until the registration callback is called
> +		 */
> +		BT_DBG(" waiting for registration completion signal from ST");
> +
> +		timeleft = wait_for_completion_timeout
> +			(&hst->wait_reg_completion,
> +			 msecs_to_jiffies(BT_REGISTER_TIMEOUT));
> +		if (!timeleft) {
> +			BT_ERR("Timeout(%d sec),didn't get reg "
> +					"completion signal from ST",
> +					BT_REGISTER_TIMEOUT / 1000);
> +			return -ETIMEDOUT;
> +		}
> +
> +		/* Is ST registration callback called with ERROR status? */
> +		if (hst->reg_status != 0) {
> +			BT_ERR("ST registration completed with invalid "
> +					"status %d", hst->reg_status);
> +			return -EAGAIN;
> +		}
> +		err = 0;
> +	} else if (err == -EPERM) {
> +		BT_ERR("st_register failed %d", err);
> +		return err;
> +	}
> +
> +	hst->st_write = ti_st_proto.write;
> +	if (!hst->st_write) {
> +		BT_ERR("undefined ST write function");
> +
> +		/* Undo registration with ST */
> +		err = st_unregister(ST_BT);
> +		if (err)
> +			BT_ERR("st_unregister() failed with error %d", err);
> +
> +		hst->st_write = NULL;
> +		return err;
> +	}
> +
> +	/* Registration with ST layer is successful,
> +	 * hardware is ready to accept commands from HCI core.
> +	 */
> +	set_bit(HCI_RUNNING, &hdev->flags);
> +
> +	return err;
> +}

I really don't like what you are doing here. So please use
test_and_set_bit and clear it in an error case.

Also you need to handle all error cases. Just not only two.

Where is the ti_st_proto.write coming from?

> +
> +/* Close device */
> +static int ti_st_close(struct hci_dev *hdev)
> +{
> +	int err;
> +	struct ti_st *hst = hdev->driver_data;
> +
> +	/* continue to unregister from transport */
> +	err = st_unregister(ST_BT);
> +	if (err)
> +		BT_ERR("st_unregister() failed with error %d", err);
> +
> +	hst->st_write = NULL;
> +
> +	return err;
> +}

You need a test_and_clear_bit for HCI_RUNNING. There is a huge imbalance
here. Have you tested this with consecutive hciconfig hci0 up/down
executions actually?

> +static int ti_st_send_frame(struct sk_buff *skb)
> +{
> +	struct hci_dev *hdev;
> +	struct ti_st *hst;
> +	long len;
> +
> +	if (!skb)
> +		return -ENOMEM;

Pointless check. The core will not call this function with a NULL
pointer SKB.

> +
> +	hdev = (struct hci_dev *)skb->dev;
> +	if (!hdev)
> +		return -ENODEV;

Even this can't really happen. Have you seen such a case?

> +	if (!test_bit(HCI_RUNNING, &hdev->flags))
> +		return -EBUSY;
> +
> +	hst = hdev->driver_data;
> +
> +	/* Prepend skb with frame type */
> +	memcpy(skb_push(skb, 1), &bt_cb(skb)->pkt_type, 1);
> +
> +	BT_DBG(" %s: type %d len %d", hdev->name, bt_cb(skb)->pkt_type,
> +			skb->len);
> +
> +	/* Insert skb to shared transport layer's transmit queue.
> +	 * Freeing skb memory is taken care in shared transport layer,
> +	 * so don't free skb memory here.
> +	 */
> +	if (!hst->st_write) {
> +		kfree_skb(skb);
> +		BT_ERR(" Could not write to ST (st_write is NULL)");
> +		return -EAGAIN;
> +	}

I don't like these crappy checks on every packet. That is just stupid.
You have checked for st_write when open happens and you set the hdev to
HCI_RUNNING. Are you saying this could change during the lifetime of the
hdev? If so then you have a serious problem here.

> +	len = hst->st_write(skb);
> +	if (len < 0) {
> +		kfree_skb(skb);
> +		BT_ERR(" ST write failed (%ld)", len);
> +		return -EAGAIN;
> +	}
> +
> +	/* ST accepted our skb. So, Go ahead and do rest */
> +	hdev->stat.byte_tx += len;
> +	ti_st_tx_complete(hst, bt_cb(skb)->pkt_type);
> +
> +	return 0;
> +}

What is the reason for this deferred stats update. That code looks
pretty much hackish to me.

> +static void ti_st_destruct(struct hci_dev *hdev)
> +{
> +	if (!hdev)
> +		return;
> +
> +	BT_DBG("%s", hdev->name);
> +
> +	/* free ti_st memory */
> +	kfree(hdev->driver_data);
> +
> +	return;
> +}

What are you checking here for? Why do you think that hdev would not be
valid? This is what the btusb and btsdio drivers do:

static void btusb_destruct(struct hci_dev *hdev)
{
        struct btusb_data *data = hdev->driver_data;

        BT_DBG("%s", hdev->name);

        kfree(data);
}


> +/* Creates new HCI device */
> +static int ti_st_register_dev(struct ti_st *hst)
> +{
> +	int err;
> +	struct hci_dev *hdev;

I prefer if err is last in the variable list.

> +
> +	/* Initialize and register HCI device */
> +	hdev = hci_alloc_dev();
> +	if (!hdev)
> +		return -ENOMEM;
> +
> +	BT_DBG("hdev %p", hdev);
> +
> +	hst->hdev = hdev;
> +	hdev->bus = HCI_UART;
> +	hdev->driver_data = hst;
> +	hdev->open = ti_st_open;
> +	hdev->close = ti_st_close;
> +	hdev->flush = NULL;

Please implement a flush callback.

> +	hdev->send = ti_st_send_frame;
> +	hdev->destruct = ti_st_destruct;
> +	hdev->owner = THIS_MODULE;
> +
> +	if (reset)
> +		set_bit(HCI_QUIRK_NO_RESET, &hdev->quirks);

Why do you need this? This should only be crappy devices. Something like
Bluetooth 1.0b old devices.

> +	err = hci_register_dev(hdev);
> +	if (err < 0) {
> +		BT_ERR("Can't register HCI device error %d", err);
> +		hci_free_dev(hdev);
> +		return err;
> +	}
> +
> +	BT_DBG(" HCI device registered (hdev %p)", hdev);
> +	return 0;
> +}
> +
> +

No double empty lines please.

> +static int bt_ti_probe(struct platform_device *pdev)
> +{
> +	int err;
> +	static struct ti_st *hst;

See above.

> +
> +	BT_DBG(" Bluetooth Driver Version %s", VERSION);

This should be in the module_init function. And should be a BT_INFO and
be precise what driver this actually this.

> +
> +	hst = kzalloc(sizeof(struct ti_st), GFP_KERNEL);
> +	if (!hst)
> +		return -ENOMEM;
> +
> +	/* Expose "hciX" device to user space */
> +	err = ti_st_register_dev(hst);
> +	if (err) {
> +		kfree(hst);
> +		return err;
> +	}

Is this ti_st_register device use anywhere else. Then please just
include that code in here to make this clear. All other drivers do all
the work in their probe() callback.

> +
> +	dev_set_drvdata(&pdev->dev, hst);
> +	return err;
> +}
> +
> +static int bt_ti_remove(struct platform_device *pdev)
> +{
> +	struct ti_st *hst;
> +	struct hci_dev *hdev;
> +
> +	hst = dev_get_drvdata(&pdev->dev);

Here I would prefer this:

	struct ti_st *hst = dev_get_drvdata(&pdev->dev);

> +
> +	if (!hst)
> +		return -EFAULT;
> +
> +	/* Deallocate local resource's memory  */
> +	hdev = hst->hdev;

That comment doesn't match what you are doing here.

> +
> +	if (!hdev) {
> +		BT_ERR("Invalid hdev memory");
> +		kfree(hst);
> +		return -EFAULT;
> +	}

No need to check for hdev here. If probe fails, then remove should never
be called, right?

And just to be safe you might wanna add this:

	dev_set_drvdata(&pdev->dev, NULL);

> +
> +	ti_st_close(hdev);
> +	hci_unregister_dev(hdev);
> +	/* Free HCI device memory */
> +	hci_free_dev(hdev);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver btwilink_driver = {
> +	.probe = bt_ti_probe,
> +	.remove = bt_ti_remove,
> +	.driver = {
> +		.name = "btwilink",
> +		.owner = THIS_MODULE,
> +	},
> +};
> +
> +/* ------- Module Init/Exit interfaces ------ */
> +static int __init bt_drv_init(void)
> +{
> +	long ret;
> +
> +	ret = platform_driver_register(&btwilink_driver);
> +	if (ret != 0) {
> +		BT_ERR("btwilink platform driver registration failed");
> +		return ret;
> +	}
> +	return 0;
> +}

please just do like we do with all other drivers;

	BT_INFO(...)

	return platform_driver_register(&btwilink_driver);

> +
> +static void __exit bt_drv_exit(void)
> +{
> +	platform_driver_unregister(&btwilink_driver);
> +}
> +
> +module_init(bt_drv_init);
> +module_exit(bt_drv_exit);

And this should be btwilink_init and btwilink_exit. Please don't try to
grab some generic namespace.

> +
> +/* ------ Module Info ------ */
> +
> +module_param(reset, bool, 0644);
> +MODULE_PARM_DESC(reset, "Send HCI reset command on initialization");

As mentioned above, that one seems wrong to me. You need to know what
your device supports. And by default it should allow sending HCI_Reset
at init. If not, then just that quirk. No need for module parameter
here.

> +MODULE_AUTHOR("Raja Mani <raja_mani@ti.com>");
> +MODULE_DESCRIPTION("Bluetooth Driver for TI Shared Transport" VERSION);
> +MODULE_VERSION(VERSION);
> +MODULE_LICENSE("GPL");

Regards

Marcel



^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox