[pulseaudio-discuss] [PATCH 2/3] bluetooth: use native and ofono backends in parallel with headset=auto

[Georg Chini]

On 10.03.2017 00:33, Tanu Kaskinen wrote:
> On Thu, 2017-03-02 at 17:04 +0100, Georg Chini wrote:
>> This patch changes the behavior of the headset=auto switch for module-bluez5-discover.
>> With headset=auto now both backends will be active at the same time for the AG role and
>> the switching between the backends is only done for the HS role.
>> headset=ofono and headset=native remain unchanged.
>>
>> This allows to use old HSP only headsets while running ofono and to have headset support
>> via pulseaudio if ofono is started with the --noplugin=hfp_ag_bluez5 option.
> Not really related to the patch, I'm just curious: Does hfp_ag_bluez5
> make ofono act in the AG role?

Yes, it does. However this only works if you have a modem in your system
or simulate one with phonesim. When I tested it (at least a year ago), ofono
would support the AT commands of the headset, but audio did not work.
This is why I switched the AG support off.

> Is the plugin loaded by default?

Yes.

> Does
> that work nowadays? Or is it still work in progress?

I'll pull a fresh ofono tree later today and check. The version on my system
is rather old.

> My understanding
> has been that ofono only supports the HF role
>
>> ---
>>   src/modules/bluetooth/backend-native.c | 24 +++++++++++++++++++++---
>>   src/modules/bluetooth/bluez5-util.c    | 10 +++-------
>>   src/modules/bluetooth/bluez5-util.h    |  4 ++--
>>   3 files changed, 26 insertions(+), 12 deletions(-)
>>
>> diff --git a/src/modules/bluetooth/backend-native.c b/src/modules/bluetooth/backend-native.c
>> index 71f1773..7573b89 100644
>> --- a/src/modules/bluetooth/backend-native.c
>> +++ b/src/modules/bluetooth/backend-native.c
>> @@ -40,6 +40,7 @@ struct pa_bluetooth_backend {
>>     pa_core *core;
>>     pa_dbus_connection *connection;
>>     pa_bluetooth_discovery *discovery;
>> +  bool enable_hs_role;
>>   
>>     PA_LLIST_HEAD(pa_dbus_pending, pending);
>>   };
>> @@ -653,10 +654,24 @@ static void profile_done(pa_bluetooth_backend *b, pa_bluetooth_profile_t profile
>>       }
>>   }
>>   
>> -pa_bluetooth_backend *pa_bluetooth_native_backend_new(pa_core *c, pa_bluetooth_discovery *y) {
>> +pa_bluetooth_backend *pa_bluetooth_native_backend_new(pa_core *c, pa_bluetooth_discovery *y, pa_bluetooth_backend *native_backend, bool enable_hs_role) {
>>       pa_bluetooth_backend *backend;
>>       DBusError err;
>>   
>> +    /* If the backend already exists just switch the HS role on or off */
> I find this interface weird. If the goal is just to switch the HS role
> on or off, I think it would be better to have function
> pa_bluetooth_native_backend_enable_hs_role().
>
It was the most simple way to achieve the goal. The function originally 
created
the native backend. Now it creates the backend if it does not exist or 
switches
the HS role on or off if the backend is already there. Otherwise I have 
to replace
the calls to pa_bluetooth_native_backend_new() with something like

if (native_backend)
    pa_bluetooth_native_backend_enable_hs_role()
else
    pa_bluetooth_native_backend_new()

And then I would still have to check within 
pa_bluetooth_native_backend_new()
if the HS role needs to be enabled or not. So I do not see the advantage of
implementing a separate function. If you prefer, I can change it anyway.