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

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

[Jeevaka Badrappan]

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

Hi,

This patch set fixes the review comments.

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 (2):
  voicecall: api for set/clear alpha and icon id
  stk: Handle set up call in handled_notify

 src/ofono.h     |    6 +++
 src/stk.c       |  113 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 src/voicecall.c |   62 ++++++++++++++++++++++++++++++
 3 files changed, 178 insertions(+), 3 deletions(-)

-- 
1.7.4.1

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

[Jeevaka Badrappan]

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

---
 src/ofono.h     |    6 +++++
 src/voicecall.c |   62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 68 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..03a0c96 100644
--- a/src/voicecall.c
+++ b/src/voicecall.c
@@ -42,6 +42,7 @@
 #define MAX_VOICE_CALLS 16
 
 #define VOICECALL_FLAG_SIM_ECC_READY 0x1
+#define VOICECALL_FLAG_STK_MODEM_CALLSETUP 0x2
 
 #define SETTINGS_STORE "voicecall"
 #define SETTINGS_GROUP "Settings"
@@ -2158,6 +2159,8 @@ void ofono_voicecall_disconnected(struct ofono_voicecall *vc, int id,
 	voicecall_dbus_unregister(vc, call);
 
 	vc->call_list = g_slist_remove(vc->call_list, call);
+
+	vc->flags &= ~VOICECALL_FLAG_STK_MODEM_CALLSETUP;
 }
 
 void ofono_voicecall_notify(struct ofono_voicecall *vc,
@@ -2204,6 +2207,31 @@ void ofono_voicecall_notify(struct ofono_voicecall *vc,
 		goto error;
 	}
 
+	if (vc->flags & VOICECALL_FLAG_STK_MODEM_CALLSETUP) {
+		struct dial_request *req = vc->dial_req;
+		const char *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);
+		}
+
+		v->message = req->message;
+		v->icon_id = req->icon_id;
+
+		req->message = NULL;
+		req->call = v;
+
+		/*
+		 * 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 +3687,40 @@ 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;
+
+	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);
+
+	vc->dial_req = req;
+
+	vc->flags |= VOICECALL_FLAG_STK_MODEM_CALLSETUP;
+}
+
+void __ofono_voicecall_clear_alpha_and_icon_id(struct ofono_voicecall *vc)
+{
+	if (vc->dial_req == NULL)
+		return;
+
+	g_free(vc->dial_req->message);
+	vc->dial_req->message = NULL;
+
+	g_free(vc->dial_req);
+	vc->dial_req = NULL;
+
+	vc->flags &= ~VOICECALL_FLAG_STK_MODEM_CALLSETUP;
+}
+
 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

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

[Denis Kenzior]

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

Hi Jeevaka,

On 07/19/2011 02:13 AM, Jeevaka Badrappan wrote:
> ---
>  src/ofono.h     |    6 +++++
>  src/voicecall.c |   62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 68 insertions(+), 0 deletions(-)

Looks nearly there, just a few minor nitpicks:

> 
> 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..03a0c96 100644
> --- a/src/voicecall.c
> +++ b/src/voicecall.c
> @@ -42,6 +42,7 @@
>  #define MAX_VOICE_CALLS 16
>  
>  #define VOICECALL_FLAG_SIM_ECC_READY 0x1
> +#define VOICECALL_FLAG_STK_MODEM_CALLSETUP 0x2
>  
>  #define SETTINGS_STORE "voicecall"
>  #define SETTINGS_GROUP "Settings"
> @@ -2158,6 +2159,8 @@ void ofono_voicecall_disconnected(struct ofono_voicecall *vc, int id,
>  	voicecall_dbus_unregister(vc, call);
>  
>  	vc->call_list = g_slist_remove(vc->call_list, call);
> +
> +	vc->flags &= ~VOICECALL_FLAG_STK_MODEM_CALLSETUP;

This doesn't seem like it belongs here.  In fact there are situations
where even with an STK call setup pending, disconnections can still
happen. See TS 102.223 Section 6.4.13, in particular the case where
existing calls are put on hold (e.g. remote disconnection) or existing
calls being dropped.

>  }
>  
>  void ofono_voicecall_notify(struct ofono_voicecall *vc,
> @@ -2204,6 +2207,31 @@ void ofono_voicecall_notify(struct ofono_voicecall *vc,
>  		goto error;
>  	}
>  
> +	if (vc->flags & VOICECALL_FLAG_STK_MODEM_CALLSETUP) {
> +		struct dial_request *req = vc->dial_req;
> +		const char *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);
> +		}
> +

Are you forgetting to copy CLIP information in case the call doesn't
provide one?

> +		v->message = req->message;
> +		v->icon_id = req->icon_id;
> +
> +		req->message = NULL;
> +		req->call = v;
> +
> +		/*
> +		 * 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;

You should probably clear the STK_MODEM_CALLSETUP flag here, unless you
have another idea.

> +	}
> +
>  	v->detect_time = time(NULL);
>  
>  	if (!voicecall_dbus_register(v)) {
> @@ -3659,6 +3687,40 @@ 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;
> +
> +	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);
> +
> +	vc->dial_req = req;
> +
> +	vc->flags |= VOICECALL_FLAG_STK_MODEM_CALLSETUP;
> +}
> +
> +void __ofono_voicecall_clear_alpha_and_icon_id(struct ofono_voicecall *vc)
> +{
> +	if (vc->dial_req == NULL)
> +		return;
> +

We should not be calling this function if dial_req is NULL, so I'd
rather crash here if we do.  Especially since this API is private.

> +	g_free(vc->dial_req->message);
> +	vc->dial_req->message = NULL;
> +
> +	g_free(vc->dial_req);
> +	vc->dial_req = NULL;
> +
> +	vc->flags &= ~VOICECALL_FLAG_STK_MODEM_CALLSETUP;
> +}
> +
>  static void ssn_mt_forwarded_notify(struct ofono_voicecall *vc,
>  					unsigned int id, int code,
>  					const struct ofono_phone_number *ph)

Regards,
-Denis

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

[Jeevaka Badrappan]

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

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

diff --git a/src/stk.c b/src/stk.c
index e8ffb42..a204e45 100644
--- a/src/stk.c
+++ b/src/stk.c
@@ -1790,6 +1790,45 @@ 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);
+
+	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)
@@ -2606,6 +2645,74 @@ static gboolean handle_command_launch_browser(const struct stk_command *cmd,
 	return FALSE;
 }
 
+static void proactive_command_handled_end(struct ofono_stk *stk)
+{
+	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;
+	}
+	case STK_COMMAND_TYPE_SEND_SMS:
+	case STK_COMMAND_TYPE_SEND_USSD:
+	case STK_COMMAND_TYPE_SEND_SS:
+	case STK_COMMAND_TYPE_SEND_DTMF:
+		stk_alpha_id_unset(stk);
+		break;
+
+	default:
+		break;
+	}
+}
+
+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 = proactive_command_handled_end;
+
+	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)
@@ -2826,7 +2933,7 @@ void ofono_stk_proactive_command_handled_notify(struct ofono_stk *stk,
 	 * responses here
 	 */
 	if (length > 0 && pdu[0] == 0x81) {
-		stk_alpha_id_unset(stk);
+		proactive_command_handled_end(stk);
 		return;
 	}
 
@@ -2863,8 +2970,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 2/2] stk: Handle set up call in handled_notify

[Denis Kenzior]

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

Hi Jeevaka,

On 07/19/2011 02:13 AM, Jeevaka Badrappan wrote:
> ---
>  src/stk.c |  113 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 files changed, 110 insertions(+), 3 deletions(-)
> 
> diff --git a/src/stk.c b/src/stk.c
> index e8ffb42..a204e45 100644
> --- a/src/stk.c
> +++ b/src/stk.c
> @@ -1790,6 +1790,45 @@ 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);
> +
> +	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)
> @@ -2606,6 +2645,74 @@ static gboolean handle_command_launch_browser(const struct stk_command *cmd,
>  	return FALSE;
>  }
>  
> +static void proactive_command_handled_end(struct ofono_stk *stk)
> +{
> +	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;
> +	}
> +	case STK_COMMAND_TYPE_SEND_SMS:
> +	case STK_COMMAND_TYPE_SEND_USSD:
> +	case STK_COMMAND_TYPE_SEND_SS:
> +	case STK_COMMAND_TYPE_SEND_DTMF:
> +		stk_alpha_id_unset(stk);
> +		break;
> +
> +	default:
> +		break;
> +	}
> +}
> +
> +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 = proactive_command_handled_end;
> +
> +	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)
> @@ -2826,7 +2933,7 @@ void ofono_stk_proactive_command_handled_notify(struct ofono_stk *stk,
>  	 * responses here
>  	 */
>  	if (length > 0 && pdu[0] == 0x81) {
> -		stk_alpha_id_unset(stk);
> +		proactive_command_handled_end(stk);

You have to be careful here, you're not setting stk->pending_cmd for
anything except SETUP_CALL.

>  		return;
>  	}
>  
> @@ -2863,8 +2970,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