This patch unifies destroy function names. We should also keep destroy
functions convention as it is in /src/shared/hci.c for example, and
check passed pointer before dereferencing it.
---
android/gatt.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/android/gatt.c b/android/gatt.c
index 04e9729..aeb0585 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -96,10 +96,13 @@ static struct queue *conn_wait_queue = NULL; /* Devs waiting to connect */
static void bt_le_discovery_stop_cb(void);
-static void free_gatt_service(void *data)
+static void destroy_service(void *data)
{
struct service *srvc = data;
+ if (!srvc)
+ return;
+
queue_destroy(srvc->chars, free);
free(srvc);
}
@@ -175,8 +178,11 @@ static void destroy_device(void *data)
{
struct gatt_device *dev = data;
+ if (!dev)
+ return;
+
queue_destroy(dev->clients, NULL);
- queue_destroy(dev->services, free_gatt_service);
+ queue_destroy(dev->services, destroy_service);
free(dev);
}
--
1.9.0
Hi Jakub,
On Wednesday 26 of March 2014 18:00:58 Jakub Tyszkowski wrote:
> We were sending wrong byte ordered uuids since the last gatt library
> changes. Two halpers were introduced for swapping uuids sent to and
> received from HAL.
> ---
> android/gatt.c | 45 ++++++++++++++++++++++++++++++++-------------
> 1 file changed, 32 insertions(+), 13 deletions(-)
>
> diff --git a/android/gatt.c b/android/gatt.c
> index fe8ba74..0b6b5a7 100644
> --- a/android/gatt.c
> +++ b/android/gatt.c
> @@ -96,6 +96,33 @@ static struct queue *conn_wait_queue = NULL; /* Devs waiting to connect */
>
> static void bt_le_discovery_stop_cb(void);
>
> +static void android2uuid(const void *buf, bt_uuid_t *dst)
Use const uint8_t * instead of void here.
> +{
> + uint8_t i;
> + char *uuid = (char *) buf;
> +
> + dst->type = BT_UUID128;
> +
> + for (i = 0; i < 16; i++)
> + dst->value.u128.data[i] = uuid[15-i];
> +
> +}
> +
> +static void uuid2android(const bt_uuid_t *src, void *buf)
Use uint8_t * instead of void * here.
> +{
> + char *uuid = (char *) buf;
> + bt_uuid_t uu128;
> + uint8_t i;
> +
> + if (src->type != BT_UUID128) {
> + bt_uuid_to_uuid128(src, &uu128);
> + src = &uu128;
> + }
> +
> + for (i = 0; i < 16; i++)
> + uuid[15-i] = src->value.u128.data[i];
> +}
> +
> static void destroy_service(void *data)
> {
> struct service *srvc = data;
> @@ -317,7 +344,7 @@ static void primary_cb(uint8_t status, GSList *services, void *user_data)
> continue;
> }
>
> - memcpy(&ev_res.srvc_id.uuid, &uuid.value, sizeof(uuid.value));
> + uuid2android(&uuid, &ev_res.srvc_id.uuid);
>
> ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT ,
> HAL_EV_GATT_CLIENT_SEARCH_RESULT,
> @@ -929,26 +956,22 @@ static void send_client_char_notify(const struct characteristic *ch,
> uint8_t status)
> {
> struct hal_ev_gatt_client_get_characteristic ev;
> - bt_uuid_t uuid;
>
> memset(&ev, 0, sizeof(ev));
> ev.status = status;
>
> if (ch) {
> ev.char_prop = ch->ch.properties;
> +
> ev.char_id.inst_id = ch->id.instance;
> - bt_string_to_uuid(&uuid, ch->ch.uuid);
> - memcpy(&ev.char_id.uuid, &uuid.value.u128.data,
> - sizeof(ev.char_id.uuid));
> + uuid2android(&ch->id.uuid, &ev.char_id.uuid);
> }
>
> ev.conn_id = conn_id;
> /* TODO need to be handled for included services too */
> ev.srvc_id.is_primary = 1;
> ev.srvc_id.inst_id = service->id.instance;
> - bt_string_to_uuid(&uuid, service->primary.uuid);
> - memcpy(&ev.srvc_id.uuid, &uuid.value.u128.data,
> - sizeof(ev.srvc_id.uuid));
> + uuid2android(&service->id.uuid, &ev.srvc_id.uuid);
>
> ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT,
> HAL_EV_GATT_CLIENT_GET_CHARACTERISTIC,
> @@ -1016,12 +1039,8 @@ static void discover_char_cb(uint8_t status, GSList *characteristics,
> static void hal_srvc_id_to_element_id(const struct hal_gatt_srvc_id *from,
> struct element_id *to)
> {
> - uint128_t uuid128;
> -
> to->instance = from->inst_id;
> -
> - memcpy(&uuid128.data, &from->uuid, sizeof(uuid128));
> - bt_uuid128_create(&to->uuid, uuid128);
> + android2uuid(&from->uuid, &to->uuid);
> }
>
> static bool find_service(int32_t conn_id, struct element_id *service_id,
>
--
Best regards,
Szymon Janc
Hi Jakub,
On Wednesday 26 of March 2014 18:00:53 Jakub Tyszkowski wrote:
> This patch unifies destroy function names. We should also keep destroy
> functions convention as it is in /src/shared/hci.c for example, and
> check passed pointer before dereferencing it.
> ---
> android/gatt.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/android/gatt.c b/android/gatt.c
> index 04e9729..aeb0585 100644
> --- a/android/gatt.c
> +++ b/android/gatt.c
> @@ -96,10 +96,13 @@ static struct queue *conn_wait_queue = NULL; /* Devs waiting to connect */
>
> static void bt_le_discovery_stop_cb(void);
>
> -static void free_gatt_service(void *data)
> +static void destroy_service(void *data)
> {
> struct service *srvc = data;
>
> + if (!srvc)
> + return;
> +
> queue_destroy(srvc->chars, free);
> free(srvc);
> }
> @@ -175,8 +178,11 @@ static void destroy_device(void *data)
> {
> struct gatt_device *dev = data;
>
> + if (!dev)
> + return;
> +
> queue_destroy(dev->clients, NULL);
> - queue_destroy(dev->services, free_gatt_service);
> + queue_destroy(dev->services, destroy_service);
> free(dev);
> }
>
>
Patches 1-5 are now applied. Thanks.
--
Best regards,
Szymon Janc
This patch removes temporary uuid as we already have uuid in services
element_id, which now is properly filled.
---
android/gatt.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/android/gatt.c b/android/gatt.c
index 0b6b5a7..d7c7c93 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -306,7 +306,6 @@ static void primary_cb(uint8_t status, GSList *services, void *user_data)
struct hal_ev_gatt_client_search_result ev_res;
struct gatt_primary *prim = l->data;
struct service *p;
- bt_uuid_t uuid;
p = new0(struct service, 1);
if (!p) {
@@ -323,6 +322,11 @@ static void primary_cb(uint8_t status, GSList *services, void *user_data)
memset(&ev_res, 0, sizeof(ev_res));
+ if (bt_string_to_uuid(&p->id.uuid, prim->uuid) < 0) {
+ error("gatt: Cannot convert string to uuid");
+ continue;
+ }
+
/* Put primary service to our local list */
memcpy(&p->primary, prim, sizeof(p->primary));
if (!queue_push_tail(dev->services, p)) {
@@ -339,12 +343,7 @@ static void primary_cb(uint8_t status, GSList *services, void *user_data)
ev_res.srvc_id.is_primary = 1;
ev_res.srvc_id.inst_id = 0;
- if (bt_string_to_uuid(&uuid, prim->uuid) < 0) {
- error("gatt: Cannot convert string to uuid");
- continue;
- }
-
- uuid2android(&uuid, &ev_res.srvc_id.uuid);
+ uuid2android(&p->id.uuid, &ev_res.srvc_id.uuid);
ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT ,
HAL_EV_GATT_CLIENT_SEARCH_RESULT,
--
1.9.0
This saves us from redundant data (de)allocation if characteristics are
already cached.
---
android/gatt.c | 34 +++++++++++++++++++---------------
1 file changed, 19 insertions(+), 15 deletions(-)
diff --git a/android/gatt.c b/android/gatt.c
index e97677d..fe8ba74 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -1054,7 +1054,6 @@ static bool find_service(int32_t conn_id, struct element_id *service_id,
static void handle_client_get_characteristic(const void *buf, uint16_t len)
{
const struct hal_cmd_gatt_client_get_characteristic *cmd = buf;
- struct discover_char_data *cb_data;
struct characteristic *ch;
struct element_id match_id;
struct gatt_device *dev;
@@ -1076,28 +1075,33 @@ static void handle_client_get_characteristic(const void *buf, uint16_t len)
goto done;
}
- cb_data = new0(struct discover_char_data, 1);
- if (!cb_data) {
- error("gatt: Cannot allocate call data");
- status = HAL_STATUS_FAILED;
- goto done;
- }
-
- cb_data->service = srvc;
- cb_data->conn_id = dev->conn_id;
-
/* Discover all characteristics for services if not cached yet */
if (queue_isempty(srvc->chars)) {
- gatt_discover_char(dev->attrib, srvc->primary.range.start,
+ struct discover_char_data *cb_data =
+ new0(struct discover_char_data, 1);
+
+ if (!cb_data) {
+ error("gatt: Cannot allocate cb data");
+ status = HAL_STATUS_FAILED;
+ goto done;
+ }
+
+ cb_data->service = srvc;
+ cb_data->conn_id = dev->conn_id;
+
+ if (!gatt_discover_char(dev->attrib, srvc->primary.range.start,
srvc->primary.range.end, NULL,
- discover_char_cb, cb_data);
+ discover_char_cb, cb_data)) {
+ free(cb_data);
+
+ status = HAL_STATUS_FAILED;
+ goto done;
+ }
status = HAL_STATUS_SUCCESS;
goto done;
}
- free(cb_data);
-
if (cmd->number)
ch = queue_find(srvc->chars, match_char_by_higher_inst_id,
INT_TO_PTR(cmd->gatt_id[0].inst_id));
--
1.9.0
We were sending wrong byte ordered uuids since the last gatt library
changes. Two halpers were introduced for swapping uuids sent to and
received from HAL.
---
android/gatt.c | 45 ++++++++++++++++++++++++++++++++-------------
1 file changed, 32 insertions(+), 13 deletions(-)
diff --git a/android/gatt.c b/android/gatt.c
index fe8ba74..0b6b5a7 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -96,6 +96,33 @@ static struct queue *conn_wait_queue = NULL; /* Devs waiting to connect */
static void bt_le_discovery_stop_cb(void);
+static void android2uuid(const void *buf, bt_uuid_t *dst)
+{
+ uint8_t i;
+ char *uuid = (char *) buf;
+
+ dst->type = BT_UUID128;
+
+ for (i = 0; i < 16; i++)
+ dst->value.u128.data[i] = uuid[15-i];
+
+}
+
+static void uuid2android(const bt_uuid_t *src, void *buf)
+{
+ char *uuid = (char *) buf;
+ bt_uuid_t uu128;
+ uint8_t i;
+
+ if (src->type != BT_UUID128) {
+ bt_uuid_to_uuid128(src, &uu128);
+ src = &uu128;
+ }
+
+ for (i = 0; i < 16; i++)
+ uuid[15-i] = src->value.u128.data[i];
+}
+
static void destroy_service(void *data)
{
struct service *srvc = data;
@@ -317,7 +344,7 @@ static void primary_cb(uint8_t status, GSList *services, void *user_data)
continue;
}
- memcpy(&ev_res.srvc_id.uuid, &uuid.value, sizeof(uuid.value));
+ uuid2android(&uuid, &ev_res.srvc_id.uuid);
ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT ,
HAL_EV_GATT_CLIENT_SEARCH_RESULT,
@@ -929,26 +956,22 @@ static void send_client_char_notify(const struct characteristic *ch,
uint8_t status)
{
struct hal_ev_gatt_client_get_characteristic ev;
- bt_uuid_t uuid;
memset(&ev, 0, sizeof(ev));
ev.status = status;
if (ch) {
ev.char_prop = ch->ch.properties;
+
ev.char_id.inst_id = ch->id.instance;
- bt_string_to_uuid(&uuid, ch->ch.uuid);
- memcpy(&ev.char_id.uuid, &uuid.value.u128.data,
- sizeof(ev.char_id.uuid));
+ uuid2android(&ch->id.uuid, &ev.char_id.uuid);
}
ev.conn_id = conn_id;
/* TODO need to be handled for included services too */
ev.srvc_id.is_primary = 1;
ev.srvc_id.inst_id = service->id.instance;
- bt_string_to_uuid(&uuid, service->primary.uuid);
- memcpy(&ev.srvc_id.uuid, &uuid.value.u128.data,
- sizeof(ev.srvc_id.uuid));
+ uuid2android(&service->id.uuid, &ev.srvc_id.uuid);
ipc_send_notif(hal_ipc, HAL_SERVICE_ID_GATT,
HAL_EV_GATT_CLIENT_GET_CHARACTERISTIC,
@@ -1016,12 +1039,8 @@ static void discover_char_cb(uint8_t status, GSList *characteristics,
static void hal_srvc_id_to_element_id(const struct hal_gatt_srvc_id *from,
struct element_id *to)
{
- uint128_t uuid128;
-
to->instance = from->inst_id;
-
- memcpy(&uuid128.data, &from->uuid, sizeof(uuid128));
- bt_uuid128_create(&to->uuid, uuid128);
+ android2uuid(&from->uuid, &to->uuid);
}
static bool find_service(int32_t conn_id, struct element_id *service_id,
--
1.9.0
This kind of search will be performed in most of the functions
implementing gatt's HAL API.
---
android/gatt.c | 40 ++++++++++++++++++++++++++++------------
1 file changed, 28 insertions(+), 12 deletions(-)
diff --git a/android/gatt.c b/android/gatt.c
index 06ae5cd..e97677d 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -1024,6 +1024,33 @@ static void hal_srvc_id_to_element_id(const struct hal_gatt_srvc_id *from,
bt_uuid128_create(&to->uuid, uuid128);
}
+static bool find_service(int32_t conn_id, struct element_id *service_id,
+ struct gatt_device **dev, struct service **srvc)
+{
+ struct gatt_device *device;
+ struct service *service;
+
+ device = queue_find(conn_list, match_dev_by_conn_id,
+ INT_TO_PTR(conn_id));
+ if (!device) {
+ error("gatt: conn_id=%d not found", conn_id);
+ return false;
+ }
+
+ service = queue_find(device->services, match_srvc_by_element_id,
+ service_id);
+ if (!service) {
+ error("gatt: Service with inst_id: %d not found",
+ service_id->instance);
+ return false;
+ }
+
+ *dev = device;
+ *srvc = service;
+
+ return true;
+}
+
static void handle_client_get_characteristic(const void *buf, uint16_t len)
{
const struct hal_cmd_gatt_client_get_characteristic *cmd = buf;
@@ -1043,19 +1070,8 @@ static void handle_client_get_characteristic(const void *buf, uint16_t len)
return;
}
- dev = queue_find(conn_list, match_dev_by_conn_id,
- INT_TO_PTR(cmd->conn_id));
- if (!dev) {
- error("gatt: conn_id=%d not found", cmd->conn_id);
- status = HAL_STATUS_FAILED;
- goto done;
- }
-
hal_srvc_id_to_element_id(&cmd->srvc_id, &match_id);
- srvc = queue_find(dev->services, match_srvc_by_element_id, &match_id);
- if (!srvc) {
- error("gatt: Service with inst_id: %d not found",
- match_id.instance);
+ if (!find_service(cmd->conn_id, &match_id, &dev, &srvc)) {
status = HAL_STATUS_FAILED;
goto done;
}
--
1.9.0
---
android/gatt.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/android/gatt.c b/android/gatt.c
index 2ea99e4..06ae5cd 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -1013,7 +1013,7 @@ static void discover_char_cb(uint8_t status, GSList *characteristics,
free(data);
}
-static void hal_srvc_id_to_gatt_id(const struct hal_gatt_srvc_id *from,
+static void hal_srvc_id_to_element_id(const struct hal_gatt_srvc_id *from,
struct element_id *to)
{
uint128_t uuid128;
@@ -1051,7 +1051,7 @@ static void handle_client_get_characteristic(const void *buf, uint16_t len)
goto done;
}
- hal_srvc_id_to_gatt_id(&cmd->srvc_id, &match_id);
+ hal_srvc_id_to_element_id(&cmd->srvc_id, &match_id);
srvc = queue_find(dev->services, match_srvc_by_element_id, &match_id);
if (!srvc) {
error("gatt: Service with inst_id: %d not found",
--
1.9.0
As there is no static type check for matching functions, we should keep
their names more descriptive, like 'match_dev_by_conn_id' is.
---
android/gatt.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/android/gatt.c b/android/gatt.c
index aeb0585..2ea99e4 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -151,7 +151,7 @@ static bool match_dev_by_conn_id(const void *data, const void *user_data)
return dev->conn_id == conn_id;
}
-static bool match_srvc_by_gatt_id(const void *data, const void *user_data)
+static bool match_srvc_by_element_id(const void *data, const void *user_data)
{
const struct element_id *exp_id = user_data;
const struct service *service = data;
@@ -1052,7 +1052,7 @@ static void handle_client_get_characteristic(const void *buf, uint16_t len)
}
hal_srvc_id_to_gatt_id(&cmd->srvc_id, &match_id);
- srvc = queue_find(dev->services, match_srvc_by_gatt_id, &match_id);
+ srvc = queue_find(dev->services, match_srvc_by_element_id, &match_id);
if (!srvc) {
error("gatt: Service with inst_id: %d not found",
match_id.instance);
--
1.9.0