Bluetooth: Host: Add support for getting controller values in public API #86092

Thalley · 2025-02-20T14:08:54Z

Adds a new API to get LE controller info.
Adds a test and a use of the new API

fixes #81872

jhedberg

Looks pretty good

rugeGerritsen

Sounds like a good idea to expose such info!
Added some suggestions and questions

rugeGerritsen · 2025-02-21T09:04:57Z

doc/releases/release-notes-4.2.rst

+
+  * Host
+
+    * :c:func:`bt_le_get_local_features`


I know it is possible to click on the link, but it would still be nice to add a short description of what it does :)

Suggested change

* :c:func:`bt_le_get_local_features`

* :c:func:`bt_le_get_local_features` returns the feature set supported by the Bluetooth Controller.

Normally I'd do that too, but the text right above says:

Link to new APIs here, in a group if you think it's necessary, no need to get
fancy just list the link, that should contain the documentation. If you feel
like you need to add more details, add them in the API documentation code
instead.

I read that as we should only provide links here and then the documentation of the added function should provide the rest

rugeGerritsen · 2025-02-21T09:05:19Z

include/zephyr/bluetooth/bluetooth.h

+ *
+ * The value of 8 correspond to page 0 in the LE Controller supported features
+ */
+#define BT_LE_LOCAL_SUPPOTRED_FEATURES_SIZE 8


Suggested change

#define BT_LE_LOCAL_SUPPOTRED_FEATURES_SIZE 8

#define BT_LE_LOCAL_SUPPORTED_FEATURES_SIZE 8

rugeGerritsen · 2025-02-21T09:05:24Z

include/zephyr/bluetooth/bluetooth.h

@@ -48,6 +49,13 @@ extern "C" {
 */
 #define BT_ID_DEFAULT 0

+/**
+ * @brief Number of octets for local supporte


Suggested change

* @brief Number of octets for local supporte

* @brief Number of octets for local supported

rugeGerritsen · 2025-02-21T09:06:31Z

include/zephyr/bluetooth/bluetooth.h

+ *
+ * The value of 8 correspond to page 0 in the LE Controller supported features
+ */
+#define BT_LE_LOCAL_SUPPOTRED_FEATURES_SIZE 8


Do we need this define in the first place or can we hardcode it in the definition of struct bt_le_local_features. If we hardcode it there, we reduce the amount of documentation needed.

We could consider adding ifdefs in the struct definition to remove entries which are not needed.

Yeah I didn't write that in the commit message.

The reason why I've added it here, is that today the Zephyr only supports page 0, but there are actually more than 1 page now.

Once we add support for reading all pages, then it would be trivial to simply increase this from e.g. 8 to 16.

If that doesn't really make sense, let me know :)

Hmm, perhaps it should be an internal Kconfig option instead?

I was wondering if we need an entry at all: uint8_t features[8];. A Kconfig option is probably not needed

I just really dislike magic numbers in the public API like 8 would be :D (and especially when it should fit the same size of a different struct's field)

Could add a define for PAGE_SIZE in hci_types.h, but that doesn't solve anything either.

I see that it is also used inside hci_core.h, so it is probably a good idea to keep it as is

rugeGerritsen · 2025-02-21T09:10:05Z

include/zephyr/bluetooth/bluetooth.h

+	 *
+	 * Refer to BT_LE_FEAT_BIT_* for values.
+	 * Refer to the BT_FEAT_LE_* macros for value comparionson.
+	 * See Bluetooth Core Specification 6.0, Vol 6, Part B, Section 4.6 table 4.10


I believe we can leave out Core revision and Table number here. Older and newer revisions will have it in the same place:

Suggested change

* See Bluetooth Core Specification 6.0, Vol 6, Part B, Section 4.6 table 4.10

* See Bluetooth Core Specification, Vol 6, Part B, Section 4.6.

I have no strong preference on this.

You are probably correct. Looks like when sections are removed they are still kept and marked as "This section is no longer used", so perhaps we can consider section numbers static across versions.

rugeGerritsen · 2025-02-21T09:17:11Z

include/zephyr/bluetooth/bluetooth.h

+	/** Total number of ISO data packets */
+	uint8_t iso_pkts;
+
+	/** Size of the controller resolving list */


The actual size can be smaller depending on controller implementation

Suggested change

/** Size of the controller resolving list */

/** Maximum size of the controller resolving list.

*

* See Bluetooth Core Specification, Vol 6, Part E, Section 7.8.41.

*/

rugeGerritsen · 2025-02-21T09:17:41Z

include/zephyr/bluetooth/bluetooth.h

+	/** Number of entries in the resolving list.
+	 *
+	 * @ref bt_le_local_features.rl_entries > @ref bt_le_local_features.rl_size means that
+	 * host-side resolving is used.
+	 */
+	uint8_t rl_entries;


rl_entries is representing state of the Bluetooth stack and is not static. Suggest to leave it out.

rugeGerritsen · 2025-02-21T09:19:12Z

include/zephyr/bluetooth/bluetooth.h

+	/** Maximum advertising data length */
+	uint16_t max_adv_data_len;


Suggested change

/** Maximum advertising data length */

uint16_t max_adv_data_len;

/** Maximum advertising data length

*

* @note The maximum advertising data length also depends on advertising

* type.

* See Bluetooth Core Specification, Vol 6, Part E, Section 7.8.57.

*/

uint16_t max_adv_data_len;

rugeGerritsen · 2025-02-21T09:19:40Z

include/zephyr/bluetooth/bluetooth.h

@@ -517,6 +525,60 @@ size_t bt_data_get_len(const struct bt_data data[], size_t data_count);
 */
 size_t bt_data_serialize(const struct bt_data *input, uint8_t *output);

+struct bt_le_local_features {


Do we want to add Filter accept list size to this struct?

I think that would make sense, but the Zephyr host isn't reading that at the moment, so that would be a new feature

rugeGerritsen · 2025-02-21T09:20:43Z

include/zephyr/bluetooth/bluetooth.h

@@ -517,6 +525,60 @@ size_t bt_data_get_len(const struct bt_data data[], size_t data_count);
 */
 size_t bt_data_serialize(const struct bt_data *input, uint8_t *output);

+struct bt_le_local_features {


I assume we want to extend this struct whenever the spec defines a new feature set. Eventually this struct can become very large. Is this something we want to do?

It's a good question. Similar to the other info structs, it's only stack allocated so it wouldn't be taking up much memory, but might run into stack overflows in some cases I guess.

An alternative would be to do similar to what I did for ISO where the internal state uses the public state to store information: https://github.com/zephyrproject-rtos/zephyr/blob/main/subsys/bluetooth/host/conn_internal.h#L182-L183

Today we are still doing an allocation + copy, but we could in theory return a const pointer directly to that struct, which would effectively require no additional memory.

The function supports reading multiple controller-based values. It is effectively a copy of struct bt_dev_le but in a more application-oriented definition. It was chosen to keep the features as an array rather than a 64-bit value, as the comparison macros work on arrays and that there already exists new bits > 64 in the core spec which is not yet supported by Zephyr. It is being smoke tested in a generic GATT client test, as the individual values may depend on several Kconfig options. Signed-off-by: Emil Gydesen <[email protected]>

The connection object is, and should not, be modified by the function. Making it const makes it possible to do this simple operation in functions that also won't change the conn object (i.e. that uses `const`). Signed-off-by: Emil Gydesen <[email protected]>

Use the recently added bt_le_get_local_features function as well as bt_conn_get_remote_info to avoid using internal APIs to check for PAST supported. Signed-off-by: Emil Gydesen <[email protected]>

Thalley mentioned this pull request Feb 20, 2025

Bluetooth: Host: Add API functions to determine PAST support for conn #85597

Closed

Thalley marked this pull request as ready for review February 21, 2025 09:01

zephyrbot added Release Notes To be mentioned in the release notes area: Bluetooth Host Bluetooth Host (excluding BR/EDR) area: Bluetooth area: Bluetooth Audio labels Feb 21, 2025

zephyrbot requested review from alwa-nordic, asbjornsabo, Casper-Bonde-Bose, cvinayak, fredrikdanebjer, hermabe, jhedberg, jthm-ot, larsgk, MariuszSkamra, niym-ot, pin-zephyr, rugeGerritsen, sjanc and theob-pro February 21, 2025 09:02

zephyrbot assigned jhedberg and alwa-nordic Feb 21, 2025

jhedberg previously approved these changes Feb 21, 2025

View reviewed changes

rugeGerritsen requested changes Feb 21, 2025

View reviewed changes

Thalley dismissed jhedberg’s stale review via e965ad3 February 21, 2025 10:42

Thalley force-pushed the local_supported_features_api branch 2 times, most recently from e965ad3 to cb40744 Compare February 21, 2025 10:44

Thalley requested review from jhedberg and rugeGerritsen February 21, 2025 10:44

Thalley force-pushed the local_supported_features_api branch from cb40744 to 38500c6 Compare February 21, 2025 10:48

rugeGerritsen previously approved these changes Feb 21, 2025

View reviewed changes

Thalley added 3 commits February 21, 2025 12:29

Bluetooth: Audio: Modifed PAST checks to use public API

987b6f4

Use the recently added bt_le_get_local_features function as well as bt_conn_get_remote_info to avoid using internal APIs to check for PAST supported. Signed-off-by: Emil Gydesen <[email protected]>

Thalley dismissed rugeGerritsen’s stale review via 987b6f4 February 21, 2025 11:29

Thalley force-pushed the local_supported_features_api branch from 38500c6 to 987b6f4 Compare February 21, 2025 11:29

rugeGerritsen approved these changes Feb 21, 2025

View reviewed changes

jhedberg approved these changes Feb 21, 2025

View reviewed changes

fabiobaltieri added this to the v4.1.0 milestone Feb 21, 2025

jhedberg modified the milestones: v4.1.0, v4.2.0 Feb 21, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bluetooth: Host: Add support for getting controller values in public API #86092

Bluetooth: Host: Add support for getting controller values in public API #86092

Thalley commented Feb 20, 2025

jhedberg left a comment

rugeGerritsen left a comment

rugeGerritsen Feb 21, 2025

Thalley Feb 21, 2025

rugeGerritsen Feb 21, 2025

rugeGerritsen Feb 21, 2025

rugeGerritsen Feb 21, 2025

Thalley Feb 21, 2025

Thalley Feb 21, 2025

rugeGerritsen Feb 21, 2025

Thalley Feb 21, 2025

rugeGerritsen Feb 21, 2025

rugeGerritsen Feb 21, 2025

Thalley Feb 21, 2025

rugeGerritsen Feb 21, 2025

rugeGerritsen Feb 21, 2025

rugeGerritsen Feb 21, 2025

rugeGerritsen Feb 21, 2025

Thalley Feb 21, 2025

rugeGerritsen Feb 21, 2025

Thalley Feb 21, 2025

	* :c:func:`bt_le_get_local_features`
	* :c:func:`bt_le_get_local_features` returns the feature set supported by the Bluetooth Controller.

	#define BT_LE_LOCAL_SUPPOTRED_FEATURES_SIZE 8
	#define BT_LE_LOCAL_SUPPORTED_FEATURES_SIZE 8

	* @brief Number of octets for local supporte
	* @brief Number of octets for local supported

	* See Bluetooth Core Specification 6.0, Vol 6, Part B, Section 4.6 table 4.10
	* See Bluetooth Core Specification, Vol 6, Part B, Section 4.6.

-	/** Size of the controller resolving list */
+	/** Maximum size of the controller resolving list.
+	  *
+	  * See Bluetooth Core Specification, Vol 6, Part E, Section 7.8.41.
+	  */

		/** Maximum advertising data length */
		uint16_t max_adv_data_len;

Bluetooth: Host: Add support for getting controller values in public API #86092

Are you sure you want to change the base?

Bluetooth: Host: Add support for getting controller values in public API #86092

Conversation

Thalley commented Feb 20, 2025

jhedberg left a comment

Choose a reason for hiding this comment

rugeGerritsen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment