[nrf fromlist] Bluetooth: Host: Rework enabling of scanner

To make the scanner module more understandable and more streamlined, I
reworked the update mechanism of the scanner. The scanner tracks now the
parameters that were used to enable it and the reason why it is running.
This facilitates state logic and allows other modules to "start the
scanner", altough it is already running.
This is mostly a refactoring and not a functional change.

Added a test to verify the behavior.

Upstream PR: zephyrproject-rtos/zephyr#73923

Signed-off-by: Jan Müller <[email protected]>

Author: Tschet1

include/zephyr/bluetooth/bluetooth.h

@@ -2294,6 +2294,7 @@ BUILD_ASSERT(BT_GAP_SCAN_FAST_WINDOW == BT_GAP_SCAN_FAST_INTERVAL_MIN,
  *
  * @return Zero on success or error code otherwise, positive in case of
  *         protocol error or negative (POSIX) in case of stack internal error.
+ * @retval -EBUSY if the scanner is already being started in a different thread.
  */
 int bt_le_scan_start(const struct bt_le_scan_param *param, bt_le_scan_cb_t cb);
 

subsys/bluetooth/host/conn.c

@@ -34,6 +34,7 @@
 #include "hci_core.h"
 #include "id.h"
 #include "adv.h"
+#include "scan.h"
 #include "conn_internal.h"
 #include "l2cap_internal.h"
 #include "keys.h"
@@ -1200,10 +1201,18 @@ void bt_conn_set_state(struct bt_conn *conn, bt_conn_state_t state)
 			 * the application through bt_conn_disconnect or by
 			 * timeout set by bt_conn_le_create_param.timeout.
 			 */
-			if (conn->err) {
-				notify_connected(conn);
-			}
+			if (IS_ENABLED(CONFIG_BT_CENTRAL)) {
+				int err = bt_le_scan_user_remove(BT_LE_SCAN_USER_CONN);
+
+				if (err) {
+					LOG_WRN("Error while removing conn user from scanner (%d)",
+						err);
+				}
 
+				if (conn->err) {
+					notify_connected(conn);
+				}
+			}
 			bt_conn_unref(conn);
 			break;
 		case BT_CONN_ADV_DIR_CONNECTABLE:
@@ -1609,7 +1618,7 @@ int bt_conn_disconnect(struct bt_conn *conn, uint8_t reason)
 		conn->err = reason;
 		bt_conn_set_state(conn, BT_CONN_DISCONNECTED);
 		if (IS_ENABLED(CONFIG_BT_CENTRAL)) {
-			bt_le_scan_update(false);
+			return bt_le_scan_user_add(BT_LE_SCAN_USER_CONN);
 		}
 		return 0;
 	case BT_CONN_INITIATING:
@@ -3173,27 +3182,32 @@ static int conn_le_create_common_checks(const bt_addr_le_t *peer,
 {
 
 	if (!atomic_test_bit(bt_dev.flags, BT_DEV_READY)) {
+		LOG_DBG("Conn check failed: BT dev not ready.");
 		return -EAGAIN;
 	}
 
 	if (!bt_le_conn_params_valid(conn_param)) {
+		LOG_DBG("Conn check failed: invalid parameters.");
 		return -EINVAL;
 	}
 
-	if (!BT_LE_STATES_SCAN_INIT(bt_dev.le.states) &&
-	    atomic_test_bit(bt_dev.flags, BT_DEV_EXPLICIT_SCAN)) {
+	if (!BT_LE_STATES_SCAN_INIT(bt_dev.le.states) && bt_le_explicit_scanner_running()) {
+		LOG_DBG("Conn check failed: scanner was explicitly requested.");
 		return -EAGAIN;
 	}
 
 	if (atomic_test_bit(bt_dev.flags, BT_DEV_INITIATING)) {
+		LOG_DBG("Conn check failed: device is already initiating.");
 		return -EALREADY;
 	}
 
 	if (!bt_id_scan_random_addr_check()) {
+		LOG_DBG("Conn check failed: invalid random address.");
 		return -EINVAL;
 	}
 
 	if (bt_conn_exists_le(BT_ID_DEFAULT, peer)) {
+		LOG_DBG("Conn check failed: ACL connection already exists.");
 		return -EINVAL;
 	}
 
@@ -3246,8 +3260,9 @@ int bt_conn_le_create(const bt_addr_le_t *peer, const struct bt_conn_le_create_p
 		/* Use host-based identity resolving. */
 		bt_conn_set_state(conn, BT_CONN_SCAN_BEFORE_INITIATING);
 
-		err = bt_le_scan_update(true);
+		err = bt_le_scan_user_add(BT_LE_SCAN_USER_CONN);
 		if (err) {
+			bt_le_scan_user_remove(BT_LE_SCAN_USER_CONN);
 			bt_conn_set_state(conn, BT_CONN_DISCONNECTED);
 			bt_conn_unref(conn);
 
@@ -3267,7 +3282,12 @@ int bt_conn_le_create(const bt_addr_le_t *peer, const struct bt_conn_le_create_p
 		bt_conn_set_state(conn, BT_CONN_DISCONNECTED);
 		bt_conn_unref(conn);
 
-		bt_le_scan_update(false);
+		/* Best-effort attempt to inform the scanner that the initiator stopped. */
+		int scan_check_err = bt_le_scan_user_add(BT_LE_SCAN_USER_NONE);
+
+		if (scan_check_err) {
+			LOG_WRN("Error while updating the scanner (%d)", scan_check_err);
+		}
 		return err;
 	}
 
@@ -3369,17 +3389,18 @@ int bt_le_set_auto_conn(const bt_addr_le_t *addr,
 		}
 	}
 
+	int err = 0;
 	if (conn->state == BT_CONN_DISCONNECTED &&
 	    atomic_test_bit(bt_dev.flags, BT_DEV_READY)) {
 		if (param) {
 			bt_conn_set_state(conn, BT_CONN_SCAN_BEFORE_INITIATING);
+			err = bt_le_scan_user_add(BT_LE_SCAN_USER_CONN);
 		}
-		bt_le_scan_update(false);
 	}
 
 	bt_conn_unref(conn);
 
-	return 0;
+	return err;
 }
 #endif /* !defined(CONFIG_BT_FILTER_ACCEPT_LIST) */
 #endif /* CONFIG_BT_CENTRAL */

subsys/bluetooth/host/hci_core.c

@@ -929,7 +929,12 @@ static void hci_disconn_complete(struct net_buf *buf)
 #if defined(CONFIG_BT_CENTRAL) && !defined(CONFIG_BT_FILTER_ACCEPT_LIST)
 	if (atomic_test_bit(conn->flags, BT_CONN_AUTO_CONNECT)) {
 		bt_conn_set_state(conn, BT_CONN_SCAN_BEFORE_INITIATING);
-		bt_le_scan_update(false);
+		/* Just a best-effort check if the scanner should be started. */
+		int err = bt_le_scan_user_remove(BT_LE_SCAN_USER_NONE);
+
+		if (err) {
+			LOG_WRN("Error while updating the scanner (%d)", err);
+		}
 	}
 #endif /* defined(CONFIG_BT_CENTRAL) && !defined(CONFIG_BT_FILTER_ACCEPT_LIST) */
 
@@ -1342,7 +1347,12 @@ void bt_hci_le_enh_conn_complete(struct bt_hci_evt_le_enh_conn_complete *evt)
 		if (IS_ENABLED(CONFIG_BT_CENTRAL) &&
 		    evt->status == BT_HCI_ERR_UNKNOWN_CONN_ID) {
 			le_conn_complete_cancel(evt->status);
-			bt_le_scan_update(false);
+			/* Just a best-effort check if the scanner should be started. */
+			int err = bt_le_scan_user_remove(BT_LE_SCAN_USER_NONE);
+
+			if (err) {
+				LOG_WRN("Error while updating the scanner (%d)", err);
+			}
 			return;
 		}
 
@@ -1501,9 +1511,14 @@ void bt_hci_le_enh_conn_complete(struct bt_hci_evt_le_enh_conn_complete *evt)
 
 	bt_conn_unref(conn);
 
-	if (IS_ENABLED(CONFIG_BT_CENTRAL) &&
-	    conn->role == BT_HCI_ROLE_CENTRAL) {
-		bt_le_scan_update(false);
+	if (IS_ENABLED(CONFIG_BT_CENTRAL) && conn->role == BT_HCI_ROLE_CENTRAL) {
+		int err;
+
+		/* Just a best-effort check if the scanner should be started. */
+		err = bt_le_scan_user_remove(BT_LE_SCAN_USER_NONE);
+		if (err) {
+			LOG_WRN("Error while updating the scanner (%d)", err);
+		}
 	}
 }
 
@@ -4083,7 +4098,7 @@ void bt_finalize_init(void)
 	atomic_set_bit(bt_dev.flags, BT_DEV_READY);
 
 	if (IS_ENABLED(CONFIG_BT_OBSERVER)) {
-		bt_le_scan_update(false);
+		bt_scan_reset();
 	}
 
 	bt_dev_show_info();

subsys/bluetooth/host/hci_core.h

@@ -35,16 +35,11 @@ enum {
 	BT_DEV_HAS_PUB_KEY,
 	BT_DEV_PUB_KEY_BUSY,
 
-	/** The application explicitly instructed the stack to scan for advertisers
-	 * using the API @ref bt_le_scan_start().
-	 */
-	BT_DEV_EXPLICIT_SCAN,
-
 	/** The application either explicitly or implicitly instructed the stack to scan
 	 * for advertisers.
 	 *
 	 * Examples of such cases
-	 *  - Explicit scanning, @ref BT_DEV_EXPLICIT_SCAN.
+	 *  - Explicit scanning, @ref BT_LE_SCAN_USER_EXPLICIT_SCAN.
 	 *  - The application instructed the stack to automatically connect if a given device
 	 *    is detected.
 	 *  - The application wants to connect to a peer device using private addresses, but
@@ -60,13 +55,9 @@ enum {
 	 */
 	BT_DEV_SCANNING,
 
-	/* Cached parameters used when initially enabling the scanner.
-	 * These are needed to ensure the same parameters are used when restarting
-	 * the scanner after refreshing an RPA.
+	/**
+	 * Scanner is configured with a timeout.
 	 */
-	BT_DEV_ACTIVE_SCAN,
-	BT_DEV_SCAN_FILTER_DUP,
-	BT_DEV_SCAN_FILTERED,
 	BT_DEV_SCAN_LIMITED,
 
 	BT_DEV_INITIATING,
@@ -472,30 +463,6 @@ uint8_t bt_get_phy(uint8_t hci_phy);
  */
 int bt_get_df_cte_type(uint8_t hci_cte_type);
 
-/** Start or restart scanner if needed
- *
- * Examples of cases where it may be required to start/restart a scanner:
- * - When the auto-connection establishement feature is used:
- *   - When the host sets a connection context for auto-connection establishment.
- *   - When a connection was established.
- *     The host may now be able to retry to automatically set up a connection.
- *   - When a connection was disconnected/lost.
- *     The host may now be able to retry to automatically set up a connection.
- *   - When the application stops explicit scanning.
- *     The host may now be able to retry to automatically set up a connection.
- *   - The application tries to connect to another device, but fails.
- *     The host may now be able to retry to automatically set up a connection.
- * - When the application wants to connect to a device, but we need
- *   to fallback to host privacy.
- * - When the application wants to establish a periodic sync to a device
- *   and the application has not already started scanning.
- *
- * @param fast_scan Use fast scan parameters or slow scan parameters
- *
- * @return 0 in case of success, or a negative error code on failure.
- */
-int bt_le_scan_update(bool fast_scan);
-
 int bt_le_create_conn(const struct bt_conn *conn);
 int bt_le_create_conn_cancel(void);
 int bt_le_create_conn_synced(const struct bt_conn *conn, const struct bt_le_ext_adv *adv,

subsys/bluetooth/host/id.c

@@ -689,11 +689,11 @@ static void rpa_timeout(struct k_work *work)
 	le_rpa_invalidate();
 
 	/* IF no roles using the RPA is running we can stop the RPA timer */
-	if (!(adv_enabled ||
-	      atomic_test_bit(bt_dev.flags, BT_DEV_INITIATING) ||
-	      (atomic_test_bit(bt_dev.flags, BT_DEV_SCANNING) &&
-	       atomic_test_bit(bt_dev.flags, BT_DEV_ACTIVE_SCAN)))) {
-		return;
+	if (IS_ENABLED(CONFIG_BT_CENTRAL)) {
+		if (!(adv_enabled || atomic_test_bit(bt_dev.flags, BT_DEV_INITIATING) ||
+		      bt_le_scan_active_scanner_running())) {
+			return;
+		}
 	}
 
 	le_update_private_addr();