Skip to content

Bluetooth: Host: More bsim refactoring #85319

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 12, 2025

Conversation

HaavardRei
Copy link
Collaborator

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.

alwa-nordic
alwa-nordic previously approved these changes Feb 6, 2025
@Thalley
Copy link
Collaborator

Thalley commented Feb 6, 2025

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

Copy link
Member

@aescolar aescolar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@@ -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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 :) )

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Collaborator

theob-pro commented Feb 7, 2025

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");
 }

Copy link
Collaborator

@Thalley Thalley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work - Some comments but no blockers

@@ -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)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@HaavardRei HaavardRei force-pushed the host_bsim_refactor branch 2 times, most recently from 0d4e595 to dc2d6c3 Compare February 10, 2025 09:37
alwa-nordic
alwa-nordic previously approved these changes Feb 10, 2025
@@ -162,5 +169,5 @@ void dut_rpa_expired_procedure(void)
start_rpa_advertising();

/* Nothing to do */
PASS("PASS\n");
TEST_PASS("PASS");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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]>
Thalley
Thalley previously approved these changes Feb 10, 2025
theob-pro
theob-pro previously approved these changes Feb 11, 2025
Copy link
Collaborator

@theob-pro theob-pro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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]>
@HaavardRei HaavardRei dismissed stale reviews from theob-pro and Thalley via c7b862a February 11, 2025 13:26
@kartben kartben merged commit 7737483 into zephyrproject-rtos:main Feb 12, 2025
24 checks passed
@HaavardRei HaavardRei deleted the host_bsim_refactor branch February 12, 2025 08:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Bluetooth Host Bluetooth Host (excluding BR/EDR) area: Bluetooth platform: nRF BSIM Nordic Semiconductors, nRF BabbleSim
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants