Closed Bug 993289 Opened 6 years ago Closed 6 years ago

[Bluetooth][Certification][PTS][Bluedroid][1.4] HFP TC_AG_TCA_BV05_I

Categories

(Firefox OS Graveyard :: Bluetooth, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:1.4+, firefox29 wontfix, firefox30 fixed, firefox31 fixed, b2g-v1.4 fixed, b2g-v2.0 fixed)

VERIFIED FIXED
1.4 S6 (25apr)
blocking-b2g 1.4+
Tracking Status
firefox29 --- wontfix
firefox30 --- fixed
firefox31 --- fixed
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed

People

(Reporter: echang, Assigned: yrliou)

References

Details

Attachments

(2 files, 2 obsolete files)

Attached file TC_AG_TCA_BV05_I.zip
### ENV
Nexus 4/Bluedroid
https://pvtbuilds.mozilla.org/pvt/mozilla.org/b2gotoro/nightly/mozilla-aurora-nexus-4/2014/03/2014-03-25-16-02-01/


### STR
1. PTS 5.0
2. TC_AG_TCA_BV05_I

### Expected
Pass 

### Actual
Test case : TC_AG_TCA_BV_05_I started
	- SDP Service record for PTS: 'Handsfree HF' successfully registered
	- The IUT claims support for the following eSCO LMP packet types: EV3, 2-EV3, 
	- AT: SPP connect succeeded
	- AT: WARNING: IUT does not report support for AT+CHLD=3 in its +CHLD response, but TSPC_HFP_2_12c is set to TRUE
	- AT: Service Level Connection established
	- AT: post SLC command sequence complete
	- AT: RING
	- AT: Received +CLIP: "0986143883",129
	- HCI: Audio Connection enabled
	- AT: Incoming call established
	- FATAL ERROR (AT): +CIEV(callheld=1) received when there is no second call process
	- MTC received unexpected EXIT message from AT component
	- HCI: Audio Connection disabled
	- AT: SPP disconnect succeeded
	- MTC: Test case ended
Final Verdict : Inconclusive
Blocks: 986297
Assignee: nobody → joliu
Depends on Bug993288 for +CCWA
Depends on: 993288
TC_AG_TCA_BV05:
   1) One active call is ongoing, second one is incoming.
   2) end the active call
   3) end the second call
This case failed because we set and notify HF callsetup=0 in step2.
Attached patch only set callsetup=0 if no other call in the call array is in callsetup. 
Also move callsetup update to HandleCallStateChange() since it should be executed whenever a call state change.
Attachment #8408868 - Flags: review?(echou)
Attachment #8408868 - Flags: feedback?(btian)
Comment on attachment 8408868 [details] [diff] [review]
Patch(v1) Bug 993289: Fix the logic while updating call setup state

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

The patch may result in build break since |UpdatePhoneCIND| still access the removed local variable |callState| to print log. Also I think the callsetup state logic can be simplified as comment below.

::: dom/bluetooth/bluedroid/hfp/BluetoothHfpManager.cpp
@@ -994,5 @@
>  void
>  BluetoothHfpManager::UpdatePhoneCIND(uint32_t aCallIndex, bool aSend)
>  {
> -  // Update callsetup state
> -  uint16_t callState = mCurrentCallArray[aCallIndex].mState;

The variable callState is used to print log below. Please replace callState below with mCurrentCallArray[aCallIndex].mState.

@@ +1123,5 @@
>      mCurrentCallArray[aCallIndex].mType = BTHF_CALL_ADDRTYPE_INTERNATIONAL;
>    }
>  
> +  // Update callsetup state
> +  switch (aCallState) {

I suggest to simplify the logic as following.

bool IsSetupCall = (callstate == INCOMING ||
                    callstate == ALERTING ||
                    callstate == DIALING);
bool RemainSetupCall = (!FindFirstCall(INCOMING) ||
                        !FindFirstCall(ALERTING) ||
                        !FindFirstCall(DIALING);

if (IsSetupCall) {
  mCallSetupState = callstate;
} else if (!RemainSetupCall) {
  mCallSetupState = DISCONNECTED.
}

@@ +1128,5 @@
> +    // This call is not in call setup
> +    case nsITelephonyProvider::CALL_STATE_CONNECTED:
> +    case nsITelephonyProvider::CALL_STATE_HELD:
> +    case nsITelephonyProvider::CALL_STATE_DISCONNECTED:
> +      if (FindFirstCall(nsITelephonyProvider::CALL_STATE_INCOMING) == 0 &&

nit: !FindFirstCall(nsITelephonyProvider::CALL_STATE_INCOMING) is succinter than FindFirstCall(nsITelephonyProvider::CALL_STATE_INCOMING) == 0.
(In reply to Ben Tian [:btian] from comment #4) 
> @@ +1123,5 @@
> >      mCurrentCallArray[aCallIndex].mType = BTHF_CALL_ADDRTYPE_INTERNATIONAL;
> >    }
> >  
> > +  // Update callsetup state
> > +  switch (aCallState) {
> 
> I suggest to simplify the logic as following.
> 
> bool IsSetupCall = (callstate == INCOMING ||
>                     callstate == ALERTING ||
>                     callstate == DIALING);
 bool RemainSetupCall = (!FindFirstCall(INCOMING) &&
                         !FindFirstCall(ALERTING) &&
                         !FindFirstCall(DIALING);

Correction. Should be && here instead of ||.
(In reply to Ben Tian [:btian] from comment #5)
> bool IsSetupCall = (callstate == INCOMING ||
>                     callstate == ALERTING ||
>                     callstate == DIALING);
  bool RemainSetupCall = (FindFirstCall(INCOMING) ||
                          FindFirstCall(ALERTING) ||
                          FindFirstCall(DIALING);

Oops, this one is the really correct logic.
Hi Ben,

Thanks a lot for your comment, I have updated the patch according to your comment.
Please let me know if you have any feedbacks for patch v2.

Thanks,
Jocelyn
Attachment #8408868 - Attachment is obsolete: true
Attachment #8408868 - Flags: review?(echou)
Attachment #8408868 - Flags: feedback?(btian)
Attachment #8409548 - Flags: feedback?(btian)
Comment on attachment 8409548 [details] [diff] [review]
Patch(v2) Bug 993289: Fix the logic while updating call setup state

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

f=me. Thanks Jocelyn.
Attachment #8409548 - Flags: feedback?(btian) → feedback+
Attachment #8409548 - Flags: review?(echou)
blocking-b2g: --- → 1.4?
blocking-b2g: 1.4? → 1.4+
Comment on attachment 8409548 [details] [diff] [review]
Patch(v2) Bug 993289: Fix the logic while updating call setup state

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

::: dom/bluetooth/bluedroid/hfp/BluetoothHfpManager.cpp
@@ +997,5 @@
>      sBluetoothHfpInterface->at_response(aResponseCode, 0));
>  }
>  
>  void
>  BluetoothHfpManager::UpdatePhoneCIND(uint32_t aCallIndex, bool aSend)

aSend seems not to be used. Can we remove it?
Attachment #8409548 - Flags: review?(echou) → review+
aSend will be removed by bug993286.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/72ad241c645c
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.0 S1 (9may)
You need to log in before you can comment on or make changes to this bug.