Skip to content

Conversation

@rettichschnidi
Copy link
Contributor

This allows to build the shell with BT_CTLR_DTM and/or BT_CTLR_ADV_EXT enabled.

The issues has been introduced by commit
bf897cf (Bluetooth: Shell: Restructure shell files).

Question:

  • For those two settings, would it be okay (tolerated? appreciated?) to add an an extra build in CI "just" to test whether enabling those flags does not break the build?

Copy link
Contributor

@cvinayak cvinayak left a comment

Choose a reason for hiding this comment

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

Question:

* For those two settings, would it be okay (tolerated? appreciated?) to add an an extra build in CI "just" to test whether enabling those flags does not break the build?

Please add a new tests target in testcases.yaml file with added extra_configs with CONFIG_BT_CTLR_DTM for nrf52840dk/nrf52840 board.

Using which board did you discover the issue?

@cvinayak cvinayak added bug The issue is a bug, or the PR is fixing a bug Regression Something, which was working, does not anymore labels Oct 21, 2024
@cvinayak cvinayak requested a review from Thalley October 21, 2024 04:11
@rettichschnidi
Copy link
Contributor Author

rettichschnidi commented Oct 21, 2024

Using which board did you discover the issue?

An nRF52840 (or maybe nRF54L15) based out-of-tree board.

@rettichschnidi
Copy link
Contributor Author

Please add a new tests target in testcases.yaml file with added extra_configs with CONFIG_BT_CTLR_DTM for nrf52840dk/nrf52840 board.

Any proposal on which testcases.yaml is should add this to? There are many:

$ find tests/bluetooth/controller/ -name testcase.yaml
tests/bluetooth/controller/ctrl_api/testcase.yaml
tests/bluetooth/controller/ctrl_chmu/testcase.yaml
tests/bluetooth/controller/ctrl_cis_create/testcase.yaml
tests/bluetooth/controller/ctrl_cis_terminate/testcase.yaml
tests/bluetooth/controller/ctrl_collision/testcase.yaml
tests/bluetooth/controller/ctrl_conn_update/testcase.yaml
tests/bluetooth/controller/ctrl_cte_req/testcase.yaml
tests/bluetooth/controller/ctrl_data_length_update/testcase.yaml
tests/bluetooth/controller/ctrl_encrypt/testcase.yaml
tests/bluetooth/controller/ctrl_feature_exchange/testcase.yaml
tests/bluetooth/controller/ctrl_hci/testcase.yaml
tests/bluetooth/controller/ctrl_invalid/testcase.yaml
tests/bluetooth/controller/ctrl_le_ping/testcase.yaml
tests/bluetooth/controller/ctrl_min_used_chans/testcase.yaml
tests/bluetooth/controller/ctrl_phy_update/testcase.yaml
tests/bluetooth/controller/ctrl_terminate/testcase.yaml
tests/bluetooth/controller/ctrl_tx_buffer_alloc/testcase.yaml
tests/bluetooth/controller/ctrl_tx_queue/testcase.yaml
tests/bluetooth/controller/ctrl_unsupported/testcase.yaml
tests/bluetooth/controller/ctrl_version/testcase.yaml
tests/bluetooth/controller/ctrl_sca_update/testcase.yaml
tests/bluetooth/controller/ctrl_isoal/testcase.yaml
tests/bluetooth/controller/ctrl_sw_privacy/testcase.yaml
tests/bluetooth/controller/ctrl_sw_privacy_unit/testcase.yaml
tests/bluetooth/controller/ctrl_user_ext/testcase.yaml
tests/bluetooth/controller/ll_settings/testcase.yaml

Or did you mean to add a directory tests/boards/nrf/bluetooth and put a new testcase.yaml in it?

@cvinayak
Copy link
Contributor

Please add a new tests target in testcases.yaml file with added extra_configs with CONFIG_BT_CTLR_DTM for nrf52840dk/nrf52840 board.

Any proposal on which testcases.yaml is should add this to? There are many:

$ find tests/bluetooth/controller/ -name testcase.yaml
tests/bluetooth/controller/ctrl_api/testcase.yaml
tests/bluetooth/controller/ctrl_chmu/testcase.yaml
tests/bluetooth/controller/ctrl_cis_create/testcase.yaml
tests/bluetooth/controller/ctrl_cis_terminate/testcase.yaml
tests/bluetooth/controller/ctrl_collision/testcase.yaml
tests/bluetooth/controller/ctrl_conn_update/testcase.yaml
tests/bluetooth/controller/ctrl_cte_req/testcase.yaml
tests/bluetooth/controller/ctrl_data_length_update/testcase.yaml
tests/bluetooth/controller/ctrl_encrypt/testcase.yaml
tests/bluetooth/controller/ctrl_feature_exchange/testcase.yaml
tests/bluetooth/controller/ctrl_hci/testcase.yaml
tests/bluetooth/controller/ctrl_invalid/testcase.yaml
tests/bluetooth/controller/ctrl_le_ping/testcase.yaml
tests/bluetooth/controller/ctrl_min_used_chans/testcase.yaml
tests/bluetooth/controller/ctrl_phy_update/testcase.yaml
tests/bluetooth/controller/ctrl_terminate/testcase.yaml
tests/bluetooth/controller/ctrl_tx_buffer_alloc/testcase.yaml
tests/bluetooth/controller/ctrl_tx_queue/testcase.yaml
tests/bluetooth/controller/ctrl_unsupported/testcase.yaml
tests/bluetooth/controller/ctrl_version/testcase.yaml
tests/bluetooth/controller/ctrl_sca_update/testcase.yaml
tests/bluetooth/controller/ctrl_isoal/testcase.yaml
tests/bluetooth/controller/ctrl_sw_privacy/testcase.yaml
tests/bluetooth/controller/ctrl_sw_privacy_unit/testcase.yaml
tests/bluetooth/controller/ctrl_user_ext/testcase.yaml
tests/bluetooth/controller/ll_settings/testcase.yaml

Or did you mean to add a directory tests/boards/nrf/bluetooth and put a new testcase.yaml in it?

This one: https://github.com/zephyrproject-rtos/zephyr/blob/9589e375ab7098a78b72121bb63c8d73d9d54095/tests/bluetooth/shell/testcase.yaml

@cvinayak cvinayak added this to the v4.0.0 milestone Oct 25, 2024
@cvinayak
Copy link
Contributor

@rettichschnidi is this something you will be able to follow up soon before the Zephyr v4.0 feature freeze or create GH issue to fix it in bug fix phase?

@rettichschnidi
Copy link
Contributor Author

rettichschnidi commented Oct 25, 2024

is this something you will be able to follow up soon

I'll take care of it today. I am a bit confused however due to your proposed changes - they essentially revert some of my patch. Need to investigate, likely an overlook on my end.

This allows to build the shell with BT_CTLR_DTM and/or BT_CTLR_ADV_EXT
enabled.

The issues has been introduced by commit
bf897cf (Bluetooth: Shell: Restructure
shell files).

Signed-off-by: Reto Schneider <[email protected]>
This ensures that enabling BT_CTLR_DTM does not break the compilation of
the Bluetooth shell.

Signed-off-by: Reto Schneider <[email protected]>
@rettichschnidi rettichschnidi force-pushed the gardena/rs/upstream/fix-ble-controller-shell-include branch from 9589e37 to cf974e2 Compare October 25, 2024 14:08
@zephyrbot zephyrbot added the area: Bluetooth Host Bluetooth Host (excluding BR/EDR) label Oct 25, 2024
@dleach02 dleach02 merged commit 17d9e75 into zephyrproject-rtos:main Oct 26, 2024
28 checks passed
@rettichschnidi rettichschnidi deleted the gardena/rs/upstream/fix-ble-controller-shell-include branch October 28, 2024 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: Bluetooth Controller area: Bluetooth Host Bluetooth Host (excluding BR/EDR) area: Bluetooth bug The issue is a bug, or the PR is fixing a bug Regression Something, which was working, does not anymore

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants