2:20 a.m.
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(-)
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;
-
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
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);
+}
+
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);
--
1.7.2.2
2:41 p.m.
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
1:09 a.m.
> 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.
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?
> 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
1:52 a.m.
Hi Tonny,
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.
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.
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?
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
2:08 a.m.
Hi Denis,
On Sat, Dec 11, 2010 at 10:52 PM, Denis Kenzior <denkenz(a)gmail.com> wrote:
Hi Tonny,
> 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.
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.
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
4299
days inactive
4307
days old
4 comments
2 participants
participants (2)
-
Denis Kenzior -
Tonny Tzeng