Received-SPF: pass (google.com: domain of linux-bluetooth+bounces-4811-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240520162757.78187-1-silviu.barbulescu@nxp.com> <20240520162757.78187-2-silviu.barbulescu@nxp.com>
In-Reply-To: <20240520162757.78187-2-silviu.barbulescu@nxp.com>
From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date: Mon, 20 May 2024 12:40:17 -0400
Message-ID: <CABBYNZLzSW1VBxZE_f=Qya31OerUxHuwHFOxQJtzNoNt29ia7g@mail.gmail.com>
Subject: Re: [PATCH BlueZ 1/2] shared/bap: Update BAP Broadcast Source state machine
To: Silviu Florian Barbulescu <silviu.barbulescu@nxp.com>
Cc: linux-bluetooth@vger.kernel.org, mihai-octavian.urzica@nxp.com, 
	vlad.pruteanu@nxp.com, andrei.istodorescu@nxp.com, iulia.tanasescu@nxp.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Silviu,

On Mon, May 20, 2024 at 12:28=E2=80=AFPM Silviu Florian Barbulescu
<silviu.barbulescu@nxp.com> wrote:
>
> Update BAP Broadcast Source state machine states to use BAP define states
>  for source Idle, Config, Streaming, and an intermediary state enabling.

Not really following, what is an intermediary state? Or do you mean
internal state? And more importantly why would we need an internal
state like that?

> Updated test-bap too.
>
> ---
>  src/shared/bap.c | 39 ++++++++++++++++++++-------------------
>  unit/test-bap.c  |  4 ++--
>  2 files changed, 22 insertions(+), 21 deletions(-)
>
> diff --git a/src/shared/bap.c b/src/shared/bap.c
> index 6572ef1d1..639149520 100644
> --- a/src/shared/bap.c
> +++ b/src/shared/bap.c
> @@ -1361,14 +1361,6 @@ static void ep_config_cb(struct bt_bap_stream *str=
eam, int err)
>         if (err)
>                 return;
>
> -       if (bt_bap_stream_get_type(stream) =3D=3D BT_BAP_STREAM_TYPE_BCAS=
T) {
> -               if (bt_bap_stream_io_dir(stream) =3D=3D BT_BAP_BCAST_SINK=
)
> -                       stream_set_state(stream, BT_BAP_STREAM_STATE_QOS)=
;
> -               else if (bt_bap_stream_io_dir(stream) =3D=3D BT_BAP_BCAST=
_SOURCE)
> -                       stream_set_state(stream, BT_BAP_STREAM_STATE_CONF=
IG);
> -               return;
> -       }
> -
>         stream_set_state(stream, BT_BAP_STREAM_STATE_CONFIG);
>  }
>
> @@ -1759,6 +1751,15 @@ static unsigned int bap_stream_metadata(struct bt_=
bap_stream *stream,
>         return req->id;
>  }
>
> +static unsigned int bap_bcast_qos(struct bt_bap_stream *stream,
> +                                       struct bt_bap_qos *data,
> +                                       bt_bap_stream_func_t func,
> +                                       void *user_data)
> +{
> +       stream->qos =3D *data;
> +       return 1;
> +}
> +
>  static unsigned int bap_bcast_config(struct bt_bap_stream *stream,
>                                      struct bt_bap_qos *qos, struct iovec=
 *data,
>                                      bt_bap_stream_func_t func, void *use=
r_data)
> @@ -2071,7 +2072,7 @@ static unsigned int bap_bcast_get_state(struct bt_b=
ap_stream *stream)
>         return stream->state;
>  }
>
> -static unsigned int bap_bcast_enable(struct bt_bap_stream *stream,
> +static unsigned int bap_bcast_sink_enable(struct bt_bap_stream *stream,
>                                         bool enable_links, struct iovec *=
data,
>                                         bt_bap_stream_func_t func,
>                                         void *user_data)
> @@ -2081,22 +2082,21 @@ static unsigned int bap_bcast_enable(struct bt_ba=
p_stream *stream,
>         return 1;
>  }
>
> -static unsigned int bap_bcast_start(struct bt_bap_stream *stream,
> +static unsigned int bap_bcast_src_enable(struct bt_bap_stream *stream,
> +                                       bool enable_links, struct iovec *=
data,
>                                         bt_bap_stream_func_t func,
>                                         void *user_data)
>  {
> -       stream_set_state(stream, BT_BAP_STREAM_STATE_STREAMING);
> +       stream_set_state(stream, BT_BAP_STREAM_STATE_ENABLING);
>
>         return 1;
>  }
>
> -static unsigned int bap_bcast_sink_disable(struct bt_bap_stream *stream,
> -                                       bool disable_links,
> +static unsigned int bap_bcast_start(struct bt_bap_stream *stream,
>                                         bt_bap_stream_func_t func,
>                                         void *user_data)
>  {
> -       bap_stream_io_detach(stream);
> -       stream_set_state(stream, BT_BAP_STREAM_STATE_CONFIG);
> +       stream_set_state(stream, BT_BAP_STREAM_STATE_STREAMING);
>
>         return 1;
>  }
> @@ -2106,7 +2106,8 @@ static unsigned int bap_bcast_disable(struct bt_bap=
_stream *stream,
>                                         bt_bap_stream_func_t func,
>                                         void *user_data)
>  {
> -       stream_set_state(stream, BT_BAP_STREAM_STATE_DISABLING);
> +       bap_stream_io_detach(stream);
> +       stream_set_state(stream, BT_BAP_STREAM_STATE_CONFIG);
>
>         return 1;
>  }
> @@ -2205,14 +2206,14 @@ static const struct bt_bap_stream_ops stream_ops[=
] =3D {
>                         bap_ucast_release, bap_ucast_detach),
>         STREAM_OPS(BT_BAP_BCAST_SINK, bap_bcast_set_state,
>                         bap_bcast_get_state,
> -                       bap_bcast_config, NULL, bap_bcast_enable,
> -                       bap_bcast_start, bap_bcast_sink_disable, NULL,
> +                       bap_bcast_config, NULL, bap_bcast_sink_enable,
> +                       bap_bcast_start, bap_bcast_disable, NULL,
>                         bap_bcast_metadata, bap_bcast_sink_get_dir,
>                         bap_bcast_get_location,
>                         bap_bcast_release, bap_bcast_sink_detach),
>         STREAM_OPS(BT_BAP_BCAST_SOURCE, bap_bcast_set_state,
>                         bap_bcast_get_state,
> -                       bap_bcast_config, NULL, bap_bcast_enable,
> +                       bap_bcast_config, bap_bcast_qos, bap_bcast_src_en=
able,
>                         bap_bcast_start, bap_bcast_disable, NULL,
>                         bap_bcast_metadata, bap_bcast_src_get_dir,
>                         bap_bcast_get_location,
> diff --git a/unit/test-bap.c b/unit/test-bap.c
> index 46ee0e4e5..10f9e348c 100644
> --- a/unit/test-bap.c
> +++ b/unit/test-bap.c
> @@ -547,10 +547,10 @@ static void bsrc_state(struct bt_bap_stream *stream=
, uint8_t old_state,
>         struct test_data *data =3D user_data;
>
>         switch (new_state) {
> -       case BT_BAP_STREAM_STATE_QOS:
> +       case BT_BAP_STREAM_STATE_CONFIG:
>                 bt_bap_stream_enable(stream, true, NULL, NULL, NULL);
>                 break;
> -       case BT_BAP_STREAM_STATE_CONFIG:
> +       case BT_BAP_STREAM_STATE_ENABLING:
>                 data->base =3D bt_bap_stream_get_base(stream);
>
>                 g_assert(data->base);
> --
> 2.40.1
>


--=20
Luiz Augusto von Dentz