Closed
Bug 831630
Opened 12 years ago
Closed 12 years ago
B2G STK: Add 'duration' property for DISPLAY_TEXT and SET_UP_CALL
Categories
(Core :: DOM: Device Interfaces, defect)
Tracking
()
RESOLVED
FIXED
mozilla22
People
(Reporter: allstars.chh, Assigned: psiddh)
References
Details
Attachments
(3 files, 3 obsolete files)
(deleted),
patch
|
allstars.chh
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
allstars.chh
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review |
In proactive command DISPLAY_TEXT and SET_UP_CALL, they both have an optional TLV called 'duration', current B2G STK implementation will ignore this duration value.
In order to have better compatibility,
we could add 'duration' property in these two proactive commands.
Reporter | ||
Comment 1•12 years ago
|
||
CCing Siddartha, see if you're interested in this. : )
Blocks: b2g-stk
Summary: B2G STK: Add duration for DISPLAY_TEXT and SET_UP_CALL → B2G STK: Add 'duration' property for DISPLAY_TEXT and SET_UP_CALL
Attachment #710841 -
Flags: superreview?(jonas)
Attachment #710841 -
Flags: review?(allstars.chh)
Attachment #710842 -
Flags: review?(allstars.chh)
Attachment #710841 -
Attachment description: Interface updated to decode 'duration' TLV tag for Display Text, Setup Call → Part 1: Interface updated to decode 'duration' TLV tag for Display Text, Setup Call
Attachment #710842 -
Attachment description: Decode comprehension TLV tag - Duration for Display Text , Setup Call → Part 2: Decode comprehension TLV tag - Duration for Display Text , Setup Call
Reporter | ||
Comment 6•12 years ago
|
||
Comment on attachment 710841 [details] [diff] [review]
Part 1: Interface updated to decode 'duration' TLV tag for Display Text, Setup Call
Review of attachment 710841 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/icc/interfaces/SimToolKit.idl
@@ +18,5 @@
> /**
> + * The length of time for which the ME shall display the dialog.
> + *
> + * @see TS 11.14, clause 12.6, Command Qualifier, Display Text, bit 8.
> + * bit 8: 0 = clear message after a delay
The comment here seems wrong here.
Your comment should be referred to the 'userClear' member.
Attachment #710841 -
Flags: review?(allstars.chh)
Reporter | ||
Comment 7•12 years ago
|
||
Comment on attachment 710842 [details] [diff] [review]
Part 2: Decode comprehension TLV tag - Duration for Display Text , Setup Call
Review of attachment 710842 [details] [diff] [review]:
-----------------------------------------------------------------
Looks good to me,
but I am wondering about the test case.
Yoshi,
As per the 31.111 spec for Display text, I read this
"A flag (see command qualifier, subclause 8.6) shall be set to inform the ME whether the availability of the screen for
subsequent information display after its use for 'Display Text' should be either after a short delay (the duration of the delay being at the discretion of the ME manufacturer), or following a user MMI action."
So therefore I interpreted as , if bit 8 == 0 then duration tag may be specified.
In other words "clear message after a delay" is interpreted as "clear message after a duration of the delay". I am not sure though. What do you think should the comment be ?
Regd. test case, do you want me to add a marionette test case ?
Reporter | ||
Comment 9•12 years ago
|
||
(In reply to Siddartha P from comment #8)
Hi, Siddartha
Whether bit 8 is 0 or 1, the terminal should display the message with a delay, when only bit 8 is 1, when the timeout expires, ME should respond "NO_RESPONSE_FROM_USER", otherwise simply response "OK".
My interpretation is the Duration is to replace the delay value from the terminal, whether bit 8 is 0 or 1.
Quote from spec (TS 102.223, clause 6.4.1)
"A duration object that represents the variable display timeout may be included by the UICC. The duration informs the ME about the required duration of the display (Precision and resolution are in accordance with clause 6.4.21 Timer Management). *The requested timeout value replaces the timeout set by the terminal manufacturer.* terminal support of
this feature is indicated in the PROFILE DOWNLOAD."
....
"if the UICC includes a duration object, the terminal shall limit the display time of the message for a period that
does not exceed the requested duration."
How do you think?
> Regd. test case, do you want me to add a marionette test case ?
Yes, it would be great if we got test cases.
Also there is a rule that if that patch without test cases should always get a r-.
Comment on attachment 710841 [details] [diff] [review]
Part 1: Interface updated to decode 'duration' TLV tag for Display Text, Setup Call
Review of attachment 710841 [details] [diff] [review]:
-----------------------------------------------------------------
If Yoshi and/or Vicamo is fine with this API then so am I :)
Attachment #710841 -
Flags: superreview?(jonas) → superreview+
Assignee | ||
Comment 11•12 years ago
|
||
Attachment #710841 -
Attachment is obsolete: true
Assignee | ||
Comment 12•12 years ago
|
||
Assignee | ||
Comment 13•12 years ago
|
||
Tested the marionette test case and other patches as well
Reporter | ||
Comment 14•12 years ago
|
||
Siddartha, ready to review?
Attachment #714228 -
Flags: review?(allstars.chh)
Attachment #714230 -
Flags: review?(allstars.chh)
Assignee | ||
Comment 15•12 years ago
|
||
Yes Yoshi.. it was up for review... I forgot to update the review flags when I uploaded the patches.. Now added you as the reviewer
Reporter | ||
Comment 16•12 years ago
|
||
Comment on attachment 714228 [details] [diff] [review]
Part 1: Interface updated to decode 'duration' TLV tag for Display Text, Setup Call
Review of attachment 714228 [details] [diff] [review]:
-----------------------------------------------------------------
As Jonas has already sr+ on this, so I don't send sr? to him again.
::: dom/icc/interfaces/SimToolKit.idl
@@ +249,5 @@
> */
> jsval callMessage;
> +
> + /**
> + * The Optional maximum duration for the redial mechanism.
s/Optional/optional/
Attachment #714228 -
Flags: review?(allstars.chh) → review+
Reporter | ||
Comment 17•12 years ago
|
||
Comment on attachment 710842 [details] [diff] [review]
Part 2: Decode comprehension TLV tag - Duration for Display Text , Setup Call
Review of attachment 710842 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/system/gonk/ril_worker.js
@@ +8542,4 @@
> }
> call.address = ctlv.value.number;
>
> + // see 3GPP TS 31.111 section 6.4.13
I though the spec is TS 102.223?
Attachment #710842 -
Flags: review?(allstars.chh) → review+
Reporter | ||
Comment 18•12 years ago
|
||
Comment on attachment 714230 [details] [diff] [review]
Part 3: Marionette test case for Display Text and Setup Call with timeout.
Review of attachment 714230 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/icc/tests/marionette/test_stk_proactive_command.js
@@ +106,5 @@
> +function testDisplayTextVariableTimeOut(cmd) {
> + log("STK CMD " + JSON.stringify(cmd));
> + is(cmd.typeOfCommand, icc.STK_CMD_DISPLAY_TEXT);
> + is(cmd.commandNumber, 0x01);
> + is(cmd.options.duration.timeUnit, 0x01);
can we use icc.STK_TIME_UNIT_SECOND here?
@@ +116,5 @@
> +function testSetUpCallVariableTimeOut(cmd) {
> + log("STK CMD " + JSON.stringify(cmd));
> + is(cmd.typeOfCommand, icc.STK_CMD_SET_UP_CALL);
> + is(cmd.commandNumber, 0x01);
> + is(cmd.options.duration.timeUnit, 0x01);
ditto
Attachment #714230 -
Flags: review?(allstars.chh) → review+
Reporter | ||
Comment 19•12 years ago
|
||
Also can you run the try server ?
And test only on B2G platform.
Assignee | ||
Comment 20•12 years ago
|
||
Yes Yoshi, we can use 'STK_TIME_UNIT_SECOND'. I will make the change. Also I will run the 'tryServer' on Tuesday as Monday is a holiday here.
Also TS 102 223 (Card Application Toolkit (CAT) ) is an ETSI standard while 3G TS 31.111 is a 3rd Generation Partnership Project; Technical Specification Group Terminals; USIM Application Toolkit (USAT). Both talk about exact same specifications.
Also, TS 102 223 initial draft and subsequent editions are based out of TS 31.111 . In other words, there is no difference as far STK specification is concerned. I think we can refer to either of specs as references, unless you have a specific preference of one over another.
Reporter | ||
Comment 21•12 years ago
|
||
okay, thanks for the info.
Assignee | ||
Comment 22•12 years ago
|
||
Attachment #714230 -
Attachment is obsolete: true
Assignee | ||
Comment 23•12 years ago
|
||
Yoshi, I am having some tryServer access related issues.
pushing to ssh://hg.mozilla.org/try/
remote: Permission denied (publickey,keyboard-interactive).
abort: no suitable response from remote hg!
Though my public key happens to be the same that I used when I raised my commenter's access, not sure what is happening here.
Reporter | ||
Comment 24•12 years ago
|
||
Hi, Siddartha
I got conflict when trying to apply your patch, so I just rebase it.
Attachment #715569 -
Attachment is obsolete: true
Reporter | ||
Comment 25•12 years ago
|
||
(In reply to Siddartha P from comment #23)
> Yoshi, I am having some tryServer access related issues.
>
> pushing to ssh://hg.mozilla.org/try/
> remote: Permission denied (publickey,keyboard-interactive).
> abort: no suitable response from remote hg!
>
> Though my public key happens to be the same that I used when I raised my
> commenter's access, not sure what is happening here.
not sure what went wrong
Maybe you could ask Garner about it
anyway I have run the try server for you
https://tbpl.mozilla.org/?tree=Try&rev=e8d5ce3af90d
Assignee | ||
Comment 26•12 years ago
|
||
thanks Yoshi. I am working on my tryServer access. I already sent a mail to see if I have the right credentials. For next time onwards, I will use Garner's tryServer a/c till mine gets resolved
Reporter | ||
Comment 27•12 years ago
|
||
(In reply to Siddartha P from comment #23)
> Yoshi, I am having some tryServer access related issues.
>
> pushing to ssh://hg.mozilla.org/try/
> remote: Permission denied (publickey,keyboard-interactive).
> abort: no suitable response from remote hg!
>
Try with -v option, which enables verbose output.
Reporter | ||
Comment 28•12 years ago
|
||
Comment 29•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/4bb3b21ec4e5
https://hg.mozilla.org/mozilla-central/rev/2be5143570bb
https://hg.mozilla.org/mozilla-central/rev/095b80fc67f7
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
You need to log in
before you can comment on or make changes to this bug.
Description
•