All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/8] stkutil: Refactor command parser error handling
@ 2010-06-22 11:20 Andrzej Zaborowski
  2010-06-22 11:20 ` [PATCH 2/8] stk: Allow registering proactive command handlers Andrzej Zaborowski
                   ` (7 more replies)
  0 siblings, 8 replies; 12+ messages in thread
From: Andrzej Zaborowski @ 2010-06-22 11:20 UTC (permalink / raw)
  To: ofono

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

When parsing the full command fails but Command Details has been parsed,
return a struct stk_command containing this information and the type of
parsing problem found.  We need the command details to be able to
even respond to the command.

This patch also makes the parser skip over unknown data objects found
in the BER-TLV, if they don't have Comprehension Required set.
---
 src/stkutil.c       |  955 +++++++++++++++++++++------------------------------
 src/stkutil.h       |    8 +
 unit/test-stkutil.c |   40 ++-
 3 files changed, 446 insertions(+), 557 deletions(-)

diff --git a/src/stkutil.c b/src/stkutil.c
index 6520c8a..c5eca3f 100644
--- a/src/stkutil.c
+++ b/src/stkutil.c
@@ -2283,16 +2283,17 @@ struct dataobj_handler_entry {
 	enum stk_data_object_type type;
 	int flags;
 	void *data;
-	gboolean parsed;
 };
 
-static gboolean parse_dataobj(struct comprehension_tlv_iter *iter,
-				enum stk_data_object_type type, ...)
+static enum stk_command_parse_result parse_dataobj(
+					struct comprehension_tlv_iter *iter,
+					enum stk_data_object_type type, ...)
 {
 	GSList *entries = NULL;
 	GSList *l;
 	va_list args;
 	gboolean minimum_set = TRUE;
+	gboolean parse_error = FALSE;
 
 	va_start(args, type);
 
@@ -2309,45 +2310,61 @@ static gboolean parse_dataobj(struct comprehension_tlv_iter *iter,
 		entries = g_slist_prepend(entries, entry);
 	}
 
-	if (comprehension_tlv_iter_next(iter) != TRUE)
-		goto out;
-
 	entries = g_slist_reverse(entries);
 
-	for (l = entries; l; l = l->next) {
+	l = entries;
+	while (comprehension_tlv_iter_next(iter) == TRUE) {
 		dataobj_handler handler;
-		struct dataobj_handler_entry *entry = l->data;
+		struct dataobj_handler_entry *entry;
+		GSList *l2;
+
+		for (l2 = l; l2; l2 = l2->next) {
+			entry = l2->data;
+
+			if (comprehension_tlv_iter_get_tag(iter) == entry->type)
+				break;
+
+			/* Can't skip over Minimum objects */
+			if (entry->flags & DATAOBJ_FLAG_MINIMUM) {
+				l2 = NULL;
+				break;
+			}
+		}
+
+		if (!l2) {
+			if (comprehension_tlv_get_cr(iter) == TRUE)
+				parse_error = TRUE;
+
+			continue;
+		}
 
 		if (entry->flags & DATAOBJ_FLAG_LIST)
 			handler = list_handler_for_type(entry->type);
 		else
 			handler = handler_for_type(entry->type);
 
-		if (handler == NULL)
-			continue;
+		if (handler(iter, entry->data) == FALSE)
+			parse_error = TRUE;
 
-		if (comprehension_tlv_iter_get_tag(iter) == entry->type) {
-			if (handler(iter, entry->data))
-				entry->parsed = TRUE;
-
-			if (comprehension_tlv_iter_next(iter) == FALSE)
-				break;
-		}
+		l = l2->next;
 	}
 
-out:
-	for (l = entries; l; l = l->next) {
+	for (; l; l = l->next) {
 		struct dataobj_handler_entry *entry = l->data;
 
-		if ((entry->flags & DATAOBJ_FLAG_MINIMUM) &&
-				entry->parsed == FALSE)
+		if (entry->flags & DATAOBJ_FLAG_MINIMUM)
 			minimum_set = FALSE;
 	}
 
 	g_slist_foreach(entries, (GFunc)g_free, NULL);
 	g_slist_free(entries);
 
-	return minimum_set;
+	if (minimum_set == FALSE)
+		return STK_PARSE_RESULT_MISSING_VALUE;
+	if (parse_error == TRUE)
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
+
+	return STK_PARSE_RESULT_OK;
 }
 
 static void destroy_display_text(struct stk_command *command)
@@ -2355,19 +2372,21 @@ static void destroy_display_text(struct stk_command *command)
 	g_free(command->display_text.text);
 }
 
-static gboolean parse_display_text(struct stk_command *command,
+static enum stk_command_parse_result parse_display_text(
+					struct stk_command *command,
 					struct comprehension_tlv_iter *iter)
 {
 	struct stk_command_display_text *obj = &command->display_text;
-	gboolean ret;
 
 	if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
 	if (command->dst != STK_DEVICE_IDENTITY_TYPE_DISPLAY)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
+
+	command->destructor = destroy_display_text;
 
-	ret = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_TEXT,
+	return parse_dataobj(iter, STK_DATA_OBJECT_TYPE_TEXT,
 				DATAOBJ_FLAG_MANDATORY | DATAOBJ_FLAG_MINIMUM,
 				&obj->text,
 				STK_DATA_OBJECT_TYPE_ICON_ID, 0,
@@ -2381,13 +2400,6 @@ static gboolean parse_display_text(struct stk_command *command,
 				STK_DATA_OBJECT_TYPE_FRAME_ID, 0,
 				&obj->frame_id,
 				STK_DATA_OBJECT_TYPE_INVALID);
-
-	command->destructor = destroy_display_text;
-
-	if (ret == FALSE)
-		return FALSE;
-
-	return TRUE;
 }
 
 static void destroy_get_inkey(struct stk_command *command)
@@ -2395,19 +2407,21 @@ static void destroy_get_inkey(struct stk_command *command)
 	g_free(command->get_inkey.text);
 }
 
-static gboolean parse_get_inkey(struct stk_command *command,
-				struct comprehension_tlv_iter *iter)
+static enum stk_command_parse_result parse_get_inkey(
+					struct stk_command *command,
+					struct comprehension_tlv_iter *iter)
 {
 	struct stk_command_get_inkey *obj = &command->get_inkey;
-	gboolean ret;
 
 	if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
 	if (command->dst != STK_DEVICE_IDENTITY_TYPE_TERMINAL)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
+
+	command->destructor = destroy_get_inkey;
 
-	ret = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_TEXT,
+	return parse_dataobj(iter, STK_DATA_OBJECT_TYPE_TEXT,
 				DATAOBJ_FLAG_MANDATORY | DATAOBJ_FLAG_MINIMUM,
 				&obj->text,
 				STK_DATA_OBJECT_TYPE_ICON_ID, 0,
@@ -2419,13 +2433,6 @@ static gboolean parse_get_inkey(struct stk_command *command,
 				STK_DATA_OBJECT_TYPE_FRAME_ID, 0,
 				&obj->frame_id,
 				STK_DATA_OBJECT_TYPE_INVALID);
-
-	command->destructor = destroy_get_inkey;
-
-	if (ret == FALSE)
-		return FALSE;
-
-	return TRUE;
 }
 
 static void destroy_get_input(struct stk_command *command)
@@ -2434,19 +2441,21 @@ static void destroy_get_input(struct stk_command *command)
 	g_free(command->get_input.default_text);
 }
 
-static gboolean parse_get_input(struct stk_command *command,
+static enum stk_command_parse_result parse_get_input(
+					struct stk_command *command,
 					struct comprehension_tlv_iter *iter)
 {
 	struct stk_command_get_input *obj = &command->get_input;
-	gboolean ret;
 
 	if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
 	if (command->dst != STK_DEVICE_IDENTITY_TYPE_TERMINAL)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
+
+	command->destructor = destroy_get_input;
 
-	ret = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_TEXT,
+	return parse_dataobj(iter, STK_DATA_OBJECT_TYPE_TEXT,
 				DATAOBJ_FLAG_MANDATORY | DATAOBJ_FLAG_MINIMUM,
 				&obj->text,
 				STK_DATA_OBJECT_TYPE_RESPONSE_LENGTH,
@@ -2461,25 +2470,19 @@ static gboolean parse_get_input(struct stk_command *command,
 				STK_DATA_OBJECT_TYPE_FRAME_ID, 0,
 				&obj->frame_id,
 				STK_DATA_OBJECT_TYPE_INVALID);
-
-	command->destructor = destroy_get_input;
-
-	if (ret == FALSE)
-		return FALSE;
-
-	return TRUE;
 }
 
-static gboolean parse_more_time(struct stk_command *command,
+static enum stk_command_parse_result parse_more_time(
+					struct stk_command *command,
 					struct comprehension_tlv_iter *iter)
 {
 	if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
 	if (command->dst != STK_DEVICE_IDENTITY_TYPE_TERMINAL)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
-	return TRUE;
+	return STK_PARSE_RESULT_OK;
 }
 
 static void destroy_play_tone(struct stk_command *command)
@@ -2487,19 +2490,21 @@ static void destroy_play_tone(struct stk_command *command)
 	g_free(command->play_tone.alpha_id);
 }
 
-static gboolean parse_play_tone(struct stk_command *command,
+static enum stk_command_parse_result parse_play_tone(
+					struct stk_command *command,
 					struct comprehension_tlv_iter *iter)
 {
 	struct stk_command_play_tone *obj = &command->play_tone;
-	gboolean ret;
 
 	if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
 	if (command->dst != STK_DEVICE_IDENTITY_TYPE_EARPIECE)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
-	ret = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_ALPHA_ID, 0,
+	command->destructor = destroy_play_tone;
+
+	return parse_dataobj(iter, STK_DATA_OBJECT_TYPE_ALPHA_ID, 0,
 				&obj->alpha_id,
 				STK_DATA_OBJECT_TYPE_TONE, 0,
 				&obj->tone,
@@ -2512,36 +2517,24 @@ static gboolean parse_play_tone(struct stk_command *command,
 				STK_DATA_OBJECT_TYPE_FRAME_ID, 0,
 				&obj->frame_id,
 				STK_DATA_OBJECT_TYPE_INVALID);
-
-	command->destructor = destroy_play_tone;
-
-	if (ret == FALSE)
-		return FALSE;
-
-	return TRUE;
 }
 
-static gboolean parse_poll_interval(struct stk_command *command,
+static enum stk_command_parse_result parse_poll_interval(
+					struct stk_command *command,
 					struct comprehension_tlv_iter *iter)
 {
 	struct stk_command_poll_interval *obj = &command->poll_interval;
-	gboolean ret;
 
 	if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
 	if (command->dst != STK_DEVICE_IDENTITY_TYPE_TERMINAL)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
-	ret = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_DURATION,
+	return parse_dataobj(iter, STK_DATA_OBJECT_TYPE_DURATION,
 				DATAOBJ_FLAG_MANDATORY | DATAOBJ_FLAG_MINIMUM,
 				&obj->duration,
 				STK_DATA_OBJECT_TYPE_INVALID);
-
-	if (ret == FALSE)
-		return FALSE;
-
-	return TRUE;
 }
 
 static void destroy_setup_menu(struct stk_command *command)
@@ -2552,21 +2545,21 @@ static void destroy_setup_menu(struct stk_command *command)
 	g_slist_free(command->setup_menu.items);
 }
 
-static gboolean parse_setup_menu(struct stk_command *command,
+static enum stk_command_parse_result parse_setup_menu(
+					struct stk_command *command,
 					struct comprehension_tlv_iter *iter)
 {
 	struct stk_command_setup_menu *obj = &command->setup_menu;
-	gboolean ret;
 
 	if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
 	if (command->dst != STK_DEVICE_IDENTITY_TYPE_TERMINAL)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
 	command->destructor = destroy_setup_menu;
 
-	ret = parse_dataobj(iter,
+	return parse_dataobj(iter,
 			STK_DATA_OBJECT_TYPE_ALPHA_ID,
 			DATAOBJ_FLAG_MANDATORY | DATAOBJ_FLAG_MINIMUM,
 			&obj->alpha_id,
@@ -2584,11 +2577,6 @@ static gboolean parse_setup_menu(struct stk_command *command,
 			STK_DATA_OBJECT_TYPE_ITEM_TEXT_ATTRIBUTE_LIST, 0,
 			&obj->item_text_attr_list,
 			STK_DATA_OBJECT_TYPE_INVALID);
-
-	if (ret == FALSE)
-		return FALSE;
-
-	return TRUE;
 }
 
 static void destroy_select_item(struct stk_command *command)
@@ -2599,21 +2587,20 @@ static void destroy_select_item(struct stk_command *command)
 	g_slist_free(command->select_item.items);
 }
 
-static gboolean parse_select_item(struct stk_command *command,
+static enum stk_command_parse_result parse_select_item(
+					struct stk_command *command,
 					struct comprehension_tlv_iter *iter)
 {
 	struct stk_command_select_item *obj = &command->select_item;
-	gboolean ret;
+	enum stk_command_parse_result status;
 
 	if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
 	if (command->dst != STK_DEVICE_IDENTITY_TYPE_TERMINAL)
-		return FALSE;
-
-	command->destructor = destroy_select_item;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
-	ret = parse_dataobj(iter,
+	status = parse_dataobj(iter,
 			STK_DATA_OBJECT_TYPE_ALPHA_ID,
 			DATAOBJ_FLAG_MANDATORY | DATAOBJ_FLAG_MINIMUM,
 			&obj->alpha_id,
@@ -2636,13 +2623,12 @@ static gboolean parse_select_item(struct stk_command *command,
 			&obj->frame_id,
 			STK_DATA_OBJECT_TYPE_INVALID);
 
-	if (ret == FALSE)
-		return FALSE;
+	command->destructor = destroy_select_item;
 
-	if (obj->items == NULL)
-		return FALSE;
+	if (status == STK_PARSE_RESULT_OK && obj->items == NULL)
+		status = STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
-	return TRUE;
+	return status;
 }
 
 static void destroy_send_sms(struct stk_command *command)
@@ -2652,21 +2638,22 @@ static void destroy_send_sms(struct stk_command *command)
 	g_free(command->send_sms.cdma_sms.array);
 }
 
-static gboolean parse_send_sms(struct stk_command *command,
+static enum stk_command_parse_result parse_send_sms(
+					struct stk_command *command,
 					struct comprehension_tlv_iter *iter)
 {
 	struct stk_command_send_sms *obj = &command->send_sms;
+	enum stk_command_parse_result status;
 	struct gsm_sms_tpdu gsm_tpdu;
-	gboolean ret;
 
 	if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
 	if (command->dst != STK_DEVICE_IDENTITY_TYPE_NETWORK)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
 	memset(&gsm_tpdu, 0, sizeof(gsm_tpdu));
-	ret = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_ALPHA_ID, 0,
+	status = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_ALPHA_ID, 0,
 				&obj->alpha_id,
 				STK_DATA_OBJECT_TYPE_ADDRESS, 0,
 				&obj->address,
@@ -2684,33 +2671,36 @@ static gboolean parse_send_sms(struct stk_command *command,
 
 	command->destructor = destroy_send_sms;
 
-	if (ret == FALSE)
-		return FALSE;
+	if (status != STK_PARSE_RESULT_OK)
+		return status;
 
 	if (gsm_tpdu.len == 0 && obj->cdma_sms.len == 0)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
 	if (gsm_tpdu.len > 0 && obj->cdma_sms.len > 0)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
 	/* We don't process CDMA pdus for now */
 	if (obj->cdma_sms.len > 0)
-		return TRUE;
+		return STK_PARSE_RESULT_OK;
 
 	/* packing is needed */
-	if (command->qualifier & 0x01)
-		return sms_decode_unpacked_stk_pdu(gsm_tpdu.tpdu, gsm_tpdu.len,
-							&obj->gsm_sms);
+	if (command->qualifier & 0x01) {
+		if (sms_decode_unpacked_stk_pdu(gsm_tpdu.tpdu, gsm_tpdu.len,
+							&obj->gsm_sms) != TRUE)
+			status = STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
+		return status;
+	}
 
 	if (sms_decode(gsm_tpdu.tpdu, gsm_tpdu.len, TRUE,
 				gsm_tpdu.len, &obj->gsm_sms) == FALSE)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
 	if (obj->gsm_sms.type != SMS_TYPE_SUBMIT &&
 			obj->gsm_sms.type != SMS_TYPE_COMMAND)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
-	return TRUE;
+	return STK_PARSE_RESULT_OK;
 }
 
 static void destroy_send_ss(struct stk_command *command)
@@ -2719,19 +2709,20 @@ static void destroy_send_ss(struct stk_command *command)
 	g_free(command->send_ss.ss.ss);
 }
 
-static gboolean parse_send_ss(struct stk_command *command,
+static enum stk_command_parse_result parse_send_ss(struct stk_command *command,
 					struct comprehension_tlv_iter *iter)
 {
 	struct stk_command_send_ss *obj = &command->send_ss;
-	gboolean ret;
 
 	if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
 	if (command->dst != STK_DEVICE_IDENTITY_TYPE_NETWORK)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
+
+	command->destructor = destroy_send_ss;
 
-	ret = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_ALPHA_ID, 0,
+	return parse_dataobj(iter, STK_DATA_OBJECT_TYPE_ALPHA_ID, 0,
 				&obj->alpha_id,
 				STK_DATA_OBJECT_TYPE_SS_STRING,
 				DATAOBJ_FLAG_MANDATORY | DATAOBJ_FLAG_MINIMUM,
@@ -2743,13 +2734,6 @@ static gboolean parse_send_ss(struct stk_command *command,
 				STK_DATA_OBJECT_TYPE_FRAME_ID, 0,
 				&obj->frame_id,
 				STK_DATA_OBJECT_TYPE_INVALID);
-
-	command->destructor = destroy_send_ss;
-
-	if (ret == FALSE)
-		return FALSE;
-
-	return TRUE;
 }
 
 static void destroy_send_ussd(struct stk_command *command)
@@ -2757,19 +2741,21 @@ static void destroy_send_ussd(struct stk_command *command)
 	g_free(command->send_ussd.alpha_id);
 }
 
-static gboolean parse_send_ussd(struct stk_command *command,
+static enum stk_command_parse_result parse_send_ussd(
+					struct stk_command *command,
 					struct comprehension_tlv_iter *iter)
 {
 	struct stk_command_send_ussd *obj = &command->send_ussd;
-	gboolean ret;
 
 	if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
 	if (command->dst != STK_DEVICE_IDENTITY_TYPE_NETWORK)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
+
+	command->destructor = destroy_send_ussd;
 
-	ret = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_ALPHA_ID, 0,
+	return parse_dataobj(iter, STK_DATA_OBJECT_TYPE_ALPHA_ID, 0,
 				&obj->alpha_id,
 				STK_DATA_OBJECT_TYPE_USSD_STRING,
 				DATAOBJ_FLAG_MANDATORY | DATAOBJ_FLAG_MINIMUM,
@@ -2781,13 +2767,6 @@ static gboolean parse_send_ussd(struct stk_command *command,
 				STK_DATA_OBJECT_TYPE_FRAME_ID, 0,
 				&obj->frame_id,
 				STK_DATA_OBJECT_TYPE_INVALID);
-
-	command->destructor = destroy_send_ussd;
-
-	if (ret == FALSE)
-		return FALSE;
-
-	return TRUE;
 }
 
 static void destroy_setup_call(struct stk_command *command)
@@ -2797,19 +2776,21 @@ static void destroy_setup_call(struct stk_command *command)
 	g_free(command->setup_call.alpha_id_call_setup);
 }
 
-static gboolean parse_setup_call(struct stk_command *command,
+static enum stk_command_parse_result parse_setup_call(
+					struct stk_command *command,
 					struct comprehension_tlv_iter *iter)
 {
 	struct stk_command_setup_call *obj = &command->setup_call;
-	gboolean ret;
 
 	if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
 	if (command->dst != STK_DEVICE_IDENTITY_TYPE_NETWORK)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
+
+	command->destructor = destroy_setup_call;
 
-	ret = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_ALPHA_ID, 0,
+	return parse_dataobj(iter, STK_DATA_OBJECT_TYPE_ALPHA_ID, 0,
 				&obj->alpha_id_usr_cfm,
 				STK_DATA_OBJECT_TYPE_ADDRESS,
 				DATAOBJ_FLAG_MANDATORY | DATAOBJ_FLAG_MINIMUM,
@@ -2833,13 +2814,6 @@ static gboolean parse_setup_call(struct stk_command *command,
 				STK_DATA_OBJECT_TYPE_FRAME_ID, 0,
 				&obj->frame_id,
 				STK_DATA_OBJECT_TYPE_INVALID);
-
-	command->destructor = destroy_setup_call;
-
-	if (ret == FALSE)
-		return FALSE;
-
-	return TRUE;
 }
 
 static void destroy_refresh(struct stk_command *command)
@@ -2849,19 +2823,21 @@ static void destroy_refresh(struct stk_command *command)
 	g_free(command->refresh.alpha_id);
 }
 
-static gboolean parse_refresh(struct stk_command *command,
+static enum stk_command_parse_result parse_refresh(
+					struct stk_command *command,
 					struct comprehension_tlv_iter *iter)
 {
 	struct stk_command_refresh *obj = &command->refresh;
-	gboolean ret;
 
 	if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
 	if (command->dst != STK_DEVICE_IDENTITY_TYPE_TERMINAL)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
-	ret = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_FILE_LIST, 0,
+	command->destructor = destroy_refresh;
+
+	return parse_dataobj(iter, STK_DATA_OBJECT_TYPE_FILE_LIST, 0,
 				&obj->file_list,
 				STK_DATA_OBJECT_TYPE_AID, 0,
 				&obj->aid,
@@ -2874,159 +2850,142 @@ static gboolean parse_refresh(struct stk_command *command,
 				STK_DATA_OBJECT_TYPE_FRAME_ID, 0,
 				&obj->frame_id,
 				STK_DATA_OBJECT_TYPE_INVALID);
-
-	command->destructor = destroy_refresh;
-
-	if (ret == FALSE)
-		return FALSE;
-
-	return TRUE;
 }
 
-static gboolean parse_polling_off(struct stk_command *command,
+static enum stk_command_parse_result parse_polling_off(
+					struct stk_command *command,
 					struct comprehension_tlv_iter *iter)
 {
 	if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
 	if (command->dst != STK_DEVICE_IDENTITY_TYPE_TERMINAL)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
-	return TRUE;
+	return STK_PARSE_RESULT_OK;
 }
 
-static gboolean parse_provide_local_info(struct stk_command *command,
+static enum stk_command_parse_result parse_provide_local_info(
+					struct stk_command *command,
 					struct comprehension_tlv_iter *iter)
 {
 	if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
 	if (command->dst != STK_DEVICE_IDENTITY_TYPE_TERMINAL)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
-	return TRUE;
+	return STK_PARSE_RESULT_OK;
 }
 
-static gboolean parse_setup_event_list(struct stk_command *command,
+static enum stk_command_parse_result parse_setup_event_list(
+					struct stk_command *command,
 					struct comprehension_tlv_iter *iter)
 {
 	struct stk_command_setup_event_list *obj = &command->setup_event_list;
-	gboolean ret;
 
 	if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
 	if (command->dst != STK_DEVICE_IDENTITY_TYPE_TERMINAL)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
-	ret = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_EVENT_LIST,
+	return parse_dataobj(iter, STK_DATA_OBJECT_TYPE_EVENT_LIST,
 				DATAOBJ_FLAG_MANDATORY | DATAOBJ_FLAG_MINIMUM,
 				&obj->event_list,
 				STK_DATA_OBJECT_TYPE_INVALID);
-
-	if (ret == FALSE)
-		return FALSE;
-
-	return TRUE;
 }
 
-static gboolean parse_perform_card_apdu(struct stk_command *command,
+static enum stk_command_parse_result parse_perform_card_apdu(
+					struct stk_command *command,
 					struct comprehension_tlv_iter *iter)
 {
 	struct stk_command_perform_card_apdu *obj = &command->perform_card_apdu;
-	gboolean ret;
 
 	if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
 	if ((command->dst < STK_DEVICE_IDENTITY_TYPE_CARD_READER_0) ||
 			(command->dst > STK_DEVICE_IDENTITY_TYPE_CARD_READER_7))
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
-	ret = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_C_APDU,
+	return parse_dataobj(iter, STK_DATA_OBJECT_TYPE_C_APDU,
 				DATAOBJ_FLAG_MANDATORY | DATAOBJ_FLAG_MINIMUM,
 				&obj->c_apdu,
 				STK_DATA_OBJECT_TYPE_INVALID);
-
-	if (ret == FALSE)
-		return FALSE;
-
-	return TRUE;
 }
 
-static gboolean parse_power_off_card(struct stk_command *command,
+static enum stk_command_parse_result parse_power_off_card(
+					struct stk_command *command,
 					struct comprehension_tlv_iter *iter)
 {
 	if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
 	if ((command->dst < STK_DEVICE_IDENTITY_TYPE_CARD_READER_0) ||
 			(command->dst > STK_DEVICE_IDENTITY_TYPE_CARD_READER_7))
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
-	return TRUE;
+	return STK_PARSE_RESULT_OK;
 }
 
-static gboolean parse_power_on_card(struct stk_command *command,
+static enum stk_command_parse_result parse_power_on_card(
+					struct stk_command *command,
 					struct comprehension_tlv_iter *iter)
 {
 	if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
 	if ((command->dst < STK_DEVICE_IDENTITY_TYPE_CARD_READER_0) ||
 			(command->dst > STK_DEVICE_IDENTITY_TYPE_CARD_READER_7))
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
-	return TRUE;
+	return STK_PARSE_RESULT_OK;
 }
 
-static gboolean parse_get_reader_status(struct stk_command *command,
+static enum stk_command_parse_result parse_get_reader_status(
+					struct stk_command *command,
 					struct comprehension_tlv_iter *iter)
 {
 	if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
 	switch (command->qualifier) {
 	case STK_QUALIFIER_TYPE_CARD_READER_STATUS:
 		if (command->dst != STK_DEVICE_IDENTITY_TYPE_TERMINAL)
-			return FALSE;
+			return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 		break;
 	case STK_QUALIFIER_TYPE_CARD_READER_ID:
 		if ((command->dst < STK_DEVICE_IDENTITY_TYPE_CARD_READER_0) ||
 				(command->dst >
 					STK_DEVICE_IDENTITY_TYPE_CARD_READER_7))
-			return FALSE;
+			return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 		break;
 	default:
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 	}
 
-	return TRUE;
+	return STK_PARSE_RESULT_OK;
 }
 
-static gboolean parse_timer_mgmt(struct stk_command *command,
+static enum stk_command_parse_result parse_timer_mgmt(
+					struct stk_command *command,
 					struct comprehension_tlv_iter *iter)
 {
 	struct stk_command_timer_mgmt *obj = &command->timer_mgmt;
-	gboolean ret;
 
 	if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
 	if (command->dst != STK_DEVICE_IDENTITY_TYPE_TERMINAL)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
-	ret = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_TIMER_ID,
+	return parse_dataobj(iter, STK_DATA_OBJECT_TYPE_TIMER_ID,
 				DATAOBJ_FLAG_MANDATORY | DATAOBJ_FLAG_MINIMUM,
 				&obj->timer_id,
 				STK_DATA_OBJECT_TYPE_TIMER_VALUE, 0,
 				&obj->timer_value,
 				STK_DATA_OBJECT_TYPE_INVALID);
-
-	if (ret == FALSE)
-		return FALSE;
-
-	return TRUE;
 }
 
 static void destroy_setup_idle_mode_text(struct stk_command *command)
@@ -3034,20 +2993,22 @@ static void destroy_setup_idle_mode_text(struct stk_command *command)
 	g_free(command->setup_idle_mode_text.text);
 }
 
-static gboolean parse_setup_idle_mode_text(struct stk_command *command,
+static enum stk_command_parse_result parse_setup_idle_mode_text(
+					struct stk_command *command,
 					struct comprehension_tlv_iter *iter)
 {
 	struct stk_command_setup_idle_mode_text *obj =
 					&command->setup_idle_mode_text;
-	gboolean ret;
 
 	if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
 	if (command->dst != STK_DEVICE_IDENTITY_TYPE_TERMINAL)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
+
+	command->destructor = destroy_setup_idle_mode_text;
 
-	ret = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_TEXT,
+	return parse_dataobj(iter, STK_DATA_OBJECT_TYPE_TEXT,
 				DATAOBJ_FLAG_MANDATORY | DATAOBJ_FLAG_MINIMUM,
 				&obj->text,
 				STK_DATA_OBJECT_TYPE_ICON_ID, 0,
@@ -3057,13 +3018,6 @@ static gboolean parse_setup_idle_mode_text(struct stk_command *command,
 				STK_DATA_OBJECT_TYPE_FRAME_ID, 0,
 				&obj->frame_id,
 				STK_DATA_OBJECT_TYPE_INVALID);
-
-	command->destructor = destroy_setup_idle_mode_text;
-
-	if (ret == FALSE)
-		return FALSE;
-
-	return TRUE;
 }
 
 static void destroy_run_at_command(struct stk_command *command)
@@ -3072,19 +3026,21 @@ static void destroy_run_at_command(struct stk_command *command)
 	g_free(command->run_at_command.at_command);
 }
 
-static gboolean parse_run_at_command(struct stk_command *command,
+static enum stk_command_parse_result parse_run_at_command(
+					struct stk_command *command,
 					struct comprehension_tlv_iter *iter)
 {
 	struct stk_command_run_at_command *obj = &command->run_at_command;
-	gboolean ret;
 
 	if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
 	if (command->dst != STK_DEVICE_IDENTITY_TYPE_TERMINAL)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
+
+	command->destructor = destroy_run_at_command;
 
-	ret = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_ALPHA_ID, 0,
+	return parse_dataobj(iter, STK_DATA_OBJECT_TYPE_ALPHA_ID, 0,
 				&obj->alpha_id,
 				STK_DATA_OBJECT_TYPE_AT_COMMAND,
 				DATAOBJ_FLAG_MANDATORY | DATAOBJ_FLAG_MINIMUM,
@@ -3096,13 +3052,6 @@ static gboolean parse_run_at_command(struct stk_command *command,
 				STK_DATA_OBJECT_TYPE_FRAME_ID, 0,
 				&obj->frame_id,
 				STK_DATA_OBJECT_TYPE_INVALID);
-
-	command->destructor = destroy_run_at_command;
-
-	if (ret == FALSE)
-		return FALSE;
-
-	return TRUE;
 }
 
 static void destroy_send_dtmf(struct stk_command *command)
@@ -3111,19 +3060,21 @@ static void destroy_send_dtmf(struct stk_command *command)
 	g_free(command->send_dtmf.dtmf);
 }
 
-static gboolean parse_send_dtmf(struct stk_command *command,
+static enum stk_command_parse_result parse_send_dtmf(
+					struct stk_command *command,
 					struct comprehension_tlv_iter *iter)
 {
 	struct stk_command_send_dtmf *obj = &command->send_dtmf;
-	gboolean ret;
 
 	if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
 	if (command->dst != STK_DEVICE_IDENTITY_TYPE_NETWORK)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
+
+	command->destructor = destroy_send_dtmf;
 
-	ret = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_ALPHA_ID, 0,
+	return parse_dataobj(iter, STK_DATA_OBJECT_TYPE_ALPHA_ID, 0,
 				&obj->alpha_id,
 				STK_DATA_OBJECT_TYPE_DTMF_STRING,
 				DATAOBJ_FLAG_MANDATORY | DATAOBJ_FLAG_MINIMUM,
@@ -3135,36 +3086,24 @@ static gboolean parse_send_dtmf(struct stk_command *command,
 				STK_DATA_OBJECT_TYPE_FRAME_ID, 0,
 				&obj->frame_id,
 				STK_DATA_OBJECT_TYPE_INVALID);
-
-	command->destructor = destroy_send_dtmf;
-
-	if (ret == FALSE)
-		return FALSE;
-
-	return TRUE;
 }
 
-static gboolean parse_language_notification(struct stk_command *command,
+static enum stk_command_parse_result parse_language_notification(
+					struct stk_command *command,
 					struct comprehension_tlv_iter *iter)
 {
 	struct stk_command_language_notification *obj =
 					&command->language_notification;
-	gboolean ret;
 
 	if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
 	if (command->dst != STK_DEVICE_IDENTITY_TYPE_TERMINAL)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
-	ret = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_LANGUAGE, 0,
+	return parse_dataobj(iter, STK_DATA_OBJECT_TYPE_LANGUAGE, 0,
 				&obj->language,
 				STK_DATA_OBJECT_TYPE_INVALID);
-
-	if (ret == FALSE)
-		return FALSE;
-
-	return TRUE;
 }
 
 static void destroy_launch_browser(struct stk_command *command)
@@ -3181,19 +3120,21 @@ static void destroy_launch_browser(struct stk_command *command)
 	g_free(command->launch_browser.text_passwd);
 }
 
-static gboolean parse_launch_browser(struct stk_command *command,
+static enum stk_command_parse_result parse_launch_browser(
+					struct stk_command *command,
 					struct comprehension_tlv_iter *iter)
 {
 	struct stk_command_launch_browser *obj = &command->launch_browser;
-	gboolean ret;
 
 	if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
 	if (command->dst != STK_DEVICE_IDENTITY_TYPE_TERMINAL)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
-	ret = parse_dataobj(iter,
+	command->destructor = destroy_launch_browser;
+
+	return parse_dataobj(iter,
 				STK_DATA_OBJECT_TYPE_BROWSER_ID, 0,
 				&obj->browser_id,
 				STK_DATA_OBJECT_TYPE_URL,
@@ -3221,13 +3162,6 @@ static gboolean parse_launch_browser(struct stk_command *command,
 				STK_DATA_OBJECT_TYPE_TEXT, 0,
 				&obj->text_passwd,
 				STK_DATA_OBJECT_TYPE_INVALID);
-
-	command->destructor = destroy_launch_browser;
-
-	if (ret == FALSE)
-		return FALSE;
-
-	return TRUE;
 }
 
 /* TODO: parse_open_channel */
@@ -3237,19 +3171,21 @@ static void destroy_close_channel(struct stk_command *command)
 	g_free(command->close_channel.alpha_id);
 }
 
-static gboolean parse_close_channel(struct stk_command *command,
+static enum stk_command_parse_result parse_close_channel(
+					struct stk_command *command,
 					struct comprehension_tlv_iter *iter)
 {
 	struct stk_command_close_channel *obj = &command->close_channel;
-	gboolean ret;
 
 	if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
 	if (command->dst != STK_DEVICE_IDENTITY_TYPE_TERMINAL)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
-	ret = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_ALPHA_ID, 0,
+	command->destructor = destroy_close_channel;
+
+	return parse_dataobj(iter, STK_DATA_OBJECT_TYPE_ALPHA_ID, 0,
 				&obj->alpha_id,
 				STK_DATA_OBJECT_TYPE_ICON_ID, 0,
 				&obj->icon_id,
@@ -3258,13 +3194,6 @@ static gboolean parse_close_channel(struct stk_command *command,
 				STK_DATA_OBJECT_TYPE_FRAME_ID, 0,
 				&obj->frame_id,
 				STK_DATA_OBJECT_TYPE_INVALID);
-
-	command->destructor = destroy_close_channel;
-
-	if (ret == FALSE)
-		return FALSE;
-
-	return TRUE;
 }
 
 static void destroy_receive_data(struct stk_command *command)
@@ -3272,20 +3201,22 @@ static void destroy_receive_data(struct stk_command *command)
 	g_free(command->receive_data.alpha_id);
 }
 
-static gboolean parse_receive_data(struct stk_command *command,
+static enum stk_command_parse_result parse_receive_data(
+					struct stk_command *command,
 					struct comprehension_tlv_iter *iter)
 {
 	struct stk_command_receive_data *obj = &command->receive_data;
-	gboolean ret;
 
 	if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
 	if ((command->dst < STK_DEVICE_IDENTITY_TYPE_CHANNEL_1) ||
 			(command->dst > STK_DEVICE_IDENTITY_TYPE_CHANNEL_7))
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
+
+	command->destructor = destroy_receive_data;
 
-	ret = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_ALPHA_ID, 0,
+	return parse_dataobj(iter, STK_DATA_OBJECT_TYPE_ALPHA_ID, 0,
 				&obj->alpha_id,
 				STK_DATA_OBJECT_TYPE_ICON_ID, 0,
 				&obj->icon_id,
@@ -3297,13 +3228,6 @@ static gboolean parse_receive_data(struct stk_command *command,
 				STK_DATA_OBJECT_TYPE_FRAME_ID, 0,
 				&obj->frame_id,
 				STK_DATA_OBJECT_TYPE_INVALID);
-
-	command->destructor = destroy_receive_data;
-
-	if (ret == FALSE)
-		return FALSE;
-
-	return TRUE;
 }
 
 static void destroy_send_data(struct stk_command *command)
@@ -3312,20 +3236,22 @@ static void destroy_send_data(struct stk_command *command)
 	g_free(command->send_data.data.array);
 }
 
-static gboolean parse_send_data(struct stk_command *command,
+static enum stk_command_parse_result parse_send_data(
+					struct stk_command *command,
 					struct comprehension_tlv_iter *iter)
 {
 	struct stk_command_send_data *obj = &command->send_data;
-	gboolean ret;
 
 	if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
 	if ((command->dst < STK_DEVICE_IDENTITY_TYPE_CHANNEL_1) ||
 			(command->dst > STK_DEVICE_IDENTITY_TYPE_CHANNEL_7))
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
+
+	command->destructor = destroy_send_data;
 
-	ret = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_ALPHA_ID, 0,
+	return parse_dataobj(iter, STK_DATA_OBJECT_TYPE_ALPHA_ID, 0,
 				&obj->alpha_id,
 				STK_DATA_OBJECT_TYPE_ICON_ID, 0,
 				&obj->icon_id,
@@ -3337,25 +3263,19 @@ static gboolean parse_send_data(struct stk_command *command,
 				STK_DATA_OBJECT_TYPE_FRAME_ID, 0,
 				&obj->frame_id,
 				STK_DATA_OBJECT_TYPE_INVALID);
-
-	command->destructor = destroy_send_data;
-
-	if (ret == FALSE)
-		return FALSE;
-
-	return TRUE;
 }
 
-static gboolean parse_get_channel_status(struct stk_command *command,
+static enum stk_command_parse_result parse_get_channel_status(
+					struct stk_command *command,
 					struct comprehension_tlv_iter *iter)
 {
 	if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
 	if (command->dst != STK_DEVICE_IDENTITY_TYPE_TERMINAL)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
-	return TRUE;
+	return STK_PARSE_RESULT_OK;
 }
 
 static void destroy_service_search(struct stk_command *command)
@@ -3365,19 +3285,21 @@ static void destroy_service_search(struct stk_command *command)
 	g_free(command->service_search.dev_filter.dev_filter);
 }
 
-static gboolean parse_service_search(struct stk_command *command,
+static enum stk_command_parse_result parse_service_search(
+					struct stk_command *command,
 					struct comprehension_tlv_iter *iter)
 {
 	struct stk_command_service_search *obj = &command->service_search;
-	gboolean ret;
 
 	if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
 	if (command->dst != STK_DEVICE_IDENTITY_TYPE_TERMINAL)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
-	ret = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_ALPHA_ID, 0,
+	command->destructor = destroy_service_search;
+
+	return parse_dataobj(iter, STK_DATA_OBJECT_TYPE_ALPHA_ID, 0,
 				&obj->alpha_id,
 				STK_DATA_OBJECT_TYPE_ICON_ID, 0,
 				&obj->icon_id,
@@ -3391,13 +3313,6 @@ static gboolean parse_service_search(struct stk_command *command,
 				STK_DATA_OBJECT_TYPE_FRAME_ID, 0,
 				&obj->frame_id,
 				STK_DATA_OBJECT_TYPE_INVALID);
-
-	command->destructor = destroy_service_search;
-
-	if (ret == FALSE)
-		return FALSE;
-
-	return TRUE;
 }
 
 static void destroy_get_service_info(struct stk_command *command)
@@ -3406,19 +3321,21 @@ static void destroy_get_service_info(struct stk_command *command)
 	g_free(command->get_service_info.attr_info.attr_info);
 }
 
-static gboolean parse_get_service_info(struct stk_command *command,
+static enum stk_command_parse_result parse_get_service_info(
+					struct stk_command *command,
 					struct comprehension_tlv_iter *iter)
 {
 	struct stk_command_get_service_info *obj = &command->get_service_info;
-	gboolean ret;
 
 	if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
 	if (command->dst != STK_DEVICE_IDENTITY_TYPE_TERMINAL)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
-	ret = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_ALPHA_ID, 0,
+	command->destructor = destroy_get_service_info;
+
+	return parse_dataobj(iter, STK_DATA_OBJECT_TYPE_ALPHA_ID, 0,
 				&obj->alpha_id,
 				STK_DATA_OBJECT_TYPE_ICON_ID, 0,
 				&obj->icon_id,
@@ -3430,13 +3347,6 @@ static gboolean parse_get_service_info(struct stk_command *command,
 				STK_DATA_OBJECT_TYPE_FRAME_ID, 0,
 				&obj->frame_id,
 				STK_DATA_OBJECT_TYPE_INVALID);
-
-	command->destructor = destroy_get_service_info;
-
-	if (ret == FALSE)
-		return FALSE;
-
-	return TRUE;
 }
 
 static void destroy_declare_service(struct stk_command *command)
@@ -3444,46 +3354,41 @@ static void destroy_declare_service(struct stk_command *command)
 	g_free(command->declare_service.serv_rec.serv_rec);
 }
 
-static gboolean parse_declare_service(struct stk_command *command,
+static enum stk_command_parse_result parse_declare_service(
+					struct stk_command *command,
 					struct comprehension_tlv_iter *iter)
 {
 	struct stk_command_declare_service *obj = &command->declare_service;
-	gboolean ret;
 
 	if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
 	if (command->dst != STK_DEVICE_IDENTITY_TYPE_TERMINAL)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
+
+	command->destructor = destroy_declare_service;
 
-	ret = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_SERVICE_RECORD,
+	return parse_dataobj(iter, STK_DATA_OBJECT_TYPE_SERVICE_RECORD,
 				DATAOBJ_FLAG_MANDATORY | DATAOBJ_FLAG_MINIMUM,
 				&obj->serv_rec,
 				STK_DATA_OBJECT_TYPE_UICC_TE_INTERFACE, 0,
 				&obj->intf,
 				STK_DATA_OBJECT_TYPE_INVALID);
-
-	command->destructor = destroy_declare_service;
-
-	if (ret == FALSE)
-		return FALSE;
-
-	return TRUE;
 }
 
-static gboolean parse_set_frames(struct stk_command *command,
+static enum stk_command_parse_result parse_set_frames(
+					struct stk_command *command,
 					struct comprehension_tlv_iter *iter)
 {
 	struct stk_command_set_frames *obj = &command->set_frames;
-	gboolean ret;
 
 	if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
 	if (command->dst != STK_DEVICE_IDENTITY_TYPE_TERMINAL)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
-	ret = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_FRAME_ID,
+	return parse_dataobj(iter, STK_DATA_OBJECT_TYPE_FRAME_ID,
 				DATAOBJ_FLAG_MANDATORY | DATAOBJ_FLAG_MINIMUM,
 				&obj->frame_id,
 				STK_DATA_OBJECT_TYPE_FRAME_LAYOUT, 0,
@@ -3491,23 +3396,19 @@ static gboolean parse_set_frames(struct stk_command *command,
 				STK_DATA_OBJECT_TYPE_FRAME_ID, 0,
 				&obj->frame_id_default,
 				STK_DATA_OBJECT_TYPE_INVALID);
-
-	if (ret == FALSE)
-		return FALSE;
-
-	return TRUE;
 }
 
-static gboolean parse_get_frames_status(struct stk_command *command,
+static enum stk_command_parse_result parse_get_frames_status(
+					struct stk_command *command,
 					struct comprehension_tlv_iter *iter)
 {
 	if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
 	if (command->dst != STK_DEVICE_IDENTITY_TYPE_TERMINAL)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
-	return TRUE;
+	return STK_PARSE_RESULT_OK;
 }
 
 static void destroy_retrieve_mms(struct stk_command *command)
@@ -3518,19 +3419,21 @@ static void destroy_retrieve_mms(struct stk_command *command)
 	g_slist_free(command->retrieve_mms.mms_rec_files);
 }
 
-static gboolean parse_retrieve_mms(struct stk_command *command,
+static enum stk_command_parse_result parse_retrieve_mms(
+					struct stk_command *command,
 					struct comprehension_tlv_iter *iter)
 {
 	struct stk_command_retrieve_mms *obj = &command->retrieve_mms;
-	gboolean ret;
 
 	if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
 	if (command->dst != STK_DEVICE_IDENTITY_TYPE_NETWORK)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
+
+	command->destructor = destroy_retrieve_mms;
 
-	ret = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_ALPHA_ID, 0,
+	return parse_dataobj(iter, STK_DATA_OBJECT_TYPE_ALPHA_ID, 0,
 				&obj->alpha_id,
 				STK_DATA_OBJECT_TYPE_ICON_ID, 0,
 				&obj->icon_id,
@@ -3550,13 +3453,6 @@ static gboolean parse_retrieve_mms(struct stk_command *command,
 				STK_DATA_OBJECT_TYPE_FRAME_ID, 0,
 				&obj->frame_id,
 				STK_DATA_OBJECT_TYPE_INVALID);
-
-	command->destructor = destroy_retrieve_mms;
-
-	if (ret == FALSE)
-		return FALSE;
-
-	return TRUE;
 }
 
 static void destroy_submit_mms(struct stk_command *command)
@@ -3567,19 +3463,21 @@ static void destroy_submit_mms(struct stk_command *command)
 	g_slist_free(command->submit_mms.mms_subm_files);
 }
 
-static gboolean parse_submit_mms(struct stk_command *command,
+static enum stk_command_parse_result parse_submit_mms(
+					struct stk_command *command,
 					struct comprehension_tlv_iter *iter)
 {
 	struct stk_command_submit_mms *obj = &command->submit_mms;
-	gboolean ret;
 
 	if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
 	if (command->dst != STK_DEVICE_IDENTITY_TYPE_NETWORK)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
-	ret = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_ALPHA_ID, 0,
+	command->destructor = destroy_submit_mms;
+
+	return parse_dataobj(iter, STK_DATA_OBJECT_TYPE_ALPHA_ID, 0,
 				&obj->alpha_id,
 				STK_DATA_OBJECT_TYPE_ICON_ID, 0,
 				&obj->icon_id,
@@ -3593,13 +3491,6 @@ static gboolean parse_submit_mms(struct stk_command *command,
 				STK_DATA_OBJECT_TYPE_FRAME_ID, 0,
 				&obj->frame_id,
 				STK_DATA_OBJECT_TYPE_INVALID);
-
-	command->destructor = destroy_submit_mms;
-
-	if (ret == FALSE)
-		return FALSE;
-
-	return TRUE;
 }
 
 static void destroy_display_mms(struct stk_command *command)
@@ -3609,19 +3500,21 @@ static void destroy_display_mms(struct stk_command *command)
 	g_slist_free(command->display_mms.mms_subm_files);
 }
 
-static gboolean parse_display_mms(struct stk_command *command,
+static enum stk_command_parse_result parse_display_mms(
+					struct stk_command *command,
 					struct comprehension_tlv_iter *iter)
 {
 	struct stk_command_display_mms *obj = &command->display_mms;
-	gboolean ret;
 
 	if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
 	if (command->dst != STK_DEVICE_IDENTITY_TYPE_TERMINAL)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
-	ret = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_FILE_LIST,
+	command->destructor = destroy_display_mms;
+
+	return parse_dataobj(iter, STK_DATA_OBJECT_TYPE_FILE_LIST,
 				DATAOBJ_FLAG_MANDATORY | DATAOBJ_FLAG_MINIMUM,
 				&obj->mms_subm_files,
 				STK_DATA_OBJECT_TYPE_MMS_ID,
@@ -3632,36 +3525,112 @@ static gboolean parse_display_mms(struct stk_command *command,
 				STK_DATA_OBJECT_TYPE_FRAME_ID, 0,
 				&obj->frame_id,
 				STK_DATA_OBJECT_TYPE_INVALID);
-
-	command->destructor = destroy_display_mms;
-
-	if (ret == FALSE)
-		return FALSE;
-
-	return TRUE;
 }
 
-static gboolean parse_activate(struct stk_command *command,
+static enum stk_command_parse_result parse_activate(
+					struct stk_command *command,
 					struct comprehension_tlv_iter *iter)
 {
 	struct stk_command_activate *obj = &command->activate;
-	gboolean ret;
 
 	if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
 	if (command->dst != STK_DEVICE_IDENTITY_TYPE_TERMINAL)
-		return FALSE;
+		return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
 
-	ret = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_ACTIVATE_DESCRIPTOR,
+	return parse_dataobj(iter, STK_DATA_OBJECT_TYPE_ACTIVATE_DESCRIPTOR,
 				DATAOBJ_FLAG_MANDATORY | DATAOBJ_FLAG_MINIMUM,
 				&obj->actv_desc,
 				STK_DATA_OBJECT_TYPE_INVALID);
+}
 
-	if (ret == FALSE)
-		return FALSE;
-
-	return TRUE;
+static enum stk_command_parse_result parse_command_body(
+					struct stk_command *command,
+					struct comprehension_tlv_iter *iter)
+{
+	switch (command->type) {
+	case STK_COMMAND_TYPE_DISPLAY_TEXT:
+		return parse_display_text(command, iter);
+	case STK_COMMAND_TYPE_GET_INKEY:
+		return parse_get_inkey(command, iter);
+	case STK_COMMAND_TYPE_GET_INPUT:
+		return parse_get_input(command, iter);
+	case STK_COMMAND_TYPE_MORE_TIME:
+		return parse_more_time(command, iter);
+	case STK_COMMAND_TYPE_PLAY_TONE:
+		return parse_play_tone(command, iter);
+	case STK_COMMAND_TYPE_POLL_INTERVAL:
+		return parse_poll_interval(command, iter);
+	case STK_COMMAND_TYPE_SETUP_MENU:
+		return parse_setup_menu(command, iter);
+	case STK_COMMAND_TYPE_SELECT_ITEM:
+		return parse_select_item(command, iter);
+	case STK_COMMAND_TYPE_SEND_SMS:
+		return parse_send_sms(command, iter);
+	case STK_COMMAND_TYPE_SEND_SS:
+		return parse_send_ss(command, iter);
+	case STK_COMMAND_TYPE_SEND_USSD:
+		return parse_send_ussd(command, iter);
+	case STK_COMMAND_TYPE_SETUP_CALL:
+		return parse_setup_call(command, iter);
+	case STK_COMMAND_TYPE_REFRESH:
+		return parse_refresh(command, iter);
+	case STK_COMMAND_TYPE_POLLING_OFF:
+		return parse_polling_off(command, iter);
+	case STK_COMMAND_TYPE_PROVIDE_LOCAL_INFO:
+		return parse_provide_local_info(command, iter);
+	case STK_COMMAND_TYPE_SETUP_EVENT_LIST:
+		return parse_setup_event_list(command, iter);
+	case STK_COMMAND_TYPE_PERFORM_CARD_APDU:
+		return parse_perform_card_apdu(command, iter);
+	case STK_COMMAND_TYPE_POWER_OFF_CARD:
+		return parse_power_off_card(command, iter);
+	case STK_COMMAND_TYPE_POWER_ON_CARD:
+		return parse_power_on_card(command, iter);
+	case STK_COMMAND_TYPE_GET_READER_STATUS:
+		return parse_get_reader_status(command, iter);
+	case STK_COMMAND_TYPE_TIMER_MANAGEMENT:
+		return parse_timer_mgmt(command, iter);
+	case STK_COMMAND_TYPE_SETUP_IDLE_MODE_TEXT:
+		return parse_setup_idle_mode_text(command, iter);
+	case STK_COMMAND_TYPE_RUN_AT_COMMAND:
+		return parse_run_at_command(command, iter);
+	case STK_COMMAND_TYPE_SEND_DTMF:
+		return parse_send_dtmf(command, iter);
+	case STK_COMMAND_TYPE_LANGUAGE_NOTIFICATION:
+		return parse_language_notification(command, iter);
+	case STK_COMMAND_TYPE_LAUNCH_BROWSER:
+		return parse_launch_browser(command, iter);
+	case STK_COMMAND_TYPE_CLOSE_CHANNEL:
+		return parse_close_channel(command, iter);
+	case STK_COMMAND_TYPE_RECEIVE_DATA:
+		return parse_receive_data(command, iter);
+	case STK_COMMAND_TYPE_SEND_DATA:
+		return parse_send_data(command, iter);
+	case STK_COMMAND_TYPE_GET_CHANNEL_STATUS:
+		return parse_get_channel_status(command, iter);
+	case STK_COMMAND_TYPE_SERVICE_SEARCH:
+		return parse_service_search(command, iter);
+	case STK_COMMAND_TYPE_GET_SERVICE_INFO:
+		return parse_get_service_info(command, iter);
+	case STK_COMMAND_TYPE_DECLARE_SERVICE:
+		return parse_declare_service(command, iter);
+	case STK_COMMAND_TYPE_SET_FRAMES:
+		return parse_set_frames(command, iter);
+	case STK_COMMAND_TYPE_GET_FRAMES_STATUS:
+		return parse_get_frames_status(command, iter);
+	case STK_COMMAND_TYPE_RETRIEVE_MMS:
+		return parse_retrieve_mms(command, iter);
+	case STK_COMMAND_TYPE_SUBMIT_MMS:
+		return parse_submit_mms(command, iter);
+	case STK_COMMAND_TYPE_DISPLAY_MMS:
+		return parse_display_mms(command, iter);
+	case STK_COMMAND_TYPE_ACTIVATE:
+		return parse_activate(command, iter);
+	default:
+		return STK_PARSE_RESULT_TYPE_NOT_UNDERSTOOD;
+	};
 }
 
 struct stk_command *stk_command_new_from_pdu(const unsigned char *pdu,
@@ -3671,7 +3640,6 @@ struct stk_command *stk_command_new_from_pdu(const unsigned char *pdu,
 	struct comprehension_tlv_iter iter;
 	const unsigned char *data;
 	struct stk_command *command;
-	gboolean ok;
 
 	ber_tlv_iter_init(&ber, pdu, len);
 
@@ -3706,154 +3674,31 @@ struct stk_command *stk_command_new_from_pdu(const unsigned char *pdu,
 	command->type = data[1];
 	command->qualifier = data[2];
 
-	if (comprehension_tlv_iter_next(&iter) != TRUE)
-		goto fail;
+	if (comprehension_tlv_iter_next(&iter) != TRUE) {
+		command->status = STK_PARSE_RESULT_MISSING_VALUE;
+		goto out;
+	}
 
 	if (comprehension_tlv_iter_get_tag(&iter) !=
-			STK_DATA_OBJECT_TYPE_DEVICE_IDENTITIES)
-		goto fail;
+			STK_DATA_OBJECT_TYPE_DEVICE_IDENTITIES) {
+		command->status = STK_PARSE_RESULT_MISSING_VALUE;
+		goto out;
+	}
 
-	if (comprehension_tlv_iter_get_length(&iter) != 0x02)
-		goto fail;
+	if (comprehension_tlv_iter_get_length(&iter) != 0x02) {
+		command->status = STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
+		goto out;
+	}
 
 	data = comprehension_tlv_iter_get_data(&iter);
 
 	command->src = data[0];
 	command->dst = data[1];
 
-	switch (command->type) {
-	case STK_COMMAND_TYPE_DISPLAY_TEXT:
-		ok = parse_display_text(command, &iter);
-		break;
-	case STK_COMMAND_TYPE_GET_INKEY:
-		ok = parse_get_inkey(command, &iter);
-		break;
-	case STK_COMMAND_TYPE_GET_INPUT:
-		ok = parse_get_input(command, &iter);
-		break;
-	case STK_COMMAND_TYPE_MORE_TIME:
-		ok = parse_more_time(command, &iter);
-		break;
-	case STK_COMMAND_TYPE_PLAY_TONE:
-		ok = parse_play_tone(command, &iter);
-		break;
-	case STK_COMMAND_TYPE_POLL_INTERVAL:
-		ok = parse_poll_interval(command, &iter);
-		break;
-	case STK_COMMAND_TYPE_SETUP_MENU:
-		ok = parse_setup_menu(command, &iter);
-		break;
-	case STK_COMMAND_TYPE_SELECT_ITEM:
-		ok = parse_select_item(command, &iter);
-		break;
-	case STK_COMMAND_TYPE_SEND_SMS:
-		ok = parse_send_sms(command, &iter);
-		break;
-	case STK_COMMAND_TYPE_SEND_SS:
-		ok = parse_send_ss(command, &iter);
-		break;
-	case STK_COMMAND_TYPE_SEND_USSD:
-		ok = parse_send_ussd(command, &iter);
-		break;
-	case STK_COMMAND_TYPE_SETUP_CALL:
-		ok = parse_setup_call(command, &iter);
-		break;
-	case STK_COMMAND_TYPE_REFRESH:
-		ok = parse_refresh(command, &iter);
-		break;
-	case STK_COMMAND_TYPE_POLLING_OFF:
-		ok = parse_polling_off(command, &iter);
-		break;
-	case STK_COMMAND_TYPE_PROVIDE_LOCAL_INFO:
-		ok = parse_provide_local_info(command, &iter);
-		break;
-	case STK_COMMAND_TYPE_SETUP_EVENT_LIST:
-		ok = parse_setup_event_list(command, &iter);
-		break;
-	case STK_COMMAND_TYPE_PERFORM_CARD_APDU:
-		ok = parse_perform_card_apdu(command, &iter);
-		break;
-	case STK_COMMAND_TYPE_POWER_OFF_CARD:
-		ok = parse_power_off_card(command, &iter);
-		break;
-	case STK_COMMAND_TYPE_POWER_ON_CARD:
-		ok = parse_power_on_card(command, &iter);
-		break;
-	case STK_COMMAND_TYPE_GET_READER_STATUS:
-		ok = parse_get_reader_status(command, &iter);
-		break;
-	case STK_COMMAND_TYPE_TIMER_MANAGEMENT:
-		ok = parse_timer_mgmt(command, &iter);
-		break;
-	case STK_COMMAND_TYPE_SETUP_IDLE_MODE_TEXT:
-		ok = parse_setup_idle_mode_text(command, &iter);
-		break;
-	case STK_COMMAND_TYPE_RUN_AT_COMMAND:
-		ok = parse_run_at_command(command, &iter);
-		break;
-	case STK_COMMAND_TYPE_SEND_DTMF:
-		ok = parse_send_dtmf(command, &iter);
-		break;
-	case STK_COMMAND_TYPE_LANGUAGE_NOTIFICATION:
-		ok = parse_language_notification(command, &iter);
-		break;
-	case STK_COMMAND_TYPE_LAUNCH_BROWSER:
-		ok = parse_launch_browser(command, &iter);
-		break;
-	case STK_COMMAND_TYPE_CLOSE_CHANNEL:
-		ok = parse_close_channel(command, &iter);
-		break;
-	case STK_COMMAND_TYPE_RECEIVE_DATA:
-		ok = parse_receive_data(command, &iter);
-		break;
-	case STK_COMMAND_TYPE_SEND_DATA:
-		ok = parse_send_data(command, &iter);
-		break;
-	case STK_COMMAND_TYPE_GET_CHANNEL_STATUS:
-		ok = parse_get_channel_status(command, &iter);
-		break;
-	case STK_COMMAND_TYPE_SERVICE_SEARCH:
-		ok = parse_service_search(command, &iter);
-		break;
-	case STK_COMMAND_TYPE_GET_SERVICE_INFO:
-		ok = parse_get_service_info(command, &iter);
-		break;
-	case STK_COMMAND_TYPE_DECLARE_SERVICE:
-		ok = parse_declare_service(command, &iter);
-		break;
-	case STK_COMMAND_TYPE_SET_FRAMES:
-		ok = parse_set_frames(command, &iter);
-		break;
-	case STK_COMMAND_TYPE_GET_FRAMES_STATUS:
-		ok = parse_get_frames_status(command, &iter);
-		break;
-	case STK_COMMAND_TYPE_RETRIEVE_MMS:
-		ok = parse_retrieve_mms(command, &iter);
-		break;
-	case STK_COMMAND_TYPE_SUBMIT_MMS:
-		ok = parse_submit_mms(command, &iter);
-		break;
-	case STK_COMMAND_TYPE_DISPLAY_MMS:
-		ok = parse_display_mms(command, &iter);
-		break;
-	case STK_COMMAND_TYPE_ACTIVATE:
-		ok = parse_activate(command, &iter);
-		break;
-	default:
-		ok = FALSE;
-		break;
-	};
-
-	if (ok)
-		return command;
-
-fail:
-	if (command->destructor)
-		command->destructor(command);
+	command->status = parse_command_body(command, &iter);
 
-	g_free(command);
-
-	return NULL;
+out:
+	return command;
 }
 
 void stk_command_free(struct stk_command *command)
diff --git a/src/stkutil.h b/src/stkutil.h
index ea0f77a..ca4817e 100644
--- a/src/stkutil.h
+++ b/src/stkutil.h
@@ -1302,12 +1302,20 @@ struct stk_command_activate {
 	unsigned char actv_desc;
 };
 
+enum stk_command_parse_result {
+	STK_PARSE_RESULT_OK,
+	STK_PARSE_RESULT_TYPE_NOT_UNDERSTOOD,
+	STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD,
+	STK_PARSE_RESULT_MISSING_VALUE,
+};
+
 struct stk_command {
 	unsigned char number;
 	unsigned char type;
 	unsigned char qualifier;
 	enum stk_device_identity_type src;
 	enum stk_device_identity_type dst;
+	enum stk_command_parse_result status;
 
 	union {
 		struct stk_command_display_text display_text;
diff --git a/unit/test-stkutil.c b/unit/test-stkutil.c
index c586a7b..8b7e254 100644
--- a/unit/test-stkutil.c
+++ b/unit/test-stkutil.c
@@ -731,6 +731,7 @@ static void test_display_text(gconstpointer data)
 	command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
 
 	g_assert(command);
+	g_assert(command->status == STK_PARSE_RESULT_OK);
 
 	g_assert(command->number == 1);
 	g_assert(command->type == STK_COMMAND_TYPE_DISPLAY_TEXT);
@@ -1668,6 +1669,7 @@ static void test_get_inkey(gconstpointer data)
 	command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
 
 	g_assert(command);
+	g_assert(command->status == STK_PARSE_RESULT_OK);
 
 	g_assert(command->number == 1);
 	g_assert(command->type == STK_COMMAND_TYPE_GET_INKEY);
@@ -2973,6 +2975,7 @@ static void test_get_input(gconstpointer data)
 	command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
 
 	g_assert(command);
+	g_assert(command->status == STK_PARSE_RESULT_OK);
 
 	g_assert(command->number == 1);
 	g_assert(command->type == STK_COMMAND_TYPE_GET_INPUT);
@@ -3018,6 +3021,7 @@ static void test_more_time(gconstpointer data)
 	command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
 
 	g_assert(command);
+	g_assert(command->status == STK_PARSE_RESULT_OK);
 
 	g_assert(command->number == 1);
 	g_assert(command->type == STK_COMMAND_TYPE_MORE_TIME);
@@ -4253,6 +4257,7 @@ static void test_play_tone(gconstpointer data)
 	command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
 
 	g_assert(command);
+	g_assert(command->status == STK_PARSE_RESULT_OK);
 
 	g_assert(command->number == 1);
 	g_assert(command->type == STK_COMMAND_TYPE_PLAY_TONE);
@@ -4301,6 +4306,7 @@ static void test_poll_interval(gconstpointer data)
 	command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
 
 	g_assert(command);
+	g_assert(command->status == STK_PARSE_RESULT_OK);
 
 	g_assert(command->number == 1);
 	g_assert(command->type == STK_COMMAND_TYPE_POLL_INTERVAL);
@@ -5522,6 +5528,7 @@ static void test_setup_menu(gconstpointer data)
 	command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
 
 	g_assert(command);
+	g_assert(command->status == STK_PARSE_RESULT_OK);
 
 	g_assert(command->number == 1);
 	g_assert(command->type == STK_COMMAND_TYPE_SETUP_MENU);
@@ -5546,6 +5553,17 @@ static void test_setup_menu(gconstpointer data)
 	stk_command_free(command);
 }
 
+static void test_setup_menu_missing_val(gconstpointer data)
+{
+	const struct setup_menu_test *test = data;
+	struct stk_command *command;
+
+	command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
+
+	g_assert(command);
+	g_assert(command->status == STK_PARSE_RESULT_MISSING_VALUE);
+}
+
 static void test_setup_menu_neg(gconstpointer data)
 {
 	const struct setup_menu_test *test = data;
@@ -5553,7 +5571,8 @@ static void test_setup_menu_neg(gconstpointer data)
 
 	command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
 
-	g_assert(!command);
+	g_assert(command);
+	g_assert(command->status == STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD);
 }
 
 struct select_item_test {
@@ -7106,6 +7125,7 @@ static void test_select_item(gconstpointer data)
 	command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
 
 	g_assert(command);
+	g_assert(command->status == STK_PARSE_RESULT_OK);
 
 	g_assert(command->number == 1);
 	g_assert(command->type == STK_COMMAND_TYPE_SELECT_ITEM);
@@ -8730,6 +8750,7 @@ static void test_send_sms(gconstpointer data)
 	command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
 
 	g_assert(command);
+	g_assert(command->status == STK_PARSE_RESULT_OK);
 
 	g_assert(command->number == 1);
 	g_assert(command->type == STK_COMMAND_TYPE_SEND_SMS);
@@ -9703,6 +9724,7 @@ static void test_send_ss(gconstpointer data)
 	command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
 
 	g_assert(command);
+	g_assert(command->status == STK_PARSE_RESULT_OK);
 
 	g_assert(command->number == 1);
 	g_assert(command->type == STK_COMMAND_TYPE_SEND_SS);
@@ -10935,6 +10957,7 @@ static void test_send_ussd(gconstpointer data)
 	command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
 
 	g_assert(command);
+	g_assert(command->status == STK_PARSE_RESULT_OK);
 
 	g_assert(command->number == 1);
 	g_assert(command->type == STK_COMMAND_TYPE_SEND_USSD);
@@ -12201,6 +12224,7 @@ static void test_setup_call(gconstpointer data)
 	command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
 
 	g_assert(command);
+	g_assert(command->status == STK_PARSE_RESULT_OK);
 
 	g_assert(command->number == 1);
 	g_assert(command->type == STK_COMMAND_TYPE_SETUP_CALL);
@@ -12275,6 +12299,7 @@ static void test_refresh(gconstpointer data)
 	command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
 
 	g_assert(command);
+	g_assert(command->status == STK_PARSE_RESULT_OK);
 
 	g_assert(command->number == 1);
 	g_assert(command->type == STK_COMMAND_TYPE_REFRESH);
@@ -12316,6 +12341,7 @@ static void test_polling_off(gconstpointer data)
 	command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
 
 	g_assert(command);
+	g_assert(command->status == STK_PARSE_RESULT_OK);
 
 	g_assert(command->number == 1);
 	g_assert(command->type == STK_COMMAND_TYPE_POLLING_OFF);
@@ -12401,6 +12427,7 @@ static void test_provide_local_info(gconstpointer data)
 	command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
 
 	g_assert(command);
+	g_assert(command->status == STK_PARSE_RESULT_OK);
 
 	g_assert(command->number == 1);
 	g_assert(command->type == STK_COMMAND_TYPE_PROVIDE_LOCAL_INFO);
@@ -12508,6 +12535,7 @@ static void test_setup_event_list(gconstpointer data)
 	command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
 
 	g_assert(command);
+	g_assert(command->status == STK_PARSE_RESULT_OK);
 
 	g_assert(command->number == 1);
 	g_assert(command->type == STK_COMMAND_TYPE_SETUP_EVENT_LIST);
@@ -12733,6 +12761,7 @@ static void test_perform_card_apdu(gconstpointer data)
 	command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
 
 	g_assert(command);
+	g_assert(command->status == STK_PARSE_RESULT_OK);
 
 	g_assert(command->number == 1);
 	g_assert(command->type == STK_COMMAND_TYPE_PERFORM_CARD_APDU);
@@ -12770,6 +12799,7 @@ static void test_get_reader_status(gconstpointer data)
 	command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
 
 	g_assert(command);
+	g_assert(command->status == STK_PARSE_RESULT_OK);
 
 	g_assert(command->number == 1);
 	g_assert(command->type == STK_COMMAND_TYPE_GET_READER_STATUS);
@@ -13290,6 +13320,7 @@ static void test_timer_mgmt(gconstpointer data)
 	command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
 
 	g_assert(command);
+	g_assert(command->status == STK_PARSE_RESULT_OK);
 
 	g_assert(command->number == 1);
 	g_assert(command->type == STK_COMMAND_TYPE_TIMER_MANAGEMENT);
@@ -14004,6 +14035,7 @@ static void test_setup_idle_mode_text(gconstpointer data)
 	command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
 
 	g_assert(command);
+	g_assert(command->status == STK_PARSE_RESULT_OK);
 
 	g_assert(command->number == 1);
 	g_assert(command->type == STK_COMMAND_TYPE_SETUP_IDLE_MODE_TEXT);
@@ -14758,6 +14790,7 @@ static void test_run_at_command(gconstpointer data)
 	command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
 
 	g_assert(command);
+	g_assert(command->status == STK_PARSE_RESULT_OK);
 
 	g_assert(command->number == 1);
 	g_assert(command->type == STK_COMMAND_TYPE_RUN_AT_COMMAND);
@@ -15423,6 +15456,7 @@ static void test_send_dtmf(gconstpointer data)
 	command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
 
 	g_assert(command);
+	g_assert(command->status == STK_PARSE_RESULT_OK);
 
 	g_assert(command->number == 1);
 	g_assert(command->type == STK_COMMAND_TYPE_SEND_DTMF);
@@ -15477,6 +15511,7 @@ static void test_language_notification(gconstpointer data)
 	command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
 
 	g_assert(command);
+	g_assert(command->status == STK_PARSE_RESULT_OK);
 
 	g_assert(command->number == 1);
 	g_assert(command->type == STK_COMMAND_TYPE_LANGUAGE_NOTIFICATION);
@@ -16135,6 +16170,7 @@ static void test_launch_browser(gconstpointer data)
 	command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
 
 	g_assert(command);
+	g_assert(command->status == STK_PARSE_RESULT_OK);
 
 	g_assert(command->number == 1);
 	g_assert(command->type == STK_COMMAND_TYPE_LAUNCH_BROWSER);
@@ -22494,7 +22530,7 @@ int main(int argc, char **argv)
 				&setup_menu_data_913, test_setup_menu);
 
 	g_test_add_data_func("/teststk/Setup Menu Negative 1",
-			&setup_menu_data_neg_1, test_setup_menu_neg);
+			&setup_menu_data_neg_1, test_setup_menu_missing_val);
 	g_test_add_data_func("/teststk/Setup Menu Negative 2",
 			&setup_menu_data_neg_2, test_setup_menu_neg);
 	g_test_add_data_func("/teststk/Setup Menu Negative 3",
-- 
1.7.1.86.g0e460.dirty


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/8] stk: Allow registering proactive command handlers.
  2010-06-22 11:20 [PATCH 1/8] stkutil: Refactor command parser error handling Andrzej Zaborowski
@ 2010-06-22 11:20 ` Andrzej Zaborowski
  2010-06-23 19:44   ` Denis Kenzior
  2010-06-22 11:20 ` [PATCH 3/8] stk: Handle the More Time command as a nop Andrzej Zaborowski
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 12+ messages in thread
From: Andrzej Zaborowski @ 2010-06-22 11:20 UTC (permalink / raw)
  To: ofono

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

Synchronous handlers can fill in the struct stk_respone *rsp
parameter and return TRUE. Async handlers can ignore that parameter
and return FALSE.  They have to call stk_respond() at some point.
---
 src/stk.c |  151 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-------
 1 files changed, 135 insertions(+), 16 deletions(-)

diff --git a/src/stk.c b/src/stk.c
index b5c6919..0d7fc33 100644
--- a/src/stk.c
+++ b/src/stk.c
@@ -39,14 +39,80 @@
 
 static GSList *g_drivers = NULL;
 
+typedef void (*generic_cb_t)(const struct ofono_error *error,
+				struct ofono_stk *stk);
+
+typedef void (*envelope_cb_t)(const struct ofono_error *error,
+				const unsigned char *data,
+				int length, struct ofono_stk *stk);
+
+typedef gboolean (*command_handler_t)(const struct stk_command *cmd,
+					struct stk_response *rsp,
+					struct ofono_stk *stk);
+
 struct ofono_stk {
 	const struct ofono_stk_driver *driver;
 	void *driver_data;
 	struct ofono_atom *atom;
+	command_handler_t handlers[STK_COMMAND_TYPE_END_SESSION + 1];
+	struct stk_command *pending_cmd;
 };
 
+static void stk_respond(struct ofono_stk *stk,
+			struct stk_response *rsp, generic_cb_t cb)
+{
+	struct ofono_error error = { .type = OFONO_ERROR_TYPE_FAILURE };
+	const guint8 *tlv;
+	unsigned int tlv_len;
+
+	rsp->src = STK_DEVICE_IDENTITY_TYPE_TERMINAL;
+	rsp->dst = STK_DEVICE_IDENTITY_TYPE_UICC;
+	rsp->number = stk->pending_cmd->number;
+	rsp->type = stk->pending_cmd->type;
+	rsp->qualifier = stk->pending_cmd->qualifier;
+
+	if (stk->driver->terminal_response == NULL) {
+		cb(&error, stk);
+		return;
+	}
+
+	tlv = stk_pdu_from_response(rsp, &tlv_len);
+	if (!tlv) {
+		cb(&error, stk);
+		return;
+	}
+
+	stk->driver->terminal_response(stk, tlv_len, tlv,
+					(ofono_stk_generic_cb_t) cb, stk);
+}
+
+static void stk_send_envelope(struct ofono_stk *stk, struct stk_envelope *e,
+				envelope_cb_t cb)
+{
+	struct ofono_error error = { .type = OFONO_ERROR_TYPE_FAILURE };
+	const guint8 *tlv;
+	unsigned int tlv_len;
+
+	e->dst = STK_DEVICE_IDENTITY_TYPE_UICC;
+
+	if (stk->driver->envelope == NULL) {
+		cb(&error, NULL, -1, stk);
+		return;
+	}
+
+	tlv = stk_pdu_from_envelope(e, &tlv_len);
+	if (!tlv) {
+		cb(&error, NULL, -1, stk);
+		return;
+	}
+
+	stk->driver->envelope(stk, tlv_len, tlv,
+				(ofono_stk_envelope_cb_t) cb, stk);
+}
+
 static void stk_cbs_download_cb(const struct ofono_error *error,
-				const unsigned char *data, int len, void *user)
+				const unsigned char *data, int len,
+				struct ofono_stk *stk)
 {
 	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
 		ofono_error("CellBroadcast download to UICC failed");
@@ -55,34 +121,46 @@ static void stk_cbs_download_cb(const struct ofono_error *error,
 		return;
 	}
 
+	if (len)
+		ofono_error("CellBroadcast download returned %i bytes of data",
+				len);
+
 	DBG("CellBroadcast download to UICC reported no error");
 }
 
 void __ofono_cbs_sim_download(struct ofono_stk *stk, const struct cbs *msg)
 {
-	const guint8 *tlv;
-	unsigned int tlv_len;
 	struct stk_envelope e;
 
-	if (stk->driver->envelope == NULL)
-		return;
+	memset(&e, 0, sizeof(e));
 
 	e.type = STK_ENVELOPE_TYPE_CBS_PP_DOWNLOAD;
 	e.src = STK_DEVICE_IDENTITY_TYPE_NETWORK;
-	e.dst = STK_DEVICE_IDENTITY_TYPE_UICC;
 	memcpy(&e.cbs_pp_download.page, msg, sizeof(msg));
 
-	tlv = stk_pdu_from_envelope(&e, &tlv_len);
-	if (!tlv)
+	stk_send_envelope(stk, &e, stk_cbs_download_cb);
+}
+
+static void stk_command_cb(const struct ofono_error *error,
+				struct ofono_stk *stk)
+{
+	stk_command_free(stk->pending_cmd);
+	stk->pending_cmd = NULL;
+
+	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
+		ofono_error("TERMINAL RESPONSE to a UICC command failed");
+		/* "The ME may retry to deliver the same Cell Broadcast
+		 * page." */
 		return;
+	}
 
-	stk->driver->envelope(stk, tlv_len, tlv, stk_cbs_download_cb, stk);
+	DBG("TERMINAL RESPONSE to a command reported no errors");
 }
 
 void ofono_stk_proactive_command_notify(struct ofono_stk *stk,
 					int length, const unsigned char *pdu)
 {
-	struct stk_command *cmd;
+	struct stk_response rsp;
 	char *buf;
 	int i;
 
@@ -93,23 +171,64 @@ void ofono_stk_proactive_command_notify(struct ofono_stk *stk,
 	for (i = 0; i < length; i ++)
 		sprintf(buf + i * 2, "%02hhx", pdu[i]);
 
-	cmd = stk_command_new_from_pdu(pdu, length);
-	if (!cmd) {
+	stk->pending_cmd = stk_command_new_from_pdu(pdu, length);
+	if (!stk->pending_cmd) {
 		ofono_error("Can't parse proactive command: %s", buf);
 
-		/* TODO: return TERMINAL RESPONSE with permanent error */
+		/*
+		 * Nothing we can do, we'd need at least Command Details
+		 * to be able to respond with an error.
+		 */
 		goto done;
 	}
 
-	/* TODO: execute */
-	ofono_info("Proactive command PDU: %s", buf);
+	ofono_debug("Proactive command PDU: %s", buf);
+
+	memset(&rsp, 0, sizeof(rsp));
 
-	stk_command_free(cmd);
+	switch (stk->pending_cmd->status) {
+	case STK_PARSE_RESULT_OK:
+		if (stk->handlers[stk->pending_cmd->type]) {
+			if (stk->handlers[stk->pending_cmd->type](
+					stk->pending_cmd, &rsp, stk) == TRUE)
+				break;
+
+			return;
+		}
+		/* Fall through */
+
+	case STK_PARSE_RESULT_TYPE_NOT_UNDERSTOOD:
+	default:
+		rsp.result.type = STK_RESULT_TYPE_COMMAND_NOT_UNDERSTOOD;
+		break;
+
+	case STK_PARSE_RESULT_MISSING_VALUE:
+		rsp.result.type = STK_RESULT_TYPE_MINIMUM_NOT_MET;
+		break;
+
+	case STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD:
+		rsp.result.type = STK_RESULT_TYPE_DATA_NOT_UNDERSTOOD;
+		break;
+	}
+
+	stk_respond(stk, &rsp, stk_command_cb);
 
 done:
 	g_free(buf);
 }
 
+static gboolean stk_command_handler_register(struct ofono_stk *stk,
+						enum stk_command_type type,
+						command_handler_t handler)
+{
+	if (stk->handlers[type])
+		return FALSE;
+
+	stk->handlers[type] = handler;
+
+	return TRUE;
+}
+
 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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/8] stk: Handle the More Time command as a nop.
  2010-06-22 11:20 [PATCH 1/8] stkutil: Refactor command parser error handling Andrzej Zaborowski
  2010-06-22 11:20 ` [PATCH 2/8] stk: Allow registering proactive command handlers Andrzej Zaborowski
@ 2010-06-22 11:20 ` Andrzej Zaborowski
  2010-06-22 11:20 ` [PATCH 4/8] stk: Add SimApplication interface Andrzej Zaborowski
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Andrzej Zaborowski @ 2010-06-22 11:20 UTC (permalink / raw)
  To: ofono

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

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

diff --git a/src/stk.c b/src/stk.c
index 0d7fc33..e3f6253 100644
--- a/src/stk.c
+++ b/src/stk.c
@@ -229,6 +229,15 @@ static gboolean stk_command_handler_register(struct ofono_stk *stk,
 	return TRUE;
 }
 
+static gboolean handle_command_more_time(const struct stk_command *cmd,
+						struct stk_response *rsp,
+						struct ofono_stk *stk)
+{
+	/* Do nothing */
+
+	return TRUE;
+}
+
 int ofono_stk_driver_register(const struct ofono_stk_driver *d)
 {
 	DBG("driver: %p, name: %s", d, d->name);
@@ -299,6 +308,9 @@ struct ofono_stk *ofono_stk_create(struct ofono_modem *modem,
 		break;
 	}
 
+	stk_command_handler_register(stk, STK_COMMAND_TYPE_MORE_TIME,
+					handle_command_more_time);
+
 	return stk;
 }
 
-- 
1.7.1.86.g0e460.dirty


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 4/8] stk: Add SimApplication interface.
  2010-06-22 11:20 [PATCH 1/8] stkutil: Refactor command parser error handling Andrzej Zaborowski
  2010-06-22 11:20 ` [PATCH 2/8] stk: Allow registering proactive command handlers Andrzej Zaborowski
  2010-06-22 11:20 ` [PATCH 3/8] stk: Handle the More Time command as a nop Andrzej Zaborowski
@ 2010-06-22 11:20 ` Andrzej Zaborowski
  2010-06-23 21:25   ` Denis Kenzior
  2010-06-22 11:20 ` [PATCH 5/8] stk: Forget previous command if a new one comes Andrzej Zaborowski
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 12+ messages in thread
From: Andrzej Zaborowski @ 2010-06-22 11:20 UTC (permalink / raw)
  To: ofono

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

---
 include/dbus.h |    1 +
 src/stk.c      |  585 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 585 insertions(+), 1 deletions(-)

diff --git a/include/dbus.h b/include/dbus.h
index d988760..6b03dd7 100644
--- a/include/dbus.h
+++ b/include/dbus.h
@@ -49,6 +49,7 @@ extern "C" {
 #define OFONO_VOICECALL_MANAGER_INTERFACE "org.ofono.VoiceCallManager"
 #define OFONO_DATA_CONNECTION_MANAGER_INTERFACE "org.ofono.DataConnectionManager"
 #define OFONO_DATA_CONTEXT_INTERFACE "org.ofono.PrimaryDataContext"
+#define OFONO_SIM_APP_INTERFACE OFONO_SERVICE ".SimApplication"
 
 /* Essentially a{sv} */
 #define OFONO_PROPERTIES_ARRAY_SIGNATURE DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING \
diff --git a/src/stk.c b/src/stk.c
index e3f6253..3959dc7 100644
--- a/src/stk.c
+++ b/src/stk.c
@@ -34,6 +34,7 @@
 
 #include "ofono.h"
 
+#include "common.h"
 #include "smsutil.h"
 #include "stkutil.h"
 
@@ -50,12 +51,29 @@ typedef gboolean (*command_handler_t)(const struct stk_command *cmd,
 					struct stk_response *rsp,
 					struct ofono_stk *stk);
 
+struct stk_menu {
+	char *title;
+	GSList *items;
+	struct stk_items_next_action_indicator next_action;
+	int default_item;
+	struct stk_text_attribute title_attr;
+	struct stk_item_text_attribute_list item_attrs;
+	gboolean soft_key;
+	gboolean has_help;
+};
+
 struct ofono_stk {
 	const struct ofono_stk_driver *driver;
 	void *driver_data;
 	struct ofono_atom *atom;
 	command_handler_t handlers[STK_COMMAND_TYPE_END_SESSION + 1];
 	struct stk_command *pending_cmd;
+	DBusMessage *pending;
+	gint cmd_timeout;
+
+	struct stk_menu *default_menu;
+	struct stk_menu *current_menu; /* For Select Item menus */
+	int timeout;
 };
 
 static void stk_respond(struct ofono_stk *stk,
@@ -148,7 +166,8 @@ static void stk_command_cb(const struct ofono_error *error,
 	stk->pending_cmd = NULL;
 
 	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
-		ofono_error("TERMINAL RESPONSE to a UICC command failed");
+		ofono_error("TERMINAL RESPONSE to a UICC command failed: %s",
+				telephony_error_to_str(error));
 		/* "The ME may retry to deliver the same Cell Broadcast
 		 * page." */
 		return;
@@ -229,6 +248,469 @@ static gboolean stk_command_handler_register(struct ofono_stk *stk,
 	return TRUE;
 }
 
+static struct stk_menu *stk_menu_create(const char *title,
+		const struct stk_text_attribute *title_attr, GSList *items,
+		const struct stk_item_text_attribute_list *item_attrs,
+		const struct stk_items_next_action_indicator *next_action,
+		int default_id, gboolean soft_key, gboolean has_help)
+{
+	struct stk_menu *ret = g_new(struct stk_menu, 1);
+	GSList *l;
+	int i;
+
+	ret->title = g_strdup(title);
+	ret->items = g_slist_copy((GSList *) items);
+	ret->default_item = -1;
+	memcpy(&ret->next_action, next_action, sizeof(ret->next_action));
+	memcpy(&ret->title_attr, title_attr, sizeof(ret->title_attr));
+	memcpy(&ret->item_attrs, item_attrs, sizeof(ret->item_attrs));
+	ret->soft_key = soft_key;
+	ret->has_help = has_help;
+
+	for (l = ret->items, i = 0; l; l = l->next, i++) {
+		struct stk_item *j = g_memdup(l->data, sizeof(*j));
+
+		j->text = g_strdup(j->text);
+		l->data = j;
+
+		if (j->id == default_id)
+			ret->default_item = i;
+	}
+
+	return ret;
+}
+
+static void stk_menu_free(struct stk_menu *menu)
+{
+	GSList *l;
+
+	for (l = menu->items; l; l = l->next) {
+		struct stk_item *i = l->data;
+
+		g_free(i->text);
+		g_free(i);
+	}
+
+	g_slist_free(menu->items);
+	g_free(menu->title);
+	g_free(menu);
+}
+
+static char **stk_menu_items_strlist(struct stk_menu *menu)
+{
+	char **items = g_new0(char *, g_slist_length(menu->items) + 1);
+	int i;
+	GSList *l;
+
+	for (i = 0, l = menu->items; l; l = l->next, i++) {
+		struct stk_item *item = l->data;
+		items[i] = item->text;
+	}
+
+	return items;
+}
+
+static void emit_menu_changed(struct ofono_stk *stk)
+{
+	static struct stk_menu no_menu = {
+		.title = "",
+		.items = NULL,
+		.has_help = FALSE,
+		.default_item = -1,
+	};
+	DBusConnection *conn = ofono_dbus_get_connection();
+	const char *path = __ofono_atom_get_path(stk->atom);
+	char **items;
+	struct stk_menu *menu = &no_menu;
+	dbus_uint16_t timeout = stk->timeout;
+	dbus_bool_t boolval;
+	uint8_t byteval;
+
+	if (stk->default_menu)
+		menu = stk->default_menu;
+	if (stk->current_menu)
+		menu = stk->current_menu;
+
+	if (stk->cmd_timeout && stk->current_menu)
+		ofono_dbus_signal_property_changed(conn, path,
+						OFONO_SIM_APP_INTERFACE,
+						"Timeout",
+						DBUS_TYPE_UINT16, &timeout);
+
+	items = stk_menu_items_strlist(menu);
+	ofono_dbus_signal_array_property_changed(conn, path,
+						OFONO_SIM_APP_INTERFACE,
+						"MenuItems",
+						DBUS_TYPE_STRING, &items);
+	g_free(items);
+
+	ofono_dbus_signal_property_changed(conn, path,
+						OFONO_SIM_APP_INTERFACE,
+						"MenuTitle",
+						DBUS_TYPE_STRING, &menu->title);
+
+	boolval = menu->has_help;
+	ofono_dbus_signal_property_changed(conn, path,
+						OFONO_SIM_APP_INTERFACE,
+						"HelpAvailable",
+						DBUS_TYPE_BOOLEAN, &boolval);
+
+	boolval = stk->current_menu ? FALSE : TRUE;
+	ofono_dbus_signal_property_changed(conn, path,
+						OFONO_SIM_APP_INTERFACE,
+						"MainMenu",
+						DBUS_TYPE_BOOLEAN, &boolval);
+
+	if (menu->default_item >= 0) {
+		byteval = menu->default_item;
+		ofono_dbus_signal_property_changed(conn, path,
+						OFONO_SIM_APP_INTERFACE,
+						"DefaultSelection",
+						DBUS_TYPE_BYTE, &byteval);
+	}
+}
+
+static gboolean select_item_timeout(gpointer user_data)
+{
+	struct ofono_stk *stk = user_data;
+	struct stk_response rsp;
+
+	stk->cmd_timeout = 0;
+
+	if (stk->pending)
+		/*
+		 * A command is currently executing, do nothing.  The
+		 * command's callback will take care of freeing the
+		 * menu.
+		 */
+		return FALSE;
+
+	ofono_debug("Select Item command timed out");
+
+	memset(&rsp, 0, sizeof(rsp));
+	rsp.result.type = STK_RESULT_TYPE_NO_RESPONSE;
+
+	stk_respond(stk, &rsp, stk_command_cb);
+
+	stk_menu_free(stk->current_menu);
+	stk->current_menu = NULL;
+
+	emit_menu_changed(stk);
+
+	return FALSE;
+}
+
+static DBusMessage *stk_get_properties(DBusConnection *conn,
+					DBusMessage *msg, void *data)
+{
+	struct ofono_stk *stk = data;
+	DBusMessage *reply;
+	DBusMessageIter iter;
+	DBusMessageIter dict;
+	char **items;
+	struct stk_menu *menu = stk->default_menu;
+	dbus_uint16_t timeout = stk->timeout;
+	dbus_bool_t boolval;
+	uint8_t byteval;
+
+	if (stk->current_menu)
+		menu = stk->current_menu;
+
+	reply = dbus_message_new_method_return(msg);
+	if (!reply)
+		return NULL;
+
+	dbus_message_iter_init_append(reply, &iter);
+
+	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
+					OFONO_PROPERTIES_ARRAY_SIGNATURE,
+					&dict);
+
+	if (!menu)
+		goto done;
+
+	if (stk->cmd_timeout && stk->current_menu)
+		ofono_dbus_dict_append(&dict, "Timeout",
+					DBUS_TYPE_UINT16, &timeout);
+
+	items = stk_menu_items_strlist(menu);
+	ofono_dbus_dict_append_array(&dict, "MenuItems",
+					DBUS_TYPE_STRING, &items);
+	g_free(items);
+
+	ofono_dbus_dict_append(&dict, "MenuTitle",
+				DBUS_TYPE_STRING, &menu->title);
+
+	boolval = menu->has_help;
+	ofono_dbus_dict_append(&dict, "HelpAvailable",
+				DBUS_TYPE_BOOLEAN, &boolval);
+
+	boolval = stk->current_menu ? FALSE : TRUE;
+	ofono_dbus_dict_append(&dict, "MainMenu",
+				DBUS_TYPE_BOOLEAN, &boolval);
+
+	if (menu->default_item >= 0) {
+		byteval = menu->default_item;
+		ofono_dbus_dict_append(&dict, "DefaultSelection",
+					DBUS_TYPE_BYTE, &byteval);
+	}
+
+done:
+	dbus_message_iter_close_container(&iter, &dict);
+
+	return reply;
+}
+
+static DBusMessage *stk_set_property(DBusConnection *conn,
+					DBusMessage *msg, void *data)
+{
+	struct ofono_stk *stk = data;
+	const char *path = __ofono_atom_get_path(stk->atom);
+	DBusMessageIter iter;
+	DBusMessageIter var;
+	const char *property;
+	dbus_uint16_t timeout;
+
+	if (!dbus_message_iter_init(msg, &iter))
+		return __ofono_error_invalid_args(msg);
+
+	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
+		return __ofono_error_invalid_args(msg);
+
+	dbus_message_iter_get_basic(&iter, &property);
+	if (!dbus_message_iter_next(&iter))
+		return __ofono_error_invalid_args(msg);
+
+	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_VARIANT)
+		return __ofono_error_invalid_args(msg);
+
+	dbus_message_iter_recurse(&iter, &var);
+
+	if (stk->cmd_timeout && stk->current_menu &&
+			!strcmp(property, "Timeout")) {
+		if (dbus_message_iter_get_arg_type(&var) != DBUS_TYPE_UINT16)
+			return __ofono_error_invalid_args(msg);
+
+		dbus_message_iter_get_basic(&var, &timeout);
+		if (dbus_message_iter_next(&iter))
+			return __ofono_error_invalid_args(msg);
+
+		stk->timeout = timeout;
+
+		g_source_remove(stk->cmd_timeout);
+		stk->cmd_timeout = g_timeout_add_seconds(stk->timeout,
+							select_item_timeout,
+							stk);
+
+		ofono_dbus_signal_property_changed(conn, path,
+						OFONO_SIM_APP_INTERFACE,
+						"Timeout",
+						DBUS_TYPE_UINT16, &timeout);
+
+		return dbus_message_new_method_return(msg);
+	}
+
+	return __ofono_error_invalid_args(msg);
+}
+
+static void select_item_cb(const struct ofono_error *error,
+				struct ofono_stk *stk)
+{
+	DBusMessage *reply;
+
+	if (stk->cmd_timeout) {
+		/*
+		 * If the command has not timed out yet, return error and
+		 * the user may then retry the selection.  Otherwise assume
+		 * the menu is no longer valid.
+		 */
+		if (error->type != OFONO_ERROR_TYPE_NO_ERROR)
+			goto out;
+
+		g_source_remove(stk->cmd_timeout);
+	}
+
+	stk_command_cb(error, stk);
+
+	stk_menu_free(stk->current_menu);
+	stk->current_menu = NULL;
+
+	emit_menu_changed(stk);
+
+out:
+	if (error->type == OFONO_ERROR_TYPE_NO_ERROR)
+		reply = __ofono_error_failed(stk->pending);
+	else
+		reply = dbus_message_new_method_return(stk->pending);
+
+	__ofono_dbus_pending_reply(&stk->pending, reply);
+}
+
+static void select_item_respond(struct ofono_stk *stk, struct stk_item *item,
+				gboolean help)
+{
+	struct stk_response rsp;
+
+	memset(&rsp, 0, sizeof(rsp));
+
+	if (help)
+		rsp.result.type = STK_RESULT_TYPE_HELP_REQUESTED;
+
+	rsp.select_item.item_id = item->id;
+
+	stk_respond(stk, &rsp, select_item_cb);
+}
+
+static void menu_selection_cb(const struct ofono_error *error,
+				const unsigned char *data, int len,
+				struct ofono_stk *stk)
+{
+	DBusMessage *reply;
+
+	if (len)
+		ofono_error("Selecting item returned %i bytes of data", len);
+
+	if (error->type != OFONO_ERROR_TYPE_NO_ERROR)
+		reply = __ofono_error_failed(stk->pending);
+	else
+		reply = dbus_message_new_method_return(stk->pending);
+
+	__ofono_dbus_pending_reply(&stk->pending, reply);
+}
+
+static void send_menu_selection(struct ofono_stk *stk, struct stk_item *item,
+				gboolean help)
+{
+	struct stk_envelope e;
+
+	memset(&e, 0, sizeof(e));
+
+	e.type = STK_ENVELOPE_TYPE_MENU_SELECTION;
+	e.src = STK_DEVICE_IDENTITY_TYPE_KEYPAD,
+	e.menu_selection.item_id = item->id;
+	e.menu_selection.help_request = help;
+
+	stk_send_envelope(stk, &e, menu_selection_cb);
+}
+
+static DBusMessage *stk_menu_select(DBusConnection *conn, DBusMessage *msg,
+					void *data, gboolean help)
+{
+	struct ofono_stk *stk = data;
+	DBusMessageIter iter;
+	uint8_t selection;
+	struct stk_menu *menu;
+	struct stk_item *item;
+
+	if (stk->pending)
+		return __ofono_error_busy(msg);
+
+	if (!dbus_message_iter_init(msg, &iter))
+		return __ofono_error_invalid_args(msg);
+
+	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_BYTE)
+		return __ofono_error_invalid_args(msg);
+
+	dbus_message_iter_get_basic(&iter, &selection);
+	if (dbus_message_iter_next(&iter))
+		return __ofono_error_invalid_args(msg);
+
+	menu = stk->default_menu;
+	if (stk->current_menu)
+		menu = stk->current_menu;
+
+	if (!menu)
+		return __ofono_error_failed(msg);
+
+	if (help == TRUE && !menu->has_help)
+		return __ofono_error_failed(msg);
+
+	item = g_slist_nth_data(menu->items, selection);
+	if (!item)
+		return __ofono_error_invalid_args(msg);
+
+	stk->pending = dbus_message_ref(msg);
+
+	if (stk->current_menu)
+		select_item_respond(stk, item, help);
+	else
+		send_menu_selection(stk, item, help);
+
+	return NULL;
+}
+
+static DBusMessage *stk_select(DBusConnection *conn,
+				DBusMessage *msg, void *data)
+{
+	return stk_menu_select(conn, msg, data, FALSE);
+}
+
+static DBusMessage *stk_get_help(DBusConnection *conn,
+					DBusMessage *msg, void *data)
+{
+	return stk_menu_select(conn, msg, data, TRUE);
+}
+
+static DBusMessage *stk_back_or_terminate(DBusConnection *conn,
+					DBusMessage *msg, void *data,
+					enum stk_result_type type)
+{
+	struct ofono_stk *stk = data;
+	DBusMessageIter iter;
+	struct stk_response rsp;
+
+	if (stk->pending)
+		return __ofono_error_busy(msg);
+
+	if (!stk->current_menu)
+		return __ofono_error_failed(msg);
+
+	if (dbus_message_iter_init(msg, &iter))
+		return __ofono_error_invalid_args(msg);
+
+	stk->pending = dbus_message_ref(msg);
+
+	memset(&rsp, 0, sizeof(rsp));
+	rsp.result.type = type;
+
+	stk_respond(stk, &rsp, select_item_cb);
+
+	return NULL;
+}
+
+static DBusMessage *stk_go_back(DBusConnection *conn,
+				DBusMessage *msg, void *data)
+{
+	return stk_back_or_terminate(conn, msg, data,
+					STK_RESULT_TYPE_GO_BACK);
+}
+
+static DBusMessage *stk_end_session(DBusConnection *conn,
+				DBusMessage *msg, void *data)
+{
+	return stk_back_or_terminate(conn, msg, data,
+					STK_RESULT_TYPE_USER_TERMINATED);
+}
+
+static GDBusMethodTable stk_methods[] = {
+	{ "GetProperties",	"",	"a{sv}",	stk_get_properties },
+	{ "SetProperty",	"sv",	"",		stk_set_property },
+	{ "Select",		"y",	"",		stk_select,
+				G_DBUS_METHOD_FLAG_ASYNC },
+	{ "RequestHelp",	"y",	"",		stk_get_help,
+				G_DBUS_METHOD_FLAG_ASYNC },
+	{ "GoBack",		"",	"",		stk_go_back,
+				G_DBUS_METHOD_FLAG_ASYNC },
+	{ "EndSession",		"",	"",		stk_end_session,
+				G_DBUS_METHOD_FLAG_ASYNC },
+	{ }
+};
+
+static GDBusSignalTable stk_signals[] = {
+	{ "PropertyChanged",	"sv" },
+	{ }
+};
+
 static gboolean handle_command_more_time(const struct stk_command *cmd,
 						struct stk_response *rsp,
 						struct ofono_stk *stk)
@@ -238,6 +720,59 @@ static gboolean handle_command_more_time(const struct stk_command *cmd,
 	return TRUE;
 }
 
+static gboolean handle_command_select_item(const struct stk_command *cmd,
+						struct stk_response *rsp,
+						struct ofono_stk *stk)
+{
+	gboolean soft_key = (cmd->qualifier & (1 << 2)) != 0;
+	gboolean has_help = (cmd->qualifier & (1 << 7)) != 0;
+
+	stk->current_menu = stk_menu_create(cmd->select_item.alpha_id,
+					&cmd->select_item.text_attr,
+					cmd->select_item.items,
+					&cmd->select_item.item_text_attr_list,
+					&cmd->select_item.next_act,
+					cmd->select_item.item_id,
+					soft_key, has_help);
+
+	emit_menu_changed(stk);
+
+	stk->cmd_timeout = g_timeout_add_seconds(stk->timeout,
+							select_item_timeout,
+							stk);
+
+	return FALSE;
+}
+
+static gboolean handle_command_set_up_menu(const struct stk_command *cmd,
+						struct stk_response *rsp,
+						struct ofono_stk *stk)
+{
+	gboolean soft_key = (cmd->qualifier & (1 << 0)) != 0;
+	gboolean has_help = (cmd->qualifier & (1 << 7)) != 0;
+
+	if (stk->default_menu) {
+		stk_menu_free(stk->default_menu);
+		stk->default_menu = NULL;
+	}
+
+	if (cmd->setup_menu.items == NULL)
+		goto out;
+
+	stk->default_menu = stk_menu_create(cmd->setup_menu.alpha_id,
+					&cmd->setup_menu.text_attr,
+					cmd->setup_menu.items,
+					&cmd->setup_menu.item_text_attr_list,
+					&cmd->setup_menu.next_act,
+					0, soft_key, has_help);
+
+out:
+	if (stk->current_menu == NULL)
+		emit_menu_changed(stk);
+
+	return TRUE;
+}
+
 int ofono_stk_driver_register(const struct ofono_stk_driver *d)
 {
 	DBG("driver: %p, name: %s", d, d->name);
@@ -259,6 +794,33 @@ void ofono_stk_driver_unregister(const struct ofono_stk_driver *d)
 
 static void stk_unregister(struct ofono_atom *atom)
 {
+	struct ofono_stk *stk = __ofono_atom_get_data(atom);
+	DBusConnection *conn = ofono_dbus_get_connection();
+	struct ofono_modem *modem = __ofono_atom_get_modem(atom);
+	const char *path = __ofono_atom_get_path(atom);
+
+	if (stk->pending_cmd) {
+		stk_command_free(stk->pending_cmd);
+		stk->pending_cmd = NULL;
+
+		if (stk->cmd_timeout) {
+			g_source_remove(stk->cmd_timeout);
+			stk->cmd_timeout = 0;
+		}
+	}
+
+	if (stk->current_menu) {
+		stk_menu_free(stk->current_menu);
+		stk->current_menu = NULL;
+	}
+
+	if (stk->default_menu) {
+		stk_menu_free(stk->default_menu);
+		stk->default_menu = NULL;
+	}
+
+	ofono_modem_remove_interface(modem, OFONO_SIM_APP_INTERFACE);
+	g_dbus_unregister_interface(conn, path, OFONO_SIM_APP_INTERFACE);
 }
 
 static void stk_remove(struct ofono_atom *atom)
@@ -308,14 +870,35 @@ struct ofono_stk *ofono_stk_create(struct ofono_modem *modem,
 		break;
 	}
 
+	stk->timeout = 20;
+
 	stk_command_handler_register(stk, STK_COMMAND_TYPE_MORE_TIME,
 					handle_command_more_time);
+	stk_command_handler_register(stk, STK_COMMAND_TYPE_SELECT_ITEM,
+					handle_command_select_item);
+	stk_command_handler_register(stk, STK_COMMAND_TYPE_SETUP_MENU,
+					handle_command_set_up_menu);
 
 	return stk;
 }
 
 void ofono_stk_register(struct ofono_stk *stk)
 {
+	DBusConnection *conn = ofono_dbus_get_connection();
+	struct ofono_modem *modem = __ofono_atom_get_modem(stk->atom);
+	const char *path = __ofono_atom_get_path(stk->atom);
+
+	if (!g_dbus_register_interface(conn, path, OFONO_SIM_APP_INTERFACE,
+					stk_methods, stk_signals, NULL,
+					stk, NULL)) {
+		ofono_error("Could not create %s interface",
+				OFONO_SIM_APP_INTERFACE);
+
+		return;
+	}
+
+	ofono_modem_add_interface(modem, OFONO_SIM_APP_INTERFACE);
+
 	__ofono_atom_register(stk->atom, stk_unregister);
 }
 
-- 
1.7.1.86.g0e460.dirty


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 5/8] stk: Forget previous command if a new one comes.
  2010-06-22 11:20 [PATCH 1/8] stkutil: Refactor command parser error handling Andrzej Zaborowski
                   ` (2 preceding siblings ...)
  2010-06-22 11:20 ` [PATCH 4/8] stk: Add SimApplication interface Andrzej Zaborowski
@ 2010-06-22 11:20 ` Andrzej Zaborowski
  2010-06-22 11:20 ` [PATCH 6/8] Add a proactive command cancel notification Andrzej Zaborowski
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Andrzej Zaborowski @ 2010-06-22 11:20 UTC (permalink / raw)
  To: ofono

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

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

diff --git a/src/stk.c b/src/stk.c
index 3959dc7..b43f090 100644
--- a/src/stk.c
+++ b/src/stk.c
@@ -51,6 +51,8 @@ typedef gboolean (*command_handler_t)(const struct stk_command *cmd,
 					struct stk_response *rsp,
 					struct ofono_stk *stk);
 
+typedef void (*command_cancel_t)(struct ofono_stk *stk);
+
 struct stk_menu {
 	char *title;
 	GSList *items;
@@ -70,6 +72,8 @@ struct ofono_stk {
 	struct stk_command *pending_cmd;
 	DBusMessage *pending;
 	gint cmd_timeout;
+	command_cancel_t cancel_cmd;
+	gboolean cancelled; /* Only valid when .pending set */
 
 	struct stk_menu *default_menu;
 	struct stk_menu *current_menu; /* For Select Item menus */
@@ -183,6 +187,19 @@ void ofono_stk_proactive_command_notify(struct ofono_stk *stk,
 	char *buf;
 	int i;
 
+	if (stk->pending_cmd) {
+		/*
+		 * In theory this shouldn't happen, but it depends on
+		 * hardware.  We received a new command before we managed
+		 * to send a TERMINAL RESPONSE to the previous one.  3GPP
+		 * says in the current revision only one command can be
+		 * executing at any time, so assume that the previous one
+		 * is cancelled and the card just expects a response to
+		 * the new one.
+		 */
+		stk->cancel_cmd(stk);
+	}
+
 	buf = g_try_malloc(length * 2 + 1);
 	if (!buf)
 		return;
@@ -513,11 +530,39 @@ static DBusMessage *stk_set_property(DBusConnection *conn,
 	return __ofono_error_invalid_args(msg);
 }
 
+static void select_item_cancel(struct ofono_stk *stk)
+{
+	if (stk->pending)
+		/*
+		 * TODO: D-bus command is just executing.  Should we go
+		 * ahead and respond to it with failure, or wait for the
+		 * result from driver?
+		 */
+		stk->cancelled = TRUE;
+
+	if (stk->cmd_timeout) {
+		g_source_remove(stk->cmd_timeout);
+		stk->cmd_timeout = 0;
+	}
+
+	stk_command_free(stk->pending_cmd);
+	stk->pending_cmd = NULL;
+
+	stk_menu_free(stk->current_menu);
+	stk->current_menu = NULL;
+
+	emit_menu_changed(stk);
+}
+
 static void select_item_cb(const struct ofono_error *error,
 				struct ofono_stk *stk)
 {
 	DBusMessage *reply;
 
+	if (stk->cancelled == TRUE)
+		/* Command cancelled */
+		goto out;
+
 	if (stk->cmd_timeout) {
 		/*
 		 * If the command has not timed out yet, return error and
@@ -629,6 +674,7 @@ static DBusMessage *stk_menu_select(DBusConnection *conn, DBusMessage *msg,
 	if (!item)
 		return __ofono_error_invalid_args(msg);
 
+	stk->cancelled = FALSE;
 	stk->pending = dbus_message_ref(msg);
 
 	if (stk->current_menu)
@@ -668,6 +714,7 @@ static DBusMessage *stk_back_or_terminate(DBusConnection *conn,
 	if (dbus_message_iter_init(msg, &iter))
 		return __ofono_error_invalid_args(msg);
 
+	stk->cancelled = FALSE;
 	stk->pending = dbus_message_ref(msg);
 
 	memset(&rsp, 0, sizeof(rsp));
@@ -740,6 +787,7 @@ static gboolean handle_command_select_item(const struct stk_command *cmd,
 	stk->cmd_timeout = g_timeout_add_seconds(stk->timeout,
 							select_item_timeout,
 							stk);
+	stk->cancel_cmd = select_item_cancel;
 
 	return FALSE;
 }
-- 
1.7.1.86.g0e460.dirty


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 6/8] Add a proactive command cancel notification.
  2010-06-22 11:20 [PATCH 1/8] stkutil: Refactor command parser error handling Andrzej Zaborowski
                   ` (3 preceding siblings ...)
  2010-06-22 11:20 ` [PATCH 5/8] stk: Forget previous command if a new one comes Andrzej Zaborowski
@ 2010-06-22 11:20 ` Andrzej Zaborowski
  2010-06-22 11:20 ` [PATCH 7/8] mbmmodem: Cancel running command on *STKEND Andrzej Zaborowski
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Andrzej Zaborowski @ 2010-06-22 11:20 UTC (permalink / raw)
  To: ofono

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

---
 include/stk.h |    2 ++
 src/stk.c     |    8 ++++++++
 2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/include/stk.h b/include/stk.h
index ad3f6c5..4e5d01e 100644
--- a/include/stk.h
+++ b/include/stk.h
@@ -65,6 +65,8 @@ void *ofono_stk_get_data(struct ofono_stk *stk);
 void ofono_stk_proactive_command_notify(struct ofono_stk *stk,
 					int length, const unsigned char *pdu);
 
+void ofono_stk_proactive_command_cancel(struct ofono_stk *stk);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/src/stk.c b/src/stk.c
index b43f090..b752e49 100644
--- a/src/stk.c
+++ b/src/stk.c
@@ -253,6 +253,14 @@ done:
 	g_free(buf);
 }
 
+void ofono_stk_proactive_command_cancel(struct ofono_stk *stk)
+{
+	if (!stk->pending_cmd)
+		return;
+
+	stk->cancel_cmd(stk);
+}
+
 static gboolean stk_command_handler_register(struct ofono_stk *stk,
 						enum stk_command_type type,
 						command_handler_t handler)
-- 
1.7.1.86.g0e460.dirty


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 7/8] mbmmodem: Cancel running command on *STKEND.
  2010-06-22 11:20 [PATCH 1/8] stkutil: Refactor command parser error handling Andrzej Zaborowski
                   ` (4 preceding siblings ...)
  2010-06-22 11:20 ` [PATCH 6/8] Add a proactive command cancel notification Andrzej Zaborowski
@ 2010-06-22 11:20 ` Andrzej Zaborowski
  2010-06-22 11:20 ` [PATCH 8/8] stk: Handle the Display Text command Andrzej Zaborowski
  2010-06-23 19:36 ` [PATCH 1/8] stkutil: Refactor command parser error handling Denis Kenzior
  7 siblings, 0 replies; 12+ messages in thread
From: Andrzej Zaborowski @ 2010-06-22 11:20 UTC (permalink / raw)
  To: ofono

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

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

diff --git a/drivers/mbmmodem/stk.c b/drivers/mbmmodem/stk.c
index 77bd7b5..53b5adf 100644
--- a/drivers/mbmmodem/stk.c
+++ b/drivers/mbmmodem/stk.c
@@ -179,6 +179,14 @@ static void stkn_notify(GAtResult *result, gpointer user_data)
 
 static void stkend_notify(GAtResult *result, gpointer user_data)
 {
+	struct ofono_stk *stk = user_data;
+
+	/* Seems to be used by the modem and/or cards to indicate
+	 * that we have taken too long to respond to a command, but
+	 * does not invalidate current application state, such as
+	 * main menu (?)
+	 */
+	ofono_stk_proactive_command_cancel(stk);
 }
 
 static void mbm_stkc_cb(gboolean ok, GAtResult *result, gpointer user_data)
-- 
1.7.1.86.g0e460.dirty


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 8/8] stk: Handle the Display Text command.
  2010-06-22 11:20 [PATCH 1/8] stkutil: Refactor command parser error handling Andrzej Zaborowski
                   ` (5 preceding siblings ...)
  2010-06-22 11:20 ` [PATCH 7/8] mbmmodem: Cancel running command on *STKEND Andrzej Zaborowski
@ 2010-06-22 11:20 ` Andrzej Zaborowski
  2010-06-23 19:36 ` [PATCH 1/8] stkutil: Refactor command parser error handling Denis Kenzior
  7 siblings, 0 replies; 12+ messages in thread
From: Andrzej Zaborowski @ 2010-06-22 11:20 UTC (permalink / raw)
  To: ofono

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

---
 src/stk.c |  255 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 244 insertions(+), 11 deletions(-)

diff --git a/src/stk.c b/src/stk.c
index b752e49..cf28a96 100644
--- a/src/stk.c
+++ b/src/stk.c
@@ -77,6 +77,7 @@ struct ofono_stk {
 
 	struct stk_menu *default_menu;
 	struct stk_menu *current_menu; /* For Select Item menus */
+	const char *display_text;
 	int timeout;
 };
 
@@ -355,6 +356,8 @@ static void emit_menu_changed(struct ofono_stk *stk)
 		menu = stk->default_menu;
 	if (stk->current_menu)
 		menu = stk->current_menu;
+	if (stk->display_text)
+		menu = &no_menu;
 
 	if (stk->cmd_timeout && stk->current_menu)
 		ofono_dbus_signal_property_changed(conn, path,
@@ -425,6 +428,78 @@ static gboolean select_item_timeout(gpointer user_data)
 	return FALSE;
 }
 
+static void emit_text_changed(struct ofono_stk *stk)
+{
+	DBusConnection *conn = ofono_dbus_get_connection();
+	const char *path = __ofono_atom_get_path(stk->atom);
+	dbus_uint16_t timeout = stk->timeout;
+	const char *text = stk->display_text ?: "";
+	dbus_bool_t boolval;
+
+	if (stk->cmd_timeout && stk->display_text)
+		ofono_dbus_signal_property_changed(conn, path,
+						OFONO_SIM_APP_INTERFACE,
+						"Timeout",
+						DBUS_TYPE_UINT16, &timeout);
+
+	ofono_dbus_signal_property_changed(conn, path,
+						OFONO_SIM_APP_INTERFACE,
+						"Message", DBUS_TYPE_STRING,
+						&text);
+
+	boolval = stk->display_text &&
+		(stk->pending_cmd->qualifier & (1 << 7)) != 0;
+	ofono_dbus_signal_property_changed(conn, path,
+						OFONO_SIM_APP_INTERFACE,
+						"UserConfirmMessage",
+						DBUS_TYPE_BOOLEAN, &boolval);
+
+	boolval = stk->display_text &&
+		(stk->pending_cmd->qualifier & (1 << 0)) != 0;
+	ofono_dbus_signal_property_changed(conn, path,
+						OFONO_SIM_APP_INTERFACE,
+						"PriorityMessage",
+						DBUS_TYPE_BOOLEAN, &boolval);
+}
+
+static gboolean display_text_timeout(gpointer user_data)
+{
+	struct ofono_stk *stk = user_data;
+	struct stk_response rsp;
+
+	stk->cmd_timeout = 0;
+
+	if (stk->pending)
+		/* A command is currently executing, do nothing.  */
+		return FALSE;
+
+	ofono_debug("Display Text command timed out");
+
+	stk->display_text = NULL;
+
+	if (stk->pending_cmd->display_text.immediate_response == TRUE) {
+		/* Already responded */
+		stk_command_free(stk->pending_cmd);
+		stk->pending_cmd = NULL;
+
+		return FALSE;
+	}
+
+	memset(&rsp, 0, sizeof(rsp));
+	if (stk->pending_cmd->qualifier & (1 << 7))
+		rsp.result.type = STK_RESULT_TYPE_NO_RESPONSE;
+
+	stk_respond(stk, &rsp, stk_command_cb);
+
+	emit_text_changed(stk);
+
+	/* If manu was overridden by text, now re-show it */
+	if (stk->default_menu)
+		emit_menu_changed(stk);
+
+	return FALSE;
+}
+
 static DBusMessage *stk_get_properties(DBusConnection *conn,
 					DBusMessage *msg, void *data)
 {
@@ -440,6 +515,8 @@ static DBusMessage *stk_get_properties(DBusConnection *conn,
 
 	if (stk->current_menu)
 		menu = stk->current_menu;
+	if (stk->display_text)
+		menu = NULL;
 
 	reply = dbus_message_new_method_return(msg);
 	if (!reply)
@@ -451,13 +528,26 @@ static DBusMessage *stk_get_properties(DBusConnection *conn,
 					OFONO_PROPERTIES_ARRAY_SIGNATURE,
 					&dict);
 
-	if (!menu)
-		goto done;
-
-	if (stk->cmd_timeout && stk->current_menu)
+	if (stk->cmd_timeout && (stk->current_menu || stk->display_text))
 		ofono_dbus_dict_append(&dict, "Timeout",
 					DBUS_TYPE_UINT16, &timeout);
 
+	if (stk->display_text) {
+		ofono_dbus_dict_append(&dict, "Message",
+					DBUS_TYPE_STRING, &stk->display_text);
+
+		boolval = (stk->pending_cmd->qualifier & (1 << 7)) != 0;
+		ofono_dbus_dict_append(&dict, "UserConfirmMessage",
+					DBUS_TYPE_BOOLEAN, &boolval);
+
+		boolval = (stk->pending_cmd->qualifier & (1 << 0)) != 0;
+		ofono_dbus_dict_append(&dict, "PriorityMessage",
+					DBUS_TYPE_BOOLEAN, &boolval);
+	}
+
+	if (!menu)
+		goto done;
+
 	items = stk_menu_items_strlist(menu);
 	ofono_dbus_dict_append_array(&dict, "MenuItems",
 					DBUS_TYPE_STRING, &items);
@@ -511,7 +601,7 @@ static DBusMessage *stk_set_property(DBusConnection *conn,
 
 	dbus_message_iter_recurse(&iter, &var);
 
-	if (stk->cmd_timeout && stk->current_menu &&
+	if (stk->cmd_timeout && (stk->current_menu && stk->display_text) &&
 			!strcmp(property, "Timeout")) {
 		if (dbus_message_iter_get_arg_type(&var) != DBUS_TYPE_UINT16)
 			return __ofono_error_invalid_args(msg);
@@ -522,10 +612,18 @@ static DBusMessage *stk_set_property(DBusConnection *conn,
 
 		stk->timeout = timeout;
 
-		g_source_remove(stk->cmd_timeout);
-		stk->cmd_timeout = g_timeout_add_seconds(stk->timeout,
+		if (stk->current_menu) {
+			g_source_remove(stk->cmd_timeout);
+			stk->cmd_timeout = g_timeout_add_seconds(stk->timeout,
 							select_item_timeout,
 							stk);
+		} else if (stk->display_text && stk->pending_cmd->display_text.
+				duration.interval == 0) {
+			g_source_remove(stk->cmd_timeout);
+			stk->cmd_timeout = g_timeout_add_seconds(stk->timeout,
+							display_text_timeout,
+							stk);
+		}
 
 		ofono_dbus_signal_property_changed(conn, path,
 						OFONO_SIM_APP_INTERFACE,
@@ -562,6 +660,37 @@ static void select_item_cancel(struct ofono_stk *stk)
 	emit_menu_changed(stk);
 }
 
+static void display_text_cancel(struct ofono_stk *stk)
+{
+	if (stk->pending)
+		stk->cancelled = TRUE;
+
+	if (stk->cmd_timeout) {
+		g_source_remove(stk->cmd_timeout);
+		stk->cmd_timeout = 0;
+	}
+
+	stk->display_text = NULL;
+
+	stk_command_free(stk->pending_cmd);
+	stk->pending_cmd = NULL;
+
+	emit_text_changed(stk);
+	emit_menu_changed(stk);
+}
+
+static void display_text_immediate_cb(const struct ofono_error *error,
+					struct ofono_stk *stk)
+{
+	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
+		ofono_error("TERMINAL RESPONSE to Display Text failed: %s",
+				telephony_error_to_str(error));
+		return;
+	}
+
+	DBG("TERMINAL RESPONSE to Display Text reported no errors");
+}
+
 static void select_item_cb(const struct ofono_error *error,
 				struct ofono_stk *stk)
 {
@@ -591,7 +720,7 @@ static void select_item_cb(const struct ofono_error *error,
 	emit_menu_changed(stk);
 
 out:
-	if (error->type == OFONO_ERROR_TYPE_NO_ERROR)
+	if (error->type != OFONO_ERROR_TYPE_NO_ERROR)
 		reply = __ofono_error_failed(stk->pending);
 	else
 		reply = dbus_message_new_method_return(stk->pending);
@@ -657,6 +786,8 @@ static DBusMessage *stk_menu_select(DBusConnection *conn, DBusMessage *msg,
 
 	if (stk->pending)
 		return __ofono_error_busy(msg);
+	if (stk->pending_cmd && !stk->current_menu)
+		return __ofono_error_busy(msg);
 
 	if (!dbus_message_iter_init(msg, &iter))
 		return __ofono_error_invalid_args(msg);
@@ -705,6 +836,47 @@ static DBusMessage *stk_get_help(DBusConnection *conn,
 	return stk_menu_select(conn, msg, data, TRUE);
 }
 
+static void display_text_cb(const struct ofono_error *error,
+				struct ofono_stk *stk)
+{
+	DBusMessage *reply;
+
+	if (stk->cancelled == TRUE)
+		/* Command cancelled */
+		goto out;
+
+	if (stk->cmd_timeout) {
+		/*
+		 * If the command has not timed out yet, return error and
+		 * the user may then retry the command.  Otherwise assume
+		 * the text is no longer valid.
+		 */
+		if (error->type != OFONO_ERROR_TYPE_NO_ERROR)
+			goto out;
+
+		g_source_remove(stk->cmd_timeout);
+	}
+
+	stk->display_text = NULL;
+
+	if (error)
+		stk_command_cb(error, stk);
+
+	emit_text_changed(stk);
+	emit_menu_changed(stk);
+
+out:
+	if (!error)
+		return;
+
+	if (error->type != OFONO_ERROR_TYPE_NO_ERROR)
+		reply = __ofono_error_failed(stk->pending);
+	else
+		reply = dbus_message_new_method_return(stk->pending);
+
+	__ofono_dbus_pending_reply(&stk->pending, reply);
+}
+
 static DBusMessage *stk_back_or_terminate(DBusConnection *conn,
 					DBusMessage *msg, void *data,
 					enum stk_result_type type)
@@ -716,19 +888,34 @@ static DBusMessage *stk_back_or_terminate(DBusConnection *conn,
 	if (stk->pending)
 		return __ofono_error_busy(msg);
 
-	if (!stk->current_menu)
+	if (!stk->current_menu && !stk->display_text)
 		return __ofono_error_failed(msg);
 
 	if (dbus_message_iter_init(msg, &iter))
 		return __ofono_error_invalid_args(msg);
 
 	stk->cancelled = FALSE;
-	stk->pending = dbus_message_ref(msg);
 
 	memset(&rsp, 0, sizeof(rsp));
 	rsp.result.type = type;
 
-	stk_respond(stk, &rsp, select_item_cb);
+	if (stk->current_menu) {
+		stk->pending = dbus_message_ref(msg);
+
+		stk_respond(stk, &rsp, select_item_cb);
+	} else if (stk->display_text) {
+		if (stk->pending_cmd->display_text.immediate_response == TRUE) {
+			if (type != STK_RESULT_TYPE_SUCCESS)
+				return __ofono_error_failed(msg);
+
+			display_text_cb(NULL, stk);
+			return dbus_message_new_method_return(msg);
+		}
+
+		stk->pending = dbus_message_ref(msg);
+
+		stk_respond(stk, &rsp, display_text_cb);
+	}
 
 	return NULL;
 }
@@ -747,6 +934,13 @@ static DBusMessage *stk_end_session(DBusConnection *conn,
 					STK_RESULT_TYPE_USER_TERMINATED);
 }
 
+static DBusMessage *stk_confirm(DBusConnection *conn,
+				DBusMessage *msg, void *data)
+{
+	return stk_back_or_terminate(conn, msg, data,
+					STK_RESULT_TYPE_SUCCESS);
+}
+
 static GDBusMethodTable stk_methods[] = {
 	{ "GetProperties",	"",	"a{sv}",	stk_get_properties },
 	{ "SetProperty",	"sv",	"",		stk_set_property },
@@ -758,6 +952,8 @@ static GDBusMethodTable stk_methods[] = {
 				G_DBUS_METHOD_FLAG_ASYNC },
 	{ "EndSession",		"",	"",		stk_end_session,
 				G_DBUS_METHOD_FLAG_ASYNC },
+	{ "Confirm",		"",	"",		stk_confirm,
+				G_DBUS_METHOD_FLAG_ASYNC },
 	{ }
 };
 
@@ -829,6 +1025,41 @@ out:
 	return TRUE;
 }
 
+static gboolean handle_command_display_text(const struct stk_command *cmd,
+						struct stk_response *rsp,
+						struct ofono_stk *stk)
+{
+	int timeout = stk->timeout * 1000;
+
+	if (cmd->display_text.duration.interval) {
+		timeout = cmd->display_text.duration.interval;
+		switch (cmd->display_text.duration.unit) {
+		case STK_DURATION_TYPE_MINUTES:
+			timeout *= 60;
+		case STK_DURATION_TYPE_SECONDS:
+			timeout *= 10;
+		case STK_DURATION_TYPE_SECOND_TENTHS:
+			timeout *= 100;
+		}
+	}
+
+	stk->display_text = cmd->display_text.text;
+
+	/* May need to hide the menu to display the message */
+	if (stk->default_menu)
+		emit_menu_changed(stk);
+
+	emit_text_changed(stk);
+
+	stk->cmd_timeout = g_timeout_add(timeout, display_text_timeout, stk);
+	stk->cancel_cmd = display_text_cancel;
+
+	if (cmd->display_text.immediate_response == TRUE)
+		stk_respond(stk, rsp, display_text_immediate_cb);
+
+	return FALSE;
+}
+
 int ofono_stk_driver_register(const struct ofono_stk_driver *d)
 {
 	DBG("driver: %p, name: %s", d, d->name);
@@ -934,6 +1165,8 @@ struct ofono_stk *ofono_stk_create(struct ofono_modem *modem,
 					handle_command_select_item);
 	stk_command_handler_register(stk, STK_COMMAND_TYPE_SETUP_MENU,
 					handle_command_set_up_menu);
+	stk_command_handler_register(stk, STK_COMMAND_TYPE_DISPLAY_TEXT,
+					handle_command_display_text);
 
 	return stk;
 }
-- 
1.7.1.86.g0e460.dirty


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/8] stkutil: Refactor command parser error handling
  2010-06-22 11:20 [PATCH 1/8] stkutil: Refactor command parser error handling Andrzej Zaborowski
                   ` (6 preceding siblings ...)
  2010-06-22 11:20 ` [PATCH 8/8] stk: Handle the Display Text command Andrzej Zaborowski
@ 2010-06-23 19:36 ` Denis Kenzior
  7 siblings, 0 replies; 12+ messages in thread
From: Denis Kenzior @ 2010-06-23 19:36 UTC (permalink / raw)
  To: ofono

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

Hi Andrew,

> When parsing the full command fails but Command Details has been parsed,
> return a struct stk_command containing this information and the type of
> parsing problem found.  We need the command details to be able to
> even respond to the command.
> 
> This patch also makes the parser skip over unknown data objects found
> in the BER-TLV, if they don't have Comprehension Required set.

I double checked the logic and it looked sound to me.  Patch has been applied, 
thanks.

Regards,
-Denis

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/8] stk: Allow registering proactive command handlers.
  2010-06-22 11:20 ` [PATCH 2/8] stk: Allow registering proactive command handlers Andrzej Zaborowski
@ 2010-06-23 19:44   ` Denis Kenzior
  2010-06-23 22:08     ` Andrzej Zaborowski
  0 siblings, 1 reply; 12+ messages in thread
From: Denis Kenzior @ 2010-06-23 19:44 UTC (permalink / raw)
  To: ofono

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

Hi Andrew,

> Synchronous handlers can fill in the struct stk_respone *rsp
> parameter and return TRUE. Async handlers can ignore that parameter
> and return FALSE.  They have to call stk_respond() at some point.
> ---
>  src/stk.c |  151
>  ++++++++++++++++++++++++++++++++++++++++++++++++++++++------- 1 files
>  changed, 135 insertions(+), 16 deletions(-)
> 
> diff --git a/src/stk.c b/src/stk.c
> index b5c6919..0d7fc33 100644
> --- a/src/stk.c
> +++ b/src/stk.c
> @@ -39,14 +39,80 @@
> 
>  static GSList *g_drivers = NULL;
> 
> +typedef void (*generic_cb_t)(const struct ofono_error *error,
> +				struct ofono_stk *stk);
> +
> +typedef void (*envelope_cb_t)(const struct ofono_error *error,
> +				const unsigned char *data,
> +				int length, struct ofono_stk *stk);
> +

I prefer to keep typedefs to a minimum.  Since these are copies from 
include/stk.h with a different type, I think I'd prefer to simply use those 
callbacks instead.

> +typedef gboolean (*command_handler_t)(const struct stk_command *cmd,
> +					struct stk_response *rsp,
> +					struct ofono_stk *stk);
> +

Since we're not using a true public-api registration framework (yet?) lets not 
typedef this one.

>  struct ofono_stk {
>  	const struct ofono_stk_driver *driver;
>  	void *driver_data;
>  	struct ofono_atom *atom;
> +	command_handler_t handlers[STK_COMMAND_TYPE_END_SESSION + 1];

As discussed on IRC, lets just do switch/case for now.

> +	struct stk_command *pending_cmd;
>  };
> 
> +static void stk_respond(struct ofono_stk *stk,
> +			struct stk_response *rsp, generic_cb_t cb)
> +{
> +	struct ofono_error error = { .type = OFONO_ERROR_TYPE_FAILURE };
> +	const guint8 *tlv;
> +	unsigned int tlv_len;
> +
> +	rsp->src = STK_DEVICE_IDENTITY_TYPE_TERMINAL;
> +	rsp->dst = STK_DEVICE_IDENTITY_TYPE_UICC;
> +	rsp->number = stk->pending_cmd->number;
> +	rsp->type = stk->pending_cmd->type;
> +	rsp->qualifier = stk->pending_cmd->qualifier;
> +
> +	if (stk->driver->terminal_response == NULL) {
> +		cb(&error, stk);
> +		return;
> +	}

What do you think of returning an int here (e.g. -ENOTSUPPORTED) instead of 
calling a callback?  I think that would make things a bit cleaner.

> +
> +	tlv = stk_pdu_from_response(rsp, &tlv_len);
> +	if (!tlv) {
> +		cb(&error, stk);
> +		return;

Perhaps -EINVAL here?
> +	}
> +
> +	stk->driver->terminal_response(stk, tlv_len, tlv,
> +					(ofono_stk_generic_cb_t) cb, stk);
> +}
> +
> +static void stk_send_envelope(struct ofono_stk *stk, struct stk_envelope
>  *e, +				envelope_cb_t cb)
> +{
> +	struct ofono_error error = { .type = OFONO_ERROR_TYPE_FAILURE };
> +	const guint8 *tlv;
> +	unsigned int tlv_len;
> +
> +	e->dst = STK_DEVICE_IDENTITY_TYPE_UICC;
> +
> +	if (stk->driver->envelope == NULL) {
> +		cb(&error, NULL, -1, stk);
> +		return;

Same comments as above for returning -errno.

> +	}
> +
> +	tlv = stk_pdu_from_envelope(e, &tlv_len);
> +	if (!tlv) {
> +		cb(&error, NULL, -1, stk);
> +		return;
> +	}
> +
> +	stk->driver->envelope(stk, tlv_len, tlv,
> +				(ofono_stk_envelope_cb_t) cb, stk);
> +}
> +
>  static void stk_cbs_download_cb(const struct ofono_error *error,
> -				const unsigned char *data, int len, void *user)
> +				const unsigned char *data, int len,
> +				struct ofono_stk *stk)
>  {
>  	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
>  		ofono_error("CellBroadcast download to UICC failed");
> @@ -55,34 +121,46 @@ static void stk_cbs_download_cb(const struct
>  ofono_error *error, return;
>  	}
> 
> +	if (len)
> +		ofono_error("CellBroadcast download returned %i bytes of data",
> +				len);
> +
>  	DBG("CellBroadcast download to UICC reported no error");
>  }
> 
>  void __ofono_cbs_sim_download(struct ofono_stk *stk, const struct cbs
>  *msg) {
> -	const guint8 *tlv;
> -	unsigned int tlv_len;
>  	struct stk_envelope e;
> 
> -	if (stk->driver->envelope == NULL)
> -		return;
> +	memset(&e, 0, sizeof(e));
> 
>  	e.type = STK_ENVELOPE_TYPE_CBS_PP_DOWNLOAD;
>  	e.src = STK_DEVICE_IDENTITY_TYPE_NETWORK;
> -	e.dst = STK_DEVICE_IDENTITY_TYPE_UICC;
>  	memcpy(&e.cbs_pp_download.page, msg, sizeof(msg));
> 
> -	tlv = stk_pdu_from_envelope(&e, &tlv_len);
> -	if (!tlv)
> +	stk_send_envelope(stk, &e, stk_cbs_download_cb);
> +}
> +
> +static void stk_command_cb(const struct ofono_error *error,
> +				struct ofono_stk *stk)
> +{
> +	stk_command_free(stk->pending_cmd);
> +	stk->pending_cmd = NULL;
> +
> +	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
> +		ofono_error("TERMINAL RESPONSE to a UICC command failed");
> +		/* "The ME may retry to deliver the same Cell Broadcast
> +		 * page." */
>  		return;
> +	}
> 
> -	stk->driver->envelope(stk, tlv_len, tlv, stk_cbs_download_cb, stk);
> +	DBG("TERMINAL RESPONSE to a command reported no errors");
>  }
> 
>  void ofono_stk_proactive_command_notify(struct ofono_stk *stk,
>  					int length, const unsigned char *pdu)
>  {
> -	struct stk_command *cmd;
> +	struct stk_response rsp;
>  	char *buf;
>  	int i;
> 
> @@ -93,23 +171,64 @@ void ofono_stk_proactive_command_notify(struct
>  ofono_stk *stk, for (i = 0; i < length; i ++)
>  		sprintf(buf + i * 2, "%02hhx", pdu[i]);
> 
> -	cmd = stk_command_new_from_pdu(pdu, length);
> -	if (!cmd) {
> +	stk->pending_cmd = stk_command_new_from_pdu(pdu, length);
> +	if (!stk->pending_cmd) {
>  		ofono_error("Can't parse proactive command: %s", buf);
> 
> -		/* TODO: return TERMINAL RESPONSE with permanent error */
> +		/*
> +		 * Nothing we can do, we'd need at least Command Details
> +		 * to be able to respond with an error.
> +		 */
>  		goto done;
>  	}
> 
> -	/* TODO: execute */
> -	ofono_info("Proactive command PDU: %s", buf);
> +	ofono_debug("Proactive command PDU: %s", buf);
> +
> +	memset(&rsp, 0, sizeof(rsp));
> 
> -	stk_command_free(cmd);
> +	switch (stk->pending_cmd->status) {
> +	case STK_PARSE_RESULT_OK:
> +		if (stk->handlers[stk->pending_cmd->type]) {
> +			if (stk->handlers[stk->pending_cmd->type](
> +					stk->pending_cmd, &rsp, stk) == TRUE)
> +				break;
> +
> +			return;
> +		}
> +		/* Fall through */
> +
> +	case STK_PARSE_RESULT_TYPE_NOT_UNDERSTOOD:
> +	default:
> +		rsp.result.type = STK_RESULT_TYPE_COMMAND_NOT_UNDERSTOOD;
> +		break;
> +
> +	case STK_PARSE_RESULT_MISSING_VALUE:
> +		rsp.result.type = STK_RESULT_TYPE_MINIMUM_NOT_MET;
> +		break;
> +
> +	case STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD:
> +		rsp.result.type = STK_RESULT_TYPE_DATA_NOT_UNDERSTOOD;
> +		break;
> +	}
> +
> +	stk_respond(stk, &rsp, stk_command_cb);
> 
>  done:
>  	g_free(buf);
>  }
> 
> +static gboolean stk_command_handler_register(struct ofono_stk *stk,
> +						enum stk_command_type type,
> +						command_handler_t handler)
> +{
> +	if (stk->handlers[type])
> +		return FALSE;
> +
> +	stk->handlers[type] = handler;
> +
> +	return TRUE;
> +}
> +
>  int ofono_stk_driver_register(const struct ofono_stk_driver *d)
>  {
>  	DBG("driver: %p, name: %s", d, d->name);
> 

Thanks,
-Denis

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/8] stk: Add SimApplication interface.
  2010-06-22 11:20 ` [PATCH 4/8] stk: Add SimApplication interface Andrzej Zaborowski
@ 2010-06-23 21:25   ` Denis Kenzior
  0 siblings, 0 replies; 12+ messages in thread
From: Denis Kenzior @ 2010-06-23 21:25 UTC (permalink / raw)
  To: ofono

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

Hi Andrew,

> ---
>  include/dbus.h |    1 +
>  src/stk.c      |  585
>  +++++++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files 
changed,
>  585 insertions(+), 1 deletions(-)
> 

Just a general comment: In the previous version of the patch you included an 
api document in the commit description.  It might be a good idea to formalize 
it into a proper doc/stk-api.txt document and include it along with the patch.  
This way people have a much easier time understanding what is going on, and we 
do want feedback on the API from many people for this one.

Another suggestion I have is to split out the Select Item proactive command 
handling from the regular Menu.  Perhaps moving Select Item handling to an 
Agent interface along with Display Text, Play Tone, Get Input, Get Inkey, etc 
might be a good idea.

Also, I really encourage you to split this patch apart some more.  E.g. one 
patch for foundation of the stk atom, one part handling the Setup Menu / Menu 
Selection envelope, another for Select Item, etc.  This makes it easier to 
review the code.  Smaller patches are always good, and higher chance that less 
controversial chunks will get accepted, leading to even smaller patches in the 
future.

Regards,
-Denis

Regards,
-Denis

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/8] stk: Allow registering proactive command handlers.
  2010-06-23 19:44   ` Denis Kenzior
@ 2010-06-23 22:08     ` Andrzej Zaborowski
  0 siblings, 0 replies; 12+ messages in thread
From: Andrzej Zaborowski @ 2010-06-23 22:08 UTC (permalink / raw)
  To: ofono

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

Hi Denis,

On 23 June 2010 21:44, Denis Kenzior <denkenz@gmail.com> wrote:
>> +     struct stk_command *pending_cmd;
>>  };
>>
>> +static void stk_respond(struct ofono_stk *stk,
>> +                     struct stk_response *rsp, generic_cb_t cb)
>> +{
>> +     struct ofono_error error = { .type = OFONO_ERROR_TYPE_FAILURE };
>> +     const guint8 *tlv;
>> +     unsigned int tlv_len;
>> +
>> +     rsp->src = STK_DEVICE_IDENTITY_TYPE_TERMINAL;
>> +     rsp->dst = STK_DEVICE_IDENTITY_TYPE_UICC;
>> +     rsp->number = stk->pending_cmd->number;
>> +     rsp->type = stk->pending_cmd->type;
>> +     rsp->qualifier = stk->pending_cmd->qualifier;
>> +
>> +     if (stk->driver->terminal_response == NULL) {
>> +             cb(&error, stk);
>> +             return;
>> +     }
>
> What do you think of returning an int here (e.g. -ENOTSUPPORTED) instead of
> calling a callback?  I think that would make things a bit cleaner.

In that case can I use another function to wrap stk_respond? :)  The
reason I added this wrapper around driver->terminal_response is to
avoid repeating the same sequence everywhere.  If the caller needs to
check the return value from stk_respond then you have lots of
repetition again, the error handling will look the same in every case.

Regards

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2010-06-23 22:08 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-06-22 11:20 [PATCH 1/8] stkutil: Refactor command parser error handling Andrzej Zaborowski
2010-06-22 11:20 ` [PATCH 2/8] stk: Allow registering proactive command handlers Andrzej Zaborowski
2010-06-23 19:44   ` Denis Kenzior
2010-06-23 22:08     ` Andrzej Zaborowski
2010-06-22 11:20 ` [PATCH 3/8] stk: Handle the More Time command as a nop Andrzej Zaborowski
2010-06-22 11:20 ` [PATCH 4/8] stk: Add SimApplication interface Andrzej Zaborowski
2010-06-23 21:25   ` Denis Kenzior
2010-06-22 11:20 ` [PATCH 5/8] stk: Forget previous command if a new one comes Andrzej Zaborowski
2010-06-22 11:20 ` [PATCH 6/8] Add a proactive command cancel notification Andrzej Zaborowski
2010-06-22 11:20 ` [PATCH 7/8] mbmmodem: Cancel running command on *STKEND Andrzej Zaborowski
2010-06-22 11:20 ` [PATCH 8/8] stk: Handle the Display Text command Andrzej Zaborowski
2010-06-23 19:36 ` [PATCH 1/8] stkutil: Refactor command parser error handling Denis Kenzior

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.