916000 - [B2G][Helix][system][wangchao]There is no sim state for 'illegal'.

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RIL, enhancement, P1)

Description

[lecky]

User Agent: Mozilla/4.0 (compatible; MSIE 8.0; Windows NT 6.1; Trident/4.0; SLCC2; .NET CLR 2.0.50727; .NET CLR 3.5.30729; .NET CLR 3.0.30729; Media Center PC 6.0; .NET4.0C; .NET4.0E; Tablet PC 2.0)

Steps to reproduce:

【Detail Description*】：
【Repro Steps*】：install a unicom usim card that owed pay for a log time.
【Expect Result*】：can read sim phonebook and change pin
【Real Result*】：display unknown simstate and can't entry sim at all
【Test Count*】：5
【Found Count*】：5
【Gaia commit ID*】:c0ea0a4943dc8d3751b07f5b5c5d3abe06364a14
【Gecko commit ID*】: 170f9e477571127cd40997fa2abe262ed43f0e4d
【Log*】：NA
【Network environment】：NA
【Resume operation】：NA
【Carrier】：UNICOM

Because the sim is a owed pay card, so it is rejected by network, and the sim turned to 'illegal' simstate, this is a regular process.

But in gaia and gecko, there is no state for 'illegal', so mapped to 'unknown'.
Then the entry for sim is disabled, but this should not happen for 'illegal' state.

More worse, when sim is 'unknown', it can not to make a emergency call. This should never happen at all.

Thanks!

Comment 1

[Wayne Chang [:wchang]]

Asking for a new state is a feature request, will not happen on current release.

QAWANTED to check on this point:

(In reply to lecky from comment #0)

> More worse, when sim is 'unknown', it can not to make a emergency call. This
> should never happen at all.
> 

If this is the case, please open a bug on this. That needs to be fixed.

Comment 2

[Wayne Chang [:wchang]]

Feature request - for sim state "illegal". 

Emergency call issue can be tracked with bug 911865.


Ken,
Can you assess if this sim state should be implemented in the future?

Thanks.

Comment 3

[Ken Chang[:kenkai]]

(In reply to Wayne Chang [:wchang] from comment #1)
> > More worse, when sim is 'unknown', it can not to make a emergency call. This
> > should never happen at all.
> > 
> 
> If this is the case, please open a bug on this. That needs to be fixed.
It seems same as bug 908004.

(In reply to Wayne Chang [:wchang] from comment #2)
> Can you assess if this sim state should be implemented in the future?
We should have this state. Maybe UX want to notify user this state in the future.
Georgia, please take this bug.

Comment 4

[Georgia Wang]

Attachment: Add illegal card state support in ril.

Comment 5

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 809106 [details] [diff] [review]
Add illegal card state support in ril.

Oh it's a magic patch,
only people smart enough can review it.

Too bad I am too dump. :(

Can you also see the patch?

Comment 6

[Georgia Wang]

Attachment: Add illegal card state support in ril.

Gaia part will put in Bug919967 for import contact & Bug919969 for setting.

Comment 7

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 809595 [details] [diff] [review]
Add illegal card state support in ril.

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

Please also update comments in nsIDOMIccManager.idl,
please add tests next time,
I don't think you ever tested it.

::: dom/system/gonk/ril_consts.js
@@ +2344,5 @@
>  this.GECKO_RADIOSTATE_READY         = "ready";
>  
>  this.GECKO_CARDSTATE_NOT_READY                     = null;
>  this.GECKO_CARDSTATE_UNKNOWN                       = "unknown";
> +this.GECKO_CARDSTATE_ILLEGAL                       = "illegal";

Move it to before UNKNOWN.

::: dom/system/gonk/ril_worker.js
@@ +2990,5 @@
>          newCardState = GECKO_CARDSTATE_READY;
>          break;
> +      case CARD_APPSTATE_ILLEGAL:
> +        newCardState = GECKO_CARDSTATE_ILLEGAL;
> +        break;

Can this code work?

app_state is read by readUint32,
however CARD_APPSTATE_ILLEGAL is -1.

Also move this 'case' in the first case of switch.

Comment 8

[Georgia Wang]

OK~I'll add tests.

Base on RIL Worker log, app_state = -1 can be pass to upper layer~
Looks like it's ok to pass -1 here.

I/Gecko   (  544): RIL Worker[0]: iccStatus: {"cardState":1,"universalPINState":0,"gsmUmtsSubscriptionAppIndex":0,"cdmaSubscriptionAppIndex":-1,"imsSubscriptionAppIndex":-1,"apps":[{"app_type":2,"app_state":-1,"perso_substate":2,"aid":"a0000000871002ff49ff0589","app_label":"54656c656e6f72205553494d","pin1_replaced":0,"pin1":3,"pin2":1},{"app_type":0,"app_state":1,"perso_substate":0,"aid":"a000000063504b43532d3135","app_label":"","pin1_replaced":0,"pin1":0,"pin2":0}]}

Comment 9

[Georgia Wang]

I/Gecko   (  544): RIL Worker[0]: Parcel (size 292): 0,0,0,0,109,0,0,0,0,0,0,0,1,0,0,0,0,0,0,0,0,0,0,0,255,255,255,255,255,255,255,255,2,0,0,0,2,0,0,0,255,255,255,255,2,0,0,0,24,0,0,0,97,0,48,0,48,0,48,0,48,0,48,0,48,0,48,0,56,0,55,0,49,0,48,0,48,0,50,0,102,0,102,0,52,0,57,0,102,0,102,0,48,0,53,0,56,0,57,0,0,0,0,0,24,0,0,0,53,0,52,0,54,0,53,0,54,0,99,0,54,0,53,0,54,0,101,0,54,0,102,0,55,0,50,0,50,0,48,0,53,0,53,0,53,0,51,0,52,0,57,0,52,0,100,0,0,0,0,0,0,0,0,0,3,0,0,0,1,0,0,0,3,0,0,0,10,0,0,0,3,0,0,0,10,0,0,0,0,0,0,0,1,0,0,0,0,0,0,0,24,0,0,0,97,0,48,0,48,0,48,0,48,0,48,0,48,0,48,0,54,0,51,0,53,0,48,0,52,0,98,0,52,0,51,0,53,0,51,0,50,0,100,0,51,0,49,0,51,0,53,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0
I/Gecko   (  544): RIL Worker[0]: We have at least one complete parcel.
I/Gecko   (  544): RIL Worker[0]: Solicited response for request type 1, token 109, error 0

Comment 10

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

(In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #7)
> app_state is read by readUint32,
Actually the return integer is signed 32-bit, I just file Bug 920391 to rename it.

Comment 11

[Georgia Wang]

Attachment: Part1: Add illegal SIM state in ril.

Comment 12

[Georgia Wang]

Attachment: Part2: SIM app_state XPC shell test case.

Comment 13

[Georgia Wang]

Comment on attachment 809701 [details] [diff] [review]
Part2: SIM app_state XPC shell test case.

For test part~ local run ok.

Comment 14

[Georgia Wang]

https://tbpl.mozilla.org/?tree=Try&rev=0ef947ed3a35 
try server link..

Comment 15

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 809701 [details] [diff] [review]
Part2: SIM app_state XPC shell test case.

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

::: dom/system/gonk/tests/test_ril_worker_icc.js
@@ +1676,5 @@
> +    let iccStatus = {
> +      gsmUmtsSubscriptionAppIndex: 0,
> +      apps: [
> +      {
> +        app_state:cardAppState

nit, put a space after ':'

Comment 16

[Georgia Wang]

Attachment: Part2: SIM app_state XPC shell test case. v2

Add space after ':' ~~

Comment 17

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

CCing Anshul,
Although we didn't update the interface, but we will add a cardState 'illegal' here.

Comment 18

[Anshul]

(In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #17)
> CCing Anshul,
> Although we didn't update the interface, but we will add a cardState
> 'illegal' here.
Appreciate you letting us know Yoshi.

Comment 19

[Michael Schwartz [:m4]]

Lucky, this issue was fixed in AU_LINUX_GECKO_ICS_STRAWBERRY.01.01.00.019.234.  For more details, please file an SR.

Comment 20

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

https://hg.mozilla.org/integration/b2g-inbound/rev/38173c90964c
https://hg.mozilla.org/integration/b2g-inbound/rev/009a8fb380da

Comment 21

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/38173c90964c
https://hg.mozilla.org/mozilla-central/rev/009a8fb380da