828193 - [Bluetooth][Hfp] Reset all local members of BluetoothHfpManager after disconnect

Status: RESOLVED DUPLICATE of bug 827212

(Firefox OS Graveyard :: General, defect)

Description

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Issue can be found during HFP PTS test case after
"AG_ECS"->"TC_AG_ECS_BV_03_I". 
Once TC_AG_ECS_BV_03_I finished, then do TC_AG_TWC_BV_02_I test case. 
In AG_TWC_BV_02, at the first incoming call, phone shall
send CLIP but +CCWA was sent, however.

Comment 1

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Attachment: Handsfree command log

Comment 2

[Gina Yeh [:gyeh] [:ginayeh]]

Let me check it. Thanks.

Comment 3

[Gina Yeh [:gyeh] [:ginayeh]]

Attachment: Patch 1(v1): Reset all local members of BluetoothHfpManager after disconnect

Since the call array is not cleared after disconnected, when we re-connect with the bluetooth headset, the call status and call index are wrong.

In the attached patch, all data members in BluetoothHfpManager are reset after disconnecting.

Comment 4

[Eric Chou [:ericchou] [:echou]]

Comment on attachment 700936 [details] [diff] [review]
Patch 1(v1): Reset all local members of BluetoothHfpManager after disconnect

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

Several places need to be revised. Would like to review again once the new patch is done.

::: dom/bluetooth/BluetoothHfpManager.cpp
@@ +264,3 @@
>  
>      if (!mNumber.IsEmpty()) {
>        nsAutoCString ClipMsg(kHfpCrlf);

Nit: The first letter of local variables should be uncapitalized. I think this will conflict with patch 2 of bug 827212

@@ +317,5 @@
> +  AfterHfpDisconnected();
> +}
> +
> +void
> +BluetoothHfpManager::AfterHfpDisconnected()

ok, since this function is more like a Reset function, I would suggest that we should define a function Reset() and call it in AfterHfpDisconnected(). Then we could call Reset() in the ctor and other places.

@@ +1020,3 @@
>          }
> +        MessageLoop::current()->PostDelayedTask(FROM_HERE,
> +          new SendRingIndicatorTask(aNumber, mCurrentCallArray[aCallIndex].type), sRingInterval);

The first parameter should be 'number' instead of 'aNumber'. Is that correct? And, 80 characters per line, please.

@@ +1164,1 @@
>   * also means we need to notify HS about the change. For more information, 

Super-nit: trailing whitespace.

Comment 5

[Gina Yeh [:gyeh] [:ginayeh]]

Attachment: Patch 1(v2): Reset all local members of BluetoothHfpManager after disconnect

New patch with Nit addressed.

- Sync with patch in Bug 827212. Local variable starting with lower-case character (ringMsg, clipMsg)
- Rename function AfterHfpDisconnected() to Reset()
- In SendRingIndicator Task, the first parameter shall be number rather than aNumber

Comment 7

[Eric Chou [:ericchou] [:echou]]

Comment on attachment 701665 [details] [diff] [review]
Patch 1(v2): Reset all local members of BluetoothHfpManager after disconnect

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

Since this has been resolved, cancelled review.