Skip to contentSkip to conversation

stk: Add parser for send ussd commands

PATCH In review Yang Gu 3 participants 9 messages Jun 17, 2010 – Jun 21, 2010

lore
Share & tools
lore

Yang Gu

author
#0 lore raw

PATCH 1/5 stk: Add parser for send ussd commands

---
 src/stkutil.c |   61 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/stkutil.h |    9 ++++++++
 2 files changed, 70 insertions(+), 0 deletions(-)
src/stkutil.c +61 -0
index 44a8eff..f03b25c 100644
@@ -585,6 +585,24 @@ static gboolean parse_dataobj_tone(struct comprehension_tlv_iter *iter,
 	return parse_dataobj_common_byte(iter, byte);
 }
 
+/* Defined in TS 102.223 Section 8.17 */
+static gboolean parse_dataobj_ussd(struct comprehension_tlv_iter *iter,
+					void *user)
+{
+	struct stk_ussd_string *us = user;
+	unsigned int len = comprehension_tlv_iter_get_length(iter);
+	const unsigned char *data = comprehension_tlv_iter_get_data(iter);
+
+	if (len <= 1)
+		return FALSE;
+
+	us->dcs = data[0];
+	us->len = len - 1;
+	memcpy(us->string, data + 1, us->len);
+
+	return TRUE;
+}
+
 /* Defined in TS 102.223 Section 8.18 */
 static gboolean parse_dataobj_file_list(struct comprehension_tlv_iter *iter,
 					void *user)
@@ -1996,6 +2014,8 @@ static dataobj_handler handler_for_type(enum stk_data_object_type type)
 		return parse_dataobj_text;
 	case STK_DATA_OBJECT_TYPE_TONE:
 		return parse_dataobj_tone;
+	case STK_DATA_OBJECT_TYPE_USSD_STRING:
+		return parse_dataobj_ussd;
 	case STK_DATA_OBJECT_TYPE_FILE_LIST:
 		return parse_dataobj_file_list;
 	case STK_DATA_OBJECT_TYPE_LOCATION_INFO:
@@ -2710,6 +2730,44 @@ static gboolean parse_send_ss(struct stk_command *command,
 	return TRUE;
 }
 
+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,
+					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;
+
+	if (command->dst != STK_DEVICE_IDENTITY_TYPE_NETWORK)
+		return FALSE;
+
+	ret = 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,
+				&obj->ussd_string,
+				STK_DATA_OBJECT_TYPE_ICON_ID, 0,
+				&obj->icon_id,
+				STK_DATA_OBJECT_TYPE_TEXT_ATTRIBUTE, 0,
+				&obj->text_attr,
+				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)
 {
 	g_free(command->setup_call.alpha_id_usr_cfm);
@@ -3672,6 +3730,9 @@ struct stk_command *stk_command_new_from_pdu(const unsigned char *pdu,
 	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;
src/stkutil.h +9 -0
index 471e10f..6fb49e0 100644
@@ -1117,6 +1117,14 @@ struct stk_command_send_ss {
 	struct stk_frame_id frame_id;
 };
 
+struct stk_command_send_ussd {
+	char *alpha_id;
+	struct stk_ussd_string ussd_string;
+	struct stk_icon_id icon_id;
+	struct stk_text_attribute text_attr;
+	struct stk_frame_id frame_id;
+};
+
 struct stk_command_setup_call {
 	char *alpha_id_usr_cfm;
 	struct stk_address addr;
@@ -1295,6 +1303,7 @@ struct stk_command {
 		struct stk_command_select_item select_item;
 		struct stk_command_send_sms send_sms;
 		struct stk_command_send_ss send_ss;
+		struct stk_command_send_ussd send_ussd;
 		struct stk_command_setup_call setup_call;
 		struct stk_command_setup_event_list setup_event_list;
 		struct stk_command_perform_card_apdu perform_card_apdu;
Signature · 1 line
1.7.0.4
author
#1 lore raw

PATCH 2/5 teststk: Add test for send ussd parser

---
 src/smsutil.c       |   44 ++
 src/smsutil.h       |    2 +
 unit/test-stkutil.c | 1319 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 1365 insertions(+), 0 deletions(-)
src/smsutil.c +44 -0
index 95eca06..5fa4be5 100644
@@ -3747,3 +3747,47 @@ gboolean cbs_topic_in_range(unsigned int topic, GSList *ranges)
 	return g_slist_find_custom(ranges, GUINT_TO_POINTER(topic),
 					cbs_topic_compare) != NULL;
 }
+
+char *ussd_decode(int dcs, int len, const unsigned char *data)
+{
+	gboolean udhi;
+	enum sms_charset charset;
+	gboolean compressed;
+	gboolean iso639;
+	char *utf8;
+
+	if (!cbs_dcs_decode(dcs, &udhi, NULL, &charset,
+				&compressed, NULL, &iso639))
+		return NULL;
+
+	if (udhi || compressed || iso639)
+		return NULL;
+
+	switch (charset) {
+	case SMS_CHARSET_7BIT:
+	{
+		long written;
+		unsigned char *unpacked = unpack_7bit(data, len, 0, TRUE, 0,
+							&written, 0);
+		if (unpacked == NULL)
+			return NULL;
+
+		utf8 = convert_gsm_to_utf8(unpacked, written, NULL, NULL, 0);
+		g_free(unpacked);
+
+		break;
+	}
+	case SMS_CHARSET_8BIT:
+		utf8 = convert_gsm_to_utf8(data, len, NULL, NULL, 0);
+		break;
+	case SMS_CHARSET_UCS2:
+		utf8 = g_convert((const gchar *) data, len,
+					"UTF-8//TRANSLIT", "UCS-2BE",
+					NULL, NULL, NULL);
+		break;
+	default:
+		utf8 = NULL;
+	}
+
+	return utf8;
+}
src/smsutil.h +2 -0
index 1bd42bb..d7026ec 100644
@@ -508,3 +508,5 @@ char *cbs_topic_ranges_to_string(GSList *ranges);
 GSList *cbs_extract_topic_ranges(const char *ranges);
 GSList *cbs_optimize_ranges(GSList *ranges);
 gboolean cbs_topic_in_range(unsigned int topic, GSList *ranges);
+
+char *ussd_decode(int dcs, int len, const unsigned char *data);
unit/test-stkutil.c +1319 -0

1,337 lines not on this page · raw message

Signature · 1 line
1.7.0.4
author
#2 lore raw

PATCH 3/5 Modify ussd parser

In reply to Yang Gu

* Original code doesn't call unpack_7bit(), which seems not correct.
* An utility function is used to handle it, which supports 8-bit and ucs2 besides 7-bit character.
* Status is assigned with correct value when error occurs

---
 drivers/atmodem/ussd.c |   32 +++++---------------------------
 1 files changed, 5 insertions(+), 27 deletions(-)
drivers/atmodem/ussd.c +5 -27
index 555ce13..4bb7031 100644
@@ -53,14 +53,10 @@ static const char *none_prefix[] = { NULL };
 static void cusd_parse(GAtResult *result, struct ofono_ussd *ussd)
 {
 	GAtResultIter iter;
-	int status;
 	int dcs;
 	const char *content;
+	int status = OFONO_USSD_STATUS_NOT_SUPPORTED;
 	char *converted = NULL;
-	gboolean udhi;
-	enum sms_charset charset;
-	gboolean compressed;
-	gboolean iso639;
 
 	g_at_result_iter_init(&iter, result);
 
@@ -76,32 +72,14 @@ static void cusd_parse(GAtResult *result, struct ofono_ussd *ussd)
 	if (!g_at_result_iter_next_number(&iter, &dcs))
 		goto out;
 
-	if (!cbs_dcs_decode(dcs, &udhi, NULL, &charset,
-				&compressed, NULL, &iso639))
-		goto out;
-
-	if (udhi || compressed || iso639)
-		goto out;
+	converted = ussd_decode(dcs, strlen(content), (unsigned char *)content);
 
-	if (charset == SMS_CHARSET_7BIT)
-		converted = convert_gsm_to_utf8((const guint8 *) content,
-						strlen(content), NULL, NULL, 0);
-
-	else if (charset == SMS_CHARSET_8BIT) {
-		/* TODO: Figure out what to do with 8 bit data */
-		ofono_error("8-bit coded USSD response received");
-		status = 4; /* Not supported */
-	} else {
-		/* No other encoding is mentioned in TS27007 7.15 */
-		ofono_error("Unsupported USSD data coding scheme (%02x)", dcs);
-		status = 4; /* Not supported */
-	}
+	if (converted)
+		status = OFONO_USSD_STATUS_NOTIFY;
 
 out:
 	ofono_ussd_notify(ussd, status, converted);
-
-	if (converted)
-		g_free(converted);
+	g_free(converted);
 }
 
 static void cusd_request_cb(gboolean ok, GAtResult *result, gpointer user_data)
Signature · 1 line
1.7.0.4
author
#3 lore raw

PATCH 4/5 Refactor ussd parser

In reply to Yang Gu
---
 drivers/isimodem/ussd.c |   35 ++++++-----------------------------
 1 files changed, 6 insertions(+), 29 deletions(-)
drivers/isimodem/ussd.c +6 -29
index 8be20b6..7002d09 100644
@@ -73,44 +73,21 @@ static void ussd_parse(struct ofono_ussd *ussd, const void *restrict data,
 			size_t len)
 {
 	const unsigned char *msg = data;
-	unsigned char buf[256];
-	unsigned char *unpacked;
-	long written;
-	int status;
+	int status = OFONO_USSD_STATUS_NOT_SUPPORTED;
 	char *converted = NULL;
-	gboolean udhi;
-	enum sms_charset charset;
-	gboolean compressed;
-	gboolean iso639;
 
 	if (!msg || len < 4)
-		goto error;
+		goto out;
 
 	status = isi_type_to_status(msg[2]);
 
 	if (msg[3] == 0 || (size_t)(msg[3] + 4) > len)
-		goto error;
-
-	if (!cbs_dcs_decode(msg[1], &udhi, NULL, &charset,
-				&compressed, NULL, &iso639))
-		goto error;
-
-	if (udhi || compressed || iso639)
-		goto error;
-
-	if (charset != SMS_CHARSET_7BIT)
-		goto error;
-
-	unpacked = unpack_7bit_own_buf(msg + 4, msg[3], 0, TRUE,
-					SS_MAX_USSD_LENGTH, &written, 0, buf);
+		goto out;
 
-	converted = convert_gsm_to_utf8((const guint8 *)unpacked, written,
-					NULL, NULL, 0);
+	converted = ussd_decode(msg[1], msg[3], msg + 4);
 
-	goto out;
-
-error:
-	status = OFONO_USSD_STATUS_NOT_SUPPORTED;
+	if (converted)
+		status = OFONO_USSD_STATUS_NOTIFY;
 
 out:
 	ofono_ussd_notify(ussd, status, converted);
Signature · 1 line
1.7.0.4
#4 lore raw

Re: [PATCH 4/5] Refactor ussd parser

Hi Yang,

Quoted 3 lines
---
 drivers/isimodem/ussd.c |   35 ++++++-----------------------------
 1 files changed, 6 insertions(+), 29 deletions(-)

Patch has been applied, thanks.

Regards,
-Denis

author
#5 lore raw

PATCH 5/5 stk: Add parser for timing advance objects

In reply to Yang Gu
---
 src/stkutil.c       |   22 ++++++++++++++++++++++
 src/stkutil.h       |   30 ++++++++++++++++++------------
 unit/test-stkutil.c |    2 +-
 3 files changed, 41 insertions(+), 13 deletions(-)
src/stkutil.c +22 -0
index f03b25c..6178059 100644
@@ -1151,6 +1151,26 @@ static gboolean parse_dataobj_language(struct comprehension_tlv_iter *iter,
 	return TRUE;
 }
 
+/* Defined in 31.111 Section 8.46 */
+static gboolean parse_dataobj_timing_advance(
+			struct comprehension_tlv_iter *iter, void *user)
+{
+	struct stk_timing_advance *ta = user;
+	const unsigned char *data;
+	unsigned int len = comprehension_tlv_iter_get_length(iter);
+
+	if (len != 2)
+		return FALSE;
+
+	data = comprehension_tlv_iter_get_data(iter);
+
+	ta->has_value = TRUE;
+	ta->status = data[0];
+	ta->advance = data[1];
+
+	return TRUE;
+}
+
 /* Defined in 102.223 Section 8.47 */
 static gboolean parse_dataobj_browser_id(struct comprehension_tlv_iter *iter,
 						void *user)
@@ -2074,6 +2094,8 @@ static dataobj_handler handler_for_type(enum stk_data_object_type type)
 		return parse_dataobj_language;
 	case STK_DATA_OBJECT_TYPE_BROWSER_ID:
 		return parse_dataobj_browser_id;
+	case STK_DATA_OBJECT_TYPE_TIMING_ADVANCE:
+		return parse_dataobj_timing_advance;
 	case STK_DATA_OBJECT_TYPE_URL:
 		return parse_dataobj_url;
 	case STK_DATA_OBJECT_TYPE_BEARER:
src/stkutil.h +18 -12
index 6fb49e0..c2fcea7 100644
@@ -552,6 +552,11 @@ enum stk_rejection_cause_code {
 	STK_CAUSE_EMM_PROTOCOL_ERROR			= 0x6f,
 };
 
+enum stk_me_status {
+	STK_ME_STATUS_IDLE = 		0x00,
+	STK_ME_STATUS_NOT_IDLE = 	0x01
+};
+
 /* For data object that only has a byte array with undetermined length */
 struct stk_common_byte_array {
 	unsigned char *array;
@@ -801,6 +806,18 @@ struct stk_bc_repeat {
 	unsigned char value;
 };
 
+/* Defined in TS 31.111 Section 8.46 */
+struct stk_timing_advance {
+	ofono_bool_t has_value;
+	enum stk_me_status status;
+	/*
+	 * Contains bit periods number according to 3GPP TS
+	 * 44.118 Section 9.3.106 / 3GPP TS 44.018 Section
+	 * 10.5.2.40.1, not microseconds
+	 */
+	unsigned char advance;
+};
+
 /*
  * According to 102.223 Section 8.52 the length of CTLV is 1 byte. This means
  * that the maximum size is 127 according to the rules of CTLVs. This size also
@@ -1381,18 +1398,7 @@ struct stk_response_local_info {
 		const char *language;
 		enum stk_battery_state battery_charge;
 		enum stk_access_technology_type access_technology;
-		struct stk_timing_advance {
-			enum {
-				STK_TIMING_ADVANCE_ME_IDLE = 0x00,
-				STK_TIMING_ADVANCE_ME_NOT_IDLE = 0x01,
-			} status;
-			/*
-			 * Contains bit periods number according to 3GPP TS
-			 * 44.118 Section 9.3.106 / 3GPP TS 44.018 Section
-			 * 10.5.2.40.1, not microseconds
-			 */
-			int advance;
-		} tadv;
+		struct stk_timing_advance tadv;
 		/* Bits[31:24]: manufacturer, bits[23:0]: serial number */
 		guint32 esn;
 		const char *imeisv;
unit/test-stkutil.c +1 -1
index e498dd6..dd22912 100644
@@ -18699,7 +18699,7 @@ static const struct terminal_response_test
 		},
 		{ .provide_local_info = {
 			{ .tadv = {
-				.status = STK_TIMING_ADVANCE_ME_IDLE,
+				.status = STK_ME_STATUS_IDLE,
 				.advance = 0,
 			}},
 		}},
Signature · 1 line
1.7.0.4
#6 lore raw
In reply to Yang Gu

Hi Yang,

Quoted 3 lines
---
 src/stkutil.c |   61

+++++++++++++++++++++++++++++++++++++++++++++++++++++++++

src/stkutil.h |

Quoted 2 lines
   9 ++++++++
 2 files changed, 70 insertions(+), 0 deletions(-)

Patches 1, 2 and 5 have been applied. As discussed previously we're skipping patches 3 and 4 for now. Please note that I broke Patch 2 into two patches (1 for decode_ussd in smsutil.[ch] and one for the unit test), but did not do any modifications beyond that.

Again, please watch out for this. If your patch touches something in two or more directories of oFono, please send multiple patches.

Regards,
-Denis

#7 lore raw

Hi Denis,

Quoted 16 lines

-----Original Message-----
From: Denis Kenzior [mailto:denkenz(a)gmail.com]
Sent: Saturday, June 19, 2010 3:04 AM
To: ofono(a)ofono.org
Cc: Gu, Yang
Subject: Re: [PATCH 1/5] stk: Add parser for send ussd commands

Hi Yang,

Quoted 3 lines
---
 src/stkutil.c |   61

+++++++++++++++++++++++++++++++++++++++++++++++++++++++++

src/stkutil.h |

Quoted 2 lines
   9 ++++++++
 2 files changed, 70 insertions(+), 0 deletions(-)
Quoted 7 lines

Patches 1, 2 and 5 have been applied. As discussed previously we're skipping patches 3 and 4 for now. Please note that I broke Patch 2 into two patches (1 for decode_ussd in smsutil.[ch] and one for the unit test), but did not do any modifications beyond that.

Again, please watch out for this. If your patch touches something in two or more directories of oFono, please send multiple patches.

Thank you for applying the patches. But I have a question on this: Should we keep the code compile-able after each patch? Due to this consideration, I put those changes all in patch 2. This would sometimes be helpful when we want to investigate a regression after a bunch of patches. If this is not a requirement for this project, I will pay more attention on how to split the patches reasonably.

Quoted 2 lines

Regards,
-Denis

Regards,
-Yang

#8 lore raw

Hi Yang,

Quoted 9 lines
Quoted 2 lines

Again, please watch out for this. If your patch touches something in two or more directories of oFono, please send multiple patches.

Thank you for applying the patches. But I have a question on this: Should
we keep the code compile-able after each patch? Due to this consideration, I put those changes all in patch 2. This would sometimes be helpful when we want to investigate a regression after a bunch of patches. If this is not a requirement for this project, I will pay more attention on how to split the patches reasonably.

Keeping the code compilable is always preferable. For instance, if you're adding a function to smsutil to use elsewhere, it is always preferable to have a commit with this function and then commit(s) using this function.

However, there are cases when you're moving code around and breaking compile in transient commits is unavoidable. In that case I still prefer clean, logical patches. I will apply them all or not at all, so as long as the code compiles & works cleanly afterward, it is fine.

Regards,
-Denis

Quoted 7 lines
Quoted 3 lines

Regards,
-Denis

Regards,
-Yang