Closed Bug 869306 Opened 8 years ago Closed 8 years ago

[Bluetooth][Certification]HFP PTS TC_AG_ECS_BV_01_I failed (CLCC dir value problem)

Categories

(Firefox OS Graveyard :: Bluetooth, defect)

x86
Windows 7
defect
Not set
normal

Tracking

(blocking-b2g:tef+, firefox21 wontfix, firefox22 wontfix, firefox23 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
firefox21 --- wontfix
firefox22 --- wontfix
firefox23 --- fixed
b2g18 --- fixed
b2g18-v1.0.0 --- wontfix
b2g18-v1.0.1 --- fixed

People

(Reporter: shawnjohnjr, Assigned: shawnjohnjr)

References

Details

(Whiteboard: [fixed-in-birch] QARegressExclude)

Attachments

(6 files, 4 obsolete files)

9.27 KB, patch
allstars.chh
: review+
Details | Diff | Splinter Review
5.46 KB, text/plain
Details
9.44 KB, patch
Details | Diff | Splinter Review
5.47 KB, patch
Details | Diff | Splinter Review
9.45 KB, patch
Details | Diff | Splinter Review
5.49 KB, patch
Details | Diff | Splinter Review
Test case : TC_AG_ECS_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, 
	- Using SDP to determine IUT RFCOMM server channel number.
	- ProtocolDescriptorList: IUT reports that its RFCOMM server channel number is 10
	- The IUT claims support for the following eSCO LMP packet types: EV3, 2-EV3, 
	- AT: SPP connect succeeded
	- HCI: Audio Connection enabled
	- AT: Service Level Connection established
	- AT: post SLC command sequence complete
	- MTC: AT+CLCC
	- AT: +CLCC received: +CLCC: 1,0,1,0,0,"0988095164",129
	- AT: +CLCC received: +CLCC: 2,0,0,0,0,"0978500925",129
	- FATAL ERROR (MTC): In +CLCC, call with idx=1 contains a dir value which does not indicate an incoming call (should be 1)
Good case shall be:
Test case : TC_AG_ECS_BV_01_I started	
	- MTC: AT+CLCC
	- AT: +CLCC received: +CLCC: 1,1,1,0,0,"0958028192",129
	- AT: +CLCC received: +CLCC: 2,1,0,0,0,"0978500925",129
	- MTC: +CLCC responses correctly indicate one active and one held call in the status field.
This test based on gecko m-c and gaia master.
Problem is in +CLCC AT command dir value shall be  dir= 0 (outgoing), 1 (incoming). As test resuls showed, dir value is invalid (outgoing), this could be incorrect.
Summary: [Bluetooth][Certification]HFP PTS TC_AG_ECS_BV_01_I failed → [Bluetooth][Certification]HFP PTS TC_AG_ECS_BV_01_I failed (CLCC dir value problem)
I will test with v.1.0.1 later after meta bugs.
Assignee: nobody → shuang
May I know if this is a must to pass Bluetooth certification? 
If this is a Bluetooth certification blocker, it should be marked as tef+, IMO. marked it as tef? now.
blocking-b2g: --- → tef?
Flags: needinfo?(shuang)
Yes. It's certification bug. And I will update this bug by the end of today.
Flags: needinfo?(shuang)
Cool, thank you, Shawn.
There is a case, we cannot easily cover here. If we did not connect headset yet, call state enters into call held, we can hardly know if it's incoming or outgoing call. Since we can only get call status, and call index value.
I think telephony shall be able to provide interface, so we can easily get call direction.
Query RIL team, see if we can get call direction directly, instead of doing tricky change for direction. Let Hsin-Yi comment the feasibility to provide extra parameter for call direction.
Flags: needinfo?(htsai)
(In reply to Shawn Huang from comment #9)
> Query RIL team, see if we can get call direction directly, instead of doing
> tricky change for direction. Let Hsin-Yi comment the feasibility to provide
> extra parameter for call direction.

Yes, I will provide a patch for RIL change (add .direction attribute) tomorrow!
Flags: needinfo?(htsai)
Thanks. Once RIL change applied, this bug shall be easily fixed.
Target Milestone: --- → 1.0.1 IOT1 (10may)
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #12)
> Created attachment 747248 [details] [diff] [review]
> RIL - add 'direction' attribute in nsITelephonyListener

Shawn, see if this works for you.
Hi Hsin-Yi,
As always great job! It works fine! Can you submit for v.1.0.1?
Flags: needinfo?(htsai)
Attachment #747248 - Flags: review?(allstars.chh)
Comment on attachment 747248 [details] [diff] [review]
RIL - add 'direction' attribute in nsITelephonyListener

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

Seems boolean is enough to me.
Attachment #747248 - Flags: review?(allstars.chh)
Comment #15 addressed. Thanks!
Attachment #747248 - Attachment is obsolete: true
Attachment #747288 - Flags: review?(allstars.chh)
Flags: needinfo?(htsai)
Comment on attachment 747288 [details] [diff] [review]
RIL - add 'isOutgoing' attribute in nsITelephonyListener (v2)

Oops, wrong file.
Attachment #747288 - Attachment is obsolete: true
Attachment #747288 - Flags: review?(allstars.chh)
Attachment #747291 - Flags: review?(allstars.chh) → review+
This patch simply checks call direction from RIL interface and update mDirection. I tested TC_AG_ECS series which all passed.
Attachment #747302 - Flags: review?(gyeh)
Attachment #747302 - Flags: review?(echou)
Comment on attachment 747302 [details] [diff] [review]
Patch 1: v1-Bug 869306 Update call direction whenever call state change

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

r+ with nits addressed.

::: dom/bluetooth/BluetoothHfpManager.cpp
@@ +1209,5 @@
> +    mCurrentCallArray[aCallIndex].mDirection = false;
> +  } else {
> +    mCurrentCallArray[aCallIndex].mDirection = true;
> +  }
> +

How about making it shorter as following?

mCurrentCallArray[aCallIndex].mDirection = !aIsOutgoing;

::: dom/bluetooth/BluetoothTelephonyListener.cpp
@@ +34,4 @@
>                                      bool aIsOutgoing)
>  {
>    BluetoothHfpManager* hfp = BluetoothHfpManager::Get();
> +  hfp->HandleCallStateChanged(aCallIndex, aCallState, aNumber, aIsOutgoing, true);

nit: 80 chars per line, please. Here and below.
Attachment #747302 - Flags: review?(gyeh) → review+
Attachment #747302 - Attachment is obsolete: true
Attachment #747302 - Flags: review?(echou)
Comment on attachment 747312 [details] [diff] [review]
Patch 1: v2-Bug 869306 Update call direction whenever call state change

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

Perfect :D
Attachment #747312 - Flags: review?(gyeh) → review+
Comment on attachment 747312 [details] [diff] [review]
Patch 1: v2-Bug 869306 Update call direction whenever call state change

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

r=me with nit addressed. Let's land it. :)

::: dom/bluetooth/BluetoothHfpManager.cpp
@@ +1205,3 @@
>    uint16_t prevCallState = mCurrentCallArray[aCallIndex].mState;
>    mCurrentCallArray[aCallIndex].mState = aCallState;
>  

super-nit: please remove this empty line
Attachment #747312 - Flags: review?(echou) → review+
This required to change RIL. It needs to go QC process, patch is ready now.
Flags: needinfo?(khu)
Whiteboard: [NO_UPLIFT]
Michael, is this on your radar?
blocking-b2g: tef? → tef+
Flags: needinfo?(mvines)
Yes, I'm sure Anshul is on it already.  Thank you for the earlier heads up this time :)
Flags: needinfo?(mvines)
Whiteboard: [NO_UPLIFT] → [NO_UPLIFT][status: r+'d patch ready, needs CAF change]
Whiteboard: [NO_UPLIFT][status: r+'d patch ready, needs CAF change] → [NO_UPLIFT][status: needs new RIL, then needs landing]
Removing NO_UPLIFT.
Whiteboard: [NO_UPLIFT][status: needs new RIL, then needs landing] → [status: justlanditplease]
(In reply to Michael Vines [:m1] [:evilmachines] from comment #28)
> Yes, I'm sure Anshul is on it already.  Thank you for the earlier heads up
> this time :)

Thank you, Michael.
Flags: needinfo?(khu)
Attachment #747797 - Attachment description: Patch1:V2-for b2g18v101 → Patch1:V2-for V1.0.1
birch:

https://hg.mozilla.org/projects/birch/rev/03e1ea0f9735
https://hg.mozilla.org/projects/birch/rev/bd2a6001614f
Whiteboard: [status: justlanditplease] → [status: justlanditplease][fixed-in-birch]
* Gecko/Bluetooth patch for b2g18
https://hg.mozilla.org/mozilla-central/rev/03e1ea0f9735
https://hg.mozilla.org/mozilla-central/rev/bd2a6001614f
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Whiteboard: [status: justlanditplease][fixed-in-birch] → [status: needs uplift][fixed-in-birch]
Unable to verify. Lack of resources. Need Bluetooth PTS tool to verify.
Marking as QARegressExclude.
Whiteboard: [fixed-in-birch] → [fixed-in-birch] QARegressExclude
No longer blocks: 869468
You need to log in before you can comment on or make changes to this bug.