[PATCH v2 0/3] Fixes for incoming connection and bonding

[PATCH v2 0/3] Fixes for incoming connection and bonding

[Lukasz Rymanowski]

V2:
Bonding state added.

V1:
With those patches it is possible to bond from remote device and in addition
you will see the name of remote device on Android pairing request pop up.

Lukasz Rymanowski (3):
  android: Update bond state on incoming bonding
  android: Update HAL with device info on incoming connection
  android: Change TODO with explaining comment

 android/adapter.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 56 insertions(+), 9 deletions(-)

-- 
1.8.4

[PATCH v2 1/3] android: Update bond state on incoming bonding

[Lukasz Rymanowski]

Before sending any ssp request or pin code request up to HAL library we
need to send bond state change with bonding state. Otherwise incoming
bonding is impossible.
Add bonding tracking to adapter.
---
 android/adapter.c | 50 +++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 45 insertions(+), 5 deletions(-)

diff --git a/android/adapter.c b/android/adapter.c
index 65b3170..f92301e 100644
--- a/android/adapter.c
+++ b/android/adapter.c
@@ -74,6 +74,8 @@ struct bt_adapter {
 
 	bool discovering;
 	uint32_t discoverable_timeout;
+
+	bool bonding;
 };
 
 struct browse_req {
@@ -486,6 +488,7 @@ static void new_link_key_callback(uint16_t index, uint16_t length,
 	send_bond_state_change(&addr->bdaddr, HAL_STATUS_SUCCESS,
 							HAL_BOND_STATE_BONDED);
 
+	adapter->bonding = false;
 	browse_remote_sdp(&addr->bdaddr);
 }
 
@@ -501,6 +504,13 @@ static void pin_code_request_callback(uint16_t index, uint16_t length,
 		return;
 	}
 
+	if (!adapter->bonding) {
+		adapter->bonding = true;
+		/* Update bonding state */
+		send_bond_state_change(&ev->addr.bdaddr, HAL_STATUS_SUCCESS,
+						HAL_BOND_STATE_BONDING);
+	}
+
 	ba2str(&ev->addr.bdaddr, dst);
 
 	DBG("%s type %u secure %u", dst, ev->addr.type, ev->secure);
@@ -542,6 +552,13 @@ static void user_confirm_request_callback(uint16_t index, uint16_t length,
 	ba2str(&ev->addr.bdaddr, dst);
 	DBG("%s confirm_hint %u", dst, ev->confirm_hint);
 
+	if (!adapter->bonding) {
+		adapter->bonding = true;
+		/* Update bonding state */
+		send_bond_state_change(&ev->addr.bdaddr, HAL_STATUS_SUCCESS,
+						HAL_BOND_STATE_BONDING);
+	}
+
 	if (ev->confirm_hint)
 		send_ssp_request(&ev->addr.bdaddr, HAL_SSP_VARIANT_CONSENT, 0);
 	else
@@ -563,6 +580,13 @@ static void user_passkey_request_callback(uint16_t index, uint16_t length,
 	ba2str(&ev->addr.bdaddr, dst);
 	DBG("%s", dst);
 
+	if (!adapter->bonding) {
+		adapter->bonding = true;
+		/* Update bonding state */
+		send_bond_state_change(&ev->addr.bdaddr, HAL_STATUS_SUCCESS,
+						HAL_BOND_STATE_BONDING);
+	}
+
 	send_ssp_request(&ev->addr.bdaddr, HAL_SSP_VARIANT_ENTRY, 0);
 }
 
@@ -581,9 +605,17 @@ static void user_passkey_notify_callback(uint16_t index, uint16_t length,
 	DBG("%s entered %u", dst, ev->entered);
 
 	/* HAL seems to not support entered characters */
-	if (!ev->entered)
-		send_ssp_request(&ev->addr.bdaddr, HAL_SSP_VARIANT_NOTIF,
-								ev->passkey);
+	if (ev->entered)
+		return;
+
+	if (!adapter->bonding) {
+		adapter->bonding=true;
+		/* Update bonding state */
+		send_bond_state_change(&ev->addr.bdaddr, HAL_STATUS_SUCCESS,
+						HAL_BOND_STATE_BONDING);
+	}
+
+	send_ssp_request(&ev->addr.bdaddr, HAL_SSP_VARIANT_NOTIF, ev->passkey);
 }
 
 static void mgmt_discovering_event(uint16_t index, uint16_t length,
@@ -1166,6 +1198,7 @@ void bt_adapter_init(uint16_t index, struct mgmt *mgmt, bt_adapter_ready cb)
 	adapter->ready = cb;
 	/* TODO: Read it from some storage */
 	adapter->discoverable_timeout = DEFAULT_DISCOVERABLE_TIMEOUT;
+	adapter->bonding = false;
 
 	if (mgmt_send(mgmt, MGMT_OP_READ_INFO, index, 0, NULL,
 					read_info_complete, NULL, NULL) > 0)
@@ -1486,6 +1519,7 @@ static void pair_device_complete(uint8_t status, uint16_t length,
 	if (status == MGMT_STATUS_SUCCESS)
 		return;
 
+	adapter->bonding = false;
 	send_bond_state_change(&rp->addr.bdaddr, status_mgmt2hal(status),
 							HAL_BOND_STATE_NONE);
 }
@@ -1504,6 +1538,7 @@ static bool create_bond(void *buf, uint16_t len)
 				NULL) == 0)
 		return false;
 
+	adapter->bonding = true;
 	send_bond_state_change(&cp.addr.bdaddr, HAL_STATUS_SUCCESS,
 						HAL_BOND_STATE_BONDING);
 
@@ -1514,13 +1549,18 @@ static bool cancel_bond(void *buf, uint16_t len)
 {
 	struct hal_cmd_cancel_bond *cmd = buf;
 	struct mgmt_addr_info cp;
+	bool result;
 
 	cp.type = BDADDR_BREDR;
 	android2bdaddr(cmd->bdaddr, &cp.bdaddr);
 
-	return mgmt_reply(adapter->mgmt, MGMT_OP_CANCEL_PAIR_DEVICE,
+	result = mgmt_reply(adapter->mgmt, MGMT_OP_CANCEL_PAIR_DEVICE,
 				adapter->index, sizeof(cp), &cp, NULL, NULL,
 				NULL) > 0;
+	if (result)
+		adapter->bonding = false;
+
+	return result;
 }
 
 static void unpair_device_complete(uint8_t status, uint16_t length,
@@ -1737,7 +1777,7 @@ void bt_adapter_handle_cmd(int sk, uint8_t opcode, void *buf, uint16_t len)
 
 		break;
 	case HAL_OP_CREATE_BOND:
-		if (!create_bond(buf, len))
+		if (adapter->bonding || !create_bond(buf, len))
 			goto error;
 
 		break;
-- 
1.8.4

Re: [PATCH v2 1/3] android: Update bond state on incoming bonding

[Johan Hedberg]

Hi Lukasz,

On Tue, Nov 12, 2013, Lukasz Rymanowski wrote:
> Before sending any ssp request or pin code request up to HAL library we
> need to send bond state change with bonding state. Otherwise incoming
> bonding is impossible.
> Add bonding tracking to adapter.
> ---
>  android/adapter.c | 50 +++++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 45 insertions(+), 5 deletions(-)
> 
> diff --git a/android/adapter.c b/android/adapter.c
> index 65b3170..f92301e 100644
> --- a/android/adapter.c
> +++ b/android/adapter.c
> @@ -74,6 +74,8 @@ struct bt_adapter {
>  
>  	bool discovering;
>  	uint32_t discoverable_timeout;
> +
> +	bool bonding;
>  };
>  
>  struct browse_req {
> @@ -486,6 +488,7 @@ static void new_link_key_callback(uint16_t index, uint16_t length,
>  	send_bond_state_change(&addr->bdaddr, HAL_STATUS_SUCCESS,
>  							HAL_BOND_STATE_BONDED);
>  
> +	adapter->bonding = false;
>  	browse_remote_sdp(&addr->bdaddr);
>  }
>  
> @@ -501,6 +504,13 @@ static void pin_code_request_callback(uint16_t index, uint16_t length,
>  		return;
>  	}
>  
> +	if (!adapter->bonding) {
> +		adapter->bonding = true;
> +		/* Update bonding state */
> +		send_bond_state_change(&ev->addr.bdaddr, HAL_STATUS_SUCCESS,
> +						HAL_BOND_STATE_BONDING);
> +	}

Is there something explicitly blocking multiple parallel bonding
attempts (to different remote devices) in Android? If not this should
really be a list or hash-table with entries identifiable using the
remote bdaddr. In fact since we need this anyway for our LE address type
cache I don't see why it would hurt to have the infrastructure ready for
it now.

Johan

[PATCH v2 2/3] android: Update HAL with device info on incoming connection

[Lukasz Rymanowski]

Make sure Android have information about connecting remote device. This
is needed for example to show device name on incoming bonding request.
---
 android/adapter.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/android/adapter.c b/android/adapter.c
index f92301e..a59ab4e 100644
--- a/android/adapter.c
+++ b/android/adapter.c
@@ -857,9 +857,8 @@ static void mgmt_device_connected_event(uint16_t index, uint16_t length,
 		return;
 	}
 
-	/* TODO: Update device */
-
-	/* TODO: Check Set bonding state */
+	update_found_device(&ev->addr.bdaddr, ev->addr.type, 0, false,
+					&ev->eir[0], btohs(ev->eir_len));
 
 	hal_ev.status = HAL_STATUS_SUCCESS;
 	hal_ev.state = HAL_ACL_STATE_CONNECTED;
-- 
1.8.4

Re: [PATCH v2 2/3] android: Update HAL with device info on incoming connection

[Johan Hedberg]

Hi Lukasz,

On Tue, Nov 12, 2013, Lukasz Rymanowski wrote:
> Make sure Android have information about connecting remote device. This
> is needed for example to show device name on incoming bonding request.
> ---
>  android/adapter.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/android/adapter.c b/android/adapter.c
> index f92301e..a59ab4e 100644
> --- a/android/adapter.c
> +++ b/android/adapter.c
> @@ -857,9 +857,8 @@ static void mgmt_device_connected_event(uint16_t index, uint16_t length,
>  		return;
>  	}
>  
> -	/* TODO: Update device */
> -
> -	/* TODO: Check Set bonding state */
> +	update_found_device(&ev->addr.bdaddr, ev->addr.type, 0, false,
> +					&ev->eir[0], btohs(ev->eir_len));
>  
>  	hal_ev.status = HAL_STATUS_SUCCESS;
>  	hal_ev.state = HAL_ACL_STATE_CONNECTED;

This patch has been applied. Thanks.

Johan

[PATCH v2 3/3] android: Change TODO with explaining comment

[Lukasz Rymanowski]

---
 android/adapter.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/android/adapter.c b/android/adapter.c
index a59ab4e..ac1ebc2 100644
--- a/android/adapter.c
+++ b/android/adapter.c
@@ -515,7 +515,10 @@ static void pin_code_request_callback(uint16_t index, uint16_t length,
 
 	DBG("%s type %u secure %u", dst, ev->addr.type, ev->secure);
 
-	/* TODO name and CoD of remote devices should probably be cached */
+	/* It is ok to have empty name and CoD of remote devices here since
+	* those information has been already provided on device_connected event
+	* or during device scaning. Android will use that instead.
+	*/
 	memset(&hal_ev, 0, sizeof(hal_ev));
 	bdaddr2android(&ev->addr.bdaddr, hal_ev.bdaddr);
 
@@ -528,7 +531,10 @@ static void send_ssp_request(const bdaddr_t *addr, uint8_t variant,
 {
 	struct hal_ev_ssp_request ev;
 
-	/* TODO name and CoD of remote devices should probably be cached */
+	/* It is ok to have empty name and CoD of remote devices here since
+	* those information has been already provided on device_connected event
+	* or during device scaning. Android will use that instead.
+	*/
 	memset(&ev, 0, sizeof(ev));
 	bdaddr2android(addr, ev.bdaddr);
 	ev.pairing_variant = variant;
-- 
1.8.4

Re: [PATCH v2 3/3] android: Change TODO with explaining comment

[Johan Hedberg]

Hi Lukasz,

On Tue, Nov 12, 2013, Lukasz Rymanowski wrote:
> ---
>  android/adapter.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/android/adapter.c b/android/adapter.c
> index a59ab4e..ac1ebc2 100644
> --- a/android/adapter.c
> +++ b/android/adapter.c
> @@ -515,7 +515,10 @@ static void pin_code_request_callback(uint16_t index, uint16_t length,
>  
>  	DBG("%s type %u secure %u", dst, ev->addr.type, ev->secure);
>  
> -	/* TODO name and CoD of remote devices should probably be cached */
> +	/* It is ok to have empty name and CoD of remote devices here since
> +	* those information has been already provided on device_connected event
> +	* or during device scaning. Android will use that instead.
> +	*/
>  	memset(&hal_ev, 0, sizeof(hal_ev));
>  	bdaddr2android(&ev->addr.bdaddr, hal_ev.bdaddr);

Have you considered security mode 3 devices? There you will get a pin
code request before you get a device_connected. You can even try this
yourself by doing "hciconfig hci0 auth" before attempting to pair. That
said, I'm not sure if there is anything we can do better regarding such
devices, but at least we should keep this use case in mind.

Johan