Fix in-system ISR profiling for advertiser and connection
role for the missing implementation when there is CRC error.
Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
The interval and latency for a CIG are set for each direction now,
allowing applications to use e.g. 10ms for sink ASEs and 7.5ms for
source ASEs.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
gcc 13 thinks max_adv_duration may be used unitialized
and warns accordingly (see below)
It seems the reason is the goto, which confuses it.
In any case, pacifying this warning is trivial,
so let's just do so.
The warning:
```
In function ‘gatt_proxy_advertise’,
inlined from ‘bt_mesh_proxy_adv_start’ at
subsys/bluetooth/mesh/proxy_srv.c:1214:9:
subsys/bluetooth/mesh/proxy_srv.c:842:44: error: ‘max_adv_duration’ may
be used uninitialized [-Werror=maybe-uninitialized]
subsys/bluetooth/mesh/proxy_srv.c: In function ‘bt_mesh_proxy_adv_start’
zephyr/subsys/bluetooth/mesh/proxy_srv.c:786:17: note:
‘max_adv_duration’ was declared here
786 | int32_t max_adv_duration;
| ^~~~~~~~~~~~~~~~
```
Signed-off-by: Alberto Escolar Piedras <alberto.escolar.piedras@nordicsemi.no>
- Made CCC_STORE_MAX configurable under the BT_SETTINGS
- Added a buffer overflow check on ccc_save
Fixes: #76838
Signed-off-by: Yago Fontoura do Rosario <yafo@demant.com>
This function call frees the buffer kept by the host for reassembling L2CAP
PDUs into.
Without this call, the current buffer will eventually be
leaked, leading to a non-functional host due to lack of RX buffers.
The effect is worse when host flow control is not enabled, as the RX
buffer pool is shared with events, which means communication with the
controller is essentially dead.
Signed-off-by: Jonathan Rico <jonathan.rico@nordicsemi.no>
`net_buf_alloc(K_FOREVER)` can now return NULL if running in the system
workqueue. `bt_hci_evt_create()` is called in that context in a few cases.
Since we can't really do anything about it, add a (default-on) assert.
This should ideally never fail. I saw it fail because of a leak in the ACL
buffer pool, which is also shared with events when host flow control is not
enabled.
In that particular case, the host is rendered non-functional, so trying to
recover using error handling is futile.
Signed-off-by: Jonathan Rico <jonathan.rico@nordicsemi.no>
Don't push the TS flag on `buf` itself.
This messes up the MTU calculations: a packet that would exactly fit the
MTU and has a timestamp would be unnecessarily fragmented.
The MTU check is done on `buf` as a whole. At the point where the
fragmentation length is decided, `buf` includes one extra byte to pass the
TS bit around. That byte shouldn't count towards the MTU.
Instead, infer the presence of the timestamp by inspecting the amount of
headroom that the buffer has. This works because we always reserve
enough memory to push the timestamp, but not always push a timestamp on
the buffer. #tightlycoupled
This method is slightly uglier IMO, but eases MTU confusion and doesn't
rely on user_data.
Signed-off-by: Jonathan Rico <jonathan.rico@nordicsemi.no>
When periodic scanning reaches timeout and BIGinfo has not yet been
received, notify with BT_HCI_ERR_CONN_FAIL_TO_ESTAB for sync receiver.
Signed-off-by: Morten Priess <mtpr@oticon.com>
When an ISO stream fails to sync to a broadcaster within the first 6
events, the establishment fails (as expected). However, it did not stop
the ticker, and subsequently it was impossible to establish a new sync
after this.
Make asynchronous call to ticker_stop when establishment fails, and exit
done handling. Cleanup is handled via ll_rx_dequeue().
Signed-off-by: Morten Priess <mtpr@oticon.com>
When the ISO sync receiver has been disabled (terminted), ULL now calls
lll_sync_iso_flush in lower link layer, to allow cleanup and releasing
of resources.
Make sure ISO sync LLL flush is also called when terminating a stream
from local side (app). Add blocking mayfly call to lll_sync_iso_flush in
ll_big_sync_terminate.
Signed-off-by: Morten Priess <mtpr@oticon.com>
If a race condition occurs between stopping CIS stream and tearing down
data path, releasing TX PDUs was not possible for vendor data path, as
the DP configuration is gone.
In that case, call a new vendor specific function for cleaning up and
returning PDUs to correct pool.
Signed-off-by: Morten Priess <mtpr@oticon.com>
The MFIFO holding TX nodes on the ack path back to the host was only
sized to hold the possible number of ACL TX nodes. However, additional
TX nodes are allocated for LLCP and use the same FIFO.
By adding LLCP_TX_CTRL_BUF_COUNT to the size, the FIFO will be able to
hold the worst-case number of TX nodes.
Signed-off-by: Morten Priess <mtpr@oticon.com>
Rename to BT_TICKER_START_REMAINDER better reflect what it
means in terms of providing remainder parameter to ticker
start interface.
Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
Score was never increased for scan_aux events since they are
one-shot events; Fixed by keeping the scan_aux score as part
of the scan structure
Signed-off-by: Troels Nilsson <trnn@demant.com>
Support for separate SDU interval for C_to_P and P_to_C
directions when setting CIG parameters.
Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
Add commands to allow requesting a subrate change via the BT shell.
A new build configuration has been added to ensure this is tested in CI.
Signed-off-by: Aleksandar Stanoev <aleksandar.stanoev@nordicsemi.no>
Adds support for LE Connection Subrating as defined in Core 5.4
Vol 6, Part B, Section 5.1.19.
As this is primarily a controller feature, the host support is mostly
a wrapper around the relevant HCI commands.
Note that subrating provides a new method to update the connection's
peripheral latency and supervision timeout alongside subrating parameters.
Signed-off-by: Aleksandar Stanoev <aleksandar.stanoev@nordicsemi.no>
With e.g. a 216MHz core clock, the 32-bit cycle counter overflows after
just 20 seconds. The 32-bit monitor timestamp (100us resolution) overflows
after around five days regardless of what we do, but we should try our best
to reach that.
Signed-off-by: Armin Brauns <armin.brauns@embedded-solutions.at>
It was not being set, and thus if the user_data contained garbage from
before, then conn.c would attempt to call that garbage.
Static channels don't have this issue, as every "SDU" fits into one PDU.
Signed-off-by: Jonathan Rico <jonathan.rico@nordicsemi.no>
Co-authored-by: Huajiang Zheng <nxf88597@lsv051208.swis.nl-cdc01.nxp.com>
Storing stuff in user_data? That's a paddlin'
We have been debugging issue after issue because ownership of this
"user" data is not clearly defined. Now it is. L2CAP owns the user_data
field entirely, as soon as `send()` is called.
Also add a warning and retval using CHECKIF.
Signed-off-by: Jonathan Rico <jonathan.rico@nordicsemi.no>
Declare __stdout_hook_install in libc-hooks.h and use it in the console
drivers rather than redeclare it every time.
Signed-off-by: Fabio Baltieri <fabiobaltieri@google.com>
Integrated the application version feature of the build system with
the default configuration of the Bluetooth DIS module and its Firmware
Revision characteristic.
The firmware revision string now defaults to APP_VERSION_TWEAK_STRING
if the application version feature is used in a project. This specific
version format is used to unify version formatting with other parts of
Zephyr like the MCUboot module and its versioning Kconfig:
CONFIG_MCUBOOT_IMGTOOL_SIGN_VERSION
Signed-off-by: Kamil Piszczek <Kamil.Piszczek@nordicsemi.no>
Since HWMv1, we also have CONFIG_$BOARD Kconfig symbols defined, ie, no
SoC/core/variant needed.
Signed-off-by: Gerard Marull-Paretas <gerard@teslabs.com>
We should disable the feature when it is not needed to
save flash and RAM.
There is no point in enabling data length extensions
when the maximum packet size used is always smaller or equal
to 27 bytes. Then data length updates would only use
parameters (octets=27, time=T) where T is some supported value
which would not improve throughput or power consumption.
Signed-off-by: Rubin Gerritsen <rubin.gerritsen@nordicsemi.no>
Fix implementation to consider event latencies due to
BIG events being skipped due to overlap with other state or
role, and generate any received/buffered pre-transmissions
towards the Host.
Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
Fix missing PTO subevent reception for second or more BISes
when current payload count associated PDUs where already
received in previous ISO events as pre-transmissions.
Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
When developing Bluetooth applications, you typically run into
some errors. If you are an experienced Bluetooth developer,
you would typically have an HCI error lookup table in your memory.
Others might not.
This commit utilizes defines CONFIG_BT_DEBUG_HCI_ERR_TO_STR
and utilizes bt_hci_err_to_str() to print out HCI error strings
when enabled to improve the user experience.
Several alternatives where considered. This approach was chosen
as it had the best balance between readability, code size, and
implementation complexity.
The alternatives are listed below as a reference.
1. Macro defined format specifier:
```c
#define HCI_ERR_FMT "%s"
#define BT_HCI_ERR_TO_STR(err) (err)
#define HCI_ERR_FMT "%d"
#define BT_HCI_ERR_TO_STR(err) bt_hci_err_to_str((err))
LOG_INF("The event contained " HCI_ERR_FMT " as status",
BT_HCI_ERR_TO_STR(err));
```
Advantage: Space efficient: Code size does not increase
Disadvantage: Code becomes hard to read
2. Format specifier to always include both integer and string:
```c
static inline const char bt_hci_err_to_str(err)
{
return "";
}
LOG_INF("The event contained %s(0x%02x) as status",
bt_hci_err_to_str(err), err);
```
Advantage: Simple to use, implement, and read,
Disadvantage: Increases code size when CONFIG_BT_DEBUG_HCI_ERR_TO_STR
is disabled. The compiler seems unable to optimize away the unused
format specifier. Note: The size increase is only present when
logging is enabled.
3. Always print as string, allocate a stack variable when printing:
```c
const char *bt_hci_err_to_str(char *dst, size_t dst_size, uint8_t err)
{
snprintf(dst, dst_size, 0x%02x, err);
return dst;
}
LOG_INF("The event contained %s as status", BT_HCI_ERR_TO_STR(err));
```
Advantage: Very easy to read.
Disadvantage: Printing error codes becomes slow as it involves calling
snprint.
4. Implement a custom printf specifier, for example E.
This requires a global CONFIG_ERR_AS_STR as I assume we cannot have
one specifier for each type of error code.
Also, I assume we cannot start adding specifiers for each subsystem.
```c
#define BT_HCI_ERR_TO_STR(err) (err)
#define BT_HCI_ERR_TO_STR(err) bt_hci_err_to_str((err))
LOG_INF("The event contained %E as status", BT_HCI_ERR_TO_STR(err));
```
Advantage: Both efficient code and readable code.
Disadvantage: This requires a global CONFIG_ERR_AS_STR as I assume
we cannot have one specifier for each type of error code.
Also, I assume we cannot start adding specifiers for each subsystem.
That is, this approach is hard to implement correctly in a scalable
way.
Signed-off-by: Rubin Gerritsen <rubin.gerritsen@nordicsemi.no>
This fixes a bug where the stack may get stuck in the
POWER_CLOCK ISR after enabling and disabling the Bluetooth
stack a couple of times. It happens after calling
`onoff_request()` after a failing call to `onoff_release()`.
Then the `lf_cli`s next points to itself, generating a
circular list of clients.
Calling `onoff_release()` may fail if there is an outstanding
request. By calling `onoff_cancel()` we enter a state where
we can safely remove the client.
This was not seen earlier because the return value
from `ll_deinit()` in `hci_driver_close()` was ignored.
Signed-off-by: Rubin Gerritsen <rubin.gerritsen@nordicsemi.no>
Modify the bt_bap_scan_delegator_add_src to take an address and
a sid instead of a PA sync object, so that the scan delegator
can add a source without syncing to the PA.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
Refactor. The deferred work is rescheduled just after it's canceled.
This is hopefully equivalent to just the reschedule, so the cancel can
be removed.
Signed-off-by: Aleksander Wasaznik <aleksander.wasaznik@nordicsemi.no>
The broadcast assistant will only be able to send
BT_BAP_BASS_PA_REQ_SYNC_PAST if
CONFIG_BT_PER_ADV_SYNC_TRANSFER_SENDER is enabled.
Similarly the scan delegator will only set
past_supported = true if
CONFIG_BT_PER_ADV_SYNC_TRANSFER_RECEIVER is enabled.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
Refactors teh BIS bitfield values used for ISO
and BAP.
Previously BIT(1) meant BIS index 1, which was a Zephyr choice
in the early days of ISO, as the BT Core spec did not use
a bitfield for BIS indexes.
Later the BASS specification came along and defined that
BIT(0) meant BIS index 1, which meant that we had to shift BIS
bitfields between BAP and ISO.
This commit refactors the ISO layer to use BIT(0) for Index 1 now,
which means that there is no longer a need for conversion
between the BAP and ISO layers, and that we can use a value
range defined by a BT Core spec (BASS).
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
The Softdevice Controller now sends the disconnect event only after
receiving all Host Num Completes for the packets it sent to the host.
This is done for security reasons.
In our current reassembly logic, it does not really matter when we
withhold the num complete.
Before this patch, it's the first fragment that is withheld, and after
the patch it will be the last fragment that is withheld until the host
is done processing.
The flow control properties are maintained, just in a different way.
Co-authored-by: Aleksander Wasaznik <aleksander.wasaznik@nordicsemi.no>
Signed-off-by: Jonathan Rico <jonathan.rico@nordicsemi.no>
The functionality is moved in preparation of the next commit which will
re-use this function from somewhere else.
Also add (default-on) asserts that we are able to allocate and send the
command. If that is not the case, we will leak buffers from the PoV of
the controller, leading to a stall in data transfer.
Depending on the error, we could probably recover using a disconnection.
For now, do the safe thing and stop the whole stack.
Signed-off-by: Jonathan Rico <jonathan.rico@nordicsemi.no>
The spec only guarantees the connection complete event parameters
are valid if the status is BT_HCI_ERR_SUCCESS. When the status is
not BT_HCI_ERR_SUCCESS, the host shall ignore the other parameters.
See Vol 4, Part E, 4.5 Command error handling:
"""
If an error occurs for a command for which an HCI_Command_Complete event
is returned, the Return Parameters field may only contain some of the
return parameters specified for the command.
...
The above also applies to commands that have associated command specific
completion events with a Status parameter in their completion event, with
the exceptions shown in Table 4.1, Event Valid parameters
...
Event | Valid parameters
------------------------------------------------------------
LE_Connection_Complete | none
LE_Enhanced_Connection_Complete | none
"""
Refactor `le_legacy_conn_complete`, `le_enh_conn_complete_v2` and
`le_enh_conn_complete` to check and handle the status before handling
any other parameters.
An issue was seen where SDC returned event with status
`BT_HCI_ERR_UNKNOWN_CONN_ID`, but because adv_handle and sync_handle
were not invalid the event was not handled.
Signed-off-by: Sean Madigan <sean.madigan@nordicsemi.no>
If the application is not explicitly scanning, then there is not
really any need to parse advertising reports nor send them to the
application.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
Fix Advertising PDU memory allocation for redundant Periodic
Advertising related PDU allocations. The buffer count
related to Periodic Advertising was included twice.
Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
