Hi Pauli, On Tue, May 30, 2023 at 10:59 AM Pauli Virtanen <pav@xxxxxx> wrote: > > Hi Luiz, > > ti, 2023-05-30 kello 10:35 -0700, Luiz Augusto von Dentz kirjoitti: > > On Sun, May 28, 2023 at 10:47 AM Pauli Virtanen <pav@xxxxxx> wrote: > > > > > > The Client may terminate a CIS when sink is in QOS and source in > > > Disabling states (BAP v1.0.1 Sec 5.6.5). It may also terminate it when > > > Receiver Stop Ready has completed successfully (BAP v1.0.1 Sec 5.6.5.1). > > > On successful Receiver Stop Ready the Server shall transition the ASE > > > back to QoS state (ASCS v1.0 Sec 5.6). > > > > > > It appears Samsung Galaxy Buds2 Pro (R510XXUOAWA5) ignores the Receiver > > > Stop Ready command if CIS is already disconnected, and then gets stuck > > > in disabling state. It works if CIS is disconnected after Receiver Stop > > > Ready. > > > > > > For better compatibility, disconnect CIS only after the source ASE is > > > back in the QoS state. This is what we also do with sinks. > > > > > > Link: https://github.com/bluez/bluez/issues/516 > > > --- > > > src/shared/bap.c | 20 ++------------------ > > > 1 file changed, 2 insertions(+), 18 deletions(-) > > > > > > diff --git a/src/shared/bap.c b/src/shared/bap.c > > > index f194f466f..16a9cec5b 100644 > > > --- a/src/shared/bap.c > > > +++ b/src/shared/bap.c > > > @@ -1115,18 +1115,6 @@ static bool match_stream_io(const void *data, const void *user_data) > > > return stream->io == io; > > > } > > > > > > -static void stream_stop_disabling(void *data, void *user_data) > > > -{ > > > - struct bt_bap_stream *stream = data; > > > - > > > - if (stream->io || stream->ep->state != BT_ASCS_ASE_STATE_DISABLING) > > > - return; > > > - > > > - DBG(stream->bap, "stream %p", stream); > > > - > > > - bt_bap_stream_stop(stream, NULL, NULL); > > > -} > > > - > > > static bool bap_stream_io_detach(struct bt_bap_stream *stream) > > > { > > > struct bt_bap_stream *link; > > > @@ -1145,9 +1133,6 @@ static bool bap_stream_io_detach(struct bt_bap_stream *stream) > > > /* Detach link if in QoS state */ > > > if (link->ep->state == BT_ASCS_ASE_STATE_QOS) > > > bap_stream_io_detach(link); > > > - } else { > > > - /* Links without IO on disabling state shall be stopped. */ > > > - queue_foreach(stream->links, stream_stop_disabling, NULL); > > > } > > > > > > stream_io_unref(io); > > > @@ -1218,7 +1203,6 @@ static void bap_stream_state_changed(struct bt_bap_stream *stream) > > > bap_stream_update_io_links(stream); > > > break; > > > case BT_ASCS_ASE_STATE_DISABLING: > > > - bap_stream_io_detach(stream); > > > break; > > > case BT_ASCS_ASE_STATE_QOS: > > > if (stream->io && !stream->io->connecting) > > > @@ -1252,8 +1236,8 @@ static void bap_stream_state_changed(struct bt_bap_stream *stream) > > > bt_bap_stream_start(stream, NULL, NULL); > > > break; > > > case BT_ASCS_ASE_STATE_DISABLING: > > > - if (!bt_bap_stream_get_io(stream)) > > > - bt_bap_stream_stop(stream, NULL, NULL); > > > + /* IO is detached when back in QOS */ > > > + bt_bap_stream_stop(stream, NULL, NULL); > > > break; > > > > Note that doing this way makes our peripheral/server role detach by > > itself because it will end up calling stream_stop, so perhaps we need > > to add a check if we acting as a client or not, that said if we do it > > Ack, as server we shall not transition to stop here. -> For v2 > > > late don't we risk the server not sending QOS until ISO is dropped? So > > perhaps we also need to detect that somehow and drop the ISO socket if > > the peripheral stays on DISABLING for too long (e.g. 2 sec) after > > stop. > > BAP does not appear to require terminating the CIS, either after > entering disabling or after sending stop. My reading of the spec is > that it is explicitly allowed to do it after Stop Ready. > > But it's possible some devices misbehave in the opposite way and > require terminating CIS in Disabling and not after it. I can in add > some timeout mechanism for stalled transitions in v2, and use it here > to detach first and then send another Stop Ready. > > Relevant parts: > > BAP (Sec. 5.6.5): > """ > If a Source ASE is in the Disabling state, and/or if a Sink ASE is in > the QoS Configured state, the Unicast Client or the Unicast Server may > terminate a CIS established for that ASE by following the Connected > Isochronous Stream Terminate procedure defined in Volume 3, Part C, > Section 9.3.15 in [1]. Termination of the CIS is not required. > """ > > BAP (Sec. 5.6.5.1): > """ > If the Receiver Stop Ready operation has completed successfully for a > Source ASE, the Unicast Client or the Unicast Server may terminate a > CIS established for that Source ASE by following the Connected > Isochronous Stream Terminate procedure defined in Volume 3, Part C, > Section 9.3.15 in [1]. Termination of the CIS is not required. > """ > > ASCS (Sec 5.6): > """ > If the server accepts the requested Receiver Stop Ready operation > parameter values for a Source ASE, the server shall transition the ASE > to the QoS Configured state and write a value of 0x02 (QoS > Configured) to the ASE_State field, and the server shall write the > previously accepted Config QoS operation parameter values to the > matching Additional_ASE_Parameters fields defined in Table 4.5. > """ The best option would be to follow the test specification with respect to Disable tests: '4.8.6 Unicast Client Initiates Disable Operation 1. The Upper Tester orders the IUT to execute the GATT Write Without Response sub-procedure for the ASE Control Point characteristic with the opcode set to 0x05 (Disable) and: • Number_of_ASEs set to 1 • ASE_ID[0] set using the value from the Initial Condition 2. The Lower Tester sends the IUT a notification of the ASE Control Point characteristic. 3. If the Lower Tester is in the Audio Source role: a. The Lower Tester sends the IUT a notification of the ASE characteristic that corresponds to the ASE_ID that was set in step 1, with ASE_State set to Disabling. b. The Upper Tester orders the IUT to execute the GATT Write Without Response sub- procedure for the ASE Control Point characteristic with the opcode set to 0x06 (Receiver Stop Ready) and: • • Number_of_ASEs set to 1 ASE_ID[0] set using the value from the Initial Condition 4. The Lower Tester sends the IUT a notification of the ASE Control Point characteristic. 5. The Lower Tester sends the IUT a notification of the ASE characteristic that corresponds to the ASE_ID that was set in step 4, with ASE_State set to QoS Configured.' Unfortunately it doesn't say what behavior it expects for the ISO transport itself so we need to check how PTS implemented it. In fact if we look into the server test case there is: '4.9.10 Unicast Server Initiates Disable While Streaming on Loss of CIS Pass verdict The IUT sends a notification of the ASE characteristic with the ASE_State field set to 0x02 (QoS Configured).' I suspect this is valid also for Disabling state, meaning if the CIS is lost while Disabling the server shall move to QoS state, so the fact that the Buds is not doing it may be a qualification issue if that is covered in the test specification in the future, anyway that is not really our problem and shouldn't stop us to perform the disconnection after Stop. -- Luiz Augusto von Dentz
