Required for certification?

Hi, Michael ETSI TS 102 223 V11.0.0, 6.4.21 TIMER MANAGEMENT, "This command requests the terminal to manage timers running physically in the terminal." "When a timer expires (i.e. reaches zero), the terminal shall use the Timer Expiration mechanism to transfer the identifier of the timer that has expired and the difference between the time when this transfer occurs and the time when the timer was initially started. The terminal shall then deactivate the timer." In Android STK document, this proactive command is marked as "Implement by Baseband". http://www.kandroid.org/online-pdk/guide/stk.html Dose it mean this command will be handled by modem? Do you have any information about this? :) Thanks

Hi Yoshi: Could you please return some feedback when you're available? Thanks! :) Below is the summarizes about this patch: 1. Add new const related to TIMER_MANAGEMENT. 2. Add new structure, MozStkTimer, used in MozStkCommand / MozStkResponse / sendStkTimerExpiration(). 3. Add new API for sending Envelope command "Timer Expiration". void sendStkTimerExpiration(in jsval timer)

Hi Yoshi: Could you please return some feedback when you are available? Thank! :) Below is the summarizes for this patch: 1. Add new const related to TIMER MANAGEMENT. 2. Add new IPC Command, RIL:SendStkTimerExpiration, for DOM API, sendStkTimerExpiration(). 3. Process new field related to TIMER MANAGEMENT in Terminal Response and Envelope command. 4. This method, writeSwappedNibbleBCDNum, is the same as the one in patch of bug 804671. I will remove it, if the patch of bug 804671 is landed first.

Miss Terminal Profile part. (Marked as supported) STK_TERMINAL_PROFILE_PROACTIVE_3 have also been defined in patch of bug 804671. I will adjust code if the patch of bug 804671 is landed first. Please also see comment #3 for the other summarizes. Thanks.

Comment on attachment 687699 [details] [diff] [review] WIP Part 2: Support TIMER_MANAGEMENT in RIL, v2 Review of attachment 687699 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/system/gonk/ril_worker.js @@ +7425,5 @@ > + * | 3 | Timer Identifier | 1 | > + */ > + retrieveTimerId: function retrieveTimerId(length) { > + let id = { > + id: GsmPDUHelper.readHexOctet() timerId @@ +7442,5 @@ > + * | 5 | Second | 1 | > + */ > + retrieveTimerValue: function retrieveTimerValue(length) { > + let value = { > + value: (GsmPDUHelper.readSwappedNibbleBcdNum(1) * 60 * 60) + timerValue @@ +7444,5 @@ > + retrieveTimerValue: function retrieveTimerValue(length) { > + let value = { > + value: (GsmPDUHelper.readSwappedNibbleBcdNum(1) * 60 * 60) + > + (GsmPDUHelper.readSwappedNibbleBcdNum(1) * 60) + > + GsmPDUHelper.readSwappedNibbleBcdNum(1) nit, align this line

Comment on attachment 687664 [details] [diff] [review] WIP Part 1: IDL For TIMER_MANAGEMENT, v1 Review of attachment 687664 [details] [diff] [review]: ----------------------------------------------------------------- Please split IccManager.cpp to other part of patch, it should be reviewed by DOM-peer. ::: dom/icc/interfaces/SimToolKit.idl @@ +347,5 @@ > +{ > + /** > + * Identifier of a timer. > + */ > + unsigned short id; For id, it's only 1 byte, maybe you could use 'octet' here. Also I suggest timerId, because this value is accessed through 'options' in MozStkCommand, and developers might not know what does id mean. @@ +352,5 @@ > + > + /** > + * Length of time during in seconds > + */ > + unsigned long value; Since it's a Web-API, can we make this member from a more high level pointer of view ? i.e. hour, minutes, seconds. Also using 'timerValue' for naming. @@ +477,5 @@ > boolean hasConfirmed; > + > + /** > + * The response for TIMER MANAGEMENT > + * Can you add more documentation here, for example, when the command STK_CMD_TIMER_MANAGEMENT .....

address review comment #7

add IccManager::SendStkTimerExpiration() in IccManager.cpp for new DOM API, sendStkTimerExpiration()

1). rebase based on patch part2 of bug 804671. 2). address review comment #6

xpcshell tests

marionette tests

Comment on attachment 689614 [details] [diff] [review] Part 1: IDL For TIMER_MANAGEMENT, v2 Review of attachment 689614 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/icc/interfaces/SimToolKit.idl @@ +363,5 @@ > + */ > + octet timerId; > + > + /** > + * Length of time during in seconds grammar seems wrong here. @@ +365,5 @@ > + > + /** > + * Length of time during in seconds > + */ > + unsigned long timeDuringSeconds; The naming seems not so good here, do you think we could also use the term 'timerValue' from spec? Also maybe in comments we could also reuse from spec "length of time during which the timer has to run. " and specify the timer resolution is 1 sec. @@ +368,5 @@ > + */ > + unsigned long timeDuringSeconds; > + > + /** > + * The action request from UICC. requested @@ +525,5 @@ > + * the ME shall response the current value of the timer to the UICC. > + * > + * @see MozStkTimer > + */ > + jsval timer; Here is inside MozStkRespose, right? But seems the comments is talking about proactive command. Or I don't quite understand what those comments mean here. ::: dom/icc/interfaces/nsIDOMIccManager.idl @@ +240,5 @@ > + * Send the "Timer Expiration" Envelope command to ICC for TIMER MANAGEMENT. > + `* > + * @param timer > + * The identifier and value for a timer > + * - MozStkTimer From spec it also mentions "Timer value: difference between the time when this command is issued and the time when the timer was initially started." We should also mention that in comments.

Address review comment #13. 1). Add more explanation in comment. 2). Change naming. (timeDuringSeconds -> timerValue)

fix some bugs.

Comment on attachment 690299 [details] [diff] [review] Part 1: IDL For TIMER_MANAGEMENT, v3 Review of attachment 690299 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/icc/interfaces/SimToolKit.idl @@ +511,5 @@ > jsval localInfo; > + > + /** > + * The response for STK_CMD_TIMER_MANAGEMENT. > + * the 'timerValue' is needed if the action of STK_CMD_TIMER_MANAGEMENT is The ::: dom/icc/interfaces/nsIDOMIccManager.idl @@ +240,5 @@ > + * Send the "Timer Expiration" Envelope command to ICC for TIMER MANAGEMENT. > + `* > + * @param timer > + * The identifier and value for a timer. > + * timerId: identifier of the timer that has expired. Identifier @@ +241,5 @@ > + `* > + * @param timer > + * The identifier and value for a timer. > + * timerId: identifier of the timer that has expired. > + * timerValue: different between the time when this command is issued Difference @@ +242,5 @@ > + * @param timer > + * The identifier and value for a timer. > + * timerId: identifier of the timer that has expired. > + * timerValue: different between the time when this command is issued > + and the time when the timer was initially started. I think we could remove 'the time' @@ +243,5 @@ > + * The identifier and value for a timer. > + * timerId: identifier of the timer that has expired. > + * timerValue: different between the time when this command is issued > + and the time when the timer was initially started. > + * - MozStkTimer @see MozStkTimer As in SimToolKit.idl you already did it this way.

Comment on attachment 689617 [details] [diff] [review] Part 2: modify IccManager for TIME_MANAGEMENT, v1 This should be reviewed by DOM peer. smaug, can you help to review this patch for Edgar? Thank you.

Comment on attachment 690315 [details] [diff] [review] Part 3: Support TIMER_MANAGEMENT in RIL, v5 Review of attachment 690315 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/system/gonk/ril_worker.js @@ +3178,5 @@ > + let timer = response.timer; > + size = size + > + ((timer.timerId ? TLV_TIMER_IDENTIFIER : 0) + > + (timer.timerValue ? TLV_TIMER_VALUE : 0)) * 2; > + } Maybe we could do as sendICCEnvelopeCommand, so we don't have to do '* 2' each time when we add a new TLV. @@ +8938,5 @@ > PDU_NL_LOCKING_SHIFT_TABLES[PDU_NL_IDENTIFIER_DEFAULT].indexOf(language[1])); > }, > > + /** > + * Write Location Info Comprehension TLV. comments should be wrong. @@ +8949,5 @@ > + GsmPDUHelper.writeHexOctet(COMPREHENSIONTLV_TAG_TIMER_VALUE | > + COMPREHENSIONTLV_FLAG_CR); > + } else { > + GsmPDUHelper.writeHexOctet(COMPREHENSIONTLV_TAG_TIMER_VALUE); > + } GsmPDUHelper.writeHexOctet(COMPREHENSIONTLV_TAG_TIMER_VALUE | (cr ? COMPREHENSIONTLV_FLAG_CR : 0))

Comment on attachment 690318 [details] [diff] [review] Part 5: marionette tests for TIMER_MANAGEMENT, v2 Review of attachment 690318 [details] [diff] [review]: ----------------------------------------------------------------- Remember to rebase your patch As Bug 816835 also updates the test script.

Address review comment #18.

Address review comment #20

rebase only

Correct reviewer in title after review+.

rebase

rebase

try server: https://tbpl.mozilla.org/?tree=Try&rev=010843010a34

https://hg.mozilla.org/integration/mozilla-inbound/rev/85508731e988 https://hg.mozilla.org/integration/mozilla-inbound/rev/16ecb1d51f68 https://hg.mozilla.org/integration/mozilla-inbound/rev/bcf02a9f81b1 https://hg.mozilla.org/integration/mozilla-inbound/rev/5c32d392d4f9 https://hg.mozilla.org/integration/mozilla-inbound/rev/2992fbea1370

https://hg.mozilla.org/mozilla-central/rev/85508731e988 https://hg.mozilla.org/mozilla-central/rev/16ecb1d51f68 https://hg.mozilla.org/mozilla-central/rev/bcf02a9f81b1 https://hg.mozilla.org/mozilla-central/rev/5c32d392d4f9 https://hg.mozilla.org/mozilla-central/rev/2992fbea1370

(In reply to Yoshi Huang[:yoshi][:allstars.chh] from comment #20) > Comment on attachment 690315 [details] [diff] [review] > Part 3: Support TIMER_MANAGEMENT in RIL, v5 > > Review of attachment 690315 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: dom/system/gonk/ril_worker.js > @@ +3178,5 @@ > > + let timer = response.timer; > > + size = size + > > + ((timer.timerId ? TLV_TIMER_IDENTIFIER : 0) + > > + (timer.timerValue ? TLV_TIMER_VALUE : 0)) * 2; > > + } > > Maybe we could do as sendICCEnvelopeCommand, > so we don't have to do '* 2' each time when we add a new TLV. > Hi Edgar I just realized you didn't address this comment, Did you just forget or you have different opinion on this?

(In reply to Yoshi Huang[:yoshi][:allstars.chh] from comment #31) > (In reply to Yoshi Huang[:yoshi][:allstars.chh] from comment #20) > > Comment on attachment 690315 [details] [diff] [review] > > Part 3: Support TIMER_MANAGEMENT in RIL, v5 > > > > Review of attachment 690315 [details] [diff] [review]: > > ----------------------------------------------------------------- > > > > ::: dom/system/gonk/ril_worker.js > > @@ +3178,5 @@ > > > + let timer = response.timer; > > > + size = size + > > > + ((timer.timerId ? TLV_TIMER_IDENTIFIER : 0) + > > > + (timer.timerValue ? TLV_TIMER_VALUE : 0)) * 2; > > > + } > > > > Maybe we could do as sendICCEnvelopeCommand, > > so we don't have to do '* 2' each time when we add a new TLV. > > > Hi Edgar > I just realized you didn't address this comment, > Did you just forget or you have different opinion on this? Hi Yoshi, Oops! I missed this comment. I will open a bug to address this comment. Thanks for your reminding.

> Hi Yoshi, > Oops! I missed this comment. > I will open a bug to address this comment. > Thanks for your reminding. Create bug 824194 for this. Thanks

Patch for mozilla-b2g18

Patch for mozilla-b2g18

Please merge bug 804671 into b2g18 first then this, or may have conflict. And for this bug, please use following patches for b2g18. - Part 1: IDL For TIMER_MANAGEMENT, v3.1 - Part 2: modify IccManager for TIME_MANAGEMENT, v1, r=bugs - (b2g18) Part 3: Support TIMER_MANAGEMENT in RIL, v5.1, r=allstars.chh - Part 4: xpcshell tests for TIMER_MANAGEMENT, v2, r=allstars.chh - (b2g18) Part 5: marionette tests for TIMER_MANAGEMENT, v4, r=allstars.chh Thanks

https://hg.mozilla.org/releases/mozilla-b2g18/rev/26f1698c9ef7 https://hg.mozilla.org/releases/mozilla-b2g18/rev/76036656de4e https://hg.mozilla.org/releases/mozilla-b2g18/rev/09f281d392a8 https://hg.mozilla.org/releases/mozilla-b2g18/rev/e7182d750b7e https://hg.mozilla.org/releases/mozilla-b2g18/rev/5c50b5891886