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
