[PATCH v0 0/8] HFP HF: Service Level/Codec negotiation

[PATCH v0 0/8] HFP HF: Service Level/Codec negotiation

[Claudio Takahasi]

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

Second group of patches related to HFP external profile.

The following patches contain the HFP 1.6 codec negotiation.

Remaining HFP HF patches:
git://git.infradead.org/users/cktakahasi/ofono.git HF-20130117

Claudio Takahasi (3):
  hfp_hf: Add NewConnection arguments parsing
  hfp_hf: Register the HFP modem
  hfp_hf: Add service level negotiation

Vinicius Costa Gomes (5):
  hfpmodem: Add version defines for HFP 1.6
  hfpmodem: Add support for sending the supported codecs
  hfpmodem: Add support for storing the supported codecs
  hfpmodem: Send the AT+BAC command with the supported codecs
  hfp_hf: Add support for codec negotiation

 Makefile.am             |   3 +-
 drivers/hfpmodem/slc.c  |  54 ++++++-
 drivers/hfpmodem/slc.h  |  13 +-
 plugins/bluez5.c        |  81 ++++++++++
 plugins/bluez5.h        |   3 +
 plugins/hfp_hf_bluez4.c |   5 +-
 plugins/hfp_hf_bluez5.c | 408 +++++++++++++++++++++++++++++++++++++++++++++++-
 plugins/media.c         |  63 ++++++++
 plugins/media.h         |  29 ++++
 plugins/phonesim.c      |  12 +-
 10 files changed, 658 insertions(+), 13 deletions(-)
 create mode 100644 plugins/media.c
 create mode 100644 plugins/media.h

-- 
1.7.11.7

[PATCH v0 1/8] hfp_hf: Add NewConnection arguments parsing

[Claudio Takahasi]

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

This patch adds NewConnection message arguments parsing: features,
version and Media Endpoints included in the fd_properties dictionary.
---
 Makefile.am             |   3 +-
 plugins/bluez5.c        |  81 +++++++++++++++++++++++++++
 plugins/bluez5.h        |   3 +
 plugins/hfp_hf_bluez5.c | 144 +++++++++++++++++++++++++++++++++++++++++++++++-
 plugins/media.c         |  63 +++++++++++++++++++++
 plugins/media.h         |  29 ++++++++++
 6 files changed, 319 insertions(+), 4 deletions(-)
 create mode 100644 plugins/media.c
 create mode 100644 plugins/media.h

diff --git a/Makefile.am b/Makefile.am
index f24cac7..f1d241f 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -438,7 +438,8 @@ builtin_modules += bluez5
 builtin_sources += plugins/bluez5.c plugins/bluez5.h
 
 builtin_modules += hfp_bluez5
-builtin_sources += plugins/hfp_hf_bluez5.c plugins/bluez5.h
+builtin_sources += plugins/bluez5.h plugins/media.h \
+			plugins/media.c plugins/hfp_hf_bluez5.c
 endif
 endif
 endif
diff --git a/plugins/bluez5.c b/plugins/bluez5.c
index 84ba47d..6ba5ea1 100644
--- a/plugins/bluez5.c
+++ b/plugins/bluez5.c
@@ -36,6 +36,87 @@
 
 #define BLUEZ_PROFILE_MGMT_INTERFACE   BLUEZ_SERVICE ".ProfileManager1"
 
+typedef void (*PropertyHandler)(DBusMessageIter *iter, gpointer user_data);
+
+struct property_handler {
+	const char *property;
+	PropertyHandler callback;
+	gpointer user_data;
+};
+
+static gint property_handler_compare(gconstpointer a, gconstpointer b)
+{
+	const struct property_handler *handler = a;
+	const char *property = b;
+
+	return g_strcmp0(handler->property, property);
+}
+
+void bluetooth_iter_parse_properties(DBusMessageIter *array,
+						const char *property, ...)
+{
+	va_list args;
+	GSList *prop_handlers = NULL;
+	DBusMessageIter dict;
+
+	if (dbus_message_iter_get_arg_type(array) != DBUS_TYPE_ARRAY)
+		goto done;
+
+	va_start(args, property);
+
+	while (property != NULL) {
+		struct property_handler *handler =
+					g_new0(struct property_handler, 1);
+
+		handler->property = property;
+		handler->callback = va_arg(args, PropertyHandler);
+		handler->user_data = va_arg(args, gpointer);
+
+		property = va_arg(args, const char *);
+
+		prop_handlers = g_slist_prepend(prop_handlers, handler);
+	}
+
+	va_end(args);
+
+	dbus_message_iter_recurse(array, &dict);
+
+	while (dbus_message_iter_get_arg_type(&dict) == DBUS_TYPE_DICT_ENTRY) {
+		DBusMessageIter entry, value;
+		const char *key;
+		GSList *l;
+
+		dbus_message_iter_recurse(&dict, &entry);
+
+		if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_STRING)
+			goto done;
+
+		dbus_message_iter_get_basic(&entry, &key);
+
+		dbus_message_iter_next(&entry);
+
+		if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_VARIANT)
+			goto done;
+
+		dbus_message_iter_recurse(&entry, &value);
+
+		l = g_slist_find_custom(prop_handlers, key,
+					property_handler_compare);
+
+		if (l) {
+			struct property_handler *handler = l->data;
+
+			handler->callback(&value, handler->user_data);
+		}
+
+		dbus_message_iter_next(&dict);
+	}
+
+done:
+	g_slist_foreach(prop_handlers, (GFunc) g_free, NULL);
+	g_slist_free(prop_handlers);
+}
+
 static void profile_register_cb(DBusPendingCall *call, gpointer user_data)
 {
 	DBusMessage *reply;
diff --git a/plugins/bluez5.h b/plugins/bluez5.h
index 01ecfe8..7eae8c4 100644
--- a/plugins/bluez5.h
+++ b/plugins/bluez5.h
@@ -29,3 +29,6 @@ int bluetooth_register_profile(DBusConnection *conn, const char *uuid,
 					const char *name, const char *object);
 
 void bluetooth_unregister_profile(DBusConnection *conn, const char *object);
+
+void bluetooth_iter_parse_properties(DBusMessageIter *array,
+						const char *property, ...);
diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
index e024838..636b1b3 100644
--- a/plugins/hfp_hf_bluez5.c
+++ b/plugins/hfp_hf_bluez5.c
@@ -35,6 +35,7 @@
 #include <ofono/log.h>
 
 #include "bluez5.h"
+#include "media.h"
 
 #ifndef DBUS_TYPE_UNIX_FD
 #define DBUS_TYPE_UNIX_FD -1
@@ -42,6 +43,106 @@
 
 #define HFP_EXT_PROFILE_PATH   "/bluetooth/profile/hfp_hf"
 
+static void parse_guint16(DBusMessageIter *iter, gpointer user_data)
+{
+	guint16 *value = user_data;
+
+	if (dbus_message_iter_get_arg_type(iter) !=  DBUS_TYPE_UINT16)
+		return;
+
+	dbus_message_iter_get_basic(iter, value);
+}
+
+static void parse_string(DBusMessageIter *iter, gpointer user_data)
+{
+	char **str = user_data;
+	int arg_type = dbus_message_iter_get_arg_type(iter);
+
+	if (arg_type != DBUS_TYPE_OBJECT_PATH && arg_type != DBUS_TYPE_STRING)
+		return;
+
+	dbus_message_iter_get_basic(iter, str);
+}
+
+static void parse_byte(DBusMessageIter *iter, gpointer user_data)
+{
+	guint8 *value = user_data;
+
+	if (dbus_message_iter_get_arg_type(iter) !=  DBUS_TYPE_BYTE)
+		return;
+
+	dbus_message_iter_get_basic(iter, value);
+}
+
+static void parse_byte_array(DBusMessageIter *iter, gpointer user_data)
+{
+	DBusMessageIter array;
+	GArray **garray = user_data;
+	guint8 *data = NULL;
+	int n = 0;
+
+	if (dbus_message_iter_get_arg_type(iter) !=  DBUS_TYPE_ARRAY)
+		return;
+
+	dbus_message_iter_recurse(iter, &array);
+	dbus_message_iter_get_fixed_array(&array, &data, &n);
+	if (n == 0)
+		return;
+
+	*garray = g_array_sized_new(TRUE, TRUE, sizeof(guint8), n);
+	*garray = g_array_append_vals(*garray, (gconstpointer) data, n);
+}
+
+static void parse_media_endpoints(DBusMessageIter *array, gpointer user_data)
+{
+	const char *path, *owner;
+	GSList **endpoints = user_data;
+	GArray *capabilities = NULL;
+	struct media_endpoint *endpoint;
+	guint8 codec;
+	DBusMessageIter dict, variant, entry;
+
+	if (dbus_message_iter_get_arg_type(array) != DBUS_TYPE_ARRAY)
+		return;
+
+	dbus_message_iter_recurse(array, &dict);
+
+	while (dbus_message_iter_get_arg_type(&dict) == DBUS_TYPE_DICT_ENTRY) {
+		path = NULL;
+		codec = 0x00;
+		capabilities = NULL;
+
+		dbus_message_iter_recurse(&dict, &entry);
+		if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_STRING)
+			return;
+
+		dbus_message_iter_get_basic(&entry, &owner);
+
+		dbus_message_iter_next(&entry);
+
+		if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_VARIANT)
+			return;
+
+		dbus_message_iter_recurse(&entry, &variant);
+
+		bluetooth_iter_parse_properties(&variant,
+				"Path", parse_string, &path,
+				"Codec", parse_byte, &codec,
+				"Capabilities", parse_byte_array, &capabilities,
+				NULL);
+
+		dbus_message_iter_next(&dict);
+
+		endpoint = media_endpoint_new(owner, path, codec, capabilities);
+		if (capabilities)
+			g_array_unref(capabilities);
+
+		*endpoints = g_slist_append(*endpoints, endpoint);
+
+		DBG("Media Endpoint %s %s codec: 0x%02X", owner, path, codec);
+	}
+}
+
 static int hfp_probe(struct ofono_modem *modem)
 {
 	DBG("modem: %p", modem);
@@ -93,11 +194,48 @@ static struct ofono_modem_driver hfp_driver = {
 static DBusMessage *profile_new_connection(DBusConnection *conn,
 					DBusMessage *msg, void *user_data)
 {
+	DBusMessageIter entry;
+	const char *device;
+	GSList *endpoints = NULL;
+	guint16 version, features;
+	int fd;
+
 	DBG("Profile handler NewConnection");
 
-	return g_dbus_create_error(msg, BLUEZ_ERROR_INTERFACE
-					".NotImplemented",
-					"Implementation not provided");
+	if (dbus_message_iter_init(msg, &entry) == FALSE)
+		goto error;
+
+	if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_OBJECT_PATH)
+		goto error;
+
+	dbus_message_iter_get_basic(&entry, &device);
+	dbus_message_iter_next(&entry);
+	if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_UNIX_FD)
+		goto error;
+
+	dbus_message_iter_get_basic(&entry, &fd);
+	if (fd < 0)
+		goto error;
+
+	dbus_message_iter_next(&entry);
+
+	bluetooth_iter_parse_properties(&entry,
+			"Version", parse_guint16, &version,
+			"Features", parse_guint16, &features,
+			"MediaEndpoints", parse_media_endpoints, &endpoints,
+			NULL);
+
+	DBG("%s version: 0x%04x features: 0x%04x", device, version, features);
+
+	if (endpoints == NULL) {
+		ofono_error("Media Endpoint missing");
+		goto error;
+	}
+	return NULL;
+
+error:
+	return g_dbus_create_error(msg, BLUEZ_ERROR_INTERFACE ".Rejected",
+					"Invalid arguments in method call");
 }
 
 static DBusMessage *profile_release(DBusConnection *conn,
diff --git a/plugins/media.c b/plugins/media.c
new file mode 100644
index 0000000..b4f0650
--- /dev/null
+++ b/plugins/media.c
@@ -0,0 +1,63 @@
+/*
+ *
+ *  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
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <glib.h>
+
+#include "media.h"
+
+struct media_endpoint {
+	char *owner;
+	char *path;
+	guint8 codec;
+	GArray *capabilities;
+};
+
+struct media_endpoint *media_endpoint_new(const char *owner,
+					const char *path,
+					guint8 codec,
+					GArray *capabilities)
+{
+	struct media_endpoint *endpoint;
+
+	endpoint = g_new0(struct media_endpoint, 1);
+	endpoint->owner = g_strdup(owner);
+	endpoint->path = g_strdup(path);
+	endpoint->codec = codec;
+	if (capabilities)
+		endpoint->capabilities = g_array_ref(capabilities);
+
+	return endpoint;
+}
+
+void media_endpoint_free(gpointer data)
+{
+	struct media_endpoint *endpoint = data;
+
+	g_free(endpoint->owner);
+	g_free(endpoint->path);
+	if (endpoint->capabilities)
+		g_array_unref(endpoint->capabilities);
+	g_free(endpoint);
+}
diff --git a/plugins/media.h b/plugins/media.h
new file mode 100644
index 0000000..0df107f
--- /dev/null
+++ b/plugins/media.h
@@ -0,0 +1,29 @@
+/*
+ *
+ *  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
+ *
+ */
+
+struct media_endpoint;
+
+struct media_endpoint *media_endpoint_new(const char *owner,
+					const char *path,
+					guint8 codec,
+					GArray *capabilities);
+
+void media_endpoint_free(gpointer data);
-- 
1.7.11.7

Re: [PATCH v0 1/8] hfp_hf: Add NewConnection arguments parsing

[Denis Kenzior]

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

Hi Claudio,

On 01/17/2013 09:13 AM, Claudio Takahasi wrote:
> This patch adds NewConnection message arguments parsing: features,
> version and Media Endpoints included in the fd_properties dictionary.
> ---
>   Makefile.am             |   3 +-
>   plugins/bluez5.c        |  81 +++++++++++++++++++++++++++
>   plugins/bluez5.h        |   3 +
>   plugins/hfp_hf_bluez5.c | 144 +++++++++++++++++++++++++++++++++++++++++++++++-
>   plugins/media.c         |  63 +++++++++++++++++++++
>   plugins/media.h         |  29 ++++++++++

Please separate this into three patches, one for bluez5.[ch] changes, 
one for media.[ch] changes and one for the hfp_hf_bluez5.c changes. 
Also, is there a compelling reason to not roll media.[ch] into 
bluez5.[ch]?  It is being put into plugins directory and it is not 
really a plugin.  We made a couple exceptions (e.g. for mbpi.c) but I do 
not really want to make this into a habit.

>   6 files changed, 319 insertions(+), 4 deletions(-)
>   create mode 100644 plugins/media.c
>   create mode 100644 plugins/media.h
>
> diff --git a/Makefile.am b/Makefile.am
> index f24cac7..f1d241f 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -438,7 +438,8 @@ builtin_modules += bluez5
>   builtin_sources += plugins/bluez5.c plugins/bluez5.h
>
>   builtin_modules += hfp_bluez5
> -builtin_sources += plugins/hfp_hf_bluez5.c plugins/bluez5.h
> +builtin_sources += plugins/bluez5.h plugins/media.h \
> +			plugins/media.c plugins/hfp_hf_bluez5.c
>   endif
>   endif
>   endif
> diff --git a/plugins/bluez5.c b/plugins/bluez5.c
> index 84ba47d..6ba5ea1 100644
> --- a/plugins/bluez5.c
> +++ b/plugins/bluez5.c
> @@ -36,6 +36,87 @@
>
>   #define BLUEZ_PROFILE_MGMT_INTERFACE   BLUEZ_SERVICE ".ProfileManager1"
>
> +typedef void (*PropertyHandler)(DBusMessageIter *iter, gpointer user_data);

In general we prefer not to use CamelCase outside of GLib-like code.

> +
> +struct property_handler {
> +	const char *property;
> +	PropertyHandler callback;
> +	gpointer user_data;
> +};
> +
> +static gint property_handler_compare(gconstpointer a, gconstpointer b)
> +{
> +	const struct property_handler *handler = a;
> +	const char *property = b;
> +
> +	return g_strcmp0(handler->property, property);
> +}
> +
> +void bluetooth_iter_parse_properties(DBusMessageIter *array,
> +						const char *property, ...)
> +{
> +	va_list args;
> +	GSList *prop_handlers = NULL;
> +	DBusMessageIter dict;
> +
> +	if (dbus_message_iter_get_arg_type(array) != DBUS_TYPE_ARRAY)
> +		goto done;
> +
> +	va_start(args, property);
> +
> +	while (property != NULL) {
> +		struct property_handler *handler =
> +					g_new0(struct property_handler, 1);
> +
> +		handler->property = property;
> +		handler->callback = va_arg(args, PropertyHandler);
> +		handler->user_data = va_arg(args, gpointer);
> +
> +		property = va_arg(args, const char *);
> +
> +		prop_handlers = g_slist_prepend(prop_handlers, handler);
> +	}
> +
> +	va_end(args);
> +
> +	dbus_message_iter_recurse(array,&dict);
> +
> +	while (dbus_message_iter_get_arg_type(&dict) == DBUS_TYPE_DICT_ENTRY) {
> +		DBusMessageIter entry, value;
> +		const char *key;
> +		GSList *l;
> +
> +		dbus_message_iter_recurse(&dict,&entry);
> +
> +		if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_STRING)
> +			goto done;
> +
> +		dbus_message_iter_get_basic(&entry,&key);
> +
> +		dbus_message_iter_next(&entry);
> +
> +		if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_VARIANT)
> +			goto done;
> +
> +		dbus_message_iter_recurse(&entry,&value);
> +
> +		l = g_slist_find_custom(prop_handlers, key,
> +					property_handler_compare);
> +
> +		if (l) {
> +			struct property_handler *handler = l->data;
> +
> +			handler->callback(&value, handler->user_data);
> +		}
> +
> +		dbus_message_iter_next(&dict);
> +	}
> +
> +done:
> +	g_slist_foreach(prop_handlers, (GFunc) g_free, NULL);
> +	g_slist_free(prop_handlers);
> +}
> +

Is this a copy-paste job out of bluez4.c?

>   static void profile_register_cb(DBusPendingCall *call, gpointer user_data)
>   {
>   	DBusMessage *reply;
> diff --git a/plugins/bluez5.h b/plugins/bluez5.h
> index 01ecfe8..7eae8c4 100644
> --- a/plugins/bluez5.h
> +++ b/plugins/bluez5.h
> @@ -29,3 +29,6 @@ int bluetooth_register_profile(DBusConnection *conn, const char *uuid,
>   					const char *name, const char *object);
>
>   void bluetooth_unregister_profile(DBusConnection *conn, const char *object);
> +
> +void bluetooth_iter_parse_properties(DBusMessageIter *array,
> +						const char *property, ...);
> diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
> index e024838..636b1b3 100644
> --- a/plugins/hfp_hf_bluez5.c
> +++ b/plugins/hfp_hf_bluez5.c
> @@ -35,6 +35,7 @@
>   #include<ofono/log.h>
>
>   #include "bluez5.h"
> +#include "media.h"
>
>   #ifndef DBUS_TYPE_UNIX_FD
>   #define DBUS_TYPE_UNIX_FD -1
> @@ -42,6 +43,106 @@
>
>   #define HFP_EXT_PROFILE_PATH   "/bluetooth/profile/hfp_hf"
>
> +static void parse_guint16(DBusMessageIter *iter, gpointer user_data)
> +{
> +	guint16 *value = user_data;
> +
> +	if (dbus_message_iter_get_arg_type(iter) !=  DBUS_TYPE_UINT16)
> +		return;
> +
> +	dbus_message_iter_get_basic(iter, value);
> +}
> +
> +static void parse_string(DBusMessageIter *iter, gpointer user_data)
> +{
> +	char **str = user_data;
> +	int arg_type = dbus_message_iter_get_arg_type(iter);
> +
> +	if (arg_type != DBUS_TYPE_OBJECT_PATH&&  arg_type != DBUS_TYPE_STRING)
> +		return;
> +
> +	dbus_message_iter_get_basic(iter, str);
> +}
> +
> +static void parse_byte(DBusMessageIter *iter, gpointer user_data)
> +{
> +	guint8 *value = user_data;
> +
> +	if (dbus_message_iter_get_arg_type(iter) !=  DBUS_TYPE_BYTE)
> +		return;
> +
> +	dbus_message_iter_get_basic(iter, value);
> +}
> +
> +static void parse_byte_array(DBusMessageIter *iter, gpointer user_data)
> +{
> +	DBusMessageIter array;
> +	GArray **garray = user_data;
> +	guint8 *data = NULL;
> +	int n = 0;
> +
> +	if (dbus_message_iter_get_arg_type(iter) !=  DBUS_TYPE_ARRAY)
> +		return;
> +
> +	dbus_message_iter_recurse(iter,&array);
> +	dbus_message_iter_get_fixed_array(&array,&data,&n);
> +	if (n == 0)
> +		return;
> +
> +	*garray = g_array_sized_new(TRUE, TRUE, sizeof(guint8), n);
> +	*garray = g_array_append_vals(*garray, (gconstpointer) data, n);

Please just use g_memdup instead and stay away from GArray.  oFono does 
not use GArray anywhere else and I do not really want to start.  The 
long-term plan is to get rid of GLib.  So whenever you can use basic types.

> +}
> +
> +static void parse_media_endpoints(DBusMessageIter *array, gpointer user_data)
> +{
> +	const char *path, *owner;
> +	GSList **endpoints = user_data;
> +	GArray *capabilities = NULL;
> +	struct media_endpoint *endpoint;
> +	guint8 codec;
> +	DBusMessageIter dict, variant, entry;
> +
> +	if (dbus_message_iter_get_arg_type(array) != DBUS_TYPE_ARRAY)
> +		return;
> +
> +	dbus_message_iter_recurse(array,&dict);
> +
> +	while (dbus_message_iter_get_arg_type(&dict) == DBUS_TYPE_DICT_ENTRY) {
> +		path = NULL;
> +		codec = 0x00;
> +		capabilities = NULL;
> +
> +		dbus_message_iter_recurse(&dict,&entry);
> +		if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_STRING)
> +			return;
> +
> +		dbus_message_iter_get_basic(&entry,&owner);
> +
> +		dbus_message_iter_next(&entry);
> +
> +		if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_VARIANT)
> +			return;
> +
> +		dbus_message_iter_recurse(&entry,&variant);
> +
> +		bluetooth_iter_parse_properties(&variant,
> +				"Path", parse_string,&path,
> +				"Codec", parse_byte,&codec,
> +				"Capabilities", parse_byte_array,&capabilities,
> +				NULL);

Sounds like you should be passing in the endpoint structure here instead

> +
> +		dbus_message_iter_next(&dict);
> +
> +		endpoint = media_endpoint_new(owner, path, codec, capabilities);
> +		if (capabilities)
> +			g_array_unref(capabilities);
> +
> +		*endpoints = g_slist_append(*endpoints, endpoint);
> +
> +		DBG("Media Endpoint %s %s codec: 0x%02X", owner, path, codec);
> +	}
> +}
> +
>   static int hfp_probe(struct ofono_modem *modem)
>   {
>   	DBG("modem: %p", modem);
> @@ -93,11 +194,48 @@ static struct ofono_modem_driver hfp_driver = {
>   static DBusMessage *profile_new_connection(DBusConnection *conn,
>   					DBusMessage *msg, void *user_data)
>   {
> +	DBusMessageIter entry;
> +	const char *device;
> +	GSList *endpoints = NULL;
> +	guint16 version, features;
> +	int fd;
> +
>   	DBG("Profile handler NewConnection");
>
> -	return g_dbus_create_error(msg, BLUEZ_ERROR_INTERFACE
> -					".NotImplemented",
> -					"Implementation not provided");
> +	if (dbus_message_iter_init(msg,&entry) == FALSE)
> +		goto error;
> +
> +	if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_OBJECT_PATH)
> +		goto error;
> +
> +	dbus_message_iter_get_basic(&entry,&device);
> +	dbus_message_iter_next(&entry);
> +	if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_UNIX_FD)
> +		goto error;
> +
> +	dbus_message_iter_get_basic(&entry,&fd);
> +	if (fd<  0)
> +		goto error;
> +
> +	dbus_message_iter_next(&entry);
> +
> +	bluetooth_iter_parse_properties(&entry,
> +			"Version", parse_guint16,&version,
> +			"Features", parse_guint16,&features,
> +			"MediaEndpoints", parse_media_endpoints,&endpoints,
> +			NULL);
> +
> +	DBG("%s version: 0x%04x features: 0x%04x", device, version, features);
> +
> +	if (endpoints == NULL) {
> +		ofono_error("Media Endpoint missing");
> +		goto error;
> +	}
> +	return NULL;
> +
> +error:
> +	return g_dbus_create_error(msg, BLUEZ_ERROR_INTERFACE ".Rejected",
> +					"Invalid arguments in method call");
>   }
>
>   static DBusMessage *profile_release(DBusConnection *conn,
> diff --git a/plugins/media.c b/plugins/media.c
> new file mode 100644
> index 0000000..b4f0650
> --- /dev/null
> +++ b/plugins/media.c
> @@ -0,0 +1,63 @@
> +/*
> + *
> + *  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
> + *
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +#include<config.h>
> +#endif
> +
> +#include<glib.h>
> +
> +#include "media.h"
> +
> +struct media_endpoint {
> +	char *owner;
> +	char *path;
> +	guint8 codec;
> +	GArray *capabilities;
> +};
> +
> +struct media_endpoint *media_endpoint_new(const char *owner,
> +					const char *path,
> +					guint8 codec,
> +					GArray *capabilities)
> +{
> +	struct media_endpoint *endpoint;
> +
> +	endpoint = g_new0(struct media_endpoint, 1);
> +	endpoint->owner = g_strdup(owner);
> +	endpoint->path = g_strdup(path);
> +	endpoint->codec = codec;
> +	if (capabilities)
> +		endpoint->capabilities = g_array_ref(capabilities);
> +
> +	return endpoint;
> +}
> +
> +void media_endpoint_free(gpointer data)
> +{
> +	struct media_endpoint *endpoint = data;
> +
> +	g_free(endpoint->owner);
> +	g_free(endpoint->path);
> +	if (endpoint->capabilities)
> +		g_array_unref(endpoint->capabilities);
> +	g_free(endpoint);
> +}
> diff --git a/plugins/media.h b/plugins/media.h
> new file mode 100644
> index 0000000..0df107f
> --- /dev/null
> +++ b/plugins/media.h
> @@ -0,0 +1,29 @@
> +/*
> + *
> + *  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
> + *
> + */
> +
> +struct media_endpoint;
> +
> +struct media_endpoint *media_endpoint_new(const char *owner,
> +					const char *path,
> +					guint8 codec,
> +					GArray *capabilities);
> +
> +void media_endpoint_free(gpointer data);

Regards,
-Denis

Re: [PATCH v0 1/8] hfp_hf: Add NewConnection arguments parsing

[Claudio Takahasi]

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

Hi Denis:

> Please separate this into three patches, one for bluez5.[ch] changes, one
> for media.[ch] changes and one for the hfp_hf_bluez5.c changes. Also, is
> there a compelling reason to not roll media.[ch] into bluez5.[ch]?  It is
> being put into plugins directory and it is not really a plugin.  We made a
> couple exceptions (e.g. for mbpi.c) but I do not really want to make this
> into a habit.

We will analyze if it is better to move media.[ch] functions to
bluez5.[ch] or hfp_hf_bluez5.[ch]

media.[ch]  contains HF related functions, and bluez5.[ch] should
contain common functions used by hfp and others BlueZ 5 based plugins:
sap, dun, ...

>>
>> +typedef void (*PropertyHandler)(DBusMessageIter *iter, gpointer
>> user_data);
>
>
> In general we prefer not to use CamelCase outside of GLib-like code.

Declaration copied from bluez4.c, I will fix this declaration.

>> +
>> +void bluetooth_iter_parse_properties(DBusMessageIter *array,
>> +                                               const char *property, ...)
>
> Is this a copy-paste job out of bluez4.c?

Yes. It is a *partial* copy of bluez4.c with minor changes. If the
plan is to drop BlueZ 4 support, we need avoid including bluez4.h
header.

>> +       *garray = g_array_sized_new(TRUE, TRUE, sizeof(guint8), n);
>
> Please just use g_memdup instead and stay away from GArray.  oFono does not
> use GArray anywhere else and I do not really want to start.  The long-term
> plan is to get rid of GLib.  So whenever you can use basic types.

OK. I will fix it.

>> +
>> +               bluetooth_iter_parse_properties(&variant,
>> +                               "Path", parse_string,&path,
>> +                               "Codec", parse_byte,&codec,
>> +                               "Capabilities",
>> parse_byte_array,&capabilities,
>> +                               NULL);
>
>
> Sounds like you should be passing in the endpoint structure here instead
>

Media endpoint structure is opaque, the field are not accessible here
unless we move media.[ch] functions to this file.

Regards,
Claudio

Re: [PATCH v0 1/8] hfp_hf: Add NewConnection arguments parsing

[Denis Kenzior]

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

Hi Claudio,

On 01/17/2013 11:58 AM, Claudio Takahasi wrote:
> Hi Denis:
>
>> Please separate this into three patches, one for bluez5.[ch] changes, one
>> for media.[ch] changes and one for the hfp_hf_bluez5.c changes. Also, is
>> there a compelling reason to not roll media.[ch] into bluez5.[ch]?  It is
>> being put into plugins directory and it is not really a plugin.  We made a
>> couple exceptions (e.g. for mbpi.c) but I do not really want to make this
>> into a habit.
>
> We will analyze if it is better to move media.[ch] functions to
> bluez5.[ch] or hfp_hf_bluez5.[ch]
>
> media.[ch]  contains HF related functions, and bluez5.[ch] should
> contain common functions used by hfp and others BlueZ 5 based plugins:
> sap, dun, ...
>

The media API is also part of BlueZ 5, so it makes sense to move it to me...

<snip>

>>> +
>>> +void bluetooth_iter_parse_properties(DBusMessageIter *array,
>>> +                                               const char *property, ...)
>>
>> Is this a copy-paste job out of bluez4.c?
>
> Yes. It is a *partial* copy of bluez4.c with minor changes. If the
> plan is to drop BlueZ 4 support, we need avoid including bluez4.h
> header.
>

Perhaps this part needs to be moved into another utility file.  Either 
inside gdbus or into src/dbus.c.

<snip>

>>> +
>>> +               bluetooth_iter_parse_properties(&variant,
>>> +                               "Path", parse_string,&path,
>>> +                               "Codec", parse_byte,&codec,
>>> +                               "Capabilities",
>>> parse_byte_array,&capabilities,
>>> +                               NULL);
>>
>>
>> Sounds like you should be passing in the endpoint structure here instead
>>
>
> Media endpoint structure is opaque, the field are not accessible here
> unless we move media.[ch] functions to this file.

One can always implement the parser inside media.c as well.  However, it 
seems to me that long-term you do not want to store both the media 
endpoint objects and the slc_info codecs at the same time.  One or the 
other is enough, they are the same information.

Regards,
-Denis

[PATCH v0 2/8] hfpmodem: Add version defines for HFP 1.6

[Claudio Takahasi]

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

From: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>

---
 drivers/hfpmodem/slc.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/hfpmodem/slc.h b/drivers/hfpmodem/slc.h
index a9f42f7..b71ffe9 100644
--- a/drivers/hfpmodem/slc.h
+++ b/drivers/hfpmodem/slc.h
@@ -29,7 +29,8 @@
 
 enum hfp_version {
 	HFP_VERSION_1_5 =	0x0105,
-	HFP_VERSION_LATEST =	HFP_VERSION_1_5,
+	HFP_VERSION_1_6 =	0x0106,
+	HFP_VERSION_LATEST =	HFP_VERSION_1_6,
 };
 
 enum hfp_indicator {
-- 
1.7.11.7

Re: [PATCH v0 2/8] hfpmodem: Add version defines for HFP 1.6

[Denis Kenzior]

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

Hi Claudio,

On 01/17/2013 09:13 AM, Claudio Takahasi wrote:
> From: Vinicius Costa Gomes<vinicius.gomes@openbossa.org>
>
> ---
>   drivers/hfpmodem/slc.h | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>

Patch has been applied, thanks.

Regards,
-Denis

[PATCH v0 3/8] hfpmodem: Add support for sending the supported codecs

[Claudio Takahasi]

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

From: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>

Right now, only the mandatory CVSD codec is supported. The mSBC
mandatory codec is "temporarily" not supported.

The spec alows this, HFP 1.6 Spec Section 4.34.1 page 92: "If wide band
speech is supported then the mandatory codec (mSBC) shall be included
unless it is temporarily unavailable."
---
 drivers/hfpmodem/slc.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/drivers/hfpmodem/slc.c b/drivers/hfpmodem/slc.c
index 4028479..646befa 100644
--- a/drivers/hfpmodem/slc.c
+++ b/drivers/hfpmodem/slc.c
@@ -67,6 +67,11 @@ void hfp_slc_info_init(struct hfp_slc_info *info, guint16 version)
 	info->hf_features |= HFP_HF_FEATURE_ENHANCED_CALL_STATUS;
 	info->hf_features |= HFP_HF_FEATURE_ENHANCED_CALL_CONTROL;
 
+	if (version < HFP_VERSION_1_6)
+		goto done;
+
+	info->hf_features |= HFP_HF_FEATURE_CODEC_NEGOTIATION;
+
 done:
 	memset(info->cind_val, 0, sizeof(info->cind_val));
 	memset(info->cind_pos, 0, sizeof(info->cind_pos));
@@ -266,6 +271,21 @@ error:
 	slc_failed(sed);
 }
 
+static void bac_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct slc_establish_data *sed = user_data;
+	struct hfp_slc_info *info = sed->info;
+
+	if (!ok) {
+		slc_failed(sed);
+		return;
+	}
+
+	slc_establish_data_ref(sed);
+	g_at_chat_send(info->chat, "AT+CIND=?", cind_prefix,
+				cind_cb, sed, slc_establish_data_unref);
+}
+
 static void brsf_cb(gboolean ok, GAtResult *result, gpointer user_data)
 {
 	struct slc_establish_data *sed = user_data;
@@ -282,6 +302,15 @@ static void brsf_cb(gboolean ok, GAtResult *result, gpointer user_data)
 
 	g_at_result_iter_next_number(&iter, (gint *)&info->ag_features);
 
+	if (info->ag_features & HFP_AG_FEATURE_CODEC_NEGOTIATION &&
+			info->hf_features & HFP_HF_FEATURE_CODEC_NEGOTIATION) {
+
+		slc_establish_data_ref(sed);
+		g_at_chat_send(info->chat, "AT+BAC=1", NULL, bac_cb, sed,
+						slc_establish_data_unref);
+		return;
+	}
+
 	slc_establish_data_ref(sed);
 	g_at_chat_send(info->chat, "AT+CIND=?", cind_prefix,
 				cind_cb, sed, slc_establish_data_unref);
-- 
1.7.11.7

Re: [PATCH v0 3/8] hfpmodem: Add support for sending the supported codecs

[Denis Kenzior]

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

Hi Claudio,

On 01/17/2013 09:13 AM, Claudio Takahasi wrote:
> From: Vinicius Costa Gomes<vinicius.gomes@openbossa.org>
>
> Right now, only the mandatory CVSD codec is supported. The mSBC
> mandatory codec is "temporarily" not supported.
>
> The spec alows this, HFP 1.6 Spec Section 4.34.1 page 92: "If wide band
> speech is supported then the mandatory codec (mSBC) shall be included
> unless it is temporarily unavailable."
> ---
>   drivers/hfpmodem/slc.c | 29 +++++++++++++++++++++++++++++
>   1 file changed, 29 insertions(+)

Patch has been applied, thanks.

Regards,
-Denis

[PATCH v0 4/8] hfpmodem: Add support for storing the supported codecs

[Claudio Takahasi]

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

From: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>

As informing the AG of the supported codecs is part of the setup
of the SLC, it makes sense to have this information inside the SLC
establishment part.

Now the hfp_slc_info structure has dynamic information, and so the
hfp_slc_info_free() function is used.
---
 drivers/hfpmodem/slc.c  | 14 +++++++++++++-
 drivers/hfpmodem/slc.h  | 10 +++++++++-
 plugins/hfp_hf_bluez4.c |  5 ++++-
 plugins/phonesim.c      | 12 +++++++-----
 4 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/drivers/hfpmodem/slc.c b/drivers/hfpmodem/slc.c
index 646befa..288afdd 100644
--- a/drivers/hfpmodem/slc.c
+++ b/drivers/hfpmodem/slc.c
@@ -52,7 +52,8 @@ struct slc_establish_data {
 	gpointer userdata;
 };
 
-void hfp_slc_info_init(struct hfp_slc_info *info, guint16 version)
+void hfp_slc_info_init(struct hfp_slc_info *info, guint16 version,
+						unsigned char *codecs, int len)
 {
 	info->ag_features = 0;
 	info->ag_mpty_features = 0;
@@ -72,11 +73,22 @@ void hfp_slc_info_init(struct hfp_slc_info *info, guint16 version)
 
 	info->hf_features |= HFP_HF_FEATURE_CODEC_NEGOTIATION;
 
+	info->codecs = g_memdup(codecs, len);
+	info->codecs_len = len;
+
 done:
 	memset(info->cind_val, 0, sizeof(info->cind_val));
 	memset(info->cind_pos, 0, sizeof(info->cind_pos));
 }
 
+void hfp_slc_info_free(struct hfp_slc_info *info)
+{
+	g_free(info->codecs);
+
+	g_at_chat_unref(info->chat);
+	info->chat = NULL;
+}
+
 static void slc_establish_data_unref(gpointer userdata)
 {
 	struct slc_establish_data *sed = userdata;
diff --git a/drivers/hfpmodem/slc.h b/drivers/hfpmodem/slc.h
index b71ffe9..d7c90a2 100644
--- a/drivers/hfpmodem/slc.h
+++ b/drivers/hfpmodem/slc.h
@@ -44,6 +44,11 @@ enum hfp_indicator {
 	HFP_INDICATOR_LAST
 };
 
+enum hfp_codec {
+	HFP_CODEC_CVSD = 0x01,
+	HFP_CODEC_MSBC = 0x02,
+};
+
 typedef void (*hfp_slc_cb_t)(void *userdata);
 
 struct hfp_slc_info {
@@ -53,9 +58,12 @@ struct hfp_slc_info {
 	unsigned int hf_features;
 	unsigned char cind_pos[HFP_INDICATOR_LAST];
 	unsigned int cind_val[HFP_INDICATOR_LAST];
+	unsigned char *codecs;
+	int codecs_len;
 };
 
-void hfp_slc_info_init(struct hfp_slc_info *info, guint16 version);
+void hfp_slc_info_init(struct hfp_slc_info *info, guint16 version,
+					unsigned char *codecs, int len);
 void hfp_slc_info_free(struct hfp_slc_info *info);
 
 void hfp_slc_establish(struct hfp_slc_info *info, hfp_slc_cb_t connect_cb,
diff --git a/plugins/hfp_hf_bluez4.c b/plugins/hfp_hf_bluez4.c
index 450c183..f27b0e1 100644
--- a/plugins/hfp_hf_bluez4.c
+++ b/plugins/hfp_hf_bluez4.c
@@ -165,12 +165,15 @@ static DBusMessage *hfp_agent_new_connection(DBusConnection *conn,
 	struct ofono_modem *modem = data;
 	struct hfp_data *hfp_data = ofono_modem_get_data(modem);
 	guint16 version;
+	unsigned char codecs[1];
 
 	if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_UNIX_FD, &fd,
 				DBUS_TYPE_UINT16, &version, DBUS_TYPE_INVALID))
 		return __ofono_error_invalid_args(msg);
 
-	hfp_slc_info_init(&hfp_data->info, version);
+	memset(&codecs, 0, sizeof(codecs));
+	codecs[0] = HFP_CODEC_CVSD;
+	hfp_slc_info_init(&hfp_data->info, version, codecs, 1);
 
 	err = service_level_connection(modem, fd);
 	if (err < 0 && err != -EINPROGRESS)
diff --git a/plugins/phonesim.c b/plugins/phonesim.c
index 26f96d0..63590a9 100644
--- a/plugins/phonesim.c
+++ b/plugins/phonesim.c
@@ -885,8 +885,7 @@ static void slc_failed(gpointer userdata)
 
 	ofono_modem_set_powered(modem, FALSE);
 
-	g_at_chat_unref(info->chat);
-	info->chat = NULL;
+	hfp_slc_info_free(info);
 }
 
 static int localhfp_enable(struct ofono_modem *modem)
@@ -896,6 +895,7 @@ static int localhfp_enable(struct ofono_modem *modem)
 	GAtSyntax *syntax;
 	GAtChat *chat;
 	const char *address;
+	unsigned char codecs[1];
 	int sk, port;
 
 	address = ofono_modem_get_string(modem, "Address");
@@ -929,7 +929,10 @@ static int localhfp_enable(struct ofono_modem *modem)
 
 	g_at_chat_set_disconnect_function(chat, slc_failed, modem);
 
-	hfp_slc_info_init(info, HFP_VERSION_LATEST);
+	memset(codecs, 0, sizeof(codecs));
+	codecs[0] = HFP_CODEC_CVSD;
+
+	hfp_slc_info_init(info, HFP_VERSION_LATEST, codecs, 1);
 	info->chat = chat;
 	hfp_slc_establish(info, slc_established, slc_failed, modem);
 
@@ -940,8 +943,7 @@ static int localhfp_disable(struct ofono_modem *modem)
 {
 	struct hfp_slc_info *info = ofono_modem_get_data(modem);
 
-	g_at_chat_unref(info->chat);
-	info->chat = NULL;
+	hfp_slc_info_free(info);
 
 	return 0;
 }
-- 
1.7.11.7

Re: [PATCH v0 4/8] hfpmodem: Add support for storing the supported codecs

[Denis Kenzior]

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

Hi Claudio,

On 01/17/2013 09:13 AM, Claudio Takahasi wrote:
> From: Vinicius Costa Gomes<vinicius.gomes@openbossa.org>
>
> As informing the AG of the supported codecs is part of the setup
> of the SLC, it makes sense to have this information inside the SLC
> establishment part.
>
> Now the hfp_slc_info structure has dynamic information, and so the
> hfp_slc_info_free() function is used.
> ---
>   drivers/hfpmodem/slc.c  | 14 +++++++++++++-
>   drivers/hfpmodem/slc.h  | 10 +++++++++-
>   plugins/hfp_hf_bluez4.c |  5 ++++-
>   plugins/phonesim.c      | 12 +++++++-----
>   4 files changed, 33 insertions(+), 8 deletions(-)

This patch needs to be broken up.  First it needs to implement the 
hfp_slc_info_free function, then update all existing plugins to use 
that.  Then introduce hfp_slc_info_init changes and whatever else.

>
> diff --git a/drivers/hfpmodem/slc.c b/drivers/hfpmodem/slc.c
> index 646befa..288afdd 100644
> --- a/drivers/hfpmodem/slc.c
> +++ b/drivers/hfpmodem/slc.c
> @@ -52,7 +52,8 @@ struct slc_establish_data {
>   	gpointer userdata;
>   };
>
> -void hfp_slc_info_init(struct hfp_slc_info *info, guint16 version)
> +void hfp_slc_info_init(struct hfp_slc_info *info, guint16 version,
> +						unsigned char *codecs, int len)
>   {
>   	info->ag_features = 0;
>   	info->ag_mpty_features = 0;
> @@ -72,11 +73,22 @@ void hfp_slc_info_init(struct hfp_slc_info *info, guint16 version)
>
>   	info->hf_features |= HFP_HF_FEATURE_CODEC_NEGOTIATION;
>
> +	info->codecs = g_memdup(codecs, len);
> +	info->codecs_len = len;
> +
>   done:
>   	memset(info->cind_val, 0, sizeof(info->cind_val));
>   	memset(info->cind_pos, 0, sizeof(info->cind_pos));
>   }
>
> +void hfp_slc_info_free(struct hfp_slc_info *info)
> +{
> +	g_free(info->codecs);
> +
> +	g_at_chat_unref(info->chat);
> +	info->chat = NULL;
> +}
> +
>   static void slc_establish_data_unref(gpointer userdata)
>   {
>   	struct slc_establish_data *sed = userdata;
> diff --git a/drivers/hfpmodem/slc.h b/drivers/hfpmodem/slc.h
> index b71ffe9..d7c90a2 100644
> --- a/drivers/hfpmodem/slc.h
> +++ b/drivers/hfpmodem/slc.h
> @@ -44,6 +44,11 @@ enum hfp_indicator {
>   	HFP_INDICATOR_LAST
>   };
>
> +enum hfp_codec {
> +	HFP_CODEC_CVSD = 0x01,
> +	HFP_CODEC_MSBC = 0x02,
> +};
> +
>   typedef void (*hfp_slc_cb_t)(void *userdata);
>
>   struct hfp_slc_info {
> @@ -53,9 +58,12 @@ struct hfp_slc_info {
>   	unsigned int hf_features;
>   	unsigned char cind_pos[HFP_INDICATOR_LAST];
>   	unsigned int cind_val[HFP_INDICATOR_LAST];
> +	unsigned char *codecs;
> +	int codecs_len;
>   };
>
> -void hfp_slc_info_init(struct hfp_slc_info *info, guint16 version);
> +void hfp_slc_info_init(struct hfp_slc_info *info, guint16 version,
> +					unsigned char *codecs, int len);
>   void hfp_slc_info_free(struct hfp_slc_info *info);
>
>   void hfp_slc_establish(struct hfp_slc_info *info, hfp_slc_cb_t connect_cb,
> diff --git a/plugins/hfp_hf_bluez4.c b/plugins/hfp_hf_bluez4.c
> index 450c183..f27b0e1 100644
> --- a/plugins/hfp_hf_bluez4.c
> +++ b/plugins/hfp_hf_bluez4.c
> @@ -165,12 +165,15 @@ static DBusMessage *hfp_agent_new_connection(DBusConnection *conn,
>   	struct ofono_modem *modem = data;
>   	struct hfp_data *hfp_data = ofono_modem_get_data(modem);
>   	guint16 version;
> +	unsigned char codecs[1];
>
>   	if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_UNIX_FD,&fd,
>   				DBUS_TYPE_UINT16,&version, DBUS_TYPE_INVALID))
>   		return __ofono_error_invalid_args(msg);
>
> -	hfp_slc_info_init(&hfp_data->info, version);
> +	memset(&codecs, 0, sizeof(codecs));
> +	codecs[0] = HFP_CODEC_CVSD;
> +	hfp_slc_info_init(&hfp_data->info, version, codecs, 1);

Why do you need this info for HFP 1.5?

>
>   	err = service_level_connection(modem, fd);
>   	if (err<  0&&  err != -EINPROGRESS)
> diff --git a/plugins/phonesim.c b/plugins/phonesim.c
> index 26f96d0..63590a9 100644
> --- a/plugins/phonesim.c
> +++ b/plugins/phonesim.c
> @@ -885,8 +885,7 @@ static void slc_failed(gpointer userdata)
>
>   	ofono_modem_set_powered(modem, FALSE);
>
> -	g_at_chat_unref(info->chat);
> -	info->chat = NULL;
> +	hfp_slc_info_free(info);
>   }
>
>   static int localhfp_enable(struct ofono_modem *modem)
> @@ -896,6 +895,7 @@ static int localhfp_enable(struct ofono_modem *modem)
>   	GAtSyntax *syntax;
>   	GAtChat *chat;
>   	const char *address;
> +	unsigned char codecs[1];
>   	int sk, port;
>
>   	address = ofono_modem_get_string(modem, "Address");
> @@ -929,7 +929,10 @@ static int localhfp_enable(struct ofono_modem *modem)
>
>   	g_at_chat_set_disconnect_function(chat, slc_failed, modem);
>
> -	hfp_slc_info_init(info, HFP_VERSION_LATEST);
> +	memset(codecs, 0, sizeof(codecs));
> +	codecs[0] = HFP_CODEC_CVSD;
> +
> +	hfp_slc_info_init(info, HFP_VERSION_LATEST, codecs, 1);
>   	info->chat = chat;
>   	hfp_slc_establish(info, slc_established, slc_failed, modem);
>
> @@ -940,8 +943,7 @@ static int localhfp_disable(struct ofono_modem *modem)
>   {
>   	struct hfp_slc_info *info = ofono_modem_get_data(modem);
>
> -	g_at_chat_unref(info->chat);
> -	info->chat = NULL;
> +	hfp_slc_info_free(info);
>
>   	return 0;
>   }

Regards,
-Denis

Re: [PATCH v0 4/8] hfpmodem: Add support for storing the supported codecs

[Vinicius Costa Gomes]

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

Hi Denis,

On 11:29 Thu 17 Jan, Denis Kenzior wrote:
> Hi Claudio,
> 
> On 01/17/2013 09:13 AM, Claudio Takahasi wrote:
> >From: Vinicius Costa Gomes<vinicius.gomes@openbossa.org>
> >
> >As informing the AG of the supported codecs is part of the setup
> >of the SLC, it makes sense to have this information inside the SLC
> >establishment part.
> >
> >Now the hfp_slc_info structure has dynamic information, and so the
> >hfp_slc_info_free() function is used.
> >---
> >  drivers/hfpmodem/slc.c  | 14 +++++++++++++-
> >  drivers/hfpmodem/slc.h  | 10 +++++++++-
> >  plugins/hfp_hf_bluez4.c |  5 ++++-
> >  plugins/phonesim.c      | 12 +++++++-----
> >  4 files changed, 33 insertions(+), 8 deletions(-)
> 
> This patch needs to be broken up.  First it needs to implement the
> hfp_slc_info_free function, then update all existing plugins to use
> that.  Then introduce hfp_slc_info_init changes and whatever else.

Ok. Sure.

[snip]

> 
> >diff --git a/plugins/hfp_hf_bluez4.c b/plugins/hfp_hf_bluez4.c
> >index 450c183..f27b0e1 100644
> >--- a/plugins/hfp_hf_bluez4.c
> >+++ b/plugins/hfp_hf_bluez4.c
> >@@ -165,12 +165,15 @@ static DBusMessage *hfp_agent_new_connection(DBusConnection *conn,
> >  	struct ofono_modem *modem = data;
> >  	struct hfp_data *hfp_data = ofono_modem_get_data(modem);
> >  	guint16 version;
> >+	unsigned char codecs[1];
> >
> >  	if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_UNIX_FD,&fd,
> >  				DBUS_TYPE_UINT16,&version, DBUS_TYPE_INVALID))
> >  		return __ofono_error_invalid_args(msg);
> >
> >-	hfp_slc_info_init(&hfp_data->info, version);
> >+	memset(&codecs, 0, sizeof(codecs));
> >+	codecs[0] = HFP_CODEC_CVSD;
> >+	hfp_slc_info_init(&hfp_data->info, version, codecs, 1);
> 
> Why do you need this info for HFP 1.5?

You are right, I don't need it. But being pedantic, it is more because
in the BlueZ 4 case it won't support codec negotiation than because of
the version.

And that reminds me that in the case that I only have one codec (CVSD),
I am sending the supported codecs needlessly. Will fix it as well.


Cheers,
-- 
Vinicius

[PATCH v0 5/8] hfpmodem: Send the AT+BAC command with the supported codecs

[Claudio Takahasi]

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

From: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>

Before, the AT+BAC command was being sent with fixed information,
now we send the command (that inform the AG of the codecs supported by
the HF) with the codecs that come from the profile implementation.
---
 drivers/hfpmodem/slc.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/hfpmodem/slc.c b/drivers/hfpmodem/slc.c
index 288afdd..e9427a9 100644
--- a/drivers/hfpmodem/slc.c
+++ b/drivers/hfpmodem/slc.c
@@ -316,10 +316,21 @@ static void brsf_cb(gboolean ok, GAtResult *result, gpointer user_data)
 
 	if (info->ag_features & HFP_AG_FEATURE_CODEC_NEGOTIATION &&
 			info->hf_features & HFP_HF_FEATURE_CODEC_NEGOTIATION) {
+		GString *str = g_string_new("AT+BAC=");
+		int i;
+
+		for (i = 0; i < info->codecs_len; i++) {
+			g_string_append_printf(str, "%d", info->codecs[i]);
+			if (i + 1 < info->codecs_len)
+				str = g_string_append(str, ",");
+		}
 
 		slc_establish_data_ref(sed);
-		g_at_chat_send(info->chat, "AT+BAC=1", NULL, bac_cb, sed,
+		g_at_chat_send(info->chat, str->str, NULL, bac_cb, sed,
 						slc_establish_data_unref);
+
+		g_string_free(str, TRUE);
+
 		return;
 	}
 
-- 
1.7.11.7

[PATCH v0 6/8] hfp_hf: Register the HFP modem

[Claudio Takahasi]

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

This patch registers the HFP modem when Bluetooth link is established.
---
 plugins/hfp_hf_bluez5.c | 105 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 104 insertions(+), 1 deletion(-)

diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
index 636b1b3..f4c59d7 100644
--- a/plugins/hfp_hf_bluez5.c
+++ b/plugins/hfp_hf_bluez5.c
@@ -24,15 +24,26 @@
 #endif
 
 #include <errno.h>
+#include <string.h>
+
 #include <glib.h>
 
 #include <gdbus.h>
+#include <gatchat.h>
+#include <gattty.h>
 
 #define OFONO_API_SUBJECT_TO_CHANGE
 #include <ofono/modem.h>
 #include <ofono/dbus.h>
 #include <ofono/plugin.h>
 #include <ofono/log.h>
+#include <ofono/devinfo.h>
+#include <ofono/netreg.h>
+#include <ofono/voicecall.h>
+#include <ofono/call-volume.h>
+#include <ofono/handsfree.h>
+
+#include <drivers/hfpmodem/slc.h>
 
 #include "bluez5.h"
 #include "media.h"
@@ -43,6 +54,62 @@
 
 #define HFP_EXT_PROFILE_PATH   "/bluetooth/profile/hfp_hf"
 
+struct hfp {
+	char *device_address;
+	char *device_path;
+	struct hfp_slc_info info;
+	DBusMessage *msg;
+	GSList *endpoints;
+};
+
+static GHashTable *modem_hash = NULL;
+
+static void hfp_free(gpointer user_data)
+{
+	struct hfp *hfp = user_data;
+
+	if (hfp->msg)
+		dbus_message_unref(hfp->msg);
+
+	g_slist_free_full(hfp->endpoints, media_endpoint_free);
+	g_free(hfp->device_address);
+	g_free(hfp->device_path);
+	g_free(hfp);
+}
+
+static void modem_data_free(gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct hfp *hfp = ofono_modem_get_data(modem);
+
+	hfp_free(hfp);
+}
+
+static int modem_register(struct hfp *hfp, const char *alias, int fd,
+							guint16 version)
+{
+	struct ofono_modem *modem;
+	int err = 0;
+	char *path;
+
+	path = g_strconcat("hfp", hfp->device_path, NULL);
+
+	modem = ofono_modem_create(path, "hfp");
+
+	g_free(path);
+
+	if (modem == NULL)
+		return -ENOMEM;
+
+	ofono_modem_set_data(modem, hfp);
+	ofono_modem_set_name(modem, alias);
+	ofono_modem_register(modem);
+
+	g_hash_table_insert(modem_hash, g_strdup(hfp->device_path), modem);
+
+	return err;
+}
+
 static void parse_guint16(DBusMessageIter *iter, gpointer user_data)
 {
 	guint16 *value = user_data;
@@ -160,6 +227,11 @@ static int hfp_enable(struct ofono_modem *modem)
 {
 	DBG("%p", modem);
 
+	if (ofono_modem_get_powered(modem))
+		return 0;
+
+	ofono_modem_set_powered(modem, TRUE);
+
 	return 0;
 }
 
@@ -167,12 +239,22 @@ static int hfp_disable(struct ofono_modem *modem)
 {
 	DBG("%p", modem);
 
+	ofono_modem_set_powered(modem, FALSE);
+
 	return 0;
 }
 
 static void hfp_pre_sim(struct ofono_modem *modem)
 {
+	struct hfp *hfp = ofono_modem_get_data(modem);
+
 	DBG("%p", modem);
+
+	ofono_devinfo_create(modem, 0, "hfpmodem", hfp->device_address);
+	ofono_voicecall_create(modem, 0, "hfpmodem", &hfp->info);
+	ofono_netreg_create(modem, 0, "hfpmodem", &hfp->info);
+	ofono_call_volume_create(modem, 0, "hfpmodem", &hfp->info);
+	ofono_handsfree_create(modem, 0, "hfpmodem", &hfp->info);
 }
 
 static void hfp_post_sim(struct ofono_modem *modem)
@@ -194,11 +276,12 @@ static struct ofono_modem_driver hfp_driver = {
 static DBusMessage *profile_new_connection(DBusConnection *conn,
 					DBusMessage *msg, void *user_data)
 {
+	struct hfp *hfp;
 	DBusMessageIter entry;
 	const char *device;
 	GSList *endpoints = NULL;
 	guint16 version, features;
-	int fd;
+	int fd, err;
 
 	DBG("Profile handler NewConnection");
 
@@ -231,6 +314,21 @@ static DBusMessage *profile_new_connection(DBusConnection *conn,
 		ofono_error("Media Endpoint missing");
 		goto error;
 	}
+
+	hfp = g_new0(struct hfp, 1);
+	hfp->device_address = g_strdup("unknown");
+	hfp->device_path = g_strdup(device);
+	hfp->endpoints = endpoints;
+	hfp->msg = dbus_message_ref(msg);
+
+	err = modem_register(hfp, "unknown", fd, version);
+	if (err < 0 && err != -EINPROGRESS) {
+		hfp_free(hfp);
+		return g_dbus_create_error(msg,
+				BLUEZ_ERROR_INTERFACE ".Rejected",
+				"%s", strerror(-err));
+	}
+
 	return NULL;
 
 error:
@@ -315,6 +413,9 @@ static int hfp_init(void)
 		return err;
 	}
 
+	modem_hash = g_hash_table_new_full(g_str_hash, g_str_equal, g_free,
+								modem_data_free);
+
 	return 0;
 }
 
@@ -326,6 +427,8 @@ static void hfp_exit(void)
 	g_dbus_unregister_interface(conn, HFP_EXT_PROFILE_PATH,
 						BLUEZ_PROFILE_INTERFACE);
 	ofono_modem_driver_unregister(&hfp_driver);
+
+	g_hash_table_destroy(modem_hash);
 }
 
 OFONO_PLUGIN_DEFINE(hfp_bluez5, "External Hands-Free Profile Plugin", VERSION,
-- 
1.7.11.7

[PATCH v0 7/8] hfp_hf: Add service level negotiation

[Claudio Takahasi]

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

This patch adds service level negotiation and callbacks to handle success
and failure.
---
 plugins/hfp_hf_bluez5.c | 113 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 112 insertions(+), 1 deletion(-)

diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
index f4c59d7..b117a1b 100644
--- a/plugins/hfp_hf_bluez5.c
+++ b/plugins/hfp_hf_bluez5.c
@@ -24,6 +24,7 @@
 #endif
 
 #include <errno.h>
+#include <stdlib.h>
 #include <string.h>
 
 #include <glib.h>
@@ -59,6 +60,7 @@ struct hfp {
 	char *device_path;
 	struct hfp_slc_info info;
 	DBusMessage *msg;
+	GIOChannel *slcio;
 	GSList *endpoints;
 };
 
@@ -71,6 +73,9 @@ static void hfp_free(gpointer user_data)
 	if (hfp->msg)
 		dbus_message_unref(hfp->msg);
 
+	if (hfp->slcio)
+		g_io_channel_unref(hfp->slcio);
+
 	g_slist_free_full(hfp->endpoints, media_endpoint_free);
 	g_free(hfp->device_address);
 	g_free(hfp->device_path);
@@ -85,12 +90,111 @@ static void modem_data_free(gpointer user_data)
 	hfp_free(hfp);
 }
 
+static void hfp_debug(const char *str, void *user_data)
+{
+	const char *prefix = user_data;
+
+	ofono_info("%s%s", prefix, str);
+}
+
+static void slc_established(gpointer userdata)
+{
+	struct ofono_modem *modem = userdata;
+	struct hfp *hfp = ofono_modem_get_data(modem);
+	DBusMessage *msg;
+
+	ofono_modem_set_powered(modem, TRUE);
+
+	msg = dbus_message_new_method_return(hfp->msg);
+	g_dbus_send_message(ofono_dbus_get_connection(), msg);
+	dbus_message_unref(hfp->msg);
+	hfp->msg = NULL;
+
+	ofono_info("Service level connection established");
+}
+
+static void slc_failed(gpointer userdata)
+{
+	struct ofono_modem *modem = userdata;
+	struct hfp *hfp = ofono_modem_get_data(modem);
+	DBusMessage *msg;
+
+	msg = g_dbus_create_error(hfp->msg, BLUEZ_ERROR_INTERFACE
+						".Failed",
+						"HFP Handshake failed");
+
+	g_dbus_send_message(ofono_dbus_get_connection(), msg);
+	dbus_message_unref(hfp->msg);
+	hfp->msg = NULL;
+
+	ofono_error("Service level connection failed");
+	ofono_modem_set_powered(modem, FALSE);
+
+	hfp_slc_info_free(&hfp->info);
+}
+
+static void hfp_disconnected_cb(gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct hfp *hfp = ofono_modem_get_data(modem);
+
+	DBG("HFP disconnected");
+
+	hfp_slc_info_free(&hfp->info);
+
+	ofono_modem_set_powered(modem, FALSE);
+}
+
+static GIOChannel *service_level_connection(struct ofono_modem *modem,
+							int fd, int *err)
+{
+	struct hfp *hfp = ofono_modem_get_data(modem);
+	GIOChannel *io;
+	GAtSyntax *syntax;
+	GAtChat *chat;
+
+	io = g_io_channel_unix_new(fd);
+	if (io == NULL) {
+		ofono_error("Service level connection failed: %s (%d)",
+						strerror(errno), errno);
+		*err = -EIO;
+		return NULL;
+	}
+
+	syntax = g_at_syntax_new_gsm_permissive();
+	chat = g_at_chat_new(io, syntax);
+	g_at_syntax_unref(syntax);
+	g_io_channel_set_close_on_unref(io, TRUE);
+
+	if (chat == NULL) {
+		*err = -ENOMEM;
+		goto fail;
+	}
+
+	g_at_chat_set_disconnect_function(chat, hfp_disconnected_cb, modem);
+
+	if (getenv("OFONO_AT_DEBUG"))
+		g_at_chat_set_debug(chat, hfp_debug, "");
+
+	hfp->info.chat = chat;
+	hfp_slc_establish(&hfp->info, slc_established, slc_failed, modem);
+
+	*err = -EINPROGRESS;
+
+	return io;
+
+fail:
+	g_io_channel_unref(io);
+	return NULL;
+}
+
 static int modem_register(struct hfp *hfp, const char *alias, int fd,
 							guint16 version)
 {
 	struct ofono_modem *modem;
-	int err = 0;
+	guint8 codecs[1];
 	char *path;
+	int err;
 
 	path = g_strconcat("hfp", hfp->device_path, NULL);
 
@@ -107,6 +211,13 @@ static int modem_register(struct hfp *hfp, const char *alias, int fd,
 
 	g_hash_table_insert(modem_hash, g_strdup(hfp->device_path), modem);
 
+	memset(codecs, 0, sizeof(codecs));
+	codecs[0] = HFP_CODEC_CVSD;
+
+	hfp_slc_info_init(&hfp->info, version, codecs, 1);
+
+	hfp->slcio = service_level_connection(modem, fd, &err);
+
 	return err;
 }
 
-- 
1.7.11.7

[PATCH v0 8/8] hfp_hf: Add support for codec negotiation

[Claudio Takahasi]

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

From: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>

The negotiation is very simple, if the codec the AG requests is present
in the supported codec list that the HF sent, the codec is accepted, if
not, we send the list of supported codecs again, and expect that the AG
re-sends the request with an acceptable codec.
---
 plugins/hfp_hf_bluez5.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
index b117a1b..3155d6f 100644
--- a/plugins/hfp_hf_bluez5.c
+++ b/plugins/hfp_hf_bluez5.c
@@ -25,6 +25,7 @@
 
 #include <errno.h>
 #include <stdlib.h>
+#include <stdio.h>
 #include <string.h>
 
 #include <glib.h>
@@ -59,6 +60,7 @@ struct hfp {
 	char *device_address;
 	char *device_path;
 	struct hfp_slc_info info;
+	guint8 current_codec;
 	DBusMessage *msg;
 	GIOChannel *slcio;
 	GSList *endpoints;
@@ -97,12 +99,59 @@ static void hfp_debug(const char *str, void *user_data)
 	ofono_info("%s%s", prefix, str);
 }
 
+static void bcs_notify(GAtResult *result, gpointer user_data)
+{
+	struct hfp *hfp = user_data;
+	struct hfp_slc_info *info = &hfp->info;
+	GAtResultIter iter;
+	GString *str;
+	int i, value;
+
+	g_at_result_iter_init(&iter, result);
+
+	if (!g_at_result_iter_next(&iter, "+BCS:"))
+		return;
+
+	if (!g_at_result_iter_next_number(&iter, &value))
+		return;
+
+	/* If some codec matches, we confirm it */
+	for (i = 0; i < info->codecs_len; i++) {
+		if (info->codecs[i] == value) {
+			char buf[32];
+
+			hfp->current_codec = value;
+			DBG("Negotiated HFP codec: %d", value);
+
+			snprintf(buf, sizeof(buf), "AT+BCS=%d", value);
+			g_at_chat_send(info->chat, buf, NULL, NULL,
+							NULL, NULL);
+			return;
+		}
+	}
+
+	/* If none matches, we send our supported codecs again */
+	str = g_string_new("AT+BAC=");
+
+	for (i = 0; i < info->codecs_len; i++) {
+		g_string_append_printf(str, "%d", info->codecs[i]);
+		if (i + 1 < info->codecs_len)
+			str = g_string_append(str, ",");
+	}
+
+	g_at_chat_send(info->chat, str->str, NULL, NULL, NULL, NULL);
+	g_string_free(str, TRUE);
+}
+
 static void slc_established(gpointer userdata)
 {
 	struct ofono_modem *modem = userdata;
 	struct hfp *hfp = ofono_modem_get_data(modem);
+	struct hfp_slc_info *info = &hfp->info;
 	DBusMessage *msg;
 
+	g_at_chat_register(info->chat, "+BCS:", bcs_notify, FALSE, hfp, NULL);
+
 	ofono_modem_set_powered(modem, TRUE);
 
 	msg = dbus_message_new_method_return(hfp->msg);
@@ -431,6 +480,7 @@ static DBusMessage *profile_new_connection(DBusConnection *conn,
 	hfp->device_path = g_strdup(device);
 	hfp->endpoints = endpoints;
 	hfp->msg = dbus_message_ref(msg);
+	hfp->current_codec = HFP_CODEC_CVSD;
 
 	err = modem_register(hfp, "unknown", fd, version);
 	if (err < 0 && err != -EINPROGRESS) {
-- 
1.7.11.7

[PATCH v1 00/10] HFP HF: Service Level/Codec negotiation

[Claudio Takahasi]

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

Second group of patches related to HFP external profile.

The following patches contain the HFP 1.6 codec negotiation.

Changed between v0-v1:
* deleted media.[ch], functions moved to bluez5.[ch]
* replaced 'bluetooth_' prefix by 'bt_' for BlueZ 5 functions
* splitted HFP SLC patches
* new oFono D-Bus utility function to parse dictionaries

Claudio Takahasi (2):
  hfp_hf: Register the HFP modem
  hfp_hf: Add service level negotiation

Vinicius Costa Gomes (8):
  dbus: Add ofono_dbus_iter_parse_properties()
  bluez5: Rename register/unregister profile
  hfp_hf: Add NewConnection arguments parsing
  hfpmodem: Add support for storing the supported codecs
  hfpmodem: Implement hfp_slc_info_free
  phonesim: Use hfp_slc_info_free()
  hfpmodem: Send the AT+BAC command with the supported codecs
  hfp_hf: Add support for codec negotiation

 drivers/hfpmodem/slc.c  |  29 +++-
 drivers/hfpmodem/slc.h  |  10 +-
 include/dbus.h          |   6 +
 plugins/bluez5.c        |  39 ++++-
 plugins/bluez5.h        |  12 +-
 plugins/hfp_hf_bluez4.c |   2 +-
 plugins/hfp_hf_bluez5.c | 417 +++++++++++++++++++++++++++++++++++++++++++++++-
 plugins/phonesim.c      |  12 +-
 src/dbus.c              |  90 +++++++++++
 9 files changed, 598 insertions(+), 19 deletions(-)

-- 
1.7.11.7

[PATCH v1 01/10] dbus: Add ofono_dbus_iter_parse_properties()

[Claudio Takahasi]

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

From: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>

With the move of BlueZ to DBus ObjectManager and the ProfileManager1
interface, the case that ofono receives a message with a properties
dictionary should be more common, so we add a function to help deal
with it.
---
 include/dbus.h |  6 ++++
 src/dbus.c     | 90 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 96 insertions(+)

diff --git a/include/dbus.h b/include/dbus.h
index 44faa7f..3901a96 100644
--- a/include/dbus.h
+++ b/include/dbus.h
@@ -100,6 +100,12 @@ int ofono_dbus_signal_dict_property_changed(DBusConnection *conn,
 						const char *name, int type,
 						void *value);
 
+typedef void (*property_handler_cb)(DBusMessageIter *iter, void *user_data);
+
+void ofono_dbus_iter_parse_properties(DBusMessageIter *array,
+				const char *property, property_handler_cb cb,
+				void *user_data, ...);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/src/dbus.c b/src/dbus.c
index 1558a51..1afbb44 100644
--- a/src/dbus.c
+++ b/src/dbus.c
@@ -30,6 +30,14 @@
 
 #define OFONO_ERROR_INTERFACE "org.ofono.Error"
 
+typedef void (*property_handler_cb)(DBusMessageIter *iter, void *user_data);
+
+struct property_handler {
+	const char *property;
+	property_handler_cb callback;
+	void *user_data;
+};
+
 static DBusConnection *g_connection;
 
 struct error_mapping_entry {
@@ -273,6 +281,88 @@ int ofono_dbus_signal_dict_property_changed(DBusConnection *conn,
 	return g_dbus_send_message(conn, signal);
 }
 
+static gint property_handler_compare(gconstpointer a, gconstpointer b)
+{
+	const struct property_handler *handler = a;
+	const char *property = b;
+
+	return g_strcmp0(handler->property, property);
+}
+
+void ofono_dbus_iter_parse_properties(DBusMessageIter *array,
+				const char *property, property_handler_cb cb,
+				void *user_data, ...)
+{
+	va_list args;
+	GSList *prop_handlers = NULL;
+	DBusMessageIter dict;
+
+	if (dbus_message_iter_get_arg_type(array) != DBUS_TYPE_ARRAY)
+		goto done;
+
+	va_start(args, user_data);
+
+	if (property == NULL)
+		return;
+
+	while (1) {
+		struct property_handler *handler =
+					g_new0(struct property_handler, 1);
+
+		handler->property = property;
+		handler->callback = cb;
+		handler->user_data = user_data;
+
+		prop_handlers = g_slist_prepend(prop_handlers, handler);
+
+		property = va_arg(args, const char *);
+		if (property == NULL)
+			break;
+
+		cb = va_arg(args, property_handler_cb);
+		user_data = va_arg(args, void *);
+	}
+
+	va_end(args);
+
+	dbus_message_iter_recurse(array, &dict);
+
+	while (dbus_message_iter_get_arg_type(&dict) == DBUS_TYPE_DICT_ENTRY) {
+		DBusMessageIter entry, value;
+		const char *key;
+		GSList *l;
+
+		dbus_message_iter_recurse(&dict, &entry);
+
+		if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_STRING)
+			goto done;
+
+		dbus_message_iter_get_basic(&entry, &key);
+
+		dbus_message_iter_next(&entry);
+
+		if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_VARIANT)
+			goto done;
+
+		dbus_message_iter_recurse(&entry, &value);
+
+		l = g_slist_find_custom(prop_handlers, key,
+					property_handler_compare);
+
+		if (l) {
+			struct property_handler *handler = l->data;
+
+			handler->callback(&value, handler->user_data);
+		}
+
+		dbus_message_iter_next(&dict);
+	}
+
+done:
+	g_slist_foreach(prop_handlers, (GFunc) g_free, NULL);
+	g_slist_free(prop_handlers);
+}
+
 DBusMessage *__ofono_error_invalid_args(DBusMessage *msg)
 {
 	return g_dbus_create_error(msg, OFONO_ERROR_INTERFACE
-- 
1.7.11.7

[PATCH v1 02/10] bluez5: Rename register/unregister profile

[Claudio Takahasi]

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

From: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>

Using 'bt_' prefix will be more suitable for the upcoming
functions, and structure names avoiding long names and improving
code readability.
---
 plugins/bluez5.c        | 4 ++--
 plugins/bluez5.h        | 4 ++--
 plugins/hfp_hf_bluez5.c | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/plugins/bluez5.c b/plugins/bluez5.c
index 84ba47d..589658a 100644
--- a/plugins/bluez5.c
+++ b/plugins/bluez5.c
@@ -80,7 +80,7 @@ done:
 	dbus_message_unref(reply);
 }
 
-int bluetooth_register_profile(DBusConnection *conn, const char *uuid,
+int bt_register_profile(DBusConnection *conn, const char *uuid,
 					const char *name, const char *object)
 {
 	DBusMessageIter iter, dict;
@@ -115,7 +115,7 @@ int bluetooth_register_profile(DBusConnection *conn, const char *uuid,
 	return 0;
 }
 
-void bluetooth_unregister_profile(DBusConnection *conn, const char *object)
+void bt_unregister_profile(DBusConnection *conn, const char *object)
 {
 	DBusMessageIter iter;
 	DBusPendingCall *c;
diff --git a/plugins/bluez5.h b/plugins/bluez5.h
index 01ecfe8..3088c7d 100644
--- a/plugins/bluez5.h
+++ b/plugins/bluez5.h
@@ -25,7 +25,7 @@
 
 #define HFP_HS_UUID	"0000111e-0000-1000-8000-00805f9b34fb"
 
-int bluetooth_register_profile(DBusConnection *conn, const char *uuid,
+int bt_register_profile(DBusConnection *conn, const char *uuid,
 					const char *name, const char *object);
 
-void bluetooth_unregister_profile(DBusConnection *conn, const char *object);
+void bt_unregister_profile(DBusConnection *conn, const char *object);
diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
index e024838..f1f8891 100644
--- a/plugins/hfp_hf_bluez5.c
+++ b/plugins/hfp_hf_bluez5.c
@@ -168,7 +168,7 @@ static int hfp_init(void)
 		return err;
 	}
 
-	err = bluetooth_register_profile(conn, HFP_HS_UUID, "hfp_hf",
+	err = bt_register_profile(conn, HFP_HS_UUID, "hfp_hf",
 						HFP_EXT_PROFILE_PATH);
 	if (err < 0) {
 		g_dbus_unregister_interface(conn, HFP_EXT_PROFILE_PATH,
@@ -184,7 +184,7 @@ static void hfp_exit(void)
 {
 	DBusConnection *conn = ofono_dbus_get_connection();
 
-	bluetooth_unregister_profile(conn, HFP_EXT_PROFILE_PATH);
+	bt_unregister_profile(conn, HFP_EXT_PROFILE_PATH);
 	g_dbus_unregister_interface(conn, HFP_EXT_PROFILE_PATH,
 						BLUEZ_PROFILE_INTERFACE);
 	ofono_modem_driver_unregister(&hfp_driver);
-- 
1.7.11.7

[PATCH v1 03/10] hfp_hf: Add NewConnection arguments parsing

[Claudio Takahasi]

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

From: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>

This patch adds NewConnection message arguments parsing: features,
version and Media Endpoints included in the fd_properties dictionary.
---
 plugins/bluez5.c        |  35 ++++++++++++
 plugins/bluez5.h        |   8 +++
 plugins/hfp_hf_bluez5.c | 149 +++++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 189 insertions(+), 3 deletions(-)

diff --git a/plugins/bluez5.c b/plugins/bluez5.c
index 589658a..d753ac9 100644
--- a/plugins/bluez5.c
+++ b/plugins/bluez5.c
@@ -36,6 +36,41 @@
 
 #define BLUEZ_PROFILE_MGMT_INTERFACE   BLUEZ_SERVICE ".ProfileManager1"
 
+struct bt_endpoint {
+	char *owner;
+	char *path;
+	guint8 codec;
+	unsigned char *capa;	/* Capabilities */
+	int capa_size;		/* Capabilities size */
+};
+
+struct bt_endpoint *bt_endpoint_new(const char *owner,
+					const char *path, guint8 codec,
+					unsigned char *capa, int capa_size)
+{
+	struct bt_endpoint *endpoint;
+
+	endpoint = g_new0(struct bt_endpoint, 1);
+	endpoint->owner = g_strdup(owner);
+	endpoint->path = g_strdup(path);
+	endpoint->codec = codec;
+	if (capa && capa_size)
+		endpoint->capa = g_memdup(capa, capa_size);
+
+	return endpoint;
+}
+
+void bt_endpoint_free(gpointer data)
+{
+	struct bt_endpoint *endpoint = data;
+
+	g_free(endpoint->owner);
+	g_free(endpoint->path);
+	if (endpoint->capa)
+		g_free(endpoint->capa);
+	g_free(endpoint);
+}
+
 static void profile_register_cb(DBusPendingCall *call, gpointer user_data)
 {
 	DBusMessage *reply;
diff --git a/plugins/bluez5.h b/plugins/bluez5.h
index 3088c7d..1ab350e 100644
--- a/plugins/bluez5.h
+++ b/plugins/bluez5.h
@@ -25,6 +25,14 @@
 
 #define HFP_HS_UUID	"0000111e-0000-1000-8000-00805f9b34fb"
 
+struct bt_endpoint;
+
+struct bt_endpoint *bt_endpoint_new(const char *owner,
+					const char *path, guint8 codec,
+					unsigned char *capa, int capa_size);
+
+void bt_endpoint_free(gpointer data);
+
 int bt_register_profile(DBusConnection *conn, const char *uuid,
 					const char *name, const char *object);
 
diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
index f1f8891..ef9af62 100644
--- a/plugins/hfp_hf_bluez5.c
+++ b/plugins/hfp_hf_bluez5.c
@@ -25,6 +25,7 @@
 
 #include <errno.h>
 #include <glib.h>
+#include <string.h>
 
 #include <gdbus.h>
 
@@ -42,6 +43,111 @@
 
 #define HFP_EXT_PROFILE_PATH   "/bluetooth/profile/hfp_hf"
 
+struct capabilities {
+	unsigned char *value;
+	int size;
+};
+
+static void parse_guint16(DBusMessageIter *iter, gpointer user_data)
+{
+	guint16 *value = user_data;
+
+	if (dbus_message_iter_get_arg_type(iter) !=  DBUS_TYPE_UINT16)
+		return;
+
+	dbus_message_iter_get_basic(iter, value);
+}
+
+static void parse_string(DBusMessageIter *iter, gpointer user_data)
+{
+	char **str = user_data;
+	int arg_type = dbus_message_iter_get_arg_type(iter);
+
+	if (arg_type != DBUS_TYPE_OBJECT_PATH && arg_type != DBUS_TYPE_STRING)
+		return;
+
+	dbus_message_iter_get_basic(iter, str);
+}
+
+static void parse_byte(DBusMessageIter *iter, gpointer user_data)
+{
+	guint8 *value = user_data;
+
+	if (dbus_message_iter_get_arg_type(iter) !=  DBUS_TYPE_BYTE)
+		return;
+
+	dbus_message_iter_get_basic(iter, value);
+}
+
+static void parse_capabilities(DBusMessageIter *iter, gpointer user_data)
+{
+	DBusMessageIter array;
+	struct capabilities *capa = user_data;
+	guint8 *data = NULL;
+	int n = 0;
+
+	if (dbus_message_iter_get_arg_type(iter) !=  DBUS_TYPE_ARRAY)
+		return;
+
+	dbus_message_iter_recurse(iter, &array);
+	dbus_message_iter_get_fixed_array(&array, &data, &n);
+	if (n == 0)
+		return;
+
+	capa->value = g_memdup(data, n);
+	capa->size = n;
+}
+
+static void parse_media_endpoints(DBusMessageIter *array, gpointer user_data)
+{
+	const char *path, *owner;
+	GSList **endpoints = user_data;
+	struct capabilities capa;
+	struct bt_endpoint *endpoint;
+	guint8 codec;
+	DBusMessageIter dict, variant, entry;
+
+	if (dbus_message_iter_get_arg_type(array) != DBUS_TYPE_ARRAY)
+		return;
+
+	dbus_message_iter_recurse(array, &dict);
+
+	while (dbus_message_iter_get_arg_type(&dict) == DBUS_TYPE_DICT_ENTRY) {
+		path = NULL;
+		codec = 0x00;
+
+		dbus_message_iter_recurse(&dict, &entry);
+		if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_STRING)
+			return;
+
+		dbus_message_iter_get_basic(&entry, &owner);
+
+		dbus_message_iter_next(&entry);
+
+		if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_VARIANT)
+			return;
+
+		dbus_message_iter_recurse(&entry, &variant);
+
+		memset(&capa, 0, sizeof(capa));
+		ofono_dbus_iter_parse_properties(&variant,
+				"Path", parse_string, &path,
+				"Codec", parse_byte, &codec,
+				"Capabilities", parse_capabilities, &capa,
+				NULL);
+
+		dbus_message_iter_next(&dict);
+
+		endpoint = bt_endpoint_new(owner, path, codec,
+						capa.value, capa.size);
+
+		g_free(capa.value);
+		*endpoints = g_slist_append(*endpoints, endpoint);
+
+		DBG("Media Endpoint %s %s codec: 0x%02X", owner, path, codec);
+	}
+}
+
 static int hfp_probe(struct ofono_modem *modem)
 {
 	DBG("modem: %p", modem);
@@ -93,11 +199,48 @@ static struct ofono_modem_driver hfp_driver = {
 static DBusMessage *profile_new_connection(DBusConnection *conn,
 					DBusMessage *msg, void *user_data)
 {
+	DBusMessageIter entry;
+	const char *device;
+	GSList *endpoints = NULL;
+	guint16 version, features;
+	int fd;
+
 	DBG("Profile handler NewConnection");
 
-	return g_dbus_create_error(msg, BLUEZ_ERROR_INTERFACE
-					".NotImplemented",
-					"Implementation not provided");
+	if (dbus_message_iter_init(msg, &entry) == FALSE)
+		goto error;
+
+	if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_OBJECT_PATH)
+		goto error;
+
+	dbus_message_iter_get_basic(&entry, &device);
+	dbus_message_iter_next(&entry);
+	if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_UNIX_FD)
+		goto error;
+
+	dbus_message_iter_get_basic(&entry, &fd);
+	if (fd < 0)
+		goto error;
+
+	dbus_message_iter_next(&entry);
+
+	ofono_dbus_iter_parse_properties(&entry,
+			"Version", parse_guint16, &version,
+			"Features", parse_guint16, &features,
+			"MediaEndpoints", parse_media_endpoints, &endpoints,
+			NULL);
+
+	DBG("%s version: 0x%04x features: 0x%04x", device, version, features);
+
+	if (endpoints == NULL) {
+		ofono_error("Media Endpoint missing");
+		goto error;
+	}
+	return NULL;
+
+error:
+	return g_dbus_create_error(msg, BLUEZ_ERROR_INTERFACE ".Rejected",
+					"Invalid arguments in method call");
 }
 
 static DBusMessage *profile_release(DBusConnection *conn,
-- 
1.7.11.7

[PATCH v1 04/10] hfpmodem: Add support for storing the supported codecs

[Claudio Takahasi]

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

From: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>

As informing the AG of the supported codecs is part of the setup
of the SLC, it makes sense to have this information inside the SLC
establishment part.

In case we have only one codec, we don't inform the other side that
we support the codec negotiation feature. From the spec: "If Wide
Band Speech is supported, Codec Negotiation shall also be supported."
HFP 1.6, Section 3, page 16.
---
 drivers/hfpmodem/slc.c  |  8 ++++++--
 drivers/hfpmodem/slc.h  | 10 +++++++++-
 plugins/hfp_hf_bluez4.c |  2 +-
 plugins/phonesim.c      |  6 +++++-
 4 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/hfpmodem/slc.c b/drivers/hfpmodem/slc.c
index 646befa..06153fc 100644
--- a/drivers/hfpmodem/slc.c
+++ b/drivers/hfpmodem/slc.c
@@ -52,7 +52,8 @@ struct slc_establish_data {
 	gpointer userdata;
 };
 
-void hfp_slc_info_init(struct hfp_slc_info *info, guint16 version)
+void hfp_slc_info_init(struct hfp_slc_info *info, guint16 version,
+						unsigned char *codecs, int len)
 {
 	info->ag_features = 0;
 	info->ag_mpty_features = 0;
@@ -67,11 +68,14 @@ void hfp_slc_info_init(struct hfp_slc_info *info, guint16 version)
 	info->hf_features |= HFP_HF_FEATURE_ENHANCED_CALL_STATUS;
 	info->hf_features |= HFP_HF_FEATURE_ENHANCED_CALL_CONTROL;
 
-	if (version < HFP_VERSION_1_6)
+	if (version < HFP_VERSION_1_6 || len <= 1)
 		goto done;
 
 	info->hf_features |= HFP_HF_FEATURE_CODEC_NEGOTIATION;
 
+	info->codecs = g_memdup(codecs, len);
+	info->codecs_len = len;
+
 done:
 	memset(info->cind_val, 0, sizeof(info->cind_val));
 	memset(info->cind_pos, 0, sizeof(info->cind_pos));
diff --git a/drivers/hfpmodem/slc.h b/drivers/hfpmodem/slc.h
index b71ffe9..d7c90a2 100644
--- a/drivers/hfpmodem/slc.h
+++ b/drivers/hfpmodem/slc.h
@@ -44,6 +44,11 @@ enum hfp_indicator {
 	HFP_INDICATOR_LAST
 };
 
+enum hfp_codec {
+	HFP_CODEC_CVSD = 0x01,
+	HFP_CODEC_MSBC = 0x02,
+};
+
 typedef void (*hfp_slc_cb_t)(void *userdata);
 
 struct hfp_slc_info {
@@ -53,9 +58,12 @@ struct hfp_slc_info {
 	unsigned int hf_features;
 	unsigned char cind_pos[HFP_INDICATOR_LAST];
 	unsigned int cind_val[HFP_INDICATOR_LAST];
+	unsigned char *codecs;
+	int codecs_len;
 };
 
-void hfp_slc_info_init(struct hfp_slc_info *info, guint16 version);
+void hfp_slc_info_init(struct hfp_slc_info *info, guint16 version,
+					unsigned char *codecs, int len);
 void hfp_slc_info_free(struct hfp_slc_info *info);
 
 void hfp_slc_establish(struct hfp_slc_info *info, hfp_slc_cb_t connect_cb,
diff --git a/plugins/hfp_hf_bluez4.c b/plugins/hfp_hf_bluez4.c
index 450c183..e7fce31 100644
--- a/plugins/hfp_hf_bluez4.c
+++ b/plugins/hfp_hf_bluez4.c
@@ -170,7 +170,7 @@ static DBusMessage *hfp_agent_new_connection(DBusConnection *conn,
 				DBUS_TYPE_UINT16, &version, DBUS_TYPE_INVALID))
 		return __ofono_error_invalid_args(msg);
 
-	hfp_slc_info_init(&hfp_data->info, version);
+	hfp_slc_info_init(&hfp_data->info, version, NULL, 0);
 
 	err = service_level_connection(modem, fd);
 	if (err < 0 && err != -EINPROGRESS)
diff --git a/plugins/phonesim.c b/plugins/phonesim.c
index 26f96d0..8631039 100644
--- a/plugins/phonesim.c
+++ b/plugins/phonesim.c
@@ -896,6 +896,7 @@ static int localhfp_enable(struct ofono_modem *modem)
 	GAtSyntax *syntax;
 	GAtChat *chat;
 	const char *address;
+	unsigned char codecs[1];
 	int sk, port;
 
 	address = ofono_modem_get_string(modem, "Address");
@@ -929,7 +930,10 @@ static int localhfp_enable(struct ofono_modem *modem)
 
 	g_at_chat_set_disconnect_function(chat, slc_failed, modem);
 
-	hfp_slc_info_init(info, HFP_VERSION_LATEST);
+	memset(codecs, 0, sizeof(codecs));
+	codecs[0] = HFP_CODEC_CVSD;
+
+	hfp_slc_info_init(info, HFP_VERSION_LATEST, codecs, 1);
 	info->chat = chat;
 	hfp_slc_establish(info, slc_established, slc_failed, modem);
 
-- 
1.7.11.7

[PATCH v1 05/10] hfpmodem: Implement hfp_slc_info_free

[Claudio Takahasi]

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

From: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>

hfp_slc_info_free() was already being declared, but it was not defined
until now.
---
 drivers/hfpmodem/slc.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/hfpmodem/slc.c b/drivers/hfpmodem/slc.c
index 06153fc..f088a46 100644
--- a/drivers/hfpmodem/slc.c
+++ b/drivers/hfpmodem/slc.c
@@ -81,6 +81,14 @@ done:
 	memset(info->cind_pos, 0, sizeof(info->cind_pos));
 }
 
+void hfp_slc_info_free(struct hfp_slc_info *info)
+{
+	g_free(info->codecs);
+
+	g_at_chat_unref(info->chat);
+	info->chat = NULL;
+}
+
 static void slc_establish_data_unref(gpointer userdata)
 {
 	struct slc_establish_data *sed = userdata;
-- 
1.7.11.7

[PATCH v1 06/10] phonesim: Use hfp_slc_info_free()

[Claudio Takahasi]

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

From: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>

Now that we have a function that takes care of free'ing that structure
we should use it.
---
 plugins/phonesim.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/plugins/phonesim.c b/plugins/phonesim.c
index 8631039..63590a9 100644
--- a/plugins/phonesim.c
+++ b/plugins/phonesim.c
@@ -885,8 +885,7 @@ static void slc_failed(gpointer userdata)
 
 	ofono_modem_set_powered(modem, FALSE);
 
-	g_at_chat_unref(info->chat);
-	info->chat = NULL;
+	hfp_slc_info_free(info);
 }
 
 static int localhfp_enable(struct ofono_modem *modem)
@@ -944,8 +943,7 @@ static int localhfp_disable(struct ofono_modem *modem)
 {
 	struct hfp_slc_info *info = ofono_modem_get_data(modem);
 
-	g_at_chat_unref(info->chat);
-	info->chat = NULL;
+	hfp_slc_info_free(info);
 
 	return 0;
 }
-- 
1.7.11.7

[PATCH v1 07/10] hfpmodem: Send the AT+BAC command with the supported codecs

[Claudio Takahasi]

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

From: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>

Before, the AT+BAC command was being sent with fixed information,
now we send the command (that inform the AG of the codecs supported by
the HF) with the codecs that come from the profile implementation.
---
 drivers/hfpmodem/slc.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/hfpmodem/slc.c b/drivers/hfpmodem/slc.c
index f088a46..54766bb 100644
--- a/drivers/hfpmodem/slc.c
+++ b/drivers/hfpmodem/slc.c
@@ -316,10 +316,21 @@ static void brsf_cb(gboolean ok, GAtResult *result, gpointer user_data)
 
 	if (info->ag_features & HFP_AG_FEATURE_CODEC_NEGOTIATION &&
 			info->hf_features & HFP_HF_FEATURE_CODEC_NEGOTIATION) {
+		GString *str = g_string_new("AT+BAC=");
+		int i;
+
+		for (i = 0; i < info->codecs_len; i++) {
+			g_string_append_printf(str, "%d", info->codecs[i]);
+			if (i + 1 < info->codecs_len)
+				str = g_string_append(str, ",");
+		}
 
 		slc_establish_data_ref(sed);
-		g_at_chat_send(info->chat, "AT+BAC=1", NULL, bac_cb, sed,
+		g_at_chat_send(info->chat, str->str, NULL, bac_cb, sed,
 						slc_establish_data_unref);
+
+		g_string_free(str, TRUE);
+
 		return;
 	}
 
-- 
1.7.11.7

[PATCH v1 08/10] hfp_hf: Register the HFP modem

[Claudio Takahasi]

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

This patch registers dynamically the HFP modem when Bluetooth link is
established. The HFP modem life-time will limited to the existence of
the service level connection.
---
 plugins/hfp_hf_bluez5.c | 105 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 104 insertions(+), 1 deletion(-)

diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
index ef9af62..b3f8a1c 100644
--- a/plugins/hfp_hf_bluez5.c
+++ b/plugins/hfp_hf_bluez5.c
@@ -24,16 +24,27 @@
 #endif
 
 #include <errno.h>
+#include <string.h>
+
 #include <glib.h>
 #include <string.h>
 
 #include <gdbus.h>
+#include <gatchat.h>
+#include <gattty.h>
 
 #define OFONO_API_SUBJECT_TO_CHANGE
 #include <ofono/modem.h>
 #include <ofono/dbus.h>
 #include <ofono/plugin.h>
 #include <ofono/log.h>
+#include <ofono/devinfo.h>
+#include <ofono/netreg.h>
+#include <ofono/voicecall.h>
+#include <ofono/call-volume.h>
+#include <ofono/handsfree.h>
+
+#include <drivers/hfpmodem/slc.h>
 
 #include "bluez5.h"
 
@@ -48,6 +59,62 @@ struct capabilities {
 	int size;
 };
 
+struct hfp {
+	char *device_address;
+	char *device_path;
+	struct hfp_slc_info info;
+	DBusMessage *msg;
+	GSList *endpoints;
+};
+
+static GHashTable *modem_hash = NULL;
+
+static void hfp_free(gpointer user_data)
+{
+	struct hfp *hfp = user_data;
+
+	if (hfp->msg)
+		dbus_message_unref(hfp->msg);
+
+	g_slist_free(hfp->endpoints);
+	g_free(hfp->device_address);
+	g_free(hfp->device_path);
+	g_free(hfp);
+}
+
+static void modem_data_free(gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct hfp *hfp = ofono_modem_get_data(modem);
+
+	hfp_free(hfp);
+}
+
+static int modem_register(struct hfp *hfp, const char *alias, int fd,
+							guint16 version)
+{
+	struct ofono_modem *modem;
+	int err = 0;
+	char *path;
+
+	path = g_strconcat("hfp", hfp->device_path, NULL);
+
+	modem = ofono_modem_create(path, "hfp");
+
+	g_free(path);
+
+	if (modem == NULL)
+		return -ENOMEM;
+
+	ofono_modem_set_data(modem, hfp);
+	ofono_modem_set_name(modem, alias);
+	ofono_modem_register(modem);
+
+	g_hash_table_insert(modem_hash, g_strdup(hfp->device_path), modem);
+
+	return err;
+}
+
 static void parse_guint16(DBusMessageIter *iter, gpointer user_data)
 {
 	guint16 *value = user_data;
@@ -165,6 +232,11 @@ static int hfp_enable(struct ofono_modem *modem)
 {
 	DBG("%p", modem);
 
+	if (ofono_modem_get_powered(modem))
+		return 0;
+
+	ofono_modem_set_powered(modem, TRUE);
+
 	return 0;
 }
 
@@ -172,12 +244,22 @@ static int hfp_disable(struct ofono_modem *modem)
 {
 	DBG("%p", modem);
 
+	ofono_modem_set_powered(modem, FALSE);
+
 	return 0;
 }
 
 static void hfp_pre_sim(struct ofono_modem *modem)
 {
+	struct hfp *hfp = ofono_modem_get_data(modem);
+
 	DBG("%p", modem);
+
+	ofono_devinfo_create(modem, 0, "hfpmodem", hfp->device_address);
+	ofono_voicecall_create(modem, 0, "hfpmodem", &hfp->info);
+	ofono_netreg_create(modem, 0, "hfpmodem", &hfp->info);
+	ofono_call_volume_create(modem, 0, "hfpmodem", &hfp->info);
+	ofono_handsfree_create(modem, 0, "hfpmodem", &hfp->info);
 }
 
 static void hfp_post_sim(struct ofono_modem *modem)
@@ -199,11 +281,12 @@ static struct ofono_modem_driver hfp_driver = {
 static DBusMessage *profile_new_connection(DBusConnection *conn,
 					DBusMessage *msg, void *user_data)
 {
+	struct hfp *hfp;
 	DBusMessageIter entry;
 	const char *device;
 	GSList *endpoints = NULL;
 	guint16 version, features;
-	int fd;
+	int fd, err;
 
 	DBG("Profile handler NewConnection");
 
@@ -236,6 +319,21 @@ static DBusMessage *profile_new_connection(DBusConnection *conn,
 		ofono_error("Media Endpoint missing");
 		goto error;
 	}
+
+	hfp = g_new0(struct hfp, 1);
+	hfp->device_address = g_strdup("unknown");
+	hfp->device_path = g_strdup(device);
+	hfp->endpoints = endpoints;
+	hfp->msg = dbus_message_ref(msg);
+
+	err = modem_register(hfp, "unknown", fd, version);
+	if (err < 0 && err != -EINPROGRESS) {
+		hfp_free(hfp);
+		return g_dbus_create_error(msg,
+				BLUEZ_ERROR_INTERFACE ".Rejected",
+				"%s", strerror(-err));
+	}
+
 	return NULL;
 
 error:
@@ -320,6 +418,9 @@ static int hfp_init(void)
 		return err;
 	}
 
+	modem_hash = g_hash_table_new_full(g_str_hash, g_str_equal, g_free,
+								modem_data_free);
+
 	return 0;
 }
 
@@ -331,6 +432,8 @@ static void hfp_exit(void)
 	g_dbus_unregister_interface(conn, HFP_EXT_PROFILE_PATH,
 						BLUEZ_PROFILE_INTERFACE);
 	ofono_modem_driver_unregister(&hfp_driver);
+
+	g_hash_table_destroy(modem_hash);
 }
 
 OFONO_PLUGIN_DEFINE(hfp_bluez5, "External Hands-Free Profile Plugin", VERSION,
-- 
1.7.11.7

[PATCH v1 09/10] hfp_hf: Add service level negotiation

[Claudio Takahasi]

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

This patch adds the service level negotiation and the callbacks to
handle success and failure situations. The HFP modem is being destroyed
when the service level connection is dropped.
---
 plugins/hfp_hf_bluez5.c | 113 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 112 insertions(+), 1 deletion(-)

diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
index b3f8a1c..17421d0 100644
--- a/plugins/hfp_hf_bluez5.c
+++ b/plugins/hfp_hf_bluez5.c
@@ -24,6 +24,7 @@
 #endif
 
 #include <errno.h>
+#include <stdlib.h>
 #include <string.h>
 
 #include <glib.h>
@@ -64,6 +65,7 @@ struct hfp {
 	char *device_path;
 	struct hfp_slc_info info;
 	DBusMessage *msg;
+	GIOChannel *slcio;
 	GSList *endpoints;
 };
 
@@ -76,6 +78,9 @@ static void hfp_free(gpointer user_data)
 	if (hfp->msg)
 		dbus_message_unref(hfp->msg);
 
+	if (hfp->slcio)
+		g_io_channel_unref(hfp->slcio);
+
 	g_slist_free(hfp->endpoints);
 	g_free(hfp->device_address);
 	g_free(hfp->device_path);
@@ -90,12 +95,111 @@ static void modem_data_free(gpointer user_data)
 	hfp_free(hfp);
 }
 
+static void hfp_debug(const char *str, void *user_data)
+{
+	const char *prefix = user_data;
+
+	ofono_info("%s%s", prefix, str);
+}
+
+static void slc_established(gpointer userdata)
+{
+	struct ofono_modem *modem = userdata;
+	struct hfp *hfp = ofono_modem_get_data(modem);
+	DBusMessage *msg;
+
+	ofono_modem_set_powered(modem, TRUE);
+
+	msg = dbus_message_new_method_return(hfp->msg);
+	g_dbus_send_message(ofono_dbus_get_connection(), msg);
+	dbus_message_unref(hfp->msg);
+	hfp->msg = NULL;
+
+	ofono_info("Service level connection established");
+}
+
+static void slc_failed(gpointer userdata)
+{
+	struct ofono_modem *modem = userdata;
+	struct hfp *hfp = ofono_modem_get_data(modem);
+	DBusMessage *msg;
+
+	msg = g_dbus_create_error(hfp->msg, BLUEZ_ERROR_INTERFACE
+						".Failed",
+						"HFP Handshake failed");
+
+	g_dbus_send_message(ofono_dbus_get_connection(), msg);
+	dbus_message_unref(hfp->msg);
+	hfp->msg = NULL;
+
+	ofono_error("Service level connection failed");
+	ofono_modem_set_powered(modem, FALSE);
+
+	hfp_slc_info_free(&hfp->info);
+}
+
+static void hfp_disconnected_cb(gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct hfp *hfp = ofono_modem_get_data(modem);
+
+	DBG("HFP disconnected");
+
+	hfp_slc_info_free(&hfp->info);
+
+	ofono_modem_set_powered(modem, FALSE);
+}
+
+static GIOChannel *service_level_connection(struct ofono_modem *modem,
+							int fd, int *err)
+{
+	struct hfp *hfp = ofono_modem_get_data(modem);
+	GIOChannel *io;
+	GAtSyntax *syntax;
+	GAtChat *chat;
+
+	io = g_io_channel_unix_new(fd);
+	if (io == NULL) {
+		ofono_error("Service level connection failed: %s (%d)",
+						strerror(errno), errno);
+		*err = -EIO;
+		return NULL;
+	}
+
+	syntax = g_at_syntax_new_gsm_permissive();
+	chat = g_at_chat_new(io, syntax);
+	g_at_syntax_unref(syntax);
+	g_io_channel_set_close_on_unref(io, TRUE);
+
+	if (chat == NULL) {
+		*err = -ENOMEM;
+		goto fail;
+	}
+
+	g_at_chat_set_disconnect_function(chat, hfp_disconnected_cb, modem);
+
+	if (getenv("OFONO_AT_DEBUG"))
+		g_at_chat_set_debug(chat, hfp_debug, "");
+
+	hfp->info.chat = chat;
+	hfp_slc_establish(&hfp->info, slc_established, slc_failed, modem);
+
+	*err = -EINPROGRESS;
+
+	return io;
+
+fail:
+	g_io_channel_unref(io);
+	return NULL;
+}
+
 static int modem_register(struct hfp *hfp, const char *alias, int fd,
 							guint16 version)
 {
 	struct ofono_modem *modem;
-	int err = 0;
+	guint8 codecs[1];
 	char *path;
+	int err;
 
 	path = g_strconcat("hfp", hfp->device_path, NULL);
 
@@ -112,6 +216,13 @@ static int modem_register(struct hfp *hfp, const char *alias, int fd,
 
 	g_hash_table_insert(modem_hash, g_strdup(hfp->device_path), modem);
 
+	memset(codecs, 0, sizeof(codecs));
+	codecs[0] = HFP_CODEC_CVSD;
+
+	hfp_slc_info_init(&hfp->info, version, codecs, 1);
+
+	hfp->slcio = service_level_connection(modem, fd, &err);
+
 	return err;
 }
 
-- 
1.7.11.7

[PATCH v1 10/10] hfp_hf: Add support for codec negotiation

[Claudio Takahasi]

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

From: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>

The negotiation is very simple, if the codec the AG requests is present
in the supported codec list that the HF sent, the codec is accepted, if
not, we send the list of supported codecs again, and expect that the AG
re-sends the request with an acceptable codec.
---
 plugins/hfp_hf_bluez5.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
index 17421d0..d8ec0f2 100644
--- a/plugins/hfp_hf_bluez5.c
+++ b/plugins/hfp_hf_bluez5.c
@@ -25,6 +25,7 @@
 
 #include <errno.h>
 #include <stdlib.h>
+#include <stdio.h>
 #include <string.h>
 
 #include <glib.h>
@@ -64,6 +65,7 @@ struct hfp {
 	char *device_address;
 	char *device_path;
 	struct hfp_slc_info info;
+	guint8 current_codec;
 	DBusMessage *msg;
 	GIOChannel *slcio;
 	GSList *endpoints;
@@ -102,12 +104,59 @@ static void hfp_debug(const char *str, void *user_data)
 	ofono_info("%s%s", prefix, str);
 }
 
+static void bcs_notify(GAtResult *result, gpointer user_data)
+{
+	struct hfp *hfp = user_data;
+	struct hfp_slc_info *info = &hfp->info;
+	GAtResultIter iter;
+	GString *str;
+	int i, value;
+
+	g_at_result_iter_init(&iter, result);
+
+	if (!g_at_result_iter_next(&iter, "+BCS:"))
+		return;
+
+	if (!g_at_result_iter_next_number(&iter, &value))
+		return;
+
+	/* If some codec matches, we confirm it */
+	for (i = 0; i < info->codecs_len; i++) {
+		if (info->codecs[i] == value) {
+			char buf[32];
+
+			hfp->current_codec = value;
+			DBG("Negotiated HFP codec: %d", value);
+
+			snprintf(buf, sizeof(buf), "AT+BCS=%d", value);
+			g_at_chat_send(info->chat, buf, NULL, NULL,
+							NULL, NULL);
+			return;
+		}
+	}
+
+	/* If none matches, we send our supported codecs again */
+	str = g_string_new("AT+BAC=");
+
+	for (i = 0; i < info->codecs_len; i++) {
+		g_string_append_printf(str, "%d", info->codecs[i]);
+		if (i + 1 < info->codecs_len)
+			str = g_string_append(str, ",");
+	}
+
+	g_at_chat_send(info->chat, str->str, NULL, NULL, NULL, NULL);
+	g_string_free(str, TRUE);
+}
+
 static void slc_established(gpointer userdata)
 {
 	struct ofono_modem *modem = userdata;
 	struct hfp *hfp = ofono_modem_get_data(modem);
+	struct hfp_slc_info *info = &hfp->info;
 	DBusMessage *msg;
 
+	g_at_chat_register(info->chat, "+BCS:", bcs_notify, FALSE, hfp, NULL);
+
 	ofono_modem_set_powered(modem, TRUE);
 
 	msg = dbus_message_new_method_return(hfp->msg);
@@ -436,6 +485,7 @@ static DBusMessage *profile_new_connection(DBusConnection *conn,
 	hfp->device_path = g_strdup(device);
 	hfp->endpoints = endpoints;
 	hfp->msg = dbus_message_ref(msg);
+	hfp->current_codec = HFP_CODEC_CVSD;
 
 	err = modem_register(hfp, "unknown", fd, version);
 	if (err < 0 && err != -EINPROGRESS) {
-- 
1.7.11.7

[PATCH v2 00/10] HFP HF: Service Level/Codec negotiation

[Claudio Takahasi]

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

Second group of patches related to HFP external profile.

The following patches contain the HFP 1.6 codec negotiation.

Changed between v0-v1:
* deleted media.[ch], functions moved to bluez5.[ch]
* replaced 'bluetooth_' prefix by 'bt_' for BlueZ 5 functions
* splitted HFP SLC patches
* new oFono D-Bus utility function to parse dictionaries

Changed between v1-v2:
* fixed wrong commit messages

Claudio Takahasi (2):
  hfp_hf: Register the HFP modem
  hfp_hf: Add service level negotiation

Vinicius Costa Gomes (8):
  dbus: Add ofono_dbus_iter_parse_properties()
  bluez5: Rename register/unregister profile
  hfp_hf: Add NewConnection arguments parsing
  hfpmodem: Add support for storing the supported codecs
  hfpmodem: Implement hfp_slc_info_free
  phonesim: Use hfp_slc_info_free()
  hfpmodem: Send the AT+BAC command with the supported codecs
  hfp_hf: Add support for codec negotiation

 drivers/hfpmodem/slc.c  |  29 +++-
 drivers/hfpmodem/slc.h  |  10 +-
 include/dbus.h          |   6 +
 plugins/bluez5.c        |  39 ++++-
 plugins/bluez5.h        |  12 +-
 plugins/hfp_hf_bluez4.c |   2 +-
 plugins/hfp_hf_bluez5.c | 417 +++++++++++++++++++++++++++++++++++++++++++++++-
 plugins/phonesim.c      |  12 +-
 src/dbus.c              |  90 +++++++++++
 9 files changed, 598 insertions(+), 19 deletions(-)

-- 
1.7.11.7

[PATCH v2 01/10] dbus: Add ofono_dbus_iter_parse_properties()

[Claudio Takahasi]

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

From: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>

With the move of BlueZ to DBus ObjectManager and the ProfileManager1
interface, the case that ofono receives a message with a properties
dictionary should be more common, so we add a function to help deal
with it.
---
 include/dbus.h |  6 ++++
 src/dbus.c     | 90 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 96 insertions(+)

diff --git a/include/dbus.h b/include/dbus.h
index 44faa7f..3901a96 100644
--- a/include/dbus.h
+++ b/include/dbus.h
@@ -100,6 +100,12 @@ int ofono_dbus_signal_dict_property_changed(DBusConnection *conn,
 						const char *name, int type,
 						void *value);
 
+typedef void (*property_handler_cb)(DBusMessageIter *iter, void *user_data);
+
+void ofono_dbus_iter_parse_properties(DBusMessageIter *array,
+				const char *property, property_handler_cb cb,
+				void *user_data, ...);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/src/dbus.c b/src/dbus.c
index 1558a51..1afbb44 100644
--- a/src/dbus.c
+++ b/src/dbus.c
@@ -30,6 +30,14 @@
 
 #define OFONO_ERROR_INTERFACE "org.ofono.Error"
 
+typedef void (*property_handler_cb)(DBusMessageIter *iter, void *user_data);
+
+struct property_handler {
+	const char *property;
+	property_handler_cb callback;
+	void *user_data;
+};
+
 static DBusConnection *g_connection;
 
 struct error_mapping_entry {
@@ -273,6 +281,88 @@ int ofono_dbus_signal_dict_property_changed(DBusConnection *conn,
 	return g_dbus_send_message(conn, signal);
 }
 
+static gint property_handler_compare(gconstpointer a, gconstpointer b)
+{
+	const struct property_handler *handler = a;
+	const char *property = b;
+
+	return g_strcmp0(handler->property, property);
+}
+
+void ofono_dbus_iter_parse_properties(DBusMessageIter *array,
+				const char *property, property_handler_cb cb,
+				void *user_data, ...)
+{
+	va_list args;
+	GSList *prop_handlers = NULL;
+	DBusMessageIter dict;
+
+	if (dbus_message_iter_get_arg_type(array) != DBUS_TYPE_ARRAY)
+		goto done;
+
+	va_start(args, user_data);
+
+	if (property == NULL)
+		return;
+
+	while (1) {
+		struct property_handler *handler =
+					g_new0(struct property_handler, 1);
+
+		handler->property = property;
+		handler->callback = cb;
+		handler->user_data = user_data;
+
+		prop_handlers = g_slist_prepend(prop_handlers, handler);
+
+		property = va_arg(args, const char *);
+		if (property == NULL)
+			break;
+
+		cb = va_arg(args, property_handler_cb);
+		user_data = va_arg(args, void *);
+	}
+
+	va_end(args);
+
+	dbus_message_iter_recurse(array, &dict);
+
+	while (dbus_message_iter_get_arg_type(&dict) == DBUS_TYPE_DICT_ENTRY) {
+		DBusMessageIter entry, value;
+		const char *key;
+		GSList *l;
+
+		dbus_message_iter_recurse(&dict, &entry);
+
+		if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_STRING)
+			goto done;
+
+		dbus_message_iter_get_basic(&entry, &key);
+
+		dbus_message_iter_next(&entry);
+
+		if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_VARIANT)
+			goto done;
+
+		dbus_message_iter_recurse(&entry, &value);
+
+		l = g_slist_find_custom(prop_handlers, key,
+					property_handler_compare);
+
+		if (l) {
+			struct property_handler *handler = l->data;
+
+			handler->callback(&value, handler->user_data);
+		}
+
+		dbus_message_iter_next(&dict);
+	}
+
+done:
+	g_slist_foreach(prop_handlers, (GFunc) g_free, NULL);
+	g_slist_free(prop_handlers);
+}
+
 DBusMessage *__ofono_error_invalid_args(DBusMessage *msg)
 {
 	return g_dbus_create_error(msg, OFONO_ERROR_INTERFACE
-- 
1.7.11.7

[PATCH v2 02/10] bluez5: Rename register/unregister profile

[Claudio Takahasi]

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

From: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>

Using 'bt_' prefix will be more suitable for the upcoming
functions, and structure names avoiding long names and improving
code readability.
---
 plugins/bluez5.c        | 4 ++--
 plugins/bluez5.h        | 4 ++--
 plugins/hfp_hf_bluez5.c | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/plugins/bluez5.c b/plugins/bluez5.c
index 84ba47d..589658a 100644
--- a/plugins/bluez5.c
+++ b/plugins/bluez5.c
@@ -80,7 +80,7 @@ done:
 	dbus_message_unref(reply);
 }
 
-int bluetooth_register_profile(DBusConnection *conn, const char *uuid,
+int bt_register_profile(DBusConnection *conn, const char *uuid,
 					const char *name, const char *object)
 {
 	DBusMessageIter iter, dict;
@@ -115,7 +115,7 @@ int bluetooth_register_profile(DBusConnection *conn, const char *uuid,
 	return 0;
 }
 
-void bluetooth_unregister_profile(DBusConnection *conn, const char *object)
+void bt_unregister_profile(DBusConnection *conn, const char *object)
 {
 	DBusMessageIter iter;
 	DBusPendingCall *c;
diff --git a/plugins/bluez5.h b/plugins/bluez5.h
index 01ecfe8..3088c7d 100644
--- a/plugins/bluez5.h
+++ b/plugins/bluez5.h
@@ -25,7 +25,7 @@
 
 #define HFP_HS_UUID	"0000111e-0000-1000-8000-00805f9b34fb"
 
-int bluetooth_register_profile(DBusConnection *conn, const char *uuid,
+int bt_register_profile(DBusConnection *conn, const char *uuid,
 					const char *name, const char *object);
 
-void bluetooth_unregister_profile(DBusConnection *conn, const char *object);
+void bt_unregister_profile(DBusConnection *conn, const char *object);
diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
index e024838..f1f8891 100644
--- a/plugins/hfp_hf_bluez5.c
+++ b/plugins/hfp_hf_bluez5.c
@@ -168,7 +168,7 @@ static int hfp_init(void)
 		return err;
 	}
 
-	err = bluetooth_register_profile(conn, HFP_HS_UUID, "hfp_hf",
+	err = bt_register_profile(conn, HFP_HS_UUID, "hfp_hf",
 						HFP_EXT_PROFILE_PATH);
 	if (err < 0) {
 		g_dbus_unregister_interface(conn, HFP_EXT_PROFILE_PATH,
@@ -184,7 +184,7 @@ static void hfp_exit(void)
 {
 	DBusConnection *conn = ofono_dbus_get_connection();
 
-	bluetooth_unregister_profile(conn, HFP_EXT_PROFILE_PATH);
+	bt_unregister_profile(conn, HFP_EXT_PROFILE_PATH);
 	g_dbus_unregister_interface(conn, HFP_EXT_PROFILE_PATH,
 						BLUEZ_PROFILE_INTERFACE);
 	ofono_modem_driver_unregister(&hfp_driver);
-- 
1.7.11.7

[PATCH v2 03/10] hfp_hf: Add NewConnection arguments parsing

[Claudio Takahasi]

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

From: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>

This patch adds NewConnection message arguments parsing: features,
version and Media Endpoints included in the fd_properties dictionary.
---
 plugins/bluez5.c        |  35 ++++++++++++
 plugins/bluez5.h        |   8 +++
 plugins/hfp_hf_bluez5.c | 149 +++++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 189 insertions(+), 3 deletions(-)

diff --git a/plugins/bluez5.c b/plugins/bluez5.c
index 589658a..d753ac9 100644
--- a/plugins/bluez5.c
+++ b/plugins/bluez5.c
@@ -36,6 +36,41 @@
 
 #define BLUEZ_PROFILE_MGMT_INTERFACE   BLUEZ_SERVICE ".ProfileManager1"
 
+struct bt_endpoint {
+	char *owner;
+	char *path;
+	guint8 codec;
+	unsigned char *capa;	/* Capabilities */
+	int capa_size;		/* Capabilities size */
+};
+
+struct bt_endpoint *bt_endpoint_new(const char *owner,
+					const char *path, guint8 codec,
+					unsigned char *capa, int capa_size)
+{
+	struct bt_endpoint *endpoint;
+
+	endpoint = g_new0(struct bt_endpoint, 1);
+	endpoint->owner = g_strdup(owner);
+	endpoint->path = g_strdup(path);
+	endpoint->codec = codec;
+	if (capa && capa_size)
+		endpoint->capa = g_memdup(capa, capa_size);
+
+	return endpoint;
+}
+
+void bt_endpoint_free(gpointer data)
+{
+	struct bt_endpoint *endpoint = data;
+
+	g_free(endpoint->owner);
+	g_free(endpoint->path);
+	if (endpoint->capa)
+		g_free(endpoint->capa);
+	g_free(endpoint);
+}
+
 static void profile_register_cb(DBusPendingCall *call, gpointer user_data)
 {
 	DBusMessage *reply;
diff --git a/plugins/bluez5.h b/plugins/bluez5.h
index 3088c7d..1ab350e 100644
--- a/plugins/bluez5.h
+++ b/plugins/bluez5.h
@@ -25,6 +25,14 @@
 
 #define HFP_HS_UUID	"0000111e-0000-1000-8000-00805f9b34fb"
 
+struct bt_endpoint;
+
+struct bt_endpoint *bt_endpoint_new(const char *owner,
+					const char *path, guint8 codec,
+					unsigned char *capa, int capa_size);
+
+void bt_endpoint_free(gpointer data);
+
 int bt_register_profile(DBusConnection *conn, const char *uuid,
 					const char *name, const char *object);
 
diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
index f1f8891..ef9af62 100644
--- a/plugins/hfp_hf_bluez5.c
+++ b/plugins/hfp_hf_bluez5.c
@@ -25,6 +25,7 @@
 
 #include <errno.h>
 #include <glib.h>
+#include <string.h>
 
 #include <gdbus.h>
 
@@ -42,6 +43,111 @@
 
 #define HFP_EXT_PROFILE_PATH   "/bluetooth/profile/hfp_hf"
 
+struct capabilities {
+	unsigned char *value;
+	int size;
+};
+
+static void parse_guint16(DBusMessageIter *iter, gpointer user_data)
+{
+	guint16 *value = user_data;
+
+	if (dbus_message_iter_get_arg_type(iter) !=  DBUS_TYPE_UINT16)
+		return;
+
+	dbus_message_iter_get_basic(iter, value);
+}
+
+static void parse_string(DBusMessageIter *iter, gpointer user_data)
+{
+	char **str = user_data;
+	int arg_type = dbus_message_iter_get_arg_type(iter);
+
+	if (arg_type != DBUS_TYPE_OBJECT_PATH && arg_type != DBUS_TYPE_STRING)
+		return;
+
+	dbus_message_iter_get_basic(iter, str);
+}
+
+static void parse_byte(DBusMessageIter *iter, gpointer user_data)
+{
+	guint8 *value = user_data;
+
+	if (dbus_message_iter_get_arg_type(iter) !=  DBUS_TYPE_BYTE)
+		return;
+
+	dbus_message_iter_get_basic(iter, value);
+}
+
+static void parse_capabilities(DBusMessageIter *iter, gpointer user_data)
+{
+	DBusMessageIter array;
+	struct capabilities *capa = user_data;
+	guint8 *data = NULL;
+	int n = 0;
+
+	if (dbus_message_iter_get_arg_type(iter) !=  DBUS_TYPE_ARRAY)
+		return;
+
+	dbus_message_iter_recurse(iter, &array);
+	dbus_message_iter_get_fixed_array(&array, &data, &n);
+	if (n == 0)
+		return;
+
+	capa->value = g_memdup(data, n);
+	capa->size = n;
+}
+
+static void parse_media_endpoints(DBusMessageIter *array, gpointer user_data)
+{
+	const char *path, *owner;
+	GSList **endpoints = user_data;
+	struct capabilities capa;
+	struct bt_endpoint *endpoint;
+	guint8 codec;
+	DBusMessageIter dict, variant, entry;
+
+	if (dbus_message_iter_get_arg_type(array) != DBUS_TYPE_ARRAY)
+		return;
+
+	dbus_message_iter_recurse(array, &dict);
+
+	while (dbus_message_iter_get_arg_type(&dict) == DBUS_TYPE_DICT_ENTRY) {
+		path = NULL;
+		codec = 0x00;
+
+		dbus_message_iter_recurse(&dict, &entry);
+		if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_STRING)
+			return;
+
+		dbus_message_iter_get_basic(&entry, &owner);
+
+		dbus_message_iter_next(&entry);
+
+		if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_VARIANT)
+			return;
+
+		dbus_message_iter_recurse(&entry, &variant);
+
+		memset(&capa, 0, sizeof(capa));
+		ofono_dbus_iter_parse_properties(&variant,
+				"Path", parse_string, &path,
+				"Codec", parse_byte, &codec,
+				"Capabilities", parse_capabilities, &capa,
+				NULL);
+
+		dbus_message_iter_next(&dict);
+
+		endpoint = bt_endpoint_new(owner, path, codec,
+						capa.value, capa.size);
+
+		g_free(capa.value);
+		*endpoints = g_slist_append(*endpoints, endpoint);
+
+		DBG("Media Endpoint %s %s codec: 0x%02X", owner, path, codec);
+	}
+}
+
 static int hfp_probe(struct ofono_modem *modem)
 {
 	DBG("modem: %p", modem);
@@ -93,11 +199,48 @@ static struct ofono_modem_driver hfp_driver = {
 static DBusMessage *profile_new_connection(DBusConnection *conn,
 					DBusMessage *msg, void *user_data)
 {
+	DBusMessageIter entry;
+	const char *device;
+	GSList *endpoints = NULL;
+	guint16 version, features;
+	int fd;
+
 	DBG("Profile handler NewConnection");
 
-	return g_dbus_create_error(msg, BLUEZ_ERROR_INTERFACE
-					".NotImplemented",
-					"Implementation not provided");
+	if (dbus_message_iter_init(msg, &entry) == FALSE)
+		goto error;
+
+	if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_OBJECT_PATH)
+		goto error;
+
+	dbus_message_iter_get_basic(&entry, &device);
+	dbus_message_iter_next(&entry);
+	if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_UNIX_FD)
+		goto error;
+
+	dbus_message_iter_get_basic(&entry, &fd);
+	if (fd < 0)
+		goto error;
+
+	dbus_message_iter_next(&entry);
+
+	ofono_dbus_iter_parse_properties(&entry,
+			"Version", parse_guint16, &version,
+			"Features", parse_guint16, &features,
+			"MediaEndpoints", parse_media_endpoints, &endpoints,
+			NULL);
+
+	DBG("%s version: 0x%04x features: 0x%04x", device, version, features);
+
+	if (endpoints == NULL) {
+		ofono_error("Media Endpoint missing");
+		goto error;
+	}
+	return NULL;
+
+error:
+	return g_dbus_create_error(msg, BLUEZ_ERROR_INTERFACE ".Rejected",
+					"Invalid arguments in method call");
 }
 
 static DBusMessage *profile_release(DBusConnection *conn,
-- 
1.7.11.7

[PATCH v2 04/10] hfpmodem: Add support for storing the supported codecs

[Claudio Takahasi]

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

From: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>

As informing the AG of the supported codecs is part of the setup
of the SLC, it makes sense to have this information inside the SLC
establishment part.

In case we have only one codec, we don't inform the other side that
we support the codec negotiation feature. From the spec: "If Wide
Band Speech is supported, Codec Negotiation shall also be supported."
HFP 1.6, Section 3, page 16.
---
 drivers/hfpmodem/slc.c  |  8 ++++++--
 drivers/hfpmodem/slc.h  | 10 +++++++++-
 plugins/hfp_hf_bluez4.c |  2 +-
 plugins/phonesim.c      |  6 +++++-
 4 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/hfpmodem/slc.c b/drivers/hfpmodem/slc.c
index 646befa..06153fc 100644
--- a/drivers/hfpmodem/slc.c
+++ b/drivers/hfpmodem/slc.c
@@ -52,7 +52,8 @@ struct slc_establish_data {
 	gpointer userdata;
 };
 
-void hfp_slc_info_init(struct hfp_slc_info *info, guint16 version)
+void hfp_slc_info_init(struct hfp_slc_info *info, guint16 version,
+						unsigned char *codecs, int len)
 {
 	info->ag_features = 0;
 	info->ag_mpty_features = 0;
@@ -67,11 +68,14 @@ void hfp_slc_info_init(struct hfp_slc_info *info, guint16 version)
 	info->hf_features |= HFP_HF_FEATURE_ENHANCED_CALL_STATUS;
 	info->hf_features |= HFP_HF_FEATURE_ENHANCED_CALL_CONTROL;
 
-	if (version < HFP_VERSION_1_6)
+	if (version < HFP_VERSION_1_6 || len <= 1)
 		goto done;
 
 	info->hf_features |= HFP_HF_FEATURE_CODEC_NEGOTIATION;
 
+	info->codecs = g_memdup(codecs, len);
+	info->codecs_len = len;
+
 done:
 	memset(info->cind_val, 0, sizeof(info->cind_val));
 	memset(info->cind_pos, 0, sizeof(info->cind_pos));
diff --git a/drivers/hfpmodem/slc.h b/drivers/hfpmodem/slc.h
index b71ffe9..d7c90a2 100644
--- a/drivers/hfpmodem/slc.h
+++ b/drivers/hfpmodem/slc.h
@@ -44,6 +44,11 @@ enum hfp_indicator {
 	HFP_INDICATOR_LAST
 };
 
+enum hfp_codec {
+	HFP_CODEC_CVSD = 0x01,
+	HFP_CODEC_MSBC = 0x02,
+};
+
 typedef void (*hfp_slc_cb_t)(void *userdata);
 
 struct hfp_slc_info {
@@ -53,9 +58,12 @@ struct hfp_slc_info {
 	unsigned int hf_features;
 	unsigned char cind_pos[HFP_INDICATOR_LAST];
 	unsigned int cind_val[HFP_INDICATOR_LAST];
+	unsigned char *codecs;
+	int codecs_len;
 };
 
-void hfp_slc_info_init(struct hfp_slc_info *info, guint16 version);
+void hfp_slc_info_init(struct hfp_slc_info *info, guint16 version,
+					unsigned char *codecs, int len);
 void hfp_slc_info_free(struct hfp_slc_info *info);
 
 void hfp_slc_establish(struct hfp_slc_info *info, hfp_slc_cb_t connect_cb,
diff --git a/plugins/hfp_hf_bluez4.c b/plugins/hfp_hf_bluez4.c
index 450c183..e7fce31 100644
--- a/plugins/hfp_hf_bluez4.c
+++ b/plugins/hfp_hf_bluez4.c
@@ -170,7 +170,7 @@ static DBusMessage *hfp_agent_new_connection(DBusConnection *conn,
 				DBUS_TYPE_UINT16, &version, DBUS_TYPE_INVALID))
 		return __ofono_error_invalid_args(msg);
 
-	hfp_slc_info_init(&hfp_data->info, version);
+	hfp_slc_info_init(&hfp_data->info, version, NULL, 0);
 
 	err = service_level_connection(modem, fd);
 	if (err < 0 && err != -EINPROGRESS)
diff --git a/plugins/phonesim.c b/plugins/phonesim.c
index 26f96d0..8631039 100644
--- a/plugins/phonesim.c
+++ b/plugins/phonesim.c
@@ -896,6 +896,7 @@ static int localhfp_enable(struct ofono_modem *modem)
 	GAtSyntax *syntax;
 	GAtChat *chat;
 	const char *address;
+	unsigned char codecs[1];
 	int sk, port;
 
 	address = ofono_modem_get_string(modem, "Address");
@@ -929,7 +930,10 @@ static int localhfp_enable(struct ofono_modem *modem)
 
 	g_at_chat_set_disconnect_function(chat, slc_failed, modem);
 
-	hfp_slc_info_init(info, HFP_VERSION_LATEST);
+	memset(codecs, 0, sizeof(codecs));
+	codecs[0] = HFP_CODEC_CVSD;
+
+	hfp_slc_info_init(info, HFP_VERSION_LATEST, codecs, 1);
 	info->chat = chat;
 	hfp_slc_establish(info, slc_established, slc_failed, modem);
 
-- 
1.7.11.7

[PATCH v2 05/10] hfpmodem: Implement hfp_slc_info_free

[Claudio Takahasi]

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

From: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>

hfp_slc_info_free() was already being declared, but it was not defined
until now.
---
 drivers/hfpmodem/slc.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/hfpmodem/slc.c b/drivers/hfpmodem/slc.c
index 06153fc..f088a46 100644
--- a/drivers/hfpmodem/slc.c
+++ b/drivers/hfpmodem/slc.c
@@ -81,6 +81,14 @@ done:
 	memset(info->cind_pos, 0, sizeof(info->cind_pos));
 }
 
+void hfp_slc_info_free(struct hfp_slc_info *info)
+{
+	g_free(info->codecs);
+
+	g_at_chat_unref(info->chat);
+	info->chat = NULL;
+}
+
 static void slc_establish_data_unref(gpointer userdata)
 {
 	struct slc_establish_data *sed = userdata;
-- 
1.7.11.7

[PATCH v2 06/10] phonesim: Use hfp_slc_info_free()

[Claudio Takahasi]

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

From: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>

Now that we have a function that takes care of free'ing that structure
we should use it.
---
 plugins/phonesim.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/plugins/phonesim.c b/plugins/phonesim.c
index 8631039..63590a9 100644
--- a/plugins/phonesim.c
+++ b/plugins/phonesim.c
@@ -885,8 +885,7 @@ static void slc_failed(gpointer userdata)
 
 	ofono_modem_set_powered(modem, FALSE);
 
-	g_at_chat_unref(info->chat);
-	info->chat = NULL;
+	hfp_slc_info_free(info);
 }
 
 static int localhfp_enable(struct ofono_modem *modem)
@@ -944,8 +943,7 @@ static int localhfp_disable(struct ofono_modem *modem)
 {
 	struct hfp_slc_info *info = ofono_modem_get_data(modem);
 
-	g_at_chat_unref(info->chat);
-	info->chat = NULL;
+	hfp_slc_info_free(info);
 
 	return 0;
 }
-- 
1.7.11.7

[PATCH v2 07/10] hfpmodem: Send the AT+BAC command with the supported codecs

[Claudio Takahasi]

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

From: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>

Before, the AT+BAC command was being sent with fixed information,
now we send the command (that inform the AG of the codecs supported by
the HF) with the codecs that come from the profile implementation.
---
 drivers/hfpmodem/slc.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/hfpmodem/slc.c b/drivers/hfpmodem/slc.c
index f088a46..54766bb 100644
--- a/drivers/hfpmodem/slc.c
+++ b/drivers/hfpmodem/slc.c
@@ -316,10 +316,21 @@ static void brsf_cb(gboolean ok, GAtResult *result, gpointer user_data)
 
 	if (info->ag_features & HFP_AG_FEATURE_CODEC_NEGOTIATION &&
 			info->hf_features & HFP_HF_FEATURE_CODEC_NEGOTIATION) {
+		GString *str = g_string_new("AT+BAC=");
+		int i;
+
+		for (i = 0; i < info->codecs_len; i++) {
+			g_string_append_printf(str, "%d", info->codecs[i]);
+			if (i + 1 < info->codecs_len)
+				str = g_string_append(str, ",");
+		}
 
 		slc_establish_data_ref(sed);
-		g_at_chat_send(info->chat, "AT+BAC=1", NULL, bac_cb, sed,
+		g_at_chat_send(info->chat, str->str, NULL, bac_cb, sed,
 						slc_establish_data_unref);
+
+		g_string_free(str, TRUE);
+
 		return;
 	}
 
-- 
1.7.11.7

[PATCH v2 08/10] hfp_hf: Register the HFP modem

[Claudio Takahasi]

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

This patch registers the HFP modem when Bluetooth link is established.
---
 plugins/hfp_hf_bluez5.c | 105 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 104 insertions(+), 1 deletion(-)

diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
index ef9af62..b3f8a1c 100644
--- a/plugins/hfp_hf_bluez5.c
+++ b/plugins/hfp_hf_bluez5.c
@@ -24,16 +24,27 @@
 #endif
 
 #include <errno.h>
+#include <string.h>
+
 #include <glib.h>
 #include <string.h>
 
 #include <gdbus.h>
+#include <gatchat.h>
+#include <gattty.h>
 
 #define OFONO_API_SUBJECT_TO_CHANGE
 #include <ofono/modem.h>
 #include <ofono/dbus.h>
 #include <ofono/plugin.h>
 #include <ofono/log.h>
+#include <ofono/devinfo.h>
+#include <ofono/netreg.h>
+#include <ofono/voicecall.h>
+#include <ofono/call-volume.h>
+#include <ofono/handsfree.h>
+
+#include <drivers/hfpmodem/slc.h>
 
 #include "bluez5.h"
 
@@ -48,6 +59,62 @@ struct capabilities {
 	int size;
 };
 
+struct hfp {
+	char *device_address;
+	char *device_path;
+	struct hfp_slc_info info;
+	DBusMessage *msg;
+	GSList *endpoints;
+};
+
+static GHashTable *modem_hash = NULL;
+
+static void hfp_free(gpointer user_data)
+{
+	struct hfp *hfp = user_data;
+
+	if (hfp->msg)
+		dbus_message_unref(hfp->msg);
+
+	g_slist_free(hfp->endpoints);
+	g_free(hfp->device_address);
+	g_free(hfp->device_path);
+	g_free(hfp);
+}
+
+static void modem_data_free(gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct hfp *hfp = ofono_modem_get_data(modem);
+
+	hfp_free(hfp);
+}
+
+static int modem_register(struct hfp *hfp, const char *alias, int fd,
+							guint16 version)
+{
+	struct ofono_modem *modem;
+	int err = 0;
+	char *path;
+
+	path = g_strconcat("hfp", hfp->device_path, NULL);
+
+	modem = ofono_modem_create(path, "hfp");
+
+	g_free(path);
+
+	if (modem == NULL)
+		return -ENOMEM;
+
+	ofono_modem_set_data(modem, hfp);
+	ofono_modem_set_name(modem, alias);
+	ofono_modem_register(modem);
+
+	g_hash_table_insert(modem_hash, g_strdup(hfp->device_path), modem);
+
+	return err;
+}
+
 static void parse_guint16(DBusMessageIter *iter, gpointer user_data)
 {
 	guint16 *value = user_data;
@@ -165,6 +232,11 @@ static int hfp_enable(struct ofono_modem *modem)
 {
 	DBG("%p", modem);
 
+	if (ofono_modem_get_powered(modem))
+		return 0;
+
+	ofono_modem_set_powered(modem, TRUE);
+
 	return 0;
 }
 
@@ -172,12 +244,22 @@ static int hfp_disable(struct ofono_modem *modem)
 {
 	DBG("%p", modem);
 
+	ofono_modem_set_powered(modem, FALSE);
+
 	return 0;
 }
 
 static void hfp_pre_sim(struct ofono_modem *modem)
 {
+	struct hfp *hfp = ofono_modem_get_data(modem);
+
 	DBG("%p", modem);
+
+	ofono_devinfo_create(modem, 0, "hfpmodem", hfp->device_address);
+	ofono_voicecall_create(modem, 0, "hfpmodem", &hfp->info);
+	ofono_netreg_create(modem, 0, "hfpmodem", &hfp->info);
+	ofono_call_volume_create(modem, 0, "hfpmodem", &hfp->info);
+	ofono_handsfree_create(modem, 0, "hfpmodem", &hfp->info);
 }
 
 static void hfp_post_sim(struct ofono_modem *modem)
@@ -199,11 +281,12 @@ static struct ofono_modem_driver hfp_driver = {
 static DBusMessage *profile_new_connection(DBusConnection *conn,
 					DBusMessage *msg, void *user_data)
 {
+	struct hfp *hfp;
 	DBusMessageIter entry;
 	const char *device;
 	GSList *endpoints = NULL;
 	guint16 version, features;
-	int fd;
+	int fd, err;
 
 	DBG("Profile handler NewConnection");
 
@@ -236,6 +319,21 @@ static DBusMessage *profile_new_connection(DBusConnection *conn,
 		ofono_error("Media Endpoint missing");
 		goto error;
 	}
+
+	hfp = g_new0(struct hfp, 1);
+	hfp->device_address = g_strdup("unknown");
+	hfp->device_path = g_strdup(device);
+	hfp->endpoints = endpoints;
+	hfp->msg = dbus_message_ref(msg);
+
+	err = modem_register(hfp, "unknown", fd, version);
+	if (err < 0 && err != -EINPROGRESS) {
+		hfp_free(hfp);
+		return g_dbus_create_error(msg,
+				BLUEZ_ERROR_INTERFACE ".Rejected",
+				"%s", strerror(-err));
+	}
+
 	return NULL;
 
 error:
@@ -320,6 +418,9 @@ static int hfp_init(void)
 		return err;
 	}
 
+	modem_hash = g_hash_table_new_full(g_str_hash, g_str_equal, g_free,
+								modem_data_free);
+
 	return 0;
 }
 
@@ -331,6 +432,8 @@ static void hfp_exit(void)
 	g_dbus_unregister_interface(conn, HFP_EXT_PROFILE_PATH,
 						BLUEZ_PROFILE_INTERFACE);
 	ofono_modem_driver_unregister(&hfp_driver);
+
+	g_hash_table_destroy(modem_hash);
 }
 
 OFONO_PLUGIN_DEFINE(hfp_bluez5, "External Hands-Free Profile Plugin", VERSION,
-- 
1.7.11.7

[PATCH v2 09/10] hfp_hf: Add service level negotiation

[Claudio Takahasi]

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

This patch adds service level negotiation and callbacks to handle
success and failure.
---
 plugins/hfp_hf_bluez5.c | 113 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 112 insertions(+), 1 deletion(-)

diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
index b3f8a1c..17421d0 100644
--- a/plugins/hfp_hf_bluez5.c
+++ b/plugins/hfp_hf_bluez5.c
@@ -24,6 +24,7 @@
 #endif
 
 #include <errno.h>
+#include <stdlib.h>
 #include <string.h>
 
 #include <glib.h>
@@ -64,6 +65,7 @@ struct hfp {
 	char *device_path;
 	struct hfp_slc_info info;
 	DBusMessage *msg;
+	GIOChannel *slcio;
 	GSList *endpoints;
 };
 
@@ -76,6 +78,9 @@ static void hfp_free(gpointer user_data)
 	if (hfp->msg)
 		dbus_message_unref(hfp->msg);
 
+	if (hfp->slcio)
+		g_io_channel_unref(hfp->slcio);
+
 	g_slist_free(hfp->endpoints);
 	g_free(hfp->device_address);
 	g_free(hfp->device_path);
@@ -90,12 +95,111 @@ static void modem_data_free(gpointer user_data)
 	hfp_free(hfp);
 }
 
+static void hfp_debug(const char *str, void *user_data)
+{
+	const char *prefix = user_data;
+
+	ofono_info("%s%s", prefix, str);
+}
+
+static void slc_established(gpointer userdata)
+{
+	struct ofono_modem *modem = userdata;
+	struct hfp *hfp = ofono_modem_get_data(modem);
+	DBusMessage *msg;
+
+	ofono_modem_set_powered(modem, TRUE);
+
+	msg = dbus_message_new_method_return(hfp->msg);
+	g_dbus_send_message(ofono_dbus_get_connection(), msg);
+	dbus_message_unref(hfp->msg);
+	hfp->msg = NULL;
+
+	ofono_info("Service level connection established");
+}
+
+static void slc_failed(gpointer userdata)
+{
+	struct ofono_modem *modem = userdata;
+	struct hfp *hfp = ofono_modem_get_data(modem);
+	DBusMessage *msg;
+
+	msg = g_dbus_create_error(hfp->msg, BLUEZ_ERROR_INTERFACE
+						".Failed",
+						"HFP Handshake failed");
+
+	g_dbus_send_message(ofono_dbus_get_connection(), msg);
+	dbus_message_unref(hfp->msg);
+	hfp->msg = NULL;
+
+	ofono_error("Service level connection failed");
+	ofono_modem_set_powered(modem, FALSE);
+
+	hfp_slc_info_free(&hfp->info);
+}
+
+static void hfp_disconnected_cb(gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct hfp *hfp = ofono_modem_get_data(modem);
+
+	DBG("HFP disconnected");
+
+	hfp_slc_info_free(&hfp->info);
+
+	ofono_modem_set_powered(modem, FALSE);
+}
+
+static GIOChannel *service_level_connection(struct ofono_modem *modem,
+							int fd, int *err)
+{
+	struct hfp *hfp = ofono_modem_get_data(modem);
+	GIOChannel *io;
+	GAtSyntax *syntax;
+	GAtChat *chat;
+
+	io = g_io_channel_unix_new(fd);
+	if (io == NULL) {
+		ofono_error("Service level connection failed: %s (%d)",
+						strerror(errno), errno);
+		*err = -EIO;
+		return NULL;
+	}
+
+	syntax = g_at_syntax_new_gsm_permissive();
+	chat = g_at_chat_new(io, syntax);
+	g_at_syntax_unref(syntax);
+	g_io_channel_set_close_on_unref(io, TRUE);
+
+	if (chat == NULL) {
+		*err = -ENOMEM;
+		goto fail;
+	}
+
+	g_at_chat_set_disconnect_function(chat, hfp_disconnected_cb, modem);
+
+	if (getenv("OFONO_AT_DEBUG"))
+		g_at_chat_set_debug(chat, hfp_debug, "");
+
+	hfp->info.chat = chat;
+	hfp_slc_establish(&hfp->info, slc_established, slc_failed, modem);
+
+	*err = -EINPROGRESS;
+
+	return io;
+
+fail:
+	g_io_channel_unref(io);
+	return NULL;
+}
+
 static int modem_register(struct hfp *hfp, const char *alias, int fd,
 							guint16 version)
 {
 	struct ofono_modem *modem;
-	int err = 0;
+	guint8 codecs[1];
 	char *path;
+	int err;
 
 	path = g_strconcat("hfp", hfp->device_path, NULL);
 
@@ -112,6 +216,13 @@ static int modem_register(struct hfp *hfp, const char *alias, int fd,
 
 	g_hash_table_insert(modem_hash, g_strdup(hfp->device_path), modem);
 
+	memset(codecs, 0, sizeof(codecs));
+	codecs[0] = HFP_CODEC_CVSD;
+
+	hfp_slc_info_init(&hfp->info, version, codecs, 1);
+
+	hfp->slcio = service_level_connection(modem, fd, &err);
+
 	return err;
 }
 
-- 
1.7.11.7

[PATCH v2 10/10] hfp_hf: Add support for codec negotiation

[Claudio Takahasi]

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

From: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>

The negotiation is very simple, if the codec the AG requests is present
in the supported codec list that the HF sent, the codec is accepted, if
not, we send the list of supported codecs again, and expect that the AG
re-sends the request with an acceptable codec.
---
 plugins/hfp_hf_bluez5.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
index 17421d0..d8ec0f2 100644
--- a/plugins/hfp_hf_bluez5.c
+++ b/plugins/hfp_hf_bluez5.c
@@ -25,6 +25,7 @@
 
 #include <errno.h>
 #include <stdlib.h>
+#include <stdio.h>
 #include <string.h>
 
 #include <glib.h>
@@ -64,6 +65,7 @@ struct hfp {
 	char *device_address;
 	char *device_path;
 	struct hfp_slc_info info;
+	guint8 current_codec;
 	DBusMessage *msg;
 	GIOChannel *slcio;
 	GSList *endpoints;
@@ -102,12 +104,59 @@ static void hfp_debug(const char *str, void *user_data)
 	ofono_info("%s%s", prefix, str);
 }
 
+static void bcs_notify(GAtResult *result, gpointer user_data)
+{
+	struct hfp *hfp = user_data;
+	struct hfp_slc_info *info = &hfp->info;
+	GAtResultIter iter;
+	GString *str;
+	int i, value;
+
+	g_at_result_iter_init(&iter, result);
+
+	if (!g_at_result_iter_next(&iter, "+BCS:"))
+		return;
+
+	if (!g_at_result_iter_next_number(&iter, &value))
+		return;
+
+	/* If some codec matches, we confirm it */
+	for (i = 0; i < info->codecs_len; i++) {
+		if (info->codecs[i] == value) {
+			char buf[32];
+
+			hfp->current_codec = value;
+			DBG("Negotiated HFP codec: %d", value);
+
+			snprintf(buf, sizeof(buf), "AT+BCS=%d", value);
+			g_at_chat_send(info->chat, buf, NULL, NULL,
+							NULL, NULL);
+			return;
+		}
+	}
+
+	/* If none matches, we send our supported codecs again */
+	str = g_string_new("AT+BAC=");
+
+	for (i = 0; i < info->codecs_len; i++) {
+		g_string_append_printf(str, "%d", info->codecs[i]);
+		if (i + 1 < info->codecs_len)
+			str = g_string_append(str, ",");
+	}
+
+	g_at_chat_send(info->chat, str->str, NULL, NULL, NULL, NULL);
+	g_string_free(str, TRUE);
+}
+
 static void slc_established(gpointer userdata)
 {
 	struct ofono_modem *modem = userdata;
 	struct hfp *hfp = ofono_modem_get_data(modem);
+	struct hfp_slc_info *info = &hfp->info;
 	DBusMessage *msg;
 
+	g_at_chat_register(info->chat, "+BCS:", bcs_notify, FALSE, hfp, NULL);
+
 	ofono_modem_set_powered(modem, TRUE);
 
 	msg = dbus_message_new_method_return(hfp->msg);
@@ -436,6 +485,7 @@ static DBusMessage *profile_new_connection(DBusConnection *conn,
 	hfp->device_path = g_strdup(device);
 	hfp->endpoints = endpoints;
 	hfp->msg = dbus_message_ref(msg);
+	hfp->current_codec = HFP_CODEC_CVSD;
 
 	err = modem_register(hfp, "unknown", fd, version);
 	if (err < 0 && err != -EINPROGRESS) {
-- 
1.7.11.7

Re: [PATCH v2 00/10] HFP HF: Service Level/Codec negotiation

[Vinicius Costa Gomes]

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

Hi,

On 20:38 Fri 18 Jan, Claudio Takahasi wrote:
> Second group of patches related to HFP external profile.
> 
> The following patches contain the HFP 1.6 codec negotiation.
> 

Please ignore this series.

As discussed on IRC, it was agreed that the series should follow a more
logical flow. And so, things that were present in later patches were moved
to earlier ones, for example, the use of GDBusProxy.

The next series should appear on the list in a few moments, with things
related to the HFP modem registration and SLC establishment.


Cheeers,
-- 
Vinicius