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)
Tracking
(blocking-b2g:tef+, 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)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
akeybl
:
approval-mozilla-b2g18+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review |
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
Reporter | ||
Updated•12 years ago
|
OS: Windows 7 → Gonk (Firefox OS)
Hardware: x86 → ARM
Assignee | ||
Updated•12 years ago
|
Blocks: b2g-bluetooth-hfp
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → gyeh
Assignee | ||
Updated•12 years ago
|
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
Assignee | ||
Comment 1•12 years ago
|
||
Attachment #702700 -
Flags: review?(echou)
Assignee | ||
Comment 2•12 years ago
|
||
Patch is updated.
Attachment #702700 -
Attachment is obsolete: true
Attachment #702700 -
Flags: review?(echou)
Attachment #702701 -
Flags: review?
Assignee | ||
Updated•12 years ago
|
Attachment #702701 -
Flags: review? → review?(echou)
Reporter | ||
Comment 3•12 years ago
|
||
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.
Assignee | ||
Comment 4•12 years ago
|
||
Attachment #702701 -
Attachment is obsolete: true
Attachment #702701 -
Flags: review?(echou)
Attachment #703802 -
Flags: review?(echou)
Reporter | ||
Comment 5•12 years ago
|
||
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
Assignee | ||
Updated•12 years ago
|
Attachment #703802 -
Flags: review?(echou)
Assignee | ||
Comment 6•12 years ago
|
||
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)
Assignee | ||
Comment 7•12 years ago
|
||
Attachment #704453 -
Attachment is obsolete: true
Attachment #704453 -
Flags: review?(echou)
Attachment #704735 -
Flags: review?(echou)
Comment 8•12 years ago
|
||
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+
Assignee | ||
Comment 9•12 years ago
|
||
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
Assignee | ||
Comment 10•12 years ago
|
||
Comment 11•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Updated•12 years ago
|
tracking-b2g18:
--- → ?
Comment 13•12 years ago
|
||
Please nominated for uplift approval with a risk assessment.
status-b2g18:
--- → affected
Updated•12 years ago
|
Summary: [Bluetooth] [HFP] Send redundant +CIEV command to bluetooth headset → [Bluetooth] [HFP] No redundant +CIEV command should be sent to remote device
Comment 14•12 years ago
|
||
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 15•12 years ago
|
||
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+
Comment 16•12 years ago
|
||
status-b2g18-v1.0.0:
--- → wontfix
status-b2g18-v1.0.1:
--- → wontfix
status-firefox20:
--- → wontfix
status-firefox21:
--- → fixed
Target Milestone: --- → B2G C4 (2jan on)
Comment 17•12 years ago
|
||
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?
Assignee | ||
Comment 18•12 years ago
|
||
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.
Assignee | ||
Comment 19•12 years ago
|
||
Comment 20•12 years ago
|
||
tef- for now until partner confirms this is blocking BT cert.
blocking-b2g: tef? → -
Updated•12 years ago
|
Blocks: bt-certi-blocking
Comment 22•12 years ago
|
||
Seems it is confirmed it blocks BT cer as per bug 868347
blocking-b2g: tef? → tef+
Assignee | ||
Comment 23•12 years ago
|
||
v1.0.1 patch updated.
Attachment #738766 -
Attachment is obsolete: true
Assignee | ||
Comment 24•12 years ago
|
||
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
Updated•12 years ago
|
Target Milestone: B2G C4 (2jan on) → 1.0.1 IOT1 (10may)
Comment 25•12 years ago
|
||
You need to log in
before you can comment on or make changes to this bug.
Description
•