Improve the robustness of RFCOMM flow control recovery by adding
session-level FCON retry logic and retry triggers for failed credit
and MSC operations.
Add `rfcomm_dlc_update_fc_on()` to check the flag and send FCON when
buffers become available, with proper error handling that retries
on failure.
Set `RFCOMM_SESSION_FLAG_FC_ON_REQ` in `rfcomm_dlc_connected()`. And
call `rfcomm_dlc_update_fc_on()` to send FCON.
Call `rfcomm_dlc_update_fc_on()` from `rfcomm_dlc_fc_check_and_on()` so
session-level flow control will be checked and processed in the DLC TX
processing.
Trigger TX processing at the end of `rfcomm_dlc_connected()` to ensure
the deferred FCON send happens promptly if the flag
`RFCOMM_SESSION_FLAG_FC_ON_REQ` is not cleared.
Add retry triggers in `rfcomm_dlc_fc_check_and_on()` and
`rfcomm_dlc_update_credits()` when send failures occur, ensuring the
state machine recovers quickly instead of waiting for the next DLC TX
event.
These changes refine the various handling mechanisms for buffer
allocation errors from the previous submission, ensuring that flow
control operations at both the session-level (FCON) and the DLC level
(MSC, credit-based) can recover when buffer allocation fails.
Signed-off-by: Lyle Zhu <lyle.zhu@nxp.com>
When rfcomm_send_msc() fails in rfcomm_dlc_connected(), set the
RFCOMM_FLAG_MSC_FC_CLEAR_REQ flag to ensure the MSC command will be
retried later instead of silently dropping the failure.
This complements the buffer allocation error handling introduced in the
previous commit by ensuring the flow control state machine can recover
when the initial MSC send fails due to buffer exhaustion.
Signed-off-by: Lyle Zhu <lyle.zhu@nxp.com>
Previously, the timeout `K_FOREVER` is used to allocate the net buffer
from the TX buffer pool. It causes the BT workqueue to be blocked by
the buffer allocation operations with unpredictable completion times,
preventing subsequent work handlers from executing.
Use `K_NO_WAIT` to replace `K_FOREVER`. It means that when the buffer
allocation function returns, there's no guarantee that a buffer has
been allocated. Therefore, add comprehensive error handling for
buffer allocation failures in the RFCOMM implementation.
- Add `rfcomm_create_pdu()` to wrap `bt_l2cap_create_pdu_timeout()`,
checking both the allocation result and available tailroom
before the buffer is used.
- Check the return value of every `rfcomm_send_*()` call and
propagate allocation failures up the call stack as -ENOBUFS.
- Handle send failures in the SABM, DISC, PN, UA, DM, MSC, RLS,
RPN, TEST, FCON, FCOFF and NSC paths, logging the error and, on
the disconnect paths, rescheduling the retransmission timer
immediately so the state machine still recovers instead of
waiting on the RTX timeout.
- Extract `rfcomm_dlc_connect()` and `rfcomm_dlc_disconn()` so the
connect/disconnect error handling lives in one place instead of
being duplicated at each call site.
The change prevents workqueue stalls and crashes when the L2CAP buffer
pool is exhausted.
Signed-off-by: Lyle Zhu <lyle.zhu@nxp.com>
Add a new API to remove a previously registered Service Record from
the SDP database. This complements bt_sdp_register_service() and
follows the same pattern as bt_l2cap_br_server_unregister().
The function checks for active SDP server L2CAP channels and returns
-EBUSY if any exist, preventing unsafe removal during active
transactions. A residual race window remains between the check and
the actual removal; the caller is responsible for ensuring no new
SDP connections are established during this call.
Signed-off-by: Kai Cheng <chengkai@xiaomi.com>
The num_services counter was only used in a single condition
(state.current_svc < num_services) within the SSA response
continuation state logic. Since record indices are assigned
sequentially from 0 and state.current_svc is always set from
record->index (which is always < num_services for any valid
record), this condition is always true — making it dead code.
Remove the variable, its increment in bt_sdp_register_service(),
and the redundant condition wrapper, unconditionally entering
dry_run mode when the packet is full.
Signed-off-by: Kai Cheng <chengkai@xiaomi.com>
Commit 1de08e0b6e ("bluetooth: mesh: dfu_cli, dfu_srv: accept SUCCESS
+ APPLYING as terminal") ended the Apply step as soon as a target
answered Success with phase Applying Update. MshDFUv1.0 Section 6.2.2.4
requires the Confirm step to wait until the targets have applied, and
that retry loop was the only thing implementing the wait. Without it the
client reads the old Current Firmware ID and Table 7.3 in Section
7.1.2.9 marks the target Apply Failed. A PTS test caught this.
Repeat Firmware Update Apply while a target reports Applying Update, as
Section 7.1.2.6 permits. Once the install completes the target answers
Wrong Phase with phase Idle, which the branch above already accepts.
Keep acknowledging the self-target: a Distributor that is also a Target
defers its own apply until the Confirm step completes, so it never
leaves Applying Update on its own.
Revert the accompanying dfu_srv change, which applied the firmware on an
unacknowledged response assuming the client would not retry. Reverting
the phase to Verification Succeeded on any send error is correct now
that it does: the retry is accepted as a fresh apply per Section
6.1.3.6.
Assisted-by: Claude:claude-opus-5
Signed-off-by: Aleksandr Khromykh <aleksandr.khromykh@nordicsemi.no>
The oacp_ind_handler only invoked the obj_checksum_calculated
callback when the indication contained a 32-bit checksum. Error
responses to BT_GATT_OTS_OACP_PROC_CHECKSUM_CALC omit the checksum,
so the callback was never called and the application had no way to
learn that the procedure failed.
Only parse the checksum when the result code is success, and always
invoke the callback with the result code, passing a checksum of 0 for
error responses. This matches the documented behavior of the
callback.
Assisted-by: Copilot:GPT-5.6
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
Previously, a single static `discov` variable was used to hold the
selected discovery parameters, which would be overwritten on each call.
This could lead to issues if multiple discovery operations were queued
or in progress.
The change makes `discov` a local pointer that references the
appropriate pre-defined discovery parameter structure based on the
input shell command parameters.
The change is used to align with the refactored SDP client queue-based
request handling and ensures each discovery request maintains its own
parameter context.
Signed-off-by: Lyle Zhu <lyle.zhu@nxp.com>
SDP clients use `sem_lock(k_sem_take(K_FOREVER))` to serialize access
to their request list and session state to avoid thread safety issues.
Because SDP client callbacks run on a shared, cooperative `bt_workq`,
blocking the semaphore from the `bt_workq` context could cause all
other items in the same work queue to stall, not just the SDP session
itself.
Replacing the semaphore-protected `sys_slist` pair (`reqs`/`reqs_next`)
with a single `k_queue reqs_pending` allows safe access from any
context without blocking. `bt_sdp_discover()` returns success
immediately after adding a new request to the `queue reqs_pending` and
submitting a new `sdp_client_worker`. Therefore, the original session
creation-related operations are moved to `sdp_client_work_handler()`
and its helper functions, which run only on `bt_workq`.
The `sdp_client_work_handler()` function iterates through all SDP
sessions after each request submission and performs one of the
following workflow:
- If a disconnection is in progress (`SDP_CLIENT_DISCONNECTING`),
skip the session;
- If the channel is connected but idle (`SDP_CLIENT_CONNECTED`, with
no active parameter, it means the `session->param` is `NULL`), resume
iterating through pending requests;
- If the channel is disconnected (`SDP_CLIENT_DISCONNECTED`) and there
are pending requests, start a new L2CAP session;
- If the backend ACL connection is no longer active, clear
`reqs_pending` using `UUID_NOT_RESOLVED` and reset the session.
SDP Client session lifecycle changes:
- `SDP_CLIENT_RELEASED` has been removed; `SDP_CLIENT_DISCONNECTED` is
used to cover the same "not connected" state. The L2CAP `.released`
channel ops has also been removed because restarting after
disconnection is now driven by `sdp_client_work_handler()` instead
of the `.released` callback. Also, since the `sdp_client_discover()`
function requests an accept buffer, the L2CAP `.alloc_buf` channel
ops is no longer needed to simplify the process and avoid potential
`session->param` overwriting issues.
- A `BT_CONN_CB.disconnected` callback has been added to monitor ACL
disconnection events. When the ACL connection itself is disconnected,
all pending requests are cleared and the session is reset (previously
handled indirectly via `reqs_next` and `sdp_client_released()`).
- Take/release ACL conn reference by the function `bt_sdp_discover()`.
when adding a request to the `queue reqs_pending`, `bt_sdp_discover()`
takes the reference of the ACL connection and releases the reference
after the request is dequeued (cleared by processing of pending
request, `bt_sdp_discover_cancel()`, or when the ACL connection is
closed).
- Handle receiving buffer allocation failures without disconnecting the
session. Notify the SDP discovery result with the `NULL` response
buffer if the receive buffer allocation failed. And go to the next SDP
discovery procedure instead of disconnecting the session.
- Use the client worker to trigger the next SDP discovery procedure
to avoid recursive calls in the functions `sdp_client_discover()` and
`sdp_client_params_iterator()`.
Updated API doxygen comments for `bt_sdp_discover()` and
`bt_sdp_discover_cancel()` to reflect the new return semantics:
`bt_sdp_discover()` always returns 0 after the request is enqueued,
and the actual success/failure of the discovery is reported later via
`params->func()`.
Signed-off-by: Lyle Zhu <lyle.zhu@nxp.com>
The ACL fragment pool declares CONFIG_BT_CONN_TX_USER_DATA_SIZE bytes of
user data that nothing reads. The requirement came from bt_buf_set_type()
and went away with 26d97164be, so the TODO next to it is stale.
Size it zero and keep the view metadata in the side table: an HCI driver
may use a sent fragment's user data, so metadata that frag_destroy()
dereferences does not belong there.
Signed-off-by: Zhijie Zhong <zhongzhijie1@xiaomi.com>
Assisted-by: Kiro:claude-opus-5
The espressif hardware variant table stopped at the esp32h2,
so the two ids defined after it were printed as "unknown".
Add both names.
Signed-off-by: Sylvio Alves <sylvio.alves@espressif.com>
Cross-Transport Key Derivation is unconditional today. With
CONFIG_BT_CLASSIC enabled, Secure Connections pairing over LE always
asks for a derived BR/EDR link key, and the Security Manager over
BR/EDR always accepts deriving the LE LTK from the link key. A device
that must not end up with a bond on the other transport as a side
effect of pairing has no way to say so. Both decisions are device
global: the responder negotiates the key distribution before
bt_conn_set_security() is ever reached, and the fixed channel mask is
built during ACL setup, so Kconfig rather than an API.
Add BT_SMP_DERIVE_LTK and BT_SMP_DERIVE_LK, both default y, so
behaviour is unchanged unless a build opts out.
Without BT_SMP_DERIVE_LK, LINK_DIST expands to 0. That keeps the
negotiated key distribution and what goes on air consistent in both
roles without touching the PDU construction sites, and
ltk_derive_link_key_allowed() refuses the derivation as well.
Without BT_SMP_DERIVE_LTK, smp_br_pairing_allowed() refuses the
procedure and the BR/EDR Security Manager fixed channel is left out of
the L2CAP information response, so a peer does not start one that would
then be rejected. The channel stays registered, so a Pairing Request
that arrives anyway still gets the 0x0E failure.
Signed-off-by: Zhijie Zhong <zhongzhijie1@xiaomi.com>
Assisted-by: Kiro:claude-opus-5
Build the HCI command packet prefix in bt_hci_cmd_send() with
bt_hci_pkt_push_cmd_hdr(), and decode Command Complete and Command
Status events in hci_cmd_complete() and hci_cmd_status() with the
corresponding pull helpers, instead of open-coding the same encoding
and decoding.
Besides removing the duplication, this exercises the helpers with the
Host's own command traffic, and it settles two edge cases. A Command
Complete whose return parameters lack the status they start with had
that status read from past the end of the event; the helper now reports
it with BT_HCI_ERR_UNSPECIFIED as the status, so the command fails and
its sender gets an error instead of the command timeout, while
Num_HCI_Command_Packets is still honoured and the command flow keeps
going. The Command Complete event with opcode 0x0000 (a
Num_HCI_Command_Packets update without return parameters) no longer has
a status byte read from past its parameters either.
bt_hci_cmd_send() now also releases the buffer when the header cannot
be pushed, as it does on its other failure paths; before, a buffer
rejected for insufficient headroom stayed referenced.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
Add hci_pkt.h, stateless helpers for framing HCI command packets and
parsing the events that carry command responses, operating on
net_buf_simple buffers and on packet bytes, implemented in
subsys/bluetooth/common and built into every Bluetooth build (the
library is static, so unused helpers cost nothing).
The helpers are independent of the Bluetooth Host and of the HCI driver
interface, and usable in any build type. The intended users are the
Host itself and HCI drivers that need to exchange HCI commands with the
controller over their own transport, typically for vendor-specific
controller initialization. Today such drivers either call Host command
APIs like bt_hci_cmd_send_sync(), which are unavailable in
controller-only builds, or duplicate the command construction with raw
buffers.
The helpers keep the Host's encoding conventions: parameters are built
and decoded with the packed structures of hci_types.h and the
net_buf_simple API, the encoded length is the buffer length, and the
header push derives the parameter length from it. BT_HCI_PKT_CMD_DEFINE()
provides a command buffer with headroom for the packet prefix reserved.
A Command Complete for a command whose return parameters lack the
status they start with is reported as -ENODATA with the rest decoded
and BT_HCI_ERR_UNSPECIFIED as the status, so that callers can fail the
command instead of guessing.
Add a unit test covering framing and parsing, including the Command
Complete event with opcode 0x0000, which carries no return parameters,
the status-less Command Complete and malformed events, building the
implementation file directly so that it stays independent of the
Bluetooth stack, plus the documentation page and release notes entry.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
The signalling response PDUs are allocated with K_FOREVER from the
AVDTP buffer pool. The command handlers run in the Bluetooth
workqueue, which is also the context that runs the AVDTP TX processing
that returns the buffers to that same pool, so waiting for a buffer
there cannot make progress. Most of the handlers wait with the
endpoint lock held, which additionally blocks any other user of the
endpoint.
The same applies to the reassembly buffer allocated on the receive
path, which already has an unreachable allocation failure path.
Allocate both without waiting. All the response allocations already
handle the failure by not responding, leaving the remote side to time
out, which is preferable to blocking the whole host.
Assisted-by: Claude:claude-opus-5
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
Rename the command to `bap stats` instead of `bap bap_stats`
and allow the value 0 to disable reporting.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
Change the LC3 priority to be a higher value (lower priority).
The purpose of this is to prioritize the USB thread higher
as that is more sensitive to time than the LC3 thread.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
If the client removes its subscription to any of the MCS
characteristics at any time, the pending appropriate flags
are now being cleared.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
Add a proper Media Proxy API for setting the local
Media Player Name and expose it through the media
player callback interface.
Signed-off-by: Alex Ciascai <alexandru.ciascai@nordicsemi.no>
It is possible to get the ISO bt_conn object from e.g.
bt_conn_foreach but it is pretty useless without
being able to get the `bt_iso_chan` that it is linked to.
Added function to get the bt_iso_chan object from the
bt_conn ISO object.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
SYS_SLIST_FOR_EACH_CONTAINER leaves the loop variable pointing at the
last node when the scan comes up empty, so avdtp_get_sep() returned that
node for a SEID the endpoint never registered. Every command handler
(get caps, set config, open, start, suspend, abort, close, ...) branches
on sep == NULL to answer Reject, so an unknown SEID was instead treated
as targeting the wrong SEP: a Close carrying SEID 62 was answered against
the only registered SEP (id 8, streaming) instead of being rejected with
BAD_ACP_SEID, and PTS AVDTP/SNK/ACP/SIG/SMG/BI-23-C timed out waiting for
the Reject.
Track the match explicitly and return NULL when none is found.
Signed-off-by: Yuheng Li <liyuheng@xiaomi.com>
Modify the buffer for control point responses to exist per client.
This is a requirement to later support handling the control point
notifications in the system workqueue, which enables support for
proper handling of notifications in case that we run out of ATT
buffers, as we need to store the response until then, and that we
may have multiple clients connected.
CONFIG_BT_MAX_PAIRED was chosen over CONFIG_BT_MAX_CONN as we only
care about paired (encrypted) connections, and this may scale
better at the cost of doing lookups on address instead of e.g.
bt_conn_index.
The unit tests for ASCS are updated to pass without any
functional changes.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
Fixes issue with VCP and MICP init failures not cleaning
up allocated AICS and VOCS instances.
Assisted-by: copilot-swe-agent
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
Notify Track Position when resetting it to zero while
playback is active. Stop resets the position before
changing the Media State to Paused, which previously
supressed the Track Position notification.
Signed-off-by: Alex Ciascai <alexandru.ciascai@nordicsemi.no>
The DELETE_IMAGE_FUNC_BIT macro was incorrectly defined as
BT_BIP_SUPP_FUNC_REMOTE_DISPLAY instead of BT_BIP_SUPP_FUNC_DELETE_IMAGE.
This caused the wrong function bit to be used for the delete image
operation.
Signed-off-by: Cheng Chang <cheng.chang@nxp.com>
sdp_client_get_attribute_id_list_len() summed the encoded size of the
attribute ID list in a uint16_t while iterating a size_t count. With
enough ranges the sum wrapped, the tailroom check in the SA and SSA
request builders passed on the wrapped value, and
sdp_client_add_attribute_id() then encoded every range into the request
PDU, writing past the end of the net_buf.
Accumulate in size_t, pass the measured length to the encoder instead of
recomputing it there, and reject lists that cannot fit in a request PDU
in bt_sdp_discover() with -EINVAL. The bound is derived from the request
with the largest fixed-size parameters, an SSA request with a 128-bit
UUID and a full-size continuation state, so a list accepted by
bt_sdp_discover() fits every request of the transaction.
Document the limit in the API.
Fixes#117579
Assisted-by: Claude:claude-fable-5
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
The `required_appl_param_tag_id` bitmask in `struct bip_function` was
unused; both `bip_server_get_req_cb()` and `bip_client_get_req_cb()`
contained a placeholder comment noting that application parameter tag
IDs were not checked.
Add `has_required_ap_tag_id()`, which iterates every set bit in the
bitmask via `__builtin_ctz()` and verifies that the corresponding
application parameter is present in the request buffer using
`bt_obex_has_app_param()`. Invoke this helper in both the server and
client request paths (GET and PUT) so BIP operations that require
specific application parameters are correctly rejected when those
parameters are absent, rather than being silently passed through.
Also rename the struct field from `required_appl_param_tag_id` to
`required_ap_tag_id` to align with the shorter naming convention
used elsewhere in the file.
Signed-off-by: Lyle Zhu <lyle.zhu@nxp.com>
Move OBEX authentication functions from PBAP to the OBEX layer to make
them available for other OBEX-based profiles. The authentication
implementation remains unchanged but is now part of the generic OBEX
API rather than being PBAP-specific.
Move `bt_pbap_calculate_nonce()` to `bt_obex_generate_nonce()`. Move
`bt_pbap_calculate_rsp_digest()` to
`bt_obex_calculate_request_digest()`. Move
`bt_pbap_verify_authentication()` to `bt_obex_verify_auth_response()`.
The refactoring allows other OBEX-based profiles to use the same
authentication mechanism without code duplication.
Signed-off-by: Lyle Zhu <lyle.zhu@nxp.com>
HID spec v1.1.2 Table 3.4 places the Size flag of the GET_REPORT header
octet at bit 3 and marks bit 2 as reserved, but the parameter mask used
bit 2. A host would therefore set the reserved bit instead of the Size
flag, leaving the device to parse the trailing BufferSize field as report
data. HID11.TS HID11/HOS/HCT/BV-01-C additionally requires all reserved
fields of the Get_Report command to be zero.
Signed-off-by: Kai Cheng <chengkai@xiaomi.com>
Convert the remaining k_sched_lock() sections that guard TX-path state
to the host lock in one step: tx_processor() with get_conn_ready(),
bt_conn_data_ready() and the L2CAP raise/cancel/lower data-ready
helpers. They all mutate the bt_dev.le.conn_ready and per-connection
l2cap data-ready lists from different threads, so converting one side
alone would replace the implicit cooperative exclusion with an actual
race; converting them together makes the exclusion explicit and
correct on SMP and with preemptible callers.
The host itself never blocks under the lock: the TX processor only
performs K_NO_WAIT acquisitions. An HCI driver's send() may still block
while the lock is held, as it could while the scheduler lock was held,
which bounds the hold time by the driver's transmit time; the native
controller's synchronous re-entry of the host from send() is covered by
the mutex being recursive.
The TX pass invokes one application callback on its normal path, the
L2CAP status() notification on credit exhaustion in chan_take_credit().
It ran under the scheduler lock and runs under the host lock now, and
re-entry into the host from it is covered by the mutex being recursive;
whether it keeps a hard non-blocking contract or moves out of the TX
processor is decided with the callback contracts (#116316).
The BR/EDR L2CAP data-ready helpers, which had no serialization at all,
take the lock the same way as the LE ones.
The fatal-error recovery paths of the TX processor, where a failed
send_buf() invokes the completion callback with an error and
disconnects synchronously, run under the lock as they ran under the
scheduler lock and are left as they are: the synchronous disconnect
already cannot complete from the TX processor thread, which would wait
for a command that only it sends, and both paths are to be reworked
with the asynchronous HCI command API (#116316).
The receive path keeps its scheduler lock; it is not converted here, as
the priority receive path is to become lock-free rather than take the
host lock.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
Add a recursive k_mutex to struct bt_dev, the host lock, with
bt_dev_lock()/bt_dev_unlock() helpers and a BT_DEV_LOCK_ASSERT()
ownership assertion. The lock is statically initialized because
bt_gatt_service_register() is legal before bt_enable(). It is never held
across blocking waits, and the sections converted here never hold it
across application callbacks; the priority receive path does not take
it.
Convert the k_sched_lock() sections that are independent of the TX path
to the host lock: the GATT database and service registration sections
(gatt.c), the ATT request send sections (att.c) and the ECC result
sections (ecc.c). These sections relied on cooperative scheduling for
mutual exclusion, which holds neither on SMP nor when the host is
entered from a preemptible thread; the mutex also provides priority
inheritance and survives a blocking caller. The recursive mutex is
load-bearing: the ATT send path nests the sections up to four deep.
The GATT registration/cancel and ATT request send sections keep the
scheduler lock nested inside the host lock: their RX-path counterparts
(the ATT request handlers iterating the database and att_handle_rsp()
processing the request state) do not take the host lock yet and rely on
cooperative scheduling, which excludes them only while the caller
cannot be preempted. The nesting preserves that guarantee for
preemptible callers; it goes away when those readers take the host
lock (#116316).
The ECC sections detach the registered callbacks under the lock and
invoke them without it, and the ECC request and disruption paths, which
touched the callback list, the pending state and the shared key storage
without any serialization, take the lock as well. A public key
generation runs on the preemptible long workqueue, so the disruption
path can run while one is in flight; the worker records the disruption
count when it starts and, if it has changed by the time it completes,
publishes nothing: the disruption completed the callbacks registered
with it, and any request made since has re-queued the work item.
The remaining k_sched_lock() sections (TX processor, connection and
L2CAP data-ready lists) share state across the TX path and are converted
together in a separate change.
The mutex adds 24 bytes of RAM, which no longer fits the extended
observer sample build for bbc_microbit; reduce the ISR stack in that
sample configuration to 960 bytes to make room.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
After a device-initiated Virtual Cable Unplug the initiator must
wait for the remote to tear down the L2CAP channels instead of
disconnecting them itself. The old code disconnected locally as
soon as the PDU was sent.
Change vcu_disconnect to a k_work_delayable and, on a successful send,
arm a 5S fallback timer (HID_VCU_DISCONNECT_TIMEOUT) as a safety net;
cleanup cancels it if the remote disconnects first. If the send fails,
keep disconnecting immediately since the remote may never have seen the
unplug.
Signed-off-by: Make Shi <make.shi@nxp.com>
Add support for testing RFCOMM credit-based flow control in the SPP
shell commands.
Add new optional parameters to SPP shell commands:
- `credit_limit`: Configure custom RX credit limit per DLC
- `hold_credit`: Enable asynchronous receive completion mode
When `hold_credit` is enabled:
- Received buffers are stored in a FIFO queue
- The recv callback returns `-EINPROGRESS` to hold credits
- New `recv_complete` command allows manual completion of receive
operations to test credit refill behavior
Update all SPP shell commands (register_with_channel,
register_with_uuid, connect_by_channel, connect_by_uuid) to accept
these optional parameters for flow control testing.
Add documentation with examples demonstrating flow control testing
scenarios for both server and client roles.
Signed-off-by: Lyle Zhu <lyle.zhu@nxp.com>
Reorder function definitions in rfcomm.c to remove the declaration of
the function `rfcomm_dlc_update_credits()`.
Move `rfcomm_dlc_get_available_credits()` and
`rfcomm_dlc_update_credits()` before `rfcomm_dlc_tx_worker()` since
the worker function calls `rfcomm_dlc_update_credits()`.
Move `rfcomm_dlc_tx_worker()` and `rfcomm_dlc_connected()` before
`rfcomm_handle_sabm()` since SABM handler calls
`rfcomm_dlc_connected()`.
Signed-off-by: Lyle Zhu <lyle.zhu@nxp.com>
Add support for asynchronous receive completion to allow applications
to control when RFCOMM data reception is considered complete.
Change the `recv` callback in `bt_rfcomm_dlc_ops` to return an `int`
instead of void. Applications can now return `-EINPROGRESS` to indicate
that data processing is asynchronous and will be completed later by
calling the new `bt_rfcomm_dlc_recv_complete()` API.
When `-EINPROGRESS` is returned:
- The application takes ownership of the buffer reference
- Increment `rx_credit_inprogress` counter
- For non-CFC sessions, send MSC command with FC bit set
- RX credits are decremented immediately but not refilled until
`bt_rfcomm_dlc_recv_complete()` is called
Update all existing RFCOMM recv callbacks (HFP AG, HFP HF, GOEP, SPP
shell, etc) to return 0 to maintain current behavior.
Signed-off-by: Lyle Zhu <lyle.zhu@nxp.com>
Add support for configurable default RX credits per DLC through the
new `rx_credit_limit` field in structure `bt_rfcomm_dlc` .
Previously, all DLCs used a fixed default credit value
RFCOMM_MAX_CREDITS. This change allows applications to specify a custom
initial RX credit count for each DLC, providing better control over
credit based flow control behavior.
When `rx_credit_limit` is set to 0, the rfcomm driver uses the
internal default value. Otherwise, the specified value (the upper limit
is RFCOMM_MAX_CREDITS) is used as the maximum credit count for the
specific DLC.
Also, limit the maximum value of RFCOMM_MAX_CREDITS to 255 to avoid
overflow.
Signed-off-by: Lyle Zhu <lyle.zhu@nxp.com>
bt_disable() tore down the ISO and ACL connections before resetting the
controller. The teardown returns the packets in flight to their senders
with an error, but the controller still owns those packets at that point
and keeps reporting their completion until it is reset. With an active
BIS broadcaster the Number Of Completed Packets event for such a packet
then finds the connection's pending list empty, which triggers the
"packets count mismatch" assertion in hci_num_completed_packets().
Reset the controller first, once the low-priority RX processing has been
stopped, and tear down the connections afterwards: completions reported
up to the reset are accounted for as usual, and the host only returns
what the controller no longer holds. Neither the ISO nor the connection
cleanup sends HCI commands, so they do not need the controller to be
running. A failed reset now also leaves the connections in place, in
line with the ready state it restores.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
In case due to unlucky scheduling we ended up with queued buffers either in
the HCI command or connection queues, be sure to purge those queues cleanly
in the TX processor functions.
The command queue is additionally purged by bt_disable() itself right
after it marks the transport closed, and bt_hci_cmd_send() re-checks the
transport state after queuing a command: a sender that passed the state
check just before the transport was closed either finds it closed on the
re-check and takes its command back, failing with -EHOSTDOWN, or has the
command dropped by the purge, which completes synchronous senders with an
error. Either way no command is left behind to be lost by the next
bt_enable() or sent to the next controller session.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
Since the HCI command sending APIs are public, we should have a check that
the transport is actually open before attempting to queue a command for
sending. If we don't do this it'll trigger an assert due to a command
timeout, which is less friendly (and harder to debug) for users.
The public command sending APIs document the new -EHOSTDOWN return
value. The buffer parameter may be NULL, so the early return only
releases a buffer that was actually passed in.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
Make a clear distinction of flags intended to protect against race
conditions related to host stack state transitions and flags intended to
indicate the current availability of the HCI transport and Bluetooth APIs
in general.
In particular, the following flags protect against state transition race
conditions: BT_DEV_ENABLING and BT_DEV_DISABLING
Whereas the following indicate the availibilty of the HCI transport and
Bluetooth APIs in general: BT_DEV_OPEN and BT_DEV_READY.
We also do an additional simplification in bt_disable() where we don't
bother doing special mapping for -ENOSYS -> -ENOTSUP since this special
error code was never documented.
The transition flags also give the enable/disable APIs a consistent
error model: -EALREADY when the requested transition is already in
progress or complete, and -EAGAIN when the opposite transition is in
progress, i.e. bt_disable() before an asynchronous bt_enable() has
completed, or bt_enable() while bt_disable() is still running. Both
APIs now document their return values.
The transition flags are kept across the flag reset performed when an
HCI_Reset completes, which happens both during initialization and in
bt_disable(): clearing ENABLING there would prevent bt_finalize_init()
from ever setting READY, and clearing DISABLING would end the RX
teardown early. The RX teardown predicate that main gained in the
meantime is converted to the DISABLING flag as part of the rebase.
The k_work_busy_get() check that bt_disable() gained for the
asynchronous bt_enable() case is replaced by the ENABLING flag, which
covers that case together with a synchronous bt_enable() running in
another thread.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
The maximum length of an attribute value is 512 octets (Core
Specification Vol 3, Part F, Section 3.2.9), but the GATT client
delivered notification and indication values of any length permitted by
the ATT MTU to subscribe callbacks. Subscribers that size their buffers
according to the specification limit can then be handed more data than
they have room for.
Drop notifications, indications and multiple handle value notification
tuples whose value is larger than BT_ATT_MAX_ATTRIBUTE_LEN before
delivering them. The remaining tuples of a multiple handle value
notification are still delivered, and oversized indications are still
confirmed, since the confirmation only acknowledges receipt of the PDU.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
bt_sdp_discover_cancel() has been declared in sdp.h since the SDP
client API was introduced, but no implementation has ever existed: any
caller would fail to link. Without it, a profile that queues a
discovery and then hits an error on an unrelated path has no way to
take the request back, and must keep the discovery parameters object
alive and its callback armed until the discovery machinery gets around
to it.
Implement the semantics the declaration documents: a waiting request,
i.e. one queued with bt_sdp_discover() whose resolution has not yet
started, is removed from the session and its callback will not be
called. The request currently being resolved cannot be canceled
cleanly - its request PDU is out and the response may already be under
processing - so canceling it is refused with -EINPROGRESS and the
callback still fires. Requests parked for the next session round
(reqs_next) are canceled the same way as waiting ones.
The declaration took the parameters object as const, but canceling
inherently modifies it: the request node is unlinked from the
session's list. Drop the const qualifier rather than casting it away
in the implementation. Since the function never had an implementation,
no existing caller can be affected by the signature change.
For the "callback will not be called" guarantee to hold, the places
that take a request off the waiting list and act on it must do so
under the session lock so that they cannot race with the removal:
- sdp_client_discover() now marks the selected request active
(session->param) while still holding the lock, rather than after
releasing it in the per-type send functions.
- sdp_client_params_iterator() now holds the lock across the removal
of the completed request instead of only taking it afterwards.
- sdp_client_disconnected() now pops each request under the lock
before invoking its callback, instead of iterating the list
unlocked.
sdp_client_alloc_buf() additionally needs to handle an empty request
list, which can now happen when every queued request is canceled while
the channel is still being established. Returning NULL there makes
sdp_client_connected() disconnect the no-longer-needed channel; the
previous code dereferenced the head of the list unconditionally.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
bt_le_scan_start() sets up the explicit scan parameters and the scan
callback with scan_explicit_params_mutex held, but bt_le_scan_stop()
clears the callback and removes the scanner user without taking it, so
a stop running concurrently with a start can tear down the state the
start is in the middle of setting up.
Take the same lock in bt_le_scan_stop(). As in bt_le_scan_start(), the
lock is taken without waiting, so that a stop from the Bluetooth
workqueue cannot block on a start that is waiting for a command
completion.
Assisted-by: Claude:claude-opus-5
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
bt_settings_store_id() and bt_settings_store_irk() defer the actual
settings write to a work item that reads bt_dev.id_addr/bt_dev.irk with
a length derived from bt_dev.id_count at execution time.
bt_disable() resets bt_dev.id_count without waiting for those work
items. If a store is still pending at that point, the handler later
computes a zero-length record and persists an empty "bt/id" or
"bt/irk" entry, erasing the identity stored in the settings backend.
If the handler is mid-execution, it can read the identity data while
it is being reset, persisting a torn record.
Simply canceling the work items would trade one bug for another: an
identity created or updated shortly before bt_disable() would silently
never reach the settings backend, and a later bt_enable() would load
stale identity data. Instead, add bt_settings_flush(), which completes
any pending stores while the identity state is still valid, and call
it from bt_disable() before the identity state is reset.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
bt_avdtp_connect() sets session->br_chan.chan.conn before calling
bt_l2cap_chan_connect() in order to detect simultaneous local and
remote AVDTP signaling connection attempts. If bt_l2cap_chan_connect()
fails before the channel has been added to the connection, for example
with -ENOTCONN when the ACL is no longer connected, or through one of
the early argument and state checks in bt_l2cap_br_chan_connect(), no
disconnected callback will ever run for the channel and the marker is
never cleared. a2dp_get_connection() treats a non-NULL marker as a
session in use, so every subsequent bt_a2dp_connect() for that
connection index fails with -ENOMEM, including attempts on future ACL
connections that reuse the same connection object.
Clear the marker on the error path of bt_l2cap_chan_connect(), holding
avdtp_sem_lock like the other accesses to the marker. It is only
cleared if it still holds the value set above, since a failure that
occurs after the channel was added to the connection has already
cleared it in bt_l2cap_br_chan_del().
The accept side sets the same marker in bt_avdtp_l2cap_accept(), but
there the L2CAP server code owns the channel lifecycle after accept
returns and deletes the channel with bt_l2cap_br_chan_del() on the
failure paths, which clears the marker.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
