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

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

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

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.

Comment 1

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

CCing Siddartha, see if you're interested in this. : )

Comment 2

[psiddh]

Sure Yoshi, please do assign it to me

Comment 3

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

All yours

Comment 4

[psiddh]

Attachment: Part 1: Interface updated to decode 'duration' TLV tag for Display Text, Setup Call

Comment 5

[psiddh]

Attachment: Part 2: Decode comprehension TLV tag - Duration for Display Text , Setup Call

Comment 6

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

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.

Comment 7

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

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.

Comment 8

[psiddh]

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 ?

Comment 9

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

(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 10

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

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 :)

Comment 11

[psiddh]

Attachment: Part 1: Interface updated to decode 'duration' TLV tag for Display Text, Setup Call

Comment 12

[psiddh]

Attachment: Part 3: Marionette test case for Display Text and Setup Call with timeout.

Comment 13

[psiddh]

Tested the marionette test case and other patches as well

Comment 14

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

Siddartha, ready to review?

Comment 15

[psiddh]

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 16

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

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/

Comment 17

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

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?

Comment 18

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

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

Comment 19

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

Also can you run the try server ?
And test only on B2G platform.

Comment 20

[psiddh]

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.

Comment 21

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

okay, thanks for the info.

Comment 22

[psiddh]

Attachment: Part 3: Marionette test case for Display Text and Setup Call with timeout.

Comment 23

[psiddh]

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.

Comment 24

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

Attachment: Part 3: Marionette test case for Display Text and Setup Call with timeout. v2

Hi, Siddartha
I got conflict when trying to apply your patch, so I just rebase it.

Comment 25

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

(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

Comment 26

[psiddh]

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

Comment 27

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

(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.

Comment 28

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/4bb3b21ec4e5
https://hg.mozilla.org/integration/mozilla-inbound/rev/2be5143570bb
https://hg.mozilla.org/integration/mozilla-inbound/rev/095b80fc67f7

Comment 29

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4bb3b21ec4e5
https://hg.mozilla.org/mozilla-central/rev/2be5143570bb
https://hg.mozilla.org/mozilla-central/rev/095b80fc67f7