[PATCH] Fix modem doesn't auto register to network issue

[PATCH] Fix modem doesn't auto register to network issue

[caiwen.zhang]

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

From: Caiwen Zhang <caiwen.zhang@intel.com>

Some modem may return registration status whitch is out of the range
(0-5). it will cause the modem don't auto register to network when
network atom initializes.
---
 src/network.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/network.c b/src/network.c
index d1bfca6..a1e317d 100644
--- a/src/network.c
+++ b/src/network.c
@@ -1481,9 +1481,9 @@ static void init_registration_status(const struct ofono_error *error,
 	}
 
 	if (netreg->mode != NETWORK_REGISTRATION_MODE_MANUAL &&
-		(status == NETWORK_REGISTRATION_STATUS_NOT_REGISTERED ||
-			status == NETWORK_REGISTRATION_STATUS_DENIED ||
-			status == NETWORK_REGISTRATION_STATUS_UNKNOWN)) {
+		(status != NETWORK_REGISTRATION_STATUS_REGISTERED &&
+			status != NETWORK_REGISTRATION_STATUS_SEARCHING &&
+			status != NETWORK_REGISTRATION_STATUS_ROAMING)) {
 		if (netreg->driver->register_auto != NULL)
 			netreg->driver->register_auto(netreg, init_register,
 							netreg);
-- 
1.7.9.5

Re: [PATCH] Fix modem doesn't auto register to network issue

[Denis Kenzior]

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

Hi Caiwen,

On 09/11/2013 12:33 PM, caiwen.zhang(a)intel.com wrote:
> From: Caiwen Zhang <caiwen.zhang@intel.com>
>
> Some modem may return registration status whitch is out of the range
> (0-5). it will cause the modem don't auto register to network when
> network atom initializes.

oFono core will not validate values outside what is defined in 27.007. 
If your driver reports such values, then it is an obvious bug in the driver.

Please fix your modem driver.

> ---
>   src/network.c |    6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>

Regards,
-Denis

RE: [PATCH] Fix modem doesn't auto register to network issue

[Zhang, Caiwen]

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

Hi Denis,

> > Some modem may return registration status whitch is out of the range
> > (0-5). it will cause the modem don't auto register to network when
> > network atom initializes.
> 
> oFono core will not validate values outside what is defined in 27.007.
> If your driver reports such values, then it is an obvious bug in the driver.
> 
> Please fix your modem driver.
> 

(status == NETWORK_REGISTRATION_STATUS_NOT_REGISTERED ||
		status == NETWORK_REGISTRATION_STATUS_DENIED ||
		status == NETWORK_REGISTRATION_STATUS_UNKNOWN)

Above checking tries to list the cases that should trigger auto network registration but for the modems which
extend network registration status may miss some cases.

With the new checking:
		(status != NETWORK_REGISTRATION_STATUS_REGISTERED &&
			status != NETWORK_REGISTRATION_STATUS_SEARCHING &&
			status != NETWORK_REGISTRATION_STATUS_ROAMING))
it tries to exclude the cases that not required to trigger auto network registration. In this way, it can avoid
above issue.

I am not sure whether other modem has the same issue. This way should be more make sense.

By the way, it should be better to change the status from "int" to " enum network_registration_status" to avoid modem driver pass
in value we don't expect.

Regards,
Caiwen

Re: [PATCH] Fix modem doesn't auto register to network issue

[Denis Kenzior]

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

Hi Caiwen,

On 09/11/2013 09:48 PM, Zhang, Caiwen wrote:
> Hi Denis,
>
>>> Some modem may return registration status whitch is out of the range
>>> (0-5). it will cause the modem don't auto register to network when
>>> network atom initializes.
>>
>> oFono core will not validate values outside what is defined in 27.007.
>> If your driver reports such values, then it is an obvious bug in the driver.
>>
>> Please fix your modem driver.
>>
>
> (status == NETWORK_REGISTRATION_STATUS_NOT_REGISTERED ||
> 		status == NETWORK_REGISTRATION_STATUS_DENIED ||
> 		status == NETWORK_REGISTRATION_STATUS_UNKNOWN)
>
> Above checking tries to list the cases that should trigger auto network registration but for the modems which
> extend network registration status may miss some cases.
>
> With the new checking:
> 		(status != NETWORK_REGISTRATION_STATUS_REGISTERED &&
> 			status != NETWORK_REGISTRATION_STATUS_SEARCHING &&
> 			status != NETWORK_REGISTRATION_STATUS_ROAMING))
> it tries to exclude the cases that not required to trigger auto network registration. In this way, it can avoid
> above issue.

Since we do not know what the meaning of the extended value actually is, 
we can be wrong in either situation.  The core only knows about values 
defined in 27.007 or values it defines explicitly inside include/*.  If 
we start having the core deal with every single vendor extension then we 
will quickly go insane and the core becomes unreadable.

I have been and will continue to be quite adamant here.  Fix. Your. Driver.

>
> I am not sure whether other modem has the same issue. This way should be more make sense.
>
> By the way, it should be better to change the status from "int" to " enum network_registration_status" to avoid modem driver pass
> in value we don't expect.

How does this help you?  If your modem reports issues outside the range, 
you still need to sanitize the values you send to the core.  If your 
modem is compliant, then converting them to an enum just leads to you 
writing more code that does nothing useful.

We've tried this already and abandoned it long ago.

Regards,
-Denis