Skip to content

bluetooth: host: Add extended feature set support #92856

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

Conversation

sean-madigan
Copy link
Contributor

@sean-madigan sean-madigan commented Jul 8, 2025

bluetooth: host: Add support for extended feature set feature

This commit adds support for the extended feature set
feature. This includes:

  • hci boilerplate
  • kconfigs, including one for a variable sized feature array
  • linking into the current auto feature exchange procedure

@sean-madigan sean-madigan force-pushed the add_extended_feature_set_support branch from aec05a5 to 85d2316 Compare July 9, 2025 07:47
@sean-madigan sean-madigan changed the title Add extended feature set support bluetooth: host: Add extended feature set support Jul 9, 2025
@sean-madigan sean-madigan force-pushed the add_extended_feature_set_support branch 5 times, most recently from f498fee to c51a877 Compare July 9, 2025 11:38
@sean-madigan sean-madigan marked this pull request as ready for review July 9, 2025 12:18

/* Read Low Energy Supported Features */
if (IS_ENABLED(CONFIG_BT_LE_EXTENDED_FEAT_SET)) {
err = bt_hci_cmd_send_sync(BT_HCI_OP_LE_READ_ALL_LOCAL_SUPPORTED_FEATURES, NULL,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is a good point. If CONFIG_BT_LE_EXTENDED_FEAT_SET=y in the host only, then I think we will actually fail reading any locally supported features.

I think Lyle makes a good argument to read the controller supported commands first

Comment on lines 1022 to 1033
bt_shell_fprintf_print(
"Read all remote features complete, Max Remote Page %d, LE Features: 0x",
params->max_remote_page);

for (uint8_t i = number_of_bytes; i > 0; i--) {
uint8_t features = params->features[i];
char features_str[(2 * sizeof(uint8_t)) + 1];

bin2hex(&features, sizeof(features), features_str, sizeof(features_str));
bt_shell_fprintf_print("%s", features_str);
}
bt_shell_fprintf_print("\n");
Copy link
Contributor

Choose a reason for hiding this comment

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

How does this look in the terminal? (Just curious)

I guess it'd be too demanding to convert these values to string / human readable values? :D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like this currently

uart:~$ bt read-all-remote-features 10
Read all remote features complete, Max Remote Page 1, LE Features: 0x0000000000000000000000000000000000000000000000010080000127f70049

@sean-madigan sean-madigan force-pushed the add_extended_feature_set_support branch from d1b017d to dfa9608 Compare July 11, 2025 08:45
@sean-madigan sean-madigan requested a review from Thalley July 14, 2025 14:49
@alwa-nordic alwa-nordic added this to the v4.3.0 milestone Jul 15, 2025
Thalley
Thalley previously approved these changes Jul 15, 2025
@Thalley Thalley requested a review from Copilot July 15, 2025 09:46
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for Bluetooth 6.0's Extended Feature Set feature to the Zephyr Bluetooth host stack. This allows the host to read extended feature pages from both local and remote devices beyond the standard page 0 features.

  • Implements HCI commands for reading all local and remote supported features with extended pages
  • Adds Kconfig options for enabling the feature and configuring maximum feature pages
  • Integrates extended feature exchange into existing connection procedures and shell commands

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
tests/bluetooth/shell/testcase.yaml Adds test configuration for extended feature set
subsys/bluetooth/host/shell/bt.c Implements shell command and callback for reading all remote features
subsys/bluetooth/host/hci_core.c Adds HCI event handling and command logic for extended features
subsys/bluetooth/host/conn_internal.h Declares notification function for extended feature complete event
subsys/bluetooth/host/conn.c Implements connection API for reading all remote features and notification logic
subsys/bluetooth/controller/Kconfig Adds controller-side Kconfig for extended feature set support
subsys/bluetooth/common/Kconfig Updates HCI event buffer size for extended feature set
subsys/bluetooth/Kconfig Adds host-side Kconfig options for extended feature set
include/zephyr/bluetooth/hci_types.h Defines HCI structures and constants for extended feature commands/events
include/zephyr/bluetooth/conn.h Adds connection callback and API for extended feature set
include/zephyr/bluetooth/bluetooth.h Updates local features structure to support variable-sized feature arrays
Comments suppressed due to low confidence (1)

subsys/bluetooth/host/shell/bt.c:1343

  • The error message references 'bt_hci_le_read_all_remote_features' but the actual function called is 'bt_conn_le_read_all_remote_features'. The error message should match the function name.
	.biginfo = per_adv_sync_biginfo_cb,

* See Bluetooth Core Specification, Vol 6, Part B, Section 4.6.
*/
uint8_t features[BT_LE_LOCAL_SUPPORTED_FEATURES_SIZE];
const uint8_t *features;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is a breaking API change. Please keep it as an array for now, and if there a significant optimization to be done here, we can open a separate PR to discuss this.

Otherwise, we need to add a point to the migration guide and document a lifetime for this pointer.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was wondering the same.

From a user perspective, if it's a pointer or an array, doesn't really matter that much does it?
The addition of const may be considered breaking if the array was modified by users.

It's a shame we didn't manage to get this PR merged for 4.2, as this was added in 4.2 as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll take this in a separate pr

Copy link
Contributor

Choose a reason for hiding this comment

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

From a user perspective, if it's a pointer or an array, doesn't really matter that much does it? The addition of const may be considered breaking if the array was modified by users.

  • const is indeed breaking, but I don't expect users would write to this anyway.
  • sizeof the field is no longer tied to BT_LE_LOCAL_SUPPORTED_FEATURES_SIZE. This one is a bit more insidious since it change the meaning of programs without causing compile errors. But maybe this is not such a big deal since you would use atomic_get_bit on this usually. 1.
  • What is the lifetime?, i.e. at what point can this value change or become invalid? It's probably valid until bt_disable, right? Then that has to be documented. And even if it's not a problem in practice (though how sure are you about this?), it's clearly more awkward and complex than a simple pass-by-value copy.

Footnotes

  1. This is a good example of why it would be good to specify the accepted interface for interacting with an object. If it was stated that this field can only be accessed as atomic_get_bit((bt_le_local_features rvalue).features, 0), then both const and sizeof changes would be non-breaking.

Thalley
Thalley previously approved these changes Jul 15, 2025
jhedberg
jhedberg previously approved these changes Jul 16, 2025
This commit adds support for the extended feature set
feature. This includes:
- hci boilerplate
- kconfigs, including one for a max local feature page
- reading remote features is done by a command and callback
- this is not linked into the auto feature request on
connection as this procedure can take quite a few connection
events, and we do not want to delay the user
- added the commands to the bt shell

Signed-off-by: Sean Madigan <[email protected]>
@sean-madigan sean-madigan force-pushed the add_extended_feature_set_support branch from e874798 to d485548 Compare July 16, 2025 14:54
@sean-madigan sean-madigan requested review from jhedberg and Thalley July 16, 2025 15:12
Copy link

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.

QUICK-ACK. As atleast one of the maintainer/assignee has approved, and no change requests have been requested.

(Added myself as assignee to unblock the merge-list process; future disagreements can be taken up as new PR, if any).

@cvinayak cvinayak self-assigned this Jul 28, 2025
@cfriedt cfriedt merged commit 3adae8b into zephyrproject-rtos:main Jul 28, 2025
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants