Last Comment Bug 791935 - B2G STK: Implement 'MT Call Event', 'Call Connected' and 'Call Disconnected' Envelope commands
: B2G STK: Implement 'MT Call Event', 'Call Connected' and 'Call Disconnected' ...
Status: RESOLVED FIXED
[LOE: M]
: dev-doc-needed
Product: Core
Classification: Components
Component: DOM: Device Interfaces (show other bugs)
: Trunk
: ARM Gonk (Firefox OS)
: -- normal (vote)
: mozilla18
Assigned To: Patrick Wang (Chih-Kai Wang) (:kk1fff)
:
Mentors:
: 791936 791938 (view as bug list)
Depends on: 792335
Blocks: b2g-stk 800264
  Show dependency treegraph
 
Reported: 2012-09-17 23:44 PDT by Yoshi Huang[:allstars.chh]
Modified: 2012-10-11 00:03 PDT (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+


Attachments
WIP: Send MT Call envelope when call arrives. (5.54 KB, patch)
2012-09-19 02:00 PDT, Patrick Wang (Chih-Kai Wang) (:kk1fff)
allstars.chh: feedback+
Details | Diff | Splinter Review
WIP: IDL Change for MT Call, Call connected, Call Disconnected Envelope (2.32 KB, patch)
2012-09-20 05:50 PDT, Patrick Wang (Chih-Kai Wang) (:kk1fff)
no flags Details | Diff | Splinter Review
WIP: IDL Change for MT Call, Call connected, Call Disconnected Envelope (2.32 KB, patch)
2012-09-20 05:56 PDT, Patrick Wang (Chih-Kai Wang) (:kk1fff)
no flags Details | Diff | Splinter Review
WIP: IDL Change for MT Call, Call connected, Call Disconnected Envelope (2.22 KB, patch)
2012-09-20 20:05 PDT, Patrick Wang (Chih-Kai Wang) (:kk1fff)
allstars.chh: feedback+
Details | Diff | Splinter Review
WIP: IDL Change for MT Call, Call connected, Call Disconnected Envelope (2.31 KB, patch)
2012-09-21 04:23 PDT, Patrick Wang (Chih-Kai Wang) (:kk1fff)
no flags Details | Diff | Splinter Review
WIP: Add call connected, call disconnected and mt call Envelope command to sendStkEventDownlad (7.91 KB, patch)
2012-09-23 21:49 PDT, Patrick Wang (Chih-Kai Wang) (:kk1fff)
allstars.chh: feedback+
Details | Diff | Splinter Review
Add call connected, call disconnected and mt call Envelope command to sendStkEventDownlad (7.81 KB, patch)
2012-09-25 03:01 PDT, Patrick Wang (Chih-Kai Wang) (:kk1fff)
no flags Details | Diff | Splinter Review
Test case for writeErrorNumber (1.35 KB, patch)
2012-09-25 03:02 PDT, Patrick Wang (Chih-Kai Wang) (:kk1fff)
no flags Details | Diff | Splinter Review
Part1: IDL Change for MT Call, Call connected, Call Disconnected Envelope (2.32 KB, patch)
2012-09-25 03:39 PDT, Patrick Wang (Chih-Kai Wang) (:kk1fff)
philipp: review+
Details | Diff | Splinter Review
Part 2: Add call connected, call disconnected and mt call Envelope command to sendStkEventDownlad (7.79 KB, patch)
2012-09-25 03:45 PDT, Patrick Wang (Chih-Kai Wang) (:kk1fff)
philipp: review+
Details | Diff | Splinter Review
Test case for writeCauseTlv (1.35 KB, patch)
2012-09-25 04:12 PDT, Patrick Wang (Chih-Kai Wang) (:kk1fff)
philipp: review+
Details | Diff | Splinter Review
Part 1: IDL Change for MT Call, Call connected, Call Disconnected Envelope (2.32 KB, patch)
2012-09-26 03:41 PDT, Patrick Wang (Chih-Kai Wang) (:kk1fff)
no flags Details | Diff | Splinter Review
Part 2: Add call connected, call disconnected and mt call Envelope command to sendStkEventDownlad (7.77 KB, patch)
2012-09-26 03:42 PDT, Patrick Wang (Chih-Kai Wang) (:kk1fff)
no flags Details | Diff | Splinter Review
Part 1: IDL Change for MT Call, Call connected, Call Disconnected Envelope (2.39 KB, patch)
2012-09-27 03:19 PDT, Patrick Wang (Chih-Kai Wang) (:kk1fff)
jonas: superreview+
Details | Diff | Splinter Review
Part 2: Add call connected, call disconnected and mt call Envelope command to sendStkEventDownlad (6.15 KB, patch)
2012-09-27 03:33 PDT, Patrick Wang (Chih-Kai Wang) (:kk1fff)
no flags Details | Diff | Splinter Review
Part 1: IDL Change for MT Call, Call connected, Call Disconnected Envelope (2.41 KB, patch)
2012-09-30 21:22 PDT, Patrick Wang (Chih-Kai Wang) (:kk1fff)
no flags Details | Diff | Splinter Review
Part 2: Add call connected, call disconnected and mt call Envelope command to sendStkEventDownlad (9.01 KB, patch)
2012-09-30 21:24 PDT, Patrick Wang (Chih-Kai Wang) (:kk1fff)
no flags Details | Diff | Splinter Review
Test case (3.49 KB, patch)
2012-09-30 21:36 PDT, Patrick Wang (Chih-Kai Wang) (:kk1fff)
no flags Details | Diff | Splinter Review

Description Yoshi Huang[:allstars.chh] 2012-09-17 23:44:14 PDT
See TS 11.14 clause 11.1
Comment 1 Patrick Wang (Chih-Kai Wang) (:kk1fff) 2012-09-19 02:00:56 PDT
Created attachment 662460 [details] [diff] [review]
WIP: Send MT Call envelope when call arrives.
Comment 2 Yoshi Huang[:allstars.chh] 2012-09-19 02:12:35 PDT
Comment on attachment 662460 [details] [diff] [review]
WIP: Send MT Call envelope when call arrives.

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

Hi, Patrick
Thanks for your great work.

::: dom/system/gonk/ril_worker.js
@@ +2198,5 @@
> +    command.deviceId = {
> +      sourceId: STK_DEVICE_ID_ME,
> +      destinationId: STK_DEVICE_ID_NETWORK
> +    };
> +    command.eventList = 00;  // MT Call

In the Part 2 Patch of Bug 790543, there is a const STK_EVENT_TYPE_MT_CALL for this.

@@ +2259,5 @@
>                   (options.locationStatus ? 3 : 0) +
>                   (options.locationInfo ?
>                      (options.locationInfo.cellId > 0xffff ? 11 : 9) :
> +                    0) +
> +                 (options.transactionId ? options.transactionId.length + 2 : 0);

From TS 11.14, clause 11.1.2 
"Transaction identifier: the transaction identifier data object shall contain *one* transaction identifier", so I think the length of the ComprehesionTLV should be 3.

@@ +2342,5 @@
> +    // Transaction Id
> +    if (options.transactionId) {
> +      GsmPDUHelper.writeHexOctet(COMPREHENSIONTLV_TAG_TRANSACTION_ID |
> +                                 COMPREHENSIONTLV_FLAG_CR);
> +      GsmPDUHelper.writeHexOctet(options.transactionId.length);

GsmPDUHelper.writeHexOctet(1);

@@ +2345,5 @@
> +                                 COMPREHENSIONTLV_FLAG_CR);
> +      GsmPDUHelper.writeHexOctet(options.transactionId.length);
> +      for (let i = 0; i < options.transactionId.length; i++) {
> +        GsmPDUHelper.writeHexOctet(options.transactionId[i]);
> +      }

ditto, there should be only 1 trans-id

@@ +2902,5 @@
>          newCall.isActive = this._isActiveCall(newCall.state);
> +        this.sendStkMTCall({
> +          address: newCall.number,
> +          transactionId: [0]
> +        });

might need to discuss here,
should we do this in gecko?
Comment 3 Patrick Wang (Chih-Kai Wang) (:kk1fff) 2012-09-20 05:50:13 PDT
Created attachment 662982 [details] [diff] [review]
WIP: IDL Change for MT Call, Call connected, Call Disconnected Envelope

1. For transaction ID, we could query it with phone number.
2. For cause of "Call disconnected" envelope, we could get it from the error string that is passed to gaia.
Comment 4 Patrick Wang (Chih-Kai Wang) (:kk1fff) 2012-09-20 05:52:05 PDT
*** Bug 791936 has been marked as a duplicate of this bug. ***
Comment 5 Patrick Wang (Chih-Kai Wang) (:kk1fff) 2012-09-20 05:56:57 PDT
Created attachment 662986 [details] [diff] [review]
WIP: IDL Change for MT Call, Call connected, Call Disconnected Envelope

Correct typo in previous version.
Comment 6 Patrick Wang (Chih-Kai Wang) (:kk1fff) 2012-09-20 05:58:01 PDT
*** Bug 791938 has been marked as a duplicate of this bug. ***
Comment 7 Yoshi Huang[:allstars.chh] 2012-09-20 20:03:22 PDT
Sorry for not writing detail description in the first place,
this feature is needed by our partners to run their SIM apps,
the SIM apps need to be notified when these call events happen.
Comment 8 Patrick Wang (Chih-Kai Wang) (:kk1fff) 2012-09-20 20:05:57 PDT
Created attachment 663258 [details] [diff] [review]
WIP: IDL Change for MT Call, Call connected, Call Disconnected Envelope

Making call events to share the same dictionary data structure.
Comment 9 Yoshi Huang[:allstars.chh] 2012-09-20 20:40:50 PDT
Comment on attachment 663258 [details] [diff] [review]
WIP: IDL Change for MT Call, Call connected, Call Disconnected Envelope

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

Excellent,
but I think we should provide a better documentation here so others can understand this more easily.

::: dom/icc/interfaces/SimToolKit.idl
@@ +415,5 @@
> +
> +dictionary MozStkCallEvent
> +{
> +  /**
> +   * Remote phone number.

Just discuss with Hsinyi, 
'Remote Party number' may be a better description.

@@ +420,5 @@
> +   */
> +  DOMString number;
> +
> +  /**
> +   * This field available in Call Connected and Call Disconnected events.

This field *is* available.

And maybe you could use those constants directly, say 
'STK_EVENT_TYPE_CALL_CONNECTED', 'STK_EVENT_TYPE_CALL_DISCONNECTED'

@@ +421,5 @@
> +  DOMString number;
> +
> +  /**
> +   * This field available in Call Connected and Call Disconnected events.
> +   * For Call Connected, setting this true means the connection is answered by

setting this *to* true.

@@ +424,5 @@
> +   * This field available in Call Connected and Call Disconnected events.
> +   * For Call Connected, setting this true means the connection is answered by
> +   * remote end, that is, this is an outgoing call. For Call Disconnected,
> +   * setting this true indicates the connection is hung up by remote.
> +   */

I think we can make this comment more simpler, as you already did.

For STK_EVENT_TYPE_CALL_CONNECTED, setting this to true for an outgoing call, false otherwise.

For STK_EVENT_TYPE_CALL_DISCONNECTED, ...

@@ +429,5 @@
> +  boolean isIssuedByRemote;
> +
> +  /**
> +   * This field is available in Call Disconnected event to indicate the cause
> +   * of disconnection, the failure string that is with notifyError

Can you provide more documentation about this 'notifyError' ?

::: dom/icc/interfaces/nsIDOMIccManager.idl
@@ +200,5 @@
>     * @param eventData
>     *        Data for this event.
>     *        When eventType is STK_EVENT_TYPE_LOCATION_STATUS,
>     *        eventData is MozStkLocationEvent.
> +   *        For call events, say, the eventType is STK_EVENT_TYPE_MT_CALL,

When eventType is 
- STK_EVENT_TYPE_MT_CALL,
- ...

@@ +202,5 @@
>     *        When eventType is STK_EVENT_TYPE_LOCATION_STATUS,
>     *        eventData is MozStkLocationEvent.
> +   *        For call events, say, the eventType is STK_EVENT_TYPE_MT_CALL,
> +   *        STK_EVENT_TYPE_CALL_CONNECTED and STK_EVENT_TYPE_CALL_DISCONNECTED,
> +   *        eventData shell be MozStkCallEvent.

*is*, or at least 'shall be'
Comment 10 Hsin-Yi Tsai [:hsinyi] 2012-09-20 20:55:09 PDT
(In reply to Yoshi Huang[:yoshi][:allstars.chh] from comment #9)
> Comment on attachment 663258 [details] [diff] [review]
> WIP: IDL Change for MT Call, Call connected, Call Disconnected Envelope
> 
> Review of attachment 663258 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Excellent,
> but I think we should provide a better documentation here so others can
> understand this more easily.
> 
> ::: dom/icc/interfaces/SimToolKit.idl
> @@ +415,5 @@
> > +
> > +dictionary MozStkCallEvent
> > +{
> > +  /**
> > +   * Remote phone number.
> 
> Just discuss with Hsinyi, 
> 'Remote Party number' may be a better description.
I think we can just use 'Call number' here.
Comment 11 Hsin-Yi Tsai [:hsinyi] 2012-09-20 23:30:47 PDT
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #10)
> (In reply to Yoshi Huang[:yoshi][:allstars.chh] from comment #9)
> > Comment on attachment 663258 [details] [diff] [review]
> > WIP: IDL Change for MT Call, Call connected, Call Disconnected Envelope
> > 
> > Review of attachment 663258 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > Excellent,
> > but I think we should provide a better documentation here so others can
> > understand this more easily.
> > 
> > ::: dom/icc/interfaces/SimToolKit.idl
> > @@ +415,5 @@
> > > +
> > > +dictionary MozStkCallEvent
> > > +{
> > > +  /**
> > > +   * Remote phone number.
> > 
> > Just discuss with Hsinyi, 
> > 'Remote Party number' may be a better description.
> I think we can just use 'Call number' here.
ril.h uses 'Remote party number'. It seems better to take yoshi's suggestion here.
Comment 12 Patrick Wang (Chih-Kai Wang) (:kk1fff) 2012-09-21 04:23:03 PDT
Created attachment 663356 [details] [diff] [review]
WIP: IDL Change for MT Call, Call connected, Call Disconnected Envelope
Comment 13 Patrick Wang (Chih-Kai Wang) (:kk1fff) 2012-09-23 21:49:23 PDT
Created attachment 663915 [details] [diff] [review]
WIP: Add call connected, call disconnected and mt call Envelope command to sendStkEventDownlad

Haven't found the transaction id and subaddress (How do we get the SETUP message that described in ts 04.08?)
Comment 14 Yoshi Huang[:allstars.chh] 2012-09-23 23:12:45 PDT
Comment on attachment 663915 [details] [diff] [review]
WIP: Add call connected, call disconnected and mt call Envelope command to sendStkEventDownlad

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

Excellent, pretty close now.
Can you check my comments below, and see if my opinion makes sense?

Thank you.

::: dom/system/gonk/ril_worker.js
@@ +2310,5 @@
> +        command.deviceId = {
> +          sourceId :STK_DEVICE_ID_NETWORK,
> +          destinationId: STK_DEVICE_ID_SIM
> +        };
> +        command.eventList = STK_EVENT_TYPE_MT_CALL;

Since each case inside switch uses this, 
do you think can we move this line before 'switch'? for instance,
command.eventList = command.eventType;

@@ +2311,5 @@
> +          sourceId :STK_DEVICE_ID_NETWORK,
> +          destinationId: STK_DEVICE_ID_SIM
> +        };
> +        command.eventList = STK_EVENT_TYPE_MT_CALL;
> +        command.transactionId = 

trailing white space, and some lines below.

@@ +2335,5 @@
> +        command.eventList = STK_EVENT_TYPE_CALL_DISCONNECTED;
> +        command.transactionId = 
> +          this._getTransactionIdByNumber(command.eventData.number);
> +        command.cause = command.eventData.error;
> +        break;

Also I notice these two cases use some similar code, can we use 'fall through' for these? 
like 
case STK_EVENT_TYPE_CALL_CONNECTED:
case STK_EVENT_TYPE_CALL_DISCONNECTED: // Fall through
...

@@ +5771,5 @@
>    },
> +
> +  /**
> +   * Give a geckoError string, this function translate into cause value
> +   * and write the value into buffer.

My English is not quite well, but these comments seems to have some problems, or the sentence seems not well formatted, for example, *Given*, or *translates*, can you check this again?

Also add @param geckoError and add some comments for it.

@@ +5773,5 @@
> +  /**
> +   * Give a geckoError string, this function translate into cause value
> +   * and write the value into buffer.
> +   */
> +  writeErrorNumber(geckoError) {

I think this utility function is for Cause dat object of Comprehension-TLV.
Can you move this function into ComprehentionTlvHelper?

@@ +5783,5 @@
> +      }
> +    }
> +    cause = (cause == -1) ? ERROR_SUCCESS : cause;
> +    this.writeHexOctet(0x60);
> +    this.writeHexOctet(0x80 | cause);

Although we talk about this, but others might not know what these means, also maybe several months later we also forget this.
Can you add some documentations for this ?
For example, You can check several examples from SMS-related or ICC-related functions in ril_worker.
Comment 15 Patrick Wang (Chih-Kai Wang) (:kk1fff) 2012-09-25 03:01:17 PDT
Created attachment 664416 [details] [diff] [review]
Add call connected, call disconnected and mt call Envelope command to sendStkEventDownlad
Comment 16 Patrick Wang (Chih-Kai Wang) (:kk1fff) 2012-09-25 03:02:15 PDT
Created attachment 664417 [details] [diff] [review]
Test case for writeErrorNumber
Comment 17 Patrick Wang (Chih-Kai Wang) (:kk1fff) 2012-09-25 03:39:43 PDT
Created attachment 664427 [details] [diff] [review]
Part1: IDL Change for MT Call, Call connected, Call Disconnected Envelope

Hi, philikon,

The IDL is based on patch of bug 790547, adds support for STK call events.
Would you help to review this for me? Thanks.
Comment 18 Patrick Wang (Chih-Kai Wang) (:kk1fff) 2012-09-25 03:45:56 PDT
Created attachment 664429 [details] [diff] [review]
Part 2: Add call connected, call disconnected and mt call Envelope command to sendStkEventDownlad

Hi, philikon,

This patch is the implementation of sending MT call, call connected and call disconnected envelopes.
Would you help to review this for me?
Thanks.
Comment 19 Patrick Wang (Chih-Kai Wang) (:kk1fff) 2012-09-25 04:12:59 PDT
Created attachment 664437 [details] [diff] [review]
Test case for writeCauseTlv

Hi, philikon,

The test case of writeCauseTlv of part 2.
Would you help to review this for me?

Thanks.
Comment 20 Philipp von Weitershausen [:philikon] 2012-09-25 16:09:56 PDT
Comment on attachment 664427 [details] [diff] [review]
Part1: IDL Change for MT Call, Call connected, Call Disconnected Envelope

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

r+ with the nits below addressed. Although keep in mind what I pointed out in bug 790547 comment 27 about keeping the object hierarchies flatter.

::: dom/icc/interfaces/SimToolKit.idl
@@ +432,5 @@
> +
> +  /**
> +   * This field is available in Call Disconnected event to indicate the cause
> +   * of disconnection, the failure string that is with
> +   * nsIRILTelephonyCallback.notifyError() callback. Null if there's no error.

nsIRILTelephonyCallback is an internal interface. It makes no sense to mention it here. (Also, the grammar of that sentence doesn't quite work.) Please write this documentation for somebody who knows what the STK is but doesn't know the implementation details of Gecko.

::: dom/icc/interfaces/nsIDOMIccManager.idl
@@ +200,5 @@
>     * @param eventData
>     *        Data for this event.
>     *        When eventType is STK_EVENT_TYPE_LOCATION_STATUS,
>     *        eventData is MozStkLocationEvent.
> +   *        When event type is STK_EVENT_TYPE_MT_CALL,

s/event type/eventType/
Comment 21 Philipp von Weitershausen [:philikon] 2012-09-25 16:14:32 PDT
Comment on attachment 664429 [details] [diff] [review]
Part 2: Add call connected, call disconnected and mt call Envelope command to sendStkEventDownlad

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

Looks good apart from the two things mentioned below. Please address them. Also, Yoshi and Hsinyi have written unit tests for their TLV helper code. Please do the same.

::: dom/system/gonk/ril_worker.js
@@ +2287,5 @@
>     * @param eventData
>     */
>    sendStkEventDownload: function sendStkEventDownload(command) {
>      command.tag = BER_EVENT_DOWNLOAD_TAG;
> +    command.eventList = command.eventType;

This is starting to look like a no-op to me. Why is this line necessary, why can't we simply look at `command.eventType` in the rest of the code?

@@ +6501,5 @@
> +   * @param geckoError Error string that is passed to gecko.
> +   */
> +  writeCauseTlv: function writeCauseTlv(geckoError) {
> +    let cause = -1;
> +    for each(let errorNo in Object.keys(RIL_ERROR_TO_GECKO_ERROR)) {

for (errorNo in RIL_ERROR_TO_GECKO_ERROR)
Comment 22 Philipp von Weitershausen [:philikon] 2012-09-25 16:15:42 PDT
Comment on attachment 664429 [details] [diff] [review]
Part 2: Add call connected, call disconnected and mt call Envelope command to sendStkEventDownlad

Bah, for some I reason I wasn't aware of your tests patch. Never mind! r=me with comment 21 addressed.
Comment 23 Philipp von Weitershausen [:philikon] 2012-09-25 16:16:52 PDT
Comment on attachment 664437 [details] [diff] [review]
Test case for writeCauseTlv

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

Great. Can we also get tests for the new envelope command type you added support for?
Comment 24 Yoshi Huang[:allstars.chh] 2012-09-26 00:02:06 PDT
(In reply to Philipp von Weitershausen [:philikon] from comment #21)
> Comment on attachment 664429 [details] [diff] [review]
> Part 2: Add call connected, call disconnected and mt call Envelope command
> to sendStkEventDownlad
> 
> ::: dom/system/gonk/ril_worker.js
> @@ +2287,5 @@
> >     * @param eventData
> >     */
> >    sendStkEventDownload: function sendStkEventDownload(command) {
> >      command.tag = BER_EVENT_DOWNLOAD_TAG;
> > +    command.eventList = command.eventType;
> 
> This is starting to look like a no-op to me. Why is this line necessary, why
> can't we simply look at `command.eventType` in the rest of the code?
> 

Hi philikon:
The eventList naming is from STK spec(TS 11.14), actually it will be written as a TLV, and its value part is a list with only *one* event in it.

So in IDL we try to prevent confusion, we simply renamed it to eventType.

That's why we did the renaming here, eventList is to confirm the STK spec, eventType is for IDL and in order to have a better understanding of this value.

How do you think?

Thank you.
Comment 25 Patrick Wang (Chih-Kai Wang) (:kk1fff) 2012-09-26 03:14:28 PDT
(In reply to Philipp von Weitershausen [:philikon] from comment #23)
> Comment on attachment 664437 [details] [diff] [review]
> Test case for writeCauseTlv
> 
> Review of attachment 664437 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Great. Can we also get tests for the new envelope command type you added
> support for?

These envelopes are processed by SIM card and they are not expected to have response values. I think it might not easy to write a test case for them. Instead, unit tests for each TLV are provided to make sure each parts of an envelop are correct.
Comment 26 Patrick Wang (Chih-Kai Wang) (:kk1fff) 2012-09-26 03:41:29 PDT
Created attachment 664871 [details] [diff] [review]
Part 1: IDL Change for MT Call, Call connected, Call Disconnected Envelope

Fix nits.
Comment 27 Patrick Wang (Chih-Kai Wang) (:kk1fff) 2012-09-26 03:42:22 PDT
Created attachment 664872 [details] [diff] [review]
Part 2: Add call connected, call disconnected and mt call Envelope command to sendStkEventDownlad

Fix previous version with comment 21.
Comment 28 Patrick Wang (Chih-Kai Wang) (:kk1fff) 2012-09-27 03:19:36 PDT
Created attachment 665328 [details] [diff] [review]
Part 1: IDL Change for MT Call, Call connected, Call Disconnected Envelope

Hi Jonas,

The interface is based on IDL proposed in bug 790547. Add MT Call, Call connected and Call disconnected envelopes.

Thank you.
Comment 29 Patrick Wang (Chih-Kai Wang) (:kk1fff) 2012-09-27 03:33:09 PDT
Created attachment 665330 [details] [diff] [review]
Part 2: Add call connected, call disconnected and mt call Envelope command to sendStkEventDownlad

Rebased on bug 790547.
Comment 30 Philipp von Weitershausen [:philikon] 2012-09-27 22:56:35 PDT
(In reply to Yoshi Huang[:yoshi][:allstars.chh] from comment #24)
> The eventList naming is from STK spec(TS 11.14), actually it will be written
> as a TLV, and its value part is a list with only *one* event in it.
> 
> So in IDL we try to prevent confusion, we simply renamed it to eventType.
> 
> That's why we did the renaming here, eventList is to confirm the STK spec,
> eventType is for IDL and in order to have a better understanding of this
> value.

Ah, ok that makes sense.
Comment 31 Yoshi Huang[:allstars.chh] 2012-09-30 19:59:27 PDT
I just discussed with kk1fff,
his patch depends on patch from Bug 791939,
but Bug 791939 might need to test on the SIM apps first.

In order not to prevent blocking this patch,
I suggested kk1fff can move the util function he will use from Bug 791939 into this patch.
Comment 32 Hsin-Yi Tsai [:hsinyi] 2012-09-30 21:09:29 PDT
(In reply to Yoshi Huang[:yoshi][:allstars.chh] from comment #31)
> I just discussed with kk1fff,
> his patch depends on patch from Bug 791939,
> but Bug 791939 might need to test on the SIM apps first.
> 
> In order not to prevent blocking this patch,
> I suggested kk1fff can move the util function he will use from Bug 791939
> into this patch.

Patrick, I agree with yoshi's comment here, and sorry for blocking you for a while. Please feel free to move the parts about "getSizeOfLengthOctets()" and "writeLength()" into yours. I'will rebase my patches then. :)
Comment 33 Patrick Wang (Chih-Kai Wang) (:kk1fff) 2012-09-30 21:22:19 PDT
Created attachment 666424 [details] [diff] [review]
Part 1: IDL Change for MT Call, Call connected, Call Disconnected Envelope

Update patch message.
Comment 34 Patrick Wang (Chih-Kai Wang) (:kk1fff) 2012-09-30 21:24:52 PDT
Created attachment 666426 [details] [diff] [review]
Part 2: Add call connected, call disconnected and mt call Envelope command to sendStkEventDownlad

1. Rebase.
2. Move "getSizeOfLengthOctets()" and "writeLength()" functions from bug 791939.
Comment 35 Patrick Wang (Chih-Kai Wang) (:kk1fff) 2012-09-30 21:36:38 PDT
Created attachment 666428 [details] [diff] [review]
Test case

1. Rebase.
2. Merge helper test cases for getSizeOfLengthOctets() and writeLength() from bug 791939.

Note You need to log in before you can comment on or make changes to this bug.