Re: [Bluez PATCH v1 1/3] policy: add checks before connecting

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

 



Hi Luiz,

On Thu, 5 Nov 2020 at 05:13, Luiz Augusto von Dentz
<luiz.dentz@xxxxxxxxx> wrote:
>
> Hi Archie,
>
> On Tue, Nov 3, 2020 at 9:35 PM Archie Pusaka <apusaka@xxxxxxxxxx> wrote:
> >
> > From: Archie Pusaka <apusaka@xxxxxxxxxxxx>
> >
> > When policy_connect() is called, there might be a case where the
> > device is not ready, or even the adapter is down. Add some checks
> > by calling btd_device_connect_services() instead of directly calling
> > btd_service_connect().
>
> But we could perform these checks in btd_service_connect or you also
> intended to use the pending list? Im a little hesitant with such a
> change though because there could be a pending connect already causing
> it to fail instead of just connecting in parallel.

Yes, we can also perform the checks in btd_service_connect or in policy_connect.
I have no intention of using the pending list, and to be frank, I also
don't know whether we should let the connection happen in parallel or
we should form a queue.

This issue correlates with the other issues addressed in the second
and third patch.
However, if we leave this patch and only fix the second and third
issue, this one would not cause a problem by itself because even when
the adapter is down and policy_connect is called, it would just fail
and we would recover cleanly.
This patch is merely just to skip the unnecessary connection attempt.
I will submit another patch which performs the check inside
btd_service_connect, and I'll leave it to you to decide whether this
optional patch is needed.

>
> > Reviewed-by: Sonny Sasaka <sonnysasaka@xxxxxxxxxxxx>
> > ---
> >
> >  plugins/policy.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/plugins/policy.c b/plugins/policy.c
> > index ba9e1be020..42b15cb65f 100644
> > --- a/plugins/policy.c
> > +++ b/plugins/policy.c
> > @@ -106,6 +106,7 @@ static void policy_connect(struct policy_data *data,
> >  {
> >         struct btd_profile *profile = btd_service_get_profile(service);
> >         struct reconnect_data *reconnect;
> > +       GSList *l = NULL;
> >
> >         reconnect = reconnect_find(btd_service_get_device(service));
> >         if (reconnect && reconnect->active)
> > @@ -113,7 +114,9 @@ static void policy_connect(struct policy_data *data,
> >
> >         DBG("%s profile %s", device_get_path(data->dev), profile->name);
> >
> > -       btd_service_connect(service);
> > +       l = g_slist_prepend(l, service);
> > +       btd_device_connect_services(data->dev, l);
> > +       g_slist_free(l);
> >  }
> >
> >  static void policy_disconnect(struct policy_data *data,
> > --
> > 2.29.1.341.ge80a0c044ae-goog
> >
>
>
> --
> Luiz Augusto von Dentz

Thanks,
Archie



[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