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
