linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] android/gatt: Stop listening while unregistering app
@ 2014-08-13 14:50 Grzegorz Kolodziejczyk
  2014-08-13 14:50 ` [PATCH 2/2] android/gatt: Add helper to create listen data Grzegorz Kolodziejczyk
  2014-08-26 10:56 ` [PATCH 1/2] android/gatt: Stop listening while unregistering app Tyszkowski Jakub
  0 siblings, 2 replies; 4+ messages in thread
From: Grzegorz Kolodziejczyk @ 2014-08-13 14:50 UTC (permalink / raw)
  To: linux-bluetooth

Client gets stucked on advertising while unregistering app with not
stopped listening.
---
 android/gatt.c | 102 ++++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 64 insertions(+), 38 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index 89748d2..7b3db06 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -1834,15 +1834,79 @@ static uint8_t unregister_app(int client_if)
 	return HAL_STATUS_SUCCESS;
 }
 
+static void send_client_listen_notify(int32_t id, int32_t status)
+{
+	struct hal_ev_gatt_client_listen ev;
+
+	/* Server if because of typo in android headers */
+	ev.server_if = id;
+	ev.status = status;
+
+	ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT, HAL_EV_GATT_CLIENT_LISTEN,
+							sizeof(ev), &ev);
+}
+
+struct listen_data {
+	int32_t client_id;
+	bool start;
+};
+
+static void set_advertising_cb(uint8_t status, void *user_data)
+{
+	struct listen_data *l = user_data;
+
+	send_client_listen_notify(l->client_id, status);
+
+	/* In case of success update advertising state*/
+	if (!status)
+		advertising_cnt = l->start ? 1 : 0;
+
+	/*
+	 * Let's remove client from the list in two cases
+	 * 1. Start failed
+	 * 2. Stop succeed
+	 */
+	if ((l->start && status) || (!l->start && !status))
+		queue_remove(listen_apps, INT_TO_PTR(l->client_id));
+
+	free(l);
+}
+
 static void handle_client_unregister(const void *buf, uint16_t len)
 {
 	const struct hal_cmd_gatt_client_unregister *cmd = buf;
 	uint8_t status;
+	void *listening_client;
+	struct listen_data *data;
 
 	DBG("");
 
+	listening_client = queue_find(listen_apps, match_by_value,
+						INT_TO_PTR(cmd->client_if));
+
+	if (listening_client) {
+		data = new0(struct listen_data, 1);
+		if (!data) {
+			error("gatt: Could not allocate memory for listen data");
+			status = HAL_STATUS_NOMEM;
+			goto reply;
+		}
+
+		data->client_id = cmd->client_if;
+		data->start = false;
+
+		if (!bt_le_set_advertising(data->start, set_advertising_cb,
+								data)) {
+			error("gatt: Could not set advertising");
+			status = HAL_STATUS_FAILED;
+			free(data);
+			goto reply;
+		}
+	}
+
 	status = unregister_app(cmd->client_if);
 
+reply:
 	ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_GATT,
 					HAL_OP_GATT_CLIENT_UNREGISTER, status);
 }
@@ -1948,44 +2012,6 @@ static void handle_client_disconnect(const void *buf, uint16_t len)
 					HAL_OP_GATT_CLIENT_DISCONNECT, status);
 }
 
-static void send_client_listen_notify(int32_t id, int32_t status)
-{
-	struct hal_ev_gatt_client_listen ev;
-
-	/* Server if because of typo in android headers */
-	ev.server_if = id;
-	ev.status = status;
-
-	ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT, HAL_EV_GATT_CLIENT_LISTEN,
-							sizeof(ev), &ev);
-}
-
-struct listen_data {
-	int32_t client_id;
-	bool start;
-};
-
-static void set_advertising_cb(uint8_t status, void *user_data)
-{
-	struct listen_data *l = user_data;
-
-	send_client_listen_notify(l->client_id, status);
-
-	/* In case of success update advertising state*/
-	if (!status)
-		advertising_cnt = l->start ? 1 : 0;
-
-	/*
-	 * Let's remove client from the list in two cases
-	 * 1. Start failed
-	 * 2. Stop succeed
-	 */
-	if ((l->start && status) || (!l->start && !status))
-		queue_remove(listen_apps, INT_TO_PTR(l->client_id));
-
-	free(l);
-}
-
 static void handle_client_listen(const void *buf, uint16_t len)
 {
 	const struct hal_cmd_gatt_client_listen *cmd = buf;
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/2] android/gatt: Add helper to create listen data
  2014-08-13 14:50 [PATCH 1/2] android/gatt: Stop listening while unregistering app Grzegorz Kolodziejczyk
@ 2014-08-13 14:50 ` Grzegorz Kolodziejczyk
  2014-08-26 10:56 ` [PATCH 1/2] android/gatt: Stop listening while unregistering app Tyszkowski Jakub
  1 sibling, 0 replies; 4+ messages in thread
From: Grzegorz Kolodziejczyk @ 2014-08-13 14:50 UTC (permalink / raw)
  To: linux-bluetooth

---
 android/gatt.c | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index 7b3db06..5d78101 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -1851,6 +1851,20 @@ struct listen_data {
 	bool start;
 };
 
+static struct listen_data *create_listen_data(int32_t client_id, bool start)
+{
+	struct listen_data *d;
+
+	d = new0(struct listen_data, 1);
+	if (!d)
+		return NULL;
+
+	d->client_id = client_id;
+	d->start = start;
+
+	return d;
+}
+
 static void set_advertising_cb(uint8_t status, void *user_data)
 {
 	struct listen_data *l = user_data;
@@ -1885,16 +1899,13 @@ static void handle_client_unregister(const void *buf, uint16_t len)
 						INT_TO_PTR(cmd->client_if));
 
 	if (listening_client) {
-		data = new0(struct listen_data, 1);
+		data = create_listen_data(cmd->client_if, false);
 		if (!data) {
-			error("gatt: Could not allocate memory for listen data");
+			error("gatt: Could not allocate listen data");
 			status = HAL_STATUS_NOMEM;
 			goto reply;
 		}
 
-		data->client_id = cmd->client_if;
-		data->start = false;
-
 		if (!bt_le_set_advertising(data->start, set_advertising_cb,
 								data)) {
 			error("gatt: Could not set advertising");
@@ -2072,16 +2083,14 @@ static void handle_client_listen(const void *buf, uint16_t len)
 		}
 	}
 
-	data = new0(struct listen_data, 1);
+	data = create_listen_data(cmd->client_if, cmd->start);
 	if (!data) {
-		error("gatt: Could not allocate memory for listen data");
+		error("gatt: Could not allocate listen data");
+
 		status = HAL_STATUS_NOMEM;
 		goto reply;
 	}
 
-	data->client_id = cmd->client_if;
-	data->start = cmd->start;
-
 	if (!bt_le_set_advertising(cmd->start, set_advertising_cb, data)) {
 		error("gatt: Could not set advertising");
 		status = HAL_STATUS_FAILED;
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/2] android/gatt: Stop listening while unregistering app
  2014-08-13 14:50 [PATCH 1/2] android/gatt: Stop listening while unregistering app Grzegorz Kolodziejczyk
  2014-08-13 14:50 ` [PATCH 2/2] android/gatt: Add helper to create listen data Grzegorz Kolodziejczyk
@ 2014-08-26 10:56 ` Tyszkowski Jakub
  2014-09-02 14:34   ` Grzegorz Kolodziejczyk
  1 sibling, 1 reply; 4+ messages in thread
From: Tyszkowski Jakub @ 2014-08-26 10:56 UTC (permalink / raw)
  To: Grzegorz Kolodziejczyk, linux-bluetooth

Hi Grzegorz,

On 08/13/2014 04:50 PM, Grzegorz Kolodziejczyk wrote:
> Client gets stucked on advertising while unregistering app with not
> stopped listening.
> ---
>   android/gatt.c | 102 ++++++++++++++++++++++++++++++++++++---------------------
>   1 file changed, 64 insertions(+), 38 deletions(-)
>
> diff --git a/android/gatt.c b/android/gatt.c
> index 89748d2..7b3db06 100644
> --- a/android/gatt.c
> +++ b/android/gatt.c
> @@ -1834,15 +1834,79 @@ static uint8_t unregister_app(int client_if)
>   	return HAL_STATUS_SUCCESS;
>   }
>
> +static void send_client_listen_notify(int32_t id, int32_t status)
> +{
> +	struct hal_ev_gatt_client_listen ev;
> +
> +	/* Server if because of typo in android headers */
> +	ev.server_if = id;
> +	ev.status = status;
> +
> +	ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT, HAL_EV_GATT_CLIENT_LISTEN,
> +							sizeof(ev), &ev);
> +}
> +
> +struct listen_data {
> +	int32_t client_id;
> +	bool start;
> +};
> +
> +static void set_advertising_cb(uint8_t status, void *user_data)
> +{
> +	struct listen_data *l = user_data;
> +
> +	send_client_listen_notify(l->client_id, status);
> +
> +	/* In case of success update advertising state*/
> +	if (!status)
> +		advertising_cnt = l->start ? 1 : 0;
> +
> +	/*
> +	 * Let's remove client from the list in two cases
> +	 * 1. Start failed
> +	 * 2. Stop succeed
> +	 */
> +	if ((l->start && status) || (!l->start && !status))
> +		queue_remove(listen_apps, INT_TO_PTR(l->client_id));
> +
> +	free(l);
> +}
> +
>   static void handle_client_unregister(const void *buf, uint16_t len)
>   {
>   	const struct hal_cmd_gatt_client_unregister *cmd = buf;
>   	uint8_t status;
> +	void *listening_client;
> +	struct listen_data *data;
>
>   	DBG("");
>
> +	listening_client = queue_find(listen_apps, match_by_value,
> +						INT_TO_PTR(cmd->client_if));
> +
> +	if (listening_client) {
> +		data = new0(struct listen_data, 1);
> +		if (!data) {
> +			error("gatt: Could not allocate memory for listen data");
> +			status = HAL_STATUS_NOMEM;
> +			goto reply;
> +		}
> +
> +		data->client_id = cmd->client_if;
> +		data->start = false;
> +
> +		if (!bt_le_set_advertising(data->start, set_advertising_cb,
> +								data)) {
> +			error("gatt: Could not set advertising");
> +			status = HAL_STATUS_FAILED;
> +			free(data);
> +			goto reply;
> +		}
> +	}

I think this should work like 'handle_client_listen': check the 
'advertising_cnt' and stop only if the client that unregisters itself is 
the last one that was advertising.

It would be good to actually extract this counter check and advertising 
set from 'handle_client_listen' so it can be reused in 
'handle_client_unregister'.

> +
>   	status = unregister_app(cmd->client_if);
>
> +reply:
>   	ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_GATT,
>   					HAL_OP_GATT_CLIENT_UNREGISTER, status);
>   }
> @@ -1948,44 +2012,6 @@ static void handle_client_disconnect(const void *buf, uint16_t len)
>   					HAL_OP_GATT_CLIENT_DISCONNECT, status);
>   }
>
> -static void send_client_listen_notify(int32_t id, int32_t status)
> -{
> -	struct hal_ev_gatt_client_listen ev;
> -
> -	/* Server if because of typo in android headers */
> -	ev.server_if = id;
> -	ev.status = status;
> -
> -	ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT, HAL_EV_GATT_CLIENT_LISTEN,
> -							sizeof(ev), &ev);
> -}
> -
> -struct listen_data {
> -	int32_t client_id;
> -	bool start;
> -};
> -
> -static void set_advertising_cb(uint8_t status, void *user_data)
> -{
> -	struct listen_data *l = user_data;
> -
> -	send_client_listen_notify(l->client_id, status);
> -
> -	/* In case of success update advertising state*/
> -	if (!status)
> -		advertising_cnt = l->start ? 1 : 0;
> -
> -	/*
> -	 * Let's remove client from the list in two cases
> -	 * 1. Start failed
> -	 * 2. Stop succeed
> -	 */
> -	if ((l->start && status) || (!l->start && !status))
> -		queue_remove(listen_apps, INT_TO_PTR(l->client_id));
> -
> -	free(l);
> -}
> -
>   static void handle_client_listen(const void *buf, uint16_t len)
>   {
>   	const struct hal_cmd_gatt_client_listen *cmd = buf;
>

Regards,
Jakub


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/2] android/gatt: Stop listening while unregistering app
  2014-08-26 10:56 ` [PATCH 1/2] android/gatt: Stop listening while unregistering app Tyszkowski Jakub
@ 2014-09-02 14:34   ` Grzegorz Kolodziejczyk
  0 siblings, 0 replies; 4+ messages in thread
From: Grzegorz Kolodziejczyk @ 2014-09-02 14:34 UTC (permalink / raw)
  To: Tyszkowski Jakub; +Cc: linux-bluetooth

Hi Jakub,

On 26 August 2014 12:56, Tyszkowski Jakub <jakub.tyszkowski@tieto.com> wrote:
> Hi Grzegorz,
>
>
> On 08/13/2014 04:50 PM, Grzegorz Kolodziejczyk wrote:
>>
>> Client gets stucked on advertising while unregistering app with not
>> stopped listening.
>> ---
>>   android/gatt.c | 102
>> ++++++++++++++++++++++++++++++++++++---------------------
>>   1 file changed, 64 insertions(+), 38 deletions(-)
>>
>> diff --git a/android/gatt.c b/android/gatt.c
>> index 89748d2..7b3db06 100644
>> --- a/android/gatt.c
>> +++ b/android/gatt.c
>> @@ -1834,15 +1834,79 @@ static uint8_t unregister_app(int client_if)
>>         return HAL_STATUS_SUCCESS;
>>   }
>>
>> +static void send_client_listen_notify(int32_t id, int32_t status)
>> +{
>> +       struct hal_ev_gatt_client_listen ev;
>> +
>> +       /* Server if because of typo in android headers */
>> +       ev.server_if = id;
>> +       ev.status = status;
>> +
>> +       ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT,
>> HAL_EV_GATT_CLIENT_LISTEN,
>> +                                                       sizeof(ev), &ev);
>> +}
>> +
>> +struct listen_data {
>> +       int32_t client_id;
>> +       bool start;
>> +};
>> +
>> +static void set_advertising_cb(uint8_t status, void *user_data)
>> +{
>> +       struct listen_data *l = user_data;
>> +
>> +       send_client_listen_notify(l->client_id, status);
>> +
>> +       /* In case of success update advertising state*/
>> +       if (!status)
>> +               advertising_cnt = l->start ? 1 : 0;
>> +
>> +       /*
>> +        * Let's remove client from the list in two cases
>> +        * 1. Start failed
>> +        * 2. Stop succeed
>> +        */
>> +       if ((l->start && status) || (!l->start && !status))
>> +               queue_remove(listen_apps, INT_TO_PTR(l->client_id));
>> +
>> +       free(l);
>> +}
>> +
>>   static void handle_client_unregister(const void *buf, uint16_t len)
>>   {
>>         const struct hal_cmd_gatt_client_unregister *cmd = buf;
>>         uint8_t status;
>> +       void *listening_client;
>> +       struct listen_data *data;
>>
>>         DBG("");
>>
>> +       listening_client = queue_find(listen_apps, match_by_value,
>> +
>> INT_TO_PTR(cmd->client_if));
>> +
>> +       if (listening_client) {
>> +               data = new0(struct listen_data, 1);
>> +               if (!data) {
>> +                       error("gatt: Could not allocate memory for listen
>> data");
>> +                       status = HAL_STATUS_NOMEM;
>> +                       goto reply;
>> +               }
>> +
>> +               data->client_id = cmd->client_if;
>> +               data->start = false;
>> +
>> +               if (!bt_le_set_advertising(data->start,
>> set_advertising_cb,
>> +                                                               data)) {
>> +                       error("gatt: Could not set advertising");
>> +                       status = HAL_STATUS_FAILED;
>> +                       free(data);
>> +                       goto reply;
>> +               }
>> +       }
>
>
> I think this should work like 'handle_client_listen': check the
> 'advertising_cnt' and stop only if the client that unregisters itself is the
> last one that was advertising.
>
> It would be good to actually extract this counter check and advertising set
> from 'handle_client_listen' so it can be reused in
> 'handle_client_unregister'.
>
Yes, I'll change it to trace advertising counter.
>
>> +
>>         status = unregister_app(cmd->client_if);
>>
>> +reply:
>>         ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_GATT,
>>                                         HAL_OP_GATT_CLIENT_UNREGISTER,
>> status);
>>   }
>> @@ -1948,44 +2012,6 @@ static void handle_client_disconnect(const void
>> *buf, uint16_t len)
>>                                         HAL_OP_GATT_CLIENT_DISCONNECT,
>> status);
>>   }
>>
>> -static void send_client_listen_notify(int32_t id, int32_t status)
>> -{
>> -       struct hal_ev_gatt_client_listen ev;
>> -
>> -       /* Server if because of typo in android headers */
>> -       ev.server_if = id;
>> -       ev.status = status;
>> -
>> -       ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT,
>> HAL_EV_GATT_CLIENT_LISTEN,
>> -                                                       sizeof(ev), &ev);
>> -}
>> -
>> -struct listen_data {
>> -       int32_t client_id;
>> -       bool start;
>> -};
>> -
>> -static void set_advertising_cb(uint8_t status, void *user_data)
>> -{
>> -       struct listen_data *l = user_data;
>> -
>> -       send_client_listen_notify(l->client_id, status);
>> -
>> -       /* In case of success update advertising state*/
>> -       if (!status)
>> -               advertising_cnt = l->start ? 1 : 0;
>> -
>> -       /*
>> -        * Let's remove client from the list in two cases
>> -        * 1. Start failed
>> -        * 2. Stop succeed
>> -        */
>> -       if ((l->start && status) || (!l->start && !status))
>> -               queue_remove(listen_apps, INT_TO_PTR(l->client_id));
>> -
>> -       free(l);
>> -}
>> -
>>   static void handle_client_listen(const void *buf, uint16_t len)
>>   {
>>         const struct hal_cmd_gatt_client_listen *cmd = buf;
>>
>
> Regards,
> Jakub
>

Best regards,
Grzegorz

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2014-09-02 14:34 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-08-13 14:50 [PATCH 1/2] android/gatt: Stop listening while unregistering app Grzegorz Kolodziejczyk
2014-08-13 14:50 ` [PATCH 2/2] android/gatt: Add helper to create listen data Grzegorz Kolodziejczyk
2014-08-26 10:56 ` [PATCH 1/2] android/gatt: Stop listening while unregistering app Tyszkowski Jakub
2014-09-02 14:34   ` Grzegorz Kolodziejczyk

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).