GCF case 31.13.1.1: Explicit Call Transfer failure

RESOLVED FIXED in 2.2 S12 (15may)

Status

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: pgravel, Assigned: gsvelto)

Tracking

unspecified
2.2 S12 (15may)
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:2.2+, tracking-b2g:backlog, b2g-v2.2 fixed, b2g-master fixed)

Details

(Whiteboard: [caf priority: p2][CR 830272])

Attachments

(1 attachment)

TC 31.13.1.1: Explicit Call Transfer invocation, successful case, both calls active, clearing using DISCONNECT

Conformance requirement: 3GPP TS 04.91 subclauses 4.1 and 4.2.

Scenario:
1. UE A has Call A-B in state U10 "Active" with auxiliary state "Call held" and Call A-C in state U10 "Active"
2. UE has to invoke explicit call transfer using MMI commands (by dialing "4") but UE is not initiating any facility message even after dialing "4", failing TC.

This scenario is failing due to a check in telephony_helper.js::Call() function where it checks for "if (openLines >= 2)".

This kind of check should probably be done by lower layers that have the ability to check if the new dial request would actually result in a dial rather than supplementary services request.
Whiteboard: [CR 830272]
Whiteboard: [CR 830272] → [caf priority: p2][CR 830272]
Comms Triage: We need more information before deciding whether blocking on it or not.

After reading the 3GPP specs[1], we're trying to merge 2 calls together ("This ECT [Explicit Call Transfer] request indicates to the network that the mobile subscriber wishes the two calls to be connected together").

As far as I know, merging call was supported by taping one button. I tried to find the MMI code (which supposed to be here[2]), but I couldn't find it. Would you mind helping us here, reporter?

FWIW, to future readers:
* UE stands for "user equipment"
* U-number (like U10) is one of the state of call[3].
* TC is "test case"

[1] http://www.3gpp.org/ftp/Specs/archive/04_series/04.91/0491-701.zip
[2] http://www.3gpp.org/ftp/Specs/archive/02_series/02.30/0230-711.zip
[3] http://www.3gpp.org/ftp/Specs/archive/24_series/24.008/24008-d10.zip #5.1.2
Flags: needinfo?(pgravel)
The test case is defined in 3GPP TS 51.010, section 31.13.1.
The list of in-call MMI codes can be found in 3GPP TS 22.030, section 6.5.5.1

Note that this is not call conferencing with 2 calls. This test case is for having one active call and one holding or waiting call and by using the in-call MMI code "4", the two calls are bridged such that they are now talking to each other but you are completely disconnected.
Flags: needinfo?(pgravel)
Hi! Sean,

Have you fixed this before?

--
Keven
Flags: needinfo?(selee)
Hi Keven, Sorry that I have not fixed this before.
Flags: needinfo?(selee)
Hi Ben:
 Please help check if 4+Send is supported by Gecko.

// 3GPP 22.030
6.5.5.1 Call Deflection, Call Waiting, Call Hold, MultiParty Services, Explicit Call
Transfer and Completion of Calls to Busy Subscriber general principles
...
Entering 4 followed by SEND - Connects the two calls and disconnects the subscriber from both
calls (ECT).
Flags: needinfo?(bhsu)
This might be fixed for free when bug 1160337 lands.
Assignee: nobody → gsvelto
See Also: → 1160337
(In reply to Doug Sherk (:drs) (use needinfo?) from comment #6)
> This might be fixed for free when bug 1160337 lands.

No it shouldn't, Doug, Bug 1160337 and 1159958 are different issue, either of them handle different scenario.
Wondering why this is a blocker as IIRC FxOS hasn't supported the explicit call transfer feature.
After tracing the code, I found out the inCall MMI is not supported by Gecko as the following link shows, but I don't know whether this feature should be supported.

https://dxr.mozilla.org/mozilla-central/source/dom/telephony/gonk/TelephonyService.js?from=telephonyservice.js#634
Flags: needinfo?(bhsu)
Comms triage: New feature.
blocking-b2g: 2.2? → ---
Hsin-Yi and others, since Gecko now supports handling in call MMI commands I would expect Gaia to support it too. Explicit call transfer is part of in-call MMI commands and when we tell our test teams we support in-call MMI they expect the entire feature to be working and not only as much as what Moz RIL supports. Moz RIL is not what we are shipping to our customers so that should not be the basis of choosing what to support in Gaia. Gaia, in my opinion, should support the entire feature set.

In this case the fix in Gaia seems pretty simple so wondering if Moz can consider this request for 2.2.
(In reply to Anshul from comment #11)
> Hsin-Yi and others, since Gecko now supports handling in call MMI commands I
> would expect Gaia to support it too. Explicit call transfer is part of
> in-call MMI commands and when we tell our test teams we support in-call MMI
> they expect the entire feature to be working and not only as much as what
> Moz RIL supports. Moz RIL is not what we are shipping to our customers so
> that should not be the basis of choosing what to support in Gaia. Gaia, in
> my opinion, should support the entire feature set.
> 
> In this case the fix in Gaia seems pretty simple so wondering if Moz can
> consider this request for 2.2.

Sounds good and I think that fixing in gaia is not difficult. Maybe we just need to remove the check.

As comment 0,

"""
This scenario is failing due to a check in telephony_helper.js::Call() function where it checks for "if (openLines >= 2)".

This kind of check should probably be done by lower layers that have the ability to check if the new dial request would actually result in a dial rather than supplementary services request.
"""

In lower layer, TelephonyService (Moz RIL), we already have the exactly same check. So removing the check from gaia is safe and the way just resolve this issue on caf devices.
Indeed, removing openLines check also benefits inCall MMI for other multiparty features, e.g. conference. But per the open gaia bug 1098144, I am not sure if it's claimed that gaia supports the inCall MMI feature. Still need the comms team's confirmation.

No matter how, as Aknow mentioned, if there's no other concern on gaia side, removing the check is also fine on mozril implementation.
Thanks for the info, I'll cook up a patch to remove that check.
Status: NEW → ASSIGNED
Comment on attachment 8602109 [details] [review]
[gaia] gabrielesvelto:bug-1159958-remove-redundant-check > mozilla-b2g:master

As per comment 12 we don't need this check anymore.
Attachment #8602109 - Flags: review?(thills)
It looks like we only need gaia change to make bug 1159958 work (only few lines change provided by :gsvelto). And partner claims it works.

For Moz RIL gecko side, we should file another bug for tracking.
blocking-b2g: --- → 2.2?
(In reply to shawn ku [:sku] from comment #17)
> It looks like we only need gaia change to make bug 1159958 work (only few
> lines change provided by :gsvelto). And partner claims it works.
> 
> For Moz RIL gecko side, we should file another bug for tracking.

Had filed, bug 1161905
Comment on attachment 8602109 [details] [review]
[gaia] gabrielesvelto:bug-1159958-remove-redundant-check > mozilla-b2g:master

Hi Gabriele,

It looks good.  

Just a note that I'm not able to test this out since I don't have the qcril.  I did do some basic testing with the patch on the mozril though.

Thanks,

-tamara
Attachment #8602109 - Flags: review?(thills) → review+
Thanks Tamara, let's land it.
https://github.com/mozilla-b2g/gaia/pull/29918

Autolander could not land the pull request due to not having collaborator rights. This is possibly due to a tree closure. Please check the tree status and request checkin again once the tree is open.
Comment on attachment 8602109 [details] [review]
[gaia] gabrielesvelto:bug-1159958-remove-redundant-check > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
[User impact] if declined:
[Testing completed]:
[Risk to taking this patch] (and alternatives if risky):
[String changes made]:
Flags: needinfo?(bbajaj)
Attachment #8602109 - Flags: approval-gaia-v2.2?
(In reply to Anshul from comment #22)
> Comment on attachment 8602109 [details] [review]
> [gaia] gabrielesvelto:bug-1159958-remove-redundant-check > mozilla-b2g:master
> 
> [Approval Request Comment]
> [Bug caused by] (feature/regressing bug #):
> [User impact] if declined:
> [Testing completed]:
> [Risk to taking this patch] (and alternatives if risky):
> [String changes made]:

Please fill the approval request form and also this needs to land on master first before approving the branch patch.
blocking-b2g: 2.2? → 2.2+
Flags: needinfo?(bbajaj) → needinfo?(gsvelto)
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Pre-existing bug
[User impact] if declined: Certain abbreviated dialing codes do not work as expected (see the bug description in comment 0 for more detail)
[Testing completed]: Tested for regressions only as testing the actual issue using MozRIL requires bug 1161905 to be fixed first. This does seem to work on the commercial RIL though which is why we're uplifting.
[Risk to taking this patch] (and alternatives if risky): Practically none, this removes a check that is also present in the RIL code and thus completely redundant.
[String changes made]: None
Flags: needinfo?(gsvelto)
Attachment #8602109 - Flags: approval-gaia-v2.2? → approval-gaia-v2.2+
Blocks: 977588
You need to log in before you can comment on or make changes to this bug.