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

[PATCH V2 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 |  245 +++++++++++++++++++++++++++++++++++++++++++++++++------
 1 files changed, 218 insertions(+), 27 deletions(-)

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

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

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

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

diff --git a/src/voicecall.c b/src/voicecall.c
index 2ab67ac..7dbb311 100644
--- a/src/voicecall.c
+++ b/src/voicecall.c
@@ -1437,59 +1437,96 @@ 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;
+	DBusMessage *reply = NULL;
+
+	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);
+	switch (err) {
+	case 0:
+		break;
+
+	case -EINVAL:
+		reply = __ofono_error_invalid_format(msg);
+		break;
 
-	vc->driver->dial(vc, &ph, clir, manager_dial_callback, vc);
+	case -ENETDOWN:
+		reply = __ofono_error_not_available(msg);
+		break;
 
-	return NULL;
+	case -ENOTSUP:
+		reply = __ofono_error_not_implemented(msg);
+		break;
+
+	default:
+		reply = __ofono_error_failed(msg);
+		break;
+	}
+
+	if (reply != NULL) {
+		dbus_message_unref(msg);
+		vc->pending = NULL;
+	}
+
+	return reply;
 }
 
 static DBusMessage *manager_transfer(DBusConnection *conn,
-- 
1.7.1

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

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

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

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

diff --git a/src/voicecall.c b/src/voicecall.c
index 7dbb311..ff1f0fc 100644
--- a/src/voicecall.c
+++ b/src/voicecall.c
@@ -2458,6 +2458,10 @@ static void emulator_hfp_unregister(struct ofono_atom *atom)
 						OFONO_ATOM_TYPE_EMULATOR_HFP,
 						emulator_remove_handler,
 						"+CHLD");
+	__ofono_modem_foreach_registered_atom(modem,
+						OFONO_ATOM_TYPE_EMULATOR_HFP,
+						emulator_remove_handler,
+						"D");
 
 	__ofono_modem_remove_atom_watch(modem, vc->hfp_watch);
 }
@@ -2903,6 +2907,79 @@ fail:
 	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);
+	}
+
+	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 (number == NULL || number[0] == '\0') {
+		result.type = OFONO_ERROR_TYPE_FAILURE;
+		goto send;
+	}
+
+	err = voicecall_dial(vc, number, OFONO_CLIR_OPTION_DEFAULT,
+					emulator_dial_callback, vc);
+	switch (err) {
+	case 0:
+		result.type = OFONO_ERROR_TYPE_NO_ERROR;
+		break;
+
+	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;
+	struct ofono_error result;
+
+	switch (ofono_emulator_request_get_type(req)) {
+	case OFONO_EMULATOR_REQUEST_TYPE_SET:
+		str = ofono_emulator_request_get_raw(req);
+		emulator_dial(em, vc, str);
+		break;
+
+	default:
+		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)
@@ -2918,6 +2995,7 @@ static void emulator_hfp_watch(struct ofono_atom *atom,
 	ofono_emulator_add_handler(em, "+CHUP", emulator_chup_cb, data, NULL);
 	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, "D", emulator_atd_cb, data, NULL);
 }
 
 void ofono_voicecall_register(struct ofono_voicecall *vc)
-- 
1.7.1

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

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

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

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

diff --git a/src/voicecall.c b/src/voicecall.c
index ff1f0fc..e91ba42 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 VOICECALL_STORE "voicecall"
+#define LASTDIAL_GROUP "LastDial"
+
 GSList *g_drivers = NULL;
 
 struct multi_release {
@@ -71,6 +75,8 @@ struct ofono_voicecall {
 	GQueue *toneq;
 	guint tone_source;
 	unsigned int hfp_watch;
+	GKeyFile *settings;
+	char *imsi;
 };
 
 struct voicecall {
@@ -1473,6 +1479,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, LASTDIAL_GROUP,
+					"Number", number);
+		storage_sync(vc->imsi, VOICECALL_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, VOICECALL_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, VOICECALL_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 V2 4/4] voicecall: add +BLDN support for HFP emulator

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

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

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

diff --git a/src/voicecall.c b/src/voicecall.c
index e91ba42..9132fed 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);
 }
@@ -3025,6 +3029,32 @@ static void emulator_atd_cb(struct ofono_emulator *em,
 	};
 }
 
+static void emulator_bldn_cb(struct ofono_emulator *em,
+			struct ofono_emulator_request *req, void *userdata)
+{
+	struct ofono_voicecall *vc = userdata;
+	const char *str;
+	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;
+
+		str = g_key_file_get_string(vc->settings, LASTDIAL_GROUP,
+						"Number", &error);
+		emulator_dial(em, vc, str);
+		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)
@@ -3041,6 +3071,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, "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 V2 1/4] voicecall: create generic dial function

[Denis Kenzior]

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

Hi Frédéric,

On 05/20/2011 04:40 AM, Frédéric Danis wrote:
> split manager_dial between generic and dbus parts
> ---
>  src/voicecall.c |   91 ++++++++++++++++++++++++++++++++++++++----------------
>  1 files changed, 64 insertions(+), 27 deletions(-)
> 

<snip>

> +
> +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;
> +	DBusMessage *reply = NULL;
> +
> +	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);
> +	switch (err) {
> +	case 0:
> +		break;

Maybe I'm missing something, but why don't you do something like:

if (err >= 0)
	return NULL;

vc->pending = NULL;
dbus_message_unref(msg);

> +
> +	case -EINVAL:
> +		reply = __ofono_error_invalid_format(msg);
> +		break;

And then simply use return __ofono_error... here?

>  
> -	vc->driver->dial(vc, &ph, clir, manager_dial_callback, vc);
> +	case -ENETDOWN:
> +		reply = __ofono_error_not_available(msg);
> +		break;
>  

And here

> -	return NULL;
> +	case -ENOTSUP:
> +		reply = __ofono_error_not_implemented(msg);
> +		break;
> +

And here

> +	default:
> +		reply = __ofono_error_failed(msg);
> +		break;
> +	}
> +

And here

> +	if (reply != NULL) {
> +		dbus_message_unref(msg);
> +		vc->pending = NULL;
> +	}
> +
> +	return reply;

And then this part is not needed.

>  }
>  
>  static DBusMessage *manager_transfer(DBusConnection *conn,

Regards,
-Denis

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

[Denis Kenzior]

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

Hi Frédéric,

<snip>

> +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);
> +	}
> +
> +	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 (number == NULL || number[0] == '\0') {
> +		result.type = OFONO_ERROR_TYPE_FAILURE;
> +		goto send;
> +	}
> +
> +	err = voicecall_dial(vc, number, OFONO_CLIR_OPTION_DEFAULT,
> +					emulator_dial_callback, vc);
> +	switch (err) {
> +	case 0:
> +		result.type = OFONO_ERROR_TYPE_NO_ERROR;
> +		break;
> +

Actually you can't do this.  There are bizarre circumstances where the
dial callback will be called synchronously, so even though all
conditions in voicecall_dial are satisfied and it returns 0, the modem
driver still fails to dispatch the ATD.  In which case you will
erroneously return OK.

e.g.:

voicecall_dial ->
	at_modem_dial ->
		emulator_dial_callback
voicecall_dial

you have to be also careful of the timing of the ofono_voicecall_notify
events and the ATD callback.  You should not report cind until you have
sent the final response to the ATD.

Regards,
-Denis

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

[Denis Kenzior]

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

Hi Frédéric,

<snip>

>  
> +#define VOICECALL_STORE "voicecall"

Can you please be consistent with the rest of the code and name this
SETTINGS_STORE?

> +#define LASTDIAL_GROUP "LastDial"
> +

Same here, call this SETTINGS_GROUP and use "Settings"

Regards,
-Denis