926053 - [B2G] correctly process SIM_IO response with sw1 0x91

Status: RESOLVED DUPLICATE of bug 1064647

(Firefox OS Graveyard :: RIL, defect)

Description

[Ognyan Kulev]

On a particular SIM card, when trying to import contacts from SIM card, the importing process is stuck at "Reading from SIM...". Device is Keon. It happens on official 1.0, 1.1 and 1.2 Geeksphone builds, updated to latest as of today.

It's also reported at http://forum.geeksphone.com/index.php?topic=4754.0 .

What can I do to resolve this?

Comment 1

[Jose Manuel Cantera]

you need to provide more details for the dev team. Operator, kind of SIM, number of contacts, bla, bla.

Comment 2

[Shiwei Zhang]

We meeted this issue on 1.3t. CMCC SIM card with 249 contacts on it. Importing contacts from SIM card is stuck at "Reading from SIM card".

build info, 
"gaia" remote="mozillaorg" revision="72444027ab1beb00a2d2d32d27f08159391b4d77" upstream="v1.3t"
"gecko" remote="mozillaorg" revision="5c16c4f81b104ca1086cff329befb892160f0517" upstream="v1.3t"

Comment 3

[Jose Manuel Cantera]

I would recommend you activate the RIL logs in order to get more info

Comment 4

[Shiwei Zhang]

Thank you for your attion. I found some suspected logs:

08-07 10:04:14.304    85   327 I Gecko   : RIL Worker: [0] ICC I/O Error code null EF id = 6f3a command = b2(91/b)
08-07 10:04:14.304    85    85 I Gecko   : -*- RadioInterface[0]: Received message from worker: {"requestId":"id{de033b7e-6518-4fce-a429-982a0e33d466}","contactType":"adn","rilMessageClientId":0,"rilMessageToken":23,"rilMessageType":"readICCContacts","errorMsg":null}
08-07 10:04:14.304    85   327 I Gecko   : RIL Worker: Parcel handler didn't consume whole parcel, 120 bytes left over
08-07 10:04:14.304    85   327 I Gecko   : RIL Worker: Next parcel size unknown, going to sleep.
08-07 10:04:14.314   496   496 I Gecko   : -*- RILContentHelper: Received message 'RIL:ReadIccContacts': {"clientId":0,"data":{"requestId":"id{de033b7e-6518-4fce-a429-982a0e33d466}","contactType":"adn","rilMessageClientId":0,"rilMessageToken":23,"rilMessageType":"readICCContacts","errorMsg":null}}
08-07 10:04:14.324   496   496 E GeckoConsole: [JavaScript Error: "contacts is undefined" {file: "jar:file:///system/b2g/omni.ja!/components/RILContentHelper.js" line: 121}]

but I can't figure out what was wrong. Please give me a hand. Thanks.

Comment 5

[Shiwei Zhang]

I can attach complete logs if you need.

Comment 6

[Jose Manuel Cantera]

Yoshi, Vicamo, any idea?

thanks

Comment 7

[Shiwei Zhang]

Attachment: 20120101000003.tar.gz

Attach complete logs.

Comment 8

[Hsin-Yi Tsai (she/her) [:hsinyi]]

According to the log, modem/rild reported icc io error with "null" error code: 
08-07 10:04:14.304    85   327 I Gecko   : RIL Worker: [0] ICC I/O Error code null EF id = 6f3a command = b2(91/b)

But in [1], we are expecting a non-null errorMsg if a underlying error occurs; however what we got from modem was null. Maybe check why the modem/rild part reported 'null' error message first. Thanks!


[1] https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/file/2017e16f1a06/dom/system/gonk/RILContentHelper.js#1979

Comment 9

[sam.hua]

Hi Hsin,
the sw1 = 0x91
08-05 17:43:41.391    97   324 D AT      : Channel1: AT> AT+CRSM=178,28474,250,4,28,0,"3f007f10"
08-05 17:43:41.411    97   201 D AT      : Channel1: AT< +CRSM: 145,11,808D3E5EF6971EFFFFFFFFFFFFFF0891683167617177F8FFFFFFFFFF
08-05 17:43:41.411    97   201 D AT      : Channel1: AT< OK
08-05 17:43:41.411    97   324 D RILC    : [0350]< SIM_IO {sw1=0x91,sw2=0xB,808D3E5EF6971EFFFFFFFFFFFFFF0891683167617177F8FFFFFFFFFF}

so in ril_worker.js   RilObject.prototype[REQUEST_SIM_IO] = function REQUEST_SIM_IO(length, options) {
  let ICCIOHelper = this.context.ICCIOHelper;
  if (!length) {
    ICCIOHelper.processICCIOError(options);
    return;
  }
it is handled as a error,but actually, it sw1:0x91,sw2:0x0B should be a normal result with extra information ..

Comment 10

[sam.hua]

RILC reported as a success RIL REQUEST to Gecko.

Comment 11

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to sam.hua from comment #9)
> Hi Hsin,
> the sw1 = 0x91
> 08-05 17:43:41.391    97   324 D AT      : Channel1: AT>
> AT+CRSM=178,28474,250,4,28,0,"3f007f10"
> 08-05 17:43:41.411    97   201 D AT      : Channel1: AT< +CRSM:
> 145,11,808D3E5EF6971EFFFFFFFFFFFFFF0891683167617177F8FFFFFFFFFF
> 08-05 17:43:41.411    97   201 D AT      : Channel1: AT< OK
> 08-05 17:43:41.411    97   324 D RILC    : [0350]< SIM_IO
> {sw1=0x91,sw2=0xB,808D3E5EF6971EFFFFFFFFFFFFFF0891683167617177F8FFFFFFFFFF}
> 
> so in ril_worker.js   RilObject.prototype[REQUEST_SIM_IO] = function
> REQUEST_SIM_IO(length, options) {
>   let ICCIOHelper = this.context.ICCIOHelper;
>   if (!length) {
>     ICCIOHelper.processICCIOError(options);
>     return;
>   }
> it is handled as a error,but actually, it sw1:0x91,sw2:0x0B should be a
> normal result with extra information ..

Thanks for the further investigation, Sam!

You are right, we don't well address the case sw1 0x91. We support the case sw1 0x90 only.

Comment 12

[sam.hua]

in V1.4
  let success = ((sw1 == ICC_STATUS_NORMAL_ENDING) && (sw2 === 0x00))
                || (sw1 == ICC_STATUS_NORMAL_ENDING_WITH_EXTRA);

but in V1.3
  if (options.sw1 != ICC_STATUS_NORMAL_ENDING) {
    ICCIOHelper.processICCIOError(options);
    return;
  }

Comment 13

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to sam.hua from comment #12)
> in V1.4
>   let success = ((sw1 == ICC_STATUS_NORMAL_ENDING) && (sw2 === 0x00))
>                 || (sw1 == ICC_STATUS_NORMAL_ENDING_WITH_EXTRA);
> 

Hmmm, this should be for the response of |REQUEST_STK_SEND_ENVELOPE_WITH_STATUS|, not for |REQUEST_SIM_IO|.
In v1.3t, we have the same handling for REQUEST_STK_SEND_ENVELOPE_WITH_STATUS, FYI.

> but in V1.3
>   if (options.sw1 != ICC_STATUS_NORMAL_ENDING) {
>     ICCIOHelper.processICCIOError(options);
>     return;
>   }

Comment 14

[Shiwei Zhang]

Attachment: bug340815.patch

Hi Hsin-Yi, this patch can handle this issue but need a review. could you please give me a hand, thanks.

Comment 15

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Comment on attachment 8473543 [details] [diff] [review]
bug340815.patch

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

Thank you for the patch :) I give you f+ instead of r+ because of the wrong patch format. Please provide hg format again.

Also, I guess the change won't break any tests; however, please make sure you've pass xpcshell tests before asking for check-in, in case.

::: dom/system/gonk/ril_worker.js
@@ +5473,4 @@
>    let Buf = this.context.Buf;
>    options.sw1 = Buf.readInt32();
>    options.sw2 = Buf.readInt32();
> +  if ((options.sw1 != ICC_STATUS_NORMAL_ENDING) && (options.sw1 != ICC_STATUS_NORMAL_ENDING_WITH_EXTRA) ) {

nit: you don't need additional parenthesis, and please wrap at 80th char, i.e.

if (options.sw1 != ICC_STATUS_NORMAL_ENDING && 
     options.sw1 != ICC_STATUS_NORMAL_ENDING_WITH_EXTRA )

Comment 16

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Hsin-Yi Tsai [:hsinyi] from comment #15)
> Comment on attachment 8473543 [details] [diff] [review]
> bug340815.patch
> 
> Review of attachment 8473543 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Thank you for the patch :) I give you f+ instead of r+ because of the wrong
> patch format. Please provide hg format again.
> 
> Also, I guess the change won't break any tests; however, please make sure
> you've pass xpcshell tests before asking for check-in, in case.
> 
> ::: dom/system/gonk/ril_worker.js
> @@ +5473,4 @@
> >    let Buf = this.context.Buf;
> >    options.sw1 = Buf.readInt32();
> >    options.sw2 = Buf.readInt32();
> > +  if ((options.sw1 != ICC_STATUS_NORMAL_ENDING) && (options.sw1 != ICC_STATUS_NORMAL_ENDING_WITH_EXTRA) ) {
> 
> nit: you don't need additional parenthesis, and please wrap at 80th char,
> i.e.
> 
> if (options.sw1 != ICC_STATUS_NORMAL_ENDING && 
>      options.sw1 != ICC_STATUS_NORMAL_ENDING_WITH_EXTRA )

Oops, please align those two lines ;P

Comment 17

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Hsin-Yi Tsai [:hsinyi] from comment #15)
> Comment on attachment 8473543 [details] [diff] [review]
> bug340815.patch
> 
> Review of attachment 8473543 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Thank you for the patch :) I give you f+ instead of r+ because of the wrong
> patch format. Please provide hg format again.
> 
> Also, I guess the change won't break any tests; however, please make sure
> you've pass xpcshell tests before asking for check-in, in case.

Hey, to be more clear, please refer to [1] to prepare your patch.
Once you provide a new revision, I think I could help run xpcshell tests.

[1] https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F

> 
> ::: dom/system/gonk/ril_worker.js
> @@ +5473,4 @@
> >    let Buf = this.context.Buf;
> >    options.sw1 = Buf.readInt32();
> >    options.sw2 = Buf.readInt32();
> > +  if ((options.sw1 != ICC_STATUS_NORMAL_ENDING) && (options.sw1 != ICC_STATUS_NORMAL_ENDING_WITH_EXTRA) ) {
> 
> nit: you don't need additional parenthesis, and please wrap at 80th char,
> i.e.
> 
> if (options.sw1 != ICC_STATUS_NORMAL_ENDING && 
>      options.sw1 != ICC_STATUS_NORMAL_ENDING_WITH_EXTRA )

Comment 18

[Shiwei Zhang]

(In reply to Hsin-Yi Tsai [:hsinyi] from comment #17)
> (In reply to Hsin-Yi Tsai [:hsinyi] from comment #15)
> > Comment on attachment 8473543 [details] [diff] [review]
> > bug340815.patch
> > 
> > Review of attachment 8473543 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > Thank you for the patch :) I give you f+ instead of r+ because of the wrong
> > patch format. Please provide hg format again.
> > 
> > Also, I guess the change won't break any tests; however, please make sure
> > you've pass xpcshell tests before asking for check-in, in case.
> 
> Hey, to be more clear, please refer to [1] to prepare your patch.
> Once you provide a new revision, I think I could help run xpcshell tests.
> 
> [1]
> https://developer.mozilla.org/en-US/docs/
> Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-
> in_for_me.3F
> 
> > 
> > ::: dom/system/gonk/ril_worker.js
> > @@ +5473,4 @@
> > >    let Buf = this.context.Buf;
> > >    options.sw1 = Buf.readInt32();
> > >    options.sw2 = Buf.readInt32();
> > > +  if ((options.sw1 != ICC_STATUS_NORMAL_ENDING) && (options.sw1 != ICC_STATUS_NORMAL_ENDING_WITH_EXTRA) ) {
> > 
> > nit: you don't need additional parenthesis, and please wrap at 80th char,
> > i.e.
> > 
> > if (options.sw1 != ICC_STATUS_NORMAL_ENDING && 
> >      options.sw1 != ICC_STATUS_NORMAL_ENDING_WITH_EXTRA )

That's very nice of you, I‘ll prepare my patch ASAP.