Closed Bug 942096 Opened 11 years ago Closed 11 years ago

[B2G] [DSDS] system message format of 'icc-stkcommand'

Categories

(Firefox OS Graveyard :: RIL, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
1.3 Sprint 6 - 12/6

People

(Reporter: hsinyi, Assigned: edgar)

References

Details

(Keywords: dev-doc-needed)

Attachments

(1 file, 3 obsolete files)

Right now, each RadioInterface sends out a 'icc-stkcommand' message as below
    gSystemMessenger.broadcastMessage("icc-stkcommand", message);

The message doesn't carry information about 'service/clientId' yet and this is a miss in DSDS.

The possible solution I see would be
    gSystemMessenger.broadcastMessage("icc-stkcommand", {iccId: iccId, message: message});

This format also makes the interface b/w system message and Icc WebAPI align. How does this sound?
(In reply to Hsin-Yi Tsai  [:hsinyi] from comment #0)
> Right now, each RadioInterface sends out a 'icc-stkcommand' message as below
>     gSystemMessenger.broadcastMessage("icc-stkcommand", message);
> 
> The message doesn't carry information about 'service/clientId' yet and this
> is a miss in DSDS.
> 
> The possible solution I see would be
>     gSystemMessenger.broadcastMessage("icc-stkcommand", {iccId: iccId,
> message: message});
> 
Thanks to point this out. I will take care of gecko part.

With this change, gaia side need to do corresponding change as well.

I am going to file two gaia stk bugs:
1. Having the backward compatibility to avoid breaking single sim behavior.
2. Support multiple sim card.

> This format also makes the interface b/w system message and Icc WebAPI
> align. How does this sound?
Assignee: nobody → echen
Blocks: 942714
We need to check iccId first then send the system message. Because I found ril may receive stk event before iccId is ready when device boot up.

BTW, I will create the gaia bug for backward compatibility once the reviewer is comfortable with the naming. :)
Attachment #8338250 - Flags: feedback?(htsai)
Comment on attachment 8338250 [details] [diff] [review]
Part 1: Extend stk system message to support multiple sim, v1

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

Looks good!
Attachment #8338250 - Flags: feedback?(htsai) → feedback+
Depends on: 943229
Keywords: dev-doc-needed
Attachment #8338250 - Flags: review?(htsai)
Attachment #8338288 - Flags: review?(htsai)
Attachment #8338250 - Flags: review?(htsai) → review+
Comment on attachment 8338288 [details] [diff] [review]
Part 2: Marionette tests for stk system message, v1

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

r=me with comments addressed. Thank you!

::: dom/icc/tests/marionette/stk_helper.js
@@ +59,5 @@
> +      runNextTest();
> +    }
> +  };
> +
> +  sendEmulatorCommand("stk pdu " + pdu);

Please move this line to the end of sendStkPduToEmulator(), i.e. line#82.

@@ +62,5 @@
> +
> +  sendEmulatorCommand("stk pdu " + pdu);
> +
> +  navigator.mozSetMessageHandler("icc-stkcommand",
> +    function callHandleSTKCommand(message) {

We are fine without function name property here. Let's remove it.
Attachment #8338288 - Flags: review?(htsai) → review+
Address review comment #5.
Attachment #8338288 - Attachment is obsolete: true
Attachment #8339271 - Flags: review+
(In reply to Edgar Chen [:edgar][:echen] from comment #7)
> try server: https://tbpl.mozilla.org/?tree=Try&rev=b3a3ed2a34dd

raise ScriptTimeoutException(message=message, status=status, stacktrace=stacktrace)
TEST-UNEXPECTED-FAIL | test_stk_proactive_command.js | ScriptTimeoutException: timed out
.....

Stk related marionette tests are all time out due didn't receive 'icc-stkcommand' but I can pass these tests in my local environment. Have not idea why failed in try server yet.
(In reply to Edgar Chen [:edgar][:echen] from comment #8)
> (In reply to Edgar Chen [:edgar][:echen] from comment #7)
> > try server: https://tbpl.mozilla.org/?tree=Try&rev=b3a3ed2a34dd
> 
> raise ScriptTimeoutException(message=message, status=status,
> stacktrace=stacktrace)
> TEST-UNEXPECTED-FAIL | test_stk_proactive_command.js |
> ScriptTimeoutException: timed out
> .....
> 
> Stk related marionette tests are all time out due didn't receive
> 'icc-stkcommand' but I can pass these tests in my local environment. Have
> not idea why failed in try server yet.

I can reproduce this in my local environment by running all marionette tests.
(In reply to Edgar Chen [:edgar][:echen] from comment #9)
> (In reply to Edgar Chen [:edgar][:echen] from comment #8)
> 
> I can reproduce this in my local environment by running all marionette tests.

It seems the failed is related to the implementation of marionette framework and system message, and needs more time to investigate and figure out how to fix. In order not to block Gaia development, I decide to separate test case into a follow-up bug.
Blocks: 945576
Correct patch title
Attachment #8338250 - Attachment is obsolete: true
Attachment #8339271 - Attachment is obsolete: true
Attachment #8341495 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/84012aceec07
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.3 Sprint 6 - 12/6
Blocks: 946165
Blocks: 934404
Blocks: 951586
Blocks: 999370
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: