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

pkg/nimble: fix the event queue size for nimble_adv_ext #18467

Merged

Conversation

gschorcht
Copy link
Contributor

Contribution description

This PR increases the event queue size to fix a bug if module nimble_adv_ext is used.

If the Bluetooth 5 Advertising Extension is enabled by the nimble_adv_ext module, the 3 events

LE Enhanced Connection Complete event
LE Channel Selection Algorithm event
LE Advertising Set Terminated event

come in from the controller during connection setup before they are processed by the host. The default size of the event queue MYNEWT_VAL_BLE_TRANSPORT_EVT_COUNT with only 2 entries is too small and the LE Advertising Set Terminated event is lost. Therefore, each connection setup fail. Increasing the event queue size MYNEWT_VAL_BLE_TRANSPORT_EVT_COUNT to 4 solves the problem.

The problem was figured out when investigating the problems for tests/nimble_*_ext in PR #18439.

Testing procedure

Flash two nRF52x nodes with

BOARD=nrf52... make -C tests/nimble_autoconn_gnrc_ext/ flash term

an check the results. Without this PR, the connection can't be established and command ble info shows the following results on both nodes

Own Address: F4:4E:D7:67:1B:2A -> [FE80::F44E:D7FF:FE67:1B2A]
Supported PHY modes: 1M 2M CODED
 Free slots: 2/3
Advertising: yes
Connections: 0
Slots:
[ 0] state: 0x0100 - advertising
[ 1] state: 0x8000 - unused
[ 2] state: 0x8000 - unused

With this PR, the connection should be established and command ble info should show the following results.

Node1:

Own Address: F4:4E:D7:67:1B:2A -> [FE80::F44E:D7FF:FE67:1B2A]
Supported PHY modes: 1M 2M CODED
 Free slots: 1/3
Advertising: yes
Connections: 1
[ 0] DD:67:34:05:2D:D6 [FE80::DD67:34FF:FE05:2DD6] (S,75ms,2500ms,0,1M)
     (role, conn itvl, superv. timeout, slave latency, PHY)
Slots:
[ 0] state: 0x0022 - GAP-slave L2CAP-server
[ 1] state: 0x0100 - advertising
[ 2] state: 0x8000 - unused

Node 2:

Own Address: DD:67:34:05:2D:D6 -> [FE80::DD67:34FF:FE05:2DD6]
Supported PHY modes: 1M 2M CODED
 Free slots: 1/3
Advertising: yes
Connections: 1
[ 0] F4:4E:D7:67:1B:2A [FE80::F44E:D7FF:FE67:1B2A] (M,75ms,2500ms,0,1M)
     (role, conn itvl, superv. timeout, slave latency, PHY)
Slots:
[ 0] state: 0x0011 - GAP-master L2CAP-client
[ 1] state: 0x0100 - advertising
[ 2] state: 0x8000 - unused

Issues/PRs references

Prerequisite for PR #18439

If the Bluetooth 5 Advertising Extension is enabled by the `nimble_adv_ext` module, up to 3 events come in from the controller during connection establishment before they are processed by the host. The default size of the event queue `MYNEWT_VAL_BLE_TRANSPORT_EVT_COUNT` with only 2 entries is therefore too small and the connection establishment fails.
@github-actions github-actions bot added Area: BLE Area: Bluetooth Low Energy support Area: pkg Area: External package ports labels Aug 18, 2022
@gschorcht gschorcht added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Aug 18, 2022
@maribu maribu added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Aug 18, 2022
@maribu
Copy link
Member

maribu commented Aug 18, 2022

I fast-tracked #18466 as all other PRs would fail anyway until that gets merged. Sorry for the delay.

@gschorcht gschorcht added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Aug 18, 2022
@benpicco benpicco requested a review from chrysn August 18, 2022 09:17
@MrKevinWeiss MrKevinWeiss added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Aug 18, 2022
@benpicco benpicco added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Aug 18, 2022
@benpicco benpicco enabled auto-merge August 18, 2022 11:59
@benpicco benpicco merged commit 3a6dac4 into RIOT-OS:master Aug 18, 2022
@gschorcht
Copy link
Contributor Author

Thanks for reviewing and merging.

@gschorcht gschorcht deleted the pkg/nimble/fix_event_queue_size_adv_ext branch August 18, 2022 20:30
@maribu maribu added this to the Release 2022.10 milestone Oct 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: BLE Area: Bluetooth Low Energy support Area: pkg Area: External package ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants