Re: Issues about GPRS state

Re: Issues about GPRS state

[Denis Kenzior]

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

Hi Caiwen,

On 08/19/2011 04:46 AM, Zhang, Caiwen wrote:
> Hi,
> 
> I find there may be some issues about GPRS state, please help check them.
> 
> (1) gprs->driver_attached is set to TURE just according to GPRS registration state.
> It may cause the following issue.
> 
> 1 ofonod[619]: PCUI: < \r\n^SRVST:1\r\n\r\n+CREG: 2\r\n\r\n+CGREG: 2\r\n\r\n^SRVST:2\r\n\r\n+CREG: 1, A807, A72B71\r\n\r\n+CGREG: 0\r\n
> 2 ofonod[619]: src/network.c:current_operator_callback() 0xa046ea0, 0xa0476e8
> 3 ofonod[619]: src/gprs.c:netreg_status_changed() 2  		#driver_attached = FALSE
> 4 ofonod[619]: src/cbs.c:cbs_location_changed() 2, -1, -1, -1, (null)(null)
> 5 ofonod[619]: src/cbs.c:cbs_location_changed() 1, 0, 0
> 6 ofonod[619]: src/gprs.c:netreg_status_changed() 2
> 7 ofonod[619]: src/cbs.c:cbs_location_changed() 2, -1, -1, -1, (null)(null)
> 8 ofonod[619]: src/gprs.c:ofono_gprs_status_notify() /huawei0 status 2
> 9 ofonod[619]: src/gprs.c:netreg_status_changed() 1     
> 10 ofonod[619]: src/cbs.c:cbs_location_changed() 1, 43015, 10955633, -1, (null)(null)
> 11 ofonod[619]: src/gprs.c:ofono_gprs_status_notify() /huawei0 status 0
> 12 ofonod[619]: PCUI: > AT+CSCB=0,"0,25,38,50,100,136,256,4352-4356"\r
> 13 ofonod[619]: PCUI: < \r\nOK\r\n
> 14 ofonod[619]: PCUI: > AT+CRSM=192,28437,0,0,255\r
> 15 ofonod[619]: PCUI: < \r\n+CREG: 1, A807, A72B71\r\n\r\n+CGREG: 1, A807, A72B71\r\n
> 16 ofonod[619]: src/gprs.c:netreg_status_changed() 1
> 17 ofonod[619]: src/cbs.c:cbs_location_changed() 1, 43015, 10955633, -1, (null)(null)
> 18 ofonod[619]: src/gprs.c:ofono_gprs_status_notify() /huawei0 status 1    #driver_attached = TRUE
> 19 ofonod[619]: PCUI: < \r\n+CRSM: 106,130,""\r\n\r\nOK\r\n
> 20 ofonod[619]: PCUI: > AT+CGATT=0\r
> 21 ofonod[619]: PCUI: < \r\n+CREG: 1, A807, A72B71\r\n\r\n+CGREG: 0\r\n\r\nOK\r\n
> 22 ofonod[619]: src/gprs.c:netreg_status_changed() 1
> 23 ofonod[619]: src/cbs.c:cbs_location_changed() 1, 43015, 10955633, -1, (null)(null)
> 24 ofonod[619]: src/gprs.c:ofono_gprs_status_notify() /huawei0 status 0
> 25 ofonod[619]: src/gprs.c:gprs_attach_callback() /huawei0 error = 0
> 
> At line 3 gprs_netreg_update() is called, driver_attached is set to FALSE. Due to there
> are some AT command in the queue, AT+CGATT=0 is not sent out immediately(till at line 20).
> At line 18, driver_attached is set to TRUE. In fact after line 21, GPRS is detached.
> In src/gprs.c there is only one place to attach GPRS, it is in gprs_netreg_update().
> due to driver_attached is TRUE, gprs_netreg_update() will always return before
> gprs->driver->set_attached() invoked.
>

It seems to me the issue is that we do not check the FLAG_ATTACHING
properly inside ofono_gprs_status_notify.  I've already proposed a fix
for this a while ago, please see it again (attached).

> For this issue I have submit two patches, please see attached.
> 
> (2) After receive "NW DETACH"/"ME DETACH" unsolicited message, GPRS is not re-attached. It will
> cause can not connect GPRS connection any more.
> 
> (3) In AT modem GPRS driver, the attach status query function(.attached_status) is implememted
> as query the GPRS registration status. Is it by mistake or intended? It is very confusable.
> 

Attached means whether we're actually attached to PS service.  The
reason the driver method is called attached_status is that some modems
(e.g. isi) do not have (and rightfully so) a concept of 'PS registration
status' as returned by CGREG.  So yes, it is on purpose and it is not a
mistake.

Regards,
-Denis

[-- Attachment #2: 0001-gprs-Experimental-fix-for-twitchy-Huawei-firmware.patch --]
[-- Type: text/plain, Size: 870 bytes --]

>From ddbacc2dda8db476924181bff49356087c8ccc70 Mon Sep 17 00:00:00 2001
From: Denis Kenzior <denkenz@gmail.com>
Date: Mon, 25 Jul 2011 21:42:35 -0500
Subject: [PATCH] gprs: Experimental fix for twitchy Huawei firmware

---
 src/gprs.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/src/gprs.c b/src/gprs.c
index acbfa56..b9e3105 100644
--- a/src/gprs.c
+++ b/src/gprs.c
@@ -2108,6 +2108,14 @@ void ofono_gprs_status_notify(struct ofono_gprs *gprs, int status)
 		return;
 	}
 
+	/*
+	 * If we're already taking action, e.g. attaching or detaching, then
+	 * ignore this notification for now, we will take appropriate action
+	 * after the set_attach operation has completed
+	 */
+	if (gprs->flags & GPRS_FLAG_ATTACHING)
+		return;
+
 	/* We registered without being powered */
 	if (gprs->powered == FALSE)
 		goto detach;
-- 
1.7.3.4

Re: Issues about GPRS state

[Denis Kenzior]

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

Hi Caiwen,

On 08/21/2011 09:06 PM, Zhang, Caiwen wrote:
> Hi Denis,
> 
>>> At line 3 gprs_netreg_update() is called, driver_attached is set to
>>> FALSE. Due to there are some AT command in the queue, AT+CGATT=0 is
>> not sent out immediately(till at line 20).
>>> At line 18, driver_attached is set to TRUE. In fact after line 21,
>> GPRS is detached.
>>> In src/gprs.c there is only one place to attach GPRS, it is in
>> gprs_netreg_update().
>>> due to driver_attached is TRUE, gprs_netreg_update() will always
>>> return before
>>> gprs->driver->set_attached() invoked.
>>>
>>
>> It seems to me the issue is that we do not check the FLAG_ATTACHING
>> properly inside ofono_gprs_status_notify.  I've already proposed a fix
>> for this a while ago, please see it again (attached).
>>
> 
> Yes, it is a better solution. I remember we have discussed it before,
> please don't forget apply it.
> 

This patch has been pushed upstream now.

>>> For this issue I have submit two patches, please see attached.
>>>
>>> (2) After receive "NW DETACH"/"ME DETACH" unsolicited message, GPRS
>> is
>>> not re-attached. It will cause can not connect GPRS connection any
>> more.
>>>
>>> (3) In AT modem GPRS driver, the attach status query
>>> function(.attached_status) is implememted as query the GPRS
>> registration status. Is it by mistake or intended? It is very
>> confusable.
>>>
>>
>> Attached means whether we're actually attached to PS service.  The
>> reason the driver method is called attached_status is that some modems
>> (e.g. isi) do not have (and rightfully so) a concept of 'PS
>> registration status' as returned by CGREG.  So yes, it is on purpose
>> and it is not a mistake.
>>
> 
> Please have a look https://bugs.meego.com/show_bug.cgi?id=17921. The issue the
> tester encountered when verified this bug should be issue (2) mentioned above.
> 

That particular bug doesn't have AT traces for me to tell what is going
on properly.  Can you please ask the tester to update the logs with
OFONO_AT_DEBUG set appropriately?

Thanks,
-Denis

Issues about GPRS state

[Zhang, Caiwen]

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

Hi,

I find there may be some issues about GPRS state, please help check them.

(1) gprs->driver_attached is set to TURE just according to GPRS registration state.
It may cause the following issue.

1 ofonod[619]: PCUI: < \r\n^SRVST:1\r\n\r\n+CREG: 2\r\n\r\n+CGREG: 2\r\n\r\n^SRVST:2\r\n\r\n+CREG: 1, A807, A72B71\r\n\r\n+CGREG: 0\r\n
2 ofonod[619]: src/network.c:current_operator_callback() 0xa046ea0, 0xa0476e8
3 ofonod[619]: src/gprs.c:netreg_status_changed() 2  		#driver_attached = FALSE
4 ofonod[619]: src/cbs.c:cbs_location_changed() 2, -1, -1, -1, (null)(null)
5 ofonod[619]: src/cbs.c:cbs_location_changed() 1, 0, 0
6 ofonod[619]: src/gprs.c:netreg_status_changed() 2
7 ofonod[619]: src/cbs.c:cbs_location_changed() 2, -1, -1, -1, (null)(null)
8 ofonod[619]: src/gprs.c:ofono_gprs_status_notify() /huawei0 status 2
9 ofonod[619]: src/gprs.c:netreg_status_changed() 1     
10 ofonod[619]: src/cbs.c:cbs_location_changed() 1, 43015, 10955633, -1, (null)(null)
11 ofonod[619]: src/gprs.c:ofono_gprs_status_notify() /huawei0 status 0
12 ofonod[619]: PCUI: > AT+CSCB=0,"0,25,38,50,100,136,256,4352-4356"\r
13 ofonod[619]: PCUI: < \r\nOK\r\n
14 ofonod[619]: PCUI: > AT+CRSM=192,28437,0,0,255\r
15 ofonod[619]: PCUI: < \r\n+CREG: 1, A807, A72B71\r\n\r\n+CGREG: 1, A807, A72B71\r\n
16 ofonod[619]: src/gprs.c:netreg_status_changed() 1
17 ofonod[619]: src/cbs.c:cbs_location_changed() 1, 43015, 10955633, -1, (null)(null)
18 ofonod[619]: src/gprs.c:ofono_gprs_status_notify() /huawei0 status 1    #driver_attached = TRUE
19 ofonod[619]: PCUI: < \r\n+CRSM: 106,130,""\r\n\r\nOK\r\n
20 ofonod[619]: PCUI: > AT+CGATT=0\r
21 ofonod[619]: PCUI: < \r\n+CREG: 1, A807, A72B71\r\n\r\n+CGREG: 0\r\n\r\nOK\r\n
22 ofonod[619]: src/gprs.c:netreg_status_changed() 1
23 ofonod[619]: src/cbs.c:cbs_location_changed() 1, 43015, 10955633, -1, (null)(null)
24 ofonod[619]: src/gprs.c:ofono_gprs_status_notify() /huawei0 status 0
25 ofonod[619]: src/gprs.c:gprs_attach_callback() /huawei0 error = 0

At line 3 gprs_netreg_update() is called, driver_attached is set to FALSE. Due to there
are some AT command in the queue, AT+CGATT=0 is not sent out immediately(till@line 20).
At line 18, driver_attached is set to TRUE. In fact after line 21, GPRS is detached.
In src/gprs.c there is only one place to attach GPRS, it is in gprs_netreg_update().
due to driver_attached is TRUE, gprs_netreg_update() will always return before
gprs->driver->set_attached() invoked.

For this issue I have submit two patches, please see attached.

(2) After receive "NW DETACH"/"ME DETACH" unsolicited message, GPRS is not re-attached. It will
cause can not connect GPRS connection any more.

(3) In AT modem GPRS driver, the attach status query function(.attached_status) is implememted
as query the GPRS registration status. Is it by mistake or intended? It is very confusable.


Thanks!

Caiwen





[-- Attachment #2: attachment.mht --]
[-- Type: message/rfc822, Size: 52 bytes --]



WzxlbWFpbC5tZXNzYWdlLk1lc3NhZ2UgaW5zdGFuY2UgYXQgMHg3ZjFiMDM3Y2VkZDA+XQ==

[-- Attachment #3: attachment.mht --]
[-- Type: message/rfc822, Size: 52 bytes --]



WzxlbWFpbC5tZXNzYWdlLk1lc3NhZ2UgaW5zdGFuY2UgYXQgMHg3ZjFiMDM3Y2VlYTg+XQ==

RE: Issues about GPRS state

[Zhang, Caiwen]

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

Hi Denis,

> > At line 3 gprs_netreg_update() is called, driver_attached is set to
> > FALSE. Due to there are some AT command in the queue, AT+CGATT=0 is
> not sent out immediately(till at line 20).
> > At line 18, driver_attached is set to TRUE. In fact after line 21,
> GPRS is detached.
> > In src/gprs.c there is only one place to attach GPRS, it is in
> gprs_netreg_update().
> > due to driver_attached is TRUE, gprs_netreg_update() will always
> > return before
> > gprs->driver->set_attached() invoked.
> >
> 
> It seems to me the issue is that we do not check the FLAG_ATTACHING
> properly inside ofono_gprs_status_notify.  I've already proposed a fix
> for this a while ago, please see it again (attached).
> 

Yes, it is a better solution. I remember we have discussed it before,
please don't forget apply it.

> > For this issue I have submit two patches, please see attached.
> >
> > (2) After receive "NW DETACH"/"ME DETACH" unsolicited message, GPRS
> is
> > not re-attached. It will cause can not connect GPRS connection any
> more.
> >
> > (3) In AT modem GPRS driver, the attach status query
> > function(.attached_status) is implememted as query the GPRS
> registration status. Is it by mistake or intended? It is very
> confusable.
> >
> 
> Attached means whether we're actually attached to PS service.  The
> reason the driver method is called attached_status is that some modems
> (e.g. isi) do not have (and rightfully so) a concept of 'PS
> registration status' as returned by CGREG.  So yes, it is on purpose
> and it is not a mistake.
> 

Please have a look https://bugs.meego.com/show_bug.cgi?id=17921. The issue the
tester encountered when verified this bug should be issue (2) mentioned above.


Regards

Caiwen

RE: Issues about GPRS state

[Zhang, Caiwen]

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

Hi Denis,

> >
> >>> At line 3 gprs_netreg_update() is called, driver_attached is set to
> >>> FALSE. Due to there are some AT command in the queue, AT+CGATT=0 is
> >> not sent out immediately(till at line 20).
> >>> At line 18, driver_attached is set to TRUE. In fact after line 21,
> >> GPRS is detached.
> >>> In src/gprs.c there is only one place to attach GPRS, it is in
> >> gprs_netreg_update().
> >>> due to driver_attached is TRUE, gprs_netreg_update() will always
> >>> return before
> >>> gprs->driver->set_attached() invoked.
> >>>
> >>
> >> It seems to me the issue is that we do not check the FLAG_ATTACHING
> >> properly inside ofono_gprs_status_notify.  I've already proposed a
> fix
> >> for this a while ago, please see it again (attached).
> >>
> >
> > Yes, it is a better solution. I remember we have discussed it before,
> > please don't forget apply it.
> >
> 
> This patch has been pushed upstream now.
> 
Great!

> >>> For this issue I have submit two patches, please see attached.
> >>>
> >>> (2) After receive "NW DETACH"/"ME DETACH" unsolicited message, GPRS
> >> is
> >>> not re-attached. It will cause can not connect GPRS connection any
> >> more.
> >>>
> >>> (3) In AT modem GPRS driver, the attach status query
> >>> function(.attached_status) is implememted as query the GPRS
> >> registration status. Is it by mistake or intended? It is very
> >> confusable.
> >>>
> >>
> >> Attached means whether we're actually attached to PS service.  The
> >> reason the driver method is called attached_status is that some
> modems
> >> (e.g. isi) do not have (and rightfully so) a concept of 'PS
> >> registration status' as returned by CGREG.  So yes, it is on purpose
> >> and it is not a mistake.
> >>
> >
> > Please have a look https://bugs.meego.com/show_bug.cgi?id=17921. The
> issue the
> > tester encountered when verified this bug should be issue (2)
> mentioned above.
> >
> 
> That particular bug doesn't have AT traces for me to tell what is going
> on properly.  Can you please ask the tester to update the logs with
> OFONO_AT_DEBUG set appropriately?
> 
I asked the tester to do it but she couldn't reproduce it. 

From the log, there is neither CGREG nor CREG unsolicited message pop up
after "NW DETACH"/"ME DETACH" unsolicited message received. The registration
status is still registered, if take no action, GPRS will always be detached.

Maybe at this time should try to attach it directly, or try to query GPRS status
and then process it according to the result.

Regards
Caiwen