[PATCH V3 0/4] Add dial support for HFP emulator

[PATCH V3 0/4] Add dial support for HFP emulator

[=?unknown-8bit?q?Fr=C3=A9d=C3=A9ric?= Danis]

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

Create a generic dial function that can be called by DBus or emulator
Add support of ATD and AT+BLDN
Save last dialed number in /var/lib/ofono/<modem>/voicecall to be used
by AT+BLDN

Frédéric Danis (4):
  voicecall: create generic dial function
  voicecall: add ATD support for HFP emulator
  voicecall: save last dialed number
  voicecall: add +BLDN support for HFP emulator

 src/voicecall.c |  270 +++++++++++++++++++++++++++++++++++++++++++++++++------
 1 files changed, 243 insertions(+), 27 deletions(-)

[PATCH V3 1/4] voicecall: create generic dial function

[=?unknown-8bit?q?Fr=C3=A9d=C3=A9ric?= Danis]

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

split manager_dial between generic and dbus parts
---
 src/voicecall.c |   82 +++++++++++++++++++++++++++++++++++++------------------
 1 files changed, 55 insertions(+), 27 deletions(-)

diff --git a/src/voicecall.c b/src/voicecall.c
index 94a20a9..26fe3e4 100644
--- a/src/voicecall.c
+++ b/src/voicecall.c
@@ -1437,59 +1437,87 @@ static void manager_dial_callback(const struct ofono_error *error, void *data)
 		voicecalls_emit_call_added(vc, v);
 }
 
-static DBusMessage *manager_dial(DBusConnection *conn,
-					DBusMessage *msg, void *data)
+static int voicecall_dial(struct ofono_voicecall *vc, const char *number,
+					enum ofono_clir_option clir,
+					ofono_voicecall_cb_t cb, void *data)
 {
-	struct ofono_voicecall *vc = data;
 	struct ofono_modem *modem = __ofono_atom_get_modem(vc->atom);
-	const char *number;
 	struct ofono_phone_number ph;
-	const char *clirstr;
-	enum ofono_clir_option clir;
-
-	if (vc->pending)
-		return __ofono_error_busy(msg);
 
 	if (g_slist_length(vc->call_list) >= MAX_VOICE_CALLS)
-		return __ofono_error_failed(msg);
-
-	if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &number,
-					DBUS_TYPE_STRING, &clirstr,
-					DBUS_TYPE_INVALID) == FALSE)
-		return __ofono_error_invalid_args(msg);
+		return -EPERM;
 
 	if (!valid_long_phone_number_format(number))
-		return __ofono_error_invalid_format(msg);
-
-	if (clir_string_to_clir(clirstr, &clir) == FALSE)
-		return __ofono_error_invalid_format(msg);
+		return -EINVAL;
 
 	if (ofono_modem_get_online(modem) == FALSE)
-		return __ofono_error_not_available(msg);
+		return -ENETDOWN;
 
 	if (vc->driver->dial == NULL)
-		return __ofono_error_not_implemented(msg);
+		return -ENOTSUP;
 
 	if (voicecalls_have_incoming(vc))
-		return __ofono_error_failed(msg);
+		return -EBUSY;
 
 	/* We can't have two dialing/alerting calls, reject outright */
 	if (voicecalls_num_connecting(vc) > 0)
-		return __ofono_error_failed(msg);
+		return -EBUSY;
 
 	if (voicecalls_have_active(vc) && voicecalls_have_held(vc))
-		return __ofono_error_failed(msg);
+		return -EBUSY;
 
 	if (is_emergency_number(vc, number) == TRUE)
 		__ofono_modem_inc_emergency_mode(modem);
 
+	string_to_phone_number(number, &ph);
+
+	vc->driver->dial(vc, &ph, clir, cb, vc);
+
+	return 0;
+}
+
+static DBusMessage *manager_dial(DBusConnection *conn,
+					DBusMessage *msg, void *data)
+{
+	struct ofono_voicecall *vc = data;
+	const char *number;
+	const char *clirstr;
+	enum ofono_clir_option clir;
+	int err;
+
+	if (vc->pending)
+		return __ofono_error_busy(msg);
+
+	if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &number,
+					DBUS_TYPE_STRING, &clirstr,
+					DBUS_TYPE_INVALID) == FALSE)
+		return __ofono_error_invalid_args(msg);
+
+	if (clir_string_to_clir(clirstr, &clir) == FALSE)
+		return __ofono_error_invalid_format(msg);
+
 	vc->pending = dbus_message_ref(msg);
 
-	string_to_phone_number(number, &ph);
+	err = voicecall_dial(vc, number, clir, manager_dial_callback, vc);
 
-	vc->driver->dial(vc, &ph, clir, manager_dial_callback, vc);
+	if (err >= 0)
+		return NULL;
 
-	return NULL;
+	vc->pending = NULL;
+	dbus_message_unref(msg);
+
+	switch (err) {
+	case -EINVAL:
+		return __ofono_error_invalid_format(msg);
+
+	case -ENETDOWN:
+		return __ofono_error_not_available(msg);
+
+	case -ENOTSUP:
+		return __ofono_error_not_implemented(msg);
+	}
+
+	return __ofono_error_failed(msg);
 }
 
 static DBusMessage *manager_transfer(DBusConnection *conn,
-- 
1.7.1

[PATCH V3 2/4] voicecall: add ATD support for HFP emulator

[=?unknown-8bit?q?Fr=C3=A9d=C3=A9ric?= Danis]

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

---
 src/voicecall.c |  109 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 109 insertions(+), 0 deletions(-)

diff --git a/src/voicecall.c b/src/voicecall.c
index 26fe3e4..a6ccddd 100644
--- a/src/voicecall.c
+++ b/src/voicecall.c
@@ -71,6 +71,7 @@ struct ofono_voicecall {
 	GQueue *toneq;
 	guint tone_source;
 	unsigned int hfp_watch;
+	struct ofono_emulator *pending_em;
 };
 
 struct voicecall {
@@ -745,10 +746,14 @@ static void notify_emulator_call_status(struct ofono_voicecall *vc)
 			break;
 
 		case CALL_STATUS_DIALING:
+			if (v->vc->pending_em)
+				return;
 			dialing = TRUE;
 			break;
 
 		case CALL_STATUS_ALERTING:
+			if (v->vc->pending_em)
+				return;
 			alerting = TRUE;
 			break;
 
@@ -2453,6 +2458,10 @@ static void emulator_hfp_unregister(struct ofono_atom *atom)
 						OFONO_ATOM_TYPE_EMULATOR_HFP,
 						emulator_remove_handler,
 						"+VTS");
+	__ofono_modem_foreach_registered_atom(modem,
+						OFONO_ATOM_TYPE_EMULATOR_HFP,
+						emulator_remove_handler,
+						"D");
 
 	__ofono_modem_remove_atom_watch(modem, vc->hfp_watch);
 }
@@ -2941,6 +2950,105 @@ static void emulator_vts_cb(struct ofono_emulator *em,
 	ofono_emulator_send_final(em, &result);
 }
 
+static void emulator_dial_callback(const struct ofono_error *error, void *data)
+{
+	struct ofono_voicecall *vc = data;
+	const char *number = NULL;
+	gboolean need_to_emit;
+	struct voicecall *v;
+
+	v = dial_handle_result(vc, error, number, &need_to_emit);
+
+	if (v == NULL) {
+		struct ofono_modem *modem = __ofono_atom_get_modem(vc->atom);
+
+		if (is_emergency_number(vc, number) == TRUE)
+			__ofono_modem_dec_emergency_mode(modem);
+	}
+
+	ofono_emulator_send_final(vc->pending_em, error);
+
+	vc->pending_em = NULL;
+
+	notify_emulator_call_status(vc);
+
+	if (need_to_emit)
+		voicecalls_emit_call_added(vc, v);
+}
+
+static void emulator_dial(struct ofono_emulator *em, struct ofono_voicecall *vc,
+				const char *number)
+{
+	struct ofono_error result;
+	int err;
+
+	result.error = 0;
+
+	if (vc->pending_em) {
+		result.type = OFONO_ERROR_TYPE_FAILURE;
+		goto send;
+	}
+
+	vc->pending_em = em;
+
+	err = voicecall_dial(vc, number, OFONO_CLIR_OPTION_DEFAULT,
+					emulator_dial_callback, vc);
+
+	if (err >= 0)
+		return;
+
+	vc->pending_em = NULL;
+
+	switch (err) {
+	case -ENETDOWN:
+		result.error = 30;
+		result.type = OFONO_ERROR_TYPE_CME;
+		break;
+
+	default:
+		result.type = OFONO_ERROR_TYPE_FAILURE;
+	}
+
+send:
+	ofono_emulator_send_final(em, &result);
+}
+
+static void emulator_atd_cb(struct ofono_emulator *em,
+			struct ofono_emulator_request *req, void *userdata)
+{
+	struct ofono_voicecall *vc = userdata;
+	const char *str;
+	size_t len;
+	char number[OFONO_MAX_PHONE_NUMBER_LENGTH + 1];
+	struct ofono_error result;
+
+	switch (ofono_emulator_request_get_type(req)) {
+	case OFONO_EMULATOR_REQUEST_TYPE_SET:
+		str = ofono_emulator_request_get_raw(req);
+
+		if (str == NULL || str[0] == '\0')
+			goto fail;
+
+		len = strlen(str);
+
+		if (len > OFONO_MAX_PHONE_NUMBER_LENGTH + 1 ||
+				str[len - 1] != ';')
+			goto fail;
+
+		strncpy(number, str, len - 1);
+		number[len - 1] = '\0';
+
+		emulator_dial(em, vc, number);
+		break;
+
+	default:
+fail:
+		result.error = 0;
+		result.type = OFONO_ERROR_TYPE_FAILURE;
+		ofono_emulator_send_final(em, &result);
+	};
+}
+
 static void emulator_hfp_watch(struct ofono_atom *atom,
 				enum ofono_atom_watch_condition cond,
 				void *data)
@@ -2957,6 +3065,7 @@ static void emulator_hfp_watch(struct ofono_atom *atom,
 	ofono_emulator_add_handler(em, "+CLCC", emulator_clcc_cb, data, NULL);
 	ofono_emulator_add_handler(em, "+CHLD", emulator_chld_cb, data, NULL);
 	ofono_emulator_add_handler(em, "+VTS", emulator_vts_cb, data, NULL);
+	ofono_emulator_add_handler(em, "D", emulator_atd_cb, data, NULL);
 }
 
 void ofono_voicecall_register(struct ofono_voicecall *vc)
-- 
1.7.1

[PATCH V3 3/4] voicecall: save last dialed number

[=?unknown-8bit?q?Fr=C3=A9d=C3=A9ric?= Danis]

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

---
 src/voicecall.c |   45 +++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 45 insertions(+), 0 deletions(-)

diff --git a/src/voicecall.c b/src/voicecall.c
index a6ccddd..f176e25 100644
--- a/src/voicecall.c
+++ b/src/voicecall.c
@@ -37,11 +37,15 @@
 #include "common.h"
 #include "simutil.h"
 #include "smsutil.h"
+#include "storage.h"
 
 #define MAX_VOICE_CALLS 16
 
 #define VOICECALL_FLAG_SIM_ECC_READY 0x1
 
+#define SETTINGS_STORE "voicecall"
+#define SETTINGS_GROUP "Settings"
+
 GSList *g_drivers = NULL;
 
 struct multi_release {
@@ -72,6 +76,8 @@ struct ofono_voicecall {
 	guint tone_source;
 	unsigned int hfp_watch;
 	struct ofono_emulator *pending_em;
+	GKeyFile *settings;
+	char *imsi;
 };
 
 struct voicecall {
@@ -1478,6 +1484,12 @@ static int voicecall_dial(struct ofono_voicecall *vc, const char *number,
 
 	vc->driver->dial(vc, &ph, clir, cb, vc);
 
+	if (vc->settings) {
+		g_key_file_set_string(vc->settings, SETTINGS_GROUP,
+					"Number", number);
+		storage_sync(vc->imsi, SETTINGS_STORE, vc->settings);
+	}
+
 	return 0;
 }
 
@@ -2466,6 +2478,33 @@ static void emulator_hfp_unregister(struct ofono_atom *atom)
 	__ofono_modem_remove_atom_watch(modem, vc->hfp_watch);
 }
 
+static void voicecall_load_settings(struct ofono_voicecall *vc)
+{
+	const char *imsi;
+
+	imsi = ofono_sim_get_imsi(vc->sim);
+	if (imsi == NULL)
+		return;
+
+	vc->settings = storage_open(imsi, SETTINGS_STORE);
+
+	if (vc->settings == NULL)
+		return;
+
+	vc->imsi = g_strdup(imsi);
+}
+
+static void voicecall_close_settings(struct ofono_voicecall *vc)
+{
+	if (vc->settings) {
+		storage_close(vc->imsi, SETTINGS_STORE, vc->settings, TRUE);
+
+		g_free(vc->imsi);
+		vc->imsi = NULL;
+		vc->settings = NULL;
+	}
+}
+
 static void voicecall_unregister(struct ofono_atom *atom)
 {
 	DBusConnection *conn = ofono_dbus_get_connection();
@@ -2476,6 +2515,8 @@ static void voicecall_unregister(struct ofono_atom *atom)
 
 	emulator_hfp_unregister(atom);
 
+	voicecall_close_settings(vc);
+
 	if (vc->sim_state_watch) {
 		ofono_sim_remove_state_watch(vc->sim, vc->sim_state_watch);
 		vc->sim_state_watch = 0;
@@ -2615,6 +2656,9 @@ static void sim_state_watch(enum ofono_sim_state new_state, void *user)
 
 		free_sim_ecc_numbers(vc, FALSE);
 		set_new_ecc(vc);
+	case OFONO_SIM_STATE_READY:
+		voicecall_load_settings(vc);
+		break;
 	default:
 		break;
 	}
@@ -2627,6 +2671,7 @@ static void sim_watch(struct ofono_atom *atom,
 	struct ofono_sim *sim = __ofono_atom_get_data(atom);
 
 	if (cond == OFONO_ATOM_WATCH_CONDITION_UNREGISTERED) {
+		voicecall_close_settings(vc);
 		vc->sim_state_watch = 0;
 		vc->sim = NULL;
 		return;
-- 
1.7.1

[PATCH V3 4/4] voicecall: add +BLDN support for HFP emulator

[=?unknown-8bit?q?Fr=C3=A9d=C3=A9ric?= Danis]

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

---
 src/voicecall.c |   34 ++++++++++++++++++++++++++++++++++
 1 files changed, 34 insertions(+), 0 deletions(-)

diff --git a/src/voicecall.c b/src/voicecall.c
index f176e25..c4a15a2 100644
--- a/src/voicecall.c
+++ b/src/voicecall.c
@@ -2474,6 +2474,10 @@ static void emulator_hfp_unregister(struct ofono_atom *atom)
 						OFONO_ATOM_TYPE_EMULATOR_HFP,
 						emulator_remove_handler,
 						"D");
+	__ofono_modem_foreach_registered_atom(modem,
+						OFONO_ATOM_TYPE_EMULATOR_HFP,
+						emulator_remove_handler,
+						"+BLDN");
 
 	__ofono_modem_remove_atom_watch(modem, vc->hfp_watch);
 }
@@ -3094,6 +3098,35 @@ fail:
 	};
 }
 
+static void emulator_bldn_cb(struct ofono_emulator *em,
+			struct ofono_emulator_request *req, void *userdata)
+{
+	struct ofono_voicecall *vc = userdata;
+	const char *number;
+	struct ofono_error result;
+	GError *error = NULL;
+
+	switch (ofono_emulator_request_get_type(req)) {
+	case OFONO_EMULATOR_REQUEST_TYPE_COMMAND_ONLY:
+		if (vc->settings == NULL)
+			goto fail;
+
+		number = g_key_file_get_string(vc->settings, SETTINGS_GROUP,
+						"Number", &error);
+		if (number == NULL || number[0] == '\0')
+			goto fail;
+
+		emulator_dial(em, vc, number);
+		break;
+
+	default:
+fail:
+		result.error = 0;
+		result.type = OFONO_ERROR_TYPE_FAILURE;
+		ofono_emulator_send_final(em, &result);
+	};
+}
+
 static void emulator_hfp_watch(struct ofono_atom *atom,
 				enum ofono_atom_watch_condition cond,
 				void *data)
@@ -3111,6 +3144,7 @@ static void emulator_hfp_watch(struct ofono_atom *atom,
 	ofono_emulator_add_handler(em, "+CHLD", emulator_chld_cb, data, NULL);
 	ofono_emulator_add_handler(em, "+VTS", emulator_vts_cb, data, NULL);
 	ofono_emulator_add_handler(em, "D", emulator_atd_cb, data, NULL);
+	ofono_emulator_add_handler(em, "+BLDN", emulator_bldn_cb, data, NULL);
 }
 
 void ofono_voicecall_register(struct ofono_voicecall *vc)
-- 
1.7.1

Re: [PATCH V3 1/4] voicecall: create generic dial function

[Denis Kenzior]

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

Hi Frédéric,

On 06/01/2011 06:24 AM, Frédéric Danis wrote:
> split manager_dial between generic and dbus parts
> ---
>  src/voicecall.c |   82 +++++++++++++++++++++++++++++++++++++------------------
>  1 files changed, 55 insertions(+), 27 deletions(-)
> 

Patch has been applied, thanks.

Regards,
-Denis

Re: [PATCH V3 3/4] voicecall: save last dialed number

[Denis Kenzior]

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

Hi Frédéric,

On 06/01/2011 06:24 AM, Frédéric Danis wrote:
> ---
>  src/voicecall.c |   45 +++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 45 insertions(+), 0 deletions(-)
> 

Patch has been applied, thanks.

Regards,
-Denis

Re: [PATCH V3 2/4] voicecall: add ATD support for HFP emulator

[Denis Kenzior]

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

Hi Frédéric,

On 06/01/2011 06:24 AM, Frédéric Danis wrote:
> ---
>  src/voicecall.c |  109 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 109 insertions(+), 0 deletions(-)
> 
> diff --git a/src/voicecall.c b/src/voicecall.c
> index 26fe3e4..a6ccddd 100644
> --- a/src/voicecall.c
> +++ b/src/voicecall.c
> @@ -71,6 +71,7 @@ struct ofono_voicecall {
>  	GQueue *toneq;
>  	guint tone_source;
>  	unsigned int hfp_watch;
> +	struct ofono_emulator *pending_em;

I think you're on the right track, but...

>  };
>  
>  struct voicecall {
> @@ -745,10 +746,14 @@ static void notify_emulator_call_status(struct ofono_voicecall *vc)
>  			break;
>  
>  		case CALL_STATUS_DIALING:
> +			if (v->vc->pending_em)
> +				return;
>  			dialing = TRUE;
>  			break;
>  
>  		case CALL_STATUS_ALERTING:
> +			if (v->vc->pending_em)
> +				return;
>  			alerting = TRUE;
>  			break;
>  

You can't really do it this way since you might have several emulators
active at once, and you want to make sure that only the one with the
pending ATD doesn't receive updates until ATD returns.

One thing you might try is to skip sending updates to the emulator if it
is equal to pending_em inside emulator_call_status_cb /
emulator_callheld_status_cb / emulator_callsetup_status_cb

> @@ -2453,6 +2458,10 @@ static void emulator_hfp_unregister(struct ofono_atom *atom)
>  						OFONO_ATOM_TYPE_EMULATOR_HFP,
>  						emulator_remove_handler,
>  						"+VTS");
> +	__ofono_modem_foreach_registered_atom(modem,
> +						OFONO_ATOM_TYPE_EMULATOR_HFP,
> +						emulator_remove_handler,
> +						"D");
>  
>  	__ofono_modem_remove_atom_watch(modem, vc->hfp_watch);
>  }
> @@ -2941,6 +2950,105 @@ static void emulator_vts_cb(struct ofono_emulator *em,
>  	ofono_emulator_send_final(em, &result);
>  }
>  
> +static void emulator_dial_callback(const struct ofono_error *error, void *data)
> +{
> +	struct ofono_voicecall *vc = data;
> +	const char *number = NULL;
> +	gboolean need_to_emit;
> +	struct voicecall *v;
> +
> +	v = dial_handle_result(vc, error, number, &need_to_emit);
> +

Setting number to NULL is actually wrong.  dial_handle_result needs a
proper number, so you must track it somehow.  The only reason we set it
to NULL in e.g. manager_dial_callback is to get around the compiler
nagging us of uninitialized variables (which will never happen unless
something truly bizarre is happening.)

> +	if (v == NULL) {
> +		struct ofono_modem *modem = __ofono_atom_get_modem(vc->atom);
> +
> +		if (is_emergency_number(vc, number) == TRUE)
> +			__ofono_modem_dec_emergency_mode(modem);
> +	}
> +
> +	ofono_emulator_send_final(vc->pending_em, error);
> +
> +	vc->pending_em = NULL;
> +
> +	notify_emulator_call_status(vc);
> +
> +	if (need_to_emit)
> +		voicecalls_emit_call_added(vc, v);
> +}
> +
> +static void emulator_dial(struct ofono_emulator *em, struct ofono_voicecall *vc,
> +				const char *number)
> +{
> +	struct ofono_error result;
> +	int err;
> +
> +	result.error = 0;
> +
> +	if (vc->pending_em) {
> +		result.type = OFONO_ERROR_TYPE_FAILURE;
> +		goto send;
> +	}

You might also want to check that the Dial is not happening via D-Bus or
STK, and vice-versa.

> +
> +	vc->pending_em = em;
> +
> +	err = voicecall_dial(vc, number, OFONO_CLIR_OPTION_DEFAULT,
> +					emulator_dial_callback, vc);
> +
> +	if (err >= 0)
> +		return;
> +
> +	vc->pending_em = NULL;
> +
> +	switch (err) {
> +	case -ENETDOWN:
> +		result.error = 30;
> +		result.type = OFONO_ERROR_TYPE_CME;
> +		break;
> +
> +	default:
> +		result.type = OFONO_ERROR_TYPE_FAILURE;
> +	}
> +
> +send:
> +	ofono_emulator_send_final(em, &result);
> +}
> +
> +static void emulator_atd_cb(struct ofono_emulator *em,
> +			struct ofono_emulator_request *req, void *userdata)
> +{
> +	struct ofono_voicecall *vc = userdata;
> +	const char *str;
> +	size_t len;
> +	char number[OFONO_MAX_PHONE_NUMBER_LENGTH + 1];
> +	struct ofono_error result;
> +
> +	switch (ofono_emulator_request_get_type(req)) {
> +	case OFONO_EMULATOR_REQUEST_TYPE_SET:
> +		str = ofono_emulator_request_get_raw(req);
> +
> +		if (str == NULL || str[0] == '\0')
> +			goto fail;
> +
> +		len = strlen(str);
> +
> +		if (len > OFONO_MAX_PHONE_NUMBER_LENGTH + 1 ||
> +				str[len - 1] != ';')
> +			goto fail;
> +
> +		strncpy(number, str, len - 1);
> +		number[len - 1] = '\0';
> +
> +		emulator_dial(em, vc, number);
> +		break;
> +
> +	default:
> +fail:
> +		result.error = 0;
> +		result.type = OFONO_ERROR_TYPE_FAILURE;
> +		ofono_emulator_send_final(em, &result);
> +	};
> +}
> +
>  static void emulator_hfp_watch(struct ofono_atom *atom,
>  				enum ofono_atom_watch_condition cond,
>  				void *data)
> @@ -2957,6 +3065,7 @@ static void emulator_hfp_watch(struct ofono_atom *atom,
>  	ofono_emulator_add_handler(em, "+CLCC", emulator_clcc_cb, data, NULL);
>  	ofono_emulator_add_handler(em, "+CHLD", emulator_chld_cb, data, NULL);
>  	ofono_emulator_add_handler(em, "+VTS", emulator_vts_cb, data, NULL);
> +	ofono_emulator_add_handler(em, "D", emulator_atd_cb, data, NULL);
>  }
>  
>  void ofono_voicecall_register(struct ofono_voicecall *vc)

Regards,
-Denis

Re: [PATCH V3 2/4] voicecall: add ATD support for HFP emulator

[Denis Kenzior]

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

Hi Frederic,

<snip>

>>> +
>>> +    if (vc->pending_em) {
>>> +        result.type = OFONO_ERROR_TYPE_FAILURE;
>>> +        goto send;
>>> +    }
>>
>> You might also want to check that the Dial is not happening via D-Bus or
>> STK, and vice-versa.
> 
> Voicecall_dial will check if there is already a pending outgoing call.
> I think this is sufficient to manage concurrent ATD between D-Bus, STK
> and emulator.
> 

No, it isn't.  voicecall_dial only checks for dialing / alerting calls.
 However, you have to remember that the call isn't created until the
dial callback returns or ofono_voicecall_notify is called with the newly
created call.  This can take some time (e.g. due to queuing on the
GAtChat object), and during this time new Dial requests have to be rejected.

D-Bus API implementation handles this by using vc->pending (and it is a
bug that it doesn't check vc->dial_req).   STK handles this by checking
vc->pending and the existence of dial_req object.

Once you introduce the emulator, there is no cross-checking between the
emulator and the existing ways.  Between emulators you handle this by
using vc->pending_em, but you should also be checking that a Dial hasn't
been scheduled via STK/D-Bus API.

Regards,
-Denis

Re: [PATCH V3 2/4] voicecall: add ATD support for HFP emulator

[Frederic Danis]

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

Hello Denis,

Le 02/06/2011 04:08, Denis Kenzior a écrit :
>> +static void emulator_dial(struct ofono_emulator *em, struct ofono_voicecall *vc,
>> +				const char *number)
>> +{
>> +	struct ofono_error result;
>> +	int err;
>> +
>> +	result.error = 0;
>> +
>> +	if (vc->pending_em) {
>> +		result.type = OFONO_ERROR_TYPE_FAILURE;
>> +		goto send;
>> +	}
>
> You might also want to check that the Dial is not happening via D-Bus or
> STK, and vice-versa.

Voicecall_dial will check if there is already a pending outgoing call.
I think this is sufficient to manage concurrent ATD between D-Bus, STK 
and emulator.

>
>> +
>> +	vc->pending_em = em;
>> +
>> +	err = voicecall_dial(vc, number, OFONO_CLIR_OPTION_DEFAULT,
>> +					emulator_dial_callback, vc);
>> +
>> +	if (err>= 0)
>> +		return;
>> +
>> +	vc->pending_em = NULL;
>> +
>> +	switch (err) {
>> +	case -ENETDOWN:
>> +		result.error = 30;
>> +		result.type = OFONO_ERROR_TYPE_CME;
>> +		break;
>> +
>> +	default:
>> +		result.type = OFONO_ERROR_TYPE_FAILURE;
>> +	}
>> +
>> +send:
>> +	ofono_emulator_send_final(em,&result);
>> +}
>> +

Regards

Fred


-- 
Frederic Danis                            Open Source Technology Centre
frederic.danis(a)intel.com                              Intel Corporation

Re: [PATCH V3 2/4] voicecall: add ATD support for HFP emulator

[Frederic Danis]

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

Hello Denis,

Le 06/06/2011 04:09, Denis Kenzior a écrit :
> Hi Frederic,
>
> <snip>
>
>>>> +
>>>> +    if (vc->pending_em) {
>>>> +        result.type = OFONO_ERROR_TYPE_FAILURE;
>>>> +        goto send;
>>>> +    }
>>>
>>> You might also want to check that the Dial is not happening via D-Bus or
>>> STK, and vice-versa.
>>
>> Voicecall_dial will check if there is already a pending outgoing call.
>> I think this is sufficient to manage concurrent ATD between D-Bus, STK
>> and emulator.
>>
>
> No, it isn't.  voicecall_dial only checks for dialing / alerting calls.
>   However, you have to remember that the call isn't created until the
> dial callback returns or ofono_voicecall_notify is called with the newly
> created call.  This can take some time (e.g. due to queuing on the
> GAtChat object), and during this time new Dial requests have to be rejected.
>
> D-Bus API implementation handles this by using vc->pending (and it is a
> bug that it doesn't check vc->dial_req).   STK handles this by checking
> vc->pending and the existence of dial_req object.
>
> Once you introduce the emulator, there is no cross-checking between the
> emulator and the existing ways.  Between emulators you handle this by
> using vc->pending_em, but you should also be checking that a Dial hasn't
> been scheduled via STK/D-Bus API.
>
> Regards,
> -Denis

OK
Thanks for your explanation, hope rest of v4 of my patches are OK, I 
will prepare the v5 to manage this correctly

Regards

Fred

-- 
Frederic Danis                            Open Source Technology Centre
frederic.danis(a)intel.com                              Intel Corporation