[pulseaudio-discuss] [PATCH v4 16/41] bluetooth: Register endpoints with BlueZ 5 adapter

[Tanu Kaskinen]

On Sat, 2013-09-21 at 14:17 -0500, João Paulo Rechi Vita wrote:
> On Sat, Sep 21, 2013 at 7:44 AM, Tanu Kaskinen
> <tanu.kaskinen at linux.intel.com> wrote:
> > On Wed, 2013-09-18 at 16:17 -0500, jprvita at gmail.com wrote:
> >> From: João Paulo Rechi Vita <jprvita at openbossa.org>
> >>
> >> ---
> >>  src/modules/bluetooth/bluez5-util.c | 82 ++++++++++++++++++++++++++++++++++++-
> >>  1 file changed, 81 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/src/modules/bluetooth/bluez5-util.c b/src/modules/bluetooth/bluez5-util.c
> >> index 4bc5967..03c73c9 100644
> >> --- a/src/modules/bluetooth/bluez5-util.c
> >> +++ b/src/modules/bluetooth/bluez5-util.c
> >> @@ -40,9 +40,12 @@
> >>  #define BLUEZ_SERVICE "org.bluez"
> >>  #define BLUEZ_ADAPTER_INTERFACE BLUEZ_SERVICE ".Adapter1"
> >>  #define BLUEZ_DEVICE_INTERFACE BLUEZ_SERVICE ".Device1"
> >> +#define BLUEZ_MEDIA_INTERFACE BLUEZ_SERVICE ".Media1"
> >>  #define BLUEZ_MEDIA_ENDPOINT_INTERFACE BLUEZ_SERVICE ".MediaEndpoint1"
> >>  #define BLUEZ_MEDIA_TRANSPORT_INTERFACE BLUEZ_SERVICE ".MediaTransport1"
> >>
> >> +#define BLUEZ_ERROR_NOT_SUPPORTED "org.bluez.Error.NotSupported"
> >> +
> >>  #define A2DP_SOURCE_ENDPOINT "/MediaEndpoint/A2DPSource"
> >>  #define A2DP_SINK_ENDPOINT "/MediaEndpoint/A2DPSink"
> >>
> >> @@ -439,6 +442,81 @@ static int parse_adapter_properties(pa_bluetooth_adapter *a, DBusMessageIter *i,
> >>      return 0;
> >>  }
> >>
> >> +static void register_endpoint_reply(DBusPendingCall *pending, void *userdata) {
> >> +    DBusMessage *r;
> >> +    pa_dbus_pending *p;
> >> +    pa_bluetooth_discovery *y;
> >> +    char *endpoint;
> >> +
> >> +    pa_assert(pending);
> >> +    pa_assert_se(p = userdata);
> >> +    pa_assert_se(y = p->context_data);
> >> +    pa_assert_se(endpoint = p->call_data);
> >> +    pa_assert_se(r = dbus_pending_call_steal_reply(pending));
> >> +
> >> +    if (dbus_message_is_error(r, DBUS_ERROR_SERVICE_UNKNOWN)) {
> >> +        pa_log_debug("Bluetooth daemon is apparently not available");
> >> +        device_remove_all(y);
> >
> > Adapters should be removed too. I think there should be a function that
> > takes care of clearing everything, so that it's not necessary to
> > remember to remove both devices and adapters everywhere where we notice
> > that bluetoothd is non-functional.
> >
> 
> We use device_remove_all and adapter_remove_all independently in
> pa_done, and I don't like the idea of having one function that simply
> wrap to functions together.

Why? The log message says: "Bluetooth daemon is apparently not
available". The thing to do in that situation is to clear all state, and
if clearing all state takes several steps (as it does) and if the same
clearing needs to be done in multiple places (as it does), then a helper
function seems like the obvious choice.

However, it looks to me that checking for DBUS_ERROR_SERVICE_UNKNOWN
here is pretty pointless. We shouldn't receive that error as long as
there's an owner for the org.bluez name, and when the owner of the name
goes away, we get a notification about that and do the cleanup at that
point.

-- 
Tanu