Re: Issues about GPRS state

[Denis Kenzior <denkenz@gmail.com>]

[-- 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