Re: [PATCH BlueZ 3/3] sco-tester: check sent SCO data is received at bthost

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Pauli,

On Thu, Mar 6, 2025 at 2:37 PM Pauli Virtanen <pav@xxxxxx> wrote:
>
> Hi,
>
> ke, 2025-03-05 kello 13:58 -0500, Luiz Augusto von Dentz kirjoitti:
> > On Wed, Mar 5, 2025 at 10:58 AM Pauli Virtanen <pav@xxxxxx> wrote:
> > >
> > > When sending data, also check that the data is received by bthost.
> > > ---
> > >  tools/sco-tester.c | 47 ++++++++++++++++++++++++++++++++++++++++++++--
> > >  1 file changed, 45 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/tools/sco-tester.c b/tools/sco-tester.c
> > > index 7f37ca5cf..9886481ff 100644
> > > --- a/tools/sco-tester.c
> > > +++ b/tools/sco-tester.c
> > > @@ -318,10 +318,51 @@ static void client_connectable_complete(uint16_t opcode, uint8_t status,
> > >                 tester_setup_complete();
> > >  }
> > >
> > > +static void bthost_recv_data(const void *buf, uint16_t len, uint8_t status,
> > > +                                                               void *user_data)
> > > +{
> > > +       struct test_data *data = user_data;
> > > +       const struct sco_client_data *scodata = data->test_data;
> >
> > I had to add the following change in order to pass with these changes:
> >
> > +       /* Ignore empty packet as that is used to confirm NOCP is being
> > +        * generated.
> > +        */
> > +       if (!len)
> > +               return;
> > +
> >
> > Now I wonder if this may be a problem since it does show up as a data
> > packet even though it is empty, I guess for the purpose of HFP
> > streaming it shouldn't cause problems but if someone attempts to do
> > packet based checks like this it may stop working.
>
> It's probably a change in behavior if the controller interprets the
> zero-length packet as an instruction to send zero-filled SCO packet
> (with valid CRC etc) over the air. I'm not sure what the controllers do
> here.
>
> The timer causes a small hiccup at startup, but that is only once per
> controller index added. Since then the controller is not spec-
> compliant, maybe it is OK and there could be a quirk...

Yeah, that said we could perhaps add a quirk to mark Sync Flow Control
as supported, rather than adding one for saying it is broken, that way
we can enable it only in the controllers that are known to work
otherwise there is always a chance that controllers don't behave as
expected.

> >
> > > +       --data->step;
> > > +
> > > +       tester_print("Client received %u bytes of data", len);
> > > +
> > > +       if (scodata->send_data && (scodata->data_len != len ||
> > > +                       memcmp(scodata->send_data, buf, len)))
> > > +               tester_test_failed();
> > > +       else if (!data->step)
> > > +               tester_test_passed();
> > > +}
> > > +
> > > +static void bthost_sco_disconnected(void *user_data)
> > > +{
> > > +       struct test_data *data = user_data;
> > > +
> > > +       tester_print("SCO handle 0x%04x disconnected", data->handle);
> > > +
> > > +       data->handle = 0x0000;
> > > +}
> > > +
> > > +static void sco_new_conn(uint16_t handle, void *user_data)
> > > +{
> > > +       struct test_data *data = user_data;
> > > +       struct bthost *host;
> > > +
> > > +       tester_print("New client connection with handle 0x%04x", handle);
> > > +
> > > +       data->handle = handle;
> > > +
> > > +       host = hciemu_client_get_host(data->hciemu);
> > > +       bthost_add_sco_hook(host, data->handle, bthost_recv_data, data,
> > > +                               bthost_sco_disconnected);
> > > +}
> > > +
> > >  static void setup_powered_callback(uint8_t status, uint16_t length,
> > >                                         const void *param, void *user_data)
> > >  {
> > >         struct test_data *data = tester_get_data();
> > > +       const struct sco_client_data *scodata = data->test_data;
> > >         struct bthost *bthost;
> > >
> > >         if (status != MGMT_STATUS_SUCCESS) {
> > > @@ -334,6 +375,9 @@ static void setup_powered_callback(uint8_t status, uint16_t length,
> > >         bthost = hciemu_client_get_host(data->hciemu);
> > >         bthost_set_cmd_complete_cb(bthost, client_connectable_complete, data);
> > >         bthost_write_scan_enable(bthost, 0x03);
> > > +
> > > +       if (scodata && scodata->send_data)
> > > +               bthost_set_sco_cb(bthost, sco_new_conn, data);
> > >  }
> > >
> > >  static void setup_powered(const void *test_data)
> > > @@ -740,8 +784,6 @@ static gboolean sco_connect_cb(GIOChannel *io, GIOCondition cond,
> > >                 ssize_t ret = 0;
> > >                 unsigned int count;
> > >
> > > -               data->step = 0;
> > > -
> > >                 sco_tx_timestamping(data, io);
> > >
> > >                 tester_print("Writing %u*%u bytes of data",
> > > @@ -751,6 +793,7 @@ static gboolean sco_connect_cb(GIOChannel *io, GIOCondition cond,
> > >                         ret = write(sk, scodata->send_data, scodata->data_len);
> > >                         if (scodata->data_len != ret)
> > >                                 break;
> > > +                       data->step++;
> > >                 }
> > >                 if (scodata->data_len != ret) {
> > >                         tester_warn("Failed to write %u bytes: %zu %s (%d)",
> > > --
> > > 2.48.1
> > >
> > >
> >
> >
>


-- 
Luiz Augusto von Dentz





[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux