838089 - [Bluetooth] When Plantronics Voyager Legend is connected, we can't recognize AT commands sent from it

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

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

After paired and connected with Plantronics Voyager Legend, it sent lots of vendor-defined AT commands to us. We sent |OK| back and everything looked fine. However, after that, the device wouldn't send any AT commands that we could recognize. According to the HFP spec, we should respond with |ERROR| when we received unknown AT commands.

Comment 1

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

Attachment: patch 1: v1: blacklisted AT commands: AT+XAPL / AT+XEVENT

* With this patch, Answer/Hang Up/Reject could be executed correctly.
* I split the solution into two phases -

(1) Blacklisted these two AT commands, AT+XAPL / AT+XEVENT
(2) Reply with |ERROR| to unknown AT commands

This patch implemented (1), which is for interoperability and avoid unnecessary regression issue. I'll file a follow-up to implement (2), which will be the final solution.

Comment 2

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

Nominate as tef+

Without this patch, some kinds of Plantronics Bluetooth headsets (at least Plantronics Voyager Legend) would be unable to control(e.g. Answer/Hang Up/Reject) phone calls.

Comment 3

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

Comment on attachment 710086 [details] [diff] [review]
patch 1: v1: blacklisted AT commands: AT+XAPL / AT+XEVENT

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

r+ with nits addressed. :)

::: dom/bluetooth/BluetoothHfpManager.cpp
@@ +854,4 @@
>      // Currently, we don't support memory dialing in Dialer app
>      SendLine("ERROR");
>      return;
> +  } else if ((msg.Find("AT+XAPL=") != -1) || 

Please remove the trailing whitespace.

@@ +857,5 @@
> +  } else if ((msg.Find("AT+XAPL=") != -1) || 
> +             (msg.Find("AT+XEVENT=") != -1)) {
> +    // FIXME: Bug 838089
> +    // This is a vendor command used on some special headsets. Eventually we
> +    // should reply with ERROR to all unknown incoming commands. However, 

Please remove the trailing whitespace.

@@ +858,5 @@
> +             (msg.Find("AT+XEVENT=") != -1)) {
> +    // FIXME: Bug 838089
> +    // This is a vendor command used on some special headsets. Eventually we
> +    // should reply with ERROR to all unknown incoming commands. However, 
> +    // in order to not cause interoperability issues for b2g18 branch, only

I think you mean "in order not to". :)

Comment 4

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

Attachment: patch 1: final: blacklisted AT commands: AT+XAPL / AT+XEVENT, r=gyeh

Comment 5

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/1a05832bcaaf

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1a05832bcaaf

Comment 7

[Alex Keybl [:akeybl]]

Setting back to tef? as we're still trying to determine whether or not to take for v1.0.0

Comment 8

[Alex Keybl [:akeybl]]

In the meantime, please nominate for uplift to mozilla-b2g18 (v1.0.1) with a risk evaluation.

Comment 9

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

Comment on attachment 710116 [details] [diff] [review]
patch 1: final: blacklisted AT commands: AT+XAPL / AT+XEVENT, r=gyeh

[Approval Request Comment]
Bug caused by (feature/regressing bug #): None
User impact if declined: Bluetooth headsets which send vendor-defined AT commands AT+XAPL/AT+XEVENT would be unable to control(Answer/Hang Up/Reject) phone calls.
Testing completed: I've got a Plantronics Voyager Legend and have it tested.
Risk to taking this patch (and alternatives if risky): None
String or UUID changes made by this patch: None

Comment 10

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

Attachment: for B2g-18: patch 1: final: blacklisted AT commands: AT+XAPL / AT+XEVENT, r=gyeh

* Re-nominate because the previous patch may cause conflict in b2g18 branch. Once it gets approval, please apply this patch instead.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): None
User impact if declined: Bluetooth headsets which send vendor-defined AT commands AT+XAPL/AT+XEVENT would be unable to control(Answer/Hang Up/Reject) phone calls.
Testing completed: I've got a Plantronics Voyager Legend and have it tested.
Risk to taking this patch (and alternatives if risky): None
String or UUID changes made by this patch: None

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/99a16270175d

Comment 12

[Lukas Blakk [:lsblakk] use ?needinfo]

Batch edit: Bugs fixed on b2g18 after 1/25 merge to v1.0 branch are fixed on v1.0.1 branch.

Comment 13

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

follow-up: Bug 848414