[pulseaudio-discuss] [PATCH] bluetooth: separate HSP and HFP

[James Bottomley]

On Fri, 2016-08-19 at 22:33 +0300, Tanu Kaskinen wrote:
> On Thu, 2016-08-18 at 11:14 -0700, James Bottomley wrote:
> > When all headsets supported both HSP and HFP, life was good and we
> > only needed to implement HSP in the native backend.  Unfortunately
> > some headsets have started supporting HFP only.  Unfortuantely, we
> > can't simply switch to HFP only because that might break older HSP
> > only headsets meaning we need to support both HSP and HFP
> > separately.
> > 
> > This patch separates them from a joint profile to being two 
> > separate ones.  The older one retains the headset_head_unit name, 
> > meaning any saved parameters will still select this (keeping us 
> > backward compatible).  It also introduces a new headset_handsfree.
> > 
> > For headsets that support both HSP and HFP, the two profiles will
> > become separately visible and selectable.  This will only matter 
> > once we start adding features to HFP that HSP can't support (like
> > wideband audio).
> > 
> > Signed-off-by: <James.Bottomley at HansenPartnership.com>
> 
> Thanks, this looks pretty neat. It's hard to tell if any changes are
> missing, but the changes that are there look good, although please 
> use spaces for all indenting.

Heh, kernel coding style built into emacs.  I assume there's some sort
of tab to space converter somewhere I can run this through.

>  One thing that is missing, though, is updating module-bluetooth
> -policy.c. It references "headset_head_unit" in one place, and that 
> code should take the new profile into account too.

in profile_available_hook_callback() will do.

> Did you test if volume control works in both directions? I don't know
> if HSP and HFP volume handling is identical. The observed volume 
> should change if you set it with some volume control GUI, and the 
> slider in the GUI should move when you set the volume from the 
> headset's own buttons.

Yes, the HFP uses the same AT commands as the HSP for volume and
microphone control.  However, the boom does have a weirdness in that
it's top volume is 14 which is directly contrary to the standard which
says it must be 15, so using the remote volume controls I can only get
up to 92% (I assume it's a manufacturer off by one reading the standard
error).

> > @@ -47,6 +47,7 @@ typedef enum profile {
> >      PA_BLUETOOTH_PROFILE_A2DP_SINK,
> >      PA_BLUETOOTH_PROFILE_A2DP_SOURCE,
> >      PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT,
> > +    PA_BLUETOOTH_PROFILE_HEADSET_HFP,
> >      PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY,
> 
> Since the enum effectively lists profile+role combinations, this 
> naming scheme would make more sense to me:
> 
> PA_BLUETOOTH_PROFILE_HSP_HS,
> PA_BLUETOOTH_PROFILE_HFP_HF,
> PA_BLUETOOTH_PROFILE_HFP_AG
> 
> (and PA_BLUETOOTH_PROFILE_HSP_AG when the patch for that finally gets
> merged). That's what the UUID constants use too.

OK, will update.

> Similarly, I'd be tempted to use "hfp_hf" as the profile name, but
> maybe "headset_handsfree" is better, since it's more in line with the
> other profile names.

Yes, I felt we were a bit stuck with that one.

James