[pulseaudio-discuss] [PATCH v4 3/8] bluetooth: ofono: Detect if Connect has been called

[Luiz Augusto von Dentz]

Hi Georg,

On Sat, Apr 29, 2017 at 4:01 PM, Georg Chini <georg at chini.tk> wrote:
> On 29.04.2017 13:28, Georg Chini wrote:
>>
>> On 26.04.2017 14:19, Luiz Augusto von Dentz wrote:
>>>
>>> From: Luiz Augusto von Dentz <luiz.von.dentz at intel.com>
>>>
>>> This detects if profile has already been called and we are waiting
>>> the response.
>>> ---
>>>   src/modules/bluetooth/backend-ofono.c | 19 ++++++++++++++++---
>>>   1 file changed, 16 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/src/modules/bluetooth/backend-ofono.c
>>> b/src/modules/bluetooth/backend-ofono.c
>>> index 3fbf153..a847ad0 100644
>>> --- a/src/modules/bluetooth/backend-ofono.c
>>> +++ b/src/modules/bluetooth/backend-ofono.c
>>> @@ -65,6 +65,7 @@ struct hf_audio_card {
>>>       char *remote_address;
>>>       char *local_address;
>>>   +    bool connecting;
>>>       int fd;
>>>       uint8_t codec;
>>>   @@ -156,12 +157,22 @@ static int
>>> hf_audio_agent_transport_acquire(pa_bluetooth_transport *t, bool opti
>>>       pa_assert(card);
>>>         if (!optional && card->fd < 0) {
>>> -        DBusMessage *m;
>>> +        DBusMessage *m, *r;
>>> +        DBusError derr;
>>>   +        if (card->connecting)
>>> +            return -1;
>>> +
>>> +        card->connecting = true;
>>> +
>>> +        dbus_error_init(&derr);
>>>           pa_assert_se(m = dbus_message_new_method_call(t->owner,
>>> t->path, "org.ofono.HandsfreeAudioCard", "Connect"));
>>> -
>>> pa_assert_se(dbus_connection_send(pa_dbus_connection_get(card->backend->connection),
>>> m, NULL));
>>> +        r =
>>> dbus_connection_send_with_reply_and_block(pa_dbus_connection_get(card->backend->connection),
>>> m, -1, &derr);
>>> +        if (!r)
>>> +            return -1;
>>>   -        return -1;
>>> +        if (card->connecting)
>>> +            return -1;
>>>       }
>>>         /* The correct block size should take into account the SCO MTU
>>> from
>>> @@ -535,6 +546,8 @@ static DBusMessage
>>> *hf_audio_agent_new_connection(DBusConnection *c, DBusMessage
>>>         card = pa_hashmap_get(backend->cards, path);
>>>   +    card->connecting = false;
>>
>>
>> Should card->connecting not be set to false immediately after entering
>> hf_audio_agent_new_connection()? If you set it here, the card will never
>> be able to connect again if the sender was wrong or
>> dbus_message_get_args() fails. Or do you expect there will be another
>> call to hf_audio_agent_new_connection() in these cases.
>
> Sorry, I did not see that the card is not known before that point, so that
> you can't set the flag before.
> But nevertheless the question remains what to do if one of the
> two errors occurs. Maybe you could do the dbus_message_get_args()
> first to identify the card. I don't know enough about the DBUS API,
> if only one argument is wrong in dbus_message_get_args(), will the other
> values be set anyway? If yes you could even try after a failure
> if the path is set.


This is the earliest we can know that the NewConnection is for the
card connecting since there could be multiple cards connecting we do
need dbus_message_get_args to complete, and no I don't think it can
parse partially as the signature doesn't match, anyway that would be
something completely wrong with ofono or something is taking its name
and pretending to be it, so I guess we are doing the right thing here,
though we probably need to add a card->connection = false to
hf_audio_agent_transport_release in case it never resumes.



-- 
Luiz Augusto von Dentz