827266 - [Bluetooth][Hfp] Support feature: call waiting notification activation (AT+CCWA)

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

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

To support "Call Waiting Notification Activation", we need to be able to handle AT command: AT+CCWA, and send call waiting notification when there's an incoming call during a call.

Please see 4.21 "Call Waiting Notification Activation" of HFP 1.5 for more detail.

Comment 1

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

Attachment: Patch 1(v1): Support feature 'call waiting notification activation'

Comment 2

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

Attachment: Patch 1(v1): Support feature 'call waiting notification activation'

Comment 3

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

Comment on attachment 698614 [details] [diff] [review]
Patch 1(v1): Support feature 'call waiting notification activation'

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

ok, r=me with nits addressed and questions answered

::: dom/bluetooth/BluetoothHfpManager.cpp
@@ +732,5 @@
>        message += "\",";
>        message.AppendInt(TOA_UNKNOWN);
>        message += ",,4";
>        SendLine(message.get());
> +  } else if (msg.Find("AT+CCWA") != -1) {

Should be | msg.Find("AT+CCWA=") |, or ParseAtCommand() below will start parsing from '=', which is wrong.

@@ +927,5 @@
> +        if (mCCWA) {
> +          nsAutoCString resultCode("+CCWA: \"");
> +          resultCode += aNumber;
> +          resultCode += "\",";
> +          resultCode.AppendInt(mCurrentCallArray[aCallIndex].type);

Question: mCurrentCallArray is not found in current m-c, do you mean mCurrentCallStateArray? Or the name would change in another patch?

@@ +949,5 @@
>          }
> +
> +        MessageLoop::current()->PostTask(FROM_HERE,
> +          new SendRingIndicatorTask(number,
> +                                    mCurrentCallArray[aCallIndex].type));

Question: mCurrentCallArray is not found in current m-c, do you mean mCurrentCallStateArray? Or the name would change in another patch?

Comment 4

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

Comment on attachment 698614 [details] [diff] [review]
Patch 1(v1): Support feature 'call waiting notification activation'

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

::: dom/bluetooth/BluetoothHfpManager.cpp
@@ +927,5 @@
> +        if (mCCWA) {
> +          nsAutoCString resultCode("+CCWA: \"");
> +          resultCode += aNumber;
> +          resultCode += "\",";
> +          resultCode.AppendInt(mCurrentCallArray[aCallIndex].type);

mCurrentCallArray is defined in bug827212, patch 1. Please see that for more details. Thanks.

Comment 5

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

Attachment: Patch 1: Support feature: call waiting notification activation (AT+CCWA), r=echou

try:
https://tbpl.mozilla.org/?tree=Try&rev=ab6734df5ecf
https://tbpl.mozilla.org/?tree=Try&rev=7f05ebd3ec60

Comment 6

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

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

Comment 7

[Ed Morley [:emorley]]

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

Comment 8

[Lukas Blakk [:lsblakk] use ?needinfo]

Please nominated for uplift approval with a risk assessment.

Comment 9

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

Attachment: patch 1: for b2g18, r=echou

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): no
User impact if declined: would miss a Bluetooth HFP feature which is neccesary for certification
Testing completed: mozilla-central
Risk to taking this patch (and alternatives if risky): Fairly low. The only thing done in this patch is parsing an AT command and keeping the value as a member variable.
String or UUID changes made by this patch: no

Comment 10

[Alex Keybl [:akeybl]]

Comment on attachment 729424 [details] [diff] [review]
patch 1: for b2g18, r=echou

Approving low risk HFB patches to bring us closer to spec.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/d781960eca6f

Comment 12

[Kai-Chih Hu [:khu]]

This bug(and other 9 bugs) block Bluetooth certification. So, these bugs need to be marked as tef+ and landed to v1.0.1 in order to pass Bluetooth certification: 

bug 827255
bug 827212
bug 827266
bug 828175
bug 823346
bug 827230
bug 828798
bug 835740
bug 846647
bug 828160

So, mark these bugs as tef?. These fixes have some dependency and it would be better to have them landed in a specific order. Gina has a good view on this.

Comment 13

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

I recommend to land these patches in the following order:

01. bug827204
02. bug827255
03. bug823346
04. bug827230
05. bug828798
06. bug827212, patch 1
06. bug827212, patch 2
07. bug827266
08. bug828175
09. bug846647
10. bug825861
11. bug825851
12. bug835740




I'm going to attach patches for b2g18_v1_0_1 for each bug. Please land them in the above order. There should be no conflict and feel free to let me know if I can be any help.

Comment 14

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

Attachment: b2g18_v1_0_1 patch

Comment 15

[Joe Cheng [:jcheng] (please needinfo)]

tef- for now until tef release partner confirms this is blocking BT cert.

Comment 16

[Daniel Coloma:dcoloma]

Seems it is confirmed as per bug 868347

Comment 17

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

Attachment: b2g18_v1_0_1 patch

v1.0.1 patch updated.

Comment 18

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

Attachment: b2g18_v1_0_1 patch

v1.0.1 patch updated.

Comment 19

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

Ryan, I've updated all patches. Please land them in the following order, thanks.

01. bug827204
02. bug827255
(03. bug823346 has been landed on v1.0.1)
04. bug827230
05. bug828798
06. bug827212, patch 1
06. bug827212, patch 2
07. bug827266
08. bug828175
09. bug846647
10. bug825861
11. bug825851
12. bug835740

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/9e28fa737168