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

[João Paulo Rechi Vita]

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. Adapter objects removal was missing in
several places due to the fact that they were introduced in the middle
of the peer-review process, and I failed reviewing the subsequent
commits with them in mind.

>> +        goto finish;
>> +    }
>> +
>> +    if (dbus_message_is_error(r, BLUEZ_ERROR_NOT_SUPPORTED)) {
>> +        pa_log_info("Couldn't register endpoint %s because it is disabled in BlueZ", endpoint);
>> +        goto finish;
>> +    }
>> +
>> +    if (dbus_message_get_type(r) == DBUS_MESSAGE_TYPE_ERROR) {
>> +        pa_log_error("org.bluez.Media.RegisterEndpoint() failed: %s: %s", dbus_message_get_error_name(r),
>> +                     pa_dbus_get_error_message(r));
>
> The log message uses wrong interface name ("Media" instead of "Media1").
>

Yes, this should be using BLUEZ_MEDIA_INTERFACE.

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