All of lore.kernel.org
 help / color / mirror / Atom feed
* [BlueZ PATCH v1 1/4] client: Implement basic interface of ADV monitor in bluetoothctl
@ 2020-08-19  4:11 Howard Chung
  2020-08-19  4:11 ` [BlueZ PATCH v1 2/4] client: Implement more interfaces " Howard Chung
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Howard Chung @ 2020-08-19  4:11 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: alainm, mcchou, mmandlik, Howard Chung

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@chromium.org>
Reviewed-by: Manish Mandlik <mmandlik@chromium.org>
---

 Makefile.tools                 |   2 +
 client/advertisement_monitor.c | 161 +++++++++++++++++++++++++++++++++
 client/advertisement_monitor.h |  23 +++++
 client/main.c                  |  28 ++++++
 4 files changed, 214 insertions(+)
 create mode 100644 client/advertisement_monitor.c
 create mode 100644 client/advertisement_monitor.h

diff --git a/Makefile.tools b/Makefile.tools
index 9b9236609..88d9684a7 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/advertisement_monitor.h \
+					client/advertisement_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/advertisement_monitor.c b/client/advertisement_monitor.c
new file mode 100644
index 000000000..bd2309537
--- /dev/null
+++ b/client/advertisement_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 "advertisement_monitor.h"
+
+#define ADV_MONITOR_APP_PATH "/org/bluez/adv_monitor_app"
+#define ADV_MONITOR_INTERFACE "org.bluez.AdvertisementMonitor1"
+
+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");
+	} else {
+		bt_shell_printf("Failed to register path: %s\n", error.name);
+		dbus_error_free(&error);
+		return bt_shell_noninteractive_quit(EXIT_FAILURE);
+	}
+}
+
+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/advertisement_monitor.h b/client/advertisement_monitor.h
new file mode 100644
index 000000000..77b0b62c6
--- /dev/null
+++ b/client/advertisement_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 9abada69f..7ddd13aa0 100644
--- a/client/main.c
+++ b/client/main.c
@@ -41,6 +41,7 @@
 #include "agent.h"
 #include "gatt.h"
 #include "advertising.h"
+#include "advertisement_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.220.ged08abb693-goog


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [BlueZ PATCH v1 2/4] client: Implement more interfaces of ADV monitor in bluetoothctl
  2020-08-19  4:11 [BlueZ PATCH v1 1/4] client: Implement basic interface of ADV monitor in bluetoothctl Howard Chung
@ 2020-08-19  4:11 ` Howard Chung
  2020-09-08 17:06   ` Luiz Augusto von Dentz
  2020-08-19  4:11 ` [BlueZ PATCH v1 3/4] client: Expose ADV monitor objects Howard Chung
  2020-08-19  4:11 ` [BlueZ PATCH v1 4/4] core: Add AdvertisementMonitor to bluetooth.conf Howard Chung
  2 siblings, 1 reply; 8+ messages in thread
From: Howard Chung @ 2020-08-19  4:11 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: alainm, mcchou, mmandlik, Howard Chung

This patch creates a submenu in bluetoothctl and implements several
commands.

new commands:
[bluetooth]# menu advmon
[bluetooth]# add-pattern-monitor or_patterns -10,10,-30,5 1,2,ab0011
Advertisement Monitor 0 added
[bluetooth]# get-pattern-monitor all
Advertisement Monitor 0
	path: /org/bluez/adv_monitor_app/0
	type: or_patterns
	rssi:
		high threshold: -10
		high threshold timer: 10
		low threshold: -30
		low threshold timer: 5
	pattern 1:
		start position: 1
		AD data type: 2
		content: ab0011
[bluetooth]# get-supported-info
Supported Features:
Supported Moniter Types: or_patterns
[bluetooth]# remove-pattern-monitor 0
Monitor 0 deleted

Signed-off-by: Howard Chung <howardchung@google.com>
---

 client/advertisement_monitor.c | 328 ++++++++++++++++++++++++++++++++-
 client/advertisement_monitor.h |   4 +
 client/main.c                  |  70 +++++++
 3 files changed, 399 insertions(+), 3 deletions(-)

diff --git a/client/advertisement_monitor.c b/client/advertisement_monitor.c
index bd2309537..ec8f23711 100644
--- a/client/advertisement_monitor.c
+++ b/client/advertisement_monitor.c
@@ -29,6 +29,7 @@
 #include <string.h>
 
 #include "gdbus/gdbus.h"
+#include "src/shared/ad.h"
 #include "src/shared/util.h"
 #include "src/shared/shell.h"
 #include "advertisement_monitor.h"
@@ -36,6 +37,27 @@
 #define ADV_MONITOR_APP_PATH "/org/bluez/adv_monitor_app"
 #define ADV_MONITOR_INTERFACE "org.bluez.AdvertisementMonitor1"
 
+struct rssi_setting {
+	int16_t high_threshold;
+	uint16_t high_timer;
+	int16_t low_threshold;
+	uint16_t low_timer;
+};
+
+struct pattern {
+	uint8_t start_pos;
+	uint8_t ad_data_type;
+	uint8_t content_len;
+	uint8_t content[BT_AD_MAX_DATA_LEN];
+};
+
+struct adv_monitor {
+	uint8_t idx;
+	char *type;
+	struct rssi_setting *rssi;
+	GSList *patterns;
+};
+
 struct adv_monitor_manager {
 	GSList *supported_types;
 	GSList *supported_features;
@@ -43,6 +65,9 @@ struct adv_monitor_manager {
 	gboolean app_registered;
 } manager = { NULL, NULL, NULL, FALSE };
 
+static uint8_t adv_mon_idx;
+static GSList *adv_mons;
+
 static void set_supported_list(GSList **list, DBusMessageIter *iter)
 {
 	char *str;
@@ -138,7 +163,10 @@ static void unregister_reply(DBusMessage *message, void *user_data)
 
 void adv_monitor_register_app(DBusConnection *conn)
 {
-	if (manager.supported_types == NULL || manager.app_registered == TRUE ||
+	if (manager.app_registered == TRUE) {
+		bt_shell_printf("Advertisement Monitor already registered\n");
+		return bt_shell_noninteractive_quit(EXIT_FAILURE);
+	} else if (manager.supported_types == NULL ||
 		g_dbus_proxy_method_call(manager.proxy, "RegisterMonitor",
 					register_setup, register_reply,
 					NULL, NULL) == FALSE) {
@@ -150,8 +178,10 @@ void adv_monitor_register_app(DBusConnection *conn)
 
 void adv_monitor_unregister_app(DBusConnection *conn)
 {
-	if (manager.app_registered == FALSE ||
-		g_dbus_proxy_method_call(manager.proxy, "UnregisterMonitor",
+	if (manager.app_registered == FALSE) {
+		bt_shell_printf("Advertisement Monitor not registered\n");
+		return bt_shell_noninteractive_quit(EXIT_FAILURE);
+	} else if (g_dbus_proxy_method_call(manager.proxy, "UnregisterMonitor",
 					unregister_setup, unregister_reply,
 					NULL, NULL) == FALSE) {
 		bt_shell_printf("Failed to unregister Advertisement Monitor\n");
@@ -159,3 +189,295 @@ void adv_monitor_unregister_app(DBusConnection *conn)
 	}
 	manager.app_registered = FALSE;
 }
+
+static void free_pattern(void *user_data)
+{
+	struct pattern *p = user_data;
+
+	g_free(p);
+}
+
+static void free_adv_monitor(void *user_data)
+{
+	struct adv_monitor *adv_monitor = user_data;
+
+	g_free(adv_monitor->type);
+	g_free(adv_monitor->rssi);
+	g_slist_free_full(adv_monitor->patterns, free_pattern);
+	g_free(adv_monitor);
+}
+
+static uint8_t str2bytearray(char *str, uint8_t *arr)
+{
+	int idx, len = strlen(str), arr_len = 0;
+
+	if (len%2 != 0)
+		return 0;
+
+	for (idx = 0; idx < len; idx += 2) {
+		if (sscanf(str+idx, "%2hhx", &arr[arr_len++]) < 1)
+			return 0;
+	}
+	return arr_len;
+}
+
+static struct rssi_setting *parse_rssi(char *rssi_str)
+{
+	struct rssi_setting *rssi;
+	int16_t high_threshold, low_threshold;
+	uint16_t high_timer, low_timer;
+
+	if (sscanf(rssi_str, "%hd,%hu,%hd,%hu", &high_threshold, &high_timer,
+					&low_threshold, &low_timer) < 4)
+		return NULL;
+
+	rssi = g_malloc0(sizeof(struct rssi_setting));
+
+	if (!rssi) {
+		bt_shell_printf("Failed to allocate rssi_setting");
+		bt_shell_noninteractive_quit(EXIT_FAILURE);
+		return NULL;
+	}
+
+	rssi->high_threshold = high_threshold;
+	rssi->high_timer = high_timer;
+	rssi->low_threshold = low_threshold;
+	rssi->low_timer = low_timer;
+
+	return rssi;
+}
+
+static struct pattern *parse_pattern(char *pattern)
+{
+	uint8_t start_pos, ad_data_type;
+	char content_str[BT_AD_MAX_DATA_LEN];
+	struct pattern *pat;
+
+	if (sscanf(pattern, "%hhu,%hhu,%s", &start_pos, &ad_data_type,
+							content_str) < 3)
+		return NULL;
+
+	pat = g_malloc0(sizeof(struct pattern));
+
+	if (!pat) {
+		bt_shell_printf("Failed to allocate pattern");
+		bt_shell_noninteractive_quit(EXIT_FAILURE);
+		return NULL;
+	}
+
+	pat->start_pos = start_pos;
+	pat->ad_data_type = ad_data_type;
+	pat->content_len = str2bytearray(content_str, pat->content);
+	if (pat->content_len == 0) {
+		free_pattern(pat);
+		return NULL;
+	}
+
+	return pat;
+}
+
+static GSList *parse_patterns(char *pattern_list[], int num)
+{
+	GSList *patterns = NULL;
+	int cnt;
+
+	if (num == 0) {
+		bt_shell_printf("No pattern provided\n");
+		return NULL;
+	}
+
+	for (cnt = 0; cnt < num; cnt++) {
+		struct pattern *pattern;
+
+		pattern = parse_pattern(pattern_list[cnt]);
+		if (pattern == NULL) {
+			g_slist_free_full(patterns, free_pattern);
+			return NULL;
+		}
+		patterns = g_slist_append(patterns, pattern);
+	}
+
+	return patterns;
+}
+
+static gint cmp_adv_monitor_with_idx(gconstpointer a, gconstpointer b)
+{
+	const struct adv_monitor *adv_monitor = a;
+	uint8_t idx = *(uint8_t *)b;
+
+	return adv_monitor->idx != idx;
+}
+
+static struct adv_monitor *find_adv_monitor_with_idx(uint8_t monitor_idx)
+{
+	GSList *list;
+
+	list = g_slist_find_custom(adv_mons, &monitor_idx,
+						cmp_adv_monitor_with_idx);
+
+	if (list)
+		return (struct adv_monitor *)list->data;
+	return NULL;
+}
+
+static void print_bytearray(char *prefix, uint8_t *arr, uint8_t len)
+{
+	int idx;
+
+	bt_shell_printf("%s", prefix);
+	for (idx = 0; idx < len; idx++)
+		bt_shell_printf("%02hhx", arr[idx]);
+	bt_shell_printf("\n");
+}
+
+static void print_adv_monitor(struct adv_monitor *adv_monitor)
+{
+	GSList *l;
+
+	bt_shell_printf("Advertisement Monitor %d\n", adv_monitor->idx);
+	bt_shell_printf("\ttype: %s\n", adv_monitor->type);
+	if (adv_monitor->rssi) {
+		bt_shell_printf("\trssi:\n");
+		bt_shell_printf("\t\thigh threshold: %hd\n",
+					adv_monitor->rssi->high_threshold);
+		bt_shell_printf("\t\thigh threshold timer: %hu\n",
+					adv_monitor->rssi->high_timer);
+		bt_shell_printf("\t\tlow threshold: %hd\n",
+					adv_monitor->rssi->low_threshold);
+		bt_shell_printf("\t\tlow threshold timer: %hu\n",
+					adv_monitor->rssi->low_timer);
+	}
+
+	if (adv_monitor->patterns) {
+		int idx = 1;
+
+		for (l = adv_monitor->patterns; l; l = g_slist_next(l), idx++) {
+			struct pattern *pattern = l->data;
+
+			bt_shell_printf("\tpattern %d:\n", idx);
+			bt_shell_printf("\t\tstart position: %hhu\n",
+							pattern->start_pos);
+			bt_shell_printf("\t\tAD data type: %hhu\n",
+							pattern->ad_data_type);
+			print_bytearray("\t\tcontent: ", pattern->content,
+							pattern->content_len);
+		}
+	}
+}
+
+void adv_monitor_add_monitor(DBusConnection *conn, int argc, char *argv[])
+{
+	struct adv_monitor *adv_monitor;
+	struct rssi_setting *rssi;
+	GSList *patterns = NULL;
+	char *type;
+
+	if (g_slist_length(adv_mons) >= UINT8_MAX) {
+		bt_shell_printf("Number of advertisement monitor exceeds "
+				"the limit");
+		return;
+	}
+
+	while (find_adv_monitor_with_idx(adv_mon_idx))
+		adv_mon_idx += 1;
+
+	type = argv[1];
+
+	if (strcmp(argv[2], "-") == 0)
+		rssi = NULL;
+	else {
+		rssi = parse_rssi(argv[2]);
+		if (rssi == NULL) {
+			bt_shell_printf("RSSIThresholdAndTimers malformed\n");
+			return;
+		}
+	}
+
+	patterns = parse_patterns(argv+3, argc-3);
+	if (patterns == NULL) {
+		bt_shell_printf("pattern-list malformed\n");
+		return;
+	}
+
+	adv_monitor = g_malloc0(sizeof(struct adv_monitor));
+
+	if (!adv_monitor) {
+		bt_shell_printf("Failed to allocate adv_monitor");
+		return bt_shell_noninteractive_quit(EXIT_FAILURE);
+	}
+
+	adv_monitor->idx = adv_mon_idx;
+	adv_monitor->type = g_strdup(type);
+	adv_monitor->rssi = rssi;
+	adv_monitor->patterns = patterns;
+
+	adv_mons = g_slist_append(adv_mons, adv_monitor);
+	bt_shell_printf("Advertisement Monitor %d added\n", adv_monitor->idx);
+}
+
+void adv_monitor_print_monitor(DBusConnection *conn, int monitor_idx)
+{
+	struct adv_monitor *adv_monitor;
+	GSList *l;
+
+	if (monitor_idx < 0) {
+		for (l = adv_mons; l; l = g_slist_next(l)) {
+			adv_monitor = l->data;
+			print_adv_monitor(adv_monitor);
+		}
+		return;
+	}
+
+	adv_monitor = find_adv_monitor_with_idx(monitor_idx);
+
+	if (adv_monitor == NULL) {
+		bt_shell_printf("Can't find monitor with index %d\n",
+								monitor_idx);
+		return;
+	}
+
+	print_adv_monitor(adv_monitor);
+}
+
+void adv_monitor_remove_monitor(DBusConnection *conn, int monitor_idx)
+{
+	struct adv_monitor *adv_monitor;
+
+	if (monitor_idx < 0) {
+		g_slist_free_full(g_steal_pointer(&adv_mons), free_adv_monitor);
+		return;
+	}
+
+	adv_monitor = find_adv_monitor_with_idx(monitor_idx);
+	if (adv_monitor == NULL) {
+		bt_shell_printf("Can't find monitor with index %d\n",
+								monitor_idx);
+		return;
+	}
+
+	adv_mons = g_slist_remove(adv_mons, adv_monitor);
+	free_adv_monitor(adv_monitor);
+	bt_shell_printf("Monitor %d deleted\n", monitor_idx);
+}
+
+static void print_supported_list(GSList *list)
+{
+	GSList *iter;
+
+	for (iter = list; iter; iter = g_slist_next(iter)) {
+		char *data = iter->data;
+
+		printf(" %s", data);
+	}
+}
+
+void adv_monitor_get_supported_info(void)
+{
+	bt_shell_printf("Supported Features:");
+	print_supported_list(manager.supported_features);
+	bt_shell_printf("\n");
+
+	bt_shell_printf("Supported Moniter Types:");
+	print_supported_list(manager.supported_types);
+	bt_shell_printf("\n");
+}
diff --git a/client/advertisement_monitor.h b/client/advertisement_monitor.h
index 77b0b62c6..f2a0caf77 100644
--- a/client/advertisement_monitor.h
+++ b/client/advertisement_monitor.h
@@ -21,3 +21,7 @@ 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);
+void adv_monitor_add_monitor(DBusConnection *conn, int argc, char *argv[]);
+void adv_monitor_print_monitor(DBusConnection *conn, int monitor_idx);
+void adv_monitor_remove_monitor(DBusConnection *conn, int monitor_idx);
+void adv_monitor_get_supported_info(void);
diff --git a/client/main.c b/client/main.c
index 7ddd13aa0..2b63ee62a 100644
--- a/client/main.c
+++ b/client/main.c
@@ -2686,6 +2686,53 @@ static void cmd_ad_clear(int argc, char *argv[])
 		return bt_shell_noninteractive_quit(EXIT_FAILURE);
 }
 
+static void print_add_monitor_usage(void)
+{
+	bt_shell_usage();
+	bt_shell_printf("RSSIThresholdAndTimers format:\n"
+			"\t<high-rssi>,<high-timeout>,<low-rssi>,<low-timeout>\n"
+			"\tor single '-' for not using RSSI as filter\n");
+	bt_shell_printf("pattern format:\n"
+			"\t<start_position>,<ad_data_type>,<content_of_pattern>\n");
+	bt_shell_printf("e.g.\n"
+			"\tadd-pattern-monitor or_patterns -10,10,-20,20 1,2,01ab55\n");
+}
+
+static void cmd_adv_monitor_add_monitor(int argc, char *argv[])
+{
+	if (argc < 3)
+		print_add_monitor_usage();
+	else
+		adv_monitor_add_monitor(dbus_conn, argc, argv);
+}
+
+static void cmd_adv_monitor_print_monitor(int argc, char *argv[])
+{
+	int monitor_idx;
+
+	if (strcmp(argv[1], "all") == 0)
+		monitor_idx = -1;
+	else
+		monitor_idx = atoi(argv[1]);
+	adv_monitor_print_monitor(dbus_conn, monitor_idx);
+}
+
+static void cmd_adv_monitor_remove_monitor(int argc, char *argv[])
+{
+	int monitor_idx;
+
+	if (strcmp(argv[1], "all") == 0)
+		monitor_idx = -1;
+	else
+		monitor_idx = atoi(argv[1]);
+	adv_monitor_remove_monitor(dbus_conn, monitor_idx);
+}
+
+static void cmd_adv_monitor_get_supported_info(int argc, char *argv[])
+{
+	adv_monitor_get_supported_info();
+}
+
 static const struct bt_shell_menu advertise_menu = {
 	.name = "advertise",
 	.desc = "Advertise Options Submenu",
@@ -2722,6 +2769,28 @@ static const struct bt_shell_menu advertise_menu = {
 	{ } },
 };
 
+static const struct bt_shell_menu advertise_monitor_menu = {
+	.name = "advmon",
+	.desc = "Advertisement Monitor Options Submenu",
+	.entries = {
+	{ "add-pattern-monitor", "<type-of-monitor/help> "
+				"[RSSIThresholdAndTimers] "
+				"[patterns=pattern1 pattern2 ...]",
+				cmd_adv_monitor_add_monitor,
+				"Add pattern monitor" },
+	{ "get-pattern-monitor", "<monitor-id/all>",
+				cmd_adv_monitor_print_monitor,
+				"Get advertisement monitor" },
+	{ "remove-pattern-monitor", "<monitor-id/all>",
+				cmd_adv_monitor_remove_monitor,
+				"Remove advertisement monitor" },
+	{ "get-supported-info", NULL,
+				cmd_adv_monitor_get_supported_info,
+				"Get advertisement manager supported "
+				"features and supported monitor types" },
+	{ } },
+};
+
 static const struct bt_shell_menu scan_menu = {
 	.name = "scan",
 	.desc = "Scan Options Submenu",
@@ -2897,6 +2966,7 @@ int main(int argc, char *argv[])
 	bt_shell_init(argc, argv, &opt);
 	bt_shell_set_menu(&main_menu);
 	bt_shell_add_submenu(&advertise_menu);
+	bt_shell_add_submenu(&advertise_monitor_menu);
 	bt_shell_add_submenu(&scan_menu);
 	bt_shell_add_submenu(&gatt_menu);
 	bt_shell_set_prompt(PROMPT_OFF);
-- 
2.28.0.220.ged08abb693-goog


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [BlueZ PATCH v1 3/4] client: Expose ADV monitor objects
  2020-08-19  4:11 [BlueZ PATCH v1 1/4] client: Implement basic interface of ADV monitor in bluetoothctl Howard Chung
  2020-08-19  4:11 ` [BlueZ PATCH v1 2/4] client: Implement more interfaces " Howard Chung
@ 2020-08-19  4:11 ` Howard Chung
  2020-08-19  4:11 ` [BlueZ PATCH v1 4/4] core: Add AdvertisementMonitor to bluetooth.conf Howard Chung
  2 siblings, 0 replies; 8+ messages in thread
From: Howard Chung @ 2020-08-19  4:11 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: alainm, mcchou, mmandlik, Howard Chung

This adds logic to expose user-defined advertisement monitor to dbus and
also implements methods for exposed objects.

Signed-off-by: Howard Chung <howardchung@google.com>
---

 client/advertisement_monitor.c | 187 ++++++++++++++++++++++++++++++++-
 1 file changed, 183 insertions(+), 4 deletions(-)

diff --git a/client/advertisement_monitor.c b/client/advertisement_monitor.c
index ec8f23711..7ec5243e7 100644
--- a/client/advertisement_monitor.c
+++ b/client/advertisement_monitor.c
@@ -53,12 +53,13 @@ struct pattern {
 
 struct adv_monitor {
 	uint8_t idx;
+	char *path;
 	char *type;
 	struct rssi_setting *rssi;
 	GSList *patterns;
 };
 
-struct adv_monitor_manager {
+static struct adv_monitor_manager {
 	GSList *supported_types;
 	GSList *supported_features;
 	GDBusProxy *proxy;
@@ -68,6 +69,163 @@ struct adv_monitor_manager {
 static uint8_t adv_mon_idx;
 static GSList *adv_mons;
 
+static void remove_adv_monitor(void *data, void *user_data);
+
+static DBusMessage *release_adv_monitor(DBusConnection *conn,
+					DBusMessage *msg, void *user_data)
+{
+	struct adv_monitor *adv_monitor = user_data;
+
+	bt_shell_printf("Advertisement monitor %d released\n",
+							adv_monitor->idx);
+	remove_adv_monitor(adv_monitor, conn);
+
+	return dbus_message_new_method_return(msg);
+}
+
+static DBusMessage *activate_adv_monitor(DBusConnection *conn,
+					DBusMessage *msg, void *user_data)
+{
+	struct adv_monitor *adv_monitor = user_data;
+
+	bt_shell_printf("Advertisement monitor %d activated\n",
+							adv_monitor->idx);
+	return dbus_message_new_method_return(msg);
+}
+
+static DBusMessage *device_found_adv_monitor(DBusConnection *conn,
+					DBusMessage *msg, void *user_data)
+{
+	struct adv_monitor *adv_monitor = user_data;
+	const char *device;
+
+	dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &device,
+							DBUS_TYPE_INVALID);
+	bt_shell_printf("Advertisement monitor %d found device %s\n",
+						adv_monitor->idx, device);
+	return dbus_message_new_method_return(msg);
+}
+
+static DBusMessage *device_lost_adv_monitor(DBusConnection *conn,
+					DBusMessage *msg, void *user_data)
+{
+	struct adv_monitor *adv_monitor = user_data;
+	const char *device;
+
+	dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &device,
+							DBUS_TYPE_INVALID);
+	bt_shell_printf("Advertisement monitor %d lost device %s\n",
+						adv_monitor->idx, device);
+	return dbus_message_new_method_return(msg);
+}
+
+static const GDBusMethodTable adv_monitor_methods[] = {
+	{ GDBUS_ASYNC_METHOD("Release", NULL, NULL, release_adv_monitor) },
+	{ GDBUS_ASYNC_METHOD("Activate", NULL, NULL, activate_adv_monitor) },
+	{ GDBUS_ASYNC_METHOD("DeviceFound", GDBUS_ARGS({ "device", "o" }),
+			NULL, device_found_adv_monitor) },
+	{ GDBUS_ASYNC_METHOD("DeviceLost", GDBUS_ARGS({ "device", "o" }),
+			NULL, device_lost_adv_monitor) },
+	{ }
+};
+
+
+static gboolean get_type(const GDBusPropertyTable *property,
+				DBusMessageIter *iter, void *user_data)
+{
+	struct adv_monitor *adv_monitor = user_data;
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING,
+							&adv_monitor->type);
+	return TRUE;
+}
+
+static gboolean get_rssi(const GDBusPropertyTable *property,
+				DBusMessageIter *iter, void *user_data)
+{
+	struct adv_monitor *adv_monitor = user_data;
+	struct rssi_setting *rssi = adv_monitor->rssi;
+	DBusMessageIter data_iter;
+
+	dbus_message_iter_open_container(iter, DBUS_TYPE_STRUCT,
+							NULL, &data_iter);
+	dbus_message_iter_append_basic(&data_iter, DBUS_TYPE_INT16,
+							&rssi->high_threshold);
+	dbus_message_iter_append_basic(&data_iter, DBUS_TYPE_UINT16,
+							&rssi->high_timer);
+	dbus_message_iter_append_basic(&data_iter, DBUS_TYPE_INT16,
+							&rssi->low_threshold);
+	dbus_message_iter_append_basic(&data_iter, DBUS_TYPE_UINT16,
+							&rssi->low_timer);
+	dbus_message_iter_close_container(iter, &data_iter);
+	return TRUE;
+}
+
+static gboolean rssi_exists(const GDBusPropertyTable *property, void *data)
+{
+	struct adv_monitor *adv_monitor = data;
+
+	return adv_monitor->rssi != NULL;
+}
+
+static void append_pattern_content_to_dbus(DBusMessageIter *iter,
+							struct pattern *pattern)
+{
+	DBusMessageIter data_iter;
+	int idx;
+
+	dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY,
+					DBUS_TYPE_BYTE_AS_STRING, &data_iter);
+	for (idx = 0; idx < pattern->content_len; idx++)
+		dbus_message_iter_append_basic(&data_iter, DBUS_TYPE_BYTE,
+							&pattern->content[idx]);
+	dbus_message_iter_close_container(iter, &data_iter);
+}
+
+static void append_pattern_to_dbus(void *data, void *user_data)
+{
+	struct pattern *pattern = data;
+	DBusMessageIter *array_iter = user_data;
+	DBusMessageIter data_iter;
+
+	dbus_message_iter_open_container(array_iter, DBUS_TYPE_STRUCT,
+							NULL, &data_iter);
+	dbus_message_iter_append_basic(&data_iter, DBUS_TYPE_BYTE,
+							&pattern->start_pos);
+	dbus_message_iter_append_basic(&data_iter, DBUS_TYPE_BYTE,
+							&pattern->ad_data_type);
+	append_pattern_content_to_dbus(&data_iter, pattern);
+	dbus_message_iter_close_container(array_iter, &data_iter);
+}
+
+static gboolean get_patterns(const GDBusPropertyTable *property,
+				DBusMessageIter *iter, void *user_data)
+{
+	struct adv_monitor *adv_monitor = user_data;
+	DBusMessageIter array_iter;
+
+	dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY, "(yyay)",
+								&array_iter);
+	g_slist_foreach(adv_monitor->patterns, append_pattern_to_dbus,
+								&array_iter);
+	dbus_message_iter_close_container(iter, &array_iter);
+	return TRUE;
+}
+
+static gboolean pattern_exists(const GDBusPropertyTable *property, void *data)
+{
+	struct adv_monitor *adv_monitor = data;
+
+	return adv_monitor->patterns != NULL;
+}
+
+static const GDBusPropertyTable adv_monitor_props[] = {
+	{ "Type", "s", get_type },
+	{ "RSSIThresholdsAndTimers", "(nqnq)", get_rssi, NULL, rssi_exists },
+	{ "Patterns", "a(yyay)", get_patterns, NULL, pattern_exists },
+	{ }
+};
+
 static void set_supported_list(GSList **list, DBusMessageIter *iter)
 {
 	char *str;
@@ -201,6 +359,7 @@ static void free_adv_monitor(void *user_data)
 {
 	struct adv_monitor *adv_monitor = user_data;
 
+	g_free(adv_monitor->path);
 	g_free(adv_monitor->type);
 	g_free(adv_monitor->rssi);
 	g_slist_free_full(adv_monitor->patterns, free_pattern);
@@ -300,6 +459,16 @@ static GSList *parse_patterns(char *pattern_list[], int num)
 	return patterns;
 }
 
+static void remove_adv_monitor(void *data, void *user_data)
+{
+	struct adv_monitor *adv_monitor = data;
+	DBusConnection *conn = user_data;
+
+	adv_mons = g_slist_remove(adv_mons, adv_monitor);
+	g_dbus_unregister_interface(conn, adv_monitor->path,
+							ADV_MONITOR_INTERFACE);
+}
+
 static gint cmp_adv_monitor_with_idx(gconstpointer a, gconstpointer b)
 {
 	const struct adv_monitor *adv_monitor = a;
@@ -335,6 +504,7 @@ static void print_adv_monitor(struct adv_monitor *adv_monitor)
 	GSList *l;
 
 	bt_shell_printf("Advertisement Monitor %d\n", adv_monitor->idx);
+	bt_shell_printf("\tpath: %s\n", adv_monitor->path);
 	bt_shell_printf("\ttype: %s\n", adv_monitor->type);
 	if (adv_monitor->rssi) {
 		bt_shell_printf("\trssi:\n");
@@ -410,6 +580,16 @@ void adv_monitor_add_monitor(DBusConnection *conn, int argc, char *argv[])
 	adv_monitor->type = g_strdup(type);
 	adv_monitor->rssi = rssi;
 	adv_monitor->patterns = patterns;
+	adv_monitor->path = g_strdup_printf("%s/%hhu", ADV_MONITOR_APP_PATH,
+								adv_mon_idx);
+	if (g_dbus_register_interface(conn, adv_monitor->path,
+					ADV_MONITOR_INTERFACE,
+					adv_monitor_methods, NULL,
+					adv_monitor_props, adv_monitor,
+					free_adv_monitor) == FALSE) {
+		bt_shell_printf("Failed to register advertisement monitor\n");
+		return bt_shell_noninteractive_quit(EXIT_FAILURE);
+	}
 
 	adv_mons = g_slist_append(adv_mons, adv_monitor);
 	bt_shell_printf("Advertisement Monitor %d added\n", adv_monitor->idx);
@@ -444,7 +624,7 @@ void adv_monitor_remove_monitor(DBusConnection *conn, int monitor_idx)
 	struct adv_monitor *adv_monitor;
 
 	if (monitor_idx < 0) {
-		g_slist_free_full(g_steal_pointer(&adv_mons), free_adv_monitor);
+		g_slist_foreach(adv_mons, remove_adv_monitor, conn);
 		return;
 	}
 
@@ -455,8 +635,7 @@ void adv_monitor_remove_monitor(DBusConnection *conn, int monitor_idx)
 		return;
 	}
 
-	adv_mons = g_slist_remove(adv_mons, adv_monitor);
-	free_adv_monitor(adv_monitor);
+	remove_adv_monitor(adv_monitor, conn);
 	bt_shell_printf("Monitor %d deleted\n", monitor_idx);
 }
 
-- 
2.28.0.220.ged08abb693-goog


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [BlueZ PATCH v1 4/4] core: Add AdvertisementMonitor to bluetooth.conf
  2020-08-19  4:11 [BlueZ PATCH v1 1/4] client: Implement basic interface of ADV monitor in bluetoothctl Howard Chung
  2020-08-19  4:11 ` [BlueZ PATCH v1 2/4] client: Implement more interfaces " Howard Chung
  2020-08-19  4:11 ` [BlueZ PATCH v1 3/4] client: Expose ADV monitor objects Howard Chung
@ 2020-08-19  4:11 ` Howard Chung
  2 siblings, 0 replies; 8+ messages in thread
From: Howard Chung @ 2020-08-19  4:11 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: alainm, mcchou, mmandlik, Howard Chung

AdvertisementMonitor must be included in bluetooth.conf in order to
be able to call Release

Signed-off-by: Howard Chung <howardchung@google.com>
---

 src/bluetooth.conf | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/bluetooth.conf b/src/bluetooth.conf
index 8a1e25801..b6c614908 100644
--- a/src/bluetooth.conf
+++ b/src/bluetooth.conf
@@ -10,6 +10,7 @@
   <policy user="root">
     <allow own="org.bluez"/>
     <allow send_destination="org.bluez"/>
+    <allow send_interface="org.bluez.AdvertisementMonitor1"/>
     <allow send_interface="org.bluez.Agent1"/>
     <allow send_interface="org.bluez.MediaEndpoint1"/>
     <allow send_interface="org.bluez.MediaPlayer1"/>
-- 
2.28.0.220.ged08abb693-goog


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [BlueZ PATCH v1 2/4] client: Implement more interfaces of ADV monitor in bluetoothctl
  2020-08-19  4:11 ` [BlueZ PATCH v1 2/4] client: Implement more interfaces " Howard Chung
@ 2020-09-08 17:06   ` Luiz Augusto von Dentz
  2020-09-09  4:01     ` Yun-hao Chung
  0 siblings, 1 reply; 8+ messages in thread
From: Luiz Augusto von Dentz @ 2020-09-08 17:06 UTC (permalink / raw)
  To: Howard Chung
  Cc: linux-bluetooth, Alain Michaud, Miao-chen Chou, Manish Mandlik

Hi Howard,

On Tue, Aug 18, 2020 at 9:15 PM Howard Chung <howardchung@google.com> wrote:
>
> This patch creates a submenu in bluetoothctl and implements several
> commands.
>
> new commands:
> [bluetooth]# menu advmon
> [bluetooth]# add-pattern-monitor or_patterns -10,10,-30,5 1,2,ab0011
> Advertisement Monitor 0 added
> [bluetooth]# get-pattern-monitor all
> Advertisement Monitor 0
>         path: /org/bluez/adv_monitor_app/0
>         type: or_patterns
>         rssi:
>                 high threshold: -10
>                 high threshold timer: 10
>                 low threshold: -30
>                 low threshold timer: 5
>         pattern 1:
>                 start position: 1
>                 AD data type: 2
>                 content: ab0011
> [bluetooth]# get-supported-info
> Supported Features:
> Supported Moniter Types: or_patterns
> [bluetooth]# remove-pattern-monitor 0
> Monitor 0 deleted
>
> Signed-off-by: Howard Chung <howardchung@google.com>
> ---
>
>  client/advertisement_monitor.c | 328 ++++++++++++++++++++++++++++++++-
>  client/advertisement_monitor.h |   4 +
>  client/main.c                  |  70 +++++++
>  3 files changed, 399 insertions(+), 3 deletions(-)
>
> diff --git a/client/advertisement_monitor.c b/client/advertisement_monitor.c
> index bd2309537..ec8f23711 100644
> --- a/client/advertisement_monitor.c
> +++ b/client/advertisement_monitor.c
> @@ -29,6 +29,7 @@
>  #include <string.h>
>
>  #include "gdbus/gdbus.h"
> +#include "src/shared/ad.h"
>  #include "src/shared/util.h"
>  #include "src/shared/shell.h"
>  #include "advertisement_monitor.h"
> @@ -36,6 +37,27 @@
>  #define ADV_MONITOR_APP_PATH "/org/bluez/adv_monitor_app"
>  #define ADV_MONITOR_INTERFACE "org.bluez.AdvertisementMonitor1"
>
> +struct rssi_setting {
> +       int16_t high_threshold;
> +       uint16_t high_timer;
> +       int16_t low_threshold;
> +       uint16_t low_timer;
> +};
> +
> +struct pattern {
> +       uint8_t start_pos;
> +       uint8_t ad_data_type;
> +       uint8_t content_len;
> +       uint8_t content[BT_AD_MAX_DATA_LEN];
> +};
> +
> +struct adv_monitor {
> +       uint8_t idx;
> +       char *type;
> +       struct rssi_setting *rssi;
> +       GSList *patterns;
> +};
> +
>  struct adv_monitor_manager {
>         GSList *supported_types;
>         GSList *supported_features;
> @@ -43,6 +65,9 @@ struct adv_monitor_manager {
>         gboolean app_registered;
>  } manager = { NULL, NULL, NULL, FALSE };
>
> +static uint8_t adv_mon_idx;
> +static GSList *adv_mons;
> +
>  static void set_supported_list(GSList **list, DBusMessageIter *iter)
>  {
>         char *str;
> @@ -138,7 +163,10 @@ static void unregister_reply(DBusMessage *message, void *user_data)
>
>  void adv_monitor_register_app(DBusConnection *conn)
>  {
> -       if (manager.supported_types == NULL || manager.app_registered == TRUE ||
> +       if (manager.app_registered == TRUE) {
> +               bt_shell_printf("Advertisement Monitor already registered\n");
> +               return bt_shell_noninteractive_quit(EXIT_FAILURE);
> +       } else if (manager.supported_types == NULL ||
>                 g_dbus_proxy_method_call(manager.proxy, "RegisterMonitor",
>                                         register_setup, register_reply,
>                                         NULL, NULL) == FALSE) {
> @@ -150,8 +178,10 @@ void adv_monitor_register_app(DBusConnection *conn)
>
>  void adv_monitor_unregister_app(DBusConnection *conn)
>  {
> -       if (manager.app_registered == FALSE ||
> -               g_dbus_proxy_method_call(manager.proxy, "UnregisterMonitor",
> +       if (manager.app_registered == FALSE) {
> +               bt_shell_printf("Advertisement Monitor not registered\n");
> +               return bt_shell_noninteractive_quit(EXIT_FAILURE);
> +       } else if (g_dbus_proxy_method_call(manager.proxy, "UnregisterMonitor",
>                                         unregister_setup, unregister_reply,
>                                         NULL, NULL) == FALSE) {
>                 bt_shell_printf("Failed to unregister Advertisement Monitor\n");
> @@ -159,3 +189,295 @@ void adv_monitor_unregister_app(DBusConnection *conn)
>         }
>         manager.app_registered = FALSE;
>  }
> +
> +static void free_pattern(void *user_data)
> +{
> +       struct pattern *p = user_data;
> +
> +       g_free(p);
> +}
> +
> +static void free_adv_monitor(void *user_data)
> +{
> +       struct adv_monitor *adv_monitor = user_data;
> +
> +       g_free(adv_monitor->type);
> +       g_free(adv_monitor->rssi);
> +       g_slist_free_full(adv_monitor->patterns, free_pattern);
> +       g_free(adv_monitor);
> +}
> +
> +static uint8_t str2bytearray(char *str, uint8_t *arr)
> +{
> +       int idx, len = strlen(str), arr_len = 0;
> +
> +       if (len%2 != 0)
> +               return 0;
> +
> +       for (idx = 0; idx < len; idx += 2) {
> +               if (sscanf(str+idx, "%2hhx", &arr[arr_len++]) < 1)
> +                       return 0;
> +       }
> +       return arr_len;
> +}
> +
> +static struct rssi_setting *parse_rssi(char *rssi_str)
> +{
> +       struct rssi_setting *rssi;
> +       int16_t high_threshold, low_threshold;
> +       uint16_t high_timer, low_timer;
> +
> +       if (sscanf(rssi_str, "%hd,%hu,%hd,%hu", &high_threshold, &high_timer,
> +                                       &low_threshold, &low_timer) < 4)
> +               return NULL;
> +
> +       rssi = g_malloc0(sizeof(struct rssi_setting));
> +
> +       if (!rssi) {
> +               bt_shell_printf("Failed to allocate rssi_setting");
> +               bt_shell_noninteractive_quit(EXIT_FAILURE);
> +               return NULL;
> +       }
> +
> +       rssi->high_threshold = high_threshold;
> +       rssi->high_timer = high_timer;
> +       rssi->low_threshold = low_threshold;
> +       rssi->low_timer = low_timer;
> +
> +       return rssi;
> +}
> +
> +static struct pattern *parse_pattern(char *pattern)
> +{
> +       uint8_t start_pos, ad_data_type;
> +       char content_str[BT_AD_MAX_DATA_LEN];
> +       struct pattern *pat;
> +
> +       if (sscanf(pattern, "%hhu,%hhu,%s", &start_pos, &ad_data_type,
> +                                                       content_str) < 3)
> +               return NULL;
> +
> +       pat = g_malloc0(sizeof(struct pattern));
> +
> +       if (!pat) {
> +               bt_shell_printf("Failed to allocate pattern");
> +               bt_shell_noninteractive_quit(EXIT_FAILURE);
> +               return NULL;
> +       }
> +
> +       pat->start_pos = start_pos;
> +       pat->ad_data_type = ad_data_type;
> +       pat->content_len = str2bytearray(content_str, pat->content);
> +       if (pat->content_len == 0) {
> +               free_pattern(pat);
> +               return NULL;
> +       }
> +
> +       return pat;
> +}
> +
> +static GSList *parse_patterns(char *pattern_list[], int num)
> +{
> +       GSList *patterns = NULL;
> +       int cnt;
> +
> +       if (num == 0) {
> +               bt_shell_printf("No pattern provided\n");
> +               return NULL;
> +       }
> +
> +       for (cnt = 0; cnt < num; cnt++) {
> +               struct pattern *pattern;
> +
> +               pattern = parse_pattern(pattern_list[cnt]);
> +               if (pattern == NULL) {
> +                       g_slist_free_full(patterns, free_pattern);
> +                       return NULL;
> +               }
> +               patterns = g_slist_append(patterns, pattern);
> +       }
> +
> +       return patterns;
> +}
> +
> +static gint cmp_adv_monitor_with_idx(gconstpointer a, gconstpointer b)
> +{
> +       const struct adv_monitor *adv_monitor = a;
> +       uint8_t idx = *(uint8_t *)b;
> +
> +       return adv_monitor->idx != idx;
> +}
> +
> +static struct adv_monitor *find_adv_monitor_with_idx(uint8_t monitor_idx)
> +{
> +       GSList *list;
> +
> +       list = g_slist_find_custom(adv_mons, &monitor_idx,
> +                                               cmp_adv_monitor_with_idx);
> +
> +       if (list)
> +               return (struct adv_monitor *)list->data;
> +       return NULL;
> +}
> +
> +static void print_bytearray(char *prefix, uint8_t *arr, uint8_t len)
> +{
> +       int idx;
> +
> +       bt_shell_printf("%s", prefix);
> +       for (idx = 0; idx < len; idx++)
> +               bt_shell_printf("%02hhx", arr[idx]);
> +       bt_shell_printf("\n");
> +}
> +
> +static void print_adv_monitor(struct adv_monitor *adv_monitor)
> +{
> +       GSList *l;
> +
> +       bt_shell_printf("Advertisement Monitor %d\n", adv_monitor->idx);
> +       bt_shell_printf("\ttype: %s\n", adv_monitor->type);
> +       if (adv_monitor->rssi) {
> +               bt_shell_printf("\trssi:\n");
> +               bt_shell_printf("\t\thigh threshold: %hd\n",
> +                                       adv_monitor->rssi->high_threshold);
> +               bt_shell_printf("\t\thigh threshold timer: %hu\n",
> +                                       adv_monitor->rssi->high_timer);
> +               bt_shell_printf("\t\tlow threshold: %hd\n",
> +                                       adv_monitor->rssi->low_threshold);
> +               bt_shell_printf("\t\tlow threshold timer: %hu\n",
> +                                       adv_monitor->rssi->low_timer);
> +       }
> +
> +       if (adv_monitor->patterns) {
> +               int idx = 1;
> +
> +               for (l = adv_monitor->patterns; l; l = g_slist_next(l), idx++) {
> +                       struct pattern *pattern = l->data;
> +
> +                       bt_shell_printf("\tpattern %d:\n", idx);
> +                       bt_shell_printf("\t\tstart position: %hhu\n",
> +                                                       pattern->start_pos);
> +                       bt_shell_printf("\t\tAD data type: %hhu\n",
> +                                                       pattern->ad_data_type);
> +                       print_bytearray("\t\tcontent: ", pattern->content,
> +                                                       pattern->content_len);
> +               }
> +       }
> +}
> +
> +void adv_monitor_add_monitor(DBusConnection *conn, int argc, char *argv[])
> +{
> +       struct adv_monitor *adv_monitor;
> +       struct rssi_setting *rssi;
> +       GSList *patterns = NULL;
> +       char *type;
> +
> +       if (g_slist_length(adv_mons) >= UINT8_MAX) {
> +               bt_shell_printf("Number of advertisement monitor exceeds "
> +                               "the limit");
> +               return;
> +       }
> +
> +       while (find_adv_monitor_with_idx(adv_mon_idx))
> +               adv_mon_idx += 1;
> +
> +       type = argv[1];
> +
> +       if (strcmp(argv[2], "-") == 0)
> +               rssi = NULL;
> +       else {
> +               rssi = parse_rssi(argv[2]);
> +               if (rssi == NULL) {
> +                       bt_shell_printf("RSSIThresholdAndTimers malformed\n");
> +                       return;
> +               }
> +       }
> +
> +       patterns = parse_patterns(argv+3, argc-3);
> +       if (patterns == NULL) {
> +               bt_shell_printf("pattern-list malformed\n");
> +               return;
> +       }
> +
> +       adv_monitor = g_malloc0(sizeof(struct adv_monitor));
> +
> +       if (!adv_monitor) {
> +               bt_shell_printf("Failed to allocate adv_monitor");
> +               return bt_shell_noninteractive_quit(EXIT_FAILURE);
> +       }
> +
> +       adv_monitor->idx = adv_mon_idx;
> +       adv_monitor->type = g_strdup(type);
> +       adv_monitor->rssi = rssi;
> +       adv_monitor->patterns = patterns;
> +
> +       adv_mons = g_slist_append(adv_mons, adv_monitor);
> +       bt_shell_printf("Advertisement Monitor %d added\n", adv_monitor->idx);
> +}
> +
> +void adv_monitor_print_monitor(DBusConnection *conn, int monitor_idx)
> +{
> +       struct adv_monitor *adv_monitor;
> +       GSList *l;
> +
> +       if (monitor_idx < 0) {
> +               for (l = adv_mons; l; l = g_slist_next(l)) {
> +                       adv_monitor = l->data;
> +                       print_adv_monitor(adv_monitor);
> +               }
> +               return;
> +       }
> +
> +       adv_monitor = find_adv_monitor_with_idx(monitor_idx);
> +
> +       if (adv_monitor == NULL) {
> +               bt_shell_printf("Can't find monitor with index %d\n",
> +                                                               monitor_idx);
> +               return;
> +       }
> +
> +       print_adv_monitor(adv_monitor);
> +}
> +
> +void adv_monitor_remove_monitor(DBusConnection *conn, int monitor_idx)
> +{
> +       struct adv_monitor *adv_monitor;
> +
> +       if (monitor_idx < 0) {
> +               g_slist_free_full(g_steal_pointer(&adv_mons), free_adv_monitor);
> +               return;
> +       }
> +
> +       adv_monitor = find_adv_monitor_with_idx(monitor_idx);
> +       if (adv_monitor == NULL) {
> +               bt_shell_printf("Can't find monitor with index %d\n",
> +                                                               monitor_idx);
> +               return;
> +       }
> +
> +       adv_mons = g_slist_remove(adv_mons, adv_monitor);
> +       free_adv_monitor(adv_monitor);
> +       bt_shell_printf("Monitor %d deleted\n", monitor_idx);
> +}
> +
> +static void print_supported_list(GSList *list)
> +{
> +       GSList *iter;
> +
> +       for (iter = list; iter; iter = g_slist_next(iter)) {
> +               char *data = iter->data;
> +
> +               printf(" %s", data);
> +       }
> +}
> +
> +void adv_monitor_get_supported_info(void)
> +{
> +       bt_shell_printf("Supported Features:");
> +       print_supported_list(manager.supported_features);
> +       bt_shell_printf("\n");
> +
> +       bt_shell_printf("Supported Moniter Types:");
> +       print_supported_list(manager.supported_types);
> +       bt_shell_printf("\n");
> +}
> diff --git a/client/advertisement_monitor.h b/client/advertisement_monitor.h
> index 77b0b62c6..f2a0caf77 100644
> --- a/client/advertisement_monitor.h
> +++ b/client/advertisement_monitor.h
> @@ -21,3 +21,7 @@ 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);
> +void adv_monitor_add_monitor(DBusConnection *conn, int argc, char *argv[]);
> +void adv_monitor_print_monitor(DBusConnection *conn, int monitor_idx);
> +void adv_monitor_remove_monitor(DBusConnection *conn, int monitor_idx);
> +void adv_monitor_get_supported_info(void);
> diff --git a/client/main.c b/client/main.c
> index 7ddd13aa0..2b63ee62a 100644
> --- a/client/main.c
> +++ b/client/main.c
> @@ -2686,6 +2686,53 @@ static void cmd_ad_clear(int argc, char *argv[])
>                 return bt_shell_noninteractive_quit(EXIT_FAILURE);
>  }
>
> +static void print_add_monitor_usage(void)
> +{
> +       bt_shell_usage();
> +       bt_shell_printf("RSSIThresholdAndTimers format:\n"
> +                       "\t<high-rssi>,<high-timeout>,<low-rssi>,<low-timeout>\n"
> +                       "\tor single '-' for not using RSSI as filter\n");
> +       bt_shell_printf("pattern format:\n"
> +                       "\t<start_position>,<ad_data_type>,<content_of_pattern>\n");
> +       bt_shell_printf("e.g.\n"
> +                       "\tadd-pattern-monitor or_patterns -10,10,-20,20 1,2,01ab55\n");

btshell parameters are space separated not comma.

> +}
> +
> +static void cmd_adv_monitor_add_monitor(int argc, char *argv[])
> +{
> +       if (argc < 3)
> +               print_add_monitor_usage();
> +       else
> +               adv_monitor_add_monitor(dbus_conn, argc, argv);
> +}
> +
> +static void cmd_adv_monitor_print_monitor(int argc, char *argv[])
> +{
> +       int monitor_idx;
> +
> +       if (strcmp(argv[1], "all") == 0)
> +               monitor_idx = -1;
> +       else
> +               monitor_idx = atoi(argv[1]);
> +       adv_monitor_print_monitor(dbus_conn, monitor_idx);
> +}
> +
> +static void cmd_adv_monitor_remove_monitor(int argc, char *argv[])
> +{
> +       int monitor_idx;
> +
> +       if (strcmp(argv[1], "all") == 0)
> +               monitor_idx = -1;
> +       else
> +               monitor_idx = atoi(argv[1]);
> +       adv_monitor_remove_monitor(dbus_conn, monitor_idx);
> +}
> +
> +static void cmd_adv_monitor_get_supported_info(int argc, char *argv[])
> +{
> +       adv_monitor_get_supported_info();
> +}
> +
>  static const struct bt_shell_menu advertise_menu = {
>         .name = "advertise",
>         .desc = "Advertise Options Submenu",
> @@ -2722,6 +2769,28 @@ static const struct bt_shell_menu advertise_menu = {
>         { } },
>  };
>
> +static const struct bt_shell_menu advertise_monitor_menu = {
> +       .name = "advmon",

I'd use monitor instead of advmon here.

> +       .desc = "Advertisement Monitor Options Submenu",
> +       .entries = {
> +       { "add-pattern-monitor", "<type-of-monitor/help> "
> +                               "[RSSIThresholdAndTimers] "
> +                               "[patterns=pattern1 pattern2 ...]",
> +                               cmd_adv_monitor_add_monitor,
> +                               "Add pattern monitor" },
> +       { "get-pattern-monitor", "<monitor-id/all>",
> +                               cmd_adv_monitor_print_monitor,
> +                               "Get advertisement monitor" },
> +       { "remove-pattern-monitor", "<monitor-id/all>",
> +                               cmd_adv_monitor_remove_monitor,
> +                               "Remove advertisement monitor" },
> +       { "get-supported-info", NULL,
> +                               cmd_adv_monitor_get_supported_info,
> +                               "Get advertisement manager supported "
> +                               "features and supported monitor types" },

We could probably drop the monitor from the commands here which should
leave us at:

add-pattern
get-pattern
remote-pattern
supported-info

> +       { } },
> +};
> +
>  static const struct bt_shell_menu scan_menu = {
>         .name = "scan",
>         .desc = "Scan Options Submenu",
> @@ -2897,6 +2966,7 @@ int main(int argc, char *argv[])
>         bt_shell_init(argc, argv, &opt);
>         bt_shell_set_menu(&main_menu);
>         bt_shell_add_submenu(&advertise_menu);
> +       bt_shell_add_submenu(&advertise_monitor_menu);
>         bt_shell_add_submenu(&scan_menu);
>         bt_shell_add_submenu(&gatt_menu);
>         bt_shell_set_prompt(PROMPT_OFF);
> --
> 2.28.0.220.ged08abb693-goog
>


-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [BlueZ PATCH v1 2/4] client: Implement more interfaces of ADV monitor in bluetoothctl
  2020-09-08 17:06   ` Luiz Augusto von Dentz
@ 2020-09-09  4:01     ` Yun-hao Chung
  2020-09-09  4:56       ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 8+ messages in thread
From: Yun-hao Chung @ 2020-09-09  4:01 UTC (permalink / raw)
  To: Luiz Augusto von Dentz
  Cc: linux-bluetooth, Alain Michaud, Miao-chen Chou, Manish Mandlik

Hi Luiz,

Thanks for the comments.


On Wed, Sep 9, 2020 at 1:06 AM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Howard,
>
> On Tue, Aug 18, 2020 at 9:15 PM Howard Chung <howardchung@google.com> wrote:
> >
> > This patch creates a submenu in bluetoothctl and implements several
> > commands.
> >
> > new commands:
> > [bluetooth]# menu advmon
> > [bluetooth]# add-pattern-monitor or_patterns -10,10,-30,5 1,2,ab0011
> > Advertisement Monitor 0 added
> > [bluetooth]# get-pattern-monitor all
> > Advertisement Monitor 0
> >         path: /org/bluez/adv_monitor_app/0
> >         type: or_patterns
> >         rssi:
> >                 high threshold: -10
> >                 high threshold timer: 10
> >                 low threshold: -30
> >                 low threshold timer: 5
> >         pattern 1:
> >                 start position: 1
> >                 AD data type: 2
> >                 content: ab0011
> > [bluetooth]# get-supported-info
> > Supported Features:
> > Supported Moniter Types: or_patterns
> > [bluetooth]# remove-pattern-monitor 0
> > Monitor 0 deleted
> >
> > Signed-off-by: Howard Chung <howardchung@google.com>
> > ---
> >
> >  client/advertisement_monitor.c | 328 ++++++++++++++++++++++++++++++++-
> >  client/advertisement_monitor.h |   4 +
> >  client/main.c                  |  70 +++++++
> >  3 files changed, 399 insertions(+), 3 deletions(-)
> >
> > diff --git a/client/advertisement_monitor.c b/client/advertisement_monitor.c
> > index bd2309537..ec8f23711 100644
> > --- a/client/advertisement_monitor.c
> > +++ b/client/advertisement_monitor.c
> > @@ -29,6 +29,7 @@
> >  #include <string.h>
> >
> >  #include "gdbus/gdbus.h"
> > +#include "src/shared/ad.h"
> >  #include "src/shared/util.h"
> >  #include "src/shared/shell.h"
> >  #include "advertisement_monitor.h"
> > @@ -36,6 +37,27 @@
> >  #define ADV_MONITOR_APP_PATH "/org/bluez/adv_monitor_app"
> >  #define ADV_MONITOR_INTERFACE "org.bluez.AdvertisementMonitor1"
> >
> > +struct rssi_setting {
> > +       int16_t high_threshold;
> > +       uint16_t high_timer;
> > +       int16_t low_threshold;
> > +       uint16_t low_timer;
> > +};
> > +
> > +struct pattern {
> > +       uint8_t start_pos;
> > +       uint8_t ad_data_type;
> > +       uint8_t content_len;
> > +       uint8_t content[BT_AD_MAX_DATA_LEN];
> > +};
> > +
> > +struct adv_monitor {
> > +       uint8_t idx;
> > +       char *type;
> > +       struct rssi_setting *rssi;
> > +       GSList *patterns;
> > +};
> > +
> >  struct adv_monitor_manager {
> >         GSList *supported_types;
> >         GSList *supported_features;
> > @@ -43,6 +65,9 @@ struct adv_monitor_manager {
> >         gboolean app_registered;
> >  } manager = { NULL, NULL, NULL, FALSE };
> >
> > +static uint8_t adv_mon_idx;
> > +static GSList *adv_mons;
> > +
> >  static void set_supported_list(GSList **list, DBusMessageIter *iter)
> >  {
> >         char *str;
> > @@ -138,7 +163,10 @@ static void unregister_reply(DBusMessage *message, void *user_data)
> >
> >  void adv_monitor_register_app(DBusConnection *conn)
> >  {
> > -       if (manager.supported_types == NULL || manager.app_registered == TRUE ||
> > +       if (manager.app_registered == TRUE) {
> > +               bt_shell_printf("Advertisement Monitor already registered\n");
> > +               return bt_shell_noninteractive_quit(EXIT_FAILURE);
> > +       } else if (manager.supported_types == NULL ||
> >                 g_dbus_proxy_method_call(manager.proxy, "RegisterMonitor",
> >                                         register_setup, register_reply,
> >                                         NULL, NULL) == FALSE) {
> > @@ -150,8 +178,10 @@ void adv_monitor_register_app(DBusConnection *conn)
> >
> >  void adv_monitor_unregister_app(DBusConnection *conn)
> >  {
> > -       if (manager.app_registered == FALSE ||
> > -               g_dbus_proxy_method_call(manager.proxy, "UnregisterMonitor",
> > +       if (manager.app_registered == FALSE) {
> > +               bt_shell_printf("Advertisement Monitor not registered\n");
> > +               return bt_shell_noninteractive_quit(EXIT_FAILURE);
> > +       } else if (g_dbus_proxy_method_call(manager.proxy, "UnregisterMonitor",
> >                                         unregister_setup, unregister_reply,
> >                                         NULL, NULL) == FALSE) {
> >                 bt_shell_printf("Failed to unregister Advertisement Monitor\n");
> > @@ -159,3 +189,295 @@ void adv_monitor_unregister_app(DBusConnection *conn)
> >         }
> >         manager.app_registered = FALSE;
> >  }
> > +
> > +static void free_pattern(void *user_data)
> > +{
> > +       struct pattern *p = user_data;
> > +
> > +       g_free(p);
> > +}
> > +
> > +static void free_adv_monitor(void *user_data)
> > +{
> > +       struct adv_monitor *adv_monitor = user_data;
> > +
> > +       g_free(adv_monitor->type);
> > +       g_free(adv_monitor->rssi);
> > +       g_slist_free_full(adv_monitor->patterns, free_pattern);
> > +       g_free(adv_monitor);
> > +}
> > +
> > +static uint8_t str2bytearray(char *str, uint8_t *arr)
> > +{
> > +       int idx, len = strlen(str), arr_len = 0;
> > +
> > +       if (len%2 != 0)
> > +               return 0;
> > +
> > +       for (idx = 0; idx < len; idx += 2) {
> > +               if (sscanf(str+idx, "%2hhx", &arr[arr_len++]) < 1)
> > +                       return 0;
> > +       }
> > +       return arr_len;
> > +}
> > +
> > +static struct rssi_setting *parse_rssi(char *rssi_str)
> > +{
> > +       struct rssi_setting *rssi;
> > +       int16_t high_threshold, low_threshold;
> > +       uint16_t high_timer, low_timer;
> > +
> > +       if (sscanf(rssi_str, "%hd,%hu,%hd,%hu", &high_threshold, &high_timer,
> > +                                       &low_threshold, &low_timer) < 4)
> > +               return NULL;
> > +
> > +       rssi = g_malloc0(sizeof(struct rssi_setting));
> > +
> > +       if (!rssi) {
> > +               bt_shell_printf("Failed to allocate rssi_setting");
> > +               bt_shell_noninteractive_quit(EXIT_FAILURE);
> > +               return NULL;
> > +       }
> > +
> > +       rssi->high_threshold = high_threshold;
> > +       rssi->high_timer = high_timer;
> > +       rssi->low_threshold = low_threshold;
> > +       rssi->low_timer = low_timer;
> > +
> > +       return rssi;
> > +}
> > +
> > +static struct pattern *parse_pattern(char *pattern)
> > +{
> > +       uint8_t start_pos, ad_data_type;
> > +       char content_str[BT_AD_MAX_DATA_LEN];
> > +       struct pattern *pat;
> > +
> > +       if (sscanf(pattern, "%hhu,%hhu,%s", &start_pos, &ad_data_type,
> > +                                                       content_str) < 3)
> > +               return NULL;
> > +
> > +       pat = g_malloc0(sizeof(struct pattern));
> > +
> > +       if (!pat) {
> > +               bt_shell_printf("Failed to allocate pattern");
> > +               bt_shell_noninteractive_quit(EXIT_FAILURE);
> > +               return NULL;
> > +       }
> > +
> > +       pat->start_pos = start_pos;
> > +       pat->ad_data_type = ad_data_type;
> > +       pat->content_len = str2bytearray(content_str, pat->content);
> > +       if (pat->content_len == 0) {
> > +               free_pattern(pat);
> > +               return NULL;
> > +       }
> > +
> > +       return pat;
> > +}
> > +
> > +static GSList *parse_patterns(char *pattern_list[], int num)
> > +{
> > +       GSList *patterns = NULL;
> > +       int cnt;
> > +
> > +       if (num == 0) {
> > +               bt_shell_printf("No pattern provided\n");
> > +               return NULL;
> > +       }
> > +
> > +       for (cnt = 0; cnt < num; cnt++) {
> > +               struct pattern *pattern;
> > +
> > +               pattern = parse_pattern(pattern_list[cnt]);
> > +               if (pattern == NULL) {
> > +                       g_slist_free_full(patterns, free_pattern);
> > +                       return NULL;
> > +               }
> > +               patterns = g_slist_append(patterns, pattern);
> > +       }
> > +
> > +       return patterns;
> > +}
> > +
> > +static gint cmp_adv_monitor_with_idx(gconstpointer a, gconstpointer b)
> > +{
> > +       const struct adv_monitor *adv_monitor = a;
> > +       uint8_t idx = *(uint8_t *)b;
> > +
> > +       return adv_monitor->idx != idx;
> > +}
> > +
> > +static struct adv_monitor *find_adv_monitor_with_idx(uint8_t monitor_idx)
> > +{
> > +       GSList *list;
> > +
> > +       list = g_slist_find_custom(adv_mons, &monitor_idx,
> > +                                               cmp_adv_monitor_with_idx);
> > +
> > +       if (list)
> > +               return (struct adv_monitor *)list->data;
> > +       return NULL;
> > +}
> > +
> > +static void print_bytearray(char *prefix, uint8_t *arr, uint8_t len)
> > +{
> > +       int idx;
> > +
> > +       bt_shell_printf("%s", prefix);
> > +       for (idx = 0; idx < len; idx++)
> > +               bt_shell_printf("%02hhx", arr[idx]);
> > +       bt_shell_printf("\n");
> > +}
> > +
> > +static void print_adv_monitor(struct adv_monitor *adv_monitor)
> > +{
> > +       GSList *l;
> > +
> > +       bt_shell_printf("Advertisement Monitor %d\n", adv_monitor->idx);
> > +       bt_shell_printf("\ttype: %s\n", adv_monitor->type);
> > +       if (adv_monitor->rssi) {
> > +               bt_shell_printf("\trssi:\n");
> > +               bt_shell_printf("\t\thigh threshold: %hd\n",
> > +                                       adv_monitor->rssi->high_threshold);
> > +               bt_shell_printf("\t\thigh threshold timer: %hu\n",
> > +                                       adv_monitor->rssi->high_timer);
> > +               bt_shell_printf("\t\tlow threshold: %hd\n",
> > +                                       adv_monitor->rssi->low_threshold);
> > +               bt_shell_printf("\t\tlow threshold timer: %hu\n",
> > +                                       adv_monitor->rssi->low_timer);
> > +       }
> > +
> > +       if (adv_monitor->patterns) {
> > +               int idx = 1;
> > +
> > +               for (l = adv_monitor->patterns; l; l = g_slist_next(l), idx++) {
> > +                       struct pattern *pattern = l->data;
> > +
> > +                       bt_shell_printf("\tpattern %d:\n", idx);
> > +                       bt_shell_printf("\t\tstart position: %hhu\n",
> > +                                                       pattern->start_pos);
> > +                       bt_shell_printf("\t\tAD data type: %hhu\n",
> > +                                                       pattern->ad_data_type);
> > +                       print_bytearray("\t\tcontent: ", pattern->content,
> > +                                                       pattern->content_len);
> > +               }
> > +       }
> > +}
> > +
> > +void adv_monitor_add_monitor(DBusConnection *conn, int argc, char *argv[])
> > +{
> > +       struct adv_monitor *adv_monitor;
> > +       struct rssi_setting *rssi;
> > +       GSList *patterns = NULL;
> > +       char *type;
> > +
> > +       if (g_slist_length(adv_mons) >= UINT8_MAX) {
> > +               bt_shell_printf("Number of advertisement monitor exceeds "
> > +                               "the limit");
> > +               return;
> > +       }
> > +
> > +       while (find_adv_monitor_with_idx(adv_mon_idx))
> > +               adv_mon_idx += 1;
> > +
> > +       type = argv[1];
> > +
> > +       if (strcmp(argv[2], "-") == 0)
> > +               rssi = NULL;
> > +       else {
> > +               rssi = parse_rssi(argv[2]);
> > +               if (rssi == NULL) {
> > +                       bt_shell_printf("RSSIThresholdAndTimers malformed\n");
> > +                       return;
> > +               }
> > +       }
> > +
> > +       patterns = parse_patterns(argv+3, argc-3);
> > +       if (patterns == NULL) {
> > +               bt_shell_printf("pattern-list malformed\n");
> > +               return;
> > +       }
> > +
> > +       adv_monitor = g_malloc0(sizeof(struct adv_monitor));
> > +
> > +       if (!adv_monitor) {
> > +               bt_shell_printf("Failed to allocate adv_monitor");
> > +               return bt_shell_noninteractive_quit(EXIT_FAILURE);
> > +       }
> > +
> > +       adv_monitor->idx = adv_mon_idx;
> > +       adv_monitor->type = g_strdup(type);
> > +       adv_monitor->rssi = rssi;
> > +       adv_monitor->patterns = patterns;
> > +
> > +       adv_mons = g_slist_append(adv_mons, adv_monitor);
> > +       bt_shell_printf("Advertisement Monitor %d added\n", adv_monitor->idx);
> > +}
> > +
> > +void adv_monitor_print_monitor(DBusConnection *conn, int monitor_idx)
> > +{
> > +       struct adv_monitor *adv_monitor;
> > +       GSList *l;
> > +
> > +       if (monitor_idx < 0) {
> > +               for (l = adv_mons; l; l = g_slist_next(l)) {
> > +                       adv_monitor = l->data;
> > +                       print_adv_monitor(adv_monitor);
> > +               }
> > +               return;
> > +       }
> > +
> > +       adv_monitor = find_adv_monitor_with_idx(monitor_idx);
> > +
> > +       if (adv_monitor == NULL) {
> > +               bt_shell_printf("Can't find monitor with index %d\n",
> > +                                                               monitor_idx);
> > +               return;
> > +       }
> > +
> > +       print_adv_monitor(adv_monitor);
> > +}
> > +
> > +void adv_monitor_remove_monitor(DBusConnection *conn, int monitor_idx)
> > +{
> > +       struct adv_monitor *adv_monitor;
> > +
> > +       if (monitor_idx < 0) {
> > +               g_slist_free_full(g_steal_pointer(&adv_mons), free_adv_monitor);
> > +               return;
> > +       }
> > +
> > +       adv_monitor = find_adv_monitor_with_idx(monitor_idx);
> > +       if (adv_monitor == NULL) {
> > +               bt_shell_printf("Can't find monitor with index %d\n",
> > +                                                               monitor_idx);
> > +               return;
> > +       }
> > +
> > +       adv_mons = g_slist_remove(adv_mons, adv_monitor);
> > +       free_adv_monitor(adv_monitor);
> > +       bt_shell_printf("Monitor %d deleted\n", monitor_idx);
> > +}
> > +
> > +static void print_supported_list(GSList *list)
> > +{
> > +       GSList *iter;
> > +
> > +       for (iter = list; iter; iter = g_slist_next(iter)) {
> > +               char *data = iter->data;
> > +
> > +               printf(" %s", data);
> > +       }
> > +}
> > +
> > +void adv_monitor_get_supported_info(void)
> > +{
> > +       bt_shell_printf("Supported Features:");
> > +       print_supported_list(manager.supported_features);
> > +       bt_shell_printf("\n");
> > +
> > +       bt_shell_printf("Supported Moniter Types:");
> > +       print_supported_list(manager.supported_types);
> > +       bt_shell_printf("\n");
> > +}
> > diff --git a/client/advertisement_monitor.h b/client/advertisement_monitor.h
> > index 77b0b62c6..f2a0caf77 100644
> > --- a/client/advertisement_monitor.h
> > +++ b/client/advertisement_monitor.h
> > @@ -21,3 +21,7 @@ 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);
> > +void adv_monitor_add_monitor(DBusConnection *conn, int argc, char *argv[]);
> > +void adv_monitor_print_monitor(DBusConnection *conn, int monitor_idx);
> > +void adv_monitor_remove_monitor(DBusConnection *conn, int monitor_idx);
> > +void adv_monitor_get_supported_info(void);
> > diff --git a/client/main.c b/client/main.c
> > index 7ddd13aa0..2b63ee62a 100644
> > --- a/client/main.c
> > +++ b/client/main.c
> > @@ -2686,6 +2686,53 @@ static void cmd_ad_clear(int argc, char *argv[])
> >                 return bt_shell_noninteractive_quit(EXIT_FAILURE);
> >  }
> >
> > +static void print_add_monitor_usage(void)
> > +{
> > +       bt_shell_usage();
> > +       bt_shell_printf("RSSIThresholdAndTimers format:\n"
> > +                       "\t<high-rssi>,<high-timeout>,<low-rssi>,<low-timeout>\n"
> > +                       "\tor single '-' for not using RSSI as filter\n");
> > +       bt_shell_printf("pattern format:\n"
> > +                       "\t<start_position>,<ad_data_type>,<content_of_pattern>\n");
> > +       bt_shell_printf("e.g.\n"
> > +                       "\tadd-pattern-monitor or_patterns -10,10,-20,20 1,2,01ab55\n");
>
> btshell parameters are space separated not comma.

The reason we use commas is because RSSIThresholdAndTimers is an
all-or-none property, it's clearer for users to either input all the
four parameters or a single '-' to leave it unset.
For a similar reason, a valid pattern requires all of the three
parameters provided. If we want to avoid comma for maintaining
consistency, we could have command usage like:

add-pattern or_patterns -10 10 -20 20 1 2 01ab55     // single content
with RSSI filter
add-pattern or_patterns - 1 2 01ab55                         // single
content without RSSI filter

Let me know if this is OK with you and other suggestions are welcome.

>
> > +}
> > +
> > +static void cmd_adv_monitor_add_monitor(int argc, char *argv[])
> > +{
> > +       if (argc < 3)
> > +               print_add_monitor_usage();
> > +       else
> > +               adv_monitor_add_monitor(dbus_conn, argc, argv);
> > +}
> > +
> > +static void cmd_adv_monitor_print_monitor(int argc, char *argv[])
> > +{
> > +       int monitor_idx;
> > +
> > +       if (strcmp(argv[1], "all") == 0)
> > +               monitor_idx = -1;
> > +       else
> > +               monitor_idx = atoi(argv[1]);
> > +       adv_monitor_print_monitor(dbus_conn, monitor_idx);
> > +}
> > +
> > +static void cmd_adv_monitor_remove_monitor(int argc, char *argv[])
> > +{
> > +       int monitor_idx;
> > +
> > +       if (strcmp(argv[1], "all") == 0)
> > +               monitor_idx = -1;
> > +       else
> > +               monitor_idx = atoi(argv[1]);
> > +       adv_monitor_remove_monitor(dbus_conn, monitor_idx);
> > +}
> > +
> > +static void cmd_adv_monitor_get_supported_info(int argc, char *argv[])
> > +{
> > +       adv_monitor_get_supported_info();
> > +}
> > +
> >  static const struct bt_shell_menu advertise_menu = {
> >         .name = "advertise",
> >         .desc = "Advertise Options Submenu",
> > @@ -2722,6 +2769,28 @@ static const struct bt_shell_menu advertise_menu = {
> >         { } },
> >  };
> >
> > +static const struct bt_shell_menu advertise_monitor_menu = {
> > +       .name = "advmon",
>
> I'd use monitor instead of advmon here.

Will address these in the next patch. Thanks!

>
> > +       .desc = "Advertisement Monitor Options Submenu",
> > +       .entries = {
> > +       { "add-pattern-monitor", "<type-of-monitor/help> "
> > +                               "[RSSIThresholdAndTimers] "
> > +                               "[patterns=pattern1 pattern2 ...]",
> > +                               cmd_adv_monitor_add_monitor,
> > +                               "Add pattern monitor" },
> > +       { "get-pattern-monitor", "<monitor-id/all>",
> > +                               cmd_adv_monitor_print_monitor,
> > +                               "Get advertisement monitor" },
> > +       { "remove-pattern-monitor", "<monitor-id/all>",
> > +                               cmd_adv_monitor_remove_monitor,
> > +                               "Remove advertisement monitor" },
> > +       { "get-supported-info", NULL,
> > +                               cmd_adv_monitor_get_supported_info,
> > +                               "Get advertisement manager supported "
> > +                               "features and supported monitor types" },
>
> We could probably drop the monitor from the commands here which should
> leave us at:
>
> add-pattern
> get-pattern
> remote-pattern
> supported-info
>
> > +       { } },
> > +};
> > +
> >  static const struct bt_shell_menu scan_menu = {
> >         .name = "scan",
> >         .desc = "Scan Options Submenu",
> > @@ -2897,6 +2966,7 @@ int main(int argc, char *argv[])
> >         bt_shell_init(argc, argv, &opt);
> >         bt_shell_set_menu(&main_menu);
> >         bt_shell_add_submenu(&advertise_menu);
> > +       bt_shell_add_submenu(&advertise_monitor_menu);
> >         bt_shell_add_submenu(&scan_menu);
> >         bt_shell_add_submenu(&gatt_menu);
> >         bt_shell_set_prompt(PROMPT_OFF);
> > --
> > 2.28.0.220.ged08abb693-goog
> >
>
>
> --
> Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [BlueZ PATCH v1 2/4] client: Implement more interfaces of ADV monitor in bluetoothctl
  2020-09-09  4:01     ` Yun-hao Chung
@ 2020-09-09  4:56       ` Luiz Augusto von Dentz
  2020-09-09  7:30         ` Yun-hao Chung
  0 siblings, 1 reply; 8+ messages in thread
From: Luiz Augusto von Dentz @ 2020-09-09  4:56 UTC (permalink / raw)
  To: Yun-hao Chung
  Cc: linux-bluetooth, Alain Michaud, Miao-chen Chou, Manish Mandlik

Hi Yun-hao,

On Tue, Sep 8, 2020 at 9:01 PM Yun-hao Chung <howardchung@google.com> wrote:
>
> Hi Luiz,
>
> Thanks for the comments.
>
>
> On Wed, Sep 9, 2020 at 1:06 AM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> >
> > Hi Howard,
> >
> > On Tue, Aug 18, 2020 at 9:15 PM Howard Chung <howardchung@google.com> wrote:
> > >
> > > This patch creates a submenu in bluetoothctl and implements several
> > > commands.
> > >
> > > new commands:
> > > [bluetooth]# menu advmon
> > > [bluetooth]# add-pattern-monitor or_patterns -10,10,-30,5 1,2,ab0011
> > > Advertisement Monitor 0 added
> > > [bluetooth]# get-pattern-monitor all
> > > Advertisement Monitor 0
> > >         path: /org/bluez/adv_monitor_app/0
> > >         type: or_patterns
> > >         rssi:
> > >                 high threshold: -10
> > >                 high threshold timer: 10
> > >                 low threshold: -30
> > >                 low threshold timer: 5
> > >         pattern 1:
> > >                 start position: 1
> > >                 AD data type: 2
> > >                 content: ab0011
> > > [bluetooth]# get-supported-info
> > > Supported Features:
> > > Supported Moniter Types: or_patterns
> > > [bluetooth]# remove-pattern-monitor 0
> > > Monitor 0 deleted
> > >
> > > Signed-off-by: Howard Chung <howardchung@google.com>
> > > ---
> > >
> > >  client/advertisement_monitor.c | 328 ++++++++++++++++++++++++++++++++-
> > >  client/advertisement_monitor.h |   4 +
> > >  client/main.c                  |  70 +++++++
> > >  3 files changed, 399 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/client/advertisement_monitor.c b/client/advertisement_monitor.c
> > > index bd2309537..ec8f23711 100644
> > > --- a/client/advertisement_monitor.c
> > > +++ b/client/advertisement_monitor.c
> > > @@ -29,6 +29,7 @@
> > >  #include <string.h>
> > >
> > >  #include "gdbus/gdbus.h"
> > > +#include "src/shared/ad.h"
> > >  #include "src/shared/util.h"
> > >  #include "src/shared/shell.h"
> > >  #include "advertisement_monitor.h"
> > > @@ -36,6 +37,27 @@
> > >  #define ADV_MONITOR_APP_PATH "/org/bluez/adv_monitor_app"
> > >  #define ADV_MONITOR_INTERFACE "org.bluez.AdvertisementMonitor1"
> > >
> > > +struct rssi_setting {
> > > +       int16_t high_threshold;
> > > +       uint16_t high_timer;
> > > +       int16_t low_threshold;
> > > +       uint16_t low_timer;
> > > +};
> > > +
> > > +struct pattern {
> > > +       uint8_t start_pos;
> > > +       uint8_t ad_data_type;
> > > +       uint8_t content_len;
> > > +       uint8_t content[BT_AD_MAX_DATA_LEN];
> > > +};
> > > +
> > > +struct adv_monitor {
> > > +       uint8_t idx;
> > > +       char *type;
> > > +       struct rssi_setting *rssi;
> > > +       GSList *patterns;
> > > +};
> > > +
> > >  struct adv_monitor_manager {
> > >         GSList *supported_types;
> > >         GSList *supported_features;
> > > @@ -43,6 +65,9 @@ struct adv_monitor_manager {
> > >         gboolean app_registered;
> > >  } manager = { NULL, NULL, NULL, FALSE };
> > >
> > > +static uint8_t adv_mon_idx;
> > > +static GSList *adv_mons;
> > > +
> > >  static void set_supported_list(GSList **list, DBusMessageIter *iter)
> > >  {
> > >         char *str;
> > > @@ -138,7 +163,10 @@ static void unregister_reply(DBusMessage *message, void *user_data)
> > >
> > >  void adv_monitor_register_app(DBusConnection *conn)
> > >  {
> > > -       if (manager.supported_types == NULL || manager.app_registered == TRUE ||
> > > +       if (manager.app_registered == TRUE) {
> > > +               bt_shell_printf("Advertisement Monitor already registered\n");
> > > +               return bt_shell_noninteractive_quit(EXIT_FAILURE);
> > > +       } else if (manager.supported_types == NULL ||
> > >                 g_dbus_proxy_method_call(manager.proxy, "RegisterMonitor",
> > >                                         register_setup, register_reply,
> > >                                         NULL, NULL) == FALSE) {
> > > @@ -150,8 +178,10 @@ void adv_monitor_register_app(DBusConnection *conn)
> > >
> > >  void adv_monitor_unregister_app(DBusConnection *conn)
> > >  {
> > > -       if (manager.app_registered == FALSE ||
> > > -               g_dbus_proxy_method_call(manager.proxy, "UnregisterMonitor",
> > > +       if (manager.app_registered == FALSE) {
> > > +               bt_shell_printf("Advertisement Monitor not registered\n");
> > > +               return bt_shell_noninteractive_quit(EXIT_FAILURE);
> > > +       } else if (g_dbus_proxy_method_call(manager.proxy, "UnregisterMonitor",
> > >                                         unregister_setup, unregister_reply,
> > >                                         NULL, NULL) == FALSE) {
> > >                 bt_shell_printf("Failed to unregister Advertisement Monitor\n");
> > > @@ -159,3 +189,295 @@ void adv_monitor_unregister_app(DBusConnection *conn)
> > >         }
> > >         manager.app_registered = FALSE;
> > >  }
> > > +
> > > +static void free_pattern(void *user_data)
> > > +{
> > > +       struct pattern *p = user_data;
> > > +
> > > +       g_free(p);
> > > +}
> > > +
> > > +static void free_adv_monitor(void *user_data)
> > > +{
> > > +       struct adv_monitor *adv_monitor = user_data;
> > > +
> > > +       g_free(adv_monitor->type);
> > > +       g_free(adv_monitor->rssi);
> > > +       g_slist_free_full(adv_monitor->patterns, free_pattern);
> > > +       g_free(adv_monitor);
> > > +}
> > > +
> > > +static uint8_t str2bytearray(char *str, uint8_t *arr)
> > > +{
> > > +       int idx, len = strlen(str), arr_len = 0;
> > > +
> > > +       if (len%2 != 0)
> > > +               return 0;
> > > +
> > > +       for (idx = 0; idx < len; idx += 2) {
> > > +               if (sscanf(str+idx, "%2hhx", &arr[arr_len++]) < 1)
> > > +                       return 0;
> > > +       }
> > > +       return arr_len;
> > > +}
> > > +
> > > +static struct rssi_setting *parse_rssi(char *rssi_str)
> > > +{
> > > +       struct rssi_setting *rssi;
> > > +       int16_t high_threshold, low_threshold;
> > > +       uint16_t high_timer, low_timer;
> > > +
> > > +       if (sscanf(rssi_str, "%hd,%hu,%hd,%hu", &high_threshold, &high_timer,
> > > +                                       &low_threshold, &low_timer) < 4)
> > > +               return NULL;
> > > +
> > > +       rssi = g_malloc0(sizeof(struct rssi_setting));
> > > +
> > > +       if (!rssi) {
> > > +               bt_shell_printf("Failed to allocate rssi_setting");
> > > +               bt_shell_noninteractive_quit(EXIT_FAILURE);
> > > +               return NULL;
> > > +       }
> > > +
> > > +       rssi->high_threshold = high_threshold;
> > > +       rssi->high_timer = high_timer;
> > > +       rssi->low_threshold = low_threshold;
> > > +       rssi->low_timer = low_timer;
> > > +
> > > +       return rssi;
> > > +}
> > > +
> > > +static struct pattern *parse_pattern(char *pattern)
> > > +{
> > > +       uint8_t start_pos, ad_data_type;
> > > +       char content_str[BT_AD_MAX_DATA_LEN];
> > > +       struct pattern *pat;
> > > +
> > > +       if (sscanf(pattern, "%hhu,%hhu,%s", &start_pos, &ad_data_type,
> > > +                                                       content_str) < 3)
> > > +               return NULL;
> > > +
> > > +       pat = g_malloc0(sizeof(struct pattern));
> > > +
> > > +       if (!pat) {
> > > +               bt_shell_printf("Failed to allocate pattern");
> > > +               bt_shell_noninteractive_quit(EXIT_FAILURE);
> > > +               return NULL;
> > > +       }
> > > +
> > > +       pat->start_pos = start_pos;
> > > +       pat->ad_data_type = ad_data_type;
> > > +       pat->content_len = str2bytearray(content_str, pat->content);
> > > +       if (pat->content_len == 0) {
> > > +               free_pattern(pat);
> > > +               return NULL;
> > > +       }
> > > +
> > > +       return pat;
> > > +}
> > > +
> > > +static GSList *parse_patterns(char *pattern_list[], int num)
> > > +{
> > > +       GSList *patterns = NULL;
> > > +       int cnt;
> > > +
> > > +       if (num == 0) {
> > > +               bt_shell_printf("No pattern provided\n");
> > > +               return NULL;
> > > +       }
> > > +
> > > +       for (cnt = 0; cnt < num; cnt++) {
> > > +               struct pattern *pattern;
> > > +
> > > +               pattern = parse_pattern(pattern_list[cnt]);
> > > +               if (pattern == NULL) {
> > > +                       g_slist_free_full(patterns, free_pattern);
> > > +                       return NULL;
> > > +               }
> > > +               patterns = g_slist_append(patterns, pattern);
> > > +       }
> > > +
> > > +       return patterns;
> > > +}
> > > +
> > > +static gint cmp_adv_monitor_with_idx(gconstpointer a, gconstpointer b)
> > > +{
> > > +       const struct adv_monitor *adv_monitor = a;
> > > +       uint8_t idx = *(uint8_t *)b;
> > > +
> > > +       return adv_monitor->idx != idx;
> > > +}
> > > +
> > > +static struct adv_monitor *find_adv_monitor_with_idx(uint8_t monitor_idx)
> > > +{
> > > +       GSList *list;
> > > +
> > > +       list = g_slist_find_custom(adv_mons, &monitor_idx,
> > > +                                               cmp_adv_monitor_with_idx);
> > > +
> > > +       if (list)
> > > +               return (struct adv_monitor *)list->data;
> > > +       return NULL;
> > > +}
> > > +
> > > +static void print_bytearray(char *prefix, uint8_t *arr, uint8_t len)
> > > +{
> > > +       int idx;
> > > +
> > > +       bt_shell_printf("%s", prefix);
> > > +       for (idx = 0; idx < len; idx++)
> > > +               bt_shell_printf("%02hhx", arr[idx]);
> > > +       bt_shell_printf("\n");
> > > +}
> > > +
> > > +static void print_adv_monitor(struct adv_monitor *adv_monitor)
> > > +{
> > > +       GSList *l;
> > > +
> > > +       bt_shell_printf("Advertisement Monitor %d\n", adv_monitor->idx);
> > > +       bt_shell_printf("\ttype: %s\n", adv_monitor->type);
> > > +       if (adv_monitor->rssi) {
> > > +               bt_shell_printf("\trssi:\n");
> > > +               bt_shell_printf("\t\thigh threshold: %hd\n",
> > > +                                       adv_monitor->rssi->high_threshold);
> > > +               bt_shell_printf("\t\thigh threshold timer: %hu\n",
> > > +                                       adv_monitor->rssi->high_timer);
> > > +               bt_shell_printf("\t\tlow threshold: %hd\n",
> > > +                                       adv_monitor->rssi->low_threshold);
> > > +               bt_shell_printf("\t\tlow threshold timer: %hu\n",
> > > +                                       adv_monitor->rssi->low_timer);
> > > +       }
> > > +
> > > +       if (adv_monitor->patterns) {
> > > +               int idx = 1;
> > > +
> > > +               for (l = adv_monitor->patterns; l; l = g_slist_next(l), idx++) {
> > > +                       struct pattern *pattern = l->data;
> > > +
> > > +                       bt_shell_printf("\tpattern %d:\n", idx);
> > > +                       bt_shell_printf("\t\tstart position: %hhu\n",
> > > +                                                       pattern->start_pos);
> > > +                       bt_shell_printf("\t\tAD data type: %hhu\n",
> > > +                                                       pattern->ad_data_type);
> > > +                       print_bytearray("\t\tcontent: ", pattern->content,
> > > +                                                       pattern->content_len);
> > > +               }
> > > +       }
> > > +}
> > > +
> > > +void adv_monitor_add_monitor(DBusConnection *conn, int argc, char *argv[])
> > > +{
> > > +       struct adv_monitor *adv_monitor;
> > > +       struct rssi_setting *rssi;
> > > +       GSList *patterns = NULL;
> > > +       char *type;
> > > +
> > > +       if (g_slist_length(adv_mons) >= UINT8_MAX) {
> > > +               bt_shell_printf("Number of advertisement monitor exceeds "
> > > +                               "the limit");
> > > +               return;
> > > +       }
> > > +
> > > +       while (find_adv_monitor_with_idx(adv_mon_idx))
> > > +               adv_mon_idx += 1;
> > > +
> > > +       type = argv[1];
> > > +
> > > +       if (strcmp(argv[2], "-") == 0)
> > > +               rssi = NULL;
> > > +       else {
> > > +               rssi = parse_rssi(argv[2]);
> > > +               if (rssi == NULL) {
> > > +                       bt_shell_printf("RSSIThresholdAndTimers malformed\n");
> > > +                       return;
> > > +               }
> > > +       }
> > > +
> > > +       patterns = parse_patterns(argv+3, argc-3);
> > > +       if (patterns == NULL) {
> > > +               bt_shell_printf("pattern-list malformed\n");
> > > +               return;
> > > +       }
> > > +
> > > +       adv_monitor = g_malloc0(sizeof(struct adv_monitor));
> > > +
> > > +       if (!adv_monitor) {
> > > +               bt_shell_printf("Failed to allocate adv_monitor");
> > > +               return bt_shell_noninteractive_quit(EXIT_FAILURE);
> > > +       }
> > > +
> > > +       adv_monitor->idx = adv_mon_idx;
> > > +       adv_monitor->type = g_strdup(type);
> > > +       adv_monitor->rssi = rssi;
> > > +       adv_monitor->patterns = patterns;
> > > +
> > > +       adv_mons = g_slist_append(adv_mons, adv_monitor);
> > > +       bt_shell_printf("Advertisement Monitor %d added\n", adv_monitor->idx);
> > > +}
> > > +
> > > +void adv_monitor_print_monitor(DBusConnection *conn, int monitor_idx)
> > > +{
> > > +       struct adv_monitor *adv_monitor;
> > > +       GSList *l;
> > > +
> > > +       if (monitor_idx < 0) {
> > > +               for (l = adv_mons; l; l = g_slist_next(l)) {
> > > +                       adv_monitor = l->data;
> > > +                       print_adv_monitor(adv_monitor);
> > > +               }
> > > +               return;
> > > +       }
> > > +
> > > +       adv_monitor = find_adv_monitor_with_idx(monitor_idx);
> > > +
> > > +       if (adv_monitor == NULL) {
> > > +               bt_shell_printf("Can't find monitor with index %d\n",
> > > +                                                               monitor_idx);
> > > +               return;
> > > +       }
> > > +
> > > +       print_adv_monitor(adv_monitor);
> > > +}
> > > +
> > > +void adv_monitor_remove_monitor(DBusConnection *conn, int monitor_idx)
> > > +{
> > > +       struct adv_monitor *adv_monitor;
> > > +
> > > +       if (monitor_idx < 0) {
> > > +               g_slist_free_full(g_steal_pointer(&adv_mons), free_adv_monitor);
> > > +               return;
> > > +       }
> > > +
> > > +       adv_monitor = find_adv_monitor_with_idx(monitor_idx);
> > > +       if (adv_monitor == NULL) {
> > > +               bt_shell_printf("Can't find monitor with index %d\n",
> > > +                                                               monitor_idx);
> > > +               return;
> > > +       }
> > > +
> > > +       adv_mons = g_slist_remove(adv_mons, adv_monitor);
> > > +       free_adv_monitor(adv_monitor);
> > > +       bt_shell_printf("Monitor %d deleted\n", monitor_idx);
> > > +}
> > > +
> > > +static void print_supported_list(GSList *list)
> > > +{
> > > +       GSList *iter;
> > > +
> > > +       for (iter = list; iter; iter = g_slist_next(iter)) {
> > > +               char *data = iter->data;
> > > +
> > > +               printf(" %s", data);
> > > +       }
> > > +}
> > > +
> > > +void adv_monitor_get_supported_info(void)
> > > +{
> > > +       bt_shell_printf("Supported Features:");
> > > +       print_supported_list(manager.supported_features);
> > > +       bt_shell_printf("\n");
> > > +
> > > +       bt_shell_printf("Supported Moniter Types:");
> > > +       print_supported_list(manager.supported_types);
> > > +       bt_shell_printf("\n");
> > > +}
> > > diff --git a/client/advertisement_monitor.h b/client/advertisement_monitor.h
> > > index 77b0b62c6..f2a0caf77 100644
> > > --- a/client/advertisement_monitor.h
> > > +++ b/client/advertisement_monitor.h
> > > @@ -21,3 +21,7 @@ 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);
> > > +void adv_monitor_add_monitor(DBusConnection *conn, int argc, char *argv[]);
> > > +void adv_monitor_print_monitor(DBusConnection *conn, int monitor_idx);
> > > +void adv_monitor_remove_monitor(DBusConnection *conn, int monitor_idx);
> > > +void adv_monitor_get_supported_info(void);
> > > diff --git a/client/main.c b/client/main.c
> > > index 7ddd13aa0..2b63ee62a 100644
> > > --- a/client/main.c
> > > +++ b/client/main.c
> > > @@ -2686,6 +2686,53 @@ static void cmd_ad_clear(int argc, char *argv[])
> > >                 return bt_shell_noninteractive_quit(EXIT_FAILURE);
> > >  }
> > >
> > > +static void print_add_monitor_usage(void)
> > > +{
> > > +       bt_shell_usage();
> > > +       bt_shell_printf("RSSIThresholdAndTimers format:\n"
> > > +                       "\t<high-rssi>,<high-timeout>,<low-rssi>,<low-timeout>\n"
> > > +                       "\tor single '-' for not using RSSI as filter\n");
> > > +       bt_shell_printf("pattern format:\n"
> > > +                       "\t<start_position>,<ad_data_type>,<content_of_pattern>\n");
> > > +       bt_shell_printf("e.g.\n"
> > > +                       "\tadd-pattern-monitor or_patterns -10,10,-20,20 1,2,01ab55\n");
> >
> > btshell parameters are space separated not comma.
>
> The reason we use commas is because RSSIThresholdAndTimers is an
> all-or-none property, it's clearer for users to either input all the
> four parameters or a single '-' to leave it unset.
> For a similar reason, a valid pattern requires all of the three
> parameters provided. If we want to avoid comma for maintaining
> consistency, we could have command usage like:
>
> add-pattern or_patterns -10 10 -20 20 1 2 01ab55     // single content
> with RSSI filter
> add-pattern or_patterns - 1 2 01ab55                         // single
> content without RSSI filter
>
> Let me know if this is OK with you and other suggestions are welcome.

Actually it might be better to have good defaults on these or have
something like:

<rssi-range=low,high> then parse the input to check if comma has been
passed otherwise we can assume only the high (or low if that value
means better signal) rssi has been passed.
<timeout=low,high> same as above.

Also I would consider merging the type into the command itself:

add-or-pattern-rssi -10,-20 10,1 01ab55
add-or-pattern 1 2 01ab55

That way the bt_shell parsing of the command description can actually
detect if enough parameters have been given since we can't really have
short of overloading command with different arguments as it would make
it rather confusing to use since there are no types to detect which
version the user is trying to use.

> >
> > > +}
> > > +
> > > +static void cmd_adv_monitor_add_monitor(int argc, char *argv[])
> > > +{
> > > +       if (argc < 3)
> > > +               print_add_monitor_usage();
> > > +       else
> > > +               adv_monitor_add_monitor(dbus_conn, argc, argv);
> > > +}
> > > +
> > > +static void cmd_adv_monitor_print_monitor(int argc, char *argv[])
> > > +{
> > > +       int monitor_idx;
> > > +
> > > +       if (strcmp(argv[1], "all") == 0)
> > > +               monitor_idx = -1;
> > > +       else
> > > +               monitor_idx = atoi(argv[1]);
> > > +       adv_monitor_print_monitor(dbus_conn, monitor_idx);
> > > +}
> > > +
> > > +static void cmd_adv_monitor_remove_monitor(int argc, char *argv[])
> > > +{
> > > +       int monitor_idx;
> > > +
> > > +       if (strcmp(argv[1], "all") == 0)
> > > +               monitor_idx = -1;
> > > +       else
> > > +               monitor_idx = atoi(argv[1]);
> > > +       adv_monitor_remove_monitor(dbus_conn, monitor_idx);
> > > +}
> > > +
> > > +static void cmd_adv_monitor_get_supported_info(int argc, char *argv[])
> > > +{
> > > +       adv_monitor_get_supported_info();
> > > +}
> > > +
> > >  static const struct bt_shell_menu advertise_menu = {
> > >         .name = "advertise",
> > >         .desc = "Advertise Options Submenu",
> > > @@ -2722,6 +2769,28 @@ static const struct bt_shell_menu advertise_menu = {
> > >         { } },
> > >  };
> > >
> > > +static const struct bt_shell_menu advertise_monitor_menu = {
> > > +       .name = "advmon",
> >
> > I'd use monitor instead of advmon here.
>
> Will address these in the next patch. Thanks!
>
> >
> > > +       .desc = "Advertisement Monitor Options Submenu",
> > > +       .entries = {
> > > +       { "add-pattern-monitor", "<type-of-monitor/help> "
> > > +                               "[RSSIThresholdAndTimers] "
> > > +                               "[patterns=pattern1 pattern2 ...]",
> > > +                               cmd_adv_monitor_add_monitor,
> > > +                               "Add pattern monitor" },
> > > +       { "get-pattern-monitor", "<monitor-id/all>",
> > > +                               cmd_adv_monitor_print_monitor,
> > > +                               "Get advertisement monitor" },
> > > +       { "remove-pattern-monitor", "<monitor-id/all>",
> > > +                               cmd_adv_monitor_remove_monitor,
> > > +                               "Remove advertisement monitor" },
> > > +       { "get-supported-info", NULL,
> > > +                               cmd_adv_monitor_get_supported_info,
> > > +                               "Get advertisement manager supported "
> > > +                               "features and supported monitor types" },
> >
> > We could probably drop the monitor from the commands here which should
> > leave us at:
> >
> > add-pattern
> > get-pattern
> > remote-pattern
> > supported-info
> >
> > > +       { } },
> > > +};
> > > +
> > >  static const struct bt_shell_menu scan_menu = {
> > >         .name = "scan",
> > >         .desc = "Scan Options Submenu",
> > > @@ -2897,6 +2966,7 @@ int main(int argc, char *argv[])
> > >         bt_shell_init(argc, argv, &opt);
> > >         bt_shell_set_menu(&main_menu);
> > >         bt_shell_add_submenu(&advertise_menu);
> > > +       bt_shell_add_submenu(&advertise_monitor_menu);
> > >         bt_shell_add_submenu(&scan_menu);
> > >         bt_shell_add_submenu(&gatt_menu);
> > >         bt_shell_set_prompt(PROMPT_OFF);
> > > --
> > > 2.28.0.220.ged08abb693-goog
> > >
> >
> >
> > --
> > Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [BlueZ PATCH v1 2/4] client: Implement more interfaces of ADV monitor in bluetoothctl
  2020-09-09  4:56       ` Luiz Augusto von Dentz
@ 2020-09-09  7:30         ` Yun-hao Chung
  0 siblings, 0 replies; 8+ messages in thread
From: Yun-hao Chung @ 2020-09-09  7:30 UTC (permalink / raw)
  To: Luiz Augusto von Dentz
  Cc: linux-bluetooth, Alain Michaud, Miao-chen Chou, Manish Mandlik

Split commands into add-or-pattern-rssi and add-or-pattern make sense
to me.

However, allowing users to pass only one argument on <rssi-range>
could cause some ambiguity when the provided value is between the
default low-rssi and high-rssi. Also high-timeout is not necessary to
be higher or lower than low-timeout. I'm thinking another option:
What if we force users to pass the comma then we know which value
should be set.

For example:
add-or-pattern-rssi -10, ,1 1 2 01ab55
would only update low-rssi and high-timeout. The rest of the two
parameters remain as default.

On Wed, Sep 9, 2020 at 12:57 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Yun-hao,
>
> On Tue, Sep 8, 2020 at 9:01 PM Yun-hao Chung <howardchung@google.com> wrote:
> >
> > Hi Luiz,
> >
> > Thanks for the comments.
> >
> >
> > On Wed, Sep 9, 2020 at 1:06 AM Luiz Augusto von Dentz
> > <luiz.dentz@gmail.com> wrote:
> > >
> > > Hi Howard,
> > >
> > > On Tue, Aug 18, 2020 at 9:15 PM Howard Chung <howardchung@google.com> wrote:
> > > >
> > > > This patch creates a submenu in bluetoothctl and implements several
> > > > commands.
> > > >
> > > > new commands:
> > > > [bluetooth]# menu advmon
> > > > [bluetooth]# add-pattern-monitor or_patterns -10,10,-30,5 1,2,ab0011
> > > > Advertisement Monitor 0 added
> > > > [bluetooth]# get-pattern-monitor all
> > > > Advertisement Monitor 0
> > > >         path: /org/bluez/adv_monitor_app/0
> > > >         type: or_patterns
> > > >         rssi:
> > > >                 high threshold: -10
> > > >                 high threshold timer: 10
> > > >                 low threshold: -30
> > > >                 low threshold timer: 5
> > > >         pattern 1:
> > > >                 start position: 1
> > > >                 AD data type: 2
> > > >                 content: ab0011
> > > > [bluetooth]# get-supported-info
> > > > Supported Features:
> > > > Supported Moniter Types: or_patterns
> > > > [bluetooth]# remove-pattern-monitor 0
> > > > Monitor 0 deleted
> > > >
> > > > Signed-off-by: Howard Chung <howardchung@google.com>
> > > > ---
> > > >
> > > >  client/advertisement_monitor.c | 328 ++++++++++++++++++++++++++++++++-
> > > >  client/advertisement_monitor.h |   4 +
> > > >  client/main.c                  |  70 +++++++
> > > >  3 files changed, 399 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/client/advertisement_monitor.c b/client/advertisement_monitor.c
> > > > index bd2309537..ec8f23711 100644
> > > > --- a/client/advertisement_monitor.c
> > > > +++ b/client/advertisement_monitor.c
> > > > @@ -29,6 +29,7 @@
> > > >  #include <string.h>
> > > >
> > > >  #include "gdbus/gdbus.h"
> > > > +#include "src/shared/ad.h"
> > > >  #include "src/shared/util.h"
> > > >  #include "src/shared/shell.h"
> > > >  #include "advertisement_monitor.h"
> > > > @@ -36,6 +37,27 @@
> > > >  #define ADV_MONITOR_APP_PATH "/org/bluez/adv_monitor_app"
> > > >  #define ADV_MONITOR_INTERFACE "org.bluez.AdvertisementMonitor1"
> > > >
> > > > +struct rssi_setting {
> > > > +       int16_t high_threshold;
> > > > +       uint16_t high_timer;
> > > > +       int16_t low_threshold;
> > > > +       uint16_t low_timer;
> > > > +};
> > > > +
> > > > +struct pattern {
> > > > +       uint8_t start_pos;
> > > > +       uint8_t ad_data_type;
> > > > +       uint8_t content_len;
> > > > +       uint8_t content[BT_AD_MAX_DATA_LEN];
> > > > +};
> > > > +
> > > > +struct adv_monitor {
> > > > +       uint8_t idx;
> > > > +       char *type;
> > > > +       struct rssi_setting *rssi;
> > > > +       GSList *patterns;
> > > > +};
> > > > +
> > > >  struct adv_monitor_manager {
> > > >         GSList *supported_types;
> > > >         GSList *supported_features;
> > > > @@ -43,6 +65,9 @@ struct adv_monitor_manager {
> > > >         gboolean app_registered;
> > > >  } manager = { NULL, NULL, NULL, FALSE };
> > > >
> > > > +static uint8_t adv_mon_idx;
> > > > +static GSList *adv_mons;
> > > > +
> > > >  static void set_supported_list(GSList **list, DBusMessageIter *iter)
> > > >  {
> > > >         char *str;
> > > > @@ -138,7 +163,10 @@ static void unregister_reply(DBusMessage *message, void *user_data)
> > > >
> > > >  void adv_monitor_register_app(DBusConnection *conn)
> > > >  {
> > > > -       if (manager.supported_types == NULL || manager.app_registered == TRUE ||
> > > > +       if (manager.app_registered == TRUE) {
> > > > +               bt_shell_printf("Advertisement Monitor already registered\n");
> > > > +               return bt_shell_noninteractive_quit(EXIT_FAILURE);
> > > > +       } else if (manager.supported_types == NULL ||
> > > >                 g_dbus_proxy_method_call(manager.proxy, "RegisterMonitor",
> > > >                                         register_setup, register_reply,
> > > >                                         NULL, NULL) == FALSE) {
> > > > @@ -150,8 +178,10 @@ void adv_monitor_register_app(DBusConnection *conn)
> > > >
> > > >  void adv_monitor_unregister_app(DBusConnection *conn)
> > > >  {
> > > > -       if (manager.app_registered == FALSE ||
> > > > -               g_dbus_proxy_method_call(manager.proxy, "UnregisterMonitor",
> > > > +       if (manager.app_registered == FALSE) {
> > > > +               bt_shell_printf("Advertisement Monitor not registered\n");
> > > > +               return bt_shell_noninteractive_quit(EXIT_FAILURE);
> > > > +       } else if (g_dbus_proxy_method_call(manager.proxy, "UnregisterMonitor",
> > > >                                         unregister_setup, unregister_reply,
> > > >                                         NULL, NULL) == FALSE) {
> > > >                 bt_shell_printf("Failed to unregister Advertisement Monitor\n");
> > > > @@ -159,3 +189,295 @@ void adv_monitor_unregister_app(DBusConnection *conn)
> > > >         }
> > > >         manager.app_registered = FALSE;
> > > >  }
> > > > +
> > > > +static void free_pattern(void *user_data)
> > > > +{
> > > > +       struct pattern *p = user_data;
> > > > +
> > > > +       g_free(p);
> > > > +}
> > > > +
> > > > +static void free_adv_monitor(void *user_data)
> > > > +{
> > > > +       struct adv_monitor *adv_monitor = user_data;
> > > > +
> > > > +       g_free(adv_monitor->type);
> > > > +       g_free(adv_monitor->rssi);
> > > > +       g_slist_free_full(adv_monitor->patterns, free_pattern);
> > > > +       g_free(adv_monitor);
> > > > +}
> > > > +
> > > > +static uint8_t str2bytearray(char *str, uint8_t *arr)
> > > > +{
> > > > +       int idx, len = strlen(str), arr_len = 0;
> > > > +
> > > > +       if (len%2 != 0)
> > > > +               return 0;
> > > > +
> > > > +       for (idx = 0; idx < len; idx += 2) {
> > > > +               if (sscanf(str+idx, "%2hhx", &arr[arr_len++]) < 1)
> > > > +                       return 0;
> > > > +       }
> > > > +       return arr_len;
> > > > +}
> > > > +
> > > > +static struct rssi_setting *parse_rssi(char *rssi_str)
> > > > +{
> > > > +       struct rssi_setting *rssi;
> > > > +       int16_t high_threshold, low_threshold;
> > > > +       uint16_t high_timer, low_timer;
> > > > +
> > > > +       if (sscanf(rssi_str, "%hd,%hu,%hd,%hu", &high_threshold, &high_timer,
> > > > +                                       &low_threshold, &low_timer) < 4)
> > > > +               return NULL;
> > > > +
> > > > +       rssi = g_malloc0(sizeof(struct rssi_setting));
> > > > +
> > > > +       if (!rssi) {
> > > > +               bt_shell_printf("Failed to allocate rssi_setting");
> > > > +               bt_shell_noninteractive_quit(EXIT_FAILURE);
> > > > +               return NULL;
> > > > +       }
> > > > +
> > > > +       rssi->high_threshold = high_threshold;
> > > > +       rssi->high_timer = high_timer;
> > > > +       rssi->low_threshold = low_threshold;
> > > > +       rssi->low_timer = low_timer;
> > > > +
> > > > +       return rssi;
> > > > +}
> > > > +
> > > > +static struct pattern *parse_pattern(char *pattern)
> > > > +{
> > > > +       uint8_t start_pos, ad_data_type;
> > > > +       char content_str[BT_AD_MAX_DATA_LEN];
> > > > +       struct pattern *pat;
> > > > +
> > > > +       if (sscanf(pattern, "%hhu,%hhu,%s", &start_pos, &ad_data_type,
> > > > +                                                       content_str) < 3)
> > > > +               return NULL;
> > > > +
> > > > +       pat = g_malloc0(sizeof(struct pattern));
> > > > +
> > > > +       if (!pat) {
> > > > +               bt_shell_printf("Failed to allocate pattern");
> > > > +               bt_shell_noninteractive_quit(EXIT_FAILURE);
> > > > +               return NULL;
> > > > +       }
> > > > +
> > > > +       pat->start_pos = start_pos;
> > > > +       pat->ad_data_type = ad_data_type;
> > > > +       pat->content_len = str2bytearray(content_str, pat->content);
> > > > +       if (pat->content_len == 0) {
> > > > +               free_pattern(pat);
> > > > +               return NULL;
> > > > +       }
> > > > +
> > > > +       return pat;
> > > > +}
> > > > +
> > > > +static GSList *parse_patterns(char *pattern_list[], int num)
> > > > +{
> > > > +       GSList *patterns = NULL;
> > > > +       int cnt;
> > > > +
> > > > +       if (num == 0) {
> > > > +               bt_shell_printf("No pattern provided\n");
> > > > +               return NULL;
> > > > +       }
> > > > +
> > > > +       for (cnt = 0; cnt < num; cnt++) {
> > > > +               struct pattern *pattern;
> > > > +
> > > > +               pattern = parse_pattern(pattern_list[cnt]);
> > > > +               if (pattern == NULL) {
> > > > +                       g_slist_free_full(patterns, free_pattern);
> > > > +                       return NULL;
> > > > +               }
> > > > +               patterns = g_slist_append(patterns, pattern);
> > > > +       }
> > > > +
> > > > +       return patterns;
> > > > +}
> > > > +
> > > > +static gint cmp_adv_monitor_with_idx(gconstpointer a, gconstpointer b)
> > > > +{
> > > > +       const struct adv_monitor *adv_monitor = a;
> > > > +       uint8_t idx = *(uint8_t *)b;
> > > > +
> > > > +       return adv_monitor->idx != idx;
> > > > +}
> > > > +
> > > > +static struct adv_monitor *find_adv_monitor_with_idx(uint8_t monitor_idx)
> > > > +{
> > > > +       GSList *list;
> > > > +
> > > > +       list = g_slist_find_custom(adv_mons, &monitor_idx,
> > > > +                                               cmp_adv_monitor_with_idx);
> > > > +
> > > > +       if (list)
> > > > +               return (struct adv_monitor *)list->data;
> > > > +       return NULL;
> > > > +}
> > > > +
> > > > +static void print_bytearray(char *prefix, uint8_t *arr, uint8_t len)
> > > > +{
> > > > +       int idx;
> > > > +
> > > > +       bt_shell_printf("%s", prefix);
> > > > +       for (idx = 0; idx < len; idx++)
> > > > +               bt_shell_printf("%02hhx", arr[idx]);
> > > > +       bt_shell_printf("\n");
> > > > +}
> > > > +
> > > > +static void print_adv_monitor(struct adv_monitor *adv_monitor)
> > > > +{
> > > > +       GSList *l;
> > > > +
> > > > +       bt_shell_printf("Advertisement Monitor %d\n", adv_monitor->idx);
> > > > +       bt_shell_printf("\ttype: %s\n", adv_monitor->type);
> > > > +       if (adv_monitor->rssi) {
> > > > +               bt_shell_printf("\trssi:\n");
> > > > +               bt_shell_printf("\t\thigh threshold: %hd\n",
> > > > +                                       adv_monitor->rssi->high_threshold);
> > > > +               bt_shell_printf("\t\thigh threshold timer: %hu\n",
> > > > +                                       adv_monitor->rssi->high_timer);
> > > > +               bt_shell_printf("\t\tlow threshold: %hd\n",
> > > > +                                       adv_monitor->rssi->low_threshold);
> > > > +               bt_shell_printf("\t\tlow threshold timer: %hu\n",
> > > > +                                       adv_monitor->rssi->low_timer);
> > > > +       }
> > > > +
> > > > +       if (adv_monitor->patterns) {
> > > > +               int idx = 1;
> > > > +
> > > > +               for (l = adv_monitor->patterns; l; l = g_slist_next(l), idx++) {
> > > > +                       struct pattern *pattern = l->data;
> > > > +
> > > > +                       bt_shell_printf("\tpattern %d:\n", idx);
> > > > +                       bt_shell_printf("\t\tstart position: %hhu\n",
> > > > +                                                       pattern->start_pos);
> > > > +                       bt_shell_printf("\t\tAD data type: %hhu\n",
> > > > +                                                       pattern->ad_data_type);
> > > > +                       print_bytearray("\t\tcontent: ", pattern->content,
> > > > +                                                       pattern->content_len);
> > > > +               }
> > > > +       }
> > > > +}
> > > > +
> > > > +void adv_monitor_add_monitor(DBusConnection *conn, int argc, char *argv[])
> > > > +{
> > > > +       struct adv_monitor *adv_monitor;
> > > > +       struct rssi_setting *rssi;
> > > > +       GSList *patterns = NULL;
> > > > +       char *type;
> > > > +
> > > > +       if (g_slist_length(adv_mons) >= UINT8_MAX) {
> > > > +               bt_shell_printf("Number of advertisement monitor exceeds "
> > > > +                               "the limit");
> > > > +               return;
> > > > +       }
> > > > +
> > > > +       while (find_adv_monitor_with_idx(adv_mon_idx))
> > > > +               adv_mon_idx += 1;
> > > > +
> > > > +       type = argv[1];
> > > > +
> > > > +       if (strcmp(argv[2], "-") == 0)
> > > > +               rssi = NULL;
> > > > +       else {
> > > > +               rssi = parse_rssi(argv[2]);
> > > > +               if (rssi == NULL) {
> > > > +                       bt_shell_printf("RSSIThresholdAndTimers malformed\n");
> > > > +                       return;
> > > > +               }
> > > > +       }
> > > > +
> > > > +       patterns = parse_patterns(argv+3, argc-3);
> > > > +       if (patterns == NULL) {
> > > > +               bt_shell_printf("pattern-list malformed\n");
> > > > +               return;
> > > > +       }
> > > > +
> > > > +       adv_monitor = g_malloc0(sizeof(struct adv_monitor));
> > > > +
> > > > +       if (!adv_monitor) {
> > > > +               bt_shell_printf("Failed to allocate adv_monitor");
> > > > +               return bt_shell_noninteractive_quit(EXIT_FAILURE);
> > > > +       }
> > > > +
> > > > +       adv_monitor->idx = adv_mon_idx;
> > > > +       adv_monitor->type = g_strdup(type);
> > > > +       adv_monitor->rssi = rssi;
> > > > +       adv_monitor->patterns = patterns;
> > > > +
> > > > +       adv_mons = g_slist_append(adv_mons, adv_monitor);
> > > > +       bt_shell_printf("Advertisement Monitor %d added\n", adv_monitor->idx);
> > > > +}
> > > > +
> > > > +void adv_monitor_print_monitor(DBusConnection *conn, int monitor_idx)
> > > > +{
> > > > +       struct adv_monitor *adv_monitor;
> > > > +       GSList *l;
> > > > +
> > > > +       if (monitor_idx < 0) {
> > > > +               for (l = adv_mons; l; l = g_slist_next(l)) {
> > > > +                       adv_monitor = l->data;
> > > > +                       print_adv_monitor(adv_monitor);
> > > > +               }
> > > > +               return;
> > > > +       }
> > > > +
> > > > +       adv_monitor = find_adv_monitor_with_idx(monitor_idx);
> > > > +
> > > > +       if (adv_monitor == NULL) {
> > > > +               bt_shell_printf("Can't find monitor with index %d\n",
> > > > +                                                               monitor_idx);
> > > > +               return;
> > > > +       }
> > > > +
> > > > +       print_adv_monitor(adv_monitor);
> > > > +}
> > > > +
> > > > +void adv_monitor_remove_monitor(DBusConnection *conn, int monitor_idx)
> > > > +{
> > > > +       struct adv_monitor *adv_monitor;
> > > > +
> > > > +       if (monitor_idx < 0) {
> > > > +               g_slist_free_full(g_steal_pointer(&adv_mons), free_adv_monitor);
> > > > +               return;
> > > > +       }
> > > > +
> > > > +       adv_monitor = find_adv_monitor_with_idx(monitor_idx);
> > > > +       if (adv_monitor == NULL) {
> > > > +               bt_shell_printf("Can't find monitor with index %d\n",
> > > > +                                                               monitor_idx);
> > > > +               return;
> > > > +       }
> > > > +
> > > > +       adv_mons = g_slist_remove(adv_mons, adv_monitor);
> > > > +       free_adv_monitor(adv_monitor);
> > > > +       bt_shell_printf("Monitor %d deleted\n", monitor_idx);
> > > > +}
> > > > +
> > > > +static void print_supported_list(GSList *list)
> > > > +{
> > > > +       GSList *iter;
> > > > +
> > > > +       for (iter = list; iter; iter = g_slist_next(iter)) {
> > > > +               char *data = iter->data;
> > > > +
> > > > +               printf(" %s", data);
> > > > +       }
> > > > +}
> > > > +
> > > > +void adv_monitor_get_supported_info(void)
> > > > +{
> > > > +       bt_shell_printf("Supported Features:");
> > > > +       print_supported_list(manager.supported_features);
> > > > +       bt_shell_printf("\n");
> > > > +
> > > > +       bt_shell_printf("Supported Moniter Types:");
> > > > +       print_supported_list(manager.supported_types);
> > > > +       bt_shell_printf("\n");
> > > > +}
> > > > diff --git a/client/advertisement_monitor.h b/client/advertisement_monitor.h
> > > > index 77b0b62c6..f2a0caf77 100644
> > > > --- a/client/advertisement_monitor.h
> > > > +++ b/client/advertisement_monitor.h
> > > > @@ -21,3 +21,7 @@ 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);
> > > > +void adv_monitor_add_monitor(DBusConnection *conn, int argc, char *argv[]);
> > > > +void adv_monitor_print_monitor(DBusConnection *conn, int monitor_idx);
> > > > +void adv_monitor_remove_monitor(DBusConnection *conn, int monitor_idx);
> > > > +void adv_monitor_get_supported_info(void);
> > > > diff --git a/client/main.c b/client/main.c
> > > > index 7ddd13aa0..2b63ee62a 100644
> > > > --- a/client/main.c
> > > > +++ b/client/main.c
> > > > @@ -2686,6 +2686,53 @@ static void cmd_ad_clear(int argc, char *argv[])
> > > >                 return bt_shell_noninteractive_quit(EXIT_FAILURE);
> > > >  }
> > > >
> > > > +static void print_add_monitor_usage(void)
> > > > +{
> > > > +       bt_shell_usage();
> > > > +       bt_shell_printf("RSSIThresholdAndTimers format:\n"
> > > > +                       "\t<high-rssi>,<high-timeout>,<low-rssi>,<low-timeout>\n"
> > > > +                       "\tor single '-' for not using RSSI as filter\n");
> > > > +       bt_shell_printf("pattern format:\n"
> > > > +                       "\t<start_position>,<ad_data_type>,<content_of_pattern>\n");
> > > > +       bt_shell_printf("e.g.\n"
> > > > +                       "\tadd-pattern-monitor or_patterns -10,10,-20,20 1,2,01ab55\n");
> > >
> > > btshell parameters are space separated not comma.
> >
> > The reason we use commas is because RSSIThresholdAndTimers is an
> > all-or-none property, it's clearer for users to either input all the
> > four parameters or a single '-' to leave it unset.
> > For a similar reason, a valid pattern requires all of the three
> > parameters provided. If we want to avoid comma for maintaining
> > consistency, we could have command usage like:
> >
> > add-pattern or_patterns -10 10 -20 20 1 2 01ab55     // single content
> > with RSSI filter
> > add-pattern or_patterns - 1 2 01ab55                         // single
> > content without RSSI filter
> >
> > Let me know if this is OK with you and other suggestions are welcome.
>
> Actually it might be better to have good defaults on these or have
> something like:
>
> <rssi-range=low,high> then parse the input to check if comma has been
> passed otherwise we can assume only the high (or low if that value
> means better signal) rssi has been passed.
> <timeout=low,high> same as above.
>
> Also I would consider merging the type into the command itself:
>
> add-or-pattern-rssi -10,-20 10,1 01ab55
> add-or-pattern 1 2 01ab55
>
> That way the bt_shell parsing of the command description can actually
> detect if enough parameters have been given since we can't really have
> short of overloading command with different arguments as it would make
> it rather confusing to use since there are no types to detect which
> version the user is trying to use.
>
> > >
> > > > +}
> > > > +
> > > > +static void cmd_adv_monitor_add_monitor(int argc, char *argv[])
> > > > +{
> > > > +       if (argc < 3)
> > > > +               print_add_monitor_usage();
> > > > +       else
> > > > +               adv_monitor_add_monitor(dbus_conn, argc, argv);
> > > > +}
> > > > +
> > > > +static void cmd_adv_monitor_print_monitor(int argc, char *argv[])
> > > > +{
> > > > +       int monitor_idx;
> > > > +
> > > > +       if (strcmp(argv[1], "all") == 0)
> > > > +               monitor_idx = -1;
> > > > +       else
> > > > +               monitor_idx = atoi(argv[1]);
> > > > +       adv_monitor_print_monitor(dbus_conn, monitor_idx);
> > > > +}
> > > > +
> > > > +static void cmd_adv_monitor_remove_monitor(int argc, char *argv[])
> > > > +{
> > > > +       int monitor_idx;
> > > > +
> > > > +       if (strcmp(argv[1], "all") == 0)
> > > > +               monitor_idx = -1;
> > > > +       else
> > > > +               monitor_idx = atoi(argv[1]);
> > > > +       adv_monitor_remove_monitor(dbus_conn, monitor_idx);
> > > > +}
> > > > +
> > > > +static void cmd_adv_monitor_get_supported_info(int argc, char *argv[])
> > > > +{
> > > > +       adv_monitor_get_supported_info();
> > > > +}
> > > > +
> > > >  static const struct bt_shell_menu advertise_menu = {
> > > >         .name = "advertise",
> > > >         .desc = "Advertise Options Submenu",
> > > > @@ -2722,6 +2769,28 @@ static const struct bt_shell_menu advertise_menu = {
> > > >         { } },
> > > >  };
> > > >
> > > > +static const struct bt_shell_menu advertise_monitor_menu = {
> > > > +       .name = "advmon",
> > >
> > > I'd use monitor instead of advmon here.
> >
> > Will address these in the next patch. Thanks!
> >
> > >
> > > > +       .desc = "Advertisement Monitor Options Submenu",
> > > > +       .entries = {
> > > > +       { "add-pattern-monitor", "<type-of-monitor/help> "
> > > > +                               "[RSSIThresholdAndTimers] "
> > > > +                               "[patterns=pattern1 pattern2 ...]",
> > > > +                               cmd_adv_monitor_add_monitor,
> > > > +                               "Add pattern monitor" },
> > > > +       { "get-pattern-monitor", "<monitor-id/all>",
> > > > +                               cmd_adv_monitor_print_monitor,
> > > > +                               "Get advertisement monitor" },
> > > > +       { "remove-pattern-monitor", "<monitor-id/all>",
> > > > +                               cmd_adv_monitor_remove_monitor,
> > > > +                               "Remove advertisement monitor" },
> > > > +       { "get-supported-info", NULL,
> > > > +                               cmd_adv_monitor_get_supported_info,
> > > > +                               "Get advertisement manager supported "
> > > > +                               "features and supported monitor types" },
> > >
> > > We could probably drop the monitor from the commands here which should
> > > leave us at:
> > >
> > > add-pattern
> > > get-pattern
> > > remote-pattern
> > > supported-info
> > >
> > > > +       { } },
> > > > +};
> > > > +
> > > >  static const struct bt_shell_menu scan_menu = {
> > > >         .name = "scan",
> > > >         .desc = "Scan Options Submenu",
> > > > @@ -2897,6 +2966,7 @@ int main(int argc, char *argv[])
> > > >         bt_shell_init(argc, argv, &opt);
> > > >         bt_shell_set_menu(&main_menu);
> > > >         bt_shell_add_submenu(&advertise_menu);
> > > > +       bt_shell_add_submenu(&advertise_monitor_menu);
> > > >         bt_shell_add_submenu(&scan_menu);
> > > >         bt_shell_add_submenu(&gatt_menu);
> > > >         bt_shell_set_prompt(PROMPT_OFF);
> > > > --
> > > > 2.28.0.220.ged08abb693-goog
> > > >
> > >
> > >
> > > --
> > > Luiz Augusto von Dentz
>
>
>
> --
> Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-09-09  7:30 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-19  4:11 [BlueZ PATCH v1 1/4] client: Implement basic interface of ADV monitor in bluetoothctl Howard Chung
2020-08-19  4:11 ` [BlueZ PATCH v1 2/4] client: Implement more interfaces " Howard Chung
2020-09-08 17:06   ` Luiz Augusto von Dentz
2020-09-09  4:01     ` Yun-hao Chung
2020-09-09  4:56       ` Luiz Augusto von Dentz
2020-09-09  7:30         ` Yun-hao Chung
2020-08-19  4:11 ` [BlueZ PATCH v1 3/4] client: Expose ADV monitor objects Howard Chung
2020-08-19  4:11 ` [BlueZ PATCH v1 4/4] core: Add AdvertisementMonitor to bluetooth.conf Howard Chung

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.