Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220822125221.Bluez.1.I541cbea9d6295f531c796bf3bda96b22db76bc19@changeid>
 <f9fd47a4-e5fc-5640-de71-dee1d52da2a8@molgen.mpg.de>
In-Reply-To: <f9fd47a4-e5fc-5640-de71-dee1d52da2a8@molgen.mpg.de>
From:   Archie Pusaka <apusaka@google.com>
Date:   Mon, 22 Aug 2022 14:33:31 +0800
Message-ID: <CAJQfnxHsRrJpQQB06bxhjc1TetK-8H20Cos366A6qH5AY9j9vw@mail.gmail.com>
Subject: Re: [Bluez PATCH] adapter: Reset pending settings when receiving MGMT error
To:     Paul Menzel <pmenzel@molgen.mpg.de>
Cc:     linux-bluetooth@vger.kernel.org,
        Luiz Augusto von Dentz <luiz.dentz@gmail.com>,
        Marcel Holtmann <marcel@holtmann.org>,
        chromeos-bluetooth-upstreaming@chromium.org,
        Archie Pusaka <apusaka@chromium.org>,
        Sonny Sasaka <sonnysasaka@chromium.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi Paul,

On Mon, 22 Aug 2022 at 14:15, Paul Menzel <pmenzel@molgen.mpg.de> wrote:
>
> Dear Archie,
>
>
> Thank you for the patch.
>
>
> Am 22.08.22 um 06:53 schrieb Archie Pusaka:
> > From: Archie Pusaka <apusaka@chromium.org>
>
> I think the tag in the email subject needs to be [PATCH BlueZ] to be
> detected by the build bot.

Is the bot the one who just commented about the test result? If so
probably it can detect this format as well.
>
> > We set the pending settings flag when sending MGMT_SETTING_*
> > commands to the MGMT layer and clear them when receiving success
> > reply, but we don't clear them when receiving error reply. This
> > might cause a setting to be stuck in pending state.
>
> Were you able to reproduce a problem on real hardware?

I only received some reports, but unfortunately I cannot repro on real
hardware. The symptom is BlueZ can't be turned off, snoop logs shows
that MGMT_OP_SET_POWERED fails to be sent, and we are stuck with it
since the next commands to toggle power are ignored.
>
> > Therefore, also clear the pending flag when receiving error.
> > Furthermore, this patch also postpone setting the pending flag
>
> postpone*s*

Thanks, will fix.
>
> > until we queue the MGMT command in order to avoid setting it too
> > soon but we return early.
>
> Maybe add a comment, that how you tested this?

The reporter claims the problem is no longer observable after this
patch. I didn't do any other intelligent way of testing,
unfortunately. Do I also need to document that?
>
> > Reviewed-by: Sonny Sasaka <sonnysasaka@chromium.org>
> >
> > ---
> >
> >   src/adapter.c | 45 +++++++++++++++++++++++++++++++++++++--------
> >   1 file changed, 37 insertions(+), 8 deletions(-)
>
> [=E2=80=A6]
>
>
> Kind regards,
>
> Paul