[PATCH 1/5] hfp_hf_bluez5: Add a card driver for HFP 1.6

[PATCH 1/5] hfp_hf_bluez5: Add a card driver for HFP 1.6

[Vinicius Costa Gomes]

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

This is just the skeleton of a Handsfree Audio Card for the HF side of
HFP 1.6.
---
 plugins/hfp_hf_bluez5.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
index ff8afba..e2fa70c 100644
--- a/plugins/hfp_hf_bluez5.c
+++ b/plugins/hfp_hf_bluez5.c
@@ -60,6 +60,8 @@
 
 #define HFP_EXT_PROFILE_PATH   "/bluetooth/profile/hfp_hf"
 
+#define HFP16_HF_DRIVER		"hfp16-hf-driver"
+
 struct hfp {
 	struct hfp_slc_info info;
 	DBusMessage *msg;
@@ -310,6 +312,31 @@ static struct ofono_modem_driver hfp_driver = {
 	.post_sim	= hfp_post_sim,
 };
 
+static int hfp16_card_probe(struct ofono_handsfree_card *card,
+					unsigned int vendor, void *data)
+{
+	return 0;
+}
+
+static void hfp16_card_remove(struct ofono_handsfree_card *card)
+{
+
+}
+
+static void hfp16_card_connect(struct ofono_handsfree_card *card,
+					ofono_handsfree_card_connect_cb_t cb,
+					void *data)
+{
+
+}
+
+static struct ofono_handsfree_card_driver hfp16_hf_driver = {
+	.name		= HFP16_HF_DRIVER,
+	.probe		= hfp16_card_probe,
+	.remove		= hfp16_card_remove,
+	.connect	= hfp16_card_connect,
+};
+
 static ofono_bool_t device_path_compare(struct ofono_modem *modem,
 					void *userdata)
 {
@@ -616,6 +643,13 @@ static int hfp_init(void)
 		return -EIO;
 	}
 
+	err = ofono_handsfree_card_driver_register(&hfp16_hf_driver);
+	if (err < 0) {
+		g_dbus_unregister_interface(conn, HFP_EXT_PROFILE_PATH,
+						BLUEZ_PROFILE_INTERFACE);
+		return err;
+	}
+
 	err = ofono_modem_driver_register(&hfp_driver);
 	if (err < 0) {
 		g_dbus_unregister_interface(conn, HFP_EXT_PROFILE_PATH,
@@ -647,6 +681,9 @@ static void hfp_exit(void)
 	bt_unregister_profile(conn, HFP_EXT_PROFILE_PATH);
 	g_dbus_unregister_interface(conn, HFP_EXT_PROFILE_PATH,
 						BLUEZ_PROFILE_INTERFACE);
+
+	ofono_handsfree_card_driver_unregister(&hfp16_hf_driver);
+
 	ofono_modem_driver_unregister(&hfp_driver);
 	g_dbus_client_unref(bluez);
 
-- 
1.8.2

[PATCH 2/5] include: Add ofono_handsfree_card_connect_sco()

[Vinicius Costa Gomes]

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

This will be used by HFP plugins to tell the core that a SCO connection
needs to be established.
---
 include/handsfree-audio.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/handsfree-audio.h b/include/handsfree-audio.h
index 8c16b22..49b0605 100644
--- a/include/handsfree-audio.h
+++ b/include/handsfree-audio.h
@@ -61,6 +61,8 @@ void ofono_handsfree_card_set_local(struct ofono_handsfree_card *card,
 					const char *local);
 const char *ofono_handsfree_card_get_local(struct ofono_handsfree_card *card);
 
+int ofono_handsfree_card_connect_sco(struct ofono_handsfree_card *card);
+
 void ofono_handsfree_audio_ref(void);
 void ofono_handsfree_audio_unref(void);
 
-- 
1.8.2

Re: [PATCH 2/5] include: Add ofono_handsfree_card_connect_sco()

[Denis Kenzior]

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

Hi Vinicius,

On 03/27/2013 07:12 PM, Vinicius Costa Gomes wrote:
> This will be used by HFP plugins to tell the core that a SCO connection
> needs to be established.
> ---
>   include/handsfree-audio.h | 2 ++
>   1 file changed, 2 insertions(+)
>

Patch has been applied, thanks.

Regards,
-Denis

[PATCH 3/5] handsfree-audio: Add ofono_handsfree_card_connect_sco

[Vinicius Costa Gomes]

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

---
 src/handsfree-audio.c | 87 ++++++++++++++++++++++++++-------------------------
 1 file changed, 44 insertions(+), 43 deletions(-)

diff --git a/src/handsfree-audio.c b/src/handsfree-audio.c
index f8df6d6..571ed54 100644
--- a/src/handsfree-audio.c
+++ b/src/handsfree-audio.c
@@ -236,40 +236,6 @@ 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 != EINPROGRESS) {
-		close(sk);
-		return -1;
-	}
-
-	return sk;
-}
-
 static gboolean sco_connect_cb(GIOChannel *io, GIOCondition cond,
 							gpointer user_data)
 
@@ -311,9 +277,8 @@ static DBusMessage *card_connect(DBusConnection *conn,
 						DBusMessage *msg, void *data)
 {
 	struct ofono_handsfree_card *card = data;
-	GIOChannel *io;
 	const char *sender;
-	int sk;
+	int err;
 
 	if (agent == NULL)
 		return __ofono_error_not_available(msg);
@@ -326,15 +291,10 @@ static DBusMessage *card_connect(DBusConnection *conn,
 	if (card->msg)
 		return __ofono_error_busy(msg);
 
-	sk = card_connect_sco(card);
-	if (sk < 0)
+	err = ofono_handsfree_card_connect_sco(card);
+	if (err < 0)
 		return __ofono_error_failed(msg);
 
-	io = g_io_channel_unix_new(sk);
-	g_io_add_watch(io, G_IO_OUT | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
-						sco_connect_cb, card);
-	g_io_channel_unref(io);
-
 	card->msg = dbus_message_ref(msg);
 
 	return NULL;
@@ -420,6 +380,47 @@ const char *ofono_handsfree_card_get_local(struct ofono_handsfree_card *card)
 	return card->local;
 }
 
+int ofono_handsfree_card_connect_sco(struct ofono_handsfree_card *card)
+{
+	GIOChannel *io;
+	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) {
+		ofono_error("Could not bind SCO socket");
+		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 != EINPROGRESS) {
+		close(sk);
+		return -1;
+	}
+
+	io = g_io_channel_unix_new(sk);
+	g_io_add_watch(io, G_IO_OUT | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
+						sco_connect_cb, card);
+	g_io_channel_unref(io);
+
+	return 0;
+}
+
 static void emit_card_added(struct ofono_handsfree_card *card)
 {
 	DBusMessage *signal;
-- 
1.8.2

Re: [PATCH 3/5] handsfree-audio: Add ofono_handsfree_card_connect_sco

[Denis Kenzior]

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

Hi Vinicius,

On 03/27/2013 07:13 PM, Vinicius Costa Gomes wrote:
> ---
>   src/handsfree-audio.c | 87 ++++++++++++++++++++++++++-------------------------
>   1 file changed, 44 insertions(+), 43 deletions(-)
>

Patch has been applied, thanks.

Regards,
-Denis

[PATCH 4/5] handsfree-audio: Add .Connect using the card driver

[Vinicius Costa Gomes]

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

Now each handsfree implementation may be notified that a card wants
its audio to be connected.
---
 src/handsfree-audio.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/src/handsfree-audio.c b/src/handsfree-audio.c
index 571ed54..3f24a2a 100644
--- a/src/handsfree-audio.c
+++ b/src/handsfree-audio.c
@@ -273,10 +273,24 @@ 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);
+
+	__ofono_dbus_pending_reply(&card->msg, 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;
 	const char *sender;
 	int err;
 
@@ -291,6 +305,17 @@ static DBusMessage *card_connect(DBusConnection *conn,
 	if (card->msg)
 		return __ofono_error_busy(msg);
 
+	if (!driver || !driver->connect)
+		goto fallback;
+
+	card->msg = dbus_message_ref(msg);
+
+	driver->connect(card, card_connect_reply_cb, card);
+
+	return NULL;
+
+fallback:
+	/* There's no driver, fallback to direct SCO connection */
 	err = ofono_handsfree_card_connect_sco(card);
 	if (err < 0)
 		return __ofono_error_failed(msg);
-- 
1.8.2

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

[Denis Kenzior]

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

Hi Vinicius,

On 03/27/2013 07:13 PM, Vinicius Costa Gomes wrote:
> Now each handsfree implementation may be notified that a card wants
> its audio to be connected.
> ---
>   src/handsfree-audio.c | 25 +++++++++++++++++++++++++
>   1 file changed, 25 insertions(+)
>

Patch has been applied, thanks.

Regards,
-Denis

[PATCH 5/5] handsfree-audio: Handle when .Connect() was already replied to

[Vinicius Costa Gomes]

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

In some cases, it is possible that when sco_connect_cb() is called
the .Connect() message (stored in card->msg) is not valid anymore.
---
 src/handsfree-audio.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/src/handsfree-audio.c b/src/handsfree-audio.c
index 3f24a2a..841cc3f 100644
--- a/src/handsfree-audio.c
+++ b/src/handsfree-audio.c
@@ -242,16 +242,19 @@ static gboolean sco_connect_cb(GIOChannel *io, GIOCondition cond,
 {
 	struct ofono_handsfree_card *card = user_data;
 	DBusMessage *reply;
-	int sk;
+	int sk, err;
 
 	if (agent == NULL) {
 		/* There's no agent, so there's no one to reply to */
-		reply = NULL;
+		if (card->msg)
+			dbus_message_unref(card->msg);
+
+		card->msg = NULL;
 		goto done;
 	}
 
 	if (cond & (G_IO_ERR | G_IO_HUP | G_IO_NVAL)) {
-		reply = __ofono_error_failed(card->msg);
+		err = -EIO;
 		goto done;
 	}
 
@@ -261,14 +264,19 @@ static gboolean sco_connect_cb(GIOChannel *io, GIOCondition cond,
 
 	close(sk);
 
-	reply = dbus_message_new_method_return(card->msg);
+	err = 0;
 
 done:
-	if (reply)
-		g_dbus_send_message(ofono_dbus_get_connection(), reply);
+	/* Or the msg was already replied to, or the agent exited */
+	if (card->msg == NULL)
+		return FALSE;
 
-	dbus_message_unref(card->msg);
-	card->msg = NULL;
+	if (err < 0)
+		reply = __ofono_error_failed(card->msg);
+	else
+		reply = dbus_message_new_method_return(card->msg);
+
+	__ofono_dbus_pending_reply(&card->msg, reply);
 
 	return FALSE;
 }
-- 
1.8.2

Re: [PATCH 5/5] handsfree-audio: Handle when .Connect() was already replied to

[Denis Kenzior]

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

Hi Vinicius,

On 03/27/2013 07:13 PM, Vinicius Costa Gomes wrote:
> In some cases, it is possible that when sco_connect_cb() is called
> the .Connect() message (stored in card->msg) is not valid anymore.

Can you tell me what cases these are?

My thinking is that we will treat the connect callback specially.

If the driver calls the callback function, then it is assumed that no 
further action is necessary.  If the driver calls connect_sco(), then we 
assume that the callback function will never be called.

> ---
>   src/handsfree-audio.c | 24 ++++++++++++++++--------
>   1 file changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/src/handsfree-audio.c b/src/handsfree-audio.c
> index 3f24a2a..841cc3f 100644
> --- a/src/handsfree-audio.c
> +++ b/src/handsfree-audio.c
> @@ -242,16 +242,19 @@ static gboolean sco_connect_cb(GIOChannel *io, GIOCondition cond,
>   {
>   	struct ofono_handsfree_card *card = user_data;
>   	DBusMessage *reply;
> -	int sk;
> +	int sk, err;
>
>   	if (agent == NULL) {
>   		/* There's no agent, so there's no one to reply to */
> -		reply = NULL;
> +		if (card->msg)
> +			dbus_message_unref(card->msg);
> +
> +		card->msg = NULL;
>   		goto done;
>   	}
>
>   	if (cond&  (G_IO_ERR | G_IO_HUP | G_IO_NVAL)) {
> -		reply = __ofono_error_failed(card->msg);
> +		err = -EIO;
>   		goto done;
>   	}
>
> @@ -261,14 +264,19 @@ static gboolean sco_connect_cb(GIOChannel *io, GIOCondition cond,
>
>   	close(sk);
>
> -	reply = dbus_message_new_method_return(card->msg);
> +	err = 0;
>
>   done:
> -	if (reply)
> -		g_dbus_send_message(ofono_dbus_get_connection(), reply);
> +	/* Or the msg was already replied to, or the agent exited */
> +	if (card->msg == NULL)
> +		return FALSE;
>
> -	dbus_message_unref(card->msg);
> -	card->msg = NULL;
> +	if (err<  0)
> +		reply = __ofono_error_failed(card->msg);
> +	else
> +		reply = dbus_message_new_method_return(card->msg);
> +
> +	__ofono_dbus_pending_reply(&card->msg, reply);
>
>   	return FALSE;
>   }

Regards,
-Denis

Re: [PATCH 5/5] handsfree-audio: Handle when .Connect() was already replied to

[Vinicius Costa Gomes]

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

Hi Denis,

Sorry for the delay.

On 09:41 Thu 28 Mar, Denis Kenzior wrote:
> Hi Vinicius,
> 
> On 03/27/2013 07:13 PM, Vinicius Costa Gomes wrote:
> >In some cases, it is possible that when sco_connect_cb() is called
> >the .Connect() message (stored in card->msg) is not valid anymore.
> 
> Can you tell me what cases these are?
> 
> My thinking is that we will treat the connect callback specially.
> 
> If the driver calls the callback function, then it is assumed that
> no further action is necessary.  If the driver calls connect_sco(),
> then we assume that the callback function will never be called.

This is indeed the case (when the HFP 1.6 driver fallbacks to direct SCO
connection). The way I was writing it I was calling connect_sco() and the
callback inside the driver.

If we consider that when you call connect_sco() you hand the connection
responsability to the core, it even makes more sense. 

So, this patch can be ignored.


Cheers,
-- 
Vinicius

Re: [PATCH 1/5] hfp_hf_bluez5: Add a card driver for HFP 1.6

[Denis Kenzior]

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

Hi Vinicius,

On 03/27/2013 07:12 PM, Vinicius Costa Gomes wrote:
> This is just the skeleton of a Handsfree Audio Card for the HF side of
> HFP 1.6.
> ---
>   plugins/hfp_hf_bluez5.c | 37 +++++++++++++++++++++++++++++++++++++
>   1 file changed, 37 insertions(+)
>

Patch has been applied, thanks.

Regards,
-Denis