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

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

 



Hi Abhishek,

On Thu, Aug 27, 2020 at 2:13 PM Abhishek Pandit-Subedi
<abhishekpandit@xxxxxxxxxxxx> wrote:
>
> Hi Luiz,
>
> On Wed, Aug 26, 2020 at 11:21 PM Luiz Augusto von Dentz
> <luiz.dentz@xxxxxxxxx> wrote:
> >
> > Hi Abhishek,
> >
> > On Wed, Aug 26, 2020 at 10:41 AM Abhishek Pandit-Subedi
> > <abhishekpandit@xxxxxxxxxxxx> 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@xxxxxxxxxxxx> 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?
>
> I am using the disconnect reason to mark the device for reconnection
> and only queueing it for reconnect on resume. As mentioned in the
> patch, this is to account for resumes that are not user driven and
> will suspend almost immediately again (i.e. periodic wake up to check
> battery level and see if we need to shut down).
>
> >
> > > > 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.
>
> Sure, I will rename this.
>
> >
> > > > 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.
>
> So the first reconnect attempt after resume should be separately
> configurable (i.e. 5s) but subsequent reconnect attempts would use the
> reconnect-intervals values? That sounds good to me.

Right, though 5 seconds seems awfully long compared to the normal
first attempt, so perhaps we could default it to just 1-2 seconds.

> >
> > > > 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



-- 
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