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

[Tanu Kaskinen]

On Fri, 2017-03-10 at 08:13 +0100, Georg Chini wrote:
> On 10.03.2017 00:33, Tanu Kaskinen wrote:
> > On Thu, 2017-03-02 at 17:04 +0100, Georg Chini wrote:
> > > -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.

The advantage is avoiding weird APIs. I expect foo_new() to create a
new instance of foo, not to sometimes create a new instance and
sometimes do something else. Am I the only one who finds the proposed
API weird?

I don't think there's need to call pa_bluetooth_native_backend_new()
from pa_bluetooth_discovery_set_ofono_running() anyway, so there
shouldn't be need to compromise on simplicity either. Your patch
removes the only place where y->native_backend was being set to NULL
during y->ofono_backend's life time (when y->ofono_backend is NULL,
pa_bluetooth_discovery_set_ofono_running() won't get called).

-- 
Tanu

https://www.patreon.com/tanuk