On 10.03.2017 19:21, Tanu Kaskinen wrote:
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).
We still need to call pa_bluetooth_native_backend_new() from
pa_bluetooth_discovery_set_ofono_running() because with
headset=auto the native backend is not created initially. It cannot
be created there, because we don't know if ofono is running.
I sent a new patch.
_______________________________________________
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
