Hi Howard,
On Sun, Sep 13, 2020 at 8:26 PM Howard Chung <howardchung@xxxxxxxxxx> wrote:
>
> This patch implements some basic functions for ADV monitor in
> bluetoothctl
>
> [bluetooth]# show
> ...
> Advertisement Monitor Features:
> SupportedMonitorTypes: or_patterns
>
> Reviewed-by: Miao-chen Chou <mcchou@xxxxxxxxxxxx>
> Reviewed-by: Manish Mandlik <mmandlik@xxxxxxxxxxxx>
> ---
>
> Changes in v4:
> - Remove PRE-UPSTREAM in commit title
>
> Changes in v3:
> - Rename advertisement_monitor to adv_monitor
>
> Changes in v2:
> - Update the add-pattern usage
> - Fix styling issue in patch 1
> - Fix storage class declaration issue
>
> Makefile.tools | 2 +
> client/adv_monitor.c | 161 +++++++++++++++++++++++++++++++++++++++++++
> client/adv_monitor.h | 23 +++++++
> client/main.c | 28 ++++++++
> 4 files changed, 214 insertions(+)
> create mode 100644 client/adv_monitor.c
> create mode 100644 client/adv_monitor.h
>
> diff --git a/Makefile.tools b/Makefile.tools
> index 9b9236609..ed0fbf8a3 100644
> --- a/Makefile.tools
> +++ b/Makefile.tools
> @@ -7,6 +7,8 @@ client_bluetoothctl_SOURCES = client/main.c \
> client/agent.h client/agent.c \
> client/advertising.h \
> client/advertising.c \
> + client/adv_monitor.h \
> + client/adv_monitor.c \
> client/gatt.h client/gatt.c
> client_bluetoothctl_LDADD = gdbus/libgdbus-internal.la src/libshared-glib.la \
> $(GLIB_LIBS) $(DBUS_LIBS) -lreadline
> diff --git a/client/adv_monitor.c b/client/adv_monitor.c
> new file mode 100644
> index 000000000..2a62389d9
> --- /dev/null
> +++ b/client/adv_monitor.c
> @@ -0,0 +1,161 @@
> +/*
> + *
> + * BlueZ - Bluetooth protocol stack for Linux
> + *
> + * Copyright (C) 2020 Google LLC
> + *
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> +#define _GNU_SOURCE
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <stdint.h>
> +#include <stdbool.h>
> +#include <string.h>
> +
> +#include "gdbus/gdbus.h"
> +#include "src/shared/util.h"
> +#include "src/shared/shell.h"
> +#include "adv_monitor.h"
> +
> +#define ADV_MONITOR_APP_PATH "/org/bluez/adv_monitor_app"
> +#define ADV_MONITOR_INTERFACE "org.bluez.AdvertisementMonitor1"
> +
> +static struct adv_monitor_manager {
> + GSList *supported_types;
> + GSList *supported_features;
> + GDBusProxy *proxy;
> + gboolean app_registered;
> +} manager = { NULL, NULL, NULL, FALSE };
> +
> +static void set_supported_list(GSList **list, DBusMessageIter *iter)
> +{
> + char *str;
> + DBusMessageIter subiter;
> +
> + dbus_message_iter_recurse(iter, &subiter);
> + while (dbus_message_iter_get_arg_type(&subiter) ==
> + DBUS_TYPE_STRING) {
> + dbus_message_iter_get_basic(&subiter, &str);
> + *list = g_slist_append(*list, str);
> + dbus_message_iter_next(&subiter);
> + }
> +}
> +
> +void adv_monitor_add_manager(DBusConnection *conn, GDBusProxy *proxy)
> +{
> + DBusMessageIter iter;
> +
> + if (manager.proxy != NULL || manager.supported_types != NULL ||
> + manager.supported_features != NULL) {
> + bt_shell_printf("advertisement monitor manager already "
> + "added\n");
> + return;
> + }
> +
> + manager.proxy = proxy;
> +
> + if (g_dbus_proxy_get_property(proxy, "SupportedMonitorTypes", &iter))
> + set_supported_list(&(manager.supported_types), &iter);
> +
> + if (g_dbus_proxy_get_property(proxy, "SupportedFeatures", &iter))
> + set_supported_list(&(manager.supported_features), &iter);
> +
> +}
> +
> +void adv_monitor_remove_manager(DBusConnection *conn)
> +{
> + if (manager.supported_types != NULL)
> + g_slist_free(g_steal_pointer(&(manager.supported_types)));
> + if (manager.supported_features != NULL)
> + g_slist_free(g_steal_pointer(&(manager.supported_features)));
> + manager.proxy = NULL;
> + manager.app_registered = FALSE;
> +}
> +
> +static void register_setup(DBusMessageIter *iter, void *user_data)
> +{
> + const char *path = ADV_MONITOR_APP_PATH;
> +
> + dbus_message_iter_append_basic(iter, DBUS_TYPE_OBJECT_PATH, &path);
> +}
> +
> +static void register_reply(DBusMessage *message, void *user_data)
> +{
> + DBusConnection *conn = user_data;
> + DBusError error;
> +
> + dbus_error_init(&error);
> +
> + if (dbus_set_error_from_message(&error, message) == FALSE) {
> + bt_shell_printf("AdvertisementMonitor path registered\n");
Missing bt_shell_noninteractive_quit(EXIT_SUCCESS);
> + } else {
> + bt_shell_printf("Failed to register path: %s\n", error.name);
> + dbus_error_free(&error);
> + return bt_shell_noninteractive_quit(EXIT_FAILURE);
> + }
Test dbus_set_error_from_message(&error, message) alone, I actually
would recommend not using == TRUE/FALSE on new code. So I would have
done without the use of else clause:
if (!dbus_set_error_from_message)
...
return bt_shell_noninteractive_quit(EXIT_FAILURE);
bt_shell_noninteractive_quit(EXIT_SUCCESS);
> +}
> +
> +static void unregister_setup(DBusMessageIter *iter, void *user_data)
> +{
> + const char *path = ADV_MONITOR_APP_PATH;
> +
> + dbus_message_iter_append_basic(iter, DBUS_TYPE_OBJECT_PATH, &path);
> +}
> +
> +static void unregister_reply(DBusMessage *message, void *user_data)
> +{
> + DBusConnection *conn = user_data;
> + DBusError error;
> +
> + dbus_error_init(&error);
> +
> + if (dbus_set_error_from_message(&error, message) == FALSE) {
> + bt_shell_printf("AdvertisementMonitor path unregistered\n");
> + return bt_shell_noninteractive_quit(EXIT_SUCCESS);
> + }
> +
> + bt_shell_printf("Failed to unregister Advertisement Monitor:"
> + " %s\n", error.name);
> + dbus_error_free(&error);
> + return bt_shell_noninteractive_quit(EXIT_FAILURE);
> +}
> +
> +void adv_monitor_register_app(DBusConnection *conn)
> +{
> + if (manager.supported_types == NULL || manager.app_registered == TRUE ||
> + g_dbus_proxy_method_call(manager.proxy, "RegisterMonitor",
> + register_setup, register_reply,
> + NULL, NULL) == FALSE) {
> + bt_shell_printf("Failed to register Advertisement Monitor\n");
> + return bt_shell_noninteractive_quit(EXIT_FAILURE);
> + }
> + manager.app_registered = TRUE;
> +}
> +
> +void adv_monitor_unregister_app(DBusConnection *conn)
> +{
> + if (manager.app_registered == FALSE ||
> + g_dbus_proxy_method_call(manager.proxy, "UnregisterMonitor",
> + unregister_setup, unregister_reply,
> + NULL, NULL) == FALSE) {
> + bt_shell_printf("Failed to unregister Advertisement Monitor\n");
> + return bt_shell_noninteractive_quit(EXIT_FAILURE);
> + }
> + manager.app_registered = FALSE;
> +}
> diff --git a/client/adv_monitor.h b/client/adv_monitor.h
> new file mode 100644
> index 000000000..77b0b62c6
> --- /dev/null
> +++ b/client/adv_monitor.h
> @@ -0,0 +1,23 @@
> +/*
> + *
> + * BlueZ - Bluetooth protocol stack for Linux
> + *
> + * Copyright (C) 2020 Google LLC
> + *
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +void adv_monitor_add_manager(DBusConnection *conn, GDBusProxy *proxy);
> +void adv_monitor_remove_manager(DBusConnection *conn);
> +void adv_monitor_register_app(DBusConnection *conn);
> +void adv_monitor_unregister_app(DBusConnection *conn);
> diff --git a/client/main.c b/client/main.c
> index da877b546..75f8bc462 100644
> --- a/client/main.c
> +++ b/client/main.c
> @@ -41,6 +41,7 @@
> #include "agent.h"
> #include "gatt.h"
> #include "advertising.h"
> +#include "adv_monitor.h"
>
> /* String display constants */
> #define COLORED_NEW COLOR_GREEN "NEW" COLOR_OFF
> @@ -58,6 +59,7 @@ static char *auto_register_agent = NULL;
> struct adapter {
> GDBusProxy *proxy;
> GDBusProxy *ad_proxy;
> + GDBusProxy *adv_monitor_proxy;
> GList *devices;
> };
>
> @@ -528,6 +530,19 @@ static void ad_manager_added(GDBusProxy *proxy)
> adapter->ad_proxy = proxy;
> }
>
> +static void admon_manager_added(GDBusProxy *proxy)
> +{
> + struct adapter *adapter;
> +
> + adapter = find_ctrl(ctrl_list, g_dbus_proxy_get_path(proxy));
> + if (!adapter)
> + adapter = adapter_new(proxy);
> +
> + adapter->adv_monitor_proxy = proxy;
> + adv_monitor_add_manager(dbus_conn, proxy);
> + adv_monitor_register_app(dbus_conn);
> +}
> +
> static void proxy_added(GDBusProxy *proxy, void *user_data)
> {
> const char *interface;
> @@ -560,6 +575,9 @@ static void proxy_added(GDBusProxy *proxy, void *user_data)
> ad_manager_added(proxy);
> } else if (!strcmp(interface, "org.bluez.Battery1")) {
> battery_added(proxy);
> + } else if (!strcmp(interface,
> + "org.bluez.AdvertisementMonitorManager1")) {
> + admon_manager_added(proxy);
> }
> }
>
> @@ -653,6 +671,9 @@ static void proxy_removed(GDBusProxy *proxy, void *user_data)
> ad_unregister(dbus_conn, NULL);
> } else if (!strcmp(interface, "org.bluez.Battery1")) {
> battery_removed(proxy);
> + } else if (!strcmp(interface,
> + "org.bluez.AdvertisementMonitorManager1")) {
> + adv_monitor_remove_manager(dbus_conn);
> }
> }
>
> @@ -935,6 +956,13 @@ static void cmd_show(int argc, char *argv[])
> print_property(adapter->ad_proxy, "SupportedSecondaryChannels");
> }
>
> + if (adapter->adv_monitor_proxy) {
> + bt_shell_printf("Advertisement Monitor Features:\n");
> + print_property(adapter->adv_monitor_proxy,
> + "SupportedMonitorTypes");
> + print_property(adapter->adv_monitor_proxy, "SupportedFeatures");
> + }
> +
> return bt_shell_noninteractive_quit(EXIT_SUCCESS);
> }
>
> --
> 2.28.0.618.gf4bc123cb7-goog
Doesn't compile:
make --no-print-directory all-am
CC client/adv_monitor.o
client/adv_monitor.c: In function ‘register_reply’:
client/adv_monitor.c:283:18: error: unused variable ‘conn’
[-Werror=unused-variable]
283 | DBusConnection *conn = user_data;
| ^~~~
client/adv_monitor.c: In function ‘unregister_reply’:
client/adv_monitor.c:306:18: error: unused variable ‘conn’
[-Werror=unused-variable]
306 | DBusConnection *conn = user_data;
| ^~~~
cc1: all warnings being treated as errors
--
Luiz Augusto von Dentz
