Re: [PATCH] handsfree-audio: Add .Connect using the card driver

[Denis Kenzior <denkenz@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3019 bytes --]

Hi Vinicius,

On 03/26/2013 04:28 PM, Vinicius Costa Gomes wrote:
> Now each handsfree implementation may be notified that a card wants
> its audio to be connected.
>
> In case, that the cards wishes to fallback to the default SCO connection
> procedure, it just needs to return -ENOSYS.

Are you interpreting section 4.11.2:
"For all HF initiated audio connection establishments for which both 
sides support the Codec Negotiation feature, the HF shall trigger the AG 
to establish a Codec Connection. This is necessary because only the AG 
knows about the codec selection and settings of the network."

... to mean that AT+BCC is only sent when both sides support codec 
negotiation?  e.g. if one or the other side does not support codec 
negotiation, then we are supposed to fall back to old SCO establishment 
rules procedures?

> ---
>   src/handsfree-audio.c | 34 +++++++++++++++++++++++++++++++++-
>   1 file changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/src/handsfree-audio.c b/src/handsfree-audio.c
> index f8df6d6..ace3a4f 100644
> --- a/src/handsfree-audio.c
> +++ b/src/handsfree-audio.c
> @@ -307,13 +307,30 @@ done:
>   	return FALSE;
>   }
>
> +static void card_connect_reply_cb(const struct ofono_error *error, void *data)
> +{
> +	struct ofono_handsfree_card *card = data;
> +	DBusMessage *reply;
> +
> +	if (error->type == OFONO_ERROR_TYPE_NO_ERROR)
> +		reply = dbus_message_new_method_return(card->msg);
> +	else
> +		reply = __ofono_error_failed(card->msg);
> +
> +	g_dbus_send_message(ofono_dbus_get_connection(), reply);
> +
> +	dbus_message_unref(card->msg);
> +	card->msg = NULL;

Please replace the above three lines with __ofono_dbus_pending_reply.

> +}
> +
>   static DBusMessage *card_connect(DBusConnection *conn,
>   						DBusMessage *msg, void *data)
>   {
>   	struct ofono_handsfree_card *card = data;
> +	const struct ofono_handsfree_card_driver *driver = card->driver;
>   	GIOChannel *io;
>   	const char *sender;
> -	int sk;
> +	int sk, err;
>
>   	if (agent == NULL)
>   		return __ofono_error_not_available(msg);
> @@ -326,6 +343,21 @@ static DBusMessage *card_connect(DBusConnection *conn,
>   	if (card->msg)
>   		return __ofono_error_busy(msg);
>
> +	if (!driver || !driver->connect)
> +		goto fallback;
> +
> +	err = driver->connect(card, card_connect_reply_cb, card);
> +	if (err == -EINPROGRESS) {
> +		card->msg = dbus_message_ref(msg);
> +		return NULL;

As mentioned earlier, the driver should not return an int, however, lets 
revisit this part.  I want to better understand the AT+BCC and codec 
negotiation interactions.

> +	}
> +
> +	/* ENOSYS means the driver doesn't want to connect itself */
> +	if (err<  0&&  err != -ENOSYS)
> +		return __ofono_error_failed(msg);
> +
> +fallback:
> +	/* There's no driver, fallback to direct SCO connection */
>   	sk = card_connect_sco(card);
>   	if (sk<  0)
>   		return __ofono_error_failed(msg);

Regards,
-Denis