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

[João Paulo Rechi Vita]

On Sun, Sep 22, 2013 at 2:34 AM, Tanu Kaskinen
<tanu.kaskinen at linux.intel.com> wrote:
> 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.
>

For several here you mean 2, and one of this steps was added later.
Whether or not having a wrapper function seems a matter of personal
preference to me.

> 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.
>

Ok, we can do it this way. I personally prefer not being
super-protective as we are in some other places.

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