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: controller: Fix ull_conn LSTO for must_expire #30132

Merged

Conversation

mtpr-ot
Copy link
Collaborator

@mtpr-ot mtpr-ot commented Nov 19, 2020

When CONFIG_BT_CTLR_CONN_META is enabled and the ticker "must_expire"
feature is used, collisions may cause incorrect decrement of the
supervision_expire counter, resulting in a too early link supervision
timeout.

When CONFIG_BT_CTLR_CONN_META is enabled and the ticker "must_expire"
feature is used, collisions may cause incorrect decrement of the
supervision_expire counter, resulting in a too early link supervision
timeout.

Signed-off-by: Morten Priess <mtpr@oticon.com>
@mtpr-ot mtpr-ot changed the title Bluetooth: controller: Fix ull_conn LSTO for must_expire and random force on lost anchor configurable Bluetooth: controller: Fix ull_conn LSTO for must_expire and make random force on lost anchor configurable Nov 19, 2020
@mtpr-ot
Copy link
Collaborator Author

mtpr-ot commented Nov 20, 2020

Can't seem to get CI passing - looks unrelated.

@mtpr-ot mtpr-ot marked this pull request as ready for review November 20, 2020 11:56
Copy link
Contributor

@cvinayak cvinayak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Separate into two different PRs, so that I can approve the Random Forced scheduling Kconfig.

Regarding is_must_expire, please add a simple upstream test to cover it, so that any future upstream regressions get caught of the features that not enabled by default in upstream samples.

@@ -353,6 +353,16 @@ config BT_CTLR_CONN_RSSI
help
Enable connection RSSI measurement.

config BT_CTLR_CONN_RANDOM_FORCE
bool "Enable random forced sceduling for peripheral on missed anchor point"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
bool "Enable random forced sceduling for peripheral on missed anchor point"
bool "Random forced scheduling for peripheral on missed anchor point"

Comment on lines 361 to 364
When enabled, controller will have legacy behavior and force priority at
next ticker scheduling, if anchor point is missed.
Note that this overrides general priority- and round-robin connection
scheduling for peripheral.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
When enabled, controller will have legacy behavior and force priority at
next ticker scheduling, if anchor point is missed.
Note that this overrides general priority- and round-robin connection
scheduling for peripheral.
When enabled, controller will have legacy behavior and randomly force
priority at next ticker scheduling for peripheral role, if anchor point is
missed.
Two are more connections with similar interval on a device connected
to a peer device having two or more connections at its end with same
interval could lock to a round robin pattern where in neither of the central
nor peripheral event would be in sync at either end. Randomness allows
to break this locked round robin pattern permitting an anchor point sync.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Split into two PRs

@mtpr-ot mtpr-ot added the DNM This PR should not be merged (Do Not Merge) label Nov 22, 2020
@mtpr-ot mtpr-ot changed the title Bluetooth: controller: Fix ull_conn LSTO for must_expire and make random force on lost anchor configurable Bluetooth: controller: Fix ull_conn LSTO for must_expire Nov 23, 2020
@mtpr-ot
Copy link
Collaborator Author

mtpr-ot commented Nov 23, 2020

Separate into two different PRs, so that I can approve the Random Forced scheduling Kconfig.

Regarding is_must_expire, please add a simple upstream test to cover it, so that any future upstream regressions get caught of the features that not enabled by default in upstream samples.

Can this be a part of a later PR? I need to figure out how to create a test running on upstream code, without support for the "must_expire" feature in Nordic ULL. Ant ideas?

@carlescufi carlescufi merged commit 392e044 into zephyrproject-rtos:master Dec 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Bluetooth DNM This PR should not be merged (Do Not Merge)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants