Closed Bug 831630 Opened 7 years ago Closed 7 years ago

B2G STK: Add 'duration' property for DISPLAY_TEXT and SET_UP_CALL

Categories

(Core :: DOM: Device Interfaces, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla22

People

(Reporter: allstars.chh, Assigned: psiddh)

References

Details

Attachments

(3 files, 3 obsolete files)

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.
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
Sure Yoshi, please do assign it to me
All yours
Assignee: nobody → psiddh
Attachment #710841 - Flags: superreview?(jonas)
Attachment #710841 - 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
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)
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 ?
(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+
Tested the marionette test case and other patches as well
Siddartha, ready to review?
Attachment #714228 - Flags: review?(allstars.chh)
Attachment #714230 - Flags: review?(allstars.chh)
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
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+
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+
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+
Also can you run the try server ?
And test only on B2G platform.
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.
okay, thanks for the info.
Attachment #714230 - Attachment is obsolete: true
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.
Hi, Siddartha
I got conflict when trying to apply your patch, so I just rebase it.
Attachment #715569 - Attachment is obsolete: true
(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
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
(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.
Depends on: 844416
You need to log in before you can comment on or make changes to this bug.