linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [Bluez PATCH v3 0/5] Support advertising monitor add pattern with RSSI opcode
@ 2021-01-14  7:44 Archie Pusaka
  2021-01-14  7:44 ` [Bluez PATCH v3 1/5] lib/mgmt: Adding Add Adv Patterns Monitor " Archie Pusaka
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Archie Pusaka @ 2021-01-14  7:44 UTC (permalink / raw)
  To: linux-bluetooth, Luiz Augusto von Dentz; +Cc: CrosBT Upstreaming, Archie Pusaka

From: Archie Pusaka <apusaka@chromium.org>

Hi linux-bluetooth,

This series of patches adds a new MGMT command for adding a monitor
with RSSI parameter. Changes are focused on passing parameters to
the kernel via btmgmt and bluetoothctl.

PTAL and thanks for your feedback!
Archie

Changes in v3:
* split the struct RSSIThresholdsAndTimers

Changes in v2:
* Remove trailing period and fix order of mgmt parameter

Archie Pusaka (5):
  lib/mgmt: Adding Add Adv Patterns Monitor RSSI opcode
  src/adv_monitor: add monitor with rssi support for mgmt
  btmgmt: advmon add rssi support
  bluetoothctl: advmon rssi support for mgmt
  monitor: Decode add advmon with RSSI parameter

 client/adv_monitor.c | 162 ++++++++++++++++----------
 client/adv_monitor.h |   1 +
 client/main.c        |  29 +++--
 lib/mgmt.h           |  15 +++
 monitor/packet.c     |  43 ++++++-
 src/adv_monitor.c    | 267 ++++++++++++++++++++++++++++---------------
 tools/btmgmt.c       | 160 +++++++++++++++++++++-----
 7 files changed, 475 insertions(+), 202 deletions(-)

-- 
2.30.0.284.gd98b1dd5eaa7-goog


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

* [Bluez PATCH v3 1/5] lib/mgmt: Adding Add Adv Patterns Monitor RSSI opcode
  2021-01-14  7:44 [Bluez PATCH v3 0/5] Support advertising monitor add pattern with RSSI opcode Archie Pusaka
@ 2021-01-14  7:44 ` Archie Pusaka
  2021-01-14  8:28   ` Support advertising monitor add pattern with " bluez.test.bot
  2021-01-14  7:44 ` [Bluez PATCH v3 2/5] src/adv_monitor: add monitor with rssi support for mgmt Archie Pusaka
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Archie Pusaka @ 2021-01-14  7:44 UTC (permalink / raw)
  To: linux-bluetooth, Luiz Augusto von Dentz
  Cc: CrosBT Upstreaming, Archie Pusaka, Miao-chen Chou, Yun-Hao Chung,
	Manish Mandlik

From: Archie Pusaka <apusaka@chromium.org>

The new op is to utilize RSSI in advertisement monitor

Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
Reviewed-by: Yun-Hao Chung <howardchung@google.com>
Reviewed-by: Manish Mandlik <mmandlik@chromium.org>
---

(no changes since v2)

Changes in v2:
* Remove trailing period and fix order of mgmt parameter

 lib/mgmt.h | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/lib/mgmt.h b/lib/mgmt.h
index f37f7e6540..76a03c9c24 100644
--- a/lib/mgmt.h
+++ b/lib/mgmt.h
@@ -738,6 +738,21 @@ struct mgmt_rp_add_ext_adv_data {
 	uint8_t	instance;
 } __packed;
 
+struct mgmt_adv_rssi_thresholds {
+	int8_t   high_threshold;
+	uint16_t high_threshold_timeout;
+	int8_t   low_threshold;
+	uint16_t low_threshold_timeout;
+	uint8_t  sampling_period;
+} __packed;
+
+#define MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI	0x0056
+struct mgmt_cp_add_adv_patterns_monitor_rssi {
+	struct mgmt_adv_rssi_thresholds rssi;
+	uint8_t pattern_count;
+	struct mgmt_adv_pattern patterns[0];
+} __packed;
+
 #define MGMT_EV_CMD_COMPLETE		0x0001
 struct mgmt_ev_cmd_complete {
 	uint16_t opcode;
-- 
2.30.0.284.gd98b1dd5eaa7-goog


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

* [Bluez PATCH v3 2/5] src/adv_monitor: add monitor with rssi support for mgmt
  2021-01-14  7:44 [Bluez PATCH v3 0/5] Support advertising monitor add pattern with RSSI opcode Archie Pusaka
  2021-01-14  7:44 ` [Bluez PATCH v3 1/5] lib/mgmt: Adding Add Adv Patterns Monitor " Archie Pusaka
@ 2021-01-14  7:44 ` Archie Pusaka
  2021-01-14  7:44 ` [Bluez PATCH v3 3/5] btmgmt: advmon add rssi support Archie Pusaka
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Archie Pusaka @ 2021-01-14  7:44 UTC (permalink / raw)
  To: linux-bluetooth, Luiz Augusto von Dentz
  Cc: CrosBT Upstreaming, Archie Pusaka, Yun-Hao Chung

From: Archie Pusaka <apusaka@chromium.org>

Using the new opcode MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI to
monitor advertisement according to some RSSI criteria.

Reviewed-by: Yun-Hao Chung <howardchung@google.com>
---

Changes in v3:
* split the struct RSSIThresholdsAndTimers

 src/adv_monitor.c | 267 ++++++++++++++++++++++++++++++----------------
 1 file changed, 174 insertions(+), 93 deletions(-)

diff --git a/src/adv_monitor.c b/src/adv_monitor.c
index 5a0498ec2e..54751db0b5 100644
--- a/src/adv_monitor.c
+++ b/src/adv_monitor.c
@@ -41,9 +41,14 @@
 #define ADV_MONITOR_UNSET_RSSI		127	/* dBm */
 #define ADV_MONITOR_MAX_RSSI		20	/* dBm */
 #define ADV_MONITOR_MIN_RSSI		-127	/* dBm */
-#define ADV_MONITOR_UNSET_TIMER		0	/* second */
-#define ADV_MONITOR_MIN_TIMER		1	/* second */
-#define ADV_MONITOR_MAX_TIMER		300	/* second */
+#define ADV_MONITOR_UNSET_TIMEOUT	0	/* second */
+#define ADV_MONITOR_MIN_TIMEOUT		1	/* second */
+#define ADV_MONITOR_MAX_TIMEOUT		300	/* second */
+#define ADV_MONITOR_DEFAULT_LOW_TIMEOUT	5	/* second */
+#define ADV_MONITOR_DEFAULT_HIGH_TIMEOUT 10	/* second */
+#define ADV_MONITOR_UNSET_SAMPLING_PERIOD 256	/* 100 ms */
+#define ADV_MONITOR_MAX_SAMPLING_PERIOD	255	/* 100 ms */
+#define ADV_MONITOR_DEFAULT_SAMPLING_PERIOD 0	/* 100 ms */
 
 struct btd_adv_monitor_manager {
 	struct btd_adapter *adapter;
@@ -95,6 +100,10 @@ struct adv_monitor {
 	uint16_t high_rssi_timeout;	/* High RSSI threshold timeout */
 	int8_t low_rssi;		/* Low RSSI threshold */
 	uint16_t low_rssi_timeout;	/* Low RSSI threshold timeout */
+	uint16_t sampling_period;	/* Merge packets in the same timeslot.
+					 * Currenly unimplemented in user space.
+					 * Used only to pass data to kernel.
+					 */
 	struct queue *devices;		/* List of adv_monitor_device objects */
 
 	enum monitor_type type;		/* MONITOR_TYPE_* */
@@ -360,9 +369,10 @@ static struct adv_monitor *monitor_new(struct adv_monitor_app *app,
 	monitor->state = MONITOR_STATE_NEW;
 
 	monitor->high_rssi = ADV_MONITOR_UNSET_RSSI;
-	monitor->high_rssi_timeout = ADV_MONITOR_UNSET_TIMER;
+	monitor->high_rssi_timeout = ADV_MONITOR_UNSET_TIMEOUT;
 	monitor->low_rssi = ADV_MONITOR_UNSET_RSSI;
-	monitor->low_rssi_timeout = ADV_MONITOR_UNSET_TIMER;
+	monitor->low_rssi_timeout = ADV_MONITOR_UNSET_TIMEOUT;
+	monitor->sampling_period = ADV_MONITOR_UNSET_SAMPLING_PERIOD;
 	monitor->devices = queue_new();
 
 	monitor->type = MONITOR_TYPE_NONE;
@@ -423,103 +433,119 @@ failed:
 	return false;
 }
 
-/* Retrieves RSSIThresholdsAndTimers from the remote Adv Monitor object,
+/* Retrieves RSSI thresholds and timeouts from the remote Adv Monitor object,
  * verifies the values and update the local Adv Monitor
  */
 static bool parse_rssi_and_timeout(struct adv_monitor *monitor,
 					const char *path)
 {
-	DBusMessageIter prop_struct, iter;
-	int16_t h_rssi, l_rssi;
-	uint16_t h_rssi_timer, l_rssi_timer;
+	DBusMessageIter iter;
+	GDBusProxy *proxy = monitor->proxy;
+	int16_t h_rssi = ADV_MONITOR_UNSET_RSSI;
+	int16_t l_rssi = ADV_MONITOR_UNSET_RSSI;
+	uint16_t h_rssi_timeout = ADV_MONITOR_UNSET_TIMEOUT;
+	uint16_t l_rssi_timeout = ADV_MONITOR_UNSET_TIMEOUT;
+	int16_t sampling_period = ADV_MONITOR_UNSET_SAMPLING_PERIOD;
 	uint16_t adapter_id = monitor->app->manager->adapter_id;
 
-	/* Property RSSIThresholdsAndTimers is optional */
-	if (!g_dbus_proxy_get_property(monitor->proxy,
-					"RSSIThresholdsAndTimers",
-					&prop_struct)) {
-		DBG("Adv Monitor at path %s provides no RSSI thresholds and "
-			"timeouts", path);
-		return true;
+	/* Extract RSSIHighThreshold */
+	if (g_dbus_proxy_get_property(proxy, "RSSIHighThreshold", &iter)) {
+		if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_INT16)
+			goto failed;
+		dbus_message_iter_get_basic(&iter, &h_rssi);
 	}
 
-	if (dbus_message_iter_get_arg_type(&prop_struct) != DBUS_TYPE_STRUCT)
-		goto failed;
-
-	dbus_message_iter_recurse(&prop_struct, &iter);
-
-	/* Extract HighRSSIThreshold */
-	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_INT16)
-		goto failed;
-	dbus_message_iter_get_basic(&iter, &h_rssi);
-	if (!dbus_message_iter_next(&iter))
-		goto failed;
+	/* Extract RSSIHighTimeout */
+	if (g_dbus_proxy_get_property(proxy, "RSSIHighTimeout", &iter)) {
+		if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_UINT16)
+			goto failed;
+		dbus_message_iter_get_basic(&iter, &h_rssi_timeout);
+	}
 
-	/* Extract HighRSSIThresholdTimer */
-	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_UINT16)
-		goto failed;
-	dbus_message_iter_get_basic(&iter, &h_rssi_timer);
-	if (!dbus_message_iter_next(&iter))
-		goto failed;
+	/* Extract RSSILowThreshold */
+	if (g_dbus_proxy_get_property(proxy, "RSSILowThreshold", &iter)) {
+		if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_INT16)
+			goto failed;
+		dbus_message_iter_get_basic(&iter, &l_rssi);
+	}
 
-	/* Extract LowRSSIThreshold */
-	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_INT16)
-		goto failed;
-	dbus_message_iter_get_basic(&iter, &l_rssi);
-	if (!dbus_message_iter_next(&iter))
-		goto failed;
+	/* Extract RSSILowTimeout */
+	if (g_dbus_proxy_get_property(proxy, "RSSILowTimeout", &iter)) {
+		if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_UINT16)
+			goto failed;
+		dbus_message_iter_get_basic(&iter, &l_rssi_timeout);
+	}
 
-	/* Extract LowRSSIThresholdTimer */
-	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_UINT16)
-		goto failed;
-	dbus_message_iter_get_basic(&iter, &l_rssi_timer);
+	/* Extract RSSISamplingPeriod */
+	if (g_dbus_proxy_get_property(proxy, "RSSISamplingPeriod", &iter)) {
+		if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_UINT16)
+			goto failed;
+		dbus_message_iter_get_basic(&iter, &sampling_period);
+	}
 
-	/* Verify the values of RSSIs and their timers. For simplicity, we
-	 * enforce the all-or-none rule to these fields. In other words, either
-	 * all are set to the unset values or all are set within valid ranges.
+	/* Verify the values of RSSIs and their timeouts. All fields should be
+	 * either set to the unset values or are set within valid ranges.
+	 * If the fields are only partially set, we would try our best to fill
+	 * in with some sane values.
 	 */
 	if (h_rssi == ADV_MONITOR_UNSET_RSSI &&
 		l_rssi == ADV_MONITOR_UNSET_RSSI &&
-		h_rssi_timer == ADV_MONITOR_UNSET_TIMER &&
-		l_rssi_timer == ADV_MONITOR_UNSET_TIMER) {
+		h_rssi_timeout == ADV_MONITOR_UNSET_TIMEOUT &&
+		l_rssi_timeout == ADV_MONITOR_UNSET_TIMEOUT &&
+		sampling_period == ADV_MONITOR_UNSET_SAMPLING_PERIOD) {
 		goto done;
 	}
 
+	if (l_rssi == ADV_MONITOR_UNSET_RSSI)
+		l_rssi = ADV_MONITOR_MIN_RSSI;
+
+	if (h_rssi == ADV_MONITOR_UNSET_RSSI)
+		h_rssi = l_rssi;
+
+	if (l_rssi_timeout == ADV_MONITOR_UNSET_TIMEOUT)
+		l_rssi_timeout = ADV_MONITOR_DEFAULT_LOW_TIMEOUT;
+
+	if (h_rssi_timeout == ADV_MONITOR_UNSET_TIMEOUT)
+		h_rssi_timeout = ADV_MONITOR_DEFAULT_HIGH_TIMEOUT;
+
+	if (sampling_period == ADV_MONITOR_UNSET_SAMPLING_PERIOD)
+		sampling_period = ADV_MONITOR_DEFAULT_SAMPLING_PERIOD;
+
 	if (h_rssi < ADV_MONITOR_MIN_RSSI || h_rssi > ADV_MONITOR_MAX_RSSI ||
 		l_rssi < ADV_MONITOR_MIN_RSSI ||
-		l_rssi > ADV_MONITOR_MAX_RSSI || h_rssi <= l_rssi) {
+		l_rssi > ADV_MONITOR_MAX_RSSI || h_rssi < l_rssi) {
 		goto failed;
 	}
 
-	if (h_rssi_timer < ADV_MONITOR_MIN_TIMER ||
-		h_rssi_timer > ADV_MONITOR_MAX_TIMER ||
-		l_rssi_timer < ADV_MONITOR_MIN_TIMER ||
-		l_rssi_timer > ADV_MONITOR_MAX_TIMER) {
+	if (h_rssi_timeout < ADV_MONITOR_MIN_TIMEOUT ||
+		h_rssi_timeout > ADV_MONITOR_MAX_TIMEOUT ||
+		l_rssi_timeout < ADV_MONITOR_MIN_TIMEOUT ||
+		l_rssi_timeout > ADV_MONITOR_MAX_TIMEOUT) {
 		goto failed;
 	}
 
+	if (sampling_period > ADV_MONITOR_MAX_SAMPLING_PERIOD)
+		goto failed;
+
 	monitor->high_rssi = h_rssi;
 	monitor->low_rssi = l_rssi;
-	monitor->high_rssi_timeout = h_rssi_timer;
-	monitor->low_rssi_timeout = l_rssi_timer;
+	monitor->high_rssi_timeout = h_rssi_timeout;
+	monitor->low_rssi_timeout = l_rssi_timeout;
+	monitor->sampling_period = sampling_period;
 
 done:
 	DBG("Adv Monitor at %s initiated with high RSSI threshold %d, high "
 		"RSSI threshold timeout %d, low RSSI threshold %d, low RSSI "
-		"threshold timeout %d", path, monitor->high_rssi,
-		monitor->high_rssi_timeout, monitor->low_rssi,
-		monitor->low_rssi_timeout);
+		"threshold timeout %d, sampling period %d", path,
+		monitor->high_rssi, monitor->high_rssi_timeout,
+		monitor->low_rssi, monitor->low_rssi_timeout,
+		monitor->sampling_period);
 
 	return true;
 
 failed:
-	monitor->high_rssi = ADV_MONITOR_UNSET_RSSI;
-	monitor->low_rssi = ADV_MONITOR_UNSET_RSSI;
-	monitor->high_rssi_timeout = ADV_MONITOR_UNSET_TIMER;
-	monitor->low_rssi_timeout = ADV_MONITOR_UNSET_TIMER;
-
 	btd_error(adapter_id,
-			"Invalid argument of property RSSIThresholdsAndTimers "
+			"Invalid argument of RSSI thresholds and timeouts "
 			"of the Adv Monitor at path %s",
 			path);
 
@@ -673,16 +699,88 @@ static void add_adv_patterns_monitor_cb(uint8_t status, uint16_t length,
 	DBG("Adv monitor with handle:0x%04x added", monitor->monitor_handle);
 }
 
-static void monitor_copy_patterns(void *data, void *user_data)
+static bool monitor_rssi_is_unset(struct adv_monitor *monitor)
 {
-	struct bt_ad_pattern *pattern = data;
-	struct mgmt_cp_add_adv_monitor *cp = user_data;
+	return monitor->high_rssi == ADV_MONITOR_UNSET_RSSI &&
+		monitor->low_rssi == ADV_MONITOR_UNSET_RSSI &&
+		monitor->high_rssi_timeout == ADV_MONITOR_UNSET_TIMEOUT &&
+		monitor->low_rssi_timeout == ADV_MONITOR_UNSET_TIMEOUT &&
+		monitor->sampling_period == ADV_MONITOR_UNSET_SAMPLING_PERIOD;
+}
 
-	if (!pattern)
-		return;
+/* sends MGMT_OP_ADD_ADV_PATTERNS_MONITOR */
+static bool monitor_send_add_pattern(struct adv_monitor *monitor)
+{
+	struct mgmt_cp_add_adv_monitor *cp = NULL;
+	uint8_t pattern_count, cp_len;
+	const struct queue_entry *e;
+	bool success = true;
+
+	pattern_count = queue_length(monitor->patterns);
+	cp_len = sizeof(*cp) + pattern_count * sizeof(struct mgmt_adv_pattern);
 
-	memcpy(cp->patterns + cp->pattern_count, pattern, sizeof(*pattern));
-	cp->pattern_count++;
+	cp = malloc0(cp_len);
+	if (!cp)
+		return false;
+
+	for (e = queue_get_entries(monitor->patterns); e; e = e->next) {
+		struct bt_ad_pattern *pattern = e->data;
+
+		memcpy(&cp->patterns[cp->pattern_count++], pattern,
+							sizeof(*pattern));
+	}
+
+	if (!mgmt_send(monitor->app->manager->mgmt,
+			MGMT_OP_ADD_ADV_PATTERNS_MONITOR,
+			monitor->app->manager->adapter_id, cp_len, cp,
+			add_adv_patterns_monitor_cb, monitor, NULL)) {
+		error("Unable to send Add Adv Patterns Monitor command");
+		success = false;
+	}
+
+	free(cp);
+	return success;
+}
+
+/* sends MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI */
+static bool monitor_send_add_pattern_rssi(struct adv_monitor *monitor)
+{
+	struct mgmt_cp_add_adv_patterns_monitor_rssi *cp = NULL;
+	uint8_t pattern_count, cp_len;
+	const struct queue_entry *e;
+	bool success = true;
+
+	pattern_count = queue_length(monitor->patterns);
+	cp_len = sizeof(*cp) + pattern_count * sizeof(struct mgmt_adv_pattern);
+
+	cp = malloc0(cp_len);
+	if (!cp)
+		return false;
+
+	cp->rssi.high_threshold = monitor->high_rssi;
+	/* High threshold timeout is unsupported in kernel. Value must be 0. */
+	cp->rssi.high_threshold_timeout = 0;
+	cp->rssi.low_threshold = monitor->low_rssi;
+	cp->rssi.low_threshold_timeout = htobs(monitor->low_rssi_timeout);
+	cp->rssi.sampling_period = monitor->sampling_period;
+
+	for (e = queue_get_entries(monitor->patterns); e; e = e->next) {
+		struct bt_ad_pattern *pattern = e->data;
+
+		memcpy(&cp->patterns[cp->pattern_count++], pattern,
+							sizeof(*pattern));
+	}
+
+	if (!mgmt_send(monitor->app->manager->mgmt,
+			MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI,
+			monitor->app->manager->adapter_id, cp_len, cp,
+			add_adv_patterns_monitor_cb, monitor, NULL)) {
+		error("Unable to send Add Adv Patterns Monitor RSSI command");
+		success = false;
+	}
+
+	free(cp);
+	return success;
 }
 
 /* Handles an Adv Monitor D-Bus proxy added event */
@@ -690,8 +788,6 @@ static void monitor_proxy_added_cb(GDBusProxy *proxy, void *user_data)
 {
 	struct adv_monitor *monitor;
 	struct adv_monitor_app *app = user_data;
-	struct mgmt_cp_add_adv_monitor *cp = NULL;
-	uint8_t pattern_count, cp_len;
 	uint16_t adapter_id = app->manager->adapter_id;
 	const char *path = g_dbus_proxy_get_path(proxy);
 	const char *iface = g_dbus_proxy_get_interface(proxy);
@@ -725,24 +821,12 @@ static void monitor_proxy_added_cb(GDBusProxy *proxy, void *user_data)
 
 	queue_push_tail(app->monitors, monitor);
 
-	pattern_count = queue_length(monitor->patterns);
-	cp_len = sizeof(struct mgmt_cp_add_adv_monitor) +
-			pattern_count * sizeof(struct mgmt_adv_pattern);
-
-	cp = malloc0(cp_len);
-	queue_foreach(monitor->patterns, monitor_copy_patterns, cp);
-
-	if (!mgmt_send(app->manager->mgmt, MGMT_OP_ADD_ADV_PATTERNS_MONITOR,
-			adapter_id, cp_len, cp, add_adv_patterns_monitor_cb,
-			monitor, NULL)) {
-		error("Unable to send Add Adv Patterns Monitor command");
-		goto done;
-	}
+	if (monitor_rssi_is_unset(monitor))
+		monitor_send_add_pattern(monitor);
+	else
+		monitor_send_add_pattern_rssi(monitor);
 
 	DBG("Adv Monitor allocated for the object at path %s", path);
-
-done:
-	free(cp);
 }
 
 /* Handles the removal of an Adv Monitor D-Bus proxy */
@@ -1428,10 +1512,7 @@ static void adv_monitor_filter_rssi(struct adv_monitor *monitor,
 	 * DeviceFound() event without tracking for the RSSI as the Adv has
 	 * already matched the pattern filter.
 	 */
-	if (monitor->high_rssi == ADV_MONITOR_UNSET_RSSI &&
-		monitor->low_rssi == ADV_MONITOR_UNSET_RSSI &&
-		monitor->high_rssi_timeout == ADV_MONITOR_UNSET_TIMER &&
-		monitor->low_rssi_timeout == ADV_MONITOR_UNSET_TIMER) {
+	if (monitor_rssi_is_unset(monitor)) {
 		DBG("Calling DeviceFound() on Adv Monitor of owner %s "
 		    "at path %s", monitor->app->owner, monitor->path);
 
-- 
2.30.0.284.gd98b1dd5eaa7-goog


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

* [Bluez PATCH v3 3/5] btmgmt: advmon add rssi support
  2021-01-14  7:44 [Bluez PATCH v3 0/5] Support advertising monitor add pattern with RSSI opcode Archie Pusaka
  2021-01-14  7:44 ` [Bluez PATCH v3 1/5] lib/mgmt: Adding Add Adv Patterns Monitor " Archie Pusaka
  2021-01-14  7:44 ` [Bluez PATCH v3 2/5] src/adv_monitor: add monitor with rssi support for mgmt Archie Pusaka
@ 2021-01-14  7:44 ` Archie Pusaka
  2021-01-14  7:44 ` [Bluez PATCH v3 4/5] bluetoothctl: advmon rssi support for mgmt Archie Pusaka
  2021-01-14  7:44 ` [Bluez PATCH v3 5/5] monitor: Decode add advmon with RSSI parameter Archie Pusaka
  4 siblings, 0 replies; 9+ messages in thread
From: Archie Pusaka @ 2021-01-14  7:44 UTC (permalink / raw)
  To: linux-bluetooth, Luiz Augusto von Dentz
  Cc: CrosBT Upstreaming, Archie Pusaka, Yun-Hao Chung

From: Archie Pusaka <apusaka@chromium.org>

Using the new opcode MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI to
monitor advertisement according to some RSSI criteria.

Reviewed-by: Yun-Hao Chung <howardchung@google.com>
---

(no changes since v1)

 tools/btmgmt.c | 160 +++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 134 insertions(+), 26 deletions(-)

diff --git a/tools/btmgmt.c b/tools/btmgmt.c
index c0e55f58e6..383e7199e4 100644
--- a/tools/btmgmt.c
+++ b/tools/btmgmt.c
@@ -4858,64 +4858,169 @@ static bool str2pattern(struct mgmt_adv_pattern *pattern, const char *str)
 	return true;
 }
 
-static void advmon_add_usage(void)
+static struct option add_monitor_rssi_options[] = {
+	{ "help",		0, 0, 'h' },
+	{ "high-threshold",	1, 0, 'R' },
+	{ "low-threshold",	1, 0, 'r' },
+	{ "high-timeout",	1, 0, 'T' },
+	{ "low-timeout",	1, 0, 't' },
+	{ "sampling",		1, 0, 's' },
+	{ 0, 0, 0, 0 }
+};
+
+static void advmon_add_pattern_usage(void)
+{
+	bt_shell_usage();
+	print("patterns format:\n"
+		"\t<ad_type:offset:pattern> [patterns]\n"
+		"e.g.:\n"
+		"\tadd-pattern 0:1:c504 ff:a:9a55beef");
+}
+
+static void advmon_add_pattern_rssi_usage(void)
 {
 	bt_shell_usage();
-	print("Monitor Types:\n\t-p <ad_type:offset:pattern>..."
-		"\tPattern Monitor\ne.g.:\n\tadd -p 0:1:c504 1:a:9a55beef");
+	print("RSSI options:\n"
+		"\t -R, --high-threshold <dBm>  "
+			"RSSI high threshold. Default: -70\n"
+		"\t -r, --low-threshold <dBm>   "
+			"RSSI low threshold. Default: -50\n"
+		"\t -T, --high-timeout <s>      "
+			"RSSI high threshold duration. Default: 0\n"
+		"\t -t, --low-timeout <s>       "
+			"RSSI low threshold duration. Default: 5\n"
+		"\t -s, --sampling <N * 100ms>  "
+			"RSSI sampling period. Default: 0\n"
+		"patterns format:\n"
+		"\t<ad_type:offset:pattern> [patterns]\n"
+		"e.g.:\n"
+		"\tadd-pattern-rssi -R 0xb2 -r -102 0:1:c504 ff:a:9a55beef");
 }
 
-static bool advmon_add_pattern(int argc, char **argv)
+static void cmd_advmon_add_pattern(int argc, char **argv)
 {
+	bool success = true;
 	uint16_t index;
 	int i, cp_len;
 	struct mgmt_cp_add_adv_monitor *cp = NULL;
-	bool success = false;
 
-	index = mgmt_index;
-	if (index == MGMT_INDEX_NONE)
-		index = 0;
+	if (!strcmp(argv[1], "-h"))
+		goto done;
 
-	cp_len = sizeof(struct mgmt_cp_add_adv_monitor) +
-			argc * sizeof(struct mgmt_adv_pattern);
+	argc -= 1;
+	argv += 1;
 
+	cp_len = sizeof(*cp) + argc * sizeof(struct mgmt_adv_pattern);
 	cp = malloc0(cp_len);
 	cp->pattern_count = argc;
 
 	for (i = 0; i < argc; i++) {
 		if (!str2pattern(&cp->patterns[i], argv[i])) {
 			error("Failed to parse monitor patterns.");
+			success = false;
 			goto done;
 		}
 	}
 
-	if (!mgmt_send(mgmt, MGMT_OP_ADD_ADV_PATTERNS_MONITOR, index, cp_len,
-					cp, advmon_add_rsp, NULL, NULL)) {
-		error("Unable to send \"Add Advertising Monitor\" command");
+	index = mgmt_index;
+	if (index == MGMT_INDEX_NONE)
+		index = 0;
+
+	if (!mgmt_send(mgmt, MGMT_OP_ADD_ADV_PATTERNS_MONITOR, index,
+				cp_len, cp, advmon_add_rsp, NULL, NULL)) {
+		error("Unable to send Add Advertising Monitor command");
+		success = false;
 		goto done;
 	}
 
-	success = true;
+	free(cp);
+	return;
 
 done:
 	free(cp);
-	return success;
+	advmon_add_pattern_usage();
+	bt_shell_noninteractive_quit(success ? EXIT_SUCCESS : EXIT_FAILURE);
 }
 
-static void cmd_advmon_add(int argc, char **argv)
+static void cmd_advmon_add_pattern_rssi(int argc, char **argv)
 {
-	bool success = false;
+	bool success = true;
+	int opt;
+	int8_t rssi_low = -70;
+	int8_t rssi_high = -50;
+	uint16_t rssi_low_timeout = 5;
+	uint16_t rssi_high_timeout = 0;
+	uint8_t rssi_sampling_period = 0;
+	uint16_t index;
+	int i, cp_len;
+	struct mgmt_cp_add_adv_patterns_monitor_rssi *cp = NULL;
 
-	if (strcasecmp(argv[1], "-p") == 0 && argc > 2) {
-		argc -= 2;
-		argv += 2;
-		success = advmon_add_pattern(argc, argv);
+	while ((opt = getopt_long(argc, argv, "+hr:R:t:T:s:",
+				add_monitor_rssi_options, NULL)) != -1) {
+		switch (opt) {
+		case 'h':
+			goto done;
+		case 'r':
+			rssi_low = strtol(optarg, NULL, 0);
+			break;
+		case 'R':
+			rssi_high = strtol(optarg, NULL, 0);
+			break;
+		case 't':
+			rssi_low_timeout = strtol(optarg, NULL, 0);
+			break;
+		case 'T':
+			rssi_high_timeout = strtol(optarg, NULL, 0);
+			break;
+		case 's':
+			rssi_sampling_period = strtol(optarg, NULL, 0);
+			break;
+		default:
+			success = false;
+			goto done;
+		}
 	}
 
-	if (!success) {
-		advmon_add_usage();
-		bt_shell_noninteractive_quit(EXIT_FAILURE);
+	argc -= optind;
+	argv += optind;
+	optind = 0;
+
+	cp_len = sizeof(*cp) + argc * sizeof(struct mgmt_adv_pattern);
+	cp = malloc0(cp_len);
+	cp->pattern_count = argc;
+	cp->rssi.high_threshold = rssi_high;
+	cp->rssi.low_threshold = rssi_low;
+	cp->rssi.high_threshold_timeout = htobs(rssi_high_timeout);
+	cp->rssi.low_threshold_timeout = htobs(rssi_low_timeout);
+	cp->rssi.sampling_period = rssi_sampling_period;
+
+	for (i = 0; i < argc; i++) {
+		if (!str2pattern(&cp->patterns[i], argv[i])) {
+			error("Failed to parse monitor patterns.");
+			success = false;
+			goto done;
+		}
+	}
+
+	index = mgmt_index;
+	if (index == MGMT_INDEX_NONE)
+		index = 0;
+
+	if (!mgmt_send(mgmt, MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI, index,
+				cp_len, cp, advmon_add_rsp, NULL, NULL)) {
+		error("Unable to send Add Advertising Monitor RSSI command");
+		success = false;
+		goto done;
 	}
+
+	free(cp);
+	return;
+
+done:
+	free(cp);
+	optind = 0;
+	advmon_add_pattern_rssi_usage();
+	bt_shell_noninteractive_quit(success ? EXIT_SUCCESS : EXIT_FAILURE);
 }
 
 static void advmon_remove_rsp(uint8_t status, uint16_t len, const void *param,
@@ -5037,8 +5142,11 @@ static const struct bt_shell_menu monitor_menu = {
 					"features"			},
 	{ "remove",		"<handle>",
 		cmd_advmon_remove,	"Remove advertisement monitor "	},
-	{ "add",		"<-p|-h> [options...]",
-		cmd_advmon_add,		"Add advertisement monitor"	},
+	{ "add-pattern",	"[-h] <patterns>",
+		cmd_advmon_add_pattern,	"Add advertisement monitor pattern" },
+	{ "add-pattern-rssi",	"[options] <patterns>",
+		cmd_advmon_add_pattern_rssi,
+		"Add advertisement monitor pattern with RSSI options"    },
 	{ } },
 };
 
-- 
2.30.0.284.gd98b1dd5eaa7-goog


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

* [Bluez PATCH v3 4/5] bluetoothctl: advmon rssi support for mgmt
  2021-01-14  7:44 [Bluez PATCH v3 0/5] Support advertising monitor add pattern with RSSI opcode Archie Pusaka
                   ` (2 preceding siblings ...)
  2021-01-14  7:44 ` [Bluez PATCH v3 3/5] btmgmt: advmon add rssi support Archie Pusaka
@ 2021-01-14  7:44 ` Archie Pusaka
  2021-01-14 18:41   ` Luiz Augusto von Dentz
  2021-01-14  7:44 ` [Bluez PATCH v3 5/5] monitor: Decode add advmon with RSSI parameter Archie Pusaka
  4 siblings, 1 reply; 9+ messages in thread
From: Archie Pusaka @ 2021-01-14  7:44 UTC (permalink / raw)
  To: linux-bluetooth, Luiz Augusto von Dentz
  Cc: CrosBT Upstreaming, Archie Pusaka, Yun-Hao Chung

From: Archie Pusaka <apusaka@chromium.org>

Using the new opcode MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI to
monitor advertisement according to some RSSI criteria.

Reviewed-by: Yun-Hao Chung <howardchung@google.com>
---

Changes in v3:
* split the struct RSSIThresholdsAndTimers

 client/adv_monitor.c | 162 ++++++++++++++++++++++++++-----------------
 client/adv_monitor.h |   1 +
 client/main.c        |  29 ++++----
 3 files changed, 113 insertions(+), 79 deletions(-)

diff --git a/client/adv_monitor.c b/client/adv_monitor.c
index f62e9f4442..37faf1edfa 100644
--- a/client/adv_monitor.c
+++ b/client/adv_monitor.c
@@ -30,9 +30,10 @@
 
 struct rssi_setting {
 	int16_t high_threshold;
-	uint16_t high_timer;
+	uint16_t high_timeout;
 	int16_t low_threshold;
-	uint16_t low_timer;
+	uint16_t low_timeout;
+	uint16_t sampling_period;
 };
 
 struct pattern {
@@ -131,24 +132,58 @@ static gboolean get_type(const GDBusPropertyTable *property,
 	return TRUE;
 }
 
-static gboolean get_rssi(const GDBusPropertyTable *property,
+static gboolean get_low_threshold(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,
+	dbus_message_iter_append_basic(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 get_high_threshold(const GDBusPropertyTable *property,
+				DBusMessageIter *iter, void *user_data)
+{
+	struct adv_monitor *adv_monitor = user_data;
+	struct rssi_setting *rssi = adv_monitor->rssi;
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_INT16,
+							&rssi->high_threshold);
+	return TRUE;
+}
+
+static gboolean get_low_timeout(const GDBusPropertyTable *property,
+				DBusMessageIter *iter, void *user_data)
+{
+	struct adv_monitor *adv_monitor = user_data;
+	struct rssi_setting *rssi = adv_monitor->rssi;
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_UINT16,
+							&rssi->low_timeout);
+	return TRUE;
+}
+
+static gboolean get_high_timeout(const GDBusPropertyTable *property,
+				DBusMessageIter *iter, void *user_data)
+{
+	struct adv_monitor *adv_monitor = user_data;
+	struct rssi_setting *rssi = adv_monitor->rssi;
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_UINT16,
+							&rssi->high_timeout);
+	return TRUE;
+}
+
+static gboolean get_sampling_period(const GDBusPropertyTable *property,
+				DBusMessageIter *iter, void *user_data)
+{
+	struct adv_monitor *adv_monitor = user_data;
+	struct rssi_setting *rssi = adv_monitor->rssi;
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_UINT16,
+							&rssi->sampling_period);
 	return TRUE;
 }
 
@@ -212,7 +247,11 @@ static gboolean pattern_exists(const GDBusPropertyTable *property, void *data)
 
 static const GDBusPropertyTable adv_monitor_props[] = {
 	{ "Type", "s", get_type },
-	{ "RSSIThresholdsAndTimers", "(nqnq)", get_rssi, NULL, rssi_exists },
+	{ "RSSILowThreshold", "n", get_low_threshold, NULL, rssi_exists },
+	{ "RSSIHighThreshold", "n", get_high_threshold, NULL, rssi_exists },
+	{ "RSSILowTimeout", "q", get_low_timeout, NULL, rssi_exists },
+	{ "RSSIHighTimeout", "q", get_high_timeout, NULL, rssi_exists },
+	{ "RSSISamplingPeriod", "q", get_sampling_period, NULL, rssi_exists },
 	{ "Patterns", "a(yyay)", get_patterns, NULL, pattern_exists },
 	{ }
 };
@@ -376,56 +415,51 @@ static uint8_t str2bytearray(char *str, uint8_t *arr)
 	return arr_len;
 }
 
-static void parse_rssi_value_pair(char *value_pair, int *low, int *high)
-{
-	char *val1, *val2;
-	bool flag = value_pair[0] == ',';
-
-	val1 = strtok(value_pair, ",");
-
-	if (!val1)
-		return;
-
-	val2 = strtok(NULL, ",");
-
-	if (!val2) {
-		if (!flag)
-			*low = atoi(val1);
-		else
-			*high = atoi(val1);
-	} else {
-		*low = atoi(val1);
-		*high = atoi(val2);
-	}
-}
-
-static struct rssi_setting *parse_rssi(char *range, char *timeout)
+static struct rssi_setting *parse_rssi(char *params)
 {
 	struct rssi_setting *rssi;
-	int high_threshold, low_threshold, high_timer, low_timer;
-
-	high_threshold = RSSI_DEFAULT_HIGH_THRESHOLD;
-	low_threshold = RSSI_DEFAULT_LOW_THRESHOLD;
-	high_timer = RSSI_DEFAULT_HIGH_TIMEOUT;
-	low_timer = RSSI_DEFAULT_LOW_TIMEOUT;
+	char *split, *endptr;
+	int i;
+	int values[5] = {RSSI_DEFAULT_LOW_THRESHOLD,
+			RSSI_DEFAULT_HIGH_THRESHOLD,
+			RSSI_DEFAULT_LOW_TIMEOUT,
+			RSSI_DEFAULT_HIGH_TIMEOUT,
+			RSSI_DEFAULT_SAMPLING_PERIOD};
+
+	for (i = 0; i < 5; i++) {
+		if (!params) /* Params too short */
+			goto bad_format;
+
+		split = strsep(&params, ",");
+		if (*split != '\0') {
+			values[i] = strtol(split, &endptr, 0);
+			if (*endptr != '\0') /* Conversion failed */
+				goto bad_format;
+		} /* Otherwise no parsing needed - use default */
+	}
 
-	parse_rssi_value_pair(range, &low_threshold, &high_threshold);
-	parse_rssi_value_pair(timeout, &low_timer, &high_timer);
+	if (params) /* There are trailing unused params */
+		goto bad_format;
 
 	rssi = g_malloc0(sizeof(struct rssi_setting));
-
 	if (!rssi) {
-		bt_shell_printf("Failed to allocate rssi_setting");
+		bt_shell_printf("Failed to allocate rssi_setting\n");
 		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;
+	rssi->low_threshold = values[0];
+	rssi->high_threshold = values[1];
+	rssi->low_timeout = values[2];
+	rssi->high_timeout = values[3];
+	rssi->sampling_period = values[4];
 
 	return rssi;
+
+bad_format:
+	bt_shell_printf("Failed to parse RSSI\n");
+	bt_shell_noninteractive_quit(EXIT_FAILURE);
+	return NULL;
 }
 
 static struct pattern *parse_pattern(char *parameter_list[])
@@ -435,7 +469,7 @@ static struct pattern *parse_pattern(char *parameter_list[])
 	pat = g_malloc0(sizeof(struct pattern));
 
 	if (!pat) {
-		bt_shell_printf("Failed to allocate pattern");
+		bt_shell_printf("Failed to allocate pattern\n");
 		bt_shell_noninteractive_quit(EXIT_FAILURE);
 		return NULL;
 	}
@@ -531,12 +565,14 @@ static void print_adv_monitor(struct adv_monitor *adv_monitor)
 		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\thigh threshold timeout: %hu\n",
+					adv_monitor->rssi->high_timeout);
 		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);
+		bt_shell_printf("\t\tlow threshold timeout: %hu\n",
+					adv_monitor->rssi->low_timeout);
+		bt_shell_printf("\t\tsampling period: %hu\n",
+					adv_monitor->rssi->sampling_period);
 	}
 
 	if (adv_monitor->patterns) {
@@ -572,15 +608,15 @@ void adv_monitor_add_monitor(DBusConnection *conn, char *type,
 	while (find_adv_monitor_with_idx(adv_mon_idx))
 		adv_mon_idx += 1;
 
-	if (rssi_enabled == FALSE)
+	if (rssi_enabled == FALSE) {
 		rssi = NULL;
-	else {
-		rssi = parse_rssi(argv[1], argv[2]);
+	} else {
+		rssi = parse_rssi(argv[1]);
 		if (rssi == NULL)
 			return;
 
-		argv += 2;
-		argc -= 2;
+		argv += 1;
+		argc -= 1;
 	}
 
 	patterns = parse_patterns(argv+1, argc-1);
diff --git a/client/adv_monitor.h b/client/adv_monitor.h
index dd6f615799..2bdc447265 100644
--- a/client/adv_monitor.h
+++ b/client/adv_monitor.h
@@ -12,6 +12,7 @@
 #define RSSI_DEFAULT_LOW_THRESHOLD -70
 #define RSSI_DEFAULT_HIGH_TIMEOUT 10
 #define RSSI_DEFAULT_LOW_TIMEOUT 5
+#define RSSI_DEFAULT_SAMPLING_PERIOD 0
 
 void adv_monitor_add_manager(DBusConnection *conn, GDBusProxy *proxy);
 void adv_monitor_remove_manager(DBusConnection *conn);
diff --git a/client/main.c b/client/main.c
index 9403f1af6e..5d84e7cd54 100644
--- a/client/main.c
+++ b/client/main.c
@@ -2709,26 +2709,23 @@ static void cmd_ad_clear(int argc, char *argv[])
 
 static void print_add_or_pattern_with_rssi_usage(void)
 {
-	bt_shell_printf("rssi-range format:\n"
-			"\t<low-rssi>,<high-rssi>\n"
-			"\tBoth parameters can be skipped, in that case the\n"
-			"\tparamter will be set to its pre-defined value\n");
-	bt_shell_printf("\tPre-defined low-rssi,high-rssi: %d,%d\n",
-						RSSI_DEFAULT_LOW_THRESHOLD,
-						RSSI_DEFAULT_HIGH_THRESHOLD);
-	bt_shell_printf("timeout format:\n"
-			"\t<low-rssi>,<high-rssi>\n"
-			"\tBoth parameters can be skipped, in that case the\n"
-			"\tparamter will be set to its pre-defined value\n");
-	bt_shell_printf("\tPre-defined low-timeout,high-timeout: %d,%d\n",
-						RSSI_DEFAULT_LOW_TIMEOUT,
+	bt_shell_printf("rssi format:\n"
+			"\t<low-rssi>,<high-rssi>,<low-rssi-timeout>,"
+					"<high-rssi-timeout>,<sampling>\n"
+			"\tAll parameters can be skipped, in that case they\n"
+			"\twill be set to pre-defined values, which are:\n");
+	bt_shell_printf("\t\tlow-rssi: %d\n", RSSI_DEFAULT_LOW_THRESHOLD);
+	bt_shell_printf("\t\thigh-rssi: %d\n", RSSI_DEFAULT_HIGH_THRESHOLD);
+	bt_shell_printf("\t\tlow-rssi-timeout: %d\n", RSSI_DEFAULT_LOW_TIMEOUT);
+	bt_shell_printf("\t\thigh-rssi-timeout: %d\n",
 						RSSI_DEFAULT_HIGH_TIMEOUT);
+	bt_shell_printf("\t\tsampling: %d\n", RSSI_DEFAULT_SAMPLING_PERIOD);
 	bt_shell_printf("pattern format:\n"
 			"\t<start_position> <ad_data_type> <content_of_pattern>\n");
 	bt_shell_printf("e.g.\n"
-			"\tadd-or-pattern-rssi -10, ,10 1 2 01ab55\n");
+			"\tadd-or-pattern-rssi -10,,,10,0 1 2 01ab55\n");
 	bt_shell_printf("or\n"
-			"\tadd-or-pattern-rssi -50,-30 , 1 2 01ab55 3 4 23cd66\n");
+			"\tadd-or-pattern-rssi -50,-30,,, 1 2 01ab55 3 4 23cd66\n");
 }
 
 static void print_add_or_pattern_usage(void)
@@ -2826,7 +2823,7 @@ static const struct bt_shell_menu advertise_monitor_menu = {
 	.name = "monitor",
 	.desc = "Advertisement Monitor Options Submenu",
 	.entries = {
-	{ "add-or-pattern-rssi", "<rssi-range=low,high> <timeout=low,high> "
+	{ "add-or-pattern-rssi", "<rssi=low,high,low-time,high-time,sampling> "
 				"[patterns=pattern1 pattern2 ...]",
 				cmd_adv_monitor_add_or_monitor_with_rssi,
 				"Add 'or pattern' type monitor with RSSI "
-- 
2.30.0.284.gd98b1dd5eaa7-goog


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

* [Bluez PATCH v3 5/5] monitor: Decode add advmon with RSSI parameter
  2021-01-14  7:44 [Bluez PATCH v3 0/5] Support advertising monitor add pattern with RSSI opcode Archie Pusaka
                   ` (3 preceding siblings ...)
  2021-01-14  7:44 ` [Bluez PATCH v3 4/5] bluetoothctl: advmon rssi support for mgmt Archie Pusaka
@ 2021-01-14  7:44 ` Archie Pusaka
  4 siblings, 0 replies; 9+ messages in thread
From: Archie Pusaka @ 2021-01-14  7:44 UTC (permalink / raw)
  To: linux-bluetooth, Luiz Augusto von Dentz
  Cc: CrosBT Upstreaming, Archie Pusaka, Yun-Hao Chung

From: Archie Pusaka <apusaka@chromium.org>

Add support for MGMT command of add advertisement monitor with RSSI
parameter (0x0056).

@ MGMT Command: Add Advertisement.. (0x0056) plen 76  {0x0003}
        RSSI data:
          high threshold: 1 dBm
          high timeout: 0 seconds
          low threshold: -2 dBm
          low timeout: 3 seconds
          sampling: just once (0xFF)
        Number of patterns: 2
          Pattern 1:
            AD type: 0
            Offset: 1
            Length: 2
            Value : c504
          Pattern 2:
            AD type: 255
            Offset: 10
            Length: 4
            Value : 9a55beef

Reviewed-by: Yun-Hao Chung <howardchung@google.com>
---

(no changes since v1)

 monitor/packet.c | 43 +++++++++++++++++++++++++++++++++++++++----
 1 file changed, 39 insertions(+), 4 deletions(-)

diff --git a/monitor/packet.c b/monitor/packet.c
index c91b91e2b2..fcd698d92a 100644
--- a/monitor/packet.c
+++ b/monitor/packet.c
@@ -13261,18 +13261,22 @@ static void mgmt_print_adv_monitor_patterns(const void *data, uint8_t len)
 
 	/* Reference: struct mgmt_adv_pattern in lib/mgmt.h. */
 	while (data_idx + 34 <= len) {
-		uint8_t ad_type = get_u8(data + data_idx);
-		uint8_t offset = get_u8(data + data_idx + 1);
-		uint8_t length = get_u8(data + data_idx + 2);
+		uint8_t ad_type = get_u8(data);
+		uint8_t offset = get_u8(data + 1);
+		uint8_t length = get_u8(data + 2);
 
 		print_field("  Pattern %d:", pattern_idx);
 		print_field("    AD type: %d", ad_type);
 		print_field("    Offset: %d", offset);
 		print_field("    Length: %d", length);
-		print_hex_field("    Value ", data + data_idx + 3, 31);
+		if (length <= 31)
+			print_hex_field("    Value ", data + 3, length);
+		else
+			print_text(COLOR_ERROR, "    invalid length");
 
 		pattern_idx += 1;
 		data_idx += 34;
+		data += 34;
 	}
 }
 
@@ -13284,6 +13288,33 @@ static void mgmt_add_adv_monitor_patterns_cmd(const void *data, uint16_t size)
 	mgmt_print_adv_monitor_patterns(data + 1, size - 1);
 }
 
+static void mgmt_add_adv_monitor_patterns_rssi_cmd(const void *data,
+								uint16_t size)
+{
+	int8_t high_rssi = get_s8(data);
+	uint16_t high_rssi_timeout = get_le16(data + 1);
+	int8_t low_rssi = get_s8(data + 3);
+	uint16_t low_rssi_timeout = get_le16(data + 4);
+	uint8_t sampling_period = get_u8(data + 6);
+	uint8_t pattern_count = get_u8(data + 7);
+
+	print_field("RSSI data:");
+	print_field("  high threshold: %d dBm", high_rssi);
+	print_field("  high timeout: %d seconds", high_rssi_timeout);
+	print_field("  low threshold: %d dBm", low_rssi);
+	print_field("  low timeout: %d seconds", low_rssi_timeout);
+
+	if (sampling_period == 0)
+		print_field("  sampling: propagate all (0x00)");
+	else if (sampling_period == 0xff)
+		print_field("  sampling: just once (0xFF)");
+	else
+		print_field("  sampling: every %d ms", 100 * sampling_period);
+
+	print_field("Number of patterns: %d", pattern_count);
+	mgmt_print_adv_monitor_patterns(data + 8, size - 8);
+}
+
 static void mgmt_add_adv_monitor_patterns_rsp(const void *data, uint16_t size)
 {
 	uint16_t handle = get_le16(data);
@@ -13553,6 +13584,10 @@ static const struct mgmt_data mgmt_command_table[] = {
 	{ 0x0055, "Add Ext Adv Data",
 				mgmt_add_ext_adv_data_cmd, 3, false,
 				mgmt_add_ext_adv_data_rsp, 1, true },
+	{ 0x0056, "Add Advertisement Monitor With RSSI",
+				mgmt_add_adv_monitor_patterns_rssi_cmd, 8,
+									false,
+				mgmt_add_adv_monitor_patterns_rsp, 2, true},
 	{ }
 };
 
-- 
2.30.0.284.gd98b1dd5eaa7-goog


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

* RE: Support advertising monitor add pattern with RSSI opcode
  2021-01-14  7:44 ` [Bluez PATCH v3 1/5] lib/mgmt: Adding Add Adv Patterns Monitor " Archie Pusaka
@ 2021-01-14  8:28   ` bluez.test.bot
  0 siblings, 0 replies; 9+ messages in thread
From: bluez.test.bot @ 2021-01-14  8:28 UTC (permalink / raw)
  To: linux-bluetooth, apusaka

[-- Attachment #1: Type: text/plain, Size: 557 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=414351

---Test result---

##############################
Test: CheckPatch - PASS

##############################
Test: CheckGitLint - PASS

##############################
Test: CheckBuild - PASS

##############################
Test: MakeCheck - PASS



---
Regards,
Linux Bluetooth


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

* Re: [Bluez PATCH v3 4/5] bluetoothctl: advmon rssi support for mgmt
  2021-01-14  7:44 ` [Bluez PATCH v3 4/5] bluetoothctl: advmon rssi support for mgmt Archie Pusaka
@ 2021-01-14 18:41   ` Luiz Augusto von Dentz
  2021-01-15 11:55     ` Archie Pusaka
  0 siblings, 1 reply; 9+ messages in thread
From: Luiz Augusto von Dentz @ 2021-01-14 18:41 UTC (permalink / raw)
  To: Archie Pusaka
  Cc: linux-bluetooth, CrosBT Upstreaming, Archie Pusaka, Yun-Hao Chung

Hi Archie,

On Wed, Jan 13, 2021 at 11:45 PM Archie Pusaka <apusaka@google.com> wrote:
>
> From: Archie Pusaka <apusaka@chromium.org>
>
> Using the new opcode MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI to
> monitor advertisement according to some RSSI criteria.
>
> Reviewed-by: Yun-Hao Chung <howardchung@google.com>
> ---
>
> Changes in v3:
> * split the struct RSSIThresholdsAndTimers
>
>  client/adv_monitor.c | 162 ++++++++++++++++++++++++++-----------------
>  client/adv_monitor.h |   1 +
>  client/main.c        |  29 ++++----
>  3 files changed, 113 insertions(+), 79 deletions(-)
>
> diff --git a/client/adv_monitor.c b/client/adv_monitor.c
> index f62e9f4442..37faf1edfa 100644
> --- a/client/adv_monitor.c
> +++ b/client/adv_monitor.c
> @@ -30,9 +30,10 @@
>
>  struct rssi_setting {
>         int16_t high_threshold;
> -       uint16_t high_timer;
> +       uint16_t high_timeout;
>         int16_t low_threshold;
> -       uint16_t low_timer;
> +       uint16_t low_timeout;
> +       uint16_t sampling_period;
>  };
>
>  struct pattern {
> @@ -131,24 +132,58 @@ static gboolean get_type(const GDBusPropertyTable *property,
>         return TRUE;
>  }
>
> -static gboolean get_rssi(const GDBusPropertyTable *property,
> +static gboolean get_low_threshold(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,
> +       dbus_message_iter_append_basic(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 get_high_threshold(const GDBusPropertyTable *property,
> +                               DBusMessageIter *iter, void *user_data)
> +{
> +       struct adv_monitor *adv_monitor = user_data;
> +       struct rssi_setting *rssi = adv_monitor->rssi;
> +
> +       dbus_message_iter_append_basic(iter, DBUS_TYPE_INT16,
> +                                                       &rssi->high_threshold);
> +       return TRUE;
> +}
> +
> +static gboolean get_low_timeout(const GDBusPropertyTable *property,
> +                               DBusMessageIter *iter, void *user_data)
> +{
> +       struct adv_monitor *adv_monitor = user_data;
> +       struct rssi_setting *rssi = adv_monitor->rssi;
> +
> +       dbus_message_iter_append_basic(iter, DBUS_TYPE_UINT16,
> +                                                       &rssi->low_timeout);
> +       return TRUE;
> +}
> +
> +static gboolean get_high_timeout(const GDBusPropertyTable *property,
> +                               DBusMessageIter *iter, void *user_data)
> +{
> +       struct adv_monitor *adv_monitor = user_data;
> +       struct rssi_setting *rssi = adv_monitor->rssi;
> +
> +       dbus_message_iter_append_basic(iter, DBUS_TYPE_UINT16,
> +                                                       &rssi->high_timeout);
> +       return TRUE;
> +}
> +
> +static gboolean get_sampling_period(const GDBusPropertyTable *property,
> +                               DBusMessageIter *iter, void *user_data)
> +{
> +       struct adv_monitor *adv_monitor = user_data;
> +       struct rssi_setting *rssi = adv_monitor->rssi;
> +
> +       dbus_message_iter_append_basic(iter, DBUS_TYPE_UINT16,
> +                                                       &rssi->sampling_period);
>         return TRUE;
>  }
>
> @@ -212,7 +247,11 @@ static gboolean pattern_exists(const GDBusPropertyTable *property, void *data)
>
>  static const GDBusPropertyTable adv_monitor_props[] = {
>         { "Type", "s", get_type },
> -       { "RSSIThresholdsAndTimers", "(nqnq)", get_rssi, NULL, rssi_exists },
> +       { "RSSILowThreshold", "n", get_low_threshold, NULL, rssi_exists },
> +       { "RSSIHighThreshold", "n", get_high_threshold, NULL, rssi_exists },
> +       { "RSSILowTimeout", "q", get_low_timeout, NULL, rssi_exists },
> +       { "RSSIHighTimeout", "q", get_high_timeout, NULL, rssi_exists },
> +       { "RSSISamplingPeriod", "q", get_sampling_period, NULL, rssi_exists },

Interesting, is the SamplingPeriod new? It seems we are missing the
documentation changes of the split.

>         { "Patterns", "a(yyay)", get_patterns, NULL, pattern_exists },
>         { }
>  };
> @@ -376,56 +415,51 @@ static uint8_t str2bytearray(char *str, uint8_t *arr)
>         return arr_len;
>  }
>
> -static void parse_rssi_value_pair(char *value_pair, int *low, int *high)
> -{
> -       char *val1, *val2;
> -       bool flag = value_pair[0] == ',';
> -
> -       val1 = strtok(value_pair, ",");
> -
> -       if (!val1)
> -               return;
> -
> -       val2 = strtok(NULL, ",");
> -
> -       if (!val2) {
> -               if (!flag)
> -                       *low = atoi(val1);
> -               else
> -                       *high = atoi(val1);
> -       } else {
> -               *low = atoi(val1);
> -               *high = atoi(val2);
> -       }
> -}
> -
> -static struct rssi_setting *parse_rssi(char *range, char *timeout)
> +static struct rssi_setting *parse_rssi(char *params)
>  {
>         struct rssi_setting *rssi;
> -       int high_threshold, low_threshold, high_timer, low_timer;
> -
> -       high_threshold = RSSI_DEFAULT_HIGH_THRESHOLD;
> -       low_threshold = RSSI_DEFAULT_LOW_THRESHOLD;
> -       high_timer = RSSI_DEFAULT_HIGH_TIMEOUT;
> -       low_timer = RSSI_DEFAULT_LOW_TIMEOUT;
> +       char *split, *endptr;
> +       int i;
> +       int values[5] = {RSSI_DEFAULT_LOW_THRESHOLD,
> +                       RSSI_DEFAULT_HIGH_THRESHOLD,
> +                       RSSI_DEFAULT_LOW_TIMEOUT,
> +                       RSSI_DEFAULT_HIGH_TIMEOUT,
> +                       RSSI_DEFAULT_SAMPLING_PERIOD};
> +
> +       for (i = 0; i < 5; i++) {
> +               if (!params) /* Params too short */
> +                       goto bad_format;
> +
> +               split = strsep(&params, ",");
> +               if (*split != '\0') {
> +                       values[i] = strtol(split, &endptr, 0);
> +                       if (*endptr != '\0') /* Conversion failed */
> +                               goto bad_format;
> +               } /* Otherwise no parsing needed - use default */
> +       }

You might want to consider taking these parameters separately in the
command itself, that way we don't have to reparse the string parameter
as they would be split in argc/argv by bt_shell.

> -       parse_rssi_value_pair(range, &low_threshold, &high_threshold);
> -       parse_rssi_value_pair(timeout, &low_timer, &high_timer);
> +       if (params) /* There are trailing unused params */
> +               goto bad_format;
>
>         rssi = g_malloc0(sizeof(struct rssi_setting));
> -
>         if (!rssi) {
> -               bt_shell_printf("Failed to allocate rssi_setting");
> +               bt_shell_printf("Failed to allocate rssi_setting\n");
>                 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;
> +       rssi->low_threshold = values[0];
> +       rssi->high_threshold = values[1];
> +       rssi->low_timeout = values[2];
> +       rssi->high_timeout = values[3];
> +       rssi->sampling_period = values[4];
>
>         return rssi;
> +
> +bad_format:
> +       bt_shell_printf("Failed to parse RSSI\n");
> +       bt_shell_noninteractive_quit(EXIT_FAILURE);
> +       return NULL;
>  }
>
>  static struct pattern *parse_pattern(char *parameter_list[])
> @@ -435,7 +469,7 @@ static struct pattern *parse_pattern(char *parameter_list[])
>         pat = g_malloc0(sizeof(struct pattern));
>
>         if (!pat) {
> -               bt_shell_printf("Failed to allocate pattern");
> +               bt_shell_printf("Failed to allocate pattern\n");
>                 bt_shell_noninteractive_quit(EXIT_FAILURE);
>                 return NULL;
>         }
> @@ -531,12 +565,14 @@ static void print_adv_monitor(struct adv_monitor *adv_monitor)
>                 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\thigh threshold timeout: %hu\n",
> +                                       adv_monitor->rssi->high_timeout);
>                 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);
> +               bt_shell_printf("\t\tlow threshold timeout: %hu\n",
> +                                       adv_monitor->rssi->low_timeout);
> +               bt_shell_printf("\t\tsampling period: %hu\n",
> +                                       adv_monitor->rssi->sampling_period);
>         }
>
>         if (adv_monitor->patterns) {
> @@ -572,15 +608,15 @@ void adv_monitor_add_monitor(DBusConnection *conn, char *type,
>         while (find_adv_monitor_with_idx(adv_mon_idx))
>                 adv_mon_idx += 1;
>
> -       if (rssi_enabled == FALSE)
> +       if (rssi_enabled == FALSE) {
>                 rssi = NULL;
> -       else {
> -               rssi = parse_rssi(argv[1], argv[2]);
> +       } else {
> +               rssi = parse_rssi(argv[1]);
>                 if (rssi == NULL)
>                         return;
>
> -               argv += 2;
> -               argc -= 2;
> +               argv += 1;
> +               argc -= 1;
>         }
>
>         patterns = parse_patterns(argv+1, argc-1);
> diff --git a/client/adv_monitor.h b/client/adv_monitor.h
> index dd6f615799..2bdc447265 100644
> --- a/client/adv_monitor.h
> +++ b/client/adv_monitor.h
> @@ -12,6 +12,7 @@
>  #define RSSI_DEFAULT_LOW_THRESHOLD -70
>  #define RSSI_DEFAULT_HIGH_TIMEOUT 10
>  #define RSSI_DEFAULT_LOW_TIMEOUT 5
> +#define RSSI_DEFAULT_SAMPLING_PERIOD 0
>
>  void adv_monitor_add_manager(DBusConnection *conn, GDBusProxy *proxy);
>  void adv_monitor_remove_manager(DBusConnection *conn);
> diff --git a/client/main.c b/client/main.c
> index 9403f1af6e..5d84e7cd54 100644
> --- a/client/main.c
> +++ b/client/main.c
> @@ -2709,26 +2709,23 @@ static void cmd_ad_clear(int argc, char *argv[])
>
>  static void print_add_or_pattern_with_rssi_usage(void)
>  {
> -       bt_shell_printf("rssi-range format:\n"
> -                       "\t<low-rssi>,<high-rssi>\n"
> -                       "\tBoth parameters can be skipped, in that case the\n"
> -                       "\tparamter will be set to its pre-defined value\n");
> -       bt_shell_printf("\tPre-defined low-rssi,high-rssi: %d,%d\n",
> -                                               RSSI_DEFAULT_LOW_THRESHOLD,
> -                                               RSSI_DEFAULT_HIGH_THRESHOLD);
> -       bt_shell_printf("timeout format:\n"
> -                       "\t<low-rssi>,<high-rssi>\n"
> -                       "\tBoth parameters can be skipped, in that case the\n"
> -                       "\tparamter will be set to its pre-defined value\n");
> -       bt_shell_printf("\tPre-defined low-timeout,high-timeout: %d,%d\n",
> -                                               RSSI_DEFAULT_LOW_TIMEOUT,
> +       bt_shell_printf("rssi format:\n"
> +                       "\t<low-rssi>,<high-rssi>,<low-rssi-timeout>,"
> +                                       "<high-rssi-timeout>,<sampling>\n"
> +                       "\tAll parameters can be skipped, in that case they\n"
> +                       "\twill be set to pre-defined values, which are:\n");
> +       bt_shell_printf("\t\tlow-rssi: %d\n", RSSI_DEFAULT_LOW_THRESHOLD);
> +       bt_shell_printf("\t\thigh-rssi: %d\n", RSSI_DEFAULT_HIGH_THRESHOLD);
> +       bt_shell_printf("\t\tlow-rssi-timeout: %d\n", RSSI_DEFAULT_LOW_TIMEOUT);
> +       bt_shell_printf("\t\thigh-rssi-timeout: %d\n",
>                                                 RSSI_DEFAULT_HIGH_TIMEOUT);
> +       bt_shell_printf("\t\tsampling: %d\n", RSSI_DEFAULT_SAMPLING_PERIOD);
>         bt_shell_printf("pattern format:\n"
>                         "\t<start_position> <ad_data_type> <content_of_pattern>\n");
>         bt_shell_printf("e.g.\n"
> -                       "\tadd-or-pattern-rssi -10, ,10 1 2 01ab55\n");
> +                       "\tadd-or-pattern-rssi -10,,,10,0 1 2 01ab55\n");
>         bt_shell_printf("or\n"
> -                       "\tadd-or-pattern-rssi -50,-30 , 1 2 01ab55 3 4 23cd66\n");
> +                       "\tadd-or-pattern-rssi -50,-30,,, 1 2 01ab55 3 4 23cd66\n");
>  }
>
>  static void print_add_or_pattern_usage(void)
> @@ -2826,7 +2823,7 @@ static const struct bt_shell_menu advertise_monitor_menu = {
>         .name = "monitor",
>         .desc = "Advertisement Monitor Options Submenu",
>         .entries = {
> -       { "add-or-pattern-rssi", "<rssi-range=low,high> <timeout=low,high> "
> +       { "add-or-pattern-rssi", "<rssi=low,high,low-time,high-time,sampling> "

Perhaps we should split the rssi setting in multiple commands as well e.g.:

rssi <low> <high>
timeout <low> <high>
interval <value>
add-or-pattern <pattern1> [...]

So when add-or-pattern is used it takes the rssi values if they have
been, we would probably need to create the instance at first command
but we only register when add is called, also don't fill the defaults
just omit them so the daemon fill them in that way if we at some point
change the defaults we don't have to change it in multiple places.

>                                 "[patterns=pattern1 pattern2 ...]",
>                                 cmd_adv_monitor_add_or_monitor_with_rssi,
>                                 "Add 'or pattern' type monitor with RSSI "
> --
> 2.30.0.284.gd98b1dd5eaa7-goog
>


-- 
Luiz Augusto von Dentz

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

* Re: [Bluez PATCH v3 4/5] bluetoothctl: advmon rssi support for mgmt
  2021-01-14 18:41   ` Luiz Augusto von Dentz
@ 2021-01-15 11:55     ` Archie Pusaka
  0 siblings, 0 replies; 9+ messages in thread
From: Archie Pusaka @ 2021-01-15 11:55 UTC (permalink / raw)
  To: Luiz Augusto von Dentz
  Cc: linux-bluetooth, CrosBT Upstreaming, Archie Pusaka, Yun-Hao Chung

On Fri, 15 Jan 2021 at 02:42, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Archie,
>
> On Wed, Jan 13, 2021 at 11:45 PM Archie Pusaka <apusaka@google.com> wrote:
> >
> > From: Archie Pusaka <apusaka@chromium.org>
> >
> > Using the new opcode MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI to
> > monitor advertisement according to some RSSI criteria.
> >
> > Reviewed-by: Yun-Hao Chung <howardchung@google.com>
> > ---
> >
> > Changes in v3:
> > * split the struct RSSIThresholdsAndTimers
> >
> >  client/adv_monitor.c | 162 ++++++++++++++++++++++++++-----------------
> >  client/adv_monitor.h |   1 +
> >  client/main.c        |  29 ++++----
> >  3 files changed, 113 insertions(+), 79 deletions(-)
> >
> > diff --git a/client/adv_monitor.c b/client/adv_monitor.c
> > index f62e9f4442..37faf1edfa 100644
> > --- a/client/adv_monitor.c
> > +++ b/client/adv_monitor.c
> > @@ -30,9 +30,10 @@
> >
> >  struct rssi_setting {
> >         int16_t high_threshold;
> > -       uint16_t high_timer;
> > +       uint16_t high_timeout;
> >         int16_t low_threshold;
> > -       uint16_t low_timer;
> > +       uint16_t low_timeout;
> > +       uint16_t sampling_period;
> >  };
> >
> >  struct pattern {
> > @@ -131,24 +132,58 @@ static gboolean get_type(const GDBusPropertyTable *property,
> >         return TRUE;
> >  }
> >
> > -static gboolean get_rssi(const GDBusPropertyTable *property,
> > +static gboolean get_low_threshold(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,
> > +       dbus_message_iter_append_basic(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 get_high_threshold(const GDBusPropertyTable *property,
> > +                               DBusMessageIter *iter, void *user_data)
> > +{
> > +       struct adv_monitor *adv_monitor = user_data;
> > +       struct rssi_setting *rssi = adv_monitor->rssi;
> > +
> > +       dbus_message_iter_append_basic(iter, DBUS_TYPE_INT16,
> > +                                                       &rssi->high_threshold);
> > +       return TRUE;
> > +}
> > +
> > +static gboolean get_low_timeout(const GDBusPropertyTable *property,
> > +                               DBusMessageIter *iter, void *user_data)
> > +{
> > +       struct adv_monitor *adv_monitor = user_data;
> > +       struct rssi_setting *rssi = adv_monitor->rssi;
> > +
> > +       dbus_message_iter_append_basic(iter, DBUS_TYPE_UINT16,
> > +                                                       &rssi->low_timeout);
> > +       return TRUE;
> > +}
> > +
> > +static gboolean get_high_timeout(const GDBusPropertyTable *property,
> > +                               DBusMessageIter *iter, void *user_data)
> > +{
> > +       struct adv_monitor *adv_monitor = user_data;
> > +       struct rssi_setting *rssi = adv_monitor->rssi;
> > +
> > +       dbus_message_iter_append_basic(iter, DBUS_TYPE_UINT16,
> > +                                                       &rssi->high_timeout);
> > +       return TRUE;
> > +}
> > +
> > +static gboolean get_sampling_period(const GDBusPropertyTable *property,
> > +                               DBusMessageIter *iter, void *user_data)
> > +{
> > +       struct adv_monitor *adv_monitor = user_data;
> > +       struct rssi_setting *rssi = adv_monitor->rssi;
> > +
> > +       dbus_message_iter_append_basic(iter, DBUS_TYPE_UINT16,
> > +                                                       &rssi->sampling_period);
> >         return TRUE;
> >  }
> >
> > @@ -212,7 +247,11 @@ static gboolean pattern_exists(const GDBusPropertyTable *property, void *data)
> >
> >  static const GDBusPropertyTable adv_monitor_props[] = {
> >         { "Type", "s", get_type },
> > -       { "RSSIThresholdsAndTimers", "(nqnq)", get_rssi, NULL, rssi_exists },
> > +       { "RSSILowThreshold", "n", get_low_threshold, NULL, rssi_exists },
> > +       { "RSSIHighThreshold", "n", get_high_threshold, NULL, rssi_exists },
> > +       { "RSSILowTimeout", "q", get_low_timeout, NULL, rssi_exists },
> > +       { "RSSIHighTimeout", "q", get_high_timeout, NULL, rssi_exists },
> > +       { "RSSISamplingPeriod", "q", get_sampling_period, NULL, rssi_exists },
>
> Interesting, is the SamplingPeriod new? It seems we are missing the
> documentation changes of the split.

Yes, it is new. The purpose is to make it compatible with the add
monitor MGMT command. Also yes, I forgot to update the doc. Will do so
in v4.
>
> >         { "Patterns", "a(yyay)", get_patterns, NULL, pattern_exists },
> >         { }
> >  };
> > @@ -376,56 +415,51 @@ static uint8_t str2bytearray(char *str, uint8_t *arr)
> >         return arr_len;
> >  }
> >
> > -static void parse_rssi_value_pair(char *value_pair, int *low, int *high)
> > -{
> > -       char *val1, *val2;
> > -       bool flag = value_pair[0] == ',';
> > -
> > -       val1 = strtok(value_pair, ",");
> > -
> > -       if (!val1)
> > -               return;
> > -
> > -       val2 = strtok(NULL, ",");
> > -
> > -       if (!val2) {
> > -               if (!flag)
> > -                       *low = atoi(val1);
> > -               else
> > -                       *high = atoi(val1);
> > -       } else {
> > -               *low = atoi(val1);
> > -               *high = atoi(val2);
> > -       }
> > -}
> > -
> > -static struct rssi_setting *parse_rssi(char *range, char *timeout)
> > +static struct rssi_setting *parse_rssi(char *params)
> >  {
> >         struct rssi_setting *rssi;
> > -       int high_threshold, low_threshold, high_timer, low_timer;
> > -
> > -       high_threshold = RSSI_DEFAULT_HIGH_THRESHOLD;
> > -       low_threshold = RSSI_DEFAULT_LOW_THRESHOLD;
> > -       high_timer = RSSI_DEFAULT_HIGH_TIMEOUT;
> > -       low_timer = RSSI_DEFAULT_LOW_TIMEOUT;
> > +       char *split, *endptr;
> > +       int i;
> > +       int values[5] = {RSSI_DEFAULT_LOW_THRESHOLD,
> > +                       RSSI_DEFAULT_HIGH_THRESHOLD,
> > +                       RSSI_DEFAULT_LOW_TIMEOUT,
> > +                       RSSI_DEFAULT_HIGH_TIMEOUT,
> > +                       RSSI_DEFAULT_SAMPLING_PERIOD};
> > +
> > +       for (i = 0; i < 5; i++) {
> > +               if (!params) /* Params too short */
> > +                       goto bad_format;
> > +
> > +               split = strsep(&params, ",");
> > +               if (*split != '\0') {
> > +                       values[i] = strtol(split, &endptr, 0);
> > +                       if (*endptr != '\0') /* Conversion failed */
> > +                               goto bad_format;
> > +               } /* Otherwise no parsing needed - use default */
> > +       }
>
> You might want to consider taking these parameters separately in the
> command itself, that way we don't have to reparse the string parameter
> as they would be split in argc/argv by bt_shell.

Done in v4
>
> > -       parse_rssi_value_pair(range, &low_threshold, &high_threshold);
> > -       parse_rssi_value_pair(timeout, &low_timer, &high_timer);
> > +       if (params) /* There are trailing unused params */
> > +               goto bad_format;
> >
> >         rssi = g_malloc0(sizeof(struct rssi_setting));
> > -
> >         if (!rssi) {
> > -               bt_shell_printf("Failed to allocate rssi_setting");
> > +               bt_shell_printf("Failed to allocate rssi_setting\n");
> >                 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;
> > +       rssi->low_threshold = values[0];
> > +       rssi->high_threshold = values[1];
> > +       rssi->low_timeout = values[2];
> > +       rssi->high_timeout = values[3];
> > +       rssi->sampling_period = values[4];
> >
> >         return rssi;
> > +
> > +bad_format:
> > +       bt_shell_printf("Failed to parse RSSI\n");
> > +       bt_shell_noninteractive_quit(EXIT_FAILURE);
> > +       return NULL;
> >  }
> >
> >  static struct pattern *parse_pattern(char *parameter_list[])
> > @@ -435,7 +469,7 @@ static struct pattern *parse_pattern(char *parameter_list[])
> >         pat = g_malloc0(sizeof(struct pattern));
> >
> >         if (!pat) {
> > -               bt_shell_printf("Failed to allocate pattern");
> > +               bt_shell_printf("Failed to allocate pattern\n");
> >                 bt_shell_noninteractive_quit(EXIT_FAILURE);
> >                 return NULL;
> >         }
> > @@ -531,12 +565,14 @@ static void print_adv_monitor(struct adv_monitor *adv_monitor)
> >                 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\thigh threshold timeout: %hu\n",
> > +                                       adv_monitor->rssi->high_timeout);
> >                 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);
> > +               bt_shell_printf("\t\tlow threshold timeout: %hu\n",
> > +                                       adv_monitor->rssi->low_timeout);
> > +               bt_shell_printf("\t\tsampling period: %hu\n",
> > +                                       adv_monitor->rssi->sampling_period);
> >         }
> >
> >         if (adv_monitor->patterns) {
> > @@ -572,15 +608,15 @@ void adv_monitor_add_monitor(DBusConnection *conn, char *type,
> >         while (find_adv_monitor_with_idx(adv_mon_idx))
> >                 adv_mon_idx += 1;
> >
> > -       if (rssi_enabled == FALSE)
> > +       if (rssi_enabled == FALSE) {
> >                 rssi = NULL;
> > -       else {
> > -               rssi = parse_rssi(argv[1], argv[2]);
> > +       } else {
> > +               rssi = parse_rssi(argv[1]);
> >                 if (rssi == NULL)
> >                         return;
> >
> > -               argv += 2;
> > -               argc -= 2;
> > +               argv += 1;
> > +               argc -= 1;
> >         }
> >
> >         patterns = parse_patterns(argv+1, argc-1);
> > diff --git a/client/adv_monitor.h b/client/adv_monitor.h
> > index dd6f615799..2bdc447265 100644
> > --- a/client/adv_monitor.h
> > +++ b/client/adv_monitor.h
> > @@ -12,6 +12,7 @@
> >  #define RSSI_DEFAULT_LOW_THRESHOLD -70
> >  #define RSSI_DEFAULT_HIGH_TIMEOUT 10
> >  #define RSSI_DEFAULT_LOW_TIMEOUT 5
> > +#define RSSI_DEFAULT_SAMPLING_PERIOD 0
> >
> >  void adv_monitor_add_manager(DBusConnection *conn, GDBusProxy *proxy);
> >  void adv_monitor_remove_manager(DBusConnection *conn);
> > diff --git a/client/main.c b/client/main.c
> > index 9403f1af6e..5d84e7cd54 100644
> > --- a/client/main.c
> > +++ b/client/main.c
> > @@ -2709,26 +2709,23 @@ static void cmd_ad_clear(int argc, char *argv[])
> >
> >  static void print_add_or_pattern_with_rssi_usage(void)
> >  {
> > -       bt_shell_printf("rssi-range format:\n"
> > -                       "\t<low-rssi>,<high-rssi>\n"
> > -                       "\tBoth parameters can be skipped, in that case the\n"
> > -                       "\tparamter will be set to its pre-defined value\n");
> > -       bt_shell_printf("\tPre-defined low-rssi,high-rssi: %d,%d\n",
> > -                                               RSSI_DEFAULT_LOW_THRESHOLD,
> > -                                               RSSI_DEFAULT_HIGH_THRESHOLD);
> > -       bt_shell_printf("timeout format:\n"
> > -                       "\t<low-rssi>,<high-rssi>\n"
> > -                       "\tBoth parameters can be skipped, in that case the\n"
> > -                       "\tparamter will be set to its pre-defined value\n");
> > -       bt_shell_printf("\tPre-defined low-timeout,high-timeout: %d,%d\n",
> > -                                               RSSI_DEFAULT_LOW_TIMEOUT,
> > +       bt_shell_printf("rssi format:\n"
> > +                       "\t<low-rssi>,<high-rssi>,<low-rssi-timeout>,"
> > +                                       "<high-rssi-timeout>,<sampling>\n"
> > +                       "\tAll parameters can be skipped, in that case they\n"
> > +                       "\twill be set to pre-defined values, which are:\n");
> > +       bt_shell_printf("\t\tlow-rssi: %d\n", RSSI_DEFAULT_LOW_THRESHOLD);
> > +       bt_shell_printf("\t\thigh-rssi: %d\n", RSSI_DEFAULT_HIGH_THRESHOLD);
> > +       bt_shell_printf("\t\tlow-rssi-timeout: %d\n", RSSI_DEFAULT_LOW_TIMEOUT);
> > +       bt_shell_printf("\t\thigh-rssi-timeout: %d\n",
> >                                                 RSSI_DEFAULT_HIGH_TIMEOUT);
> > +       bt_shell_printf("\t\tsampling: %d\n", RSSI_DEFAULT_SAMPLING_PERIOD);
> >         bt_shell_printf("pattern format:\n"
> >                         "\t<start_position> <ad_data_type> <content_of_pattern>\n");
> >         bt_shell_printf("e.g.\n"
> > -                       "\tadd-or-pattern-rssi -10, ,10 1 2 01ab55\n");
> > +                       "\tadd-or-pattern-rssi -10,,,10,0 1 2 01ab55\n");
> >         bt_shell_printf("or\n"
> > -                       "\tadd-or-pattern-rssi -50,-30 , 1 2 01ab55 3 4 23cd66\n");
> > +                       "\tadd-or-pattern-rssi -50,-30,,, 1 2 01ab55 3 4 23cd66\n");
> >  }
> >
> >  static void print_add_or_pattern_usage(void)
> > @@ -2826,7 +2823,7 @@ static const struct bt_shell_menu advertise_monitor_menu = {
> >         .name = "monitor",
> >         .desc = "Advertisement Monitor Options Submenu",
> >         .entries = {
> > -       { "add-or-pattern-rssi", "<rssi-range=low,high> <timeout=low,high> "
> > +       { "add-or-pattern-rssi", "<rssi=low,high,low-time,high-time,sampling> "
>
> Perhaps we should split the rssi setting in multiple commands as well e.g.:
>
> rssi <low> <high>
> timeout <low> <high>
> interval <value>
> add-or-pattern <pattern1> [...]
>
> So when add-or-pattern is used it takes the rssi values if they have
> been, we would probably need to create the instance at first command
> but we only register when add is called, also don't fill the defaults
> just omit them so the daemon fill them in that way if we at some point
> change the defaults we don't have to change it in multiple places.

Done in v4. Please tell me what you think!
>
> >                                 "[patterns=pattern1 pattern2 ...]",
> >                                 cmd_adv_monitor_add_or_monitor_with_rssi,
> >                                 "Add 'or pattern' type monitor with RSSI "
> > --
> > 2.30.0.284.gd98b1dd5eaa7-goog
> >
>
>
> --
> Luiz Augusto von Dentz

Thanks,
Archie

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

end of thread, other threads:[~2021-01-15 11:56 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-14  7:44 [Bluez PATCH v3 0/5] Support advertising monitor add pattern with RSSI opcode Archie Pusaka
2021-01-14  7:44 ` [Bluez PATCH v3 1/5] lib/mgmt: Adding Add Adv Patterns Monitor " Archie Pusaka
2021-01-14  8:28   ` Support advertising monitor add pattern with " bluez.test.bot
2021-01-14  7:44 ` [Bluez PATCH v3 2/5] src/adv_monitor: add monitor with rssi support for mgmt Archie Pusaka
2021-01-14  7:44 ` [Bluez PATCH v3 3/5] btmgmt: advmon add rssi support Archie Pusaka
2021-01-14  7:44 ` [Bluez PATCH v3 4/5] bluetoothctl: advmon rssi support for mgmt Archie Pusaka
2021-01-14 18:41   ` Luiz Augusto von Dentz
2021-01-15 11:55     ` Archie Pusaka
2021-01-14  7:44 ` [Bluez PATCH v3 5/5] monitor: Decode add advmon with RSSI parameter Archie Pusaka

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).