2022-04-14 06:22:34

by Manish Mandlik

[permalink] [raw]
Subject: [BlueZ PATCH v2 1/9] adv_monitor: Disable RSSIHighTimeout for SW based filtering

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 <[email protected]>
---

(no changes since v1)

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.36.0.rc0.470.gd361397f0d-goog


2022-04-14 09:44:55

by Manish Mandlik

[permalink] [raw]
Subject: [BlueZ PATCH v2 8/9] client: Display the AdvMonitor Release reason

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 <[email protected]>
---

(no changes since v1)

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.36.0.rc0.470.gd361397f0d-goog

2022-04-14 10:45:09

by Manish Mandlik

[permalink] [raw]
Subject: [BlueZ PATCH v2 9/9] test: Display the AdvMonitor Release reason

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 <[email protected]>
---

(no changes since v1)

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.36.0.rc0.470.gd361397f0d-goog

2022-04-14 13:40:09

by Manish Mandlik

[permalink] [raw]
Subject: [BlueZ PATCH v2 3/9] adv_monitor: Clear tracked devices on resume

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 <[email protected]>
---

Changes in v2:
- Fix compiler error by replacing btd_adv_monitor_offload_supported()
with btd_adv_monitor_offload_enabled().

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..18ce839e9 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_enabled(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.36.0.rc0.470.gd361397f0d-goog

2022-04-14 14:02:34

by Manish Mandlik

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

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 <[email protected]>
---

(no changes since v1)

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 c01f8b154..d88e1bbbb 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.36.0.rc0.470.gd361397f0d-goog

2022-04-15 21:48:40

by bluez.test.bot

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

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=632021

---Test result---

Test Summary:
CheckPatch PASS 13.07 seconds
GitLint PASS 8.92 seconds
Prep - Setup ELL PASS 42.51 seconds
Build - Prep PASS 0.67 seconds
Build - Configure PASS 8.45 seconds
Build - Make PASS 1277.32 seconds
Make Check PASS 11.44 seconds
Make Check w/Valgrind PASS 448.86 seconds
Make Distcheck PASS 234.77 seconds
Build w/ext ELL - Configure PASS 8.45 seconds
Build w/ext ELL - Make PASS 1244.94 seconds
Incremental Build with patchesPASS 11371.40 seconds



---
Regards,
Linux Bluetooth