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

RESOLVED DUPLICATE of bug 827212

Status

Firefox OS
General
RESOLVED DUPLICATE of bug 827212
5 years ago
5 years ago

People

(Reporter: shawnjohnjr, Assigned: gyeh)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

5 years ago
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.
(Reporter)

Comment 1

5 years ago
Created attachment 699638 [details]
Handsfree command log
(Assignee)

Comment 2

5 years ago
Let me check it. Thanks.
Assignee: nobody → gyeh
(Assignee)

Updated

5 years ago
Blocks: 829396
(Assignee)

Updated

5 years ago
Depends on: 827212
(Assignee)

Updated

5 years ago
Summary: [Bluetooth]HFP PTS After conference call finished, AT command CCWA will be sent instead of CLIP next incoming call. → [Bluetooth][Hfp] Reset all local members of BluetoothHfpManager after disconnect
(Assignee)

Comment 3

5 years ago
Created attachment 700936 [details] [diff] [review]
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.
Attachment #700936 - Flags: review?(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.
Attachment #700936 - Flags: review?(echou)
(Assignee)

Comment 5

5 years ago
Created attachment 701665 [details] [diff] [review]
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
Attachment #700936 - Attachment is obsolete: true
Attachment #701665 - Flags: review?(echou)
(Assignee)

Updated

5 years ago
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 827212
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.
Attachment #701665 - Flags: review?(echou)
You need to log in before you can comment on or make changes to this bug.