-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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
tests: bsim: Bluetooth: Enable some high reliability CAP tests #80788
tests: bsim: Bluetooth: Enable some high reliability CAP tests #80788
Conversation
9c535d8
to
913f41d
Compare
0e48d6e
to
d05a193
Compare
d05a193
to
58705a3
Compare
Failure log snippet in CI run that is different from local run:
Local run PA Sync quicker:
|
81c48ba
to
a706cf5
Compare
c133cd2
to
4950663
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since these test scripts are already taking a long time to finish given the many test cases in each of them, we should consider either running them in parallel in this script somehow, or split the files.
We could either split them like
AC_14_8 -> Run all 8_x_x tests
AC_14_16 -> Run all 16_x_x tests
AC_14_24 -> Run all 24_x_x tests
etc.
or split them by high and light reliability.
4950663
to
211084d
Compare
211084d
to
5522ef0
Compare
5522ef0
to
58ddb33
Compare
2f1a9f0
to
8498046
Compare
8498046
to
51b5ac5
Compare
# In theory, CONFIG_BT_ISO_TX_BUF_COUNT=1, should be sufficient but this count | ||
# is used in the context of IPC which falls into a "Newton's Cradle" effect | ||
# where probably (CONFIG_BT_CTLR_ISO_TX_BUFFERS - CONFIG_BT_ISO_TX_BUF_COUNT) | ||
# buffers get throttled. Hence, always have the value equal or greater. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest to create the GH issue and reference it here rather than having this comment all the places :) Then it's easier to find and cleanup later
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still on my todo list to create a IPC test!...
Just not to loose track created this: #81866
|
||
#define BROADCAST_STREMT_CNT CONFIG_BT_BAP_BROADCAST_SRC_STREAM_COUNT | ||
#define BROADCAST_ENQUEUE_COUNT 2U | ||
#define BROADCAST_ENQUEUE_COUNT 18U |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This refers to the number of SDUs to enqueue - 18 seems excessive (180ms of audio data). Is that intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
50 ms ISO interval with PTO=1 (BN=5 + 1) needs to have 12 SDUs buffered at an ISO interval and additional 6 SDUs to ensure subsequent ISO intervals have SDUs ready in the Controller. This is equivalent to how 3 Tx ACL buffers are required to ensure a peripheral ACL sends data every connection interval.
@@ -51,7 +51,7 @@ CONFIG_BT_BAP_BROADCAST_SNK_STREAM_COUNT=2 | |||
CONFIG_BT_ISO_PERIPHERAL=y | |||
CONFIG_BT_ISO_MAX_CHAN=4 | |||
CONFIG_BT_ISO_TX_MTU=310 | |||
CONFIG_BT_ISO_TX_BUF_COUNT=4 | |||
CONFIG_BT_ISO_TX_BUF_COUNT=36 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yikes :D I assume this is 2 x BROADCAST_ENQUEUE_COUNT
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, 2x due to 2 BISes support (though test only uses 1 BIS) and the build error check in the test needs this 2x.
Fix ISO Sync Receiver implementation to correctly reflect the payload number and timestamp for the skipped SDU. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
Fix ISO Sync Receiver time reservation calculation to use peer broadcasted bis_spacing and sub_interval, instead of incorrectly calculating using local implementation used tMSS value. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
Fix ISO Sync Receiver implementation to correctly prevent subevent from pre-empted in the unreserve time space. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
Align audio test Controller Kconfig value same as used with nRF53bsim. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
Enable some high reliability CAP tests by increasing ISO Tx buffer counts in the Controller to sufficiently generate number of complete when multiple SDUs are transmitted in single ISO interval with use of pre-transmissions. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
51b5ac5
to
366f050
Compare
Enable some high reliability CAP tests by increasing ISO Tx buffer counts in the Controller to sufficiently generate number of complete when multiple SDUs are transmitted in single ISO interval with use of pre-transmissions.