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

[Tanu Kaskinen]

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

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.

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

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.

-- 
Tanu