Closed Bug 795235 Opened 7 years ago Closed 7 years ago

B2G RIL: Use system message to notify a 'STK dialling' call

Categories

(Core :: DOM: Device Interfaces, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla18
blocking-basecamp +

People

(Reporter: hsinyi, Assigned: hsinyi)

References

Details

Attachments

(1 file, 1 obsolete file)

With support by STK, a user can make an outgoing call from STK app when dialer app is closed. We need to launch dialer when there comes a dailing call. Then a user can handle the call as usual.
See also https://github.com/mozilla-b2g/gaia/issues/5394,

This feature is needed by our partner to run their SIM apps.

In STK, SIM has a proactive command called CALL Set up to request user if he wants to make the MO call,
once user has confirmed this request,
SIM will set up the MO call by itself.

However we need to send this MO call system message to dialer app.
blocking-basecamp: --- → ?
Assignee: nobody → htsai
Will all the dialing calls be notified, or just the ones relative to the STK?
(In reply to Etienne Segonzac from comment #2)
> Will all the dialing calls be notified, or just the ones relative to the STK?
Hi Etienne,

I am studying the call flow of stk. I need to know first whether I can get an accurate timing to notify dialer only about a stk call. 

Do you have any comments or preference about these two ways, from the perspective of dialer app?
I think the easiest thing would be to have an STK specific path (with a dedicated system message).
STK is required for v1
blocking-basecamp: ? → +
Attached patch WIP (obsolete) — Splinter Review
According to Etienne's comment 4, this patch sends a system message 'icc-dialing' when *stk* places a call. 

However, after my second thought, I have a question...

The stk call flow is "stk app confirms the call" --> "chrome process receives the confirmation from content (stk app)" --> "RIL (chrome) process sends system message & makes the call". 

Since the confirmation message is sent from the stk app, can't stk app launch the dialer app when stk sends confirmation to chrome? In this way, we can some message transmission. If the answer here is no, then this patch would be the solution.

Etienne, could you please share some thoughts here, because I am not familiar with gaia implementation? Thanks!
Attachment #666478 - Flags: feedback?(etienne)
OS: Linux → Gonk
Hardware: x86_64 → ARM
Comment on attachment 666478 [details] [diff] [review]
WIP

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

Why are we sending a "icc-dialing" system message? Is there any reason we cant use a "dial" system message that could be used by the dialer to know it has to dial some number? Given that a system message is sent by the system, this is completely safe.
"icc-dialing" (I would prefer "icc-dial") system message can be used if there is something very ICC specific here but I'm not sure I see what.
(In reply to Mounir Lamouri (:mounir) from comment #7)
> Comment on attachment 666478 [details] [diff] [review]
> WIP
> 
> Review of attachment 666478 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Why are we sending a "icc-dialing" system message? Is there any reason we
> cant use a "dial" system message that could be used by the dialer to know it
> has to dial some number? 

This message is NOT used to notify the dialer that it has to dial some number. The message is instead used to notify the dialer that *STK just placed a call*. 

The stk dialing path is different, but after making a call by sending rild a specific request, we will receive the same notification from rild, and all the following events are the same as we call via the dialer app.

So, in the stk case, the dialer needs to be launched when stk places a call, and the dialer adds event listener to *mozTelephony.oncallschange* to obtain the dialing call object. 

> Given that a system message is sent by the system,
> this is completely safe.
> "icc-dialing" (I would prefer "icc-dial") system message can be used if
> there is something very ICC specific here but I'm not sure I see what.
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #8)
> (In reply to Mounir Lamouri (:mounir) from comment #7)
> > Comment on attachment 666478 [details] [diff] [review]
> > WIP
> > 
> > Review of attachment 666478 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > Why are we sending a "icc-dialing" system message? Is there any reason we
> > cant use a "dial" system message that could be used by the dialer to know it
> > has to dial some number? 
> 
> This message is NOT used to notify the dialer that it has to dial some
> number. The message is instead used to notify the dialer that *STK just
> placed a call*. 
> 
> The stk dialing path is different, but after making a call by sending rild a
> specific request, we will receive the same notification from rild, and all
> the following events are the same as we call via the dialer app.
> 
> So, in the stk case, the dialer needs to be launched when stk places a call,
> and the dialer adds event listener to *mozTelephony.oncallschange* to obtain
> the dialing call object. 

More explicitly, the dialer should handle 'icc-dial' system message as it handles 'telephony-incoming' system message.

> 
> > Given that a system message is sent by the system,
> > this is completely safe.
> > "icc-dialing" (I would prefer "icc-dial") system message can be used if
> > there is something very ICC specific here but I'm not sure I see what.
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #6)
> Created attachment 666478 [details] [diff] [review]
> WIP
> 
> According to Etienne's comment 4, this patch sends a system message
> 'icc-dialing' when *stk* places a call. 
> 
> However, after my second thought, I have a question...
> 
> The stk call flow is "stk app confirms the call" --> "chrome process
> receives the confirmation from content (stk app)" --> "RIL (chrome) process
> sends system message & makes the call". 
> 
> Since the confirmation message is sent from the stk app, can't stk app
> launch the dialer app when stk sends confirmation to chrome? In this way, we
> can some message transmission. If the answer here is no, then this patch
> would be the solution.
> 
> Etienne, could you please share some thoughts here, because I am not
> familiar with gaia implementation? Thanks!

The dialer app itself does not listen for |oncallschanged|, the call screen does.
When we get a |telephony-incoming| system message we open the call screen which then handle the current calls and listens to |oncallschanged|.

So I think we need the "icc-dialing" system message here in order to do the same (and the change should be pretty trivial on the Gaia side).

If the call could be dialed by the Dialer app itself the STK app could start a dial web activity but from what I understand it's not the case here.
(In reply to Etienne Segonzac from comment #10)
> (In reply to Hsin-Yi Tsai [:hsinyi] from comment #6)
> > Created attachment 666478 [details] [diff] [review]
> > WIP
> > 
> > According to Etienne's comment 4, this patch sends a system message
> > 'icc-dialing' when *stk* places a call. 
> > 
> > However, after my second thought, I have a question...
> > 
> > The stk call flow is "stk app confirms the call" --> "chrome process
> > receives the confirmation from content (stk app)" --> "RIL (chrome) process
> > sends system message & makes the call". 
> > 
> > Since the confirmation message is sent from the stk app, can't stk app
> > launch the dialer app when stk sends confirmation to chrome? In this way, we
> > can some message transmission. If the answer here is no, then this patch
> > would be the solution.
> > 
> > Etienne, could you please share some thoughts here, because I am not
> > familiar with gaia implementation? Thanks!
> 
> The dialer app itself does not listen for |oncallschanged|, the call screen
> does.
> When we get a |telephony-incoming| system message we open the call screen
> which then handle the current calls and listens to |oncallschanged|.
> 
> So I think we need the "icc-dialing" system message here in order to do the
> same (and the change should be pretty trivial on the Gaia side).
> 
> If the call could be dialed by the Dialer app itself the STK app could start
> a dial web activity but from what I understand it's not the case here.

Etienne, thanks for the details! Then this patch is ready for review.
Attachment #666478 - Flags: feedback?(etienne) → review?(vyang)
Hsin, if we make certain Web Activities to work only behind a permission and we add a 'call' web activity that would be allowed only if the caller has the 'dial' permission. That way, we could prevent this hack, right?
Comment on attachment 666478 [details] [diff] [review]
WIP

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

::: dom/system/gonk/RadioInterfaceLayer.js
@@ +379,5 @@
>          this.saveRequestTarget(msg);
>          this.cancelUSSD(msg.json);
>          break;
>        case "RIL:SendStkResponse":
> +        if ((msg.json.hasConfirmed !== undefined) && msg.json.hasConfirmed) {

AFAIK, the response can carry many possible commands. Don't you have to check it first before emitting an "icc-dialing" system message?
Attachment #666478 - Flags: review?(vyang)
Comment on attachment 666478 [details] [diff] [review]
WIP

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

::: dom/system/gonk/RadioInterfaceLayer.js
@@ +379,5 @@
>          this.saveRequestTarget(msg);
>          this.cancelUSSD(msg.json);
>          break;
>        case "RIL:SendStkResponse":
> +        if ((msg.json.hasConfirmed !== undefined) && msg.json.hasConfirmed) {

Hi, Hsinyi:
Why do you send system message here?

Is there any problem if you notify the message in the response of CALL_SET_UP_REQUESTED_FROM_SIM?
(In reply to Vicamo Yang [:vicamo][:vyang] from comment #13)
> Comment on attachment 666478 [details] [diff] [review]
> WIP
> 
> Review of attachment 666478 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/system/gonk/RadioInterfaceLayer.js
> @@ +379,5 @@
> >          this.saveRequestTarget(msg);
> >          this.cancelUSSD(msg.json);
> >          break;
> >        case "RIL:SendStkResponse":
> > +        if ((msg.json.hasConfirmed !== undefined) && msg.json.hasConfirmed) {
> 
> AFAIK, the response can carry many possible commands. Don't you have to
> check it first before emitting an "icc-dialing" system message?

I followed the SimToolKit.idl that says "
/**
 * User has confirmed or rejected the call during STK_CMD_CALL_SET_UP.
 *
 * @see RIL_REQUEST_STK_HANDLE_CALL_SETUP_REQUESTED_FROM_SIM
 *
 * true: Confirmed by User.
 * false: Rejected by User.
 */
 boolean hasConfirmed;" 

and followed ril_worker.js [1]. From above of them "hasConfirmed" is used only for "call setup". 

[1] http://mxr.mozilla.org/mozilla-central/source/dom/system/gonk/ril_worker.js#2246
(In reply to Mounir Lamouri (:mounir) from comment #12)
> Hsin, if we make certain Web Activities to work only behind a permission and
> we add a 'call' web activity that would be allowed only if the caller has
> the 'dial' permission. That way, we could prevent this hack, right?

Hi Mounir, I am not very sure about the details of web activities. I think web activities can to some extent reach the goal here. But according to Etienne's comment 10, system message could be a more appropriate way for this case.
Attached patch WIP (v2)Splinter Review
Agree with yoshi's comment 14.

If we decide to send a system message of notifying an stk-outgoing call (not all the dialing calls), I think this is that point.
Attachment #666478 - Attachment is obsolete: true
Hi, Hsinyi
is this patch ready for review ?
Attachment #666907 - Flags: review?(vyang)
(In reply to Yoshi Huang[:yoshi][:allstars.chh] from comment #18)
> Hi, Hsinyi
> is this patch ready for review ?

Yes, it's ready.

b.t.w. so far, i still cannot tell how android launches the call screen when stk makes a call. If some person knows, welcome to share :)
Comment on attachment 666907 [details] [diff] [review]
WIP (v2)

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

Please file an bug for STK marionette tests :)
Attachment #666907 - Flags: review?(vyang) → review+
(In reply to Vicamo Yang [:vicamo][:vyang] from comment #20)
> Comment on attachment 666907 [details] [diff] [review]
> WIP (v2)
> 
> Review of attachment 666907 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Please file an bug for STK marionette tests :)

Sure, i will! Thanks!
https://hg.mozilla.org/mozilla-central/rev/c7a62be5fdd0
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Summary: B2G RIL: Use system message to notify a 'dialling' call → B2G RIL: Use system message to notify a 'STK dialling' call
You need to log in before you can comment on or make changes to this bug.