2014-05-14 11:26:34

by Lukasz Rymanowski

[permalink] [raw]
Subject: [PATCH] android/gatt: Fix handling advertising state

This patch fixes enabling advertising.
It was not possible to enable it if we had server registered
which is on listen_apps list but does not trigger advertising.

This patch introduces static counter to track number of clients
requested advertising.
---
android/gatt.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index 157ebe6..ad89233 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -167,6 +167,7 @@ struct app_connection {
static struct ipc *hal_ipc = NULL;
static bdaddr_t adapter_addr;
static bool scanning = false;
+static unsigned int advertising_cnt = 0;

static struct queue *gatt_apps = NULL;
static struct queue *gatt_devices = NULL;
@@ -1374,6 +1375,10 @@ static void set_advertising_cb(uint8_t status, void *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
@@ -1418,7 +1423,8 @@ static void handle_client_listen(const void *buf, uint16_t len)
}

/* If listen is already on just return success*/
- if (queue_length(listen_apps) > 1) {
+ if (advertising_cnt > 0) {
+ advertising_cnt++;
status = HAL_STATUS_SUCCESS;
goto reply;
}
@@ -1435,7 +1441,8 @@ static void handle_client_listen(const void *buf, uint16_t len)
* In case there is more listening clients don't stop
* advertising
*/
- if (queue_length(listen_apps) > 1) {
+ if (advertising_cnt > 1) {
+ advertising_cnt--;
queue_remove(listen_apps,
INT_TO_PTR(cmd->client_if));
status = HAL_STATUS_SUCCESS;
--
1.8.4



2014-05-14 17:54:49

by Lukasz Rymanowski

[permalink] [raw]
Subject: Re: [PATCH] android/gatt: Fix handling advertising state

Hi Marcel

On 14 May 2014 18:10, Marcel Holtmann <[email protected]> wrote:
> Hi Lukasz,
>
>>>> This patch fixes enabling advertising.
>>>> It was not possible to enable it if we had server registered
>>>> which is on listen_apps list but does not trigger advertising.
>>>
>>> does this mean we always advertise when registering GATT server?
>> No. We just put each server on the listen_apps queue and once remote
>> device connects to us, this server gets connection callback.
>
> this sounds all a bit weird. Did Android get this wrong. We can connect to a remote device and we can still receive GATT server exchange. It has really nothing to do with central or peripheral roles.

This is bit weird I agree.
The fact is that if we want to send any read/write request to Android
Server App we need to have connection id for the server. So probably
we need to invoke connection cb for all the servers on any
incoming/outgoing connections. For now we do it only for incoming
connections just for testing. There is no single app using Server HAL
so we don't know how this actually should work.

>
> Regards
>
> Marcel
>

BR
Lukasz

2014-05-14 16:10:20

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH] android/gatt: Fix handling advertising state

Hi Lukasz,

>>> This patch fixes enabling advertising.
>>> It was not possible to enable it if we had server registered
>>> which is on listen_apps list but does not trigger advertising.
>>
>> does this mean we always advertise when registering GATT server?
> No. We just put each server on the listen_apps queue and once remote
> device connects to us, this server gets connection callback.

this sounds all a bit weird. Did Android get this wrong. We can connect to a remote device and we can still receive GATT server exchange. It has really nothing to do with central or peripheral roles.

Regards

Marcel


2014-05-14 16:02:17

by Lukasz Rymanowski

[permalink] [raw]
Subject: Re: [PATCH] android/gatt: Fix handling advertising state

Hi Marcel,

On Wed, May 14, 2014 at 5:12 PM, Marcel Holtmann <[email protected]> wrote:
> Hi Lukasz,
>
>> This patch fixes enabling advertising.
>> It was not possible to enable it if we had server registered
>> which is on listen_apps list but does not trigger advertising.
>
> does this mean we always advertise when registering GATT server?
No. We just put each server on the listen_apps queue and once remote
device connects to us, this server gets connection callback.

> If so, then this is wrong. The GATT server has nothing to do with advertising.
>
> Regards
>
> Marcel
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

BR
Lukasz

2014-05-14 15:12:27

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH] android/gatt: Fix handling advertising state

Hi Lukasz,

> This patch fixes enabling advertising.
> It was not possible to enable it if we had server registered
> which is on listen_apps list but does not trigger advertising.

does this mean we always advertise when registering GATT server? If so, then this is wrong. The GATT server has nothing to do with advertising.

Regards

Marcel


2014-05-14 13:19:11

by Szymon Janc

[permalink] [raw]
Subject: Re: [PATCH] android/gatt: Fix handling advertising state

Hi Ɓukasz,

On Wednesday 14 of May 2014 13:26:34 Lukasz Rymanowski wrote:
> This patch fixes enabling advertising.
> It was not possible to enable it if we had server registered
> which is on listen_apps list but does not trigger advertising.
>
> This patch introduces static counter to track number of clients
> requested advertising.
> ---
> android/gatt.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/android/gatt.c b/android/gatt.c
> index 157ebe6..ad89233 100644
> --- a/android/gatt.c
> +++ b/android/gatt.c
> @@ -167,6 +167,7 @@ struct app_connection {
> static struct ipc *hal_ipc = NULL;
> static bdaddr_t adapter_addr;
> static bool scanning = false;
> +static unsigned int advertising_cnt = 0;
>
> static struct queue *gatt_apps = NULL;
> static struct queue *gatt_devices = NULL;
> @@ -1374,6 +1375,10 @@ static void set_advertising_cb(uint8_t status, void *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
> @@ -1418,7 +1423,8 @@ static void handle_client_listen(const void *buf, uint16_t len)
> }
>
> /* If listen is already on just return success*/
> - if (queue_length(listen_apps) > 1) {
> + if (advertising_cnt > 0) {
> + advertising_cnt++;
> status = HAL_STATUS_SUCCESS;
> goto reply;
> }
> @@ -1435,7 +1441,8 @@ static void handle_client_listen(const void *buf, uint16_t len)
> * In case there is more listening clients don't stop
> * advertising
> */
> - if (queue_length(listen_apps) > 1) {
> + if (advertising_cnt > 1) {
> + advertising_cnt--;
> queue_remove(listen_apps,
> INT_TO_PTR(cmd->client_if));
> status = HAL_STATUS_SUCCESS;
>

Patch applied, thanks.

--
Best regards,
Szymon Janc