[PATCH 0/5] android: Pairing improvements

[PATCH 0/5] android: Pairing improvements

[Szymon Janc]

Hi,

This improves (fix) the way dual mode devices are handled. Bearer type is
now tracked and allows to use correct one for dual mode device.

This also adds tracking if remote device was bonded or not. This is in
preparation for fixing no bond pairing. Android HAL does not distinguish
between pairing and bonding and appears to be ignoring bond state change
bonded->no_bond from stack. This leads to situation when device is no longer
paired (after being disconnected) but still being listed as bonded in
Android Framework. Possible solution would be to not call bond_state change
to bonded for no bond case (this seems to be what Bluedroid is doing [1])
but this still needs further analyze and testing.

BR
Szymon Janc


[1] http://androidxref.com/4.4.2_r1/xref/external/bluetooth/bluedroid/btif/src/btif_dm.c#338

Szymon Janc (5):
  android/bluetooth: Track device last connected bearer
  android/bluetooth: Fix pairing of dual mode devices
  android/bluetooth: Fix unpairing dual mode device
  android/bluetooth: Track if paired device is bonded
  android/bluetooth: Simplify check if device type changed

 android/bluetooth.c | 315 ++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 233 insertions(+), 82 deletions(-)

-- 
1.9.0

[PATCH 1/5] android/bluetooth: Track device last connected bearer

[Szymon Janc]

This allows to correctly choose address type when pairing dual mode
devices.
---
 android/bluetooth.c | 75 +++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 62 insertions(+), 13 deletions(-)

diff --git a/android/bluetooth.c b/android/bluetooth.c
index 1e760d4..bf562bc 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -104,7 +104,8 @@ struct device {
 	uint32_t class;
 	int32_t rssi;
 
-	uint32_t timestamp;
+	time_t bredr_seen;
+	time_t le_seen;
 
 	GSList *uuids;
 
@@ -263,7 +264,12 @@ static void store_device_info(struct device *dev, const char *path)
 	else
 		g_key_file_remove_key(key_file, addr, "Class", NULL);
 
-	g_key_file_set_integer(key_file, addr, "Timestamp", dev->timestamp);
+	if (dev->bredr_seen > dev->le_seen)
+		g_key_file_set_integer(key_file, addr, "Timestamp",
+							dev->bredr_seen);
+	else
+		g_key_file_set_integer(key_file, addr, "Timestamp",
+								dev->le_seen);
 
 	if (dev->uuids) {
 		GSList *l;
@@ -374,7 +380,6 @@ static void cache_device(struct device *new_dev)
 
 cache:
 	cached_devices = g_slist_prepend(cached_devices, new_dev);
-	new_dev->timestamp = time(NULL);
 	store_device_info(new_dev, CACHE_FILE);
 }
 
@@ -392,13 +397,14 @@ static struct device *create_device(const bdaddr_t *bdaddr, uint8_t bdaddr_type)
 
 	if (bdaddr_type == BDADDR_BREDR) {
 		dev->bredr = true;
+		dev->bredr_seen = time(NULL);
 	} else {
 		dev->le = true;
 		dev->bdaddr_type = bdaddr_type;
+		dev->le_seen = time(NULL);
 	}
 
 	dev->bond_state = HAL_BOND_STATE_NONE;
-	dev->timestamp = time(NULL);
 
 	/*
 	 * Use address for name, will be change if one is present
@@ -1400,6 +1406,11 @@ static void update_found_device(const bdaddr_t *bdaddr, uint8_t bdaddr_type,
 
 	dev = get_device(bdaddr, bdaddr_type);
 
+	if (bdaddr_type == BDADDR_BREDR)
+		dev->bredr_seen = time(NULL);
+	else
+		dev->le_seen = time(NULL);
+
 	/*
 	 * Device found event needs to be send also for known device if this is
 	 * new discovery session. Otherwise framework will ignore it.
@@ -2178,8 +2189,13 @@ static struct device *create_device_from_info(GKeyFile *key_file,
 
 	dev->class = g_key_file_get_integer(key_file, peer, "Class", NULL);
 
-	dev->timestamp = g_key_file_get_integer(key_file, peer, "Timestamp",
-									NULL);
+	if (dev->bredr)
+		dev->bredr_seen = g_key_file_get_integer(key_file, peer,
+								"Timestamp",
+								NULL);
+	else
+		dev->le_seen = g_key_file_get_integer(key_file, peer,
+							"Timestamp", NULL);
 
 	uuids = g_key_file_get_string_list(key_file, peer, "Services", NULL,
 									NULL);
@@ -2279,12 +2295,27 @@ failed:
 	return info;
 }
 
+static time_t device_timestamp(const struct device *dev)
+{
+	if (dev->bredr && dev->le) {
+		if (dev->le_seen > dev->bredr_seen)
+			return dev->le_seen;
+
+		return dev->bredr_seen;
+	}
+
+	if (dev->bredr)
+		return dev->bredr_seen;
+
+	return dev->le_seen;
+}
+
 static int device_timestamp_cmp(gconstpointer  a, gconstpointer  b)
 {
 	const struct device *deva = a;
 	const struct device *devb = b;
 
-	return deva->timestamp < devb->timestamp;
+	return device_timestamp(deva) < device_timestamp(devb);
 }
 
 static void load_devices_cache(void)
@@ -3155,6 +3186,18 @@ static void pair_device_complete(uint8_t status, uint16_t length,
 							HAL_BOND_STATE_NONE);
 }
 
+static uint8_t select_device_bearer(struct device *dev)
+{
+	if (dev->bredr && dev->le) {
+		if (dev->le_seen > dev->bredr_seen)
+			return dev->bdaddr_type;
+
+		return BDADDR_BREDR;
+	}
+
+	return dev->bredr ? BDADDR_BREDR : dev->bdaddr_type;
+}
+
 static void handle_create_bond_cmd(const void *buf, uint16_t len)
 {
 	const struct hal_cmd_create_bond *cmd = buf;
@@ -3173,7 +3216,7 @@ static void handle_create_bond_cmd(const void *buf, uint16_t len)
 		goto fail;
 	}
 
-	cp.addr.type = dev->bredr ? BDADDR_BREDR : dev->bdaddr_type;
+	cp.addr.type = select_device_bearer(dev);
 
 	if (mgmt_send(mgmt_if, MGMT_OP_PAIR_DEVICE, adapter.index, sizeof(cp),
 				&cp, pair_device_complete, NULL, NULL) == 0) {
@@ -3205,8 +3248,7 @@ static void handle_cancel_bond_cmd(const void *buf, uint16_t len)
 		goto failed;
 	}
 
-	cp.type = dev->bredr ? BDADDR_BREDR : dev->bdaddr_type;
-
+	cp.type = select_device_bearer(dev);
 
 	if (mgmt_reply(mgmt_if, MGMT_OP_CANCEL_PAIR_DEVICE,
 					adapter.index, sizeof(cp), &cp,
@@ -3256,7 +3298,7 @@ static void handle_remove_bond_cmd(const void *buf, uint16_t len)
 		goto failed;
 	}
 
-	cp.addr.type = dev->bredr ? BDADDR_BREDR : dev->bdaddr_type;
+	cp.addr.type = select_device_bearer(dev);
 
 	if (mgmt_send(mgmt_if, MGMT_OP_UNPAIR_DEVICE, adapter.index,
 				sizeof(cp), &cp, unpair_device_complete,
@@ -3494,8 +3536,12 @@ static uint8_t get_device_version_info(struct device *dev)
 
 static uint8_t get_device_timestamp(struct device *dev)
 {
+	uint32_t timestamp;
+
+	timestamp = device_timestamp(dev);
+
 	send_device_property(&dev->bdaddr, HAL_PROP_DEVICE_TIMESTAMP,
-				sizeof(dev->timestamp), &dev->timestamp);
+						sizeof(timestamp), &timestamp);
 
 	return HAL_STATUS_SUCCESS;
 }
@@ -3506,6 +3552,7 @@ static void get_remote_device_props(struct device *dev)
 	struct hal_ev_remote_device_props *ev = (void *) buf;
 	uint128_t uuids[g_slist_length(dev->uuids)];
 	uint8_t android_type;
+	uint32_t timestamp;
 	int size, i;
 	GSList *l;
 
@@ -3550,8 +3597,10 @@ static void get_remote_device_props(struct device *dev)
 									uuids);
 	ev->num_props++;
 
+	timestamp = get_device_timestamp(dev);
+
 	size += fill_hal_prop(buf + size, HAL_PROP_DEVICE_TIMESTAMP,
-				sizeof(dev->timestamp), &dev->timestamp);
+						sizeof(timestamp), &timestamp);
 	ev->num_props++;
 
 	ipc_send_notif(hal_ipc, HAL_SERVICE_ID_BLUETOOTH,
-- 
1.9.0

[PATCH 2/5] android/bluetooth: Fix pairing of dual mode devices

[Szymon Janc]

It is possible that dual mode device is paired both for LE and BR/EDR
link. This patch add tracking of this.

Due to HAL API contraints second pairing will result in following bond
state changed events:
bonded -> bonding (success) -> bonded (success/failed)
---
 android/bluetooth.c | 172 +++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 124 insertions(+), 48 deletions(-)

diff --git a/android/bluetooth.c b/android/bluetooth.c
index bf562bc..6743b81 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -96,7 +96,10 @@ struct device {
 	bool le;
 	bool bredr;
 
-	int bond_state;
+	bool pairing;
+
+	bool bredr_paired;
+	bool le_paired;
 
 	char *name;
 	char *friendly_name;
@@ -404,8 +407,6 @@ static struct device *create_device(const bdaddr_t *bdaddr, uint8_t bdaddr_type)
 		dev->le_seen = time(NULL);
 	}
 
-	dev->bond_state = HAL_BOND_STATE_NONE;
-
 	/*
 	 * Use address for name, will be change if one is present
 	 * eg. in EIR or set by set_property.
@@ -648,34 +649,93 @@ static void send_bond_state_change(const bdaddr_t *addr, uint8_t status,
 				HAL_EV_BOND_STATE_CHANGED, sizeof(ev), &ev);
 }
 
-static void set_device_bond_state(struct device *dev, uint8_t status,
-								int state)
+static void update_bredr_state(struct device *dev, bool pairing, bool paired)
 {
-	if (dev->bond_state == state)
+	if (pairing == dev->pairing && paired == dev->bredr_paired)
 		return;
 
-	switch (state) {
-	case HAL_BOND_STATE_NONE:
-		if (dev->bond_state == HAL_BOND_STATE_BONDED) {
-			bonded_devices = g_slist_remove(bonded_devices, dev);
-			remove_device_info(dev, DEVICES_FILE);
-			cache_device(dev);
-		}
-		break;
-	case HAL_BOND_STATE_BONDED:
+	/* avoid unpairing device on incoming pairing request */
+	if (pairing && dev->bredr_paired)
+		goto done;
+
+	/* avoid unpairing device if pairing failed */
+	if (!pairing && !paired && dev->pairing && dev->bredr_paired)
+		goto done;
+
+	if (paired && !dev->le_paired) {
 		cached_devices = g_slist_remove(cached_devices, dev);
 		bonded_devices = g_slist_prepend(bonded_devices, dev);
 		remove_device_info(dev, CACHE_FILE);
 		store_device_info(dev, DEVICES_FILE);
-		break;
-	case HAL_BOND_STATE_BONDING:
-	default:
-		break;
+	} else if (!paired && !dev->le_paired) {
+		bonded_devices = g_slist_remove(bonded_devices, dev);
+		remove_device_info(dev, DEVICES_FILE);
+		cache_device(dev);
+	}
+
+	dev->bredr_paired = paired;
+
+done:
+	dev->pairing = pairing;
+}
+
+static void update_le_state(struct device *dev, bool pairing, bool paired)
+{
+	if (pairing == dev->pairing && paired == dev->le_paired)
+		return;
+
+	/* avoid unpairing device on incoming pairing request */
+	if (pairing && dev->le_paired)
+		goto done;
+
+	/* avoid unpairing device if pairing failed */
+	if (!pairing && !paired && dev->pairing && dev->le_paired)
+		goto done;
+
+	if (paired && !dev->bredr_paired) {
+		cached_devices = g_slist_remove(cached_devices, dev);
+		bonded_devices = g_slist_prepend(bonded_devices, dev);
+		remove_device_info(dev, CACHE_FILE);
+		store_device_info(dev, DEVICES_FILE);
+	} else if (!paired && !dev->bredr_paired) {
+		bonded_devices = g_slist_remove(bonded_devices, dev);
+		remove_device_info(dev, DEVICES_FILE);
+		cache_device(dev);
 	}
 
-	dev->bond_state = state;
+	dev->le_paired = paired;
+
+done:
+	dev->pairing = pairing;
+}
+
+static uint8_t device_bond_state(struct device *dev)
+{
+	if (dev->pairing)
+		return HAL_BOND_STATE_BONDING;
+
+	if (dev->bredr_paired || dev->le_paired)
+		return HAL_BOND_STATE_BONDED;
+
+	return HAL_BOND_STATE_NONE;
+}
+
+static void update_device_state(struct device *dev, uint8_t addr_type,
+				uint8_t status, bool pairing, bool paired)
+{
+	uint8_t old_bond, new_bond;
+
+	old_bond = device_bond_state(dev);
+
+	if (addr_type == BDADDR_BREDR)
+		update_bredr_state(dev, pairing, paired);
+	else
+		update_le_state(dev, pairing, paired);
 
-	send_bond_state_change(&dev->bdaddr, status, state);
+	new_bond = device_bond_state(dev);
+
+	if (old_bond != new_bond)
+		send_bond_state_change(&dev->bdaddr, status, new_bond);
 }
 
 static  void send_device_property(const bdaddr_t *bdaddr, uint8_t type,
@@ -715,7 +775,7 @@ static void set_device_uuids(struct device *dev, GSList *uuids)
 	g_slist_free_full(dev->uuids, g_free);
 	dev->uuids = uuids;
 
-	if (dev->bond_state == HAL_BOND_STATE_BONDED)
+	if (dev->le_paired || dev->bredr_paired)
 		store_device_info(dev, DEVICES_FILE);
 	else
 		store_device_info(dev, CACHE_FILE);
@@ -882,7 +942,8 @@ static void new_link_key_callback(uint16_t index, uint16_t length,
 	if (!dev)
 		return;
 
-	set_device_bond_state(dev, HAL_STATUS_SUCCESS, HAL_BOND_STATE_BONDED);
+	update_device_state(dev, ev->key.addr.type, HAL_STATUS_SUCCESS, false,
+									true);
 
 	if (ev->store_hint) {
 		const struct mgmt_link_key_info *key = &ev->key;
@@ -925,7 +986,8 @@ static void pin_code_request_callback(uint16_t index, uint16_t length,
 	 */
 	get_device_name(dev);
 
-	set_device_bond_state(dev, HAL_STATUS_SUCCESS, HAL_BOND_STATE_BONDING);
+	update_device_state(dev, ev->addr.type, HAL_STATUS_SUCCESS, true,
+									false);
 
 	DBG("%s type %u secure %u", dst, ev->addr.type, ev->secure);
 
@@ -973,7 +1035,8 @@ static void user_confirm_request_callback(uint16_t index, uint16_t length,
 	if (!dev)
 		return;
 
-	set_device_bond_state(dev, HAL_STATUS_SUCCESS, HAL_BOND_STATE_BONDING);
+	update_device_state(dev, ev->addr.type, HAL_STATUS_SUCCESS, true,
+									false);
 
 	if (ev->confirm_hint)
 		send_ssp_request(dev, HAL_SSP_VARIANT_CONSENT, 0);
@@ -1000,7 +1063,8 @@ static void user_passkey_request_callback(uint16_t index, uint16_t length,
 	if (!dev)
 		return;
 
-	set_device_bond_state(dev, HAL_STATUS_SUCCESS, HAL_BOND_STATE_BONDING);
+	update_device_state(dev, ev->addr.type, HAL_STATUS_SUCCESS, true,
+									false);
 
 	send_ssp_request(dev, HAL_SSP_VARIANT_ENTRY, 0);
 }
@@ -1029,7 +1093,8 @@ static void user_passkey_notify_callback(uint16_t index, uint16_t length,
 	if (!dev)
 		return;
 
-	set_device_bond_state(dev, HAL_STATUS_SUCCESS, HAL_BOND_STATE_BONDING);
+	update_device_state(dev, ev->addr.type, HAL_STATUS_SUCCESS, true,
+									false);
 
 	send_ssp_request(dev, HAL_SSP_VARIANT_NOTIF, ev->passkey);
 }
@@ -1383,7 +1448,7 @@ static bool is_new_device(const struct device *dev, unsigned int flags)
 	if (dev->found)
 		return false;
 
-	if (dev->bond_state == HAL_BOND_STATE_BONDED)
+	if (dev->bredr_paired || dev->le_paired)
 		return false;
 
 	if (dev->bdaddr_type != BDADDR_BREDR &&
@@ -1432,7 +1497,7 @@ static void update_found_device(const bdaddr_t *bdaddr, uint8_t bdaddr_type,
 								discoverable);
 	}
 
-	if (dev->bond_state != HAL_BOND_STATE_BONDED)
+	if (!dev->bredr_paired && !dev->le_paired)
 		cache_device(dev);
 
 	if (confirm) {
@@ -1583,11 +1648,11 @@ static void mgmt_connect_failed_event(uint16_t index, uint16_t length,
 	 * bonding, if so update bond state
 	 */
 
-	if (dev->bond_state != HAL_BOND_STATE_BONDING)
+	if (!dev->pairing)
 		return;
 
-	set_device_bond_state(dev, status_mgmt2hal(ev->status),
-							HAL_BOND_STATE_NONE);
+	update_device_state(dev, ev->addr.type, status_mgmt2hal(ev->status),
+								false, false);
 }
 
 static void mgmt_auth_failed_event(uint16_t index, uint16_t length,
@@ -1607,11 +1672,11 @@ static void mgmt_auth_failed_event(uint16_t index, uint16_t length,
 	if (!dev)
 		return;
 
-	if (dev->bond_state != HAL_BOND_STATE_BONDING)
+	if (!dev->pairing)
 		return;
 
-	set_device_bond_state(dev, status_mgmt2hal(ev->status),
-							HAL_BOND_STATE_NONE);
+	update_device_state(dev, ev->addr.type, status_mgmt2hal(ev->status),
+								false, false);
 }
 
 static void mgmt_device_unpaired_event(uint16_t index, uint16_t length,
@@ -1633,7 +1698,8 @@ static void mgmt_device_unpaired_event(uint16_t index, uint16_t length,
 	if (!dev)
 		return;
 
-	set_device_bond_state(dev, HAL_STATUS_SUCCESS, HAL_BOND_STATE_NONE);
+	update_device_state(dev, ev->addr.type, HAL_STATUS_SUCCESS, false,
+									false);
 }
 
 static void store_ltk(const bdaddr_t *dst, uint8_t bdaddr_type, bool master,
@@ -1703,7 +1769,8 @@ static void new_long_term_key_event(uint16_t index, uint16_t length,
 	if (!dev)
 		return;
 
-	set_device_bond_state(dev, HAL_STATUS_SUCCESS, HAL_BOND_STATE_BONDED);
+	update_device_state(dev, ev->key.addr.type, HAL_STATUS_SUCCESS, false,
+									true);
 
 	if (ev->store_hint) {
 		const struct mgmt_ltk_info *key = &ev->key;
@@ -2160,19 +2227,19 @@ static struct device *create_device_from_info(GKeyFile *key_file,
 	str = g_key_file_get_string(key_file, peer, "LinkKey", NULL);
 	if (str) {
 		g_free(str);
-		dev->bond_state = HAL_BOND_STATE_BONDED;
+		dev->bredr_paired = true;
 	}
 
 	str = g_key_file_get_string(key_file, peer, "LongTermKey", NULL);
 	if (str) {
 		g_free(str);
-		dev->bond_state = HAL_BOND_STATE_BONDED;
+		dev->le_paired = true;
 	}
 
 	str = g_key_file_get_string(key_file, peer, "SlaveLongTermKey", NULL);
 	if (str) {
 		g_free(str);
-		dev->bond_state = HAL_BOND_STATE_BONDED;
+		dev->le_paired = true;
 	}
 
 	str = g_key_file_get_string(key_file, peer, "Name", NULL);
@@ -3182,8 +3249,8 @@ static void pair_device_complete(uint8_t status, uint16_t length,
 	if (!dev)
 		return;
 
-	set_device_bond_state(dev, status_mgmt2hal(status),
-							HAL_BOND_STATE_NONE);
+	update_device_state(dev, rp->addr.type, status_mgmt2hal(status), false,
+									false);
 }
 
 static uint8_t select_device_bearer(struct device *dev)
@@ -3198,6 +3265,14 @@ static uint8_t select_device_bearer(struct device *dev)
 	return dev->bredr ? BDADDR_BREDR : dev->bdaddr_type;
 }
 
+static bool device_is_paired(struct device *dev, uint8_t addr_type)
+{
+	if (addr_type == BDADDR_BREDR)
+		return dev->bredr_paired;
+
+	return dev->le_paired;
+}
+
 static void handle_create_bond_cmd(const void *buf, uint16_t len)
 {
 	const struct hal_cmd_create_bond *cmd = buf;
@@ -3211,13 +3286,13 @@ static void handle_create_bond_cmd(const void *buf, uint16_t len)
 	/* type is used only as fallback when device is not in cache */
 	dev = get_device(&cp.addr.bdaddr, BDADDR_BREDR);
 
-	if (dev->bond_state != HAL_BOND_STATE_NONE) {
+	cp.addr.type = select_device_bearer(dev);
+
+	if (device_is_paired(dev, cp.addr.type)) {
 		status = HAL_STATUS_FAILED;
 		goto fail;
 	}
 
-	cp.addr.type = select_device_bearer(dev);
-
 	if (mgmt_send(mgmt_if, MGMT_OP_PAIR_DEVICE, adapter.index, sizeof(cp),
 				&cp, pair_device_complete, NULL, NULL) == 0) {
 		status = HAL_STATUS_FAILED;
@@ -3226,7 +3301,7 @@ static void handle_create_bond_cmd(const void *buf, uint16_t len)
 
 	status = HAL_STATUS_SUCCESS;
 
-	set_device_bond_state(dev, HAL_STATUS_SUCCESS, HAL_BOND_STATE_BONDING);
+	update_device_state(dev, cp.addr.type, HAL_STATUS_SUCCESS, true, false);
 
 fail:
 	ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_BLUETOOTH, HAL_OP_CREATE_BOND,
@@ -3279,7 +3354,8 @@ static void unpair_device_complete(uint8_t status, uint16_t length,
 	if (!dev)
 		return;
 
-	set_device_bond_state(dev, HAL_STATUS_SUCCESS, HAL_BOND_STATE_NONE);
+	update_device_state(dev, rp->addr.type, HAL_STATUS_SUCCESS, false,
+									false);
 }
 
 static void handle_remove_bond_cmd(const void *buf, uint16_t len)
@@ -3765,7 +3841,7 @@ static uint8_t set_device_friendly_name(struct device *dev, const uint8_t *val,
 	g_free(dev->friendly_name);
 	dev->friendly_name = g_strndup((const char *) val, len);
 
-	if (dev->bond_state == HAL_BOND_STATE_BONDED)
+	if (dev->bredr_paired || dev->le_paired)
 		store_device_info(dev, DEVICES_FILE);
 	else
 		store_device_info(dev, CACHE_FILE);
-- 
1.9.0

[PATCH 3/5] android/bluetooth: Fix unpairing dual mode device

[Szymon Janc]

If device is paired on LE and BR/EDR it should be unpaired on both
bearers.
---
 android/bluetooth.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/android/bluetooth.c b/android/bluetooth.c
index 6743b81..29c4ab6 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -3374,13 +3374,26 @@ static void handle_remove_bond_cmd(const void *buf, uint16_t len)
 		goto failed;
 	}
 
-	cp.addr.type = select_device_bearer(dev);
+	if (dev->le_paired) {
+		cp.addr.type = dev->bdaddr_type;
 
-	if (mgmt_send(mgmt_if, MGMT_OP_UNPAIR_DEVICE, adapter.index,
-				sizeof(cp), &cp, unpair_device_complete,
-				NULL, NULL) ==  0) {
-		status = HAL_STATUS_FAILED;
-		goto failed;
+		if (mgmt_send(mgmt_if, MGMT_OP_UNPAIR_DEVICE, adapter.index,
+					sizeof(cp), &cp, unpair_device_complete,
+					NULL, NULL) == 0) {
+			status = HAL_STATUS_FAILED;
+			goto failed;
+		}
+	}
+
+	if (dev->bredr_paired) {
+		cp.addr.type = BDADDR_BREDR;
+
+		if (mgmt_send(mgmt_if, MGMT_OP_UNPAIR_DEVICE, adapter.index,
+					sizeof(cp), &cp, unpair_device_complete,
+					NULL, NULL) == 0) {
+			status = HAL_STATUS_FAILED;
+			goto failed;
+		}
 	}
 
 	status = HAL_STATUS_SUCCESS;
-- 
1.9.0

[PATCH 4/5] android/bluetooth: Track if paired device is bonded

[Szymon Janc]

This is a preparation for correctly handling no bond pairing.

Currently paired but not bonded devices are left on list of bonded
devices in Framework until BT is restarted on device. This is due to
Android Framework is not allowing stack to unpair device by its own
(setting bond state bonded->no_bond is ignored).
---
 android/bluetooth.c | 55 +++++++++++++++++++++++++++++++++++------------------
 1 file changed, 36 insertions(+), 19 deletions(-)

diff --git a/android/bluetooth.c b/android/bluetooth.c
index 29c4ab6..5163291 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -99,7 +99,9 @@ struct device {
 	bool pairing;
 
 	bool bredr_paired;
+	bool bredr_bonded;
 	bool le_paired;
+	bool le_bonded;
 
 	char *name;
 	char *friendly_name;
@@ -649,9 +651,11 @@ static void send_bond_state_change(const bdaddr_t *addr, uint8_t status,
 				HAL_EV_BOND_STATE_CHANGED, sizeof(ev), &ev);
 }
 
-static void update_bredr_state(struct device *dev, bool pairing, bool paired)
+static void update_bredr_state(struct device *dev, bool pairing, bool paired,
+								bool bonded)
 {
-	if (pairing == dev->pairing && paired == dev->bredr_paired)
+	if (pairing == dev->pairing && paired == dev->bredr_paired &&
+						bonded == dev->bredr_bonded)
 		return;
 
 	/* avoid unpairing device on incoming pairing request */
@@ -674,14 +678,17 @@ static void update_bredr_state(struct device *dev, bool pairing, bool paired)
 	}
 
 	dev->bredr_paired = paired;
+	dev->bredr_bonded = bonded;
 
 done:
 	dev->pairing = pairing;
 }
 
-static void update_le_state(struct device *dev, bool pairing, bool paired)
+static void update_le_state(struct device *dev, bool pairing, bool paired,
+								bool bonded)
 {
-	if (pairing == dev->pairing && paired == dev->le_paired)
+	if (pairing == dev->pairing && paired == dev->le_paired &&
+						bonded == dev->le_bonded)
 		return;
 
 	/* avoid unpairing device on incoming pairing request */
@@ -704,6 +711,7 @@ static void update_le_state(struct device *dev, bool pairing, bool paired)
 	}
 
 	dev->le_paired = paired;
+	dev->le_bonded = bonded;
 
 done:
 	dev->pairing = pairing;
@@ -714,6 +722,10 @@ static uint8_t device_bond_state(struct device *dev)
 	if (dev->pairing)
 		return HAL_BOND_STATE_BONDING;
 
+	/*
+	 * We are checking for paired here instead of bonded as HAL API is
+	 * using BOND state also if there was no bonding pairing.
+	 */
 	if (dev->bredr_paired || dev->le_paired)
 		return HAL_BOND_STATE_BONDED;
 
@@ -721,16 +733,17 @@ static uint8_t device_bond_state(struct device *dev)
 }
 
 static void update_device_state(struct device *dev, uint8_t addr_type,
-				uint8_t status, bool pairing, bool paired)
+				uint8_t status, bool pairing, bool paired,
+				bool bonded)
 {
 	uint8_t old_bond, new_bond;
 
 	old_bond = device_bond_state(dev);
 
 	if (addr_type == BDADDR_BREDR)
-		update_bredr_state(dev, pairing, paired);
+		update_bredr_state(dev, pairing, paired, bonded);
 	else
-		update_le_state(dev, pairing, paired);
+		update_le_state(dev, pairing, paired, bonded);
 
 	new_bond = device_bond_state(dev);
 
@@ -943,7 +956,7 @@ static void new_link_key_callback(uint16_t index, uint16_t length,
 		return;
 
 	update_device_state(dev, ev->key.addr.type, HAL_STATUS_SUCCESS, false,
-									true);
+							true, !!ev->store_hint);
 
 	if (ev->store_hint) {
 		const struct mgmt_link_key_info *key = &ev->key;
@@ -987,7 +1000,7 @@ static void pin_code_request_callback(uint16_t index, uint16_t length,
 	get_device_name(dev);
 
 	update_device_state(dev, ev->addr.type, HAL_STATUS_SUCCESS, true,
-									false);
+								false, false);
 
 	DBG("%s type %u secure %u", dst, ev->addr.type, ev->secure);
 
@@ -1036,7 +1049,7 @@ static void user_confirm_request_callback(uint16_t index, uint16_t length,
 		return;
 
 	update_device_state(dev, ev->addr.type, HAL_STATUS_SUCCESS, true,
-									false);
+								false, false);
 
 	if (ev->confirm_hint)
 		send_ssp_request(dev, HAL_SSP_VARIANT_CONSENT, 0);
@@ -1064,7 +1077,7 @@ static void user_passkey_request_callback(uint16_t index, uint16_t length,
 		return;
 
 	update_device_state(dev, ev->addr.type, HAL_STATUS_SUCCESS, true,
-									false);
+								false, false);
 
 	send_ssp_request(dev, HAL_SSP_VARIANT_ENTRY, 0);
 }
@@ -1094,7 +1107,7 @@ static void user_passkey_notify_callback(uint16_t index, uint16_t length,
 		return;
 
 	update_device_state(dev, ev->addr.type, HAL_STATUS_SUCCESS, true,
-									false);
+								false, false);
 
 	send_ssp_request(dev, HAL_SSP_VARIANT_NOTIF, ev->passkey);
 }
@@ -1652,7 +1665,7 @@ static void mgmt_connect_failed_event(uint16_t index, uint16_t length,
 		return;
 
 	update_device_state(dev, ev->addr.type, status_mgmt2hal(ev->status),
-								false, false);
+							false, false, false);
 }
 
 static void mgmt_auth_failed_event(uint16_t index, uint16_t length,
@@ -1676,7 +1689,7 @@ static void mgmt_auth_failed_event(uint16_t index, uint16_t length,
 		return;
 
 	update_device_state(dev, ev->addr.type, status_mgmt2hal(ev->status),
-								false, false);
+							false, false, false);
 }
 
 static void mgmt_device_unpaired_event(uint16_t index, uint16_t length,
@@ -1699,7 +1712,7 @@ static void mgmt_device_unpaired_event(uint16_t index, uint16_t length,
 		return;
 
 	update_device_state(dev, ev->addr.type, HAL_STATUS_SUCCESS, false,
-									false);
+								false, false);
 }
 
 static void store_ltk(const bdaddr_t *dst, uint8_t bdaddr_type, bool master,
@@ -1770,7 +1783,7 @@ static void new_long_term_key_event(uint16_t index, uint16_t length,
 		return;
 
 	update_device_state(dev, ev->key.addr.type, HAL_STATUS_SUCCESS, false,
-									true);
+							true, !!ev->store_hint);
 
 	if (ev->store_hint) {
 		const struct mgmt_ltk_info *key = &ev->key;
@@ -2228,18 +2241,21 @@ static struct device *create_device_from_info(GKeyFile *key_file,
 	if (str) {
 		g_free(str);
 		dev->bredr_paired = true;
+		dev->bredr_bonded = true;
 	}
 
 	str = g_key_file_get_string(key_file, peer, "LongTermKey", NULL);
 	if (str) {
 		g_free(str);
 		dev->le_paired = true;
+		dev->le_bonded = true;
 	}
 
 	str = g_key_file_get_string(key_file, peer, "SlaveLongTermKey", NULL);
 	if (str) {
 		g_free(str);
 		dev->le_paired = true;
+		dev->le_bonded = true;
 	}
 
 	str = g_key_file_get_string(key_file, peer, "Name", NULL);
@@ -3250,7 +3266,7 @@ static void pair_device_complete(uint8_t status, uint16_t length,
 		return;
 
 	update_device_state(dev, rp->addr.type, status_mgmt2hal(status), false,
-									false);
+								false, false);
 }
 
 static uint8_t select_device_bearer(struct device *dev)
@@ -3301,7 +3317,8 @@ static void handle_create_bond_cmd(const void *buf, uint16_t len)
 
 	status = HAL_STATUS_SUCCESS;
 
-	update_device_state(dev, cp.addr.type, HAL_STATUS_SUCCESS, true, false);
+	update_device_state(dev, cp.addr.type, HAL_STATUS_SUCCESS, true, false,
+									false);
 
 fail:
 	ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_BLUETOOTH, HAL_OP_CREATE_BOND,
@@ -3355,7 +3372,7 @@ static void unpair_device_complete(uint8_t status, uint16_t length,
 		return;
 
 	update_device_state(dev, rp->addr.type, HAL_STATUS_SUCCESS, false,
-									false);
+								false, false);
 }
 
 static void handle_remove_bond_cmd(const void *buf, uint16_t len)
-- 
1.9.0

[PATCH 5/5] android/bluetooth: Simplify check if device type changed

[Szymon Janc]

This makes code easier to follow. Also LE address type is not lost
if dual mode device connected over BR/EDR.
---
 android/bluetooth.c | 30 +++++++++++++-----------------
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/android/bluetooth.c b/android/bluetooth.c
index 5163291..ca6d242 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -1389,7 +1389,7 @@ static void update_device(struct device *dev, int8_t rssi,
 {
 	uint8_t buf[IPC_MTU];
 	struct hal_ev_remote_device_props *ev = (void *) buf;
-	uint8_t android_type;
+	uint8_t old_type, new_type;
 	int size;
 
 	memset(buf, 0, sizeof(buf));
@@ -1399,25 +1399,21 @@ static void update_device(struct device *dev, int8_t rssi,
 	ev->status = HAL_STATUS_SUCCESS;
 	bdaddr2android(&dev->bdaddr, ev->bdaddr);
 
-	if (dev->bdaddr_type != bdaddr_type) {
-		bool type_changed = false;
+	old_type = get_device_android_type(dev);
 
+	if (bdaddr_type == BDADDR_BREDR) {
+		dev->bredr = true;
+	} else {
+		dev->le = true;
 		dev->bdaddr_type = bdaddr_type;
-		if (bdaddr_type == BDADDR_BREDR) {
-			type_changed = !dev->bredr;
-			dev->bredr = true;
-		} else {
-			type_changed = !dev->le;
-			dev->le = true;
-		}
+	}
 
-		if (type_changed) {
-			android_type = get_device_android_type(dev);
-			size += fill_hal_prop(buf + size, HAL_PROP_DEVICE_TYPE,
-							sizeof(android_type),
-							&android_type);
-			ev->num_props++;
-		}
+	new_type = get_device_android_type(dev);
+
+	if (old_type != new_type) {
+		size += fill_hal_prop(buf + size, HAL_PROP_DEVICE_TYPE,
+						sizeof(new_type), &new_type);
+		ev->num_props++;
 	}
 
 	if (eir->class && dev->class != eir->class) {
-- 
1.9.0

Re: [PATCH 0/5] android: Pairing improvements

[Szymon Janc]

On Sunday 18 of May 2014 13:53:55 Szymon Janc wrote:
> Hi,
> 
> This improves (fix) the way dual mode devices are handled. Bearer type is
> now tracked and allows to use correct one for dual mode device.
> 
> This also adds tracking if remote device was bonded or not. This is in
> preparation for fixing no bond pairing. Android HAL does not distinguish
> between pairing and bonding and appears to be ignoring bond state change
> bonded->no_bond from stack. This leads to situation when device is no longer
> paired (after being disconnected) but still being listed as bonded in
> Android Framework. Possible solution would be to not call bond_state change
> to bonded for no bond case (this seems to be what Bluedroid is doing [1])
> but this still needs further analyze and testing.
> 
> BR
> Szymon Janc
> 
> 
> [1] http://androidxref.com/4.4.2_r1/xref/external/bluetooth/bluedroid/btif/src/btif_dm.c#338
> 
> Szymon Janc (5):
>   android/bluetooth: Track device last connected bearer
>   android/bluetooth: Fix pairing of dual mode devices
>   android/bluetooth: Fix unpairing dual mode device
>   android/bluetooth: Track if paired device is bonded
>   android/bluetooth: Simplify check if device type changed
> 
>  android/bluetooth.c | 315 ++++++++++++++++++++++++++++++++++++++--------------
>  1 file changed, 233 insertions(+), 82 deletions(-)
> 
> 

Pushed.

-- 
Best regards, 
Szymon Janc