Hi Miao,
On Tue, Aug 18, 2020 at 3:30 PM Miao-chen Chou <mcchou@xxxxxxxxxxxx> wrote:
>
> This introduces the org.bluez.AdvertisementMonitorManager1 without
> implementing handlers of methods and properties.
>
> The following test was performed.
> - Upon adapter registration, the info line of creating an ADV monitor
> manager gets printed, and system bus emits the interface events of
> org.bluez.AdvertisementMonitorManager1.
>
> Reviewed-by: Yun-Hao Chung <howardchung@xxxxxxxxxx>
> Reviewed-by: Manish Mandlik <mmandlik@xxxxxxxxxxxx>
> Reviewed-by: Abhishek Pandit-Subedi <abhishekpandit@xxxxxxxxxxxx>
> ---
>
> Makefile.am | 3 +-
> src/adapter.c | 14 +++++
> src/adapter.h | 3 +
> src/adv_monitor.c | 149 ++++++++++++++++++++++++++++++++++++++++++++++
> src/adv_monitor.h | 32 ++++++++++
> 5 files changed, 200 insertions(+), 1 deletion(-)
> create mode 100644 src/adv_monitor.c
> create mode 100644 src/adv_monitor.h
>
> diff --git a/Makefile.am b/Makefile.am
> index 7719c06f8..b14ee950e 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -293,7 +293,8 @@ src_bluetoothd_SOURCES = $(builtin_sources) \
> src/gatt-client.h src/gatt-client.c \
> src/device.h src/device.c \
> src/dbus-common.c src/dbus-common.h \
> - src/eir.h src/eir.c
> + src/eir.h src/eir.c \
> + src/adv_monitor.h src/adv_monitor.c
Id just name it monitor.{c, h}
> src_bluetoothd_LDADD = lib/libbluetooth-internal.la \
> gdbus/libgdbus-internal.la \
> src/libshared-glib.la \
> diff --git a/src/adapter.c b/src/adapter.c
> index 5e896a9f0..41e9de286 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -77,6 +77,7 @@
> #include "attrib-server.h"
> #include "gatt-database.h"
> #include "advertising.h"
> +#include "adv_monitor.h"
> #include "eir.h"
>
> #define ADAPTER_INTERFACE "org.bluez.Adapter1"
> @@ -272,6 +273,8 @@ struct btd_adapter {
> struct btd_gatt_database *database;
> struct btd_adv_manager *adv_manager;
>
> + struct btd_adv_monitor_manager *adv_monitor_manager;
> +
> gboolean initialized;
>
> GSList *pin_callbacks;
> @@ -6346,6 +6349,9 @@ static void adapter_remove(struct btd_adapter *adapter)
> btd_adv_manager_destroy(adapter->adv_manager);
> adapter->adv_manager = NULL;
>
> + btd_adv_monitor_manager_destroy(adapter->adv_monitor_manager);
> + adapter->adv_monitor_manager = NULL;
> +
> g_slist_free(adapter->pin_callbacks);
> adapter->pin_callbacks = NULL;
>
> @@ -8623,6 +8629,14 @@ static int adapter_register(struct btd_adapter *adapter)
>
> adapter->adv_manager = btd_adv_manager_new(adapter, adapter->mgmt);
>
> + adapter->adv_monitor_manager = btd_adv_monitor_manager_create(
> + adapter, adapter->mgmt);
> + if (!adapter->adv_monitor_manager) {
> + btd_error(adapter->dev_id,
> + "Failed to create Adv Monitor Manager for adapter");
> + return -EINVAL;
> + }
> +
> db = btd_gatt_database_get_db(adapter->database);
> adapter->db_id = gatt_db_register(db, services_modified,
> services_modified,
> diff --git a/src/adapter.h b/src/adapter.h
> index f8ac20261..5cb467141 100644
> --- a/src/adapter.h
> +++ b/src/adapter.h
> @@ -26,6 +26,9 @@
> #include <dbus/dbus.h>
> #include <glib.h>
>
> +#include "lib/bluetooth.h"
> +#include "lib/sdp.h"
> +
It might be better to have this included locally in a .c file needing them.
> #define MAX_NAME_LENGTH 248
>
> /* Invalid SSP passkey value used to indicate negative replies */
> diff --git a/src/adv_monitor.c b/src/adv_monitor.c
> new file mode 100644
> index 000000000..7044d3cca
> --- /dev/null
> +++ b/src/adv_monitor.c
> @@ -0,0 +1,149 @@
> +/*
> + *
> + * 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 Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 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
> + * Lesser General Public License for more details.
> + *
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> +#define _GNU_SOURCE
> +#include <stdint.h>
> +
> +#include <glib.h>
> +#include <dbus/dbus.h>
> +#include <gdbus/gdbus.h>
> +
> +#include "adapter.h"
> +#include "dbus-common.h"
> +#include "log.h"
> +#include "src/shared/mgmt.h"
> +
> +#include "adv_monitor.h"
> +
> +#define ADV_MONITOR_MGR_INTERFACE "org.bluez.AdvertisementMonitorManager1"
> +
> +struct btd_adv_monitor_manager {
> + struct btd_adapter *adapter;
> + struct mgmt *mgmt;
> + uint16_t adapter_id;
> + char *path;
> +};
> +
> +static const GDBusMethodTable adv_monitor_methods[] = {
> + { GDBUS_METHOD("RegisterMonitor",
> + GDBUS_ARGS({ "application", "o" }),
> + NULL, NULL) },
> + { GDBUS_ASYNC_METHOD("UnregisterMonitor",
> + GDBUS_ARGS({ "application", "o" }),
> + NULL, NULL) },
> + { }
> +};
> +
> +static const GDBusPropertyTable adv_monitor_properties[] = {
> + {"SupportedMonitorTypes", "as", NULL, NULL, NULL},
> + {"SupportedFeatures", "as", NULL, NULL, NULL},
> + { }
> +};
> +
> +/* Allocates a manager object */
> +static struct btd_adv_monitor_manager *manager_new(
> + struct btd_adapter *adapter,
> + struct mgmt *mgmt)
> +{
> + struct btd_adv_monitor_manager *manager;
> +
> + if (!adapter || !mgmt)
> + return NULL;
> +
> + manager = g_new0(struct btd_adv_monitor_manager, 1);
Use new0.
> + if (!manager)
> + return NULL;
> +
> + manager->adapter = adapter;
> + manager->mgmt = mgmt_ref(mgmt);
> + manager->adapter_id = btd_adapter_get_index(adapter);
> + manager->path = g_strdup(adapter_get_path(manager->adapter));
If we are doing to reference the adapter we don't really need the
duplicate its path since we can just use adapter_get_path.
> +
> + return manager;
> +}
> +
> +/* Frees a manager object */
> +static void manager_free(struct btd_adv_monitor_manager *manager)
> +{
> + manager->adapter = NULL;
No need to assign NULL if you are going to free the whole object at the end.
> + mgmt_unref(manager->mgmt);
> + manager->mgmt = NULL;
Ditto.
> + g_free(manager->path);
> + manager->path = NULL;
Ditto.
> +
> + g_free(manager);
> +}
> +
> +/* Destroys a manager object and unregisters its D-Bus interface */
> +static void manager_destroy(struct btd_adv_monitor_manager *manager)
> +{
> + if (!manager)
> + return;
> +
> + g_dbus_unregister_interface(btd_get_dbus_connection(),
> + manager->path,
> + ADV_MONITOR_MGR_INTERFACE);
> +
> + manager_free(manager);
> +}
> +
> +/* Creates a manager and registers its D-Bus interface */
> +struct btd_adv_monitor_manager *btd_adv_monitor_manager_create(
> + struct btd_adapter *adapter,
> + struct mgmt *mgmt)
> +{
> + struct btd_adv_monitor_manager *manager;
> +
> + manager = manager_new(adapter, mgmt);
> + if (!manager)
> + return NULL;
> +
> + if (g_dbus_register_interface(btd_get_dbus_connection(), manager->path,
> + ADV_MONITOR_MGR_INTERFACE,
> + adv_monitor_methods, NULL,
> + adv_monitor_properties, manager, NULL)
> + == FALSE) {
We haven't been consistent with boolean checks but lately we start
using more the ! form instead of == FALSE.
> + btd_error(manager->adapter_id,
> + "Failed to register "
> + ADV_MONITOR_MGR_INTERFACE);
> + manager_free(manager);
> + return NULL;
> + }
> +
> + btd_info(manager->adapter_id,
> + "Adv Monitor Manager created for adapter %s",
> + adapter_get_path(manager->adapter));
> +
> + return manager;
> +}
> +
> +/* Destroys a manager and unregisters its D-Bus interface */
> +void btd_adv_monitor_manager_destroy(struct btd_adv_monitor_manager *manager)
> +{
> + if (!manager)
> + return;
> +
> + btd_info(manager->adapter_id, "Destroy Adv Monitor Manager");
> +
> + manager_destroy(manager);
> +}
> diff --git a/src/adv_monitor.h b/src/adv_monitor.h
> new file mode 100644
> index 000000000..69ea348f8
> --- /dev/null
> +++ b/src/adv_monitor.h
> @@ -0,0 +1,32 @@
> +/*
> + *
> + * 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 Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 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
> + * Lesser General Public License for more details.
> + *
> + */
> +
> +#ifndef __ADV_MONITOR_H
> +#define __ADV_MONITOR_H
> +
> +struct mgmt;
> +struct btd_adapter;
> +struct btd_adv_monitor_manager;
> +
> +struct btd_adv_monitor_manager *btd_adv_monitor_manager_create(
> + struct btd_adapter *adapter,
> + struct mgmt *mgmt);
> +void btd_adv_monitor_manager_destroy(struct btd_adv_monitor_manager *manager);
> +
> +#endif /* __ADV_MONITOR_H */
> --
> 2.26.2
>
--
Luiz Augusto von Dentz
