From 6cec495d8a4f61c92c9217a14cd00641e7ca57df Mon Sep 17 00:00:00 2001 From: Pavel Vasilyev Date: Wed, 30 Oct 2024 13:29:30 +0100 Subject: [PATCH 1/2] bluetooth: mesh: adv: legacy: Check suspended flag in the adv thread Instead of checking the `enabled` flag, check if BT_MESH_SUSPENDED is set in the legacy advertiser thread. BT_MESH_SUSPENDED is set earlier than advertiser is stopped and will prevent the advertiser send anything earlier. Signed-off-by: Pavel Vasilyev --- subsys/bluetooth/mesh/adv_legacy.c | 31 ++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/subsys/bluetooth/mesh/adv_legacy.c b/subsys/bluetooth/mesh/adv_legacy.c index c79a118cf1554..207ff718566e4 100644 --- a/subsys/bluetooth/mesh/adv_legacy.c +++ b/subsys/bluetooth/mesh/adv_legacy.c @@ -38,7 +38,11 @@ LOG_MODULE_REGISTER(bt_mesh_adv_legacy); static struct k_thread adv_thread_data; static K_KERNEL_STACK_DEFINE(adv_thread_stack, CONFIG_BT_MESH_ADV_STACK_SIZE); static int32_t adv_timeout; -static bool enabled; + +static bool is_mesh_suspended(void) +{ + return atomic_test_bit(bt_mesh.flags, BT_MESH_SUSPENDED); +} static int bt_data_send(uint8_t num_events, uint16_t adv_int, const struct bt_data *ad, size_t ad_len, @@ -104,7 +108,7 @@ static int bt_data_send(uint8_t num_events, uint16_t adv_int, bt_mesh_adv_send_start(duration, err, ctx); } - if (enabled) { + if (!is_mesh_suspended()) { k_sleep(K_MSEC(duration)); } @@ -148,7 +152,7 @@ static void adv_thread(void *p1, void *p2, void *p3) LOG_DBG("started"); struct bt_mesh_adv *adv; - while (enabled) { + while (!is_mesh_suspended()) { if (IS_ENABLED(CONFIG_BT_MESH_GATT_SERVER)) { adv = bt_mesh_adv_get(K_NO_WAIT); if (IS_ENABLED(CONFIG_BT_MESH_PROXY_SOLICITATION) && !adv) { @@ -234,7 +238,13 @@ void bt_mesh_adv_init(void) int bt_mesh_adv_enable(void) { - enabled = true; + /* The advertiser thread relies on BT_MESH_SUSPENDED flag. No point in starting the + * advertiser thread if the flag is not set. + */ + if (is_mesh_suspended()) { + return -EINVAL; + } + k_thread_start(&adv_thread_data); return 0; } @@ -243,12 +253,21 @@ int bt_mesh_adv_disable(void) { int err; - enabled = false; + /* k_thread_join will sleep forever if BT_MESH_SUSPENDED flag is not set. The advertiser + * thread will exit once the flag is set. The flag is set by the higher layer function. Here + * we need to check that the flag is dropped and ensure that the thread is stopped. + */ + if (!is_mesh_suspended()) { + return -EINVAL; + } err = k_thread_join(&adv_thread_data, K_FOREVER); LOG_DBG("Advertising disabled: %d", err); - return 0; + /* Since the thread will immediately stop after this function call and won’t perform any + * further operations, it’s safe to ignore the deadlock error (EDEADLK). + */ + return err == -EDEADLK ? 0 : err; } int bt_mesh_adv_gatt_start(const struct bt_le_adv_param *param, int32_t duration, From 5d9deb60f9508bc4c28276371661e9f5f66b402e Mon Sep 17 00:00:00 2001 From: Pavel Vasilyev Date: Wed, 30 Oct 2024 14:29:43 +0100 Subject: [PATCH 2/2] test: bsim: bluetooth: mesh: Wait until adv is actually sent k_sleep may not be enough to let advertiser send the message. Instead we should rely on the bt_mesh_send_cb. Signed-off-by: Pavel Vasilyev --- tests/bsim/bluetooth/mesh/src/mesh_test.c | 8 +++++- tests/bsim/bluetooth/mesh/src/mesh_test.h | 2 ++ tests/bsim/bluetooth/mesh/src/test_suspend.c | 26 ++++++++++++++++++-- 3 files changed, 33 insertions(+), 3 deletions(-) diff --git a/tests/bsim/bluetooth/mesh/src/mesh_test.c b/tests/bsim/bluetooth/mesh/src/mesh_test.c index c092c9135b179..757e150c35013 100644 --- a/tests/bsim/bluetooth/mesh/src/mesh_test.c +++ b/tests/bsim/bluetooth/mesh/src/mesh_test.c @@ -569,11 +569,17 @@ uint16_t bt_mesh_test_own_addr_get(uint16_t start_addr) } void bt_mesh_test_send_over_adv(void *data, size_t len) +{ + bt_mesh_test_send_over_adv_cb(data, len, NULL, NULL); +} + +void bt_mesh_test_send_over_adv_cb(void *data, size_t len, const struct bt_mesh_send_cb *cb, + void *cb_data) { struct bt_mesh_adv *adv = bt_mesh_adv_create(BT_MESH_ADV_DATA, BT_MESH_ADV_TAG_LOCAL, BT_MESH_TRANSMIT(0, 20), K_NO_WAIT); net_buf_simple_add_mem(&adv->b, data, len); - bt_mesh_adv_send(adv, NULL, NULL); + bt_mesh_adv_send(adv, cb, cb_data); } int bt_mesh_test_wait_for_packet(bt_le_scan_cb_t scan_cb, struct k_sem *observer_sem, uint16_t wait) diff --git a/tests/bsim/bluetooth/mesh/src/mesh_test.h b/tests/bsim/bluetooth/mesh/src/mesh_test.h index d3af115c8814b..2a88d4156ca2b 100644 --- a/tests/bsim/bluetooth/mesh/src/mesh_test.h +++ b/tests/bsim/bluetooth/mesh/src/mesh_test.h @@ -204,6 +204,8 @@ void bt_mesh_test_ra_cb_setup(void (*cb)(uint8_t *, size_t)); uint16_t bt_mesh_test_own_addr_get(uint16_t start_addr); void bt_mesh_test_send_over_adv(void *data, size_t len); +void bt_mesh_test_send_over_adv_cb(void *data, size_t len, const struct bt_mesh_send_cb *cb, + void *cb_data); /* Wait for a packet (i. e. an advertisement or a GATT frame) sent by a device. * `scan_cb` is triggered if the packet is received, and must release `observer_sem` when finished. */ diff --git a/tests/bsim/bluetooth/mesh/src/test_suspend.c b/tests/bsim/bluetooth/mesh/src/test_suspend.c index 5ec6a8dc68525..58154f3744640 100644 --- a/tests/bsim/bluetooth/mesh/src/test_suspend.c +++ b/tests/bsim/bluetooth/mesh/src/test_suspend.c @@ -318,8 +318,29 @@ static void dut_pub_common(bool disable_bt) ASSERT_OK(bt_mesh_suspend()); } +static void send_start(uint16_t duration, int err, void *cb_data) +{ + if (err) { + FAIL("Failed to send message (err %d)", err); + } +} + +static void send_end(int err, void *cb_data) +{ + k_sem_give((struct k_sem *)cb_data); +} + static void dut_gatt_common(bool disable_bt) { + struct k_sem send_sem; + + k_sem_init(&send_sem, 0, 1); + + const struct bt_mesh_send_cb send_cb = { + .start = send_start, + .end = send_end, + }; + bt_mesh_test_cfg_set(NULL, WAIT_TIME); bt_mesh_device_setup(&prov, &comp); ASSERT_OK_MSG(bt_mesh_prov_enable(BT_MESH_PROV_GATT), "Failed to enable GATT provisioner"); @@ -336,8 +357,9 @@ static void dut_gatt_common(bool disable_bt) /* Send a mesh message to notify Tester that DUT is about to be suspended. */ dut_status = DUT_SUSPENDED; - bt_mesh_test_send_over_adv(&dut_status, sizeof(enum dut_mesh_status)); - k_sleep(K_MSEC(150)); + bt_mesh_test_send_over_adv_cb(&dut_status, sizeof(enum dut_mesh_status), &send_cb, + &send_sem); + ASSERT_OK(k_sem_take(&send_sem, K_MSEC(200))); ASSERT_OK_MSG(bt_mesh_suspend(), "Failed to suspend Mesh.");