Closed Bug 944574 Opened 6 years ago Closed 6 years ago

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

Categories

(Firefox OS Graveyard :: Bluetooth, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
1.3 Sprint 6 - 12/6

People

(Reporter: ben.tian, Assigned: ben.tian)

References

Details

Attachments

(1 file, 3 obsolete files)

Implement ConnectSco/DisconnectSco into BluetoothServiceBluedroid for speaker/receiver audiopath switch during call.
Assignee: nobody → btian
Attachment #8340198 - Flags: review?(echou)
Depends on: 915533
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.
Attachment #8340198 - Flags: review?(echou) → review-
Attachment #8340198 - Attachment is obsolete: true
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.
Attachment #8340262 - Attachment is obsolete: true
Attachment #8340859 - Flags: review?(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? :)
Attachment #8340859 - Flags: review?(echou) → review+
(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
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/fb14e88896ea
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.3 Sprint 6 - 12/6
You need to log in before you can comment on or make changes to this bug.