Hi Denis,
On 22:38 Tue 19 Mar, Denis Kenzior wrote:
> Hi Vinicius,
>
> On 03/19/2013 07:10 PM, Vinicius Costa Gomes wrote:
> >When calling the card's .Connect() method, we should be able to
> >establish a SCO connection.
> >
> >Right now, we only have support for establishing the SCO connection
> >directly, this is what is expected from HFP 1.5 HF/AG devices.
> >---
> > src/handsfree-audio.c | 94
> > ++++++++++++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 93 insertions(+), 1 deletion(-)
> >
> >diff --git a/src/handsfree-audio.c b/src/handsfree-audio.c
> >index c7fa2fb..9eecb8e 100644
> >--- a/src/handsfree-audio.c
> >+++ b/src/handsfree-audio.c
> >@@ -53,6 +53,8 @@ struct ofono_handsfree_card {
> > char *remote;
> > char *local;
> > char *path;
> >+ DBusMessage *msg;
> >+ guint sco_watch;
> > const struct ofono_handsfree_card_driver *driver;
> > void *driver_data;
> > };
> >@@ -235,10 +237,100 @@ static DBusMessage
> >*card_get_properties(DBusConnection *conn,
> > return reply;
> > }
> >
> >+static int card_connect_sco(struct ofono_handsfree_card *card)
> >+{
> >+ struct sockaddr_sco addr;
> >+ int sk, ret;
> >+
> >+ sk = socket(PF_BLUETOOTH, SOCK_SEQPACKET | O_NONBLOCK | SOCK_CLOEXEC,
> >+ BTPROTO_SCO);
> >+ if (sk< 0)
> >+ return -1;
> >+
> >+ /* Bind to local address */
> >+ memset(&addr, 0, sizeof(addr));
> >+ addr.sco_family = AF_BLUETOOTH;
> >+ bt_str2ba(card->local,&addr.sco_bdaddr);
> >+
> >+ if (bind(sk, (struct sockaddr *)&addr, sizeof(addr))< 0) {
> >+ close(sk);
> >+ return -1;
> >+ }
> >+
> >+ /* Connect to remote device */
> >+ memset(&addr, 0, sizeof(addr));
> >+ addr.sco_family = AF_BLUETOOTH;
> >+ bt_str2ba(card->remote,&addr.sco_bdaddr);
> >+
> >+ ret = connect(sk, (struct sockaddr *)&addr, sizeof(addr));
> >+ if (ret< 0&& !(errno == EAGAIN || errno == EINPROGRESS)) {
>
> Why do we use EAGAIN here? I thought only EINPROGRESS can be
> returned from a non-blocking connect. Or are Bluetooth semantics
> different?
Just checked, you are right: only EINPROGRSS will be returned in this case.
Will fix.
>
> >+ close(sk);
> >+ return -1;
> >+ }
> >+
> >+ return sk;
> >+}
> >+
> >+static gboolean sco_connect_cb(GIOChannel *io, GIOCondition cond,
> >+ gpointer user_data)
> >+
> >+{
> >+ struct ofono_handsfree_card *card = user_data;
> >+ DBusMessage *reply;
> >+ int sk;
> >+
> >+ if (cond& (G_IO_ERR | G_IO_HUP | G_IO_NVAL)) {
> >+ reply = __ofono_error_failed(card->msg);
> >+ goto done;
> >+ }
> >+
> >+ sk = g_io_channel_unix_get_fd(io);
> >+
> >+ close(sk);
> >+
> >+ reply = dbus_message_new_method_return(card->msg);
> >+
>
> We probably should be paranoid and check whether we still have the agent.
For the method return message of the 'Connect()' call, I can't see any gains of
this check, but for sending the 'NewConnection()' message I can (and that is
what the next patch does). Could be that I am missing something.
One thing I just noticed that is missing is that there's no check that only
the agent may call 'Connect()'.
>
> >+done:
> >+ g_dbus_send_message(ofono_dbus_get_connection(), reply);
> >+
> >+ return FALSE;
> >+}
> >+
> >+static void sco_watch_destroy(gpointer user_data)
> >+{
> >+ struct ofono_handsfree_card *card = user_data;
> >+
> >+ card->sco_watch = 0;
> >+ dbus_message_unref(card->msg);
> >+}
> >+
> > static DBusMessage *card_connect(DBusConnection *conn,
> > DBusMessage *msg, void *data)
> > {
> >- return __ofono_error_not_implemented(msg);
> >+ struct ofono_handsfree_card *card = data;
> >+ GIOChannel *io;
> >+ int sk;
> >+
> >+ if (agent == NULL)
> >+ return __ofono_error_not_available(msg);
> >+
> >+ if (card->msg)
> >+ return __ofono_error_busy(msg);
> >+
> >+ sk = card_connect_sco(card);
>
> I presume this fails if we already have a SCO link?
It will, but I will replace the check above (card->msg) with a check for
sco_watch, to make it clearer.
>
> >+ if (sk< 0)
> >+ return __ofono_error_failed(msg);
> >+
> >+ io = g_io_channel_unix_new(sk);
> >+ card->sco_watch = g_io_add_watch_full(io, G_PRIORITY_DEFAULT,
> >+ G_IO_OUT | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
> >+ sco_connect_cb, card, sco_watch_destroy);
> >+
> >+ g_io_channel_unref(io);
> >+
> >+ card->msg = dbus_message_ref(msg);
> >+
> >+ return NULL;
> > }
> >
> > static const GDBusMethodTable card_methods[] = {
>
> Regards,
> -Denis
Cheers,
--
Vinicius
_______________________________________________
ofono mailing list
[email protected]
http://lists.ofono.org/listinfo/ofono
