tests: Bluetooth: CAP and GMAP: Add TX/RX for AC tests #83539
Conversation
Thalley
force-pushed
the
cap_bsim_tx
branch
2 times, most recently
from
cfa46c2 to
e20d3e2
Compare
Thalley
force-pushed
the
cap_bsim_tx
branch
2 times, most recently
from
8161fd2 to
a458b6b
Compare
zephyrbot
added
area: Bluetooth Audio
platform: nRF BSIM
zephyrbot
requested review from
Casper-Bonde-Bose,
MariuszSkamra,
aescolar,
asbjornsabo,
babrsn,
fredrikdanebjer,
jhedberg,
jthm-ot,
kruithofa,
larsgk,
niym-ot,
pin-zephyr and
sjanc
There was a problem hiding this comment.
This commit is disabling quite many tests. Do you have an associated bug report you can link from here and the commit message?
|
@Thalley (i'm quite out of the loop here, so sorry for what is probably a silly question but) given that this PR is only modifying tests and needs to disable quite many, is it better to wait until the issues you pointed at are resolved? |
Maybe. The problem is that we see more and more controlled-related issues that is starting to block host development. So we can either block host development and wait for someone to fix the controller issues, or we undo some of the tests so that host development can continue and the whomever will work on the controller can more easily work on and fix the reported errors. The CAP and GMAP AC tests are sort of special types of test in the sense that they are technically application/host tests, but whether the test passes or fails is exclusively up to the controller and/or the use of the controller. So arguably these tests test the controller more than the host, but is blocking some host development. #81093 is one such case where some host changes triggers some controller issues. I do not know what the correct way is, but blocking host PRs for months due to the currently low activity in the controller isn't great. @cvinayak may also have an opinion here :) |
|
Rebased to solve merge conflicts |
|
Rebased to solve merge conflicts |
aescolar
left a comment
aescolar
left a comment
There was a problem hiding this comment.
+0
I'm ok with disabling tests which trigger issues in the controller (though I'd be happier if there was a link in each disabled test to the github issue),
For the audio changes somebody who is closer should review there.
I'll add that :) Thanks |
|
The audio configurations and the implemented test use overlapping connection or periodic intervals with the ISO events; large no. of subevents make the ISO event lengths large enough to overlap the other roles. (Zephyr Controller does not implement yield and resume of sub-events, experimental implementation here: #75760) These tests are passing on the main branch on a specific timing in the tests timeline, and as the timings are being changed in this PR leading to different simulated devices interacting at differing time in the timeline, the tests are susceptible to overlapping events. MIC failures, bugs in framed SDUs and assertions have GH issues already created. These test being disabled in this PR is ok. |
Added references and additional GH issue for the issues found :) |
Thalley
force-pushed
the
cap_bsim_tx
branch
3 times, most recently
from
8cdf0e5 to
99ff8cc
Compare
Add TX and RX verification for the audio configuration tests. This requires modifying some of the underlying structures used in those tests, as well as initializating and triggering TX, and with verification of RX as well. These tests were implemented to ensure that the streams are not just established, but can send ISO data without issues. Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
6 participants
Add TX and RX verification for the audio configuration tests. This requires modifying some of the underlying structures used in those tests, as well as initializating and triggering TX, and with verification of RX as well.
These tests were implemented to ensure that the streams are not just established, but can send ISO data without issues.