Closed Bug 1031640 Opened 5 years ago Closed 5 years ago

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

Categories

(Firefox OS Graveyard :: RIL, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:2.0M+, b2g-v2.0 wontfix, b2g-v2.0M fixed, b2g-v2.1 fixed, b2g-v2.2 fixed)

RESOLVED FIXED
2.0 S6 (18july)
blocking-b2g 2.0M+
Tracking Status
b2g-v2.0 --- wontfix
b2g-v2.0M --- fixed
b2g-v2.1 --- fixed
b2g-v2.2 --- fixed

People

(Reporter: sku, Assigned: sku)

References

Details

Attachments

(3 files, 3 obsolete files)

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
wrong reference line of code.

update correct one -
http://dxr.mozilla.org/mozilla-central/source/dom/system/gonk/ril_worker.js#10676
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.
Assignee: nobody → 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
Attachment #8452895 - Flags: review?(echen)
Attachment #8452896 - Flags: review?(echen)
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 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.
Attachment #8452895 - Flags: review?(echen) → review+
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
Attachment #8452896 - Flags: review?(echen)
Attachment #8454306 - Flags: review?(echen)
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)
Attachment #8454306 - Flags: review?(echen) → review+
(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. :)
https://hg.mozilla.org/mozilla-central/rev/4ecf16fdc396
https://hg.mozilla.org/mozilla-central/rev/e89a338aa489
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.0 S6 (18july)
need a patch for 2.0m due to incorrect handling for STK event notify.
nom. 2.0m? here.
blocking-b2g: --- → 2.0M?
Flags: needinfo?(sku)
Blocks: Woodduck
blocking-b2g: 2.0M? → 2.0M+
No longer blocks: Woodduck
Blocks: Woodduck
OS: Linux → Gonk (Firefox OS)
Hardware: x86_64 → ARM
Blocks: 1080481
2.0m try green - https://tbpl.mozilla.org/?tree=Try&rev=69007fbad987
Flags: needinfo?(kli)
Keywords: checkin-needed
Duplicate of this bug: 1081796
You need to log in before you can comment on or make changes to this bug.