821636 - [b2g-bluetooth] No subscriber number information in Bluetooth Handsfree

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect, P1)

Description

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

This is a mandatory item in HFP(Handsfree Profile). When HS send AT+CNUM command to AG, we shall answer our phone number within a +CNUM command.

Comment 1

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

Attachment: Patch 1(v1): No subscriber number information in Bluetooth Handsfree

If we can get subscriber number information from RIL, then we will reply AT+CNUM command with our phone number. If not, just send "OK" back.

Comment 2

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

Comment on attachment 692251 [details] [diff] [review]
Patch 1(v1): No subscriber number information in Bluetooth Handsfree

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

Looks good, but there are several places need to be checked again. I'm happy to review again once it has been done.

::: dom/bluetooth/BluetoothHfpManager.cpp
@@ +672,5 @@
>          break;
>      }
>      SendLine("OK");
> +  } else if (!strncmp(msg, "AT+CNUM", 7)) {
> +    if (mMsisdn.Length()) {

Not-even-a-nit: we are used to using IsEmpty() instead of Length()

@@ +674,5 @@
>      SendLine("OK");
> +  } else if (!strncmp(msg, "AT+CNUM", 7)) {
> +    if (mMsisdn.Length()) {
> +      nsAutoCString message("+CNUM: ,");
> +      message += NS_ConvertUTF16toUTF8(mMsisdn).get();

Question: according to the HFP spec, the subscriber number shall be double-quoted. Will mMsisdn be double-quoted? If not, we have to do something.

@@ +676,5 @@
> +    if (mMsisdn.Length()) {
> +      nsAutoCString message("+CNUM: ,");
> +      message += NS_ConvertUTF16toUTF8(mMsisdn).get();
> +      message += ",";
> +      message.AppendInt(TOA_UNKNOWN);

According to the HFP spec, there should be a value <service(4 or 5)> at the end of +CNUM string. Could you check again please?

"<service>, Indicates which service this phone number relates to. Shall be either 4(voice) or 5 (fax).
Example: [+CNUM: ,"5551212",129,,4]

Comment 3

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

Attachment: Patch 1(v2): No subscriber number information in Bluetooth Handsfree

Phone number should be double-quoted and revised in the new patch.
Service field is optional, however, it would be a good idea to set it to 4(voice).

Comment 4

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

Comment on attachment 692811 [details] [diff] [review]
Patch 1(v2): No subscriber number information in Bluetooth Handsfree

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

Looks good!

Comment 5

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

try: 
https://tbpl.mozilla.org/?tree=Try&rev=0e8ff7903fd9
https://tbpl.mozilla.org/?tree=Try&rev=06257b6ca239

Comment 6

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

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

Comment 7

[Andrew Overholt [:overholt]]

(In reply to Gina Yeh from comment #0)
> This is a mandatory item in HFP(Handsfree Profile).

Okay, blocking.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

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

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/c2281eb93c35
https://hg.mozilla.org/releases/mozilla-b2g18/rev/ac9a7eafe0ad