Hi Claudio, On 04/19/2013 12:34 PM, Claudio Takahasi wrote: I really prefer we used full words instead of an acronym. You'll find we have no acronyms in the oFono APIs. Regards, -Denis

--- 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

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

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

Hi Johan: On Fri, Apr 19, 2013 at 5:11 PM, Johan Hedberg <johan.hedberg(a)gmail.com&gt; wrote: 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.

Hi Claudio, On 04/22/2013 07:58 AM, Claudio Takahasi wrote: 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. Regards, -Denis

Hi Claudio, On 04/22/2013 11:46 AM, Claudio Takahasi wrote: This interface is specific to HFP, so don't worry about real modems. 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

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

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

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

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

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

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

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

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

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

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