Re: [PATCH v0 1/7] doc: Add ECNR to Handsfree

Re: [PATCH v0 1/7] doc: Add ECNR to Handsfree

[Denis Kenzior]

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

Hi Claudio,

On 04/19/2013 12:34 PM, Claudio Takahasi wrote:
> This patch adds echo canceling and noise reduction property to Handsfree
> interface, allowing the handsfree unit to enable or disable this feature
> in the audio gateway.
>
> According to Bluetooth HFP 1.6 spec: By default, if the AG supports its
> own embedded echo canceling and/or noise reduction functions, it shall
> have them activated until the AT+NREC command is received.
> ---
>   doc/handsfree-api.txt | 7 +++++++
>   1 file changed, 7 insertions(+)
>
> diff --git a/doc/handsfree-api.txt b/doc/handsfree-api.txt
> index 0b5d421..f877db0 100644
> --- a/doc/handsfree-api.txt
> +++ b/doc/handsfree-api.txt
> @@ -56,6 +56,13 @@ Properties	array{string} Features [readonly]
>   			to activate or deactivate the function from the HF, or
>   			the AG could autonomously initiate it.
>
> +		boolean ECNR [readwrite]
> +
> +			Boolean representing whether echo canceling and noise
> +			reduction is enabled in the AG. This property may be
> +			written to activate or deactivate this feature from
> +			the HF unit.
> +

I really prefer we used full words instead of an acronym.  You'll find 
we have no acronyms in the oFono APIs.

>   		byte BatteryChargeLevel [readonly]
>
>   			The current charge level of the battery.  The value

Regards,
-Denis

[PATCH v0 1/7] doc: Add ECNR to Handsfree

[Claudio Takahasi]

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

This patch adds echo canceling and noise reduction property to Handsfree
interface, allowing the handsfree unit to enable or disable this feature
in the audio gateway.

According to Bluetooth HFP 1.6 spec: By default, if the AG supports its
own embedded echo canceling and/or noise reduction functions, it shall
have them activated until the AT+NREC command is received.
---
 doc/handsfree-api.txt | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/doc/handsfree-api.txt b/doc/handsfree-api.txt
index 0b5d421..f877db0 100644
--- a/doc/handsfree-api.txt
+++ b/doc/handsfree-api.txt
@@ -56,6 +56,13 @@ Properties	array{string} Features [readonly]
 			to activate or deactivate the function from the HF, or
 			the AG could autonomously initiate it.
 
+		boolean ECNR [readwrite]
+
+			Boolean representing whether echo canceling and noise
+			reduction is enabled in the AG. This property may be
+			written to activate or deactivate this feature from
+			the HF unit.
+
 		byte BatteryChargeLevel [readonly]
 
 			The current charge level of the battery.  The value
-- 
1.7.11.7

[PATCH v0 2/7] core: Add ECNR to Handsfree Features

[Claudio Takahasi]

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

This patch adds "echo-canceling-and-noise-reduction" to "Features"
property of the Handsfree interface.
---
 src/handsfree.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/handsfree.c b/src/handsfree.c
index 8a0374d..6d7f263 100644
--- a/src/handsfree.c
+++ b/src/handsfree.c
@@ -62,6 +62,9 @@ static const char **ag_features_list(unsigned int features)
 	static const char *list[33];
 	unsigned int i = 0;
 
+	if (features & HFP_AG_FEATURE_ECNR)
+		list[i++] = "echo-canceling-and-noise-reduction";
+
 	if (features & HFP_AG_FEATURE_VOICE_RECOG)
 		list[i++] = "voice-recognition";
 
-- 
1.7.11.7

[PATCH v0 3/7] core: Add ECNR in Handsfree GetProperty

[Claudio Takahasi]

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

---
 src/handsfree.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/handsfree.c b/src/handsfree.c
index 6d7f263..041d3f2 100644
--- a/src/handsfree.c
+++ b/src/handsfree.c
@@ -45,6 +45,7 @@
 static GSList *g_drivers = NULL;
 
 struct ofono_handsfree {
+	ofono_bool_t ecnr;
 	ofono_bool_t inband_ringing;
 	ofono_bool_t voice_recognition;
 	ofono_bool_t voice_recognition_pending;
@@ -175,6 +176,10 @@ static DBusMessage *handsfree_get_properties(DBusConnection *conn,
 	ofono_dbus_dict_append(&dict, "InbandRinging", DBUS_TYPE_BOOLEAN,
 				&inband_ringing);
 
+	if (hf->ag_features & HFP_AG_FEATURE_ECNR)
+		ofono_dbus_dict_append(&dict, "ECNR", DBUS_TYPE_BOOLEAN,
+								&hf->ecnr);
+
 	voice_recognition = hf->voice_recognition;
 	ofono_dbus_dict_append(&dict, "VoiceRecognition", DBUS_TYPE_BOOLEAN,
 				&voice_recognition);
-- 
1.7.11.7

[PATCH v0 4/7] core: Set ECNR to TRUE by default

[Claudio Takahasi]

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

According to Bluetooth HFP spec: By default, if the AG supports its
own embedded echo canceling and/or noise reduction functions, it shall
have them activated until the AT+NREC command is received. The
configuration set by the HF shall by used by the AG while the Service
Level Connection is active.

Since there isn't a command to query the current value, it is being
assumed that ECNR is enabled when the connection is established and
the gateway supports ECNR feature.
---
 src/handsfree.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/handsfree.c b/src/handsfree.c
index 041d3f2..31f2cea 100644
--- a/src/handsfree.c
+++ b/src/handsfree.c
@@ -361,6 +361,7 @@ struct ofono_handsfree *ofono_handsfree_create(struct ofono_modem *modem,
 	hf->atom = __ofono_modem_add_atom(modem,
 					OFONO_ATOM_TYPE_HANDSFREE,
 					handsfree_remove, hf);
+	hf->ecnr = TRUE;
 
 	for (l = g_drivers; l; l = l->next) {
 		const struct ofono_handsfree_driver *drv = l->data;
-- 
1.7.11.7

[PATCH v0 5/7] include: Add ECNR to handsfree driver

[Claudio Takahasi]

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

This patch extends the handsfree driver adding echo canceling and noise
reduction callback to allow enabling or disabling this feature in the
audio gateway.
---
 include/handsfree.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/handsfree.h b/include/handsfree.h
index b5e0d5f..16d9be5 100644
--- a/include/handsfree.h
+++ b/include/handsfree.h
@@ -48,6 +48,9 @@ struct ofono_handsfree_driver {
 	void (*voice_recognition)(struct ofono_handsfree *hf,
 					ofono_bool_t enabled,
 					ofono_handsfree_cb_t cb, void *data);
+	void (*ecnr)(struct ofono_handsfree *hf,
+					ofono_bool_t enabled,
+					ofono_handsfree_cb_t cb, void *data);
 };
 
 void ofono_handsfree_set_ag_features(struct ofono_handsfree *hf,
-- 
1.7.11.7

[PATCH v0 6/7] hfpmodem: Add ECNR implementation

[Claudio Takahasi]

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

This patch adds echo canceling and noise reduction implementation to
hfpmodem.
---
 drivers/hfpmodem/handsfree.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/hfpmodem/handsfree.c b/drivers/hfpmodem/handsfree.c
index ce28de3..239bcf3 100644
--- a/drivers/hfpmodem/handsfree.c
+++ b/drivers/hfpmodem/handsfree.c
@@ -260,12 +260,31 @@ static void hfp_voice_recognition(struct ofono_handsfree *hf,
 	CALLBACK_WITH_FAILURE(cb, data);
 }
 
+static void hfp_ecnr(struct ofono_handsfree *hf, ofono_bool_t enabled,
+					ofono_handsfree_cb_t cb, void *data)
+{
+	struct hf_data *hd = ofono_handsfree_get_data(hf);
+	struct cb_data *cbd = cb_data_new(cb, data);
+	char buf[16];
+
+	snprintf(buf, sizeof(buf), "AT+NREC=%d", (int)(enabled));
+
+	if (g_at_chat_send(hd->chat, buf, NULL, hf_generic_set_cb,
+							cbd, g_free) > 0)
+		return;
+
+	g_free(cbd);
+
+	CALLBACK_WITH_FAILURE(cb, data);
+}
+
 static struct ofono_handsfree_driver driver = {
 	.name			= "hfpmodem",
 	.probe			= hfp_handsfree_probe,
 	.remove			= hfp_handsfree_remove,
 	.request_phone_number	= hfp_request_phone_number,
 	.voice_recognition	= hfp_voice_recognition,
+	.ecnr			= hfp_ecnr,
 };
 
 void hfp_handsfree_init(void)
-- 
1.7.11.7

[PATCH v0 7/7] core: Add SetProperty for ECNR

[Claudio Takahasi]

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

This patch extends SetProperty method to allow enabling or disabling the
echo canceling and noise reduction feature in the audio gateway through
a D-Bus method call.
---
 src/handsfree.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 47 insertions(+), 9 deletions(-)

diff --git a/src/handsfree.c b/src/handsfree.c
index 31f2cea..4beef44 100644
--- a/src/handsfree.c
+++ b/src/handsfree.c
@@ -46,6 +46,7 @@ static GSList *g_drivers = NULL;
 
 struct ofono_handsfree {
 	ofono_bool_t ecnr;
+	ofono_bool_t ecnr_pending;
 	ofono_bool_t inband_ringing;
 	ofono_bool_t voice_recognition;
 	ofono_bool_t voice_recognition_pending;
@@ -220,11 +221,36 @@ static void voicerec_set_cb(const struct ofono_error *error, void *data)
 					&hf->voice_recognition);
 }
 
+static void ecnr_set_cb(const struct ofono_error *error, void *data)
+{
+	struct ofono_handsfree *hf = data;
+	DBusConnection *conn = ofono_dbus_get_connection();
+	const char *path = __ofono_atom_get_path(hf->atom);
+
+	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
+		__ofono_dbus_pending_reply(&hf->pending,
+					__ofono_error_failed(hf->pending));
+		return;
+	}
+
+	hf->ecnr = hf->ecnr_pending;
+
+	__ofono_dbus_pending_reply(&hf->pending,
+				dbus_message_new_method_return(hf->pending));
+
+	ofono_dbus_signal_property_changed(conn, path,
+					OFONO_HANDSFREE_INTERFACE,
+					"ECNR",
+					DBUS_TYPE_BOOLEAN,
+					&hf->ecnr);
+}
+
 static DBusMessage *handsfree_set_property(DBusConnection *conn,
 						DBusMessage *msg, void *data)
 {
 	struct ofono_handsfree *hf = data;
 	DBusMessageIter iter, var;
+	ofono_bool_t enabled;
 	const char *name;
 
 	if (hf->pending)
@@ -244,28 +270,40 @@ static DBusMessage *handsfree_set_property(DBusConnection *conn,
 
 	dbus_message_iter_recurse(&iter, &var);
 
+	if (dbus_message_iter_get_arg_type(&var) != DBUS_TYPE_BOOLEAN)
+		return __ofono_error_invalid_args(msg);
+
+	dbus_message_iter_get_basic(&var, &enabled);
+
 	if (g_str_equal(name, "VoiceRecognition") == TRUE) {
-		ofono_bool_t enabled;
 
 		if (!hf->driver->voice_recognition)
 			return __ofono_error_not_implemented(msg);
 
-		if (dbus_message_iter_get_arg_type(&var) != DBUS_TYPE_BOOLEAN)
-			return __ofono_error_invalid_args(msg);
-
-		dbus_message_iter_get_basic(&var, &enabled);
-
 		if (hf->voice_recognition == enabled)
 			return dbus_message_new_method_return(msg);
 
 		hf->voice_recognition_pending = enabled;
 		hf->pending = dbus_message_ref(msg);
 		hf->driver->voice_recognition(hf, enabled, voicerec_set_cb, hf);
+	} else if (g_str_equal(name, "ECNR") == TRUE) {
 
-		return NULL;
-	}
+		if (!(hf->ag_features & HFP_AG_FEATURE_ECNR))
+			return __ofono_error_not_supported(msg);
 
-	return __ofono_error_invalid_args(msg);
+		if (!hf->driver->ecnr)
+			return __ofono_error_not_implemented(msg);
+
+		if (hf->ecnr == enabled)
+			return dbus_message_new_method_return(msg);
+
+		hf->pending = dbus_message_ref(msg);
+		hf->ecnr_pending = enabled;
+		hf->driver->ecnr(hf, enabled, ecnr_set_cb, hf);
+	} else
+		return __ofono_error_invalid_args(msg);
+
+	return NULL;
 }
 
 static void request_phone_number_cb(const struct ofono_error *error,
-- 
1.7.11.7

Re: [PATCH v0 1/7] doc: Add ECNR to Handsfree

[Denis Kenzior]

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

Hi Claudio,

>> I really prefer we used full words instead of an acronym.  You'll find we
>> have no acronyms in the oFono APIs.
>
> The feature's name doesn't help, which name do you prefer?
>
> * EchoCancelingAndNoiseReduction
> * EchoNoise
>
> Any other suggestion?
>

I'd go for EchoCancelingNoiseReduction

Regards,
-Denis

Re: [PATCH v0 1/7] doc: Add ECNR to Handsfree

[Johan Hedberg]

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

Hi Claudio,

On Fri, Apr 19, 2013, Claudio Takahasi wrote:
> This patch adds echo canceling and noise reduction property to Handsfree
> interface, allowing the handsfree unit to enable or disable this feature
> in the audio gateway.
> 
> According to Bluetooth HFP 1.6 spec: By default, if the AG supports its
> own embedded echo canceling and/or noise reduction functions, it shall
> have them activated until the AT+NREC command is received.
> ---
>  doc/handsfree-api.txt | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/doc/handsfree-api.txt b/doc/handsfree-api.txt
> index 0b5d421..f877db0 100644
> --- a/doc/handsfree-api.txt
> +++ b/doc/handsfree-api.txt
> @@ -56,6 +56,13 @@ Properties	array{string} Features [readonly]
>  			to activate or deactivate the function from the HF, or
>  			the AG could autonomously initiate it.
>  
> +		boolean ECNR [readwrite]
> +
> +			Boolean representing whether echo canceling and noise
> +			reduction is enabled in the AG. This property may be
> +			written to activate or deactivate this feature from
> +			the HF unit.

Why would we want to have two different acronyms, NREC and ECNR, for the
same thing? We can't get rid of what's defined in the AT command
specification, so why add more confusion by inventing a second acronym?

Johan

Re: [PATCH v0 1/7] doc: Add ECNR to Handsfree

[Claudio Takahasi]

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

Hi Johan:

On Fri, Apr 19, 2013 at 5:11 PM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Claudio,
>
> On Fri, Apr 19, 2013, Claudio Takahasi wrote:
>> This patch adds echo canceling and noise reduction property to Handsfree
>> interface, allowing the handsfree unit to enable or disable this feature
>> in the audio gateway.
>>
>> According to Bluetooth HFP 1.6 spec: By default, if the AG supports its
>> own embedded echo canceling and/or noise reduction functions, it shall
>> have them activated until the AT+NREC command is received.
>> ---
>>  doc/handsfree-api.txt | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/doc/handsfree-api.txt b/doc/handsfree-api.txt
>> index 0b5d421..f877db0 100644
>> --- a/doc/handsfree-api.txt
>> +++ b/doc/handsfree-api.txt
>> @@ -56,6 +56,13 @@ Properties array{string} Features [readonly]
>>                       to activate or deactivate the function from the HF, or
>>                       the AG could autonomously initiate it.
>>
>> +             boolean ECNR [readwrite]
>> +
>> +                     Boolean representing whether echo canceling and noise
>> +                     reduction is enabled in the AG. This property may be
>> +                     written to activate or deactivate this feature from
>> +                     the HF unit.
>
> Why would we want to have two different acronyms, NREC and ECNR, for the
> same thing? We can't get rid of what's defined in the AT command
> specification, so why add more confusion by inventing a second acronym?
>
> Johan

I did myself the same question. I started the implementation using
NREC, but I realized later that oFono uses ECNR: HFP_AG_FEATURE_ECNR.
The HFP spec also mention "echo canceling and noise reduction".

If everybody agrees I can rename it to NREC, no problem.

Regards,
Claudio.

Re: [PATCH v0 1/7] doc: Add ECNR to Handsfree

[Claudio Takahasi]

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

Hi Denis/Johan:

On Fri, Apr 19, 2013 at 12:24 PM, Denis Kenzior <denkenz@gmail.com> wrote:
> Hi Claudio,
>
>
> On 04/19/2013 12:34 PM, Claudio Takahasi wrote:
>>
>> This patch adds echo canceling and noise reduction property to Handsfree
>> interface, allowing the handsfree unit to enable or disable this feature
>> in the audio gateway.
>>
>> According to Bluetooth HFP 1.6 spec: By default, if the AG supports its
>> own embedded echo canceling and/or noise reduction functions, it shall
>> have them activated until the AT+NREC command is received.
>> ---
>>   doc/handsfree-api.txt | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/doc/handsfree-api.txt b/doc/handsfree-api.txt
>> index 0b5d421..f877db0 100644
>> --- a/doc/handsfree-api.txt
>> +++ b/doc/handsfree-api.txt
>> @@ -56,6 +56,13 @@ Properties   array{string} Features [readonly]
>>                         to activate or deactivate the function from the
>> HF, or
>>                         the AG could autonomously initiate it.
>>
>> +               boolean ECNR [readwrite]
>> +
>> +                       Boolean representing whether echo canceling and
>> noise
>> +                       reduction is enabled in the AG. This property may
>> be
>> +                       written to activate or deactivate this feature
>> from
>> +                       the HF unit.
>> +
>
>
> I really prefer we used full words instead of an acronym.  You'll find we
> have no acronyms in the oFono APIs.

The feature's name doesn't help, which name do you prefer?

* EchoCancelingAndNoiseReduction
* EchoNoise

Any other suggestion?

Regards,
Claudio

Re: [PATCH v1 5/7] include: Add EchoCancelingNoiseReduction to handsfree driver

[Denis Kenzior]

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

Hi Claudio,

On 04/22/2013 07:58 AM, Claudio Takahasi wrote:
> This patch extends the handsfree driver adding echo canceling and noise
> reduction callback to allow enabling or disabling this feature in the
> audio gateway.
> ---
>   include/handsfree.h | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/include/handsfree.h b/include/handsfree.h
> index b5e0d5f..dcc4d74 100644
> --- a/include/handsfree.h
> +++ b/include/handsfree.h
> @@ -48,6 +48,9 @@ struct ofono_handsfree_driver {
>   	void (*voice_recognition)(struct ofono_handsfree *hf,
>   					ofono_bool_t enabled,
>   					ofono_handsfree_cb_t cb, void *data);
> +	void (*echocanceling_noisereduction)(struct ofono_handsfree *hf,
> +					ofono_bool_t enabled,
> +					ofono_handsfree_cb_t cb, void *data);

You can use nrec here.  We are only pedantic about no Acronyms in the 
public D-Bus APIs.

Also note that NREC can only be turned off, not on.  Hence the enabled 
argument is likely not needed.

>   };
>
>   void ofono_handsfree_set_ag_features(struct ofono_handsfree *hf,

Regards,
-Denis

Re: [PATCH v1 1/7] doc: Add EchoCancelingNoiseReduction to Handsfree

[Denis Kenzior]

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

Hi Claudio,

On 04/22/2013 07:58 AM, Claudio Takahasi wrote:
> This patch adds echo canceling and noise reduction property to Handsfree
> interface, allowing the handsfree unit to enable or disable this feature
> in the audio gateway.
>
> According to Bluetooth HFP 1.6 spec: By default, if the AG supports its
> own embedded echo canceling and/or noise reduction functions, it shall
> have them activated until the AT+NREC command is received.
> ---
>   doc/handsfree-api.txt | 7 +++++++
>   1 file changed, 7 insertions(+)
>
> diff --git a/doc/handsfree-api.txt b/doc/handsfree-api.txt
> index 0b5d421..6855d93 100644
> --- a/doc/handsfree-api.txt
> +++ b/doc/handsfree-api.txt
> @@ -56,6 +56,13 @@ Properties	array{string} Features [readonly]
>   			to activate or deactivate the function from the HF, or
>   			the AG could autonomously initiate it.
>
> +		boolean EchoCancelingNoiseReduction [readwrite]
> +
> +			Boolean representing whether echo canceling and noise
> +			reduction is enabled in the AG. This property may be
> +			written to activate or deactivate this feature from
> +			the HF unit.
> +

This feature can only be disabled, not enabled for a given SLC.  So 
setting this to 'True' should return an error or do something way fancier.

I suspect that to implement this properly we need to store the ECNR 
setting on disk (e.g. /var/lib/ofono/<local bdaddr>/<remote 
bdaddr>/handsfree or something) and manage it appropriately inside the core.

>   		byte BatteryChargeLevel [readonly]
>
>   			The current charge level of the battery.  The value

Regards,
-Denis

Re: [PATCH v1 1/7] doc: Add EchoCancelingNoiseReduction to Handsfree

[Denis Kenzior]

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

Hi Claudio,

On 04/22/2013 11:46 AM, Claudio Takahasi wrote:
> Hi Denis:
>
> On Mon, Apr 22, 2013 at 6:04 AM, Denis Kenzior<denkenz@gmail.com>  wrote:
>> Hi Claudio,
>>
>>
>> On 04/22/2013 07:58 AM, Claudio Takahasi wrote:
>>>
>>> This patch adds echo canceling and noise reduction property to Handsfree
>>> interface, allowing the handsfree unit to enable or disable this feature
>>> in the audio gateway.
>>>
>>> According to Bluetooth HFP 1.6 spec: By default, if the AG supports its
>>> own embedded echo canceling and/or noise reduction functions, it shall
>>> have them activated until the AT+NREC command is received.
>>> ---
>>>    doc/handsfree-api.txt | 7 +++++++
>>>    1 file changed, 7 insertions(+)
>>>
>>> diff --git a/doc/handsfree-api.txt b/doc/handsfree-api.txt
>>> index 0b5d421..6855d93 100644
>>> --- a/doc/handsfree-api.txt
>>> +++ b/doc/handsfree-api.txt
>>> @@ -56,6 +56,13 @@ Properties   array{string} Features [readonly]
>>>                          to activate or deactivate the function from the
>>> HF, or
>>>                          the AG could autonomously initiate it.
>>>
>>> +               boolean EchoCancelingNoiseReduction [readwrite]
>>> +
>>> +                       Boolean representing whether echo canceling and
>>> noise
>>> +                       reduction is enabled in the AG. This property may
>>> be
>>> +                       written to activate or deactivate this feature
>>> from
>>> +                       the HF unit.
>>> +
>>
>>
>> This feature can only be disabled, not enabled for a given SLC.  So setting
>> this to 'True' should return an error or do something way fancier.
>>
>> I suspect that to implement this properly we need to store the ECNR setting
>> on disk (e.g. /var/lib/ofono/<local bdaddr>/<remote bdaddr>/handsfree or
>> something) and manage it appropriately inside the core.
>
> It is true, the HFP spec defines the procedure to disable it only.
> I implemented an option to disable it because I noticed different
> behavior between the phones/modems. Anyway, I will remove it, we can
> extend the API later if necessary.

This interface is specific to HFP, so don't worry about real modems.

>
> Regarding the storage, the spec (page 53) is a little bit confusing to
> me, I understood that the AG will enable it again when the connection
> is dropped. So, we don't need to stored it.
>

In theory we could remember the previous setting and just simply send it 
automatically the next time a device is connected.  However, your 
approach will be fine for now.

Regards,
-Denis

[PATCH v1 1/7] doc: Add EchoCancelingNoiseReduction to Handsfree

[Claudio Takahasi]

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

This patch adds echo canceling and noise reduction property to Handsfree
interface, allowing the handsfree unit to enable or disable this feature
in the audio gateway.

According to Bluetooth HFP 1.6 spec: By default, if the AG supports its
own embedded echo canceling and/or noise reduction functions, it shall
have them activated until the AT+NREC command is received.
---
 doc/handsfree-api.txt | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/doc/handsfree-api.txt b/doc/handsfree-api.txt
index 0b5d421..6855d93 100644
--- a/doc/handsfree-api.txt
+++ b/doc/handsfree-api.txt
@@ -56,6 +56,13 @@ Properties	array{string} Features [readonly]
 			to activate or deactivate the function from the HF, or
 			the AG could autonomously initiate it.
 
+		boolean EchoCancelingNoiseReduction [readwrite]
+
+			Boolean representing whether echo canceling and noise
+			reduction is enabled in the AG. This property may be
+			written to activate or deactivate this feature from
+			the HF unit.
+
 		byte BatteryChargeLevel [readonly]
 
 			The current charge level of the battery.  The value
-- 
1.7.11.7

[PATCH v1 2/7] core: Add "echo-canceling-and-noise-reduction"

[Claudio Takahasi]

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

This patch adds "echo-canceling-and-noise-reduction" to "Features"
property of the Handsfree interface.
---
 src/handsfree.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/handsfree.c b/src/handsfree.c
index 8a0374d..6d7f263 100644
--- a/src/handsfree.c
+++ b/src/handsfree.c
@@ -62,6 +62,9 @@ static const char **ag_features_list(unsigned int features)
 	static const char *list[33];
 	unsigned int i = 0;
 
+	if (features & HFP_AG_FEATURE_ECNR)
+		list[i++] = "echo-canceling-and-noise-reduction";
+
 	if (features & HFP_AG_FEATURE_VOICE_RECOG)
 		list[i++] = "voice-recognition";
 
-- 
1.7.11.7

[PATCH v1 3/7] core: Add EchoCancelingNoiseReduction to GetProperties

[Claudio Takahasi]

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

This patch adds EchoCancelingNoiseReduction property to GetProperties
method of Handsfree interface.
---
 src/handsfree.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/handsfree.c b/src/handsfree.c
index 6d7f263..d60841b 100644
--- a/src/handsfree.c
+++ b/src/handsfree.c
@@ -45,6 +45,7 @@
 static GSList *g_drivers = NULL;
 
 struct ofono_handsfree {
+	ofono_bool_t ecnr;
 	ofono_bool_t inband_ringing;
 	ofono_bool_t voice_recognition;
 	ofono_bool_t voice_recognition_pending;
@@ -175,6 +176,10 @@ static DBusMessage *handsfree_get_properties(DBusConnection *conn,
 	ofono_dbus_dict_append(&dict, "InbandRinging", DBUS_TYPE_BOOLEAN,
 				&inband_ringing);
 
+	if (hf->ag_features & HFP_AG_FEATURE_ECNR)
+		ofono_dbus_dict_append(&dict, "EchoCancelingNoiseReduction",
+						DBUS_TYPE_BOOLEAN, &hf->ecnr);
+
 	voice_recognition = hf->voice_recognition;
 	ofono_dbus_dict_append(&dict, "VoiceRecognition", DBUS_TYPE_BOOLEAN,
 				&voice_recognition);
-- 
1.7.11.7

[PATCH v1 4/7] core: Set EchoCancelingNoiseReduction to TRUE by default

[Claudio Takahasi]

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

According to Bluetooth HFP spec: By default, if the AG supports its
own embedded echo canceling and/or noise reduction functions, it shall
have them activated until the AT+NREC command is received. The
configuration set by the HF shall by used by the AG while the Service
Level Connection is active.

Since there isn't a command to query the current value, it is being
assumed that Echo Canceling and Noise Reduction is enabled when the
connection is established and the gateway supports this feature.
---
 src/handsfree.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/handsfree.c b/src/handsfree.c
index d60841b..c89f5e3 100644
--- a/src/handsfree.c
+++ b/src/handsfree.c
@@ -361,6 +361,7 @@ struct ofono_handsfree *ofono_handsfree_create(struct ofono_modem *modem,
 	hf->atom = __ofono_modem_add_atom(modem,
 					OFONO_ATOM_TYPE_HANDSFREE,
 					handsfree_remove, hf);
+	hf->ecnr = TRUE;
 
 	for (l = g_drivers; l; l = l->next) {
 		const struct ofono_handsfree_driver *drv = l->data;
-- 
1.7.11.7

[PATCH v1 5/7] include: Add EchoCancelingNoiseReduction to handsfree driver

[Claudio Takahasi]

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

This patch extends the handsfree driver adding echo canceling and noise
reduction callback to allow enabling or disabling this feature in the
audio gateway.
---
 include/handsfree.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/handsfree.h b/include/handsfree.h
index b5e0d5f..dcc4d74 100644
--- a/include/handsfree.h
+++ b/include/handsfree.h
@@ -48,6 +48,9 @@ struct ofono_handsfree_driver {
 	void (*voice_recognition)(struct ofono_handsfree *hf,
 					ofono_bool_t enabled,
 					ofono_handsfree_cb_t cb, void *data);
+	void (*echocanceling_noisereduction)(struct ofono_handsfree *hf,
+					ofono_bool_t enabled,
+					ofono_handsfree_cb_t cb, void *data);
 };
 
 void ofono_handsfree_set_ag_features(struct ofono_handsfree *hf,
-- 
1.7.11.7

[PATCH v1 6/7] hfpmodem: Add EchoCancelingNoiseReduction

[Claudio Takahasi]

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

This patch adds echo canceling and noise reduction implementation to
hfpmodem.
---
 drivers/hfpmodem/handsfree.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/hfpmodem/handsfree.c b/drivers/hfpmodem/handsfree.c
index ce28de3..2b96566 100644
--- a/drivers/hfpmodem/handsfree.c
+++ b/drivers/hfpmodem/handsfree.c
@@ -260,12 +260,32 @@ static void hfp_voice_recognition(struct ofono_handsfree *hf,
 	CALLBACK_WITH_FAILURE(cb, data);
 }
 
+static void hfp_echocanceling_noisereduction(struct ofono_handsfree *hf,
+				ofono_bool_t enabled, ofono_handsfree_cb_t cb,
+				void *data)
+{
+	struct hf_data *hd = ofono_handsfree_get_data(hf);
+	struct cb_data *cbd = cb_data_new(cb, data);
+	char buf[16];
+
+	snprintf(buf, sizeof(buf), "AT+NREC=%d", (int)(enabled));
+
+	if (g_at_chat_send(hd->chat, buf, NULL, hf_generic_set_cb,
+							cbd, g_free) > 0)
+		return;
+
+	g_free(cbd);
+
+	CALLBACK_WITH_FAILURE(cb, data);
+}
+
 static struct ofono_handsfree_driver driver = {
 	.name			= "hfpmodem",
 	.probe			= hfp_handsfree_probe,
 	.remove			= hfp_handsfree_remove,
 	.request_phone_number	= hfp_request_phone_number,
 	.voice_recognition	= hfp_voice_recognition,
+	.echocanceling_noisereduction = hfp_echocanceling_noisereduction,
 };
 
 void hfp_handsfree_init(void)
-- 
1.7.11.7

[PATCH v1 7/7] core: Add SetProperty for EchoCancelingNoiseReduction

[Claudio Takahasi]

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

This patch extends SetProperty method of the Handsfree interface
allowing to enable or disable echo canceling and noise reduction
feature in the audio gateway through a D-Bus method call.
---
 src/handsfree.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 48 insertions(+), 9 deletions(-)

diff --git a/src/handsfree.c b/src/handsfree.c
index c89f5e3..1af55c3 100644
--- a/src/handsfree.c
+++ b/src/handsfree.c
@@ -46,6 +46,7 @@ static GSList *g_drivers = NULL;
 
 struct ofono_handsfree {
 	ofono_bool_t ecnr;
+	ofono_bool_t ecnr_pending;
 	ofono_bool_t inband_ringing;
 	ofono_bool_t voice_recognition;
 	ofono_bool_t voice_recognition_pending;
@@ -220,11 +221,36 @@ static void voicerec_set_cb(const struct ofono_error *error, void *data)
 					&hf->voice_recognition);
 }
 
+static void ecnr_set_cb(const struct ofono_error *error, void *data)
+{
+	struct ofono_handsfree *hf = data;
+	DBusConnection *conn = ofono_dbus_get_connection();
+	const char *path = __ofono_atom_get_path(hf->atom);
+
+	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
+		__ofono_dbus_pending_reply(&hf->pending,
+					__ofono_error_failed(hf->pending));
+		return;
+	}
+
+	hf->ecnr = hf->ecnr_pending;
+
+	__ofono_dbus_pending_reply(&hf->pending,
+				dbus_message_new_method_return(hf->pending));
+
+	ofono_dbus_signal_property_changed(conn, path,
+					OFONO_HANDSFREE_INTERFACE,
+					"EchoCancelingNoiseReduction",
+					DBUS_TYPE_BOOLEAN,
+					&hf->ecnr);
+}
+
 static DBusMessage *handsfree_set_property(DBusConnection *conn,
 						DBusMessage *msg, void *data)
 {
 	struct ofono_handsfree *hf = data;
 	DBusMessageIter iter, var;
+	ofono_bool_t enabled;
 	const char *name;
 
 	if (hf->pending)
@@ -244,28 +270,41 @@ static DBusMessage *handsfree_set_property(DBusConnection *conn,
 
 	dbus_message_iter_recurse(&iter, &var);
 
+	if (dbus_message_iter_get_arg_type(&var) != DBUS_TYPE_BOOLEAN)
+		return __ofono_error_invalid_args(msg);
+
+	dbus_message_iter_get_basic(&var, &enabled);
+
 	if (g_str_equal(name, "VoiceRecognition") == TRUE) {
-		ofono_bool_t enabled;
 
 		if (!hf->driver->voice_recognition)
 			return __ofono_error_not_implemented(msg);
 
-		if (dbus_message_iter_get_arg_type(&var) != DBUS_TYPE_BOOLEAN)
-			return __ofono_error_invalid_args(msg);
-
-		dbus_message_iter_get_basic(&var, &enabled);
-
 		if (hf->voice_recognition == enabled)
 			return dbus_message_new_method_return(msg);
 
 		hf->voice_recognition_pending = enabled;
 		hf->pending = dbus_message_ref(msg);
 		hf->driver->voice_recognition(hf, enabled, voicerec_set_cb, hf);
+	} else if (g_str_equal(name, "EchoCancelingNoiseReduction") == TRUE) {
 
-		return NULL;
-	}
+		if (!(hf->ag_features & HFP_AG_FEATURE_ECNR))
+			return __ofono_error_not_supported(msg);
 
-	return __ofono_error_invalid_args(msg);
+		if (!hf->driver->echocanceling_noisereduction)
+			return __ofono_error_not_implemented(msg);
+
+		if (hf->ecnr == enabled)
+			return dbus_message_new_method_return(msg);
+
+		hf->pending = dbus_message_ref(msg);
+		hf->ecnr_pending = enabled;
+		hf->driver->echocanceling_noisereduction(hf, enabled,
+							ecnr_set_cb, hf);
+	} else
+		return __ofono_error_invalid_args(msg);
+
+	return NULL;
 }
 
 static void request_phone_number_cb(const struct ofono_error *error,
-- 
1.7.11.7

Re: [PATCH v1 1/7] doc: Add EchoCancelingNoiseReduction to Handsfree

[Claudio Takahasi]

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

Hi Denis:

On Mon, Apr 22, 2013 at 6:04 AM, Denis Kenzior <denkenz@gmail.com> wrote:
> Hi Claudio,
>
>
> On 04/22/2013 07:58 AM, Claudio Takahasi wrote:
>>
>> This patch adds echo canceling and noise reduction property to Handsfree
>> interface, allowing the handsfree unit to enable or disable this feature
>> in the audio gateway.
>>
>> According to Bluetooth HFP 1.6 spec: By default, if the AG supports its
>> own embedded echo canceling and/or noise reduction functions, it shall
>> have them activated until the AT+NREC command is received.
>> ---
>>   doc/handsfree-api.txt | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/doc/handsfree-api.txt b/doc/handsfree-api.txt
>> index 0b5d421..6855d93 100644
>> --- a/doc/handsfree-api.txt
>> +++ b/doc/handsfree-api.txt
>> @@ -56,6 +56,13 @@ Properties   array{string} Features [readonly]
>>                         to activate or deactivate the function from the
>> HF, or
>>                         the AG could autonomously initiate it.
>>
>> +               boolean EchoCancelingNoiseReduction [readwrite]
>> +
>> +                       Boolean representing whether echo canceling and
>> noise
>> +                       reduction is enabled in the AG. This property may
>> be
>> +                       written to activate or deactivate this feature
>> from
>> +                       the HF unit.
>> +
>
>
> This feature can only be disabled, not enabled for a given SLC.  So setting
> this to 'True' should return an error or do something way fancier.
>
> I suspect that to implement this properly we need to store the ECNR setting
> on disk (e.g. /var/lib/ofono/<local bdaddr>/<remote bdaddr>/handsfree or
> something) and manage it appropriately inside the core.

It is true, the HFP spec defines the procedure to disable it only.
I implemented an option to disable it because I noticed different
behavior between the phones/modems. Anyway, I will remove it, we can
extend the API later if necessary.

Regarding the storage, the spec (page 53) is a little bit confusing to
me, I understood that the AG will enable it again when the connection
is dropped. So, we don't need to stored it.

Regards,
Claudio

[PATCH v2 1/7] doc: Add EchoCancelingNoiseReduction to Handsfree

[Claudio Takahasi]

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

This patch adds echo canceling and noise reduction property to Handsfree
interface, allowing the handsfree unit to disable this feature in the
audio gateway. The HFP spec defines the procedure to turn off echo
canceling and noise reduction only. There are some modems that also
allow to turn on this feature from the HF unit, but it is not a
standard procedure.

According to Bluetooth HFP 1.6 spec: By default, if the AG supports its
own embedded echo canceling and/or noise reduction functions, it shall
have them activated until the AT+NREC command is received.
---
 doc/handsfree-api.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/doc/handsfree-api.txt b/doc/handsfree-api.txt
index 0b5d421..527a865 100644
--- a/doc/handsfree-api.txt
+++ b/doc/handsfree-api.txt
@@ -56,6 +56,12 @@ Properties	array{string} Features [readonly]
 			to activate or deactivate the function from the HF, or
 			the AG could autonomously initiate it.
 
+		boolean EchoCancelingNoiseReduction [readwrite]
+
+			Boolean representing whether echo canceling and noise
+			reduction is enabled in the AG. This property may be
+			written to deactivate this feature from the HF unit.
+
 		byte BatteryChargeLevel [readonly]
 
 			The current charge level of the battery.  The value
-- 
1.7.11.7

[PATCH v2 2/7] core: Add "echo-canceling-and-noise-reduction"

[Claudio Takahasi]

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

This patch adds "echo-canceling-and-noise-reduction" to "Features"
property of the Handsfree interface.
---
 src/handsfree.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/handsfree.c b/src/handsfree.c
index 8a0374d..6d7f263 100644
--- a/src/handsfree.c
+++ b/src/handsfree.c
@@ -62,6 +62,9 @@ static const char **ag_features_list(unsigned int features)
 	static const char *list[33];
 	unsigned int i = 0;
 
+	if (features & HFP_AG_FEATURE_ECNR)
+		list[i++] = "echo-canceling-and-noise-reduction";
+
 	if (features & HFP_AG_FEATURE_VOICE_RECOG)
 		list[i++] = "voice-recognition";
 
-- 
1.7.11.7

[PATCH v2 3/7] core: Add EchoCancelingNoiseReduction to GetProperties

[Claudio Takahasi]

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

This patch adds EchoCancelingNoiseReduction property to GetProperties
method of Handsfree interface.
---
 src/handsfree.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/handsfree.c b/src/handsfree.c
index 6d7f263..9e4fc6b 100644
--- a/src/handsfree.c
+++ b/src/handsfree.c
@@ -45,6 +45,7 @@
 static GSList *g_drivers = NULL;
 
 struct ofono_handsfree {
+	ofono_bool_t nrec;
 	ofono_bool_t inband_ringing;
 	ofono_bool_t voice_recognition;
 	ofono_bool_t voice_recognition_pending;
@@ -175,6 +176,10 @@ static DBusMessage *handsfree_get_properties(DBusConnection *conn,
 	ofono_dbus_dict_append(&dict, "InbandRinging", DBUS_TYPE_BOOLEAN,
 				&inband_ringing);
 
+	if (hf->ag_features & HFP_AG_FEATURE_ECNR)
+		ofono_dbus_dict_append(&dict, "EchoCancelingNoiseReduction",
+						DBUS_TYPE_BOOLEAN, &hf->nrec);
+
 	voice_recognition = hf->voice_recognition;
 	ofono_dbus_dict_append(&dict, "VoiceRecognition", DBUS_TYPE_BOOLEAN,
 				&voice_recognition);
-- 
1.7.11.7

[PATCH v2 4/7] core: Set EchoCancelingNoiseReduction to TRUE by default

[Claudio Takahasi]

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

According to Bluetooth HFP spec: By default, if the AG supports its
own embedded echo canceling and/or noise reduction functions, it shall
have them activated until the AT+NREC command is received. The
configuration set by the HF shall by used by the AG while the Service
Level Connection is active.

Since there isn't a command to query the current value, it is being
assumed that Echo Canceling and Noise Reduction is enabled when the
connection is established and the gateway supports this feature.
---
 src/handsfree.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/handsfree.c b/src/handsfree.c
index 9e4fc6b..75751e8 100644
--- a/src/handsfree.c
+++ b/src/handsfree.c
@@ -361,6 +361,7 @@ struct ofono_handsfree *ofono_handsfree_create(struct ofono_modem *modem,
 	hf->atom = __ofono_modem_add_atom(modem,
 					OFONO_ATOM_TYPE_HANDSFREE,
 					handsfree_remove, hf);
+	hf->nrec = TRUE;
 
 	for (l = g_drivers; l; l = l->next) {
 		const struct ofono_handsfree_driver *drv = l->data;
-- 
1.7.11.7

[PATCH v2 5/7] include: Add EchoCancelingNoiseReduction to handsfree driver

[Claudio Takahasi]

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

This patch extends the handsfree driver adding echo canceling and noise
reduction callback to allow enabling or disabling this feature in the
audio gateway.
---
 include/handsfree.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/handsfree.h b/include/handsfree.h
index b5e0d5f..dcb55c5 100644
--- a/include/handsfree.h
+++ b/include/handsfree.h
@@ -48,6 +48,8 @@ struct ofono_handsfree_driver {
 	void (*voice_recognition)(struct ofono_handsfree *hf,
 					ofono_bool_t enabled,
 					ofono_handsfree_cb_t cb, void *data);
+	void (*nrec)(struct ofono_handsfree *hf, ofono_bool_t enabled,
+					ofono_handsfree_cb_t cb, void *data);
 };
 
 void ofono_handsfree_set_ag_features(struct ofono_handsfree *hf,
-- 
1.7.11.7

[PATCH v2 6/7] hfpmodem: Add EchoCancelingNoiseReduction

[Claudio Takahasi]

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

This patch adds echo canceling and noise reduction implementation to
hfpmodem.
---
 drivers/hfpmodem/handsfree.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/hfpmodem/handsfree.c b/drivers/hfpmodem/handsfree.c
index ce28de3..d7561e7 100644
--- a/drivers/hfpmodem/handsfree.c
+++ b/drivers/hfpmodem/handsfree.c
@@ -260,12 +260,31 @@ static void hfp_voice_recognition(struct ofono_handsfree *hf,
 	CALLBACK_WITH_FAILURE(cb, data);
 }
 
+static void hfp_nrec(struct ofono_handsfree *hf, ofono_bool_t enabled,
+				ofono_handsfree_cb_t cb, void *data)
+{
+	struct hf_data *hd = ofono_handsfree_get_data(hf);
+	struct cb_data *cbd = cb_data_new(cb, data);
+	char buf[16];
+
+	snprintf(buf, sizeof(buf), "AT+NREC=%d", (int)(enabled));
+
+	if (g_at_chat_send(hd->chat, buf, NULL, hf_generic_set_cb,
+							cbd, g_free) > 0)
+		return;
+
+	g_free(cbd);
+
+	CALLBACK_WITH_FAILURE(cb, data);
+}
+
 static struct ofono_handsfree_driver driver = {
 	.name			= "hfpmodem",
 	.probe			= hfp_handsfree_probe,
 	.remove			= hfp_handsfree_remove,
 	.request_phone_number	= hfp_request_phone_number,
 	.voice_recognition	= hfp_voice_recognition,
+	.nrec			= hfp_nrec,
 };
 
 void hfp_handsfree_init(void)
-- 
1.7.11.7

[PATCH v2 7/7] core: Add SetProperty for EchoCancelingNoiseReduction

[Claudio Takahasi]

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

This patch extends SetProperty method of the Handsfree interface
allowing to disable echo canceling and noise reduction feature in
the audio gateway through a D-Bus method call. Once disabled, it
is not allowed to enable it using this procedure.
---
 src/handsfree.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 47 insertions(+), 9 deletions(-)

diff --git a/src/handsfree.c b/src/handsfree.c
index 75751e8..9487aec 100644
--- a/src/handsfree.c
+++ b/src/handsfree.c
@@ -46,6 +46,7 @@ static GSList *g_drivers = NULL;
 
 struct ofono_handsfree {
 	ofono_bool_t nrec;
+	ofono_bool_t nrec_pending;
 	ofono_bool_t inband_ringing;
 	ofono_bool_t voice_recognition;
 	ofono_bool_t voice_recognition_pending;
@@ -220,11 +221,36 @@ static void voicerec_set_cb(const struct ofono_error *error, void *data)
 					&hf->voice_recognition);
 }
 
+static void nrec_set_cb(const struct ofono_error *error, void *data)
+{
+	struct ofono_handsfree *hf = data;
+	DBusConnection *conn = ofono_dbus_get_connection();
+	const char *path = __ofono_atom_get_path(hf->atom);
+
+	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
+		__ofono_dbus_pending_reply(&hf->pending,
+					__ofono_error_failed(hf->pending));
+		return;
+	}
+
+	hf->nrec = hf->nrec_pending;
+
+	__ofono_dbus_pending_reply(&hf->pending,
+				dbus_message_new_method_return(hf->pending));
+
+	ofono_dbus_signal_property_changed(conn, path,
+					OFONO_HANDSFREE_INTERFACE,
+					"EchoCancelingNoiseReduction",
+					DBUS_TYPE_BOOLEAN,
+					&hf->nrec);
+}
+
 static DBusMessage *handsfree_set_property(DBusConnection *conn,
 						DBusMessage *msg, void *data)
 {
 	struct ofono_handsfree *hf = data;
 	DBusMessageIter iter, var;
+	ofono_bool_t enabled;
 	const char *name;
 
 	if (hf->pending)
@@ -244,28 +270,40 @@ static DBusMessage *handsfree_set_property(DBusConnection *conn,
 
 	dbus_message_iter_recurse(&iter, &var);
 
+	if (dbus_message_iter_get_arg_type(&var) != DBUS_TYPE_BOOLEAN)
+		return __ofono_error_invalid_args(msg);
+
+	dbus_message_iter_get_basic(&var, &enabled);
+
 	if (g_str_equal(name, "VoiceRecognition") == TRUE) {
-		ofono_bool_t enabled;
 
 		if (!hf->driver->voice_recognition)
 			return __ofono_error_not_implemented(msg);
 
-		if (dbus_message_iter_get_arg_type(&var) != DBUS_TYPE_BOOLEAN)
-			return __ofono_error_invalid_args(msg);
-
-		dbus_message_iter_get_basic(&var, &enabled);
-
 		if (hf->voice_recognition == enabled)
 			return dbus_message_new_method_return(msg);
 
 		hf->voice_recognition_pending = enabled;
 		hf->pending = dbus_message_ref(msg);
 		hf->driver->voice_recognition(hf, enabled, voicerec_set_cb, hf);
+	} else if (g_str_equal(name, "EchoCancelingNoiseReduction") == TRUE) {
 
-		return NULL;
-	}
+		if (!(hf->ag_features & HFP_AG_FEATURE_ECNR))
+			return __ofono_error_not_supported(msg);
 
-	return __ofono_error_invalid_args(msg);
+		if (!hf->driver->nrec || enabled == TRUE)
+			return __ofono_error_not_implemented(msg);
+
+		if (hf->nrec == enabled)
+			return dbus_message_new_method_return(msg);
+
+		hf->pending = dbus_message_ref(msg);
+		hf->nrec_pending = enabled;
+		hf->driver->nrec(hf, enabled, nrec_set_cb, hf);
+	} else
+		return __ofono_error_invalid_args(msg);
+
+	return NULL;
 }
 
 static void request_phone_number_cb(const struct ofono_error *error,
-- 
1.7.11.7

Re: [PATCH v3 1/7] doc: Add EchoCancelingNoiseReduction to Handsfree

[Denis Kenzior]

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

Hi Claudio,

On 04/23/2013 12:21 PM, Claudio Takahasi wrote:
> This patch adds echo canceling and noise reduction property to Handsfree
> interface, allowing the handsfree unit to disable this feature in the
> audio gateway. The HFP spec defines the procedure to turn off echo
> canceling and noise reduction only, it is not allowed to enable this
> feature from the HF unit.
>
> According to Bluetooth HFP 1.6 spec: By default, if the AG supports its
> own embedded echo canceling and/or noise reduction functions, it shall
> have them activated until the AT+NREC command is received from the HF
> unit. From then on, and until the current Service Level Connection is
> dropped for any reason, the AG shall disable these functions every time
> an Audio Connection is established.
> ---
>   doc/handsfree-api.txt | 8 ++++++++
>   1 file changed, 8 insertions(+)
>

All patches in this series have been applied, thanks.

Regards,
-Denis

[PATCH v3 1/7] doc: Add EchoCancelingNoiseReduction to Handsfree

[Claudio Takahasi]

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

This patch adds echo canceling and noise reduction property to Handsfree
interface, allowing the handsfree unit to disable this feature in the
audio gateway. The HFP spec defines the procedure to turn off echo
canceling and noise reduction only, it is not allowed to enable this
feature from the HF unit.

According to Bluetooth HFP 1.6 spec: By default, if the AG supports its
own embedded echo canceling and/or noise reduction functions, it shall
have them activated until the AT+NREC command is received from the HF
unit. From then on, and until the current Service Level Connection is
dropped for any reason, the AG shall disable these functions every time
an Audio Connection is established.
---
 doc/handsfree-api.txt | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/doc/handsfree-api.txt b/doc/handsfree-api.txt
index 0b5d421..66c93e5 100644
--- a/doc/handsfree-api.txt
+++ b/doc/handsfree-api.txt
@@ -44,6 +44,7 @@ Properties	array{string} Features [readonly]
 			supported values are:
 				"voice-recognition"
 				"attach-voice-tag"
+				"echo-canceling-and-noise-reduction"
 
 		boolean InbandRinging [readonly]
 
@@ -56,6 +57,13 @@ Properties	array{string} Features [readonly]
 			to activate or deactivate the function from the HF, or
 			the AG could autonomously initiate it.
 
+		boolean EchoCancelingNoiseReduction [readwrite]
+
+			Non-persistent Boolean property representing whether
+			echo canceling and noise reduction is enabled in the
+			AG. This feature can only be disabled once from the
+			HF unit, and it is not allowed to activate it.
+
 		byte BatteryChargeLevel [readonly]
 
 			The current charge level of the battery.  The value
-- 
1.7.11.7

[PATCH v3 2/7] core: Add "echo-canceling-and-noise-reduction"

[Claudio Takahasi]

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

This patch adds "echo-canceling-and-noise-reduction" to "Features"
property of the Handsfree interface.
---
 src/handsfree.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/handsfree.c b/src/handsfree.c
index 8a0374d..6d7f263 100644
--- a/src/handsfree.c
+++ b/src/handsfree.c
@@ -62,6 +62,9 @@ static const char **ag_features_list(unsigned int features)
 	static const char *list[33];
 	unsigned int i = 0;
 
+	if (features & HFP_AG_FEATURE_ECNR)
+		list[i++] = "echo-canceling-and-noise-reduction";
+
 	if (features & HFP_AG_FEATURE_VOICE_RECOG)
 		list[i++] = "voice-recognition";
 
-- 
1.7.11.7

[PATCH v3 3/7] core: Add EchoCancelingNoiseReduction to GetProperties

[Claudio Takahasi]

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

This patch adds EchoCancelingNoiseReduction property to GetProperties
method of Handsfree interface.
---
 src/handsfree.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/handsfree.c b/src/handsfree.c
index 6d7f263..9e4fc6b 100644
--- a/src/handsfree.c
+++ b/src/handsfree.c
@@ -45,6 +45,7 @@
 static GSList *g_drivers = NULL;
 
 struct ofono_handsfree {
+	ofono_bool_t nrec;
 	ofono_bool_t inband_ringing;
 	ofono_bool_t voice_recognition;
 	ofono_bool_t voice_recognition_pending;
@@ -175,6 +176,10 @@ static DBusMessage *handsfree_get_properties(DBusConnection *conn,
 	ofono_dbus_dict_append(&dict, "InbandRinging", DBUS_TYPE_BOOLEAN,
 				&inband_ringing);
 
+	if (hf->ag_features & HFP_AG_FEATURE_ECNR)
+		ofono_dbus_dict_append(&dict, "EchoCancelingNoiseReduction",
+						DBUS_TYPE_BOOLEAN, &hf->nrec);
+
 	voice_recognition = hf->voice_recognition;
 	ofono_dbus_dict_append(&dict, "VoiceRecognition", DBUS_TYPE_BOOLEAN,
 				&voice_recognition);
-- 
1.7.11.7

[PATCH v3 4/7] core: Set EchoCancelingNoiseReduction to TRUE by default

[Claudio Takahasi]

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

According to Bluetooth HFP spec: By default, if the AG supports its
own embedded echo canceling and/or noise reduction functions, it shall
have them activated until the AT+NREC command is received. The
configuration set by the HF shall by used by the AG while the Service
Level Connection is active.

Since there isn't a command to query the current value, it is being
assumed that Echo Canceling and Noise Reduction is enabled when the
connection is established and the gateway supports this feature.
---
 src/handsfree.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/handsfree.c b/src/handsfree.c
index 9e4fc6b..75751e8 100644
--- a/src/handsfree.c
+++ b/src/handsfree.c
@@ -361,6 +361,7 @@ struct ofono_handsfree *ofono_handsfree_create(struct ofono_modem *modem,
 	hf->atom = __ofono_modem_add_atom(modem,
 					OFONO_ATOM_TYPE_HANDSFREE,
 					handsfree_remove, hf);
+	hf->nrec = TRUE;
 
 	for (l = g_drivers; l; l = l->next) {
 		const struct ofono_handsfree_driver *drv = l->data;
-- 
1.7.11.7

[PATCH v3 5/7] include: Add EchoCancelingNoiseReduction to handsfree driver

[Claudio Takahasi]

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

This patch extends the handsfree driver adding echo canceling and noise
reduction callback to allow disabling this feature in the audio gateway.
---
 include/handsfree.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/handsfree.h b/include/handsfree.h
index b5e0d5f..dde7000 100644
--- a/include/handsfree.h
+++ b/include/handsfree.h
@@ -48,6 +48,8 @@ struct ofono_handsfree_driver {
 	void (*voice_recognition)(struct ofono_handsfree *hf,
 					ofono_bool_t enabled,
 					ofono_handsfree_cb_t cb, void *data);
+	void (*disable_nrec)(struct ofono_handsfree *hf,
+					ofono_handsfree_cb_t cb, void *data);
 };
 
 void ofono_handsfree_set_ag_features(struct ofono_handsfree *hf,
-- 
1.7.11.7

[PATCH v3 6/7] hfpmodem: Add EchoCancelingNoiseReduction

[Claudio Takahasi]

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

This patch adds echo canceling and noise reduction implementation to
hfpmodem. It adds the implementation to send the command "AT+NREC=0"
to the AG.
---
 drivers/hfpmodem/handsfree.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/hfpmodem/handsfree.c b/drivers/hfpmodem/handsfree.c
index ce28de3..78ee801 100644
--- a/drivers/hfpmodem/handsfree.c
+++ b/drivers/hfpmodem/handsfree.c
@@ -260,12 +260,29 @@ static void hfp_voice_recognition(struct ofono_handsfree *hf,
 	CALLBACK_WITH_FAILURE(cb, data);
 }
 
+static void hfp_disable_nrec(struct ofono_handsfree *hf,
+				ofono_handsfree_cb_t cb, void *data)
+{
+	struct hf_data *hd = ofono_handsfree_get_data(hf);
+	struct cb_data *cbd = cb_data_new(cb, data);
+	const char *buf = "AT+NREC=0";
+
+	if (g_at_chat_send(hd->chat, buf, NULL, hf_generic_set_cb,
+							cbd, g_free) > 0)
+		return;
+
+	g_free(cbd);
+
+	CALLBACK_WITH_FAILURE(cb, data);
+}
+
 static struct ofono_handsfree_driver driver = {
 	.name			= "hfpmodem",
 	.probe			= hfp_handsfree_probe,
 	.remove			= hfp_handsfree_remove,
 	.request_phone_number	= hfp_request_phone_number,
 	.voice_recognition	= hfp_voice_recognition,
+	.disable_nrec		= hfp_disable_nrec,
 };
 
 void hfp_handsfree_init(void)
-- 
1.7.11.7

[PATCH v3 7/7] core: Add SetProperty for EchoCancelingNoiseReduction

[Claudio Takahasi]

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

This patch extends SetProperty method of the Handsfree interface
allowing to disable echo canceling and noise reduction feature in
the audio gateway through a D-Bus method call. Once disabled, it
is not allowed to enable it using this procedure.
---
 src/handsfree.c | 54 +++++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 45 insertions(+), 9 deletions(-)

diff --git a/src/handsfree.c b/src/handsfree.c
index 75751e8..2808df8 100644
--- a/src/handsfree.c
+++ b/src/handsfree.c
@@ -220,11 +220,36 @@ static void voicerec_set_cb(const struct ofono_error *error, void *data)
 					&hf->voice_recognition);
 }
 
+static void nrec_set_cb(const struct ofono_error *error, void *data)
+{
+	struct ofono_handsfree *hf = data;
+	DBusConnection *conn = ofono_dbus_get_connection();
+	const char *path = __ofono_atom_get_path(hf->atom);
+
+	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
+		__ofono_dbus_pending_reply(&hf->pending,
+					__ofono_error_failed(hf->pending));
+		return;
+	}
+
+	hf->nrec = FALSE;
+
+	__ofono_dbus_pending_reply(&hf->pending,
+				dbus_message_new_method_return(hf->pending));
+
+	ofono_dbus_signal_property_changed(conn, path,
+					OFONO_HANDSFREE_INTERFACE,
+					"EchoCancelingNoiseReduction",
+					DBUS_TYPE_BOOLEAN,
+					&hf->nrec);
+}
+
 static DBusMessage *handsfree_set_property(DBusConnection *conn,
 						DBusMessage *msg, void *data)
 {
 	struct ofono_handsfree *hf = data;
 	DBusMessageIter iter, var;
+	ofono_bool_t enabled;
 	const char *name;
 
 	if (hf->pending)
@@ -244,28 +269,39 @@ static DBusMessage *handsfree_set_property(DBusConnection *conn,
 
 	dbus_message_iter_recurse(&iter, &var);
 
+	if (dbus_message_iter_get_arg_type(&var) != DBUS_TYPE_BOOLEAN)
+		return __ofono_error_invalid_args(msg);
+
+	dbus_message_iter_get_basic(&var, &enabled);
+
 	if (g_str_equal(name, "VoiceRecognition") == TRUE) {
-		ofono_bool_t enabled;
 
 		if (!hf->driver->voice_recognition)
 			return __ofono_error_not_implemented(msg);
 
-		if (dbus_message_iter_get_arg_type(&var) != DBUS_TYPE_BOOLEAN)
-			return __ofono_error_invalid_args(msg);
-
-		dbus_message_iter_get_basic(&var, &enabled);
-
 		if (hf->voice_recognition == enabled)
 			return dbus_message_new_method_return(msg);
 
 		hf->voice_recognition_pending = enabled;
 		hf->pending = dbus_message_ref(msg);
 		hf->driver->voice_recognition(hf, enabled, voicerec_set_cb, hf);
+	} else if (g_str_equal(name, "EchoCancelingNoiseReduction") == TRUE) {
 
-		return NULL;
-	}
+		if (!(hf->ag_features & HFP_AG_FEATURE_ECNR))
+			return __ofono_error_not_supported(msg);
 
-	return __ofono_error_invalid_args(msg);
+		if (!hf->driver->disable_nrec || enabled == TRUE)
+			return __ofono_error_not_implemented(msg);
+
+		if (hf->nrec == FALSE)
+			return dbus_message_new_method_return(msg);
+
+		hf->pending = dbus_message_ref(msg);
+		hf->driver->disable_nrec(hf, nrec_set_cb, hf);
+	} else
+		return __ofono_error_invalid_args(msg);
+
+	return NULL;
 }
 
 static void request_phone_number_cb(const struct ofono_error *error,
-- 
1.7.11.7