[PATCH v0 00/10] handsfree-audio: Add Agent NewConnection()

[PATCH v0 00/10] handsfree-audio: Add Agent NewConnection()

[Claudio Takahasi]

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

This patch series moves the SCO connection handling from hfp_hf_bluez5.c
to src/handsfree-audio.c, and adds the Agent NewConnection() method call
to pass the SCO file descriptor from oFono to the Agent implementation.

Claudio Takahasi (10):
  bluetooth: Add new Bluetooth header
  handsfree-audio: Move SCO to handsfree-audio.c
  handsfree-audio: Remove modem dependency
  handsfree-audio: Add NewConnection
  handsfree-audio: Check local SCO address
  handsfree-audio: Reject SCO if Card is not ready
  handsfree-audio: Reject SCO if agent is unavailable
  handsfree-audio: Check CVSD when registering agent
  handsfree-audio: Add function to get hfp version
  hfp_hf_bluez5: Fix hard-coded hfp version

 Makefile.am               |   2 +-
 include/handsfree-audio.h |   1 +
 plugins/bluez5.c          |  17 -----
 plugins/bluez5.h          |  41 ------------
 plugins/hfp_hf_bluez5.c   | 105 +-----------------------------
 src/bluetooth.h           |  71 ++++++++++++++++++++
 src/handsfree-audio.c     | 162 +++++++++++++++++++++++++++++++++++++++++++++-
 7 files changed, 235 insertions(+), 164 deletions(-)
 create mode 100644 src/bluetooth.h

-- 
1.7.11.7

[PATCH v0 01/10] bluetooth: Add new Bluetooth header

[Claudio Takahasi]

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

This patch moves the Bluetooth utility funtions and socket type
declarations to a new header src/bluetooth.h, allowing to share it
between core, and plugins.
---
 Makefile.am             |  2 +-
 plugins/bluez5.c        | 17 ------------
 plugins/bluez5.h        | 41 ----------------------------
 plugins/hfp_hf_bluez5.c |  1 +
 src/bluetooth.h         | 71 +++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 73 insertions(+), 59 deletions(-)
 create mode 100644 src/bluetooth.h

diff --git a/Makefile.am b/Makefile.am
index 557f499..3b998af 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -509,7 +509,7 @@ src_ofonod_SOURCES = $(gdbus_sources) $(builtin_sources) src/ofono.ver \
 			src/cdma-smsutil.h src/cdma-smsutil.c \
 			src/cdma-sms.c src/private-network.c src/cdma-netreg.c \
 			src/cdma-provision.c src/handsfree.c \
-			src/handsfree-audio.c
+			src/handsfree-audio.c src/bluetooth.h
 
 src_ofonod_LDADD = $(builtin_libadd) @GLIB_LIBS@ @DBUS_LIBS@ -ldl
 
diff --git a/plugins/bluez5.c b/plugins/bluez5.c
index 788f3a2..278f062 100644
--- a/plugins/bluez5.c
+++ b/plugins/bluez5.c
@@ -26,7 +26,6 @@
 #include <errno.h>
 #include <stdint.h>
 #include <stdio.h>
-#include <sys/socket.h>
 #include <string.h>
 
 #include <glib.h>
@@ -48,22 +47,6 @@ struct finish_callback {
 	char *member;
 };
 
-void bt_bacpy(bdaddr_t *dst, const bdaddr_t *src)
-{
-	memcpy(dst, src, sizeof(bdaddr_t));
-}
-
-int bt_ba2str(const bdaddr_t *ba, char *str)
-{
-	return sprintf(str, "%2.2X:%2.2X:%2.2X:%2.2X:%2.2X:%2.2X",
-		ba->b[5], ba->b[4], ba->b[3], ba->b[2], ba->b[1], ba->b[0]);
-}
-
-int bt_bacmp(const bdaddr_t *ba1, const bdaddr_t *ba2)
-{
-	return memcmp(ba1, ba2, sizeof(bdaddr_t));
-}
-
 static void profile_register_cb(DBusPendingCall *call, gpointer user_data)
 {
 	DBusMessage *reply;
diff --git a/plugins/bluez5.h b/plugins/bluez5.h
index 1432068..f6c3b66 100644
--- a/plugins/bluez5.h
+++ b/plugins/bluez5.h
@@ -28,47 +28,6 @@
 #define HFP_HS_UUID	"0000111e-0000-1000-8000-00805f9b34fb"
 #define HFP_AG_UUID	"0000111f-0000-1000-8000-00805f9b34fb"
 
-#ifndef AF_BLUETOOTH
-#define AF_BLUETOOTH		31
-#define PF_BLUETOOTH		AF_BLUETOOTH
-#endif
-
-#define BTPROTO_SCO		2
-
-#define SOL_SCO			17
-
-#ifndef SOL_BLUETOOTH
-#define SOL_BLUETOOTH		274
-#endif
-
-#define BT_DEFER_SETUP		7
-
-/* BD Address */
-typedef struct {
-	uint8_t b[6];
-} __attribute__((packed)) bdaddr_t;
-
-#define BDADDR_ANY   (&(bdaddr_t) {{0, 0, 0, 0, 0, 0}})
-
-/* RFCOMM socket address */
-struct sockaddr_rc {
-	sa_family_t	rc_family;
-	bdaddr_t	rc_bdaddr;
-	uint8_t		rc_channel;
-};
-
-/* SCO socket address */
-struct sockaddr_sco {
-	sa_family_t	sco_family;
-	bdaddr_t	sco_bdaddr;
-};
-
-void bt_bacpy(bdaddr_t *dst, const bdaddr_t *src);
-
-int bt_ba2str(const bdaddr_t *ba, char *str);
-
-int bt_bacmp(const bdaddr_t *ba1, const bdaddr_t *ba2);
-
 int bt_register_profile(DBusConnection *conn, const char *uuid,
 					uint16_t version, const char *name,
 					const char *object);
diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
index 2db2fa5..dea0f34 100644
--- a/plugins/hfp_hf_bluez5.c
+++ b/plugins/hfp_hf_bluez5.c
@@ -51,6 +51,7 @@
 
 #include <drivers/hfpmodem/slc.h>
 
+#include "bluetooth.h"
 #include "bluez5.h"
 
 #ifndef DBUS_TYPE_UNIX_FD
diff --git a/src/bluetooth.h b/src/bluetooth.h
new file mode 100644
index 0000000..1584109
--- /dev/null
+++ b/src/bluetooth.h
@@ -0,0 +1,71 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2013  Intel Corporation. All rights reserved.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifndef AF_BLUETOOTH
+#define AF_BLUETOOTH		31
+#define PF_BLUETOOTH		AF_BLUETOOTH
+#endif
+
+#define BTPROTO_SCO		2
+
+#define SOL_SCO			17
+
+#ifndef SOL_BLUETOOTH
+#define SOL_BLUETOOTH		274
+#endif
+
+#define BT_DEFER_SETUP		7
+
+/* BD Address */
+typedef struct {
+	uint8_t b[6];
+} __attribute__((packed)) bdaddr_t;
+
+#define BDADDR_ANY   (&(bdaddr_t) {{0, 0, 0, 0, 0, 0}})
+
+/* RFCOMM socket address */
+struct sockaddr_rc {
+	sa_family_t	rc_family;
+	bdaddr_t	rc_bdaddr;
+	uint8_t		rc_channel;
+};
+
+/* SCO socket address */
+struct sockaddr_sco {
+	sa_family_t	sco_family;
+	bdaddr_t	sco_bdaddr;
+};
+
+static inline void bt_bacpy(bdaddr_t *dst, const bdaddr_t *src)
+{
+	memcpy(dst, src, sizeof(bdaddr_t));
+}
+
+static inline int bt_ba2str(const bdaddr_t *ba, char *str)
+{
+	return sprintf(str, "%2.2X:%2.2X:%2.2X:%2.2X:%2.2X:%2.2X",
+		ba->b[5], ba->b[4], ba->b[3], ba->b[2], ba->b[1], ba->b[0]);
+}
+
+static inline int bt_bacmp(const bdaddr_t *ba1, const bdaddr_t *ba2)
+{
+	return memcmp(ba1, ba2, sizeof(bdaddr_t));
+}
-- 
1.7.11.7

[PATCH v0 02/10] handsfree-audio: Move SCO to handsfree-audio.c

[Claudio Takahasi]

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

 This patch moves the SCO socket handling from hfp_hf_bluez5 plugin to
 handsfree-audio.c file.

 This is the initial step to be able to support sending the file
 descriptor through the Agent NewConnection method.
---
 plugins/hfp_hf_bluez5.c | 102 +----------------------------------------------
 src/handsfree-audio.c   | 103 +++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 103 insertions(+), 102 deletions(-)

diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
index dea0f34..ea2c848 100644
--- a/plugins/hfp_hf_bluez5.c
+++ b/plugins/hfp_hf_bluez5.c
@@ -67,8 +67,6 @@ struct hfp {
 };
 
 static GDBusClient *bluez = NULL;
-static guint sco_watch = 0;
-static uint16_t local_hfp_version = HFP_VERSION_1_6;
 
 static void hfp_debug(const char *str, void *user_data)
 {
@@ -482,100 +480,11 @@ static const GDBusMethodTable profile_methods[] = {
 	{ }
 };
 
-static ofono_bool_t slc_match(struct ofono_modem *modem, void *userdata)
-{
-	const char *remote = userdata;
-	const char *value = ofono_modem_get_string(modem, "Remote");
-
-	if (value == NULL)
-		return FALSE;
-
-	/* Make sure SLC has been established */
-	if (ofono_modem_get_powered(modem) != TRUE)
-		return FALSE;
-
-	return g_str_equal(remote, value);
-}
-
-static gboolean sco_accept(GIOChannel *io, GIOCondition cond,
-							gpointer user_data)
-{
-	struct sockaddr_sco saddr;
-	socklen_t alen;
-	int sk, nsk;
-	char remote[18];
-
-	if (cond & (G_IO_ERR | G_IO_HUP | G_IO_NVAL))
-		return FALSE;
-
-	sk = g_io_channel_unix_get_fd(io);
-
-	memset(&saddr, 0, sizeof(saddr));
-	alen = sizeof(saddr);
-
-	nsk = accept(sk, (struct sockaddr *) &saddr, &alen);
-	if (nsk < 0)
-		return TRUE;
-
-	bt_ba2str(&saddr.sco_bdaddr, remote);
-
-	if (ofono_modem_find(slc_match, remote) == NULL) {
-		ofono_error("Rejecting SCO: No SLC connection found!");
-		close(nsk);
-		return TRUE;
-	}
-
-	return TRUE;
-}
-
-static int sco_init(void)
-{
-	GIOChannel *sco_io;
-	struct sockaddr_sco saddr;
-	int sk, defer_setup = 1;
-
-	sk = socket(PF_BLUETOOTH, SOCK_SEQPACKET | O_NONBLOCK | SOCK_CLOEXEC,
-								BTPROTO_SCO);
-	if (sk < 0)
-		return -errno;
-
-	/* Bind to local address */
-	memset(&saddr, 0, sizeof(saddr));
-	saddr.sco_family = AF_BLUETOOTH;
-	bt_bacpy(&saddr.sco_bdaddr, BDADDR_ANY);
-
-	if (bind(sk, (struct sockaddr *) &saddr, sizeof(saddr)) < 0) {
-		close(sk);
-		return -errno;
-	}
-
-	if (setsockopt(sk, SOL_BLUETOOTH, BT_DEFER_SETUP,
-				&defer_setup, sizeof(defer_setup)) < 0) {
-		ofono_warn("Can't enable deferred setup: %s (%d)",
-						strerror(errno), errno);
-		local_hfp_version = HFP_VERSION_1_5;
-	}
-
-	if (listen(sk, 5) < 0) {
-		close(sk);
-		return -errno;
-	}
-
-	sco_io = g_io_channel_unix_new(sk);
-	sco_watch = g_io_add_watch(sco_io,
-				G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
-				sco_accept, NULL);
-
-	g_io_channel_unref(sco_io);
-
-	return 0;
-}
-
 static void connect_handler(DBusConnection *conn, void *user_data)
 {
 	DBG("Registering External Profile handler ...");
 
-	bt_register_profile(conn, HFP_HS_UUID, local_hfp_version, "hfp_hf",
+	bt_register_profile(conn, HFP_HS_UUID, HFP_VERSION_1_5, "hfp_hf",
 						HFP_EXT_PROFILE_PATH);
 }
 
@@ -695,12 +604,6 @@ static int hfp_init(void)
 	if (DBUS_TYPE_UNIX_FD < 0)
 		return -EBADF;
 
-	err = sco_init();
-	if (err < 0) {
-		ofono_error("SCO: %s(%d)", strerror(-err), -err);
-		return err;
-	}
-
 	/* Registers External Profile handler */
 	if (!g_dbus_register_interface(conn, HFP_EXT_PROFILE_PATH,
 					BLUEZ_PROFILE_INTERFACE,
@@ -742,9 +645,6 @@ static void hfp_exit(void)
 						BLUEZ_PROFILE_INTERFACE);
 	ofono_modem_driver_unregister(&hfp_driver);
 	g_dbus_client_unref(bluez);
-
-	if (sco_watch > 0)
-		g_source_remove(sco_watch);
 }
 
 OFONO_PLUGIN_DEFINE(hfp_bluez5, "External Hands-Free Profile Plugin", VERSION,
diff --git a/src/handsfree-audio.c b/src/handsfree-audio.c
index b2d4b97..14488ac 100644
--- a/src/handsfree-audio.c
+++ b/src/handsfree-audio.c
@@ -25,12 +25,19 @@
 
 #include <errno.h>
 #include <stdio.h>
+#include <stdint.h>
 #include <string.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <sys/socket.h>
 
 #include <gdbus.h>
+#include <gatchat.h>
 
+#include <drivers/hfpmodem/slc.h>
 #include <ofono/handsfree-audio.h>
 
+#include "bluetooth.h"
 #include "ofono.h"
 
 #define HFP_AUDIO_MANAGER_INTERFACE	OFONO_SERVICE ".HandsfreeAudioManager"
@@ -60,6 +67,97 @@ struct agent {
 static struct agent *agent = NULL;
 static int ref_count = 0;
 static GSList *card_list = 0;
+static guint sco_watch = 0;
+static uint16_t local_hfp_version = HFP_VERSION_1_6;
+
+static ofono_bool_t slc_match(struct ofono_modem *modem, void *userdata)
+{
+	const char *remote = userdata;
+	const char *value = ofono_modem_get_string(modem, "Remote");
+
+	if (value == NULL)
+		return FALSE;
+
+	/* Make sure SLC has been established */
+	if (ofono_modem_get_powered(modem) != TRUE)
+		return FALSE;
+
+	return g_str_equal(remote, value);
+}
+
+static gboolean sco_accept(GIOChannel *io, GIOCondition cond,
+							gpointer user_data)
+{
+	struct sockaddr_sco saddr;
+	socklen_t alen;
+	int sk, nsk;
+	char remote[18];
+
+	if (cond & (G_IO_ERR | G_IO_HUP | G_IO_NVAL))
+		return FALSE;
+
+	sk = g_io_channel_unix_get_fd(io);
+
+	memset(&saddr, 0, sizeof(saddr));
+	alen = sizeof(saddr);
+
+	nsk = accept(sk, (struct sockaddr *) &saddr, &alen);
+	if (nsk < 0)
+		return TRUE;
+
+	bt_ba2str(&saddr.sco_bdaddr, remote);
+
+	if (ofono_modem_find(slc_match, remote) == NULL) {
+		ofono_error("Rejecting SCO: No SLC connection found!");
+		close(nsk);
+		return TRUE;
+	}
+
+	return TRUE;
+}
+
+static int sco_init(void)
+{
+	GIOChannel *sco_io;
+	struct sockaddr_sco saddr;
+	int sk, defer_setup = 1;
+
+	sk = socket(PF_BLUETOOTH, SOCK_SEQPACKET | O_NONBLOCK | SOCK_CLOEXEC,
+								BTPROTO_SCO);
+	if (sk < 0)
+		return -errno;
+
+	/* Bind to local address */
+	memset(&saddr, 0, sizeof(saddr));
+	saddr.sco_family = AF_BLUETOOTH;
+	bt_bacpy(&saddr.sco_bdaddr, BDADDR_ANY);
+
+	if (bind(sk, (struct sockaddr *) &saddr, sizeof(saddr)) < 0) {
+		close(sk);
+		return -errno;
+	}
+
+	if (setsockopt(sk, SOL_BLUETOOTH, BT_DEFER_SETUP,
+				&defer_setup, sizeof(defer_setup)) < 0) {
+		ofono_warn("Can't enable deferred setup: %s (%d)",
+						strerror(errno), errno);
+		local_hfp_version = HFP_VERSION_1_5;
+	}
+
+	if (listen(sk, 5) < 0) {
+		close(sk);
+		return -errno;
+	}
+
+	sco_io = g_io_channel_unix_new(sk);
+	sco_watch = g_io_add_watch(sco_io,
+				G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
+				sco_accept, NULL);
+
+	g_io_channel_unref(sco_io);
+
+	return 0;
+}
 
 static void card_append_properties(struct ofono_handsfree_card *card,
 					DBusMessageIter *dict)
@@ -430,7 +528,7 @@ void ofono_handsfree_audio_unref(void)
 
 int __ofono_handsfree_audio_manager_init(void)
 {
-	return 0;
+	return sco_init();
 }
 
 void __ofono_handsfree_audio_manager_cleanup(void)
@@ -443,4 +541,7 @@ void __ofono_handsfree_audio_manager_cleanup(void)
 
 	ref_count = 1;
 	ofono_handsfree_audio_unref();
+
+	if (sco_watch > 0)
+		g_source_remove(sco_watch);
 }
-- 
1.7.11.7

Re: [PATCH v0 02/10] handsfree-audio: Move SCO to handsfree-audio.c

[Denis Kenzior]

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

Hi Claudio,

On 02/28/2013 12:52 PM, Claudio Takahasi wrote:
>   This patch moves the SCO socket handling from hfp_hf_bluez5 plugin to
>   handsfree-audio.c file.
>
>   This is the initial step to be able to support sending the file
>   descriptor through the Agent NewConnection method.
> ---
>   plugins/hfp_hf_bluez5.c | 102 +----------------------------------------------
>   src/handsfree-audio.c   | 103 +++++++++++++++++++++++++++++++++++++++++++++++-
>   2 files changed, 103 insertions(+), 102 deletions(-)
>

<snip>

> diff --git a/src/handsfree-audio.c b/src/handsfree-audio.c
> index b2d4b97..14488ac 100644
> --- a/src/handsfree-audio.c
> +++ b/src/handsfree-audio.c
> @@ -25,12 +25,19 @@
>
>   #include<errno.h>
>   #include<stdio.h>
> +#include<stdint.h>
>   #include<string.h>
> +#include<fcntl.h>
> +#include<unistd.h>
> +#include<sys/socket.h>
>
>   #include<gdbus.h>
> +#include<gatchat.h>

Why do you need this?

>
> +#include<drivers/hfpmodem/slc.h>

Or this?

>   #include<ofono/handsfree-audio.h>
>
> +#include "bluetooth.h"
>   #include "ofono.h"
>
>   #define HFP_AUDIO_MANAGER_INTERFACE	OFONO_SERVICE ".HandsfreeAudioManager"
> @@ -60,6 +67,97 @@ struct agent {
>   static struct agent *agent = NULL;
>   static int ref_count = 0;
>   static GSList *card_list = 0;
> +static guint sco_watch = 0;
> +static uint16_t local_hfp_version = HFP_VERSION_1_6;

Why do we need this?

> +
> +static ofono_bool_t slc_match(struct ofono_modem *modem, void *userdata)
> +{
> +	const char *remote = userdata;
> +	const char *value = ofono_modem_get_string(modem, "Remote");
> +
> +	if (value == NULL)
> +		return FALSE;
> +
> +	/* Make sure SLC has been established */
> +	if (ofono_modem_get_powered(modem) != TRUE)
> +		return FALSE;
> +
> +	return g_str_equal(remote, value);

The matching should be done on cards, not modems.

> +}
> +
> +static gboolean sco_accept(GIOChannel *io, GIOCondition cond,
> +							gpointer user_data)
> +{
> +	struct sockaddr_sco saddr;
> +	socklen_t alen;
> +	int sk, nsk;
> +	char remote[18];
> +
> +	if (cond&  (G_IO_ERR | G_IO_HUP | G_IO_NVAL))
> +		return FALSE;
> +
> +	sk = g_io_channel_unix_get_fd(io);
> +
> +	memset(&saddr, 0, sizeof(saddr));
> +	alen = sizeof(saddr);
> +
> +	nsk = accept(sk, (struct sockaddr *)&saddr,&alen);
> +	if (nsk<  0)
> +		return TRUE;
> +
> +	bt_ba2str(&saddr.sco_bdaddr, remote);
> +
> +	if (ofono_modem_find(slc_match, remote) == NULL) {
> +		ofono_error("Rejecting SCO: No SLC connection found!");
> +		close(nsk);
> +		return TRUE;
> +	}
> +
> +	return TRUE;
> +}
> +
> +static int sco_init(void)
> +{
> +	GIOChannel *sco_io;
> +	struct sockaddr_sco saddr;
> +	int sk, defer_setup = 1;
> +
> +	sk = socket(PF_BLUETOOTH, SOCK_SEQPACKET | O_NONBLOCK | SOCK_CLOEXEC,
> +								BTPROTO_SCO);
> +	if (sk<  0)
> +		return -errno;
> +
> +	/* Bind to local address */
> +	memset(&saddr, 0, sizeof(saddr));
> +	saddr.sco_family = AF_BLUETOOTH;
> +	bt_bacpy(&saddr.sco_bdaddr, BDADDR_ANY);
> +
> +	if (bind(sk, (struct sockaddr *)&saddr, sizeof(saddr))<  0) {
> +		close(sk);
> +		return -errno;
> +	}
> +
> +	if (setsockopt(sk, SOL_BLUETOOTH, BT_DEFER_SETUP,
> +				&defer_setup, sizeof(defer_setup))<  0) {
> +		ofono_warn("Can't enable deferred setup: %s (%d)",
> +						strerror(errno), errno);
> +		local_hfp_version = HFP_VERSION_1_5;
> +	}
> +
> +	if (listen(sk, 5)<  0) {
> +		close(sk);
> +		return -errno;
> +	}
> +
> +	sco_io = g_io_channel_unix_new(sk);
> +	sco_watch = g_io_add_watch(sco_io,
> +				G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
> +				sco_accept, NULL);
> +
> +	g_io_channel_unref(sco_io);
> +
> +	return 0;
> +}
>
>   static void card_append_properties(struct ofono_handsfree_card *card,
>   					DBusMessageIter *dict)
> @@ -430,7 +528,7 @@ void ofono_handsfree_audio_unref(void)
>
>   int __ofono_handsfree_audio_manager_init(void)
>   {
> -	return 0;
> +	return sco_init();
>   }
>
>   void __ofono_handsfree_audio_manager_cleanup(void)
> @@ -443,4 +541,7 @@ void __ofono_handsfree_audio_manager_cleanup(void)
>
>   	ref_count = 1;
>   	ofono_handsfree_audio_unref();
> +
> +	if (sco_watch>  0)
> +		g_source_remove(sco_watch);
>   }

Regards,
-Denis

Re: [PATCH v0 02/10] handsfree-audio: Move SCO to handsfree-audio.c

[Claudio Takahasi]

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

Hi Denis

On Fri, Mar 1, 2013 at 4:20 PM, Denis Kenzior <denkenz@gmail.com> wrote:
> Hi Claudio,
>
>
> On 02/28/2013 12:52 PM, Claudio Takahasi wrote:
>>
>>   This patch moves the SCO socket handling from hfp_hf_bluez5 plugin to
>>   handsfree-audio.c file.
>>
>>   This is the initial step to be able to support sending the file
>>   descriptor through the Agent NewConnection method.
>> ---
>>   plugins/hfp_hf_bluez5.c | 102
>> +----------------------------------------------
>>   src/handsfree-audio.c   | 103
>> +++++++++++++++++++++++++++++++++++++++++++++++-
>>   2 files changed, 103 insertions(+), 102 deletions(-)
>>
>
> <snip>
>
>
>> diff --git a/src/handsfree-audio.c b/src/handsfree-audio.c
>> index b2d4b97..14488ac 100644
>> --- a/src/handsfree-audio.c
>> +++ b/src/handsfree-audio.c
>> @@ -25,12 +25,19 @@
>>
>>   #include<errno.h>
>>   #include<stdio.h>
>> +#include<stdint.h>
>>   #include<string.h>
>> +#include<fcntl.h>
>> +#include<unistd.h>
>> +#include<sys/socket.h>
>>
>>   #include<gdbus.h>
>> +#include<gatchat.h>
>
>
> Why do you need this?

drivers/hfpmodem/slc.h requires:
struct hfp_slc_info {
        GAtChat *chat;
...
}

>
>>
>> +#include<drivers/hfpmodem/slc.h>
>
>
> Or this?
enum hfp_version {
....
HFP_VERSION_1_6
....
}


>
>
>>   #include<ofono/handsfree-audio.h>
>>
>> +#include "bluetooth.h"
>>   #include "ofono.h"
>>
>>   #define HFP_AUDIO_MANAGER_INTERFACE   OFONO_SERVICE
>> ".HandsfreeAudioManager"
>> @@ -60,6 +67,97 @@ struct agent {
>>   static struct agent *agent = NULL;
>>   static int ref_count = 0;
>>   static GSList *card_list = 0;
>> +static guint sco_watch = 0;
>> +static uint16_t local_hfp_version = HFP_VERSION_1_6;
>
>
> Why do we need this?

We gonna need it later to set the socket options for SCO: CVSD or
Transparent (mSBC). I will try to post-pone it until we really need
it.

BlueZ uses the values directly instead of defines/enum, this approach
will avoid "#include<drivers/hfpmodem/slc.h>". Move the "enum
hfp_version" to a new header (eg: bluetooth.h ) without GAtChat
dependency is another alternative. Which approach do you prefer?

>
>
>> +
>> +static ofono_bool_t slc_match(struct ofono_modem *modem, void *userdata)
>> +{
>> +       const char *remote = userdata;
>> +       const char *value = ofono_modem_get_string(modem, "Remote");
>> +
>> +       if (value == NULL)
>> +               return FALSE;
>> +
>> +       /* Make sure SLC has been established */
>> +       if (ofono_modem_get_powered(modem) != TRUE)
>> +               return FALSE;
>> +
>> +       return g_str_equal(remote, value);
>
>
> The matching should be done on cards, not modems.

Yes, I know. My approach was: move the code first and fix it later
making clear which changes are required when the code was moved from
hfp_hf_bluez5.c to src/handsfree-audio.c

Regards,
Claudio

Regards,
Claudio

[PATCH v0 03/10] handsfree-audio: Remove modem dependency

[Claudio Takahasi]

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

This patch replaces the handsfree modem verification by the Audio Card.
Audio Cards are created when the RFCOMM fd descriptor is received, and
registered when the service level connetion is established.
---
 src/handsfree-audio.c | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/src/handsfree-audio.c b/src/handsfree-audio.c
index 14488ac..6657bc5 100644
--- a/src/handsfree-audio.c
+++ b/src/handsfree-audio.c
@@ -70,19 +70,12 @@ static GSList *card_list = 0;
 static guint sco_watch = 0;
 static uint16_t local_hfp_version = HFP_VERSION_1_6;
 
-static ofono_bool_t slc_match(struct ofono_modem *modem, void *userdata)
+static int card_cmp(gconstpointer a, gconstpointer b)
 {
-	const char *remote = userdata;
-	const char *value = ofono_modem_get_string(modem, "Remote");
+	const struct ofono_handsfree_card *card = a;
+	const char *remote = b;
 
-	if (value == NULL)
-		return FALSE;
-
-	/* Make sure SLC has been established */
-	if (ofono_modem_get_powered(modem) != TRUE)
-		return FALSE;
-
-	return g_str_equal(remote, value);
+	return g_strcmp0(card->remote, remote);
 }
 
 static gboolean sco_accept(GIOChannel *io, GIOCondition cond,
@@ -107,8 +100,8 @@ static gboolean sco_accept(GIOChannel *io, GIOCondition cond,
 
 	bt_ba2str(&saddr.sco_bdaddr, remote);
 
-	if (ofono_modem_find(slc_match, remote) == NULL) {
-		ofono_error("Rejecting SCO: No SLC connection found!");
+	if (g_slist_find_custom(card_list, remote, card_cmp) == NULL) {
+		ofono_error("Rejecting SCO: Audio Card not found!");
 		close(nsk);
 		return TRUE;
 	}
-- 
1.7.11.7

Re: [PATCH v0 03/10] handsfree-audio: Remove modem dependency

[Denis Kenzior]

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

Hi Claudio,

On 02/28/2013 12:52 PM, Claudio Takahasi wrote:
> This patch replaces the handsfree modem verification by the Audio Card.
> Audio Cards are created when the RFCOMM fd descriptor is received, and
> registered when the service level connetion is established.
> ---
>   src/handsfree-audio.c | 19 ++++++-------------
>   1 file changed, 6 insertions(+), 13 deletions(-)
>
> diff --git a/src/handsfree-audio.c b/src/handsfree-audio.c
> index 14488ac..6657bc5 100644
> --- a/src/handsfree-audio.c
> +++ b/src/handsfree-audio.c
> @@ -70,19 +70,12 @@ static GSList *card_list = 0;
>   static guint sco_watch = 0;
>   static uint16_t local_hfp_version = HFP_VERSION_1_6;
>
> -static ofono_bool_t slc_match(struct ofono_modem *modem, void *userdata)
> +static int card_cmp(gconstpointer a, gconstpointer b)
>   {
> -	const char *remote = userdata;
> -	const char *value = ofono_modem_get_string(modem, "Remote");
> +	const struct ofono_handsfree_card *card = a;
> +	const char *remote = b;
>
> -	if (value == NULL)
> -		return FALSE;
> -
> -	/* Make sure SLC has been established */
> -	if (ofono_modem_get_powered(modem) != TRUE)
> -		return FALSE;
> -
> -	return g_str_equal(remote, value);
> +	return g_strcmp0(card->remote, remote);
>   }
>
>   static gboolean sco_accept(GIOChannel *io, GIOCondition cond,
> @@ -107,8 +100,8 @@ static gboolean sco_accept(GIOChannel *io, GIOCondition cond,
>
>   	bt_ba2str(&saddr.sco_bdaddr, remote);
>
> -	if (ofono_modem_find(slc_match, remote) == NULL) {
> -		ofono_error("Rejecting SCO: No SLC connection found!");
> +	if (g_slist_find_custom(card_list, remote, card_cmp) == NULL) {
> +		ofono_error("Rejecting SCO: Audio Card not found!");
>   		close(nsk);
>   		return TRUE;
>   	}

Please don't do it this way, I do not want to introduce irrelevant code 
just to remove it right away.  Merge this patch into the previous one or 
cut down the first patch to just the bare necessities.

Regards,
-Denis

Re: [PATCH v0 03/10] handsfree-audio: Remove modem dependency

[Claudio Takahasi]

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

Hi Denis:

On Fri, Mar 1, 2013 at 4:29 PM, Denis Kenzior <denkenz@gmail.com> wrote:
> Hi Claudio,
>
>
> On 02/28/2013 12:52 PM, Claudio Takahasi wrote:
>>
>> This patch replaces the handsfree modem verification by the Audio Card.
>> Audio Cards are created when the RFCOMM fd descriptor is received, and
>> registered when the service level connetion is established.
>> ---
>>   src/handsfree-audio.c | 19 ++++++-------------
>>   1 file changed, 6 insertions(+), 13 deletions(-)
>>
>> diff --git a/src/handsfree-audio.c b/src/handsfree-audio.c
>> index 14488ac..6657bc5 100644
>> --- a/src/handsfree-audio.c
>> +++ b/src/handsfree-audio.c
>> @@ -70,19 +70,12 @@ static GSList *card_list = 0;
>>   static guint sco_watch = 0;
>>   static uint16_t local_hfp_version = HFP_VERSION_1_6;
>>
>> -static ofono_bool_t slc_match(struct ofono_modem *modem, void *userdata)
>> +static int card_cmp(gconstpointer a, gconstpointer b)
>>   {
>> -       const char *remote = userdata;
>> -       const char *value = ofono_modem_get_string(modem, "Remote");
>> +       const struct ofono_handsfree_card *card = a;
>> +       const char *remote = b;
>>
>> -       if (value == NULL)
>> -               return FALSE;
>> -
>> -       /* Make sure SLC has been established */
>> -       if (ofono_modem_get_powered(modem) != TRUE)
>> -               return FALSE;
>> -
>> -       return g_str_equal(remote, value);
>> +       return g_strcmp0(card->remote, remote);
>>   }
>>
>>   static gboolean sco_accept(GIOChannel *io, GIOCondition cond,
>> @@ -107,8 +100,8 @@ static gboolean sco_accept(GIOChannel *io,
>> GIOCondition cond,
>>
>>         bt_ba2str(&saddr.sco_bdaddr, remote);
>>
>> -       if (ofono_modem_find(slc_match, remote) == NULL) {
>> -               ofono_error("Rejecting SCO: No SLC connection found!");
>> +       if (g_slist_find_custom(card_list, remote, card_cmp) == NULL) {
>> +               ofono_error("Rejecting SCO: Audio Card not found!");
>>                 close(nsk);
>>                 return TRUE;
>>         }
>
>
> Please don't do it this way, I do not want to introduce irrelevant code just
> to remove it right away.  Merge this patch into the previous one or cut down
> the first patch to just the bare necessities.
>
> Regards,
> -Denis

OK. I will squash it in the previous commit.

Regards,
Claudio

[PATCH v0 04/10] handsfree-audio: Add NewConnection

[Claudio Takahasi]

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

This patch adds Agent NewConnection call. The card object path, the SCO
file descriptor, and the codec are being passed to the agent. This
initial version supports CVSD codec only.
---
 src/handsfree-audio.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/src/handsfree-audio.c b/src/handsfree-audio.c
index 6657bc5..a861c8f 100644
--- a/src/handsfree-audio.c
+++ b/src/handsfree-audio.c
@@ -78,10 +78,31 @@ static int card_cmp(gconstpointer a, gconstpointer b)
 	return g_strcmp0(card->remote, remote);
 }
 
+static void send_new_connection(const char *card, int fd)
+{
+	DBusMessage *msg;
+	DBusMessageIter iter;
+	uint8_t codec = HFP_CODEC_CVSD;
+
+	msg = dbus_message_new_method_call(agent->owner, agent->path,
+				HFP_AUDIO_AGENT_INTERFACE, "NewConnection");
+	if (msg == NULL)
+		return;
+
+	dbus_message_iter_init_append(msg, &iter);
+	dbus_message_iter_append_basic(&iter, DBUS_TYPE_OBJECT_PATH, &card);
+	dbus_message_iter_append_basic(&iter, DBUS_TYPE_UNIX_FD, &fd);
+	dbus_message_iter_append_basic(&iter, DBUS_TYPE_BYTE, &codec);
+
+	g_dbus_send_message(ofono_dbus_get_connection(), msg);
+}
+
 static gboolean sco_accept(GIOChannel *io, GIOCondition cond,
 							gpointer user_data)
 {
+	struct ofono_handsfree_card *card;
 	struct sockaddr_sco saddr;
+	GSList *list;
 	socklen_t alen;
 	int sk, nsk;
 	char remote[18];
@@ -100,12 +121,17 @@ static gboolean sco_accept(GIOChannel *io, GIOCondition cond,
 
 	bt_ba2str(&saddr.sco_bdaddr, remote);
 
-	if (g_slist_find_custom(card_list, remote, card_cmp) == NULL) {
+	list = g_slist_find_custom(card_list, remote, card_cmp);
+	if (list == NULL) {
 		ofono_error("Rejecting SCO: Audio Card not found!");
 		close(nsk);
 		return TRUE;
 	}
 
+	card = list->data;
+	send_new_connection(card->path, nsk);
+	close(nsk);
+
 	return TRUE;
 }
 
-- 
1.7.11.7

[PATCH v0 05/10] handsfree-audio: Check local SCO address

[Claudio Takahasi]

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

This patch verifies if the local Bluetooth address of the incoming
connection also matches with one of available audio cards.
---
 src/handsfree-audio.c | 44 +++++++++++++++++++++++++++++++-------------
 1 file changed, 31 insertions(+), 13 deletions(-)

diff --git a/src/handsfree-audio.c b/src/handsfree-audio.c
index a861c8f..054c454 100644
--- a/src/handsfree-audio.c
+++ b/src/handsfree-audio.c
@@ -70,14 +70,6 @@ static GSList *card_list = 0;
 static guint sco_watch = 0;
 static uint16_t local_hfp_version = HFP_VERSION_1_6;
 
-static int card_cmp(gconstpointer a, gconstpointer b)
-{
-	const struct ofono_handsfree_card *card = a;
-	const char *remote = b;
-
-	return g_strcmp0(card->remote, remote);
-}
-
 static void send_new_connection(const char *card, int fd)
 {
 	DBusMessage *msg;
@@ -97,15 +89,30 @@ static void send_new_connection(const char *card, int fd)
 	g_dbus_send_message(ofono_dbus_get_connection(), msg);
 }
 
+static struct ofono_handsfree_card *card_find(const char *remote,
+							const char *local)
+{
+	GSList *list;
+
+	for (list = card_list; list; list = g_slist_next(list)) {
+		struct ofono_handsfree_card *card = list->data;
+
+		if (g_str_equal(card->remote, remote) &&
+				g_str_equal(card->local, local))
+			return card;
+	}
+
+	return NULL;
+}
+
 static gboolean sco_accept(GIOChannel *io, GIOCondition cond,
 							gpointer user_data)
 {
 	struct ofono_handsfree_card *card;
 	struct sockaddr_sco saddr;
-	GSList *list;
 	socklen_t alen;
 	int sk, nsk;
-	char remote[18];
+	char local[18], remote[18];
 
 	if (cond & (G_IO_ERR | G_IO_HUP | G_IO_NVAL))
 		return FALSE;
@@ -121,14 +128,25 @@ static gboolean sco_accept(GIOChannel *io, GIOCondition cond,
 
 	bt_ba2str(&saddr.sco_bdaddr, remote);
 
-	list = g_slist_find_custom(card_list, remote, card_cmp);
-	if (list == NULL) {
+	memset(&saddr, 0, sizeof(saddr));
+	alen = sizeof(saddr);
+
+	if (getsockname(nsk, (struct sockaddr *) &saddr, &alen) < 0) {
+		ofono_error("SCO getsockname(): %s (%d)",
+						strerror(errno), errno);
+		close(nsk);
+		return TRUE;
+	}
+
+	bt_ba2str(&saddr.sco_bdaddr, local);
+
+	card = card_find(remote, local);
+	if (card == NULL) {
 		ofono_error("Rejecting SCO: Audio Card not found!");
 		close(nsk);
 		return TRUE;
 	}
 
-	card = list->data;
 	send_new_connection(card->path, nsk);
 	close(nsk);
 
-- 
1.7.11.7

[PATCH v0 06/10] handsfree-audio: Reject SCO if Card is not ready

[Claudio Takahasi]

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

The Audio Card is being created when the NewConnection from BlueZ
Profile is received, and registered when the service level connection
negotiation finishes. This patch rejects SCO connection if the SCO
incoming connection arrives when the service level negotiation is
ongoing.
---
 src/handsfree-audio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/handsfree-audio.c b/src/handsfree-audio.c
index 054c454..50aafc1 100644
--- a/src/handsfree-audio.c
+++ b/src/handsfree-audio.c
@@ -141,7 +141,7 @@ static gboolean sco_accept(GIOChannel *io, GIOCondition cond,
 	bt_ba2str(&saddr.sco_bdaddr, local);
 
 	card = card_find(remote, local);
-	if (card == NULL) {
+	if (card == NULL || card->path == NULL) {
 		ofono_error("Rejecting SCO: Audio Card not found!");
 		close(nsk);
 		return TRUE;
-- 
1.7.11.7

[PATCH v0 07/10] handsfree-audio: Reject SCO if agent is unavailable

[Claudio Takahasi]

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

This patch rejects the incoming SCO connection if there isn't a
Handsfree Audio Agent registered.
---
 src/handsfree-audio.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/handsfree-audio.c b/src/handsfree-audio.c
index 50aafc1..50bd861 100644
--- a/src/handsfree-audio.c
+++ b/src/handsfree-audio.c
@@ -126,6 +126,12 @@ static gboolean sco_accept(GIOChannel *io, GIOCondition cond,
 	if (nsk < 0)
 		return TRUE;
 
+	if (agent == NULL) {
+		ofono_error("Reject SCO: Agent not registered");
+		close(nsk);
+		return TRUE;
+	}
+
 	bt_ba2str(&saddr.sco_bdaddr, remote);
 
 	memset(&saddr, 0, sizeof(saddr));
-- 
1.7.11.7

[PATCH v0 08/10] handsfree-audio: Check CVSD when registering agent

[Claudio Takahasi]

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

This patch makes CVSD codec mandatory when registering a Handsfree
Audio Agent.
---
 src/handsfree-audio.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/handsfree-audio.c b/src/handsfree-audio.c
index 50bd861..d0ccf29 100644
--- a/src/handsfree-audio.c
+++ b/src/handsfree-audio.c
@@ -448,6 +448,7 @@ static DBusMessage *am_agent_register(DBusConnection *conn,
 	unsigned char *codecs;
 	DBusMessageIter iter, array;
 	int length, i;
+	gboolean has_cvsd = FALSE;
 
 	if (agent)
 		return __ofono_error_in_use(msg);
@@ -467,11 +468,17 @@ static DBusMessage *am_agent_register(DBusConnection *conn,
 		return __ofono_error_invalid_args(msg);
 
 	for (i = 0; i < length; i++) {
-		if (codecs[i] != HFP_CODEC_CVSD &&
-				codecs[i] != HFP_CODEC_MSBC)
+		if (codecs[i] == HFP_CODEC_CVSD)
+			has_cvsd = TRUE;
+		else if (codecs[i] != HFP_CODEC_MSBC)
 			return __ofono_error_invalid_args(msg);
 	}
 
+	if (has_cvsd == FALSE) {
+		ofono_error("CVSD codec is mandatory");
+		return __ofono_error_invalid_args(msg);
+	}
+
 	agent = g_new0(struct agent, 1);
 	agent->owner = g_strdup(sender);
 	agent->path = g_strdup(path);
-- 
1.7.11.7

Re: [PATCH v0 08/10] handsfree-audio: Check CVSD when registering agent

[Denis Kenzior]

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

Hi Claudio,

On 02/28/2013 12:52 PM, Claudio Takahasi wrote:
> This patch makes CVSD codec mandatory when registering a Handsfree
> Audio Agent.
> ---
>   src/handsfree-audio.c | 11 +++++++++--
>   1 file changed, 9 insertions(+), 2 deletions(-)
>

Patch has been applied, thanks.

Regards,
-Denis

[PATCH v0 09/10] handsfree-audio: Add function to get hfp version

[Claudio Takahasi]

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

This patch adds a new function to read the local supported Handsfree
Profile version.
---
 include/handsfree-audio.h | 1 +
 src/handsfree-audio.c     | 5 +++++
 2 files changed, 6 insertions(+)

diff --git a/include/handsfree-audio.h b/include/handsfree-audio.h
index c5403c7..96597c2 100644
--- a/include/handsfree-audio.h
+++ b/include/handsfree-audio.h
@@ -34,6 +34,7 @@ struct ofono_handsfree_card *ofono_handsfree_card_create(const char *remote,
 							const char *local);
 int ofono_handsfree_card_register(struct ofono_handsfree_card *card);
 void ofono_handsfree_card_remove(struct ofono_handsfree_card *card);
+enum hfp_version ofono_handsfree_get_version();
 
 void ofono_handsfree_audio_ref(void);
 void ofono_handsfree_audio_unref(void);
diff --git a/src/handsfree-audio.c b/src/handsfree-audio.c
index d0ccf29..955c6ef 100644
--- a/src/handsfree-audio.c
+++ b/src/handsfree-audio.c
@@ -363,6 +363,11 @@ void ofono_handsfree_card_remove(struct ofono_handsfree_card *card)
 	g_free(card);
 }
 
+enum hfp_version ofono_handsfree_get_version()
+{
+	return local_hfp_version;
+}
+
 static void agent_free(struct agent *agent)
 {
 	if (agent->watch > 0)
-- 
1.7.11.7

Re: [PATCH v0 09/10] handsfree-audio: Add function to get hfp version

[Denis Kenzior]

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

Hi Claudio,

On 02/28/2013 12:52 PM, Claudio Takahasi wrote:
> This patch adds a new function to read the local supported Handsfree
> Profile version.
> ---
>   include/handsfree-audio.h | 1 +
>   src/handsfree-audio.c     | 5 +++++
>   2 files changed, 6 insertions(+)
>
> diff --git a/include/handsfree-audio.h b/include/handsfree-audio.h
> index c5403c7..96597c2 100644
> --- a/include/handsfree-audio.h
> +++ b/include/handsfree-audio.h
> @@ -34,6 +34,7 @@ struct ofono_handsfree_card *ofono_handsfree_card_create(const char *remote,
>   							const char *local);
>   int ofono_handsfree_card_register(struct ofono_handsfree_card *card);
>   void ofono_handsfree_card_remove(struct ofono_handsfree_card *card);
> +enum hfp_version ofono_handsfree_get_version();
>

I'd like to avoid such details in the card functionality.

Are our possible scenarios the following?

1. SCO defer in kernel, CVSD mSBC supported
2. SCO defer not in kernel, CVSD mSBC supported
3. SCO defer in kernel, CVSD supported
4. SCO defer not in kernel, CVSD supported

If so, can we always advertise HFP 1.6, but only declare support for 
CVSD in case 2?

>   void ofono_handsfree_audio_ref(void);
>   void ofono_handsfree_audio_unref(void);
> diff --git a/src/handsfree-audio.c b/src/handsfree-audio.c
> index d0ccf29..955c6ef 100644
> --- a/src/handsfree-audio.c
> +++ b/src/handsfree-audio.c
> @@ -363,6 +363,11 @@ void ofono_handsfree_card_remove(struct ofono_handsfree_card *card)
>   	g_free(card);
>   }
>
> +enum hfp_version ofono_handsfree_get_version()
> +{
> +	return local_hfp_version;
> +}
> +
>   static void agent_free(struct agent *agent)
>   {
>   	if (agent->watch>  0)

Regards,
-Denis

Re: [PATCH v0 09/10] handsfree-audio: Add function to get hfp version

[Claudio Takahasi]

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

Hi Denis:

On Fri, Mar 1, 2013 at 4:40 PM, Denis Kenzior <denkenz@gmail.com> wrote:
> Hi Claudio,
>
>
> On 02/28/2013 12:52 PM, Claudio Takahasi wrote:
>>
>> This patch adds a new function to read the local supported Handsfree
>> Profile version.
>> ---
>>   include/handsfree-audio.h | 1 +
>>   src/handsfree-audio.c     | 5 +++++
>>   2 files changed, 6 insertions(+)
>>
>> diff --git a/include/handsfree-audio.h b/include/handsfree-audio.h
>> index c5403c7..96597c2 100644
>> --- a/include/handsfree-audio.h
>> +++ b/include/handsfree-audio.h
>> @@ -34,6 +34,7 @@ struct ofono_handsfree_card
>> *ofono_handsfree_card_create(const char *remote,
>>                                                         const char
>> *local);
>>   int ofono_handsfree_card_register(struct ofono_handsfree_card *card);
>>   void ofono_handsfree_card_remove(struct ofono_handsfree_card *card);
>> +enum hfp_version ofono_handsfree_get_version();
>>
>
> I'd like to avoid such details in the card functionality.
>
> Are our possible scenarios the following?
>
> 1. SCO defer in kernel, CVSD mSBC supported

yes.

> 2. SCO defer not in kernel, CVSD mSBC supported

not possible. Wide band speech requires defer setup to set the SCO
parameters properly.

> 3. SCO defer in kernel, CVSD supported

yes.

> 4. SCO defer not in kernel, CVSD supported

yes.

>
> If so, can we always advertise HFP 1.6, but only declare support for CVSD in
> case 2?

I am not a spec (or qualification) expert, but  I will try to expose
my understanding of the docs:

For case 2, my understanding is: if HF supports *only* CVSD
(mandatory), it doesn't make sense to inform that codec negotiation is
supported. HFP 1.5 (or older) shall not indicate that it supports
codec negotiation (during service level connection negotiation) and it
should not set the wide band speech support in the service record.

One thing that I can't answer is if it is allowed to inform in the
service record that the HF supports HFP 1.6 (version), but don't set
the wide band speech feature in record.

Regards,
Claudio

Re: [PATCH v0 09/10] handsfree-audio: Add function to get hfp version

[Denis Kenzior]

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

Hi Claudio,

On 03/04/2013 10:02 AM, Claudio Takahasi wrote:
> Hi Denis:
>
> On Fri, Mar 1, 2013 at 4:40 PM, Denis Kenzior<denkenz@gmail.com>  wrote:
>> Hi Claudio,
>>
>>
>> On 02/28/2013 12:52 PM, Claudio Takahasi wrote:
>>>
>>> This patch adds a new function to read the local supported Handsfree
>>> Profile version.
>>> ---
>>>    include/handsfree-audio.h | 1 +
>>>    src/handsfree-audio.c     | 5 +++++
>>>    2 files changed, 6 insertions(+)
>>>
>>> diff --git a/include/handsfree-audio.h b/include/handsfree-audio.h
>>> index c5403c7..96597c2 100644
>>> --- a/include/handsfree-audio.h
>>> +++ b/include/handsfree-audio.h
>>> @@ -34,6 +34,7 @@ struct ofono_handsfree_card
>>> *ofono_handsfree_card_create(const char *remote,
>>>                                                          const char
>>> *local);
>>>    int ofono_handsfree_card_register(struct ofono_handsfree_card *card);
>>>    void ofono_handsfree_card_remove(struct ofono_handsfree_card *card);
>>> +enum hfp_version ofono_handsfree_get_version();
>>>
>>
>> I'd like to avoid such details in the card functionality.
>>
>> Are our possible scenarios the following?
>>
>> 1. SCO defer in kernel, CVSD mSBC supported
>
> yes.
>
>> 2. SCO defer not in kernel, CVSD mSBC supported
>
> not possible. Wide band speech requires defer setup to set the SCO
> parameters properly.
>
>> 3. SCO defer in kernel, CVSD supported
>
> yes.
>
>> 4. SCO defer not in kernel, CVSD supported
>
> yes.
>
>>
>> If so, can we always advertise HFP 1.6, but only declare support for CVSD in
>> case 2?
>
> I am not a spec (or qualification) expert, but  I will try to expose
> my understanding of the docs:
>
> For case 2, my understanding is: if HF supports *only* CVSD
> (mandatory), it doesn't make sense to inform that codec negotiation is
> supported. HFP 1.5 (or older) shall not indicate that it supports
> codec negotiation (during service level connection negotiation) and it
> should not set the wide band speech support in the service record.
>

There are other features of 1.6 though that might be useful.  For 
example, BTRH status is properly shown in CLCC, and a few other 
niceties.  I see no reason to fall back to 1.5 just because we do not 
support wide-band speech.

Either way, the version decision is not up to the card, it is up to the 
plugin.  The function to get the version information needs to be dropped.

> One thing that I can't answer is if it is allowed to inform in the
> service record that the HF supports HFP 1.6 (version), but don't set
> the wide band speech feature in record.
>

I don't see why not.  Both Codec Negotiation and Wide-band speech are 
labeled optional for HF/AG in Table 3.1.  The only note there says: "If 
Wide Band Speech is supported, Codec Negotiation shall also be 
supported."  However, there is no note vice-versa.  So my interpretation 
is that Codec Negotiation can always be supported.  Can we check the 
test spec?

The problem I see is that by the time we register the SDP record, we 
still do not know for sure whether the audio framework truly supports mSBC.

Regards,
-Denis

Re: [PATCH v0 09/10] handsfree-audio: Add function to get hfp version

[Marcel Holtmann]

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

Hi Denis,

>>>> This patch adds a new function to read the local supported Handsfree
>>>> Profile version.
>>>> ---
>>>>   include/handsfree-audio.h | 1 +
>>>>   src/handsfree-audio.c     | 5 +++++
>>>>   2 files changed, 6 insertions(+)
>>>> 
>>>> diff --git a/include/handsfree-audio.h b/include/handsfree-audio.h
>>>> index c5403c7..96597c2 100644
>>>> --- a/include/handsfree-audio.h
>>>> +++ b/include/handsfree-audio.h
>>>> @@ -34,6 +34,7 @@ struct ofono_handsfree_card
>>>> *ofono_handsfree_card_create(const char *remote,
>>>>                                                         const char
>>>> *local);
>>>>   int ofono_handsfree_card_register(struct ofono_handsfree_card *card);
>>>>   void ofono_handsfree_card_remove(struct ofono_handsfree_card *card);
>>>> +enum hfp_version ofono_handsfree_get_version();
>>>> 
>>> 
>>> I'd like to avoid such details in the card functionality.
>>> 
>>> Are our possible scenarios the following?
>>> 
>>> 1. SCO defer in kernel, CVSD mSBC supported
>> 
>> yes.
>> 
>>> 2. SCO defer not in kernel, CVSD mSBC supported
>> 
>> not possible. Wide band speech requires defer setup to set the SCO
>> parameters properly.
>> 
>>> 3. SCO defer in kernel, CVSD supported
>> 
>> yes.
>> 
>>> 4. SCO defer not in kernel, CVSD supported
>> 
>> yes.
>> 
>>> 
>>> If so, can we always advertise HFP 1.6, but only declare support for CVSD in
>>> case 2?
>> 
>> I am not a spec (or qualification) expert, but  I will try to expose
>> my understanding of the docs:
>> 
>> For case 2, my understanding is: if HF supports *only* CVSD
>> (mandatory), it doesn't make sense to inform that codec negotiation is
>> supported. HFP 1.5 (or older) shall not indicate that it supports
>> codec negotiation (during service level connection negotiation) and it
>> should not set the wide band speech support in the service record.
>> 
> 
> There are other features of 1.6 though that might be useful.  For example, BTRH status is properly shown in CLCC, and a few other niceties.  I see no reason to fall back to 1.5 just because we do not support wide-band speech.
> 
> Either way, the version decision is not up to the card, it is up to the plugin.  The function to get the version information needs to be dropped.
> 
>> One thing that I can't answer is if it is allowed to inform in the
>> service record that the HF supports HFP 1.6 (version), but don't set
>> the wide band speech feature in record.
>> 
> 
> I don't see why not.  Both Codec Negotiation and Wide-band speech are labeled optional for HF/AG in Table 3.1.  The only note there says: "If Wide Band Speech is supported, Codec Negotiation shall also be supported."  However, there is no note vice-versa.  So my interpretation is that Codec Negotiation can always be supported.  Can we check the test spec?
> 
> The problem I see is that by the time we register the SDP record, we still do not know for sure whether the audio framework truly supports mSBC.

so we want to expose HFP 1.6 and codec negogiation all the time. Even if mSBC codec is not available, because PA does not support it or is not running. I read the spec the same way that this fully spec compliant.

If at some point in the future this changes, we can deal with that at that point. No need to over engineer it at this point.

Regards

Marcel

Re: [PATCH v0 09/10] handsfree-audio: Add function to get hfp version

[Denis Kenzior]

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

Hi Marcel,

>>
>> I don't see why not.  Both Codec Negotiation and Wide-band speech are labeled optional for HF/AG in Table 3.1.  The only note there says: "If Wide Band Speech is supported, Codec Negotiation shall also be supported."  However, there is no note vice-versa.  So my interpretation is that Codec Negotiation can always be supported.  Can we check the test spec?
>>
>> The problem I see is that by the time we register the SDP record, we still do not know for sure whether the audio framework truly supports mSBC.
>
> so we want to expose HFP 1.6 and codec negogiation all the time. Even if mSBC codec is not available, because PA does not support it or is not running. I read the spec the same way that this fully spec compliant.
>
> If at some point in the future this changes, we can deal with that at that point. No need to over engineer it at this point.
>

Exactly,  Codec negotiation should always be supported.
.  What my last comment was about is that:

If mSBC is not supported by PA, then it would be nice to not set the 
wide-band speech bit in the SDP record to true.  However, we do not know 
this information in the current set up until too late.

Regards,
-Denis

Re: [PATCH v0 09/10] handsfree-audio: Add function to get hfp version

[Claudio Takahasi]

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

Hi Denis:

On Mon, Mar 4, 2013 at 4:30 PM, Denis Kenzior <denkenz@gmail.com> wrote:
> Hi Marcel,
>
>
>>>
>>> I don't see why not.  Both Codec Negotiation and Wide-band speech are
>>> labeled optional for HF/AG in Table 3.1.  The only note there says: "If Wide
>>> Band Speech is supported, Codec Negotiation shall also be supported."
>>> However, there is no note vice-versa.  So my interpretation is that Codec
>>> Negotiation can always be supported.  Can we check the test spec?
>>>
>>> The problem I see is that by the time we register the SDP record, we
>>> still do not know for sure whether the audio framework truly supports mSBC.
>>
>>
>> so we want to expose HFP 1.6 and codec negogiation all the time. Even if
>> mSBC codec is not available, because PA does not support it or is not
>> running. I read the spec the same way that this fully spec compliant.
>>
>> If at some point in the future this changes, we can deal with that at that
>> point. No need to over engineer it at this point.
>>
>
> Exactly,  Codec negotiation should always be supported.
> .  What my last comment was about is that:
>
> If mSBC is not supported by PA, then it would be nice to not set the
> wide-band speech bit in the SDP record to true.  However, we do not know
> this information in the current set up until too late.

F.Y.I. some results from the test plan generator consistency check:
* if wide band speech is Not supported Codec negotiation is excluded
* if wide band speech is Supported, mSBC is mandatory

Anyway, I will implementing what you are suggesting. We can fix it
later if necessary.

Regards,
Claudio

[PATCH v0 10/10] hfp_hf_bluez5: Fix hard-coded hfp version

[Claudio Takahasi]

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

This patch removes the hard-coded Handsfree Profile version to register
the external BlueZ profile.
---
 plugins/hfp_hf_bluez5.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
index ea2c848..c1f3ba2 100644
--- a/plugins/hfp_hf_bluez5.c
+++ b/plugins/hfp_hf_bluez5.c
@@ -484,8 +484,8 @@ static void connect_handler(DBusConnection *conn, void *user_data)
 {
 	DBG("Registering External Profile handler ...");
 
-	bt_register_profile(conn, HFP_HS_UUID, HFP_VERSION_1_5, "hfp_hf",
-						HFP_EXT_PROFILE_PATH);
+	bt_register_profile(conn, HFP_HS_UUID, ofono_handsfree_get_version(),
+					"hfp_hf", HFP_EXT_PROFILE_PATH);
 }
 
 static gboolean has_hfp_ag_uuid(DBusMessageIter *array)
-- 
1.7.11.7

[PATCH v1 0/6] handsfree-audio: Add Agent NewConnection()

[Claudio Takahasi]

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

This patch series moves the SCO connection handling from hfp_hf_bluez5.c
to src/handsfree-audio.c, and adds the Agent NewConnection() method call
to pass the SCO file descriptor from oFono to the Agent implementation.

Changes between v0-v1:
* Merged commits "Move SCO to handsfree-audio.c" and "Remove modem
dependency"
* Removed tracking of supported local HFP version in handsfree-audio
* Removed ofono_handsfree_get_version()

Claudio Takahasi (6):
  bluetooth: Add new Bluetooth header
  handsfree-audio: Move SCO to handsfree-audio.c
  handsfree-audio: Add NewConnection
  handsfree-audio: Check local SCO address
  handsfree-audio: Reject SCO if Card is not ready
  handsfree-audio: Reject SCO if agent is unavailable

 Makefile.am             |   2 +-
 plugins/bluez5.c        |  17 ------
 plugins/bluez5.h        |  41 --------------
 plugins/hfp_hf_bluez5.c | 103 +----------------------------------
 src/bluetooth.h         |  71 ++++++++++++++++++++++++
 src/handsfree-audio.c   | 141 +++++++++++++++++++++++++++++++++++++++++++++++-
 6 files changed, 214 insertions(+), 161 deletions(-)
 create mode 100644 src/bluetooth.h

-- 
1.7.11.7

[PATCH v1 1/6] bluetooth: Add new Bluetooth header

[Claudio Takahasi]

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

This patch moves the Bluetooth utility funtions and socket type
declarations to a new header src/bluetooth.h, allowing to share it
between core, and plugins.
---
 Makefile.am             |  2 +-
 plugins/bluez5.c        | 17 ------------
 plugins/bluez5.h        | 41 ----------------------------
 plugins/hfp_hf_bluez5.c |  1 +
 src/bluetooth.h         | 71 +++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 73 insertions(+), 59 deletions(-)
 create mode 100644 src/bluetooth.h

diff --git a/Makefile.am b/Makefile.am
index 557f499..3b998af 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -509,7 +509,7 @@ src_ofonod_SOURCES = $(gdbus_sources) $(builtin_sources) src/ofono.ver \
 			src/cdma-smsutil.h src/cdma-smsutil.c \
 			src/cdma-sms.c src/private-network.c src/cdma-netreg.c \
 			src/cdma-provision.c src/handsfree.c \
-			src/handsfree-audio.c
+			src/handsfree-audio.c src/bluetooth.h
 
 src_ofonod_LDADD = $(builtin_libadd) @GLIB_LIBS@ @DBUS_LIBS@ -ldl
 
diff --git a/plugins/bluez5.c b/plugins/bluez5.c
index 7cc11fd..0f997da 100644
--- a/plugins/bluez5.c
+++ b/plugins/bluez5.c
@@ -26,7 +26,6 @@
 #include <errno.h>
 #include <stdint.h>
 #include <stdio.h>
-#include <sys/socket.h>
 #include <string.h>
 
 #include <glib.h>
@@ -47,22 +46,6 @@ struct finish_callback {
 	char *member;
 };
 
-void bt_bacpy(bdaddr_t *dst, const bdaddr_t *src)
-{
-	memcpy(dst, src, sizeof(bdaddr_t));
-}
-
-int bt_ba2str(const bdaddr_t *ba, char *str)
-{
-	return sprintf(str, "%2.2X:%2.2X:%2.2X:%2.2X:%2.2X:%2.2X",
-		ba->b[5], ba->b[4], ba->b[3], ba->b[2], ba->b[1], ba->b[0]);
-}
-
-int bt_bacmp(const bdaddr_t *ba1, const bdaddr_t *ba2)
-{
-	return memcmp(ba1, ba2, sizeof(bdaddr_t));
-}
-
 static void profile_register_cb(DBusPendingCall *call, gpointer user_data)
 {
 	DBusMessage *reply;
diff --git a/plugins/bluez5.h b/plugins/bluez5.h
index f7f3d7e..573a54c 100644
--- a/plugins/bluez5.h
+++ b/plugins/bluez5.h
@@ -28,47 +28,6 @@
 #define HFP_HS_UUID	"0000111e-0000-1000-8000-00805f9b34fb"
 #define HFP_AG_UUID	"0000111f-0000-1000-8000-00805f9b34fb"
 
-#ifndef AF_BLUETOOTH
-#define AF_BLUETOOTH		31
-#define PF_BLUETOOTH		AF_BLUETOOTH
-#endif
-
-#define BTPROTO_SCO		2
-
-#define SOL_SCO			17
-
-#ifndef SOL_BLUETOOTH
-#define SOL_BLUETOOTH		274
-#endif
-
-#define BT_DEFER_SETUP		7
-
-/* BD Address */
-typedef struct {
-	uint8_t b[6];
-} __attribute__((packed)) bdaddr_t;
-
-#define BDADDR_ANY   (&(bdaddr_t) {{0, 0, 0, 0, 0, 0}})
-
-/* RFCOMM socket address */
-struct sockaddr_rc {
-	sa_family_t	rc_family;
-	bdaddr_t	rc_bdaddr;
-	uint8_t		rc_channel;
-};
-
-/* SCO socket address */
-struct sockaddr_sco {
-	sa_family_t	sco_family;
-	bdaddr_t	sco_bdaddr;
-};
-
-void bt_bacpy(bdaddr_t *dst, const bdaddr_t *src);
-
-int bt_ba2str(const bdaddr_t *ba, char *str);
-
-int bt_bacmp(const bdaddr_t *ba1, const bdaddr_t *ba2);
-
 int bt_register_profile_with_role(DBusConnection *conn, const char *uuid,
 					uint16_t version, const char *name,
 					const char *object, const char *role);
diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
index e84af73..3a48a51 100644
--- a/plugins/hfp_hf_bluez5.c
+++ b/plugins/hfp_hf_bluez5.c
@@ -52,6 +52,7 @@
 
 #include <drivers/hfpmodem/slc.h>
 
+#include "bluetooth.h"
 #include "bluez5.h"
 
 #ifndef DBUS_TYPE_UNIX_FD
diff --git a/src/bluetooth.h b/src/bluetooth.h
new file mode 100644
index 0000000..1584109
--- /dev/null
+++ b/src/bluetooth.h
@@ -0,0 +1,71 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2013  Intel Corporation. All rights reserved.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifndef AF_BLUETOOTH
+#define AF_BLUETOOTH		31
+#define PF_BLUETOOTH		AF_BLUETOOTH
+#endif
+
+#define BTPROTO_SCO		2
+
+#define SOL_SCO			17
+
+#ifndef SOL_BLUETOOTH
+#define SOL_BLUETOOTH		274
+#endif
+
+#define BT_DEFER_SETUP		7
+
+/* BD Address */
+typedef struct {
+	uint8_t b[6];
+} __attribute__((packed)) bdaddr_t;
+
+#define BDADDR_ANY   (&(bdaddr_t) {{0, 0, 0, 0, 0, 0}})
+
+/* RFCOMM socket address */
+struct sockaddr_rc {
+	sa_family_t	rc_family;
+	bdaddr_t	rc_bdaddr;
+	uint8_t		rc_channel;
+};
+
+/* SCO socket address */
+struct sockaddr_sco {
+	sa_family_t	sco_family;
+	bdaddr_t	sco_bdaddr;
+};
+
+static inline void bt_bacpy(bdaddr_t *dst, const bdaddr_t *src)
+{
+	memcpy(dst, src, sizeof(bdaddr_t));
+}
+
+static inline int bt_ba2str(const bdaddr_t *ba, char *str)
+{
+	return sprintf(str, "%2.2X:%2.2X:%2.2X:%2.2X:%2.2X:%2.2X",
+		ba->b[5], ba->b[4], ba->b[3], ba->b[2], ba->b[1], ba->b[0]);
+}
+
+static inline int bt_bacmp(const bdaddr_t *ba1, const bdaddr_t *ba2)
+{
+	return memcmp(ba1, ba2, sizeof(bdaddr_t));
+}
-- 
1.7.11.7

[PATCH v1 2/6] handsfree-audio: Move SCO to handsfree-audio.c

[Claudio Takahasi]

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

This patch moves the SCO socket handling from hfp_hf_bluez5 plugin to
handsfree-audio.c file. This is the initial step to be able to support
sending the file descriptor through the Agent NewConnection method.

This patch replaces the handsfree modem verification by the Audio Card.
Audio Cards are created when the RFCOMM fd descriptor is received, and
registered when the service level connetion is established.
---
 plugins/hfp_hf_bluez5.c | 102 +-----------------------------------------------
 src/handsfree-audio.c   |  91 +++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 91 insertions(+), 102 deletions(-)

diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
index 3a48a51..381a541 100644
--- a/plugins/hfp_hf_bluez5.c
+++ b/plugins/hfp_hf_bluez5.c
@@ -68,8 +68,6 @@ struct hfp {
 };
 
 static GDBusClient *bluez = NULL;
-static guint sco_watch = 0;
-static uint16_t local_hfp_version = HFP_VERSION_1_6;
 
 static void hfp_debug(const char *str, void *user_data)
 {
@@ -483,100 +481,11 @@ static const GDBusMethodTable profile_methods[] = {
 	{ }
 };
 
-static ofono_bool_t slc_match(struct ofono_modem *modem, void *userdata)
-{
-	const char *remote = userdata;
-	const char *value = ofono_modem_get_string(modem, "Remote");
-
-	if (value == NULL)
-		return FALSE;
-
-	/* Make sure SLC has been established */
-	if (ofono_modem_get_powered(modem) != TRUE)
-		return FALSE;
-
-	return g_str_equal(remote, value);
-}
-
-static gboolean sco_accept(GIOChannel *io, GIOCondition cond,
-							gpointer user_data)
-{
-	struct sockaddr_sco saddr;
-	socklen_t alen;
-	int sk, nsk;
-	char remote[18];
-
-	if (cond & (G_IO_ERR | G_IO_HUP | G_IO_NVAL))
-		return FALSE;
-
-	sk = g_io_channel_unix_get_fd(io);
-
-	memset(&saddr, 0, sizeof(saddr));
-	alen = sizeof(saddr);
-
-	nsk = accept(sk, (struct sockaddr *) &saddr, &alen);
-	if (nsk < 0)
-		return TRUE;
-
-	bt_ba2str(&saddr.sco_bdaddr, remote);
-
-	if (ofono_modem_find(slc_match, remote) == NULL) {
-		ofono_error("Rejecting SCO: No SLC connection found!");
-		close(nsk);
-		return TRUE;
-	}
-
-	return TRUE;
-}
-
-static int sco_init(void)
-{
-	GIOChannel *sco_io;
-	struct sockaddr_sco saddr;
-	int sk, defer_setup = 1;
-
-	sk = socket(PF_BLUETOOTH, SOCK_SEQPACKET | O_NONBLOCK | SOCK_CLOEXEC,
-								BTPROTO_SCO);
-	if (sk < 0)
-		return -errno;
-
-	/* Bind to local address */
-	memset(&saddr, 0, sizeof(saddr));
-	saddr.sco_family = AF_BLUETOOTH;
-	bt_bacpy(&saddr.sco_bdaddr, BDADDR_ANY);
-
-	if (bind(sk, (struct sockaddr *) &saddr, sizeof(saddr)) < 0) {
-		close(sk);
-		return -errno;
-	}
-
-	if (setsockopt(sk, SOL_BLUETOOTH, BT_DEFER_SETUP,
-				&defer_setup, sizeof(defer_setup)) < 0) {
-		ofono_warn("Can't enable deferred setup: %s (%d)",
-						strerror(errno), errno);
-		local_hfp_version = HFP_VERSION_1_5;
-	}
-
-	if (listen(sk, 5) < 0) {
-		close(sk);
-		return -errno;
-	}
-
-	sco_io = g_io_channel_unix_new(sk);
-	sco_watch = g_io_add_watch(sco_io,
-				G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
-				sco_accept, NULL);
-
-	g_io_channel_unref(sco_io);
-
-	return 0;
-}
-
 static void connect_handler(DBusConnection *conn, void *user_data)
 {
 	DBG("Registering External Profile handler ...");
 
-	bt_register_profile(conn, HFP_HS_UUID, local_hfp_version, "hfp_hf",
+	bt_register_profile(conn, HFP_HS_UUID, HFP_VERSION_1_6, "hfp_hf",
 						HFP_EXT_PROFILE_PATH);
 }
 
@@ -696,12 +605,6 @@ static int hfp_init(void)
 	if (DBUS_TYPE_UNIX_FD < 0)
 		return -EBADF;
 
-	err = sco_init();
-	if (err < 0) {
-		ofono_error("SCO: %s(%d)", strerror(-err), -err);
-		return err;
-	}
-
 	/* Registers External Profile handler */
 	if (!g_dbus_register_interface(conn, HFP_EXT_PROFILE_PATH,
 					BLUEZ_PROFILE_INTERFACE,
@@ -746,9 +649,6 @@ static void hfp_exit(void)
 	ofono_modem_driver_unregister(&hfp_driver);
 	g_dbus_client_unref(bluez);
 
-	if (sco_watch > 0)
-		g_source_remove(sco_watch);
-
 	ofono_handsfree_audio_unref();
 }
 
diff --git a/src/handsfree-audio.c b/src/handsfree-audio.c
index bfce42a..24bb2ad 100644
--- a/src/handsfree-audio.c
+++ b/src/handsfree-audio.c
@@ -25,12 +25,17 @@
 
 #include <errno.h>
 #include <stdio.h>
+#include <stdint.h>
 #include <string.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <sys/socket.h>
 
 #include <gdbus.h>
 
 #include <ofono/handsfree-audio.h>
 
+#include "bluetooth.h"
 #include "ofono.h"
 
 #define HFP_AUDIO_MANAGER_INTERFACE	OFONO_SERVICE ".HandsfreeAudioManager"
@@ -60,6 +65,87 @@ struct agent {
 static struct agent *agent = NULL;
 static int ref_count = 0;
 static GSList *card_list = 0;
+static guint sco_watch = 0;
+
+static int card_cmp(gconstpointer a, gconstpointer b)
+{
+	const struct ofono_handsfree_card *card = a;
+	const char *remote = b;
+
+	return g_strcmp0(card->remote, remote);
+}
+
+static gboolean sco_accept(GIOChannel *io, GIOCondition cond,
+							gpointer user_data)
+{
+	struct sockaddr_sco saddr;
+	socklen_t alen;
+	int sk, nsk;
+	char remote[18];
+
+	if (cond & (G_IO_ERR | G_IO_HUP | G_IO_NVAL))
+		return FALSE;
+
+	sk = g_io_channel_unix_get_fd(io);
+
+	memset(&saddr, 0, sizeof(saddr));
+	alen = sizeof(saddr);
+
+	nsk = accept(sk, (struct sockaddr *) &saddr, &alen);
+	if (nsk < 0)
+		return TRUE;
+
+	bt_ba2str(&saddr.sco_bdaddr, remote);
+
+	if (g_slist_find_custom(card_list, remote, card_cmp) == NULL) {
+		ofono_error("Rejecting SCO: Audio Card not found!");
+		close(nsk);
+		return TRUE;
+	}
+
+	return TRUE;
+}
+
+static int sco_init(void)
+{
+	GIOChannel *sco_io;
+	struct sockaddr_sco saddr;
+	int sk, defer_setup = 1;
+
+	sk = socket(PF_BLUETOOTH, SOCK_SEQPACKET | O_NONBLOCK | SOCK_CLOEXEC,
+								BTPROTO_SCO);
+	if (sk < 0)
+		return -errno;
+
+	/* Bind to local address */
+	memset(&saddr, 0, sizeof(saddr));
+	saddr.sco_family = AF_BLUETOOTH;
+	bt_bacpy(&saddr.sco_bdaddr, BDADDR_ANY);
+
+	if (bind(sk, (struct sockaddr *) &saddr, sizeof(saddr)) < 0) {
+		close(sk);
+		return -errno;
+	}
+
+	if (setsockopt(sk, SOL_BLUETOOTH, BT_DEFER_SETUP,
+				&defer_setup, sizeof(defer_setup)) < 0)
+		ofono_warn("Can't enable deferred setup: %s (%d)",
+						strerror(errno), errno);
+
+	if (listen(sk, 5) < 0) {
+		close(sk);
+		return -errno;
+	}
+
+	sco_io = g_io_channel_unix_new(sk);
+	sco_watch = g_io_add_watch(sco_io,
+				G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
+				sco_accept, NULL);
+
+	g_io_channel_unref(sco_io);
+
+	return 0;
+}
 
 static void card_append_properties(struct ofono_handsfree_card *card,
 					DBusMessageIter *dict)
@@ -437,7 +523,7 @@ void ofono_handsfree_audio_unref(void)
 
 int __ofono_handsfree_audio_manager_init(void)
 {
-	return 0;
+	return sco_init();
 }
 
 void __ofono_handsfree_audio_manager_cleanup(void)
@@ -450,4 +536,7 @@ void __ofono_handsfree_audio_manager_cleanup(void)
 
 	ref_count = 1;
 	ofono_handsfree_audio_unref();
+
+	if (sco_watch > 0)
+		g_source_remove(sco_watch);
 }
-- 
1.7.11.7

[PATCH v1 3/6] handsfree-audio: Add NewConnection

[Claudio Takahasi]

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

This patch adds Agent NewConnection call. The card object path, the SCO
file descriptor, and the codec are being passed to the agent. This
initial version supports CVSD codec only.
---
 src/handsfree-audio.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/src/handsfree-audio.c b/src/handsfree-audio.c
index 24bb2ad..b463844 100644
--- a/src/handsfree-audio.c
+++ b/src/handsfree-audio.c
@@ -75,10 +75,31 @@ static int card_cmp(gconstpointer a, gconstpointer b)
 	return g_strcmp0(card->remote, remote);
 }
 
+static void send_new_connection(const char *card, int fd)
+{
+	DBusMessage *msg;
+	DBusMessageIter iter;
+	uint8_t codec = HFP_CODEC_CVSD;
+
+	msg = dbus_message_new_method_call(agent->owner, agent->path,
+				HFP_AUDIO_AGENT_INTERFACE, "NewConnection");
+	if (msg == NULL)
+		return;
+
+	dbus_message_iter_init_append(msg, &iter);
+	dbus_message_iter_append_basic(&iter, DBUS_TYPE_OBJECT_PATH, &card);
+	dbus_message_iter_append_basic(&iter, DBUS_TYPE_UNIX_FD, &fd);
+	dbus_message_iter_append_basic(&iter, DBUS_TYPE_BYTE, &codec);
+
+	g_dbus_send_message(ofono_dbus_get_connection(), msg);
+}
+
 static gboolean sco_accept(GIOChannel *io, GIOCondition cond,
 							gpointer user_data)
 {
+	struct ofono_handsfree_card *card;
 	struct sockaddr_sco saddr;
+	GSList *list;
 	socklen_t alen;
 	int sk, nsk;
 	char remote[18];
@@ -97,12 +118,17 @@ static gboolean sco_accept(GIOChannel *io, GIOCondition cond,
 
 	bt_ba2str(&saddr.sco_bdaddr, remote);
 
-	if (g_slist_find_custom(card_list, remote, card_cmp) == NULL) {
+	list = g_slist_find_custom(card_list, remote, card_cmp);
+	if (list == NULL) {
 		ofono_error("Rejecting SCO: Audio Card not found!");
 		close(nsk);
 		return TRUE;
 	}
 
+	card = list->data;
+	send_new_connection(card->path, nsk);
+	close(nsk);
+
 	return TRUE;
 }
 
-- 
1.7.11.7

[PATCH v1 4/6] handsfree-audio: Check local SCO address

[Claudio Takahasi]

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

This patch verifies if the local Bluetooth address of the incoming
connection also matches with one of available audio cards.
---
 src/handsfree-audio.c | 44 +++++++++++++++++++++++++++++++-------------
 1 file changed, 31 insertions(+), 13 deletions(-)

diff --git a/src/handsfree-audio.c b/src/handsfree-audio.c
index b463844..14872b0 100644
--- a/src/handsfree-audio.c
+++ b/src/handsfree-audio.c
@@ -67,14 +67,6 @@ static int ref_count = 0;
 static GSList *card_list = 0;
 static guint sco_watch = 0;
 
-static int card_cmp(gconstpointer a, gconstpointer b)
-{
-	const struct ofono_handsfree_card *card = a;
-	const char *remote = b;
-
-	return g_strcmp0(card->remote, remote);
-}
-
 static void send_new_connection(const char *card, int fd)
 {
 	DBusMessage *msg;
@@ -94,15 +86,30 @@ static void send_new_connection(const char *card, int fd)
 	g_dbus_send_message(ofono_dbus_get_connection(), msg);
 }
 
+static struct ofono_handsfree_card *card_find(const char *remote,
+							const char *local)
+{
+	GSList *list;
+
+	for (list = card_list; list; list = g_slist_next(list)) {
+		struct ofono_handsfree_card *card = list->data;
+
+		if (g_str_equal(card->remote, remote) &&
+				g_str_equal(card->local, local))
+			return card;
+	}
+
+	return NULL;
+}
+
 static gboolean sco_accept(GIOChannel *io, GIOCondition cond,
 							gpointer user_data)
 {
 	struct ofono_handsfree_card *card;
 	struct sockaddr_sco saddr;
-	GSList *list;
 	socklen_t alen;
 	int sk, nsk;
-	char remote[18];
+	char local[18], remote[18];
 
 	if (cond & (G_IO_ERR | G_IO_HUP | G_IO_NVAL))
 		return FALSE;
@@ -118,14 +125,25 @@ static gboolean sco_accept(GIOChannel *io, GIOCondition cond,
 
 	bt_ba2str(&saddr.sco_bdaddr, remote);
 
-	list = g_slist_find_custom(card_list, remote, card_cmp);
-	if (list == NULL) {
+	memset(&saddr, 0, sizeof(saddr));
+	alen = sizeof(saddr);
+
+	if (getsockname(nsk, (struct sockaddr *) &saddr, &alen) < 0) {
+		ofono_error("SCO getsockname(): %s (%d)",
+						strerror(errno), errno);
+		close(nsk);
+		return TRUE;
+	}
+
+	bt_ba2str(&saddr.sco_bdaddr, local);
+
+	card = card_find(remote, local);
+	if (card == NULL) {
 		ofono_error("Rejecting SCO: Audio Card not found!");
 		close(nsk);
 		return TRUE;
 	}
 
-	card = list->data;
 	send_new_connection(card->path, nsk);
 	close(nsk);
 
-- 
1.7.11.7

[PATCH v1 5/6] handsfree-audio: Reject SCO if Card is not ready

[Claudio Takahasi]

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

The Audio Card is being created when the NewConnection from BlueZ
Profile is received, and registered when the service level connection
negotiation finishes. This patch rejects SCO connection if the SCO
incoming connection arrives when the service level negotiation is
ongoing.
---
 src/handsfree-audio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/handsfree-audio.c b/src/handsfree-audio.c
index 14872b0..857c258 100644
--- a/src/handsfree-audio.c
+++ b/src/handsfree-audio.c
@@ -138,7 +138,7 @@ static gboolean sco_accept(GIOChannel *io, GIOCondition cond,
 	bt_ba2str(&saddr.sco_bdaddr, local);
 
 	card = card_find(remote, local);
-	if (card == NULL) {
+	if (card == NULL || card->path == NULL) {
 		ofono_error("Rejecting SCO: Audio Card not found!");
 		close(nsk);
 		return TRUE;
-- 
1.7.11.7

[PATCH v1 6/6] handsfree-audio: Reject SCO if agent is unavailable

[Claudio Takahasi]

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

This patch rejects the incoming SCO connection if there isn't a
Handsfree Audio Agent registered.
---
 src/handsfree-audio.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/handsfree-audio.c b/src/handsfree-audio.c
index 857c258..73c6183 100644
--- a/src/handsfree-audio.c
+++ b/src/handsfree-audio.c
@@ -123,6 +123,12 @@ static gboolean sco_accept(GIOChannel *io, GIOCondition cond,
 	if (nsk < 0)
 		return TRUE;
 
+	if (agent == NULL) {
+		ofono_error("Reject SCO: Agent not registered");
+		close(nsk);
+		return TRUE;
+	}
+
 	bt_ba2str(&saddr.sco_bdaddr, remote);
 
 	memset(&saddr, 0, sizeof(saddr));
-- 
1.7.11.7

Re: [PATCH v1 0/6] handsfree-audio: Add Agent NewConnection()

[Denis Kenzior]

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

Hi Claudio,

On 03/04/2013 02:48 PM, Claudio Takahasi wrote:
> This patch series moves the SCO connection handling from hfp_hf_bluez5.c
> to src/handsfree-audio.c, and adds the Agent NewConnection() method call
> to pass the SCO file descriptor from oFono to the Agent implementation.
>
> Changes between v0-v1:
> * Merged commits "Move SCO to handsfree-audio.c" and "Remove modem
> dependency"
> * Removed tracking of supported local HFP version in handsfree-audio
> * Removed ofono_handsfree_get_version()
>
> Claudio Takahasi (6):
>    bluetooth: Add new Bluetooth header
>    handsfree-audio: Move SCO to handsfree-audio.c
>    handsfree-audio: Add NewConnection
>    handsfree-audio: Check local SCO address
>    handsfree-audio: Reject SCO if Card is not ready
>    handsfree-audio: Reject SCO if agent is unavailable
>
>   Makefile.am             |   2 +-
>   plugins/bluez5.c        |  17 ------
>   plugins/bluez5.h        |  41 --------------
>   plugins/hfp_hf_bluez5.c | 103 +----------------------------------
>   src/bluetooth.h         |  71 ++++++++++++++++++++++++
>   src/handsfree-audio.c   | 141 +++++++++++++++++++++++++++++++++++++++++++++++-
>   6 files changed, 214 insertions(+), 161 deletions(-)
>   create mode 100644 src/bluetooth.h
>

I applied this series.  I did re-order and split the patches to be in 
accordance to our patch submission guidelines.  Please refer to the 
HACKING document for more details.

Regards,
-Denis

Re: [PATCH v1 0/6] handsfree-audio: Add Agent NewConnection()

[Claudio Takahasi]

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

Hi Denis:

On Mon, Mar 4, 2013 at 7:44 PM, Denis Kenzior <denkenz@gmail.com> wrote:
> Hi Claudio,
>
>
> On 03/04/2013 02:48 PM, Claudio Takahasi wrote:
>>
>> This patch series moves the SCO connection handling from hfp_hf_bluez5.c
>> to src/handsfree-audio.c, and adds the Agent NewConnection() method call
>> to pass the SCO file descriptor from oFono to the Agent implementation.
>>
>> Changes between v0-v1:
>> * Merged commits "Move SCO to handsfree-audio.c" and "Remove modem
>> dependency"
>> * Removed tracking of supported local HFP version in handsfree-audio
>> * Removed ofono_handsfree_get_version()
>>
>> Claudio Takahasi (6):
>>    bluetooth: Add new Bluetooth header
>>    handsfree-audio: Move SCO to handsfree-audio.c
>>    handsfree-audio: Add NewConnection
>>    handsfree-audio: Check local SCO address
>>    handsfree-audio: Reject SCO if Card is not ready
>>    handsfree-audio: Reject SCO if agent is unavailable
>>
>>   Makefile.am             |   2 +-
>>   plugins/bluez5.c        |  17 ------
>>   plugins/bluez5.h        |  41 --------------
>>   plugins/hfp_hf_bluez5.c | 103 +----------------------------------
>>   src/bluetooth.h         |  71 ++++++++++++++++++++++++
>>   src/handsfree-audio.c   | 141
>> +++++++++++++++++++++++++++++++++++++++++++++++-
>>   6 files changed, 214 insertions(+), 161 deletions(-)
>>   create mode 100644 src/bluetooth.h
>>
>
> I applied this series.  I did re-order and split the patches to be in
> accordance to our patch submission guidelines.  Please refer to the HACKING
> document for more details.
>
> Regards,
> -Denis

I thought that taking care not to break compilation was more
important. Sometimes I avoid to split patches to avoid breaking the
compilation. Especially when it is necessary to change header files.

Regards,
Claudio.

Re: [PATCH v1 0/6] handsfree-audio: Add Agent NewConnection()

[Denis Kenzior]

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

Hi Claudio,

>>
>> I applied this series.  I did re-order and split the patches to be in
>> accordance to our patch submission guidelines.  Please refer to the HACKING
>> document for more details.
>>
>
> I thought that taking care not to break compilation was more
> important. Sometimes I avoid to split patches to avoid breaking the
> compilation. Especially when it is necessary to change header files.
>

Understandable, however we actually optimize the other way.  Because all 
patches are thoroughly reviewed we tend not to need git bisect often. 
In situations like these the primary goal is to still follow the general 
patch submission guidelines.  e.g. breaking up patches per directory. 
The secondary goal is to keep git bisect happy if at all possible.  If 
it is not, then you are allowed to break it.

The rules of thumb are
1. Break up patches appropriately (e.g. in line with HACKING)
2. When adding new code everything must compile individually
3. When refactoring code (which should not happen often), try very hard 
to do same as 2.

Regards,
-Denis