On 12/03/2010 09:20 AM, Tonny Tzeng wrote:
> 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 +
We like to see patches split on a top level directory boundary. Please
see the 'Submitting patches' section in the HACKING document.
> 3 files changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/atmodem/atutil.c b/drivers/atmodem/atutil.c
> index b6f0d92..c190aa2 100644
> --- a/drivers/atmodem/atutil.c
> +++ b/drivers/atmodem/atutil.c
> @@ -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;
> -
Skipping this check is still a bad idea.
> switch (vendor) {
> case OFONO_VENDOR_HUAWEI:
> case OFONO_VENDOR_NOVATEL:
> + if (g_at_result_iter_eol(&iter))
> + continue;
Coding style item M1.
> 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;
Coding style item M1.
> 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;
Coding style item M1. And continuing is not the right thing to do here
> 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;
Ditto
> if (g_at_result_iter_next_string(&iter, &str) == TRUE)
> c = strtol(str, NULL, 16);
> else
> diff --git a/gatchat/gatresult.c b/gatchat/gatresult.c
> index 8a6dfae..c919305 100644
> --- a/gatchat/gatresult.c
> +++ b/gatchat/gatresult.c
> @@ -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);
> +}
> +
And I pretty much don't see the need for this. It doesn't really help
you here.
> gboolean g_at_result_iter_next(GAtResultIter *iter, const char *prefix)
> {
> char *line;
> diff --git a/gatchat/gatresult.h b/gatchat/gatresult.h
> index a74741f..33c0b2c 100644
> --- a/gatchat/gatresult.h
> +++ b/gatchat/gatresult.h
> @@ -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);
Anyhow, can you please try the attached patch and see if it solves your
problems.
Regards,
-Denis
>From f5473c398a2e29a98c74ad4c98a427564e25d9d0 Mon Sep 17 00:00:00 2001
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(-)
diff --git a/drivers/atmodem/atutil.c b/drivers/atmodem/atutil.c
index b6f0d92..427b098 100644
--- a/drivers/atmodem/atutil.c
+++ b/drivers/atmodem/atutil.c
@@ -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
_______________________________________________
ofono mailing list
[email protected]
http://lists.ofono.org/listinfo/ofono
