1126687 - [FFOS2.0][Woodduck][STK]Setup Event Envelope structure is incorrect.

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

[pengfei.huang]

Attachment: envelop_wrong_structure.zip

DEFECT DESCRIPTION:[GCF][STK]27.22.7.8.1/1 EVENT DOWNLOAD - LANGUAGE SELECTION
fail in TMC

 REPRODUCING PROCEDURES:
1. Load a simcard to the phone which can send "event list";
2. Execute"27.22.7.8.1/1 EVENT DOWNLOAD - LANGUAGE SELECTION" ,envelop error.


 EXPECTED BEHAVIOUR:
2. Execute"27.22.7.8.1/1 EVENT DOWNLOAD - LANGUAGE SELECTION" ,envelop should
be ok.

please refer to the attachment.

reporter's phone number: 0752-2639695

 ASSOCIATE SPECIFICATION:

 TEST PLAN REFERENCE:

 TOOLS AND PLATFORMS USED:

 USER IMPACT:

 REPRODUCING RATE:

 For FT PR, Please list reference mobile's behavior:

Comment 1

[pengfei.huang]

Dear Mozilla,
  The envelope structure of Language selection command is incorrect. We can see at the log.
use-Rlog/RLOG-AT: AT> AT+STKENV="D6 0B 82 02 82 81 99 01 07 2D02656E"
 D6                  ---->Event Download tag
 0B                  ---->length
 82 02 82 81         ---->Device identities 
 99 01 07            
 2D02  
 656E                ---->language

correct envelope structure protocol:
D6
0B
19 01 07
82 02 82 81         ---->Device identities 
2D02 
6465                ---->language

and the value 91 and 19 are the same meaning. we can ignore it.

Comment 2

[Josh Cheng [:josh]]

Hi Sean,
Could you please help to check the problem? Thanks!

Comment 3

[Sean Lee [:seanlee][:weilonge]]

Hi Pengfei,

I see there are two differences in your comment 1:
1. The sequence of [19 01 07] and [82 02 82 81 ---->Device identities] is incorrect.
2. Language code difference -> [656E]"en" and [6465]"de"

Should we focus on the first one only?

Thank you!

Comment 4

[pengfei.huang]

Hi Sean,
  Sorry to remind you the language code is different due to different language we choose. They are correct behavior. We can ignore the situation below,
  2. Language code difference -> [656E]"en" and [6465]"de"

  Thanks.

Comment 5

[Sean Lee [:seanlee][:weilonge]]

Hi Bevis,

The sequence of
[19 01 07]
and
[82 02 82 81 ---->Device identities]
is incorrect.

Could you help to take a look this issue? Thank you!

Comment 6

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

Attachment: Patch: Ensure the order of TLVs in STK Envelope Command is the same to the order defined in TS 11.14/TS 102 223.

We didn't expect that the order of the TLVs in the Envelope matters due to the nature of TLV structure.

This patch is to ensure that the order of the TLVs in the STK Envelope is the same to the one specified in 3GPP standard.

Hi Pengfei,

Would you please give this patch a trial to see if the problem is resolved?

Thanks!

Comment 7

[pengfei.huang]

Hi Bevis,
  Your patch correct the order of TLVs in the envelope. Resolve the problem.
  Many thanks.

Comment 8

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

Comment on attachment 8556289 [details] [diff] [review]
Patch: Ensure the order of TLVs in STK Envelope Command is the same to the order defined in TS 11.14/TS 102 223.

Per comment 7, the test case was passed after applying this patch.

Hi Edgar,

May I have your review for this quick fix?

Thanks!

Comment 9

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

Comment on attachment 8556289 [details] [diff] [review]
Patch: Ensure the order of TLVs in STK Envelope Command is the same to the order defined in TS 11.14/TS 102 223.

Per comment 7, the test case was passed after applying this patch.

Hi Edgar,

May I have your review for this quick fix?

Thanks!

Comment 10

[Edgar Chen [:edgar]]

Comment on attachment 8556289 [details] [diff] [review]
Patch: Ensure the order of TLVs in STK Envelope Command is the same to the order defined in TS 11.14/TS 102 223.

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

Looks good, but I guess test_ril_worker_stk.js [1] needs a revise, too.
Thank you.

[1] https://dxr.mozilla.org/mozilla-central/source/dom/system/gonk/tests/test_ril_worker_stk.js#1632-1688

Comment 11

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

(In reply to Edgar Chen [:edgar][:echen] from comment #10)
> Comment on attachment 8556289 [details] [diff] [review]
> Patch: Ensure the order of TLVs in STK Envelope Command is the same to the
> order defined in TS 11.14/TS 102 223.
> 
> Review of attachment 8556289 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good, but I guess test_ril_worker_stk.js [1] needs a revise, too.
> Thank you.
> 
> [1]
> https://dxr.mozilla.org/mozilla-central/source/dom/system/gonk/tests/
> test_ril_worker_stk.js#1632-1688

Thanks for reminding!
I wasn't aware that this is also in the test coverage. :(
I review the related test cases as well and update accordingly.

Comment 12

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

Attachment: Part 1: Ensure the order of TLVs in STK Envelope Command is the same to the order defined in TS 11.14/TS 102 223. r=echen

Comment 13

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

Attachment: Part 2: Modify Test Cases Accordingly. r=echen

update test cases accordingly.

Comment 14

[Edgar Chen [:edgar]]

Comment on attachment 8556899 [details] [diff] [review]
Part 1: Ensure the order of TLVs in STK Envelope Command is the same to the order defined in TS 11.14/TS 102 223. r=echen

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

Thank you.

Comment 15

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

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

Comment 16

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

Comment on attachment 8556899 [details] [diff] [review]
Part 1: Ensure the order of TLVs in STK Envelope Command is the same to the order defined in TS 11.14/TS 102 223. r=echen

[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 17

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

Comment on attachment 8556900 [details] [diff] [review]
Part 2: Modify Test Cases Accordingly. r=echen

[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 18

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

Won't fix in v2.0 because commercial-ril is applied.

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/b2g-inbound/rev/49c741facd2a
https://hg.mozilla.org/integration/b2g-inbound/rev/1fc4f2b9d876

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/49c741facd2a
https://hg.mozilla.org/mozilla-central/rev/1fc4f2b9d876

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/e1afa494cf22
https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/a3c080f1b524

https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/c039304d7aef
https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/c2210e7a21a3

Comment 22

[jinghua.xing]

Ryan:

Can you help to uplift this changes to v2.1s? Thank you.

Comment 23

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

"The v2.1 repos (b2g34 / v2.1) are regularly merged by the device team to the v2.1S branches. Patches with v2.1 approval should not be double-landed on 2.1 and 2.1S branches." [1]

[1] https://wiki.mozilla.org/Release_Management/B2G_Landing#v2.1S

Comment 24

[Ryan VanderMeulen [:RyanVM]]

Vincent is handling the 2.1s branch.

Comment 25

[Josh Cheng [:josh]]

Hi Vincent,
Could you help to check whether this patch on 2.1 also merged to 2.1S?
Thanks!

Comment 26

[Vincent Liu[:vliu]]

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