From 5efa3f84e5abc58e9bedd734badc727803c20625 Mon Sep 17 00:00:00 2001 From: Kyra Lengfeld Date: Tue, 21 Jan 2025 17:24:51 +0100 Subject: [PATCH] Bluetooth: Controller: add Unit tests for hci_internal wrapper This commit extracts the wrapper function in its own file, to avoid bloating the hci_internal file, as well as tests its funcionality in a unit test. Signed-off-by: Kyra Lengfeld --- CODEOWNERS | 1 + scripts/ci/tags.yaml | 5 ++ subsys/bluetooth/controller/CMakeLists.txt | 1 + subsys/bluetooth/controller/hci_internal.c | 45 ++------------ .../controller/hci_internal_wrappers.c | 49 +++++++++++++++ .../controller/hci_internal_wrappers.h | 21 +++++++ .../bluetooth/controller/CMakeLists.txt | 26 ++++++++ tests/subsys/bluetooth/controller/prj.conf | 13 ++++ .../src/hci_cmd_cb_host_buffer_size_test.c | 60 +++++++++++++++++++ .../subsys/bluetooth/controller/testcase.yaml | 8 +++ 10 files changed, 188 insertions(+), 41 deletions(-) create mode 100644 subsys/bluetooth/controller/hci_internal_wrappers.c create mode 100644 subsys/bluetooth/controller/hci_internal_wrappers.h create mode 100644 tests/subsys/bluetooth/controller/CMakeLists.txt create mode 100644 tests/subsys/bluetooth/controller/prj.conf create mode 100644 tests/subsys/bluetooth/controller/src/hci_cmd_cb_host_buffer_size_test.c create mode 100644 tests/subsys/bluetooth/controller/testcase.yaml diff --git a/CODEOWNERS b/CODEOWNERS index 47d7f19e41e2..7ef348baca59 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -842,6 +842,7 @@ /tests/subsys/app_event_manager/ @nrfconnect/ncs-si-muffin @nrfconnect/ncs-si-bluebagel /tests/subsys/audio/audio_module_template/ @nrfconnect/ncs-audio /tests/subsys/audio_module/ @nrfconnect/ncs-audio +/tests/subsys/bluetooth/controller @nrfconnect/ncs-dragoon /tests/subsys/bluetooth/gatt_dm/ @nrfconnect/ncs-si-muffin /tests/subsys/bluetooth/enocean/ @nrfconnect/ncs-paladin /tests/subsys/bluetooth/fast_pair/ @nrfconnect/ncs-si-bluebagel diff --git a/scripts/ci/tags.yaml b/scripts/ci/tags.yaml index fe0b06a6d43f..e5c1c266d296 100644 --- a/scripts/ci/tags.yaml +++ b/scripts/ci/tags.yaml @@ -938,6 +938,11 @@ ci_tests_subsys_audio_module: - nrf/subsys/audio/ - nrf/tests/subsys/audio/ +ci_tests_subsys_bluetooth_controller: + files: + - nrf/subsys/bluetooth/controller + - nrf/tests/subsys/bluetooth/controller + ci_tests_subsys_mpsl: files: - nrf/subsys/mpsl/ diff --git a/subsys/bluetooth/controller/CMakeLists.txt b/subsys/bluetooth/controller/CMakeLists.txt index 6f8d09b6a906..b4b9f6670f41 100644 --- a/subsys/bluetooth/controller/CMakeLists.txt +++ b/subsys/bluetooth/controller/CMakeLists.txt @@ -9,6 +9,7 @@ zephyr_library() zephyr_library_sources( hci_driver.c hci_internal.c + hci_internal_wrappers.c ) zephyr_library_sources_ifdef( diff --git a/subsys/bluetooth/controller/hci_internal.c b/subsys/bluetooth/controller/hci_internal.c index 46312b9e078a..44daf2fea869 100644 --- a/subsys/bluetooth/controller/hci_internal.c +++ b/subsys/bluetooth/controller/hci_internal.c @@ -16,6 +16,9 @@ #include #include "hci_internal.h" +#if defined(CONFIG_BT_HCI_ACL_FLOW_CONTROL) +#include "hci_internal_wrappers.h" +#endif #include "ecdh.h" #define CMD_COMPLETE_MIN_SIZE (BT_HCI_EVT_HDR_SIZE \ @@ -930,46 +933,6 @@ static uint8_t link_control_cmd_put(uint8_t const * const cmd) } #endif -#if defined(CONFIG_BT_HCI_ACL_FLOW_CONTROL) -/* - * The Host will generate up to acl_pkts number of Host Number of Completed Packets command plus a - * number of normal HCI commands, as such we need to ensure the tx command buffer count is big - * enough to not block incoming ACKs from the host. - * - * When Controller to Host data flow control is supported, ensure that BT_BUF_CMD_TX_COUNT is - * greater than or equal to (BT_BUF_ACL_RX_COUNT + Ncmd), where Ncmd is the supported maximum - * Num_HCI_Command_Packets. - * - * The SDC controller (currently) does not support Num_HCI_Command_Packets > 1, which means Ncmd - * is always 1. - */ -#if defined(CONFIG_BT_CONN) && defined(CONFIG_BT_HCI_HOST) -BUILD_ASSERT(BT_BUF_ACL_RX_COUNT < CONFIG_BT_BUF_CMD_TX_COUNT, - "Too low HCI command buffers compared to ACL Rx buffers, \ - reduce CONFIG_BT_BUF_ACL_RX_COUNT_EXTRA or increase CONFIG_BT_BUF_CMD_TX_COUNT."); -#else /* controller-only build */ -BUILD_ASSERT(CONFIG_BT_BUF_CMD_TX_COUNT - 1 > 0, - "We need at least two HCI command buffers to avoid deadlocks."); -#endif /* CONFIG_BT_CONN && CONFIG_BT_HCI_HOST */ - -/* Note: When host and controller are built separately, the RX Buffer Count communicated here from - * the host might be exeeding the controller's TX command buffer count. This may result in the - * controller sending up to host_total_num_acl_data_packets, it cannot receive the acknoledgements - * via its TX command buffers for. - * As we do not propagate the TX command buffer count information to the controller, the simplest - * approach is to align the communicated host RX Buffer Count to it here. - */ -static int sdc_hci_cmd_cb_host_buffer_size_wrapper(void *cmd_params) -{ - sdc_hci_cmd_cb_host_buffer_size_t ctrl_cmd_params = - *(const sdc_hci_cmd_cb_host_buffer_size_t *)cmd_params; - ctrl_cmd_params.host_total_num_acl_data_packets = MIN( - ctrl_cmd_params.host_total_num_acl_data_packets, (CONFIG_BT_BUF_CMD_TX_COUNT - 1)); - - return sdc_hci_cmd_cb_host_buffer_size((const void *)&ctrl_cmd_params); -} -#endif /* CONFIG_BT_HCI_ACL_FLOW_CONTROL */ - static uint8_t controller_and_baseband_cmd_put(uint8_t const * const cmd, uint8_t * const raw_event_out, uint8_t *param_length_out) @@ -1968,7 +1931,7 @@ int hci_internal_msg_get(uint8_t *msg_out, sdc_hci_msg_type_t *msg_type_out) return 0; } - const int retval = sdc_hci_get(msg_out, msg_type_out); + const int retval = sdc_hci_get(msg_out, (uint8_t *)msg_type_out); #if defined(CONFIG_BT_CTLR_SDC_PAWR_SYNC) if (retval == 0 && *msg_type_out == SDC_HCI_MSG_TYPE_EVT diff --git a/subsys/bluetooth/controller/hci_internal_wrappers.c b/subsys/bluetooth/controller/hci_internal_wrappers.c new file mode 100644 index 000000000000..53605e83f125 --- /dev/null +++ b/subsys/bluetooth/controller/hci_internal_wrappers.c @@ -0,0 +1,49 @@ +/* + * Copyright (c) 2025 Nordic Semiconductor ASA + * + * SPDX-License-Identifier: LicenseRef-Nordic-5-Clause + */ + +#include "hci_internal_wrappers.h" +#include +#include + +#if defined(CONFIG_BT_HCI_ACL_FLOW_CONTROL) +/* + * The Host will generate up to acl_pkts number of Host Number of Completed Packets command plus a + * number of normal HCI commands, as such we need to ensure the tx command buffer count is big + * enough to not block incoming ACKs from the host. + * + * When Controller to Host data flow control is supported, ensure that BT_BUF_CMD_TX_COUNT is + * greater than or equal to (BT_BUF_ACL_RX_COUNT + Ncmd), where Ncmd is the supported maximum + * Num_HCI_Command_Packets. + * + * The SDC controller (currently) does not support Num_HCI_Command_Packets > 1, which means Ncmd + * is always 1. + */ +#if defined(CONFIG_BT_CONN) && defined(CONFIG_BT_HCI_HOST) +BUILD_ASSERT(BT_BUF_ACL_RX_COUNT < CONFIG_BT_BUF_CMD_TX_COUNT, + "Too low HCI command buffers compared to ACL Rx buffers, \ + reduce CONFIG_BT_BUF_ACL_RX_COUNT_EXTRA or increase CONFIG_BT_BUF_CMD_TX_COUNT."); +#else /* controller-only build */ +BUILD_ASSERT(CONFIG_BT_BUF_CMD_TX_COUNT - 1 > 0, + "We need at least two HCI command buffers to avoid deadlocks."); +#endif /* CONFIG_BT_CONN && CONFIG_BT_HCI_HOST */ + +/* Note: When host and controller are built separately, the RX Buffer Count communicated here from + * the host might be exeeding the controller's TX command buffer count. This may result in the + * controller sending up to host_total_num_acl_data_packets, it cannot receive the acknoledgements + * via its TX command buffers for. + * As we do not propagate the TX command buffer count information to the controller, the simplest + * approach is to align the communicated host RX Buffer Count to it here. + */ +int sdc_hci_cmd_cb_host_buffer_size_wrapper(const sdc_hci_cmd_cb_host_buffer_size_t *cmd_params) +{ + sdc_hci_cmd_cb_host_buffer_size_t ctrl_cmd_params = + *(const sdc_hci_cmd_cb_host_buffer_size_t *)cmd_params; + ctrl_cmd_params.host_total_num_acl_data_packets = MIN( + ctrl_cmd_params.host_total_num_acl_data_packets, (CONFIG_BT_BUF_CMD_TX_COUNT - 1)); + + return sdc_hci_cmd_cb_host_buffer_size((const void *)&ctrl_cmd_params); +} +#endif /* CONFIG_BT_HCI_ACL_FLOW_CONTROL */ diff --git a/subsys/bluetooth/controller/hci_internal_wrappers.h b/subsys/bluetooth/controller/hci_internal_wrappers.h new file mode 100644 index 000000000000..94a31ffd84c0 --- /dev/null +++ b/subsys/bluetooth/controller/hci_internal_wrappers.h @@ -0,0 +1,21 @@ + +/* + * Copyright (c) 2020 Nordic Semiconductor ASA + * + * SPDX-License-Identifier: LicenseRef-Nordic-5-Clause + */ + +/** @file + * @brief Internal HCI interface Wrappers + */ + +#include +#include +#include + +#ifndef HCI_INTERNAL_WRAPPERS_H__ +#define HCI_INTERNAL_WRAPPERS_H__ + +int sdc_hci_cmd_cb_host_buffer_size_wrapper(const sdc_hci_cmd_cb_host_buffer_size_t *cmd_params); + +#endif diff --git a/tests/subsys/bluetooth/controller/CMakeLists.txt b/tests/subsys/bluetooth/controller/CMakeLists.txt new file mode 100644 index 000000000000..68f2a9255e72 --- /dev/null +++ b/tests/subsys/bluetooth/controller/CMakeLists.txt @@ -0,0 +1,26 @@ +# +# Copyright (c) 2025 Nordic Semiconductor ASA +# +# SPDX-License-Identifier: LicenseRef-Nordic-5-Clause +# + +cmake_minimum_required(VERSION 3.20.0) + +find_package(Zephyr REQUIRED HINTS $ENV{ZEPHYR_BASE}) +project(hci_cmd_cb_host_buffer_size_test) + +# Generate runner for the test +test_runner_generate(src/hci_cmd_cb_host_buffer_size_test.c) + +# Add Unit Under Test source files +target_sources(app PRIVATE ${ZEPHYR_NRF_MODULE_DIR}/subsys/bluetooth/controller/hci_internal_wrappers.c) + +# Add test source file +target_sources(app PRIVATE src/hci_cmd_cb_host_buffer_size_test.c) + +# Create mocks +cmock_handle(${ZEPHYR_NRFXLIB_MODULE_DIR}/softdevice_controller/include/sdc_hci_cmd_controller_baseband.h) + +# Include paths +include_directories(${ZEPHYR_HAL_NORDIC_MODULE_DIR}/nrfx) +include_directories(${ZEPHYR_NRF_MODULE_DIR}/subsys/bluetooth/controller) diff --git a/tests/subsys/bluetooth/controller/prj.conf b/tests/subsys/bluetooth/controller/prj.conf new file mode 100644 index 000000000000..8c317c5c4c56 --- /dev/null +++ b/tests/subsys/bluetooth/controller/prj.conf @@ -0,0 +1,13 @@ +# +# Copyright (c) 2025 Nordic Semiconductor ASA +# +# SPDX-License-Identifier: LicenseRef-Nordic-5-Clause +# +CONFIG_BT_CENTRAL=y +CONFIG_BT_HCI=y +CONFIG_BT=y + +CONFIG_BT_HCI_ACL_FLOW_CONTROL=y +CONFIG_BT_BUF_CMD_TX_COUNT=10 + +CONFIG_UNITY=y diff --git a/tests/subsys/bluetooth/controller/src/hci_cmd_cb_host_buffer_size_test.c b/tests/subsys/bluetooth/controller/src/hci_cmd_cb_host_buffer_size_test.c new file mode 100644 index 000000000000..bc727162e6c2 --- /dev/null +++ b/tests/subsys/bluetooth/controller/src/hci_cmd_cb_host_buffer_size_test.c @@ -0,0 +1,60 @@ +/* + * Copyright (c) 2025 Nordic Semiconductor ASA + * + * SPDX-License-Identifier: LicenseRef-Nordic-5-Clause + */ +#include +#include +#include +#include +#include + +#include "hci_internal_wrappers.h" +#include "cmock_sdc_hci_cmd_controller_baseband.h" + +#define TEST_HOST_ACL_DATA_PCKT_LNGTH 55 +#define TEST_HOST_SYNC_DATA_PCKT_LNGTH 55 +#define TEST_HOST_NUM_SYNC_DATA_PCKTS 5555 + +/* The unity_main is not declared in any header file. It is only defined in the generated test + * runner because of ncs' unity configuration. It is therefore declared here to avoid a compiler + * warning. + */ +extern int unity_main(void); + +typedef struct { + uint16_t acl_input_pckts; + uint16_t exp_acl_pckts; +} test_vector_t; + +static const test_vector_t test_vectors[] = { + {5, 5}, /* Input within range, no adjustment needed */ + {15, (CONFIG_BT_BUF_CMD_TX_COUNT - 1)}, /* Input exceeds TX buffer count - 1 limit */ + {0xFF, (CONFIG_BT_BUF_CMD_TX_COUNT - 1)}, /* Maximum input value, corner case */ +}; + +void test_sdc_hci_cmd_cb_host_buffer_size_wrapper(void) +{ + sdc_hci_cmd_cb_host_buffer_size_t cmd_params; + cmd_params.host_acl_data_packet_length = TEST_HOST_ACL_DATA_PCKT_LNGTH; + cmd_params.host_sync_data_packet_length = TEST_HOST_SYNC_DATA_PCKT_LNGTH; + cmd_params.host_total_num_sync_data_packets = TEST_HOST_NUM_SYNC_DATA_PCKTS; + + for (size_t i = 0; i < ARRAY_SIZE(test_vectors); i++) { + const test_vector_t *v = &test_vectors[i]; + + sdc_hci_cmd_cb_host_buffer_size_t exp_cmd_params = cmd_params; + exp_cmd_params.host_total_num_acl_data_packets = v->exp_acl_pckts; + __cmock_sdc_hci_cmd_cb_host_buffer_size_ExpectAndReturn(&exp_cmd_params, 0); + + cmd_params.host_total_num_acl_data_packets = v->acl_input_pckts; + sdc_hci_cmd_cb_host_buffer_size_wrapper(&cmd_params); + } +} + +int main(void) +{ + (void)unity_main(); + + return 0; +} diff --git a/tests/subsys/bluetooth/controller/testcase.yaml b/tests/subsys/bluetooth/controller/testcase.yaml new file mode 100644 index 000000000000..b6796c77bcc3 --- /dev/null +++ b/tests/subsys/bluetooth/controller/testcase.yaml @@ -0,0 +1,8 @@ +tests: + bluetooth.controller: + platform_allow: native_sim + integration_platforms: + - native_sim + tags: + - unittest + - ci_tests_subsys_bluetooth_controller