798000 - B2G RIL: support EF_PNN and EF_OPL

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect, P1)

Description

[Beatriz Rodríguez [:brg]]

Attachment: screenshot of lock screen

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:

Insert VIVO SIM card


Actual results:

Carrier name is misspelt. 
This name is wrong in several places, I have found: 
- lock screen
- Notification bar
- Settings --> Cellular&Data
- Settings --> Cellular&Data --> Carrier


Expected results:

The suitable name is VIVO (capital letters), device shows Vivo.

According to the requirements, the name of the carrier is displayed following this rule:
If the first byte of EF_SPN is set to "01" (Display of reg. PLMN required), the device shall display the content of EF_PNN if one is defined in EF_OPL for the network code of the registered PLMN. (If no valid entry is defined in EF_PNN/OPL, the operator name as stored in the device firmware may be displayed.)

I do not know if device firmware is storing this name, but in case so, the name is wrong.

Comment 1

[Daniel Coloma:dcoloma]

Nominating for Blocking Basecamp as this is a VIVO certification requirement

Comment 2

[Andrew Overholt [:overholt]]

Is this something we need in the RIL or is it something we can do in Gaia?

Comment 4

[Vicamo Yang [:vicamo][:vyang]]

Andrew, solution to this issue depends on several unhandled SIM records reading/processing and Gaia works as well. Bug 793111 is part of them, but not completed. More missing parts can also be found in the description of bug 804688. I think we should solve all the mess in bug 793111 at once for Gecko RIL part.

Comment 5

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #4)
> Andrew, solution to this issue depends on several unhandled SIM records
> reading/processing and Gaia works as well. Bug 793111 is part of them, but
> not completed. More missing parts can also be found in the description of
> bug 804688. I think we should solve all the mess in bug 793111 at once for
> Gecko RIL part.

https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/mobile_info.js

I don't think this is a Gaia issue. All the strings in Gaia spells "VIVO". 

Gaia is getting "Vivo" from MobileComm API.

Comment 6

[Patrick Wang (Chih-Kai Wang) (:kk1fff)]

Attachment: WIP Patch

Able to read OPL and PNN file from SIM. Function for updating network name haven't been done yet.

Comment 7

[Patrick Wang (Chih-Kai Wang) (:kk1fff)]

Attachment: WIP Patch

Comment 8

[Patrick Wang (Chih-Kai Wang) (:kk1fff)]

Attachment: Patch: Read EF_PNN, EF_OPL and use them to override network name

Hi Vicamo, would you help to review this patch? Thanks.

Comment 9

[Patrick Wang (Chih-Kai Wang) (:kk1fff)]

Attachment: Patch: test case

Comment 10

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 688710 [details] [diff] [review]
Patch: Read EF_PNN, EF_OPL and use them to override network name

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

Could you help add more details like document sources. I'd really want to know the reason for each line to prevent things happened in SPN/SPDI. I'll review/search for document again tomorrow.

::: dom/system/gonk/ril_consts.js
@@ +571,5 @@
>  this.SPDI_TAG_SPDI = 0xa3;
>  this.SPDI_TAG_PLMN_LIST = 0x80;
>  
> +// Full Name IEI from TS 24.008
> +this.TAG_FULL_NETWORK_NAME = 0x43;

How about having PNN_IEI_ as prefix? And please be more specific like:

  // MM INFORMATION message content IEIs
  // See 3GPP TS 24.008 table 9.2.18
  this.PNN_IEI_FULL_NETWORK_NAME = 0x43;

@@ +941,5 @@
>      CBMIR: 16,
>      SPN: 19,
>      DATA_DOWNLOAD_SMS_PP: 28,
> +    PNN: 45,
> +    OPL: 46,

SIM also has both OPL (TS 51.011 section 10.3.42) and PNN (TS 51.011 section 10.3.41)

::: dom/system/gonk/ril_worker.js
@@ +1729,5 @@
> +   */
> +  getOPL: function getOPL() {
> +    let opl = [];
> +    function callback(options) {
> +      if (DEBUG) debug("OPL: Process OPL file");

Looks like a personal debug print.

Comment 11

[Patrick Wang (Chih-Kai Wang) (:kk1fff)]

Attachment: Patch: Read EF_PNN, EF_OPL and use them to override network name

Fix according to previous review.

Comment 12

[Patrick Wang (Chih-Kai Wang) (:kk1fff)]

Attachment: Patch: Read EF_PNN, EF_OPL and use them to override network name

Add related spec to the parsing LAI part in getOPL() function.

Comment 13

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 690693 [details] [diff] [review]
Patch: Read EF_PNN, EF_OPL and use them to override network name

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

\(^^)/

::: dom/system/gonk/ril_worker.js
@@ +1713,5 @@
> +              // A value of '00' indicates that the name is to be taken from other
> +              // sources.
> +              return null;
> +            }
> +            pnnIndex = opl.pnnRecordId - 1;

How about:

  pnnEntry = iccPriv.PNN[opl.pnnRecordId - 1];

@@ +1725,5 @@
> +    // the first record in this EF is used for the default network name when
> +    // registered to the HPLMN.
> +    // If we haven't get pnnIndex assigned, we should try to assign default
> +    // value to it.
> +    if (pnnIndex == -1 && mcc == iccInfo.mcc && mnc == iccInfo.mnc) {

if (!pnnEntry && ...)

@@ +1730,5 @@
> +      pnnIndex = 0;
> +    }
> +
> +    if (DEBUG) {
> +      if (pnnIndex != -1) {

if (pnnEntry)

@@ +1738,5 @@
> +        debug("updateNetworkName: Network names will not be overriden");
> +      }
> +    }
> +
> +    if (pnnIndex != -1) {

if (pnnEntry)

@@ +1742,5 @@
> +    if (pnnIndex != -1) {
> +      return [pnn[pnnIndex].fullName, pnn[pnnIndex].shortName];
> +    } else {
> +      return null;
> +    }

if (...) {
  return xxx;
}

return yyy;

@@ +1755,5 @@
> +  getOPL: function getOPL() {
> +    let opl = [];
> +    function callback(options) {
> +      let len = Buf.readUint32();
> +      let readByte = 0;

Orphaned variable.

@@ +1848,5 @@
> +        let name;
> +        switch (tlvTag) {
> +        case PNN_IEI_FULL_NETWORK_NAME:
> +          name = GsmPDUHelper.readNetworkName(tlvLen);
> +          // TODO: Add Country's initials into network name.

let's move this TODO into readNetworkName() and make that function returns a string name directly.

@@ +1858,5 @@
> +          pnnElement.shortName = name.networkName;
> +          break;
> +        case PNN_TAG_PLMN_ADDITIONAL_INFO:
> +          pnnElement.plmnInfo = GsmPDUHelper.readAlphaIdentifier(tlvLen);
> +          break;

We're not pretty sure what to do for this tag, and we may also have problems, although it should never happen, when tlvTag doesn't fall into one of the three. Might have to call to Buf.seekIncoming(PDU_HEX_OCTET_SIZE * tlvLen) instead.

Comment 14

[Patrick Wang (Chih-Kai Wang) (:kk1fff)]

Attachment: Patch: Read EF_PNN, EF_OPL and use them to override network name

Fix according to previous review.

Comment 15

[Patrick Wang (Chih-Kai Wang) (:kk1fff)]

Attachment: Patch: test case

Rebase and modified with primary patch.

Comment 16

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 690757 [details] [diff] [review]
Patch: Read EF_PNN, EF_OPL and use them to override network name

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

::: dom/system/gonk/ril_worker.js
@@ +8133,5 @@
> +      return null;
> +    }
> +
> +    // TODO: According to shouldIncludeCountryInitials, add country initials
> +    // to the resulting string.

Please have a bug id for each TODO label :)

Comment 17

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 690758 [details] [diff] [review]
Patch: test case

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

::: dom/system/gonk/tests/test_ril_worker_icc.js
@@ +780,5 @@
> +    }
> +  ];
> +
> +  testNetworkName(123, 456, "PNN4Long", "PNN4Short");
> +  testNetworkName(123, 457, "PNN2Long", "PNN2Short");

Have some comments on the two test case, please~~~

Comment 18

[Patrick Wang (Chih-Kai Wang) (:kk1fff)]

Attachment: Patch: Read EF_PNN, EF_OPL and use them to override network name

r+ in comment 16, adding comment in patch.

Comment 19

[Patrick Wang (Chih-Kai Wang) (:kk1fff)]

Attachment: Patch: Test case

r+ in comment 17, adding comments to patch.

Comment 20

[Vicamo Yang [:vicamo][:vyang]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e2172b8ef3cc
https://hg.mozilla.org/integration/mozilla-inbound/rev/6fe9b3023000

Comment 21

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/e2172b8ef3cc
https://hg.mozilla.org/mozilla-central/rev/6fe9b3023000

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/a0c3f5ad1b62
https://hg.mozilla.org/releases/mozilla-aurora/rev/68390f6167ff

https://hg.mozilla.org/releases/mozilla-b2g18/rev/3448243577f3
https://hg.mozilla.org/releases/mozilla-b2g18/rev/5b0b2deac27b