Skip unsolicied CREG/CGREG correctly while checking GPRS attach status

This patch is to skip unsolicited CGREG while checking the GPRS attach status, especially for Huawei modem which sends lac and ci strings in unquoted format, and casues at_util_parse_reg() return lac value as status.

---
 drivers/atmodem/atutil.c |   12 ++++++++----
 gatchat/gatresult.c      |    5 +++++
 gatchat/gatresult.h      |    1 +
 3 files changed, 14 insertions(+), 4 deletions(-)

index b6f0d92..c190aa2 100644
@@ -243,13 +243,11 @@ gboolean at_util_parse_reg(GAtResult *result,

const char *prefix,
if (g_at_result_iter_next_number(&iter, &s) == FALSE)
continue;

- /* Some firmware will report bogus lac/ci when unregistered */
- if (s != 1 && s != 5)
- goto out;
-
switch (vendor) {
case OFONO_VENDOR_HUAWEI:
case OFONO_VENDOR_NOVATEL:
+ if (g_at_result_iter_eol(&iter))
+ continue;
r = g_at_result_iter_next_unquoted_string(&iter, &str);

if (r == TRUE)
@@ -257,6 +255,8 @@ gboolean at_util_parse_reg(GAtResult *result, const char *prefix,
else
goto out;

+ if (g_at_result_iter_eol(&iter))
+ continue;
r = g_at_result_iter_next_unquoted_string(&iter, &str);

if (r == TRUE)
@@ -266,11 +266,15 @@ gboolean at_util_parse_reg(GAtResult *result, const char *prefix,

break;
default:
+ if (g_at_result_iter_eol(&iter))
+ continue;
if (g_at_result_iter_next_string(&iter, &str) == TRUE)
l = strtol(str, NULL, 16);
else
goto out;

+ if (g_at_result_iter_eol(&iter))
+ continue;
if (g_at_result_iter_next_string(&iter, &str) == TRUE)
c = strtol(str, NULL, 16);
else

index 8a6dfae..c919305 100644
@@ -39,6 +39,11 @@ void g_at_result_iter_init(GAtResultIter *iter,

GAtResult *result)
iter->line_pos = 0;
}

+gboolean g_at_result_iter_eol(GAtResultIter *iter)
+{
+ return iter->line_pos >= strlen(iter->l->data);
+}
+
gboolean g_at_result_iter_next(GAtResultIter *iter, const char *prefix) {
char *line;

index a74741f..33c0b2c 100644
@@ -46,6 +46,7 @@ struct _GAtResultIter {
 typedef struct _GAtResultIter GAtResultIter;
 
 void g_at_result_iter_init(GAtResultIter *iter, GAtResult *result);
+gboolean g_at_result_iter_eol(GAtResultIter *iter);
 
 gboolean g_at_result_iter_next(GAtResultIter *iter, const char

*prefix);
gboolean g_at_result_iter_open_list(GAtResultIter *iter);

1.7.2.2

We like to see patches split on a top level directory boundary. Please see the 'Submitting patches' section in the HACKING document.

Anyhow, can you please try the attached patch and see if it solves your problems.

Regards,
-Denis

From: Denis Kenzior <[email protected]>
Date: Mon, 6 Dec 2010 21:36:54 -0600
Subject: [PATCH] atutil: Fix parsing of un-quoted CREG / CGREG

On broken hardware like the Huawei, it is possible to receive both an unsolicited and a solicited version of the CREG / CGREG within within the same response set. Skipping of the unsolicited version was not handled correctly. This attempts to fix this issue.

---
 drivers/atmodem/atutil.c |   19 +++++++++++++++++--
 1 files changed, 17 insertions(+), 2 deletions(-)

index b6f0d92..427b098 100644
@@ -240,8 +240,23 @@ gboolean at_util_parse_reg(GAtResult *result, const char *prefix,
 		g_at_result_iter_next_number(&iter, &m);
 
 		/* Sometimes we get an unsolicited CREG/CGREG here, skip it */
-		if (g_at_result_iter_next_number(&iter, &s) == FALSE)
-			continue;
+		switch (vendor) {
+		case OFONO_VENDOR_HUAWEI:
+		case OFONO_VENDOR_NOVATEL:
+			r = g_at_result_iter_next_unquoted_string(&iter, &str);
+
+			if (r == FALSE || strlen(str) != 1)
+				continue;
+
+			s = strtol(str, NULL, 10);
+
+			break;
+		default:
+			if (g_at_result_iter_next_number(&iter, &s) == FALSE)
+				continue;
+
+			break;
+		}
 
 		/* Some firmware will report bogus lac/ci when unregistered */
 		if (s != 1 && s != 5)

1.7.0.4

No, this function can help to ignore unsolicited response properly. I think the difference between solicited and unsolicited CGREG is the number of parameters after +CGREG:, so it would be nice if we have a utility function to know we are at the end of AT return string.

This is what we want to parse the state info from the solicited response:
+CGREG: <n>, <state>, <lac>, <ci>
but in a race condition, we received this unsolicited response from Huawei modem:
+CGREG: <state>, <lac>, <ci>

In your proposed patch, it bypass the unsolicited response only if the 2nd number after +CGREG: is not single digit, however if the lac is single digit, we will still get the wrong state number.

So I still believe we should check whether we are at the end of the return string before parsing the unquoted lac and ci. If we are at the end of the string, then this is a unsolicited code and we should ignore it. How do you think?

Hi Tonny,

The lac is given as a 2 byte number and should always be printed as four hexadecimal letters. If this is not the case, then you should have a serious talk with your vendor.

I'm against this, if you really want to guard against this, then use the approach similar to comprehension_tlv_iter_copy in src/simutil.[ch]. You can use that to 'peek' ahead and figure out whether the response contains 3 or 4 elements.

But again, you would not be having this issue in the first place if your vendor followed standards ;)

Regards,
-Denis

Hi Denis,

Well, if this (i.e. lac are 4 hex-digits) is the case, then I totally agree with you that it could ignore unsolicited response properly in at_util_parse_reg() for my case. Thank you so much for the coach :)

Best Regards,
Tonny