zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.71k stars 6.54k forks source link

Bluetooth: controller: workaround to use DLE activation with multiple connections #51509

Open lanwer50 opened 2 years ago

lanwer50 commented 2 years ago

Is your enhancement proposal related to a problem? Please describe.

I am opening this issue to further discuss a workaround here instead of on discord, so that the information is available for others.

As workaround before the issue #21995 is closed, I want to use the DLE with multiple connections(20) in parallel using zephyr v3.2.0.

Describe the solution you'd like

A suggestion from @cvinayak on discord was to set the max tx and rx time: https://github.com/zephyrproject-rtos/zephyr/blob/183370277b41d1f92fa56fd9c6c56ee864145977/subsys/bluetooth/controller/ll_sw/ull_central.c#L238-L239.

In Order to do that I had to activate CONFIG_BT_LL_SW_LLCP_LEGACY=y in prj.conf. Any problems to expect with this config? I think it also makes sense to edit the max number tx and rx octes to PDU_DC_PAYLOAD_SIZE_MAX.

So that part of the code looks as follows now:

`#if defined(CONFIG_BT_LL_SW_LLCP_LEGACY)

if defined(CONFIG_BT_CTLR_DATA_LENGTH)

conn_lll->max_tx_octets = PDU_DC_PAYLOAD_SIZE_MAX;
conn_lll->max_rx_octets = PDU_DC_PAYLOAD_SIZE_MAX;

if defined(CONFIG_BT_CTLR_PHY)

/* Use the default 1M packet Tx time, extended connection initiation
 * in LLL will update this with the correct PHY.
 */
conn_lll->max_tx_time = PDU_DC_PAYLOAD_TIME_MAX;
conn_lll->max_rx_time = PDU_DC_PAYLOAD_TIME_MAX;

endif / CONFIG_BT_CTLR_PHY /

endif / CONFIG_BT_CTLR_DATA_LENGTH /

else / CONFIG_BT_LL_SW_LLCP_LEGACY /`

@cvinayak : any comments on this?

Describe alternatives you've considered A clear and concise description of any alternative solutions or features you've considered.

Additional context Add any other context or graphics (drag-and-drop an image) about the feature request here.

lanwer50 commented 1 year ago

Along with using lower priorities for other SoC peripherals (SERIAL) by adding the line below #define NRF_DEFAULT_IRQ_PRIORITY 5 inside the application folder in dts/arm/nordic/override.dtsi file, I am facing on the central side a few Asserts every few hours (2-24hrs).

Application description: The central has 20s time to connect to as many as 20 peripherals and pull data from them. Then the central disconnects randomly a few of the peripherals.

Connection parameters:

define SCAN_INTERVAL 0x00d0 / 130 ms /

define SCAN_WINDOW 0x0010 / 10 ms /

define INIT_INTERVAL 0x0010 / 10 ms /

define INIT_WINDOW 0x0010 / 10 ms /

define CONN_INTERVAL 0x0068 / 130 ms /

define CONN_LATENCY 0

define CONN_TIMEOUT 400

Any idea what might be the reason?

cvinayak commented 1 year ago

Please send a draft pull request with your changes to controller code to better understand the consequences of the changes. The suggested workaround is to only change the time reservation for the connections, not to change the parameters (max octets or max time) in the connection context structures.

Add:

    max_tx_time = PDU_DC_PAYLOAD_TIME_MAX; /* 2120 us */
    max_rx_time = PDU_DC_PAYLOAD_TIME_MAX; /* 2120 us */

before: https://github.com/zephyrproject-rtos/zephyr/blob/c022dd7756209b9ea20228757829ea13b362f7ee/subsys/bluetooth/controller/ll_sw/ull_central.c#L472-L477

lanwer-en commented 1 year ago

I created a draft pull request as you suggested (https://github.com/zephyrproject-rtos/zephyr/pull/51648) and removed the previous changes (changing the parameters (max octets or max time) in the connection context structures).

I will check if only changing the time reservation for the connections would help and I will let you know either way. But for now: could you take a look at the draft PR to check/understand the consequences of the changes?

lanwer-en commented 1 year ago

@cvinayak : I unfortunately got the following Assert after 34 hours: ASSERTION FAIL @ WEST_TOPDIR/zephyr/subsys/bluetooth/host/att.c:794

I have 2 questions:

cvinayak commented 1 year ago

I unfortunately got the following Assert after 34 hours: ASSERTION FAIL @ WEST_TOPDIR/zephyr/subsys/bluetooth/host/att.c:794

Please create a new github issue for this.

  • Should I try to use CONFIG_BT_LL_SW_LLCP_LEGACY=y. I have no idea if that would help, I have just saw your issue where this config might help #51693

No, #51693 does not occur on target as the h/w radio peripheral truncates the PDU on reception and does not cause any invalid behaviors, but may be connection timeouts.

  • The changes to the controller code to use DLE with multiple connections are only needed on the central side, right? Or should the peripheral also have these changes.

Yes, central tries to scheduling each of n connection with the reserved spacing between them to form a group in the timeline. Peripherals having only one connection do not need the workaround, the connection event are connection interval apart anyway.

carlescufi commented 1 year ago

@cvinayak : I unfortunately got the following Assert after 34 hours: ASSERTION FAIL @ WEST_TOPDIR/zephyr/subsys/bluetooth/host/att.c:794

Can you please create a separate issue (bug report) for this? Before that I suggest you bump stack sizes before posting it just to make sure it's not a stack overflow.

lanwer-en commented 1 year ago

@carlescufi:

@cvinayak : I unfortunately got the following Assert after 34 hours: ASSERTION FAIL @ WEST_TOPDIR/zephyr/subsys/bluetooth/host/att.c:794

Can you please create a separate issue (bug report) for this? Before that I suggest you bump stack sizes before posting it just to make sure it's not a stack overflow.

will do that. Do you have any particular threads/ stack sizes in mind? (20 connections are needed)

carlescufi commented 1 year ago

@lanwer-en not really, no. Just play around with numbers.

zephyrbot commented 8 months ago

Hi @cvinayak, @jhedberg,

This issue, marked as an Enhancement, was opened a while ago and did not get any traction. It was just assigned to you based on the labels. If you don't consider yourself the right person to address this issue, please re-assing it to the right person.

Please take a moment to review if the issue is still relevant to the project. If it is, please provide feedback and direction on how to move forward. If it is not, has already been addressed, is a duplicate, or is no longer relevant, please close it with a short comment explaining the reason.

@lanwer50 you are also encouraged to help moving this issue forward by providing additional information and confirming this request/issue is still relevant to you.

Thanks!