Re: [PATCH v2] Skip unsolicied CREG/CGREG correctly while checking GPRS attach status

2010-12-11 Thread Tonny Tzeng
Hi Denis,

On Sat, Dec 11, 2010 at 10:52 PM, Denis Kenzior denk...@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
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH] Specify vendor ID for Huawei modem while creating GPRS context

2010-12-11 Thread Tonny Tzeng
Hi Denis,

No, you don't.  You are confusing gprs atom and the gprs-context atom.
 Grep for VENDOR in drivers/atmodem/gprs-context.c if you do not believe
 me.  There is no vendor specific code in the atmodem gprs-context driver.


Yes, my bad, I misunderstood the program flow...
So, is huawei_post_online() in plugins/huawei.c the right place to pass the
vendor ID while calling to ofono_gprs_create()?

Together with the patch to fix parsing of unquoted CGREG, now I can get the
CGREG status correctly from the Huawei modem.  :)  Thanks!

Best Regards,
Tonny
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH] Skip unsolicited CREG/CGREG correctly

2010-12-03 Thread Tonny Tzeng
Hi Denis,

Thanks for reviewing.

On Fri, Dec 3, 2010 at 5:22 AM, Denis Kenzior denk...@gmail.com wrote:
               /* Some firmware will report bogus lac/ci when unregistered */
 +             /* in this case, we should skip it                          */
               if (s != 1  s != 5)
 -                     goto out;
 +                     continue;

 And this fix is wrong.  what this is doing is skipping the parsing of
 the lac/ci values if we're not registered / roaming.  Using continue
 here will cause the parser to fail for those cases.

But if we jump to label 'out', this routine will return the wrong
status code parsed from a unsolicited CREG/CGREG.  For example, in my
case, the real status value goes to the mode argument, and the status
argument got lac value.

Since huawei modems send these strings without quotes, I think we need
different logic to parse these values, for example, we may need a new
function for testing whether there are more codes left?

 What you probably meant was continuing if the status was not between 1
 and 5.  But even that won't really help you if an lac of 1..5 is
 encountered ;)

Yes, you got my point, I will propose another way to avoid this.  Thanks.

Best Regards,
Tonny
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


[PATCH v2] Skip unsolicied CREG/CGREG correctly while checking GPRS attach status

2010-12-03 Thread Tonny Tzeng
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


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


[PATCH] Specify vendor ID for Huawei modem while creating GPRS context

2010-12-03 Thread Tonny Tzeng
---
 plugins/huawei.c |8 
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/plugins/huawei.c b/plugins/huawei.c
index 25dfaca..32cf70d 100644
--- a/plugins/huawei.c
+++ b/plugins/huawei.c
@@ -454,7 +454,7 @@ static void huawei_disconnect(gpointer user_data)
data-sim_state == HUAWEI_SIM_STATE_INVALID_CS) {
ofono_info(Reopened GPRS context channel);
 
-   data-gc = ofono_gprs_context_create(modem, 0, atmodem,
+   data-gc = ofono_gprs_context_create(modem, OFONO_VENDOR_HUAWEI,
atmodem,
data-modem);
 
if (data-gprs  data-gc)
@@ -631,12 +631,12 @@ static void huawei_post_online(struct ofono_modem
*modem)
 
if (data-sim_state == HUAWEI_SIM_STATE_VALID ||
data-sim_state == HUAWEI_SIM_STATE_INVALID_CS) {
-   data-gprs = ofono_gprs_create(modem, 0, atmodem, data-pcui);
+   data-gprs = ofono_gprs_create(modem, OFONO_VENDOR_HUAWEI, 
atmodem,
data-pcui);
if (data-ndis == TRUE)
-   data-gc = ofono_gprs_context_create(modem, 0,
+   data-gc = ofono_gprs_context_create(modem, 
OFONO_VENDOR_HUAWEI,
huaweimodem, data-pcui);
else
-   data-gc = ofono_gprs_context_create(modem, 0,
+   data-gc = ofono_gprs_context_create(modem, 
OFONO_VENDOR_HUAWEI,
atmodem, data-modem);
 
if (data-gprs  data-gc)
-- 
1.7.2.2


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


[PATCH] Skip unsolicited CREG/CGREG correctly

2010-12-02 Thread Tonny Tzeng
This patch skip unsolicited CREG/CGREG correctly.

Signed-off-by: Tonny Tzeng tonny.tz...@gmail.com
---
 drivers/atmodem/atutil.c |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/atmodem/atutil.c b/drivers/atmodem/atutil.c
index b6f0d92..2ca7b44 100644
--- a/drivers/atmodem/atutil.c
+++ b/drivers/atmodem/atutil.c
@@ -244,8 +244,9 @@ gboolean at_util_parse_reg(GAtResult *result, const
char *prefix,
continue;
 
/* Some firmware will report bogus lac/ci when unregistered */
+   /* in this case, we should skip it  */
if (s != 1  s != 5)
-   goto out;
+   continue;
 
switch (vendor) {
case OFONO_VENDOR_HUAWEI:
-- 
1.7.2.2


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [ofono] vendor code

2010-11-30 Thread Tonny Tzeng
Hi Dennis,

On Tue, Nov 30, 2010 at 8:22 PM, Denis Kenzior denk...@gmail.com wrote:
 While debugging a connection issue using huawei EM770W modem, I added
 a DBG code to print out the status value received in at_cgreg_cb() in
 drivers/atmodem/gprs.c, I found the status got from the
 at_util_parse_reg() is incorrect sometimes, because ofono does not set
 vendor ID in the gprs_data, and it looks to me the vendor code is
 required so that the at_util_parse_reg() will read unquoted strings in
 the solicited events for huawei modem.  For example, when we got
 +CGREG: 1, 2833, 1231B60 followed by +CGREG: 2,1, 2733, 1B60
 unsolicited codes, the status parsed from this code is 2833 (i.e. the
 1st code), instead of the expected 1 in the 2nd code.

 I suggest you complain to the vendor that they do not follow standards,
 and ask them to fix their firmware.

Any possibility that this is a race condition?  Just while ofono
sending AT+CGREG? to poll the CGREG state, modem already sent out
unsolicited CGREG?  I saw at_util_parse_reg() also tries to skip
unsolicited CREG/CGREG, so it looks like this is valid concern?

 It looks like all modem plugins invoke ofono_gprs_context_create()
 with vendor=0, is this on purpose?  then how could at_util_parse_reg()
 parses strings correctly without knowing vendor code?

 This can be added to the huawei driver, however this won't help you
 completely as the firmware still reports bizarre values.  Care to send a
 patch?

Sure I will patch and send out the huawei plugins patch, but since I
am not familiar with the overall ofono code, so I'd like to verify
what my understanding so far -- modem plugins should send the vendor
ID while invoke ofono_gprs_context_create()?  Would you please
confirm?  Thanks.


 Regards,
 -Denis

___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono