[PATCH 01/13] voicecall: __ofono_voicecall_send_tone internal api.

[PATCH 01/13] voicecall: __ofono_voicecall_send_tone internal api.

[Andrzej Zaborowski]

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

This provides a way for other atoms to send DTMF tones during a call.
---
 src/ofono.h     |    4 ++++
 src/voicecall.c |   16 ++++++++++++++++
 2 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/src/ofono.h b/src/ofono.h
index 6c7f649..6efd9ac 100644
--- a/src/ofono.h
+++ b/src/ofono.h
@@ -218,6 +218,10 @@ 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);
 
+int __ofono_voicecall_send_tone(struct ofono_voicecall *vc,
+				const char *tone_str,
+				ofono_voicecall_cb_t cb, void *user_data);
+
 #include <ofono/sms.h>
 
 struct sms;
diff --git a/src/voicecall.c b/src/voicecall.c
index 7b5fe3b..45e19ce 100644
--- a/src/voicecall.c
+++ b/src/voicecall.c
@@ -2326,3 +2326,19 @@ void __ofono_voicecall_dial_cancel(struct ofono_voicecall *vc)
 
 	vc->dial_req->cb = NULL;
 }
+
+int __ofono_voicecall_send_tone(struct ofono_voicecall *vc,
+				const char *tone_str,
+				ofono_voicecall_cb_t cb, void *user_data)
+{
+	if (!vc->driver->send_tones)
+		return -ENOSYS;
+
+	/* Send DTMFs only if we have at least one connected call */
+	if (!voicecalls_can_dtmf(vc))
+		return -ENOENT;
+
+	vc->driver->send_tones(vc, tone_str, cb, user_data);
+
+	return 0;
+}
-- 
1.7.1.86.g0e460.dirty

[PATCH 02/13] stk: Handle the Send DTMF proactive command.

[Andrzej Zaborowski]

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

In this version 'p' is used as the pause character in DTMF strings as
it seems in more widespread use than comma.  ISImodem already
understands 'p'.  atmodem support comes in the next patch.
---
 src/stk.c |  177 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 168 insertions(+), 9 deletions(-)

diff --git a/src/stk.c b/src/stk.c
index 80b4d23..3792d2f 100644
--- a/src/stk.c
+++ b/src/stk.c
@@ -70,7 +70,7 @@ struct ofono_stk {
 	gboolean respond_on_exit;
 	ofono_bool_t immediate_response;
 	guint remove_agent_source;
-	struct sms_submit_req *sms_submit_req;
+	struct extern_req *extern_req;
 	char *idle_mode_text;
 	struct timeval get_inkey_start_ts;
 };
@@ -83,7 +83,7 @@ struct envelope_op {
 			const unsigned char *data, int length);
 };
 
-struct sms_submit_req {
+struct extern_req {
 	struct ofono_stk *stk;
 	gboolean cancelled;
 };
@@ -435,8 +435,12 @@ static void default_agent_notify(gpointer user_data)
 {
 	struct ofono_stk *stk = user_data;
 
-	if (stk->current_agent == stk->default_agent && stk->respond_on_exit)
+	if (stk->current_agent == stk->default_agent && stk->respond_on_exit) {
+		if (stk->pending_cmd)
+			stk->cancel_cmd(stk);
+
 		send_simple_response(stk, STK_RESULT_TYPE_USER_TERMINATED);
+	}
 
 	stk->default_agent = NULL;
 	stk->current_agent = stk->session_agent;
@@ -450,6 +454,9 @@ static void session_agent_notify(gpointer user_data)
 	DBG("Session Agent removed");
 
 	if (stk->current_agent == stk->session_agent && stk->respond_on_exit) {
+		if (stk->pending_cmd)
+			stk->cancel_cmd(stk);
+
 		DBG("Sending Terminate response for session agent");
 		send_simple_response(stk, STK_RESULT_TYPE_USER_TERMINATED);
 	}
@@ -660,7 +667,7 @@ static gboolean handle_command_more_time(const struct stk_command *cmd,
 
 static void send_sms_cancel(struct ofono_stk *stk)
 {
-	stk->sms_submit_req->cancelled = TRUE;
+	stk->extern_req->cancelled = TRUE;
 
 	if (!stk->pending_cmd->send_sms.alpha_id ||
 			!stk->pending_cmd->send_sms.alpha_id[0])
@@ -671,7 +678,7 @@ static void send_sms_cancel(struct ofono_stk *stk)
 
 static void send_sms_submit_cb(gboolean ok, void *data)
 {
-	struct sms_submit_req *req = data;
+	struct extern_req *req = data;
 	struct ofono_stk *stk = req->stk;
 	struct ofono_error failure = { .type = OFONO_ERROR_TYPE_FAILURE };
 	struct stk_response rsp;
@@ -697,6 +704,12 @@ static void send_sms_submit_cb(gboolean ok, void *data)
 		stk_command_cb(&failure, stk);
 }
 
+static void extern_req_start(struct ofono_stk *stk)
+{
+	stk->extern_req = g_new0(struct extern_req, 1);
+	stk->extern_req->stk = stk;
+}
+
 static gboolean handle_command_send_sms(const struct stk_command *cmd,
 					struct stk_response *rsp,
 					struct ofono_stk *stk)
@@ -715,15 +728,14 @@ static gboolean handle_command_send_sms(const struct stk_command *cmd,
 
 	sms = __ofono_atom_get_data(sms_atom);
 
-	stk->sms_submit_req = g_new0(struct sms_submit_req, 1);
-	stk->sms_submit_req->stk = stk;
+	extern_req_start(stk);
 
 	msg_list.data = (void *) &cmd->send_sms.gsm_sms;
 	msg_list.next = NULL;
 
 	if (__ofono_sms_txq_submit(sms, &msg_list, 0, NULL, send_sms_submit_cb,
-				stk->sms_submit_req, g_free) < 0) {
-		g_free(stk->sms_submit_req);
+				stk->extern_req, g_free) < 0) {
+		g_free(stk->extern_req);
 		rsp->result.type = STK_RESULT_TYPE_TERMINAL_BUSY;
 		return TRUE;
 	}
@@ -1824,6 +1836,148 @@ static gboolean handle_command_refresh(const struct stk_command *cmd,
 	return TRUE;
 }
 
+static void send_dtmf_cancel(struct ofono_stk *stk)
+{
+	stk->respond_on_exit = FALSE;
+	stk->extern_req->cancelled = TRUE;
+
+	if (stk->pending_cmd->send_dtmf.alpha_id &&
+			stk->pending_cmd->send_dtmf.alpha_id[0])
+		stk_alpha_id_unset(stk);
+}
+
+static void dtmf_sent_cb(const struct ofono_error *error, void *user_data)
+{
+	struct extern_req *req = user_data;
+	struct ofono_stk *stk = req->stk;
+	gboolean cancelled = req->cancelled;
+
+	g_free(req);
+
+	if (cancelled) {
+		DBG("Received a DTMF Sent callback after the "
+				"proactive command was cancelled");
+		return;
+	}
+
+	stk->respond_on_exit = FALSE;
+
+	if (stk->pending_cmd->send_dtmf.alpha_id &&
+			stk->pending_cmd->send_dtmf.alpha_id[0])
+		stk_alpha_id_unset(stk);
+
+	if (error->type == OFONO_ERROR_TYPE_FAILURE &&
+			error->error == ENOENT) {
+		struct stk_response rsp;
+		static unsigned char not_in_speech_call_result[] = { 0x07 };
+		static struct ofono_error failure =
+			{ .type = OFONO_ERROR_TYPE_FAILURE };
+
+		memset(&rsp, 0, sizeof(rsp));
+
+		rsp.result.type = STK_RESULT_TYPE_TERMINAL_BUSY;
+		rsp.result.additional_len = sizeof(not_in_speech_call_result);
+		rsp.result.additional = not_in_speech_call_result;
+
+		if (stk_respond(stk, &rsp, stk_command_cb))
+			stk_command_cb(&failure, stk);
+
+		return;
+	}
+
+	if (error->type == OFONO_ERROR_TYPE_FAILURE &&
+			error->error == ENOENT) {
+		send_simple_response(stk, STK_RESULT_TYPE_DATA_NOT_UNDERSTOOD);
+		return;
+	}
+
+	if (error->type == OFONO_ERROR_TYPE_NO_ERROR)
+		send_simple_response(stk, STK_RESULT_TYPE_SUCCESS);
+	else
+		send_simple_response(stk, STK_RESULT_TYPE_NOT_CAPABLE);
+}
+
+static gboolean handle_command_send_dtmf(const struct stk_command *cmd,
+						struct stk_response *rsp,
+						struct ofono_stk *stk)
+{
+	static unsigned char not_in_speech_call_result[] = { 0x07 };
+	struct ofono_voicecall *vc = NULL;
+	struct ofono_atom *vc_atom;
+	char dtmf[256], *digit;
+	char *dtmf_from = "01234567890abcABC";
+	char *dtmf_to = "01234567890*#p*#p";
+	int err, pos;
+
+	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) {
+		rsp->result.type = STK_RESULT_TYPE_NOT_CAPABLE;
+		return TRUE;
+	}
+
+	/* Convert the DTMF string to phone number format */
+	for (pos = 0; cmd->send_dtmf.dtmf[pos] != 0; pos++) {
+		digit = strchr(dtmf_from, cmd->send_dtmf.dtmf[pos]);
+		if (!digit) {
+			rsp->result.type = STK_RESULT_TYPE_DATA_NOT_UNDERSTOOD;
+			return TRUE;
+		}
+
+		dtmf[pos] = dtmf_to[digit - dtmf_from];
+	}
+	dtmf[pos] = 0;
+
+	extern_req_start(stk);
+
+	err = __ofono_voicecall_send_tone(vc, dtmf,
+						dtmf_sent_cb, stk->extern_req);
+	if (err < 0)
+		g_free(stk->extern_req);
+
+	if (err == -EBUSY) {
+		rsp->result.type = STK_RESULT_TYPE_TERMINAL_BUSY;
+		return TRUE;
+	}
+
+	if (err == -ENOSYS) {
+		rsp->result.type = STK_RESULT_TYPE_NOT_CAPABLE;
+		return TRUE;
+	}
+
+	if (err == -ENOENT) {
+		rsp->result.type = STK_RESULT_TYPE_TERMINAL_BUSY;
+		rsp->result.additional_len = sizeof(not_in_speech_call_result);
+		rsp->result.additional = not_in_speech_call_result;
+		return TRUE;
+	}
+
+	if (err < 0) {
+		/*
+		 * We most likely got an out of memory error, tell SIM
+		 * to retry
+		 */
+		rsp->result.type = STK_RESULT_TYPE_TERMINAL_BUSY;
+		return TRUE;
+	}
+
+	if (cmd->send_dtmf.alpha_id && cmd->send_dtmf.alpha_id[0])
+		stk_alpha_id_set(stk, cmd->send_dtmf.alpha_id);
+
+	/*
+	 * Note that we don't strictly require an agent to be connected,
+	 * but to comply with 6.4.24 we need to send a End Session when
+	 * the user decides so.
+	 */
+	stk->respond_on_exit = TRUE;
+	stk->cancel_cmd = send_dtmf_cancel;
+
+	return FALSE;
+}
+
 static void stk_proactive_command_cancel(struct ofono_stk *stk)
 {
 	if (stk->immediate_response)
@@ -1996,6 +2150,11 @@ void ofono_stk_proactive_command_notify(struct ofono_stk *stk,
 							&rsp, stk);
 		break;
 
+	case STK_COMMAND_TYPE_SEND_DTMF:
+		respond = handle_command_send_dtmf(stk->pending_cmd,
+							&rsp, stk);
+		break;
+
 	default:
 		rsp.result.type = STK_RESULT_TYPE_COMMAND_NOT_UNDERSTOOD;
 		break;
-- 
1.7.1.86.g0e460.dirty

[PATCH 03/13] atmodem: Handle pauses in DTMF string

[Andrzej Zaborowski]

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

---
 drivers/atmodem/voicecall.c |  163 +++++++++++++++++++++++++++++++++++--------
 1 files changed, 135 insertions(+), 28 deletions(-)

diff --git a/drivers/atmodem/voicecall.c b/drivers/atmodem/voicecall.c
index b3c658f..54cbe4f 100644
--- a/drivers/atmodem/voicecall.c
+++ b/drivers/atmodem/voicecall.c
@@ -47,6 +47,9 @@
  /* Amount of time we give for CLIP to arrive before we commence CLCC poll */
 #define CLIP_INTERVAL 200
 
+ /* When +VTD returns 0, an unspecified manufacturer specific delay is used */
+#define TONE_DURATION 1000
+
 static const char *clcc_prefix[] = { "+CLCC:", NULL };
 static const char *none_prefix[] = { NULL };
 
@@ -59,6 +62,8 @@ struct voicecall_data {
 	unsigned int clcc_source;
 	GAtChat *chat;
 	unsigned int vendor;
+	struct tone_req *tr;
+	unsigned int tone_duration;
 };
 
 struct release_id_req {
@@ -75,7 +80,18 @@ struct change_state_req {
 	int affected_types;
 };
 
+struct tone_req {
+	struct ofono_voicecall *vc;
+	ofono_voicecall_cb_t cb;
+	void *data;
+	char *tone_str;
+	char *left;
+	int delay;
+	guint source;
+};
+
 static gboolean poll_clcc(gpointer user_data);
+static gboolean tone_request_run(gpointer user_data);
 
 static int class_to_call_type(int cls)
 {
@@ -522,51 +538,116 @@ static void at_deflect(struct ofono_voicecall *vc,
 	at_template(buf, vc, generic_cb, incoming_or_waiting, cb, data);
 }
 
-static void vts_cb(gboolean ok, GAtResult *result, gpointer user_data)
+static void tone_request_finish(struct tone_req *tr, GAtResult *result, int err)
 {
-	struct cb_data *cbd = user_data;
-	ofono_voicecall_cb_t cb = cbd->cb;
 	struct ofono_error error;
+	struct voicecall_data *vd = ofono_voicecall_get_data(tr->vc);
 
-	decode_at_error(&error, g_at_result_final_response(result));
-	cb(&error, cbd->data);
+	if (tr->source)
+		g_source_remove(tr->source);
+
+	if (result)
+		decode_at_error(&error, g_at_result_final_response(result));
+	else {
+		error.type = err ? OFONO_ERROR_TYPE_FAILURE :
+			OFONO_ERROR_TYPE_NO_ERROR;
+		error.error = err;
+	}
+
+	tr->cb(&error, tr->data);
+
+	g_free(vd->tr);
+	vd->tr = NULL;
 }
 
-static void at_send_dtmf(struct ofono_voicecall *vc, const char *dtmf,
-			ofono_voicecall_cb_t cb, void *data)
+static void vts_cb(gboolean ok, GAtResult *result, gpointer user_data)
 {
-	struct voicecall_data *vd = ofono_voicecall_get_data(vc);
-	struct cb_data *cbd = cb_data_new(cb, data);
-	int len = strlen(dtmf);
+	struct tone_req *tr = user_data;
+
+	if (!ok) {
+		tone_request_finish(tr, result, 0);
+		return;
+	}
+
+	tr->source = g_timeout_add(tr->delay, tone_request_run, tr);
+}
+
+static gboolean tone_request_run(gpointer user_data)
+{
+	struct tone_req *tr = user_data;
+	struct voicecall_data *vd = ofono_voicecall_get_data(tr->vc);
+	static const char *dtmf = "0123456789#*ABCDabcd";
+	char buf[2048], *i;
 	int s;
-	int i;
-	char *buf;
 
-	if (!cbd)
-		goto error;
+	tr->source = 0;
 
-	/* strlen("+VTS=T;") = 7 + initial AT + null */
-	buf = g_try_new(char, len * 9 + 3);
-	if (!buf)
-		goto error;
+	if (*tr->left == '\0') {
+		tone_request_finish(tr, NULL, 0);
+		return FALSE;
+	}
+
+	strcpy(buf, "AT");
+	i = buf + 2;
+
+	tr->delay = 0;
 
-	s = sprintf(buf, "AT+VTS=%c", dtmf[0]);
+	while (*tr->left && strchr(dtmf, *tr->left)) {
+		if (tr->delay)
+			*i++ = ';';
+		i += sprintf(i, "+VTS=%c", *tr->left++);
 
-	for (i = 1; i < len; i++)
-		s += sprintf(buf + s, ";+VTS=%c", dtmf[i]);
+		tr->delay += vd->tone_duration;
+	}
+
+	if (*tr->left && *tr->left != 'p') {
+		tone_request_finish(tr, NULL, EINVAL);
+		return FALSE;
+	}
 
-	s = g_at_chat_send(vd->chat, buf, none_prefix,
-				vts_cb, cbd, g_free);
+	/*
+	 * Wait 3 seconds per PAUSE, same as for DTMF separator characters
+	 * passed in a telephone number according to TS 22.101 A.21,
+	 * although 27.007 claims this delay can be set using S8 and
+	 * defaults to 2 seconds.
+	 */
+	while (*tr->left == 'p') {
+		tr->left++;
+		tr->delay += 3000;
+	}
 
-	g_free(buf);
+	if (i > buf + 2) {
+		s = g_at_chat_send(vd->chat, buf, none_prefix, vts_cb,
+					tr, NULL);
+
+		if (s <= 0)
+			tone_request_finish(tr, NULL, EIO);
+	} else
+		vts_cb(TRUE, NULL, tr);
+
+	return FALSE;
+}
 
-	if (s > 0)
+static void at_send_dtmf(struct ofono_voicecall *vc, const char *dtmf,
+			ofono_voicecall_cb_t cb, void *data)
+{
+	struct voicecall_data *vd = ofono_voicecall_get_data(vc);
+	struct tone_req *tr = g_try_new0(struct tone_req, 1);
+
+	if (!tr || vd->tr) {
+		CALLBACK_WITH_FAILURE(cb, data);
 		return;
+	}
 
-error:
-	g_free(cbd);
+	tr->vc = vc;
+	tr->cb = cb;
+	tr->data = data;
+	tr->tone_str = g_strdup(dtmf);
+	tr->left = tr->tone_str;
 
-	CALLBACK_WITH_FAILURE(cb, data);
+	vd->tr = tr;
+
+	tone_request_run(tr);
 }
 
 static void ring_notify(GAtResult *result, gpointer user_data)
@@ -799,6 +880,26 @@ static void busy_notify(GAtResult *result, gpointer user_data)
 			clcc_poll_cb, vc, NULL);
 }
 
+static void vtd_query_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct ofono_voicecall *vc = user_data;
+	struct voicecall_data *vd = ofono_voicecall_get_data(vc);
+	GAtResultIter iter;
+	int duration;
+
+	if (!ok)
+		return;
+
+	g_at_result_iter_init(&iter, result);
+	g_at_result_iter_next(&iter, "+VTD:");
+
+	if (!g_at_result_iter_next_number(&iter, &duration))
+		return;
+
+	if (duration)
+		vd->tone_duration = duration * 100;
+}
+
 static void at_voicecall_initialized(gboolean ok, GAtResult *result,
 					gpointer user_data)
 {
@@ -839,12 +940,15 @@ static int at_voicecall_probe(struct ofono_voicecall *vc, unsigned int vendor,
 
 	vd->chat = g_at_chat_clone(chat);
 	vd->vendor = vendor;
+	vd->tone_duration = TONE_DURATION;
 
 	ofono_voicecall_set_data(vc, vd);
 
 	g_at_chat_send(vd->chat, "AT+CRC=1", NULL, NULL, NULL, NULL);
 	g_at_chat_send(vd->chat, "AT+CLIP=1", NULL, NULL, NULL, NULL);
 	g_at_chat_send(vd->chat, "AT+COLP=1", NULL, NULL, NULL, NULL);
+	g_at_chat_send(vd->chat, "AT+VTD?", NULL,
+				vtd_query_cb, vc, NULL);
 	g_at_chat_send(vd->chat, "AT+CCWA=1", NULL,
 				at_voicecall_initialized, vc, NULL);
 
@@ -858,6 +962,9 @@ static void at_voicecall_remove(struct ofono_voicecall *vc)
 	if (vd->clcc_source)
 		g_source_remove(vd->clcc_source);
 
+	if (vd->tr)
+		tone_request_finish(vd->tr, NULL, ESHUTDOWN);
+
 	g_slist_foreach(vd->calls, (GFunc) g_free, NULL);
 	g_slist_free(vd->calls);
 
-- 
1.7.1.86.g0e460.dirty

[PATCH 04/13] doc: Update property name to match code.

[Andrzej Zaborowski]

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

---
 doc/stk-api.txt |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/stk-api.txt b/doc/stk-api.txt
index f5b9ebc..e4993c1 100644
--- a/doc/stk-api.txt
+++ b/doc/stk-api.txt
@@ -55,10 +55,10 @@ Signals		PropertyChanged(string property, variant value)
 			Signal is emitted whenever a property has changed.
 			The new value is passed as the signal argument.
 
-Properties	string IdleText
+Properties	string IdleModeText
 
 			Contains the text to be used when the home screen is
-			idle.  This text is set by the SIM and can be changed
+			idle.  This text is set by the SIM and can change
 			at any time.
 
 		array{struct{string, byte}} MainMenu
-- 
1.7.1.86.g0e460.dirty

[PATCH 05/13] doc: Add STK properties relevant for icons.

[Andrzej Zaborowski]

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

---
 doc/stk-api.txt |   12 +++++++++++-
 1 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/doc/stk-api.txt b/doc/stk-api.txt
index e4993c1..c29872a 100644
--- a/doc/stk-api.txt
+++ b/doc/stk-api.txt
@@ -61,17 +61,27 @@ Properties	string IdleModeText
 			idle.  This text is set by the SIM and can change
 			at any time.
 
+		byte IdleModeIcon
+
+			Contains the identifier of the icon accompanying
+			the idle mode text.
+
 		array{struct{string, byte}} MainMenu
 
 			Contains the items that make up the main menu.  This
 			is populated by the SIM when it sends the Setup Menu
 			Proactive Command.  The main menu is always available,
-			but its contents can be changed at any time.
+			but its contents can be changed at any time.  Each
+			item contains the item label and icon identifier.
 
 		string MainMenuTitle
 
 			Contains the title of the main menu.
 
+		string MainMenuIcon
+
+			Contains the identifier of the icon for the main menu.
+
 		array{byte} Icons
 
 			Contains the identifiers of all available icons for
-- 
1.7.1.86.g0e460.dirty

[PATCH 06/13] stk: Pass icon IDs in stk agent request parameters.

[Andrzej Zaborowski]

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

---
 src/stk.c      |   28 ++++++++++++++--------------
 src/stkagent.c |   39 +++++++++++++++++++++++----------------
 src/stkagent.h |   32 ++++++++++++++++++--------------
 3 files changed, 55 insertions(+), 44 deletions(-)

diff --git a/src/stk.c b/src/stk.c
index 3792d2f..c5478b8 100644
--- a/src/stk.c
+++ b/src/stk.c
@@ -1151,8 +1151,8 @@ static gboolean handle_command_display_text(const struct stk_command *cmd,
 	}
 
 	/* We most likely got an out of memory error, tell SIM to retry */
-	if (stk_agent_display_text(stk->current_agent, dt->text, 0, priority,
-					display_text_cb, stk,
+	if (stk_agent_display_text(stk->current_agent, dt->text, &dt->icon_id,
+					priority, display_text_cb, stk,
 					display_text_destroy, timeout) < 0) {
 		rsp->result.type = STK_RESULT_TYPE_TERMINAL_BUSY;
 		return TRUE;
@@ -1295,7 +1295,6 @@ static gboolean handle_command_get_inkey(const struct stk_command *cmd,
 	 * Note: immediate response and help parameter values are not
 	 * provided by current api.
 	 */
-	uint8_t icon_id = 0;
 	int err;
 
 	if (gi->duration.interval) {
@@ -1314,16 +1313,17 @@ static gboolean handle_command_get_inkey(const struct stk_command *cmd,
 
 	if (yesno)
 		err = stk_agent_request_confirmation(stk->current_agent,
-							gi->text, icon_id,
+							gi->text, &gi->icon_id,
 							request_confirmation_cb,
 							stk, NULL, timeout);
 	else if (alphabet)
 		err = stk_agent_request_key(stk->current_agent, gi->text,
-						icon_id, ucs2, request_key_cb,
-						stk, NULL, timeout);
+						&gi->icon_id, ucs2,
+						request_key_cb, stk, NULL,
+						timeout);
 	else
 		err = stk_agent_request_digit(stk->current_agent, gi->text,
-						icon_id, request_key_cb,
+						&gi->icon_id, request_key_cb,
 						stk, NULL, timeout);
 
 	if (err < 0) {
@@ -1389,19 +1389,18 @@ static gboolean handle_command_get_input(const struct stk_command *cmd,
 	gboolean alphabet = (qualifier & (1 << 0)) != 0;
 	gboolean ucs2 = (qualifier & (1 << 1)) != 0;
 	gboolean hidden = (qualifier & (1 << 2)) != 0;
-	uint8_t icon_id = 0;
 	int err;
 
 	if (alphabet)
 		err = stk_agent_request_input(stk->current_agent, gi->text,
-						icon_id, gi->default_text, ucs2,
-						gi->resp_len.min,
+						&gi->icon_id, gi->default_text,
+						ucs2, gi->resp_len.min,
 						gi->resp_len.max, hidden,
 						request_string_cb,
 						stk, NULL, timeout);
 	else
 		err = stk_agent_request_digits(stk->current_agent, gi->text,
-						icon_id, gi->default_text,
+						&gi->icon_id, gi->default_text,
 						gi->resp_len.min,
 						gi->resp_len.max, hidden,
 						request_string_cb,
@@ -1508,7 +1507,8 @@ static void confirm_call_cb(enum stk_agent_result result, gboolean confirm,
 	}
 
 	err = __ofono_voicecall_dial(vc, sc->addr.number, sc->addr.ton_npi,
-					sc->alpha_id_call_setup, 0,
+					sc->alpha_id_call_setup,
+					sc->icon_id_call_setup.id,
 					qualifier >> 1, call_setup_connected,
 					stk);
 	if (err >= 0) {
@@ -1589,8 +1589,8 @@ static gboolean handle_command_set_up_call(const struct stk_command *cmd,
 	}
 
 	err = stk_agent_confirm_call(stk->current_agent, sc->alpha_id_usr_cfm,
-					0, confirm_call_cb, stk, NULL,
-					stk->timeout * 1000);
+					&sc->icon_id_usr_cfm, confirm_call_cb,
+					stk, NULL, stk->timeout * 1000);
 
 	if (err < 0) {
 		/*
diff --git a/src/stkagent.c b/src/stkagent.c
index f62c2bd..82518f1 100644
--- a/src/stkagent.c
+++ b/src/stkagent.c
@@ -34,6 +34,8 @@
 #include "ofono.h"
 
 #include "common.h"
+#include "smsutil.h"
+#include "stkutil.h"
 #include "stkagent.h"
 
 enum allowed_error {
@@ -396,7 +398,8 @@ static void display_text_cb(DBusPendingCall *call, void *data)
 }
 
 int stk_agent_display_text(struct stk_agent *agent, const char *text,
-				uint8_t icon_id, ofono_bool_t urgent,
+				const struct stk_icon_id *icon,
+				ofono_bool_t urgent,
 				stk_agent_display_text_cb cb,
 				void *user_data, ofono_destroy_func destroy,
 				int timeout)
@@ -412,7 +415,7 @@ int stk_agent_display_text(struct stk_agent *agent, const char *text,
 
 	dbus_message_append_args(agent->msg,
 					DBUS_TYPE_STRING, &text,
-					DBUS_TYPE_BYTE, &icon_id,
+					DBUS_TYPE_BYTE, &icon->id,
 					DBUS_TYPE_BOOLEAN, &priority,
 					DBUS_TYPE_INVALID);
 
@@ -465,8 +468,8 @@ static void get_confirmation_cb(DBusPendingCall *call, void *data)
 	CALLBACK_END();
 }
 
-int stk_agent_request_confirmation(struct stk_agent *agent,
-					const char *text, uint8_t icon_id,
+int stk_agent_request_confirmation(struct stk_agent *agent, const char *text,
+					const struct stk_icon_id *icon,
 					stk_agent_confirmation_cb cb,
 					void *user_data,
 					ofono_destroy_func destroy,
@@ -482,7 +485,7 @@ int stk_agent_request_confirmation(struct stk_agent *agent,
 
 	dbus_message_append_args(agent->msg,
 					DBUS_TYPE_STRING, &text,
-					DBUS_TYPE_BYTE, &icon_id,
+					DBUS_TYPE_BYTE, &icon->id,
 					DBUS_TYPE_INVALID);
 
 	if (dbus_connection_send_with_reply(conn, agent->msg, &agent->call,
@@ -536,8 +539,8 @@ static void get_digit_cb(DBusPendingCall *call, void *data)
 	CALLBACK_END();
 }
 
-int stk_agent_request_digit(struct stk_agent *agent,
-				const char *text, uint8_t icon_id,
+int stk_agent_request_digit(struct stk_agent *agent, const char *text,
+				const struct stk_icon_id *icon,
 				stk_agent_string_cb cb, void *user_data,
 				ofono_destroy_func destroy, int timeout)
 {
@@ -551,7 +554,7 @@ int stk_agent_request_digit(struct stk_agent *agent,
 
 	dbus_message_append_args(agent->msg,
 					DBUS_TYPE_STRING, &text,
-					DBUS_TYPE_BYTE, &icon_id,
+					DBUS_TYPE_BYTE, &icon->id,
 					DBUS_TYPE_INVALID);
 
 	if (dbus_connection_send_with_reply(conn, agent->msg, &agent->call,
@@ -604,7 +607,8 @@ static void get_key_cb(DBusPendingCall *call, void *data)
 }
 
 int stk_agent_request_key(struct stk_agent *agent, const char *text,
-				uint8_t icon_id, ofono_bool_t unicode_charset,
+				const struct stk_icon_id *icon,
+				ofono_bool_t unicode_charset,
 				stk_agent_string_cb cb, void *user_data,
 				ofono_destroy_func destroy, int timeout)
 {
@@ -618,7 +622,7 @@ int stk_agent_request_key(struct stk_agent *agent, const char *text,
 
 	dbus_message_append_args(agent->msg,
 					DBUS_TYPE_STRING, &text,
-					DBUS_TYPE_BYTE, &icon_id,
+					DBUS_TYPE_BYTE, &icon->id,
 					DBUS_TYPE_INVALID);
 
 	if (dbus_connection_send_with_reply(conn, agent->msg, &agent->call,
@@ -670,7 +674,8 @@ static void get_digits_cb(DBusPendingCall *call, void *data)
 }
 
 int stk_agent_request_digits(struct stk_agent *agent, const char *text,
-				uint8_t icon_id, const char *default_text,
+				const struct stk_icon_id *icon,
+				const char *default_text,
 				int min, int max, ofono_bool_t hidden,
 				stk_agent_string_cb cb, void *user_data,
 				ofono_destroy_func destroy, int timeout)
@@ -691,7 +696,7 @@ int stk_agent_request_digits(struct stk_agent *agent, const char *text,
 
 	dbus_message_append_args(agent->msg,
 					DBUS_TYPE_STRING, &text,
-					DBUS_TYPE_BYTE, &icon_id,
+					DBUS_TYPE_BYTE, &icon->id,
 					DBUS_TYPE_STRING, &default_text,
 					DBUS_TYPE_BYTE, &min_val,
 					DBUS_TYPE_BYTE, &max_val,
@@ -747,7 +752,8 @@ static void get_input_cb(DBusPendingCall *call, void *data)
 }
 
 int stk_agent_request_input(struct stk_agent *agent, const char *text,
-				uint8_t icon_id, const char *default_text,
+				const struct stk_icon_id *icon,
+				const char *default_text,
 				ofono_bool_t unicode_charset, int min, int max,
 				ofono_bool_t hidden, stk_agent_string_cb cb,
 				void *user_data, ofono_destroy_func destroy,
@@ -769,7 +775,7 @@ int stk_agent_request_input(struct stk_agent *agent, const char *text,
 
 	dbus_message_append_args(agent->msg,
 					DBUS_TYPE_STRING, &text,
-					DBUS_TYPE_BYTE, &icon_id,
+					DBUS_TYPE_BYTE, &icon->id,
 					DBUS_TYPE_STRING, &default_text,
 					DBUS_TYPE_BYTE, &min_val,
 					DBUS_TYPE_BYTE, &max_val,
@@ -824,7 +830,8 @@ static void confirm_call_cb(DBusPendingCall *call, void *data)
 }
 
 int stk_agent_confirm_call(struct stk_agent *agent, const char *text,
-				uint8_t icon_id, stk_agent_confirmation_cb cb,
+				const struct stk_icon_id *icon,
+				stk_agent_confirmation_cb cb,
 				void *user_data, ofono_destroy_func destroy,
 				int timeout)
 {
@@ -838,7 +845,7 @@ int stk_agent_confirm_call(struct stk_agent *agent, const char *text,
 
 	dbus_message_append_args(agent->msg,
 					DBUS_TYPE_STRING, &text,
-					DBUS_TYPE_BYTE, &icon_id,
+					DBUS_TYPE_BYTE, &icon->id,
 					DBUS_TYPE_INVALID);
 
 	if (dbus_connection_send_with_reply(conn, agent->msg, &agent->call,
diff --git a/src/stkagent.h b/src/stkagent.h
index c644210..ea84a49 100644
--- a/src/stkagent.h
+++ b/src/stkagent.h
@@ -77,45 +77,49 @@ int stk_agent_request_selection(struct stk_agent *agent,
 				int timeout);
 
 int stk_agent_display_text(struct stk_agent *agent, const char *text,
-				uint8_t icon_id, ofono_bool_t urgent,
+				const struct stk_icon_id *icon,
+				ofono_bool_t urgent,
 				stk_agent_display_text_cb cb,
 				void *user_data, ofono_destroy_func destroy,
 				int timeout);
 
-int stk_agent_request_confirmation(struct stk_agent *agent,
-					const char *text, uint8_t icon_id,
+int stk_agent_request_confirmation(struct stk_agent *agent, const char *text,
+					const struct stk_icon_id *icon,
 					stk_agent_confirmation_cb cb,
 					void *user_data,
 					ofono_destroy_func destroy,
 					int timeout);
 
-int stk_agent_request_digit(struct stk_agent *agent,
-				const char *text, uint8_t icon_id,
+int stk_agent_request_digit(struct stk_agent *agent, const char *text,
+				const struct stk_icon_id *icon,
 				stk_agent_string_cb cb, void *user_data,
 				ofono_destroy_func destroy, int timeout);
 
 int stk_agent_request_key(struct stk_agent *agent, const char *text,
-				uint8_t icon_id, ofono_bool_t unicode_charset,
+				const struct stk_icon_id *icon,
+				ofono_bool_t unicode_charset,
 				stk_agent_string_cb cb, void *user_data,
 				ofono_destroy_func destroy, int timeout);
 
 int stk_agent_request_digits(struct stk_agent *agent, const char *text,
-				uint8_t icon_id, const char *default_text,
-				int min, int max, ofono_bool_t hidden,
-				stk_agent_string_cb cb, void *user_data,
-				ofono_destroy_func destroy, int timeout);
+				const struct stk_icon_id *icon,
+				const char *default_text, int min, int max,
+				ofono_bool_t hidden, stk_agent_string_cb cb,
+				void *user_data, ofono_destroy_func destroy,
+				int timeout);
 
 int stk_agent_request_input(struct stk_agent *agent, const char *text,
-				uint8_t icon_id, const char *default_text,
+				const struct stk_icon_id *icon,
+				const char *default_text,
 				ofono_bool_t unicode_charset, int min, int max,
 				ofono_bool_t hidden, stk_agent_string_cb cb,
 				void *user_data, ofono_destroy_func destroy,
 				int timeout);
 
 int stk_agent_confirm_call(struct stk_agent *agent, const char *text,
-				uint8_t icon_id, stk_agent_confirmation_cb cb,
-				void *user_data, ofono_destroy_func destroy,
-				int timeout);
+				const struct stk_icon_id *icon,
+				stk_agent_confirmation_cb cb, void *user_data,
+				ofono_destroy_func destroy, int timeout);
 
 void append_menu_items_variant(DBusMessageIter *iter,
 				const struct stk_menu_item *items);
-- 
1.7.1.86.g0e460.dirty

[PATCH 07/13] stk: Add icon ID information in stk_menu.

[Andrzej Zaborowski]

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

---
 src/stk.c      |   25 ++++++++++++++++++++++---
 src/stkagent.c |    2 +-
 src/stkagent.h |    2 +-
 3 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/src/stk.c b/src/stk.c
index c5478b8..3635579 100644
--- a/src/stk.c
+++ b/src/stk.c
@@ -256,19 +256,28 @@ void __ofono_cbs_sim_download(struct ofono_stk *stk, const struct cbs *msg)
 }
 
 static struct stk_menu *stk_menu_create(const char *title,
-		const struct stk_text_attribute *title_attr, GSList *items,
+		const struct stk_text_attribute *title_attr,
+		const struct stk_icon_id *icon, GSList *items,
 		const struct stk_item_text_attribute_list *item_attrs,
+		const struct stk_item_icon_id_list *item_icon_ids,
 		int default_id, gboolean soft_key, gboolean has_help)
 {
 	struct stk_menu *ret = g_new(struct stk_menu, 1);
+	unsigned int len = g_slist_length(items);
 	GSList *l;
 	int i;
 
 	DBG("");
 
+	if (item_attrs && item_attrs->len && item_attrs->len != len * 4)
+		goto error;
+
+	if (item_icon_ids && item_icon_ids->len && item_icon_ids->len != len)
+		goto error;
+
 	ret->title = g_strdup(title ? title : "");
-	ret->icon_id = 0;
-	ret->items = g_new0(struct stk_menu_item, g_slist_length(items) + 1);
+	memcpy(&ret->icon, icon, sizeof(ret->icon));
+	ret->items = g_new0(struct stk_menu_item, len + 1);
 	ret->default_item = -1;
 	ret->soft_key = soft_key;
 	ret->has_help = has_help;
@@ -278,12 +287,18 @@ static struct stk_menu *stk_menu_create(const char *title,
 
 		ret->items[i].text = g_strdup(item->text);
 		ret->items[i].item_id = item->id;
+		if (item_icon_ids && item_icon_ids->len)
+			ret->items[i].icon_id = item_icon_ids->list[i];
 
 		if (item->id == default_id)
 			ret->default_item = i;
 	}
 
 	return ret;
+
+error:
+	g_free(ret);
+	return NULL;
 }
 
 static struct stk_menu *stk_menu_create_from_set_up_menu(
@@ -294,8 +309,10 @@ static struct stk_menu *stk_menu_create_from_set_up_menu(
 
 	return stk_menu_create(cmd->setup_menu.alpha_id,
 				&cmd->setup_menu.text_attr,
+				&cmd->setup_menu.icon_id,
 				cmd->setup_menu.items,
 				&cmd->setup_menu.item_text_attr_list,
+				&cmd->setup_menu.item_icon_id_list,
 				0, soft_key, has_help);
 }
 
@@ -307,8 +324,10 @@ static struct stk_menu *stk_menu_create_from_select_item(
 
 	return stk_menu_create(cmd->select_item.alpha_id,
 				&cmd->select_item.text_attr,
+				&cmd->select_item.icon_id,
 				cmd->select_item.items,
 				&cmd->select_item.item_text_attr_list,
+				&cmd->select_item.item_icon_id_list,
 				cmd->select_item.item_id, soft_key, has_help);
 }
 
diff --git a/src/stkagent.c b/src/stkagent.c
index 82518f1..354b87d 100644
--- a/src/stkagent.c
+++ b/src/stkagent.c
@@ -345,7 +345,7 @@ int stk_agent_request_selection(struct stk_agent *agent,
 	dbus_message_iter_init_append(agent->msg, &iter);
 
 	dbus_message_iter_append_basic(&iter, DBUS_TYPE_STRING, &menu->title);
-	dbus_message_iter_append_basic(&iter, DBUS_TYPE_BYTE, &menu->icon_id);
+	dbus_message_iter_append_basic(&iter, DBUS_TYPE_BYTE, &menu->icon.id);
 	append_menu_items(&iter, menu->items);
 	dbus_message_iter_append_basic(&iter, DBUS_TYPE_INT16, &default_item);
 
diff --git a/src/stkagent.h b/src/stkagent.h
index ea84a49..97c4eca 100644
--- a/src/stkagent.h
+++ b/src/stkagent.h
@@ -36,7 +36,7 @@ struct stk_menu_item {
 
 struct stk_menu {
 	char *title;
-	uint8_t icon_id;
+	struct stk_icon_id icon;
 	struct stk_menu_item *items;
 	int default_item;
 	gboolean soft_key;
-- 
1.7.1.86.g0e460.dirty

[PATCH 08/13] stk: IdleModeIcon and MainMenuIcon properties.

[Andrzej Zaborowski]

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

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

diff --git a/src/stk.c b/src/stk.c
index 3635579..863a6dd 100644
--- a/src/stk.c
+++ b/src/stk.c
@@ -72,6 +72,7 @@ struct ofono_stk {
 	guint remove_agent_source;
 	struct extern_req *extern_req;
 	char *idle_mode_text;
+	struct stk_icon_id idle_mode_icon;
 	struct timeval get_inkey_start_ts;
 };
 
@@ -364,6 +365,11 @@ static void emit_menu_changed(struct ofono_stk *stk)
 						"MainMenuTitle",
 						DBUS_TYPE_STRING, &menu->title);
 
+	ofono_dbus_signal_property_changed(conn, path,
+						OFONO_STK_INTERFACE,
+						"MainMenuIcon",
+						DBUS_TYPE_BYTE, &menu->icon.id);
+
 	signal = dbus_message_new_signal(path, OFONO_STK_INTERFACE,
 						"PropertyChanged");
 	if (!signal) {
@@ -390,6 +396,9 @@ static void dict_append_menu(DBusMessageIter *dict, struct stk_menu *menu)
 	ofono_dbus_dict_append(dict, "MainMenuTitle",
 				DBUS_TYPE_STRING, &menu->title);
 
+	ofono_dbus_dict_append(dict, "MainMenuIcon",
+				DBUS_TYPE_BYTE, &menu->icon.id);
+
 	dbus_message_iter_open_container(dict, DBUS_TYPE_DICT_ENTRY,
 						NULL, &entry);
 
@@ -433,6 +442,10 @@ static DBusMessage *stk_get_properties(DBusConnection *conn,
 	ofono_dbus_dict_append(&dict, "IdleModeText",
 				DBUS_TYPE_STRING, &idle_mode_text);
 
+	if (stk->idle_mode_icon.id)
+		ofono_dbus_dict_append(&dict, "IdleModeIcon", DBUS_TYPE_BYTE,
+					&stk->idle_mode_icon.id);
+
 	if (stk->main_menu)
 		dict_append_menu(&dict, stk->main_menu);
 
@@ -789,6 +802,16 @@ static gboolean handle_command_set_idle_text(const struct stk_command *cmd,
 						DBUS_TYPE_STRING,
 						&idle_mode_text);
 
+	if (stk->idle_mode_icon.id != cmd->setup_idle_mode_text.icon_id.id) {
+		memcpy(&stk->idle_mode_icon, &cmd->setup_idle_mode_text.icon_id,
+				sizeof(stk->idle_mode_icon));
+
+		ofono_dbus_signal_property_changed(conn, path,
+						OFONO_STK_INTERFACE,
+						"IdleModeIcon", DBUS_TYPE_BYTE,
+						&stk->idle_mode_icon.id);
+	}
+
 	return TRUE;
 }
 
-- 
1.7.1.86.g0e460.dirty

[PATCH 09/13] stk: Simplify and add icon to alphaId api.

[Andrzej Zaborowski]

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

The assumption is now that stk_alpha_id_set will handle NULL
or empty alphaIds or icons.
---
 src/stk.c |   36 ++++++++++--------------------------
 1 files changed, 10 insertions(+), 26 deletions(-)

diff --git a/src/stk.c b/src/stk.c
index 863a6dd..01f4e0b 100644
--- a/src/stk.c
+++ b/src/stk.c
@@ -409,7 +409,8 @@ static void dict_append_menu(DBusMessageIter *dict, struct stk_menu *menu)
 	dbus_message_iter_close_container(dict, &entry);
 }
 
-static void stk_alpha_id_set(struct ofono_stk *stk, const char *text)
+static void stk_alpha_id_set(struct ofono_stk *stk, const char *text,
+		const struct stk_icon_id *icon)
 {
 	/* TODO */
 }
@@ -701,10 +702,6 @@ static void send_sms_cancel(struct ofono_stk *stk)
 {
 	stk->extern_req->cancelled = TRUE;
 
-	if (!stk->pending_cmd->send_sms.alpha_id ||
-			!stk->pending_cmd->send_sms.alpha_id[0])
-		return;
-
 	stk_alpha_id_unset(stk);
 }
 
@@ -723,9 +720,7 @@ static void send_sms_submit_cb(gboolean ok, void *data)
 		return;
 	}
 
-	if (stk->pending_cmd->send_sms.alpha_id &&
-			stk->pending_cmd->send_sms.alpha_id[0])
-		stk_alpha_id_unset(stk);
+	stk_alpha_id_unset(stk);
 
 	memset(&rsp, 0, sizeof(rsp));
 
@@ -774,8 +769,7 @@ static gboolean handle_command_send_sms(const struct stk_command *cmd,
 
 	stk->cancel_cmd = send_sms_cancel;
 
-	if (cmd->send_sms.alpha_id && cmd->send_sms.alpha_id[0])
-		stk_alpha_id_set(stk, cmd->send_sms.alpha_id);
+	stk_alpha_id_set(stk, cmd->send_sms.alpha_id, &cmd->send_sms.icon_id);
 
 	return FALSE;
 }
@@ -1663,9 +1657,7 @@ static void send_ussd_cancel(struct ofono_stk *stk)
 	if (ussd)
 		__ofono_ussd_initiate_cancel(ussd);
 
-	if (stk->pending_cmd->send_ussd.alpha_id &&
-			stk->pending_cmd->send_ussd.alpha_id[0])
-		stk_alpha_id_unset(stk);
+	stk_alpha_id_unset(stk);
 }
 
 static void send_ussd_callback(int error, int dcs, const unsigned char *msg,
@@ -1677,9 +1669,7 @@ static void send_ussd_callback(int error, int dcs, const unsigned char *msg,
 	enum sms_charset charset;
 	unsigned char no_cause[] = { 0x00 };
 
-	if (stk->pending_cmd->send_ussd.alpha_id &&
-			stk->pending_cmd->send_ussd.alpha_id[0])
-		stk_alpha_id_unset(stk);
+	stk_alpha_id_unset(stk);
 
 	memset(&rsp, 0, sizeof(rsp));
 
@@ -1811,8 +1801,7 @@ static gboolean handle_command_send_ussd(const struct stk_command *cmd,
 		return TRUE;
 	}
 
-	if (cmd->send_ussd.alpha_id && cmd->send_ussd.alpha_id[0])
-		stk_alpha_id_set(stk, cmd->send_ussd.alpha_id);
+	stk_alpha_id_set(stk, cmd->send_ussd.alpha_id, &cmd->send_ussd.icon_id);
 
 	return FALSE;
 }
@@ -1883,9 +1872,7 @@ static void send_dtmf_cancel(struct ofono_stk *stk)
 	stk->respond_on_exit = FALSE;
 	stk->extern_req->cancelled = TRUE;
 
-	if (stk->pending_cmd->send_dtmf.alpha_id &&
-			stk->pending_cmd->send_dtmf.alpha_id[0])
-		stk_alpha_id_unset(stk);
+	stk_alpha_id_unset(stk);
 }
 
 static void dtmf_sent_cb(const struct ofono_error *error, void *user_data)
@@ -1904,9 +1891,7 @@ static void dtmf_sent_cb(const struct ofono_error *error, void *user_data)
 
 	stk->respond_on_exit = FALSE;
 
-	if (stk->pending_cmd->send_dtmf.alpha_id &&
-			stk->pending_cmd->send_dtmf.alpha_id[0])
-		stk_alpha_id_unset(stk);
+	stk_alpha_id_unset(stk);
 
 	if (error->type == OFONO_ERROR_TYPE_FAILURE &&
 			error->error == ENOENT) {
@@ -2006,8 +1991,7 @@ static gboolean handle_command_send_dtmf(const struct stk_command *cmd,
 		return TRUE;
 	}
 
-	if (cmd->send_dtmf.alpha_id && cmd->send_dtmf.alpha_id[0])
-		stk_alpha_id_set(stk, cmd->send_dtmf.alpha_id);
+	stk_alpha_id_set(stk, cmd->send_dtmf.alpha_id, &cmd->send_dtmf.icon_id);
 
 	/*
 	 * Note that we don't strictly require an agent to be connected,
-- 
1.7.1.86.g0e460.dirty

[PATCH 10/13] stk: Apply STK text attributes as html.

[Andrzej Zaborowski]

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

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

diff --git a/src/stk.c b/src/stk.c
index 01f4e0b..c06bf99 100644
--- a/src/stk.c
+++ b/src/stk.c
@@ -256,6 +256,22 @@ void __ofono_cbs_sim_download(struct ofono_stk *stk, const struct cbs *msg)
 		stk_cbs_download_cb(stk, FALSE, NULL, -1);
 }
 
+static char *dbus_apply_text_attributes(const char *text,
+					const struct stk_text_attribute *attr)
+{
+	uint16_t buf[256], *i = buf;
+	const uint8_t *j = attr->attributes;
+	const uint8_t *end = j + attr->len;
+
+	if (attr->len & 3)
+		return NULL;
+
+	while (j < end)
+		*i++ = *j++;
+
+	return stk_text_to_html(text, buf, attr->len / 4);
+}
+
 static struct stk_menu *stk_menu_create(const char *title,
 		const struct stk_text_attribute *title_attr,
 		const struct stk_icon_id *icon, GSList *items,
@@ -267,6 +283,7 @@ static struct stk_menu *stk_menu_create(const char *title,
 	unsigned int len = g_slist_length(items);
 	GSList *l;
 	int i;
+	struct stk_text_attribute attr;
 
 	DBG("");
 
@@ -276,7 +293,11 @@ static struct stk_menu *stk_menu_create(const char *title,
 	if (item_icon_ids && item_icon_ids->len && item_icon_ids->len != len)
 		goto error;
 
-	ret->title = g_strdup(title ? title : "");
+	ret->title = dbus_apply_text_attributes(title ? title : "",
+						title_attr);
+	if (!ret->title)
+		goto error;
+
 	memcpy(&ret->icon, icon, sizeof(ret->icon));
 	ret->items = g_new0(struct stk_menu_item, len + 1);
 	ret->default_item = -1;
@@ -285,9 +306,21 @@ static struct stk_menu *stk_menu_create(const char *title,
 
 	for (l = items, i = 0; l; l = l->next, i++) {
 		struct stk_item *item = l->data;
+		char *text;
 
-		ret->items[i].text = g_strdup(item->text);
 		ret->items[i].item_id = item->id;
+
+		text = NULL;
+		if (item_attrs && item_attrs->len) {
+			memcpy(attr.attributes, &item_attrs->list[i * 4], 4);
+			attr.len = 4;
+
+			text = dbus_apply_text_attributes(item->text, &attr);
+		}
+		if (!text)
+			text = strdup(item->text);
+		ret->items[i].text = text;
+
 		if (item_icon_ids && item_icon_ids->len)
 			ret->items[i].icon_id = item_icon_ids->list[i];
 
@@ -409,7 +442,8 @@ static void dict_append_menu(DBusMessageIter *dict, struct stk_menu *menu)
 	dbus_message_iter_close_container(dict, &entry);
 }
 
-static void stk_alpha_id_set(struct ofono_stk *stk, const char *text,
+static void stk_alpha_id_set(struct ofono_stk *stk,
+		const char *text, const struct stk_text_attribute *attr,
 		const struct stk_icon_id *icon)
 {
 	/* TODO */
@@ -769,7 +803,8 @@ static gboolean handle_command_send_sms(const struct stk_command *cmd,
 
 	stk->cancel_cmd = send_sms_cancel;
 
-	stk_alpha_id_set(stk, cmd->send_sms.alpha_id, &cmd->send_sms.icon_id);
+	stk_alpha_id_set(stk, cmd->send_sms.alpha_id, &cmd->send_sms.text_attr,
+				&cmd->send_sms.icon_id);
 
 	return FALSE;
 }
@@ -780,17 +815,26 @@ static gboolean handle_command_set_idle_text(const struct stk_command *cmd,
 {
 	DBusConnection *conn = ofono_dbus_get_connection();
 	const char *path = __ofono_atom_get_path(stk->atom);
-	const char *idle_mode_text;
+	char *idle_mode_text = NULL;
 
-	if (stk->idle_mode_text) {
-		g_free(stk->idle_mode_text);
-		stk->idle_mode_text = NULL;
+	if (cmd->setup_idle_mode_text.text) {
+		idle_mode_text = dbus_apply_text_attributes(
+					cmd->setup_idle_mode_text.text,
+					&cmd->setup_idle_mode_text.text_attr);
+
+		if (!idle_mode_text) {
+			rsp->result.type = STK_RESULT_TYPE_DATA_NOT_UNDERSTOOD;
+
+			return TRUE;
+		}
 	}
 
-	if (cmd->setup_idle_mode_text.text)
-		stk->idle_mode_text = g_strdup(cmd->setup_idle_mode_text.text);
+	if (stk->idle_mode_text)
+		g_free(stk->idle_mode_text);
+
+	stk->idle_mode_text = idle_mode_text;
 
-	idle_mode_text = stk->idle_mode_text ? stk->idle_mode_text : "";
+	idle_mode_text = idle_mode_text ? idle_mode_text : "";
 	ofono_dbus_signal_property_changed(conn, path, OFONO_STK_INTERFACE,
 						"IdleModeText",
 						DBUS_TYPE_STRING,
@@ -1173,6 +1217,14 @@ static gboolean handle_command_display_text(const struct stk_command *cmd,
 	struct stk_command_display_text *dt = &stk->pending_cmd->display_text;
 	uint8_t qualifier = stk->pending_cmd->qualifier;
 	ofono_bool_t priority = (qualifier & (1 << 0)) != 0;
+	char *text = dbus_apply_text_attributes(dt->text, &dt->text_attr);
+	int err;
+
+	if (!text) {
+		rsp->result.type = STK_RESULT_TYPE_DATA_NOT_UNDERSTOOD;
+
+		return TRUE;
+	}
 
 	if (dt->duration.interval) {
 		timeout = dt->duration.interval;
@@ -1186,10 +1238,13 @@ static gboolean handle_command_display_text(const struct stk_command *cmd,
 		}
 	}
 
-	/* We most likely got an out of memory error, tell SIM to retry */
-	if (stk_agent_display_text(stk->current_agent, dt->text, &dt->icon_id,
+	err = stk_agent_display_text(stk->current_agent, text, &dt->icon_id,
 					priority, display_text_cb, stk,
-					display_text_destroy, timeout) < 0) {
+					display_text_destroy, timeout);
+	g_free(text);
+
+	/* We most likely got an out of memory error, tell SIM to retry */
+	if (err < 0) {
 		rsp->result.type = STK_RESULT_TYPE_TERMINAL_BUSY;
 		return TRUE;
 	}
@@ -1323,6 +1378,7 @@ static gboolean handle_command_get_inkey(const struct stk_command *cmd,
 {
 	int timeout = stk->timeout * 1000;
 	const struct stk_command_get_inkey *gi = &cmd->get_inkey;
+	char *text = dbus_apply_text_attributes(gi->text, &gi->text_attr);
 	uint8_t qualifier = stk->pending_cmd->qualifier;
 	gboolean alphabet = (qualifier & (1 << 0)) != 0;
 	gboolean ucs2 = (qualifier & (1 << 1)) != 0;
@@ -1333,6 +1389,12 @@ static gboolean handle_command_get_inkey(const struct stk_command *cmd,
 	 */
 	int err;
 
+	if (!text) {
+		rsp->result.type = STK_RESULT_TYPE_DATA_NOT_UNDERSTOOD;
+
+		return TRUE;
+	}
+
 	if (gi->duration.interval) {
 		timeout = gi->duration.interval;
 		switch (gi->duration.unit) {
@@ -1349,18 +1411,19 @@ static gboolean handle_command_get_inkey(const struct stk_command *cmd,
 
 	if (yesno)
 		err = stk_agent_request_confirmation(stk->current_agent,
-							gi->text, &gi->icon_id,
+							text, &gi->icon_id,
 							request_confirmation_cb,
 							stk, NULL, timeout);
 	else if (alphabet)
-		err = stk_agent_request_key(stk->current_agent, gi->text,
+		err = stk_agent_request_key(stk->current_agent, text,
 						&gi->icon_id, ucs2,
 						request_key_cb, stk, NULL,
 						timeout);
 	else
-		err = stk_agent_request_digit(stk->current_agent, gi->text,
+		err = stk_agent_request_digit(stk->current_agent, text,
 						&gi->icon_id, request_key_cb,
 						stk, NULL, timeout);
+	g_free(text);
 
 	if (err < 0) {
 		/*
@@ -1421,26 +1484,34 @@ static gboolean handle_command_get_input(const struct stk_command *cmd,
 {
 	int timeout = stk->timeout * 1000;
 	const struct stk_command_get_input *gi = &cmd->get_input;
+	char *text = dbus_apply_text_attributes(gi->text, &gi->text_attr);
 	uint8_t qualifier = stk->pending_cmd->qualifier;
 	gboolean alphabet = (qualifier & (1 << 0)) != 0;
 	gboolean ucs2 = (qualifier & (1 << 1)) != 0;
 	gboolean hidden = (qualifier & (1 << 2)) != 0;
 	int err;
 
+	if (!text) {
+		rsp->result.type = STK_RESULT_TYPE_DATA_NOT_UNDERSTOOD;
+
+		return TRUE;
+	}
+
 	if (alphabet)
-		err = stk_agent_request_input(stk->current_agent, gi->text,
+		err = stk_agent_request_input(stk->current_agent, text,
 						&gi->icon_id, gi->default_text,
 						ucs2, gi->resp_len.min,
 						gi->resp_len.max, hidden,
 						request_string_cb,
 						stk, NULL, timeout);
 	else
-		err = stk_agent_request_digits(stk->current_agent, gi->text,
+		err = stk_agent_request_digits(stk->current_agent, text,
 						&gi->icon_id, gi->default_text,
 						gi->resp_len.min,
 						gi->resp_len.max, hidden,
 						request_string_cb,
 						stk, NULL, timeout);
+	g_free(text);
 
 	if (err < 0) {
 		/*
@@ -1507,6 +1578,7 @@ static void confirm_call_cb(enum stk_agent_result result, gboolean confirm,
 	uint8_t qualifier = stk->pending_cmd->qualifier;
 	static unsigned char busy_on_call_result[] = { 0x02 };
 	static unsigned char no_cause_result[] = { 0x00 };
+	char *alpha_id = NULL;
 	struct ofono_voicecall *vc = NULL;
 	struct ofono_atom *vc_atom;
 	struct stk_response rsp;
@@ -1542,11 +1614,22 @@ static void confirm_call_cb(enum stk_agent_result result, gboolean confirm,
 		return;
 	}
 
+	if (sc->alpha_id_call_setup) {
+		alpha_id = dbus_apply_text_attributes(sc->alpha_id_call_setup,
+						&sc->text_attr_call_setup);
+		if (!alpha_id) {
+			send_simple_response(stk,
+					STK_RESULT_TYPE_DATA_NOT_UNDERSTOOD);
+			return;
+		}
+	}
+
 	err = __ofono_voicecall_dial(vc, sc->addr.number, sc->addr.ton_npi,
-					sc->alpha_id_call_setup,
-					sc->icon_id_call_setup.id,
+					alpha_id, sc->icon_id_call_setup.id,
 					qualifier >> 1, call_setup_connected,
 					stk);
+	g_free(alpha_id);
+
 	if (err >= 0) {
 		stk->cancel_cmd = call_setup_cancel;
 
@@ -1589,6 +1672,7 @@ static gboolean handle_command_set_up_call(const struct stk_command *cmd,
 	const struct stk_command_setup_call *sc = &cmd->setup_call;
 	uint8_t qualifier = cmd->qualifier;
 	static unsigned char busy_on_call_result[] = { 0x02 };
+	char *alpha_id = NULL;
 	struct ofono_voicecall *vc = NULL;
 	struct ofono_atom *vc_atom;
 	int err;
@@ -1624,9 +1708,19 @@ static gboolean handle_command_set_up_call(const struct stk_command *cmd,
 		return TRUE;
 	}
 
-	err = stk_agent_confirm_call(stk->current_agent, sc->alpha_id_usr_cfm,
+	if (sc->alpha_id_usr_cfm) {
+		alpha_id = dbus_apply_text_attributes(sc->alpha_id_usr_cfm,
+							&sc->text_attr_usr_cfm);
+		if (!alpha_id) {
+			rsp->result.type = STK_RESULT_TYPE_DATA_NOT_UNDERSTOOD;
+			return TRUE;
+		}
+	}
+
+	err = stk_agent_confirm_call(stk->current_agent, alpha_id,
 					&sc->icon_id_usr_cfm, confirm_call_cb,
 					stk, NULL, stk->timeout * 1000);
+	g_free(alpha_id);
 
 	if (err < 0) {
 		/*
@@ -1801,7 +1895,9 @@ static gboolean handle_command_send_ussd(const struct stk_command *cmd,
 		return TRUE;
 	}
 
-	stk_alpha_id_set(stk, cmd->send_ussd.alpha_id, &cmd->send_ussd.icon_id);
+	stk_alpha_id_set(stk, cmd->send_ussd.alpha_id,
+				&cmd->send_ussd.text_attr,
+				&cmd->send_ussd.icon_id);
 
 	return FALSE;
 }
@@ -1991,7 +2087,9 @@ static gboolean handle_command_send_dtmf(const struct stk_command *cmd,
 		return TRUE;
 	}
 
-	stk_alpha_id_set(stk, cmd->send_dtmf.alpha_id, &cmd->send_dtmf.icon_id);
+	stk_alpha_id_set(stk, cmd->send_dtmf.alpha_id,
+				&cmd->send_dtmf.text_attr,
+				&cmd->send_dtmf.icon_id);
 
 	/*
 	 * Note that we don't strictly require an agent to be connected,
-- 
1.7.1.86.g0e460.dirty

[PATCH 11/13] stkagent: Add PlayTone and LoopTone requests.

[Andrzej Zaborowski]

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

---
 src/stkagent.c |   94 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/stkagent.h |   13 ++++++++
 2 files changed, 107 insertions(+), 0 deletions(-)

diff --git a/src/stkagent.c b/src/stkagent.c
index 354b87d..e123bd2 100644
--- a/src/stkagent.c
+++ b/src/stkagent.c
@@ -861,3 +861,97 @@ int stk_agent_confirm_call(struct stk_agent *agent, const char *text,
 
 	return 0;
 }
+
+static void play_tone_cb(DBusPendingCall *call, void *data)
+{
+	struct stk_agent *agent = data;
+	stk_agent_tone_cb cb = agent->user_cb;
+	DBusMessage *reply = dbus_pending_call_steal_reply(call);
+	enum stk_agent_result result;
+	gboolean remove_agent;
+
+	if (check_error(agent, reply,
+			ALLOWED_ERROR_TERMINATE, &result) == -EINVAL) {
+		remove_agent = TRUE;
+		goto error;
+	}
+
+	if (dbus_message_get_args(reply, NULL, DBUS_TYPE_INVALID) == FALSE) {
+		ofono_error("Can't parse the reply to PlayTone()");
+		remove_agent = TRUE;
+		goto error;
+	}
+
+	cb(result, agent->user_data);
+	goto done;
+
+	CALLBACK_END();
+}
+
+int stk_agent_play_tone(struct stk_agent *agent, const char *text,
+			const struct stk_icon_id *icon, ofono_bool_t vibrate,
+			const char *tone, stk_agent_tone_cb cb, void *user_data,
+			ofono_destroy_func destroy, int timeout)
+{
+	DBusConnection *conn = ofono_dbus_get_connection();
+
+	agent->msg = dbus_message_new_method_call(agent->bus, agent->path,
+							OFONO_SIM_APP_INTERFACE,
+							"PlayTone");
+	if (agent->msg == NULL)
+		return -ENOMEM;
+
+	dbus_message_append_args(agent->msg,
+					DBUS_TYPE_STRING, &tone,
+					DBUS_TYPE_STRING, &text,
+					DBUS_TYPE_BYTE, &icon->id,
+					DBUS_TYPE_INVALID);
+
+	if (dbus_connection_send_with_reply(conn, agent->msg, &agent->call,
+						timeout) == FALSE ||
+			agent->call == NULL)
+		return -EIO;
+
+	agent->user_cb = cb;
+	agent->user_data = user_data;
+	agent->user_destroy = destroy;
+
+	dbus_pending_call_set_notify(agent->call, play_tone_cb,
+					agent, NULL);
+
+	return 0;
+}
+
+int stk_agent_loop_tone(struct stk_agent *agent, const char *text,
+			const struct stk_icon_id *icon, ofono_bool_t vibrate,
+			const char *tone, stk_agent_tone_cb cb, void *user_data,
+			ofono_destroy_func destroy, int timeout)
+{
+	DBusConnection *conn = ofono_dbus_get_connection();
+
+	agent->msg = dbus_message_new_method_call(agent->bus, agent->path,
+							OFONO_SIM_APP_INTERFACE,
+							"LoopTone");
+	if (agent->msg == NULL)
+		return -ENOMEM;
+
+	dbus_message_append_args(agent->msg,
+					DBUS_TYPE_STRING, &tone,
+					DBUS_TYPE_STRING, &text,
+					DBUS_TYPE_BYTE, &icon->id,
+					DBUS_TYPE_INVALID);
+
+	if (dbus_connection_send_with_reply(conn, agent->msg, &agent->call,
+						timeout) == FALSE ||
+			agent->call == NULL)
+		return -EIO;
+
+	agent->user_cb = cb;
+	agent->user_data = user_data;
+	agent->user_destroy = destroy;
+
+	dbus_pending_call_set_notify(agent->call, play_tone_cb,
+					agent, NULL);
+
+	return 0;
+}
diff --git a/src/stkagent.h b/src/stkagent.h
index 97c4eca..c8e1886 100644
--- a/src/stkagent.h
+++ b/src/stkagent.h
@@ -56,6 +56,9 @@ typedef void (*stk_agent_confirmation_cb)(enum stk_agent_result result,
 typedef void (*stk_agent_string_cb)(enum stk_agent_result result,
 					char *string, void *user_data);
 
+typedef void (*stk_agent_tone_cb)(enum stk_agent_result result,
+						void *user_data);
+
 struct stk_agent *stk_agent_new(const char *path, const char *sender,
 					ofono_bool_t remove_on_terminate);
 
@@ -121,5 +124,15 @@ int stk_agent_confirm_call(struct stk_agent *agent, const char *text,
 				stk_agent_confirmation_cb cb, void *user_data,
 				ofono_destroy_func destroy, int timeout);
 
+int stk_agent_play_tone(struct stk_agent *agent, const char *text,
+			const struct stk_icon_id *icon, ofono_bool_t vibrate,
+			const char *tone, stk_agent_tone_cb cb, void *user_data,
+			ofono_destroy_func destroy, int timeout);
+
+int stk_agent_loop_tone(struct stk_agent *agent, const char *text,
+			const struct stk_icon_id *icon, ofono_bool_t vibrate,
+			const char *tone, stk_agent_tone_cb cb, void *user_data,
+			ofono_destroy_func destroy, int timeout);
+
 void append_menu_items_variant(DBusMessageIter *iter,
 				const struct stk_menu_item *items);
-- 
1.7.1.86.g0e460.dirty

[PATCH 12/13] stk: Handle the Play Tone proactive command.

[Andrzej Zaborowski]

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

A memory leak in error path has been fixed since the previous version.
---
 src/stk.c |  139 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 139 insertions(+), 0 deletions(-)

diff --git a/src/stk.c b/src/stk.c
index c06bf99..b3d2b40 100644
--- a/src/stk.c
+++ b/src/stk.c
@@ -89,6 +89,11 @@ struct extern_req {
 	gboolean cancelled;
 };
 
+struct tone_info {
+	const char *name;
+	gboolean continuous;
+};
+
 #define ENVELOPE_RETRIES_DEFAULT 5
 
 static void envelope_queue_run(struct ofono_stk *stk);
@@ -2102,6 +2107,135 @@ static gboolean handle_command_send_dtmf(const struct stk_command *cmd,
 	return FALSE;
 }
 
+static void play_tone_cb(enum stk_agent_result result, void *user_data)
+{
+	struct ofono_stk *stk = user_data;
+
+	stk->respond_on_exit = FALSE;
+
+	switch (result) {
+	case STK_AGENT_RESULT_OK:
+	case STK_AGENT_RESULT_TIMEOUT:
+		send_simple_response(stk, STK_RESULT_TYPE_SUCCESS);
+		break;
+
+	default:
+		send_simple_response(stk, STK_RESULT_TYPE_USER_TERMINATED);
+		break;
+	}
+}
+
+static gboolean handle_command_play_tone(const struct stk_command *cmd,
+						struct stk_response *rsp,
+						struct ofono_stk *stk)
+{
+	static int manufacturer_timeout = 10000; /* 10 seconds */
+	/* Continuous true/false according to 02.40 */
+	static const struct tone_info tone_infos[] = {
+		/* Default */
+		[0x00] = { "dial-tone", TRUE },
+
+		/* Standard */
+		[0x01] = { "dial-tone", TRUE },
+		[0x02] = { "busy", TRUE },
+		[0x03] = { "congestion", TRUE },
+		[0x04] = { "radio-path-acknowledge", FALSE },
+		[0x05] = { "radio-path-not-available", FALSE },
+		[0x06] = { "error", TRUE },
+		[0x07] = { "call-waiting", FALSE },
+		[0x08] = { "ringing-tone", TRUE },
+
+		/* Proprietary */
+		[0x10] = { "general-beep", FALSE },
+		[0x11] = { "positive-acknowledgement", FALSE },
+		[0x12] = { "negative-acknowledgement", FALSE },
+		[0x13] = { "user-ringing-tone", TRUE },
+		[0x14] = { "user-sms-alert", FALSE },
+		[0x15] = { "critical", FALSE },
+		[0x20] = { "vibrate", TRUE },
+
+		/* Themed */
+		[0x30] = { "happy", FALSE },
+		[0x31] = { "sad", FALSE },
+		[0x32] = { "urgent-action", FALSE },
+		[0x33] = { "question", FALSE },
+		[0x34] = { "message-received", FALSE },
+
+		/* Melody */
+		[0x40] = { "melody-1", FALSE },
+		[0x41] = { "melody-2", FALSE },
+		[0x42] = { "melody-3", FALSE },
+		[0x43] = { "melody-4", FALSE },
+		[0x44] = { "melody-5", FALSE },
+		[0x45] = { "melody-6", FALSE },
+		[0x46] = { "melody-7", FALSE },
+		[0x47] = { "melody-8", FALSE },
+	};
+
+	const struct stk_command_play_tone *pt = &cmd->play_tone;
+	uint8_t qualifier = stk->pending_cmd->qualifier;
+	gboolean vibrate = (qualifier & (1 << 0)) != 0;
+	char *text;
+	int timeout;
+	int err;
+
+	if (!tone_infos[pt->tone].name) {
+		rsp->result.type = STK_RESULT_TYPE_DATA_NOT_UNDERSTOOD;
+
+		return TRUE;
+	}
+
+	text = dbus_apply_text_attributes(pt->alpha_id ? pt->alpha_id : "",
+						&pt->text_attr);
+	if (!text) {
+		rsp->result.type = STK_RESULT_TYPE_DATA_NOT_UNDERSTOOD;
+
+		return TRUE;
+	}
+
+	if (pt->duration.interval) {
+		timeout = pt->duration.interval;
+		switch (pt->duration.unit) {
+		case STK_DURATION_TYPE_MINUTES:
+			timeout *= 60;
+		case STK_DURATION_TYPE_SECONDS:
+			timeout *= 10;
+		case STK_DURATION_TYPE_SECOND_TENTHS:
+			timeout *= 100;
+		}
+	} else
+		timeout = manufacturer_timeout;
+
+	if (!tone_infos[pt->tone].continuous)
+		/* Duration ignored */
+		err = stk_agent_play_tone(stk->current_agent, text,
+						&pt->icon_id, vibrate,
+						tone_infos[pt->tone].name,
+						play_tone_cb, stk, NULL,
+						stk->timeout * 1000);
+	else
+		err = stk_agent_loop_tone(stk->current_agent, text,
+						&pt->icon_id, vibrate,
+						tone_infos[pt->tone].name,
+						play_tone_cb, stk, NULL,
+						timeout);
+	g_free(text);
+
+	if (err < 0) {
+		/*
+		 * We most likely got an out of memory error, tell SIM
+		 * to retry
+		 */
+		rsp->result.type = STK_RESULT_TYPE_TERMINAL_BUSY;
+		return TRUE;
+	}
+
+	stk->respond_on_exit = TRUE;
+	stk->cancel_cmd = stk_request_cancel;
+
+	return FALSE;
+}
+
 static void stk_proactive_command_cancel(struct ofono_stk *stk)
 {
 	if (stk->immediate_response)
@@ -2279,6 +2413,11 @@ void ofono_stk_proactive_command_notify(struct ofono_stk *stk,
 							&rsp, stk);
 		break;
 
+	case STK_COMMAND_TYPE_PLAY_TONE:
+		respond = handle_command_play_tone(stk->pending_cmd,
+							&rsp, stk);
+		break;
+
 	default:
 		rsp.result.type = STK_RESULT_TYPE_COMMAND_NOT_UNDERSTOOD;
 		break;
-- 
1.7.1.86.g0e460.dirty

[PATCH 13/13] [RfC] API for STK driver to signal executed commands.

[Andrzej Zaborowski]

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

Some modems are able to handle some proactive commands in their
firmware or otherwise, if the command doesn't require input from user.
Nevertheles ofono may need to update internal state or notify the user
where necessary.  With this api the driver can notify core that a
command is being executed in the modem or that a command is finished
executing and the TERMINAL RESPONSE has been sent to SIM.  It would
also be possible for a driver to handle a command.
---
 include/stk.h |    7 +++++++
 src/stk.c     |   39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 0 deletions(-)

diff --git a/include/stk.h b/include/stk.h
index 638da9c..8f548d3 100644
--- a/include/stk.h
+++ b/include/stk.h
@@ -67,6 +67,13 @@ void ofono_stk_proactive_command_notify(struct ofono_stk *stk,
 
 void ofono_stk_proactive_session_end_notify(struct ofono_stk *stk);
 
+void ofono_stk_proactive_command_handled_notify(struct ofono_stk *stk,
+						int length,
+						const unsigned char *pdu);
+void ofono_stk_terminal_response_sent_notify(struct ofono_stk *stk,
+						int length,
+						const unsigned char *pdu);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/src/stk.c b/src/stk.c
index b3d2b40..69a5353 100644
--- a/src/stk.c
+++ b/src/stk.c
@@ -2431,6 +2431,45 @@ void ofono_stk_proactive_command_notify(struct ofono_stk *stk,
 		stk_command_cb(&error, stk);
 }
 
+void ofono_stk_proactive_command_handled_notify(struct ofono_stk *stk,
+						int length,
+						const unsigned char *pdu)
+{
+	struct stk_command *cmd;
+
+	stk_proactive_command_cancel(stk);
+
+	cmd = stk_command_new_from_pdu(pdu, length);
+
+	if (!cmd || cmd->status != STK_PARSE_RESULT_OK) {
+		ofono_error("Can't parse proactive command");
+
+		if (cmd)
+			stk_command_free(cmd);
+		return;
+	}
+
+	switch (cmd->type) {
+	case STK_COMMAND_TYPE_MORE_TIME:
+		break;
+
+	case STK_COMMAND_TYPE_SEND_SMS:
+		stk_alpha_id_set(stk, cmd->send_sms.alpha_id,
+					&cmd->send_sms.text_attr,
+					&cmd->send_sms.icon_id);
+		break;
+	}
+
+	stk_command_free(cmd);
+}
+
+void ofono_stk_terminal_response_sent_notify(struct ofono_stk *stk,
+						int length,
+						const unsigned char *pdu)
+{
+	stk_alpha_id_unset(stk);
+}
+
 int ofono_stk_driver_register(const struct ofono_stk_driver *d)
 {
 	DBG("driver: %p, name: %s", d, d->name);
-- 
1.7.1.86.g0e460.dirty

Re: [PATCH 04/13] doc: Update property name to match code.

[Denis Kenzior]

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

Hi Andrew,

On 10/13/2010 08:54 AM, Andrzej Zaborowski wrote:
> ---
>  doc/stk-api.txt |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 

Patch has been applied, thanks.

Regards,
-Denis

Re: [PATCH 05/13] doc: Add STK properties relevant for icons.

[Denis Kenzior]

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

Hi Andrew,

On 10/13/2010 08:54 AM, Andrzej Zaborowski wrote:
> ---
>  doc/stk-api.txt |   12 +++++++++++-
>  1 files changed, 11 insertions(+), 1 deletions(-)

Patch has been applied, thanks.

Regards,
-Denis

Re: [PATCH 06/13] stk: Pass icon IDs in stk agent request parameters.

[Denis Kenzior]

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

Hi Andrew,

On 10/13/2010 08:54 AM, Andrzej Zaborowski wrote:
> ---
>  src/stk.c      |   28 ++++++++++++++--------------
>  src/stkagent.c |   39 +++++++++++++++++++++++----------------
>  src/stkagent.h |   32 ++++++++++++++++++--------------
>  3 files changed, 55 insertions(+), 44 deletions(-)

Patch has been applied, thanks.

Regards,
-Denis

Re: [PATCH 07/13] stk: Add icon ID information in stk_menu.

[Denis Kenzior]

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

Hi Andrew,

On 10/13/2010 08:54 AM, Andrzej Zaborowski wrote:
> ---
>  src/stk.c      |   25 ++++++++++++++++++++++---
>  src/stkagent.c |    2 +-
>  src/stkagent.h |    2 +-
>  3 files changed, 24 insertions(+), 5 deletions(-)

Patch has been applied, thanks.

Regards,
-Denis

Re: [PATCH 08/13] stk: IdleModeIcon and MainMenuIcon properties.

[Denis Kenzior]

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

Hi Andrew,

On 10/13/2010 08:54 AM, Andrzej Zaborowski wrote:
> ---
>  src/stk.c |   23 +++++++++++++++++++++++
>  1 files changed, 23 insertions(+), 0 deletions(-)
> 

Patch has been applied, thanks.

Regards,
-Denis

list-modems patch

[Alexander A Khryukin]

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

Sometimes i see 

[root(a)alexander-desktop ofono-0.31]# /usr/lib/ofono/test/list-modems 
[ /huawei5 ]
    Features = sim 
    Powered = 1
    Interfaces = org.ofono.Phonebook org.ofono.AudioSettings
org.ofono.VoiceCallManager org.ofono.SimManager 
    Online = 0
    Model = E1550
    Manufacturer = huawei
    Serial = 353142033084081
    Revision = 11.608.12.00.143
    [ org.ofono.Phonebook ]
    [ org.ofono.AudioSettings ]
Traceback (most recent call last):
  File "/usr/lib/ofono/test/list-modems", line 61, in <module>
    print "        %s = %s" % (key, val).encode('ascii')


Patch



diff --git a/test/list-modems b/test/list-modems
index 557efd5..df1dca8 100755
--- a/test/list-modems
+++ b/test/list-modems
@@ -58,6 +58,10 @@ for path, properties in modems:
                                        ")" for text, icon in
properties[key] ])
                        else:
                             	val = str(properties[key])
-                       print "        %s = %s" % (key, val)
+                       try:
+                               print  "       %s = %s" % (key, val)
+                       except:
+                               continue
+

        print

Re: list-modems patch

[Marcel Holtmann]

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

Hi Alex,

> Sometimes i see 
> 
> [root(a)alexander-desktop ofono-0.31]# /usr/lib/ofono/test/list-modems 
> [ /huawei5 ]
>     Features = sim 
>     Powered = 1
>     Interfaces = org.ofono.Phonebook org.ofono.AudioSettings
> org.ofono.VoiceCallManager org.ofono.SimManager 
>     Online = 0
>     Model = E1550
>     Manufacturer = huawei
>     Serial = 353142033084081
>     Revision = 11.608.12.00.143
>     [ org.ofono.Phonebook ]
>     [ org.ofono.AudioSettings ]
> Traceback (most recent call last):
>   File "/usr/lib/ofono/test/list-modems", line 61, in <module>
>     print "        %s = %s" % (key, val).encode('ascii')
> 
> 
> Patch
> 
> 
> 
> diff --git a/test/list-modems b/test/list-modems
> index 557efd5..df1dca8 100755
> --- a/test/list-modems
> +++ b/test/list-modems
> @@ -58,6 +58,10 @@ for path, properties in modems:
>                                         ")" for text, icon in
> properties[key] ])
>                         else:
>                              	val = str(properties[key])
> -                       print "        %s = %s" % (key, val)
> +                       try:
> +                               print  "       %s = %s" % (key, val)
> +                       except:
> +                               continue
> +

can you at least print the key value. Just not printing that property at
all is bad since it is there. Just marking the value as not printable
seems to be the better approach.

Regards

Marcel

Re: [PATCH 01/13] voicecall: __ofono_voicecall_send_tone internal api.

[Denis Kenzior]

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

Hi Andrew,

On 10/13/2010 08:54 AM, Andrzej Zaborowski wrote:
> This provides a way for other atoms to send DTMF tones during a call.
> ---
>  src/ofono.h     |    4 ++++
>  src/voicecall.c |   16 ++++++++++++++++
>  2 files changed, 20 insertions(+), 0 deletions(-)
> 
> diff --git a/src/ofono.h b/src/ofono.h
> index 6c7f649..6efd9ac 100644
> --- a/src/ofono.h
> +++ b/src/ofono.h
> @@ -218,6 +218,10 @@ 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);
>  
> +int __ofono_voicecall_send_tone(struct ofono_voicecall *vc,
> +				const char *tone_str,
> +				ofono_voicecall_cb_t cb, void *user_data);
> +

So this one is structured according to __ofono_voicecall_dial, should we
also have __ofono_voicecall_send_tone_cancel?

>  #include <ofono/sms.h>
>  
>  struct sms;
> diff --git a/src/voicecall.c b/src/voicecall.c
> index 7b5fe3b..45e19ce 100644
> --- a/src/voicecall.c
> +++ b/src/voicecall.c
> @@ -2326,3 +2326,19 @@ void __ofono_voicecall_dial_cancel(struct ofono_voicecall *vc)
>  
>  	vc->dial_req->cb = NULL;
>  }
> +
> +int __ofono_voicecall_send_tone(struct ofono_voicecall *vc,
> +				const char *tone_str,
> +				ofono_voicecall_cb_t cb, void *user_data)
> +{
> +	if (!vc->driver->send_tones)
> +		return -ENOSYS;
> +
> +	/* Send DTMFs only if we have at least one connected call */
> +	if (!voicecalls_can_dtmf(vc))
> +		return -ENOENT;

I'm a bit worried that we never check for BUSY conditions here, in
particular since we can get into this situation:

Send DTMF
Cancel
Send DTMF

Which results in two DTMF operations being outstanding.

> +
> +	vc->driver->send_tones(vc, tone_str, cb, user_data);
> +
> +	return 0;
> +}

We also don't busy out any D-Bus operations when Send DTMF is in progress...

Regards,
-Denis

Re: [PATCH 02/13] stk: Handle the Send DTMF proactive command.

[Denis Kenzior]

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

Hi Andrew,

On 10/13/2010 08:54 AM, Andrzej Zaborowski wrote:
> In this version 'p' is used as the pause character in DTMF strings as
> it seems in more widespread use than comma.  ISImodem already
> understands 'p'.  atmodem support comes in the next patch.
> ---

I cannibalized this patch a little bit, see comments below:

> @@ -70,7 +70,7 @@ struct ofono_stk {
>  	gboolean respond_on_exit;
>  	ofono_bool_t immediate_response;
>  	guint remove_agent_source;
> -	struct sms_submit_req *sms_submit_req;
> +	struct extern_req *extern_req;
>  	char *idle_mode_text;
>  	struct timeval get_inkey_start_ts;
>  };

This chunk was applied

> @@ -83,7 +83,7 @@ struct envelope_op {
>  			const unsigned char *data, int length);
>  };
>  
> -struct sms_submit_req {
> +struct extern_req {
>  	struct ofono_stk *stk;
>  	gboolean cancelled;
>  };

This chunk was applied

> @@ -435,8 +435,12 @@ static void default_agent_notify(gpointer user_data)
>  {
>  	struct ofono_stk *stk = user_data;
>  
> -	if (stk->current_agent == stk->default_agent && stk->respond_on_exit)
> +	if (stk->current_agent == stk->default_agent && stk->respond_on_exit) {
> +		if (stk->pending_cmd)
> +			stk->cancel_cmd(stk);
> +
>  		send_simple_response(stk, STK_RESULT_TYPE_USER_TERMINATED);
> +	}
>  
>  	stk->default_agent = NULL;
>  	stk->current_agent = stk->session_agent;
> @@ -450,6 +454,9 @@ static void session_agent_notify(gpointer user_data)
>  	DBG("Session Agent removed");
>  
>  	if (stk->current_agent == stk->session_agent && stk->respond_on_exit) {
> +		if (stk->pending_cmd)
> +			stk->cancel_cmd(stk);
> +
>  		DBG("Sending Terminate response for session agent");
>  		send_simple_response(stk, STK_RESULT_TYPE_USER_TERMINATED);
>  	}

Can you explain a bit more what these two chunks are doing?

> @@ -660,7 +667,7 @@ static gboolean handle_command_more_time(const struct stk_command *cmd,
>  
>  static void send_sms_cancel(struct ofono_stk *stk)
>  {
> -	stk->sms_submit_req->cancelled = TRUE;
> +	stk->extern_req->cancelled = TRUE;
>  
>  	if (!stk->pending_cmd->send_sms.alpha_id ||
>  			!stk->pending_cmd->send_sms.alpha_id[0])

This chunk has been applied

> @@ -671,7 +678,7 @@ static void send_sms_cancel(struct ofono_stk *stk)
>  
>  static void send_sms_submit_cb(gboolean ok, void *data)
>  {
> -	struct sms_submit_req *req = data;
> +	struct extern_req *req = data;
>  	struct ofono_stk *stk = req->stk;
>  	struct ofono_error failure = { .type = OFONO_ERROR_TYPE_FAILURE };
>  	struct stk_response rsp;

This chunk has been applied

> @@ -697,6 +704,12 @@ static void send_sms_submit_cb(gboolean ok, void *data)
>  		stk_command_cb(&failure, stk);
>  }
>  
> +static void extern_req_start(struct ofono_stk *stk)
> +{
> +	stk->extern_req = g_new0(struct extern_req, 1);
> +	stk->extern_req->stk = stk;
> +}
> +
>  static gboolean handle_command_send_sms(const struct stk_command *cmd,
>  					struct stk_response *rsp,
>  					struct ofono_stk *stk)

This chunk has been applied

> @@ -715,15 +728,14 @@ static gboolean handle_command_send_sms(const struct stk_command *cmd,
>  
>  	sms = __ofono_atom_get_data(sms_atom);
>  
> -	stk->sms_submit_req = g_new0(struct sms_submit_req, 1);
> -	stk->sms_submit_req->stk = stk;
> +	extern_req_start(stk);
>  
>  	msg_list.data = (void *) &cmd->send_sms.gsm_sms;
>  	msg_list.next = NULL;
>  
>  	if (__ofono_sms_txq_submit(sms, &msg_list, 0, NULL, send_sms_submit_cb,
> -				stk->sms_submit_req, g_free) < 0) {
> -		g_free(stk->sms_submit_req);
> +				stk->extern_req, g_free) < 0) {
> +		g_free(stk->extern_req);
>  		rsp->result.type = STK_RESULT_TYPE_TERMINAL_BUSY;
>  		return TRUE;
>  	}

This chunk has been applied

> @@ -1824,6 +1836,148 @@ static gboolean handle_command_refresh(const struct stk_command *cmd,
>  	return TRUE;
>  }
>  
> +static void send_dtmf_cancel(struct ofono_stk *stk)
> +{
> +	stk->respond_on_exit = FALSE;
> +	stk->extern_req->cancelled = TRUE;
> +
> +	if (stk->pending_cmd->send_dtmf.alpha_id &&
> +			stk->pending_cmd->send_dtmf.alpha_id[0])
> +		stk_alpha_id_unset(stk);
> +}
> +
> +static void dtmf_sent_cb(const struct ofono_error *error, void *user_data)
> +{
> +	struct extern_req *req = user_data;
> +	struct ofono_stk *stk = req->stk;
> +	gboolean cancelled = req->cancelled;
> +
> +	g_free(req);
> +
> +	if (cancelled) {
> +		DBG("Received a DTMF Sent callback after the "
> +				"proactive command was cancelled");
> +		return;
> +	}
> +
> +	stk->respond_on_exit = FALSE;
> +
> +	if (stk->pending_cmd->send_dtmf.alpha_id &&
> +			stk->pending_cmd->send_dtmf.alpha_id[0])
> +		stk_alpha_id_unset(stk);
> +
> +	if (error->type == OFONO_ERROR_TYPE_FAILURE &&
> +			error->error == ENOENT) {

I don't see how this if can be triggered from reviewing the previous
patch.  Should the error be -ENOENT?

> +		struct stk_response rsp;
> +		static unsigned char not_in_speech_call_result[] = { 0x07 };
> +		static struct ofono_error failure =
> +			{ .type = OFONO_ERROR_TYPE_FAILURE };
> +
> +		memset(&rsp, 0, sizeof(rsp));
> +
> +		rsp.result.type = STK_RESULT_TYPE_TERMINAL_BUSY;
> +		rsp.result.additional_len = sizeof(not_in_speech_call_result);
> +		rsp.result.additional = not_in_speech_call_result;
> +
> +		if (stk_respond(stk, &rsp, stk_command_cb))
> +			stk_command_cb(&failure, stk);
> +
> +		return;
> +	}
> +
> +	if (error->type == OFONO_ERROR_TYPE_FAILURE &&
> +			error->error == ENOENT) {

And now we're repeating the if condition above? Have you tested this part?

> +		send_simple_response(stk, STK_RESULT_TYPE_DATA_NOT_UNDERSTOOD);
> +		return;
> +	}
> +
> +	if (error->type == OFONO_ERROR_TYPE_NO_ERROR)
> +		send_simple_response(stk, STK_RESULT_TYPE_SUCCESS);
> +	else
> +		send_simple_response(stk, STK_RESULT_TYPE_NOT_CAPABLE);
> +}
> +
> +static gboolean handle_command_send_dtmf(const struct stk_command *cmd,
> +						struct stk_response *rsp,
> +						struct ofono_stk *stk)
> +{
> +	static unsigned char not_in_speech_call_result[] = { 0x07 };
> +	struct ofono_voicecall *vc = NULL;
> +	struct ofono_atom *vc_atom;
> +	char dtmf[256], *digit;
> +	char *dtmf_from = "01234567890abcABC";
> +	char *dtmf_to = "01234567890*#p*#p";
> +	int err, pos;
> +
> +	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) {
> +		rsp->result.type = STK_RESULT_TYPE_NOT_CAPABLE;
> +		return TRUE;
> +	}
> +
> +	/* Convert the DTMF string to phone number format */
> +	for (pos = 0; cmd->send_dtmf.dtmf[pos] != 0; pos++) {
> +		digit = strchr(dtmf_from, cmd->send_dtmf.dtmf[pos]);
> +		if (!digit) {
> +			rsp->result.type = STK_RESULT_TYPE_DATA_NOT_UNDERSTOOD;
> +			return TRUE;
> +		}
> +
> +		dtmf[pos] = dtmf_to[digit - dtmf_from];
> +	}
> +	dtmf[pos] = 0;
> +
> +	extern_req_start(stk);

I'm not sure that modeling this after the Send SMS implementation is a
good idea.  Send SMS uses the SMS submit queue, which does not support
cancellation.  Wouldn't the Setup Call implementation be a better
template? E.g. a voicecall_send_tone and voicecall_send_tone_cancel?

> +
> +	err = __ofono_voicecall_send_tone(vc, dtmf,
> +						dtmf_sent_cb, stk->extern_req);
> +	if (err < 0)
> +		g_free(stk->extern_req);
> +
> +	if (err == -EBUSY) {
> +		rsp->result.type = STK_RESULT_TYPE_TERMINAL_BUSY;
> +		return TRUE;
> +	}
> +
> +	if (err == -ENOSYS) {
> +		rsp->result.type = STK_RESULT_TYPE_NOT_CAPABLE;
> +		return TRUE;
> +	}
> +
> +	if (err == -ENOENT) {
> +		rsp->result.type = STK_RESULT_TYPE_TERMINAL_BUSY;
> +		rsp->result.additional_len = sizeof(not_in_speech_call_result);
> +		rsp->result.additional = not_in_speech_call_result;
> +		return TRUE;
> +	}
> +
> +	if (err < 0) {
> +		/*
> +		 * We most likely got an out of memory error, tell SIM
> +		 * to retry
> +		 */
> +		rsp->result.type = STK_RESULT_TYPE_TERMINAL_BUSY;
> +		return TRUE;
> +	}
> +
> +	if (cmd->send_dtmf.alpha_id && cmd->send_dtmf.alpha_id[0])
> +		stk_alpha_id_set(stk, cmd->send_dtmf.alpha_id);
> +
> +	/*
> +	 * Note that we don't strictly require an agent to be connected,
> +	 * but to comply with 6.4.24 we need to send a End Session when
> +	 * the user decides so.
> +	 */
> +	stk->respond_on_exit = TRUE;
> +	stk->cancel_cmd = send_dtmf_cancel;
> +
> +	return FALSE;
> +}
> +
>  static void stk_proactive_command_cancel(struct ofono_stk *stk)
>  {
>  	if (stk->immediate_response)
> @@ -1996,6 +2150,11 @@ void ofono_stk_proactive_command_notify(struct ofono_stk *stk,
>  							&rsp, stk);
>  		break;
>  
> +	case STK_COMMAND_TYPE_SEND_DTMF:
> +		respond = handle_command_send_dtmf(stk->pending_cmd,
> +							&rsp, stk);
> +		break;
> +
>  	default:
>  		rsp.result.type = STK_RESULT_TYPE_COMMAND_NOT_UNDERSTOOD;
>  		break;

Regards,
-Denis

Re: [PATCH 09/13] stk: Simplify and add icon to alphaId api.

[Denis Kenzior]

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

Hi Andrew,

On 10/13/2010 08:54 AM, Andrzej Zaborowski wrote:
> The assumption is now that stk_alpha_id_set will handle NULL
> or empty alphaIds or icons.
> ---
>  src/stk.c |   36 ++++++++++--------------------------
>  1 files changed, 10 insertions(+), 26 deletions(-)
> 

I applied most of the chunks in this patch except for:

> @@ -1883,9 +1872,7 @@ static void send_dtmf_cancel(struct ofono_stk *stk)
>  	stk->respond_on_exit = FALSE;
>  	stk->extern_req->cancelled = TRUE;
>  
> -	if (stk->pending_cmd->send_dtmf.alpha_id &&
> -			stk->pending_cmd->send_dtmf.alpha_id[0])
> -		stk_alpha_id_unset(stk);
> +	stk_alpha_id_unset(stk);
>  }
>  
>  static void dtmf_sent_cb(const struct ofono_error *error, void *user_data)
> @@ -1904,9 +1891,7 @@ static void dtmf_sent_cb(const struct ofono_error *error, void *user_data)
>  
>  	stk->respond_on_exit = FALSE;
>  
> -	if (stk->pending_cmd->send_dtmf.alpha_id &&
> -			stk->pending_cmd->send_dtmf.alpha_id[0])
> -		stk_alpha_id_unset(stk);
> +	stk_alpha_id_unset(stk);
>  
>  	if (error->type == OFONO_ERROR_TYPE_FAILURE &&
>  			error->error == ENOENT) {
> @@ -2006,8 +1991,7 @@ static gboolean handle_command_send_dtmf(const struct stk_command *cmd,
>  		return TRUE;
>  	}
>  
> -	if (cmd->send_dtmf.alpha_id && cmd->send_dtmf.alpha_id[0])
> -		stk_alpha_id_set(stk, cmd->send_dtmf.alpha_id);
> +	stk_alpha_id_set(stk, cmd->send_dtmf.alpha_id, &cmd->send_dtmf.icon_id);
>  
>  	/*
>  	 * Note that we don't strictly require an agent to be connected,

Thanks.

Regards,
-Denis

Re: [PATCH 10/13] stk: Apply STK text attributes as html.

[Denis Kenzior]

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

Hi Andrew,

On 10/13/2010 08:54 AM, Andrzej Zaborowski wrote:
> ---
>  src/stk.c |  146 +++++++++++++++++++++++++++++++++++++++++++++++++++----------
>  1 files changed, 122 insertions(+), 24 deletions(-)
> 

Can you please rebase this patch as it no longer applies.

Thanks,
-Denis

Re: [PATCH 11/13] stkagent: Add PlayTone and LoopTone requests.

[Denis Kenzior]

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

Hi Andrew,

On 10/13/2010 08:54 AM, Andrzej Zaborowski wrote:
> ---
>  src/stkagent.c |   94 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/stkagent.h |   13 ++++++++
>  2 files changed, 107 insertions(+), 0 deletions(-)

This patch has been applied. Thanks.

Regards,
-Denis

Re: [PATCH 12/13] stk: Handle the Play Tone proactive command.

[Denis Kenzior]

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

Hi Andrew,

On 10/13/2010 08:54 AM, Andrzej Zaborowski wrote:
> A memory leak in error path has been fixed since the previous version.
> ---
>  src/stk.c |  139 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 139 insertions(+), 0 deletions(-)
> 
> diff --git a/src/stk.c b/src/stk.c
> index c06bf99..b3d2b40 100644
> --- a/src/stk.c
> +++ b/src/stk.c
> @@ -89,6 +89,11 @@ struct extern_req {
>  	gboolean cancelled;
>  };
>  
> +struct tone_info {
> +	const char *name;
> +	gboolean continuous;
> +};
> +

Can we make this into an anonymous struct inside handle_command_play_tone?

>  #define ENVELOPE_RETRIES_DEFAULT 5
>  
>  static void envelope_queue_run(struct ofono_stk *stk);
> @@ -2102,6 +2107,135 @@ static gboolean handle_command_send_dtmf(const struct stk_command *cmd,
>  	return FALSE;
>  }
>  
> +static void play_tone_cb(enum stk_agent_result result, void *user_data)
> +{
> +	struct ofono_stk *stk = user_data;
> +
> +	stk->respond_on_exit = FALSE;
> +
> +	switch (result) {
> +	case STK_AGENT_RESULT_OK:
> +	case STK_AGENT_RESULT_TIMEOUT:
> +		send_simple_response(stk, STK_RESULT_TYPE_SUCCESS);
> +		break;
> +
> +	default:
> +		send_simple_response(stk, STK_RESULT_TYPE_USER_TERMINATED);
> +		break;
> +	}
> +}
> +
> +static gboolean handle_command_play_tone(const struct stk_command *cmd,
> +						struct stk_response *rsp,
> +						struct ofono_stk *stk)
> +{
> +	static int manufacturer_timeout = 10000; /* 10 seconds */
> +	/* Continuous true/false according to 02.40 */
> +	static const struct tone_info tone_infos[] = {
> +		/* Default */
> +		[0x00] = { "dial-tone", TRUE },
> +
> +		/* Standard */
> +		[0x01] = { "dial-tone", TRUE },
> +		[0x02] = { "busy", TRUE },
> +		[0x03] = { "congestion", TRUE },
> +		[0x04] = { "radio-path-acknowledge", FALSE },
> +		[0x05] = { "radio-path-not-available", FALSE },
> +		[0x06] = { "error", TRUE },
> +		[0x07] = { "call-waiting", FALSE },
> +		[0x08] = { "ringing-tone", TRUE },
> +
> +		/* Proprietary */
> +		[0x10] = { "general-beep", FALSE },
> +		[0x11] = { "positive-acknowledgement", FALSE },
> +		[0x12] = { "negative-acknowledgement", FALSE },
> +		[0x13] = { "user-ringing-tone", TRUE },
> +		[0x14] = { "user-sms-alert", FALSE },
> +		[0x15] = { "critical", FALSE },
> +		[0x20] = { "vibrate", TRUE },
> +
> +		/* Themed */
> +		[0x30] = { "happy", FALSE },
> +		[0x31] = { "sad", FALSE },
> +		[0x32] = { "urgent-action", FALSE },
> +		[0x33] = { "question", FALSE },
> +		[0x34] = { "message-received", FALSE },
> +
> +		/* Melody */
> +		[0x40] = { "melody-1", FALSE },
> +		[0x41] = { "melody-2", FALSE },
> +		[0x42] = { "melody-3", FALSE },
> +		[0x43] = { "melody-4", FALSE },
> +		[0x44] = { "melody-5", FALSE },
> +		[0x45] = { "melody-6", FALSE },
> +		[0x46] = { "melody-7", FALSE },
> +		[0x47] = { "melody-8", FALSE },
> +	};
> +
> +	const struct stk_command_play_tone *pt = &cmd->play_tone;
> +	uint8_t qualifier = stk->pending_cmd->qualifier;
> +	gboolean vibrate = (qualifier & (1 << 0)) != 0;
> +	char *text;
> +	int timeout;
> +	int err;
> +
> +	if (!tone_infos[pt->tone].name) {
> +		rsp->result.type = STK_RESULT_TYPE_DATA_NOT_UNDERSTOOD;
> +
> +		return TRUE;
> +	}
> +
> +	text = dbus_apply_text_attributes(pt->alpha_id ? pt->alpha_id : "",
> +						&pt->text_attr);
> +	if (!text) {
> +		rsp->result.type = STK_RESULT_TYPE_DATA_NOT_UNDERSTOOD;
> +
> +		return TRUE;
> +	}
> +
> +	if (pt->duration.interval) {
> +		timeout = pt->duration.interval;
> +		switch (pt->duration.unit) {
> +		case STK_DURATION_TYPE_MINUTES:
> +			timeout *= 60;
> +		case STK_DURATION_TYPE_SECONDS:
> +			timeout *= 10;
> +		case STK_DURATION_TYPE_SECOND_TENTHS:
> +			timeout *= 100;
> +		}

This switch is repeated so many times throughout the code it should
really be turned into its own function.  Also, whenever you have
fall-throughs, they should be documented (e.g. with /* Fall through */)
otherwise me or Marcel are likely to go in and put in some break
statements in accidentally.

> +	} else
> +		timeout = manufacturer_timeout;
> +
> +	if (!tone_infos[pt->tone].continuous)

So this one makes me a bit worried.  Should we be using a search or at
least performing boundary checking?  Otherwise I'm afraid there is a
possibility of a crash if the SIM sends us something we do not expect.

> +		/* Duration ignored */
> +		err = stk_agent_play_tone(stk->current_agent, text,
> +						&pt->icon_id, vibrate,
> +						tone_infos[pt->tone].name,
> +						play_tone_cb, stk, NULL,
> +						stk->timeout * 1000);
> +	else
> +		err = stk_agent_loop_tone(stk->current_agent, text,
> +						&pt->icon_id, vibrate,
> +						tone_infos[pt->tone].name,
> +						play_tone_cb, stk, NULL,
> +						timeout);
> +	g_free(text);
> +
> +	if (err < 0) {
> +		/*
> +		 * We most likely got an out of memory error, tell SIM
> +		 * to retry
> +		 */
> +		rsp->result.type = STK_RESULT_TYPE_TERMINAL_BUSY;
> +		return TRUE;
> +	}
> +
> +	stk->respond_on_exit = TRUE;
> +	stk->cancel_cmd = stk_request_cancel;
> +
> +	return FALSE;
> +}
> +
>  static void stk_proactive_command_cancel(struct ofono_stk *stk)
>  {
>  	if (stk->immediate_response)
> @@ -2279,6 +2413,11 @@ void ofono_stk_proactive_command_notify(struct ofono_stk *stk,
>  							&rsp, stk);
>  		break;
>  
> +	case STK_COMMAND_TYPE_PLAY_TONE:
> +		respond = handle_command_play_tone(stk->pending_cmd,
> +							&rsp, stk);
> +		break;
> +
>  	default:
>  		rsp.result.type = STK_RESULT_TYPE_COMMAND_NOT_UNDERSTOOD;
>  		break;

Regards,
-Denis

Re: list-modems patch

[Alexander A Khryukin]

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

В Чтв, 14/10/2010 в 11:45 +0300, Marcel Holtmann пишет:
> Hi Alex,
> 
> > Sometimes i see 
> > 
> > [root(a)alexander-desktop ofono-0.31]# /usr/lib/ofono/test/list-modems 
> > [ /huawei5 ]
> >     Features = sim 
> >     Powered = 1
> >     Interfaces = org.ofono.Phonebook org.ofono.AudioSettings
> > org.ofono.VoiceCallManager org.ofono.SimManager 
> >     Online = 0
> >     Model = E1550
> >     Manufacturer = huawei
> >     Serial = 353142033084081
> >     Revision = 11.608.12.00.143
> >     [ org.ofono.Phonebook ]
> >     [ org.ofono.AudioSettings ]
> > Traceback (most recent call last):
> >   File "/usr/lib/ofono/test/list-modems", line 61, in <module>
> >     print "        %s = %s" % (key, val).encode('ascii')
> > 
> > 
> > Patch
> > 
> > 
> > 
> > diff --git a/test/list-modems b/test/list-modems
> > index 557efd5..df1dca8 100755
> > --- a/test/list-modems
> > +++ b/test/list-modems
> > @@ -58,6 +58,10 @@ for path, properties in modems:
> >                                         ")" for text, icon in
> > properties[key] ])
> >                         else:
> >                              	val = str(properties[key])
> > -                       print "        %s = %s" % (key, val)
> > +                       try:
> > +                               print  "       %s = %s" % (key, val)
> > +                       except:
> > +                               continue
> > +
> 
> can you at least print the key value. Just not printing that property at
> all is bad since it is there. Just marking the value as not printable
> seems to be the better approach.
> 
> Regards
> 
> Marcel
> 
> 
> 

Little fix

diff --git a/test/list-modems b/test/list-modems
index 557efd5..f8f10ac 100755
--- a/test/list-modems
+++ b/test/list-modems
@@ -58,6 +58,12 @@ for path, properties in modems:
                                        ")" for text, icon in
properties[key] ])
                        else:
                                val = str(properties[key])
-                       print "        %s = %s" % (key, val)
+                       try:
+                               print  "       %s = %s" % (key, val)
+                       except:
+                               print "Script mesaage:maybe wrong ASCII
characters\n"
+                       else:
+                               continue
+

        print

Re: [PATCH 13/13] [RfC] API for STK driver to signal executed commands.

[Denis Kenzior]

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

Hi Andrew,

On 10/13/2010 08:54 AM, Andrzej Zaborowski wrote:
> Some modems are able to handle some proactive commands in their
> firmware or otherwise, if the command doesn't require input from user.
> Nevertheles ofono may need to update internal state or notify the user
> where necessary.  With this api the driver can notify core that a
> command is being executed in the modem or that a command is finished
> executing and the TERMINAL RESPONSE has been sent to SIM.  It would
> also be possible for a driver to handle a command.

I applied this patch with a small tweak to make it compile.

Thanks,
-Denis

Re: list-modems patch

[Alexander A Khryukin]

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

В Чтв, 14/10/2010 в 11:45 +0300, Marcel Holtmann пишет:
> Hi Alex,
> 
> > Sometimes i see 
> > 
> > [root(a)alexander-desktop ofono-0.31]# /usr/lib/ofono/test/list-modems 
> > [ /huawei5 ]
> >     Features = sim 
> >     Powered = 1
> >     Interfaces = org.ofono.Phonebook org.ofono.AudioSettings
> > org.ofono.VoiceCallManager org.ofono.SimManager 
> >     Online = 0
> >     Model = E1550
> >     Manufacturer = huawei
> >     Serial = 353142033084081
> >     Revision = 11.608.12.00.143
> >     [ org.ofono.Phonebook ]
> >     [ org.ofono.AudioSettings ]
> > Traceback (most recent call last):
> >   File "/usr/lib/ofono/test/list-modems", line 61, in <module>
> >     print "        %s = %s" % (key, val).encode('ascii')
> > 
> > 
> > Patch
> > 
> > 
> > 
> > diff --git a/test/list-modems b/test/list-modems
> > index 557efd5..df1dca8 100755
> > --- a/test/list-modems
> > +++ b/test/list-modems
> > @@ -58,6 +58,10 @@ for path, properties in modems:
> >                                         ")" for text, icon in
> > properties[key] ])
> >                         else:
> >                              	val = str(properties[key])
> > -                       print "        %s = %s" % (key, val)
> > +                       try:
> > +                               print  "       %s = %s" % (key, val)
> > +                       except:
> > +                               continue
> > +
> 
> can you at least print the key value. Just not printing that property at
> all is bad since it is there. Just marking the value as not printable
> seems to be the better approach.
> 
> Regards
> 
> Marcel
> 
> 
> 

Last fully-working script

[ /huawei0 ]


del overquote

        ServiceNumbers = [Моб. Помощник] = '111' [MTС] =
'+78003330890' [Служба спасения] = '112' 
        CardIdentifier = 89701012417666587513
        LockedPins = 
        PinRequired = none
        SubscriberIdentity = 250011766658751
        Present = 1


[Моб. Помощник] - in russian means "Mobile Partner"
[Служба спасения] - rescue rangers :3           




diff --git a/test/list-modems b/test/list-modems
index 557efd5..59765f9 100755
--- a/test/list-modems
+++ b/test/list-modems
@@ -58,6 +58,10 @@ for path, properties in modems:
                                        ")" for text, icon in
properties[key] ])
                        else:
                                val = str(properties[key])
-                       print "        %s = %s" % (key, val)
-
+                       try:
+                               print "        %s = %s" %
(key.encode('utf8'), val.encode('utf8'))
+                       except:
+                               print "Cannot encode some charcters
please change locale"
+                       else:
+                                continue
        print

Re: list-modems patch

[Marcel Holtmann]

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

Hi Alex,

> > > Sometimes i see 
> > > 
> > > [root(a)alexander-desktop ofono-0.31]# /usr/lib/ofono/test/list-modems 
> > > [ /huawei5 ]
> > >     Features = sim 
> > >     Powered = 1
> > >     Interfaces = org.ofono.Phonebook org.ofono.AudioSettings
> > > org.ofono.VoiceCallManager org.ofono.SimManager 
> > >     Online = 0
> > >     Model = E1550
> > >     Manufacturer = huawei
> > >     Serial = 353142033084081
> > >     Revision = 11.608.12.00.143
> > >     [ org.ofono.Phonebook ]
> > >     [ org.ofono.AudioSettings ]
> > > Traceback (most recent call last):
> > >   File "/usr/lib/ofono/test/list-modems", line 61, in <module>
> > >     print "        %s = %s" % (key, val).encode('ascii')
> > > 
> > > 
> > > Patch
> > > 
> > > 
> > > 
> > > diff --git a/test/list-modems b/test/list-modems
> > > index 557efd5..df1dca8 100755
> > > --- a/test/list-modems
> > > +++ b/test/list-modems
> > > @@ -58,6 +58,10 @@ for path, properties in modems:
> > >                                         ")" for text, icon in
> > > properties[key] ])
> > >                         else:
> > >                              	val = str(properties[key])
> > > -                       print "        %s = %s" % (key, val)
> > > +                       try:
> > > +                               print  "       %s = %s" % (key, val)
> > > +                       except:
> > > +                               continue
> > > +
> > 
> > can you at least print the key value. Just not printing that property at
> > all is bad since it is there. Just marking the value as not printable
> > seems to be the better approach.
> > 
> > Regards
> > 
> > Marcel
> > 
> > 
> > 
> 
> Last fully-working script
> 
> [ /huawei0 ]
> 
> 
> del overquote
> 
>         ServiceNumbers = [Моб. Помощник] = '111' [MTС] =
> '+78003330890' [Служба спасения] = '112' 
>         CardIdentifier = 89701012417666587513
>         LockedPins = 
>         PinRequired = none
>         SubscriberIdentity = 250011766658751
>         Present = 1
> 
> 
> [Моб. Помощник] - in russian means "Mobile Partner"
> [Служба спасения] - rescue rangers :3           
> 
> 
> 
> 
> diff --git a/test/list-modems b/test/list-modems
> index 557efd5..59765f9 100755
> --- a/test/list-modems
> +++ b/test/list-modems
> @@ -58,6 +58,10 @@ for path, properties in modems:
>                                         ")" for text, icon in
> properties[key] ])
>                         else:
>                                 val = str(properties[key])
> -                       print "        %s = %s" % (key, val)
> -
> +                       try:
> +                               print "        %s = %s" %
> (key.encode('utf8'), val.encode('utf8'))
> +                       except:
> +                               print "Cannot encode some charcters
> please change locale"
> +                       else:
> +                                continue
>         print

patch does not apply. Please always check with git am that your patch
would apply cleanly. And that your mailer doesn't screw it up.

Regards

Marcel

Re: list-modems patch

[Alexander A Khryukin]

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

В Чтв, 14/10/2010 в 13:13 +0300, Marcel Holtmann пишет:
> Hi Alex,
> 
> > > > Sometimes i see 
> > > > 
> > > > [root(a)alexander-desktop ofono-0.31]# /usr/lib/ofono/test/list-modems 
> > > > [ /huawei5 ]
> > > >     Features = sim 
> > > >     Powered = 1
> > > >     Interfaces = org.ofono.Phonebook org.ofono.AudioSettings
> > > > org.ofono.VoiceCallManager org.ofono.SimManager 
> > > >     Online = 0
> > > >     Model = E1550
> > > >     Manufacturer = huawei
> > > >     Serial = 353142033084081
> > > >     Revision = 11.608.12.00.143
> > > >     [ org.ofono.Phonebook ]
> > > >     [ org.ofono.AudioSettings ]
> > > > Traceback (most recent call last):
> > > >   File "/usr/lib/ofono/test/list-modems", line 61, in <module>
> > > >     print "        %s = %s" % (key, val).encode('ascii')
> > > > 
> > > > 
> > > > Patch
> > > > 
> > > > 
> > > > 
> > > > diff --git a/test/list-modems b/test/list-modems
> > > > index 557efd5..df1dca8 100755
> > > > --- a/test/list-modems
> > > > +++ b/test/list-modems
> > > > @@ -58,6 +58,10 @@ for path, properties in modems:
> > > >                                         ")" for text, icon in
> > > > properties[key] ])
> > > >                         else:
> > > >                              	val = str(properties[key])
> > > > -                       print "        %s = %s" % (key, val)
> > > > +                       try:
> > > > +                               print  "       %s = %s" % (key, val)
> > > > +                       except:
> > > > +                               continue
> > > > +
> > > 
> > > can you at least print the key value. Just not printing that property at
> > > all is bad since it is there. Just marking the value as not printable
> > > seems to be the better approach.
> > > 
> > > Regards
> > > 
> > > Marcel
> > > 
> > > 
> > > 
> > 
> > Last fully-working script
> > 
> > [ /huawei0 ]
> > 
> > 
> > del overquote
> > 
> >         ServiceNumbers = [Моб. Помощник] = '111' [MTС] =
> > '+78003330890' [Служба спасения] = '112' 
> >         CardIdentifier = 89701012417666587513
> >         LockedPins = 
> >         PinRequired = none
> >         SubscriberIdentity = 250011766658751
> >         Present = 1
> > 
> > 
> > [Моб. Помощник] - in russian means "Mobile Partner"
> > [Служба спасения] - rescue rangers :3           
> > 
> > 
> > 
> > 
> > diff --git a/test/list-modems b/test/list-modems
> > index 557efd5..59765f9 100755
> > --- a/test/list-modems
> > +++ b/test/list-modems
> > @@ -58,6 +58,10 @@ for path, properties in modems:
> >                                         ")" for text, icon in
> > properties[key] ])
> >                         else:
> >                                 val = str(properties[key])
> > -                       print "        %s = %s" % (key, val)
> > -
> > +                       try:
> > +                               print "        %s = %s" %
> > (key.encode('utf8'), val.encode('utf8'))
> > +                       except:
> > +                               print "Cannot encode some charcters
> > please change locale"
> > +                       else:
> > +                                continue
> >         print
> 
> patch does not apply. Please always check with git am that your patch
> would apply cleanly. And that your mailer doesn't screw it up.
> 
> Regards
> 
> Marcel
> 
> 
> 

Evolution reformate message anyway
Please click on link.
Was checked and applied for me with success

http://pastie.org/pastes/1220422/text

Re: list-modems patch

[Marcel Holtmann]

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

Hi Alex,

> > > > > Sometimes i see 
> > > > > 
> > > > > [root(a)alexander-desktop ofono-0.31]# /usr/lib/ofono/test/list-modems 
> > > > > [ /huawei5 ]
> > > > >     Features = sim 
> > > > >     Powered = 1
> > > > >     Interfaces = org.ofono.Phonebook org.ofono.AudioSettings
> > > > > org.ofono.VoiceCallManager org.ofono.SimManager 
> > > > >     Online = 0
> > > > >     Model = E1550
> > > > >     Manufacturer = huawei
> > > > >     Serial = 353142033084081
> > > > >     Revision = 11.608.12.00.143
> > > > >     [ org.ofono.Phonebook ]
> > > > >     [ org.ofono.AudioSettings ]
> > > > > Traceback (most recent call last):
> > > > >   File "/usr/lib/ofono/test/list-modems", line 61, in <module>
> > > > >     print "        %s = %s" % (key, val).encode('ascii')
> > > > > 
> > > > > 
> > > > > Patch
> > > > > 
> > > > > 
> > > > > 
> > > > > diff --git a/test/list-modems b/test/list-modems
> > > > > index 557efd5..df1dca8 100755
> > > > > --- a/test/list-modems
> > > > > +++ b/test/list-modems
> > > > > @@ -58,6 +58,10 @@ for path, properties in modems:
> > > > >                                         ")" for text, icon in
> > > > > properties[key] ])
> > > > >                         else:
> > > > >                              	val = str(properties[key])
> > > > > -                       print "        %s = %s" % (key, val)
> > > > > +                       try:
> > > > > +                               print  "       %s = %s" % (key, val)
> > > > > +                       except:
> > > > > +                               continue
> > > > > +
> > > > 
> > > > can you at least print the key value. Just not printing that property at
> > > > all is bad since it is there. Just marking the value as not printable
> > > > seems to be the better approach.
> > > > 
> > > > Regards
> > > > 
> > > > Marcel
> > > > 
> > > > 
> > > > 
> > > 
> > > Last fully-working script
> > > 
> > > [ /huawei0 ]
> > > 
> > > 
> > > del overquote
> > > 
> > >         ServiceNumbers = [Моб. Помощник] = '111' [MTС] =
> > > '+78003330890' [Служба спасения] = '112' 
> > >         CardIdentifier = 89701012417666587513
> > >         LockedPins = 
> > >         PinRequired = none
> > >         SubscriberIdentity = 250011766658751
> > >         Present = 1
> > > 
> > > 
> > > [Моб. Помощник] - in russian means "Mobile Partner"
> > > [Служба спасения] - rescue rangers :3           
> > > 
> > > 
> > > 
> > > 
> > > diff --git a/test/list-modems b/test/list-modems
> > > index 557efd5..59765f9 100755
> > > --- a/test/list-modems
> > > +++ b/test/list-modems
> > > @@ -58,6 +58,10 @@ for path, properties in modems:
> > >                                         ")" for text, icon in
> > > properties[key] ])
> > >                         else:
> > >                                 val = str(properties[key])
> > > -                       print "        %s = %s" % (key, val)
> > > -
> > > +                       try:
> > > +                               print "        %s = %s" %
> > > (key.encode('utf8'), val.encode('utf8'))
> > > +                       except:
> > > +                               print "Cannot encode some charcters
> > > please change locale"
> > > +                       else:
> > > +                                continue
> > >         print
> > 
> > patch does not apply. Please always check with git am that your patch
> > would apply cleanly. And that your mailer doesn't screw it up.
> > 
> > Regards
> > 
> > Marcel
> > 
> > 
> > 
> 
> Evolution reformate message anyway
> Please click on link.
> Was checked and applied for me with success
> 
> http://pastie.org/pastes/1220422/text

I see a whitespace damage in the patch and it is still not possible to
apply this. Please send a proper patch to the mailing list. You do need
to get your email client fixed anyway.

Regards

Marcel

Re: [PATCH 01/13] voicecall: __ofono_voicecall_send_tone internal api.

[Andrzej Zaborowski]

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

Hi Denis,

sorry for responding late.

On 14 October 2010 10:47, Denis Kenzior <denkenz@gmail.com> wrote:
> On 10/13/2010 08:54 AM, Andrzej Zaborowski wrote:
>> This provides a way for other atoms to send DTMF tones during a call.
>> ---
>>  src/ofono.h     |    4 ++++
>>  src/voicecall.c |   16 ++++++++++++++++
>>  2 files changed, 20 insertions(+), 0 deletions(-)
>>
>> diff --git a/src/ofono.h b/src/ofono.h
>> index 6c7f649..6efd9ac 100644
>> --- a/src/ofono.h
>> +++ b/src/ofono.h
>> @@ -218,6 +218,10 @@ 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);
>>
>> +int __ofono_voicecall_send_tone(struct ofono_voicecall *vc,
>> +                             const char *tone_str,
>> +                             ofono_voicecall_cb_t cb, void *user_data);
>> +
>
> So this one is structured according to __ofono_voicecall_dial, should we
> also have __ofono_voicecall_send_tone_cancel?
>
>>  #include <ofono/sms.h>
>>
>>  struct sms;
>> diff --git a/src/voicecall.c b/src/voicecall.c
>> index 7b5fe3b..45e19ce 100644
>> --- a/src/voicecall.c
>> +++ b/src/voicecall.c
>> @@ -2326,3 +2326,19 @@ void __ofono_voicecall_dial_cancel(struct ofono_voicecall *vc)
>>
>>       vc->dial_req->cb = NULL;
>>  }
>> +
>> +int __ofono_voicecall_send_tone(struct ofono_voicecall *vc,
>> +                             const char *tone_str,
>> +                             ofono_voicecall_cb_t cb, void *user_data)
>> +{
>> +     if (!vc->driver->send_tones)
>> +             return -ENOSYS;
>> +
>> +     /* Send DTMFs only if we have at least one connected call */
>> +     if (!voicecalls_can_dtmf(vc))
>> +             return -ENOENT;
>
> I'm a bit worried that we never check for BUSY conditions here, in
> particular since we can get into this situation:
>
> Send DTMF
> Cancel
> Send DTMF
>
> Which results in two DTMF operations being outstanding.
>
>> +
>> +     vc->driver->send_tones(vc, tone_str, cb, user_data);
>> +
>> +     return 0;
>> +}
>
> We also don't busy out any D-Bus operations when Send DTMF is in progress...

So I gave this some thought now and made an implementation based on
the __ofono_voicecall_dial api, but I think that this old approach is
actually working better for us.  There are a couple of scenarios
(let's consider only atmodem first)

1. Send DTMF - Cancel - Send DTMF

in this case the second Send DTMF is going to return an error from the
driver (not from core voicecall.c).  I don't think it's a big deal,
but we could think of something like cancelling the driver .send_tones
call, because the operation can potentially take a while -- this would
need a little change in the driver api.

2. Send DTMF - DBus command arrives,

Now any DBus command (other than SendTone) will actually be sent to
the driver and executed.  I think that's the correct thing to do --
the user needs to be able to end the call at any time.  The tones
being emitted should not actually interfere with anything.  The only
thing we need to do is respond to the STK command with an error if the
call is terminated before all tones finished being emitted.

3. Modem is doing something and a Send DTMF stk command arrives,

In this case the DTMFs will be delayed because of command
serialisation in gatchat / gisi.  Should we return "terminal busy" to
the card instead?

What do you think?  What behaviour do we want?

The handling may be different in case of isimodem, so this would need
checking anyway.

Best regards

Re: [PATCH 01/13] voicecall: __ofono_voicecall_send_tone internal api.

[Denis Kenzior]

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

Hi Andrew,

> So I gave this some thought now and made an implementation based on
> the __ofono_voicecall_dial api, but I think that this old approach is
> actually working better for us.  There are a couple of scenarios
> (let's consider only atmodem first)
> 
> 1. Send DTMF - Cancel - Send DTMF
> 
> in this case the second Send DTMF is going to return an error from the
> driver (not from core voicecall.c).  I don't think it's a big deal,
> but we could think of something like cancelling the driver .send_tones
> call, because the operation can potentially take a while -- this would
> need a little change in the driver api.

Why would this return an error from the driver? All current drivers will
simply g_at_chat_send the next command, which will get appended to the
queue and executed in turn.

Command cancellation is not something I think will ever work.  Most
modems don't get this part right at all or have weird timing
requirements which are impossible to get right.

> 
> 2. Send DTMF - DBus command arrives,
> 
> Now any DBus command (other than SendTone) will actually be sent to
> the driver and executed.  I think that's the correct thing to do --
> the user needs to be able to end the call at any time.  The tones

This won't really work on any AT modem today.  The ATH will be queued
until after the AT+VTS is executed...

> being emitted should not actually interfere with anything.  The only
> thing we need to do is respond to the STK command with an error if the
> call is terminated before all tones finished being emitted.

Hanging up while tones are active is a valid point though. Perhaps the
core should be sending one tone at a time to make this feasible?

> 
> 3. Modem is doing something and a Send DTMF stk command arrives,
> 
> In this case the DTMFs will be delayed because of command
> serialisation in gatchat / gisi.  Should we return "terminal busy" to
> the card instead?
> 
> What do you think?  What behaviour do we want?

This one is tricky, as we don't really know what the outcome of the
pending operation is.  You also cannot rely on the queuing behavior in
the driver.  Remember, the driver is free to choose the multiplexing
strategy, so nothing is preventing it from putting send_tones onto a
different channel than CHLD/ATH/ATD/ATA.  This is why the core is fairly
adamant about having 1 op outstanding at any one time.

My feeling here is that we should return busy for now and revisit this
decision if it ever becomes a problem.

Regards,
-Denis

Re: [PATCH 01/13] voicecall: __ofono_voicecall_send_tone internal api.

[Andrzej Zaborowski]

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

On 19 October 2010 16:58, Denis Kenzior <denkenz@gmail.com> wrote:
> Hi Andrew,
>
>> So I gave this some thought now and made an implementation based on
>> the __ofono_voicecall_dial api, but I think that this old approach is
>> actually working better for us.  There are a couple of scenarios
>> (let's consider only atmodem first)
>>
>> 1. Send DTMF - Cancel - Send DTMF
>>
>> in this case the second Send DTMF is going to return an error from the
>> driver (not from core voicecall.c).  I don't think it's a big deal,
>> but we could think of something like cancelling the driver .send_tones
>> call, because the operation can potentially take a while -- this would
>> need a little change in the driver api.
>
> Why would this return an error from the driver? All current drivers will
> simply g_at_chat_send the next command, which will get appended to the
> queue and executed in turn.

In case of send_tone, we are always waiting for a timeout and not
doing anything because +VTS returns immediately.  In the patch 3 I
sent ([1]) we just check if vc->tr is non-null and this way we know
that we're already executing a send_tone, so we error out immediately.

1. http://lists.ofono.org/pipermail/ofono/2010-October/005065.html

>
> Command cancellation is not something I think will ever work.  Most
> modems don't get this part right at all or have weird timing
> requirements which are impossible to get right.

Yes, but in this case we don't need to cancel an AT command, so
send_tone would be a special case.

>
>>
>> 2. Send DTMF - DBus command arrives,
>>
>> Now any DBus command (other than SendTone) will actually be sent to
>> the driver and executed.  I think that's the correct thing to do --
>> the user needs to be able to end the call at any time.  The tones
>
> This won't really work on any AT modem today.  The ATH will be queued
> until after the AT+VTS is executed...

Yes, but again AT+VTS is executed immediately (on some modems anyway)
and the delay is not significant.

>
>> being emitted should not actually interfere with anything.  The only
>> thing we need to do is respond to the STK command with an error if the
>> call is terminated before all tones finished being emitted.
>
> Hanging up while tones are active is a valid point though. Perhaps the
> core should be sending one tone at a time to make this feasible?

Either the core or the driver I guess (the initial patchset did all
the delays handling in the core, assuming that the driver->send_tone
returns immediately, and in the latest patches the delay was
implemented in atmodem according to some discussion we had on IRC,
though it was a while ago because I had some time off)

>
>>
>> 3. Modem is doing something and a Send DTMF stk command arrives,
>>
>> In this case the DTMFs will be delayed because of command
>> serialisation in gatchat / gisi.  Should we return "terminal busy" to
>> the card instead?
>>
>> What do you think?  What behaviour do we want?
>
> This one is tricky, as we don't really know what the outcome of the
> pending operation is.  You also cannot rely on the queuing behavior in
> the driver.  Remember, the driver is free to choose the multiplexing
> strategy, so nothing is preventing it from putting send_tones onto a
> different channel than CHLD/ATH/ATD/ATA.  This is why the core is fairly
> adamant about having 1 op outstanding at any one time.

In this case I think it doesn't seem so problematic because +VTS
doesn't occupy the channel for a long time.  So to comply with 102.223
it seems that all we need to do is:

* check if at the start of executing SendDTMF we are in a call.

* if yes, send the tones one after another with delays and let other
commands run as normal in between,

* once we've waited enough, return success if we're still in a call,
or failure if we're not in a call anymore.

What are your thoughts on this?

Best regards

Re: [PATCH 01/13] voicecall: __ofono_voicecall_send_tone internal api.

[Denis Kenzior]

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

Hi Andrew,

On 10/19/2010 10:34 AM, Andrzej Zaborowski wrote:
> On 19 October 2010 16:58, Denis Kenzior <denkenz@gmail.com> wrote:
>> Hi Andrew,
>>
>>> So I gave this some thought now and made an implementation based on
>>> the __ofono_voicecall_dial api, but I think that this old approach is
>>> actually working better for us.  There are a couple of scenarios
>>> (let's consider only atmodem first)
>>>
>>> 1. Send DTMF - Cancel - Send DTMF
>>>
>>> in this case the second Send DTMF is going to return an error from the
>>> driver (not from core voicecall.c).  I don't think it's a big deal,
>>> but we could think of something like cancelling the driver .send_tones
>>> call, because the operation can potentially take a while -- this would
>>> need a little change in the driver api.
>>
>> Why would this return an error from the driver? All current drivers will
>> simply g_at_chat_send the next command, which will get appended to the
>> queue and executed in turn.
> 
> In case of send_tone, we are always waiting for a timeout and not
> doing anything because +VTS returns immediately.  In the patch 3 I
> sent ([1]) we just check if vc->tr is non-null and this way we know
> that we're already executing a send_tone, so we error out immediately.
> 
> 1. http://lists.ofono.org/pipermail/ofono/2010-October/005065.html
> 

I honestly haven't looked at this patch yet, but it seems to be a bit
crazy to duplicate in all voicecall drivers, which we have many of these
days.  We should really solve this in the core.  About the only thing we
can force drivers to do is return from send_tone when the tone has finished.

I also don't really see relying on the driver to tell the core it is
busy in send_tones as the right thing to do here.

>> This won't really work on any AT modem today.  The ATH will be queued
>> until after the AT+VTS is executed...
> 
> Yes, but again AT+VTS is executed immediately (on some modems anyway)
> and the delay is not significant.

This isn't the case on IFX for example, which delays the OK until all
the tones have been sent.

>> This one is tricky, as we don't really know what the outcome of the
>> pending operation is.  You also cannot rely on the queuing behavior in
>> the driver.  Remember, the driver is free to choose the multiplexing
>> strategy, so nothing is preventing it from putting send_tones onto a
>> different channel than CHLD/ATH/ATD/ATA.  This is why the core is fairly
>> adamant about having 1 op outstanding at any one time.
> 
> In this case I think it doesn't seem so problematic because +VTS
> doesn't occupy the channel for a long time.  So to comply with 102.223
> it seems that all we need to do is:

Honestly I think we should make it mandatory for the send_tone operation
to tell us when it is done.  If it returns earlier than the actual tone
duration the driver has to take care of it.

> 
> * check if at the start of executing SendDTMF we are in a call.
> 
> * if yes, send the tones one after another with delays and let other
> commands run as normal in between,

We can probably check if we have active | held calls here and bail out
early if no calls are active.  Allowing other commands in between still
seems dangerous, but lets try it and see how it works out.

> 
> * once we've waited enough, return success if we're still in a call,
> or failure if we're not in a call anymore.
> 
Otherwise sounds good.

Regards,
-Denis