On Thu, 2017-05-04 at 12:58 +0300, Luiz Augusto von Dentz wrote:
> From: Luiz Augusto von Dentz <luiz.von.dentz at intel.com>
>
> This means something went wrong, which in case of ofono backend it is
> probably due to the profile not connecting immediately, but it can be
> safely restored in that case the transport is playing which means the
> profile has recovered connectivity.
> ---
> src/modules/bluetooth/module-bluez5-device.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/src/modules/bluetooth/module-bluez5-device.c b/src/modules/bluetooth/module-bluez5-device.c
> index a96da17..2f0ec97 100644
> --- a/src/modules/bluetooth/module-bluez5-device.c
> +++ b/src/modules/bluetooth/module-bluez5-device.c
> @@ -2060,8 +2060,14 @@ static pa_hook_result_t transport_state_changed_cb(pa_bluetooth_discovery *y, pa
> if (t == u->transport && t->state <= PA_BLUETOOTH_TRANSPORT_STATE_DISCONNECTED)
> pa_assert_se(pa_card_set_profile(u->card, pa_hashmap_get(u->card->profiles, "off"), false) >= 0);
>
> - if (t->device == u->device)
> + if (t->device == u->device) {
> + /* Auto recover from errors causing the profile to be set to off */
> + if (u->profile == PA_BLUETOOTH_PROFILE_OFF && t->state == PA_BLUETOOTH_TRANSPORT_STATE_PLAYING) {
> + pa_log_debug("Switching to profile %s", pa_bluetooth_profile_to_string(t->profile));
> + pa_assert_se(pa_card_set_profile(u->card, pa_hashmap_get(u->card->profiles, pa_bluetooth_profile_to_string(t->profile)), false) >= 0);
Regarding the assertion, how do you know that the card profile switch
will always succeed? Is there no IO involved? If you somehow know that
it will always succeed, there should be a comment explaining how you
know that.
I know there are already assertions when switching to the off profile,
but that's a special profile - activating it should never fail.
This change seems dubious in another way too. So far we've kept all
automatic profile switch code out of module-bluez5-device (apart from
switches to "off" when things fail). Automatic switching is handled by
module-bluetooth-policy instead. If I understood correctly, this is a
fix for a situation where we already tried to switch to a profile, but
it failed first. The code is lacking any checks that would do the
profile change only if there really was a prior profile switch attempt.
--
Tanu
https://www.patreon.com/tanuk
