2014-04-10 14:34:51

by Jakub Tyszkowski

[permalink] [raw]
Subject: [PATCH 1/4] android/gatt: Trivial whitespace fixes

---
android/gatt.c | 25 ++++++++++++-------------
1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index a7a7204..ff19767 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -745,8 +745,8 @@ connect:
return;

/* We are ok to perform connect now. Stop discovery
- * and once it is stopped continue with creating ACL
- */
+ * and once it is stopped continue with creating ACL
+ */
bt_le_discovery_stop(bt_le_discovery_stop_cb);
}

@@ -792,7 +792,7 @@ done:

queue_foreach(dev->clients, client_disconnect_notify, dev);

- /* Reset conn_id and put on disconnected list.*/
+ /* Reset conn_id and put on disconnected list. */
put_device_on_disc_list(dev);

return FALSE;
@@ -875,7 +875,7 @@ reply:
if (scanning)
bt_le_discovery_start(le_device_found_handler);

- /*FIXME: What to do if discovery won't start here. */
+ /* FIXME: What to do if discovery won't start here. */
}

static int connect_le(struct gatt_device *dev)
@@ -895,11 +895,10 @@ static int connect_le(struct gatt_device *dev)

DBG("Connection attempt to: %s", addr);

- /*TODO: If we are bonded then we should use higier sec level */
+ /* TODO: If we are bonded then we should use higier sec level */
sec_level = BT_IO_SEC_LOW;

- /*
- * This connection will help us catch any PDUs that comes before
+ /* This connection will help us catch any PDUs that comes before
* pairing finishes
*/
io = bt_io_connect(connect_cb, dev, NULL, &gerr,
@@ -996,7 +995,7 @@ static void bt_le_discovery_stop_cb(void)
{
DBG("");

- /* Check now if there is any device ready to connect*/
+ /* Check now if there is any device ready to connect */
if (connect_next_dev() < 0)
bt_le_discovery_start(le_device_found_handler);
}
@@ -1061,9 +1060,9 @@ static void handle_client_connect(const void *buf, uint16_t len)
android2bdaddr(&cmd->bdaddr, &addr);

/* We do support many clients for one device connection so lets check
- * If device is connected or in connecting state just update list of
- * clients
- */
+ * If device is connected or in connecting state just update list of
+ * clients
+ */
dev = find_device(&addr);
if (dev) {
/* Remeber to send dummy notification event if we area
@@ -1127,13 +1126,13 @@ reply:
ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_GATT, HAL_OP_GATT_CLIENT_CONNECT,
status);

- /* If there is an error here we should make sure dev is out.*/
+ /* If there is an error here we should make sure dev is out. */
if ((status != HAL_STATUS_SUCCESS) && dev) {
destroy_device(dev);
return;
}

- /* Send dummy notification since ACL is already up*/
+ /* Send dummy notification since ACL is already up */
if (send_notify) {
struct hal_ev_gatt_client_connect ev;

--
1.9.1



2014-04-11 13:43:48

by Szymon Janc

[permalink] [raw]
Subject: Re: [PATCH 1/4] android/gatt: Trivial whitespace fixes

Hi Jakub,

On Thursday 10 of April 2014 16:34:51 Jakub Tyszkowski wrote:
> ---
> android/gatt.c | 25 ++++++++++++-------------
> 1 file changed, 12 insertions(+), 13 deletions(-)
>
> diff --git a/android/gatt.c b/android/gatt.c
> index a7a7204..ff19767 100644
> --- a/android/gatt.c
> +++ b/android/gatt.c
> @@ -745,8 +745,8 @@ connect:
> return;
>
> /* We are ok to perform connect now. Stop discovery
> - * and once it is stopped continue with creating ACL
> - */
> + * and once it is stopped continue with creating ACL
> + */
> bt_le_discovery_stop(bt_le_discovery_stop_cb);
> }
>
> @@ -792,7 +792,7 @@ done:
>
> queue_foreach(dev->clients, client_disconnect_notify, dev);
>
> - /* Reset conn_id and put on disconnected list.*/
> + /* Reset conn_id and put on disconnected list. */
> put_device_on_disc_list(dev);
>
> return FALSE;
> @@ -875,7 +875,7 @@ reply:
> if (scanning)
> bt_le_discovery_start(le_device_found_handler);
>
> - /*FIXME: What to do if discovery won't start here. */
> + /* FIXME: What to do if discovery won't start here. */
> }
>
> static int connect_le(struct gatt_device *dev)
> @@ -895,11 +895,10 @@ static int connect_le(struct gatt_device *dev)
>
> DBG("Connection attempt to: %s", addr);
>
> - /*TODO: If we are bonded then we should use higier sec level */
> + /* TODO: If we are bonded then we should use higier sec level */
> sec_level = BT_IO_SEC_LOW;
>
> - /*
> - * This connection will help us catch any PDUs that comes before
> + /* This connection will help us catch any PDUs that comes before
> * pairing finishes
> */
> io = bt_io_connect(connect_cb, dev, NULL, &gerr,
> @@ -996,7 +995,7 @@ static void bt_le_discovery_stop_cb(void)
> {
> DBG("");
>
> - /* Check now if there is any device ready to connect*/
> + /* Check now if there is any device ready to connect */
> if (connect_next_dev() < 0)
> bt_le_discovery_start(le_device_found_handler);
> }
> @@ -1061,9 +1060,9 @@ static void handle_client_connect(const void *buf, uint16_t len)
> android2bdaddr(&cmd->bdaddr, &addr);
>
> /* We do support many clients for one device connection so lets check
> - * If device is connected or in connecting state just update list of
> - * clients
> - */
> + * If device is connected or in connecting state just update list of
> + * clients
> + */
> dev = find_device(&addr);
> if (dev) {
> /* Remeber to send dummy notification event if we area
> @@ -1127,13 +1126,13 @@ reply:
> ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_GATT, HAL_OP_GATT_CLIENT_CONNECT,
> status);
>
> - /* If there is an error here we should make sure dev is out.*/
> + /* If there is an error here we should make sure dev is out. */
> if ((status != HAL_STATUS_SUCCESS) && dev) {
> destroy_device(dev);
> return;
> }
>
> - /* Send dummy notification since ACL is already up*/
> + /* Send dummy notification since ACL is already up */
> if (send_notify) {
> struct hal_ev_gatt_client_connect ev;
>
>

All patches in this set are now applied, thanks.

--
Best regards,
Szymon Janc

2014-04-10 14:34:53

by Jakub Tyszkowski

[permalink] [raw]
Subject: [PATCH 3/4] android/gatt: Add Client's refresh function call

This adds clearing remote devices cache (services, characteristics and
descriptors).
---
android/gatt.c | 25 ++++++++++++++++++++++++-
1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/android/gatt.c b/android/gatt.c
index 525ad41..5c33253 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -1206,10 +1206,33 @@ static void handle_client_listen(const void *buf, uint16_t len)

static void handle_client_refresh(const void *buf, uint16_t len)
{
+ const struct hal_cmd_gatt_client_refresh *cmd = buf;
+ struct gatt_device *dev;
+ uint8_t status;
+ bdaddr_t bda;
+
+ /* This is Android's framework hidden API call. It seams that no
+ * notification is expected and Bluedroid silently updates device's
+ * cache under the hood. As we use lazy caching ,we can just clear the
+ * cache and we're done.
+ */
+
DBG("");

+ android2bdaddr(&cmd->bdaddr, &bda);
+ dev = find_device(&bda);
+ if (!dev) {
+ status = HAL_STATUS_FAILED;
+ goto done;
+ }
+
+ queue_remove_all(dev->services, NULL, NULL, destroy_service);
+
+ status = HAL_STATUS_SUCCESS;
+
+done:
ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_GATT, HAL_OP_GATT_CLIENT_REFRESH,
- HAL_STATUS_FAILED);
+ status);
}

static void handle_client_search_service(const void *buf, uint16_t len)
--
1.9.1


2014-04-10 14:34:54

by Jakub Tyszkowski

[permalink] [raw]
Subject: [PATCH 4/4] android/gatt: Add cache check in Clients caching callbacks

As we were only checking in API call functions if cache exist and not
in asynchronous callbacks, it was possible to schedule multiple
discovery sessions before cache was filled with data. This resulted in
adding duplicate entries, when all scheduled discovery sessions started
to cache data.
---
android/gatt.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index 5c33253..aa2a5fd 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -623,6 +623,12 @@ static void primary_cb(uint8_t status, GSList *services, void *user_data)
goto done;
}

+ if (!queue_isempty(dev->services)) {
+ info("gatt: Services already cached");
+ gatt_status = GATT_SUCCESS;
+ goto done;
+ }
+
/* There might be multiply services with same uuid. Therefore make sure
* each primary service one has unique instance_id
*/
@@ -1564,7 +1570,8 @@ static void discover_char_cb(uint8_t status, GSList *characteristics,
{
struct discover_char_data *data = user_data;

- cache_all_srvc_chars(characteristics, data->service->chars);
+ if (queue_isempty(data->service->chars))
+ cache_all_srvc_chars(characteristics, data->service->chars);

send_client_char_notify(queue_peek_head(data->service->chars),
data->conn_id, data->service);
@@ -1731,7 +1738,7 @@ static void gatt_discover_desc_cb(guint8 status, const guint8 *pdu, guint16 len,
if (status)
error("gatt: Discover all char descriptors failed: %s",
att_ecode2str(status));
- else
+ else if (queue_isempty(data->ch->descriptors))
cache_all_descr(pdu, len, data->ch->descriptors);

descr = queue_peek_head(data->ch->descriptors);
--
1.9.1


2014-04-10 14:34:52

by Jakub Tyszkowski

[permalink] [raw]
Subject: [PATCH 2/4] android/gatt: Fix sending device to (dis)connect callback

This fixes casting void* at wrong structure. We pass device, not bdaddr
as user data.
---
android/gatt.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index ff19767..525ad41 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -760,12 +760,11 @@ static void put_device_on_disc_list(struct gatt_device *dev)
static gboolean disconnected_cb(GIOChannel *io, GIOCondition cond,
gpointer user_data)
{
- bdaddr_t *addr = user_data;
- struct gatt_device *dev;
+ struct gatt_device *dev = user_data;
int sock, err = 0;
socklen_t len;

- dev = queue_remove_if(conn_list, match_dev_by_bdaddr, addr);
+ queue_remove(conn_list, dev);

sock = g_io_channel_unix_get_fd(io);
len = sizeof(err);
@@ -811,16 +810,14 @@ static void send_client_connect_notify(void *data, void *user_data)

static void connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
{
- bdaddr_t *addr = user_data;
- struct gatt_device *dev;
+ struct gatt_device *dev = user_data;
struct hal_ev_gatt_client_connect ev;
GAttrib *attrib;
static uint32_t conn_id = 0;
int32_t status;

/* Take device from conn waiting queue */
- dev = queue_remove_if(conn_wait_queue, match_dev_by_bdaddr, addr);
- if (!dev) {
+ if (!queue_remove(conn_wait_queue, dev)) {
error("gatt: Device not on the connect wait queue!?");
g_io_channel_shutdown(io, TRUE, NULL);
return;
--
1.9.1