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.61k stars 6.5k forks source link

Bluetooth: Controller: Assert when creating CIG for unidir 7.5ms CIS #59605

Closed Thalley closed 1 year ago

Thalley commented 1 year ago

Describe the bug Using the BAP 16_1_1 image

hits an assert when using the hci_rpmsg sample with the nrf5340_cpunet_iso-bt_ll_sw_split.conf.

To Reproduce Steps to reproduce the behavior:

  1. Checkout https://github.com/zephyrproject-rtos/zephyr/pull/59107
  2. Build and flash hci_rpmsg with the above .conf file to the nRF5340 netcore
  3. Build and flash the audio shell to the appcore

On the peripheral do:

bt init sync
bap init
bt advertise on

On the central do

bt init sync
bap init
bap preset sink 16_1_1
bt connect <addr of peripheral>
bap discover
cap_initiator discover
cap_initiator unicast-start sources 0

That should trigger a HCI timeout on the appcore and

ASSERTION FAIL [c_latency <= cig->c_latency] @ WEST_TOPDIR/zephyr/subsys/bluetooth/controller/ll_sw/ull_central_iso.c:480

on the netcore. This is not triggered by the BSIM test in https://github.com/zephyrproject-rtos/zephyr/pull/59073, but it is unclear if it is due to the patches in https://github.com/zephyrproject-rtos/zephyr/pull/59107 or due to bsim vs nRF5340.

Expected behavior Expect the request to create the CIG to pass or be rejected with an error instead of an assert.

Impact Showstopper

Logs and console output See attached log. The CIG parameters can be seen by

[00:02:15.638,732] <dbg> bt_iso: hci_le_set_cig_params: id 0, latency 8, interval 7500, sca 0, packing 0, framing 0, num_cis 1
[00:02:15.638,763] <dbg> bt_iso: hci_le_set_cig_params: [0]: id 0, c_phy 2, c_sdu 30, c_rtn 2, p_phy 2, p_sdu 0, p_rtn 0

central.txt

Environment (please complete the following information):

Additional context N/A

cvinayak commented 1 year ago

@mtpr-ot Is this something you could check why the central does not support BAP 16_1_1 ?

mtpr-ot commented 1 year ago

@mtpr-ot Is this something you could check why the central does not support BAP 16_1_1 ?

Yes, I can take a look. May take a couple of days.

Thalley commented 1 year ago

@mtpr-ot Is this something you could check why the central does not support BAP 16_1_1 ?

Yes, I can take a look. May take a couple of days.

Keep in mind that this is not always the case, and as mentioned in the issue, it cannot be reproduced by BSIM

mtpr-ot commented 1 year ago

I can reproduce the issue in simulation. The latency is determined to be too high because the algorithm wants to set FT=2. I will try to figure out what goes wrong.

mtpr-ot commented 1 year ago

I can reproduce the issue in simulation. The latency is determined to be too high because the algorithm wants to set FT=2. I will try to figure out what goes wrong.

I think there is a CEIL/DIV_ROUND_UP mistake in the calculation of the FT. Will run a fix by @thoh-ot for confirmation.

github-actions[bot] commented 1 year ago

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

mtpr-ot commented 1 year ago

This is fixed by #61534