[PATCH v2 1/3] bluez5: Unify bt_register_profile_* into a single function

[PATCH v2 1/3] bluez5: Unify bt_register_profile_* into a single function

[Vinicius Costa Gomes]

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

It was a cause of confusion for little gain to have separate
functions for registering profiles with extra information, for
example "role" and "features". We remove those helper functions
in favor of a single one with more parameters, "role" and
"feature" when NULL, will be ignored.
---
 dundee/bluez5.c         |  4 ++--
 plugins/bluez5.c        | 17 +++++++----------
 plugins/bluez5.h        |  7 ++-----
 plugins/dun_gw_bluez5.c |  2 +-
 plugins/hfp_ag_bluez5.c |  2 +-
 plugins/hfp_hf_bluez5.c |  2 +-
 6 files changed, 14 insertions(+), 20 deletions(-)

diff --git a/dundee/bluez5.c b/dundee/bluez5.c
index 3f9c646..79b5ade 100644
--- a/dundee/bluez5.c
+++ b/dundee/bluez5.c
@@ -379,8 +379,8 @@ static void connect_handler(DBusConnection *conn, void *user_data)
 {
 	DBG("");
 
-	bt_register_profile_with_role(conn, DUN_GW_UUID, DUN_VERSION_1_2,
-				"dun_dt", DUN_DT_PROFILE_PATH, "client");
+	bt_register_profile(conn, DUN_GW_UUID, DUN_VERSION_1_2, "dun_dt",
+					DUN_DT_PROFILE_PATH, "client", 0);
 }
 
 int __dundee_bluetooth_init(void)
diff --git a/plugins/bluez5.c b/plugins/bluez5.c
index 0f997da..979076e 100644
--- a/plugins/bluez5.c
+++ b/plugins/bluez5.c
@@ -90,9 +90,10 @@ done:
 	dbus_message_unref(reply);
 }
 
-int bt_register_profile_with_role(DBusConnection *conn, const char *uuid,
+int bt_register_profile(DBusConnection *conn, const char *uuid,
 					uint16_t version, const char *name,
-					const char *object, const char *role)
+					const char *object, const char *role,
+					uint16_t features)
 {
 	DBusMessageIter iter, dict;
 	DBusPendingCall *c;
@@ -114,6 +115,10 @@ int bt_register_profile_with_role(DBusConnection *conn, const char *uuid,
 	if (role)
 		ofono_dbus_dict_append(&dict, "Role", DBUS_TYPE_STRING, &role);
 
+	if (features)
+		ofono_dbus_dict_append(&dict, "Features", DBUS_TYPE_UINT16,
+								&features);
+
 	dbus_message_iter_close_container(&iter, &dict);
 
 	if (!dbus_connection_send_with_reply(conn, msg, &c, -1)) {
@@ -130,14 +135,6 @@ int bt_register_profile_with_role(DBusConnection *conn, const char *uuid,
 	return 0;
 }
 
-int bt_register_profile(DBusConnection *conn, const char *uuid,
-					uint16_t version, const char *name,
-							const char *object)
-{
-	return bt_register_profile_with_role(conn, uuid, version, name, object,
-									NULL);
-}
-
 void bt_unregister_profile(DBusConnection *conn, const char *object)
 {
 	DBusMessageIter iter;
diff --git a/plugins/bluez5.h b/plugins/bluez5.h
index 5b8c1f5..db24b1a 100644
--- a/plugins/bluez5.h
+++ b/plugins/bluez5.h
@@ -29,13 +29,10 @@
 #define HFP_HS_UUID	"0000111e-0000-1000-8000-00805f9b34fb"
 #define HFP_AG_UUID	"0000111f-0000-1000-8000-00805f9b34fb"
 
-int bt_register_profile_with_role(DBusConnection *conn, const char *uuid,
-					uint16_t version, const char *name,
-					const char *object, const char *role);
-
 int bt_register_profile(DBusConnection *conn, const char *uuid,
 					uint16_t version, const char *name,
-					const char *object);
+					const char *object, const char *role,
+					uint16_t features);
 
 void bt_unregister_profile(DBusConnection *conn, const char *object);
 
diff --git a/plugins/dun_gw_bluez5.c b/plugins/dun_gw_bluez5.c
index 9844092..faea12b 100644
--- a/plugins/dun_gw_bluez5.c
+++ b/plugins/dun_gw_bluez5.c
@@ -172,7 +172,7 @@ static void gprs_watch(struct ofono_atom *atom,
 
 	if (modems->next == NULL)
 		bt_register_profile(conn, DUN_GW_UUID, DUN_GW_VERSION_1_0,
-					"dun_gw", DUN_GW_EXT_PROFILE_PATH);
+				"dun_gw", DUN_GW_EXT_PROFILE_PATH, NULL, 0);
 }
 
 static void modem_watch(struct ofono_modem *modem, gboolean added, void *user)
diff --git a/plugins/hfp_ag_bluez5.c b/plugins/hfp_ag_bluez5.c
index 59e84d2..245de21 100644
--- a/plugins/hfp_ag_bluez5.c
+++ b/plugins/hfp_ag_bluez5.c
@@ -233,7 +233,7 @@ static void sim_state_watch(enum ofono_sim_state new_state, void *data)
 		return;
 
 	bt_register_profile(conn, HFP_AG_UUID, HFP_VERSION_1_5, "hfp_ag",
-						HFP_AG_EXT_PROFILE_PATH);
+					HFP_AG_EXT_PROFILE_PATH, NULL, 0);
 }
 
 static gboolean sim_watch_remove(gpointer key, gpointer value,
diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
index 2b9275b..826796f 100644
--- a/plugins/hfp_hf_bluez5.c
+++ b/plugins/hfp_hf_bluez5.c
@@ -639,7 +639,7 @@ static void connect_handler(DBusConnection *conn, void *user_data)
 	DBG("Registering External Profile handler ...");
 
 	bt_register_profile(conn, HFP_HS_UUID, HFP_VERSION_1_6, "hfp_hf",
-						HFP_EXT_PROFILE_PATH);
+					HFP_EXT_PROFILE_PATH, NULL, 0);
 }
 
 static gboolean has_hfp_ag_uuid(DBusMessageIter *array)
-- 
1.8.2.1

[PATCH v2 2/3] hfp: Add defines for HFP SDP feature bits

[Vinicius Costa Gomes]

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

---
 src/hfp.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/src/hfp.h b/src/hfp.h
index b54de56..0d36ae5 100644
--- a/src/hfp.h
+++ b/src/hfp.h
@@ -45,6 +45,24 @@ enum hfp_hf_feature {
 	HFP_HF_FEATURE_CODEC_NEGOTIATION =	0x80,
 };
 
+enum hfp_sdp_hf_features {
+	HFP_SDP_HF_FEATURE_ECNR =			0x1,
+	HFP_SDP_HF_FEATURE_3WAY =			0x2,
+	HFP_SDP_HF_FEATURE_CLIP =			0x4,
+	HFP_SDP_HF_FEATURE_VOICE_RECOGNITION =		0x8,
+	HFP_SDP_HF_FEATURE_REMOTE_VOLUME_CONTROL =	0x10,
+	HFP_SDP_HF_FEATURE_WIDEBAND_SPEECH =		0x20,
+};
+
+enum hfp_sdp_ag_features {
+	HFP_SDP_AG_FEATURE_3WAY =			0x1,
+	HFP_SDP_AG_FEATURE_ECNR =			0x2,
+	HFP_SDP_AG_FEATURE_VOICE_RECOG =		0x4,
+	HFP_SDP_AG_FEATURE_IN_BAND_RING_TONE =		0x8,
+	HFP_SDP_AG_FEATURE_ATTACH_VOICE_TAG =		0x10,
+	HFP_SDP_AG_FEATURE_WIDEBAND_SPEECH =		0x20,
+};
+
 /* Supported agent codecs */
 enum hfp_codec {
 	HFP_CODEC_CVSD = 0x01,
-- 
1.8.2.1

[PATCH v2 3/3] hfp_hf_bluez5: Register the SDP record with correct features

[Vinicius Costa Gomes]

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

---
 plugins/hfp_hf_bluez5.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
index 826796f..06b6fd3 100644
--- a/plugins/hfp_hf_bluez5.c
+++ b/plugins/hfp_hf_bluez5.c
@@ -636,10 +636,15 @@ static const GDBusMethodTable profile_methods[] = {
 
 static void connect_handler(DBusConnection *conn, void *user_data)
 {
+	uint16_t features = HFP_SDP_HF_FEATURE_3WAY |
+				HFP_SDP_HF_FEATURE_CLIP |
+				HFP_SDP_HF_FEATURE_REMOTE_VOLUME_CONTROL |
+				HFP_SDP_HF_FEATURE_WIDEBAND_SPEECH;
+
 	DBG("Registering External Profile handler ...");
 
 	bt_register_profile(conn, HFP_HS_UUID, HFP_VERSION_1_6, "hfp_hf",
-					HFP_EXT_PROFILE_PATH, NULL, 0);
+					HFP_EXT_PROFILE_PATH, NULL, features);
 }
 
 static gboolean has_hfp_ag_uuid(DBusMessageIter *array)
-- 
1.8.2.1

Re: [PATCH v2 1/3] bluez5: Unify bt_register_profile_* into a single function

[Denis Kenzior]

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

Hi Vinicius,

On 04/23/2013 04:21 PM, Vinicius Costa Gomes wrote:
> It was a cause of confusion for little gain to have separate
> functions for registering profiles with extra information, for
> example "role" and "features". We remove those helper functions
> in favor of a single one with more parameters, "role" and
> "feature" when NULL, will be ignored.
> ---
>   dundee/bluez5.c         |  4 ++--
>   plugins/bluez5.c        | 17 +++++++----------
>   plugins/bluez5.h        |  7 ++-----
>   plugins/dun_gw_bluez5.c |  2 +-
>   plugins/hfp_ag_bluez5.c |  2 +-
>   plugins/hfp_hf_bluez5.c |  2 +-
>   6 files changed, 14 insertions(+), 20 deletions(-)
>

Patch has been applied, thanks.

Regards,
-Denis

Re: [PATCH v2 2/3] hfp: Add defines for HFP SDP feature bits

[Denis Kenzior]

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

Hi Vinicius,

On 04/23/2013 04:21 PM, Vinicius Costa Gomes wrote:
> ---
>   src/hfp.h | 18 ++++++++++++++++++
>   1 file changed, 18 insertions(+)
>

Patch has been applied, thanks.

Regards,
-Denis

Re: [PATCH v2 3/3] hfp_hf_bluez5: Register the SDP record with correct features

[Denis Kenzior]

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

Hi Vinicius,

On 04/23/2013 04:21 PM, Vinicius Costa Gomes wrote:
> ---
>   plugins/hfp_hf_bluez5.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
>

Patch has been applied, thanks.

> diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
> index 826796f..06b6fd3 100644
> --- a/plugins/hfp_hf_bluez5.c
> +++ b/plugins/hfp_hf_bluez5.c
> @@ -636,10 +636,15 @@ static const GDBusMethodTable profile_methods[] = {
>
>   static void connect_handler(DBusConnection *conn, void *user_data)
>   {
> +	uint16_t features = HFP_SDP_HF_FEATURE_3WAY |
> +				HFP_SDP_HF_FEATURE_CLIP |
> +				HFP_SDP_HF_FEATURE_REMOTE_VOLUME_CONTROL |
> +				HFP_SDP_HF_FEATURE_WIDEBAND_SPEECH;
> +

Can we check whether the kernel supports wideband speech features here 
and not set this flag if it does not?

>   	DBG("Registering External Profile handler ...");
>
>   	bt_register_profile(conn, HFP_HS_UUID, HFP_VERSION_1_6, "hfp_hf",
> -					HFP_EXT_PROFILE_PATH, NULL, 0);
> +					HFP_EXT_PROFILE_PATH, NULL, features);
>   }
>
>   static gboolean has_hfp_ag_uuid(DBusMessageIter *array)

Regards,
-Denis

Re: [PATCH v2 3/3] hfp_hf_bluez5: Register the SDP record with correct features

[Vinicius Costa Gomes]

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

Hi Denis,

On 03:41 Thu 25 Apr, Denis Kenzior wrote:
> Hi Vinicius,
> 
> On 04/23/2013 04:21 PM, Vinicius Costa Gomes wrote:
> >---
> >  plugins/hfp_hf_bluez5.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> 
> Patch has been applied, thanks.
> 
> >diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
> >index 826796f..06b6fd3 100644
> >--- a/plugins/hfp_hf_bluez5.c
> >+++ b/plugins/hfp_hf_bluez5.c
> >@@ -636,10 +636,15 @@ static const GDBusMethodTable profile_methods[] = {
> >
> >  static void connect_handler(DBusConnection *conn, void *user_data)
> >  {
> >+	uint16_t features = HFP_SDP_HF_FEATURE_3WAY |
> >+				HFP_SDP_HF_FEATURE_CLIP |
> >+				HFP_SDP_HF_FEATURE_REMOTE_VOLUME_CONTROL |
> >+				HFP_SDP_HF_FEATURE_WIDEBAND_SPEECH;
> >+
> 
> Can we check whether the kernel supports wideband speech features
> here and not set this flag if it does not?

What we can do is exporting something like
ofono_handsfree_audio_has_defer_setup() or something like it, and set the
feature bit if it's available. I send the patches and let's see how it looks.

The other alternative would be only registering the profile after the Agent
registers, that way we can be sure that wideband speech is supported. But it
may bring little benefit, for example, the other side may cache the SDP search
results.

> 
> >  	DBG("Registering External Profile handler ...");
> >
> >  	bt_register_profile(conn, HFP_HS_UUID, HFP_VERSION_1_6, "hfp_hf",
> >-					HFP_EXT_PROFILE_PATH, NULL, 0);
> >+					HFP_EXT_PROFILE_PATH, NULL, features);
> >  }
> >
> >  static gboolean has_hfp_ag_uuid(DBusMessageIter *array)
> 
> Regards,
> -Denis


Cheers,
-- 
Vinicius