[PATCH v2 1/8] android/bluetooth: Refactor handle_start_discovery_cmd

[PATCH v2 1/8] android/bluetooth: Refactor handle_start_discovery_cmd

[Szymon Janc]

This makes function flow easier to follow. Also fix usage of
adapter.exp_discovery_type which should be used only when stopping
currently running discovery session to restart it with new type.
---
 android/bluetooth.c | 43 +++++++++++++++++++++----------------------
 1 file changed, 21 insertions(+), 22 deletions(-)

diff --git a/android/bluetooth.c b/android/bluetooth.c
index 7d82aba..3931618 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -3704,39 +3704,38 @@ static void handle_start_discovery_cmd(const void *buf, uint16_t len)
 {
 	uint8_t status;
 
-	/* Check if there is discovery with BREDR type */
-	if (adapter.cur_discovery_type & SCAN_TYPE_BREDR) {
-		status = HAL_STATUS_SUCCESS;
-		goto reply;
-	}
-
 	if (!(adapter.current_settings & MGMT_SETTING_POWERED)) {
 		status = HAL_STATUS_NOT_READY;
-		goto reply;
+		goto failed;
 	}
 
-	adapter.exp_discovery_type |= SCAN_TYPE_DUAL;
-
-	/* If there is no discovery ongoing, try to start discovery */
-	if (!adapter.cur_discovery_type) {
-		if (!start_discovery(adapter.exp_discovery_type))
+	switch (adapter.cur_discovery_type) {
+	case SCAN_TYPE_DUAL:
+	case SCAN_TYPE_BREDR:
+		break;
+	case SCAN_TYPE_NONE:
+		if (!start_discovery(SCAN_TYPE_DUAL)) {
 			status = HAL_STATUS_FAILED;
-		else
-			status = HAL_STATUS_SUCCESS;
+			goto failed;
+		}
 
-		goto reply;
-	}
+		break;
+	case SCAN_TYPE_LE:
+		if (get_adapter_discovering_type() == SCAN_TYPE_LE)
+			break;
 
-	/* Stop discovery here. Once it is stop we will restart it
-	 * with exp_discovery_settings */
-	if (!stop_discovery(adapter.cur_discovery_type)) {
-		status = HAL_STATUS_FAILED;
-		goto reply;
+		if (!stop_discovery(SCAN_TYPE_LE)) {
+			status = HAL_STATUS_FAILED;
+			goto failed;
+		}
+
+		adapter.exp_discovery_type = SCAN_TYPE_DUAL;
+		break;
 	}
 
 	status = HAL_STATUS_SUCCESS;
 
-reply:
+failed:
 	ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_BLUETOOTH, HAL_OP_START_DISCOVERY,
 									status);
 }
-- 
1.9.1

[PATCH v2 2/8] android/bluetooth: Refactor handle_cancel_discovery_cmd

[Szymon Janc]

This makes function flow easier to follow. Also fix usage of
adapter.exp_discovery_type which should be used only when stopping
currently running discovery session to restart it with new type.
---
 android/bluetooth.c | 41 +++++++++++++++++++++++++++++------------
 1 file changed, 29 insertions(+), 12 deletions(-)

diff --git a/android/bluetooth.c b/android/bluetooth.c
index 3931618..51243a0 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -3744,27 +3744,44 @@ static void handle_cancel_discovery_cmd(const void *buf, uint16_t len)
 {
 	uint8_t status;
 
-	if (!adapter.cur_discovery_type) {
-		status = HAL_STATUS_SUCCESS;
-		goto reply;
-	}
-
 	if (!(adapter.current_settings & MGMT_SETTING_POWERED)) {
 		status = HAL_STATUS_NOT_READY;
-		goto reply;
+		goto failed;
 	}
 
-	/* Take into account that gatt might want to keep discover */
-	adapter.exp_discovery_type = gatt_device_found_cb ? SCAN_TYPE_LE : 0;
+	switch (adapter.cur_discovery_type) {
+	case SCAN_TYPE_NONE:
+		break;
+	case SCAN_TYPE_LE:
+		if (get_adapter_discovering_type() != SCAN_TYPE_LE)
+			break;
 
-	if (!stop_discovery(adapter.cur_discovery_type)) {
-		status = HAL_STATUS_FAILED;
-		goto reply;
+		if (gatt_device_found_cb) {
+			status = HAL_STATUS_BUSY;
+			goto failed;
+		}
+
+		if (!stop_discovery(SCAN_TYPE_LE)) {
+			status = HAL_STATUS_FAILED;
+			goto failed;
+		}
+
+		break;
+	case SCAN_TYPE_DUAL:
+	case SCAN_TYPE_BREDR:
+		if (!stop_discovery(SCAN_TYPE_DUAL)) {
+			status = HAL_STATUS_FAILED;
+			goto failed;
+		}
+
+		adapter.exp_discovery_type = gatt_device_found_cb ?
+						SCAN_TYPE_LE : SCAN_TYPE_NONE;
+		break;
 	}
 
 	status = HAL_STATUS_SUCCESS;
 
-reply:
+failed:
 	ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_BLUETOOTH, HAL_OP_CANCEL_DISCOVERY,
 									status);
 }
-- 
1.9.1

[PATCH v2 3/8] android/bluetooth: Fix bt_le_discovery_start

[Szymon Janc]

This fix wrong use of adapter.exp_discovery_type and setting callback
even if failed to start discovery.
---
 android/bluetooth.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/android/bluetooth.c b/android/bluetooth.c
index 51243a0..20f8993 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -2944,15 +2944,18 @@ bool bt_le_discovery_start(bt_le_device_found cb)
 	if (!(adapter.current_settings & MGMT_SETTING_POWERED))
 		return false;
 
-	gatt_device_found_cb = cb;
-
-	adapter.exp_discovery_type |= SCAN_TYPE_LE;
-
 	/* If core is discovering, don't bother */
-	if (adapter.cur_discovery_type)
+	if (adapter.cur_discovery_type != SCAN_TYPE_NONE) {
+		gatt_device_found_cb = cb;
 		return true;
+	}
 
-	return start_discovery(adapter.exp_discovery_type);
+	if (start_discovery(SCAN_TYPE_LE)) {
+		gatt_device_found_cb = cb;
+		return true;
+	}
+
+	return false;
 }
 
 static uint8_t set_adapter_scan_mode(const void *buf, uint16_t len)
-- 
1.9.1

[PATCH v2 4/8] android/bluetooth: Fix bt_le_discovery_stop

[Szymon Janc]

This fix invalid use of adapter.exp_discovery_type and not clearing
callback.
---
 android/bluetooth.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/android/bluetooth.c b/android/bluetooth.c
index 20f8993..28ad91a 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -2925,18 +2925,23 @@ bool bt_le_set_advertising(bool advertising, bt_le_set_advertising_done cb,
 
 bool bt_le_discovery_stop(bt_le_discovery_stopped cb)
 {
-	if (!adapter.cur_discovery_type) {
+	if (adapter.cur_discovery_type != SCAN_TYPE_LE) {
 		if (cb)
 			cb();
+
+		gatt_device_found_cb = NULL;
+
 		return true;
 	}
 
-	gatt_discovery_stopped_cb = cb;
-	/* Remove device found callback */
+	if (!stop_discovery(SCAN_TYPE_LE))
+		return false;
+
 	gatt_device_found_cb = NULL;
-	adapter.exp_discovery_type &= ~SCAN_TYPE_LE;
+	gatt_discovery_stopped_cb = cb;
+	adapter.exp_discovery_type = SCAN_TYPE_NONE;
 
-	return stop_discovery(adapter.cur_discovery_type);
+	return true;
 }
 
 bool bt_le_discovery_start(bt_le_device_found cb)
-- 
1.9.1

[PATCH v2 5/8] android/bluetooth: Refactor mgmt_discovering_event

[Szymon Janc]

This make funtion simpler and fix bogus discovery restart (which
should only happen for LE scanning triggered by GATT HAL).
---
 android/bluetooth.c | 55 +++++++++++++++++++++++------------------------------
 1 file changed, 24 insertions(+), 31 deletions(-)

diff --git a/android/bluetooth.c b/android/bluetooth.c
index 28ad91a..af5c8ad 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -1061,56 +1061,49 @@ static void mgmt_discovering_event(uint16_t index, uint16_t length,
 {
 	const struct mgmt_ev_discovering *ev = param;
 	struct hal_ev_discovery_state_changed cp;
-	bool is_discovering = adapter.cur_discovery_type;
+	uint8_t type;
 
 	if (length < sizeof(*ev)) {
 		error("Too small discovering event");
 		return;
 	}
 
-	DBG("hci%u type %u discovering %u", index, ev->type,
-							ev->discovering);
+	DBG("type %u discovering %u", ev->type, ev->discovering);
 
-	if (is_discovering == !!ev->discovering)
+	if (!!adapter.cur_discovery_type == !!ev->discovering)
 		return;
 
-	adapter.cur_discovery_type = ev->discovering ?
-						ev->type : SCAN_TYPE_NONE;
-
-	DBG("new discovering state %u", ev->discovering);
-
-	if (adapter.cur_discovery_type != SCAN_TYPE_NONE) {
+	if (ev->discovering) {
+		adapter.cur_discovery_type = ev->type;
 		cp.state = HAL_DISCOVERY_STATE_STARTED;
-	} else {
-		g_slist_foreach(bonded_devices, clear_device_found, NULL);
-		g_slist_foreach(cached_devices, clear_device_found, NULL);
-		cp.state = HAL_DISCOVERY_STATE_STOPPED;
+
+		goto done;
 	}
 
-	ipc_send_notif(hal_ipc, HAL_SERVICE_ID_BLUETOOTH,
-			HAL_EV_DISCOVERY_STATE_CHANGED, sizeof(cp), &cp);
+	adapter.cur_discovery_type = SCAN_TYPE_NONE;
+	cp.state = HAL_DISCOVERY_STATE_STOPPED;
+
+	g_slist_foreach(bonded_devices, clear_device_found, NULL);
+	g_slist_foreach(cached_devices, clear_device_found, NULL);
 
-	if (gatt_discovery_stopped_cb &&
-			(adapter.cur_discovery_type == SCAN_TYPE_NONE)) {
-		/* One shot notification about discovery stopped send to gatt*/
+	/* One shot notification about discovery stopped */
+	if (gatt_discovery_stopped_cb) {
 		gatt_discovery_stopped_cb();
 		gatt_discovery_stopped_cb = NULL;
 	}
 
-	/* If discovery is ON or there is no expected next discovery session
-	 * then just return
-	 */
-	if ((adapter.cur_discovery_type != SCAN_TYPE_NONE) ||
-		(adapter.exp_discovery_type == SCAN_TYPE_NONE))
-		return;
+	type = adapter.exp_discovery_type;
+	adapter.exp_discovery_type = SCAN_TYPE_NONE;
 
-	start_discovery(adapter.exp_discovery_type);
+	if (type == SCAN_TYPE_NONE && gatt_device_found_cb)
+		type = SCAN_TYPE_LE;
 
-	/* Maintain expected discovery type if there is gatt client
-	 * registered
-	 */
-	adapter.exp_discovery_type = gatt_device_found_cb ?
-						SCAN_TYPE_LE : SCAN_TYPE_NONE;
+	if (type != SCAN_TYPE_NONE)
+		start_discovery(type);
+
+done:
+	ipc_send_notif(hal_ipc, HAL_SERVICE_ID_BLUETOOTH,
+			HAL_EV_DISCOVERY_STATE_CHANGED, sizeof(cp), &cp);
 }
 
 static void confirm_device_name_cb(uint8_t status, uint16_t length,
-- 
1.9.1

[PATCH v2 6/8] android/bluetooth: Avoid starting/stopping discovery with no type

[Szymon Janc]

---
 android/bluetooth.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/android/bluetooth.c b/android/bluetooth.c
index af5c8ad..e60b6ff 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -1026,12 +1026,10 @@ static void clear_device_found(gpointer data, gpointer user_data)
 
 static uint8_t get_adapter_discovering_type(void)
 {
-	uint8_t type;
+	uint8_t type = SCAN_TYPE_NONE;
 
 	if (adapter.current_settings & MGMT_SETTING_BREDR)
-		type = SCAN_TYPE_BREDR;
-	else
-		type = 0;
+		type |= SCAN_TYPE_BREDR;
 
 	if (adapter.current_settings & MGMT_SETTING_LE)
 		type |= SCAN_TYPE_LE;
@@ -1047,6 +1045,9 @@ static bool start_discovery(uint8_t type)
 
 	DBG("type=0x%x", cp.type);
 
+	if (cp.type == SCAN_TYPE_NONE)
+		return false;
+
 	if (mgmt_send(mgmt_if, MGMT_OP_START_DISCOVERY, adapter.index,
 				sizeof(cp), &cp, NULL, NULL, NULL) > 0)
 		return true;
@@ -2864,6 +2865,9 @@ static bool stop_discovery(uint8_t type)
 
 	DBG("type=0x%x", cp.type);
 
+	if (cp.type == SCAN_TYPE_NONE)
+		return false;
+
 	/* Lets drop all confirm name request as we don't need it anymore */
 	g_slist_foreach(cached_devices, cancel_pending_confirm_name, NULL);
 
-- 
1.9.1

[PATCH v2 7/8] android/bluetooth: Fix sending discovery state changed events

[Szymon Janc]

Those events should be send only if discovery was started/stoped from
bluetooth HAL.
---
 android/bluetooth.c | 45 +++++++++++++++++++++++++++++++--------------
 1 file changed, 31 insertions(+), 14 deletions(-)

diff --git a/android/bluetooth.c b/android/bluetooth.c
index e60b6ff..d6bc013 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -1057,11 +1057,36 @@ static bool start_discovery(uint8_t type)
 	return false;
 }
 
+/* send discovery state change event only if it is related to dual type
+ * discovery session (triggered by start/cancel discovery commands)
+ */
+static void check_discovery_state(uint8_t new_type, uint8_t old_type)
+{
+	struct hal_ev_discovery_state_changed ev;
+
+	DBG("%u %u", new_type, old_type);
+
+	if (new_type == get_adapter_discovering_type()) {
+		g_slist_foreach(bonded_devices, clear_device_found, NULL);
+		g_slist_foreach(cached_devices, clear_device_found, NULL);
+		ev.state = HAL_DISCOVERY_STATE_STARTED;
+		goto done;
+	}
+
+	if (old_type != get_adapter_discovering_type())
+		return;
+
+	ev.state = HAL_DISCOVERY_STATE_STOPPED;
+
+done:
+	ipc_send_notif(hal_ipc, HAL_SERVICE_ID_BLUETOOTH,
+			HAL_EV_DISCOVERY_STATE_CHANGED, sizeof(ev), &ev);
+}
+
 static void mgmt_discovering_event(uint16_t index, uint16_t length,
 					const void *param, void *user_data)
 {
 	const struct mgmt_ev_discovering *ev = param;
-	struct hal_ev_discovery_state_changed cp;
 	uint8_t type;
 
 	if (length < sizeof(*ev)) {
@@ -1074,18 +1099,14 @@ static void mgmt_discovering_event(uint16_t index, uint16_t length,
 	if (!!adapter.cur_discovery_type == !!ev->discovering)
 		return;
 
-	if (ev->discovering) {
-		adapter.cur_discovery_type = ev->type;
-		cp.state = HAL_DISCOVERY_STATE_STARTED;
+	type = ev->discovering ? ev->type : SCAN_TYPE_NONE;
 
-		goto done;
-	}
+	check_discovery_state(type, adapter.cur_discovery_type);
 
-	adapter.cur_discovery_type = SCAN_TYPE_NONE;
-	cp.state = HAL_DISCOVERY_STATE_STOPPED;
+	adapter.cur_discovery_type = type;
 
-	g_slist_foreach(bonded_devices, clear_device_found, NULL);
-	g_slist_foreach(cached_devices, clear_device_found, NULL);
+	if (ev->discovering)
+		return;
 
 	/* One shot notification about discovery stopped */
 	if (gatt_discovery_stopped_cb) {
@@ -1101,10 +1122,6 @@ static void mgmt_discovering_event(uint16_t index, uint16_t length,
 
 	if (type != SCAN_TYPE_NONE)
 		start_discovery(type);
-
-done:
-	ipc_send_notif(hal_ipc, HAL_SERVICE_ID_BLUETOOTH,
-			HAL_EV_DISCOVERY_STATE_CHANGED, sizeof(cp), &cp);
 }
 
 static void confirm_device_name_cb(uint8_t status, uint16_t length,
-- 
1.9.1

[PATCH v2 8/8] android/bluetooth: Rename get_adapter_discovering_type function

[Szymon Janc]

Rename it to get_supported_discovery_type which better describes
function purpose.
---
 android/bluetooth.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/android/bluetooth.c b/android/bluetooth.c
index d6bc013..9516911 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -1024,7 +1024,7 @@ static void clear_device_found(gpointer data, gpointer user_data)
 	dev->found = false;
 }
 
-static uint8_t get_adapter_discovering_type(void)
+static uint8_t get_supported_discovery_type(void)
 {
 	uint8_t type = SCAN_TYPE_NONE;
 
@@ -1041,7 +1041,7 @@ static bool start_discovery(uint8_t type)
 {
 	struct mgmt_cp_start_discovery cp;
 
-	cp.type = get_adapter_discovering_type() & type;
+	cp.type = get_supported_discovery_type() & type;
 
 	DBG("type=0x%x", cp.type);
 
@@ -1066,14 +1066,14 @@ static void check_discovery_state(uint8_t new_type, uint8_t old_type)
 
 	DBG("%u %u", new_type, old_type);
 
-	if (new_type == get_adapter_discovering_type()) {
+	if (new_type == get_supported_discovery_type()) {
 		g_slist_foreach(bonded_devices, clear_device_found, NULL);
 		g_slist_foreach(cached_devices, clear_device_found, NULL);
 		ev.state = HAL_DISCOVERY_STATE_STARTED;
 		goto done;
 	}
 
-	if (old_type != get_adapter_discovering_type())
+	if (old_type != get_supported_discovery_type())
 		return;
 
 	ev.state = HAL_DISCOVERY_STATE_STOPPED;
@@ -2878,7 +2878,7 @@ static bool stop_discovery(uint8_t type)
 {
 	struct mgmt_cp_stop_discovery cp;
 
-	cp.type = get_adapter_discovering_type() & type;
+	cp.type = get_supported_discovery_type() & type;
 
 	DBG("type=0x%x", cp.type);
 
@@ -3743,7 +3743,7 @@ static void handle_start_discovery_cmd(const void *buf, uint16_t len)
 
 		break;
 	case SCAN_TYPE_LE:
-		if (get_adapter_discovering_type() == SCAN_TYPE_LE)
+		if (get_supported_discovery_type() == SCAN_TYPE_LE)
 			break;
 
 		if (!stop_discovery(SCAN_TYPE_LE)) {
@@ -3775,7 +3775,7 @@ static void handle_cancel_discovery_cmd(const void *buf, uint16_t len)
 	case SCAN_TYPE_NONE:
 		break;
 	case SCAN_TYPE_LE:
-		if (get_adapter_discovering_type() != SCAN_TYPE_LE)
+		if (get_supported_discovery_type() != SCAN_TYPE_LE)
 			break;
 
 		if (gatt_device_found_cb) {
-- 
1.9.1

Re: [PATCH v2 1/8] android/bluetooth: Refactor handle_start_discovery_cmd

[Szymon Janc]

On Sunday 27 of April 2014 16:54:51 Szymon Janc wrote:
> This makes function flow easier to follow. Also fix usage of
> adapter.exp_discovery_type which should be used only when stopping
> currently running discovery session to restart it with new type.
> ---
>  android/bluetooth.c | 43 +++++++++++++++++++++----------------------
>  1 file changed, 21 insertions(+), 22 deletions(-)
> 
> diff --git a/android/bluetooth.c b/android/bluetooth.c
> index 7d82aba..3931618 100644
> --- a/android/bluetooth.c
> +++ b/android/bluetooth.c
> @@ -3704,39 +3704,38 @@ static void handle_start_discovery_cmd(const void *buf, uint16_t len)
>  {
>  	uint8_t status;
>  
> -	/* Check if there is discovery with BREDR type */
> -	if (adapter.cur_discovery_type & SCAN_TYPE_BREDR) {
> -		status = HAL_STATUS_SUCCESS;
> -		goto reply;
> -	}
> -
>  	if (!(adapter.current_settings & MGMT_SETTING_POWERED)) {
>  		status = HAL_STATUS_NOT_READY;
> -		goto reply;
> +		goto failed;
>  	}
>  
> -	adapter.exp_discovery_type |= SCAN_TYPE_DUAL;
> -
> -	/* If there is no discovery ongoing, try to start discovery */
> -	if (!adapter.cur_discovery_type) {
> -		if (!start_discovery(adapter.exp_discovery_type))
> +	switch (adapter.cur_discovery_type) {
> +	case SCAN_TYPE_DUAL:
> +	case SCAN_TYPE_BREDR:
> +		break;
> +	case SCAN_TYPE_NONE:
> +		if (!start_discovery(SCAN_TYPE_DUAL)) {
>  			status = HAL_STATUS_FAILED;
> -		else
> -			status = HAL_STATUS_SUCCESS;
> +			goto failed;
> +		}
>  
> -		goto reply;
> -	}
> +		break;
> +	case SCAN_TYPE_LE:
> +		if (get_adapter_discovering_type() == SCAN_TYPE_LE)
> +			break;
>  
> -	/* Stop discovery here. Once it is stop we will restart it
> -	 * with exp_discovery_settings */
> -	if (!stop_discovery(adapter.cur_discovery_type)) {
> -		status = HAL_STATUS_FAILED;
> -		goto reply;
> +		if (!stop_discovery(SCAN_TYPE_LE)) {
> +			status = HAL_STATUS_FAILED;
> +			goto failed;
> +		}
> +
> +		adapter.exp_discovery_type = SCAN_TYPE_DUAL;
> +		break;
>  	}
>  
>  	status = HAL_STATUS_SUCCESS;
>  
> -reply:
> +failed:
>  	ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_BLUETOOTH, HAL_OP_START_DISCOVERY,
>  									status);
>  }
> 

Rebased and pushed.

-- 
Best regards, 
Szymon Janc