Skip to content

Conversation

@theob-pro
Copy link
Contributor

@theob-pro theob-pro commented Mar 17, 2025

Cherry-pick Bluetooth Host commits

Copy link
Contributor

@alwa-nordic alwa-nordic left a comment

Choose a reason for hiding this comment

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

lgtm

@theob-pro theob-pro force-pushed the bring-bt-host-changes branch from ba3abbc to e098314 Compare March 18, 2025 06:59
@theob-pro theob-pro force-pushed the bring-bt-host-changes branch from e098314 to c18a0e4 Compare March 20, 2025 08:47
PavelVPV and others added 20 commits March 20, 2025 13:33
…eration fails

When `bt_le_oob_get_local` or `bt_le_ext_adv_oob_get_local` is called
and SMP is enabled, `bt_smp_le_oob_generate_sc_data` is called to
generate a Random Number and a Confirmation Value needed for OOB data.
These values are based on the device's public key.

The public key is generated only once when `bt_smp_init` is called.
If public key generation fails, the callback passed to `bt_pub_key_get`
is called with `pkey` set to NULL. The `bt_smp_pkey_ready` callback
gets called, but it doesn't release the `sc_local_pkey_ready` semaphore
thus leaving `bt_smp_le_oob_generate_sc_data` wait for semaphore with
`K_FOREVER`.

This commit replaces the semaphore with a conditional variable and
requests a public key again if the key is NULL thus solving 2 issues:
- handling the case where the callback was triggered notifying about the
  completion of the public key request, but the key was not generated,
- handling the case where multiple threads trying to acquire the same
  sempahore.

The timeout is used instead of K_FOREVER to avoid cases when callback
has never been triggered.

Signed-off-by: Pavel Vasilyev <[email protected]>
(cherry picked from commit 9757ffa)
…_WHITELIST macro

Macro has been deprecated since 2.7 LTS(!) and is using non-inclusive
terminology. Drop it.

Signed-off-by: Benjamin Cabé <[email protected]>
(cherry picked from commit 110c49f)
When scanning is started on bt_bap_broadcast_assistant_scan_start then
BAP_BA_FLAG_SCANNING flag for the broadcast assistant instance should
not be cleared.

Fixes #85937

Signed-off-by: Jens Rehhoff Thomsen <[email protected]>
(cherry picked from commit c443343)
…nd buffer

Remove nested allocation of HCI command buffer so that one
less HCI command buffer is needed starting extended
advertising with privacy enabled.

Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
(cherry picked from commit cc1b534)
The gatt docs are very sparce, this commit adds imrpovements for the
GATT server API part. Others will follow.

Signed-off-by: Kyra Lengfeld <[email protected]>
(cherry picked from commit 775cc35)
… BT RX thread

This commit alignes the timeout value for allocating buffers within att
on the BT RX thread, making it consistent within att.c, see
bt_att_req_alloc.

We are inferring in many bt_gatt_* functions that if called from a BT RX
thread (which is inherently the case if called from a callback when
running a Bluetooth application), we don't block and instead return
-ENOMEM when the ATT request queue is full, avoiding a deadlock.
This promise is fulfilled within bt_att_req_alloc, where the timeout for
allocation of the request slab is set to K_NO_WAIT if we are on the BT
RX thread. Unfortunately, we break this promise in
bt_att_chan_create_pdu, where the timeout for allocation of the att pool
is still K_FOREVER and deadlocks can (and do) occur when too many
requests are sent yet the pool is depleted.

Note: Both req_slab and att_pool sizes are defined by
CONFIG_BT_ATT_TX_COUNT. If applications start getting -ENOMEM with this
change, they were at risk of such a deadlock, and may increase
CONFIG_BT_ATT_TX_COUNT to allocate the att pool for their requests.

Note: This possible deadlock has been flying under the radar, as
att_pools are freed when the HCI driver has sent it to the controller
(instead of when receiving the response, as it happens with req_slabs)
and due to the att_pool and the req_slab being both sized by
CONFIG_BT_ATT_TX_COUNT, and req_slab being allocated before and
returning -ENOMEM already if there is no space, it takes a more specific
situation to deplete the att_pool but not the req_slab pool at this
point.

Note: Ideally, we don't want functions to behave differently depending
on which thread they are running, and while this commit makes it more
consistent, it should be considered a workaround solution.

Signed-off-by: Kyra Lengfeld <[email protected]>
(cherry picked from commit 6464ffa)
…_uuid

bt_gatt_find_by_uuid used bt_gatt_attr_value_handle but that
function only works to get the value handle of a characteristic
declaration, i.e. if the UUID is not BT_UUID_GATT_CHRC then it
would always return handle = 0. This meant that
bt_gatt_find_by_uuid would always use handle = 0 as the starting
handle for non-BT_UUID_GATT_CHRC attributes, instead of the handle
of the provided attr.

This was not an issue for any UUIDs that may only exist once on a
GATT server, which is most UUIDs, but for UUIDs like the
BT_UUID_TBS_* UUIDs that may be multiple instances of, it would
always return the first attribute rather than the one starting
from the provided start attr.

This commit also ensures that we do not overflow the `end_handle`
when adding 2 uint16_t values.

Signed-off-by: Emil Gydesen <[email protected]>
(cherry picked from commit e4c5bb9)
Adds improvement to the GATT API documentation by providing examples of
where (and how) certain structs are used, and adds specification
references to the fields of bt_gatt_cpf to clarify their purpose.

Signed-off-by: Håvard Reierstad <[email protected]>
(cherry picked from commit c7a19da)
This commit adds improvements for the
GATT client API part.

Signed-off-by: Kyra Lengfeld <[email protected]>
(cherry picked from commit b136f6d)
…tures

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]>
(cherry picked from commit 44c5c1d)
When allocation fails in `hci_rx_cb`, we should return immediately
instead of continuing the execution of the function.

Also, we need to free the allocated buffer from the heap when sending
to the message queue fails to avoid memory leak.

Signed-off-by: Axel Le Bourhis <[email protected]>
(cherry picked from commit 0034e12)
…mplete event

As the previous version of the MCXW71 BLE Controller wasn't sending a
command complete event after setting the BD address, we used a
workaround to directly send the HCI command without using zephyr's API.
Now, on the latest version of MCXW72 and MCXW71, this issue is fixed,
so we need to use `bt_hci_cmd_send_sync` to properly expect the
command complete event.

Signed-off-by: Axel Le Bourhis <[email protected]>
(cherry picked from commit e5a1c0b)
…when bt disable

remove the power off BLE controller function since the hci_reset cmd
is enough, also for wifi-ble coex scenario, need to maintain BLE controller
for some info in SMU region

Signed-off-by: Ying Zhang <[email protected]>
(cherry picked from commit 6b16d10)
- added HCI command to send hci-reset in bt_disable() func, and
  remove its call from the driver close() call.
- remove rsp buf pass to 'hci_reset_complete()' func as status is
  already checked under 'bt_hci_cmd_send_sync()'.

Signed-off-by: Nirav Agrawal <[email protected]>
(cherry picked from commit 438701f)
Clarifying `bt_gatt_attr` docs.

Signed-off-by: Kyra Lengfeld <[email protected]>
(cherry picked from commit c31b962)
This commit adds more detailed description to the "Bluetooth APIs" and
"GAP" group within the bluetooth header.

Signed-off-by: Kyra Lengfeld <[email protected]>
(cherry picked from commit 3570783)
This commit changes log level for already registered callback from
warning to debug as this is cause unnecessary noise and doesn't indicate
actual issue. Caller gets a certain error code for this case which can
be handled properly.

Signed-off-by: Pavel Vasilyev <[email protected]>
(cherry picked from commit 886faba)
The api has not changed since its introduction some time ago,
mark it as stable.

Signed-off-by: Herman Berget <[email protected]>
(cherry picked from commit 8b909b4)
…ection Subrating

The API has not changed since it was introduced so should no longer be
considered experimental.

Signed-off-by: Timothy Keys <[email protected]>
(cherry picked from commit 7ef8116)
Fix the following warning
att.c:734:3: warning: label followed by a declaration is a C23 extension
[-Wc23-extensions]
  734 |                 k_tid_t current_thread = k_current_get();
      |                 ^
By wrapping that code as a compound statement

Signed-off-by: Alberto Escolar Piedras <[email protected]>
(cherry picked from commit 1dab77b)
@theob-pro theob-pro force-pushed the bring-bt-host-changes branch from c18a0e4 to f0f398b Compare March 20, 2025 12:33
@sonarqubecloud
Copy link

@nordicjm nordicjm merged commit e62a931 into nrfconnect:main Mar 24, 2025
18 checks passed
@theob-pro theob-pro deleted the bring-bt-host-changes branch March 26, 2025 06:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.