Re: [pulseaudio-discuss] [PATCH v2 2/3] bluetooth: separate HSP and HFP
On Sun, 2016-08-21 at 19:30 -0400, James Bottomley wrote: > On Sun, 2016-08-21 at 15:19 +0300, Tanu Kaskinen wrote: > > > > One thing that needs fixing is the profile waiting logic - we wait > > until all supported profiles are connected (or until a timeout > > expires) before loading module-bluez5-device. Since we will now > > connect only HFP or HSP, it doesn't make sense to wait for both of > > them getting connected. The waiting logic is implemented in > > pa_bluetooth_transport_set_state(). > > This actually seems to be broken today. I unwound all my patches and I > still see the debug message > > Aug 21 19:23:28 jarvis pulseaudio[12479]: [pulseaudio] bluez5-util.c: > Timeout expired, and device /org/bluez/hci0/dev_B8_AD_3E_8E_DE_EF still > has disconnected profiles: > > meaning the timer expired even though all profiles were connected. I > think the bug is that pa_bluetooth_transport_set_state() starts the > timer when we go from 0->1 disconnected profiles, but after that, the > test (old_any_connected != > pa_bluetooth_device_any_transport_connected(t->device) is always true > and so the timer never gets stopped if we're waiting for more than one > transport connection. Oops! I added this code recently, but I couldn't test it properly, because my headset most of the time connects only A2DP for some reason, and on the one occasion when it did connect HSP, A2DP got connected only much later. > I think the fix is this. Yes, that should do the trick. It will fire the CONNECTION_CHANGED hook more often than necessary, though. I'll make a bit different patch. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v2 2/3] bluetooth: separate HSP and HFP
On Sun, 2016-08-21 at 21:03 +0300, Tanu Kaskinen wrote:
> On Sun, 2016-08-21 at 15:19 +0300, Tanu Kaskinen wrote:
> > On Sat, 2016-08-20 at 15:03 -0700, James Bottomley wrote:
> > > static const char* const valid_modargs[] = {
> > > "path",
> > > +"disable_profile_hfp",
> >
> > We try to avoid negative option names on the basis that double
> > negatives are not nice ("disable_foo=no"), so I'd prefer
> > "enable_profile_hfp".
>
> Hmm, maybe that's not a good name, if the option only affects HFP HF
> support, and not HFP AG. Would "enable_profile_hfp_hf" work? It's
> pretty awful from user-friendliness perspective, but I have trouble
> coming up with better names.
As long as I can put a comment saying it was your suggestion ...
However, yes, I can do it. I don't really see it as a huge problem.
descriptive options aren't bad just because they're long (unless
they're 100s of characters long).
James
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v2 2/3] bluetooth: separate HSP and HFP
On Sun, 2016-08-21 at 15:19 +0300, Tanu Kaskinen wrote:
> On Sat, 2016-08-20 at 15:03 -0700, James Bottomley wrote:
> > static const char* const valid_modargs[] = {
> > "path",
> > +"disable_profile_hfp",
>
> We try to avoid negative option names on the basis that double
> negatives are not nice ("disable_foo=no"), so I'd prefer
> "enable_profile_hfp".
Hmm, maybe that's not a good name, if the option only affects HFP HF
support, and not HFP AG. Would "enable_profile_hfp_hf" work? It's
pretty awful from user-friendliness perspective, but I have trouble
coming up with better names.
--
Tanu
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
