[pulseaudio-discuss] [RFC 11/24] bluetooth: Add new profile type PROFILE_HFP_AG

[João Paulo Rechi Vita]

On Wed, Apr 3, 2013 at 6:47 AM, Mikel Astiz <mikel.astiz.oss at gmail.com> wrote:
> Hi João Paulo,
>
> On Tue, Apr 2, 2013 at 4:10 PM, João Paulo Rechi Vita <jprvita at gmail.com>
> wrote:
>>
>> On Wed, Mar 27, 2013 at 1:11 PM, Mikel Astiz <mikel.astiz.oss at gmail.com>
>> wrote:
>> > Hi João Paulo,
>> >
>> > On Wed, Mar 27, 2013 at 4:54 PM, João Paulo Rechi Vita
>> > <jprvita at gmail.com> wrote:
>> >> On Wed, Mar 27, 2013 at 5:20 AM, Mikel Astiz
>> >> <mikel.astiz.oss at gmail.com> wrote:
>> >>> Hi João Paulo,
>> >>>
>> >>> On Wed, Mar 27, 2013 at 6:16 AM,  <jprvita at gmail.com> wrote:
>> >>>> From: João Paulo Rechi Vita <jprvita at openbossa.org>
>> >>>>
>> >>>> ---
>> >>>>  src/modules/bluetooth/bluetooth-util.c | 6 ++++++
>> >>>>  src/modules/bluetooth/bluetooth-util.h | 1 +
>> >>>>  2 files changed, 7 insertions(+)
>> >>>>
>> >>>> diff --git a/src/modules/bluetooth/bluetooth-util.c
>> >>>> b/src/modules/bluetooth/bluetooth-util.c
>> >>>> index 2a4b694..59d0854 100644
>> >>>> --- a/src/modules/bluetooth/bluetooth-util.c
>> >>>> +++ b/src/modules/bluetooth/bluetooth-util.c
>> >>>> @@ -184,6 +184,8 @@ const char *pa_bt_profile_to_string(enum profile
>> >>>> profile) {
>> >>>>              return "hsp";
>> >>>>          case PROFILE_HFGW:
>> >>>>              return "hfgw";
>> >>>> +        case PROFILE_HFP_AG:
>> >>>> +            return "hfp_ag";
>> >>>>          case PROFILE_OFF:
>> >>>>              pa_assert_not_reached();
>> >>>>      }
>> >>>> @@ -232,6 +234,10 @@ static void profile_to_remote_uuid(enum profile
>> >>>> p, const char *res[2]) {
>> >>>>              res[0] = HSP_AG_UUID;
>> >>>>              res[1] = HFP_AG_UUID;
>> >>>>              return;
>> >>>> +        case PROFILE_HFP_AG:
>> >>>> +            res[0] = HFP_AG_UUID;
>> >>>> +            res[1] = NULL;
>> >>>> +            return;
>> >>>>          case PROFILE_OFF:
>> >>>>              break;
>> >>>>      }
>> >>>> diff --git a/src/modules/bluetooth/bluetooth-util.h
>> >>>> b/src/modules/bluetooth/bluetooth-util.h
>> >>>> index 6423f88..28f63c8 100644
>> >>>> --- a/src/modules/bluetooth/bluetooth-util.h
>> >>>> +++ b/src/modules/bluetooth/bluetooth-util.h
>> >>>> @@ -62,6 +62,7 @@ enum profile {
>> >>>>      PROFILE_A2DP_SOURCE,
>> >>>>      PROFILE_HSP,
>> >>>>      PROFILE_HFGW,
>> >>>> +    PROFILE_HFP_AG,
>> >>>>      PROFILE_OFF
>> >>>>  };
>> >>>>
>> >>>> --
>> >>>> 1.7.11.7
>> >>>>
>> >>>
>> >>> I don't see the need for such an additional profile, since we already
>> >>> have HFGW.
>> >>>
>> >>> If you want to avoid associating the HSP UUID to this specific card
>> >>> profile, you need to handle that internally. Profiles are exposed
>> >>> outside the module and even in the UI, so we shouldn't change this
>> >>> unless strictly needed.
>> >>>
>> >>
>> >> I did it this way to be able to support HFP_HF implemented in oFono
>> >> without breaking support for HFP_HF implemented in BlueZ (pre external
>> >
>> > This is an internal problem that shouldn't affect the rest of the
>> > modules and the UI. You're already suffering this hassle in some of
>> > your patches, such as 24/24.
>> >
>>
>> I don't think this is such a big deal. Modules (as in patch 24/24) are
>> part of PulseAudio, their versioning follows PulseAudio's versioning,
>> so this can be easily managed. Regading UIs, they shoudn't make
>> decisions based on hardcoded profile names. Do you have any concrete
>> example where this would break for you?
>
>
> As I said before, I'm not concerned about renaming but about splitting.
> There's no need to add additional card profiles.
>
>>
>>
>> >> profiles support). Additionally, our profile naming scheme today is
>> >> imprecise, "a2dp" refers to A2DP Sink, "hsp" refers to both HSP HF and
>> >> HFP HF, HFGW refers to HFP AG. I think having one profile matching
>> >> each Bluetooth profile in a descriptive way would be much less
>> >> confusing for the end user. Ideally that would be: a2dp_source,
>> >> a2dp_sink, hsp_hs, hsp_ag, hfp_hf, hfp_ag, were the role part of the
>> >> name always refer to the role implemented by the remote device.
>> >
>> > We can discuss if the profiles should be renamed but this would be
>> > unrelated to this patch. In particular, I don't think the user cares
>> > about HSP vs HFP, so I'd be against splitting this.
>> >
>>
>> But we will want to support HSP devices, specially bluetooth headsets
>> to be used with VoIP programs, and we don't want to depend on oFono
>> for that, so these need to be separate card profiles anyway. I suggest
>> we break this once and create the following profiles to accommodate
>> future extensions:
>>
>> enum profile {
>>     PROFILE_A2DP_SINK,
>>     PROFILE_A2DP_SOURCE,
>>     PROFILE_HSP_HS,
>>     PROFILE_HSP_GW,
>>     PROFILE_HFP_HF,
>>     PROFILE_HFP_GW,
>>     PROFILE_OFF
>> }
>
>
> I'm not following here. A simple bluetooth headset may support HSP or HFP
> equally.
>
> Regarding the dependency to oFono, I think the original proposal was to
> depend on oFono for both HSP and HFP, as well as for both roles (HS and GW).
>

Getting back to this point, IMO HSP should be implemented in BlueZ
instead of oFono, since it needs only 2 mandatory AT commands (+ 2
optional for volume control), but this discussion is a bit OT for this
mailing list. Anyway, this is a different problem, so I'm keeping only
PROFILE_HFGW in this series.


--
João Paulo Rechi Vita
http://about.me/jprvita