820143 - bluetooth Hfp Calling Line Identification (CLI) not supported

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Greg Grisco]

Was wondering if this is intentionally not supported or something that was overlooked.  I took a quick look at BluetoothHfpManager.cpp and didn't see any handling of "CLI" commands.

Comment 1

[Chris Lee [:clee]]

We're also looking into this component of the HFP to see if it needs to be supported for v1.

Comment 2

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

Just synced with product team, we will fix this for v1.

Comment 3

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

Attachment: patch 1: v1: implemented Calling Line Identification(CLI)

* Implemented Calling Ling Identification(CLI) Notification. For more information, please see section 4.23 [Calling Line Identification (CLI) Notification], HFP spec 1.6.
* Avoid compiler warnings (comparison between unsigned int and int)
*

Comment 4

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

Comment on attachment 691769 [details] [diff] [review]
patch 1: v1: implemented Calling Line Identification(CLI)

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

Please help to add an AT Command Parser which separates string by ',' or something else, trimming strings, and then returns an array. r+ after seeing the next patch with the parser.

::: dom/bluetooth/BluetoothHfpManager.cpp
@@ +594,5 @@
> +     */
> +    int length = strlen(msg);
> +    mCLIP = (msg[length - 1] == '0') ? false : true;
> +
> +    SendLine("OK");

How about making an AT Command Parser for all commands?

@@ +812,5 @@
> +                                           new SendRingIndicatorTask(""));
> +        } else {
> +          int type = TOA_UNKNOWN;
> +
> +          if (aNumber && aNumber[0] == '+') {

We probably need to check length of aNumber here.

@@ +911,3 @@
>            }
>  
>            // There is no call 

Please help to remove the trailing whitespace.

@@ +915,3 @@
>              mCurrentCallIndex = 0;
>              CloseScoSocket();
>            } 

Please help to remove the trailing whitespace.

::: dom/bluetooth/BluetoothHfpManager.h
@@ +59,2 @@
>    nsString mDevicePath;
> +  nsCString mIncomingCallPhoneNumber;

Are we going to use this elsewhere?

Comment 5

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

Attachment: patch 1: v2: implemented Calling Line Identification(CLI) and a simple AT command parser function

* nits picked
* implemented a simple AT command parser to get values of an AT command.

Comment 6

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

Comment on attachment 692816 [details] [diff] [review]
patch 1: v2: implemented Calling Line Identification(CLI) and a simple AT command parser function

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

Looks good, r+ with nits addressed. Thanks for helping to build the AT command parser.

::: dom/bluetooth/BluetoothHfpManager.cpp
@@ +540,2 @@
>      if (NS_FAILED(rv)) {
>        NS_WARNING("Failed to extract volume value from bluetooth headset!");

We probably need to response OK here. Please help to add it.

::: dom/bluetooth/BluetoothUtils.cpp
@@ +136,5 @@
>    }
>  }
>  
> +void
> +ParseAtCommand(const nsACString& aAtCommand, int aStart,

The value of aStart won't be/shouldn't be changed in this function. Let's make it as a const.

@@ +142,5 @@
> +{
> +  int length = aAtCommand.Length();
> +  int begin = aStart;
> +
> +  for (int i = aStart; i < length; ++i) {

How about directly putting aAtCommand.Length() here? Then we don't one more variable.

Comment 7

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

Attachment: patch 1: final: implemented Calling Line Identification(CLI) and a simple AT command parser function, r=gyeh

* Modified, except the third item. To avoid .Length() being invoked for many times, I prefer declaring it outside the loop.

Comment 8

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

try:

https://hg.mozilla.org/try/rev/182588e0f3f5
https://hg.mozilla.org/try/rev/0e5b647756b4

Comment 9

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/a88f372dacaf

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a88f372dacaf

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/19c3298b173c
https://hg.mozilla.org/releases/mozilla-b2g18/rev/7acbb30432ef