[PATCH] Parse +CUSD responses.

[PATCH] Parse +CUSD responses.

[Andrzej Zaborowski]

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

Register for "+CUSD:" unsolicited responses, also parse +CUSD
lines appearing as part of response to AT+CUSD (this form is still
used on Nokia phones that emulate AT when connected to a PC, and
both forms are allowed by the spec).  The latter form is treated
differently to make sure the Initiate callback happens before
ofono_ussd_notify is called.

USSD.Initiate still returns a string, this may be wrong because
when the network returns data coded in 8-bits we have no information
on the character set used, everything is "user-specified" for the
8-bit coding in the 23.038 and technically we should return an array
of bytes.  I only handle 7-bit and 8-bit responses, other codings
are not specified and I didn't manage to make the network send me
anything other than 7-bit coded string, although I managed to send a
UCS2-coded USSD string -- the network still only responded with DCS
= 15 (but the response content didn't make any sense).  The responses
from my network were always in the local language but with non-ASCII
characters stripped, regardless of the language set in the DCS by me.

I removed the "TODO: be able to send UCS2 string" because the string
we send is first checked to be a valid USSD string so we already know
it can be encoded with GSM 7-bits.
---
 drivers/atmodem/ussd.c |  110 ++++++++++++++++++++++++++++++++++++++++++------
 src/ussd.c             |    3 +
 2 files changed, 100 insertions(+), 13 deletions(-)

diff --git a/drivers/atmodem/ussd.c b/drivers/atmodem/ussd.c
index 1d819a3..e35cf72 100644
--- a/drivers/atmodem/ussd.c
+++ b/drivers/atmodem/ussd.c
@@ -40,39 +40,103 @@
 
 #include "atmodem.h"
 
+struct cusd_req {
+	ofono_ussd_cb_t cb;
+	void *data;
+	struct ofono_ussd *ussd;
+};
+
+static const char *cusd_prefix[] = { "+CUSD:", NULL };
 static const char *none_prefix[] = { NULL };
 
+static void cusd_parse(GAtResult *result, struct ofono_ussd *ussd)
+{
+	GAtResultIter iter;
+	int status;
+	int dcs;
+	const guint8 *content;
+	int length;
+	char *converted = NULL;
+
+	g_at_result_iter_init(&iter, result);
+
+	if (!g_at_result_iter_next(&iter, "+CUSD:"))
+		return;
+
+	if (!g_at_result_iter_next_number(&iter, &status))
+		return;
+
+	if (!g_at_result_iter_next_hexstring(&iter, &content, &length))
+		goto out;
+
+	if (!g_at_result_iter_next_number(&iter, &dcs))
+		goto out;
+
+	/* All 7-bit coding schemes - there's no need to distinguish
+	 * between the different schemes because the modem is tasked
+	 * with presenting us with only raw 7-bit characters.
+	 */
+	if ((dcs & 0xf0) == 0x00 || dcs == 0x10 || (dcs & 0xf0) == 0x20 ||
+			(dcs & 0xf0) == 0x30 || (dcs & 0xcc) == 0x40 ||
+			(dcs & 0xfc) == 0x90 || (dcs & 0xf4) == 0xf0)
+		converted = convert_gsm_to_utf8(content, length,
+						NULL, NULL, 0);
+
+	/* All 8-bit coding schemes - again we can treat all of them
+	 * the same, however we have no information about the character
+	 * set used, for the moment return raw (upper layer will assume
+	 * what we sent it is UTF-8 which may be wrong).
+	 */
+	else if ((dcs & 0xcc) == 0x44 || (dcs & 0xfc) == 0x94 ||
+			(dcs & 0xf4) == 0xf4)
+		converted = g_strndup((char *) content, length);
+
+	/* No other encoding is mentioned in TS27007 7.15 */
+	else
+		ofono_error("Unsupported USSD data coding scheme (%02x)", dcs);
+
+out:
+	ofono_ussd_notify(ussd, status, converted);
+
+	if (converted)
+		g_free(converted);
+}
+
 static void cusd_request_cb(gboolean ok, GAtResult *result, gpointer user_data)
 {
-	struct cb_data *cbd = user_data;
-	ofono_ussd_cb_t cb = cbd->cb;
+	struct cusd_req *cbd = user_data;
 	struct ofono_error error;
 
 	dump_response("cusd_request_cb", ok, result);
 	decode_at_error(&error, g_at_result_final_response(result));
 
-	cb(&error, cbd->data);
+	cbd->cb(&error, cbd->data);
+
+	cusd_parse(result, cbd->ussd);
 }
 
 static void at_ussd_request(struct ofono_ussd *ussd, const char *str,
 				ofono_ussd_cb_t cb, void *data)
 {
 	GAtChat *chat = ofono_ussd_get_data(ussd);
-	struct cb_data *cbd = cb_data_new(cb, data);
+	struct cusd_req *cbd = g_try_new0(struct cusd_req, 1);
 	unsigned char *converted = NULL;
 	int dcs;
 	int max_len;
+	int i;
+	int len;
 	long written;
 	char buf[256];
 
 	if (!cbd)
 		goto error;
 
+	cbd->cb = cb;
+	cbd->data = data;
+	cbd->ussd = ussd;
+
 	converted = convert_utf8_to_gsm(str, strlen(str), NULL, &written, 0);
 
-	/* TODO: Be able to convert to UCS2, although the standard does not
-	 * indicate that this is actually possible
-	 */
 	if (!converted)
 		goto error;
 	else {
@@ -83,12 +147,15 @@ static void at_ussd_request(struct ofono_ussd *ussd, const char *str,
 	if (written > max_len)
 		goto error;
 
-	sprintf(buf, "AT+CUSD=1,\"%*s\",%d", (int) written, converted, dcs);
+	len = sprintf(buf, "AT+CUSD=1,\"");
+	for (i = 0; i < written; i ++)
+		len += sprintf(buf + len, "%02hhx", converted[i]);
+	sprintf(buf + len, "\",%d", dcs);
 
 	g_free(converted);
 	converted = NULL;
 
-	if (g_at_chat_send(chat, buf, none_prefix,
+	if (g_at_chat_send(chat, buf, cusd_prefix,
 				cusd_request_cb, cbd, g_free) > 0)
 		return;
 
@@ -133,13 +200,28 @@ error:
 	CALLBACK_WITH_FAILURE(cb, data);
 }
 
-static gboolean at_ussd_register(gpointer user)
+static void cusd_notify(GAtResult *result, gpointer user_data)
+{
+	struct ofono_ussd *ussd = user_data;
+
+	dump_response("cssu_notify", TRUE, result);
+
+	cusd_parse(result, ussd);
+}
+
+static void at_ussd_register(gboolean ok, GAtResult *result, gpointer user)
 {
 	struct ofono_ussd *ussd = user;
+	GAtChat *chat = ofono_ussd_get_data(ussd);
 
-	ofono_ussd_register(ussd);
+	if (!ok) {
+		ofono_error("Could not enable CUSD notifications");
+		return;
+	}
+
+	g_at_chat_register(chat, "+CUSD:", cusd_notify, FALSE, ussd, NULL);
 
-	return FALSE;
+	ofono_ussd_register(ussd);
 }
 
 static int at_ussd_probe(struct ofono_ussd *ussd, unsigned int vendor,
@@ -148,7 +230,9 @@ static int at_ussd_probe(struct ofono_ussd *ussd, unsigned int vendor,
 	GAtChat *chat = data;
 
 	ofono_ussd_set_data(ussd, chat);
-	g_idle_add(at_ussd_register, ussd);
+
+	g_at_chat_send(chat, "AT+CSCS=\"HEX\"", NULL, NULL, NULL, NULL);
+	g_at_chat_send(chat, "AT+CUSD=1", NULL, at_ussd_register, ussd, NULL);
 
 	return 0;
 }
diff --git a/src/ussd.c b/src/ussd.c
index be50296..2bf378d 100644
--- a/src/ussd.c
+++ b/src/ussd.c
@@ -313,6 +313,9 @@ void ofono_ussd_notify(struct ofono_ussd *ussd, int status, const char *str)
 
 		reply = dbus_message_new_method_return(ussd->pending);
 
+		if (!str)
+			str = "";
+
 		dbus_message_iter_init_append(reply, &iter);
 
 		dbus_message_iter_append_basic(&iter, DBUS_TYPE_STRING,
-- 
1.6.1

Re: [PATCH] Parse +CUSD responses.

[Denis Kenzior]

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

Hi Andrew,

> USSD.Initiate still returns a string, this may be wrong because
> when the network returns data coded in 8-bits we have no information
> on the character set used, everything is "user-specified" for the
> 8-bit coding in the 23.038 and technically we should return an array

Right now we should simply ignore 8bit data and return an error.

> I removed the "TODO: be able to send UCS2 string" because the string
> we send is first checked to be a valid USSD string so we already know
> it can be encoded with GSM 7-bits.

Actually valid_ussd_string will accept just about everything.

> +	/* All 7-bit coding schemes - there's no need to distinguish
> +	 * between the different schemes because the modem is tasked
> +	 * with presenting us with only raw 7-bit characters.
> +	 */
> +	if ((dcs & 0xf0) == 0x00 || dcs == 0x10 || (dcs & 0xf0) == 0x20 ||
> +			(dcs & 0xf0) == 0x30 || (dcs & 0xcc) == 0x40 ||
> +			(dcs & 0xfc) == 0x90 || (dcs & 0xf4) == 0xf0)
> +		converted = convert_gsm_to_utf8(content, length,
> +						NULL, NULL, 0);
> +

What about the evil set of DCSes which indicate a ISO639 2 character code 
preceding the message?

> -	sprintf(buf, "AT+CUSD=1,\"%*s\",%d", (int) written, converted, dcs);
> +	len = sprintf(buf, "AT+CUSD=1,\"");
> +	for (i = 0; i < written; i ++)
> +		len += sprintf(buf + len, "%02hhx", converted[i]);
> +	sprintf(buf + len, "\",%d", dcs);
>

So this part concerns me, see below.

> +	dump_response("cssu_notify", TRUE, result);

cssd_notify :)

> +
> +	g_at_chat_send(chat, "AT+CSCS=\"HEX\"", NULL, NULL, NULL, NULL);

This part of 27.007 is utterly and completely broken.  We have to be extra 
careful whenever we mess with CSCS.  Does this have any unfortunate side 
effects, in particular with +COPS, +CPUC, etc?  Also note that since we change 
the CSCS during phonebook import, any USSDs sent/received during this time 
won't actually work properly :)

Regards,
-Denis

Re: [PATCH] Parse +CUSD responses.

[andrzej zaborowski]

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

Hi,

2009/10/16 Denis Kenzior <denkenz@gmail.com>:
> Right now we should simply ignore 8bit data and return an error.

Ok, now we return the "unsupported" status on either an 8-bit response
or a dcs that we don't understand.

>
>> I removed the "TODO: be able to send UCS2 string" because the string
>> we send is first checked to be a valid USSD string so we already know
>> it can be encoded with GSM 7-bits.
>
> Actually valid_ussd_string will accept just about everything.

Ah true.  However I think we should never try to send a UCS2 USSD - we
would first need to know that the modem supports this (so a vendor
quirk set by the plugin would tell us this) and then that the network
supports this too.

Neither of the modems I tried yesterday was able to send a string with
DCS of 0x11 or 0x98 or 0xf8 (all of these are supposed to mean UCS2)
with language code prepended or without, only with 0x48 on TI Calypso.

>
>> +     /* All 7-bit coding schemes - there's no need to distinguish
>> +      * between the different schemes because the modem is tasked
>> +      * with presenting us with only raw 7-bit characters.
>> +      */
>> +     if ((dcs & 0xf0) == 0x00 || dcs == 0x10 || (dcs & 0xf0) == 0x20 ||
>> +                     (dcs & 0xf0) == 0x30 || (dcs & 0xcc) == 0x40 ||
>> +                     (dcs & 0xfc) == 0x90 || (dcs & 0xf4) == 0xf0)
>> +             converted = convert_gsm_to_utf8(content, length,
>> +                                             NULL, NULL, 0);
>> +
>
> What about the evil set of DCSes which indicate a ISO639 2 character code
> preceding the message?

The comment refers to this.  I think 27.007 tries to say the modem has
to take care of this and only give us the actual characters of the
string.

"-       if TE character set other than "HEX" (refer command Select TE
Character Set +CSCS): MT/TA converts GSM alphabet into current TE
character set according to rules of 3GPP TS 27.005 [24] Annex A

-       if TE character set is "HEX": MT/TA converts each 7-bit character of
GSM alphabet into two IRA character long hexadecimal number (e.g.
character ( (GSM 23) is presented as 17 (IRA 49 and 55))"

If we can assume that the same rules apply to encoding and decoding
then the TI Calypso modem and the nokia modems confirm this because
sending a string with DCS = 0x10 works without the language code and
doesn't work with the language prepended.

>
>> +     g_at_chat_send(chat, "AT+CSCS=\"HEX\"", NULL, NULL, NULL, NULL);
>
> This part of 27.007 is utterly and completely broken.  We have to be extra
> careful whenever we mess with CSCS.  Does this have any unfortunate side
> effects, in particular with +COPS, +CPUC, etc?  Also note that since we change
> the CSCS during phonebook import, any USSDs sent/received during this time
> won't actually work properly :)

Oh right, I had forgotten about +COPS etc, I noticed phonebook used
+CSCS and nothing else used it.  So if we can assume that the default
character set is GSM, then we can use it for 7-bit encoded responses,
but we will need to think of something else to interpret the 8-bit
data.  I changed the patch to leave the encoding alone and assume GSM
alphabet.

Regards

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Parse-CUSD-responses.patch --]
[-- Type: text/x-patch, Size: 5320 bytes --]

From 84fd8b9907bf212f6781d647bc85466f6a657b2c Mon Sep 17 00:00:00 2001
From: Andrzej Zaborowski <andrew.zaborowski@intel.com>
Date: Sat, 17 Oct 2009 00:45:15 +0200
Subject: [PATCH] Parse +CUSD responses.

---
 drivers/atmodem/ussd.c |  106 ++++++++++++++++++++++++++++++++++++++++++------
 src/ussd.c             |    3 +
 2 files changed, 96 insertions(+), 13 deletions(-)

diff --git a/drivers/atmodem/ussd.c b/drivers/atmodem/ussd.c
index 1d819a3..e96db90 100644
--- a/drivers/atmodem/ussd.c
+++ b/drivers/atmodem/ussd.c
@@ -40,25 +40,88 @@
 
 #include "atmodem.h"
 
+struct cusd_req {
+	ofono_ussd_cb_t cb;
+	void *data;
+	struct ofono_ussd *ussd;
+};
+
+static const char *cusd_prefix[] = { "+CUSD:", NULL };
 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;
+	char *converted = NULL;
+
+	g_at_result_iter_init(&iter, result);
+
+	if (!g_at_result_iter_next(&iter, "+CUSD:"))
+		return;
+
+	if (!g_at_result_iter_next_number(&iter, &status))
+		return;
+
+	if (!g_at_result_iter_next_string(&iter, &content))
+		goto out;
+
+	if (!g_at_result_iter_next_number(&iter, &dcs))
+		goto out;
+
+	/* All 7-bit coding schemes - there's no need to distinguish
+	 * between the different schemes because the modem is tasked
+	 * with presenting us with only raw 7-bit characters.
+	 */
+	if ((dcs & 0xf0) == 0x00 || dcs == 0x10 || (dcs & 0xf0) == 0x20 ||
+			(dcs & 0xf0) == 0x30 || (dcs & 0xcc) == 0x40 ||
+			(dcs & 0xfc) == 0x90 || (dcs & 0xf4) == 0xf0)
+		converted = convert_gsm_to_utf8((const guint8 *) content,
+						strlen(content), NULL, NULL,
+						0);
+
+	/* All 8-bit coding schemes are treated the same again.
+	 * TODO
+	 */
+	else if ((dcs & 0xcc) == 0x44 || (dcs & 0xfc) == 0x94 ||
+			(dcs & 0xf4) == 0xf4) {
+		ofono_error("8-bit coded USSD response received");
+		status = 4; /* Not supported */
+	}
+
+	/* No other encoding is mentioned in TS27007 7.15 */
+	else {
+		ofono_error("Unsupported USSD data coding scheme (%02x)", dcs);
+		status = 4; /* Not supported */
+	}
+
+out:
+	ofono_ussd_notify(ussd, status, converted);
+
+	if (converted)
+		g_free(converted);
+}
+
 static void cusd_request_cb(gboolean ok, GAtResult *result, gpointer user_data)
 {
-	struct cb_data *cbd = user_data;
-	ofono_ussd_cb_t cb = cbd->cb;
+	struct cusd_req *cbd = user_data;
 	struct ofono_error error;
 
 	dump_response("cusd_request_cb", ok, result);
 	decode_at_error(&error, g_at_result_final_response(result));
 
-	cb(&error, cbd->data);
+	cbd->cb(&error, cbd->data);
+
+	cusd_parse(result, cbd->ussd);
 }
 
 static void at_ussd_request(struct ofono_ussd *ussd, const char *str,
 				ofono_ussd_cb_t cb, void *data)
 {
 	GAtChat *chat = ofono_ussd_get_data(ussd);
-	struct cb_data *cbd = cb_data_new(cb, data);
+	struct cusd_req *cbd = g_try_new0(struct cusd_req, 1);
 	unsigned char *converted = NULL;
 	int dcs;
 	int max_len;
@@ -68,11 +131,12 @@ static void at_ussd_request(struct ofono_ussd *ussd, const char *str,
 	if (!cbd)
 		goto error;
 
+	cbd->cb = cb;
+	cbd->data = data;
+	cbd->ussd = ussd;
+
 	converted = convert_utf8_to_gsm(str, strlen(str), NULL, &written, 0);
 
-	/* TODO: Be able to convert to UCS2, although the standard does not
-	 * indicate that this is actually possible
-	 */
 	if (!converted)
 		goto error;
 	else {
@@ -83,12 +147,12 @@ static void at_ussd_request(struct ofono_ussd *ussd, const char *str,
 	if (written > max_len)
 		goto error;
 
-	sprintf(buf, "AT+CUSD=1,\"%*s\",%d", (int) written, converted, dcs);
+	sprintf(buf, "AT+CUSD=1,\"%*s\", %d", (int) written, converted, dcs);
 
 	g_free(converted);
 	converted = NULL;
 
-	if (g_at_chat_send(chat, buf, none_prefix,
+	if (g_at_chat_send(chat, buf, cusd_prefix,
 				cusd_request_cb, cbd, g_free) > 0)
 		return;
 
@@ -133,13 +197,28 @@ error:
 	CALLBACK_WITH_FAILURE(cb, data);
 }
 
-static gboolean at_ussd_register(gpointer user)
+static void cusd_notify(GAtResult *result, gpointer user_data)
+{
+	struct ofono_ussd *ussd = user_data;
+
+	dump_response("cusd_notify", TRUE, result);
+
+	cusd_parse(result, ussd);
+}
+
+static void at_ussd_register(gboolean ok, GAtResult *result, gpointer user)
 {
 	struct ofono_ussd *ussd = user;
+	GAtChat *chat = ofono_ussd_get_data(ussd);
 
-	ofono_ussd_register(ussd);
+	if (!ok) {
+		ofono_error("Could not enable CUSD notifications");
+		return;
+	}
+
+	g_at_chat_register(chat, "+CUSD:", cusd_notify, FALSE, ussd, NULL);
 
-	return FALSE;
+	ofono_ussd_register(ussd);
 }
 
 static int at_ussd_probe(struct ofono_ussd *ussd, unsigned int vendor,
@@ -148,7 +227,8 @@ static int at_ussd_probe(struct ofono_ussd *ussd, unsigned int vendor,
 	GAtChat *chat = data;
 
 	ofono_ussd_set_data(ussd, chat);
-	g_idle_add(at_ussd_register, ussd);
+
+	g_at_chat_send(chat, "AT+CUSD=1", NULL, at_ussd_register, ussd, NULL);
 
 	return 0;
 }
diff --git a/src/ussd.c b/src/ussd.c
index be50296..2bf378d 100644
--- a/src/ussd.c
+++ b/src/ussd.c
@@ -313,6 +313,9 @@ void ofono_ussd_notify(struct ofono_ussd *ussd, int status, const char *str)
 
 		reply = dbus_message_new_method_return(ussd->pending);
 
+		if (!str)
+			str = "";
+
 		dbus_message_iter_init_append(reply, &iter);
 
 		dbus_message_iter_append_basic(&iter, DBUS_TYPE_STRING,
-- 
1.6.1

Re: [PATCH] Parse +CUSD responses.

[Denis Kenzior]

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

Hi Andrew,

> > Actually valid_ussd_string will accept just about everything.
>
> Ah true.  However I think we should never try to send a UCS2 USSD - we
> would first need to know that the modem supports this (so a vendor
> quirk set by the plugin would tell us this) and then that the network
> supports this too.

You're absolutely right that we shouldn't, I was just pointing out the need to 
always be paranoid ;)

> > What about the evil set of DCSes which indicate a ISO639 2 character code
> > preceding the message?
>
> The comment refers to this.  I think 27.007 tries to say the modem has
> to take care of this and only give us the actual characters of the
> string.
>
> "-       if TE character set other than "HEX" (refer command Select TE
> Character Set +CSCS): MT/TA converts GSM alphabet into current TE
> character set according to rules of 3GPP TS 27.005 [24] Annex A

So this part really only talks about the modem performing character set 
conversion between GSM and IRA/UCS2/UTF8 or whatever is set by CSCS.  I 
suspect the DCS is simply never looked at by most networks/modems :)

Patch has been applied, with some fixes afterward.  I think this is good enough 
until we find a network / modem that supports this stuff properly.

Regards,
-Denis