B2G RIL: False roaming situation

RESOLVED FIXED in mozilla19

Status

()

RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: brg, Assigned: allstars.chh)

Tracking

Trunk
mozilla19
x86
Mac OS X
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

6 years ago
Created attachment 668099 [details]
device-2012-10-02-223022_SP-SIM_card.png

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:15.0) Gecko/20100101 Firefox/15.0.1
Build ID: 20120905151427

Steps to reproduce:

I am in Rio with a VIVO sim card that belongs to VIVO SaoPaulo area.
The mcc/mnc in Sao Paulo Vivo SIM card is: 724 10
The mcc/mnc in Rio is: 724 11

I am not in Roaming. Other devices with the Vivo SaoPaulo SIM card are not reporting roaming while staying in Rio.


Actual results:

I can see Roaming indicator ON while i am under VIVO coverage


Expected results:

I am not in Roaming.
As I check the log
...
I/Gecko   (  107): RIL Worker: SPN: spn=VIVO, spnDisplayCondition=0
...
I/Gecko   (  255): -*- RILContentHelper: Received message 'RIL:VoiceInfoChanged': {"connected":true,"emergencyCallsOnly":false,"roaming":true,"network":{"rilMessageType":"operatorchange","longName":"Vivo S.A.","shortName":"Vivo","mcc":724,"mnc":11},"cell":{"gsmLocationAreaCode":42421,"gsmCellId":34170378},"type":"gsm","signalStrength":-51,"relSignalStrength":100,"state":"registered"}

SPN from is "VIVO", and short name is "Vivo"
Assignee: nobody → allstars.chh
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Created attachment 668211 [details] [diff] [review]
Patch
Attachment #668211 - Flags: review?(vyang)
Summary: False roaming situation → B2G RIL: False roaming situation
Comment on attachment 668211 [details] [diff] [review]
Patch

Review of attachment 668211 [details] [diff] [review]:
-----------------------------------------------------------------

I'll r+ this patch, but please fix nullity of short/long name.
Beside, since we can test operator names as done in bug 785988, please also have a test case for this issue before landing.

::: dom/system/gonk/RadioInterfaceLayer.js
@@ +618,5 @@
>      if (!icc || !registration.connected) {
>        return;
>      }
>  
> +    let spn = icc.spn.toLowerCase();

Could SPN be null?

@@ +623,3 @@
>      let operator = registration.network;
> +    let longName = operator.longName.toLowerCase();
> +    let shortName = operator.shortName.toLowerCase();

See bug 785988, longName, shortName could be null
Attachment #668211 - Flags: review?(vyang) → review+
Created attachment 668218 [details] [diff] [review]
Patch v2.

check for null.
Attachment #668211 - Attachment is obsolete: true
Created attachment 671253 [details] [diff] [review]
Part 2: test cases
Attachment #671253 - Flags: review?(marshall)
Comment on attachment 671253 [details] [diff] [review]
Part 2: test cases

Review of attachment 671253 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good!

::: dom/network/tests/marionette/test_mobile_operator_names.js
@@ +105,5 @@
> +function doTestRoamingCheck(longName, shortName, callback) {
> +  log("Testing roaming check '" + longName + "', '" + shortName + "':");
> +
> +  setEmulatorOperatorNames(OPERATOR_ROAMING, longName, shortName,
> +                           window.setTimeout.bind(window, function () {

ooh. clever. :)

@@ +144,5 @@
> +            doTestRoamingCheck("android", "Xxx", function () {
> +              doTestRoamingCheck("Xxx", "Android", function () {
> +                doTestRoamingCheck("Xxx", "android", function () {
> +                  setEmulatorOperatorNames(OPERATOR_ROAMING, "TelKila", "TelKila",
> +                                           window.setTimeout.bind(window, cleanUp, 3000));

sigh. This deeply embedded callback pattern probably isn't a good choice for long term maintainability. I really wish we had some simple async helpers to do this kind of ordering for us..
Attachment #671253 - Flags: review?(marshall) → review+
(In reply to Marshall Culpepper [:marshall_law] from comment #6)
> @@ +144,5 @@
> > +            doTestRoamingCheck("android", "Xxx", function () {
> > +              doTestRoamingCheck("Xxx", "Android", function () {
> > +                doTestRoamingCheck("Xxx", "android", function () {
> > +                  setEmulatorOperatorNames(OPERATOR_ROAMING, "TelKila", "TelKila",
> > +                                           window.setTimeout.bind(window, cleanUp, 3000));
> 
> sigh. This deeply embedded callback pattern probably isn't a good choice for
> long term maintainability. I really wish we had some simple async helpers to
> do this kind of ordering for us..

I can have something like:

  function helper(func, array, oncomplete) {
    (function do_call(index) {
      let next = index < (array.length - 1) ? do_call.bind(null, index + 1) : oncomplete;
      func.apply(null, array[index].concat(next));
    })(0);
  }

  let argsArray = [
    ["1", "2"],
    ["3", "4"],
    ["5", "6"],
    ...
  ];
  helper(doTestRoamingCheck, argsArray, oncomplete);

How do you think? I can fire another bug to rewrite them all.
(In reply to Vicamo Yang [:vicamo][:vyang] from comment #7)
> I can have something like:
> 
>   function helper(func, array, oncomplete) {
>     (function do_call(index) {
>       let next = index < (array.length - 1) ? do_call.bind(null, index + 1)
> : oncomplete;
>       func.apply(null, array[index].concat(next));
>     })(0);
>   }
> 
>   let argsArray = [
>     ["1", "2"],
>     ["3", "4"],
>     ["5", "6"],
>     ...
>   ];
>   helper(doTestRoamingCheck, argsArray, oncomplete);
> 
> How do you think? I can fire another bug to rewrite them all.

That looks ok, but it's probably in the "nice to have" category. Go ahead and file, and we can ask jgriffin where the best place for such an async helper might live..
https://hg.mozilla.org/mozilla-central/rev/1602ef5c86e2
https://hg.mozilla.org/mozilla-central/rev/4430d306f3f7
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
You need to log in before you can comment on or make changes to this bug.