On Sun, 7 Aug 2016, at 09:15 PM, Tanu Kaskinen wrote:
> The CONNECTION_CHANGED hook is used to notify the discovery module
> about new and removed devices. When a bluetooth device connects, the
> hook used to be called immediately when the first profile connected.
> That meant that only one profile was marked as available during the
> card creation, other profiles would get marked as available later.
>
> That makes it hard for module-card-restore to restore the saved
> profile, if the saved profile becomes available with some delay.
> module-card-restore has a workaround for this problem, but that turned
> out to interfere with module-bluetooth-policy, so the workaround will
> be removed in the next patch.
>
> The BlueZ 4 code doesn't need changes, because we use the
> org.bluez.Audio interface to get a notification when all profiles are
> connected.
> ---
Except one trivial comment I have below, the first 4 patches of the
series look okay to me. I'm not too fussed about changes to the bluez4
side of things, so unless there's something that you'd specifically like
a second pair of eyes on, if it works, just go ahead and push it.
-- Arun
> src/modules/bluetooth/bluez5-util.c | 120
> +++++++++++++++++++++++++++++++++++-
> src/modules/bluetooth/bluez5-util.h | 2 +
> 2 files changed, 120 insertions(+), 2 deletions(-)
>
> diff --git a/src/modules/bluetooth/bluez5-util.c
> b/src/modules/bluetooth/bluez5-util.c
> index 03c76bf..f6c18e7 100644
> --- a/src/modules/bluetooth/bluez5-util.c
> +++ b/src/modules/bluetooth/bluez5-util.c
> @@ -21,6 +21,8 @@
> #include <config.h>
> #endif
>
> +#include <pulse/rtclock.h>
> +#include <pulse/timeval.h>
> #include <pulse/xmalloc.h>
>
> #include <pulsecore/core.h>
> @@ -35,6 +37,8 @@
>
> #include "bluez5-util.h"
>
> +#define WAIT_FOR_PROFILES_TIMEOUT_USEC (3 * PA_USEC_PER_SEC)
> +
> #define BLUEZ_SERVICE "org.bluez"
> #define BLUEZ_ADAPTER_INTERFACE BLUEZ_SERVICE ".Adapter1"
> #define BLUEZ_DEVICE_INTERFACE BLUEZ_SERVICE ".Device1"
> @@ -164,6 +168,97 @@ static const char
> *transport_state_to_string(pa_bluetooth_transport_state_t stat
> return "invalid";
> }
>
> +static bool device_supports_profile(pa_bluetooth_device *device,
> pa_bluetooth_profile_t profile) {
> + switch (profile) {
> + case PA_BLUETOOTH_PROFILE_A2DP_SINK:
> + return !!pa_hashmap_get(device->uuids,
> PA_BLUETOOTH_UUID_A2DP_SINK);
> + case PA_BLUETOOTH_PROFILE_A2DP_SOURCE:
> + return !!pa_hashmap_get(device->uuids,
> PA_BLUETOOTH_UUID_A2DP_SOURCE);
> + case PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT:
> + return !!pa_hashmap_get(device->uuids,
> PA_BLUETOOTH_UUID_HSP_HS)
> + || !!pa_hashmap_get(device->uuids,
> PA_BLUETOOTH_UUID_HFP_HF);
> + case PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY:
> + return !!pa_hashmap_get(device->uuids,
> PA_BLUETOOTH_UUID_HSP_AG)
> + || !!pa_hashmap_get(device->uuids,
> PA_BLUETOOTH_UUID_HFP_AG);
> + case PA_BLUETOOTH_PROFILE_OFF:
> + pa_assert_not_reached();
> + }
> +
> + pa_assert_not_reached();
> +}
> +
> +static bool device_is_profile_connected(pa_bluetooth_device *device,
> pa_bluetooth_profile_t profile) {
> + if (device->transports[profile] &&
> device->transports[profile]->state !=
> PA_BLUETOOTH_TRANSPORT_STATE_DISCONNECTED)
> + return true;
> + else
> + return false;
> +}
> +
> +static unsigned device_count_disconnected_profiles(pa_bluetooth_device
> *device) {
> + pa_bluetooth_profile_t profile;
> + unsigned count = 0;
> +
> + for (profile = 0; profile < PA_BLUETOOTH_PROFILE_COUNT; profile++) {
> + if (!device_supports_profile(device, profile))
> + continue;
> +
> + if (!device_is_profile_connected(device, profile))
> + count++;
> + }
> +
> + return count;
> +}
> +
> +static void device_stop_waiting_for_profiles(pa_bluetooth_device
> *device);
I think it's better to just have the code here rather than just the
declaration.
> +static void wait_for_profiles_cb(pa_mainloop_api *api, pa_time_event*
> event, const struct timeval *tv, void *userdata) {
> + pa_bluetooth_device *device = userdata;
> + pa_strbuf *buf;
> + pa_bluetooth_profile_t profile;
> + bool first = true;
> + char *profiles_str;
> +
> + device_stop_waiting_for_profiles(device);
> +
> + buf = pa_strbuf_new();
> +
> + for (profile = 0; profile < PA_BLUETOOTH_PROFILE_COUNT; profile++) {
> + if (device_is_profile_connected(device, profile))
> + continue;
> +
> + if (!device_supports_profile(device, profile))
> + continue;
> +
> + if (first)
> + first = false;
> + else
> + pa_strbuf_puts(buf, ", ");
> +
> + pa_strbuf_puts(buf, pa_bluetooth_profile_to_string(profile));
> + }
> +
> + profiles_str = pa_strbuf_to_string_free(buf);
> + pa_log_debug("Timeout expired, and device %s still has disconnected
> profiles: %s",
> + device->path, profiles_str);
> + pa_xfree(profiles_str);
> +
> pa_hook_fire(&device->discovery->hooks[PA_BLUETOOTH_HOOK_DEVICE_CONNECTION_CHANGED],
> device);
> +}
> +
> +static void device_start_waiting_for_profiles(pa_bluetooth_device
> *device) {
> + pa_assert(!device->wait_for_profiles_timer);
> + device->wait_for_profiles_timer =
> pa_core_rttime_new(device->discovery->core,
> +
> pa_rtclock_now() + WAIT_FOR_PROFILES_TIMEOUT_USEC,
> +
> wait_for_profiles_cb, device);
> +}
> +
> +static void device_stop_waiting_for_profiles(pa_bluetooth_device
> *device) {
> + if (!device->wait_for_profiles_timer)
> + return;
> +
> +
> device->discovery->core->mainloop->time_free(device->wait_for_profiles_timer);
> + device->wait_for_profiles_timer = NULL;
> +}
> +
> void pa_bluetooth_transport_set_state(pa_bluetooth_transport *t,
> pa_bluetooth_transport_state_t state) {
> bool old_any_connected;
>
> @@ -181,8 +276,27 @@ void
> pa_bluetooth_transport_set_state(pa_bluetooth_transport *t,
> pa_bluetooth_tr
>
> pa_hook_fire(&t->device->discovery->hooks[PA_BLUETOOTH_HOOK_TRANSPORT_STATE_CHANGED],
> t);
>
> - if (old_any_connected !=
> pa_bluetooth_device_any_transport_connected(t->device))
> -
> pa_hook_fire(&t->device->discovery->hooks[PA_BLUETOOTH_HOOK_DEVICE_CONNECTION_CHANGED],
> t->device);
> + if (old_any_connected !=
> pa_bluetooth_device_any_transport_connected(t->device)) {
> + unsigned n_disconnected_profiles;
> +
> + /* If there are profiles that are expected to get connected soon
> (based
> + * on the UUID list), we wait for a bit before announcing the
> new
> + * device, so that all profiles have time to get connected
> before the
> + * card object is created. If we didn't wait, the card would
> always
> + * have only one profile marked as available in the initial
> state,
> + * which would prevent module-card-restore from restoring the
> initial
> + * profile properly. */
> +
> + n_disconnected_profiles =
> device_count_disconnected_profiles(t->device);
> +
> + if (n_disconnected_profiles == 0)
> + device_stop_waiting_for_profiles(t->device);
> +
> + if (!old_any_connected && n_disconnected_profiles > 0)
> + device_start_waiting_for_profiles(t->device);
> + else
> +
> pa_hook_fire(&t->device->discovery->hooks[PA_BLUETOOTH_HOOK_DEVICE_CONNECTION_CHANGED],
> t->device);
> + }
> }
>
> void pa_bluetooth_transport_put(pa_bluetooth_transport *t) {
> @@ -416,6 +530,8 @@ static void device_free(pa_bluetooth_device *d) {
>
> pa_assert(d);
>
> + device_stop_waiting_for_profiles(d);
> +
> for (i = 0; i < PA_BLUETOOTH_PROFILE_COUNT; i++) {
> pa_bluetooth_transport *t;
>
> diff --git a/src/modules/bluetooth/bluez5-util.h
> b/src/modules/bluetooth/bluez5-util.h
> index d66e8a3..2aa6ea6 100644
> --- a/src/modules/bluetooth/bluez5-util.h
> +++ b/src/modules/bluetooth/bluez5-util.h
> @@ -105,6 +105,8 @@ struct pa_bluetooth_device {
> pa_hashmap *uuids;
>
> pa_bluetooth_transport *transports[PA_BLUETOOTH_PROFILE_COUNT];
> +
> + pa_time_event *wait_for_profiles_timer;
> };
>
> struct pa_bluetooth_adapter {
> --
> 2.8.1
>
> _______________________________________________
> pulseaudio-discuss mailing list
> pulseaudio-discuss at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
