[Bluetooth] Unable to accept MT call by PCM CK

RESOLVED FIXED in 1.1 QE2 (6jun)

Status

P1
normal
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: leo.bugzilla.gecko, Assigned: shawnjohnjr)

Tracking

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

Firefox Tracking Flags

(blocking-b2g:leo+, b2g18 fixed, b2g18-v1.0.1 fixed)

Details

(Whiteboard: [status:needs uplift bug 875719][Interoperability issues][TD-24253])

Attachments

(3 attachments)

(Reporter)

Description

6 years ago
Created attachment 746907 [details]
porsche_call_air_log

1. Title : [BT/ BT CAR KIT] - Unable to accept MT call by PCM CK
2. Precondition : Pair and connect to Porsche PCM
3. Tester's Action : Receive MT call. Try to accept it by CK
4. Detailed Symptom (ENG.) : Unable to accept MT call using CK button
5. Reproducibility: Y
6. Frequency Rate : 100%

Dear Mozilla Team,

Incoming call cannot be accepted by Porsche PCM Carkit.
If accept button click, incoming call screen appears again after about one second.
In the air log, carkit send CHLD=1 to b2g phone.
I attached the air log and repoduced movie, please check it.

Thanks.
(Reporter)

Comment 1

6 years ago
Created attachment 746909 [details]
porsche_call_reproduced_movie
Assignee: nobody → shuang
Whiteboard: [Interoperability issues]
(Reporter)

Updated

6 years ago
Hardware: x86 → ARM
Whiteboard: [Interoperability issues] → [Interoperability issues][TD-24253]
Target Milestone: --- → 1.1 QE2
(Reporter)

Updated

6 years ago
Priority: -- → P1
Not blocking until it's clear this will impact a large number of CKs and it's also clear that we're doing something wrong (as opposed to a car issue).
blocking-b2g: --- → -
From my observation, we sent "+CLLC:1,1,5,0,0, "582513", 129. We are somehow makes things incorrect here. We shall send indicator of status is "4" (incoming call) not "5" (waiting call). This looks like really bad if car kit actually checks +CLCC parameter, which decides to switch on incoming call/waiting call mode.
I guess this is why car kit tries to send "CHLD=1", because "5" means one call held!

This also impact v1.0.1. I'm surprised doing certification, this incorrect parameter.
Unless mCurrentCallIndex is not 1.
        case nsIRadioInterfaceLayer::CALL_STATE_INCOMING:
          message.AppendInt((i == mCurrentCallIndex) ? 4 : 5);
blocking-b2g: - → leo?
Ok, it looks like mCurrentCallIndex will be assigned unless the 1st call changes to call connected. So we're fine with 3way call situation, however, in 2 way call, we probably screw up CLCC. Since we don't have PCM car kit, can leo help to give it a quick try to prove this theory is correct?
Flags: needinfo?(leo.bugzilla.gecko)
I cannot enumerate how many car kits get involved but it shall be fixed into v.1.1.
> This also impact v1.0.1. I'm surprised doing certification, this incorrect
> parameter.
I'm surprised when doing certification, this incorrect parameter problem was not been identified.
blocking-b2g: leo? → leo+
(Reporter)

Comment 7

6 years ago
Dear Mozilla Team,

I'll test with PCM Carkit in our side and discuss with you again.

Thanks.
Flags: needinfo?(leo.bugzilla.gecko)
(Reporter)

Comment 8

6 years ago
Dear Mozilla Team,

I have tested it with PCM carkit.
As your comment, 

==> message.AppendInt((i == mCurrentCallIndex) ? 4 : 5);

this code is always assigned 5. So carkit send the CHLD=1 to phone.
So, I fixed this as below.

==> message.AppendInt((i == 1) ? 4 : 5);

And then carkit send the ATA command to phone normally and I can receive an incoming call by answer button of carkit.

Good to here. But in this state, when second incoming call is received, carkit does not have any behavior but the second call is displayed on phone.

I attached the log, please check this.
Thanks.
(Reporter)

Comment 9

6 years ago
Created attachment 752143 [details]
fix_CLCC_log
Thanks again for trying with PCM CK! I got your point, and this is a shameful bug. The second call CLCC with +CLCC:2,1,5,0,0, "",129. How come the 2nd call number is missing!
Do you need any more information here from partners/QA to help your investigation?
> ==> message.AppendInt((i == 1) ? 4 : 5); 

I don't think this would be a good idea. Please refer my attachment in Bug 875719. That patch should fix the issue of wrong status in CLCC. Please let me know if we still fail to pass the test case. Thanks.
(In reply to Shawn Huang from comment #10)
> Thanks again for trying with PCM CK! I got your point, and this is a
> shameful bug. The second call CLCC with +CLCC:2,1,5,0,0, "",129. How come
> the 2nd call number is missing!

It seems like we failed to get call number from RIL?

05-21 08:23:15.429 I/Gecko   (  153): -*- RILContentHelper: Received message 'RIL:CallStateChanged': {"state":11,"callIndex":2,"toa":129,"isMpty":false,"isMT":true,"als":0,"isVoice":true,"isVoicePrivacy":false,"number":null,"numberPresentation":1,"name":null,"namePresentation":1,"uusInfo":null,"isActive":false}

05-21 08:23:15.439 V/BluetoothHfpManager(  153): HandleCallStateChanged:1238 [LGBT_gecko] aCallIndex = 2, aCallState = 11, aSend = 1
Gina is right, I only checked air trace, so I missed something and left stupid comments on Comment 10. As Comment 13 shows exactly what happened, I also checked mozilla ril, it seems to work fine. So please help to check your RIL code first.
I set this bug dependency on Bug 875719 for CLCC call status.
Thanks.
Depends on: 875719
Flags: needinfo?(leo.bugzilla.gecko)
(Reporter)

Comment 15

6 years ago
(In reply to Gina Yeh [:gyeh] [:ginayeh] from comment #12)
> > ==> message.AppendInt((i == 1) ? 4 : 5); 
> 
> I don't think this would be a good idea. Please refer my attachment in Bug
> 875719. That patch should fix the issue of wrong status in CLCC. Please let
> me know if we still fail to pass the test case. Thanks.

I have fixed this code as temporary for right working. bug 875719 is right.
However, this code does not seem enough.
Because it does not consider the incoming situation.

So, patch is needed as below

case nsIRadioInterfaceLayer::CALL_STATE_INCOMING:
     mCurrentCallIndex = aCallIndex; => added

     if (mCurrentCallIndex)     { => //before.
     if (mCurrentCallIndex > 1) { => //after. When mCurrentCallIndex is 2, CCWA is worked.

And in CLCC command should be fixed as below,

    case nsIRadioInterfaceLayer::CALL_STATE_INCOMING:
        message.AppendInt((i == mCurrentCallIndex) ? 4 : 5);  //before
        message.AppendInt((mCurrentCallIndex == 1) ? 4 : 5);  //after

Because i == mCurrentCallIndex is always same value, so if second call is receiving, CLCC command send the 4 value always.

Please review this point.
Thanks.
Flags: needinfo?(leo.bugzilla.gecko)
Is there any specific case for the proposed changes in comment 15?

The value of |mCurrentCallIndex| means the index of current "active" call object in |mCurrentCallArray|, and that's why I didn't assign a new value to |mCurrentCallIndex| when there's an incoming call.

However, I would like to make sure that I didn't miss any case. So, please let me know if the changes in comment 15 is necessary. Thanks.
(Reporter)

Comment 17

6 years ago
I understand your comment about |mCurrentCallIndex|.
But if value of |mCurrentCallIndex| means the index of current "active" call, CALL_STATE_DIALING seems to be a non-active state. 
I think that if outgoing is active, incoming is same, too.
I wonder that you have not assigned a this value when incoming call.

And I have modified that code is for bluetooth CLCC command.
If first call is incoming, parameter value should set the 4 and when a second call set the 5. This is defined in the Bluetooth specification.
If it does not correct, the car kit does not work normally.

Please discuss with your bluetooth team and fix to good solution. :)
Thanks.
For CLCC part, I'm afraid that we couldn't modify our code like this way since it would generate side effect.

Let's say we have two calls now, the first is active, and the second is on hold.
Then, we hang up the first call and answer the second call.
After that, there's incoming call, which is the third call.

In this case, the value mCurrentCallIndex should be 2, and we'll fail to set correct status for CLCC response code.
As for mCurrentCallIndex, please see my new patch in Bug 875719. In order to clarify the meaning of variable, it is renamed as mActiveCallIndex which is re-assigned whenever a SCO is set up.

Thanks for your suggestion. We'll check whether the new solution works or not.
See Also: → bug 875719
This problem should be fixed if the patch of bug 875719 is applied. The patch has just landed in birch and we still need to wait for triage to decide if it's a tef+ or a leo+. Please retry after it's merged into b2g18.

Thank you.
Flags: needinfo?(wchang)
Whiteboard: [Interoperability issues][TD-24253] → [status:needs uplift bug 875719][Interoperability issues][TD-24253]
bug 875719 has just been tef+'ed and should get uplifted to b2g18 in the next day or so as well. please retest per comment 20.
Flags: needinfo?(wchang)
Flags: needinfo?(leo.bugzilla.gecko)
(Reporter)

Comment 22

6 years ago
Dear Mozilla Team,

This bug is fixed on leo device after bug 875719 is applied.
Thanks for your help.
Flags: needinfo?(leo.bugzilla.gecko)
Hi Leo,

Thanks.

Close this per comment 22.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
status-b2g18: --- → fixed
status-b2g18-v1.0.1: --- → fixed
Flags: in-moztrap-
You need to log in before you can comment on or make changes to this bug.