794903 - [b2g-bluetooth] BluetoothSCOManager prototype

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Comment 1

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

Attachment: v1 patch

This patch is a prototype of BluetoothScoManager.

Sco socket is created when we answer an phone call, and then set audio route to bluetooth sco. It will be disconnected when the phone call is hanged up.

Comment 2

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

Attachment: v1 patch

Comment 3

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

Attachment: v1 patch

Sorry for the wrong attachment. Please review this patch. Thanks.

Comment 4

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Comment on attachment 665404 [details] [diff] [review]
v1 patch

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

r+ with comments fixed. Especially changing the static to a RefPtr.

::: dom/bluetooth/BluetoothHfpManager.cpp
@@ +51,5 @@
>  
> +void
> +OpenScoSocket(const nsAString& aDevicePath)
> +{
> +  BluetoothScoManager* sco = BluetoothScoManager::Get();

Test for Main thread.

@@ +65,5 @@
> +
> +void
> +CloseScoSocket()
> +{
> +  BluetoothScoManager* sco = BluetoothScoManager::Get();

Test for Main thread.

::: dom/bluetooth/BluetoothScoManager.cpp
@@ +16,5 @@
> +#include "mozilla/dom/bluetooth/BluetoothTypes.h"
> +
> +USING_BLUETOOTH_NAMESPACE
> +
> +static BluetoothScoManager* sInstance = nullptr;

RefPtr, otherwise you're going to crash whenever you try to create a socket since the runnables expect a nsRefPtr.

@@ +49,5 @@
> +  if (sInstance == nullptr) {
> +    sInstance = new BluetoothScoManager();
> +  }
> +
> +  return sInstance;

Remember to file a follow up to actually destroy this on shutdown.

@@ +56,5 @@
> +// Virtual function of class SocketConsumer
> +void
> +BluetoothScoManager::ReceiveSocketData(mozilla::ipc::UnixSocketRawData* aMessage)
> +{
> +  // SCO socket do nothing here

NS_ASSERTION here. If we actually DO get something from the socket, something has gone weird. Though honestly, I should make some flag to enable R/W perms on a socket. Will file followup myself on that.

::: dom/bluetooth/BluetoothScoManager.h
@@ +23,5 @@
> +  void ReceiveSocketData(mozilla::ipc::UnixSocketRawData* aMessage);
> +
> +  bool Connect(const nsAString& aDeviceObjectPath);
> +  void Disconnect();
> +  void SetConnected(bool aConnected);

We'll leave this here for now just to get this landed for the platform milestone, but we need to detect connection in UnixSocketConsumer, not here. I'll file a followup.

::: dom/bluetooth/linux/BluetoothDBusService.cpp
@@ +2208,5 @@
> +                           bool aAuth,
> +                           bool aEncrypt)
> +    : mConsumer(aConsumer),
> +      mObjectPath(aObjectPath),
> +      mType(aType),

Nit: We can just hardcode mType to be SCO here, can't we?

@@ +2232,5 @@
> +    }
> +    sco->SetConnected(true);
> +
> +    nsCOMPtr<nsIAudioManager> am = do_GetService("@mozilla.org/telephony/audiomanager;1");
> +    am->SetForceForUse(am->USE_COMMUNICATION, am->FORCE_BT_SCO);

Not a bug: So we want connection == communication? Not really a platform issue, but does the gaia team know to relay something in the UI about this?

Comment 6

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

Attachment: Final version

try server: https://tbpl.mozilla.org/?tree=Try&rev=7a105dce51f6

Comment 7

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/097994d8c0bc

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/097994d8c0bc