Closed Bug 905503 Opened 6 years ago Closed 6 years ago

B2G RIL: Add a new card state for personalization ready

Categories

(Core :: DOM: Device Interfaces, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla26

People

(Reporter: edgar, Assigned: edgar)

References

Details

Attachments

(1 file, 2 obsolete files)

Currently we change sim card state to "ready" when personalization substate becomes to CARD_PERSOSUBSTATE_READY. This may cause some timing problems as bug 869778 comment #13.
(In reply to Edgar Chen [:edgar][:echen] from comment #0)
> Currently we change sim card state to "ready" when personalization substate
> becomes to CARD_PERSOSUBSTATE_READY. This may cause some timing problems as
> bug 869778 comment #13.
Hi Edgar,
personsubstate is a temporary state. we should expect APP_STATE to ready eventually no matter how long shuld it take.

sku.
Attachment #790627 - Flags: review?(allstars.chh)
Comment on attachment 790627 [details] [diff] [review]
Add a new card state and xpcshell tests for personalization ready, v1

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

You need to update the comments in nsIDOMIccManager.idl as well.
Attachment #790627 - Flags: review?(allstars.chh)
Update the comments in nsIDOMIccManager.idl
Attachment #790627 - Attachment is obsolete: true
Attachment #790706 - Flags: review?(allstars.chh)
Comment on attachment 790706 [details] [diff] [review]
Add a new card state and xpcshell tests for personalization ready, v2

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

::: dom/icc/interfaces/nsIDOMIccManager.idl
@@ +9,5 @@
>  interface nsIDOMDOMRequest;
>  interface nsIDOMEventListener;
>  interface nsIDOMMozIccInfo;
>  
> +[scriptable, builtinclass, uuid(572afe3d-561a-46b4-b87d-9a46781c5da7)]

Updating comments doesn't need to update uuid.

@@ +296,5 @@
>     *
>     * Possible values: null, 'unknown', 'absent', 'pinRequired', 'pukRequired',
> +   * 'personalizationInProgress', 'personalizationReady' 'networkLocked',
> +   * 'corporateLocked', 'serviceProviderLocked', 'networkPukRequired',
> +   * 'corporatePukRequired', 'serviceProviderPukRequired', 'ready'.

I think we could put 'personalizationReady' after 'serviceProviderPukRequired' to help people understand the state transition in cardState.

::: dom/system/gonk/ril_consts.js
@@ +2339,5 @@
>  this.GECKO_CARDSTATE_ABSENT                        = "absent";
>  this.GECKO_CARDSTATE_PIN_REQUIRED                  = "pinRequired";
>  this.GECKO_CARDSTATE_PUK_REQUIRED                  = "pukRequired";
>  this.GECKO_CARDSTATE_PERSONALIZATION_IN_PROGRESS   = "personalizationInProgress";
> +this.GECKO_CARDSTATE_PERSONALIZATION_READY         = "personalizationReady";

in ril_consts, it's okay for where you want to put it,
but I'd suggest you put in the same order in PERSONSUBSTATE,
so either you update here or update the code below.
Attachment #790706 - Flags: review?(allstars.chh) → review+
(In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #5)
> Comment on attachment 790706 [details] [diff] [review]
> Add a new card state and xpcshell tests for personalization ready, v2
> 
> Review of attachment 790706 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/icc/interfaces/nsIDOMIccManager.idl
> @@ +9,5 @@
> >  interface nsIDOMDOMRequest;
> >  interface nsIDOMEventListener;
> >  interface nsIDOMMozIccInfo;
> >  
> > +[scriptable, builtinclass, uuid(572afe3d-561a-46b4-b87d-9a46781c5da7)]
> 
> Updating comments doesn't need to update uuid.
> 
> @@ +296,5 @@
> >     *
> >     * Possible values: null, 'unknown', 'absent', 'pinRequired', 'pukRequired',
> > +   * 'personalizationInProgress', 'personalizationReady' 'networkLocked',
> > +   * 'corporateLocked', 'serviceProviderLocked', 'networkPukRequired',
> > +   * 'corporatePukRequired', 'serviceProviderPukRequired', 'ready'.
> 
> I think we could put 'personalizationReady' after
> 'serviceProviderPukRequired' to help people understand the state transition
> in cardState.

Okay, I will put it after 'serviceProviderPukRequired'.

> 
> ::: dom/system/gonk/ril_consts.js
> @@ +2339,5 @@
> >  this.GECKO_CARDSTATE_ABSENT                        = "absent";
> >  this.GECKO_CARDSTATE_PIN_REQUIRED                  = "pinRequired";
> >  this.GECKO_CARDSTATE_PUK_REQUIRED                  = "pukRequired";
> >  this.GECKO_CARDSTATE_PERSONALIZATION_IN_PROGRESS   = "personalizationInProgress";
> > +this.GECKO_CARDSTATE_PERSONALIZATION_READY         = "personalizationReady";
> 
> in ril_consts, it's okay for where you want to put it,
> but I'd suggest you put in the same order in PERSONSUBSTATE,
> so either you update here or update the code below.

Okay, I will update the order of PERSONSUBSTATE to have the same order in ril.h.

Thanks
https://hg.mozilla.org/mozilla-central/rev/1aee5bb06429
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
You need to log in before you can comment on or make changes to this bug.