The API documentation already states that the controller may require
the scan interval and window used for scanning and connection
establishment to be equal to obtain the best performance.
This commit prints out a warning when this is not the case. The code
size is unchanged when `CONFIG_BT_SCAN_AND_INITIATE_IN_PARALLEL=n`.
This makes application developers aware that using the parameters
`BT_LE_SCAN_ACTIVE_CONTINUOUS` with `BT_CONN_LE_CREATE_CONN` may not
give the best performance.
Signed-off-by: Rubin Gerritsen <rubin.gerritsen@nordicsemi.no>
Fix scan_update() implementation for missing flags clear
that prevented starting scanning after previous attempt to
start failed due to parameters being rejected by the
Controller.
Example, attempting to start coded PHY scanning with a
Controller implementation that does not support it can fail,
but attempting again without coded PHY scanning should
succeed.
Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
To make the scanner module more understandable and more streamlined, I
reworked the update mechanism of the scanner. The scanner tracks now the
parameters that were used to enable it and the reason why it is running.
This facilitates state logic and allows other modules to "start the
scanner", altough it is already running.
This is mostly a refactoring and not a functional change.
Added a test to verify the behavior.
Signed-off-by: Jan Müller <jan.mueller@nordicsemi.no>
add to task
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>
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>
When implementing vendor specific HCI APIs and events,
we want to be able to convert between host objects,
handles and back again.
Exposing this API makes that possible.
Signed-off-by: Rubin Gerritsen <rubin.gerritsen@nordicsemi.no>
When syncing to a PA using PAST then the sync_info.recv_enabled
was always just set to true, regardless of what mode was set
during the subscribe parameters.
The mode(s) are now stored in an array (with the default value
as well) so that we can retrieve that information when the PA
has synced via PAST.
It was considered to put the `mode` value into the `bt_conn`
struct, but that would require an API change as the `bt_conn`
parameter for the subcribe function uses `const`.
This commit also modifies the guard for PAST to be the more
correct value CONFIG_BT_PER_ADV_SYNC_TRANSFER_RECEIVER instead
of just CONFIG_BT_CONN.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
The `type` parameter of `struct bt_le_scan_param` is documented as
taking a `BT_LE_SCAN_TYPE_*` value, not a `BT_HCI_LE_SCAN_*` value.
In practice this makes no difference as the values are defined as the
same integer, but does result in `<zephyr/bluetooth/hci.h>` not needing
to be included.
Signed-off-by: Jordan Yates <jordan@embeint.com>
Modified to check the length of the remaining data in buffer
before processing the next report. The length check is missing
in the cont routine.
Signed-off-by: Eunkyu Lee <mochaccino.00.00@gmail.com>
The HCI command LE Read Supported States command returns
if the controller supports running the scanner and initiator
roles in parallel.
This commit utilizes this information in the host:
- It does not prevent initiating a connection when the scanner is
running
- It does not prevent the host from restarting the background
scanner when there the host wants to auto-initiate a connection.
- It does not stop the scanner when the host wants to auto-initiate
a connection.
To support this feature, the scanner and initiator
always have to use the same address.
This because the HCI command LE Set Random Address
cannot be issued after the initiator or scanner has started.
1. When privacy is disabled, the scanner has to use its identity
address to ensure it uses the same address as the initiator.
2. Only one identity is supported.
To simplify the implementation, it is a requirement to use
extended advertising commands to avoid interfering with
the random address used by the advertiser(s).
Also, it is a requirement to never use time-limited scanning,
as RPA this feature does not work when privacy is being used.
See https://github.com/zephyrproject-rtos/zephyr/issues/73634.
The changes in this commit will be tested out of tree as the
Zephyr Bluetooth Controller does not support this functionality.
Signed-off-by: Rubin Gerritsen <rubin.gerritsen@nordicsemi.no>
Returning a status code will allow the application developer
to detect logic issues.
We consider this as not breaking the API.
If `-Werror -Wunused-result` is enabled, the application developer needs
to validate the return code.
Signed-off-by: Rubin Gerritsen <rubin.gerritsen@nordicsemi.no>
Callbacks can only be registered once. Otherwise the slist
will become circular.
In this commit we have choosen to ignore the second registration
call if the callback has already been registed. The alternative
is to trigger an assertion. That doesn't work if the assertions
are turned off.
Signed-off-by: Rubin Gerritsen <rubin.gerritsen@nordicsemi.no>
As documented in bt_le_scan_update(), the host may start scanning
automatically for various reasons.
Until now scanning was only done on 1M PHY, making it impossible
to use auto-connect/sync establishment if the advertiser was using
Coded PHY as its primary PHY.
Auto-connection would never work. Sync establishment would work if
the scanner was started by the application.
This commit fixes this by instructing the controller to scan on
both 1M and Coded PHY if the controller supports Coded PHY.
The application may see increased power consumption as a result
of this change as the controller may now spend 2*scan_window
listening for advertising packets.
Signed-off-by: Rubin Gerritsen <rubin.gerritsen@nordicsemi.no>
This function is used in many places, but just by reading its
name it is not obvious why it is needed.
By adding some documentation it will hopefully become a bit more
clear that this function is mainly used for auto connection
establishment.
Signed-off-by: Rubin Gerritsen <rubin.gerritsen@nordicsemi.no>
To make it easier to understand the code, the following was done:
- Use INITIATING/ADV for state names that are exclusive to central
or peripheral. Previously it was not necessarily clear that the state
BT_CONN_CONNECTING was for central only by just looking at where
it was used. The terms INITIATING/ADV were used in favor
of central and peripheral as these terms also work for SCO connection
establishment.
- BT_CONN_CONNECTING_SCAN -> BT_CONN_SCAN_BEFORE_INITIATING
to make it more clear that we are not scanning and connecting at
the same time. The new name should make it more clear why we are
scanning - only with the intention to start the initiator later.
- BT_CONN_CONNECTING_AUTO -> BT_CONN_INITIATING_FILTER_LIST.
This makes it clear that this state is something different than
BT_CONN_AUTO_CONNECT.
Signed-off-by: Rubin Gerritsen <rubin.gerritsen@nordicsemi.no>
During manual testing with UBSAN enabled, it was found that
bt_le_per_adv_sync_cb synced() callback in application
sometimes gets unexpected different values in
`sync_info.recv_enabled` field.
The UBSAN output contained the following warnings message:
runtime error:
load of value 65, which is not a valid value for type '_Bool'
It tunrned out that struct bt_le_per_adv_sync_synced_info sync_info
inside bt_hci_le_past_received_common() contains uninitialized
recv_enabled member.
Set it to true by default to avoid possible issues.
Signed-off-by: Ivan Iushkov <ivan.iushkov@nordicsemi.no>
during local testling, UBSAN reported the following warnings:
- bluetooth/host/adv.c:2067:19: runtime error: shift exponent
255 is too large for 32-bit type 'long unsigned int'
- bluetooth/host/scan.c:828:18: runtime error: shift exponent
255 is too large for 32-bit type 'long unsigned int'
It turned out that we can't use BIT() macro directly on
bt_hci_evt_le_per_advertising_report::cte_type field.
According to Core Spec, `cte_type = 0xFF` corresponds
to `No contstant tone extension`.
Added separate function to convert CTE bit field from
HCI format to bt_df_cte_type
Signed-off-by: Ivan Iushkov <ivan.iushkov@nordicsemi.no>
Fail gracefully if an HCI event of one type arrives for a handle of a
different connection type. The requested types are currently based on
what fields are used, not on the usage context, in order to keep every
correct use so far still working.
A warning is logged if the connection identified by the handle does not
match the requested connection type.
Signed-off-by: Arkadiusz Kozdra <akozdra@antmicro.com>
The phy was converted both when reading from the event
and when reading from the sync, leading to incorrect
value in the synced callback.
Signed-off-by: Herman Berget <herman.berget@nordicsemi.no>
The reassembly buffer for periodic sync was not initialized
if the sync was established via PAST.
Move the initialization of the reassembly buffer to a common place.
Signed-off-by: Herman Berget <herman.berget@nordicsemi.no>
Adds API for Periodic Advertising with Responses - Scanner:
- Synchronize to a PAwR train
- Choose which subevents to synchronize to
- Receive advertising reports from subevents
- Send responses
The support is enabled by CONFIG_BT_PER_ADV_SYNC_RSP, and requires
a controller that selects CONFIG_BT_CTLR_SYNC_PERIODIC_RSP_SUPPORT.
Signed-off-by: Herman Berget <herman.berget@nordicsemi.no>
When receiving extended advertising reports with incomplete
data status, it is not necessary to mark for recovering from
currently assembled fragments, but rather drop them and
start a fresh assembly of subsequently received extended
advertising reports.
Timing changes in the Controller cause Periodic Advertising
PDUs AUX_SYNC_IND + AUX_CHAIN_IND to be placed between
primary channel ADV_EXT_IND and AUX_ADV_IND. This causes the
Controller to generate alternating incomplete and complete
data status reports, exposing the Host bug that is fixed in
this commit.
Relates to commit ba09a252ec ("bluetooth: host: refactor
bt_hci_le_per_adv_report data reassembly").
Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
When we supply the conn pointer of the remote device that
provides the PAST information, we do a lookup on the
conn_handle using bt_conn_lookup_handle which takes a
reference count. This count is never dereferenced again.
This commit adds calls to bt_conn_unref to ensure that we
do not keep the reference count after the function
returns.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
This gives our users some indication of why they are not receiving
long advertisements.
Signed-off-by: Aleksander Wasaznik <aleksander.wasaznik@nordicsemi.no>
If an event with corrupted length arrives, the extra check makes the
handler return early to avoid reading data out of bounds.
Signed-off-by: Arkadiusz Kozdra <akozdra@antmicro.com>
Refactoring the periodic advertisment report receive function
to make the handling of data reassembly more readable.
Signed-off-by: Pierce Lowe <pierce.lowe@nordicsemi.no>
This changes the periodic scanner to drop periodic adv
reports with incomplete data. This avoids incorrect data
being sent to application in the case where the periodic
adv data is not successfully received by the scanner.
Fixes zephyrproject-rtos#54072
Signed-off-by: Pierce Lowe <pierce.lowe@nordicsemi.no>
allow disabling reports (synchronise but don't generate sync reports)
and allows enabling sync reporting with filtering of duplicates.
the default option remains to establish sync, with sync reports,
but without duplicate filtering.
Signed-off-by: Raphael Treccani-Chinelli <raphael.treccani@nordicsemi.no>
There is special handling done for resolved addresses to convert
them to "regular" addresses for the upper layers.
This commits adds two helper functions to check if they are
resolved, and if so, then properly copied.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
We only expose random/public address types to the upper layers.
This is done by checking if the address type of events are
resolved addresses, and if so, then we translate them to
public/random.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
The `bluetooth/common/log.h` and `bluetooth/common/log.c` files have been
removed. Files that were using them have been updated to use
`zephyr/logging/log.h` instead.
Those replacement have been done consequently:
- `/BT_DBG/LOG_DBG/`
- `/BT_ERR/LOG_ERR/`
- `/BT_WARN/LOG_WRN/`
- `/BT_INFO/LOG_INF/`
- `/BT_HEXDUMP_DBG/LOG_HEXDUMP_DBG/`
- `/BT_DBG_OBJ_ID/LOG_DBG_OBJ_ID/`
Also, some files were relying on the `common/log.h` include to include
`zephyr/bluetooth/hci.h`, in those cases the include of `hci.h` has
been added.
For files that were including `common/log.h` but not using any logs,
the include has been removed and not replaced.
Signed-off-by: Théo Battrel <theo.battrel@nordicsemi.no>
Functions related to string manipulation that were defined in
`common/log.h` has been moved to the `common/bt_str.h` file and their
implementation in `common/bt_str.c`.
Files that were using those functions has been updated consequently.
Signed-off-by: Théo Battrel <theo.battrel@nordicsemi.no>
If using the advertiser list when syncing to a PA, then the
address address needs to be updated for the PA sync object.
This is a small optimization to avoid doing a copy when
the address and SID is already set.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
When deleting a PA sync with bt_le_per_adv_sync_delete
Zephyr should call the `term` callback for the PA sync
as per the documentation for the callback.
This was not done in the case that the PA sync was
terminated by local request.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
There should be functional equivalence between these two forms. And the
'_eq'-form is more readable.
Signed-off-by: Aleksander Wasaznik <aleksander.wasaznik@nordicsemi.no>
Gives the application a two way mapping between array index
and a per_adv_sync object instead of current only per_adv_sync
obj -> index using bt_le_per_adv_sync_get_index.
Signed-off-by: Jakob Krantz <jakob.krantz@u-blox.com>
When using periodic advertising list and receiving
hci_le_per_adv_sync_established event from controller with
an error code the bt_le_per_adv_sync_term_info would be
incorrectly populated with le_addr and sid. This is because
the current pending advertising sync object is not populated
with any le_addr and sid from bt_le_per_adv_sync_create as
those are not used when option
BT_LE_PER_ADV_SYNC_OPT_USE_PER_ADV_LIST is set.
Instead the bt_le_per_adv_sync_term_info shall be populated
with the le_addr and sid coming in the event from controller.
Signed-off-by: Jakob Krantz <jakob.krantz@u-blox.com>
Bluetooth Controller has a vendor specific extensions that allows it
to send IQ report events with IQ samples that are 8 bits or 16 bits
signed integer. To use that functionality, there is added common
handler of vendor specific events.
Vendor specific events handling is prioritized to be done by user
provided event handler. If that is not available, then Host generic
implementation enters.
Added vendor specific events that are handled by common Host code
are BT_HCI_EVT_VS_LE_CONNECTIONLESS_IQ_REPORT and BT_HCI_EVT_VS_LE-
_CONNECTION_IQ_REPORT.
The only difference between regular IQ report events is size of
IQ samples, hence implementation of IQ report events is changed to
use the same user callback. To avoid differentiation of user callbacks
new member sample_type was added to bt_df_per_adv_sync_iq_samples-
_report. Also sample member is changed to be a union, to allow easy
access to IQ samples without type casting.
Signed-off-by: Piotr Pryga <piotr.pryga@nordicsemi.no>
Clear pointer to the le_adv_recv() stack frame before returning to the
calling function. This fixes a potential compiler warning newer gcc
versions.
zephyr/subsys/bluetooth/host/scan.c: In function ‘le_adv_recv’:
zephyr/subsys/bluetooth/host/scan.c:463:20: error: storing the address
of local variable ‘id_addr’ in ‘*info.addr’ [-Werror=dangling-pointer=]
463 | info->addr = &id_addr;
| ~~~~~~~~~~~^~~~~~~~~~
zephyr/subsys/bluetooth/host/scan.c:439:22: note: ‘id_addr’ declared here
439 | bt_addr_le_t id_addr;
| ^~~~~~~
Fixes#48459
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
While execution of bt_disable there is missing de-initialization
of per_adv_sync_pool. If there was a sync in progress, (flags set
to BT_PER_ADV_SYNC_SYNCING) before bt_disable, then when the stack
is re-enabled by bt_enable new sync create fails.
The commit fixes the issue by new function that clears flags for
all per_adv_sync_pool entries. The function is executed by bt_disable.
Signed-off-by: Piotr Pryga <piotr.pryga@nordicsemi.no>
