From 3d9f76b8e5e8a8f80e107aee4e86aa4cf85ed9ec Mon Sep 17 00:00:00 2001 From: huajiang zheng Date: Tue, 31 Oct 2023 15:01:49 +0800 Subject: [PATCH] Bluetooth: Host: add unregister connection callback function [Description] tests: shell: Restart bt will register the same connection callback twice. Callback next node point to itself, when link established callback function loop infinitely. [Fix] Unregister the previous callback to avoid register repeatedly. [Test] After bt init/disable times, create connection successfully. Signed-off-by: huajiang zheng --- include/zephyr/bluetooth/conn.h | 13 + subsys/bluetooth/host/conn.c | 27 ++ subsys/bluetooth/shell/bt.c | 2 + tests/bsim/bluetooth/host/compile.sh | 1 + .../misc/unregister_conn_cb/CMakeLists.txt | 14 + .../host/misc/unregister_conn_cb/prj.conf | 4 + .../host/misc/unregister_conn_cb/src/common.c | 20 ++ .../host/misc/unregister_conn_cb/src/common.h | 55 ++++ .../host/misc/unregister_conn_cb/src/main.c | 240 ++++++++++++++++++ .../tests_scripts/_compile.sh | 17 ++ .../tests_scripts/unregister_conn_cb.sh | 26 ++ 11 files changed, 419 insertions(+) create mode 100644 tests/bsim/bluetooth/host/misc/unregister_conn_cb/CMakeLists.txt create mode 100644 tests/bsim/bluetooth/host/misc/unregister_conn_cb/prj.conf create mode 100644 tests/bsim/bluetooth/host/misc/unregister_conn_cb/src/common.c create mode 100644 tests/bsim/bluetooth/host/misc/unregister_conn_cb/src/common.h create mode 100644 tests/bsim/bluetooth/host/misc/unregister_conn_cb/src/main.c create mode 100755 tests/bsim/bluetooth/host/misc/unregister_conn_cb/tests_scripts/_compile.sh create mode 100755 tests/bsim/bluetooth/host/misc/unregister_conn_cb/tests_scripts/unregister_conn_cb.sh diff --git a/include/zephyr/bluetooth/conn.h b/include/zephyr/bluetooth/conn.h index f934ea54162..edd37bde31e 100644 --- a/include/zephyr/bluetooth/conn.h +++ b/include/zephyr/bluetooth/conn.h @@ -1165,6 +1165,19 @@ struct bt_conn_cb { */ void bt_conn_cb_register(struct bt_conn_cb *cb); +/** + * @brief Unregister connection callbacks. + * + * Unregister the state of connections callbacks. + * + * @param cb Callback struct point to memory that remains valid. + * + * @retval 0 Success + * @retval -EINVAL If @p cb is NULL + * @retval -ENOENT if @p cb was not registered + */ +int bt_conn_cb_unregister(struct bt_conn_cb *cb); + /** * @brief Register a callback structure for connection events. * diff --git a/subsys/bluetooth/host/conn.c b/subsys/bluetooth/host/conn.c index cad3cf32f23..36b2692ee9d 100644 --- a/subsys/bluetooth/host/conn.c +++ b/subsys/bluetooth/host/conn.c @@ -2381,6 +2381,33 @@ void bt_conn_cb_register(struct bt_conn_cb *cb) callback_list = cb; } +int bt_conn_cb_unregister(struct bt_conn_cb *cb) +{ + struct bt_conn_cb *previous_callback; + + CHECKIF(cb == NULL) { + return -EINVAL; + } + + if (callback_list == cb) { + callback_list = callback_list->_next; + return 0; + } + + previous_callback = callback_list; + + while (previous_callback->_next) { + if (previous_callback->_next == cb) { + previous_callback->_next = previous_callback->_next->_next; + return 0; + } + + previous_callback = previous_callback->_next; + } + + return -ENOENT; +} + bool bt_conn_exists_le(uint8_t id, const bt_addr_le_t *peer) { struct bt_conn *conn = bt_conn_lookup_addr_le(id, peer); diff --git a/subsys/bluetooth/shell/bt.c b/subsys/bluetooth/shell/bt.c index e65e85262a7..bbe794969d7 100644 --- a/subsys/bluetooth/shell/bt.c +++ b/subsys/bluetooth/shell/bt.c @@ -1061,6 +1061,8 @@ static void bt_ready(int err) #if defined(CONFIG_BT_CONN) default_conn = NULL; + /* Unregister to avoid register repeatedly */ + bt_conn_cb_unregister(&conn_callbacks); bt_conn_cb_register(&conn_callbacks); #endif /* CONFIG_BT_CONN */ diff --git a/tests/bsim/bluetooth/host/compile.sh b/tests/bsim/bluetooth/host/compile.sh index 36387ef5dd0..1500ba66b5e 100755 --- a/tests/bsim/bluetooth/host/compile.sh +++ b/tests/bsim/bluetooth/host/compile.sh @@ -76,6 +76,7 @@ app=tests/bsim/bluetooth/host/misc/disconnect/dut compile app=tests/bsim/bluetooth/host/misc/disconnect/tester compile app=tests/bsim/bluetooth/host/misc/conn_stress/central compile app=tests/bsim/bluetooth/host/misc/conn_stress/peripheral compile +app=tests/bsim/bluetooth/host/misc/unregister_conn_cb compile app=tests/bsim/bluetooth/host/privacy/central compile app=tests/bsim/bluetooth/host/privacy/peripheral compile diff --git a/tests/bsim/bluetooth/host/misc/unregister_conn_cb/CMakeLists.txt b/tests/bsim/bluetooth/host/misc/unregister_conn_cb/CMakeLists.txt new file mode 100644 index 00000000000..65762a174c2 --- /dev/null +++ b/tests/bsim/bluetooth/host/misc/unregister_conn_cb/CMakeLists.txt @@ -0,0 +1,14 @@ +# SPDX-License-Identifier: Apache-2.0 + +cmake_minimum_required(VERSION 3.20.0) + +find_package(Zephyr REQUIRED HINTS $ENV{ZEPHYR_BASE}) +project(bsim_test_unregister_conn_cb) + +FILE(GLOB app_sources src/*.c) +target_sources(app PRIVATE ${app_sources} ) + +zephyr_include_directories( + ${BSIM_COMPONENTS_PATH}/libUtilv1/src/ + ${BSIM_COMPONENTS_PATH}/libPhyComv1/src/ + ) diff --git a/tests/bsim/bluetooth/host/misc/unregister_conn_cb/prj.conf b/tests/bsim/bluetooth/host/misc/unregister_conn_cb/prj.conf new file mode 100644 index 00000000000..2d054ed9031 --- /dev/null +++ b/tests/bsim/bluetooth/host/misc/unregister_conn_cb/prj.conf @@ -0,0 +1,4 @@ +CONFIG_BT=y +CONFIG_BT_DEVICE_NAME="conn tester" +CONFIG_BT_PERIPHERAL=y +CONFIG_BT_CENTRAL=y diff --git a/tests/bsim/bluetooth/host/misc/unregister_conn_cb/src/common.c b/tests/bsim/bluetooth/host/misc/unregister_conn_cb/src/common.c new file mode 100644 index 00000000000..df438607c5f --- /dev/null +++ b/tests/bsim/bluetooth/host/misc/unregister_conn_cb/src/common.c @@ -0,0 +1,20 @@ +/* + * Copyright (c) 2022 Nordic Semiconductor ASA + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include "common.h" + +void test_tick(bs_time_t HW_device_time) +{ + if (bst_result != Passed) { + FAIL("test failed (not passed after %i seconds)\n", WAIT_TIME); + } +} + +void test_init(void) +{ + bst_ticker_set_next_tick_absolute(WAIT_TIME); + bst_result = In_progress; +} diff --git a/tests/bsim/bluetooth/host/misc/unregister_conn_cb/src/common.h b/tests/bsim/bluetooth/host/misc/unregister_conn_cb/src/common.h new file mode 100644 index 00000000000..159f27a8a21 --- /dev/null +++ b/tests/bsim/bluetooth/host/misc/unregister_conn_cb/src/common.h @@ -0,0 +1,55 @@ +/** + * Common functions and helpers for unregister connection callback tests + * + * Copyright (c) 2024 NXP + * Copyright (c) 2022 Nordic Semiconductor ASA + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include + +#include "bs_types.h" +#include "bs_tracing.h" +#include "time_machine.h" +#include "bstests.h" + +#include +#include +#include + +#include +#include +#include + +extern enum bst_result_t bst_result; + +#define WAIT_SECONDS (30) /*seconds*/ +#define WAIT_TIME (WAIT_SECONDS * USEC_PER_SEC) /*microseconds*/ + +#define CREATE_FLAG(flag) static atomic_t flag = (atomic_t) false +#define SET_FLAG(flag) (void)atomic_set(&flag, (atomic_t) true) +#define UNSET_FLAG(flag) (void)atomic_set(&flag, (atomic_t) false) +#define WAIT_FOR_FLAG(flag) \ + while (!(bool)atomic_get(&flag)) { \ + (void)k_sleep(K_MSEC(1)); \ + } +#define WAIT_FOR_FLAG_UNSET(flag) \ + while ((bool)atomic_get(&flag)) { \ + (void)k_sleep(K_MSEC(1)); \ + } + +#define FAIL(...) \ + do { \ + bst_result = Failed; \ + bs_trace_error_time_line(__VA_ARGS__); \ + } while (0) + +#define PASS(...) \ + do { \ + bst_result = Passed; \ + bs_trace_info_time(1, __VA_ARGS__); \ + } while (0) + +void test_tick(bs_time_t HW_device_time); +void test_init(void); diff --git a/tests/bsim/bluetooth/host/misc/unregister_conn_cb/src/main.c b/tests/bsim/bluetooth/host/misc/unregister_conn_cb/src/main.c new file mode 100644 index 00000000000..f11b179ee34 --- /dev/null +++ b/tests/bsim/bluetooth/host/misc/unregister_conn_cb/src/main.c @@ -0,0 +1,240 @@ +/* + * The goal of this test is to verify the bt_conn_cb_unregister() API works as expected + * + * Copyright (c) 2024 NXP + * Copyright (c) 2022 Nordic Semiconductor ASA + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include "bstests.h" +#include + +#include "common.h" + +CREATE_FLAG(flag_is_connected); + +static struct bt_conn *g_conn; + +static void connected(struct bt_conn *conn, uint8_t err) +{ + char addr[BT_ADDR_LE_STR_LEN]; + + bt_addr_le_to_str(bt_conn_get_dst(conn), addr, sizeof(addr)); + + if (err != 0) { + FAIL("Failed to connect to %s (%u)\n", addr, err); + return; + } + + printk("conn_callback:Connected to %s\n", addr); + + g_conn = conn; + SET_FLAG(flag_is_connected); +} + +static void disconnected(struct bt_conn *conn, uint8_t reason) +{ + char addr[BT_ADDR_LE_STR_LEN]; + + if (conn != g_conn) { + return; + } + + bt_addr_le_to_str(bt_conn_get_dst(conn), addr, sizeof(addr)); + + printk("conn_callback:Disconnected: %s (reason 0x%02x)\n", addr, reason); + + bt_conn_unref(g_conn); + + g_conn = NULL; + UNSET_FLAG(flag_is_connected); +} + +static struct bt_conn_cb conn_callbacks = { + .connected = connected, + .disconnected = disconnected, +}; + +void device_found(const bt_addr_le_t *addr, int8_t rssi, uint8_t type, struct net_buf_simple *ad) +{ + char addr_str[BT_ADDR_LE_STR_LEN]; + int err; + + if (g_conn != NULL) { + printk("g_conn != NULL\n"); + return; + } + + /* We're only interested in connectable events */ + if (type != BT_HCI_ADV_IND && type != BT_HCI_ADV_DIRECT_IND) { + printk("type not connectable\n"); + return; + } + + bt_addr_le_to_str(addr, addr_str, sizeof(addr_str)); + printk("Device found: %s (RSSI %d)\n", addr_str, rssi); + + printk("Stopping scan\n"); + err = bt_le_scan_stop(); + if (err != 0) { + FAIL("Could not stop scan: %d"); + return; + } + + err = bt_conn_le_create(addr, BT_CONN_LE_CREATE_CONN, BT_LE_CONN_PARAM_DEFAULT, &g_conn); + if (err != 0) { + FAIL("Could not connect to peer: %d", err); + } + printk("%s: connected to found device\n", __func__); +} + +static void connection_info(struct bt_conn *conn, void *user_data) +{ + char addr[BT_ADDR_LE_STR_LEN]; + int *conn_count = user_data; + struct bt_conn_info info; + + if (bt_conn_get_info(conn, &info) < 0) { + printk("Unable to get info: conn %p", conn); + return; + } + + switch (info.type) { + case BT_CONN_TYPE_LE: + (*conn_count)++; + bt_addr_le_to_str(bt_conn_get_dst(conn), addr, sizeof(addr)); + printk("%s: Connected to %s\n", __func__, addr); + break; + default: + break; + } +} + +static void test_peripheral_main(void) +{ + int err; + const struct bt_data ad[] = { + BT_DATA_BYTES(BT_DATA_FLAGS, (BT_LE_AD_GENERAL | BT_LE_AD_NO_BREDR))}; + + err = bt_enable(NULL); + if (err != 0) { + FAIL("Bluetooth init failed (err %d)\n", err); + return; + } + + printk("Bluetooth initialized\n"); + + bt_conn_cb_register(&conn_callbacks); + + err = bt_le_adv_start(BT_LE_ADV_CONN_NAME, ad, ARRAY_SIZE(ad), NULL, 0); + if (err != 0) { + FAIL("Advertising failed to start (err %d)\n", err); + return; + } + + printk("Advertising successfully started\n"); + + WAIT_FOR_FLAG(flag_is_connected); + + WAIT_FOR_FLAG_UNSET(flag_is_connected); + + bt_conn_cb_unregister(&conn_callbacks); + + k_sleep(K_SECONDS(1)); + + err = bt_disable(); + if (err != 0) { + FAIL("Bluetooth disable failed (err %d)\n", err); + return; + } + + printk("Bluetooth successfully disabled\n"); + + PASS("Peripheral device passed\n"); +} + +static void test_central_main(void) +{ + int err; + int conn_count = 0; + + err = bt_enable(NULL); + if (err != 0) { + FAIL("Bluetooth discover failed (err %d)\n", err); + } + printk("Bluetooth initialized\n"); + bt_conn_cb_register(&conn_callbacks); + /* Connect to peer device after conn_callbacks registered*/ + err = bt_le_scan_start(BT_LE_SCAN_PASSIVE, device_found); + if (err != 0) { + FAIL("Scanning failed to start (err %d)\n", err); + } + + printk("Scanning successfully started\n"); + + WAIT_FOR_FLAG(flag_is_connected); + + err = bt_conn_disconnect(g_conn, 0x13); + + if (err != 0) { + FAIL("Disconnect failed (err %d)\n", err); + return; + } + + WAIT_FOR_FLAG_UNSET(flag_is_connected); + bt_conn_cb_unregister(&conn_callbacks); + /* Reconnect to the device after conn_callbacks unregistered */ + err = bt_le_scan_start(BT_LE_SCAN_PASSIVE, device_found); + if (err != 0) { + FAIL("Scanning failed to start (err %d)\n", err); + } + printk("Scanning successfully started\n"); + + k_sleep(K_SECONDS(1)); + bt_conn_foreach(BT_CONN_TYPE_LE, connection_info, &conn_count); + if (!conn_count) { + FAIL("Reconnect to peer device failed!"); + } + + /* flag_is_connected not set means no conn_callbacks being called */ + if (flag_is_connected) { + FAIL("Unregister conn_callback didn't work"); + } + printk("Unregister connection callbacks succeed!\n"); + + err = bt_disable(); + if (err != 0) { + FAIL("Bluetooth disable failed (err %d)\n", err); + } + printk("Bluetooth successfully disabled\n"); + + PASS("Central device passed\n"); +} + +static const struct bst_test_instance test_def[] = {{.test_id = "peripheral", + .test_descr = "Peripheral device", + .test_post_init_f = test_init, + .test_tick_f = test_tick, + .test_main_f = test_peripheral_main}, + {.test_id = "central", + .test_descr = "Central device", + .test_post_init_f = test_init, + .test_tick_f = test_tick, + .test_main_f = test_central_main}, + BSTEST_END_MARKER}; + +struct bst_test_list *test_unregister_conn_cb_install(struct bst_test_list *tests) +{ + return bst_add_tests(tests, test_def); +} + +extern struct bst_test_list *test_unregister_conn_cb_install(struct bst_test_list *tests); + +bst_test_install_t test_installers[] = {test_unregister_conn_cb_install, NULL}; + +int main(void) +{ + bst_main(); + return 0; +} diff --git a/tests/bsim/bluetooth/host/misc/unregister_conn_cb/tests_scripts/_compile.sh b/tests/bsim/bluetooth/host/misc/unregister_conn_cb/tests_scripts/_compile.sh new file mode 100755 index 00000000000..b6b361b86d4 --- /dev/null +++ b/tests/bsim/bluetooth/host/misc/unregister_conn_cb/tests_scripts/_compile.sh @@ -0,0 +1,17 @@ +#!/usr/bin/env bash +#Copyright (c) 2024 NXP +#Copyright (c) 2024 Nordic Semiconductor ASA +# SPDX-License-Identifier: Apache-2.0 + +# Path checks, etc +source ${ZEPHYR_BASE}/tests/bsim/sh_common.source + +# Place yourself in the test's root (i.e. ./../) +rm -rf ${BSIM_OUT_PATH}/bin/bs_nrf52_bsim_tests* + +# terminate running simulations (if any) +${BSIM_COMPONENTS_PATH}/common/stop_bsim.sh + +bsim_exe=bs_nrf52_bsim_tests_bsim_bluetooth_host_misc_unregister_conn_cb_prj_conf +west build -b nrf52_bsim && \ + cp build/zephyr/zephyr.exe ${BSIM_OUT_PATH}/bin/${bsim_exe} diff --git a/tests/bsim/bluetooth/host/misc/unregister_conn_cb/tests_scripts/unregister_conn_cb.sh b/tests/bsim/bluetooth/host/misc/unregister_conn_cb/tests_scripts/unregister_conn_cb.sh new file mode 100755 index 00000000000..8fc8bcafaf7 --- /dev/null +++ b/tests/bsim/bluetooth/host/misc/unregister_conn_cb/tests_scripts/unregister_conn_cb.sh @@ -0,0 +1,26 @@ +#!/usr/bin/env bash +# Copyright (c) 2024 NXP +# Copyright (c) 2024 Nordic Semiconductor +# SPDX-License-Identifier: Apache-2.0 + +source ${ZEPHYR_BASE}/tests/bsim/sh_common.source + +#Unregister connection callbacks : A central device scans for and connects to a peripheral +#after registered connection callbacks.When the connection state changes, few printing +#will be printed and the flag_is_connected will be changed by the callback function. +#After unregister the connection callbacks, reconnect to peer device, then no printing +#neither flag change can be found, callback function was unregistered as expected +simulation_id="unregister_conn_cb" +verbosity_level=2 +EXECUTE_TIMEOUT=20 + +cd ${BSIM_OUT_PATH}/bin + +bsim_exe=./bs_nrf52_bsim_tests_bsim_bluetooth_host_misc_unregister_conn_cb_prj_conf + +Execute "${bsim_exe}" -v=${verbosity_level} -s=${simulation_id} -d=0 -testid=central -rs=420 +Execute "${bsim_exe}" -v=${verbosity_level} -s=${simulation_id} -d=1 -testid=peripheral -rs=100 + +Execute ./bs_2G4_phy_v1 -v=${verbosity_level} -s=${simulation_id} -D=2 -sim_length=30e6 $@ + +wait_for_background_jobs