bluetooth: tester: audio: Add verification for bis stopping #79329
Conversation
zephyrbot
added
area: Bluetooth Qualification
zephyrbot
requested review from
asbjornsabo,
Casper-Bonde-Bose,
fredrikdanebjer,
jhedberg,
jthm-ot,
kruithofa,
larsgk,
MariuszSkamra,
niym-ot,
pin-zephyr,
sjanc and
Thalley
Frodevan
force-pushed
the
oct-3161
branch
2 times, most recently
from
5874234
to
1c326c0
Compare
Frodevan
changed the title
| err = k_sem_take(&sem_bis_stopped, K_MSEC(2000)); | ||
| if (err != 0) { | ||
| LOG_DBG("Timed out waiting to stop BIS stream"); | ||
|
|
||
| return BTP_STATUS_FAILED; | ||
| } |
There was a problem hiding this comment.
stream_stopped that gives sem_bis_stopped may be called multiple times.
You should take the same number of semaphores as we have streams.
There was a problem hiding this comment.
This should now be resolved, if bt_bap_broadcast_source_stop stops broadcast source streams only.
Frodevan
force-pushed
the
oct-3161
branch
3 times, most recently
from
443be52
to
d2a09d6
Compare
|
#AutoPTS run zephyr BAP/BSRC/SCC/BV-34-C |
| err = bt_bap_broadcast_source_stop(source->bap_broadcast); | ||
| if (err != 0) { | ||
| LOG_DBG("Unable to stop broadcast source: %d", err); | ||
|
|
||
| return BTP_STATUS_FAILED; | ||
| } | ||
|
|
||
| for (int i = 0; i < ARRAY_SIZE(source->streams); i++) { |
There was a problem hiding this comment.
this should check for actual number of streams, not max supported, shouldn't it?
There was a problem hiding this comment.
Right, must this be done by checking stream->in_use, or is this number stored somewhere in the source struct?
There was a problem hiding this comment.
Right, must this be done by checking stream->in_use, or is this number stored somewhere in the source struct?
I don't think we currently store it anywhere, but it's pretty easy to just add a counter in the struct btp_bap_broadcast_remote_source so that we always know how many sem_stream_stopped we should expect if we ever start fewer than ARRAY_SIZE(source->streams) streams
There was a problem hiding this comment.
I added the count to the struct.
One observation along the way, I am not sure setup_broadcast_source in btp_bap_broadcast is supposed to set up subgroups and streams separately like it is doing, unless every broadcast source stream is shared among each subgroup.. However, this makes me unsure if stream_stopped will be called streams_per_subgroup times, or streams_per_subgroup multiplied with the number of subgroups times. Of course, this is not relevant if number of subgroups is always 1.
There was a problem hiding this comment.
stream_stopped will be called per stream, regardless of the number of subgroups
|
Scheduled PR #79329 (comment), board: nrf53, estimated start time: 13:34:00, test case count: 1, estimated duration: 0:00:20 Test cases to be runBAP/BSRC/SCC/BV-34-C |
|
AutoPTS Bot results: Successful tests (1)BAP BAP/BSRC/SCC/BV-34-C PASS |
Adds a semaphor to make sure the stream has properly been stopped by the controller before returning from btp_bap_broadcast_source_stop. Signed-off-by: Frode van der Meeren <[email protected]>
|
My reasoning for the timeout is that I have not checked if there is another timeout for this call in Auto-PTS, so a potential infinite wait here will have far greater consequences for a (typically very long) test run than just an error-code returned during a test-case. |
Adds a semaphor to make sure the stream has properly been stopped by the
controller before returning from btp_bap_broadcast_source_stop.
fixes: #73411