Re: [Bluez PATCH v2 0/3] adapter: Reconnect audio when resuming from suspend

From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
To: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
Cc: Marcel Holtmann <marcel@holtmann.org>,
	ChromeOS Bluetooth Upstreaming 
	<chromeos-bluetooth-upstreaming@chromium.org>,
	Bluez mailing list <linux-bluetooth@vger.kernel.org>
Subject: Re: [Bluez PATCH v2 0/3] adapter: Reconnect audio when resuming from suspend
Date: Wed, 26 Aug 2020 23:20:49 -0700	[thread overview]
Message-ID: <CABBYNZLmfWWBYZjHDkHmMpzYC02zFpWMV_xAiD1pK3cc09JwSA@mail.gmail.com> (raw)
In-Reply-To: <CANFp7mWqT1mO0OcJAt4AycTgFLz22p7zyRGjR1Hvj4bJmHHsTw@mail.gmail.com>

Hi Abhishek,

On Wed, Aug 26, 2020 at 10:41 AM Abhishek Pandit-Subedi
<abhishekpandit@chromium.org> wrote:
>
> Hi Luiz,
>
> Could you PTAL at this v2 patch series. I refactored the audio
> reconnect to use the policy plugin instead of doing the reconnect as
> part of the adapter logic, as requested.
>
> Thanks
> Abhishek
>
> On Tue, Aug 18, 2020 at 4:28 PM Abhishek Pandit-Subedi
> <abhishekpandit@chromium.org> wrote:
> >
> >
> > Hi Luiz and Marcel,
> >
> > This is a quality of life improvement for the behavior of audio devices
> > during system suspend. This depends on a kernel change that emits
> > suspend/resume events:
> >
> > https://patchwork.kernel.org/project/bluetooth/list/?series=325771

So we could not just use the disconnect reason like I suggested?

> > Right now, audio devices will be disconnected as part of suspend but
> > won't be reconnected when the system resumes without user interaction.
> > This is annoying to some users as it causes an interruption to their
> > normal work flow.
> >
> > This change reconnects audio devices that were disconnected for suspend
> > using the following logic:
> >
> >  * Register a callback for controller resume in the policy plugin.
> >  * In the device disconnect callback, mark any devices with the A2DP
> >    service uuid for reconnect on resume after a delay.
> >  * In the controller resume callback, queue any policy items that are
> >    marked to reconnect on resume for connection with the
> >    ReconnectAudioDelay value (default = 5s for Wi-Fi coexistence).

Something like ResumeDelay would probably be better.

> > A reconnect is only attempted once after the controller resumes and the
> > delay between resume and reconnect is configurable via the
> > ReconnectAudioDelay key in the Policy settings. The 5s delay was chosen
> > arbitrarily and I think anywhere up to 10s is probably ok. A longer
> > delay is better to account for spurious wakeups and Wi-Fi reconnection
> > time (avoiding any co-ex issues) at the downside of reconnection speed.

I would keep the same logic as out of range so the platforms can
customize the number of attempts.

> > Here are the tests I have done with this:
> > - Single suspend and verified the headphones reconnect
> > - Suspend stress test for 25 iterations and verify both Wi-Fi and
> >   Bluetooth audio reconnect on resume. (Ran with wake minimum time of
> >   10s)
> > - Suspend test with wake time < 5s to verify that BT reconnect isn't
> >   attempted. Ran 5 iterations with low wake time and then let it stay
> >   awake to confirm reconnect finally completed after 5s+ wake time.
> > - Suspend test with wake time between 3s - 6s. Ran with 5 iterations and
> >   verified it wasn't connected at the end. A connection attempt was
> >   made but not completed due to suspend. A reconnect attempt was not
> >   made afterwards, which is by design.
> >
> >   Luiz@ Marcel@: Does this sound ok (give up after an attempt)?
> >
> > I've tested this on a Pixelbook Go (AC-9260 controller) and HP
> > Chromebook 14a (RTL8822CE controller) with GID6B headset.
> >
> > I've also tested this with the Pixel Buds 2. These earbuds actually
> > reconnect automatically to the Chromebook (even without this policy
> > change) and I verified that the new changes don't break the reconnection
> > mechanism.
> >
> > Thanks
> > Abhishek
> >
> >
> > Changes in v2:
> > - Refactored to use policy instead of connecting directly in adapter
> >
> > Abhishek Pandit-Subedi (3):
> >   mgmt: Add controller suspend and resume events
> >   monitor: Add btmon support for Suspend and Resume events
> >   policy: Reconnect audio on controller resume
> >
> >  lib/mgmt.h       | 14 +++++++++++
> >  monitor/packet.c | 55 +++++++++++++++++++++++++++++++++++++++++
> >  plugins/policy.c | 64 +++++++++++++++++++++++++++++++++++++++++++++---
> >  src/adapter.c    | 45 ++++++++++++++++++++++++++++++++++
> >  src/adapter.h    |  6 +++++
> >  src/main.c       |  1 +
> >  src/main.conf    |  9 +++++++
> >  7 files changed, 190 insertions(+), 4 deletions(-)
> >
> > --
> > 2.28.0.297.g1956fa8f8d-goog
> >

-- 
Luiz Augusto von Dentz