1031640 - B2G RIL: incorrect STK TR for handling SEND_SS/SEND_USSD/SEND_SHORT_MESSAGE/SEND_DTMF in ril_worker.js.

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

[shawn ku [:sku]]

By checking the 3GPP TS 11.14, 6.6.9 SEND SHORT MESSAGE, 6.6.10 SEND SS, 6.6.11 SEND USSD and 6.6.12 SET UP CALL, Alpha identifiers are all optional field.

However, processEventNotify@ril_worker.js (see [1]) will send STK_RESULT_REQUIRED_VALUES_MISSING TR to modem/uicc if alpha identifier is absent. That is not a correct behavior which will cause malfunction of STK in modem side.

[1] - http://dxr.mozilla.org/mozilla-central/source/dom/system/gonk/ril_worker.js?from=ril_worker.js&case=true#10612

Comment 1

[shawn ku [:sku]]

wrong reference line of code.

update correct one -
http://dxr.mozilla.org/mozilla-central/source/dom/system/gonk/ril_worker.js#10676

Comment 2

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

Check http://www.kandroid.org/online-pdk/guide/stk.html

These proactive commands have been processed by the modem so they come in another RIL Unsolicited response 'RIL_UNSOL_STK_EVENT_NOTIFY'.

From the page,
'This distinction is an implementation detail of Android and is not defined in the 3GPP sepcifications.'

And at that time I checked AOSP and CAF will check the AlphaId as mandatory item.

Comment 3

[shawn ku [:sku]]

(In reply to Yoshi Huang[:allstars.chh] from comment #2)
> Check http://www.kandroid.org/online-pdk/guide/stk.html
> 
> These proactive commands have been processed by the modem so they come in
> another RIL Unsolicited response 'RIL_UNSOL_STK_EVENT_NOTIFY'.
> 
> From the page,
> 'This distinction is an implementation detail of Android and is not defined
> in the 3GPP sepcifications.'
> 
> And at that time I checked AOSP and CAF will check the AlphaId as mandatory
> item.

Hi Yoshi:
 Bug 1033974 was filed for gaia to reply if they would like to follow AOSP behavior or not.
Thanks!!
Shawn

Comment 4

[shawn ku [:sku]]

Attachment: Bug 1031640 Part 1: RIL patch - B2G RIL: incorrect STK TR for handling SEND_SS/SEND_USSD/SEND_SHORT_MESSAGE/SEND_DTMF in ril_worker.js.

Comment 5

[shawn ku [:sku]]

Attachment: Bug 1031640 Part 2: test patch - B2G RIL: incorrect STK TR for handling SEND_SS/SEND_USSD/SEND_SHORT_MESSAGE/SEND_DTMF in ril_worker.js.

Comment 6

[Edgar Chen [:edgar]]

Checking the document again [1]:
'The upper layers handle the UI and the STK RIL handles all other aspects of each command, which means that the STK RIL sends the terminal response (it is never sent by the STK App)'

It seems we don't need to send any terminal response for RIL_UNSOL_STK_EVENT_NOTIFY.

[1] http://www.kandroid.org/online-pdk/guide/stk.html

Comment 7

[Edgar Chen [:edgar]]

Comment on attachment 8452895 [details] [diff] [review]
Bug 1031640 Part 1: RIL patch - B2G RIL: incorrect STK TR for handling SEND_SS/SEND_USSD/SEND_SHORT_MESSAGE/SEND_DTMF in ril_worker.js.

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

Thank you.

Comment 8

[Edgar Chen [:edgar]]

Comment on attachment 8452896 [details] [diff] [review]
Bug 1031640 Part 2: test patch - B2G RIL: incorrect STK TR for handling SEND_SS/SEND_USSD/SEND_SHORT_MESSAGE/SEND_DTMF in ril_worker.js.

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

::: dom/icc/tests/marionette/test_stk_send_dtmf.js
@@ +19,3 @@
>              commandQualifier: 0x00,
>              text: "Send DTMF"}},
> +  {command: "d010810301140082028183ac052143658709",

I found many tests have the same pdu.
Please help to remove the duplicated one.
Thank you.

::: dom/icc/tests/marionette/test_stk_send_sms.js
@@ +64,3 @@
>              commandQualifier: 0x00,
>              title: "Two types are defined: - A short message to be sent to the network in an SMS-SUBMIT message, or an SMS-COMMAND message, where the user data can be passed transparently; - A short message to be sent to the network in an SMS-SUBMIT "}},
> +  {command: "d0148103011300820281838b09010002911040f00120",

Ditto

::: dom/icc/tests/marionette/test_stk_send_ss.js
@@ +19,3 @@
>              commandQualifier: 0x00,
>              title: "Call Forward"}},
> +  {command: "d01b810301110082028183891091aa120a214365870921436587a901fb",

Ditto

::: dom/icc/tests/marionette/test_stk_send_ussd.js
@@ +19,3 @@
>              commandQualifier: 0x00,
>              title: "7-bit USSD"}},
> +  {command: "d0448103011200820281838a39f041e19058341e9149e592d9743ea151e9945ab55eb1596d2b2c1e93cbe6333aad5eb3dbee373c2e9fd3ebf63b3eaf6fc564335acd76c3e560",

Ditto

Comment 9

[shawn ku [:sku]]

Attachment: Bug 1031640 Part 2: test patch - B2G RIL: incorrect STK TR for handling SEND_SS/SEND_USSD/SEND_SHORT_MESSAGE/SEND_DTMF in ril_worker.js. v2.

remove redundant test cases.

Comment 10

[Edgar Chen [:edgar]]

Comment on attachment 8454306 [details] [diff] [review]
Bug 1031640 Part 2: test patch - B2G RIL: incorrect STK TR for handling SEND_SS/SEND_USSD/SEND_SHORT_MESSAGE/SEND_DTMF in ril_worker.js. v2.

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

Looks good to me, thank you.
(I have to confess I didn't check every pdu very carefully, but I did verify all tests by running them on my local environment)

Comment 11

[shawn ku [:sku]]

(In reply to Edgar Chen [:edgar][:echen] from comment #10)
> Comment on attachment 8454306 [details] [diff] [review]
> Bug 1031640 Part 2: test patch - B2G RIL: incorrect STK TR for handling
> SEND_SS/SEND_USSD/SEND_SHORT_MESSAGE/SEND_DTMF in ril_worker.js. v2.
> 
> Review of attachment 8454306 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good to me, thank you.
> (I have to confess I didn't check every pdu very carefully, but I did verify
> all tests by running them on my local environment)

Don't worry, I check every single PDU before submit patch. :)

Comment 12

[shawn ku [:sku]]

Attachment: Bug 1031640 Part 1: RIL patch - B2G RIL: incorrect STK TR for handling SEND_SS/SEND_USSD/SEND_SHORT_MESSAGE/SEND_DTMF in ril_worker.js. r=echen.

Comment 13

[shawn ku [:sku]]

Attachment: Bug 1031640 Part 2: test patch - B2G RIL: incorrect STK TR for handling SEND_SS/SEND_USSD/SEND_SHORT_MESSAGE/SEND_DTMF in ril_worker.js. v2. r=echen.

Comment 14

[shawn ku [:sku]]

try is green - https://tbpl.mozilla.org/?tree=Try&rev=8a64ff9c78b5

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/4ecf16fdc396
https://hg.mozilla.org/integration/b2g-inbound/rev/e89a338aa489

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4ecf16fdc396
https://hg.mozilla.org/mozilla-central/rev/e89a338aa489

Comment 17

[shawn ku [:sku]]

need a patch for 2.0m due to incorrect handling for STK event notify.
nom. 2.0m? here.

Comment 18

[shawn ku [:sku]]

Attachment: [2.0m] Bug 1031640 - B2G RIL: incorrect STK TR for handling SEND_SS/SEND_USSD/SEND_SHORT_MESSAGE/SEND_DTMF in ril_worker.js. r=Edgar.

Comment 19

[shawn ku [:sku]]

2.0m try green - https://tbpl.mozilla.org/?tree=Try&rev=69007fbad987

Comment 20

[Kai-Zhen Li [:seinlin][:kli]]

http://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/d97ea9baa399