Closed Bug 828798 Opened 12 years ago Closed 12 years ago

[Bluetooth] [HFP] No redundant +CIEV command should be sent to remote device

Categories

(Firefox OS Graveyard :: General, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:tef+, firefox20 wontfix, firefox21 fixed, b2g18+ fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 fixed)

RESOLVED FIXED
1.0.1 IOT1 (10may)
blocking-b2g tef+
Tracking Status
firefox20 --- wontfix
firefox21 --- fixed
b2g18 + fixed
b2g18-v1.0.0 --- wontfix
b2g18-v1.0.1 --- fixed

People

(Reporter: shawnjohnjr, Assigned: gyeh)

References

Details

(Whiteboard: QARegressExclude)

Attachments

(3 files, 6 obsolete files)

CIEV was sent incorrectly during conference call test cases. This test was based on patch from Bug 828160. Test case as the folllowing: - 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: Service Level Connection established - AT: post SLC command sequence complete - AT: RING - AT: Received +CLIP: "0988095164",129 - AT: Incoming call established - HCI: Audio Connection enabled - AT: Received +CCWA: "0939835820",129 - AT: WARNING: received redunant +CIEV(call): +CIEV: 2,1 - AT: Call terminated - FATAL ERROR (MTC): The AG should, but does not, have a call in progress - HCI: Audio Connection disabled - AT: Service Level Connection disabled - MTC: Test case ended Final Verdict : Fail Frame 1045 cause problem. It shall not be in "Call in progress". 226 Slave 47 ATA. Answer mode 16 00:00:00.032000 2013/1/10 上午 11:45:55.702000 229 Master 47 ..OK.. Success 18 00:00:00.062000 2013/1/10 上午 11:45:55.764000 234 Master 47 ..+CIEV: 2,1.. Call Status indicator's status report 26 00:00:00.640000 2013/1/10 上午 11:45:56.404000 235 Master 47 ..+CIEV: 4,0.. Call Setup indicator's status report 26 00:00:00.000000 2013/1/10 上午 11:45:56.404000 262 Master 47 ..+CIEV: 6,4.. Signal indicator's status report 26 00:00:01.201000 2013/1/10 上午 11:45:57.605000 647 Master 47 ..+CCWA: "0939835820",129.. Type: 129 39 00:00:17.160000 2013/1/10 上午 11:46:14.765000 648 Master 47 ..+CIEV: 4,1.. Call Setup indicator's status report 26 00:00:00.000000 2013/1/10 上午 11:46:14.765000 652 Slave 47 AT+CHLD=2. Execute "Call hold and multiparty" Command 22 00:00:00.062000 2013/1/10 上午 11:46:14.827000 654 Master 47 ..OK.. Success 18 00:00:00.047000 2013/1/10 上午 11:46:14.874000 675 Master 47 ..+CIEV: 2,1.. Call Status indicator's status report 26 00:00:00.858000 2013/1/10 上午 11:46:15.732000 676 Master 47 ..+CIEV: 4,0.. Call Setup indicator's status report 26 00:00:00.000000 2013/1/10 上午 11:46:15.732000 677 Master 47 ..+CIEV: 3,1.. Call Held indicator's status report 26 00:00:00.016000 2013/1/10 上午 11:46:15.748000 1,026 Slave 47 AT+CHLD=2. Execute "Call hold and multiparty" Command 22 00:00:15.568000 2013/1/10 上午 11:46:31.316000 1,030 Master 47 ..OK.. Success 18 00:00:00.078000 2013/1/10 上午 11:46:31.394000 1,045 Master 47 ..+CIEV: 3,1.. Call Held indicator's status report 26 00:00:00.640000 2013/1/10 上午 11:46:32.034000 1,392 Slave 47 AT+CHLD=1. Execute "Call hold and multiparty" Command 22 00:00:15.522000 2013/1/10 上午 11:46:47.556000 1,396 Master 47 ..OK.. Success 18 00:00:00.063000 2013/1/10 上午 11:46:47.619000 1,407 Master 47 ..+CIEV: 2,0.. Call Status indicator's status report 26 00:00:00.421000 2013/1/10 上午 11:46:48.040000
OS: Windows 7 → Gonk (Firefox OS)
Hardware: x86 → ARM
Assignee: nobody → gyeh
Summary: [Bluetooth]HFP PTS test case TC_AG_TWC_BV_03_I fail due to incorrect CIEV AT command issued → [Bluetooth] [HFP] Send redundant +CIEV command to bluetooth headset
Patch is updated.
Attachment #702700 - Attachment is obsolete: true
Attachment #702700 - Flags: review?(echou)
Attachment #702701 - Flags: review?
Attachment #702701 - Flags: review? → review?(echou)
Verify on bt-master branch for early testing. Redundant +CIEV command fixed. But TC_AG_TWC_BV_03_I, TC_AG_TWC_BV_05_I were blocked by AT+CHLD=2 timeout. You can track on Bug 828804. They shared the same root cause.
Attachment #702701 - Attachment is obsolete: true
Attachment #702701 - Flags: review?(echou)
Attachment #703802 - Flags: review?(echou)
1,154 Slave 46 ATA. Answer mode 16 00:00:00.031000 2013/1/18 下午 06:32:06.383000 1,156 Master 46 ..OK.. Success 19 00:00:00.000000 2013/1/18 下午 06:32:06.383000 1,158 Master 46 ..+CIEV: 2,1.. Call Status indicator's status report 26 00:00:01.030000 2013/1/18 下午 06:32:07.413000 1,159 Master 46 ..+CIEV: 3,0.. Call Setup indicator's status report 26 00:00:00.000000 2013/1/18 下午 06:32:07.413000 1,786 Master 46 ..+CCWA: "0939835820",129.. Type: 129 39 00:00:27.970000 2013/1/18 下午 06:32:35.383000 1,787 Master 46 ..+CIEV: 3,1.. Call Setup indicator's status report 26 00:00:00.000000 2013/1/18 下午 06:32:35.383000 1,791 Slave 46 AT+CHLD=2. Execute "Call hold and multiparty" Command 22 00:00:00.078000 2013/1/18 下午 06:32:35.461000 1,793 Master 46 ..OK.. Success 19 00:00:00.016000 2013/1/18 下午 06:32:35.477000 1,808 Master 46 ..+CIEV: 3,0.. Call Setup indicator's status report 26 00:00:00.655000 2013/1/18 下午 06:32:36.132000 1,809 Master 46 ..+CIEV: 4,1.. Call Held indicator's status report 26 00:00:00.000000 2013/1/18 下午 06:32:36.132000 1,861 Slave 46 AT+CHLD=2. Execute "Call hold and multiparty" Command 22 00:00:02.247000 2013/1/18 下午 06:32:38.379000 1,865 Master 46 ..OK.. Success 19 00:00:00.031000 2013/1/18 下午 06:32:38.410000 1,876 Master 46 ..+CIEV: 4,1.. Call Held indicator's status report 26 00:00:00.499000 2013/1/18 下午 06:32:38.909000 2,031 Slave 46 AT+CHLD=1. Execute "Call hold and multiparty" Command 22 00:00:06.786000 2013/1/18 下午 06:32:45.695000 2,033 Master 46 ..OK.. Success 19 00:00:00.078000 2013/1/18 下午 06:32:45.773000 2,052 Master 46 ..+CIEV: 4,0.. Call Held indicator's status report 26 00:00:00.749000 2013/1/18 下午 06:32:46.522000 2,581 Master 46 ..+CIEV: 2,0.. Call Status indicator's status report 26 00:00:23.712000 2013/1/18 下午 06:33:10.234000
Attachment #703802 - Flags: review?(echou)
When we receive "AT+CHLD=2" from bluetooth headset, we will ask dialer app to hold the current call and accept the other (held or waiting) call. The expected call status change after executing "AT+CHLD=2" in sequence is: * Current call is on hold -> another call is connected However, the call status changes sometimes in reverse order, which is: * Another call is connected -> current call is on hold In this case, after receiving the first call state change (and before the second change), two calls in mCurrentCallArray are connected and no call is on hold. But, we shouldn't send out "+CIEV: <Callheld Indicator>, 0" in the expected result of PTS test. Thus, status of all calls are checked to make sure whether to send update or not.
Attachment #703802 - Attachment is obsolete: true
Attachment #704453 - Flags: review?(echou)
Depends on: 828804
Attachment #704453 - Attachment is obsolete: true
Attachment #704453 - Flags: review?(echou)
Attachment #704735 - Flags: review?(echou)
Comment on attachment 704735 [details] [diff] [review] Patch 1(v2): No redundant +CIEV command to bluetooth headset Review of attachment 704735 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, r=me with nits addressed. Because this patch is a little complicated, please make sure the logic is right before checking in. Thanks. ::: dom/bluetooth/BluetoothHfpManager.cpp @@ +1147,5 @@ > + UpdateCIND(CINDType::CALLSETUP, CallSetupState::OUTGOING_ALERTING, aSend); > + > + // If there's an ongoing call when the headset is just connected, we have > + // to open a sco socket here. > + if (!aSend) { Question: Using variable |aSend| to decide if we're going to create a SCO connection is fine for now, because EnumerateCallState() is the only place calling HandleCallStateChanged() and assigning value 'false' to |aSend|. I was wondering if we could use the connection status of SCO to decide if we have to create a SCO link. That would be more clear than than using |aSend|. Make sense? @@ +1168,5 @@ > + UpdateCIND(CINDType::CALL, CallState::IN_PROGRESS, aSend); > + UpdateCIND(CINDType::CALLSETUP, CallSetupState::NO_CALLSETUP, aSend); > + break; > + case nsIRadioInterfaceLayer::CALL_STATE_HELD: > + // Check whether to updatd CINDType::CALLHELD or not typo: s/updatd/update @@ +1179,5 @@ > + uint16_t state = mCurrentCallArray[index].mState; > + // If there's another call on hold or other calls exist, no need to > + // update CINDType::CALLHELD > + if ((state == nsIRadioInterfaceLayer::CALL_STATE_HELD) || > + (state != nsIRadioInterfaceLayer::CALL_STATE_DISCONNECTED)) { Just use "if (state != nsIRadioInterfaceLayer::CALL_STATE_DISCONNECTED)" should be o.k.
Attachment #704735 - Flags: review?(echou) → review+
Final patch with nits addressed and modified as review comments. try server: https://tbpl.mozilla.org/?tree=Try&rev=63f2994e9240 https://tbpl.mozilla.org/?tree=Try&rev=a60984a3053e
Attachment #704735 - Attachment is obsolete: true
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Please nominated for uplift approval with a risk assessment.
Summary: [Bluetooth] [HFP] Send redundant +CIEV command to bluetooth headset → [Bluetooth] [HFP] No redundant +CIEV command should be sent to remote device
Attached patch patch 1: for b2g18, r=echou (deleted) — Splinter Review
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 #): No User impact if declined: Gecko Bluetooth would send result code CIEV incorrectly during conference call test cases. Testing completed: mozilla-central Risk to taking this patch (and alternatives if risky): Fairly low. We have tested HFP with m-c build for a while. String or UUID changes made by this patch: No
Attachment #729441 - Flags: approval-mozilla-b2g18?
Comment on attachment 729441 [details] [diff] [review] patch 1: for b2g18, r=echou Approving low risk HFB patches to bring us closer to spec.
Attachment #729441 - Flags: approval-mozilla-b2g18? → approval-mozilla-b2g18+
This bug(and other 9 bugs) block Bluetooth certification. So, these bugs need to be marked as tef+ and landed to v1.0.1 in order to pass Bluetooth certification: bug 827255 bug 827212 bug 827266 bug 828175 bug 823346 bug 827230 bug 828798 bug 835740 bug 846647 bug 828160 So, mark these bugs as tef?. These fixes have some dependency and it would be better to have them landed in a specific order. Gina has a good view on this.
blocking-b2g: --- → tef?
I recommend to land these patches in the following order: 01. bug827204 02. bug827255 03. bug823346 04. bug827230 05. bug828798 06. bug827212, patch 1 06. bug827212, patch 2 07. bug827266 08. bug828175 09. bug846647 10. bug825861 11. bug825851 12. bug835740 I'm going to attach patches for b2g18_v1_0_1 for each bug. Please land them in the above order. There should be no conflict and feel free to let me know if I can be any help.
Attached patch b2g18_v1_0_1 patch (obsolete) (deleted) — Splinter Review
tef- for now until partner confirms this is blocking BT cert.
blocking-b2g: tef? → -
needed for tef BT cert.
blocking-b2g: - → tef?
Seems it is confirmed it blocks BT cer as per bug 868347
blocking-b2g: tef? → tef+
Attached patch b2g18_v1_0_1 patch (deleted) — Splinter Review
v1.0.1 patch updated.
Attachment #738766 - Attachment is obsolete: true
Ryan, I've updated all patches. Please land them in the following order, thanks. 01. bug827204 02. bug827255 (03. bug823346 has been landed on v1.0.1) 04. bug827230 05. bug828798 06. bug827212, patch 1 06. bug827212, patch 2 07. bug827266 08. bug828175 09. bug846647 10. bug825861 11. bug825851 12. bug835740
Target Milestone: B2G C4 (2jan on) → 1.0.1 IOT1 (10may)
Whiteboard: QARegressExclude
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: