2016-08-29 09:14:48

by Per Thomas Jahr

[permalink] [raw]
Subject: [PATCH BlueZ] Include UUID in output

Added UUID in the output when listing attributes. This makes it more convenient to work with as it gives a mapping between UUID and attribute id.

Per Thomas Jahr (1):
client: Include UUID when listing attributes

client/gatt.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)

--
2.5.5



2016-08-29 09:31:54

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH BlueZ] client: Include UUID when listing attributes

Hi Per,

On Mon, Aug 29, 2016, Per Thomas Jahr wrote:
> Show the full UUID for services, characteristics and descriptors.
> ---
> client/gatt.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)

The general idea is fine with me but you've managed to change the
whitespace to something incompatible with the coding style - this needs
to be fixed before the patch is merged. I'd also suggest that you
provide some sample output of your changes in the commit message so that
reviewers can easily see how the result looks like in practice.

> diff --git a/client/gatt.c b/client/gatt.c
> index fee1cf9..201d668 100644
> --- a/client/gatt.c
> +++ b/client/gatt.c
> @@ -75,12 +75,13 @@ static void print_service(GDBusProxy *proxy, const char *description)
> if (!text)
> text = uuid;
>
> - rl_printf("%s%s%s%s Service\n\t%s\n\t%s\n",
> + rl_printf("%s%s%s%s Service\n\t%s\n\t%s\n\t%s\n",

Here the indentation of rl_printf has changed from a tab to spaces.

> description ? "[" : "",
> description ? : "",
> description ? "] " : "",
> primary ? "Primary" : "Secondary",
> - g_dbus_proxy_get_path(proxy),
> + g_dbus_proxy_get_path(proxy),
> + uuid,

Here the indentation is also spaces when it should be tabs. Also, you
can put 'text' on the same line as uuid to save some vertical space.

> @@ -118,12 +119,13 @@ static void print_characteristic(GDBusProxy *proxy, const char *description)
> if (!text)
> text = uuid;
>
> - rl_printf("%s%s%sCharacteristic\n\t%s\n\t%s\n",
> + rl_printf("%s%s%sCharacteristic\n\t%s\n\t%s\n\t%s\n",
> description ? "[" : "",
> description ? : "",
> description ? "] " : "",
> g_dbus_proxy_get_path(proxy),
> - text);
> + uuid,
> + text);

Same thing here.

> @@ -186,12 +188,13 @@ static void print_descriptor(GDBusProxy *proxy, const char *description)
> if (!text)
> text = uuid;
>
> - rl_printf("%s%s%sDescriptor\n\t%s\n\t%s\n",
> + rl_printf("%s%s%sDescriptor\n\t%s\n\t%s\n\t%s\n",
> description ? "[" : "",
> description ? : "",
> description ? "] " : "",
> g_dbus_proxy_get_path(proxy),
> - text);
> + uuid,
> + text);

And here.

Once these issues are fixed the patch should be good to go in (at least
on my behalf).

Johan

2016-08-29 09:14:49

by Per Thomas Jahr

[permalink] [raw]
Subject: [PATCH BlueZ] client: Include UUID when listing attributes

Show the full UUID for services, characteristics and descriptors.
---
client/gatt.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/client/gatt.c b/client/gatt.c
index fee1cf9..201d668 100644
--- a/client/gatt.c
+++ b/client/gatt.c
@@ -75,12 +75,13 @@ static void print_service(GDBusProxy *proxy, const char *description)
if (!text)
text = uuid;

- rl_printf("%s%s%s%s Service\n\t%s\n\t%s\n",
+ rl_printf("%s%s%s%s Service\n\t%s\n\t%s\n\t%s\n",
description ? "[" : "",
description ? : "",
description ? "] " : "",
primary ? "Primary" : "Secondary",
- g_dbus_proxy_get_path(proxy),
+ g_dbus_proxy_get_path(proxy),
+ uuid,
text);
}

@@ -118,12 +119,13 @@ static void print_characteristic(GDBusProxy *proxy, const char *description)
if (!text)
text = uuid;

- rl_printf("%s%s%sCharacteristic\n\t%s\n\t%s\n",
+ rl_printf("%s%s%sCharacteristic\n\t%s\n\t%s\n\t%s\n",
description ? "[" : "",
description ? : "",
description ? "] " : "",
g_dbus_proxy_get_path(proxy),
- text);
+ uuid,
+ text);
}

static gboolean characteristic_is_child(GDBusProxy *characteristic)
@@ -186,12 +188,13 @@ static void print_descriptor(GDBusProxy *proxy, const char *description)
if (!text)
text = uuid;

- rl_printf("%s%s%sDescriptor\n\t%s\n\t%s\n",
+ rl_printf("%s%s%sDescriptor\n\t%s\n\t%s\n\t%s\n",
description ? "[" : "",
description ? : "",
description ? "] " : "",
g_dbus_proxy_get_path(proxy),
- text);
+ uuid,
+ text);
}

static gboolean descriptor_is_child(GDBusProxy *characteristic)
--
2.5.5