There is a test case for the shell to build with
CONFIG_BT_CONN_DYNAMIC_CALLBACKS=n. Registering the conn
callbacks dynamically in the cmd_bap_scan_delegator_init
fails that test.
Change to use BT_CONN_CB_DEFINE.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
Add validation to prevent sending additional OBEX requests if the
SRM has been enabled after the final bit has been set in PUT or GET
operations on the Client side.
When the SRM has been enabled Without the waiting flag, if the final
bit of the request has been set in the last packet in a multi-packet
operation, the following request packet should not be set.
This ensures proper OBEX protocol sequencing by preventing clients
from sending additional requests if the SRM has been enabled once
they have indicated the final request in a multi-packet operation.
Signed-off-by: Lyle Zhu <lyle.zhu@nxp.com>
Check Single Response Mode (SRM) and SRM Parameters (SRMP) for
OBEX PUT and GET operations on the Server side to avoid the responses
that violate the GOEP specification.
Changes include:
- Add BT_OBEX_REQ_RECV and BT_OBEX_REQ_F_BIT flag definitions for
tracking request reception and final bit state
- Implement obex_server_parse_srm() for parsing and validating SRM
headers in both requests and responses
- Add obex_server_parse_req_srm() and obex_server_parse_rsp_srm()
to handle SRM headers with first request tracking
- Implement obex_server_parse_srmp() for parsing and validating SRMP
headers with proper sequencing checks
- Add obex_server_rsp_check() to validate response sequencing when
SRM is enabled, ensuring responses are only sent when required
- Update obex_server_put_common() and obex_server_get_common() to
parse SRM/SRMP headers and track request state
- Enhance bt_obex_put_rsp() and bt_obex_get_rsp() to validate
response conditions, parse SRM/SRMP headers, and handle state
rollback on failure
- Move opcode validation before buffer allocation to prevent
resource leaks
- Improve error handling with proper flag restoration on failure
This implementation ensures the server properly validates SRM/SRMP
headers and enforces protocol rules for when responses must be sent,
complementing the existing client-side SRM/SRMP support.
Signed-off-by: Lyle Zhu <lyle.zhu@nxp.com>
Check Single Response Mode (SRM) and SRM Parameters (SRMP) for
OBEX PUT and GET operations on the Client side to avoid the requests
that violate the GOEP specification.
Changes include:
- Parse and validate SRM/SRMP headers in client requests and responses
- Track SRM/SRMP state using client flags (BT_OBEX_REQ_1ST,
BT_OBEX_REQ_SRM, BT_OBEX_REQ_SRMP, BT_OBEX_RSP_SRM, BT_OBEX_RSP_SRMP,
BT_OBEX_RSP_RECV)
- Add request validation to ensure proper sequencing when SRM is
enabled or disabled
- Refactor client request completion handling into
obex_client_req_complete(), add obex_client_save_last_operation(),
and obex_client_clear_active_state() helper
- Add obex_client_req_check() to validate request state before sending
- Restore client state on error in PUT and GET operations
- Move bt_obex flags enum from header to implementation file
SRM allows multiple requests to be sent without waiting for responses,
while SRMP provides flow control when the receiver needs the sender to
wait. This implementation ensures proper validation of SRM/SRMP headers
and enforces protocol rules for when responses must be received.
Signed-off-by: Lyle Zhu <lyle.zhu@nxp.com>
Standardize pointer and error code comparisons throughout the OBEX
implementation to follow Zephyr coding guidelines:
- Replace implicit pointer checks (!ptr) with explicit NULL
comparisons (ptr == NULL)
- Replace implicit error checks (!err) with explicit zero
comparisons (err != 0 or err == 0)
- Update length checks to use explicit comparisons (len > 0
instead of len, len == 0 instead of !len)
This improves code readability and consistency across the obex.c file
with the project's coding standards without changing any functional
behavior.
Signed-off-by: Lyle Zhu <lyle.zhu@nxp.com>
bt_conn_ref can return NULL if the application passes a stale conn
pointer. Check the return value in all public LE Audio discover APIs
and return -ENOTCONN on failure.
Signed-off-by: Rishav Chakraborty <reshav01@gmail.com>
The get_progress() function in the BLOB shell had two bugs:
1. The loop iterated over byte count (DIV_ROUND_UP(total_blocks, 8))
instead of total block count, and the bit indexing used i % 8
for both the byte index and bit position, meaning it only ever
inspected the first byte of the missing_blocks array.
2. The final calculation (total_blocks - blocks_not_rxed) / total_blocks
performed integer division without scaling, always yielding 0 or 1
instead of a percentage.
Fix the loop to iterate over all block bits using proper byte/bit
indexing, and multiply the numerator by 100 to produce a correct
0-100 percentage result.
Signed-off-by: Aleksandr Khromykh <aleksandr.khromykh@nordicsemi.no>
Remove `$(ZEPHYR_BASE)` from `source "<file>"`.
Source is always relative to ZEPHYR_BASE, so no need to specify it.
Signed-off-by: Torsten Rasmussen <Torsten.Rasmussen@nordicsemi.no>
Use buf->len (actual data length) instead of buf->size (buffer
capacity) when calculating the number of chunks to allocate in
bt_mesh_delayable_msg_manage(). Using buf->size causes over-allocation
when the buffer capacity is much larger than the payload, starving
the chunk pool for other delayable messages.
Signed-off-by: Aleksandr Khromykh <aleksandr.khromykh@nordicsemi.no>
BT_LONG_WQ was enabled for BT_GATT_CACHING through a "default y if" on the
option itself, but for BT_ECC through an "imply BT_LONG_WQ" on BT_ECC.
Use a consistent mechanism for both by replacing the implication with a
"default y if BT_ECC" on BT_LONG_WQ.
Besides being more consistent, this makes the BT_ECC driven enablement
overridable through a configdefault: an "imply" sets a weak reverse
dependency that a (config)default cannot turn off, whereas a plain default
can. This allows platforms to disable the long workqueue (and fall back to
running the long-running operations on the system workqueue) via a
configdefault in their Kconfig.defconfig, which a subsequent change makes
use of on nRF51.
This change has no functional effect on its own: BT_ECC continues to enable
BT_LONG_WQ by default.
Assisted-by: Claude:claude-opus-4.8
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
When the LPN terminates a friendship by sending Friend Clear, the
Confirm was never received due to two issues:
1. The Friend's immediate Confirm was enqueued into the Friend
Queue and then purged by friend_clear(). Fixed by calling
friend_clear() before sending.
2. The Confirm advertisement completed before the LPN finished
its own retransmissions and started scanning. Fixed by delaying
the Confirm send based on network transmit parameters.
Additionally, bypass the LPN transport filter during the CLEAR
state so the Confirm is not dropped on arrival.
The clear procedure and Confirm contexts share a union since they
are mutually exclusive for the same Friend slot.
Signed-off-by: Aleksandr Khromykh <aleksandr.khromykh@nordicsemi.no>
The "dfd receivers-get" shell command prints the firmware distribution
progress under the JSON "progress" key, but first divides
bt_mesh_dfu_cli_progress() by two.
bt_mesh_dfu_cli_progress() is documented to return "the progress of the
current transfer in percent", i.e. a 0-100 value. The division mirrors
the on-wire encoding of the Transfer Progress field in the Firmware
Distribution Receivers List message, which is expressed in 2% increments
(0-50, see handle_receivers_get() in dfd_srv.c). That encoding is correct
for the over-the-air field, but applying it to a human-facing diagnostic
command is misleading: the command reports half of the actual percentage
(e.g. 25 while the transfer is at 50%).
It is also inconsistent with the Firmware Update Client shell command:
cmd_dfu_tx_progress() in shell/dfu.c prints bt_mesh_dfu_cli_progress()
directly as "DFU progress: %u %%", i.e. a 0-100 percentage.
Drop the division so the dfd shell reports the 0-100 percentage that the
API returns, aligning it with the dfu cli command. The on-wire encoding
in dfd_srv.c is intentionally left unchanged.
Signed-off-by: Francesco Vatteroni <francescov.90@gmail.com>
Instead of logging based on the bap stats interval, the USB
part will not just use a flat rate limit of 30 seconds.
Since the bap_stats_interval and USB rates are used for 2 very different
things, and since USB will likely be trigger way more often, it did
not make a lot of sense to use the same value for both.
30 seconds was chosen as it still provides information to
the user on a reasonable interval, without filling up the
shell console too fast.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
Add command to loop through known receive states and print
their content. This also adds the request BIS sync value for
comparison to the currently synced value.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
Change the order of the pa_interval and bis_sync arguments for
the add_src command. The reason is that the user will generally
care more about setting the right bis_sync value (e.g. to 0x01)
rather than the PA interval which may be set to "unknown" in
any case. With this change the user can set the bis_sync without
the pa_interval and just defer to the "unknown" PA interval value.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
The do_plc applies to the entire PLC, and not the individual frames,
so we should not clear it after doing PLC on the first frame.
Additionally this will now also do a memset after k_mem_slab_alloc
to avoid having to clearing data manually before free'ing it.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
Since update_usb_streams rely on the sh_stream->rx to determine
whether to update the streams, we shall not clear that before calling
update_usb_streams
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
Add calls to print_codec_cfg whenever the codec configuration
data or metadata may be changed.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
Suffix the LOCATION and CONTEXT macros with DEFAULT to make it
clearer that these are just the default value, since both the
location and context are configurable at runtime.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
There's a for loop in parse_read_std_char_desc() that'll go on forever if
rsp->len is 0. Add a minimum length check to the beginning of it to avoid
this issue.
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
This workaround addresses an assert that appears when nRF52 hardware
running as central connects to a peripheral with certain connection
parameters.
Fixes#106450
Signed-off-by: Xudong Zheng <7pkvm5aw@slicealias.com>
The relasing state can go to IDLE or CODEC_CONFIGURED but
was defined to only allow IDLE or QOS_CONFIGURED.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
The `err` variable was overwritten by the k_mutex functions
before being logged, so it did not log the actual error.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
Apply the U suffix to unsigned integer literal constants used in
contexts involving unsigned types (uint8_t, uint16_t, uint32_t,
uint64_t, size_t, etc.) with coding guideline 40 which requires that
"A u or U suffix shall be applied to all integer constants that are
represented in an unsigned type".
In addition to this requirement, this commit also ads U to any unsigned
literal integer for the sake of completeness.
Changes are limited to literal values in assignments, initializations,
comparisons, and for-loop bounds where the context is unambiguously
unsigned.
Assisted-by: GitHub Copilot:claude-sonnet-4.6
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
The broadcast ID is printed in the shell various places. Often the
shell user wants to copy this value and use it for future commands.
Remove any following commas, so that the user can easier click-to-mark
the value to copy it.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
If the server wrongly sends us a QoS state notification before
the stream was ever added to a group, we should just ignore it
rather than hitting the assert or dereferencing the group
later.
Additionally, LOG_ERR should not be used when validating input
from a remote device.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
Instead of just checking the lower limit, use
is_valid_num_ases to also test the upper limit.
This approach is used by all other control point operations.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
Add missing type check in sdp_attr_parse() before recursively parsing
data element sequences. When checking the type of the next data if it
is a sequence, it did not first check if the current type is a
sequence.
It could lead to incorrect recursive parsing when the current type is
not a sequence but the next byte was incorrectly identified as a
sequence.
Fix the issue by checking the current data type firstly.
Signed-off-by: Lyle Zhu <lyle.zhu@nxp.com>
bt_hci_le_enh_conn_complete() obtains a reference via
find_pending_connect() but drops it immediately after the
bt_conn_connected() callback, while the function still dereferences
conn->role afterwards in the central-role scanner restart block.
Using a pointer after its reference has been dropped is a
use-after-free in API contract terms, even if in practice the
connection survives because other references (the conn array's
intrinsic reference, L2CAP, etc.) keep the object alive at this
point.
Move the bt_conn_unref() call to the end of the function so the
locally held reference outlives all uses of conn.
Assisted-by: GitHub Copilot:claude-opus-4.7
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
bt_avrcp_cover_art_ct_l2cap_connect() obtains a reference to the ACL
connection via bt_avrcp_ct_get_acl_conn() but then immediately drops
that reference before continuing to use the conn pointer for
bt_conn_index() and bt_bip_l2cap_connect(). Using a pointer after
its reference has been dropped is a use-after-free in API contract
terms, even if in practice the underlying connection survives because
other references (e.g. the L2CAP channel, the ACL link itself) keep
the object alive while AVRCP cover art is in use.
Move the bt_conn_unref() call to after the last use of conn on each
return path so that the locally held reference outlives all uses.
Assisted-by: GitHub Copilot:claude-opus-4.7
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
Replace the two-line pattern:
bt_conn_unref(conn);
conn = NULL;
(and equivalent if-guarded variants) with bt_conn_drop(&conn) which
performs both operations atomically in a single step. The guard is
unnecessary because bt_conn_drop() handles a NULL pointer internally.
Assisted-by: GitHub Copilot:claude-opus-4.7
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
Replace the two-line pattern:
bt_conn_unref(conn);
conn = NULL;
(and equivalent if-guarded variants) with bt_conn_drop(&conn) which
performs both operations atomically in a single step. The guard is
unnecessary because bt_conn_drop() handles a NULL pointer internally.
Assisted-by: GitHub Copilot:claude-opus-4.7
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
Replace the two-line pattern:
bt_conn_unref(conn);
conn = NULL;
(and equivalent if-guarded variants) with bt_conn_drop(&conn) which
performs both operations atomically in a single step. The guard is
unnecessary because bt_conn_drop() handles a NULL pointer internally.
Assisted-by: GitHub Copilot:claude-opus-4.7
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
Replace the two-line pattern:
bt_conn_unref(conn);
conn = NULL;
(and equivalent if-guarded or take-then-unref-then-NULL variants) with
bt_conn_drop(&conn) which performs both operations atomically in a
single step. The guard is unnecessary because bt_conn_drop() handles a
NULL pointer internally.
Assisted-by: GitHub Copilot:claude-opus-4.7
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
Add a new bt_conn_drop() function that atomically drops a connection
reference and clears the source pointer in a single step. This mirrors
the recently added net_buf_drop() API and replaces the common pattern:
bt_conn_unref(conn);
conn = NULL;
The new API handles a NULL pointer internally, so any guard around it
is unnecessary.
Assisted-by: GitHub Copilot:claude-opus-4.7
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
Replace the manual two-step pointer transfer:
struct bt_conn *acl = iso->iso.acl;
if (acl != NULL) {
iso->iso.acl = NULL;
...
}
with bt_conn_take(), which atomically extracts the reference and clears
the original pointer in a single step.
Assisted-by: GitHub Copilot:claude-opus-4.7
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
Add calculations to set the cig->c_latency and cig->p_latency
so that the CIS established event contains valid values.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
There was a missing check in le_ecred_reconf_req() that the provided CIDs
are dynamic (only those can be reconfigured). Add proper handling in case
a non-dynamic CID is encountered in the PDU.
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
The HCI LE LTK Request event parameters for `rand` and `ediv` should be
byte arrays, not integer types, to match the HCI specification and avoid
unaligned structure member access.
- Change `rand` from uint64_t to uint8_t[8]
- Change `ediv` from uint16_t to uint8_t[2]
- Add local variables in le_ltk_request() to convert byte arrays to
integer types before passing to bt_smp_request_ltk()
Signed-off-by: Lyle Zhu <lyle.zhu@nxp.com>
