[Bluetooth][PTS][2.2][L][FC] TC_AG_ACR_BV_01_I failed

VERIFIED FIXED in Firefox 41

Status

defect
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: twen, Assigned: wiwang)

Tracking

unspecified
2.2 S12 (15may)
ARM
Gonk (Firefox OS)

Firefox Tracking Flags

(blocking-b2g:2.2+, firefox39 wontfix, firefox40 wontfix, firefox41 fixed, b2g-v2.2 fixed, b2g-master fixed)

Details

Attachments

(5 attachments, 1 obsolete attachment)

(Reporter)

Description

4 years ago
##STR
Test case : TC_AG_ACR_BV_01_I started
	- SDP Service record for PTS: 'Headset HS' successfully registered
	- MTC: Creating and closing a service level connection with the IUT in order to create a pairing
	- MTC: TS_spp_disable_tester: SPP already disabled
	- Using SDP to determine IUT RFCOMM server channel number.
	- ProtocolDescriptorList: IUT reports that its RFCOMM server channel number is 2
	- ProfileDescriptorList: Headset Profile version retrieved successfully
	- AT: SPP connect succeeded
	- AT: SPP connection enabled
	- AT: Service Level Connection disabled
	- AT: SPP connect succeeded
	- MTC: SPP Connection established from IUT
	- AT: RING
	- MTC: MTC: Call is connected
	- HCI: Audio Connection enabled
	- MTC: Call disabled by tester
	- MTC: Disable call/audio connection successfully
	- FATAL ERROR (MTC): The IUT and PTS have an Audio Connection, when they should not
	- HCI: Audio Connection disabled
	- AT: Service Level Connection disabled
	- MTC: Test case ended
Final Verdict:FAIL
TC_AG_ACR_BV_01_I finished

## Version
Build Number: 20150428041949

Updated

4 years ago
Blocks: 1159612
(Reporter)

Updated

4 years ago
Summary: [Bluetooth][PTS][2.2] TC_AG_ACR_BV_01_I failed → [Bluetooth][PTS][2.2][L][FC] TC_AG_ACR_BV_01_I failed
(Assignee)

Updated

4 years ago
Assignee: nobody → wiwang
(Reporter)

Comment 1

4 years ago
#Updated Build Version
Gaia-Rev        9f6b1b9082662ba2c14168fc66bb02b4df3141e5
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/e79c19bf19bf
Build-ID        20150428002500
Version         37.0
Device-Name     hammerhead
FW-Release      5.1
FW-Incremental  eng.cltbld.20150428.041949
FW-Date         Tue Apr 28 04:20:07 EDT 2015
Bootloader      HHZ12d
(Assignee)

Comment 2

4 years ago
Attach PASS log for reference
(Assignee)

Comment 3

4 years ago
Root cause found:

Because of [1], NotifyDialer(NS_LITERAL_STRING("CHUP")) in the |BluetoothHfpManager| was not executed when getting AT+CKPD=200, so the "Audio Connection" was not released as Verdict description mentioned[2]


[1] 05-06 19:59:10.314 13370 13370 I GeckoBluetooth: KeyPressedNotification: HSP: mFirstCKPD is true
[2] FATAL ERROR (MTC): The IUT and PTS have an Audio Connection, when they should not

Updated

4 years ago
blocking-b2g: 2.2? → 2.2+
(Assignee)

Comment 4

4 years ago
This PTS failure was due to side effect of bug 1142390, which used a first CKPD flag to solve a PTS HSP TC_AG_OAC_BV_01_I issue.

The side effect will cause a key pressed event (which aim to disconnect audio connection) was ignored, so PTS failed [1].

Per offline discussion with Jamin, so we:
1. remove first CKPD flag => to solve this bug 1161888(TC_AG_ACR_BV_01_I) ,and
2. do not set another |mDialingRequestProcessed| flag to avoid the redundant HFP_AT_RESPONSE_OK [2] => to solve bug 1142390 (TC_AG_OAC_BV_01_I)

After examination, the attached patch can pass following related PTS test cases:
HFP: 
TC_AG_OCL_BV_01_I, TC_AG_OCL_BV_02_I (re-dial related [3])

HSP: 
TC_AG_IAC_BV_01_I
TC_AG_OAC_BV_01_I
TC_AG_ACR_BV_01_I
TC_AG_ACR_BV_02_I
TC_AG_ACT_BV_01_I
TC_AG_ACT_BV_02_I
(all our current HSP test cases)


Hi Shawn, 
May you help to review this patch?
thanks!


[1] FATAL ERROR (MTC): The IUT and PTS have an Audio Connection, when they should not
[2] Bug 1142390 comment 4 ~ comment 7
[3] please refer to HFP spec v1.6, section 4.20 "Last Number Re-Dial from the HF"
Attachment #8604084 - Flags: review?(shuang)
Comment on attachment 8604084 [details] [diff] [review]
0001-Bug-1161888-remove-first-CKPD-flag-and-do-not-set-mD.patch

Jamin, please help feedback on Will's patch. Also we can resolve bug 1161992 after landing this patch, right?
Attachment #8604084 - Flags: feedback?(jaliu)
(Assignee)

Updated

4 years ago
Duplicate of this bug: 1157626
Comment on attachment 8604084 [details] [diff] [review]
0001-Bug-1161888-remove-first-CKPD-flag-and-do-not-set-mD.patch

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

Thank Will for fixing the bug.
Good catch.

::: dom/bluetooth/bluedroid/hfp/BluetoothHfpManager.cpp
@@ +1684,3 @@
>         */
>        NotifyDialer(NS_LITERAL_STRING("CHUP"));
>      }

We might consider to move this statement "mFirstCKPD = false;" to the end of function.
Please refer to https://bugzilla.mozilla.org/show_bug.cgi?id=860166#c23.

This this case, we can handle "4.6.1 Audio Connection Transfer from AG to HS" even if SCO link is established before KeyPressedNotification() does.

However, I don't have a strong opinion on this though.
Since the priority objective is to pass the PTS set, please feel free to land this patch without mFirstCKPD.

::: dom/bluetooth/bluedroid/hfp/BluetoothHfpManager.h
@@ +213,3 @@
>    int mCurrentVgm;
>    bool mReceiveVgsFlag;
>    bool mDialingRequestProcessed;

Since this variable wouldn't be set even when a dialing request is made by HSP.
Please leave a comment or revise the naming to let people know it's only used by HFP.
Thank you.
Attachment #8604084 - Flags: feedback?(jaliu) → feedback+
Hi Will,
Please revise the patch first based on feedback and will take a look soon.
(Assignee)

Comment 9

4 years ago
(In reply to Jamin Liu [:jaliu] from comment #7)
> Comment on attachment 8604084 [details] [diff] [review]
> 0001-Bug-1161888-remove-first-CKPD-flag-and-do-not-set-mD.patch
> 
> Review of attachment 8604084 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Thank Will for fixing the bug.
> Good catch.
> 
> ::: dom/bluetooth/bluedroid/hfp/BluetoothHfpManager.cpp
> @@ +1684,3 @@
> >         */
> >        NotifyDialer(NS_LITERAL_STRING("CHUP"));
> >      }
> 
> We might consider to move this statement "mFirstCKPD = false;" to the end of
> function.
> Please refer to https://bugzilla.mozilla.org/show_bug.cgi?id=860166#c23.
> 
> This this case, we can handle "4.6.1 Audio Connection Transfer from AG to
> HS" even if SCO link is established before KeyPressedNotification() does.
> 
Thank you for pointing out the possibility!
To confirm the timing is identical with expectation,
I made several PTS HSP TC_AG_ACT_BV_01_I tests for "4.6.1 Audio Connection Transfer from AG to HS",
all logs show that FxOS behavior can meet the spec:
|KeyPressedNotification| first(line 834), then SCO connecting(line 838) and connected(line 839). [1]

> However, I don't have a strong opinion on this though.
> Since the priority objective is to pass the PTS set, please feel free to
> land this patch without mFirstCKPD.
> 
> ::: dom/bluetooth/bluedroid/hfp/BluetoothHfpManager.h
> @@ +213,3 @@
> >    int mCurrentVgm;
> >    bool mReceiveVgsFlag;
> >    bool mDialingRequestProcessed;
> 
> Since this variable wouldn't be set even when a dialing request is made by
> HSP.
> Please leave a comment or revise the naming to let people know it's only
> used by HFP.
> Thank you.
OK, I will add comment and made another patch,
thanks for your feedback :)



[1]

 813 05-09 19:21:09.230 I/GeckoBluetooth(29670): ConnectionStateNotification: state 2
 814 05-09 19:21:09.230 I/GeckoBluetooth(29670): ConnectionStateNotification: state 3
 834 05-09 19:21:09.390 I/GeckoBluetooth(29670): KeyPressedNotification: ConnectSco
 838 05-09 19:21:09.480 I/GeckoBluetooth(29670): AudioStateNotification: state 1    
 839 05-09 19:21:09.480 I/GeckoBluetooth(29670): AudioStateNotification: state 2
(Assignee)

Comment 10

4 years ago
Hi Shawn

Could you help to review this patch?
Thanks!
Attachment #8604084 - Attachment is obsolete: true
Attachment #8604084 - Flags: review?(shuang)
Attachment #8605044 - Flags: review?(shuang)
Comment on attachment 8605044 [details] [diff] [review]
0001-Bug-1161888-remove-first-CKPD-flag-and-do-not-set-mD-v2.patch

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

::: dom/bluetooth/bluedroid/hfp/BluetoothHfpManager.h
@@ +212,4 @@
>    int mCurrentVgs;
>    int mCurrentVgm;
>    bool mReceiveVgsFlag;
> +  bool mDialingRequestProcessed; // This flag is for HFP only, not for HSP.

nit:Please move comments above.
Attachment #8605044 - Flags: review?(shuang) → review+
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
(Assignee)

Comment 13

4 years ago
This patch(v3) is basically identical with [1] which is r+, except for a slight difference in comment format.

====

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 1142390
User impact if declined: Bluetooth certification(PTS test) will fail.
Testing completed: Several related PTS tests are tested and PASS.
Risk to taking this patch (and alternatives if risky): Very low.
String or UUID changes made by this patch: None


[1] 8605044: 0001-Bug-1161888-remove-first-CKPD-flag-and-do-not-set-mD-v2.patch
Attachment #8605203 - Flags: approval-mozilla-b2g37?

Updated

4 years ago
Attachment #8605203 - Flags: approval-mozilla-b2g37? → approval-mozilla-b2g37+
Comment hidden (typo)
https://hg.mozilla.org/mozilla-central/rev/a9c99f2a2e3a
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S12 (15may)
See Also: → 1161992
Hi Teri,
Could you help to verify the patch on mc?
Thanks!
Flags: needinfo?(twen)
(Assignee)

Updated

4 years ago
Flags: needinfo?(wiwang)
(Reporter)

Comment 18

4 years ago
Verified on V3.0. All HSP test cases passed. 

HSP: 
TC_AG_IAC_BV_01_I
TC_AG_OAC_BV_01_I
TC_AG_ACR_BV_01_I
TC_AG_ACR_BV_02_I
TC_AG_ACT_BV_01_I
TC_AG_ACT_BV_02_I

Gaia-Rev        afea16de7a76c3b6d15c35fb4c37bac71c8ddc6a
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/35918b0441b4
Build-ID        20150517160201
Version         41.0a1
Device-Name     hammerhead
FW-Release      5.1
FW-Incremental  eng.cltbld.20150517.194053
FW-Date         Sun May 17 19:41:09 EDT 2015
Bootloader      HHZ12f
Flags: needinfo?(twen)
(Reporter)

Comment 20

4 years ago
Verified HSP test cases again on today's latest V2.2.

TC_AG_OAC_BV_01_I   ERROR
TC_AG_ACR_BV_01_I   FAIL


Test case : TC_AG_OAC_BV_01_I started
	- SDP Service record for PTS: 'Headset HS' successfully registered
	- MTC: Creating and closing a service level connection with the IUT in order to create a pairing
	- MTC: TS_spp_disable_tester: SPP already disabled
	- Using SDP to determine IUT RFCOMM server channel number.
	- ProtocolDescriptorList: IUT reports that its RFCOMM server channel number is 2
	- ProfileDescriptorList: Headset Profile version retrieved successfully
	- AT: SPP connect succeeded
	- AT: SPP connection enabled
	- AT: Service Level Connection disabled
	- AT: SPP connect succeeded
	- AT: SPP connection enabled
	- MTC: SPP Connection established from IUT
	- Warning: unknown AT command: ERROR
	- FATAL ERROR (MTC): Audio Connection established from IUT timed out
	- AT: Service Level Connection disabled
	- MTC: Test case ended
	 -Final Verdict: INCONC
TC_AG_OAC_BV_01_I finished


Test case : TC_AG_ACR_BV_01_I started
	- SDP Service record for PTS: 'Headset HS' successfully registered
	- MTC: Creating and closing a service level connection with the IUT in order to create a pairing
	- MTC: TS_spp_disable_tester: SPP already disabled
	- Using SDP to determine IUT RFCOMM server channel number.
	- ProtocolDescriptorList: IUT reports that its RFCOMM server channel number is 2
	- ProfileDescriptorList: Headset Profile version retrieved successfully
	- AT: SPP connect succeeded
	- AT: SPP connection enabled
	- AT: Service Level Connection disabled
	- AT: SPP connect succeeded
	- AT: RING
	- MTC: MTC: Call is connected
	- HCI: Audio Connection enabled
	- MTC: Call disabled by tester
	- MTC: Disable call/audio connection successfully
	- FATAL ERROR (MTC): The IUT and PTS have an Audio Connection, when they should not
	- HCI: Audio Connection disabled
	- AT: Service Level Connection disabled
	- MTC: Test case ended
	 -Final Verdict: FAIL
TC_AG_ACR_BV_01_I finished
(Reporter)

Comment 21

4 years ago
All other cases passed on V2.2.

##Build info
Gaia-Rev        5212c658a651e04d6d84dfc1bce06b499c0d0d96
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/9e9e0d0f45f0
Build-ID        20150518162501
Version         37.0
Device-Name     hammerhead
FW-Release      5.1
FW-Incremental  eng.cltbld.20150518.211334
FW-Date         Mon May 18 21:13:53 EDT 2015
Bootloader      HHZ12f
Will try to verify tomorrow, as the patch might not yet merged into PVT build. ni myself.
Flags: needinfo?(echang)
Test case : TC_AG_ACR_BV_01_I started
	- SDP Service record for PTS: 'Headset HS' successfully registered
	- MTC: Creating and closing a service level connection with the IUT in order to create a pairing
	- MTC: TS_spp_disable_tester: SPP already disabled
	- Using SDP to determine IUT RFCOMM server channel number.
	- ProtocolDescriptorList: IUT reports that its RFCOMM server channel number is 2
	- ProfileDescriptorList: Headset Profile version retrieved successfully
	- AT: SPP connect succeeded
	- AT: SPP connection enabled
	- AT: Service Level Connection disabled
	- AT: SPP connect succeeded
	- AT: RING
	- MTC: MTC: Call is connected
	- HCI: Audio Connection enabled
	- MTC: Call disabled by tester
	- MTC: Disable call/audio connection successfully
	- HCI: Audio Connection disabled
	- AT: Service Level Connection disabled
	- MTC: Test case ended
	 -Final Verdict: PASS
TC_AG_ACR_BV_01_I finished
Status: RESOLVED → VERIFIED
Flags: needinfo?(echang)
Build ID               20150520162503
Gaia Revision          bc42fbc12d622bffd7e8afcb8d56f8a1d9773c60
Gaia Date              2015-05-20 22:32:56
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/8663598512f7
Gecko Version          37.0
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150520.193900
Firmware Date          Wed May 20 19:39:15 EDT 2015
Bootloader             HHZ12f
Test case : TC_AG_OAC_BV_01_I started
	- SDP Service record for PTS: 'Headset HS' successfully registered
	- MTC: Creating and closing a service level connection with the IUT in order to create a pairing
	- MTC: TS_spp_disable_tester: SPP already disabled
	- Using SDP to determine IUT RFCOMM server channel number.
	- ProtocolDescriptorList: IUT reports that its RFCOMM server channel number is 2
	- ProfileDescriptorList: Headset Profile version retrieved successfully
	- AT: SPP connect succeeded
	- AT: SPP connection enabled
	- AT: Service Level Connection disabled
	- AT: SPP connect succeeded
	- AT: SPP connection enabled
	- MTC: SPP Connection established from IUT
	- HCI: Audio Connection enabled
	- MTC: Audio Connection enabled by IUT
	- HCI: Audio Connection disabled
	- MTC: Audio Connection disabled by IUT
	- AT: Service Level Connection disabled
	- MTC: Test case ended
	 -Final Verdict: PASS
TC_AG_OAC_BV_01_I finished
You need to log in before you can comment on or make changes to this bug.