925663 - Macro IS_HEADSET() can't be used to decide which profile connection should be established, HFP or HSP.

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Bluetooth, defect)

Description

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

Symptom: Only HSP connection would be established when I tried to connect with my Bluetooth headset (Nokia BH-503).
Build Info: latest gecko:m-c/gaia:master
Impact: Users can't normally use their Bluetooth headset.

In m-c, macro IS_HEADSET() has been used to decide which profile connection should be established, HFP or HSP. However, it can't be used this way.

According to HFP and HSP spec, in section "Link Control (LC) Interoperability Requirements", "Audio" bit in the Service Class field should be set for both specs, and that's a convenient way for AG to distinguish if the device is an Audio device or not while inquiring. The only difference for HFP and HSP is the Minor Device class field. In HFP, it should be set to "Hands-Free"; in HSP, it should be set to "Headset". Nevertheless, in HSP spec 1.2, it mentions that "The Device Class fields should not be used as filters since many devices support multiple profiles." In other words, even if there is a Bluetooth headset and only "Headset" is set to its Device Class field, it doesn't mean that it has no HFP service.

Comment 1

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

This is a regression of bug 906305. Nominate as koi+.

Comment 2

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

Attachment: patch 1: v1: Establish HSP connection if and only if HFP service record couldn't be found

Already tested on PTS. (HFP:3 cases / HSP:2 cases)

Comment 3

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

Comment on attachment 815785 [details] [diff] [review]
patch 1: v1: Establish HSP connection if and only if HFP service record couldn't be found

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

Since the values of major/minor device class are not well-defined in spec, it would be a good idea that we try to connect HFP/HSP without them. So, r+'d.

Please see following suggestions.

::: dom/bluetooth/BluetoothHfpManager.cpp
@@ +994,3 @@
>  
>    if (NS_FAILED(bs->GetServiceChannel(aDeviceAddress, uuid, this))) {
>      aController->OnConnect(NS_LITERAL_STRING(ERR_SERVICE_CHANNEL_NOT_FOUND));

It seems like that we report this error string in two cases.

First, we failed to send an async request to dbus.
Second, the service channel returned by dbus is incorrent.

Can we use different error strings for distinguishing these two cases?

This should be the case 1.

@@ +1523,5 @@
> +    // try to create HSP connection instead.
> +    nsString hspUuid;
> +    BluetoothUuidHelper::GetString(BluetoothServiceClass::HEADSET, hspUuid);
> +
> +    if (aServiceUuid.Equals(hspUuid) ||

This should be case 2.

@@ +1524,5 @@
> +    nsString hspUuid;
> +    BluetoothUuidHelper::GetString(BluetoothServiceClass::HEADSET, hspUuid);
> +
> +    if (aServiceUuid.Equals(hspUuid) ||
> +        NS_FAILED(bs->GetServiceChannel(aDeviceAddress, hspUuid, this))) {

This should be the case 1.

Comment 4

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

Attachment: patch 1: final: Establish HSP connection if and only if HFP service record couldn't be found, r=gyeh

* nits picked.

Comment 5

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

https://hg.mozilla.org/integration/b2g-inbound/rev/90869b0d8c91

Comment 6

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/90869b0d8c91

Comment 7

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

Attachment: final patch : for mozilla-aurora

* patch for mozilla-aurora

Comment 8

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

triage: koi+ for regression

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/22472f19a489