2014-05-27 10:15:29

by Jakub Tyszkowski

Subject: [PATCH 1/6] attrib: Fix minor whitespace issue

Replace spaces with tabs.
---
attrib/att.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/attrib/att.h b/attrib/att.h
index 068ddf8..c92cd5d 100644
--- a/attrib/att.h
+++ b/attrib/att.h
@@ -86,8 +86,8 @@
#define ATT_PSM 31

/* Flags for Execute Write Request Operation */
-#define ATT_CANCEL_ALL_PREP_WRITES 0x00
-#define ATT_WRITE_ALL_PREP_WRITES 0x01
+#define ATT_CANCEL_ALL_PREP_WRITES 0x00
+#define ATT_WRITE_ALL_PREP_WRITES 0x01

/* Find Information Response Formats */
#define ATT_FIND_INFO_RESP_FMT_16BIT 0x01
--
1.9.3

2014-05-27 20:56:15

by Szymon Janc

Subject: Re: [PATCH 1/6] attrib: Fix minor whitespace issue

Hi Jakub,

On Tuesday 27 of May 2014 12:15:29 Jakub Tyszkowski wrote:
> Replace spaces with tabs.
> ---
> attrib/att.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/attrib/att.h b/attrib/att.h
> index 068ddf8..c92cd5d 100644
> --- a/attrib/att.h
> +++ b/attrib/att.h
> @@ -86,8 +86,8 @@
> #define ATT_PSM 31
>
> /* Flags for Execute Write Request Operation */
> -#define ATT_CANCEL_ALL_PREP_WRITES 0x00
> -#define ATT_WRITE_ALL_PREP_WRITES 0x01
> +#define ATT_CANCEL_ALL_PREP_WRITES 0x00
> +#define ATT_WRITE_ALL_PREP_WRITES 0x01
>
> /* Find Information Response Formats */
> #define ATT_FIND_INFO_RESP_FMT_16BIT 0x01

Patches 1-4 are now applied, thanks.

--
BR
Szymon Janc

2014-05-27 20:51:56

by Szymon Janc

Subject: Re: [PATCH 5/6] android/gatt: Initialize crypto first

Hi Jakub,

On Tuesday 27 of May 2014 12:15:33 Jakub Tyszkowski wrote:
> This fixes the missleading error on crypto setup failure:
>
> 02-17 20:19:44.639 I/bluetoothd( 1705): bluetoothd[1706]: gatt: Failed
> to allocate memory for queues
> ---
> android/gatt.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/android/gatt.c b/android/gatt.c
> index dab9781..a8f072f 100644
> --- a/android/gatt.c
> +++ b/android/gatt.c
> @@ -5491,15 +5491,20 @@ bool bt_gatt_register(struct ipc *ipc, const
> bdaddr_t *addr) if (!start_listening_io())
> return false;
>
> + crypto = bt_crypto_new();
> + if (!crypto) {
> + error("gatt: Failed to setup crypto.");

listening_io should be cleaned up here.
Also no need for dot at the end of error message.

> + return false;
> + }
> +
> gatt_devices = queue_new();
> gatt_apps = queue_new();
> app_connections = queue_new();
> listen_apps = queue_new();
> gatt_db = gatt_db_new();
> - crypto = bt_crypto_new();
>
> - if (!gatt_devices || !gatt_apps || !listen_apps ||
> - !app_connections || !gatt_db || !crypto) {
> + if (!gatt_devices || !gatt_apps || !listen_apps || !app_connections ||
> + !gatt_db) {
> error("gatt: Failed to allocate memory for queues");
>
> queue_destroy(gatt_apps, NULL);

--
BR
Szymon Janc

2014-05-27 20:41:57

by Szymon Janc

Subject: Re: [PATCH 6/6] android/gatt: Exchange mtu on connect if acting as client

Hi Jakub,

On Tuesday 27 of May 2014 12:15:34 Jakub Tyszkowski wrote:
> When no client apps are registered we basically act as server only and
> mtu exchange request handling is enough. When acting as client we send
> request.
> ---
> android/gatt.c | 19 ++++++++++++++++---
> 1 file changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/android/gatt.c b/android/gatt.c
> index a8f072f..b120834 100644
> --- a/android/gatt.c
> +++ b/android/gatt.c
> @@ -320,11 +320,24 @@ static bool match_app_by_id(const void *data, const
> void *user_data) return client->id == exp_id;
> }
>
> +static bool match_app_by_type(const void *data, const void *user_data)
> +{
> + gatt_app_type_t app_type = PTR_TO_INT(user_data);
> + const struct gatt_app *app = data;
> +
> + return app->type == app_type;
> +}
> +
> static struct gatt_app *find_app_by_id(int32_t id)
> {
> return queue_find(gatt_apps, match_app_by_id, INT_TO_PTR(id));
> }
>
> +static struct gatt_app *find_app_by_type(gatt_app_type_t type)
> +{
> + return queue_find(gatt_apps, match_app_by_type, INT_TO_PTR(type));
> +}
> +
> static bool match_by_value(const void *data, const void *user_data)
> {
> return data == user_data;
> @@ -1116,9 +1129,9 @@ static void connect_cb(GIOChannel *io, GError *gerr,
> gpointer user_data)
>
> device_set_state(dev, DEVICE_CONNECTED);
>
> - /* Send exchange mtu request as we assume being client and server */
> - /* TODO: Dont exchange mtu if no client apps */
> - send_exchange_mtu_request(dev);
> + /* Send exchange mtu request if any client app was registered */
> + if (find_app_by_type(APP_CLIENT))
> + send_exchange_mtu_request(dev);
>
> status = GATT_SUCCESS;

I think we should also exchange MTU if first client app (since connection) is
registered and exchange MTU was not performed by remote client.

--
BR
Szymon Janc

2014-05-27 10:15:34

by Jakub Tyszkowski

Subject: [PATCH 6/6] android/gatt: Exchange mtu on connect if acting as client

When no client apps are registered we basically act as server only and
mtu exchange request handling is enough. When acting as client we send
request.
---
android/gatt.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index a8f072f..b120834 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -320,11 +320,24 @@ static bool match_app_by_id(const void *data, const void *user_data)
return client->id == exp_id;
}

+static bool match_app_by_type(const void *data, const void *user_data)
+{
+ gatt_app_type_t app_type = PTR_TO_INT(user_data);
+ const struct gatt_app *app = data;
+
+ return app->type == app_type;
+}
+
static struct gatt_app *find_app_by_id(int32_t id)
{
return queue_find(gatt_apps, match_app_by_id, INT_TO_PTR(id));
}

+static struct gatt_app *find_app_by_type(gatt_app_type_t type)
+{
+ return queue_find(gatt_apps, match_app_by_type, INT_TO_PTR(type));
+}
+
static bool match_by_value(const void *data, const void *user_data)
{
return data == user_data;
@@ -1116,9 +1129,9 @@ static void connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)

device_set_state(dev, DEVICE_CONNECTED);

- /* Send exchange mtu request as we assume being client and server */
- /* TODO: Dont exchange mtu if no client apps */
- send_exchange_mtu_request(dev);
+ /* Send exchange mtu request if any client app was registered */
+ if (find_app_by_type(APP_CLIENT))
+ send_exchange_mtu_request(dev);

status = GATT_SUCCESS;

--
1.9.3

2014-05-27 10:15:32

by Jakub Tyszkowski

Subject: [PATCH 4/6] shared/gatt-db: Fix cropping permissions value

---
src/shared/gatt-db.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
index d6f3143..998e93e 100644
--- a/src/shared/gatt-db.c
+++ b/src/shared/gatt-db.c
@@ -258,7 +258,7 @@ static uint16_t update_attribute_handle(struct gatt_db_service *service,
static void set_attribute_data(struct gatt_db_attribute *attribute,
gatt_db_read_t read_func,
gatt_db_write_t write_func,
- uint8_t permissions,
+ uint32_t permissions,
void *user_data)
{
attribute->permissions = permissions;
--
1.9.3

2014-05-27 10:15:33

by Jakub Tyszkowski

Subject: [PATCH 5/6] android/gatt: Initialize crypto first

This fixes the missleading error on crypto setup failure:

02-17 20:19:44.639 I/bluetoothd( 1705): bluetoothd[1706]: gatt: Failed
to allocate memory for queues
---
android/gatt.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index dab9781..a8f072f 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -5491,15 +5491,20 @@ bool bt_gatt_register(struct ipc *ipc, const bdaddr_t *addr)
if (!start_listening_io())
return false;

+ crypto = bt_crypto_new();
+ if (!crypto) {
+ error("gatt: Failed to setup crypto.");
+ return false;
+ }
+
gatt_devices = queue_new();
gatt_apps = queue_new();
app_connections = queue_new();
listen_apps = queue_new();
gatt_db = gatt_db_new();
- crypto = bt_crypto_new();

- if (!gatt_devices || !gatt_apps || !listen_apps ||
- !app_connections || !gatt_db || !crypto) {
+ if (!gatt_devices || !gatt_apps || !listen_apps || !app_connections ||
+ !gatt_db) {
error("gatt: Failed to allocate memory for queues");

queue_destroy(gatt_apps, NULL);
--
1.9.3

2014-05-27 10:15:31

by Jakub Tyszkowski

Subject: [PATCH 3/6] android/gatt: Use proper variable to store response length

This fixes confirmations being not send.
---
android/gatt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/android/gatt.c b/android/gatt.c
index 18e1e03..dab9781 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -5101,7 +5101,7 @@ static void att_handler(const uint8_t *ipdu, uint16_t len, gpointer user_data)
* registered for this indication, event will be send in
* handle_notification
*/
- length = enc_confirmation(opdu, sizeof(opdu));
+ resp_length = enc_confirmation(opdu, sizeof(opdu));
status = 0;
break;
case ATT_OP_HANDLE_NOTIFY:
--
1.9.3

2014-05-27 10:15:30

by Jakub Tyszkowski

Subject: [PATCH 2/6] gatt: Fix not freeing GError on failure

---
profiles/gatt/gas.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/profiles/gatt/gas.c b/profiles/gatt/gas.c
index d240450..b51b4a8 100644
--- a/profiles/gatt/gas.c
+++ b/profiles/gatt/gas.c
@@ -329,6 +329,11 @@ static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
DBG("MTU Exchange: Requesting %d", imtu);
}

+ if (gerr) {
+ error("Could not acquire att imtu and cid: %s", gerr->message);
+ g_error_free(gerr);
+ }
+
if (device_get_appearance(gas->device, &app) < 0) {
bt_uuid_t uuid;

--
1.9.3