Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bluetooth: Host: Selection of cherry-picks #1877

Merged

Conversation

rugeGerritsen
Copy link
Contributor

This PR contains a selection of cherry-picks to support concurrent scanning and initiating.
In addition, this PR adds the following commits:

  • Commits that fixes bugs related to concurrent scanning and initiating or bugfixes required to verify this feature.
  • Commits that ensure the other commits can be applied without merge conflicts.
  • Simple useability improvements.

The commits were selected from the outputs from the commands:

git rev-list HEAD...upstream/main --grep "[Bb]luetooth: [Hh]ost" --cherry-pick --oneline --reverse

and

git rev-list HEAD...upstream/main [email protected] --cherry-pick --oneline --reverse

@rugeGerritsen rugeGerritsen force-pushed the bt-host-cherry-picks-concurrent-scan-init branch from f6b55a1 to d5b1642 Compare July 8, 2024 08:38
@rugeGerritsen
Copy link
Contributor Author

Removed commits that relied on the new test bluetooth/smp. That test uses CONFIG_BT_H4=n which was introduced in the PR that creates a new driver interface. To avoid dragging in too much, these commits were removed for now.

rugeGerritsen and others added 26 commits July 10, 2024 16:16
…Connection Subrating"

This reverts commit f6325a5.

Signed-off-by: Rubin Gerritsen <[email protected]>
This patch removes all uses of the adv auto-resume feature in the host
bsim tests, except for the test of that feature itself, and instead
makes all adv starts explicit.

The auto-resume feature is planned for deprecation. And, explicit
starting of adv makes what happens in the test more explicit as well.

Signed-off-by: Aleksander Wasaznik <[email protected]>
(cherry picked from commit 765b244)
…_disable()

Expectation: After calling `bt_disable()` it is possible to
use the Bluetooth APIs as if `bt_enable()` was never called.

This was not the case for `bt_id_create()`, it was not possible
to set the default identity. This prevented an application
developer to restart the stack as a different identity.

Keys also need to be cleared to avoid the following pattern:
1. Pair two devices
2. Central calls `bt_disable()` and `bt_enable()`.
   The central will now generate a new identity address.
3. Connect the two devices.
4. Re-establish encryption. Now the central will try to use
   the previously used keys. The procedure will fail
   because the peripheral does not have any keys associated
   with the new central address.

The API documentation is updated accordingly.

Signed-off-by: Rubin Gerritsen <[email protected]>
(cherry picked from commit 3ce106c)
… timeout

Before this commit, the following bugs were present:
- When `CONFIG_BT_FILTER_ACCEPT_LIST` was set, connection establishment
  was cancelled upon RPA timeout. This required the application
  to restart the initiator every RPA timeout.
- When `CONFIG_BT_FILTER_ACCEPT_LIST` was not set, the RPA was not updated
  while the initiator was running.

This commit unifies the RPA timeout handling for both these cases.
Upon RPA timeout the initiator is cancelled and restarted when
the controller raises the LE Connection Complete event.
The workqueue state is checked when restarting the initiator to prevent
it being restarted when the timeout is hit.

Corresponding test cases have been added to ensure that this
feature works.

Signed-off-by: Rubin Gerritsen <[email protected]>
(cherry picked from commit ff80c0b)
When using the post_init_f to initialize the `bst_result`,
it is not possible to mark the test as
passed immediately as the `bst_result` will be
initialized after the test completes.

This change should overcome this limitation.

Bluetooth mesh tests are kept as is as we are not
sure if this will change the behavior.

Signed-off-by: Rubin Gerritsen <[email protected]>
(cherry picked from commit 1aa33fe)
When reading the error message:
"ASSERTION_FAIL: command opcode 0x0c03 timeout with err -11" it may not be
obvious what is wrong with their setup unless you are very familiar
with HCI.

This commit adds some more documentation to make this more obvious.

Signed-off-by: Rubin Gerritsen <[email protected]>
(cherry picked from commit 3609d97)
These macros allows us to compare strings in a simpler way.

Signed-off-by: Rubin Gerritsen <[email protected]>
(cherry picked from commit a35d5e7)
…meout

It seemed like there was lacking test coverage for this
functionality.

Signed-off-by: Rubin Gerritsen <[email protected]>
(cherry picked from commit b30d2d1)
… RPA timeout

zephyrproject-rtos/zephyr#72674 fixed
a bug where this configuration did not work.

Now that this configuration is tested, we should mark it
as supported.

The timeout check that was present in the code before
was useless and was not working because the check was
run before a default timeout of 0 was converted to a timeout.

Signed-off-by: Rubin Gerritsen <[email protected]>
(cherry picked from commit 9cf6839)
…llbacks

A naive implementation would look like

```c
void bt_init()
{
  bt_le_conn_cb_register();
  bt_enable();
}
```

When the app later adds the possibility to enable and disable
Bluetooth, it may happen that the application developer calls
`bt_init()` instead of `bt_enable()`. This results in invalid
behavior. This kind of bug is currently a bit harder to debug
as the callback register APIs do not reject registering a
callback twice.

Improving the API documentation is the first step towards making
this a bit more user friendly.

Signed-off-by: Rubin Gerritsen <[email protected]>
(cherry picked from commit 2712b32)
This commit adds host support for the Path Loss Monitoring
feature see Bluetooth Core specification, Version 5.4,
Vol 6, Part B, Section 4.6.32.

Limited logic is required, just adding a wrapper around the
HCI command and callback for HCI event.

Add new zone - BT_CONN_LE_PATH_LOSS_ZONE_UNAVAILABLE, to
convert 0xFF path loss to a useful zone.

Add new Kconfigs and functionality to the bt shell.

Signed-off-by: Sean Madigan <[email protected]>
(cherry picked from commit 0b327db)
This allows us to use functionality provided by slist.
First use case: Avoid adding an element twice.

Signed-off-by: Rubin Gerritsen <[email protected]>
(cherry picked from commit 6e6bb26)
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 <[email protected]>
(cherry picked from commit 2ec3cd3)
This improves consistency with other callback lists like
scan_cbs and pa_sync_cbs.

Signed-off-by: Rubin Gerritsen <[email protected]>
(cherry picked from commit 3eb975d)
…urn status

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 <[email protected]>
(cherry picked from commit 5098bf3)
…nning

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 zephyrproject-rtos/zephyr#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 <[email protected]>
(cherry picked from commit 9984adf)
…cros

When brackets are used in macros, there may sometimes be a space in
front of them. The checkpatch script should allow this.
The change includes the example that triggered the need for this
change.

Signed-off-by: Rubin Gerritsen <[email protected]>
(cherry picked from commit f78c51d)
This API converts a HCI error code to a string.
This can be useful if application developers want to print them
in the applications.

Later we can also use them in the host to improve debuggability.

Signed-off-by: Rubin Gerritsen <[email protected]>
(cherry picked from commit bfba19d)
We want to able to use cocinelle on ztest functions as
well when transforming APIs.

Provide a simple macro so that test functions are recognized.

Signed-off-by: Rubin Gerritsen <[email protected]>
(cherry picked from commit fc37736)
…ssertions

ztest now provides functionality to compare strings.
These are simpler to use than the strcmp ways.

The semantic patch transforms many of the commonly used patterns.
It does not handle variable length macros.

Signed-off-by: Rubin Gerritsen <[email protected]>
(cherry picked from commit 470a0fc)
… timeout

This makes it more clear what is happening when the host cancels
connection creation where the controller raises
the connection complete event with status set to
"UNKNOWN CONNECTION IDENTIFIER (0x02)".

This is especially useful for developers not familiar with this
detail in the spec.

See also: Core_v5.4, Vol 4, Part E, Section 7.8.13,
LE Create Connection Cancel command.

Signed-off-by: Rubin Gerritsen <[email protected]>
(cherry picked from commit 1b33616)
This can be useful if application developers
want to print them in the applications.

Later we can also use them in
the host to improve debuggability.

Signed-off-by: Rubin Gerritsen <[email protected]>
(cherry picked from commit 94d712e)
The function reuses the ATT implementation.
To make the function simpler to use, the function handles both positive
and negative values.

Unfortunately the APIs do not document if the API returns an
errno val or a GATT return value.

Signed-off-by: Rubin Gerritsen <[email protected]>
(cherry picked from commit 4e30803)
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 <[email protected]>
(cherry picked from commit cf870e8)
…IVACY=y

See comment in code.

Fixes #73634

Signed-off-by: Jonathan Rico <[email protected]>
(cherry picked from commit be61ae4)
…on Subrating

Add Kconfigs to enable experimental subrating HCI commands.

Signed-off-by: Timothy Keys <[email protected]>
(cherry picked from commit 570c86d)
@rugeGerritsen rugeGerritsen force-pushed the bt-host-cherry-picks-concurrent-scan-init branch from d5b1642 to 66cd439 Compare July 10, 2024 14:19
@rugeGerritsen
Copy link
Contributor Author

Resolved the conflict by first reverting the commit that generated the conflict and applying the reverted commit again later so that they appear in the same order as in upstream zephyr.

(As per manual)

@rlubos rlubos merged commit 00266aa into nrfconnect:main Jul 11, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants