Bluetooth: Host: More bsim refactoring #85319

HaavardRei · 2025-02-06T14:09:34Z

Commit does the following changes:

Use functionality from the babbelkit library for common functions related to flags, test progression (failing, passing etc.) and synchronization between two devices. Locally defined equivalents are removed.
Removes the files containing only functionality that is provided by the babblekit library.
Remove the test_pre_init_f and test_tick_f functions (commonly implemented as test_init and test_tick) from the modified tests. These functions are not needed as they were only used to fail the test if a device didn't complete the test within a certain time frame. This is already handled by the sim_length argument used in the test scripts.

Thalley · 2025-02-06T14:19:37Z

Love to see PRs like that :D Less code, less maintenance (but will take a while to review)

aescolar

This is mostly a +1 from me after a very fast review.

tests/bsim/bluetooth/host/att/mtu_update/src/main_peripheral.c

aescolar · 2025-02-06T14:24:55Z

tests/bsim/bluetooth/host/privacy/peripheral/src/tester_rpa_expired.c

@@ -22,6 +31,14 @@ struct adv_set_data_t {
 static	uint8_t	adv_index;
 static	struct	adv_set_data_t	adv_set_data[CONFIG_BT_EXT_ADV_MAX_ADV_SET];

+static void print_address(bt_addr_le_t *addr)


It seems a bit silly to copy this around.. don't you want to add it to the babblekit library?
(or leave it in that bs_bt_utils.c as a more evident thing somebody will be inclined to clean later :) )

I agree, but I don't think it belongs in the babblekit library as it doesn't do anything bsim related. Any idea of where else to put it?
(I'd rather not keep bs_bt_utils.c/h only for this function)

You could put it in the testlib maybe (tests/bluetooth/common/testlib)

Could we perhaps instead pull in bt_addr_le_str from https://github.com/zephyrproject-rtos/zephyr/blob/main/subsys/bluetooth/common/bt_str.c ?

I added a macro to testlib (addr.h) which allows you to print multiple addresses in the same printk/LOG_DBG etc. (bt_addr_le_str will print the same address if used multiple times in a format string)

theob-pro · 2025-02-07T08:59:23Z

Found some typo and some line return not deleted probably due to the script used. I put everything in a patch as I was at it:

diff --git a/tests/bsim/bluetooth/host/adv/long_ad/src/advertiser.c b/tests/bsim/bluetooth/host/adv/long_ad/src/advertiser.c
index 308d96eee24..b4fbc02b0d2 100644
--- a/tests/bsim/bluetooth/host/adv/long_ad/src/advertiser.c
+++ b/tests/bsim/bluetooth/host/adv/long_ad/src/advertiser.c
@@ -32,7 +32,7 @@ static void create_adv(struct bt_le_ext_adv **adv)
 
 	err = bt_le_ext_adv_create(&params, NULL, adv);
 	if (err) {
-		TEST_FAIL("Failed to create advertiser (%d)", err);
+		TEST_FAIL("Failed to create advertiser (%d)\n", err);
 	}
 }
 
@@ -49,7 +49,7 @@ static void start_adv(struct bt_le_ext_adv *adv)
 
 	err = bt_le_ext_adv_start(adv, &start_params);
 	if (err) {
-		TEST_FAIL("Failed to start advertiser (%d)", err);
+		TEST_FAIL("Failed to start advertiser (%d)\n", err);
 	}
 }
 
@@ -99,7 +99,7 @@ static int set_ad_data(struct bt_le_ext_adv *adv, const uint8_t *serialized_ad,
 
 	err = bt_le_ext_adv_set_data(adv, ad, ad_len, NULL, 0);
 	if (err != 0 && err != -EDOM) {
-		TEST_FAIL("Failed to set advertising data (%d)", err);
+		TEST_FAIL("Failed to set advertising data (%d)\n", err);
 	}
 
 	return err;
diff --git a/tests/bsim/bluetooth/host/adv/long_ad/src/scanner.c b/tests/bsim/bluetooth/host/adv/long_ad/src/scanner.c
index e2ca5b60816..b1d17877b74 100644
--- a/tests/bsim/bluetooth/host/adv/long_ad/src/scanner.c
+++ b/tests/bsim/bluetooth/host/adv/long_ad/src/scanner.c
@@ -62,7 +62,7 @@ static void start_scan(void)
 
 	err = bt_le_scan_start(BT_LE_SCAN_PASSIVE, device_found);
 	if (err) {
-		TEST_FAIL("Scanning failed to start (err %d)", err);
+		TEST_FAIL("Scanning failed to start (err %d)\n", err);
 	}
 
 	LOG_DBG("Scanning successfully started");
diff --git a/tests/bsim/bluetooth/host/att/pipeline/dut/src/main.c b/tests/bsim/bluetooth/host/att/pipeline/dut/src/main.c
index 9b96347f4d1..fa4d089fee4 100644
--- a/tests/bsim/bluetooth/host/att/pipeline/dut/src/main.c
+++ b/tests/bsim/bluetooth/host/att/pipeline/dut/src/main.c
@@ -83,7 +83,7 @@ static void do_dlu(struct bt_conn *conn)
 
 	LOG_INF("update DL");
 	err = bt_conn_le_data_len_update(conn, &param);
-	TEST_ASSERT(err == 0, "Can't update data length (err %d)\n", err);
+	TEST_ASSERT(err == 0, "Can't update data length (err %d)", err);
 
 	WAIT_FOR_FLAG(flag_data_length_updated);
 }
@@ -119,7 +119,7 @@ static void device_found(const bt_addr_le_t *addr, int8_t rssi, uint8_t type,
 	}
 }
 
-static struct bt_conn *connecc(void)
+static struct bt_conn *connect(void)
 {
 	int err;
 	struct bt_conn *conn;
@@ -127,12 +127,12 @@ static struct bt_conn *connecc(void)
 	UNSET_FLAG(is_connected);
 
 	err = bt_le_adv_start(BT_LE_ADV_CONN_FAST_1, NULL, 0, NULL, 0);
-	TEST_ASSERT(!err, "Adving failed to start (err %d)\n", err);
+	TEST_ASSERT(!err, "Adving failed to start (err %d)", err);
 
-	LOG_DBG(" wait connecc...");
+	LOG_DBG(" wait connect...");
 
 	WAIT_FOR_FLAG(is_connected);
-	LOG_INF("conecd");
+	LOG_INF("conected");
 
 	conn = dconn;
 	dconn = NULL;
@@ -148,7 +148,7 @@ static struct bt_conn *connect(void)
 	UNSET_FLAG(is_connected);
 
 	err = bt_le_scan_start(BT_LE_SCAN_ACTIVE_CONTINUOUS, device_found);
-	TEST_ASSERT(!err, "Scanning failed to start (err %d)\n", err);
+	TEST_ASSERT(!err, "Scanning failed to start (err %d)", err);
 
 	LOG_DBG("Central initiating connection...");
 	WAIT_FOR_FLAG(is_connected);
@@ -218,7 +218,7 @@ static void send_write_handle(struct bt_conn *conn)
 	sys_put_le16(handle, data);
 
 	err = bt_gatt_notify(conn, attr, data, sizeof(data));
-	TEST_ASSERT(!err, "Failed to transmit handle for write (err %d)\n", err);
+	TEST_ASSERT(!err, "Failed to transmit handle for write (err %d)", err);
 }
 
 static void gatt_read(struct bt_conn *conn, uint16_t handle)
@@ -269,10 +269,10 @@ void good_peer_procedure(void)
 	struct bt_conn *conn;
 
 	err = bt_enable(NULL);
-	TEST_ASSERT(err == 0, "Can't enable Bluetooth (err %d)\n", err);
+	TEST_ASSERT(err == 0, "Can't enable Bluetooth (err %d)", err);
 	LOG_DBG("Central: Bluetooth initialized.");
 
-	conn = connecc();
+	conn = connect();
 
 	find_the_chrc(conn, test_service_uuid, test_characteristic_uuid, &handle);
 
diff --git a/tests/bsim/bluetooth/host/att/sequential/common/common_defs.h b/tests/bsim/bluetooth/host/att/sequential/common/common_defs.h
index e962391d690..122d2f7830b 100644
--- a/tests/bsim/bluetooth/host/att/sequential/common/common_defs.h
+++ b/tests/bsim/bluetooth/host/att/sequential/common/common_defs.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2023 Nordic Semiconductor ASA
+ * Copyright (c) 2025 Nordic Semiconductor ASA
  *
  * SPDX-License-Identifier: Apache-2.0
  */
diff --git a/tests/bsim/bluetooth/host/att/sequential/dut/src/main.c b/tests/bsim/bluetooth/host/att/sequential/dut/src/main.c
index 995e0de0838..b9a3f8655d8 100644
--- a/tests/bsim/bluetooth/host/att/sequential/dut/src/main.c
+++ b/tests/bsim/bluetooth/host/att/sequential/dut/src/main.c
@@ -84,7 +84,7 @@ static void do_dlu(void)
 	param.tx_max_time = 2500;
 
 	err = bt_conn_le_data_len_update(dconn, &param);
-	TEST_ASSERT(err == 0, "Can't update data length (err %d)\n", err);
+	TEST_ASSERT(err == 0, "Can't update data length (err %d)", err);
 
 	WAIT_FOR_FLAG(flag_data_length_updated);
 }
@@ -133,7 +133,7 @@ static void connect(void)
 	UNSET_FLAG(is_connected);
 
 	err = bt_le_scan_start(&scan_param, device_found);
-	TEST_ASSERT(!err, "Scanning failed to start (err %d)\n", err);
+	TEST_ASSERT(!err, "Scanning failed to start (err %d)", err);
 
 	LOG_DBG("Central initiating connection...");
 	WAIT_FOR_FLAG(is_connected);
@@ -210,9 +210,9 @@ static void subscribed(struct bt_conn *conn,
 		       uint8_t err,
 		       struct bt_gatt_subscribe_params *params)
 {
-	TEST_ASSERT(!err, "Subscribe failed (err %d)\n", err);
+	TEST_ASSERT(!err, "Subscribe failed (err %d)", err);
 
-	TEST_ASSERT(params, "params is NULL\n");
+	TEST_ASSERT(params, "params is NULL");
 
 	SET_FLAG(is_subscribed);
 	/* spoiler: tester doesn't really have attributes */
@@ -233,7 +233,7 @@ void subscribe(void)
 	};
 
 	err = bt_gatt_subscribe(dconn, &params);
-	TEST_ASSERT(!err, "Subscribe failed (err %d)\n", err);
+	TEST_ASSERT(!err, "Subscribe failed (err %d)", err);
 
 	WAIT_FOR_FLAG(is_subscribed);
 }
@@ -250,7 +250,7 @@ static void send_write_handle(void)
 	sys_put_le16(handle, data);
 
 	err = bt_gatt_notify(dconn, attr, data, sizeof(data));
-	TEST_ASSERT(!err, "Failed to transmit handle for write (err %d)\n", err);
+	TEST_ASSERT(!err, "Failed to transmit handle for write (err %d)", err);
 }
 
 void test_procedure_0(void)
@@ -259,7 +259,7 @@ void test_procedure_0(void)
 	int err;
 
 	err = bt_enable(NULL);
-	TEST_ASSERT(err == 0, "Can't enable Bluetooth (err %d)\n", err);
+	TEST_ASSERT(err == 0, "Can't enable Bluetooth (err %d)", err);
 	LOG_DBG("Central: Bluetooth initialized.");
 
 	/* Test purpose:
diff --git a/tests/bsim/bluetooth/host/gatt/sc_indicate/src/bs_bt_utils.c b/tests/bsim/bluetooth/host/gatt/sc_indicate/src/bs_bt_utils.c
index f31b7e8405e..df054cc3321 100644
--- a/tests/bsim/bluetooth/host/gatt/sc_indicate/src/bs_bt_utils.c
+++ b/tests/bsim/bluetooth/host/gatt/sc_indicate/src/bs_bt_utils.c
@@ -52,7 +52,7 @@ void clear_g_conn(void)
 
 	conn = g_conn;
 	g_conn = NULL;
-	TEST_ASSERT(conn, "Test error: no g_conn!\n");
+	TEST_ASSERT(conn, "Test error: no g_conn!");
 	bt_conn_unref(conn);
 }
 
diff --git a/tests/bsim/bluetooth/host/gatt/sc_indicate/src/central.c b/tests/bsim/bluetooth/host/gatt/sc_indicate/src/central.c
index 72a436be9bd..9caef6cdda8 100644
--- a/tests/bsim/bluetooth/host/gatt/sc_indicate/src/central.c
+++ b/tests/bsim/bluetooth/host/gatt/sc_indicate/src/central.c
@@ -105,7 +105,7 @@ static uint8_t discover_func(struct bt_conn *conn,
 			params->type = BT_GATT_DISCOVER_DESCRIPTOR;
 
 			err = bt_gatt_discover(conn, params);
-			TEST_ASSERT(!err, "bt_gatt_discover failed (%d)\n", err);
+			TEST_ASSERT(!err, "bt_gatt_discover failed (%d)", err);
 
 			return BT_GATT_ITER_STOP;
 		}
@@ -194,5 +194,5 @@ void central(void)
 	/* wait for service change indication */
 	WAIT_FOR_FLAG(flag_indicated);
 
-	TEST_PASS("TEST_PASS");
+	TEST_PASS("PASS");
 }
diff --git a/tests/bsim/bluetooth/host/gatt/settings/src/client.c b/tests/bsim/bluetooth/host/gatt/settings/src/client.c
index 85091296b1e..23e947fbabd 100644
--- a/tests/bsim/bluetooth/host/gatt/settings/src/client.c
+++ b/tests/bsim/bluetooth/host/gatt/settings/src/client.c
@@ -170,5 +170,5 @@ void client_procedure(void)
 	client_round_5();
 	client_round_6();
 
-	TEST_PASS("TEST_PASS");
+	TEST_PASS("PASS");
 }
diff --git a/tests/bsim/bluetooth/host/l2cap/split/dut/src/main.c b/tests/bsim/bluetooth/host/l2cap/split/dut/src/main.c
index 66c9eb7f59c..e5f85de2387 100644
--- a/tests/bsim/bluetooth/host/l2cap/split/dut/src/main.c
+++ b/tests/bsim/bluetooth/host/l2cap/split/dut/src/main.c
@@ -193,7 +193,7 @@ static void connect(void)
 
 	int err = bt_le_scan_start(&scan_param, device_found);
 
-	TEST_ASSERT(!err, "Scanning failed to start (err %d)\n", err);
+	TEST_ASSERT(!err, "Scanning failed to start (err %d)", err);
 
 	LOG_DBG("Central initiating connection...");
 	WAIT_FOR_FLAG(is_connected);
@@ -222,7 +222,7 @@ static void do_dlu(struct bt_conn *conn, void *data)
 	param.tx_max_time = 1712;
 
 	err = bt_conn_le_data_len_update(conn, &param);
-	TEST_ASSERT(err == 0, "Can't update data length (err %d)\n", err);
+	TEST_ASSERT(err == 0, "Can't update data length (err %d)", err);
 }
 
 void test_procedure_0(void)
@@ -231,7 +231,7 @@ void test_procedure_0(void)
 	int err;
 
 	err = bt_enable(NULL);
-	TEST_ASSERT(err == 0, "Can't enable Bluetooth (err %d)\n", err);
+	TEST_ASSERT(err == 0, "Can't enable Bluetooth (err %d)", err);
 	LOG_DBG("Central Bluetooth initialized.");
 
 	int psm = l2cap_server_register(BT_SECURITY_L1);
diff --git a/tests/bsim/bluetooth/host/misc/disconnect/common/common.h b/tests/bsim/bluetooth/host/misc/disconnect/common/common.h
index 498e4347c9d..9ebed1cda6c 100644
--- a/tests/bsim/bluetooth/host/misc/disconnect/common/common.h
+++ b/tests/bsim/bluetooth/host/misc/disconnect/common/common.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2023 Nordic Semiconductor ASA
+ * Copyright (c) 2025 Nordic Semiconductor ASA
  *
  * SPDX-License-Identifier: Apache-2.0
  */
diff --git a/tests/bsim/bluetooth/host/misc/disconnect/dut/src/main.c b/tests/bsim/bluetooth/host/misc/disconnect/dut/src/main.c
index fee4bad7f91..b40d735ea07 100644
--- a/tests/bsim/bluetooth/host/misc/disconnect/dut/src/main.c
+++ b/tests/bsim/bluetooth/host/misc/disconnect/dut/src/main.c
@@ -214,7 +214,7 @@ void subscribe(void)
 	};
 
 	err = bt_gatt_subscribe(dconn, &params);
-	TEST_ASSERT(!err, "Subscribe failed (err %d)\n", err);
+	TEST_ASSERT(!err, "Subscribe failed (err %d)", err);
 
 	WAIT_FOR_FLAG(is_subscribed);
 }
diff --git a/tests/bsim/bluetooth/host/misc/hfc/src/main.c b/tests/bsim/bluetooth/host/misc/hfc/src/main.c
index 34941c2a6ac..cdb0b9ba72c 100644
--- a/tests/bsim/bluetooth/host/misc/hfc/src/main.c
+++ b/tests/bsim/bluetooth/host/misc/hfc/src/main.c
@@ -114,7 +114,7 @@ static struct bt_conn *connect_as_peripheral(void)
 	UNSET_FLAG(is_connected);
 
 	err = bt_le_adv_start(BT_LE_ADV_CONN_FAST_1, NULL, 0, NULL, 0);
-	TEST_ASSERT(!err, "Adving failed to start (err %d)\n", err);
+	TEST_ASSERT(!err, "Adving failed to start (err %d)", err);
 
 	LOG_DBG("advertising");
 	WAIT_FOR_FLAG(is_connected);
@@ -140,7 +140,7 @@ static struct bt_conn *connect_as_central(void)
 	UNSET_FLAG(is_connected);
 
 	err = bt_le_scan_start(&scan_param, device_found);
-	TEST_ASSERT(!err, "Scanning failed to start (err %d)\n", err);
+	TEST_ASSERT(!err, "Scanning failed to start (err %d)", err);
 
 	LOG_DBG("Central initiating connection...");
 	WAIT_FOR_FLAG(is_connected);
@@ -199,9 +199,9 @@ static void subscribed(struct bt_conn *conn,
 		       uint8_t err,
 		       struct bt_gatt_subscribe_params *params)
 {
-	TEST_ASSERT(!err, "Subscribe failed (err %d)\n", err);
+	TEST_ASSERT(!err, "Subscribe failed (err %d)", err);
 
-	TEST_ASSERT(params, "params is NULL\n");
+	TEST_ASSERT(params, "params is NULL");
 
 	SET_FLAG(is_subscribed);
 
@@ -222,7 +222,7 @@ static void subscribe(struct bt_conn *conn,
 	params.ccc_handle = handle + 1;
 
 	err = bt_gatt_subscribe(conn, &params);
-	TEST_ASSERT(!err, "Subscribe failed (err %d)\n", err);
+	TEST_ASSERT(!err, "Subscribe failed (err %d)", err);
 
 	WAIT_FOR_FLAG(is_subscribed);
 }
@@ -269,7 +269,7 @@ static bool is_disconnected(struct bt_conn *conn)
 	struct bt_conn_info info;
 
 	err = bt_conn_get_info(conn, &info);
-	TEST_ASSERT(err == 0, "Failed to get info for %p\n", conn);
+	TEST_ASSERT(err == 0, "Failed to get info for %p", conn);
 
 	/* Return if fully disconnected */
 	return info.state == BT_CONN_STATE_DISCONNECTED;
@@ -322,7 +322,7 @@ static void entrypoint_dut(void)
 	s->rx = 0;
 
 	err = bt_enable(NULL);
-	TEST_ASSERT(err == 0, "Can't enable Bluetooth (err %d)\n", err);
+	TEST_ASSERT(err == 0, "Can't enable Bluetooth (err %d)", err);
 	LOG_DBG("Central: Bluetooth initialized.");
 
 	s->conn = connect_and_subscribe();
@@ -364,7 +364,7 @@ static void entrypoint_peer(void)
 	LOG_DBG("Test start: peer 0");
 
 	err = bt_enable(NULL);
-	TEST_ASSERT(err == 0, "Can't enable Bluetooth (err %d)\n", err);
+	TEST_ASSERT(err == 0, "Can't enable Bluetooth (err %d)", err);
 	LOG_DBG("Bluetooth initialized.");
 
 	/* prepare data for notifications */
diff --git a/tests/bsim/bluetooth/host/privacy/central/src/dut.c b/tests/bsim/bluetooth/host/privacy/central/src/dut.c
index a165edc39bc..ebf9c7fe891 100644
--- a/tests/bsim/bluetooth/host/privacy/central/src/dut.c
+++ b/tests/bsim/bluetooth/host/privacy/central/src/dut.c
@@ -23,7 +23,7 @@ void start_scanning(void)
 	/* Enable bluetooth */
 	err = bt_enable(NULL);
 	if (err) {
-		TEST_FAIL("Failed to enable bluetooth (err %d\n)", err);
+		TEST_FAIL("Failed to enable bluetooth (err %d)", err);
 	}
 
 	/* Start active scanning */
@@ -47,7 +47,7 @@ void dut_procedure(void)
 
 	/* Nothing to do */
 
-	TEST_PASS("TEST_PASS");
+	TEST_PASS("PASS");
 }
 
 void dut_procedure_connect_short_rpa_timeout(void)
@@ -78,7 +78,7 @@ void dut_procedure_connect_short_rpa_timeout(void)
 	err = bt_testlib_connect(&peer, &conn);
 	TEST_ASSERT(!err, "Failed to initiate connection (err %d)", err);
 
-	TEST_PASS("TEST_PASS");
+	TEST_PASS("PASS");
 }
 
 void dut_procedure_connect_timeout(void)
diff --git a/tests/bsim/bluetooth/host/privacy/central/src/tester.c b/tests/bsim/bluetooth/host/privacy/central/src/tester.c
index 4a3b62328eb..7d8148d5cba 100644
--- a/tests/bsim/bluetooth/host/privacy/central/src/tester.c
+++ b/tests/bsim/bluetooth/host/privacy/central/src/tester.c
@@ -41,10 +41,10 @@ void scanned_cb(struct bt_le_ext_adv *adv, struct bt_le_ext_adv_scanned_info *in
 	/* Check if the scan request comes from a new address */
 	if (bt_addr_le_cmp(&old_addr, &new_addr)) {
 		int64_t new_time, diff, time_diff_ms, rpa_timeout_ms;
-		char array[BT_ADDR_LE_STR_LEN];
+		char addr_str[BT_ADDR_LE_STR_LEN];
 
-		bt_addr_le_to_str(info->addr, array, sizeof(array));
-		printk("Scanned request from new address : %s\n", array);
+		bt_addr_le_to_str(info->addr, addr_str, sizeof(addr_str));
+		printk("Scanned request from new address : %s\n", addr_str);
 
 		/* Ensure the RPA rotation occurs within +-10% of CONFIG_BT_RPA_TIMEOUT */
 		new_time = k_uptime_get();
@@ -134,7 +134,7 @@ void tester_procedure(void)
 		UNSET_FLAG(flag_new_address);
 	}
 
-	TEST_PASS("TEST_PASS");
+	TEST_PASS("PASS");
 }
 
 void tester_procedure_periph_delayed_start_of_conn_adv(void)
@@ -184,5 +184,5 @@ void tester_procedure_periph_delayed_start_of_conn_adv(void)
 
 	WAIT_FOR_FLAG(flag_connected);
 
-	TEST_PASS("TEST_PASS");
+	TEST_PASS("PASS");
 }
diff --git a/tests/bsim/bluetooth/host/privacy/legacy/src/tester.c b/tests/bsim/bluetooth/host/privacy/legacy/src/tester.c
index 5729ae6ba0c..f02dc44806a 100644
--- a/tests/bsim/bluetooth/host/privacy/legacy/src/tester.c
+++ b/tests/bsim/bluetooth/host/privacy/legacy/src/tester.c
@@ -86,7 +86,7 @@ static void cb_expect_rpa(const bt_addr_le_t *addr, int8_t rssi, uint8_t type,
 	test_data.old_time = k_uptime_get();
 
 	if (test_data.rpa_rotations > EXPECTED_NUM_ROTATIONS) {
-		TEST_PASS("TEST_PASS");
+		TEST_PASS("PASS");
 	}
 }
 
diff --git a/tests/bsim/bluetooth/host/privacy/peripheral/src/dut_rpa_expired.c b/tests/bsim/bluetooth/host/privacy/peripheral/src/dut_rpa_expired.c
index 205e5ea6424..4c0e2f561e5 100644
--- a/tests/bsim/bluetooth/host/privacy/peripheral/src/dut_rpa_expired.c
+++ b/tests/bsim/bluetooth/host/privacy/peripheral/src/dut_rpa_expired.c
@@ -169,5 +169,5 @@ void dut_rpa_expired_procedure(void)
 	start_rpa_advertising();
 
 	/* Nothing to do */
-	TEST_PASS("TEST_PASS");
+	TEST_PASS("PASS");
 }
diff --git a/tests/bsim/bluetooth/host/privacy/peripheral/src/dut_rpa_rotation.c b/tests/bsim/bluetooth/host/privacy/peripheral/src/dut_rpa_rotation.c
index 89cd5557f4b..b5392e1907a 100644
--- a/tests/bsim/bluetooth/host/privacy/peripheral/src/dut_rpa_rotation.c
+++ b/tests/bsim/bluetooth/host/privacy/peripheral/src/dut_rpa_rotation.c
@@ -145,5 +145,5 @@ void dut_procedure(void)
 	start_advertising();
 
 	/* Nothing to do */
-	TEST_PASS("TEST_PASS");
+	TEST_PASS("PASS");
 }
diff --git a/tests/bsim/bluetooth/host/privacy/peripheral/src/tester_rpa_expired.c b/tests/bsim/bluetooth/host/privacy/peripheral/src/tester_rpa_expired.c
index 393caa882d0..6b5d169711a 100644
--- a/tests/bsim/bluetooth/host/privacy/peripheral/src/tester_rpa_expired.c
+++ b/tests/bsim/bluetooth/host/privacy/peripheral/src/tester_rpa_expired.c
@@ -33,10 +33,10 @@ static	struct	adv_set_data_t	adv_set_data[CONFIG_BT_EXT_ADV_MAX_ADV_SET];
 
 static void print_address(bt_addr_le_t *addr)
 {
-	char array[BT_ADDR_LE_STR_LEN];
+	char addr_str[BT_ADDR_LE_STR_LEN];
 
-	bt_addr_le_to_str(addr, array, sizeof(array));
-	printk("Address : %s\n", array);
+	bt_addr_le_to_str(addr, addr_str, sizeof(addr_str));
+	printk("Address : %s\n", addr_str);
 }
 
 static bool data_cb(struct bt_data *data, void *user_data)
@@ -107,7 +107,7 @@ static void test_address(bt_addr_le_t *addr)
 
 	adv_set_data[adv_index].rpa_rotations++;
 	if (adv_set_data[adv_index].rpa_rotations > EXPECTED_NUM_ROTATIONS) {
-		TEST_PASS("TEST_PASS");
+		TEST_PASS("PASS");
 	}
 
 	adv_set_data[adv_index].old_time = k_uptime_get();
diff --git a/tests/bsim/bluetooth/host/privacy/peripheral/src/tester_rpa_rotation.c b/tests/bsim/bluetooth/host/privacy/peripheral/src/tester_rpa_rotation.c
index 6419c4536f7..0e8ecd3b105 100644
--- a/tests/bsim/bluetooth/host/privacy/peripheral/src/tester_rpa_rotation.c
+++ b/tests/bsim/bluetooth/host/privacy/peripheral/src/tester_rpa_rotation.c
@@ -34,10 +34,10 @@ static	struct	adv_set_data_t	adv_set_data[CONFIG_BT_EXT_ADV_MAX_ADV_SET];
 
 static void print_address(bt_addr_le_t *addr)
 {
-	char array[BT_ADDR_LE_STR_LEN];
+	char addr_str[BT_ADDR_LE_STR_LEN];
 
-	bt_addr_le_to_str(addr, array, sizeof(array));
-	printk("Address : %s\n", array);
+	bt_addr_le_to_str(addr, addr_str, sizeof(addr_str));
+	printk("Address : %s\n", addr_str);
 }
 
 static bool data_cb(struct bt_data *data, void *user_data)
@@ -124,7 +124,7 @@ static void test_address(bt_addr_le_t *addr)
 	validate_rpa_addr_generated_for_adv_sets();
 
 	if (adv_set_data[adv_index].rpa_rotations > EXPECTED_NUM_ROTATIONS) {
-		TEST_PASS("TEST_PASS");
+		TEST_PASS("PASS");
 	}
 }
 
diff --git a/tests/bsim/bluetooth/host/security/bond_overwrite_allowed/src/bs_bt_utils.c b/tests/bsim/bluetooth/host/security/bond_overwrite_allowed/src/bs_bt_utils.c
index a886aff8829..a71907acf12 100644
--- a/tests/bsim/bluetooth/host/security/bond_overwrite_allowed/src/bs_bt_utils.c
+++ b/tests/bsim/bluetooth/host/security/bond_overwrite_allowed/src/bs_bt_utils.c
@@ -55,7 +55,7 @@ void clear_g_conn(void)
 
 	conn = g_conn;
 	g_conn = NULL;
-	TEST_ASSERT(conn, "Test error: No g_conn!\n");
+	TEST_ASSERT(conn, "Test error: No g_conn!");
 	bt_conn_unref(conn);
 }
 
@@ -83,9 +83,9 @@ void bs_bt_utils_setup(void)
 	int err;
 
 	err = bt_enable(NULL);
-	TEST_ASSERT(!err, "bt_enable failed.\n");
+	TEST_ASSERT(!err, "bt_enable failed.");
 	err = bt_conn_auth_info_cb_register(&bt_conn_auth_info_cb);
-	TEST_ASSERT(!err, "bt_conn_auth_info_cb_register failed.\n");
+	TEST_ASSERT(!err, "bt_conn_auth_info_cb_register failed.");
 }
 
 static void scan_connect_to_first_result__device_found(const bt_addr_le_t *addr, int8_t rssi,
@@ -153,5 +153,5 @@ void advertise_connectable(int id, bt_addr_le_t *directed_dst)
 	}
 
 	err = bt_le_adv_start(&param, NULL, 0, NULL, 0);
-	TEST_ASSERT(err == 0, "Advertising failed to start (err %d)\n", err);
+	TEST_ASSERT(err == 0, "Advertising failed to start (err %d)", err);
 }
diff --git a/tests/bsim/bluetooth/host/security/bond_overwrite_allowed/src/central.c b/tests/bsim/bluetooth/host/security/bond_overwrite_allowed/src/central.c
index 5ac9f3b171b..c2e03e71dac 100644
--- a/tests/bsim/bluetooth/host/security/bond_overwrite_allowed/src/central.c
+++ b/tests/bsim/bluetooth/host/security/bond_overwrite_allowed/src/central.c
@@ -46,5 +46,5 @@ void central(void)
 	TEST_ASSERT(bt_addr_le_eq(bt_conn_get_dst(g_conn), &id_b),
 		    "Unexpected Peer. Did something resolve incorrectly?");
 
-	TEST_PASS("TEST_PASS");
+	TEST_PASS("PASS");
 }
diff --git a/tests/bsim/bluetooth/host/security/bond_overwrite_allowed/src/peripheral.c b/tests/bsim/bluetooth/host/security/bond_overwrite_allowed/src/peripheral.c
index 4360b5b4da0..11d0fe51ff3 100644
--- a/tests/bsim/bluetooth/host/security/bond_overwrite_allowed/src/peripheral.c
+++ b/tests/bsim/bluetooth/host/security/bond_overwrite_allowed/src/peripheral.c
@@ -56,5 +56,5 @@ void peripheral(void)
 	wait_connected();
 	/* Central should verify that its bond with id_b works as expected. */
 
-	TEST_PASS("TEST_PASS");
+	TEST_PASS("PASS");
 }
diff --git a/tests/bsim/bluetooth/host/security/bond_overwrite_denied/src/bs_bt_utils.c b/tests/bsim/bluetooth/host/security/bond_overwrite_denied/src/bs_bt_utils.c
index a886aff8829..a71907acf12 100644
--- a/tests/bsim/bluetooth/host/security/bond_overwrite_denied/src/bs_bt_utils.c
+++ b/tests/bsim/bluetooth/host/security/bond_overwrite_denied/src/bs_bt_utils.c
@@ -55,7 +55,7 @@ void clear_g_conn(void)
 
 	conn = g_conn;
 	g_conn = NULL;
-	TEST_ASSERT(conn, "Test error: No g_conn!\n");
+	TEST_ASSERT(conn, "Test error: No g_conn!");
 	bt_conn_unref(conn);
 }
 
@@ -83,9 +83,9 @@ void bs_bt_utils_setup(void)
 	int err;
 
 	err = bt_enable(NULL);
-	TEST_ASSERT(!err, "bt_enable failed.\n");
+	TEST_ASSERT(!err, "bt_enable failed.");
 	err = bt_conn_auth_info_cb_register(&bt_conn_auth_info_cb);
-	TEST_ASSERT(!err, "bt_conn_auth_info_cb_register failed.\n");
+	TEST_ASSERT(!err, "bt_conn_auth_info_cb_register failed.");
 }
 
 static void scan_connect_to_first_result__device_found(const bt_addr_le_t *addr, int8_t rssi,
@@ -153,5 +153,5 @@ void advertise_connectable(int id, bt_addr_le_t *directed_dst)
 	}
 
 	err = bt_le_adv_start(&param, NULL, 0, NULL, 0);
-	TEST_ASSERT(err == 0, "Advertising failed to start (err %d)\n", err);
+	TEST_ASSERT(err == 0, "Advertising failed to start (err %d)", err);
 }
diff --git a/tests/bsim/bluetooth/host/security/bond_overwrite_denied/src/central.c b/tests/bsim/bluetooth/host/security/bond_overwrite_denied/src/central.c
index b79601d2185..6bf8d38bbf0 100644
--- a/tests/bsim/bluetooth/host/security/bond_overwrite_denied/src/central.c
+++ b/tests/bsim/bluetooth/host/security/bond_overwrite_denied/src/central.c
@@ -33,5 +33,5 @@ void central(void)
 	wait_connected();
 	set_security(BT_SECURITY_L2);
 	wait_disconnected();
-	TEST_PASS("TEST_PASS");
+	TEST_PASS("PASS");
 }
diff --git a/tests/bsim/bluetooth/host/security/bond_overwrite_denied/src/peripheral.c b/tests/bsim/bluetooth/host/security/bond_overwrite_denied/src/peripheral.c
index b6759e8d56a..e9d0f484f30 100644
--- a/tests/bsim/bluetooth/host/security/bond_overwrite_denied/src/peripheral.c
+++ b/tests/bsim/bluetooth/host/security/bond_overwrite_denied/src/peripheral.c
@@ -44,5 +44,5 @@ void peripheral(void)
 	/* Central should bond here. */
 	BUILD_ASSERT(!IS_ENABLED(CONFIG_BT_ID_UNPAIR_MATCHING_BONDS), "");
 	WAIT_FOR_FLAG(flag_pairing_failed);
-	TEST_PASS("TEST_PASS");
+	TEST_PASS("PASS");
 }

Thalley

Great work - Some comments but no blockers

tests/bsim/bluetooth/host/att/mtu_update/src/main_central.c

tests/bsim/bluetooth/host/att/pipeline/dut/src/main.c

tests/bsim/bluetooth/host/att/pipeline/tester/src/main.c

tests/bsim/bluetooth/host/att/read_fill_buf/test_scripts/run_tests.sh

tests/bsim/bluetooth/host/att/sequential/dut/src/main.c

tests/bsim/bluetooth/host/misc/disconnect/common/common.h

tests/bsim/bluetooth/host/privacy/legacy/src/tester.c

tests/bsim/bluetooth/host/privacy/peripheral/src/dut_rpa_rotation.c

Thalley · 2025-02-07T12:30:48Z

tests/bsim/bluetooth/host/privacy/peripheral/src/tester_rpa_expired.c

@@ -22,6 +31,14 @@ struct adv_set_data_t {
 static	uint8_t	adv_index;
 static	struct	adv_set_data_t	adv_set_data[CONFIG_BT_EXT_ADV_MAX_ADV_SET];

+static void print_address(bt_addr_le_t *addr)


Could we perhaps instead pull in bt_addr_le_str from https://github.com/zephyrproject-rtos/zephyr/blob/main/subsys/bluetooth/common/bt_str.c ?

tests/bsim/babblekit/include/babblekit/flags.h

tests/bluetooth/common/testlib/include/testlib/addr.h

tests/bsim/babblekit/include/babblekit/testcase.h

tests/bsim/bluetooth/host/misc/disconnect/tester/src/main.c

Thalley · 2025-02-10T13:23:08Z

tests/bsim/bluetooth/host/privacy/peripheral/src/dut_rpa_expired.c

@@ -162,5 +169,5 @@ void dut_rpa_expired_procedure(void)
 	start_rpa_advertising();

 	/* Nothing to do */
-	PASS("PASS\n");
+	TEST_PASS("PASS");


Not now, but maybe later TEST_PASS should be refactored to print "Pass" automatically and perhaps with the test_id as well

Adds a macro returning a string representation of a Bluetooth address. This allows you to print multiple addresses in a single printk or LOG... call. Signed-off-by: Håvard Reierstad <[email protected]>

Adds two macros to the babblekit library: * DEFINE_FLAG_STATIC * TEST_ASSERT_NO_MSG Signed-off-by: Håvard Reierstad <[email protected]>

theob-pro

Sorry, I missed some script typo in my first review. That's not a big deal anyway.

tests/bsim/bluetooth/host/att/read_fill_buf/client/main.c

tests/bsim/bluetooth/host/att/read_fill_buf/server/main.c

tests/bsim/bluetooth/host/att/retry_on_sec_err/client/main.c

Commit does the following changes: * Use functionality from the `babbelkit` library for common functions related to flags, test progression (failing, passing etc.) and synchronization between two devices. Locally defined equivalents are removed. * Removes the files containing only functionality that is provided by the `babblekit` library. * Remove the `test_pre_init_f` and `test_tick_f` functions (commonly implemented as `test_init` and `test_tick`) from the modified tests. These functions are not needed as they were only used to fail the test if a device didn't complete the test within a certain time frame. This is already handled by the `sim_length` argument used in the test scripts. Signed-off-by: Håvard Reierstad <[email protected]>

zephyrbot added area: Bluetooth Host Bluetooth Host (excluding BR/EDR) area: Bluetooth platform: nRF BSIM Nordic Semiconductors, nRF BabbleSim labels Feb 6, 2025

zephyrbot requested review from aescolar, alwa-nordic, cvinayak, hermabe, jhedberg, rugeGerritsen, sjanc, Thalley and theob-pro February 6, 2025 14:10

zephyrbot assigned jhedberg and alwa-nordic Feb 6, 2025

HaavardRei force-pushed the host_bsim_refactor branch from aece18e to 2767fd4 Compare February 6, 2025 14:14

alwa-nordic previously approved these changes Feb 6, 2025

View reviewed changes

aescolar reviewed Feb 6, 2025

View reviewed changes

HaavardRei dismissed alwa-nordic’s stale review via 0072335 February 7, 2025 07:10

HaavardRei force-pushed the host_bsim_refactor branch from 2767fd4 to 0072335 Compare February 7, 2025 07:10

Thalley reviewed Feb 7, 2025

View reviewed changes

HaavardRei force-pushed the host_bsim_refactor branch 2 times, most recently from 0d4e595 to dc2d6c3 Compare February 10, 2025 09:37

alwa-nordic previously approved these changes Feb 10, 2025

View reviewed changes

HaavardRei dismissed alwa-nordic’s stale review via d8839c9 February 10, 2025 11:21

HaavardRei force-pushed the host_bsim_refactor branch from dc2d6c3 to d8839c9 Compare February 10, 2025 11:21

HaavardRei requested review from alwa-nordic, Thalley and aescolar February 10, 2025 13:22

Thalley reviewed Feb 10, 2025

View reviewed changes

HaavardRei added 2 commits February 10, 2025 14:55

Bluetooth: testlib: Add addr string macro

5f87803

Adds a macro returning a string representation of a Bluetooth address. This allows you to print multiple addresses in a single printk or LOG... call. Signed-off-by: Håvard Reierstad <[email protected]>

Bluetooth: tests: Add macros to babblekit

ac2becc

Adds two macros to the babblekit library: * DEFINE_FLAG_STATIC * TEST_ASSERT_NO_MSG Signed-off-by: Håvard Reierstad <[email protected]>

HaavardRei force-pushed the host_bsim_refactor branch from d8839c9 to 57b85e3 Compare February 10, 2025 13:55

Thalley previously approved these changes Feb 10, 2025

View reviewed changes

theob-pro previously approved these changes Feb 11, 2025

View reviewed changes

HaavardRei dismissed stale reviews from theob-pro and Thalley via c7b862a February 11, 2025 13:26

HaavardRei force-pushed the host_bsim_refactor branch from 57b85e3 to c7b862a Compare February 11, 2025 13:26

Thalley approved these changes Feb 11, 2025

View reviewed changes

theob-pro approved these changes Feb 12, 2025

View reviewed changes

jhedberg approved these changes Feb 12, 2025

View reviewed changes

kartben merged commit 7737483 into zephyrproject-rtos:main Feb 12, 2025
24 checks passed

HaavardRei deleted the host_bsim_refactor branch February 12, 2025 08:48

Bluetooth: Host: More bsim refactoring #85319

Bluetooth: Host: More bsim refactoring #85319

Uh oh!

Conversation

HaavardRei commented Feb 6, 2025

Uh oh!

Thalley commented Feb 6, 2025

Uh oh!

aescolar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

aescolar Feb 6, 2025

Choose a reason for hiding this comment

Uh oh!

HaavardRei Feb 7, 2025

Choose a reason for hiding this comment

Uh oh!

theob-pro Feb 7, 2025

Choose a reason for hiding this comment

Uh oh!

Thalley Feb 7, 2025

Choose a reason for hiding this comment

Uh oh!

HaavardRei Feb 10, 2025

Choose a reason for hiding this comment

Uh oh!

theob-pro commented Feb 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Thalley left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Thalley Feb 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Thalley Feb 10, 2025

Choose a reason for hiding this comment

Uh oh!

theob-pro left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

theob-pro commented Feb 7, 2025 •

edited

Loading