If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

[Bluetooth][Hfp] Support feature: Query Operator Selection (AT+COPS?)

RESOLVED FIXED in Firefox 21

Status

()

Core
DOM: Device Interfaces
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: gyeh, Assigned: gyeh)

Tracking

unspecified
mozilla21
ARM
Gonk (Firefox OS)
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:tef+, firefox20 wontfix, firefox21 fixed, b2g18+ fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 fixed)

Details

Attachments

(3 attachments, 5 obsolete attachments)

(Assignee)

Description

5 years ago
To support "Query Operator Selection", we need to handle AT command: "AT+COPS?", and "AT+COPS=3,0" shall be sent by the HF to the AG prior to sending the AT+COPS? command.


Please see 4.8 "Query Operator Selection" of HFP 1.5 for more detail.
(Assignee)

Comment 1

5 years ago
Created attachment 698577 [details] [diff] [review]
Patch 1(v1): Support feature 'Query Operator Selection' (AT+COPS?)

Note that values of AT+COPS command will be examined in a follow-up with the "Mobile Equipment Error" report code "+CME ERROR:".
Assignee: nobody → gyeh
Attachment #698577 - Flags: review?(echou)
(Assignee)

Updated

5 years ago
Depends on: 827255
(Assignee)

Updated

5 years ago
Blocks: 829396
(Assignee)

Comment 2

5 years ago
Created attachment 700823 [details] [diff] [review]
Patch 1(v2): Support feature 'Query Operator Selection' (AT+COPS?)

Move "AT+COPS=3,0" part from Bug827255 to this patch
Attachment #698577 - Attachment is obsolete: true
Attachment #698577 - Flags: review?(echou)
Attachment #700823 - Flags: review?(echou)
Comment on attachment 700823 [details] [diff] [review]
Patch 1(v2): Support feature 'Query Operator Selection' (AT+COPS?)

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

Please send me another review after questions are answered or revisions are made. Thank you.

::: dom/bluetooth/BluetoothHfpManager.cpp
@@ +567,5 @@
>  
> +  nsIDOMMozMobileNetworkInfo* network;
> +  voiceInfo->GetNetwork(&network);
> +  NS_ENSURE_TRUE(network, NS_ERROR_FAILURE);
> +  network->GetShortName(mOperatorName);

Question: According to HFP and GSM spec, the result code +COPS is composed of three arguments: <mode>,<format> and <operator>. Since the <format> argument shall be 0 in HFP, and 0 means "long format alphanumeric <operator>", I was wondering if we should call GetLongName() instead of GetShortName().

@@ +664,5 @@
> +    if (!atCommandValues[0].EqualsLiteral("3") ||
> +        !atCommandValues[1].EqualsLiteral("0")) {
> +      if (mCMEE) {
> +        SendCommand("+CME ERROR: ", BluetoothCmeError::OPERATION_NOT_SUPPORTED);
> +      }

I think we still need to respond with "ERROR" even when mCMEE is false, or it may cause an IOT issue because of no response.

@@ +668,5 @@
> +      }
> +      return;
> +    }
> +  } else if (msg.Find("AT+COPS?") != -1) {
> +    nsAutoCString message("+COPS: 0,0,\"");

Question: Is it correct to set the first argument to 0? I'm curious if the variable |networkSelectionMode| should be what we are looking for.
Attachment #700823 - Flags: review?(echou)
(Assignee)

Comment 4

5 years ago
Created attachment 703730 [details] [diff] [review]
Patch 1(v3): Support feature 'Query Operator Selection' (AT+COPS?)

New patch with the following changes:
- Get networkSelectionMode from nsIMobileConnectionProvider. It's value could be null(unknown)/automatic/manual. The value of null(unknown) is mapped to auto since there's no appropriate value of mode could be set. I also check the implementation in Android, they use a fix value of 0 for mode.
- Reply ERROR when CMEE is not enabled and the values of AT+COPS command is wrong
- Operator name is included in the response of AT+COPS? command although it might be ignored with specific mode
Attachment #700823 - Attachment is obsolete: true
Attachment #703730 - Flags: review?(echou)
Comment on attachment 703730 [details] [diff] [review]
Patch 1(v3): Support feature 'Query Operator Selection' (AT+COPS?)

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

Gina, It seems you attached a wrong patch. :|
Attachment #703730 - Flags: review?(echou) → review-
(Assignee)

Comment 6

5 years ago
Created attachment 704386 [details] [diff] [review]
Patch 1(v3): Support feature 'Query Operator Selection' (AT+COPS?)

Oops! Eric, I updated the attachment last Friday, but I just found something sad a few minutes ago, the attachment wasn't successfully updated somehow...
Attachment #703730 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Attachment #704386 - Flags: review?(echou)
Comment on attachment 704386 [details] [diff] [review]
Patch 1(v3): Support feature 'Query Operator Selection' (AT+COPS?)

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

r+ with nits addressed

::: dom/bluetooth/BluetoothHfpManager.cpp
@@ +725,5 @@
> +    message.AppendInt(mNetworkSelectionMode);
> +    message += ",0,\"";
> +    message += NS_ConvertUTF16toUTF8(mOperatorName);
> +    message += "\"";
> +    SendLine(message.get());

We should call |return| here otherwise "OK" would be sent.

::: dom/bluetooth/BluetoothHfpManager.h
@@ +98,5 @@
>    bool mCCWA;
>    bool mCLIP;
>    bool mCMEE;
>    bool mCMER;
> +  bool mNetworkSelectionMode;

Since mNetworkSelectionMode is used as an integer in BluetoothHfpManager.cpp, we should use |int| to declare it.
Attachment #704386 - Flags: review?(echou) → review+
(Assignee)

Comment 8

5 years ago
Created attachment 704793 [details] [diff] [review]
Final version, Support feature: Query Operator Selection (AT+COPS?), r=echou

try:
https://tbpl.mozilla.org/?tree=Try&rev=3a0cc1514743
https://tbpl.mozilla.org/?tree=Try&rev=aac48a5b4787
Attachment #704386 - Attachment is obsolete: true
(Assignee)

Comment 9

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/0118758d92c2
https://hg.mozilla.org/mozilla-central/rev/0118758d92c2
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
tracking-b2g18: --- → ?
Please nominated for uplift approval with a risk assessment.
status-b2g18: --- → affected
tracking-b2g18: ? → +
Created attachment 729436 [details] [diff] [review]
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(AT+COPS) which is neccesary for certification
Testing completed: mozilla-central
Risk to taking this patch (and alternatives if risky): Fairly low. The stuff done in this patch only includes parsing an AT command, responding with proper response, and keeping the value as a member variable.
String or UUID changes made by this patch: no
Attachment #729436 - Flags: approval-mozilla-b2g18?
Comment on attachment 729436 [details] [diff] [review]
patch 1: for b2g18, r=echou

Approving low risk HFB patches to bring us closer to spec.
Attachment #729436 - Flags: approval-mozilla-b2g18? → approval-mozilla-b2g18+
https://hg.mozilla.org/releases/mozilla-b2g18/rev/db47de9e72db
status-b2g18: affected → fixed
status-b2g18-v1.0.0: --- → wontfix
status-b2g18-v1.0.1: --- → wontfix
status-firefox20: --- → wontfix
status-firefox21: --- → fixed

Comment 15

5 years ago
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.
blocking-b2g: --- → tef?
(Assignee)

Comment 16

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

Comment 17

5 years ago
Created attachment 738765 [details] [diff] [review]
b2g18_v1_0_1 patch
tef- for now until tef release partner confirms this is blocking BT cert.
blocking-b2g: tef? → -
Seems it is confirmed as per bug 868347
blocking-b2g: - → tef+
status-b2g18-v1.0.1: wontfix → affected
(Assignee)

Comment 20

4 years ago
Created attachment 746216 [details] [diff] [review]
b2g18_v1_0_1 patch

v1.0.1 patch updated.
Attachment #738765 - Attachment is obsolete: true
(Assignee)

Comment 21

4 years ago
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
Duplicate of this bug: 869337
https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/1417685a2616
status-b2g18-v1.0.1: affected → fixed
You need to log in before you can comment on or make changes to this bug.