diff mbox series

[BlueZ] bap: handle state transitions with old_state == new_state

Message ID 5c65fa8da174de15074310ec368f537c717ded43.1699646238.git.pav@iki.fi (mailing list archive)
State Superseded
Headers show
Series [BlueZ] bap: handle state transitions with old_state == new_state | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success
tedd_an/CheckPatch success CheckPatch PASS
tedd_an/GitLint fail WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search 20: B1 Line exceeds max length (108>80): "src/shared/bap.c:bap_ep_set_status() ASE status: ep 0x604000039a90 id 0x01 handle 0x000f state config len 42" 21: B1 Line exceeds max length (118>80): "src/shared/bap.c:ep_status_config() codec 0x06 framing 0x00 phy 0x02 rtn 2 latency 10 pd 4000 - 40000 ppd 4000 - 40000" 27: B1 Line exceeds max length (90>80): "src/shared/bap.c:bap_stream_state_changed() stream 0x60c0000334c0 dir 0x01: idle -> config" 32: B1 Line exceeds max length (145>80): "profiles/audio/bap.c:ep_register() ep 0x60d000006910 lpac 0x608000017520 rpac 0x6080000183a0 path /org/bluez/hci0/dev_C9_C9_76_21_08_4F/pac_sink0" 33: B1 Line exceeds max length (128>80): "profiles/audio/media.c:media_endpoint_async_call() Calling SelectProperties: name = :1.604 path = /MediaEndpointLE/BAPSource/lc3" 35: B1 Line exceeds max length (92>80): "src/shared/bap.c:bap_stream_state_changed() stream 0x60c0000334c0 dir 0x01: config -> config"
tedd_an/BuildEll success Build ELL PASS
tedd_an/BluezMake success Bluez Make PASS
tedd_an/MakeCheck success Bluez Make Check PASS
tedd_an/MakeDistcheck success Make Distcheck PASS
tedd_an/CheckValgrind success Check Valgrind PASS
tedd_an/CheckSmatch success CheckSparse PASS
tedd_an/bluezmakeextell success Make External ELL PASS
tedd_an/IncrementalBuild success Incremental Build PASS
tedd_an/ScanBuild success Scan Build PASS

Commit Message

Pauli Virtanen Nov. 10, 2023, 7:57 p.m. UTC 
ASCS allows transitions from Codec/QoS Configured back to the same
state.

E.g. NRF5340_AUDIO devkit starts in the config(1) state, which is
allowed (only Config QoS, Release, Enable, Receiver Stop Ready
transition are client-only). In this case, as client, we do Config Codec
ourselves and end up with config(1)->config(1) transition.  We currently
ignore that event, so QoS won't be setup and transports won't be
created.

Handle the config(1)->config(1) transition by continuing to Config QoS
if it occurs.

Log:

src/gatt-client.c:btd_gatt_client_connected() Device connected.
src/shared/gatt-client.c:exchange_mtu_cb() MTU exchange complete, with MTU: 65
src/shared/bap.c:bap_ep_set_status() ASE status: ep 0x604000039a90 id 0x01 handle 0x000f state config len 42
src/shared/bap.c:ep_status_config() codec 0x06 framing 0x00 phy 0x02 rtn 2 latency 10 pd 4000 - 40000 ppd 4000 - 40000
src/shared/bap.c:ep_status_config() Codec Config #0: type 0x01 len 2
src/shared/bap.c:ep_status_config() Codec Config #1: type 0x02 len 2
src/shared/bap.c:ep_status_config() Codec Config #2: type 0x03 len 5
src/shared/bap.c:ep_status_config() Codec Config #3: type 0x04 len 3
src/shared/bap.c:ep_status_config() Codec Config #4: type 0x05 len 2
src/shared/bap.c:bap_stream_state_changed() stream 0x60c0000334c0 dir 0x01: idle -> config
src/shared/bap.c:bap_stream_update_io_links() stream 0x60c0000334c0
profiles/audio/bap.c:bap_state() stream 0x60c0000334c0: idle(0) -> config(1)
profiles/audio/bap.c:bap_ready() bap 0x60e000001d20
profiles/audio/bap.c:pac_found() lpac 0x608000017520 rpac 0x6080000183a0
profiles/audio/bap.c:ep_register() ep 0x60d000006910 lpac 0x608000017520 rpac 0x6080000183a0 path /org/bluez/hci0/dev_C9_C9_76_21_08_4F/pac_sink0
profiles/audio/media.c:media_endpoint_async_call() Calling SelectProperties: name = :1.604 path = /MediaEndpointLE/BAPSource/lc3
...
src/shared/bap.c:bap_stream_state_changed() stream 0x60c0000334c0 dir 0x01: config -> config
src/shared/bap.c:bap_stream_update_io_links() stream 0x60c0000334c0
profiles/audio/bap.c:bap_state() stream 0x60c0000334c0: config(1) -> config(1)
---
 profiles/audio/bap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Luiz Augusto von Dentz Nov. 10, 2023, 8:44 p.m. UTC | #1 
Hi Pauli,

On Fri, Nov 10, 2023 at 2:58 PM Pauli Virtanen <pav@iki.fi> wrote:
>
> ASCS allows transitions from Codec/QoS Configured back to the same
> state.
>
> E.g. NRF5340_AUDIO devkit starts in the config(1) state, which is
> allowed (only Config QoS, Release, Enable, Receiver Stop Ready
> transition are client-only). In this case, as client, we do Config Codec
> ourselves and end up with config(1)->config(1) transition.  We currently
> ignore that event, so QoS won't be setup and transports won't be
> created.

Nice catch, that said I do wonder if we shouldn't be checking if the
configuration is the same and not attempt to reconfigure if it
matches.

> Handle the config(1)->config(1) transition by continuing to Config QoS
> if it occurs.
>
> Log:
>
> src/gatt-client.c:btd_gatt_client_connected() Device connected.
> src/shared/gatt-client.c:exchange_mtu_cb() MTU exchange complete, with MTU: 65
> src/shared/bap.c:bap_ep_set_status() ASE status: ep 0x604000039a90 id 0x01 handle 0x000f state config len 42
> src/shared/bap.c:ep_status_config() codec 0x06 framing 0x00 phy 0x02 rtn 2 latency 10 pd 4000 - 40000 ppd 4000 - 40000
> src/shared/bap.c:ep_status_config() Codec Config #0: type 0x01 len 2
> src/shared/bap.c:ep_status_config() Codec Config #1: type 0x02 len 2
> src/shared/bap.c:ep_status_config() Codec Config #2: type 0x03 len 5
> src/shared/bap.c:ep_status_config() Codec Config #3: type 0x04 len 3
> src/shared/bap.c:ep_status_config() Codec Config #4: type 0x05 len 2
> src/shared/bap.c:bap_stream_state_changed() stream 0x60c0000334c0 dir 0x01: idle -> config
> src/shared/bap.c:bap_stream_update_io_links() stream 0x60c0000334c0
> profiles/audio/bap.c:bap_state() stream 0x60c0000334c0: idle(0) -> config(1)
> profiles/audio/bap.c:bap_ready() bap 0x60e000001d20
> profiles/audio/bap.c:pac_found() lpac 0x608000017520 rpac 0x6080000183a0
> profiles/audio/bap.c:ep_register() ep 0x60d000006910 lpac 0x608000017520 rpac 0x6080000183a0 path /org/bluez/hci0/dev_C9_C9_76_21_08_4F/pac_sink0
> profiles/audio/media.c:media_endpoint_async_call() Calling SelectProperties: name = :1.604 path = /MediaEndpointLE/BAPSource/lc3
> ...
> src/shared/bap.c:bap_stream_state_changed() stream 0x60c0000334c0 dir 0x01: config -> config
> src/shared/bap.c:bap_stream_update_io_links() stream 0x60c0000334c0
> profiles/audio/bap.c:bap_state() stream 0x60c0000334c0: config(1) -> config(1)
> ---
>  profiles/audio/bap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/profiles/audio/bap.c b/profiles/audio/bap.c
> index b74498c4c..30a585ede 100644
> --- a/profiles/audio/bap.c
> +++ b/profiles/audio/bap.c
> @@ -1887,7 +1887,7 @@ static void bap_state(struct bt_bap_stream *stream, uint8_t old_state,
>                         bt_bap_stream_statestr(old_state), old_state,
>                         bt_bap_stream_statestr(new_state), new_state);
>
> -       if (new_state == old_state)

Add a comment regarding reconfiguring allowing the state machine to
transition to the same state.

> +       if (old_state == new_state && new_state != BT_BAP_STREAM_STATE_CONFIG)
>                 return;
>
>         ep = bap_find_ep_by_stream(data, stream);
> --
> 2.41.0
>
>
bluez.test.bot@gmail.com Nov. 10, 2023, 9:36 p.m. UTC | #2 
This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=800308

---Test result---

Test Summary:
CheckPatch                    PASS      0.38 seconds
GitLint                       FAIL      0.52 seconds
BuildEll                      PASS      35.07 seconds
BluezMake                     PASS      1196.96 seconds
MakeCheck                     PASS      13.47 seconds
MakeDistcheck                 PASS      218.17 seconds
CheckValgrind                 PASS      333.14 seconds
CheckSmatch                   PASS      453.58 seconds
bluezmakeextell               PASS      145.57 seconds
IncrementalBuild              PASS      1064.96 seconds
ScanBuild                     PASS      1400.66 seconds

Details
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[BlueZ] bap: handle state transitions with old_state == new_state

WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
20: B1 Line exceeds max length (108>80): "src/shared/bap.c:bap_ep_set_status() ASE status: ep 0x604000039a90 id 0x01 handle 0x000f state config len 42"
21: B1 Line exceeds max length (118>80): "src/shared/bap.c:ep_status_config() codec 0x06 framing 0x00 phy 0x02 rtn 2 latency 10 pd 4000 - 40000 ppd 4000 - 40000"
27: B1 Line exceeds max length (90>80): "src/shared/bap.c:bap_stream_state_changed() stream 0x60c0000334c0 dir 0x01: idle -> config"
32: B1 Line exceeds max length (145>80): "profiles/audio/bap.c:ep_register() ep 0x60d000006910 lpac 0x608000017520 rpac 0x6080000183a0 path /org/bluez/hci0/dev_C9_C9_76_21_08_4F/pac_sink0"
33: B1 Line exceeds max length (128>80): "profiles/audio/media.c:media_endpoint_async_call() Calling SelectProperties: name = :1.604 path = /MediaEndpointLE/BAPSource/lc3"
35: B1 Line exceeds max length (92>80): "src/shared/bap.c:bap_stream_state_changed() stream 0x60c0000334c0 dir 0x01: config -> config"


---
Regards,
Linux Bluetooth
diff mbox series

Patch

diff --git a/profiles/audio/bap.c b/profiles/audio/bap.c
index b74498c4c..30a585ede 100644
--- a/profiles/audio/bap.c
+++ b/profiles/audio/bap.c
@@ -1887,7 +1887,7 @@  static void bap_state(struct bt_bap_stream *stream, uint8_t old_state,
 			bt_bap_stream_statestr(old_state), old_state,
 			bt_bap_stream_statestr(new_state), new_state);
 
-	if (new_state == old_state)
+	if (old_state == new_state && new_state != BT_BAP_STREAM_STATE_CONFIG)
 		return;
 
 	ep = bap_find_ep_by_stream(data, stream);