Re: [PATCH 3/4] stk: Handle set up call in handled_notify

Re: [PATCH 3/4] stk: Handle set up call in handled_notify

[Denis Kenzior]

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

Hi Jeevaka,

On 07/06/2011 05:06 AM, Jeevaka Badrappan wrote:
> ---
>  src/stk.c |  108 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 106 insertions(+), 2 deletions(-)
> 
> diff --git a/src/stk.c b/src/stk.c
> index 9575f0e..26faa11 100644
> --- a/src/stk.c
> +++ b/src/stk.c
> @@ -77,6 +77,8 @@ struct ofono_stk {
>  	struct timeval get_inkey_start_ts;
>  	int dtmf_id;
>  
> +	gboolean modem_handled_cmd;
> +
>  	__ofono_sms_sim_download_cb_t sms_pp_cb;
>  	void *sms_pp_userdata;
>  };
> @@ -1785,6 +1787,44 @@ static void confirm_call_cb(enum stk_agent_result result, gboolean confirm,
>  		stk_command_cb(&error, stk);
>  }
>  
> +static void confirm_handled_call_cb(enum stk_agent_result result,
> +					gboolean confirm, void *user_data)
> +{
> +	struct ofono_stk *stk = user_data;
> +	const struct stk_command_setup_call *sc =
> +					&stk->pending_cmd->setup_call;
> +	struct ofono_voicecall *vc = NULL;
> +	struct ofono_atom *vc_atom;
> +
> +	if (stk->driver->user_confirmation == NULL)
> +		goto out;

This situation is quite funny, I'd almost rather crash here, but fair
enough.

> +
> +	if (result != STK_AGENT_RESULT_OK) {
> +		stk->driver->user_confirmation(stk, FALSE);
> +		goto out;
> +	}
> +
> +	stk->driver->user_confirmation(stk, confirm);
> +
> +	vc_atom = __ofono_modem_find_atom(__ofono_atom_get_modem(stk->atom),
> +						OFONO_ATOM_TYPE_VOICECALL);
> +	if (vc_atom)
> +		vc = __ofono_atom_get_data(vc_atom);
> +
> +	if (vc == NULL)
> +		goto out;
> +
> +	__ofono_voicecall_set_alpha_and_icon_id(vc, sc->alpha_id_call_setup,
> +						sc->icon_id_call_setup.id);
> +
> +	stk->modem_handled_cmd = TRUE;
> +
> +	return;

Please insert a newline here

> +out:
> +	stk_command_free(stk->pending_cmd);
> +	stk->pending_cmd = NULL;
> +}
> +
>  static gboolean handle_command_set_up_call(const struct stk_command *cmd,
>  						struct stk_response *rsp,
>  						struct ofono_stk *stk)
> @@ -2601,6 +2641,40 @@ static gboolean handle_command_launch_browser(const struct stk_command *cmd,
>  	return FALSE;
>  }
>  
> +static void handle_setup_call_confirmation_req(struct stk_command *cmd,
> +						struct ofono_stk *stk)
> +{
> +	const struct stk_command_setup_call *sc = &cmd->setup_call;
> +	int err;
> +	char *alpha_id = dbus_apply_text_attributes(
> +					sc->alpha_id_usr_cfm ?
> +					sc->alpha_id_usr_cfm : "",
> +					&sc->text_attr_usr_cfm);
> +	if (alpha_id == NULL)
> +		goto out;
> +
> +	err = stk_agent_confirm_call(stk->current_agent, alpha_id,
> +					&sc->icon_id_usr_cfm,
> +					confirm_handled_call_cb,
> +					stk, NULL,
> +					stk->timeout * 1000);
> +	g_free(alpha_id);
> +
> +	if (err < 0)
> +		goto out;
> +
> +	stk->pending_cmd = cmd;
> +	stk->cancel_cmd = stk_request_cancel;
> +
> +	return;
> +
> +out:
> +	if (stk->driver->user_confirmation)
> +		stk->driver->user_confirmation(stk, FALSE);
> +
> +	stk_command_free(cmd);
> +}
> +
>  static void stk_proactive_command_cancel(struct ofono_stk *stk)
>  {
>  	if (stk->immediate_response)
> @@ -2615,8 +2689,38 @@ static void stk_proactive_command_cancel(struct ofono_stk *stk)
>  	}
>  }
>  
> +static void proactive_session_end_handled_cmd(struct ofono_stk *stk)
> +{
> +	stk->modem_handled_cmd = FALSE;
> +
> +	switch(stk->pending_cmd->type) {
> +	case STK_COMMAND_TYPE_SETUP_CALL:
> +	{
> +		struct ofono_voicecall *vc = NULL;
> +		struct ofono_atom *vc_atom;
> +
> +		vc_atom = __ofono_modem_find_atom(
> +					__ofono_atom_get_modem(stk->atom),
> +					OFONO_ATOM_TYPE_VOICECALL);
> +		if (vc_atom)
> +			vc = __ofono_atom_get_data(vc_atom);
> +
> +		if (vc != NULL)
> +			__ofono_voicecall_clear_alpha_and_icon_id(vc);
> +
> +		break;
> +	}
> +
> +	default:
> +		break;
> +	}
> +}
> +
>  void ofono_stk_proactive_session_end_notify(struct ofono_stk *stk)
>  {
> +	if (stk->modem_handled_cmd == TRUE)
> +		proactive_session_end_handled_cmd(stk);
> +

Are we getting information on the terminal responses sent by the modem
for these?  I'd rather handle this sort of stuff by parsing the terminal
response than relying on the session end notification.  This might also
help alleviate the concerns Andrew has raised as well.

>  	/* Wait until we receive the next command */
>  	if (stk->immediate_response)
>  		return;
> @@ -2858,8 +2962,8 @@ void ofono_stk_proactive_command_handled_notify(struct ofono_stk *stk,
>  		break;
>  
>  	case STK_COMMAND_TYPE_SETUP_CALL:
> -		/* TODO */
> -		break;
> +		handle_setup_call_confirmation_req(cmd, stk);
> +		return;
>  
>  	case STK_COMMAND_TYPE_SEND_USSD:
>  		stk_alpha_id_set(stk, cmd->send_ussd.alpha_id,

Regards,
-Denis

[PATCH 3/4] stk: Handle set up call in handled_notify

[Jeevaka Badrappan]

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

---
 src/stk.c |  108 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 106 insertions(+), 2 deletions(-)

diff --git a/src/stk.c b/src/stk.c
index 9575f0e..26faa11 100644
--- a/src/stk.c
+++ b/src/stk.c
@@ -77,6 +77,8 @@ struct ofono_stk {
 	struct timeval get_inkey_start_ts;
 	int dtmf_id;
 
+	gboolean modem_handled_cmd;
+
 	__ofono_sms_sim_download_cb_t sms_pp_cb;
 	void *sms_pp_userdata;
 };
@@ -1785,6 +1787,44 @@ static void confirm_call_cb(enum stk_agent_result result, gboolean confirm,
 		stk_command_cb(&error, stk);
 }
 
+static void confirm_handled_call_cb(enum stk_agent_result result,
+					gboolean confirm, void *user_data)
+{
+	struct ofono_stk *stk = user_data;
+	const struct stk_command_setup_call *sc =
+					&stk->pending_cmd->setup_call;
+	struct ofono_voicecall *vc = NULL;
+	struct ofono_atom *vc_atom;
+
+	if (stk->driver->user_confirmation == NULL)
+		goto out;
+
+	if (result != STK_AGENT_RESULT_OK) {
+		stk->driver->user_confirmation(stk, FALSE);
+		goto out;
+	}
+
+	stk->driver->user_confirmation(stk, confirm);
+
+	vc_atom = __ofono_modem_find_atom(__ofono_atom_get_modem(stk->atom),
+						OFONO_ATOM_TYPE_VOICECALL);
+	if (vc_atom)
+		vc = __ofono_atom_get_data(vc_atom);
+
+	if (vc == NULL)
+		goto out;
+
+	__ofono_voicecall_set_alpha_and_icon_id(vc, sc->alpha_id_call_setup,
+						sc->icon_id_call_setup.id);
+
+	stk->modem_handled_cmd = TRUE;
+
+	return;
+out:
+	stk_command_free(stk->pending_cmd);
+	stk->pending_cmd = NULL;
+}
+
 static gboolean handle_command_set_up_call(const struct stk_command *cmd,
 						struct stk_response *rsp,
 						struct ofono_stk *stk)
@@ -2601,6 +2641,40 @@ static gboolean handle_command_launch_browser(const struct stk_command *cmd,
 	return FALSE;
 }
 
+static void handle_setup_call_confirmation_req(struct stk_command *cmd,
+						struct ofono_stk *stk)
+{
+	const struct stk_command_setup_call *sc = &cmd->setup_call;
+	int err;
+	char *alpha_id = dbus_apply_text_attributes(
+					sc->alpha_id_usr_cfm ?
+					sc->alpha_id_usr_cfm : "",
+					&sc->text_attr_usr_cfm);
+	if (alpha_id == NULL)
+		goto out;
+
+	err = stk_agent_confirm_call(stk->current_agent, alpha_id,
+					&sc->icon_id_usr_cfm,
+					confirm_handled_call_cb,
+					stk, NULL,
+					stk->timeout * 1000);
+	g_free(alpha_id);
+
+	if (err < 0)
+		goto out;
+
+	stk->pending_cmd = cmd;
+	stk->cancel_cmd = stk_request_cancel;
+
+	return;
+
+out:
+	if (stk->driver->user_confirmation)
+		stk->driver->user_confirmation(stk, FALSE);
+
+	stk_command_free(cmd);
+}
+
 static void stk_proactive_command_cancel(struct ofono_stk *stk)
 {
 	if (stk->immediate_response)
@@ -2615,8 +2689,38 @@ static void stk_proactive_command_cancel(struct ofono_stk *stk)
 	}
 }
 
+static void proactive_session_end_handled_cmd(struct ofono_stk *stk)
+{
+	stk->modem_handled_cmd = FALSE;
+
+	switch(stk->pending_cmd->type) {
+	case STK_COMMAND_TYPE_SETUP_CALL:
+	{
+		struct ofono_voicecall *vc = NULL;
+		struct ofono_atom *vc_atom;
+
+		vc_atom = __ofono_modem_find_atom(
+					__ofono_atom_get_modem(stk->atom),
+					OFONO_ATOM_TYPE_VOICECALL);
+		if (vc_atom)
+			vc = __ofono_atom_get_data(vc_atom);
+
+		if (vc != NULL)
+			__ofono_voicecall_clear_alpha_and_icon_id(vc);
+
+		break;
+	}
+
+	default:
+		break;
+	}
+}
+
 void ofono_stk_proactive_session_end_notify(struct ofono_stk *stk)
 {
+	if (stk->modem_handled_cmd == TRUE)
+		proactive_session_end_handled_cmd(stk);
+
 	/* Wait until we receive the next command */
 	if (stk->immediate_response)
 		return;
@@ -2858,8 +2962,8 @@ void ofono_stk_proactive_command_handled_notify(struct ofono_stk *stk,
 		break;
 
 	case STK_COMMAND_TYPE_SETUP_CALL:
-		/* TODO */
-		break;
+		handle_setup_call_confirmation_req(cmd, stk);
+		return;
 
 	case STK_COMMAND_TYPE_SEND_USSD:
 		stk_alpha_id_set(stk, cmd->send_ussd.alpha_id,
-- 
1.7.4.1

Re: [PATCH 3/4] stk: Handle set up call in handled_notify

[Andrzej Zaborowski]

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

Hi Jeevaka,

On 6 July 2011 12:06, Jeevaka Badrappan
<jeevaka.badrappan@linux.intel.com> wrote:
> ---
>  src/stk.c |  108 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 106 insertions(+), 2 deletions(-)
>
> diff --git a/src/stk.c b/src/stk.c
> index 9575f0e..26faa11 100644
> --- a/src/stk.c
> +++ b/src/stk.c
> @@ -77,6 +77,8 @@ struct ofono_stk {
>        struct timeval get_inkey_start_ts;
>        int dtmf_id;
>
> +       gboolean modem_handled_cmd;
> +
>        __ofono_sms_sim_download_cb_t sms_pp_cb;
>        void *sms_pp_userdata;
>  };
> @@ -1785,6 +1787,44 @@ static void confirm_call_cb(enum stk_agent_result result, gboolean confirm,
>                stk_command_cb(&error, stk);
>  }
>
> +static void confirm_handled_call_cb(enum stk_agent_result result,
> +                                       gboolean confirm, void *user_data)
> +{
> +       struct ofono_stk *stk = user_data;
> +       const struct stk_command_setup_call *sc =
> +                                       &stk->pending_cmd->setup_call;
> +       struct ofono_voicecall *vc = NULL;
> +       struct ofono_atom *vc_atom;
> +
> +       if (stk->driver->user_confirmation == NULL)
> +               goto out;
> +
> +       if (result != STK_AGENT_RESULT_OK) {
> +               stk->driver->user_confirmation(stk, FALSE);
> +               goto out;
> +       }
> +
> +       stk->driver->user_confirmation(stk, confirm);
> +
> +       vc_atom = __ofono_modem_find_atom(__ofono_atom_get_modem(stk->atom),
> +                                               OFONO_ATOM_TYPE_VOICECALL);
> +       if (vc_atom)
> +               vc = __ofono_atom_get_data(vc_atom);
> +
> +       if (vc == NULL)
> +               goto out;
> +
> +       __ofono_voicecall_set_alpha_and_icon_id(vc, sc->alpha_id_call_setup,
> +                                               sc->icon_id_call_setup.id);
> +
> +       stk->modem_handled_cmd = TRUE;
> +
> +       return;
> +out:
> +       stk_command_free(stk->pending_cmd);
> +       stk->pending_cmd = NULL;
> +}
> +
>  static gboolean handle_command_set_up_call(const struct stk_command *cmd,
>                                                struct stk_response *rsp,
>                                                struct ofono_stk *stk)
> @@ -2601,6 +2641,40 @@ static gboolean handle_command_launch_browser(const struct stk_command *cmd,
>        return FALSE;
>  }
>
> +static void handle_setup_call_confirmation_req(struct stk_command *cmd,
> +                                               struct ofono_stk *stk)
> +{
> +       const struct stk_command_setup_call *sc = &cmd->setup_call;
> +       int err;
> +       char *alpha_id = dbus_apply_text_attributes(
> +                                       sc->alpha_id_usr_cfm ?
> +                                       sc->alpha_id_usr_cfm : "",
> +                                       &sc->text_attr_usr_cfm);
> +       if (alpha_id == NULL)
> +               goto out;
> +
> +       err = stk_agent_confirm_call(stk->current_agent, alpha_id,
> +                                       &sc->icon_id_usr_cfm,
> +                                       confirm_handled_call_cb,
> +                                       stk, NULL,
> +                                       stk->timeout * 1000);
> +       g_free(alpha_id);
> +
> +       if (err < 0)
> +               goto out;
> +
> +       stk->pending_cmd = cmd;
> +       stk->cancel_cmd = stk_request_cancel;
> +
> +       return;
> +
> +out:
> +       if (stk->driver->user_confirmation)
> +               stk->driver->user_confirmation(stk, FALSE);
> +
> +       stk_command_free(cmd);
> +}
> +
>  static void stk_proactive_command_cancel(struct ofono_stk *stk)
>  {
>        if (stk->immediate_response)
> @@ -2615,8 +2689,38 @@ static void stk_proactive_command_cancel(struct ofono_stk *stk)
>        }
>  }
>
> +static void proactive_session_end_handled_cmd(struct ofono_stk *stk)
> +{
> +       stk->modem_handled_cmd = FALSE;
> +
> +       switch(stk->pending_cmd->type) {
> +       case STK_COMMAND_TYPE_SETUP_CALL:
> +       {
> +               struct ofono_voicecall *vc = NULL;
> +               struct ofono_atom *vc_atom;
> +
> +               vc_atom = __ofono_modem_find_atom(
> +                                       __ofono_atom_get_modem(stk->atom),
> +                                       OFONO_ATOM_TYPE_VOICECALL);
> +               if (vc_atom)
> +                       vc = __ofono_atom_get_data(vc_atom);
> +
> +               if (vc != NULL)
> +                       __ofono_voicecall_clear_alpha_and_icon_id(vc);
> +
> +               break;

This part seems fragile.  A new command may arrive and the alpha id
and icon id won't be cleared.  Additionally modem_handled_cmd is not
reset when the pending_cmd is cancelled so it may affect a future
command.  Maybe voicecall.c should automatically clear the alpha &
icon id after the first new call?

Best regards

[PATCH 0/4] Add support for modem handled setup call proactive command

[Jeevaka Badrappan]

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

Hi,

 This patch set fixes the review comments and also fixes some of the corner cases.

 If the modem is able to setup the call, then the alpha id will be freed when the
initiated call ends.

 If the modem is not able to setup the call due to some reason, then the memory
allocated for alpha id and dial request are freed when the command end/session
end is received.

Thanks and Regards,
Jeevaka

Jeevaka Badrappan (4):
  include: Add driver api for user confirmation
  voicecall: api for set/clear alpha and icon id
  stk: Handle set up call in handled_notify
  ifxmodem: add support for user_confirmation in stk

 drivers/ifxmodem/stk.c |   11 ++++
 include/stk.h          |    1 +
 src/ofono.h            |    6 ++
 src/stk.c              |  122 ++++++++++++++++++++++++++++++++++++++++++++++-
 src/voicecall.c        |   76 ++++++++++++++++++++++++++++++
 5 files changed, 213 insertions(+), 3 deletions(-)

-- 
1.7.4.1

[PATCH 1/4] include: Add driver api for user confirmation

[Jeevaka Badrappan]

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

---
 include/stk.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/include/stk.h b/include/stk.h
index c276c84..60636b5 100644
--- a/include/stk.h
+++ b/include/stk.h
@@ -47,6 +47,7 @@ struct ofono_stk_driver {
 	void (*terminal_response)(struct ofono_stk *stk,
 					int length, const unsigned char *resp,
 					ofono_stk_generic_cb_t cb, void *data);
+	void (*user_confirmation)(struct ofono_stk *stk, ofono_bool_t confirm);
 };
 
 int ofono_stk_driver_register(const struct ofono_stk_driver *d);
-- 
1.7.4.1

[PATCH 2/4] voicecall: api for set/clear alpha and icon id

[Jeevaka Badrappan]

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

---
 src/ofono.h     |    6 ++++
 src/voicecall.c |   76 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 82 insertions(+), 0 deletions(-)

diff --git a/src/ofono.h b/src/ofono.h
index 6524806..808a8f1 100644
--- a/src/ofono.h
+++ b/src/ofono.h
@@ -266,6 +266,12 @@ int __ofono_voicecall_dial(struct ofono_voicecall *vc,
 				ofono_voicecall_dial_cb_t cb, void *user_data);
 void __ofono_voicecall_dial_cancel(struct ofono_voicecall *vc);
 
+void __ofono_voicecall_set_alpha_and_icon_id(struct ofono_voicecall *vc,
+						const char *addr, int addr_type,
+						const char *message,
+						unsigned char icon_id);
+void __ofono_voicecall_clear_alpha_and_icon_id(struct ofono_voicecall *vc);
+
 int __ofono_voicecall_tone_send(struct ofono_voicecall *vc,
 				const char *tone_str,
 				ofono_voicecall_tone_cb_t cb, void *user_data);
diff --git a/src/voicecall.c b/src/voicecall.c
index 3646951..33041a1 100644
--- a/src/voicecall.c
+++ b/src/voicecall.c
@@ -2204,6 +2204,25 @@ void ofono_voicecall_notify(struct ofono_voicecall *vc,
 		goto error;
 	}
 
+	if (vc->dial_req) {
+		v->message = vc->dial_req->message;
+		v->icon_id = vc->dial_req->icon_id;
+
+		vc->dial_req->message = NULL;
+		vc->dial_req->call = v;
+
+		newcall->phone_number.type = vc->dial_req->ph.type;
+		strncpy(newcall->phone_number.number, vc->dial_req->ph.number,
+									20);
+
+		/*
+		 * TS 102 223 Section 6.4.13: The terminal shall not store
+		 * in the UICC the call set-up details (called party number
+		 * and associated parameters)
+		 */
+		v->untracked = TRUE;
+	}
+
 	v->detect_time = time(NULL);
 
 	if (!voicecall_dbus_register(v)) {
@@ -3659,6 +3678,63 @@ void __ofono_voicecall_tone_cancel(struct ofono_voicecall *vc, int id)
 	}
 }
 
+void __ofono_voicecall_set_alpha_and_icon_id(struct ofono_voicecall *vc,
+						const char *addr, int addr_type,
+						const char *message,
+						unsigned char icon_id)
+{
+	struct dial_request *req;
+	const char *number;
+
+	req = g_new0(struct dial_request, 1);
+
+	req->message = g_strdup(message);
+	req->icon_id = icon_id;
+
+	req->ph.type = addr_type;
+	strncpy(req->ph.number, addr, 20);
+
+	number = phone_number_to_string(&req->ph);
+
+	if (!strcmp(number, "112")) {
+		struct ofono_modem *modem = __ofono_atom_get_modem(vc->atom);
+
+		__ofono_modem_inc_emergency_mode(modem);
+	}
+
+	vc->dial_req = req;
+}
+
+void __ofono_voicecall_clear_alpha_and_icon_id(struct ofono_voicecall *vc)
+{
+	const char *number;
+
+	if (vc->dial_req == NULL)
+		return;
+
+	number = phone_number_to_string(&vc->dial_req->ph);
+
+	/*
+	 * Incase the modem fails to setup the call and if there is no call
+	 * created, then the emergency mode has to be cleared.
+	 */
+	if (!strcmp(number, "112")) {
+		if (voicecalls_num_active(vc) == 0 &&
+				voicecalls_num_connecting(vc) == 0) {
+			struct ofono_modem *modem =
+					__ofono_atom_get_modem(vc->atom);
+
+			__ofono_modem_dec_emergency_mode(modem);
+		}
+	}
+
+	g_free(vc->dial_req->message);
+	vc->dial_req->message = NULL;
+
+	g_free(vc->dial_req);
+	vc->dial_req = NULL;
+}
+
 static void ssn_mt_forwarded_notify(struct ofono_voicecall *vc,
 					unsigned int id, int code,
 					const struct ofono_phone_number *ph)
-- 
1.7.4.1

[PATCH 3/4] stk: Handle set up call in handled_notify

[Jeevaka Badrappan]

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

---
 src/stk.c |  122 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 119 insertions(+), 3 deletions(-)

diff --git a/src/stk.c b/src/stk.c
index 4df23b5..6cc3be3 100644
--- a/src/stk.c
+++ b/src/stk.c
@@ -77,6 +77,8 @@ struct ofono_stk {
 	struct timeval get_inkey_start_ts;
 	int dtmf_id;
 
+	gboolean modem_handled_cmd;
+
 	__ofono_sms_sim_download_cb_t sms_pp_cb;
 	void *sms_pp_userdata;
 };
@@ -529,7 +531,12 @@ static void stk_alpha_id_set(struct ofono_stk *stk,
 
 static void stk_alpha_id_unset(struct ofono_stk *stk)
 {
-	stk_agent_request_cancel(stk->current_agent);
+	/*
+	 * If there is no default agent, then current agent also will be NULL.
+	 * So, call request cancel only when there is a valid current agent.
+	 */
+	if (stk->current_agent)
+		stk_agent_request_cancel(stk->current_agent);
 }
 
 static int duration_to_msecs(const struct stk_duration *duration)
@@ -1785,6 +1792,47 @@ static void confirm_call_cb(enum stk_agent_result result, gboolean confirm,
 		stk_command_cb(&error, stk);
 }
 
+static void confirm_handled_call_cb(enum stk_agent_result result,
+					gboolean confirm, void *user_data)
+{
+	struct ofono_stk *stk = user_data;
+	const struct stk_command_setup_call *sc =
+					&stk->pending_cmd->setup_call;
+	struct ofono_voicecall *vc = NULL;
+	struct ofono_atom *vc_atom;
+
+	if (stk->driver->user_confirmation == NULL)
+		goto out;
+
+	if (result != STK_AGENT_RESULT_OK) {
+		stk->driver->user_confirmation(stk, FALSE);
+		goto out;
+	}
+
+	stk->driver->user_confirmation(stk, confirm);
+
+	vc_atom = __ofono_modem_find_atom(__ofono_atom_get_modem(stk->atom),
+						OFONO_ATOM_TYPE_VOICECALL);
+	if (vc_atom)
+		vc = __ofono_atom_get_data(vc_atom);
+
+	if (vc == NULL)
+		goto out;
+
+	__ofono_voicecall_set_alpha_and_icon_id(vc, sc->addr.number,
+						sc->addr.ton_npi,
+						sc->alpha_id_call_setup,
+						sc->icon_id_call_setup.id);
+
+	stk->modem_handled_cmd = TRUE;
+
+	return;
+
+out:
+	stk_command_free(stk->pending_cmd);
+	stk->pending_cmd = NULL;
+}
+
 static gboolean handle_command_set_up_call(const struct stk_command *cmd,
 						struct stk_response *rsp,
 						struct ofono_stk *stk)
@@ -2601,8 +2649,75 @@ static gboolean handle_command_launch_browser(const struct stk_command *cmd,
 	return FALSE;
 }
 
+static void handle_setup_call_confirmation_req(struct stk_command *cmd,
+						struct ofono_stk *stk)
+{
+	const struct stk_command_setup_call *sc = &cmd->setup_call;
+	int err;
+	char *alpha_id = dbus_apply_text_attributes(
+					sc->alpha_id_usr_cfm ?
+					sc->alpha_id_usr_cfm : "",
+					&sc->text_attr_usr_cfm);
+	if (alpha_id == NULL)
+		goto out;
+
+	err = stk_agent_confirm_call(stk->current_agent, alpha_id,
+					&sc->icon_id_usr_cfm,
+					confirm_handled_call_cb,
+					stk, NULL,
+					stk->timeout * 1000);
+	g_free(alpha_id);
+
+	if (err < 0)
+		goto out;
+
+	stk->pending_cmd = cmd;
+	stk->cancel_cmd = stk_request_cancel;
+
+	return;
+
+out:
+	if (stk->driver->user_confirmation)
+		stk->driver->user_confirmation(stk, FALSE);
+
+	stk_command_free(cmd);
+}
+
+static void proactive_command_handled_end(struct ofono_stk *stk)
+{
+	stk->modem_handled_cmd = FALSE;
+
+	if (stk->pending_cmd == NULL)
+		return;
+
+	switch(stk->pending_cmd->type) {
+	case STK_COMMAND_TYPE_SETUP_CALL:
+	{
+		struct ofono_voicecall *vc = NULL;
+		struct ofono_atom *vc_atom;
+
+		vc_atom = __ofono_modem_find_atom(
+					__ofono_atom_get_modem(stk->atom),
+					OFONO_ATOM_TYPE_VOICECALL);
+		if (vc_atom)
+			vc = __ofono_atom_get_data(vc_atom);
+
+		if (vc != NULL)
+			__ofono_voicecall_clear_alpha_and_icon_id(vc);
+
+		break;
+	}
+
+	default:
+		break;
+	}
+}
+
 static void stk_proactive_command_cancel(struct ofono_stk *stk)
 {
+	if (stk->modem_handled_cmd == TRUE)
+		proactive_command_handled_end(stk);
+
 	if (stk->immediate_response)
 		stk_request_cancel(stk);
 
@@ -2821,6 +2936,7 @@ void ofono_stk_proactive_command_handled_notify(struct ofono_stk *stk,
 	 * responses here
 	 */
 	if (length > 0 && pdu[0] == 0x81) {
+		proactive_command_handled_end(stk);
 		stk_alpha_id_unset(stk);
 		return;
 	}
@@ -2858,8 +2974,8 @@ void ofono_stk_proactive_command_handled_notify(struct ofono_stk *stk,
 		break;
 
 	case STK_COMMAND_TYPE_SETUP_CALL:
-		/* TODO */
-		break;
+		handle_setup_call_confirmation_req(cmd, stk);
+		return;
 
 	case STK_COMMAND_TYPE_SEND_USSD:
 		stk_alpha_id_set(stk, cmd->send_ussd.alpha_id,
-- 
1.7.4.1

[PATCH 4/4] ifxmodem: add support for user_confirmation in stk

[Jeevaka Badrappan]

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

---
 drivers/ifxmodem/stk.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/drivers/ifxmodem/stk.c b/drivers/ifxmodem/stk.c
index f08cf47..d2082c4 100644
--- a/drivers/ifxmodem/stk.c
+++ b/drivers/ifxmodem/stk.c
@@ -173,6 +173,16 @@ error:
 	CALLBACK_WITH_FAILURE(cb, data);
 }
 
+static void ifx_stk_user_confirmation(struct ofono_stk *stk, gboolean confirm)
+{
+	struct stk_data *sd = ofono_stk_get_data(stk);
+	char buf[20];
+
+	snprintf(buf, sizeof(buf), "AT+SATD=%i", confirm ? 1 : 0);
+
+	g_at_chat_send(sd->chat, buf, none_prefix, NULL, NULL, NULL);
+}
+
 static void sati_notify(GAtResult *result, gpointer user_data)
 {
 	struct ofono_stk *stk = user_data;
@@ -303,6 +313,7 @@ static struct ofono_stk_driver driver = {
 	.remove			= ifx_stk_remove,
 	.envelope		= ifx_stk_envelope,
 	.terminal_response	= ifx_stk_terminal_response,
+	.user_confirmation	= ifx_stk_user_confirmation,
 };
 
 void ifx_stk_init(void)
-- 
1.7.4.1

Re: [PATCH 3/4] stk: Handle set up call in handled_notify

[jeevaka.badrappan]

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

Hi,

> @@ -529,7 +531,12 @@ static void stk_alpha_id_set(struct ofono_stk *stk,
>
>  static void stk_alpha_id_unset(struct ofono_stk *stk)
>  {
> -	stk_agent_request_cancel(stk->current_agent);
> +	/*
> +	 * If there is no default agent, then current agent also will be NULL.
> +	 * So, call request cancel only when there is a valid current agent.
> +	 */
> +	if (stk->current_agent)
> +		stk_agent_request_cancel(stk->current_agent);
>  }
>

If needed, I can send this code change alone in a separate fix patch.

Regards,
Jeevaka

Re: [PATCH 2/4] voicecall: api for set/clear alpha and icon id

[Denis Kenzior]

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

Hi Jeevaka,

On 07/15/2011 07:42 AM, Jeevaka Badrappan wrote:
> ---
>  src/ofono.h     |    6 ++++
>  src/voicecall.c |   76 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 82 insertions(+), 0 deletions(-)
> 
> diff --git a/src/ofono.h b/src/ofono.h
> index 6524806..808a8f1 100644
> --- a/src/ofono.h
> +++ b/src/ofono.h
> @@ -266,6 +266,12 @@ int __ofono_voicecall_dial(struct ofono_voicecall *vc,
>  				ofono_voicecall_dial_cb_t cb, void *user_data);
>  void __ofono_voicecall_dial_cancel(struct ofono_voicecall *vc);
>  
> +void __ofono_voicecall_set_alpha_and_icon_id(struct ofono_voicecall *vc,
> +						const char *addr, int addr_type,
> +						const char *message,
> +						unsigned char icon_id);
> +void __ofono_voicecall_clear_alpha_and_icon_id(struct ofono_voicecall *vc);
> +
>  int __ofono_voicecall_tone_send(struct ofono_voicecall *vc,
>  				const char *tone_str,
>  				ofono_voicecall_tone_cb_t cb, void *user_data);
> diff --git a/src/voicecall.c b/src/voicecall.c
> index 3646951..33041a1 100644
> --- a/src/voicecall.c
> +++ b/src/voicecall.c
> @@ -2204,6 +2204,25 @@ void ofono_voicecall_notify(struct ofono_voicecall *vc,
>  		goto error;
>  	}
>  
> +	if (vc->dial_req) {
> +		v->message = vc->dial_req->message;
> +		v->icon_id = vc->dial_req->icon_id;
> +
> +		vc->dial_req->message = NULL;
> +		vc->dial_req->call = v;
> +
> +		newcall->phone_number.type = vc->dial_req->ph.type;
> +		strncpy(newcall->phone_number.number, vc->dial_req->ph.number,
> +									20);

Do you really want to do this?  I suspect we should override the phone
number information only if the CLIP validity is not valid.

> +
> +		/*
> +		 * TS 102 223 Section 6.4.13: The terminal shall not store
> +		 * in the UICC the call set-up details (called party number
> +		 * and associated parameters)
> +		 */
> +		v->untracked = TRUE;
> +	}
> +

This looks fine, but there's a very subtle thing going on which breaks
one particular case.  Some background, voicecall driver is free to
notify the call before returning from the atd callback, or vice versa.

So e.g.

#1
driver->dial(.., cb, data);
ofono_voicecall_notify()
cb(data);

or

#2
driver->dial(..., cb, data);
cb(data);
ofono_voicecall_notify()

In the case that oFono generates the Set Up Call Request and assuming
driver behavior #1, you end up performing some steps twice.  Perhaps we
should set a flag in vc->flags to indicate that the call setup is being
initiated by the modem, and only trigger this path if this flag is set.

e.g.

if (vc->flags & OFONO_VOICECALL_FLAG_STK_MODEM_CALLSETUP) {
	...
}

This flag should be set by set_alpha_and_icon_id

>  	v->detect_time = time(NULL);
>  
>  	if (!voicecall_dbus_register(v)) {
> @@ -3659,6 +3678,63 @@ void __ofono_voicecall_tone_cancel(struct ofono_voicecall *vc, int id)
>  	}
>  }
>  
> +void __ofono_voicecall_set_alpha_and_icon_id(struct ofono_voicecall *vc,
> +						const char *addr, int addr_type,
> +						const char *message,
> +						unsigned char icon_id)
> +{
> +	struct dial_request *req;
> +	const char *number;
> +
> +	req = g_new0(struct dial_request, 1);
> +
> +	req->message = g_strdup(message);
> +	req->icon_id = icon_id;
> +
> +	req->ph.type = addr_type;
> +	strncpy(req->ph.number, addr, 20);
> +
> +	number = phone_number_to_string(&req->ph);
> +
> +	if (!strcmp(number, "112")) {
> +		struct ofono_modem *modem = __ofono_atom_get_modem(vc->atom);
> +
> +		__ofono_modem_inc_emergency_mode(modem);
> +	}

I think we should do this step in the block of code in
ofono_voicecall_notify dealing with modem callsetup.  See below for more:

> +
> +	vc->dial_req = req;
> +}
> +
> +void __ofono_voicecall_clear_alpha_and_icon_id(struct ofono_voicecall *vc)
> +{
> +	const char *number;
> +
> +	if (vc->dial_req == NULL)
> +		return;
> +
> +	number = phone_number_to_string(&vc->dial_req->ph);
> +
> +	/*
> +	 * Incase the modem fails to setup the call and if there is no call
> +	 * created, then the emergency mode has to be cleared.
> +	 */
> +	if (!strcmp(number, "112")) {
> +		if (voicecalls_num_active(vc) == 0 &&
> +				voicecalls_num_connecting(vc) == 0) {
> +			struct ofono_modem *modem =
> +					__ofono_atom_get_modem(vc->atom);
> +
> +			__ofono_modem_dec_emergency_mode(modem);
> +		}
> +	}

I'm guessing what you're trying to take care of here is the case that
the set up call might have been canceled before the call was notified,
right?

Wouldn't it be better to do:

if (vc->dial_req->call == NULL)
	...

?

However, in the end I think it is simpler to only initiate the emergency
mode when we have the call, and let it be deactivated by the disconnect
notification.  Then this function becomes way simpler.

> +
> +	g_free(vc->dial_req->message);
> +	vc->dial_req->message = NULL;
> +
> +	g_free(vc->dial_req);
> +	vc->dial_req = NULL;
> +}
> +

However, I'm actually questioning the need for this function since the
cancellation can only be accomplished by hanging up the call before it
is active, in which case the modem will take care of sending the
appropriate terminal response.

>  static void ssn_mt_forwarded_notify(struct ofono_voicecall *vc,
>  					unsigned int id, int code,
>  					const struct ofono_phone_number *ph)

Regards,
-Denis

Re: [PATCH 1/4] include: Add driver api for user confirmation

[Denis Kenzior]

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

Hi Jeevaka,

On 07/15/2011 07:42 AM, Jeevaka Badrappan wrote:
> ---
>  include/stk.h |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 

Patch has been applied, thanks.

Regards,
-Denis

Re: [PATCH 4/4] ifxmodem: add support for user_confirmation in stk

[Denis Kenzior]

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

Hi Jeevaka,

On 07/15/2011 07:42 AM, Jeevaka Badrappan wrote:
> ---
>  drivers/ifxmodem/stk.c |   11 +++++++++++
>  1 files changed, 11 insertions(+), 0 deletions(-)
> 

Patch has been applied, thanks.

Regards,
-Denis

Re: [PATCH 3/4] stk: Handle set up call in handled_notify

[Denis Kenzior]

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

Hi Jeevaka,

On 07/15/2011 07:46 AM, jeevaka.badrappan(a)linux.intel.com wrote:
> Hi,
> 
>> @@ -529,7 +531,12 @@ static void stk_alpha_id_set(struct ofono_stk *stk,
>>
>>  static void stk_alpha_id_unset(struct ofono_stk *stk)
>>  {
>> -	stk_agent_request_cancel(stk->current_agent);
>> +	/*
>> +	 * If there is no default agent, then current agent also will be NULL.
>> +	 * So, call request cancel only when there is a valid current agent.
>> +	 */
>> +	if (stk->current_agent)
>> +		stk_agent_request_cancel(stk->current_agent);
>>  }
>>
> 
> If needed, I can send this code change alone in a separate fix patch.
> 

It sounds like you were getting a crash with this one, so please send it
as a separate patch.

Regards,
-Denis

Re: [PATCH 3/4] stk: Handle set up call in handled_notify

[Denis Kenzior]

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

Hi Andrew, Jeevaka,

<snip>

> +static void proactive_command_handled_end(struct ofono_stk *stk)
> +{
> +	stk->modem_handled_cmd = FALSE;
> +
> +	if (stk->pending_cmd == NULL)
> +		return;
> +
> +	switch(stk->pending_cmd->type) {
> +	case STK_COMMAND_TYPE_SETUP_CALL:
> +	{
> +		struct ofono_voicecall *vc = NULL;
> +		struct ofono_atom *vc_atom;
> +
> +		vc_atom = __ofono_modem_find_atom(
> +					__ofono_atom_get_modem(stk->atom),
> +					OFONO_ATOM_TYPE_VOICECALL);
> +		if (vc_atom)
> +			vc = __ofono_atom_get_data(vc_atom);
> +
> +		if (vc != NULL)
> +			__ofono_voicecall_clear_alpha_and_icon_id(vc);
> +
> +		break;
> +	}
> +
> +	default:
> +		break;
> +	}
> +}
> +

So I'm still not sure this part is actually needed.  Once the user has
acknowledged the call, there is no way for us to cancel it until the
call is notified to the voicecall driver.  So unless the modem is drunk,
I don't really see the need to cover this case.

Once the call is notified to voicecall driver, then the need for
clearing alpha/icon id disappears.  The voicecall driver will do this
automatically based on the ofono_voicecall_disconnected notification.

What do you guys think?

Regards,
-Denis

Re: [PATCH 2/4] voicecall: api for set/clear alpha and icon id

[jeevaka.badrappan]

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

Hi Denis,

>>  	}
>>
>> +	if (vc->dial_req) {
>> +		v->message = vc->dial_req->message;
>> +		v->icon_id = vc->dial_req->icon_id;
>> +
>> +		vc->dial_req->message = NULL;
>> +		vc->dial_req->call = v;
>> +
>> +		newcall->phone_number.type = vc->dial_req->ph.type;
>> +		strncpy(newcall->phone_number.number, vc->dial_req->ph.number,
>> +									20);
>
> Do you really want to do this?  I suspect we should override the phone
> number information only if the CLIP validity is not valid.

Agreed.

>
>> +
>> +		/*
>> +		 * TS 102 223 Section 6.4.13: The terminal shall not store
>> +		 * in the UICC the call set-up details (called party number
>> +		 * and associated parameters)
>> +		 */
>> +		v->untracked = TRUE;
>> +	}
>> +
>
> This looks fine, but there's a very subtle thing going on which breaks
> one particular case.  Some background, voicecall driver is free to
> notify the call before returning from the atd callback, or vice versa.
>
> So e.g.
>
> #1
> driver->dial(.., cb, data);
> ofono_voicecall_notify()
> cb(data);
>
> or
>
> #2
> driver->dial(..., cb, data);
> cb(data);
> ofono_voicecall_notify()
>
> In the case that oFono generates the Set Up Call Request and assuming
> driver behavior #1, you end up performing some steps twice.  Perhaps we
> should set a flag in vc->flags to indicate that the call setup is being
> initiated by the modem, and only trigger this path if this flag is set.
>

I haven't seen the case where the call status is notified before dial
callback. But agreed, we will end up in steps twice in this case. Proposed
change is fine with me. Will be incorporated in the next version.

>> +void __ofono_voicecall_clear_alpha_and_icon_id(struct ofono_voicecall
>> *vc)
>> +{
>> +	const char *number;
>> +
>> +	if (vc->dial_req == NULL)
>> +		return;
>> +
>> +	number = phone_number_to_string(&vc->dial_req->ph);
>> +
>> +	/*
>> +	 * Incase the modem fails to setup the call and if there is no call
>> +	 * created, then the emergency mode has to be cleared.
>> +	 */
>> +	if (!strcmp(number, "112")) {
>> +		if (voicecalls_num_active(vc) == 0 &&
>> +				voicecalls_num_connecting(vc) == 0) {
>> +			struct ofono_modem *modem =
>> +					__ofono_atom_get_modem(vc->atom);
>> +
>> +			__ofono_modem_dec_emergency_mode(modem);
>> +		}
>> +	}
>
> I'm guessing what you're trying to take care of here is the case that
> the set up call might have been canceled before the call was notified,
> right?
>
> Wouldn't it be better to do:
>
> if (vc->dial_req->call == NULL)
> 	...
>
> ?
>
> However, in the end I think it is simpler to only initiate the emergency
> mode when we have the call, and let it be deactivated by the disconnect
> notification.  Then this function becomes way simpler.
>
>> +
>> +	g_free(vc->dial_req->message);
>> +	vc->dial_req->message = NULL;
>> +
>> +	g_free(vc->dial_req);
>> +	vc->dial_req = NULL;
>> +}
>> +
>
> However, I'm actually questioning the need for this function since the
> cancellation can only be accomplished by hanging up the call before it
> is active, in which case the modem will take care of sending the
> appropriate terminal response.

Here, I'm trying to cover the following case:

1. oFono core is informed of the modem handled setup call and requests
user confirmation
2. User confirms the call
3. User confirmation sent to modem.
4. Due to some reason, modem is not able to setup the call. So, basically
oFono core is not notified of any call status notifications
5. Modem sends the terminal response to SAT and notifies oFono core of the
command/session completion

Regards,
Jeevaka

Re: [PATCH 3/4] stk: Handle set up call in handled_notify

[jeevaka.badrappan]

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

Hi Denis,

>> +static void proactive_command_handled_end(struct ofono_stk *stk)
>> +{
>> +	stk->modem_handled_cmd = FALSE;
>> +
>> +	if (stk->pending_cmd == NULL)
>> +		return;
>> +
>> +	switch(stk->pending_cmd->type) {
>> +	case STK_COMMAND_TYPE_SETUP_CALL:
>> +	{
>> +		struct ofono_voicecall *vc = NULL;
>> +		struct ofono_atom *vc_atom;
>> +
>> +		vc_atom = __ofono_modem_find_atom(
>> +					__ofono_atom_get_modem(stk->atom),
>> +					OFONO_ATOM_TYPE_VOICECALL);
>> +		if (vc_atom)
>> +			vc = __ofono_atom_get_data(vc_atom);
>> +
>> +		if (vc != NULL)
>> +			__ofono_voicecall_clear_alpha_and_icon_id(vc);
>> +
>> +		break;
>> +	}
>> +
>> +	default:
>> +		break;
>> +	}
>> +}
>> +
>
> So I'm still not sure this part is actually needed.  Once the user has
> acknowledged the call, there is no way for us to cancel it until the
> call is notified to the voicecall driver.  So unless the modem is drunk,
> I don't really see the need to cover this case.
>
> Once the call is notified to voicecall driver, then the need for
> clearing alpha/icon id disappears.  The voicecall driver will do this
> automatically based on the ofono_voicecall_disconnected notification.
>
> What do you guys think?

As pointed in other mail thred, trying to cover the following case:

1. oFono core is informed of the modem handled setup call and requests
user confirmation
2. User confirms the call
3. User confirmation sent to modem.
4. Due to some reason, modem is not able to setup the call. So, basically
oFono core is not notified of any call status notifications
5. Modem sends the terminal response to SAT and notifies oFono core of the
command/session completion

Basically, if the modem due to some reason fails to initiate/setup the
call and call status notifications not sent to the voice call driver side.
This case, we will be left with dial_req, message and icon id set but not
freed.

Regards,
Jeevaka

Re: [PATCH 3/4] stk: Handle set up call in handled_notify

[Denis Kenzior]

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

Hi Jeevaka,

On 07/15/2011 11:45 AM, jeevaka.badrappan(a)linux.intel.com wrote:
> Hi Denis,
> 
>>> +static void proactive_command_handled_end(struct ofono_stk *stk)
>>> +{
>>> +	stk->modem_handled_cmd = FALSE;
>>> +
>>> +	if (stk->pending_cmd == NULL)
>>> +		return;
>>> +
>>> +	switch(stk->pending_cmd->type) {
>>> +	case STK_COMMAND_TYPE_SETUP_CALL:
>>> +	{
>>> +		struct ofono_voicecall *vc = NULL;
>>> +		struct ofono_atom *vc_atom;
>>> +
>>> +		vc_atom = __ofono_modem_find_atom(
>>> +					__ofono_atom_get_modem(stk->atom),
>>> +					OFONO_ATOM_TYPE_VOICECALL);
>>> +		if (vc_atom)
>>> +			vc = __ofono_atom_get_data(vc_atom);
>>> +
>>> +		if (vc != NULL)
>>> +			__ofono_voicecall_clear_alpha_and_icon_id(vc);
>>> +
>>> +		break;
>>> +	}
>>> +
>>> +	default:
>>> +		break;
>>> +	}
>>> +}
>>> +
>>
>> So I'm still not sure this part is actually needed.  Once the user has
>> acknowledged the call, there is no way for us to cancel it until the
>> call is notified to the voicecall driver.  So unless the modem is drunk,
>> I don't really see the need to cover this case.
>>
>> Once the call is notified to voicecall driver, then the need for
>> clearing alpha/icon id disappears.  The voicecall driver will do this
>> automatically based on the ofono_voicecall_disconnected notification.
>>
>> What do you guys think?
> 
> As pointed in other mail thred, trying to cover the following case:
> 
> 1. oFono core is informed of the modem handled setup call and requests
> user confirmation
> 2. User confirms the call
> 3. User confirmation sent to modem.
> 4. Due to some reason, modem is not able to setup the call. So, basically
> oFono core is not notified of any call status notifications
> 5. Modem sends the terminal response to SAT and notifies oFono core of the
> command/session completion
> 
> Basically, if the modem due to some reason fails to initiate/setup the
> call and call status notifications not sent to the voice call driver side.
> This case, we will be left with dial_req, message and icon id set but not
> freed.
> 

OK I see now.  In theory we could do some basic sanity checks (e.g.
modem is online, etc) to alleviate this, but your proposal is probably
safer even if more complicated.

Also, you probably can formalize proactive_command_handled_end and not
call stk_unset_alpha_id unnecessarily in the case of modem-handled Set
Up Call.

And I'm still not sure about modem_handled_cmd variable.  Can't you set
cancel_cmd to proactive_command_handled_end?

Regards,
-Denis

Re: [PATCH 2/4] voicecall: api for set/clear alpha and icon id

[jeevaka.badrappan]

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

Hi Denis,

>> +
>> +	number = phone_number_to_string(&req->ph);
>> +
>> +	if (!strcmp(number, "112")) {
>> +		struct ofono_modem *modem = __ofono_atom_get_modem(vc->atom);
>> +
>> +		__ofono_modem_inc_emergency_mode(modem);
>> +	}
>
> I think we should do this step in the block of code in
> ofono_voicecall_notify dealing with modem callsetup.  See below for more:

Emergency mode for all other dial(user initiated, ofono handled setup
call) has been activated before we issue dial request to modem. Also I
believe its better to activate it here rather than in
ofono_voicecall_notify.

Regards,
Jeevaka

Re: [PATCH 2/4] voicecall: api for set/clear alpha and icon id

[Denis Kenzior]

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

Hi Jeevaka,

On 07/15/2011 12:24 PM, jeevaka.badrappan(a)linux.intel.com wrote:
> Hi Denis,
> 
>>> +
>>> +	number = phone_number_to_string(&req->ph);
>>> +
>>> +	if (!strcmp(number, "112")) {
>>> +		struct ofono_modem *modem = __ofono_atom_get_modem(vc->atom);
>>> +
>>> +		__ofono_modem_inc_emergency_mode(modem);
>>> +	}
>>
>> I think we should do this step in the block of code in
>> ofono_voicecall_notify dealing with modem callsetup.  See below for more:
> 
> Emergency mode for all other dial(user initiated, ofono handled setup
> call) has been activated before we issue dial request to modem. Also I
> believe its better to activate it here rather than in
> ofono_voicecall_notify.
> 

This is debatable since the real world result on the IFX modem is that
we activate emergency mode 15 ms earlier than we otherwise would, but
might cause funny behavior out of our control (for example, if STK
attempts an emergency call while offline, we just flip EmergencyMode to
true & back and go nowhere)

However, in the end I'm fine doing it this way, you just need to make
sure that the logic in clear_alpha_and_icon_id can handle this case.

Regards,
-Denis