Closed Bug 812391 Opened 12 years ago Closed 12 years ago

[b2g-bluetooth] Support audio stream of bt_sco

Categories

(Core :: DOM: Device Interfaces, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla19
blocking-basecamp +
Tracking Status
firefox18 --- fixed
firefox19 --- fixed

People

(Reporter: gyeh, Assigned: gyeh)

References

Details

Attachments

(1 file, 1 obsolete file)

We have a volume settings for bt_sco, 'audio.volume.bt_sco', which is an integer between 0 to 15.

When there's a connected bluetooth headset, we can adjust this volume value by pressing volume buttons from headset. Plus, we can also press hardware buttons on our phone to change its value during a phone call.
Blocks: 796658
Attachment #682289 - Flags: review?(echou)
Comment on attachment 682289 [details] [diff] [review]
Patch 1(v1): Support audio stream of bt_sco

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

r+ with nits addressed and questions answered.

::: dom/bluetooth/BluetoothHfpManager.cpp
@@ +19,5 @@
>  #include "mozilla/StaticPtr.h"
>  #include "nsContentUtils.h"
>  #include "nsIAudioManager.h"
>  #include "nsIObserverService.h"
> +#include "nsISettingsService.h"

nit: please keep these in alphabetical order

@@ +138,5 @@
>      Shutdown();
>    }
>  };
>  
> +class BluetoothHfpManager::GetVolumeTask : public nsISettingsServiceCallback

Since we don't use any member variable or non-public functions in this class, maybe we can move it out of BluetoothHfpManager.

@@ +398,5 @@
>    MOZ_ASSERT(NS_IsMainThread());
>  
>    // The string that we're interested in will be a JSON string that looks like:
> +  //  {"key":"volumeup", "value":10}
> +  //  {"key":"volumedown", "value":}

Not-even-a-nit: miss the value of "value" here?

@@ +463,1 @@
>    SendCommand("+VGS: ", mCurrentVgs);

How about changing the condition of the previous if-expression to "if (GetConnectionStatus() == SocketConnectionStatus::SOCKET_CONNECTED)" and calling SendCommand() in the block? It needs less code.

::: dom/bluetooth/BluetoothHfpManager.h
@@ +40,4 @@
>  
>  private:
> +  class GetVolumeTask;
> +  friend class GetVolumeTask;

We can remove these two lines if we move class GetVolumeTask out of BluetoothHfpManager.

::: dom/bluetooth/BluetoothUtils.cpp
@@ +9,2 @@
>  #include "BluetoothUtils.h"
> +#include "BluetoothReplyRunnable.h"

nit: please keep these in alphabetical order (I think lines below need to be revised as well)

@@ +119,5 @@
> +void
> +mozilla::dom::bluetooth::DispatchBluetoothReply(
> +                                              BluetoothReplyRunnable* aRunnable,
> +                                                   const BluetoothValue& aValue,
> +                                                     const nsAString& aErrorStr)

The function name seems to be too long. I found that functions in BluetoothUtils.cpp all belong to namespace mozilla::dom::bluetooth, how about using BEGIN_BLUETOOTH_NAMESPACE/END_BLUETOOTH_NAMESPACE to wrap all these functions? Then we can put the first parameter at the same line as the function name.
Attachment #682289 - Flags: review?(echou) → review+
https://hg.mozilla.org/mozilla-central/rev/314db04e1fa7
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
blocking-basecamp: ? → +
Had to back this out due to build failures. Looks like this may need a branch-specific patch.
https://hg.mozilla.org/releases/mozilla-beta/rev/28a256a273f8

https://tbpl.mozilla.org/php/getParsedLog.php?id=17265745&tree=Mozilla-Beta

/builds/slave/m-beta-ics-armv7a-gecko/build/dom/bluetooth/BluetoothUtils.cpp: In function 'nsresult mozilla::dom::bluetooth::SetJsObject(JSContext*, JSObject*, const InfallibleTArray<mozilla::dom::bluetooth::BluetoothNamedValue>&)':
/builds/slave/m-beta-ics-armv7a-gecko/build/dom/bluetooth/BluetoothUtils.cpp:24: error: new declaration 'nsresult mozilla::dom::bluetooth::SetJsObject(JSContext*, JSObject*, const InfallibleTArray<mozilla::dom::bluetooth::BluetoothNamedValue>&)'
/builds/slave/m-beta-ics-armv7a-gecko/build/dom/bluetooth/BluetoothUtils.h:32: error: ambiguates old declaration 'bool mozilla::dom::bluetooth::SetJsObject(JSContext*, JSObject*, const InfallibleTArray<mozilla::dom::bluetooth::BluetoothNamedValue>&)'
/builds/slave/m-beta-ics-armv7a-gecko/build/dom/bluetooth/BluetoothUtils.cpp:29: error: 'aCx' was not declared in this scope
/builds/slave/m-beta-ics-armv7a-gecko/build/dom/bluetooth/BluetoothUtils.cpp:30: error: 'aGlobal' was not declared in this scope
/builds/slave/m-beta-ics-armv7a-gecko/build/dom/bluetooth/BluetoothUtils.cpp:34: error: 'aSourceArray' was not declared in this scope
/builds/slave/m-beta-ics-armv7a-gecko/build/dom/bluetooth/BluetoothUtils.cpp:63: error: 'aResultArray' was not declared in this scope
/builds/slave/m-beta-ics-armv7a-gecko/build/dom/bluetooth/BluetoothUtils.cpp: At global scope:
/builds/slave/m-beta-ics-armv7a-gecko/build/dom/bluetooth/BluetoothUtils.cpp:69: error: 'BluetoothDevice' was not declared in this scope
/builds/slave/m-beta-ics-armv7a-gecko/build/dom/bluetooth/BluetoothUtils.cpp:69: error: template argument 1 is invalid
/builds/slave/m-beta-ics-armv7a-gecko/build/dom/bluetooth/BluetoothUtils.cpp:69: error: template argument 1 is invalid
/builds/slave/m-beta-ics-armv7a-gecko/build/dom/bluetooth/BluetoothUtils.cpp:70: error: explicit qualification in declaration of 'nsresult mozilla::dom::bluetooth::BluetoothDeviceArrayToJSArray(JSContext*, JSObject*, const int&, JSObject**)'
/builds/slave/m-beta-ics-armv7a-gecko/build/dom/bluetooth/BluetoothUtils.cpp: In function 'nsresult mozilla::dom::bluetooth::BluetoothDeviceArrayToJSArray(JSContext*, JSObject*, const int&, JSObject**)':
/builds/slave/m-beta-ics-armv7a-gecko/build/dom/bluetooth/BluetoothUtils.cpp:80: error: request for member 'IsEmpty' in 'aSourceArray', which is of non-class type 'const int'
/builds/slave/m-beta-ics-armv7a-gecko/build/dom/bluetooth/BluetoothUtils.cpp:83: error: request for member 'Length' in 'aSourceArray', which is of non-class type 'const int'
/builds/slave/m-beta-ics-armv7a-gecko/build/dom/bluetooth/BluetoothUtils.cpp:87: error: invalid types 'const int[uint32_t]' for array subscript
/builds/slave/m-beta-ics-armv7a-gecko/build/dom/bluetooth/BluetoothUtils.cpp: At global scope:
/builds/slave/m-beta-ics-armv7a-gecko/build/dom/bluetooth/BluetoothUtils.cpp:111: error: explicit qualification in declaration of 'bool mozilla::dom::bluetooth::SetJsObject(JSContext*, JSObject*, const InfallibleTArray<mozilla::dom::bluetooth::BluetoothNamedValue>&)'
I think it's because Bug798123 is not merged into Mozilla-beta(mozilla 18). Add Bug798123 as blocker, can we mark it as a bb+, too?
Depends on: 798123
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: