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.

Sign up for GitHub

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

Jump to bottom

Bluetooth: controller: Add support for initiator on secondary adv channels #34353

Merged
merged 17 commits into from
May 5, 2021
Merged

Bluetooth: controller: Add support for initiator on secondary adv channels #34353

merged 17 commits into from
May 5, 2021

Conversation

andrzej-kaczmarek
Copy link
Collaborator

@andrzej-kaczmarek andrzej-kaczmarek commented Apr 16, 2021 •
edited by cvinayak
Loading

This adds support to initiate connection on secondary adv channels. Should pass all LL/CON/INI/* test cases.

Fixes #3514

@andrzej-kaczmarek
Copy link
Collaborator Author

The only problem I noticed on qualification test cases is that 1st connection event is sometimes sent too early - usually several usecs. Initiator on legacy adv has the same problem so seems like some calculations are a bit off, but I was not able to figure it out yet.

@cvinayak cvinayak self-assigned this Apr 20, 2021
@andrzej-kaczmarek andrzej-kaczmarek force-pushed the bt-init-secch branch 4 times, most recently from 9c27f15 to 829ffef Compare April 23, 2021 09:57
cvinayak
cvinayak requested changes Apr 25, 2021
View reviewed changes
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.

Good work. Minor regression otherwise looks good to me.

subsys/bluetooth/controller/ll_sw/nordic/lll/lll_scan.c Outdated Show resolved Hide resolved
@github-actions
Copy link

Bluetooth Test Results

  1 files    1 suites   0s ⏱️
29 tests 29 ✔️ 0 💤 0 ❌

Results for commit 0f3c1e57.

@cvinayak cvinayak self-requested a review April 27, 2021 02:04
@cvinayak cvinayak requested a review from wopu-ot April 28, 2021 01:21
@cvinayak
Copy link
Contributor

I have discovered during testing that there has always been a bug during connection establishment when it comes to stopping the initiating scan events, continuous scanning could use prepare pipeline and the pipeline is not flushed under this scenario. Now I remember, the previously used is_stop flag in ULL hdr was getting used such that the scan events in pipeline got aborted, but this flag no longer exists (not that I want it back, it was incorrect to use it in first place).

The correct fix will be:

  1. Remove the is_enabled = 0U; reset when handling NODE_RX_TYPE_CONNECTION
  2. Remove the reset of lll_scan->conn = NULL; being done in ull_master_setup. This will allow scan events to abort early when connection has already been aborted
  3. Hold on to NODE_RX_TYPE_CONNECTION from being available to LL context until the scan event is done and disabled_cb is called.

Will think more on this.

@cvinayak cvinayak force-pushed the bt-init-secch branch 3 times, most recently from 4e247e3 to 47e3ed7 Compare April 28, 2021 08:47
@github-actions github-actions bot added the area: Tests Issues related to a particular existing or missing test label Apr 29, 2021
@andrzej-kaczmarek andrzej-kaczmarek marked this pull request as ready for review April 29, 2021 11:06
@github-actions github-actions bot added the area: API Changes to public APIs label Apr 29, 2021
cvinayak
cvinayak approved these changes Apr 29, 2021
View reviewed changes
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.

Have tested on target to satisfactory results. LGTM,

@cvinayak
Copy link
Contributor

cvinayak commented May 3, 2021

@andrzej-kaczmarek please rebase and push to trigger CI, I think the build failure is unrelated.

carlescufi
carlescufi approved these changes May 3, 2021
View reviewed changes
Copy link
Member

@carlescufi carlescufi left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM

andrzej-kaczmarek and others added 17 commits May 5, 2021 09:55
@andrzej-kaczmarek
Bluetooth: controller: Refactor CONNECT_IND prepare to separate func
100264f 
We can use the same code to create both CONNECT_IND and AUX_CONNECT_REQ
since they are basically that same PDUs.

Signed-off-by: Andrzej Kaczmarek <[email protected]>
@andrzej-kaczmarek
Bluetooth: controller: Add public helpers for scan checks
8a9293e 
We will need the same checks for AdvA and TargetA/InitA in lll_scan_aux
so let's make them public and use address explicitly instead of passed
via pdu.

Signed-off-by: Andrzej Kaczmarek <[email protected]>
@andrzej-kaczmarek
Bluetooth: controller: Use proper transmitWindowDelay for central
0e5ebaf 
On secondary advertising channel transmitWindowDelay depends on PHY
used to create connection so we need to adjust it.

Signed-off-by: Andrzej Kaczmarek <[email protected]>
@andrzej-kaczmarek
Bluetooth: controller: Fix 1st connection event offset
e7176d0 
We need to take RX chain delay and PHY used to send CONNECT_IND into
account when calculating 1st connection event offset.

Signed-off-by: Andrzej Kaczmarek <[email protected]>
@andrzej-kaczmarek
Bluetooth: controller: Fix aux scanning for connectable ADV_EXT_IND
cd9a4d1 
When in initiating state we need to handle ADV_EXT_IND as in regular
scan sice we always want to scan AUX_ADV_IND in order to be able to
connect.

Signed-off-by: Andrzej Kaczmarek <[email protected]>
@andrzej-kaczmarek
Bluetooth: controller: Add support for initiator on secondary adv ch
a268745 
This allows to initiate a connection on secondary advertising channels,
i.e. when advertises uses advertising extensions.

Signed-off-by: Andrzej Kaczmarek <[email protected]>
@cvinayak @andrzej-kaczmarek
Bluetooth: controller: Ensure only initiating on selected PHY
34e6e86 
Reset the scanning context for the PHY not selected in the
Extended Create Connection command.

Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
@cvinayak @andrzej-kaczmarek
Bluetooth: controller: Fixes and use of defines related to initiator
0f87a6e 
Fixed incorrect LLL context used to generate done event for
auxiliary channel scanning. Other minor comments and fixes.

Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
@cvinayak @andrzej-kaczmarek
Bluetooth: controller: Rename use of params to param
fef88bd 
Rename the use of params word with param.

Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
@cvinayak @andrzej-kaczmarek
Bluetooth: controller: Reserve an additional node rx for Ext. Initiator
df474b8 
Reserve an additional node rx buffer when Extended Initiator
is supported as the received ADV_EXT_IND PDU is being
buffered in the auxiliary channel scanning instance and is
only released/flushed in the done event of the initiating
auxiliary channel radio event.

Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
@cvinayak @andrzej-kaczmarek
Bluetooth: controller: Implement disabling the other PHY initiator
f971969 
When connection is initiated in one of either 1M or Coded
PHY initiating scan instance then the other scanning
instance's scheduling and memory allocation needs to be
cleaned up.

Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
@cvinayak @andrzej-kaczmarek
Bluetooth: controller: Ensure both 1M and Coded PHY can initiate
0f46440 
Ensure that the advertiser address type and address is setup
in both 1M and Coded PHY scanning instance when extended
create connection is enabled on both the PHYs.

Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
@cvinayak @andrzej-kaczmarek
Bluetooth: controller: Do not report Aux PDUs while initiating
dccc00c 
Do not report auxiliary PDUs as advertising reports when in
initiator state on auxiliary channels.

Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
@andrzej-kaczmarek
Bluetooth: host: Add adv parameters def for connectable ext adv
6bfb58b 
This is the same as for legacy, but with ext adv flag.

Signed-off-by: Andrzej Kaczmarek <[email protected]>
@andrzej-kaczmarek
tests: Bluetooth: bsim: Fix conn ref
a827bd7 
'disconnected' callback always unrefs conn object, but this is only
valid if connection was created in scanx since app owns reference to
that conn object. In case of advx, app does not own any reference so
should not unref.

Basically we do not really need to hold any reference to conn object
in either scanx or advx so just drop reference immediately after conn
is created in scanx, no need to worry about it later.

Signed-off-by: Andrzej Kaczmarek <[email protected]>
@andrzej-kaczmarek
tests: Bluetooth: bsim: Handle consecutive connection in tests
42c4a08 
If we want to test conn on legacy and ext adv, we need scanx to
handle connection more than once. Change local flag to global and
let it indicate whether we want scanx to initaite connection when
device is scanned.

Signed-off-by: Andrzej Kaczmarek <[email protected]>
@andrzej-kaczmarek
tests: Bluetooth: bsim: Add test for initaiting conn on ext adv
b034081 
advx will do connectable advertising using ext adv, scanx will try
to connect.

Signed-off-by: Andrzej Kaczmarek <[email protected]>
@carlescufi carlescufi merged commit 5652d59 into zephyrproject-rtos:master May 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cvinayak cvinayak cvinayak approved these changes

@carlescufi carlescufi carlescufi approved these changes

@wopu-ot wopu-ot Awaiting requested review from wopu-ot

@aescolar aescolar Awaiting requested review from aescolar

@jhedberg jhedberg Awaiting requested review from jhedberg

@joerchan joerchan Awaiting requested review from joerchan

@kruithofa kruithofa Awaiting requested review from kruithofa

@thoh-ot thoh-ot Awaiting requested review from thoh-ot

@Vudentz Vudentz Awaiting requested review from Vudentz

Assignees

@cvinayak cvinayak

Labels
area: API Changes to public APIs area: Bluetooth Controller area: Bluetooth area: Tests Issues related to a particular existing or missing test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Bluetooth: controller: LE Advertising Extensions
3 participants
@andrzej-kaczmarek @cvinayak @carlescufi