Hi Miao,
On Tue, Aug 18, 2020 at 3:31 PM Miao-chen Chou <mcchou@xxxxxxxxxxxx> wrote:
>
> This adds two handlers, one for adding and one for removing, of D-Bus proxy
> events. The handler of property changes is set to NULL as intended,
> since for simplicity no further changes on monitor's properties would
> affect the ongoing monitoring.
>
> The following test steps were performed with bluetoothctl.
> - After registering the root path, expose two monitors and verify that
> the proxy added event are received.
> - Have two monitors added, unexpose the monitors, and verify that the
> proxy removed events are received for those two monitors.
> - Have two monitors added, unregister the monitors and verify that the
> proxy removed events are received for those two monitors.
>
> Reviewed-by: Yun-Hao Chung <howardchung@xxxxxxxxxx>
> Reviewed-by: Manish Mandlik <mmandlik@xxxxxxxxxxxx>
> ---
>
> src/adv_monitor.c | 492 ++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 479 insertions(+), 13 deletions(-)
>
> diff --git a/src/adv_monitor.c b/src/adv_monitor.c
> index b5ea5ee99..23fbc2b45 100644
> --- a/src/adv_monitor.c
> +++ b/src/adv_monitor.c
> @@ -37,14 +37,23 @@
> #include "dbus-common.h"
> #include "log.h"
> #include "src/error.h"
> +#include "src/shared/ad.h"
> #include "src/shared/mgmt.h"
> #include "src/shared/queue.h"
> #include "src/shared/util.h"
>
> #include "adv_monitor.h"
>
> +#define ADV_MONITOR_INTERFACE "org.bluez.AdvertisementMonitor1"
> #define ADV_MONITOR_MGR_INTERFACE "org.bluez.AdvertisementMonitorManager1"
>
> +#define ADV_MONITOR_UNSET_RSSI 127 /* dBm */
> +#define ADV_MONITOR_MAX_RSSI 20 /* dBm */
> +#define ADV_MONITOR_MIN_RSSI -127 /* dBm */
> +#define ADV_MONITOR_UNSET_TIMER 0 /* second */
> +#define ADV_MONITOR_MIN_TIMER 1 /* second */
> +#define ADV_MONITOR_MAX_TIMER 300 /* second */
> +
> struct btd_adv_monitor_manager {
> struct btd_adapter *adapter;
> struct mgmt *mgmt;
> @@ -66,6 +75,43 @@ struct adv_monitor_app {
>
> DBusMessage *reg;
> GDBusClient *client;
> +
> + struct queue *monitors;
> +};
> +
> +enum monitor_type {
> + MONITOR_TYPE_NONE,
> + MONITOR_TYPE_OR_PATTERNS,
> +};
> +
> +enum monitor_state {
> + MONITOR_STATE_NEW, /* New but not yet init'ed with actual values */
> + MONITOR_STATE_FAILED, /* Failed to be init'ed */
> + MONITOR_STATE_INITED, /* Init'ed but not yet sent to kernel */
> + MONITOR_STATE_HONORED, /* Accepted by kernel */
> +};
> +
> +struct pattern {
> + uint8_t ad_type;
> + uint8_t offset;
> + uint8_t length;
> + uint8_t value[BT_AD_MAX_DATA_LEN];
> +};
> +
> +struct adv_monitor {
> + struct adv_monitor_app *app;
> + GDBusProxy *proxy;
> + char *path;
> +
> + enum monitor_state state; /* MONITOR_STATE_* */
> +
> + int8_t high_rssi; /* high RSSI threshold */
> + uint16_t high_rssi_timeout; /* high RSSI threshold timeout */
> + int8_t low_rssi; /* low RSSI threshold */
> + uint16_t low_rssi_timeout; /* low RSSI threshold timeout */
> +
> + enum monitor_type type; /* MONITOR_TYPE_* */
> + struct queue *patterns;
> };
>
> struct app_match_data {
> @@ -73,6 +119,14 @@ struct app_match_data {
> const char *path;
> };
>
> +const struct adv_monitor_type {
> + enum monitor_type type;
> + const char *name;
> +} supported_types[] = {
> + { MONITOR_TYPE_OR_PATTERNS, "or_patterns" },
> + { },
> +};
> +
> /* Replies to an app's D-Bus message and unref it */
> static void app_reply_msg(struct adv_monitor_app *app, DBusMessage *reply)
> {
> @@ -84,6 +138,52 @@ static void app_reply_msg(struct adv_monitor_app *app, DBusMessage *reply)
> app->reg = NULL;
> }
>
> +/* Frees a pattern */
> +static void pattern_free(void *data)
> +{
> + struct pattern *pattern = data;
> +
> + if (!pattern)
> + return;
> +
> + g_free(pattern);
> +}
> +
> +/* Frees a monitor object */
> +static void monitor_free(void *data)
> +{
> + struct adv_monitor *monitor = data;
> +
> + if (!monitor)
> + return;
> +
> + monitor->app = NULL;
> + g_dbus_proxy_unref(monitor->proxy);
> + monitor->proxy = NULL;
> + g_free(monitor->path);
> + monitor->path = NULL;
> +
> + queue_destroy(monitor->patterns, pattern_free);
> + monitor->patterns = NULL;
> +
> + g_free(monitor);
> +}
> +
> +/* Calls Release() method of the remote Adv Monitor */
> +static void monitor_release(void *data, void *user_data)
> +{
> + struct adv_monitor *monitor = data;
> +
> + if (!monitor)
> + return;
> +
> + DBG("Calling Release() on Adv Monitor of owner %s at path %s",
> + monitor->app->owner, monitor->path);
> +
> + g_dbus_proxy_method_call(monitor->proxy, "Release", NULL, NULL, NULL,
> + NULL);
> +}
> +
> /* Destroys an app object along with related D-Bus handlers */
> static void app_destroy(void *data)
> {
> @@ -94,6 +194,9 @@ static void app_destroy(void *data)
>
> DBG("Destroy Adv Monitor app %s at path %s", app->owner, app->path);
>
> + queue_foreach(app->monitors, monitor_release, NULL);
> + queue_destroy(app->monitors, monitor_free);
> +
> if (app->reg) {
> app_reply_msg(app, btd_error_failed(app->reg,
> "Adv Monitor app destroyed"));
> @@ -139,6 +242,372 @@ static void app_ready_cb(GDBusClient *client, void *user_data)
> app_reply_msg(app, dbus_message_new_method_return(app->reg));
> }
>
> +/* Allocates an Adv Monitor */
> +static struct adv_monitor *monitor_new(struct adv_monitor_app *app,
> + GDBusProxy *proxy)
> +{
> + struct adv_monitor *monitor;
> +
> + if (!app || !proxy)
> + return NULL;
> +
> + monitor = g_new0(struct adv_monitor, 1);
new0
> + if (!monitor)
> + return NULL;
> +
> + monitor->app = app;
> + monitor->proxy = g_dbus_proxy_ref(proxy);
> + monitor->path = g_strdup(g_dbus_proxy_get_path(proxy));
> +
> + monitor->state = MONITOR_STATE_NEW;
> +
> + monitor->high_rssi = ADV_MONITOR_UNSET_RSSI;
> + monitor->high_rssi_timeout = ADV_MONITOR_UNSET_TIMER;
> + monitor->low_rssi = ADV_MONITOR_UNSET_RSSI;
> + monitor->low_rssi_timeout = ADV_MONITOR_UNSET_TIMER;
> +
> + monitor->type = MONITOR_TYPE_NONE;
> + monitor->patterns = NULL;
> +
> + return monitor;
> +}
> +
> +/* Matches a monitor based on its D-Bus path */
> +static bool monitor_match(const void *a, const void *b)
> +{
> + const GDBusProxy *proxy = b;
> + const struct adv_monitor *monitor = a;
> +
> + if (!proxy || !monitor)
> + return false;
> +
> + if (strcmp(g_dbus_proxy_get_path(proxy), monitor->path) != 0)
> + return false;
> +
> + return true;
> +}
> +
> +/* Retrieves Type from the remote Adv Monitor object, verifies the value and
> + * update the local Adv Monitor
> + */
> +static bool parse_monitor_type(struct adv_monitor *monitor, const char *path)
> +{
> + DBusMessageIter iter;
> + const struct adv_monitor_type *t;
> + const char *type_str;
> + uint16_t adapter_id = monitor->app->manager->adapter_id;
> +
> + if (!g_dbus_proxy_get_property(monitor->proxy, "Type", &iter)) {
> + btd_error(adapter_id, "Failed to retrieve property Type from "
> + "the Adv Monitor at path %s", path);
> + return false;
> + }
> +
> + if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
> + goto failed;
> +
> + dbus_message_iter_get_basic(&iter, &type_str);
> +
> + for (t = supported_types; t->name; t++) {
> + if (strcmp(t->name, type_str) == 0) {
> + monitor->type = t->type;
> + return true;
> + }
> + }
> +
> +failed:
> + btd_error(adapter_id, "Invalid argument of property Type of the Adv "
> + "Monitor at path %s", path);
> +
> + return false;
> +}
> +
> +/* Retrieves RSSIThresholdsAndTimers from the remote Adv Monitor object,
> + * verifies the values and update the local Adv Monitor
> + */
> +static bool parse_rssi_and_timeout(struct adv_monitor *monitor,
> + const char *path)
> +{
> + DBusMessageIter prop_struct, iter;
> + int16_t h_rssi, l_rssi;
> + uint16_t h_rssi_timer, l_rssi_timer;
> + uint16_t adapter_id = monitor->app->manager->adapter_id;
> +
> + /* Property RSSIThresholdsAndTimers is optional */
> + if (!g_dbus_proxy_get_property(monitor->proxy,
> + "RSSIThresholdsAndTimers",
> + &prop_struct)) {
> + DBG("Adv Monitor at path %s provides no RSSI thresholds and "
> + "timeouts", path);
> + return true;
> + }
> +
> + if (dbus_message_iter_get_arg_type(&prop_struct) != DBUS_TYPE_STRUCT)
> + goto failed;
> +
> + dbus_message_iter_recurse(&prop_struct, &iter);
> +
> + /* Extract HighRSSIThreshold */
> + if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_INT16)
> + goto failed;
> + dbus_message_iter_get_basic(&iter, &h_rssi);
> + if (!dbus_message_iter_next(&iter))
> + goto failed;
> +
> + /* Extract HighRSSIThresholdTimer */
> + if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_UINT16)
> + goto failed;
> + dbus_message_iter_get_basic(&iter, &h_rssi_timer);
> + if (!dbus_message_iter_next(&iter))
> + goto failed;
> +
> + /* Extract LowRSSIThreshold */
> + if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_INT16)
> + goto failed;
> + dbus_message_iter_get_basic(&iter, &l_rssi);
> + if (!dbus_message_iter_next(&iter))
> + goto failed;
> +
> + /* Extract LowRSSIThresholdTimer */
> + if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_UINT16)
> + goto failed;
> + dbus_message_iter_get_basic(&iter, &l_rssi_timer);
> +
> + /* Verify the values of RSSIs and their timers. For simplicity, we
> + * enforce the all-or-none rule to these fields. In other words, either
> + * all are set to the unset values or all are set within valid ranges.
> + */
> + if (h_rssi == ADV_MONITOR_UNSET_RSSI &&
> + l_rssi == ADV_MONITOR_UNSET_RSSI &&
> + h_rssi_timer == ADV_MONITOR_UNSET_TIMER &&
> + l_rssi_timer == ADV_MONITOR_UNSET_TIMER) {
> + goto done;
> + }
> +
> + if (h_rssi < ADV_MONITOR_MIN_RSSI || h_rssi > ADV_MONITOR_MAX_RSSI ||
> + l_rssi < ADV_MONITOR_MIN_RSSI ||
> + l_rssi > ADV_MONITOR_MAX_RSSI || h_rssi <= l_rssi) {
> + goto failed;
> + }
> +
> + if (h_rssi_timer < ADV_MONITOR_MIN_TIMER ||
> + h_rssi_timer > ADV_MONITOR_MAX_TIMER ||
> + l_rssi_timer < ADV_MONITOR_MIN_TIMER ||
> + l_rssi_timer > ADV_MONITOR_MAX_TIMER) {
> + goto failed;
> + }
> +
> + monitor->high_rssi = h_rssi;
> + monitor->low_rssi = l_rssi;
> + monitor->high_rssi_timeout = h_rssi_timer;
> + monitor->low_rssi_timeout = l_rssi_timer;
> +
> +done:
> + DBG("Adv Monitor at %s initiated with high RSSI threshold %d, high "
> + "RSSI threshold timeout %d, low RSSI threshold %d, low RSSI "
> + "threshold timeout %d", path, monitor->high_rssi,
> + monitor->high_rssi_timeout, monitor->low_rssi,
> + monitor->low_rssi_timeout);
> +
> + return true;
> +
> +failed:
> + monitor->high_rssi = ADV_MONITOR_UNSET_RSSI;
> + monitor->low_rssi = ADV_MONITOR_UNSET_RSSI;
> + monitor->high_rssi_timeout = ADV_MONITOR_UNSET_TIMER;
> + monitor->low_rssi_timeout = ADV_MONITOR_UNSET_TIMER;
> +
> + btd_error(adapter_id, "Invalid argument of property "
> + "RSSIThresholdsAndTimers of the Adv Monitor at path %s",
> + path);
> +
> + return false;
> +}
> +
> +/* Retrieves Patterns from the remote Adv Monitor object, verifies the values
> + * and update the local Adv Monitor
> + */
> +static bool parse_patterns(struct adv_monitor *monitor, const char *path)
> +{
> + DBusMessageIter array, array_iter;
> + uint16_t num_patterns = 0;
> + uint16_t adapter_id = monitor->app->manager->adapter_id;
> +
> + if (!g_dbus_proxy_get_property(monitor->proxy, "Patterns", &array)) {
> + btd_error(adapter_id, "Failed to retrieve property Patterns "
> + "from the Adv Monitor at path %s", path);
> + return false;
> + }
> +
> + monitor->patterns = queue_new();
> +
> + if (dbus_message_iter_get_arg_type(&array) != DBUS_TYPE_ARRAY ||
> + dbus_message_iter_get_element_type(&array) !=
> + DBUS_TYPE_STRUCT) {
> + goto failed;
> + }
> +
> + dbus_message_iter_recurse(&array, &array_iter);
> +
> + while (dbus_message_iter_get_arg_type(&array_iter) ==
> + DBUS_TYPE_STRUCT) {
> + int value_len;
> + uint8_t *value;
> + uint8_t offset, ad_type;
> + struct pattern *pattern;
> + DBusMessageIter struct_iter, value_iter;
> +
> + dbus_message_iter_recurse(&array_iter, &struct_iter);
> +
> + // Extract start position
> + if (dbus_message_iter_get_arg_type(&struct_iter) !=
> + DBUS_TYPE_BYTE) {
> + goto failed;
> + }
> + dbus_message_iter_get_basic(&struct_iter, &offset);
> + if (!dbus_message_iter_next(&struct_iter))
> + goto failed;
> +
> + // Extract AD data type
> + if (dbus_message_iter_get_arg_type(&struct_iter) !=
> + DBUS_TYPE_BYTE) {
> + goto failed;
> + }
> + dbus_message_iter_get_basic(&struct_iter, &ad_type);
> + if (!dbus_message_iter_next(&struct_iter))
> + goto failed;
> +
> + // Extract value of a pattern
> + if (dbus_message_iter_get_arg_type(&struct_iter) !=
> + DBUS_TYPE_ARRAY) {
> + goto failed;
> + }
> + dbus_message_iter_recurse(&struct_iter, &value_iter);
> + dbus_message_iter_get_fixed_array(&value_iter, &value,
> + &value_len);
> +
> + // Verify the values
> + if (offset > BT_AD_MAX_DATA_LEN - 1)
> + goto failed;
> +
> + if (ad_type > BT_AD_3D_INFO_DATA &&
> + ad_type != BT_AD_MANUFACTURER_DATA
> + || ad_type < BT_AD_FLAGS) {
> + goto failed;
> + }
> +
> + if (!value || value_len <= 0 || value_len > BT_AD_MAX_DATA_LEN)
> + goto failed;
> +
> + pattern = g_new0(struct pattern, 1);
new0
> + if (!pattern)
> + goto failed;
> +
> + pattern->ad_type = ad_type;
> + pattern->offset = offset;
> + pattern->length = value_len;
> + g_memmove(pattern->value, value, pattern->length);
This looks wrong, we shouldn't be changing the memory value points to,
this might be equivalent to memcpy so Id just use that instead.
> +
> + queue_push_tail(monitor->patterns, pattern);
> +
> + dbus_message_iter_next(&array_iter);
> + }
> +
> + /* There must be at least one pattern. */
> + if (queue_isempty(monitor->patterns))
> + goto failed;
> +
> + return true;
> +
> +failed:
> + queue_destroy(monitor->patterns, pattern_free);
> + monitor->patterns = NULL;
> +
> + btd_error(adapter_id, "Invalid argument of property Patterns of the "
> + "Adv Monitor at path %s", path);
> +
> + return false;
> +}
> +
> +/* Processes the content of the remote Adv Monitor */
> +static bool monitor_process(struct adv_monitor *monitor,
> + struct adv_monitor_app *app)
> +{
> + const char *path = g_dbus_proxy_get_path(monitor->proxy);
> +
> + monitor->state = MONITOR_STATE_FAILED;
> +
> + if (!parse_monitor_type(monitor, path))
> + goto done;
> +
> + if (!parse_rssi_and_timeout(monitor, path))
> + goto done;
> +
> + if (monitor->type == MONITOR_TYPE_OR_PATTERNS &&
> + parse_patterns(monitor, path)) {
> + monitor->state = MONITOR_STATE_INITED;
> + }
> +
> +done:
> + return monitor->state != MONITOR_STATE_FAILED;
> +}
> +
> +/* Handles an Adv Monitor D-Bus proxy added event */
> +static void monitor_proxy_added_cb(GDBusProxy *proxy, void *user_data)
> +{
> + struct adv_monitor *monitor;
> + struct adv_monitor_app *app = user_data;
> + uint16_t adapter_id = app->manager->adapter_id;
> + const char *path = g_dbus_proxy_get_path(proxy);
> + const char *iface = g_dbus_proxy_get_interface(proxy);
> +
> + if (strcmp(iface, ADV_MONITOR_INTERFACE) != 0 ||
> + !g_str_has_prefix(path, app->path)) {
> + return;
> + }
> +
> + if (queue_find(app->monitors, monitor_match, proxy)) {
> + btd_error(adapter_id, "Adv Monitor proxy already exists with "
> + "path %s", path);
> + return;
> + }
> +
> + monitor = monitor_new(app, proxy);
> + if (!monitor) {
> + btd_error(adapter_id, "Failed to allocate an Adv Monitor for "
> + "the object at %s", path);
> + return;
> + }
> +
> + if (!monitor_process(monitor, app)) {
> + monitor_release(monitor, NULL);
> + monitor_free(monitor);
> + DBG("Adv Monitor at path %s released due to invalid content",
> + path);
> + return;
> + }
> +
> + queue_push_tail(app->monitors, monitor);
> +
> + DBG("Adv Monitor allocated for the object at path %s", path);
> +}
> +
> +/* Handles the removal of an Adv Monitor D-Bus proxy */
> +static void monitor_proxy_removed_cb(GDBusProxy *proxy, void *user_data)
> +{
> + struct adv_monitor *monitor;
> + struct adv_monitor_app *app = user_data;
> +
> + monitor = queue_remove_if(app->monitors, monitor_match, proxy);
> + if (monitor) {
> + DBG("Adv Monitor removed for the object at path %s",
> + monitor->path);
> +
> + /* The object was gone, so we don't need to call Release() */
> + monitor_free(monitor);
> + }
> +}
> +
> /* Creates an app object, initiates it and sets D-Bus event handlers */
> static struct adv_monitor_app *app_create(DBusConnection *conn,
> DBusMessage *msg, const char *sender,
> @@ -165,8 +634,17 @@ static struct adv_monitor_app *app_create(DBusConnection *conn,
> return NULL;
> }
>
> + app->monitors = queue_new();
> +
> g_dbus_client_set_disconnect_watch(app->client, app_disconnect_cb, app);
> - g_dbus_client_set_proxy_handlers(app->client, NULL, NULL, NULL, NULL);
> +
> + /* Note that any property changes on a monitor object would not affect
> + * the content of the corresponding monitor.
> + */
> + g_dbus_client_set_proxy_handlers(app->client, monitor_proxy_added_cb,
> + monitor_proxy_removed_cb, NULL,
> + app);
> +
> g_dbus_client_set_ready_watch(app->client, app_ready_cb, app);
>
> app->reg = dbus_message_ref(msg);
> @@ -273,18 +751,6 @@ static const GDBusMethodTable adv_monitor_methods[] = {
> { }
> };
>
> -enum monitor_type {
> - MONITOR_TYPE_OR_PATTERNS,
> -};
> -
> -const struct adv_monitor_type {
> - enum monitor_type type;
> - const char *name;
> -} supported_types[] = {
> - { MONITOR_TYPE_OR_PATTERNS, "or_patterns" },
> - { },
> -};
> -
> /* Gets SupportedMonitorTypes property */
> static gboolean get_supported_monitor_types(const GDBusPropertyTable *property,
> DBusMessageIter *iter,
> --
> 2.26.2
>
--
Luiz Augusto von Dentz
