2020-07-01 19:23:08

by Sonny Sasaka

[permalink] [raw]
Subject: [PATCH v2] client: Add battery percentage to info command

This adds the Battery Percentage to the info command based on
org.bluez.Battery1 API. Example usage:

[bluetooth]# info XX:XX:XX:XX:XX:XX
Device XX:XX:XX:XX:XX:XX (random)
Name: ...
Alias: ...
...
Modalias: ...
Battery Percentage: 100%
---
client/main.c | 43 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 43 insertions(+)

diff --git a/client/main.c b/client/main.c
index 422da5593..4b787240e 100644
--- a/client/main.c
+++ b/client/main.c
@@ -65,6 +65,7 @@ static struct adapter *default_ctrl;
static GDBusProxy *default_dev;
static GDBusProxy *default_attr;
static GList *ctrl_list;
+static GList *battery_proxies;

static const char *agent_arguments[] = {
"on",
@@ -107,7 +108,9 @@ static void disconnect_handler(DBusConnection *connection, void *user_data)
bt_shell_set_prompt(PROMPT_OFF);

g_list_free_full(ctrl_list, proxy_leak);
+ g_list_free_full(battery_proxies, proxy_leak);
ctrl_list = NULL;
+ battery_proxies = NULL;

default_ctrl = NULL;
}
@@ -445,6 +448,16 @@ done:
g_free(desc);
}

+static void battery_added(GDBusProxy *proxy)
+{
+ battery_proxies = g_list_append(battery_proxies, proxy);
+}
+
+static void battery_removed(GDBusProxy *proxy)
+{
+ battery_proxies = g_list_remove(battery_proxies, proxy);
+}
+
static void device_added(GDBusProxy *proxy)
{
DBusMessageIter iter;
@@ -539,6 +552,8 @@ static void proxy_added(GDBusProxy *proxy, void *user_data)
gatt_add_manager(proxy);
} else if (!strcmp(interface, "org.bluez.LEAdvertisingManager1")) {
ad_manager_added(proxy);
+ } else if (!strcmp(interface, "org.bluez.Battery1")) {
+ battery_added(proxy);
}
}

@@ -630,6 +645,8 @@ static void proxy_removed(GDBusProxy *proxy, void *user_data)
gatt_remove_manager(proxy);
} else if (!strcmp(interface, "org.bluez.LEAdvertisingManager1")) {
ad_unregister(dbus_conn, NULL);
+ } else if (!strcmp(interface, "org.bluez.Battery1")) {
+ battery_removed(proxy);
}
}

@@ -763,6 +780,20 @@ static struct adapter *find_ctrl_by_address(GList *source, const char *address)
return NULL;
}

+static GDBusProxy *find_battery_by_path(GList *source, const char *path)
+{
+ GList *list;
+
+ for (list = g_list_first(source); list; list = g_list_next(list)) {
+ GDBusProxy *proxy = list->data;
+
+ if (strcmp(g_dbus_proxy_get_path(proxy), path) == 0)
+ return proxy;
+ }
+
+ return NULL;
+}
+
static GDBusProxy *find_proxy_by_address(GList *source, const char *address)
{
GList *list;
@@ -1606,8 +1637,10 @@ static struct GDBusProxy *find_device(int argc, char *argv[])
static void cmd_info(int argc, char *argv[])
{
GDBusProxy *proxy;
+ GDBusProxy *battery_proxy;
DBusMessageIter iter;
const char *address;
+ unsigned char percentage;

proxy = find_device(argc, argv);
if (!proxy)
@@ -1647,6 +1680,16 @@ static void cmd_info(int argc, char *argv[])
print_property(proxy, "AdvertisingFlags");
print_property(proxy, "AdvertisingData");

+ battery_proxy = find_battery_by_path(battery_proxies,
+ g_dbus_proxy_get_path(proxy));
+ if (battery_proxy && g_dbus_proxy_get_property(
+ battery_proxy, "Percentage", &iter)) {
+ dbus_message_iter_get_basic(&iter, &percentage);
+ bt_shell_printf("\tBattery Percentage: %d%%\n", percentage);
+ } else {
+ bt_shell_printf("\tNo battery information\n");
+ }
+
return bt_shell_noninteractive_quit(EXIT_SUCCESS);
}

--
2.17.1


2020-07-02 05:20:58

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH v2] client: Add battery percentage to info command

Hi Sonny,

On Wed, Jul 1, 2020 at 12:24 PM Sonny Sasaka <[email protected]> wrote:
>
> This adds the Battery Percentage to the info command based on
> org.bluez.Battery1 API. Example usage:
>
> [bluetooth]# info XX:XX:XX:XX:XX:XX
> Device XX:XX:XX:XX:XX:XX (random)
> Name: ...
> Alias: ...
> ...
> Modalias: ...
> Battery Percentage: 100%
> ---
> client/main.c | 43 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 43 insertions(+)
>
> diff --git a/client/main.c b/client/main.c
> index 422da5593..4b787240e 100644
> --- a/client/main.c
> +++ b/client/main.c
> @@ -65,6 +65,7 @@ static struct adapter *default_ctrl;
> static GDBusProxy *default_dev;
> static GDBusProxy *default_attr;
> static GList *ctrl_list;
> +static GList *battery_proxies;
>
> static const char *agent_arguments[] = {
> "on",
> @@ -107,7 +108,9 @@ static void disconnect_handler(DBusConnection *connection, void *user_data)
> bt_shell_set_prompt(PROMPT_OFF);
>
> g_list_free_full(ctrl_list, proxy_leak);
> + g_list_free_full(battery_proxies, proxy_leak);
> ctrl_list = NULL;
> + battery_proxies = NULL;
>
> default_ctrl = NULL;
> }
> @@ -445,6 +448,16 @@ done:
> g_free(desc);
> }
>
> +static void battery_added(GDBusProxy *proxy)
> +{
> + battery_proxies = g_list_append(battery_proxies, proxy);
> +}
> +
> +static void battery_removed(GDBusProxy *proxy)
> +{
> + battery_proxies = g_list_remove(battery_proxies, proxy);
> +}
> +
> static void device_added(GDBusProxy *proxy)
> {
> DBusMessageIter iter;
> @@ -539,6 +552,8 @@ static void proxy_added(GDBusProxy *proxy, void *user_data)
> gatt_add_manager(proxy);
> } else if (!strcmp(interface, "org.bluez.LEAdvertisingManager1")) {
> ad_manager_added(proxy);
> + } else if (!strcmp(interface, "org.bluez.Battery1")) {
> + battery_added(proxy);
> }
> }
>
> @@ -630,6 +645,8 @@ static void proxy_removed(GDBusProxy *proxy, void *user_data)
> gatt_remove_manager(proxy);
> } else if (!strcmp(interface, "org.bluez.LEAdvertisingManager1")) {
> ad_unregister(dbus_conn, NULL);
> + } else if (!strcmp(interface, "org.bluez.Battery1")) {
> + battery_removed(proxy);
> }
> }
>
> @@ -763,6 +780,20 @@ static struct adapter *find_ctrl_by_address(GList *source, const char *address)
> return NULL;
> }
>
> +static GDBusProxy *find_battery_by_path(GList *source, const char *path)
> +{
> + GList *list;
> +
> + for (list = g_list_first(source); list; list = g_list_next(list)) {
> + GDBusProxy *proxy = list->data;
> +
> + if (strcmp(g_dbus_proxy_get_path(proxy), path) == 0)
> + return proxy;
> + }
> +
> + return NULL;
> +}
> +
> static GDBusProxy *find_proxy_by_address(GList *source, const char *address)
> {
> GList *list;
> @@ -1606,8 +1637,10 @@ static struct GDBusProxy *find_device(int argc, char *argv[])
> static void cmd_info(int argc, char *argv[])
> {
> GDBusProxy *proxy;
> + GDBusProxy *battery_proxy;
> DBusMessageIter iter;
> const char *address;
> + unsigned char percentage;
>
> proxy = find_device(argc, argv);
> if (!proxy)
> @@ -1647,6 +1680,16 @@ static void cmd_info(int argc, char *argv[])
> print_property(proxy, "AdvertisingFlags");
> print_property(proxy, "AdvertisingData");
>
> + battery_proxy = find_battery_by_path(battery_proxies,
> + g_dbus_proxy_get_path(proxy));

I'd replace the lines below with just print_property(battery_proxy,
"Percentage"); and just make print_property check for NULL proxy if it
doesn't already.

> + if (battery_proxy && g_dbus_proxy_get_property(
> + battery_proxy, "Percentage", &iter)) {
> + dbus_message_iter_get_basic(&iter, &percentage);
> + bt_shell_printf("\tBattery Percentage: %d%%\n", percentage);
> + } else {
> + bt_shell_printf("\tNo battery information\n");
> + }
> +
> return bt_shell_noninteractive_quit(EXIT_SUCCESS);
> }
>
> --
> 2.17.1
>


--
Luiz Augusto von Dentz

2020-07-02 06:07:05

by Sonny Sasaka

[permalink] [raw]
Subject: Re: [PATCH v2] client: Add battery percentage to info command

Hi Luiz,

On Wed, Jul 1, 2020 at 10:20 PM Luiz Augusto von Dentz
<[email protected]> wrote:
>
> Hi Sonny,
>
> On Wed, Jul 1, 2020 at 12:24 PM Sonny Sasaka <[email protected]> wrote:
> >
> > This adds the Battery Percentage to the info command based on
> > org.bluez.Battery1 API. Example usage:
> >
> > [bluetooth]# info XX:XX:XX:XX:XX:XX
> > Device XX:XX:XX:XX:XX:XX (random)
> > Name: ...
> > Alias: ...
> > ...
> > Modalias: ...
> > Battery Percentage: 100%
> > ---
> > client/main.c | 43 +++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 43 insertions(+)
> >
> > diff --git a/client/main.c b/client/main.c
> > index 422da5593..4b787240e 100644
> > --- a/client/main.c
> > +++ b/client/main.c
> > @@ -65,6 +65,7 @@ static struct adapter *default_ctrl;
> > static GDBusProxy *default_dev;
> > static GDBusProxy *default_attr;
> > static GList *ctrl_list;
> > +static GList *battery_proxies;
> >
> > static const char *agent_arguments[] = {
> > "on",
> > @@ -107,7 +108,9 @@ static void disconnect_handler(DBusConnection *connection, void *user_data)
> > bt_shell_set_prompt(PROMPT_OFF);
> >
> > g_list_free_full(ctrl_list, proxy_leak);
> > + g_list_free_full(battery_proxies, proxy_leak);
> > ctrl_list = NULL;
> > + battery_proxies = NULL;
> >
> > default_ctrl = NULL;
> > }
> > @@ -445,6 +448,16 @@ done:
> > g_free(desc);
> > }
> >
> > +static void battery_added(GDBusProxy *proxy)
> > +{
> > + battery_proxies = g_list_append(battery_proxies, proxy);
> > +}
> > +
> > +static void battery_removed(GDBusProxy *proxy)
> > +{
> > + battery_proxies = g_list_remove(battery_proxies, proxy);
> > +}
> > +
> > static void device_added(GDBusProxy *proxy)
> > {
> > DBusMessageIter iter;
> > @@ -539,6 +552,8 @@ static void proxy_added(GDBusProxy *proxy, void *user_data)
> > gatt_add_manager(proxy);
> > } else if (!strcmp(interface, "org.bluez.LEAdvertisingManager1")) {
> > ad_manager_added(proxy);
> > + } else if (!strcmp(interface, "org.bluez.Battery1")) {
> > + battery_added(proxy);
> > }
> > }
> >
> > @@ -630,6 +645,8 @@ static void proxy_removed(GDBusProxy *proxy, void *user_data)
> > gatt_remove_manager(proxy);
> > } else if (!strcmp(interface, "org.bluez.LEAdvertisingManager1")) {
> > ad_unregister(dbus_conn, NULL);
> > + } else if (!strcmp(interface, "org.bluez.Battery1")) {
> > + battery_removed(proxy);
> > }
> > }
> >
> > @@ -763,6 +780,20 @@ static struct adapter *find_ctrl_by_address(GList *source, const char *address)
> > return NULL;
> > }
> >
> > +static GDBusProxy *find_battery_by_path(GList *source, const char *path)
> > +{
> > + GList *list;
> > +
> > + for (list = g_list_first(source); list; list = g_list_next(list)) {
> > + GDBusProxy *proxy = list->data;
> > +
> > + if (strcmp(g_dbus_proxy_get_path(proxy), path) == 0)
> > + return proxy;
> > + }
> > +
> > + return NULL;
> > +}
> > +
> > static GDBusProxy *find_proxy_by_address(GList *source, const char *address)
> > {
> > GList *list;
> > @@ -1606,8 +1637,10 @@ static struct GDBusProxy *find_device(int argc, char *argv[])
> > static void cmd_info(int argc, char *argv[])
> > {
> > GDBusProxy *proxy;
> > + GDBusProxy *battery_proxy;
> > DBusMessageIter iter;
> > const char *address;
> > + unsigned char percentage;
> >
> > proxy = find_device(argc, argv);
> > if (!proxy)
> > @@ -1647,6 +1680,16 @@ static void cmd_info(int argc, char *argv[])
> > print_property(proxy, "AdvertisingFlags");
> > print_property(proxy, "AdvertisingData");
> >
> > + battery_proxy = find_battery_by_path(battery_proxies,
> > + g_dbus_proxy_get_path(proxy));
>
> I'd replace the lines below with just print_property(battery_proxy,
> "Percentage"); and just make print_property check for NULL proxy if it
> doesn't already.
I tried this but the result doesn't look quite good:
Device XX:XX:XX:XX:XX:XX (random)
Name: ...
...
Modalias: ...
Percentage: 0x64

The name "Percentage" is not clear (what kind of percentage does it
mean) when shown in the "info" command. And also the format for byte
data type is hex and there is no percent sign. I tried to modify the
print_property function to support custom format but the code gets
overly complex and I think it's more straightforward to use custom
logic like in this patch.

>
> > + if (battery_proxy && g_dbus_proxy_get_property(
> > + battery_proxy, "Percentage", &iter)) {
> > + dbus_message_iter_get_basic(&iter, &percentage);
> > + bt_shell_printf("\tBattery Percentage: %d%%\n", percentage);
> > + } else {
> > + bt_shell_printf("\tNo battery information\n");
> > + }
> > +
> > return bt_shell_noninteractive_quit(EXIT_SUCCESS);
> > }
> >
> > --
> > 2.17.1
> >
>
>
> --
> Luiz Augusto von Dentz

2020-07-02 15:44:12

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH v2] client: Add battery percentage to info command

Hi Sonny,

On Wed, Jul 1, 2020 at 11:06 PM Sonny Sasaka <[email protected]> wrote:
>
> Hi Luiz,
>
> On Wed, Jul 1, 2020 at 10:20 PM Luiz Augusto von Dentz
> <[email protected]> wrote:
> >
> > Hi Sonny,
> >
> > On Wed, Jul 1, 2020 at 12:24 PM Sonny Sasaka <[email protected]> wrote:
> > >
> > > This adds the Battery Percentage to the info command based on
> > > org.bluez.Battery1 API. Example usage:
> > >
> > > [bluetooth]# info XX:XX:XX:XX:XX:XX
> > > Device XX:XX:XX:XX:XX:XX (random)
> > > Name: ...
> > > Alias: ...
> > > ...
> > > Modalias: ...
> > > Battery Percentage: 100%
> > > ---
> > > client/main.c | 43 +++++++++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 43 insertions(+)
> > >
> > > diff --git a/client/main.c b/client/main.c
> > > index 422da5593..4b787240e 100644
> > > --- a/client/main.c
> > > +++ b/client/main.c
> > > @@ -65,6 +65,7 @@ static struct adapter *default_ctrl;
> > > static GDBusProxy *default_dev;
> > > static GDBusProxy *default_attr;
> > > static GList *ctrl_list;
> > > +static GList *battery_proxies;
> > >
> > > static const char *agent_arguments[] = {
> > > "on",
> > > @@ -107,7 +108,9 @@ static void disconnect_handler(DBusConnection *connection, void *user_data)
> > > bt_shell_set_prompt(PROMPT_OFF);
> > >
> > > g_list_free_full(ctrl_list, proxy_leak);
> > > + g_list_free_full(battery_proxies, proxy_leak);
> > > ctrl_list = NULL;
> > > + battery_proxies = NULL;
> > >
> > > default_ctrl = NULL;
> > > }
> > > @@ -445,6 +448,16 @@ done:
> > > g_free(desc);
> > > }
> > >
> > > +static void battery_added(GDBusProxy *proxy)
> > > +{
> > > + battery_proxies = g_list_append(battery_proxies, proxy);
> > > +}
> > > +
> > > +static void battery_removed(GDBusProxy *proxy)
> > > +{
> > > + battery_proxies = g_list_remove(battery_proxies, proxy);
> > > +}
> > > +
> > > static void device_added(GDBusProxy *proxy)
> > > {
> > > DBusMessageIter iter;
> > > @@ -539,6 +552,8 @@ static void proxy_added(GDBusProxy *proxy, void *user_data)
> > > gatt_add_manager(proxy);
> > > } else if (!strcmp(interface, "org.bluez.LEAdvertisingManager1")) {
> > > ad_manager_added(proxy);
> > > + } else if (!strcmp(interface, "org.bluez.Battery1")) {
> > > + battery_added(proxy);
> > > }
> > > }
> > >
> > > @@ -630,6 +645,8 @@ static void proxy_removed(GDBusProxy *proxy, void *user_data)
> > > gatt_remove_manager(proxy);
> > > } else if (!strcmp(interface, "org.bluez.LEAdvertisingManager1")) {
> > > ad_unregister(dbus_conn, NULL);
> > > + } else if (!strcmp(interface, "org.bluez.Battery1")) {
> > > + battery_removed(proxy);
> > > }
> > > }
> > >
> > > @@ -763,6 +780,20 @@ static struct adapter *find_ctrl_by_address(GList *source, const char *address)
> > > return NULL;
> > > }
> > >
> > > +static GDBusProxy *find_battery_by_path(GList *source, const char *path)
> > > +{
> > > + GList *list;
> > > +
> > > + for (list = g_list_first(source); list; list = g_list_next(list)) {
> > > + GDBusProxy *proxy = list->data;
> > > +
> > > + if (strcmp(g_dbus_proxy_get_path(proxy), path) == 0)
> > > + return proxy;
> > > + }
> > > +
> > > + return NULL;
> > > +}
> > > +
> > > static GDBusProxy *find_proxy_by_address(GList *source, const char *address)
> > > {
> > > GList *list;
> > > @@ -1606,8 +1637,10 @@ static struct GDBusProxy *find_device(int argc, char *argv[])
> > > static void cmd_info(int argc, char *argv[])
> > > {
> > > GDBusProxy *proxy;
> > > + GDBusProxy *battery_proxy;
> > > DBusMessageIter iter;
> > > const char *address;
> > > + unsigned char percentage;
> > >
> > > proxy = find_device(argc, argv);
> > > if (!proxy)
> > > @@ -1647,6 +1680,16 @@ static void cmd_info(int argc, char *argv[])
> > > print_property(proxy, "AdvertisingFlags");
> > > print_property(proxy, "AdvertisingData");
> > >
> > > + battery_proxy = find_battery_by_path(battery_proxies,
> > > + g_dbus_proxy_get_path(proxy));
> >
> > I'd replace the lines below with just print_property(battery_proxy,
> > "Percentage"); and just make print_property check for NULL proxy if it
> > doesn't already.
> I tried this but the result doesn't look quite good:
> Device XX:XX:XX:XX:XX:XX (random)
> Name: ...
> ...
> Modalias: ...
> Percentage: 0x64
>
> The name "Percentage" is not clear (what kind of percentage does it
> mean) when shown in the "info" command. And also the format for byte
> data type is hex and there is no percent sign. I tried to modify the
> print_property function to support custom format but the code gets
> overly complex and I think it's more straightforward to use custom
> logic like in this patch.

Just make it always print both the decimal and (hex).

> >
> > > + if (battery_proxy && g_dbus_proxy_get_property(
> > > + battery_proxy, "Percentage", &iter)) {
> > > + dbus_message_iter_get_basic(&iter, &percentage);
> > > + bt_shell_printf("\tBattery Percentage: %d%%\n", percentage);
> > > + } else {
> > > + bt_shell_printf("\tNo battery information\n");
> > > + }
> > > +
> > > return bt_shell_noninteractive_quit(EXIT_SUCCESS);
> > > }
> > >
> > > --
> > > 2.17.1
> > >
> >
> >
> > --
> > Luiz Augusto von Dentz



--
Luiz Augusto von Dentz

2020-07-02 18:04:14

by Sonny Sasaka

[permalink] [raw]
Subject: [PATCH v3] client: Add battery percentage to info command

This adds the Battery Percentage to the info command based on
org.bluez.Battery1 API. Example usage:

[bluetooth]# info XX:XX:XX:XX:XX:XX
Device XX:XX:XX:XX:XX:XX (random)
Name: ...
Alias: ...
...
Modalias: ...
Battery Percentage: 0x64 (100)
---
client/main.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 46 insertions(+), 3 deletions(-)

diff --git a/client/main.c b/client/main.c
index 422da5593..c0b351aed 100644
--- a/client/main.c
+++ b/client/main.c
@@ -65,6 +65,7 @@ static struct adapter *default_ctrl;
static GDBusProxy *default_dev;
static GDBusProxy *default_attr;
static GList *ctrl_list;
+static GList *battery_proxies;

static const char *agent_arguments[] = {
"on",
@@ -107,7 +108,9 @@ static void disconnect_handler(DBusConnection *connection, void *user_data)
bt_shell_set_prompt(PROMPT_OFF);

g_list_free_full(ctrl_list, proxy_leak);
+ g_list_free_full(battery_proxies, proxy_leak);
ctrl_list = NULL;
+ battery_proxies = NULL;

default_ctrl = NULL;
}
@@ -271,7 +274,7 @@ static void print_iter(const char *label, const char *name,
break;
case DBUS_TYPE_BYTE:
dbus_message_iter_get_basic(iter, &byte);
- bt_shell_printf("%s%s: 0x%02x\n", label, name, byte);
+ bt_shell_printf("%s%s: 0x%02x (%d)\n", label, name, byte, byte);
break;
case DBUS_TYPE_VARIANT:
dbus_message_iter_recurse(iter, &subiter);
@@ -309,14 +312,20 @@ static void print_iter(const char *label, const char *name,
}
}

-static void print_property(GDBusProxy *proxy, const char *name)
+static void print_property_with_label(GDBusProxy *proxy, const char *name,
+ const char *label)
{
DBusMessageIter iter;

if (g_dbus_proxy_get_property(proxy, name, &iter) == FALSE)
return;

- print_iter("\t", name, &iter);
+ print_iter("\t", label ? label : name, &iter);
+}
+
+static void print_property(GDBusProxy *proxy, const char *name)
+{
+ print_property_with_label(proxy, name, NULL);
}

static void print_uuid(const char *uuid)
@@ -445,6 +454,16 @@ done:
g_free(desc);
}

+static void battery_added(GDBusProxy *proxy)
+{
+ battery_proxies = g_list_append(battery_proxies, proxy);
+}
+
+static void battery_removed(GDBusProxy *proxy)
+{
+ battery_proxies = g_list_remove(battery_proxies, proxy);
+}
+
static void device_added(GDBusProxy *proxy)
{
DBusMessageIter iter;
@@ -539,6 +558,8 @@ static void proxy_added(GDBusProxy *proxy, void *user_data)
gatt_add_manager(proxy);
} else if (!strcmp(interface, "org.bluez.LEAdvertisingManager1")) {
ad_manager_added(proxy);
+ } else if (!strcmp(interface, "org.bluez.Battery1")) {
+ battery_added(proxy);
}
}

@@ -630,6 +651,8 @@ static void proxy_removed(GDBusProxy *proxy, void *user_data)
gatt_remove_manager(proxy);
} else if (!strcmp(interface, "org.bluez.LEAdvertisingManager1")) {
ad_unregister(dbus_conn, NULL);
+ } else if (!strcmp(interface, "org.bluez.Battery1")) {
+ battery_removed(proxy);
}
}

@@ -763,6 +786,20 @@ static struct adapter *find_ctrl_by_address(GList *source, const char *address)
return NULL;
}

+static GDBusProxy *find_battery_by_path(GList *source, const char *path)
+{
+ GList *list;
+
+ for (list = g_list_first(source); list; list = g_list_next(list)) {
+ GDBusProxy *proxy = list->data;
+
+ if (strcmp(g_dbus_proxy_get_path(proxy), path) == 0)
+ return proxy;
+ }
+
+ return NULL;
+}
+
static GDBusProxy *find_proxy_by_address(GList *source, const char *address)
{
GList *list;
@@ -1606,6 +1643,7 @@ static struct GDBusProxy *find_device(int argc, char *argv[])
static void cmd_info(int argc, char *argv[])
{
GDBusProxy *proxy;
+ GDBusProxy *battery_proxy;
DBusMessageIter iter;
const char *address;

@@ -1647,6 +1685,11 @@ static void cmd_info(int argc, char *argv[])
print_property(proxy, "AdvertisingFlags");
print_property(proxy, "AdvertisingData");

+ battery_proxy = find_battery_by_path(battery_proxies,
+ g_dbus_proxy_get_path(proxy));
+ print_property_with_label(battery_proxy, "Percentage",
+ "Battery Percentage");
+
return bt_shell_noninteractive_quit(EXIT_SUCCESS);
}

--
2.17.1

2020-07-02 18:06:26

by Sonny Sasaka

[permalink] [raw]
Subject: Re: [PATCH v2] client: Add battery percentage to info command

Hi Luiz,

I just sent another patch following your feedback. Please take another
look. Thanks!

On Thu, Jul 2, 2020 at 8:43 AM Luiz Augusto von Dentz
<[email protected]> wrote:
>
> Hi Sonny,
>
> On Wed, Jul 1, 2020 at 11:06 PM Sonny Sasaka <[email protected]> wrote:
> >
> > Hi Luiz,
> >
> > On Wed, Jul 1, 2020 at 10:20 PM Luiz Augusto von Dentz
> > <[email protected]> wrote:
> > >
> > > Hi Sonny,
> > >
> > > On Wed, Jul 1, 2020 at 12:24 PM Sonny Sasaka <[email protected]> wrote:
> > > >
> > > > This adds the Battery Percentage to the info command based on
> > > > org.bluez.Battery1 API. Example usage:
> > > >
> > > > [bluetooth]# info XX:XX:XX:XX:XX:XX
> > > > Device XX:XX:XX:XX:XX:XX (random)
> > > > Name: ...
> > > > Alias: ...
> > > > ...
> > > > Modalias: ...
> > > > Battery Percentage: 100%
> > > > ---
> > > > client/main.c | 43 +++++++++++++++++++++++++++++++++++++++++++
> > > > 1 file changed, 43 insertions(+)
> > > >
> > > > diff --git a/client/main.c b/client/main.c
> > > > index 422da5593..4b787240e 100644
> > > > --- a/client/main.c
> > > > +++ b/client/main.c
> > > > @@ -65,6 +65,7 @@ static struct adapter *default_ctrl;
> > > > static GDBusProxy *default_dev;
> > > > static GDBusProxy *default_attr;
> > > > static GList *ctrl_list;
> > > > +static GList *battery_proxies;
> > > >
> > > > static const char *agent_arguments[] = {
> > > > "on",
> > > > @@ -107,7 +108,9 @@ static void disconnect_handler(DBusConnection *connection, void *user_data)
> > > > bt_shell_set_prompt(PROMPT_OFF);
> > > >
> > > > g_list_free_full(ctrl_list, proxy_leak);
> > > > + g_list_free_full(battery_proxies, proxy_leak);
> > > > ctrl_list = NULL;
> > > > + battery_proxies = NULL;
> > > >
> > > > default_ctrl = NULL;
> > > > }
> > > > @@ -445,6 +448,16 @@ done:
> > > > g_free(desc);
> > > > }
> > > >
> > > > +static void battery_added(GDBusProxy *proxy)
> > > > +{
> > > > + battery_proxies = g_list_append(battery_proxies, proxy);
> > > > +}
> > > > +
> > > > +static void battery_removed(GDBusProxy *proxy)
> > > > +{
> > > > + battery_proxies = g_list_remove(battery_proxies, proxy);
> > > > +}
> > > > +
> > > > static void device_added(GDBusProxy *proxy)
> > > > {
> > > > DBusMessageIter iter;
> > > > @@ -539,6 +552,8 @@ static void proxy_added(GDBusProxy *proxy, void *user_data)
> > > > gatt_add_manager(proxy);
> > > > } else if (!strcmp(interface, "org.bluez.LEAdvertisingManager1")) {
> > > > ad_manager_added(proxy);
> > > > + } else if (!strcmp(interface, "org.bluez.Battery1")) {
> > > > + battery_added(proxy);
> > > > }
> > > > }
> > > >
> > > > @@ -630,6 +645,8 @@ static void proxy_removed(GDBusProxy *proxy, void *user_data)
> > > > gatt_remove_manager(proxy);
> > > > } else if (!strcmp(interface, "org.bluez.LEAdvertisingManager1")) {
> > > > ad_unregister(dbus_conn, NULL);
> > > > + } else if (!strcmp(interface, "org.bluez.Battery1")) {
> > > > + battery_removed(proxy);
> > > > }
> > > > }
> > > >
> > > > @@ -763,6 +780,20 @@ static struct adapter *find_ctrl_by_address(GList *source, const char *address)
> > > > return NULL;
> > > > }
> > > >
> > > > +static GDBusProxy *find_battery_by_path(GList *source, const char *path)
> > > > +{
> > > > + GList *list;
> > > > +
> > > > + for (list = g_list_first(source); list; list = g_list_next(list)) {
> > > > + GDBusProxy *proxy = list->data;
> > > > +
> > > > + if (strcmp(g_dbus_proxy_get_path(proxy), path) == 0)
> > > > + return proxy;
> > > > + }
> > > > +
> > > > + return NULL;
> > > > +}
> > > > +
> > > > static GDBusProxy *find_proxy_by_address(GList *source, const char *address)
> > > > {
> > > > GList *list;
> > > > @@ -1606,8 +1637,10 @@ static struct GDBusProxy *find_device(int argc, char *argv[])
> > > > static void cmd_info(int argc, char *argv[])
> > > > {
> > > > GDBusProxy *proxy;
> > > > + GDBusProxy *battery_proxy;
> > > > DBusMessageIter iter;
> > > > const char *address;
> > > > + unsigned char percentage;
> > > >
> > > > proxy = find_device(argc, argv);
> > > > if (!proxy)
> > > > @@ -1647,6 +1680,16 @@ static void cmd_info(int argc, char *argv[])
> > > > print_property(proxy, "AdvertisingFlags");
> > > > print_property(proxy, "AdvertisingData");
> > > >
> > > > + battery_proxy = find_battery_by_path(battery_proxies,
> > > > + g_dbus_proxy_get_path(proxy));
> > >
> > > I'd replace the lines below with just print_property(battery_proxy,
> > > "Percentage"); and just make print_property check for NULL proxy if it
> > > doesn't already.
> > I tried this but the result doesn't look quite good:
> > Device XX:XX:XX:XX:XX:XX (random)
> > Name: ...
> > ...
> > Modalias: ...
> > Percentage: 0x64
> >
> > The name "Percentage" is not clear (what kind of percentage does it
> > mean) when shown in the "info" command. And also the format for byte
> > data type is hex and there is no percent sign. I tried to modify the
> > print_property function to support custom format but the code gets
> > overly complex and I think it's more straightforward to use custom
> > logic like in this patch.
>
> Just make it always print both the decimal and (hex).
>
> > >
> > > > + if (battery_proxy && g_dbus_proxy_get_property(
> > > > + battery_proxy, "Percentage", &iter)) {
> > > > + dbus_message_iter_get_basic(&iter, &percentage);
> > > > + bt_shell_printf("\tBattery Percentage: %d%%\n", percentage);
> > > > + } else {
> > > > + bt_shell_printf("\tNo battery information\n");
> > > > + }
> > > > +
> > > > return bt_shell_noninteractive_quit(EXIT_SUCCESS);
> > > > }
> > > >
> > > > --
> > > > 2.17.1
> > > >
> > >
> > >
> > > --
> > > Luiz Augusto von Dentz
>
>
>
> --
> Luiz Augusto von Dentz

2020-07-02 23:25:01

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH v3] client: Add battery percentage to info command

Hi Sonny,

On Thu, Jul 2, 2020 at 11:06 AM Sonny Sasaka <[email protected]> wrote:
>
> This adds the Battery Percentage to the info command based on
> org.bluez.Battery1 API. Example usage:
>
> [bluetooth]# info XX:XX:XX:XX:XX:XX
> Device XX:XX:XX:XX:XX:XX (random)
> Name: ...
> Alias: ...
> ...
> Modalias: ...
> Battery Percentage: 0x64 (100)
> ---
> client/main.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 46 insertions(+), 3 deletions(-)
>
> diff --git a/client/main.c b/client/main.c
> index 422da5593..c0b351aed 100644
> --- a/client/main.c
> +++ b/client/main.c
> @@ -65,6 +65,7 @@ static struct adapter *default_ctrl;
> static GDBusProxy *default_dev;
> static GDBusProxy *default_attr;
> static GList *ctrl_list;
> +static GList *battery_proxies;
>
> static const char *agent_arguments[] = {
> "on",
> @@ -107,7 +108,9 @@ static void disconnect_handler(DBusConnection *connection, void *user_data)
> bt_shell_set_prompt(PROMPT_OFF);
>
> g_list_free_full(ctrl_list, proxy_leak);
> + g_list_free_full(battery_proxies, proxy_leak);
> ctrl_list = NULL;
> + battery_proxies = NULL;
>
> default_ctrl = NULL;
> }
> @@ -271,7 +274,7 @@ static void print_iter(const char *label, const char *name,
> break;
> case DBUS_TYPE_BYTE:
> dbus_message_iter_get_basic(iter, &byte);
> - bt_shell_printf("%s%s: 0x%02x\n", label, name, byte);
> + bt_shell_printf("%s%s: 0x%02x (%d)\n", label, name, byte, byte);
> break;
> case DBUS_TYPE_VARIANT:
> dbus_message_iter_recurse(iter, &subiter);
> @@ -309,14 +312,20 @@ static void print_iter(const char *label, const char *name,
> }
> }
>
> -static void print_property(GDBusProxy *proxy, const char *name)
> +static void print_property_with_label(GDBusProxy *proxy, const char *name,
> + const char *label)
> {
> DBusMessageIter iter;
>
> if (g_dbus_proxy_get_property(proxy, name, &iter) == FALSE)
> return;
>
> - print_iter("\t", name, &iter);
> + print_iter("\t", label ? label : name, &iter);
> +}
> +
> +static void print_property(GDBusProxy *proxy, const char *name)
> +{
> + print_property_with_label(proxy, name, NULL);
> }
>
> static void print_uuid(const char *uuid)
> @@ -445,6 +454,16 @@ done:
> g_free(desc);
> }
>
> +static void battery_added(GDBusProxy *proxy)
> +{
> + battery_proxies = g_list_append(battery_proxies, proxy);
> +}
> +
> +static void battery_removed(GDBusProxy *proxy)
> +{
> + battery_proxies = g_list_remove(battery_proxies, proxy);
> +}
> +
> static void device_added(GDBusProxy *proxy)
> {
> DBusMessageIter iter;
> @@ -539,6 +558,8 @@ static void proxy_added(GDBusProxy *proxy, void *user_data)
> gatt_add_manager(proxy);
> } else if (!strcmp(interface, "org.bluez.LEAdvertisingManager1")) {
> ad_manager_added(proxy);
> + } else if (!strcmp(interface, "org.bluez.Battery1")) {
> + battery_added(proxy);
> }
> }
>
> @@ -630,6 +651,8 @@ static void proxy_removed(GDBusProxy *proxy, void *user_data)
> gatt_remove_manager(proxy);
> } else if (!strcmp(interface, "org.bluez.LEAdvertisingManager1")) {
> ad_unregister(dbus_conn, NULL);
> + } else if (!strcmp(interface, "org.bluez.Battery1")) {
> + battery_removed(proxy);
> }
> }
>
> @@ -763,6 +786,20 @@ static struct adapter *find_ctrl_by_address(GList *source, const char *address)
> return NULL;
> }
>
> +static GDBusProxy *find_battery_by_path(GList *source, const char *path)
> +{
> + GList *list;
> +
> + for (list = g_list_first(source); list; list = g_list_next(list)) {
> + GDBusProxy *proxy = list->data;
> +
> + if (strcmp(g_dbus_proxy_get_path(proxy), path) == 0)
> + return proxy;
> + }
> +
> + return NULL;
> +}
> +
> static GDBusProxy *find_proxy_by_address(GList *source, const char *address)
> {
> GList *list;
> @@ -1606,6 +1643,7 @@ static struct GDBusProxy *find_device(int argc, char *argv[])
> static void cmd_info(int argc, char *argv[])
> {
> GDBusProxy *proxy;
> + GDBusProxy *battery_proxy;
> DBusMessageIter iter;
> const char *address;
>
> @@ -1647,6 +1685,11 @@ static void cmd_info(int argc, char *argv[])
> print_property(proxy, "AdvertisingFlags");
> print_property(proxy, "AdvertisingData");
>
> + battery_proxy = find_battery_by_path(battery_proxies,
> + g_dbus_proxy_get_path(proxy));
> + print_property_with_label(battery_proxy, "Percentage",
> + "Battery Percentage");
> +
> return bt_shell_noninteractive_quit(EXIT_SUCCESS);
> }
>
> --
> 2.17.1
>

Applied, thanks.

--
Luiz Augusto von Dentz