Bluetooth: Controller: Fix missing ISO Receiver address capture #95348

cvinayak · 2025-09-02T15:47:51Z

Fix missing ISO Receiver access address capture that caused ISO PDU reception error. Missing access address capture for subsequent subevent when prior subevent did not have an anchor point sync prevent proper drift compensation hence causing ISO PDU reception error for the entire BIG event.

Fix the jitter considered in the PDU reception in the subevents of ISO Sync Receiver to not exceed such that the reception is scheduled late (causing assertion).

Fixes #90097.

Overnight testing between bap_broadcast_source on nrf52833dk and bap_broadcast_sink on nrf54l15dk:

[2025-09-03 06:09:34.562] [00:02:52.835,401] <inf> stream_rx: [2593000]: Incoming audio on stream 0x2000246c len 40, flags 0x09, seq_num 37095 and ts 172835214: Valid 2593000 | Error 0 | Loss 0 | Dup TS 0 | Dup PSN 0
[2025-09-03 06:09:34.579] [00:02:52.835,459] <inf> stream_rx: [2593000]: Incoming audio on stream 0x200024b8 len 40, flags 0x09, seq_num 37095 and ts 172835214: Valid 2593000 | Error 0 | Loss 0 | Dup TS 0 | Dup PSN 0
[2025-09-03 06:09:44.562] [00:03:02.835,921] <inf> stream_rx: [2594000]: Incoming audio on stream 0x2000246c len 40, flags 0x09, seq_num 38095 and ts 182835728: Valid 2594000 | Error 0 | Loss 0 | Dup TS 0 | Dup PSN 0
[2025-09-03 06:09:44.579] [00:03:02.835,981] <inf> stream_rx: [2594000]: Incoming audio on stream 0x200024b8 len 40, flags 0x09, seq_num 38095 and ts 182835728: Valid 2594000 | Error 0 | Loss 0 | Dup TS 0 | Dup PSN 0
[2025-09-03 06:09:54.562] [00:03:12.836,436] <inf> stream_rx: [2595000]: Incoming audio on stream 0x2000246c len 40, flags 0x09, seq_num 39095 and ts 192836243: Valid 2595000 | Error 0 | Loss 0 | Dup TS 0 | Dup PSN 0
[2025-09-03 06:09:54.579] [00:03:12.836,494] <inf> stream_rx: [2595000]: Incoming audio on stream 0x200024b8 len 40, flags 0x09, seq_num 39095 and ts 192836243: Valid 2595000 | Error 0 | Loss 0 | Dup TS 0 | Dup PSN 0
[2025-09-03 06:10:04.562] [00:03:22.836,945] <inf> stream_rx: [2596000]: Incoming audio on stream 0x2000246c len 40, flags 0x09, seq_num 40095 and ts 202836757: Valid 2596000 | Error 0 | Loss 0 | Dup TS 0 | Dup PSN 0
[2025-09-03 06:10:04.579] [00:03:22.837,003] <inf> stream_rx: [2596000]: Incoming audio on stream 0x200024b8 len 40, flags 0x09, seq_num 40095 and ts 202836757: Valid 2596000 | Error 0 | Loss 0 | Dup TS 0 | Dup PSN 0
[2025-09-03 06:10:14.562] [00:03:32.837,465] <inf> stream_rx: [2597000]: Incoming audio on stream 0x2000246c len 40, flags 0x09, seq_num 41095 and ts 212837272: Valid 2597000 | Error 0 | Loss 0 | Dup TS 0 | Dup PSN 0
[2025-09-03 06:10:14.579] [00:03:32.837,524] <inf> stream_rx: [2597000]: Incoming audio on stream 0x200024b8 len 40, flags 0x09, seq_num 41095 and ts 212837272: Valid 2597000 | Error 0 | Loss 0 | Dup TS 0 | Dup PSN 0
[2025-09-03 06:10:24.562] [00:03:42.837,979] <inf> stream_rx: [2598000]: Incoming audio on stream 0x2000246c len 40, flags 0x09, seq_num 42095 and ts 222837786: Valid 2598000 | Error 0 | Loss 0 | Dup TS 0 | Dup PSN 0
[2025-09-03 06:10:24.580] [00:03:42.838,037] <inf> stream_rx: [2598000]: Incoming audio on stream 0x200024b8 len 40, flags 0x09, seq_num 42095 and ts 222837786: Valid 2598000 | Error 0 | Loss 0 | Dup TS 0 | Dup PSN 0
[2025-09-03 06:10:34.562] [00:03:52.838,488] <inf> stream_rx: [2599000]: Incoming audio on stream 0x2000246c len 40, flags 0x09, seq_num 43095 and ts 232838301: Valid 2599000 | Error 0 | Loss 0 | Dup TS 0 | Dup PSN 0
[2025-09-03 06:10:34.579] [00:03:52.838,546] <inf> stream_rx: [2599000]: Incoming audio on stream 0x200024b8 len 40, flags 0x09, seq_num 43095 and ts 232838301: Valid 2599000 | Error 0 | Loss 0 | Dup TS 0 | Dup PSN 0
[2025-09-03 06:10:44.562] [00:04:02.839,008] <inf> stream_rx: [2600000]: Incoming audio on stream 0x2000246c len 40, flags 0x09, seq_num 44095 and ts 242838816: Valid 2600000 | Error 0 | Loss 0 | Dup TS 0 | Dup PSN 0
[2025-09-03 06:10:44.580] [00:04:02.839,067] <inf> stream_rx: [2600000]: Incoming audio on stream 0x200024b8 len 40, flags 0x09, seq_num 44095 and ts 242838816: Valid 2600000 | Error 0 | Loss 0 | Dup TS 0 | Dup PSN 0
[2025-09-03 06:10:54.562] [00:04:12.839,523] <inf> stream_rx: [2601000]: Incoming audio on stream 0x2000246c len 40, flags 0x09, seq_num 45095 and ts 252839329: Valid 2601000 | Error 0 | Loss 0 | Dup TS 0 | Dup PSN 0
[2025-09-03 06:10:54.580] [00:04:12.839,583] <inf> stream_rx: [2601000]: Incoming audio on stream 0x200024b8 len 40, flags 0x09, seq_num 45095 and ts 252839329: Valid 2601000 | Error 0 | Loss 0 | Dup TS 0 | Dup PSN 0

Fix the jitter considered in the PDU reception in the subevents of ISO Sync Receiver to not exceed such that the reception is scheduled late (causing assertion). Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>

Fix missing ISO Receiver access address capture that caused ISO PDU reception error. Missing access address capture for subsequent subevent when prior subevent did not have an anchor point sync prevent proper drift compensation hence causing ISO PDU reception error for the entire BIG event. Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>

sonarqubecloud · 2025-09-02T21:17:53Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Thalley · 2025-09-03T06:53:24Z

subsys/bluetooth/controller/ll_sw/nordic/lll/lll_sync_iso.c

+
+		LL_ASSERT(hcto > jitter_us);
+


Funny; was about to suggest this in my last review, but noticed that it wasn't there before either, so I didn't :) Glad to see it added

Well, I noticed that a newly added assert was caught in CI for high number of subevents which I had to convert to if-then-else; hence, decided to add a assert here as well.

Do not hesitate to leave a comment, your reviews help think differently, and hence better solutions.

That said, I do not like the calculations here for the jitter, it is always calculating for nse i.e. from the first subevent; ideally, current jitter should be from the last sync-ed subevent.

The problem to solve, active clock is permitted a +/-2 us jitter which means over the duration of the subevents, a subevent can jitter by +/- 2 us multiplied by current subevent count nse when none of the previous subevent where received (anchor point sync).

I will comeback and solve it some other day, for now, bsim is happy; my other changes does expose too little jitter or drift causing assertion... fix for that another day.

Consider creating a GH issue for the above so you don't forget it ;)

Not going to forget it, listed (updated with the above problem statement now) in #82399

Copilot

Pull Request Overview

This PR fixes ISO Receiver functionality by ensuring proper access address capture and correcting jitter calculations in Bluetooth Low Energy controller code. The fix addresses ISO PDU reception errors that occurred when previous subevents lacked anchor point synchronization.

Key changes:

Add access address capture for subsequent subevents when no prior anchor point sync exists
Refactor jitter calculation logic to prevent reception scheduling delays and assertions
Remove RANGE_DELAY_US from jitter calculations to improve timing accuracy

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-03T19:21:30Z

subsys/bluetooth/controller/ll_sw/nordic/lll/lll_sync_iso.c

 		jitter_max_us = (EVENT_IFS_US - overhead_us) >> 1;
-		jitter_max_us -= RANGE_DELAY_US + HAL_RADIO_TMR_START_DELAY_US;
+		jitter_max_us = (jitter_max_us * nse) / (lll->num_bis * lll->nse);


The expression can be simplified to jitter_max_us = jitter_max_us / lll->num_bis since nse appears in both numerator and denominator and cancels out.

Suggested change

jitter_max_us = (jitter_max_us * nse) / (lll->num_bis * lll->nse);

jitter_max_us = jitter_max_us / lll->num_bis;

No, nse and lll->nse are different values.

Copilot · 2025-09-03T19:21:30Z

subsys/bluetooth/controller/ll_sw/nordic/lll/lll_sync_iso.c

 		overhead_us = radio_rx_chain_delay_get(lll->phy, PHY_FLAGS_S8);
 		overhead_us += addr_us_get(lll->phy);
 		overhead_us += radio_rx_ready_delay_get(lll->phy, PHY_FLAGS_S8);
 		overhead_us += (EVENT_CLOCK_JITTER_US << 1);
+
+		LL_ASSERT(EVENT_IFS_US > overhead_us);


[nitpick] Consider adding descriptive error messages to these assertions to help with debugging when they fail. For example: LL_ASSERT_MSG(EVENT_IFS_US > overhead_us, \"IFS time insufficient for overhead\");

Suggested change

LL_ASSERT(EVENT_IFS_US > overhead_us);

LL_ASSERT_MSG(EVENT_IFS_US > overhead_us, "IFS time insufficient for overhead");

Do not want a message.

subsys/bluetooth/controller/ll_sw/nordic/lll/lll_sync_iso.c

cvinayak marked this pull request as ready for review September 2, 2025 18:12

cvinayak requested a review from Thalley September 2, 2025 18:13

zephyrbot added area: Bluetooth area: Bluetooth Controller labels Sep 2, 2025

zephyrbot requested review from carlescufi, erbr-ot, ppryga, thoh-ot, Tronil and wopu-ot September 2, 2025 18:13

zephyrbot assigned cvinayak Sep 2, 2025

Thalley previously approved these changes Sep 2, 2025

View reviewed changes

cvinayak dismissed Thalley’s stale review via 7d36bdb September 2, 2025 19:41

cvinayak force-pushed the github_sync_iso_aa_capture_fix branch from 27eaa22 to 7d36bdb Compare September 2, 2025 19:41

Thalley previously approved these changes Sep 2, 2025

View reviewed changes

cvinayak dismissed Thalley’s stale review via e314b28 September 2, 2025 20:17

cvinayak force-pushed the github_sync_iso_aa_capture_fix branch from 7d36bdb to e314b28 Compare September 2, 2025 20:17

cvinayak added 2 commits September 2, 2025 23:12

Bluetooth: Controller: Fix ISO Sync Receiver subevent jitter

1273947

Fix the jitter considered in the PDU reception in the subevents of ISO Sync Receiver to not exceed such that the reception is scheduled late (causing assertion). Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>

cvinayak force-pushed the github_sync_iso_aa_capture_fix branch from e314b28 to ff7e93a Compare September 2, 2025 21:13

Thalley approved these changes Sep 3, 2025

View reviewed changes

cvinayak mentioned this pull request Sep 3, 2025

Bluetooth: Controller: Place holder for a list of known assertions #82399

Open

14 tasks

cvinayak requested a review from Copilot September 3, 2025 19:20

Copilot AI reviewed Sep 3, 2025

View reviewed changes

carlescufi approved these changes Sep 8, 2025

View reviewed changes

kartben merged commit aefeeb9 into zephyrproject-rtos:main Sep 8, 2025
30 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Bluetooth: Controller: Fix missing ISO Receiver address capture #95348

Bluetooth: Controller: Fix missing ISO Receiver address capture #95348

Uh oh!

cvinayak commented Sep 2, 2025 •

edited

Loading

Uh oh!

sonarqubecloud bot commented Sep 2, 2025

Uh oh!

Thalley Sep 3, 2025

Uh oh!

cvinayak Sep 3, 2025

Uh oh!

Thalley Sep 3, 2025

Uh oh!

cvinayak Sep 3, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Sep 3, 2025

Uh oh!

cvinayak Sep 7, 2025

Uh oh!

Copilot AI Sep 3, 2025

Uh oh!

cvinayak Sep 7, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

	jitter_max_us = (jitter_max_us * nse) / (lll->num_bis * lll->nse);
	jitter_max_us = jitter_max_us / lll->num_bis;

	LL_ASSERT(EVENT_IFS_US > overhead_us);
	LL_ASSERT_MSG(EVENT_IFS_US > overhead_us, "IFS time insufficient for overhead");

Bluetooth: Controller: Fix missing ISO Receiver address capture #95348

Bluetooth: Controller: Fix missing ISO Receiver address capture #95348

Uh oh!

Conversation

cvinayak commented Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sonarqubecloud bot commented Sep 2, 2025

Quality Gate passed

Uh oh!

Thalley Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

cvinayak Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

Thalley Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

cvinayak Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

cvinayak Sep 7, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

cvinayak Sep 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cvinayak commented Sep 2, 2025 •

edited

Loading