944574 - [Bluetooth][bluedroid] Add ConnectSco/DisconnectSco into BluetoothServiceBluedroid

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Bluetooth, defect)

Description

[Ben Tian (inactive)]

Implement ConnectSco/DisconnectSco into BluetoothServiceBluedroid for speaker/receiver audiopath switch during call.

Comment 1

[Ben Tian (inactive)]

Attachment: Patch 1 (v1): ConnectSco/DisconnectSco in BluetoothServiceBluedroid

Comment 2

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

Comment on attachment 8340198 [details] [diff] [review]
Patch 1 (v1): ConnectSco/DisconnectSco in BluetoothServiceBluedroid

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

Hi Ben, please see my comments below.

::: dom/bluetooth/bluedroid/gonk/BluetoothServiceBluedroid.cpp
@@ +1360,3 @@
>  
> +  BluetoothHfpManager* hfp = BluetoothHfpManager::Get();
> +  NS_ENSURE_TRUE_VOID(hfp);

We can't leave this function without handling aRuannble.

@@ +1363,5 @@
> +  if (!hfp->ConnectSco()) {
> +    NS_NAMED_LITERAL_STRING(replyError,
> +      "SCO exists or HFP is not connected");
> +    DispatchBluetoothReply(aRunnable, BluetoothValue(), replyError);
> +  }

DispatchBluetoothReply is also necessary if hfp->ConnectSco() succeeds.

@@ +1374,3 @@
>  
> +  BluetoothHfpManager* hfp = BluetoothHfpManager::Get();
> +  NS_ENSURE_TRUE_VOID(hfp);

We can't leave this function without handling aRuannble.

@@ +1392,3 @@
>  
> +  BluetoothHfpManager* hfp = BluetoothHfpManager::Get();
> +  NS_ENSURE_TRUE_VOID(hfp);

We can't leave this function without handling aRuannble.

Comment 3

[Ben Tian (inactive)]

Attachment: Patch 1 (v2): ConnectSco/DisconnectSco in BluetoothServiceBluedroid

Comment 4

[Ben Tian (inactive)]

Attachment: Patch 1 (v3): ConnectSco/DisconnectSco in BluetoothServiceBluedroid

Changes:
- Implemnt ConnectSco, DisconnectSco, and IsScoConnected functions in BluetoothServiceBluedroid
- Revise ConnectSco, DisconnectSco, and IsScoConnected functions in BluetoothDBusService

Note the runnable reply timing of ConnectSco is different between bluez and bluedroid: bluez replies at BluetoothHfpManager::OnScoConnectSuccess/OnScoConnectError, whereas bluedroid replies at BluetoothServiceBluedroid::ConnectSco. The reason is that bluedroid HFP is unaware of Sco connection error but only state change.

Comment 5

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

Comment on attachment 8340859 [details] [diff] [review]
Patch 1 (v3): ConnectSco/DisconnectSco in BluetoothServiceBluedroid

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

r=me with nits addressed. Thanks.

::: dom/bluetooth/bluedroid/gonk/BluetoothServiceBluedroid.cpp
@@ +1367,5 @@
> +    return;
> +  }
> +
> +  DispatchBluetoothReply(aRunnable,
> +                         BluetoothValue(true), EmptyString());

super-nit: no need to wrap.

@@ +1383,5 @@
> +    return;
> +  }
> +
> +  DispatchBluetoothReply(aRunnable,
> +                         BluetoothValue(true), EmptyString());

ditto.

@@ +1399,5 @@
> +    return;
> +  }
> +
> +  DispatchBluetoothReply(aRunnable,
> +                         hfp->IsScoConnected(), EmptyString());

ditto.

::: dom/bluetooth/bluez/linux/BluetoothDBusService.cpp
@@ +3028,5 @@
>      return;
>    }
>  
> +  DispatchBluetoothReply(aRunnable,
> +                         BluetoothValue(true), EmptyString());

super-nit: no need to wrap.

@@ +3047,2 @@
>    DispatchBluetoothReply(aRunnable,
>                           hfp->IsScoConnected(), EmptyString());

Ditto. Would you mind revising this as well? :)

Comment 6

[Ben Tian (inactive)]

Attachment: [final] Patch 1: ConnectSco/DisconnectSco in BluetoothServiceBluedroid, r=echou

Revise nits.

Comment 7

[Ben Tian (inactive)]

try server: https://tbpl.mozilla.org/?tree=Try&rev=5828cc1c595b

Comment 8

[Ben Tian (inactive)]

(In reply to Ben Tian [:btian] from comment #7)
> try server: https://tbpl.mozilla.org/?tree=Try&rev=5828cc1c595b

The JB emulator build failure resulted from certain ccache permission denial error that is irrelate to bluetooth. Re-try: https://tbpl.mozilla.org/?tree=Try&rev=229d1969f686

Comment 9

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

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

Comment 10

[Ryan VanderMeulen [:RyanVM]]

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