[BlueZ PATCH 1/9] adv_monitor: Disable RSSIHighTimeout for SW based filtering

[BlueZ PATCH 1/9] adv_monitor: Disable RSSIHighTimeout for SW based filtering

[Manish Mandlik]

Controller offloading does not support High RSSI Timeout. Disable High
RSSI Timeout for SW based filtering as well to provide a consistent
behavior between SW based and controller based monitoring.

Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
---

 doc/advertisement-monitor-api.txt | 5 +++++
 src/adv_monitor.c                 | 6 ++++++
 2 files changed, 11 insertions(+)

diff --git a/doc/advertisement-monitor-api.txt b/doc/advertisement-monitor-api.txt
index 9189f2cce..942d44b2f 100644
--- a/doc/advertisement-monitor-api.txt
+++ b/doc/advertisement-monitor-api.txt
@@ -79,6 +79,11 @@ Properties	string Type [read-only]
 			in-range (found). Valid range is 1 to 300 (seconds),
 			while 0 indicates unset.
 
+			NOTE: Controller offloading does not support High RSSI
+			Timeout. So, to provide a consistent behavior between
+			SW based and controller based monitoring, this property
+			has been disabled and deprecated.
+
 		Uint16 RSSISamplingPeriod [read-only, optional]
 
 			Grouping rules on how to propagate the received
diff --git a/src/adv_monitor.c b/src/adv_monitor.c
index 33f4d9619..a1778248f 100644
--- a/src/adv_monitor.c
+++ b/src/adv_monitor.c
@@ -860,6 +860,12 @@ static bool parse_rssi_and_timeout(struct adv_monitor *monitor,
 	monitor->rssi.low_rssi_timeout = l_rssi_timeout;
 	monitor->rssi.sampling_period = sampling_period;
 
+	/* Controller offloading does not support High RSSI Timeout. Disable
+	 * High RSSI Timeout for SW based filtering to provide a consistent
+	 * behavior between SW based and controller based monitoring.
+	 */
+	monitor->rssi.high_rssi_timeout = ADV_MONITOR_UNSET_TIMEOUT;
+
 done:
 	DBG("Adv Monitor at %s initiated with high RSSI threshold %d, high "
 		"RSSI threshold timeout %d, low RSSI threshold %d, low RSSI "
-- 
2.35.1.894.gb6a874cedc-goog

[BlueZ PATCH 2/9] adv_monitor: Don't send DeviceFound for already found devices

[Manish Mandlik]

When monitors with same patterns are merged, BlueZ removes the existing
monitor from the kernel and re-registers it with updated/merged RSSI
parameters. The controller then triggers a new Device Found event for
the device that matches the updated monitor. Don't notify the D-Bus
client again with another DeviceFound for that device.

Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
---

 src/adv_monitor.c | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/src/adv_monitor.c b/src/adv_monitor.c
index a1778248f..a7763df10 100644
--- a/src/adv_monitor.c
+++ b/src/adv_monitor.c
@@ -188,6 +188,11 @@ static void merged_pattern_send_add(
 static void merged_pattern_send_remove(
 			struct adv_monitor_merged_pattern *merged_pattern);
 
+static bool monitor_device_match(const void *a, const void *b);
+static struct adv_monitor_device *monitor_device_create(
+			struct adv_monitor *monitor,
+			struct btd_device *device);
+
 const struct adv_monitor_type {
 	enum monitor_type type;
 	const char *name;
@@ -1555,8 +1560,26 @@ static void notify_device_found_per_monitor(void *data, void *user_data)
 {
 	struct adv_monitor *monitor = data;
 	struct monitored_device_info *info = user_data;
+	struct adv_monitor_device *dev = NULL;
+	uint16_t adapter_id = monitor->app->manager->adapter_id;
 
 	if (monitor->merged_pattern->monitor_handle == info->monitor_handle) {
+		dev = queue_find(monitor->devices, monitor_device_match,
+				 info->device);
+		if (!dev) {
+			dev = monitor_device_create(monitor, info->device);
+			if (!dev) {
+				btd_error(adapter_id, "Failed to create "
+					  "Adv Monitor device object.");
+				return;
+			}
+		}
+
+		if (dev->found)
+			return;
+
+		dev->found = true;
+
 		DBG("Calling DeviceFound() on Adv Monitor of owner %s "
 		    "at path %s", monitor->app->owner, monitor->path);
 
@@ -1652,8 +1675,25 @@ static void notify_device_lost_per_monitor(void *data, void *user_data)
 {
 	struct adv_monitor *monitor = data;
 	struct monitored_device_info *info = user_data;
+	struct adv_monitor_device *dev = NULL;
+	uint16_t adapter_id = monitor->app->manager->adapter_id;
 
 	if (monitor->merged_pattern->monitor_handle == info->monitor_handle) {
+		dev = queue_find(monitor->devices, monitor_device_match,
+				 info->device);
+		if (!dev) {
+			btd_error(adapter_id, "Adv Monitor device object "
+				  "not found.");
+			return;
+		}
+
+		if (!dev->found) {
+			btd_error(adapter_id, "Device not tracked.");
+			return;
+		}
+
+		dev->found = false;
+
 		DBG("Calling DeviceLost() on Adv Monitor of owner %s "
 		    "at path %s", monitor->app->owner, monitor->path);
 
-- 
2.35.1.894.gb6a874cedc-goog

[BlueZ PATCH 3/9] adv_monitor: Clear tracked devices on resume

[Manish Mandlik]

Clear any tracked devices on system resume. Matched devices will be
found again if they are in range after resume.

Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
---

 src/adapter.c     |  1 +
 src/adv_monitor.c | 19 +++++++++++++++++++
 src/adv_monitor.h |  2 ++
 3 files changed, 22 insertions(+)

diff --git a/src/adapter.c b/src/adapter.c
index 97ce26f8e..2ae8a9ae9 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -9183,6 +9183,7 @@ static void controller_resume_callback(uint16_t index, uint16_t length,
 	info("Controller resume with wake event 0x%x", ev->wake_reason);
 
 	controller_resume_notify(adapter);
+	btd_adv_monitor_resume(adapter->adv_monitor_manager);
 }
 
 static void device_blocked_callback(uint16_t index, uint16_t length,
diff --git a/src/adv_monitor.c b/src/adv_monitor.c
index a7763df10..35d9bc9c8 100644
--- a/src/adv_monitor.c
+++ b/src/adv_monitor.c
@@ -2248,6 +2248,7 @@ static void clear_device_lost_timer(void *data, void *user_data)
 	if (dev->lost_timer) {
 		timeout_remove(dev->lost_timer);
 		dev->lost_timer = 0;
+		dev->found = false;
 
 		monitor = dev->monitor;
 
@@ -2289,3 +2290,21 @@ void btd_adv_monitor_power_down(struct btd_adv_monitor_manager *manager)
 	/* Clear any running DeviceLost timers in case of power down */
 	queue_foreach(manager->apps, clear_lost_timers_from_app, NULL);
 }
+
+/* Handles wake from system suspend scenario */
+void btd_adv_monitor_resume(struct btd_adv_monitor_manager *manager)
+{
+	if (!manager) {
+		error("Unexpected NULL btd_adv_monitor_manager object upon "
+				"system resume");
+		return;
+	}
+
+	/* Clear any tracked devices on system resume. Matched devices will be
+	 * found again if they are in range after resume. (No need to do this if
+	 * the controller based monitoring is supported as the kernel clears all
+	 * monitored devices on resume.
+	 */
+	if (!btd_adv_monitor_offload_supported(manager))
+		queue_foreach(manager->apps, clear_lost_timers_from_app, NULL);
+}
diff --git a/src/adv_monitor.h b/src/adv_monitor.h
index c6bb8a68a..3b5b1200a 100644
--- a/src/adv_monitor.h
+++ b/src/adv_monitor.h
@@ -42,4 +42,6 @@ void btd_adv_monitor_device_remove(struct btd_adv_monitor_manager *manager,
 
 void btd_adv_monitor_power_down(struct btd_adv_monitor_manager *manager);
 
+void btd_adv_monitor_resume(struct btd_adv_monitor_manager *manager);
+
 #endif /* __ADV_MONITOR_H */
-- 
2.35.1.894.gb6a874cedc-goog

[BlueZ PATCH 4/9] adv_monitor: Do not remove the device while monitoring

[Manish Mandlik]

Bluetoothd clears temporary devices if they are not seen for 30 seconds.
When controller offloading is enabled and SamplingPeriod is set to 0xFF,
the controller sends only one advertisement report per device during the
monitoring period. In such a case, don't remove the temporary devices if
they are being monitored.

Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
---

 src/adv_monitor.c |  4 ++++
 src/device.c      | 22 +++++++++++++++++++++-
 src/device.h      |  1 +
 3 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/src/adv_monitor.c b/src/adv_monitor.c
index 35d9bc9c8..77b8ea10d 100644
--- a/src/adv_monitor.c
+++ b/src/adv_monitor.c
@@ -1663,6 +1663,8 @@ static void adv_monitor_device_found_callback(uint16_t index, uint16_t length,
 			return;
 		}
 
+		btd_device_set_monitored(info.device, true);
+
 		/* Check for matched monitor in all apps */
 		info.monitor_handle = handle;
 		queue_foreach(manager->apps, notify_device_found_per_app,
@@ -1745,6 +1747,8 @@ static void adv_monitor_device_lost_callback(uint16_t index, uint16_t length,
 	/* Check for matched monitor in all apps */
 	info.monitor_handle = handle;
 	queue_foreach(manager->apps, notify_device_lost_per_app, &info);
+
+	btd_device_set_monitored(info.device, false);
 }
 
 /* Allocates a manager object */
diff --git a/src/device.c b/src/device.c
index 3992f9a0c..00d0cc2fb 100644
--- a/src/device.c
+++ b/src/device.c
@@ -218,6 +218,7 @@ struct btd_device {
 	GSList		*services;		/* List of btd_service */
 	GSList		*pending;		/* Pending services */
 	GSList		*watches;		/* List of disconnect_data */
+	bool		monitored;		/* Tracked by Adv Monitor */
 	bool		temporary;
 	bool		connectable;
 	unsigned int	disconn_timer;
@@ -3206,11 +3207,30 @@ static bool device_disappeared(gpointer user_data)
 
 	dev->temporary_timer = 0;
 
-	btd_adapter_remove_device(dev->adapter, dev);
+	/* Do not remove the device if it is being tracked by an Advertisement
+	 * Monitor. It will be removed when the Advertisement Monitor stops
+	 * tracking that device.
+	 */
+	if (!dev->monitored)
+		btd_adapter_remove_device(dev->adapter, dev);
 
 	return FALSE;
 }
 
+void btd_device_set_monitored(struct btd_device *device, bool monitored)
+{
+	if (!device)
+		return;
+
+	device->monitored = monitored;
+
+	/* If the device is not being monitored and the temporary_timer has
+	 * already expired, it indicates that the device can be removed.
+	 */
+	if (!monitored && device->temporary && !device->temporary_timer)
+		device_disappeared(device);
+}
+
 static void set_temporary_timer(struct btd_device *dev, unsigned int timeout)
 {
 	clear_temporary_timer(dev);
diff --git a/src/device.h b/src/device.h
index 071576d6b..0a4103747 100644
--- a/src/device.h
+++ b/src/device.h
@@ -87,6 +87,7 @@ bool device_is_connectable(struct btd_device *device);
 bool device_is_paired(struct btd_device *device, uint8_t bdaddr_type);
 bool device_is_bonded(struct btd_device *device, uint8_t bdaddr_type);
 gboolean device_is_trusted(struct btd_device *device);
+void btd_device_set_monitored(struct btd_device *device, bool monitored);
 void device_set_paired(struct btd_device *dev, uint8_t bdaddr_type);
 void device_set_unpaired(struct btd_device *dev, uint8_t bdaddr_type);
 void btd_device_set_temporary(struct btd_device *device, bool temporary);
-- 
2.35.1.894.gb6a874cedc-goog

[BlueZ PATCH 5/9] monitor: Display AdvMonitor DeviceFound/Lost events

[Manish Mandlik]

Display information about MGMT_EV_ADV_MONITOR_DEVICE_FOUND and
MGMT_EV_ADV_MONITOR_DEVICE_LOST events in the btmon output.

Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
---

 monitor/packet.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/monitor/packet.c b/monitor/packet.c
index b7431b57d..6f615f7ba 100644
--- a/monitor/packet.c
+++ b/monitor/packet.c
@@ -14103,6 +14103,31 @@ static void mgmt_device_found_evt(const void *data, uint16_t size)
 	print_eir(data + 14, size - 14, false);
 }
 
+static void mgmt_adv_monitor_device_found_evt(const void *data, uint16_t size)
+{
+	uint16_t handle = get_le16(data);
+	uint8_t address_type = get_u8(data + 8);
+	int8_t rssi = get_s8(data + 9);
+	uint32_t flags = get_le32(data + 10);
+	uint16_t data_len = get_le16(data + 14);
+
+	print_field("Handle: %u", handle);
+	mgmt_print_address(data + 2, address_type);
+	print_rssi(rssi);
+	mgmt_print_device_flags(flags);
+	print_field("Data length: %u", data_len);
+	print_eir(data + 16, size - 16, false);
+}
+
+static void mgmt_adv_monitor_device_lost_evt(const void *data, uint16_t size)
+{
+	uint16_t handle = get_le16(data);
+	uint8_t address_type = get_u8(data + 8);
+
+	print_field("Handle: %u", handle);
+	mgmt_print_address(data + 2, address_type);
+}
+
 static void mgmt_discovering_evt(const void *data, uint16_t size)
 {
 	uint8_t type = get_u8(data);
@@ -14414,6 +14439,10 @@ static const struct mgmt_data mgmt_event_table[] = {
 			mgmt_controller_suspend_evt, 1, true },
 	{ 0x002e, "Controller Resumed",
 			mgmt_controller_resume_evt, 8, true },
+	{ 0x002f, "Adv Monitor Device Found",
+			mgmt_adv_monitor_device_found_evt, 16, false },
+	{ 0x0030, "Adv Monitor Device Lost",
+			mgmt_adv_monitor_device_lost_evt, 9, true },
 	{ }
 };
 
-- 
2.35.1.894.gb6a874cedc-goog

[BlueZ PATCH 6/9] adv_monitor: Do not merge monitors with non-overlapping RSSI

[Manish Mandlik]

Merging two monitors with different RSSI thresholds is not possible
if their RSSI ranges do not overlap.

Example:
         Monitor 1: -40 -80         Result: Merge with updated RSSI
         Monitor 2: -60 -100                thresholds -60 -100

         Monitor 1: -40 -100        Result: Merge with updated RSSI
         Monitor 2: -60 -80                 thresholds -60 -100

         Monitor 1: -40 -60         Result: Do not merge
         Monitor 2: -80 -100

Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
---

 src/adv_monitor.c | 58 ++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 50 insertions(+), 8 deletions(-)

diff --git a/src/adv_monitor.c b/src/adv_monitor.c
index 77b8ea10d..85231557e 100644
--- a/src/adv_monitor.c
+++ b/src/adv_monitor.c
@@ -251,6 +251,46 @@ static void merged_pattern_free(void *data)
 	free(merged_pattern);
 }
 
+/* Checks if merging monitors with different RSSI Thresh is possible or not */
+static bool merge_is_possible(
+			struct adv_monitor_merged_pattern *existing_pattern,
+			struct adv_monitor *monitor)
+{
+	const struct queue_entry *q_entry;
+	struct adv_monitor *q_data;
+
+	/* Merging two monitors with different RSSI thresholds is not possible
+	 * if their RSSI ranges do not overlap.
+	 */
+
+	q_entry = queue_get_entries(existing_pattern->monitors);
+
+	while (q_entry) {
+		q_data = q_entry->data;
+
+		if (q_data->rssi.low_rssi >= monitor->rssi.high_rssi ||
+		    monitor->rssi.low_rssi >= q_data->rssi.high_rssi)
+			goto fail;
+
+		q_entry = q_entry->next;
+	}
+
+	return true;
+
+fail:
+	monitor->state = MONITOR_STATE_FAILED;
+	merged_pattern_free(monitor->merged_pattern);
+	monitor->merged_pattern = NULL;
+
+	btd_error(monitor->app->manager->adapter_id,
+			"Adv Monitor at path %s is in conflict with "
+			"an existing Adv Monitor at path %s",
+			g_dbus_proxy_get_path(monitor->proxy),
+			g_dbus_proxy_get_path(q_data->proxy));
+
+	return false;
+}
+
 /* Returns the smaller of the two integers |a| and |b| which is not equal to the
  * |unset| value. If both are unset, return unset.
  */
@@ -291,14 +331,9 @@ static void merge_rssi(const struct rssi_parameters *a,
 	 */
 	merged->high_rssi_timeout = 0;
 
-	/* Sampling period is not implemented yet in userspace. There is no
-	 * good value if the two values are different, so just choose 0 for
-	 * always reporting, to avoid missing packets.
-	 */
-	if (a->sampling_period != b->sampling_period)
-		merged->sampling_period = 0;
-	else
-		merged->sampling_period = a->sampling_period;
+	merged->sampling_period = get_smaller_not_unset(a->sampling_period,
+					b->sampling_period,
+					ADV_MONITOR_UNSET_SAMPLING_PERIOD);
 }
 
 /* Two merged_pattern are considered equal if all the following are true:
@@ -1249,6 +1284,13 @@ static void monitor_proxy_added_cb(GDBusProxy *proxy, void *user_data)
 						monitor->merged_pattern);
 		merged_pattern_add(monitor->merged_pattern);
 	} else {
+		if (!merge_is_possible(existing_pattern, monitor)) {
+			monitor_destroy(monitor);
+			DBG("Adv Monitor at path %s released due to existing "
+				"monitor", path);
+			return;
+		}
+
 		/* Since there is a matching pattern, abandon the one we have */
 		merged_pattern_free(monitor->merged_pattern);
 		monitor->merged_pattern = existing_pattern;
-- 
2.35.1.894.gb6a874cedc-goog

[BlueZ PATCH 7/9] adv_monitor: Add the monitor Release reason

[Manish Mandlik]

Adv Monitor is released for various reasons. For example, incorrect RSSI
parameters, incorrect monitor type, non-overlapping RSSI thresholds,
etc.

Return this release reason along with the Release event for the better
visibility to clients.

Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
---

 doc/advertisement-monitor-api.txt | 12 ++++++-
 src/adv_monitor.c                 | 56 ++++++++++++++++++++++++++++---
 2 files changed, 63 insertions(+), 5 deletions(-)

diff --git a/doc/advertisement-monitor-api.txt b/doc/advertisement-monitor-api.txt
index 942d44b2f..fcbd9c5c2 100644
--- a/doc/advertisement-monitor-api.txt
+++ b/doc/advertisement-monitor-api.txt
@@ -17,12 +17,22 @@ Service		org.bluez
 Interface	org.bluez.AdvertisementMonitor1 [experimental]
 Object path	freely definable
 
-Methods		void Release() [noreply]
+Methods		void Release(Int8 reason) [noreply]
 
 			This gets called as a signal for a client to perform
 			clean-up when (1)a monitor cannot be activated after it
 			was exposed or (2)a monitor has been deactivated.
 
+			Possible reasons:  0  Unknown reason
+					   1  Invalid monitor type
+					   2  Invalid RSSI parameter(s)
+					   3  Invalid pattern(s)
+					   4  Monitor already exists
+					   5  Kernel failed to add monitor
+					   6  Kernel failed to remove monitor
+					   7  Monitor removed by kernel
+					   8  App unregistered/destroyed
+
 		void Activate() [noreply]
 
 			After a monitor was exposed, this gets called as a
diff --git a/src/adv_monitor.c b/src/adv_monitor.c
index 85231557e..6ee3736d2 100644
--- a/src/adv_monitor.c
+++ b/src/adv_monitor.c
@@ -90,6 +90,18 @@ enum monitor_state {
 	MONITOR_STATE_RELEASED,	/* Dbus Object removed by app */
 };
 
+enum monitor_release_reason {
+	REASON_UNKNOWN,
+	REASON_INVALID_TYPE,
+	REASON_INVALID_RSSI_PARAMS,
+	REASON_INVALID_PATTERNS,
+	REASON_ALREADY_EXISTS,
+	REASON_FAILED_TO_ADD,
+	REASON_FAILED_TO_REMOVE,
+	REASON_REMOVED_BY_KERNEL,
+	REASON_APP_DESTROYED,
+};
+
 enum merged_pattern_state {
 	MERGED_PATTERN_STATE_ADDING,	/* Adding pattern to kernel */
 	MERGED_PATTERN_STATE_REMOVING,	/* Removing pattern from kernel */
@@ -113,6 +125,7 @@ struct adv_monitor {
 	char *path;
 
 	enum monitor_state state;	/* MONITOR_STATE_* */
+	enum monitor_release_reason release_reason;
 
 	struct rssi_parameters rssi;	/* RSSI parameter for this monitor */
 	struct adv_monitor_merged_pattern *merged_pattern;
@@ -137,6 +150,7 @@ struct adv_monitor_merged_pattern {
 	struct queue *patterns;		/* List of bt_ad_pattern objects */
 	enum merged_pattern_state current_state; /* MERGED_PATTERN_STATE_* */
 	enum merged_pattern_state next_state;	 /* MERGED_PATTERN_STATE_* */
+	enum monitor_release_reason release_reason;
 };
 
 /* Some data like last_seen, timer/timeout values need to be maintained
@@ -541,6 +555,18 @@ static void monitor_free(struct adv_monitor *monitor)
 	free(monitor);
 }
 
+/* Includes monitor release reason into the dbus message */
+static void report_release_reason_setup(DBusMessageIter *iter, void *user_data)
+{
+	const struct adv_monitor *monitor = user_data;
+	int8_t release_reason = REASON_UNKNOWN;
+
+	if (monitor)
+		release_reason = monitor->release_reason;
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_BYTE, &release_reason);
+}
+
 /* Calls Release() method of the remote Adv Monitor */
 static void monitor_release(struct adv_monitor *monitor)
 {
@@ -560,8 +586,9 @@ static void monitor_release(struct adv_monitor *monitor)
 	DBG("Calling Release() on Adv Monitor of owner %s at path %s",
 		monitor->app->owner, monitor->path);
 
-	g_dbus_proxy_method_call(monitor->proxy, "Release", NULL, NULL, NULL,
-					NULL);
+	g_dbus_proxy_method_call(monitor->proxy, "Release",
+					report_release_reason_setup, NULL,
+					monitor, NULL);
 }
 
 /* Removes monitor from the merged_pattern. This would result in removing it
@@ -635,13 +662,20 @@ static void monitor_destroy(void *data)
 static void app_destroy(void *data)
 {
 	struct adv_monitor_app *app = data;
+	const struct queue_entry *e;
 
 	if (!app)
 		return;
 
 	DBG("Destroy Adv Monitor app %s at path %s", app->owner, app->path);
 
-	queue_destroy(app->monitors, monitor_destroy);
+	for (e = queue_get_entries(app->monitors); e; e = e->next) {
+		struct adv_monitor *m = e->data;
+
+		m->release_reason = REASON_APP_DESTROYED;
+		monitor_destroy(m);
+	}
+	queue_destroy(app->monitors, NULL);
 
 	if (app->reg) {
 		app_reply_msg(app, btd_error_failed(app->reg,
@@ -793,6 +827,7 @@ static bool parse_monitor_type(struct adv_monitor *monitor, const char *path)
 	}
 
 failed:
+	monitor->release_reason = REASON_INVALID_TYPE;
 	btd_error(adapter_id,
 			"Invalid argument of property Type of the Adv Monitor "
 			"at path %s", path);
@@ -919,6 +954,7 @@ done:
 	return true;
 
 failed:
+	monitor->release_reason = REASON_INVALID_RSSI_PARAMS;
 	btd_error(adapter_id,
 			"Invalid argument of RSSI thresholds and timeouts "
 			"of the Adv Monitor at path %s",
@@ -1005,6 +1041,7 @@ static bool parse_patterns(struct adv_monitor *monitor, const char *path)
 	return true;
 
 failed:
+	monitor->release_reason = REASON_INVALID_PATTERNS;
 	btd_error(adapter_id, "Invalid argument of property Patterns of the "
 			"Adv Monitor at path %s", path);
 
@@ -1053,6 +1090,7 @@ static void merged_pattern_destroy_monitors(
 		struct adv_monitor *monitor = e->data;
 
 		monitor->merged_pattern = NULL;
+		monitor->release_reason = merged_pattern->release_reason;
 		monitor_destroy(monitor);
 	}
 }
@@ -1086,6 +1124,7 @@ static void remove_adv_monitor_cb(uint8_t status, uint16_t length,
 	return;
 
 fail:
+	merged_pattern->release_reason = REASON_FAILED_TO_REMOVE;
 	merged_pattern_destroy_monitors(merged_pattern);
 	merged_pattern_free(merged_pattern);
 }
@@ -1142,6 +1181,7 @@ static void add_adv_patterns_monitor_cb(uint8_t status, uint16_t length,
 	return;
 
 fail:
+	merged_pattern->release_reason = REASON_FAILED_TO_ADD;
 	merged_pattern_destroy_monitors(merged_pattern);
 	merged_pattern_free(merged_pattern);
 }
@@ -1285,6 +1325,7 @@ static void monitor_proxy_added_cb(GDBusProxy *proxy, void *user_data)
 		merged_pattern_add(monitor->merged_pattern);
 	} else {
 		if (!merge_is_possible(existing_pattern, monitor)) {
+			monitor->release_reason = REASON_ALREADY_EXISTS;
 			monitor_destroy(monitor);
 			DBG("Adv Monitor at path %s released due to existing "
 				"monitor", path);
@@ -1551,6 +1592,7 @@ static void remove_merged_pattern(void *data, void *user_data)
 {
 	struct adv_monitor_merged_pattern *merged_pattern = data;
 	uint16_t *handle = user_data;
+	const struct queue_entry *e;
 
 	if (!handle)
 		return;
@@ -1562,8 +1604,14 @@ static void remove_merged_pattern(void *data, void *user_data)
 	DBG("Adv monitor with handle:0x%04x removed by kernel",
 		merged_pattern->monitor_handle);
 
+	for (e = queue_get_entries(merged_pattern->monitors); e; e = e->next) {
+		struct adv_monitor *m = e->data;
+
+		m->release_reason = REASON_REMOVED_BY_KERNEL;
+		monitor_destroy(m);
+	}
 	queue_foreach(merged_pattern->monitors, monitor_state_removed, NULL);
-	queue_destroy(merged_pattern->monitors, monitor_destroy);
+	queue_destroy(merged_pattern->monitors, NULL);
 	merged_pattern_free(merged_pattern);
 }
 
-- 
2.35.1.894.gb6a874cedc-goog

[BlueZ PATCH 8/9] client: Display the AdvMonitor Release reason

[Manish Mandlik]

Bluetoothd returns the release reason when a monitor is released. Read
the release reason received as part of the Release event and print it
using the bluetoothctl.

Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
---

 client/adv_monitor.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/client/adv_monitor.c b/client/adv_monitor.c
index 792379fc4..6ee9d2b42 100644
--- a/client/adv_monitor.c
+++ b/client/adv_monitor.c
@@ -72,9 +72,13 @@ static DBusMessage *release_adv_monitor(DBusConnection *conn,
 					DBusMessage *msg, void *user_data)
 {
 	struct adv_monitor *adv_monitor = user_data;
+	int8_t release_reason;
 
-	bt_shell_printf("Advertisement monitor %d released\n",
-							adv_monitor->idx);
+	dbus_message_get_args(msg, NULL, DBUS_TYPE_BYTE, &release_reason,
+							DBUS_TYPE_INVALID);
+	bt_shell_printf("Advertisement monitor %d released (reason: %d)\n",
+							adv_monitor->idx,
+							release_reason);
 	remove_adv_monitor(adv_monitor, conn);
 
 	return dbus_message_new_method_return(msg);
@@ -117,7 +121,8 @@ static DBusMessage *device_lost_adv_monitor(DBusConnection *conn,
 }
 
 static const GDBusMethodTable adv_monitor_methods[] = {
-	{ GDBUS_ASYNC_METHOD("Release", NULL, NULL, release_adv_monitor) },
+	{ GDBUS_ASYNC_METHOD("Release", GDBUS_ARGS({"reason", "y"}),
+			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) },
-- 
2.35.1.894.gb6a874cedc-goog

[BlueZ PATCH 9/9] test: Display the AdvMonitor Release reason

[Manish Mandlik]

Bluetoothd returns the release reason when a monitor is released. Read
the release reason received as part of the Release event and print it
using the example python app.

Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
---

 test/example-adv-monitor | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/test/example-adv-monitor b/test/example-adv-monitor
index a405fc7b0..b01114c6b 100644
--- a/test/example-adv-monitor
+++ b/test/example-adv-monitor
@@ -117,10 +117,11 @@ class AdvMonitor(dbus.service.Object):
 
 
     @dbus.service.method(ADV_MONITOR_IFACE,
-                         in_signature='',
+                         in_signature='y',
                          out_signature='')
-    def Release(self):
-        print('{}: Monitor Released'.format(self.path))
+    def Release(self, reason):
+        print('{}: Monitor Released (reason: {})'.format(self.path,
+                                                         format(reason, 'd')))
 
 
     @dbus.service.method(ADV_MONITOR_IFACE,
@@ -352,7 +353,10 @@ def test(bus, mainloop, advmon_mgr, app_id):
     # Run until user hits the 'Enter' key. If any peer device is advertising
     # during this time, DeviceFound() should get triggered for monitors
     # matching the advertisements.
-    raw_input('Press "Enter" key to quit...\n')
+    try:
+        raw_input('Press "Enter" key to quit...\n')  # python2
+    except:
+        input('Press "Enter" key to quit...\n')  # python3
 
     # Remove a monitor. DeviceFound() for this monitor should not get
     # triggered any more.
-- 
2.35.1.894.gb6a874cedc-goog

RE: [BlueZ,1/9] adv_monitor: Disable RSSIHighTimeout for SW based filtering

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 7611 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=624998

---Test result---

Test Summary:
CheckPatch                    PASS      13.09 seconds
GitLint                       PASS      8.89 seconds
Prep - Setup ELL              PASS      46.69 seconds
Build - Prep                  PASS      0.76 seconds
Build - Configure             PASS      9.23 seconds
Build - Make                  FAIL      1504.46 seconds
Make Check                    FAIL      2.42 seconds
Make Check w/Valgrind         FAIL      324.61 seconds
Make Distcheck                FAIL      173.77 seconds
Build w/ext ELL - Configure   PASS      9.30 seconds
Build w/ext ELL - Make        FAIL      1466.64 seconds
Incremental Build with patchesFAIL      4186.16 seconds

Details
##############################
Test: Build - Make - FAIL
Desc: Build the BlueZ source tree
Output:
tools/mgmt-tester.c: In function ‘main’:
tools/mgmt-tester.c:12364:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
12364 | int main(int argc, char *argv[])
      |     ^~~~
unit/test-avdtp.c: In function ‘main’:
unit/test-avdtp.c:766:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
  766 | int main(int argc, char *argv[])
      |     ^~~~
unit/test-avrcp.c: In function ‘main’:
unit/test-avrcp.c:989:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
  989 | int main(int argc, char *argv[])
      |     ^~~~
src/adv_monitor.c: In function ‘btd_adv_monitor_resume’:
src/adv_monitor.c:2402:7: error: implicit declaration of function ‘btd_adv_monitor_offload_supported’; did you mean ‘btd_adv_monitor_offload_enabled’? [-Werror=implicit-function-declaration]
 2402 |  if (!btd_adv_monitor_offload_supported(manager))
      |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |       btd_adv_monitor_offload_enabled
cc1: all warnings being treated as errors
make[1]: *** [Makefile:10284: src/bluetoothd-adv_monitor.o] Error 1
make: *** [Makefile:4307: all] Error 2


##############################
Test: Make Check - FAIL
Desc: Run 'make check'
Output:
src/adv_monitor.c: In function ‘btd_adv_monitor_resume’:
src/adv_monitor.c:2402:7: error: implicit declaration of function ‘btd_adv_monitor_offload_supported’; did you mean ‘btd_adv_monitor_offload_enabled’? [-Werror=implicit-function-declaration]
 2402 |  if (!btd_adv_monitor_offload_supported(manager))
      |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |       btd_adv_monitor_offload_enabled
cc1: all warnings being treated as errors
make[1]: *** [Makefile:10284: src/bluetoothd-adv_monitor.o] Error 1
make: *** [Makefile:11277: check] Error 2


##############################
Test: Make Check w/Valgrind - FAIL
Desc: Run 'make check' with Valgrind
Output:
tools/mgmt-tester.c: In function ‘main’:
tools/mgmt-tester.c:12364:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
12364 | int main(int argc, char *argv[])
      |     ^~~~
src/adv_monitor.c: In function ‘btd_adv_monitor_resume’:
src/adv_monitor.c:2402:7: error: implicit declaration of function ‘btd_adv_monitor_offload_supported’; did you mean ‘btd_adv_monitor_offload_enabled’? [-Werror=implicit-function-declaration]
 2402 |  if (!btd_adv_monitor_offload_supported(manager))
      |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |       btd_adv_monitor_offload_enabled
cc1: all warnings being treated as errors
make[1]: *** [Makefile:10284: src/bluetoothd-adv_monitor.o] Error 1
make: *** [Makefile:4307: all] Error 2


##############################
Test: Make Distcheck - FAIL
Desc: Run distcheck to check the distribution
Output:
../../src/adv_monitor.c: In function ‘btd_adv_monitor_resume’:
../../src/adv_monitor.c:2402:7: warning: implicit declaration of function ‘btd_adv_monitor_offload_supported’; did you mean ‘btd_adv_monitor_offload_enabled’? [-Wimplicit-function-declaration]
 2402 |  if (!btd_adv_monitor_offload_supported(manager))
      |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |       btd_adv_monitor_offload_enabled
/usr/bin/ld: src/bluetoothd-adv_monitor.o: in function `btd_adv_monitor_resume':
/github/workspace/src/bluez-5.63/_build/sub/../../src/adv_monitor.c:2402: undefined reference to `btd_adv_monitor_offload_supported'
collect2: error: ld returned 1 exit status
make[2]: *** [Makefile:5878: src/bluetoothd] Error 1
make[1]: *** [Makefile:4307: all] Error 2
make: *** [Makefile:11198: distcheck] Error 1


##############################
Test: Build w/ext ELL - Make - FAIL
Desc: Build BlueZ source with '--enable-external-ell' configuration
Output:
tools/mgmt-tester.c: In function ‘main’:
tools/mgmt-tester.c:12364:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
12364 | int main(int argc, char *argv[])
      |     ^~~~
unit/test-avdtp.c: In function ‘main’:
unit/test-avdtp.c:766:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
  766 | int main(int argc, char *argv[])
      |     ^~~~
unit/test-avrcp.c: In function ‘main’:
unit/test-avrcp.c:989:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
  989 | int main(int argc, char *argv[])
      |     ^~~~
src/adv_monitor.c: In function ‘btd_adv_monitor_resume’:
src/adv_monitor.c:2402:7: error: implicit declaration of function ‘btd_adv_monitor_offload_supported’; did you mean ‘btd_adv_monitor_offload_enabled’? [-Werror=implicit-function-declaration]
 2402 |  if (!btd_adv_monitor_offload_supported(manager))
      |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |       btd_adv_monitor_offload_enabled
cc1: all warnings being treated as errors
make[1]: *** [Makefile:10284: src/bluetoothd-adv_monitor.o] Error 1
make: *** [Makefile:4307: all] Error 2


##############################
Test: Incremental Build with patches - FAIL
Desc: Incremental build per patch in the series
Output:
tools/mgmt-tester.c: In function ‘main’:
tools/mgmt-tester.c:12364:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
12364 | int main(int argc, char *argv[])
      |     ^~~~
unit/test-avdtp.c: In function ‘main’:
unit/test-avdtp.c:766:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
  766 | int main(int argc, char *argv[])
      |     ^~~~
unit/test-avrcp.c: In function ‘main’:
unit/test-avrcp.c:989:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
  989 | int main(int argc, char *argv[])
      |     ^~~~
src/adv_monitor.c: In function ‘btd_adv_monitor_resume’:
src/adv_monitor.c:2308:7: error: implicit declaration of function ‘btd_adv_monitor_offload_supported’; did you mean ‘btd_adv_monitor_offload_enabled’? [-Werror=implicit-function-declaration]
 2308 |  if (!btd_adv_monitor_offload_supported(manager))
      |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |       btd_adv_monitor_offload_enabled
cc1: all warnings being treated as errors
make[1]: *** [Makefile:10284: src/bluetoothd-adv_monitor.o] Error 1
make: *** [Makefile:4307: all] Error 2




---
Regards,
Linux Bluetooth

Re: [BlueZ PATCH 5/9] monitor: Display AdvMonitor DeviceFound/Lost events

[Paul Menzel]

Dear Manish,


Thank you for the patch.

Am 21.03.22 um 02:36 schrieb Manish Mandlik:
> Display information about MGMT_EV_ADV_MONITOR_DEVICE_FOUND and
> MGMT_EV_ADV_MONITOR_DEVICE_LOST events in the btmon output.

If you should reroll this patch, could you please add example messages 
to the commit message?


Kind regards,

Paul


> Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
> ---
> 
>   monitor/packet.c | 29 +++++++++++++++++++++++++++++
>   1 file changed, 29 insertions(+)
> 
> diff --git a/monitor/packet.c b/monitor/packet.c
> index b7431b57d..6f615f7ba 100644
> --- a/monitor/packet.c
> +++ b/monitor/packet.c
> @@ -14103,6 +14103,31 @@ static void mgmt_device_found_evt(const void *data, uint16_t size)
>   	print_eir(data + 14, size - 14, false);
>   }
>   
> +static void mgmt_adv_monitor_device_found_evt(const void *data, uint16_t size)
> +{
> +	uint16_t handle = get_le16(data);
> +	uint8_t address_type = get_u8(data + 8);
> +	int8_t rssi = get_s8(data + 9);
> +	uint32_t flags = get_le32(data + 10);
> +	uint16_t data_len = get_le16(data + 14);
> +
> +	print_field("Handle: %u", handle);
> +	mgmt_print_address(data + 2, address_type);
> +	print_rssi(rssi);
> +	mgmt_print_device_flags(flags);
> +	print_field("Data length: %u", data_len);
> +	print_eir(data + 16, size - 16, false);
> +}
> +
> +static void mgmt_adv_monitor_device_lost_evt(const void *data, uint16_t size)
> +{
> +	uint16_t handle = get_le16(data);
> +	uint8_t address_type = get_u8(data + 8);
> +
> +	print_field("Handle: %u", handle);
> +	mgmt_print_address(data + 2, address_type);
> +}
> +
>   static void mgmt_discovering_evt(const void *data, uint16_t size)
>   {
>   	uint8_t type = get_u8(data);
> @@ -14414,6 +14439,10 @@ static const struct mgmt_data mgmt_event_table[] = {
>   			mgmt_controller_suspend_evt, 1, true },
>   	{ 0x002e, "Controller Resumed",
>   			mgmt_controller_resume_evt, 8, true },
> +	{ 0x002f, "Adv Monitor Device Found",
> +			mgmt_adv_monitor_device_found_evt, 16, false },
> +	{ 0x0030, "Adv Monitor Device Lost",
> +			mgmt_adv_monitor_device_lost_evt, 9, true },
>   	{ }
>   };
>   

Re: [BlueZ PATCH 7/9] adv_monitor: Add the monitor Release reason

[Luiz Augusto von Dentz]

Hi Manish,

On Sun, Mar 20, 2022 at 6:37 PM Manish Mandlik <mmandlik@google.com> wrote:
>
> Adv Monitor is released for various reasons. For example, incorrect RSSI
> parameters, incorrect monitor type, non-overlapping RSSI thresholds,
> etc.
>
> Return this release reason along with the Release event for the better
> visibility to clients.

Shouldn't this be sent as a reply to the method registering the monitor?

> Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
> ---
>
>  doc/advertisement-monitor-api.txt | 12 ++++++-
>  src/adv_monitor.c                 | 56 ++++++++++++++++++++++++++++---
>  2 files changed, 63 insertions(+), 5 deletions(-)
>
> diff --git a/doc/advertisement-monitor-api.txt b/doc/advertisement-monitor-api.txt
> index 942d44b2f..fcbd9c5c2 100644
> --- a/doc/advertisement-monitor-api.txt
> +++ b/doc/advertisement-monitor-api.txt
> @@ -17,12 +17,22 @@ Service             org.bluez
>  Interface      org.bluez.AdvertisementMonitor1 [experimental]
>  Object path    freely definable
>
> -Methods                void Release() [noreply]
> +Methods                void Release(Int8 reason) [noreply]
>
>                         This gets called as a signal for a client to perform
>                         clean-up when (1)a monitor cannot be activated after it
>                         was exposed or (2)a monitor has been deactivated.
>
> +                       Possible reasons:  0  Unknown reason
> +                                          1  Invalid monitor type
> +                                          2  Invalid RSSI parameter(s)
> +                                          3  Invalid pattern(s)
> +                                          4  Monitor already exists
> +                                          5  Kernel failed to add monitor
> +                                          6  Kernel failed to remove monitor
> +                                          7  Monitor removed by kernel
> +                                          8  App unregistered/destroyed
> +
>                 void Activate() [noreply]
>
>                         After a monitor was exposed, this gets called as a
> diff --git a/src/adv_monitor.c b/src/adv_monitor.c
> index 85231557e..6ee3736d2 100644
> --- a/src/adv_monitor.c
> +++ b/src/adv_monitor.c
> @@ -90,6 +90,18 @@ enum monitor_state {
>         MONITOR_STATE_RELEASED, /* Dbus Object removed by app */
>  };
>
> +enum monitor_release_reason {
> +       REASON_UNKNOWN,
> +       REASON_INVALID_TYPE,
> +       REASON_INVALID_RSSI_PARAMS,
> +       REASON_INVALID_PATTERNS,
> +       REASON_ALREADY_EXISTS,
> +       REASON_FAILED_TO_ADD,
> +       REASON_FAILED_TO_REMOVE,
> +       REASON_REMOVED_BY_KERNEL,
> +       REASON_APP_DESTROYED,
> +};
> +
>  enum merged_pattern_state {
>         MERGED_PATTERN_STATE_ADDING,    /* Adding pattern to kernel */
>         MERGED_PATTERN_STATE_REMOVING,  /* Removing pattern from kernel */
> @@ -113,6 +125,7 @@ struct adv_monitor {
>         char *path;
>
>         enum monitor_state state;       /* MONITOR_STATE_* */
> +       enum monitor_release_reason release_reason;
>
>         struct rssi_parameters rssi;    /* RSSI parameter for this monitor */
>         struct adv_monitor_merged_pattern *merged_pattern;
> @@ -137,6 +150,7 @@ struct adv_monitor_merged_pattern {
>         struct queue *patterns;         /* List of bt_ad_pattern objects */
>         enum merged_pattern_state current_state; /* MERGED_PATTERN_STATE_* */
>         enum merged_pattern_state next_state;    /* MERGED_PATTERN_STATE_* */
> +       enum monitor_release_reason release_reason;
>  };
>
>  /* Some data like last_seen, timer/timeout values need to be maintained
> @@ -541,6 +555,18 @@ static void monitor_free(struct adv_monitor *monitor)
>         free(monitor);
>  }
>
> +/* Includes monitor release reason into the dbus message */
> +static void report_release_reason_setup(DBusMessageIter *iter, void *user_data)
> +{
> +       const struct adv_monitor *monitor = user_data;
> +       int8_t release_reason = REASON_UNKNOWN;
> +
> +       if (monitor)
> +               release_reason = monitor->release_reason;
> +
> +       dbus_message_iter_append_basic(iter, DBUS_TYPE_BYTE, &release_reason);
> +}
> +
>  /* Calls Release() method of the remote Adv Monitor */
>  static void monitor_release(struct adv_monitor *monitor)
>  {
> @@ -560,8 +586,9 @@ static void monitor_release(struct adv_monitor *monitor)
>         DBG("Calling Release() on Adv Monitor of owner %s at path %s",
>                 monitor->app->owner, monitor->path);
>
> -       g_dbus_proxy_method_call(monitor->proxy, "Release", NULL, NULL, NULL,
> -                                       NULL);
> +       g_dbus_proxy_method_call(monitor->proxy, "Release",
> +                                       report_release_reason_setup, NULL,
> +                                       monitor, NULL);
>  }
>
>  /* Removes monitor from the merged_pattern. This would result in removing it
> @@ -635,13 +662,20 @@ static void monitor_destroy(void *data)
>  static void app_destroy(void *data)
>  {
>         struct adv_monitor_app *app = data;
> +       const struct queue_entry *e;
>
>         if (!app)
>                 return;
>
>         DBG("Destroy Adv Monitor app %s at path %s", app->owner, app->path);
>
> -       queue_destroy(app->monitors, monitor_destroy);
> +       for (e = queue_get_entries(app->monitors); e; e = e->next) {
> +               struct adv_monitor *m = e->data;
> +
> +               m->release_reason = REASON_APP_DESTROYED;
> +               monitor_destroy(m);
> +       }
> +       queue_destroy(app->monitors, NULL);
>
>         if (app->reg) {
>                 app_reply_msg(app, btd_error_failed(app->reg,
> @@ -793,6 +827,7 @@ static bool parse_monitor_type(struct adv_monitor *monitor, const char *path)
>         }
>
>  failed:
> +       monitor->release_reason = REASON_INVALID_TYPE;
>         btd_error(adapter_id,
>                         "Invalid argument of property Type of the Adv Monitor "
>                         "at path %s", path);
> @@ -919,6 +954,7 @@ done:
>         return true;
>
>  failed:
> +       monitor->release_reason = REASON_INVALID_RSSI_PARAMS;
>         btd_error(adapter_id,
>                         "Invalid argument of RSSI thresholds and timeouts "
>                         "of the Adv Monitor at path %s",
> @@ -1005,6 +1041,7 @@ static bool parse_patterns(struct adv_monitor *monitor, const char *path)
>         return true;
>
>  failed:
> +       monitor->release_reason = REASON_INVALID_PATTERNS;
>         btd_error(adapter_id, "Invalid argument of property Patterns of the "
>                         "Adv Monitor at path %s", path);
>
> @@ -1053,6 +1090,7 @@ static void merged_pattern_destroy_monitors(
>                 struct adv_monitor *monitor = e->data;
>
>                 monitor->merged_pattern = NULL;
> +               monitor->release_reason = merged_pattern->release_reason;
>                 monitor_destroy(monitor);
>         }
>  }
> @@ -1086,6 +1124,7 @@ static void remove_adv_monitor_cb(uint8_t status, uint16_t length,
>         return;
>
>  fail:
> +       merged_pattern->release_reason = REASON_FAILED_TO_REMOVE;
>         merged_pattern_destroy_monitors(merged_pattern);
>         merged_pattern_free(merged_pattern);
>  }
> @@ -1142,6 +1181,7 @@ static void add_adv_patterns_monitor_cb(uint8_t status, uint16_t length,
>         return;
>
>  fail:
> +       merged_pattern->release_reason = REASON_FAILED_TO_ADD;
>         merged_pattern_destroy_monitors(merged_pattern);
>         merged_pattern_free(merged_pattern);
>  }
> @@ -1285,6 +1325,7 @@ static void monitor_proxy_added_cb(GDBusProxy *proxy, void *user_data)
>                 merged_pattern_add(monitor->merged_pattern);
>         } else {
>                 if (!merge_is_possible(existing_pattern, monitor)) {
> +                       monitor->release_reason = REASON_ALREADY_EXISTS;
>                         monitor_destroy(monitor);
>                         DBG("Adv Monitor at path %s released due to existing "
>                                 "monitor", path);
> @@ -1551,6 +1592,7 @@ static void remove_merged_pattern(void *data, void *user_data)
>  {
>         struct adv_monitor_merged_pattern *merged_pattern = data;
>         uint16_t *handle = user_data;
> +       const struct queue_entry *e;
>
>         if (!handle)
>                 return;
> @@ -1562,8 +1604,14 @@ static void remove_merged_pattern(void *data, void *user_data)
>         DBG("Adv monitor with handle:0x%04x removed by kernel",
>                 merged_pattern->monitor_handle);
>
> +       for (e = queue_get_entries(merged_pattern->monitors); e; e = e->next) {
> +               struct adv_monitor *m = e->data;
> +
> +               m->release_reason = REASON_REMOVED_BY_KERNEL;
> +               monitor_destroy(m);
> +       }
>         queue_foreach(merged_pattern->monitors, monitor_state_removed, NULL);
> -       queue_destroy(merged_pattern->monitors, monitor_destroy);
> +       queue_destroy(merged_pattern->monitors, NULL);
>         merged_pattern_free(merged_pattern);
>  }
>
> --
> 2.35.1.894.gb6a874cedc-goog
>


-- 
Luiz Augusto von Dentz

Re: [BlueZ,1/9] adv_monitor: Disable RSSIHighTimeout for SW based filtering

[Luiz Augusto von Dentz]

Hi Manish,

On Mon, Mar 21, 2022 at 4:53 AM <bluez.test.bot@gmail.com> wrote:
>
> 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=624998
>
> ---Test result---
>
> Test Summary:
> CheckPatch                    PASS      13.09 seconds
> GitLint                       PASS      8.89 seconds
> Prep - Setup ELL              PASS      46.69 seconds
> Build - Prep                  PASS      0.76 seconds
> Build - Configure             PASS      9.23 seconds
> Build - Make                  FAIL      1504.46 seconds
> Make Check                    FAIL      2.42 seconds
> Make Check w/Valgrind         FAIL      324.61 seconds
> Make Distcheck                FAIL      173.77 seconds
> Build w/ext ELL - Configure   PASS      9.30 seconds
> Build w/ext ELL - Make        FAIL      1466.64 seconds
> Incremental Build with patchesFAIL      4186.16 seconds
>
> Details
> ##############################
> Test: Build - Make - FAIL
> Desc: Build the BlueZ source tree
> Output:
> tools/mgmt-tester.c: In function ‘main’:
> tools/mgmt-tester.c:12364:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
> 12364 | int main(int argc, char *argv[])
>       |     ^~~~
> unit/test-avdtp.c: In function ‘main’:
> unit/test-avdtp.c:766:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
>   766 | int main(int argc, char *argv[])
>       |     ^~~~
> unit/test-avrcp.c: In function ‘main’:
> unit/test-avrcp.c:989:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
>   989 | int main(int argc, char *argv[])
>       |     ^~~~
> src/adv_monitor.c: In function ‘btd_adv_monitor_resume’:
> src/adv_monitor.c:2402:7: error: implicit declaration of function ‘btd_adv_monitor_offload_supported’; did you mean ‘btd_adv_monitor_offload_enabled’? [-Werror=implicit-function-declaration]
>  2402 |  if (!btd_adv_monitor_offload_supported(manager))
>       |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>       |       btd_adv_monitor_offload_enabled
> cc1: all warnings being treated as errors
> make[1]: *** [Makefile:10284: src/bluetoothd-adv_monitor.o] Error 1
> make: *** [Makefile:4307: all] Error 2
>
>
> ##############################
> Test: Make Check - FAIL
> Desc: Run 'make check'
> Output:
> src/adv_monitor.c: In function ‘btd_adv_monitor_resume’:
> src/adv_monitor.c:2402:7: error: implicit declaration of function ‘btd_adv_monitor_offload_supported’; did you mean ‘btd_adv_monitor_offload_enabled’? [-Werror=implicit-function-declaration]
>  2402 |  if (!btd_adv_monitor_offload_supported(manager))
>       |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>       |       btd_adv_monitor_offload_enabled
> cc1: all warnings being treated as errors
> make[1]: *** [Makefile:10284: src/bluetoothd-adv_monitor.o] Error 1
> make: *** [Makefile:11277: check] Error 2
>
>
> ##############################
> Test: Make Check w/Valgrind - FAIL
> Desc: Run 'make check' with Valgrind
> Output:
> tools/mgmt-tester.c: In function ‘main’:
> tools/mgmt-tester.c:12364:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
> 12364 | int main(int argc, char *argv[])
>       |     ^~~~
> src/adv_monitor.c: In function ‘btd_adv_monitor_resume’:
> src/adv_monitor.c:2402:7: error: implicit declaration of function ‘btd_adv_monitor_offload_supported’; did you mean ‘btd_adv_monitor_offload_enabled’? [-Werror=implicit-function-declaration]
>  2402 |  if (!btd_adv_monitor_offload_supported(manager))
>       |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>       |       btd_adv_monitor_offload_enabled
> cc1: all warnings being treated as errors
> make[1]: *** [Makefile:10284: src/bluetoothd-adv_monitor.o] Error 1
> make: *** [Makefile:4307: all] Error 2
>
>
> ##############################
> Test: Make Distcheck - FAIL
> Desc: Run distcheck to check the distribution
> Output:
> ../../src/adv_monitor.c: In function ‘btd_adv_monitor_resume’:
> ../../src/adv_monitor.c:2402:7: warning: implicit declaration of function ‘btd_adv_monitor_offload_supported’; did you mean ‘btd_adv_monitor_offload_enabled’? [-Wimplicit-function-declaration]
>  2402 |  if (!btd_adv_monitor_offload_supported(manager))
>       |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>       |       btd_adv_monitor_offload_enabled
> /usr/bin/ld: src/bluetoothd-adv_monitor.o: in function `btd_adv_monitor_resume':
> /github/workspace/src/bluez-5.63/_build/sub/../../src/adv_monitor.c:2402: undefined reference to `btd_adv_monitor_offload_supported'
> collect2: error: ld returned 1 exit status
> make[2]: *** [Makefile:5878: src/bluetoothd] Error 1
> make[1]: *** [Makefile:4307: all] Error 2
> make: *** [Makefile:11198: distcheck] Error 1
>
>
> ##############################
> Test: Build w/ext ELL - Make - FAIL
> Desc: Build BlueZ source with '--enable-external-ell' configuration
> Output:
> tools/mgmt-tester.c: In function ‘main’:
> tools/mgmt-tester.c:12364:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
> 12364 | int main(int argc, char *argv[])
>       |     ^~~~
> unit/test-avdtp.c: In function ‘main’:
> unit/test-avdtp.c:766:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
>   766 | int main(int argc, char *argv[])
>       |     ^~~~
> unit/test-avrcp.c: In function ‘main’:
> unit/test-avrcp.c:989:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
>   989 | int main(int argc, char *argv[])
>       |     ^~~~
> src/adv_monitor.c: In function ‘btd_adv_monitor_resume’:
> src/adv_monitor.c:2402:7: error: implicit declaration of function ‘btd_adv_monitor_offload_supported’; did you mean ‘btd_adv_monitor_offload_enabled’? [-Werror=implicit-function-declaration]
>  2402 |  if (!btd_adv_monitor_offload_supported(manager))
>       |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>       |       btd_adv_monitor_offload_enabled
> cc1: all warnings being treated as errors
> make[1]: *** [Makefile:10284: src/bluetoothd-adv_monitor.o] Error 1
> make: *** [Makefile:4307: all] Error 2
>
>
> ##############################
> Test: Incremental Build with patches - FAIL
> Desc: Incremental build per patch in the series
> Output:
> tools/mgmt-tester.c: In function ‘main’:
> tools/mgmt-tester.c:12364:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
> 12364 | int main(int argc, char *argv[])
>       |     ^~~~
> unit/test-avdtp.c: In function ‘main’:
> unit/test-avdtp.c:766:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
>   766 | int main(int argc, char *argv[])
>       |     ^~~~
> unit/test-avrcp.c: In function ‘main’:
> unit/test-avrcp.c:989:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
>   989 | int main(int argc, char *argv[])
>       |     ^~~~
> src/adv_monitor.c: In function ‘btd_adv_monitor_resume’:
> src/adv_monitor.c:2308:7: error: implicit declaration of function ‘btd_adv_monitor_offload_supported’; did you mean ‘btd_adv_monitor_offload_enabled’? [-Werror=implicit-function-declaration]
>  2308 |  if (!btd_adv_monitor_offload_supported(manager))
>       |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>       |       btd_adv_monitor_offload_enabled
> cc1: all warnings being treated as errors
> make[1]: *** [Makefile:10284: src/bluetoothd-adv_monitor.o] Error 1
> make: *** [Makefile:4307: all] Error 2

Will you be fixing the errors above?

-- 
Luiz Augusto von Dentz

Re: [BlueZ PATCH 7/9] adv_monitor: Add the monitor Release reason

[Luiz Augusto von Dentz]

Hi Manish,

On Wed, Apr 13, 2022 at 1:56 PM Manish Mandlik <mmandlik@google.com> wrote:
>
> Hi Luiz,
>
> On Mon, Mar 21, 2022 at 5:49 PM Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
>>
>> Hi Manish,
>>
>> On Sun, Mar 20, 2022 at 6:37 PM Manish Mandlik <mmandlik@google.com> wrote:
>> >
>> > Adv Monitor is released for various reasons. For example, incorrect RSSI
>> > parameters, incorrect monitor type, non-overlapping RSSI thresholds,
>> > etc.
>> >
>> > Return this release reason along with the Release event for the better
>> > visibility to clients.
>>
>> Shouldn't this be sent as a reply to the method registering the monitor?
>
> Applications can create an Advertisement Monitor D-Bus object any time before or after registering the root path with BlueZ. BlueZ processes monitors once it receives ProxyAdded callback from D-Bus once a Monitor is created on an already registered root path or once the root path is registered for already created monitors. So, monitor Activate/Release methods are called later asynchronously to notify applications about acceptance or rejection of the monitor.

We should probably document this then, so I suppose one can register
the root path without having any monitors so they can be added/removed
later?

>>
>>
>> > Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
>> > ---
>> >
>> >  doc/advertisement-monitor-api.txt | 12 ++++++-
>> >  src/adv_monitor.c                 | 56 ++++++++++++++++++++++++++++---
>> >  2 files changed, 63 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/doc/advertisement-monitor-api.txt b/doc/advertisement-monitor-api.txt
>> > index 942d44b2f..fcbd9c5c2 100644
>> > --- a/doc/advertisement-monitor-api.txt
>> > +++ b/doc/advertisement-monitor-api.txt
>> > @@ -17,12 +17,22 @@ Service             org.bluez
>> >  Interface      org.bluez.AdvertisementMonitor1 [experimental]
>> >  Object path    freely definable
>> >
>> > -Methods                void Release() [noreply]
>> > +Methods                void Release(Int8 reason) [noreply]
>> >
>> >                         This gets called as a signal for a client to perform
>> >                         clean-up when (1)a monitor cannot be activated after it
>> >                         was exposed or (2)a monitor has been deactivated.
>> >
>> > +                       Possible reasons:  0  Unknown reason
>> > +                                          1  Invalid monitor type
>> > +                                          2  Invalid RSSI parameter(s)
>> > +                                          3  Invalid pattern(s)
>> > +                                          4  Monitor already exists
>> > +                                          5  Kernel failed to add monitor
>> > +                                          6  Kernel failed to remove monitor
>> > +                                          7  Monitor removed by kernel
>> > +                                          8  App unregistered/destroyed
>> > +
>> >                 void Activate() [noreply]
>> >
>> >                         After a monitor was exposed, this gets called as a
>> > diff --git a/src/adv_monitor.c b/src/adv_monitor.c
>> > index 85231557e..6ee3736d2 100644
>> > --- a/src/adv_monitor.c
>> > +++ b/src/adv_monitor.c
>> > @@ -90,6 +90,18 @@ enum monitor_state {
>> >         MONITOR_STATE_RELEASED, /* Dbus Object removed by app */
>> >  };
>> >
>> > +enum monitor_release_reason {
>> > +       REASON_UNKNOWN,
>> > +       REASON_INVALID_TYPE,
>> > +       REASON_INVALID_RSSI_PARAMS,
>> > +       REASON_INVALID_PATTERNS,
>> > +       REASON_ALREADY_EXISTS,
>> > +       REASON_FAILED_TO_ADD,
>> > +       REASON_FAILED_TO_REMOVE,
>> > +       REASON_REMOVED_BY_KERNEL,
>> > +       REASON_APP_DESTROYED,
>> > +};
>> > +
>> >  enum merged_pattern_state {
>> >         MERGED_PATTERN_STATE_ADDING,    /* Adding pattern to kernel */
>> >         MERGED_PATTERN_STATE_REMOVING,  /* Removing pattern from kernel */
>> > @@ -113,6 +125,7 @@ struct adv_monitor {
>> >         char *path;
>> >
>> >         enum monitor_state state;       /* MONITOR_STATE_* */
>> > +       enum monitor_release_reason release_reason;
>> >
>> >         struct rssi_parameters rssi;    /* RSSI parameter for this monitor */
>> >         struct adv_monitor_merged_pattern *merged_pattern;
>> > @@ -137,6 +150,7 @@ struct adv_monitor_merged_pattern {
>> >         struct queue *patterns;         /* List of bt_ad_pattern objects */
>> >         enum merged_pattern_state current_state; /* MERGED_PATTERN_STATE_* */
>> >         enum merged_pattern_state next_state;    /* MERGED_PATTERN_STATE_* */
>> > +       enum monitor_release_reason release_reason;
>> >  };
>> >
>> >  /* Some data like last_seen, timer/timeout values need to be maintained
>> > @@ -541,6 +555,18 @@ static void monitor_free(struct adv_monitor *monitor)
>> >         free(monitor);
>> >  }
>> >
>> > +/* Includes monitor release reason into the dbus message */
>> > +static void report_release_reason_setup(DBusMessageIter *iter, void *user_data)
>> > +{
>> > +       const struct adv_monitor *monitor = user_data;
>> > +       int8_t release_reason = REASON_UNKNOWN;
>> > +
>> > +       if (monitor)
>> > +               release_reason = monitor->release_reason;
>> > +
>> > +       dbus_message_iter_append_basic(iter, DBUS_TYPE_BYTE, &release_reason);
>> > +}
>> > +
>> >  /* Calls Release() method of the remote Adv Monitor */
>> >  static void monitor_release(struct adv_monitor *monitor)
>> >  {
>> > @@ -560,8 +586,9 @@ static void monitor_release(struct adv_monitor *monitor)
>> >         DBG("Calling Release() on Adv Monitor of owner %s at path %s",
>> >                 monitor->app->owner, monitor->path);
>> >
>> > -       g_dbus_proxy_method_call(monitor->proxy, "Release", NULL, NULL, NULL,
>> > -                                       NULL);
>> > +       g_dbus_proxy_method_call(monitor->proxy, "Release",
>> > +                                       report_release_reason_setup, NULL,
>> > +                                       monitor, NULL);
>> >  }
>> >
>> >  /* Removes monitor from the merged_pattern. This would result in removing it
>> > @@ -635,13 +662,20 @@ static void monitor_destroy(void *data)
>> >  static void app_destroy(void *data)
>> >  {
>> >         struct adv_monitor_app *app = data;
>> > +       const struct queue_entry *e;
>> >
>> >         if (!app)
>> >                 return;
>> >
>> >         DBG("Destroy Adv Monitor app %s at path %s", app->owner, app->path);
>> >
>> > -       queue_destroy(app->monitors, monitor_destroy);
>> > +       for (e = queue_get_entries(app->monitors); e; e = e->next) {
>> > +               struct adv_monitor *m = e->data;
>> > +
>> > +               m->release_reason = REASON_APP_DESTROYED;
>> > +               monitor_destroy(m);
>> > +       }
>> > +       queue_destroy(app->monitors, NULL);
>> >
>> >         if (app->reg) {
>> >                 app_reply_msg(app, btd_error_failed(app->reg,
>> > @@ -793,6 +827,7 @@ static bool parse_monitor_type(struct adv_monitor *monitor, const char *path)
>> >         }
>> >
>> >  failed:
>> > +       monitor->release_reason = REASON_INVALID_TYPE;
>> >         btd_error(adapter_id,
>> >                         "Invalid argument of property Type of the Adv Monitor "
>> >                         "at path %s", path);
>> > @@ -919,6 +954,7 @@ done:
>> >         return true;
>> >
>> >  failed:
>> > +       monitor->release_reason = REASON_INVALID_RSSI_PARAMS;
>> >         btd_error(adapter_id,
>> >                         "Invalid argument of RSSI thresholds and timeouts "
>> >                         "of the Adv Monitor at path %s",
>> > @@ -1005,6 +1041,7 @@ static bool parse_patterns(struct adv_monitor *monitor, const char *path)
>> >         return true;
>> >
>> >  failed:
>> > +       monitor->release_reason = REASON_INVALID_PATTERNS;
>> >         btd_error(adapter_id, "Invalid argument of property Patterns of the "
>> >                         "Adv Monitor at path %s", path);
>> >
>> > @@ -1053,6 +1090,7 @@ static void merged_pattern_destroy_monitors(
>> >                 struct adv_monitor *monitor = e->data;
>> >
>> >                 monitor->merged_pattern = NULL;
>> > +               monitor->release_reason = merged_pattern->release_reason;
>> >                 monitor_destroy(monitor);
>> >         }
>> >  }
>> > @@ -1086,6 +1124,7 @@ static void remove_adv_monitor_cb(uint8_t status, uint16_t length,
>> >         return;
>> >
>> >  fail:
>> > +       merged_pattern->release_reason = REASON_FAILED_TO_REMOVE;
>> >         merged_pattern_destroy_monitors(merged_pattern);
>> >         merged_pattern_free(merged_pattern);
>> >  }
>> > @@ -1142,6 +1181,7 @@ static void add_adv_patterns_monitor_cb(uint8_t status, uint16_t length,
>> >         return;
>> >
>> >  fail:
>> > +       merged_pattern->release_reason = REASON_FAILED_TO_ADD;
>> >         merged_pattern_destroy_monitors(merged_pattern);
>> >         merged_pattern_free(merged_pattern);
>> >  }
>> > @@ -1285,6 +1325,7 @@ static void monitor_proxy_added_cb(GDBusProxy *proxy, void *user_data)
>> >                 merged_pattern_add(monitor->merged_pattern);
>> >         } else {
>> >                 if (!merge_is_possible(existing_pattern, monitor)) {
>> > +                       monitor->release_reason = REASON_ALREADY_EXISTS;
>> >                         monitor_destroy(monitor);
>> >                         DBG("Adv Monitor at path %s released due to existing "
>> >                                 "monitor", path);
>> > @@ -1551,6 +1592,7 @@ static void remove_merged_pattern(void *data, void *user_data)
>> >  {
>> >         struct adv_monitor_merged_pattern *merged_pattern = data;
>> >         uint16_t *handle = user_data;
>> > +       const struct queue_entry *e;
>> >
>> >         if (!handle)
>> >                 return;
>> > @@ -1562,8 +1604,14 @@ static void remove_merged_pattern(void *data, void *user_data)
>> >         DBG("Adv monitor with handle:0x%04x removed by kernel",
>> >                 merged_pattern->monitor_handle);
>> >
>> > +       for (e = queue_get_entries(merged_pattern->monitors); e; e = e->next) {
>> > +               struct adv_monitor *m = e->data;
>> > +
>> > +               m->release_reason = REASON_REMOVED_BY_KERNEL;
>> > +               monitor_destroy(m);
>> > +       }
>> >         queue_foreach(merged_pattern->monitors, monitor_state_removed, NULL);
>> > -       queue_destroy(merged_pattern->monitors, monitor_destroy);
>> > +       queue_destroy(merged_pattern->monitors, NULL);
>> >         merged_pattern_free(merged_pattern);
>> >  }
>> >
>> > --
>> > 2.35.1.894.gb6a874cedc-goog
>> >
>>
>>
>> --
>> Luiz Augusto von Dentz
>
>
> Regards,
> Manish.
>


-- 
Luiz Augusto von Dentz