Skip to content

Bluetooh: Host: conn: Resume adv after disconn for periph role only#51256

Merged
carlescufi merged 1 commit intozephyrproject-rtos:mainfrom
ppryga-nordic:github-ble-fix-multirole-resume-adv-after-disconnect
Oct 17, 2022
Merged

Bluetooh: Host: conn: Resume adv after disconn for periph role only#51256
carlescufi merged 1 commit intozephyrproject-rtos:mainfrom
ppryga-nordic:github-ble-fix-multirole-resume-adv-after-disconnect

Conversation

@ppryga-nordic
Copy link
Contributor

@ppryga-nordic ppryga-nordic commented Oct 13, 2022

Zephyrs Host has by default enabled automatic resume of advertising in case of disconnection when peripheral role is enabled.

The feature becomes a bit problematic in case of multirole usage. For example assume a use case where a device is working as peripheral and central, where it may establish single connection for each role.

In case there are two connections established and connection in central role is dropped by peer, Host will automatically resume advertising. After that an application can resume scanning, e.g. in disconnected callback. That should not happen.

If one of connections was used for central role, it should not be stolen by Host to run peripheral role. Host should verify if a disconnected connection role was peripheral and then resume advertising.

This approach will not break backward compatibility and change correct resume behavior. What more, Host will follow an application decisions about use of connection objects.

Partially fixes: #50438

Signed-off-by: Piotr Pryga piotr.pryga@nordicsemi.no

Thalley
Thalley previously approved these changes Oct 13, 2022
@ppryga-nordic ppryga-nordic added bug The issue is a bug, or the PR is fixing a bug area: Bluetooth Host Bluetooth Host (excluding BR/EDR) labels Oct 13, 2022
jhedberg
jhedberg previously approved these changes Oct 13, 2022
@ppryga-nordic
Copy link
Contributor Author

CI failing due to non-PR related issue. @ahmedmoheb-nordic is looking on it.

@ahmedmoheb-nordic
Copy link
Contributor

ahmedmoheb-nordic commented Oct 14, 2022

CI failing due to non-PR related issue. @ahmedmoheb-nordic is looking on it.

This test is using zassume_true which was marking the test case as skipped without affecting the final test result.
That behavior of zassume_true was changed by this a40a2f5.
While it still marks the test as skipped, but it now affects the final test result.

I created a PR which fixes this problem by combining ztest_test_skip() with a conditional 'if' statement to restore the required logic
#51293

Zephyrs Host has by default enabled automatic resume of advertising
in case of disconnection when peripheral role is enabled.

The feature becomes a bit problematic in case of multirole usage.
For example assume a use case where a device is working as peripheral
and central, where it may establish single connection for each role.

In case there are two connections established and connection in
central role is dropped by peer, Host will automatically resume
advertising. After that an application can resume scanning, e.g.
in disconnected callback. That should not happen.

If one of connections was used for central role, it should not
be stolen by Host to run peripheral role. Host should verify
if a disconnected connection role was peripheral and then resume
advertising.

This approach will not break backward compatibility and change
correct resume behavior. What more, Host will follow an application
decisions about use of connection objects.

Signed-off-by: Piotr Pryga <piotr.pryga@nordicsemi.no>
@ppryga-nordic ppryga-nordic dismissed stale reviews from jhedberg and Thalley via 372c8f2 October 14, 2022 11:21
@ppryga-nordic ppryga-nordic force-pushed the github-ble-fix-multirole-resume-adv-after-disconnect branch from 9618878 to 372c8f2 Compare October 14, 2022 11:21
@ppryga-nordic
Copy link
Contributor Author

Rebased because CI fix PR has been merged.
@Thalley @jhedberg please re-approve the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: Bluetooth Host Bluetooth Host (excluding BR/EDR) area: Bluetooth bug The issue is a bug, or the PR is fixing a bug

6 participants