Closed Bug 993288 Opened 10 years ago Closed 10 years ago

[Bluetooth][Certification][PTS][Bluedroid][1.4] HFP TC_AG_TWC_BV_01_I, TC_AG_ECS_BV_03_I

Categories

(Firefox OS Graveyard :: Bluetooth, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

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: ericcc, Assigned: yrliou)

References

Details

Attachments

(4 files, 3 obsolete files)

Attached file TC_AG_TWC_BV_01_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_TWC_BV_04_I

### Expected
Pass 

### Actual
Test case : TC_AG_TWC_BV_01_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
Attached file TC_AG_TWC_BV_02_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_TWC_BV_02_I

### Expected
Pass 

### Actual
Test case : TC_AG_TWC_BV_02_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 :
Attached file TC_AG_TWC_BV_03_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_TWC_BV_03_I

### Expected
Pass 

### Actual
Test case : TC_AG_TWC_BV_03_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
Summary: [Bluetooth][Certification][PTS][Bluedroid][1.4] HFP TC_AG_TWC_BV_04_I → [Bluetooth][Certification][PTS][Bluedroid][1.4] HFP TC_AG_TWC_BV_01_I, TC_AG_TWC_BV_02_I, TC_AG_TWC_BV_03_I
Blocks: 986297
Assignee: nobody → joliu
Blocks: 993280
Root cause for these three cases:
1) TWC_BV_01_I: bluedroid won't send +CCWA if call state is BTHF_CALL_STATE_WAITING.
2) TWC_BV_02_I: calls_handler.js in gaia will not answer the incoming call while handling CHLD=1.
3) TWC_BV_03_I: When handling AT+CHLD=2, we will send +CIEV indicates the intermidiate status (active:2, held:0) which is not a valid status PTS expected.
Blocks: 997578
Blocks: 997580
Summary: [Bluetooth][Certification][PTS][Bluedroid][1.4] HFP TC_AG_TWC_BV_01_I, TC_AG_TWC_BV_02_I, TC_AG_TWC_BV_03_I → [Bluetooth][Certification][PTS][Bluedroid][1.4] HFP TC_AG_TWC_BV_01_I, TC_AG_ECS_BV_03_I
Change bug title.
TC_AG_TWC_BV_02 and 03 have been seperated to new bugs.
Add TC_AG_ECS_BV_03_I since it has the same cause as TC_AG_TWC_BV_01.
No longer blocks: 993280
Although bluedroid defines BTHF_CALL_STATE_WAITING status, it treats this state as an invalid state when handling phone state change.
http://androidxref.com/4.2_r1/xref/external/bluetooth/bluedroid/btif/src/btif_hf.c#1037
Hence, no +CCWA will be sent for these test cases.
Change BTHF_CALL_STATE_WAITING to BTHF_CALL_STATE_INCOMING for phone_state_change usage only.
Attachment #8408078 - Flags: review?(echou)
Blocks: 993289
Comment on attachment 8408078 [details] [diff] [review]
Patch1(v1) Bug 993288: use BTHF_CALL_STATE_INCOMING for both incoming and waiting while calling bluedroid phone_state_change

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

This patch makes sense to me and it can solve the issue. However I've got a question: why do we insist converting BTHF_CALL_STATE_INCOMING to BTHF_CALL_STATE_WAITING in function ConvertToBthfCallState()? Based on current Gecko implementation, bluedroid and your patch, BTHF_CALL_STATE_WAITING seems not to be used anywhere.
Flags: needinfo?(joliu)
Hi Eric,

BTHF_CALL_STATE_WAITING is needed for clcc_response().
According to HFP spec, we have to reply different status code for incoming and waiting.

After checking again cind_response() in bluedroid, we should also use BTHF_CALL_STATE_INCOMING only since we should reply callsetup=1 for both incoming and waiting.

I have updated the patch, please review it.

Thanks,
Jocelyn
Attachment #8408078 - Attachment is obsolete: true
Attachment #8408078 - Flags: review?(echou)
Attachment #8409517 - Flags: review?(echou)
Flags: needinfo?(joliu)
Comment on attachment 8409517 [details] [diff] [review]
Patch1(v2) Bug 993288: Separate BTHF_CALL_STATE_INCOMING and BTHF_CALL_STATE_WAITING for CLCC response only

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

Hm, r+ because I'm ok with the patch and it would fix the problem. However I don't really like to make the signature more complicated just because of a special case, which is CLCC here. Since ConvertToBthfCallState is only called at two different spots, how about doing 1-on-1 converting in this function and move

  if (FindFirstCall(nsITelephonyProvider::CALL_STATE_CONNECTED)

into BluetoothHfpManager::SendCLCC()?

::: dom/bluetooth/bluedroid/hfp/BluetoothHfpManager.cpp
@@ +1072,5 @@
> +                BTHF_CALL_STATE_WAITING :
> +                BTHF_CALL_STATE_INCOMING;
> +    } else {
> +      state = BTHF_CALL_STATE_INCOMING;
> +    }  

nit: trailing whitespaces
Attachment #8409517 - Flags: review?(echou) → review+
Hi Eric,

Thanks for your comment.
I agreed it makes sense to move the special case into SendCLCC in order to keep the function signature simple.
Please find the attachment for the updated version.

Thanks,
Jocelyn
Attachment #8409517 - Attachment is obsolete: true
Attachment #8409541 - Flags: review?(echou)
Comment on attachment 8409541 [details] [diff] [review]
Patch1(v3) Bug 993288: Separate BTHF_CALL_STATE_INCOMING and BTHF_CALL_STATE_WAITING for CLCC response only

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

r=me with nits fixed.

::: dom/bluetooth/bluedroid/hfp/BluetoothHfpManager.cpp
@@ +962,5 @@
>    if (mPhoneType == PhoneType::CDMA && aIndex == 1 && aCall.IsActive()) {
>      callState = (mCdmaSecondCall.IsActive()) ? BTHF_CALL_STATE_HELD :
>                                                 BTHF_CALL_STATE_ACTIVE;
>    }
> +  

nit: trailing whitespace

@@ +967,5 @@
> +  if (callState == BTHF_CALL_STATE_INCOMING) {
> +    callState = (FindFirstCall(nsITelephonyProvider::CALL_STATE_CONNECTED)) ?
> +                BTHF_CALL_STATE_WAITING :
> +                BTHF_CALL_STATE_INCOMING;
> +  }

nit: personally I would do

  if (callState == BTHF_CALL_STATE_INCOMING &&
      FindFirstCall(nsITelephonyProvider::CALL_STATE_CONNECTED)) {
    callState = BTHF_CALL_STATE_WAITING;
  }
Attachment #8409541 - Flags: review?(echou) → review+
blocking-b2g: --- → 1.4?
blocking-b2g: 1.4? → 1.4+
https://hg.mozilla.org/mozilla-central/rev/dc482bcf17e0
Status: NEW → RESOLVED
Closed: 10 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.