1117658 - [FFOS7715 v2.1][STK]USAT case 27.22.4.2.8 GET INKEY SEQ 8.1 GET INKEY，No Duration field in the TR.

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

[jinghua.xing]

[Testing Steps     ]:USAT case 27.22.4.2.8 GET INKEY SEQ 8.1 GET INKEY, variable time out of 10 seconds

[Expected Result  ]:case pass

[Test Result      ]:TERMINAL RESPONSE: No Duration field in the GET INKEY TR（04 02 01 XX）

Comment 1

[jinghua.xing]

The duration field has not been write into the parcel which will be sended to the sim in sendStkTerminalResponse() in ril_worker.js.

Comment 2

[孟宪茂]

Attachment: TR_duration_section.patch

Comment 3

[孟宪茂]

Dear Shawn,

I have made a patch for duration section processing in Stk terminal response in attachment, please help review and check if it is the fine way to fix the issue.

Thanks!

Comment 4

[shawn ku [:sku]]

Hi Xianmao:
 After checking the 3GPP TS 11.14, 6.8 Structure of TERMINAL RESPONSE, 
Duration (only required in response to a POLL INTERVAL proactive command).

Why duration is needed for GET INKEY TR?

Comment 5

[孟宪茂]

(In reply to shawn ku [:sku] (OOO 01/06 ~ 01/09) from comment #4)
> Hi Xianmao:
>  After checking the 3GPP TS 11.14, 6.8 Structure of TERMINAL RESPONSE, 
> Duration (only required in response to a POLL INTERVAL proactive command).
> 
> Why duration is needed for GET INKEY TR?

Dear shawn，

In the ETSI TS 102 384 V7.1.0 (2010-04),27.22.4.2.8.4.2, it requires the Duration section,

the required terminal response is:
81 03 01 22 00 82 02 82 81 83 01 12 04 02 01 Cond001

but our gecko RIL sends:
81 03 01 22 00 02 02 82 81 83 01 12
lack of Duration section.

In the 3GPP TS 11.14, 6.8 Structure of TERMINAL RESPONSE,it seems the duration is optional for other commends (I am not sure), please help check.

Thanks!

Comment 6

[shawn ku [:sku]]

Hi xianmao:
 Yes, for test case part (102 384), duration is part of test case, however, if we go to 11.14 6.8, it tells duration is only mandatory for POLL interval.

Besides, I don't think there is a way for user to assign "10 sec" TR to SIM card (unless hard code), 
And is this test case mandatory? 

please correct me if anything is wrong.

Comment 7

[孟宪茂]

(In reply to shawn ku [:sku] from comment #6)
> Hi xianmao:
>  Yes, for test case part (102 384), duration is part of test case, however,
> if we go to 11.14 6.8, it tells duration is only mandatory for POLL interval.
> 
> Besides, I don't think there is a way for user to assign "10 sec" TR to SIM
> card (unless hard code), 
> And is this test case mandatory? 
> 
> please correct me if anything is wrong.
Hi shawn,

Yes,the duration is mandatory for this test, it will fail for case test in the instrument IT3 if lacking of it.
This test case does not require user to assign any value, the duration section {"timeUnit":1,"timeInterval":10} is from SIM.

The duration is mandatory for POLL interval and it seems the duration is optional for other commends, but our gecko TR does not send duration section,if it is possible to add the duration in TR,

or if it is possible for us to add the duration for this case ,and it does not affect other TR.

Please help check which is the fine way to fix this issue.
Thanks!

Comment 8

[shawn ku [:sku]]

Hi Bevis:
 Per offline talk, this should involve Gaia/WebIDL/Gecko fix.
Please help check if we can support this on 2.1.

Thanks!!
Shawn

Comment 9

[Bevis Tseng[:bevis][:btseng](Exited)]

I'll take this bug first.
Please help to set the proper blocking-b2g flag if needed. :)

Comment 10

[Bevis Tseng[:bevis][:btseng](Exited)]

According to 6.4.2 GET INKEY in 3GPP TS102 223, 
"
if the UICC requests a variable timeout, the terminal shall wait until either the user enters a single character or
the timeout expires. The timer starts when the text is displayed on the screen and stops when the TERMINAL
RESPONSE is sent. The terminal shall pass the total display text duration (command execution duration) to
the UICC using the TERMINAL RESPONSE. The time unit of the response is identical to the time unit of the
requested variable timeout. The timeout may be used with other options of this command.
"

In addition,
1. In current implementation, gaia will adopt the duration provided in proactive command as input timer for GET INKEY command if provided, and the |resultCode| with |NO_RESPONSE_FROM_USER| will be replied when timeout.
2. In sendStkTerminalResponse() in ril_worker.js, the |response| also contains the original proactive command for reference.

To simply support this test case without further change in Gaia, we could put the |duration| TLV in the proactive command back to the TR if |duration| in proactive command is available and the |resultCode| is equal to |NO_RESPONSE_FROM_USER|.

The minor impact for this change is in DISPLAY_TEXT because Gaia also replies NO_RESPONSE_FROM_USER when DISPLAY requires |userClear|, because |duration| TLV in TR is optional and is not provided in the proactive command of  the test case 27.22.4.1.2 (DISPLAY TEXT, no response from user) in TS 102 384.

I'll provide a patch to address this fix later.

Comment 11

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: Part 1 v1: Provide Duration TLV in TR for "USAT case 27.22.4.2.8 GET INKEY SEQ 8.1 GET INKEY".

This is to comply the test criteria of USAT case 27.22.4.2.8 GET INKEY SEQ 8.1 GET INKEY.
That's is to send the TR with |Duration| TLV when duration is requested in the proactive command & there is no response from the user.

Hi Edgar,

May I have your review for this change?

Thanks!

Comment 12

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: Part 2: Add Corresponding Test Case to Improve Test Coverage.

Put "USAT case 27.22.4.2.8 GET INKEY SEQ 8.1 GET INKEY" into our test coverage. :)

Comment 13

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8551713 [details] [diff] [review]
Part 2: Add Corresponding Test Case to Improve Test Coverage.

Put "USAT case 27.22.4.2.8 GET INKEY SEQ 8.1 GET INKEY" into our test coverage.

Comment 14

[Edgar Chen [:edgar]]

Comment on attachment 8551712 [details] [diff] [review]
Part 1 v1: Provide Duration TLV in TR for "USAT case 27.22.4.2.8 GET INKEY SEQ 8.1 GET INKEY".

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

Looks good to me. Thank you, Bevis.

::: dom/system/gonk/ril_worker.js
@@ +2953,5 @@
>      // Command Details
>      GsmPDUHelper.writeHexOctet(COMPREHENSIONTLV_TAG_COMMAND_DETAILS |
>                                 COMPREHENSIONTLV_FLAG_CR);
>      GsmPDUHelper.writeHexOctet(3);
> +    if (command) {

Nice catch!!

Comment 15

[Bevis Tseng[:bevis][:btseng](Exited)]

[Blocking Requested - why for this release]:
Block partner's certificate for GCF/PTCRB.

Comment 16

[shawn ku [:sku]]

Hi Steven:
 Please nominate this bug as 2.1s+.
Thanks!!
Shawn

Comment 17

[shawn ku [:sku]]

Update status: 
This bug is waiting on 1123204 even got r+ already.
We need this patch to be in m-c anyhow.

Comment 18

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8551712 [details] [diff] [review]
Part 1 v1: Provide Duration TLV in TR for "USAT case 27.22.4.2.8 GET INKEY SEQ 8.1 GET INKEY".

[Approval Request Comment]
Bug caused by (feature/regressing bug #): NA
User impact if declined: Block partner's certificate for GCF/PTCRB test cases.
Testing completed: Yes. Test case is also included.
Risk to taking this patch (and alternatives if risky): No.
String or UUID changes made by this patch:NA

Comment 19

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8551713 [details] [diff] [review]
Part 2: Add Corresponding Test Case to Improve Test Coverage.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): NA
User impact if declined: Block partner's certificate for GCF/PTCRB test cases.
Testing completed: Yes. Test case is also included.
Risk to taking this patch (and alternatives if risky): No.
String or UUID changes made by this patch:NA

Comment 20

[Bevis Tseng[:bevis][:btseng](Exited)]

update try server result:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c7430c3bd2de

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/4ab5bff0fbdc
https://hg.mozilla.org/integration/b2g-inbound/rev/3fb08bdf5767

Comment 22

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/4ab5bff0fbdc
https://hg.mozilla.org/mozilla-central/rev/3fb08bdf5767

Comment 23

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/4b8c63467dec
https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/3de1176fd6e4

https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/fece3eefc5ab
https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/1aa570f8519f

Comment 24

[Vincent Liu[:vliu]]

https://hg.mozilla.org/releases/mozilla-b2g34_v2_1s/rev/fece3eefc5ab