diff mbox series

[BlueZ,v1,1/8] adv_monitor: Implement RSSI Filter logic for background scanning

Message ID 20200916212926.BlueZ.v1.1.I2830b9c1212a64b062201ed9f2b71294f50ad22d@changeid (mailing list archive)
State Superseded
Headers show
Series [BlueZ,v1,1/8] adv_monitor: Implement RSSI Filter logic for background scanning | expand

Commit Message

Miao-chen Chou Sept. 17, 2020, 4:29 a.m. UTC
From: Manish Mandlik <mmandlik@google.com>

This patch implements the RSSI Filter logic for background scanning.

This was unit-tested by running tests in unit/test-adv-monitor.c

     unit/test-adv-monitor.c file. Verified all tests PASS by running:
     USE="-bluez-next bluez-upstream" FEATURES=test emerge-hatch bluez

Reviewed-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
Reviewed-by: Alain Michaud <alainm@chromium.org>
Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
Reviewed-by: Howard Chung <howardchung@google.com>
---

 doc/advertisement-monitor-api.txt |   5 +
 src/adapter.c                     |   1 +
 src/adv_monitor.c                 | 286 +++++++++++++++++++++++++++++-
 src/adv_monitor.h                 |   4 +
 4 files changed, 292 insertions(+), 4 deletions(-)

Comments

bluez.test.bot@gmail.com Sept. 17, 2020, 4:52 a.m. UTC | #1
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.
While we are preparing for reviewing the patches, we found the following
issue/warning.

Test Result:
checkbuild Failed

Outputs:
ar: `u' modifier ignored since `D' is the default (see `U')
ar: `u' modifier ignored since `D' is the default (see `U')
ar: `u' modifier ignored since `D' is the default (see `U')
ar: `u' modifier ignored since `D' is the default (see `U')
ar: `u' modifier ignored since `D' is the default (see `U')
unit/test-adv-monitor.c: In function ‘main’:
unit/test-adv-monitor.c:44:34: error: assignment to ‘struct content_filter_test *’ from incompatible pointer type ‘struct rssi_filter_test *’ [-Werror=incompatible-pointer-types]
   44 |    test.content_filter_test_data = &data;  \
      |                                  ^
unit/test-adv-monitor.c:514:2: note: in expansion of macro ‘define_test’
  514 |  define_test("/advmon/rssi/1", TEST_RSSI_FILTER, rssi_data_1,
      |  ^~~~~~~~~~~
unit/test-adv-monitor.c:44:34: error: assignment to ‘struct content_filter_test *’ from incompatible pointer type ‘struct rssi_filter_test *’ [-Werror=incompatible-pointer-types]
   44 |    test.content_filter_test_data = &data;  \
      |                                  ^
unit/test-adv-monitor.c:516:2: note: in expansion of macro ‘define_test’
  516 |  define_test("/advmon/rssi/2", TEST_RSSI_FILTER, rssi_data_2,
      |  ^~~~~~~~~~~
unit/test-adv-monitor.c:44:34: error: assignment to ‘struct content_filter_test *’ from incompatible pointer type ‘struct rssi_filter_test *’ [-Werror=incompatible-pointer-types]
   44 |    test.content_filter_test_data = &data;  \
      |                                  ^
unit/test-adv-monitor.c:518:2: note: in expansion of macro ‘define_test’
  518 |  define_test("/advmon/rssi/3", TEST_RSSI_FILTER, rssi_data_3,
      |  ^~~~~~~~~~~
unit/test-adv-monitor.c:44:34: error: assignment to ‘struct content_filter_test *’ from incompatible pointer type ‘struct rssi_filter_test *’ [-Werror=incompatible-pointer-types]
   44 |    test.content_filter_test_data = &data;  \
      |                                  ^
unit/test-adv-monitor.c:520:2: note: in expansion of macro ‘define_test’
  520 |  define_test("/advmon/rssi/4", TEST_RSSI_FILTER, rssi_data_4,
      |  ^~~~~~~~~~~
unit/test-adv-monitor.c:44:34: error: assignment to ‘struct content_filter_test *’ from incompatible pointer type ‘struct rssi_filter_test *’ [-Werror=incompatible-pointer-types]
   44 |    test.content_filter_test_data = &data;  \
      |                                  ^
unit/test-adv-monitor.c:522:2: note: in expansion of macro ‘define_test’
  522 |  define_test("/advmon/rssi/5", TEST_RSSI_FILTER, rssi_data_5,
      |  ^~~~~~~~~~~
unit/test-adv-monitor.c:41:31: error: assignment to ‘struct rssi_filter_test *’ from incompatible pointer type ‘struct content_filter_test *’ [-Werror=incompatible-pointer-types]
   41 |    test.rssi_filter_test_data = &data;  \
      |                               ^
unit/test-adv-monitor.c:525:2: note: in expansion of macro ‘define_test’
  525 |  define_test("/advmon/content/1", TEST_CONTENT_FILTER, content_data_1,
      |  ^~~~~~~~~~~
unit/test-adv-monitor.c:41:31: error: assignment to ‘struct rssi_filter_test *’ from incompatible pointer type ‘struct content_filter_test *’ [-Werror=incompatible-pointer-types]
   41 |    test.rssi_filter_test_data = &data;  \
      |                               ^
unit/test-adv-monitor.c:527:2: note: in expansion of macro ‘define_test’
  527 |  define_test("/advmon/content/2", TEST_CONTENT_FILTER, content_data_2,
      |  ^~~~~~~~~~~
unit/test-adv-monitor.c:41:31: error: assignment to ‘struct rssi_filter_test *’ from incompatible pointer type ‘struct content_filter_test *’ [-Werror=incompatible-pointer-types]
   41 |    test.rssi_filter_test_data = &data;  \
      |                               ^
unit/test-adv-monitor.c:529:2: note: in expansion of macro ‘define_test’
  529 |  define_test("/advmon/content/3", TEST_CONTENT_FILTER, content_data_3,
      |  ^~~~~~~~~~~
unit/test-adv-monitor.c:41:31: error: assignment to ‘struct rssi_filter_test *’ from incompatible pointer type ‘struct content_filter_test *’ [-Werror=incompatible-pointer-types]
   41 |    test.rssi_filter_test_data = &data;  \
      |                               ^
unit/test-adv-monitor.c:531:2: note: in expansion of macro ‘define_test’
  531 |  define_test("/advmon/content/4", TEST_CONTENT_FILTER, content_data_4,
      |  ^~~~~~~~~~~
cc1: all warnings being treated as errors
make[1]: *** [Makefile:6863: unit/test-adv-monitor.o] Error 1
make: *** [Makefile:4056: all] Error 2



---
Regards,
Linux Bluetooth
diff mbox series

Patch

diff --git a/doc/advertisement-monitor-api.txt b/doc/advertisement-monitor-api.txt
index e09b6fd25..92c8ffc38 100644
--- a/doc/advertisement-monitor-api.txt
+++ b/doc/advertisement-monitor-api.txt
@@ -70,6 +70,11 @@  Properties	string Type [read-only]
 			dBm indicates unset. The valid range of a timer is 1 to
 			300 seconds while 0 indicates unset.
 
+			If the peer device advertising interval is greater than the
+			HighRSSIThresholdTimer, the device will never be found. Similarly,
+			if it is greater than LowRSSIThresholdTimer, the device will be
+			considered as lost. Consider configuring these values accordingly.
+
 		array{(uint8, uint8, array{byte})} Patterns [read-only, optional]
 
 			If Type is set to 0x01, this must exist and has at least
diff --git a/src/adapter.c b/src/adapter.c
index b2bd8b3f1..415d6e06b 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -1227,6 +1227,7 @@  void btd_adapter_remove_device(struct btd_adapter *adapter,
 	adapter->connect_list = g_slist_remove(adapter->connect_list, dev);
 
 	adapter->devices = g_slist_remove(adapter->devices, dev);
+	btd_adv_monitor_device_remove(adapter->adv_monitor_manager, dev);
 
 	adapter->discovery_found = g_slist_remove(adapter->discovery_found,
 									dev);
diff --git a/src/adv_monitor.c b/src/adv_monitor.c
index 737da1c90..7baa5317f 100644
--- a/src/adv_monitor.c
+++ b/src/adv_monitor.c
@@ -35,6 +35,7 @@ 
 
 #include "adapter.h"
 #include "dbus-common.h"
+#include "device.h"
 #include "log.h"
 #include "src/error.h"
 #include "src/shared/ad.h"
@@ -44,6 +45,8 @@ 
 
 #include "adv_monitor.h"
 
+static void monitor_device_free(void *data);
+
 #define ADV_MONITOR_INTERFACE		"org.bluez.AdvertisementMonitor1"
 #define ADV_MONITOR_MGR_INTERFACE	"org.bluez.AdvertisementMonitorManager1"
 
@@ -104,15 +107,36 @@  struct adv_monitor {
 
 	enum monitor_state state;	/* MONITOR_STATE_* */
 
-	int8_t high_rssi;		/* high RSSI threshold */
-	uint16_t high_rssi_timeout;	/* high RSSI threshold timeout */
-	int8_t low_rssi;		/* low RSSI threshold */
-	uint16_t low_rssi_timeout;	/* low RSSI threshold timeout */
+	int8_t high_rssi;		/* High RSSI threshold */
+	uint16_t high_rssi_timeout;	/* High RSSI threshold timeout */
+	int8_t low_rssi;		/* Low RSSI threshold */
+	uint16_t low_rssi_timeout;	/* Low RSSI threshold timeout */
+	struct queue *devices;		/* List of adv_monitor_device objects */
 
 	enum monitor_type type;		/* MONITOR_TYPE_* */
 	struct queue *patterns;
 };
 
+/* Some data like last_seen, timer/timeout values need to be maintained
+ * per device. struct adv_monitor_device maintains such data.
+ */
+struct adv_monitor_device {
+	struct adv_monitor *monitor;
+	struct btd_device *device;
+
+	time_t high_rssi_first_seen;	/* Start time when RSSI climbs above
+					 * the high RSSI threshold
+					 */
+	time_t low_rssi_first_seen;	/* Start time when RSSI drops below
+					 * the low RSSI threshold
+					 */
+	time_t last_seen;		/* Time when last Adv was received */
+	bool device_found;		/* State of the device - lost/found */
+	guint device_lost_timer;	/* Timer to track if the device goes
+					 * offline/out-of-range
+					 */
+};
+
 struct app_match_data {
 	const char *owner;
 	const char *path;
@@ -159,6 +183,9 @@  static void monitor_free(void *data)
 	g_dbus_proxy_unref(monitor->proxy);
 	g_free(monitor->path);
 
+	queue_destroy(monitor->devices, monitor_device_free);
+	monitor->devices = NULL;
+
 	queue_destroy(monitor->patterns, pattern_free);
 
 	free(monitor);
@@ -257,6 +284,7 @@  static struct adv_monitor *monitor_new(struct adv_monitor_app *app,
 	monitor->high_rssi_timeout = ADV_MONITOR_UNSET_TIMER;
 	monitor->low_rssi = ADV_MONITOR_UNSET_RSSI;
 	monitor->low_rssi_timeout = ADV_MONITOR_UNSET_TIMER;
+	monitor->devices = queue_new();
 
 	monitor->type = MONITOR_TYPE_NONE;
 	monitor->patterns = NULL;
@@ -932,3 +960,253 @@  void btd_adv_monitor_manager_destroy(struct btd_adv_monitor_manager *manager)
 
 	manager_destroy(manager);
 }
+
+/* Matches a device based on btd_device object */
+static bool monitor_device_match(const void *a, const void *b)
+{
+	const struct adv_monitor_device *dev = a;
+	const struct btd_device *device = b;
+
+	if (!dev)
+		return false;
+
+	if (dev->device != device)
+		return false;
+
+	return true;
+}
+
+/* Frees a monitor device object */
+static void monitor_device_free(void *data)
+{
+	struct adv_monitor_device *dev = data;
+
+	if (!dev)
+		return;
+
+	if (dev->device_lost_timer) {
+		g_source_remove(dev->device_lost_timer);
+		dev->device_lost_timer = 0;
+	}
+
+	dev->monitor = NULL;
+	dev->device = NULL;
+
+	g_free(dev);
+}
+
+/* Removes a device from monitor->devices list */
+static void remove_device_from_monitor(void *data, void *user_data)
+{
+	struct adv_monitor *monitor = data;
+	struct btd_device *device = user_data;
+	struct adv_monitor_device *dev = NULL;
+
+	if (!monitor)
+		return;
+
+	dev = queue_remove_if(monitor->devices, monitor_device_match, device);
+	if (dev) {
+		DBG("Device removed from the Adv Monitor at path %s",
+		    monitor->path);
+		monitor_device_free(dev);
+	}
+}
+
+/* Removes a device from every monitor in an app */
+static void remove_device_from_app(void *data, void *user_data)
+{
+	struct adv_monitor_app *app = data;
+	struct btd_device *device = user_data;
+
+	if (!app)
+		return;
+
+	queue_foreach(app->monitors, remove_device_from_monitor, device);
+}
+
+/* Removes a device from every monitor in all apps */
+void btd_adv_monitor_device_remove(struct btd_adv_monitor_manager *manager,
+				   struct btd_device *device)
+{
+	if (!manager || !device)
+		return;
+
+	queue_foreach(manager->apps, remove_device_from_app, device);
+}
+
+/* Creates a device object to track the per-device information */
+static struct adv_monitor_device *monitor_device_create(
+			struct adv_monitor *monitor,
+			struct btd_device *device)
+{
+	struct adv_monitor_device *dev = NULL;
+
+	dev = g_try_malloc0(sizeof(struct adv_monitor_device));
+	if (!dev)
+		return NULL;
+
+	dev->monitor = monitor;
+	dev->device = device;
+
+	queue_push_tail(monitor->devices, dev);
+
+	return dev;
+}
+
+/* Includes found/lost device's object path into the dbus message */
+static void report_device_state_setup(DBusMessageIter *iter, void *user_data)
+{
+	const char *path = device_get_path(user_data);
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_OBJECT_PATH, &path);
+}
+
+/* Handles a situation where the device goes offline/out-of-range */
+static gboolean handle_device_lost_timeout(gpointer user_data)
+{
+	struct adv_monitor_device *dev = user_data;
+	struct adv_monitor *monitor = dev->monitor;
+	time_t curr_time = time(NULL);
+
+	DBG("Device Lost timeout triggered for device %p "
+	    "for the Adv Monitor at path %s", dev->device, monitor->path);
+
+	dev->device_lost_timer = 0;
+
+	if (dev->device_found && dev->last_seen) {
+		/* We were tracking for the Low RSSI filter. Check if there is
+		 * any Adv received after the timeout function is invoked.
+		 * If not, report the Device Lost event.
+		 */
+		if (difftime(curr_time, dev->last_seen) >=
+		    monitor->low_rssi_timeout) {
+			dev->device_found = false;
+
+			DBG("Calling DeviceLost() on Adv Monitor of owner %s "
+			    "at path %s", monitor->app->owner, monitor->path);
+
+			g_dbus_proxy_method_call(monitor->proxy, "DeviceLost",
+						 report_device_state_setup,
+						 NULL, dev->device, NULL);
+		}
+	}
+
+	return FALSE;
+}
+
+/* Filters an Adv based on its RSSI value */
+static void adv_monitor_filter_rssi(struct adv_monitor *monitor,
+				    struct btd_device *device, int8_t rssi)
+{
+	struct adv_monitor_device *dev = NULL;
+	time_t curr_time = time(NULL);
+	uint16_t adapter_id = monitor->app->manager->adapter_id;
+
+	/* If the RSSI thresholds and timeouts are not specified, report the
+	 * 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) {
+		DBG("Calling DeviceFound() on Adv Monitor of owner %s "
+		    "at path %s", monitor->app->owner, monitor->path);
+
+		g_dbus_proxy_method_call(monitor->proxy, "DeviceFound",
+					 report_device_state_setup, NULL,
+					 device, NULL);
+
+		return;
+	}
+
+	dev = queue_find(monitor->devices, monitor_device_match, device);
+	if (!dev)
+		dev = monitor_device_create(monitor, device);
+	if (!dev) {
+		btd_error(adapter_id, "Failed to create Adv Monitor "
+				      "device object.");
+		return;
+	}
+
+	if (dev->device_lost_timer) {
+		g_source_remove(dev->device_lost_timer);
+		dev->device_lost_timer = 0;
+	}
+
+	/* Reset the timings of found/lost if a device has been offline for
+	 * longer than the high/low timeouts.
+	 */
+	if (dev->last_seen) {
+		if (difftime(curr_time, dev->last_seen) >
+		    monitor->high_rssi_timeout) {
+			dev->high_rssi_first_seen = 0;
+		}
+
+		if (difftime(curr_time, dev->last_seen) >
+		    monitor->low_rssi_timeout) {
+			dev->low_rssi_first_seen = 0;
+		}
+	}
+	dev->last_seen = curr_time;
+
+	/* Check for the found devices (if the device is not already found) */
+	if (!dev->device_found && rssi > monitor->high_rssi) {
+		if (dev->high_rssi_first_seen) {
+			if (difftime(curr_time, dev->high_rssi_first_seen) >=
+			    monitor->high_rssi_timeout) {
+				dev->device_found = true;
+
+				DBG("Calling DeviceFound() on Adv Monitor "
+				    "of owner %s at path %s",
+				    monitor->app->owner, monitor->path);
+
+				g_dbus_proxy_method_call(
+					monitor->proxy, "DeviceFound",
+					report_device_state_setup, NULL,
+					dev->device, NULL);
+			}
+		} else {
+			dev->high_rssi_first_seen = curr_time;
+		}
+	} else {
+		dev->high_rssi_first_seen = 0;
+	}
+
+	/* Check for the lost devices (only if the device is already found, as
+	 * it doesn't make any sense to report the Device Lost event if the
+	 * device is not found yet)
+	 */
+	if (dev->device_found && rssi < monitor->low_rssi) {
+		if (dev->low_rssi_first_seen) {
+			if (difftime(curr_time, dev->low_rssi_first_seen) >=
+			    monitor->low_rssi_timeout) {
+				dev->device_found = false;
+
+				DBG("Calling DeviceLost() on Adv Monitor "
+				    "of owner %s at path %s",
+				    monitor->app->owner, monitor->path);
+
+				g_dbus_proxy_method_call(
+					monitor->proxy, "DeviceLost",
+					report_device_state_setup, NULL,
+					dev->device, NULL);
+			}
+		} else {
+			dev->low_rssi_first_seen = curr_time;
+		}
+	} else {
+		dev->low_rssi_first_seen = 0;
+	}
+
+	/* Setup a timer to track if the device goes offline/out-of-range, only
+	 * if we are tracking for the Low RSSI Threshold. If we are tracking
+	 * the High RSSI Threshold, nothing needs to be done.
+	 */
+	if (dev->device_found) {
+		dev->device_lost_timer =
+			g_timeout_add_seconds(monitor->low_rssi_timeout,
+					      handle_device_lost_timeout, dev);
+	}
+}
diff --git a/src/adv_monitor.h b/src/adv_monitor.h
index 69ea348f8..14508e7d1 100644
--- a/src/adv_monitor.h
+++ b/src/adv_monitor.h
@@ -21,6 +21,7 @@ 
 #define __ADV_MONITOR_H
 
 struct mgmt;
+struct btd_device;
 struct btd_adapter;
 struct btd_adv_monitor_manager;
 
@@ -29,4 +30,7 @@  struct btd_adv_monitor_manager *btd_adv_monitor_manager_create(
 						struct mgmt *mgmt);
 void btd_adv_monitor_manager_destroy(struct btd_adv_monitor_manager *manager);
 
+void btd_adv_monitor_device_remove(struct btd_adv_monitor_manager *manager,
+				   struct btd_device *device);
+
 #endif /* __ADV_MONITOR_H */