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

samples: Bluetooth: multiple central connections to peripherals #30735

Merged
merged 5 commits into from
Jul 21, 2021

Conversation

cvinayak
Copy link
Contributor

@cvinayak cvinayak commented Dec 15, 2020

This sample and babblesim test following issues:

  1. Uncovers an out-of-bound memory access in controller related to ULL reference count accessed after ULL context was released, assertion in ull.c file Bluetooth: Controller: Out-of-Bound ULL context access during connection completion #35013
  2. New connection establishments fail with reason 0x3E when peripheral does not except connection indication, suspected to be related to PHY update or Connection Update being in progress
  3. MTU exchange fails when performing on simultaneous connections, this could be due to deadlock in the peripheral as single device used, but stil worth investigating
  4. SMP pairing failures when performing simultaneous pairing procedures on multiple connections, this could be due to deadlock in the peripheral as single device used, but stil worth investigating
  5. Assertion in hci_core.c:312

Application demonstrating BLE Central role functionality by scanning for other
BLE devices and establishing connection to upto 62 peripherals with a strong
enough signal.

Signed-off-by: Vinayak Kariappa Chettimada vich@nordicsemi.no

Below is the CONFIG_BT_CTLR_DEBUG_PINS=y logic analyzer capture of closely scheduled 62 BLE connections of 200ms connection interval between two nRF52840 DK:
image

Sniffer trace (simulated nrf52_bsim target):
image

sniffer trace file:
SixtyTwoConnections.zip

@cvinayak cvinayak force-pushed the github_central_multilink branch 2 times, most recently from e3f8352 to d827e73 Compare December 15, 2020 13:58
@cvinayak cvinayak marked this pull request as ready for review December 16, 2020 04:44
@cvinayak cvinayak force-pushed the github_central_multilink branch from d827e73 to 4f30d83 Compare December 17, 2020 02:06
@cvinayak cvinayak marked this pull request as draft December 17, 2020 02:07
@cvinayak
Copy link
Contributor Author

cvinayak commented Dec 17, 2020

Converted back to draft, as following issues observed, :

  1. Deadlock/assert in HCI command status/response in hci_core.c: Guessing here, possible race condition attempting to stop advertising while LE connection parameter request is being responded to by host? (This PR has auto connection update disabled in the peripheral sample for this reason).
  2. PHY update procedure on few connections end up with supervision timeout, it is observed that packets are being nack-ed. Fixed by Bluetooth: controller: Fix free Rx PDU queue starvation #30807

Need analysis.

@cvinayak cvinayak force-pushed the github_central_multilink branch from 4f30d83 to 94000c6 Compare December 17, 2020 07:23
@cvinayak cvinayak requested review from mtpr-ot and wopu-ot December 17, 2020 07:31
@cvinayak cvinayak force-pushed the github_central_multilink branch from 94000c6 to 385170b Compare December 17, 2020 09:56
Copy link
Collaborator

@mtpr-ot mtpr-ot left a comment

Choose a reason for hiding this comment

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

LGTM

@cvinayak cvinayak added the DNM This PR should not be merged (Do Not Merge) label Feb 15, 2021
@carlescufi
Copy link
Member

@cvinayak can we move this out of draft?

@cvinayak cvinayak force-pushed the github_central_multilink branch 2 times, most recently from 91df080 to f869034 Compare February 24, 2021 05:09
@cvinayak cvinayak marked this pull request as ready for review February 24, 2021 05:12
@cvinayak cvinayak removed the DNM This PR should not be merged (Do Not Merge) label Feb 24, 2021
@joerchan
Copy link
Contributor

@cvinayak I think this would be better as a test. We still have these two issues related to using the identities API:
#31588
#22049

I would prefer to not use the identities API in samples until these are resolved.
I also think using the API in this way is kind of hacky in order to establish multiple connections between the same devices, which shouldn't be done, so I don't think it works good as a sample.

@cvinayak cvinayak force-pushed the github_central_multilink branch from f869034 to 7d129bc Compare May 9, 2021 02:15
@github-actions github-actions bot added the area: Tests Issues related to a particular existing or missing test label May 9, 2021
@cvinayak cvinayak requested a review from mtpr-ot May 9, 2021 02:17
@cvinayak cvinayak removed the DNM This PR should not be merged (Do Not Merge) label May 9, 2021
@cvinayak cvinayak marked this pull request as ready for review May 9, 2021 02:36
@cvinayak cvinayak requested a review from aescolar as a code owner May 9, 2021 02:36
@cvinayak cvinayak force-pushed the github_central_multilink branch from 7d129bc to d2fc453 Compare May 9, 2021 02:45
cvinayak added a commit to cvinayak/zephyr that referenced this pull request May 9, 2021
Fix advertiser and scanning context being accessed on done
event when connection complete node rx that is processed
earlier has release them.

Relates to zephyrproject-rtos#30735.
Fixes zephyrproject-rtos#35013.

Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
carlescufi pushed a commit that referenced this pull request May 10, 2021
Fix advertiser and scanning context being accessed on done
event when connection complete node rx that is processed
earlier has release them.

Relates to #30735.
Fixes #35013.

Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
@cvinayak cvinayak force-pushed the github_central_multilink branch from d2fc453 to 81a4568 Compare May 19, 2021 07:01
@cvinayak cvinayak added this to the v2.7.0 milestone May 21, 2021
@cvinayak cvinayak force-pushed the github_central_multilink branch from 81a4568 to 019cf65 Compare May 21, 2021 11:50
@cvinayak cvinayak force-pushed the github_central_multilink branch 2 times, most recently from 009920c to ba5f76b Compare June 5, 2021 01:20
@cvinayak cvinayak force-pushed the github_central_multilink branch 2 times, most recently from ebea32a to 15ebba6 Compare June 18, 2021 09:51
@cvinayak cvinayak force-pushed the github_central_multilink branch from 15ebba6 to 3538905 Compare July 10, 2021 21:15
@cvinayak cvinayak added the Enhancement Changes/Updates/Additions to existing features label Jul 13, 2021
@cvinayak cvinayak force-pushed the github_central_multilink branch from 3538905 to 13fcd9b Compare July 20, 2021 11:18
cvinayak added 5 commits July 21, 2021 15:20
Increase the maximum allowed identity range to 64.

Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
Added sample to demonstrate use of multiple identity and be
able to be connected to multiple times from same central
device.

Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
Application demonstrating Bluetooth Low Energy Central role
functionality by scanning for other devices and establishing
connection to upto 62 peripherals with a strong enough
signal.

Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
Babblesim test of Bluetooth Low Energy Central role
functionality by scanning for other devices and establishing
connection to upto 62 peripherals with a strong enough
signal.

Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
Option to have continous scanning simultaneously while
advertising and multiple peripheral role is active.

Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
@cvinayak cvinayak force-pushed the github_central_multilink branch from 13fcd9b to 58f8ba5 Compare July 21, 2021 09:55
@cfriedt cfriedt merged commit ac82fbf into zephyrproject-rtos:main Jul 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Bluetooth Controller area: Bluetooth Host Bluetooth Host (excluding BR/EDR) area: Bluetooth area: Documentation area: Samples Samples area: Tests Issues related to a particular existing or missing test Enhancement Changes/Updates/Additions to existing features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants