| Message ID | 20200630184629.95013-1-sonnysasaka@chromium.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] client: Add battery command | expand |
Hi Sonny, On Tue, Jun 30, 2020 at 1:48 PM Sonny Sasaka <sonnysasaka@chromium.org> wrote: > > This adds the "battery" command to show battery information of a peer > device based on org.bluez.Battery1 API. Example usage: > > [bluetooth]# battery XX:XX:XX:XX:XX:XX > Percentage: 100% It might be better to put the battery level under info command. > --- > client/main.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 62 insertions(+) > > diff --git a/client/main.c b/client/main.c > index 422da5593..8c1ed00fb 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; > @@ -1650,6 +1681,35 @@ static void cmd_info(int argc, char *argv[]) > return bt_shell_noninteractive_quit(EXIT_SUCCESS); > } > > +static void cmd_battery(int argc, char *argv[]) > +{ > + DBusMessageIter iter; > + GDBusProxy *device_proxy; > + GDBusProxy *battery_proxy; > + unsigned char percentage; > + > + device_proxy = find_device(argc, argv); > + if (!device_proxy) > + return bt_shell_noninteractive_quit(EXIT_FAILURE); > + > + battery_proxy = find_battery_by_path(battery_proxies, > + g_dbus_proxy_get_path(device_proxy)); > + if (!battery_proxy) { > + bt_shell_printf("Device %s does not have battery information\n", > + argv[1]); > + return bt_shell_noninteractive_quit(EXIT_FAILURE); > + } > + > + if (g_dbus_proxy_get_property(battery_proxy, "Percentage", &iter) == > + FALSE) > + return bt_shell_noninteractive_quit(EXIT_FAILURE); > + > + dbus_message_iter_get_basic(&iter, &percentage); > + bt_shell_printf("Percentage: %d%%\n", percentage); > + > + return bt_shell_noninteractive_quit(EXIT_SUCCESS); > +} > + > static void pair_reply(DBusMessage *message, void *user_data) > { > DBusError error; > @@ -2785,6 +2845,8 @@ static const struct bt_shell_menu main_menu = { > dev_generator }, > { "disconnect", "[dev]", cmd_disconn, "Disconnect device", > dev_generator }, > + { "battery", "[dev]", cmd_battery, "Show device battery", > + dev_generator }, > { } }, > }; > > -- > 2.26.2 >
Hi Luiz, I thought of doing that (combining with info command). But we foresee that the Battery API will get more complex (such as support of multiple batteries like Pixel Buds and Airpods) and deserve its own command. Furthermore, it's a different API (org.bluez.Device1 vs org.bluez.Battery1) and its inner working uses a specific profile (BAS over GATT), so I think that it deserves its own command. But anyway I sent another patch that combines this into the "info" command and if you think this is more appropriate we can combine it for now and separate it in the future when needed. On Wed, Jul 1, 2020 at 10:56 AM Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > > Hi Sonny, > > On Tue, Jun 30, 2020 at 1:48 PM Sonny Sasaka <sonnysasaka@chromium.org> wrote: > > > > This adds the "battery" command to show battery information of a peer > > device based on org.bluez.Battery1 API. Example usage: > > > > [bluetooth]# battery XX:XX:XX:XX:XX:XX > > Percentage: 100% > > It might be better to put the battery level under info command. > > > --- > > client/main.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 62 insertions(+) > > > > diff --git a/client/main.c b/client/main.c > > index 422da5593..8c1ed00fb 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; > > @@ -1650,6 +1681,35 @@ static void cmd_info(int argc, char *argv[]) > > return bt_shell_noninteractive_quit(EXIT_SUCCESS); > > } > > > > +static void cmd_battery(int argc, char *argv[]) > > +{ > > + DBusMessageIter iter; > > + GDBusProxy *device_proxy; > > + GDBusProxy *battery_proxy; > > + unsigned char percentage; > > + > > + device_proxy = find_device(argc, argv); > > + if (!device_proxy) > > + return bt_shell_noninteractive_quit(EXIT_FAILURE); > > + > > + battery_proxy = find_battery_by_path(battery_proxies, > > + g_dbus_proxy_get_path(device_proxy)); > > + if (!battery_proxy) { > > + bt_shell_printf("Device %s does not have battery information\n", > > + argv[1]); > > + return bt_shell_noninteractive_quit(EXIT_FAILURE); > > + } > > + > > + if (g_dbus_proxy_get_property(battery_proxy, "Percentage", &iter) == > > + FALSE) > > + return bt_shell_noninteractive_quit(EXIT_FAILURE); > > + > > + dbus_message_iter_get_basic(&iter, &percentage); > > + bt_shell_printf("Percentage: %d%%\n", percentage); > > + > > + return bt_shell_noninteractive_quit(EXIT_SUCCESS); > > +} > > + > > static void pair_reply(DBusMessage *message, void *user_data) > > { > > DBusError error; > > @@ -2785,6 +2845,8 @@ static const struct bt_shell_menu main_menu = { > > dev_generator }, > > { "disconnect", "[dev]", cmd_disconn, "Disconnect device", > > dev_generator }, > > + { "battery", "[dev]", cmd_battery, "Show device battery", > > + dev_generator }, > > { } }, > > }; > > > > -- > > 2.26.2 > > > > > -- > Luiz Augusto von Dentz
Hi Sonny, On Wed, Jul 1, 2020 at 12:23 PM Sonny Sasaka <sonnysasaka@chromium.org> wrote: > > Hi Luiz, > > I thought of doing that (combining with info command). But we foresee > that the Battery API will get more complex (such as support of > multiple batteries like Pixel Buds and Airpods) and deserve its own > command. Furthermore, it's a different API (org.bluez.Device1 vs > org.bluez.Battery1) and its inner working uses a specific profile (BAS > over GATT), so I think that it deserves its own command. But anyway I > sent another patch that combines this into the "info" command and if > you think this is more appropriate we can combine it for now and > separate it in the future when needed. It should be a problem to combine different interfaces, and if the Battery1 start to have more properties we just list them as well, our D-Bus client will have them cached anything since we do get the properties of all interface due to use of ObjectManager. > > On Wed, Jul 1, 2020 at 10:56 AM Luiz Augusto von Dentz > <luiz.dentz@gmail.com> wrote: > > > > Hi Sonny, > > > > On Tue, Jun 30, 2020 at 1:48 PM Sonny Sasaka <sonnysasaka@chromium.org> wrote: > > > > > > This adds the "battery" command to show battery information of a peer > > > device based on org.bluez.Battery1 API. Example usage: > > > > > > [bluetooth]# battery XX:XX:XX:XX:XX:XX > > > Percentage: 100% > > > > It might be better to put the battery level under info command. > > > > > --- > > > client/main.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++ > > > 1 file changed, 62 insertions(+) > > > > > > diff --git a/client/main.c b/client/main.c > > > index 422da5593..8c1ed00fb 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; > > > @@ -1650,6 +1681,35 @@ static void cmd_info(int argc, char *argv[]) > > > return bt_shell_noninteractive_quit(EXIT_SUCCESS); > > > } > > > > > > +static void cmd_battery(int argc, char *argv[]) > > > +{ > > > + DBusMessageIter iter; > > > + GDBusProxy *device_proxy; > > > + GDBusProxy *battery_proxy; > > > + unsigned char percentage; > > > + > > > + device_proxy = find_device(argc, argv); > > > + if (!device_proxy) > > > + return bt_shell_noninteractive_quit(EXIT_FAILURE); > > > + > > > + battery_proxy = find_battery_by_path(battery_proxies, > > > + g_dbus_proxy_get_path(device_proxy)); > > > + if (!battery_proxy) { > > > + bt_shell_printf("Device %s does not have battery information\n", > > > + argv[1]); > > > + return bt_shell_noninteractive_quit(EXIT_FAILURE); > > > + } > > > + > > > + if (g_dbus_proxy_get_property(battery_proxy, "Percentage", &iter) == > > > + FALSE) > > > + return bt_shell_noninteractive_quit(EXIT_FAILURE); > > > + > > > + dbus_message_iter_get_basic(&iter, &percentage); > > > + bt_shell_printf("Percentage: %d%%\n", percentage); > > > + > > > + return bt_shell_noninteractive_quit(EXIT_SUCCESS); > > > +} > > > + > > > static void pair_reply(DBusMessage *message, void *user_data) > > > { > > > DBusError error; > > > @@ -2785,6 +2845,8 @@ static const struct bt_shell_menu main_menu = { > > > dev_generator }, > > > { "disconnect", "[dev]", cmd_disconn, "Disconnect device", > > > dev_generator }, > > > + { "battery", "[dev]", cmd_battery, "Show device battery", > > > + dev_generator }, > > > { } }, > > > }; > > > > > > -- > > > 2.26.2 > > > > > > > > > -- > > Luiz Augusto von Dentz
diff --git a/client/main.c b/client/main.c index 422da5593..8c1ed00fb 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; @@ -1650,6 +1681,35 @@ static void cmd_info(int argc, char *argv[]) return bt_shell_noninteractive_quit(EXIT_SUCCESS); } +static void cmd_battery(int argc, char *argv[]) +{ + DBusMessageIter iter; + GDBusProxy *device_proxy; + GDBusProxy *battery_proxy; + unsigned char percentage; + + device_proxy = find_device(argc, argv); + if (!device_proxy) + return bt_shell_noninteractive_quit(EXIT_FAILURE); + + battery_proxy = find_battery_by_path(battery_proxies, + g_dbus_proxy_get_path(device_proxy)); + if (!battery_proxy) { + bt_shell_printf("Device %s does not have battery information\n", + argv[1]); + return bt_shell_noninteractive_quit(EXIT_FAILURE); + } + + if (g_dbus_proxy_get_property(battery_proxy, "Percentage", &iter) == + FALSE) + return bt_shell_noninteractive_quit(EXIT_FAILURE); + + dbus_message_iter_get_basic(&iter, &percentage); + bt_shell_printf("Percentage: %d%%\n", percentage); + + return bt_shell_noninteractive_quit(EXIT_SUCCESS); +} + static void pair_reply(DBusMessage *message, void *user_data) { DBusError error; @@ -2785,6 +2845,8 @@ static const struct bt_shell_menu main_menu = { dev_generator }, { "disconnect", "[dev]", cmd_disconn, "Disconnect device", dev_generator }, + { "battery", "[dev]", cmd_battery, "Show device battery", + dev_generator }, { } }, };