STK Setup call confirmation does not display properly

RESOLVED FIXED

Status

Firefox OS
Gaia::Settings
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: cyang, Assigned: frsela)

Tracking

unspecified
ARM
Gonk (Firefox OS)
Dependency tree / graph
Bug Flags:
in-moztrap -

Firefox Tracking Flags

(blocking-b2g:tef+, b2g18 fixed, b2g18-v1.0.0 fixed)

Details

(Whiteboard: [cr 436922] [cr 436926] [cr 437311] [EU_TPE_TRIAGED] QARegressExclude)

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
In GCF TC 27.22.4.13.1.5, the test is ran as follows:

- Start GCF test which prompts for device power up
- After power up and network registration is successful, GCF test prompts to call 321
- After call to 321 connects, GCF test will send a SET_UP_CALL proactive command to display "Disconnect" confirmation message

At this point, nothing is displayed. I noticed that after I hang up the call to 321, the dialer goes away and then now the "Disconnect confirmation message is seen.

This is sent when GCF test issues SET_UP_CALL proactive command:

RILContentHelper: Received message 'RIL:StkCommand': {"commandNumber":1,"typeOfCommand":16,"commandQualifier":4,"options":{"confirmMessage":"Disconnect","address":"012340123456,1,2"}}

Updated

5 years ago
blocking-b2g: --- → tef?
This is because the diconnect message is recived by the settings app which is in background (since the dialer is of the fg)

Which should be the theoretical way of working?
Assignee: nobody → frsela
blocking-b2g: tef? → tef+

Updated

5 years ago
Flags: needinfo?(cyang)
(Reporter)

Comment 2

5 years ago
(In reply to Fernando R. Sela [:frsela] from comment #1)
> This is because the diconnect message is recived by the settings app which
> is in background (since the dialer is of the fg)
> 
> Which should be the theoretical way of working?

According to 3GPP TS 11.14, section 12.6, the command qualifier can indicate the following:

SET UP CALL:
'00' = set up call, but only if not currently busy on another call;
'01' = set up call, but only if not currently busy on another call, with redial;
'02' = set up call, putting all other calls (if any) on hold;
'03' = set up call, putting all other calls (if any) on hold, with redial;
'04' = set up call, disconnecting all other calls (if any);
'05' = set up call, disconnecting all other calls (if any), with redial;
'06' to 'FF' = reserved values.

Could this information be used to help determine which take precedence?
Flags: needinfo?(cyang)
CC Edger,
please help to check if gecko needs provide other information to gaia.

Comment 4

5 years ago
Although we have exposed commandQualifier in MozStkCommand, but I think it is better to have a new property in MozStkSetUpCall determine which action shall be performed as mention in comment #2.

Hi Fernando and Yoshi, how do you think?
(In reply to Edgar Chen [:edgar][:echen] from comment #4)
> Although we have exposed commandQualifier in MozStkCommand, but I think it
> is better to have a new property in MozStkSetUpCall determine which action
> shall be performed as mention in comment #2.
> 
> Hi Fernando and Yoshi, how do you think?

Go ahead!
Yoshi, I tested this on my handset and after user confirmation the response is sent to the icc object which should sent the call.

I'm not sure, but the address is a number with commas, take care with it ;)

E/GeckoConsole(  389): Content JS LOG at app://settings.gaiamobile.org/js/utils.js:20 in debug: [DEBUG # Settings] sendStkResponse to command: {"commandNumber":1,"typeOfCommand":16,"commandQualifier":4,"options":{"confirmMessage":"Disconnect","address":"012340123456,1,2"}}
E/GeckoConsole(  389): Content JS LOG at app://settings.gaiamobile.org/js/utils.js:20 in debug: [DEBUG # Settings] sendStkResponse -- # response = {"hasConfirmed":true,"resultCode":0}

-> I reassign the bug to platform.
blocking-b2g: tef+ → tef?
Assignee: frsela → allstars.chh

Updated

5 years ago
Depends on: 830696
blocking-b2g: tef? → tef+
So is this supposed to be in Gaia or Gecko now?
Flags: needinfo?
Based on comment 6 I would say it is on Gecko.
Flags: needinfo?
Okay.  I imagine Yoshi will be online in the next few hours and comment or work on a patch :)
(In reply to Fernando R. Sela [:frsela] from comment #6)
> Yoshi, I tested this on my handset and after user confirmation the response
> is sent to the icc object which should sent the call.
> 
> I'm not sure, but the address is a number with commas, take care with it ;)
> 
> E/GeckoConsole(  389): Content JS LOG at
> app://settings.gaiamobile.org/js/utils.js:20 in debug: [DEBUG # Settings]
> sendStkResponse to command:
> {"commandNumber":1,"typeOfCommand":16,"commandQualifier":4,"options":
> {"confirmMessage":"Disconnect","address":"012340123456,1,2"}}
> E/GeckoConsole(  389): Content JS LOG at
> app://settings.gaiamobile.org/js/utils.js:20 in debug: [DEBUG # Settings]
> sendStkResponse -- # response = {"hasConfirmed":true,"resultCode":0}
> 
> -> I reassign the bug to platform.

Hi, frsela

Can you explain with more detail why it's a platform bug ?

The bug is when user dialling to 321, then icc sends a proactive command,
but Gaia doesn't show that message to user.

The address is correct, ',' is the pause character in GSM, it's defined in TS 151.011, clause 10.5.1, extended BCD number.
 
Currently you can use commandQualifier in command to decide what to do, but for better API use, Edgar has filed a bug 830696 to achieve this.

Reassign this bug back to you, as I think this is gaia bug,
or if you still think this is platform bug, can you help to provide more information about you opinion?

Thanks
Assignee: allstars.chh → frsela
blocking-b2g: tef+ → tef?
(In reply to Yoshi Huang[:yoshi][:allstars.chh] from comment #10)
> (In reply to Fernando R. Sela [:frsela] from comment #6)
> > Yoshi, I tested this on my handset and after user confirmation the response
> > is sent to the icc object which should sent the call.
> > 
> > I'm not sure, but the address is a number with commas, take care with it ;)
> > 
> > E/GeckoConsole(  389): Content JS LOG at
> > app://settings.gaiamobile.org/js/utils.js:20 in debug: [DEBUG # Settings]
> > sendStkResponse to command:
> > {"commandNumber":1,"typeOfCommand":16,"commandQualifier":4,"options":
> > {"confirmMessage":"Disconnect","address":"012340123456,1,2"}}
> > E/GeckoConsole(  389): Content JS LOG at
> > app://settings.gaiamobile.org/js/utils.js:20 in debug: [DEBUG # Settings]
> > sendStkResponse -- # response = {"hasConfirmed":true,"resultCode":0}
> > 
> > -> I reassign the bug to platform.
> 
> Hi, frsela
> 
> Can you explain with more detail why it's a platform bug ?
> 
> The bug is when user dialling to 321, then icc sends a proactive command,
> but Gaia doesn't show that message to user.
> 
> The address is correct, ',' is the pause character in GSM, it's defined in
> TS 151.011, clause 10.5.1, extended BCD number.
>  
> Currently you can use commandQualifier in command to decide what to do, but
> for better API use, Edgar has filed a bug 830696 to achieve this.
> 
> Reassign this bug back to you, as I think this is gaia bug,
> or if you still think this is platform bug, can you help to provide more
> information about you opinion?
> 
> Thanks

Because I understood that the call is not dialed.

On the UI I:

show the confirmMessage and after user confirmation I sent the ICC response with the hasConfirmed attribute.

I tested the UI and is showing and working as expected. Could you check if the call is dialed after this response?

      case icc.STK_CMD_SET_UP_CALL:
        debug(' STK:Setup Phone Call. Number: ' + options.address);
        var msg = '';
        if (options.confirmMessage) {
          msg += options.confirmMessage;
        }
        var confirmed = confirm(msg + ' ' + options.address);
        iccLastCommandProcessed = true;
        responseSTKCommand({
          hasConfirmed: confirmed,
          resultCode: icc.STK_RESULT_OK
        });
Flags: needinfo?(allstars.chh)
Hi Frsela,

Sorry I got confused about what you said.

Carol, if I am not correct please also help to provide your comments.

From description.

1. User dials "321" first.
2. Then icc sends proactive command, with SET_UP_CALL.

But now I think the bug is the dialog of SET_UP_CALL confirmation didn't pop up now.

Quoted from Carol
"- After call to 321 connects, GCF test will send a SET_UP_CALL proactive command to display "Disconnect" confirmation message

At this point, nothing is displayed."

I think this bug is not you sent the TERMINAL RESPONSE or not, it's about "Display the confirm message when user is dialling.
Flags: needinfo?(allstars.chh)
(In reply to Carol Yang from comment #2)
> According to 3GPP TS 11.14, section 12.6, the command qualifier can indicate
> the following:
> 
> SET UP CALL:
> '00' = set up call, but only if not currently busy on another call;
> '01' = set up call, but only if not currently busy on another call, with
> redial;
> '02' = set up call, putting all other calls (if any) on hold;
> '03' = set up call, putting all other calls (if any) on hold, with redial;
> '04' = set up call, disconnecting all other calls (if any);
> '05' = set up call, disconnecting all other calls (if any), with redial;
> '06' to 'FF' = reserved values.
> 
> Could this information be used to help determine which take precedence?

Hi, Carol
As we found Android doesn't handle the Command Qualifier from SET_UP_CALL,
does Firefox OS also need to support this?

Thanks
Triage: mvines, could you confirm if this is a blocker or not base on comment 13?
Flags: needinfo?(mvines)
Whiteboard: [cr 436922] [cr 436926] [cr 437311] → [cr 436922] [cr 436926] [cr 437311] [EU_TPE_TRIAGED]
(In reply to Yoshi Huang[:yoshi][:allstars.chh] from comment #12)
> Hi Frsela,
> 
> Sorry I got confused about what you said.
> 
> Carol, if I am not correct please also help to provide your comments.
> 
> From description.
> 
> 1. User dials "321" first.
> 2. Then icc sends proactive command, with SET_UP_CALL.
> 
> But now I think the bug is the dialog of SET_UP_CALL confirmation didn't pop
> up now.
> 
> Quoted from Carol
> "- After call to 321 connects, GCF test will send a SET_UP_CALL proactive
> command to display "Disconnect" confirmation message
> 
> At this point, nothing is displayed."
> 
> I think this bug is not you sent the TERMINAL RESPONSE or not, it's about
> "Display the confirm message when user is dialling.

So this could be cause the dialer is on the frontend. So the question is:

If SET_UP_CALL is received SHALL be managed if FOREGROUND when another call is on going?
(moving ni? to cyang right now as she's in the process of digging through android stuff to validate the assertion made in comment 13)
Flags: needinfo?(mvines) → needinfo?(cyang)
blocking-b2g: tef? → tef+
(Reporter)

Comment 17

5 years ago
As Yoshi mentioned in Comment 13, looks like Android does not handle Command Qualifier.

However, I believe modem will take care of that. By this, I mean the STK proactive command for SET_UP_CALL will not come if Command Qualifier specifies so, i.e. 0x00, and it will come when Command Qualifier specifies so, i.e. 0x04. I need to verify this particular comment still and will update when I find out.

For now, still seems like this current bug needs to be addressed since modem is doing the filtering.
Flags: needinfo?(cyang)
(In reply to Carol Yang from comment #17)
> As Yoshi mentioned in Comment 13, looks like Android does not handle Command
> Qualifier.
> 
> However, I believe modem will take care of that. By this, I mean the STK
> proactive command for SET_UP_CALL will not come if Command Qualifier
> specifies so, i.e. 0x00, and it will come when Command Qualifier specifies
> so, i.e. 0x04. I need to verify this particular comment still and will
> update when I find out.
> 
> For now, still seems like this current bug needs to be addressed since modem
> is doing the filtering.

so... who should be the owner of this bug?
Flags: needinfo?(cyang)

Comment 19

5 years ago
So the issue is not handling of command qualifiers as in bug "https://bugzilla.mozilla.org/show_bug.cgi?id=830696" but the fact that the user confirmation dialog is shown underneath the OnCall UI and so not visible to the user to take any action.
Flags: needinfo?(cyang)
(In reply to Anshul from comment #19)
> So the issue is not handling of command qualifiers as in bug
> "https://bugzilla.mozilla.org/show_bug.cgi?id=830696" but the fact that the
> user confirmation dialog is shown underneath the OnCall UI and so not
> visible to the user to take any action.

Without verify the behavior with UX, you should be able to use reopenApp() trick here to bring the Settings app to the foreground.

https://github.com/alivedise/gaia/blob/9058317544abe7b4759c6535e1471c65ed9ef71f/apps/communications/dialer/js/keypad.js#L281-L286

Let me know if it works.
(Reporter)

Comment 21

5 years ago
Just FYI. I've verified that the modem supports all the command qualifiers mentioned in Comment 2. So if a proactive command for SET_UP_CALL comes, that means an option for the user to confirm/reject the call should always be displayed to the user.

Comment 22

5 years ago
Any update on this issue?
Created attachment 706277 [details]
Move settings to foreground on any STK command

A patch to move settings to foreground on any STK commads as suggested by :timdream on comment 20.

Carol, can you check this works and pass GCF?
Attachment #706277 - Flags: review?(21)
Attachment #706277 - Flags: feedback?(cyang)
Comment on attachment 706277 [details]
Move settings to foreground on any STK command

But remove the additional 'settings' stuff.
Attachment #706277 - Flags: review?(21) → review+
Landed: https://github.com/mozilla-b2g/gaia/commit/bbd6ae98a82ad1e71ae6b7ae0dad94c9850102a0
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Reporter)

Comment 27

5 years ago
(In reply to Fernando R. Sela [:frsela] from comment #23)
> Created attachment 706277 [details]
> Move settings to foreground on any STK command
> 
> A patch to move settings to foreground on any STK commads as suggested by
> :timdream on comment 20.
> 
> Carol, can you check this works and pass GCF?

Hi Fernando,

Looks like this patch will bring up the SET_UP_CALL confirmation dialog. The GCF test hasn't passed yet but we're still looking into whether the remaining issues is gaia or not. Either way, the purpose of this bug has been resolved.
(In reply to Carol Yang from comment #27)
> (In reply to Fernando R. Sela [:frsela] from comment #23)
> > Created attachment 706277 [details]
> > Move settings to foreground on any STK command
> > 
> > A patch to move settings to foreground on any STK commads as suggested by
> > :timdream on comment 20.
> > 
> > Carol, can you check this works and pass GCF?
> 
> Hi Fernando,
> 
> Looks like this patch will bring up the SET_UP_CALL confirmation dialog. The
> GCF test hasn't passed yet but we're still looking into whether the
> remaining issues is gaia or not. Either way, the purpose of this bug has
> been resolved.

\o/ Thanks
(Reporter)

Comment 29

5 years ago
Hi Fernando,

This change looks to only be partially merged in. The changes in utils.js are there but I don't see the call to reopenSettings() being made in https://github.com/mozilla-b2g/gaia/blob/v1.0.0/apps/settings/js/icc.js. Do you know if this will just take some time or if something has gone wrong with the merge?
Flags: needinfo?(frsela)
(In reply to Carol Yang from comment #29)
> Hi Fernando,
> 
> This change looks to only be partially merged in. The changes in utils.js
> are there but I don't see the call to reopenSettings() being made in
> https://github.com/mozilla-b2g/gaia/blob/v1.0.0/apps/settings/js/icc.js. Do
> you know if this will just take some time or if something has gone wrong
> with the merge?

We should check what happened in the v1.0.0 merge, but in master branch is ok (https://github.com/mozilla-b2g/gaia/blob/master/apps/settings/js/icc.js#L208) and as you point out, in v1.0.0 is not completely merged or was removed in a future commit.

Some bisect in the v1.0.0 branch told me that the "evil" commit is this one: https://github.com/mozilla-b2g/gaia/commit/c47396cc278918acc1798b513cab9dc21d0f83f5
Flags: needinfo?(frsela)
(In reply to Fernando R. Sela [:frsela] from comment #30)

> Some bisect in the v1.0.0 branch told me that the "evil" commit is this one:
> https://github.com/mozilla-b2g/gaia/commit/
> c47396cc278918acc1798b513cab9dc21d0f83f5

One thing more, in master branch, the 7799 PR (cbff8d447d357588c2e411c8120a59eb221a75cf) was merged BEFORE 7798 (this one - bbd6ae98a82ad1e71ae6b7ae0dad94c9850102a0) and in the v1.0.0 was merged AFTER so probably the merge tool differed both and removed the previous commited code.
(Reporter)

Comment 32

5 years ago
(In reply to Fernando R. Sela [:frsela] from comment #31)
> (In reply to Fernando R. Sela [:frsela] from comment #30)
> 
> > Some bisect in the v1.0.0 branch told me that the "evil" commit is this one:
> > https://github.com/mozilla-b2g/gaia/commit/
> > c47396cc278918acc1798b513cab9dc21d0f83f5
> 
> One thing more, in master branch, the 7799 PR
> (cbff8d447d357588c2e411c8120a59eb221a75cf) was merged BEFORE 7798 (this one
> - bbd6ae98a82ad1e71ae6b7ae0dad94c9850102a0) and in the v1.0.0 was merged
> AFTER so probably the merge tool differed both and removed the previous
> commited code.

Thanks for looking into this Fernando! What's next? What's the process to get 7798 properly/fully merged into v1.0.0 (and looks like v1-train as well) given the merge error already occurred?
Flags: needinfo?(frsela)
I uplifted the merged-over part of the patch with v1-train@ccb490e and v1.0.0@3f505ae
(Reporter)

Comment 34

5 years ago
(In reply to John Ford [:jhford] from comment #33)
> I uplifted the merged-over part of the patch with v1-train@ccb490e and
> v1.0.0@3f505ae

Thanks John! I see it now.
Flags: needinfo?(frsela)
status-b2g18-v1.0.0: --- → fixed
(In reply to Carol Yang from comment #32)
> (In reply to Fernando R. Sela [:frsela] from comment #31)
> > (In reply to Fernando R. Sela [:frsela] from comment #30)
> > 
> > > Some bisect in the v1.0.0 branch told me that the "evil" commit is this one:
> > > https://github.com/mozilla-b2g/gaia/commit/
> > > c47396cc278918acc1798b513cab9dc21d0f83f5
> > 
> > One thing more, in master branch, the 7799 PR
> > (cbff8d447d357588c2e411c8120a59eb221a75cf) was merged BEFORE 7798 (this one
> > - bbd6ae98a82ad1e71ae6b7ae0dad94c9850102a0) and in the v1.0.0 was merged
> > AFTER so probably the merge tool differed both and removed the previous
> > commited code.
> 
> Thanks for looking into this Fernando! What's next? What's the process to
> get 7798 properly/fully merged into v1.0.0 (and looks like v1-train as well)
> given the merge error already occurred?

We wrote to :jhford explaining the issue. He fixed it (as he answered before me ;)

Thanks John.
(Reporter)

Updated

5 years ago
Attachment #706277 - Flags: feedback?(cyang)

Updated

5 years ago
Whiteboard: [cr 436922] [cr 436926] [cr 437311] [EU_TPE_TRIAGED] → [cr 436922] [cr 436926] [cr 437311] [EU_TPE_TRIAGED] QARegressExclude

Comment 36

5 years ago
No need to create a TC in Moztrap for this defect.
Flags: in-moztrap-
You need to log in before you can comment on or make changes to this bug.