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)
Tracking
()
People
(Reporter: gyeh, Assigned: gyeh)
References
Details
Attachments
(1 file, 1 obsolete file)
20.92 KB,
patch
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•12 years ago
|
||
Attachment #682289 -
Flags: review?(echou)
Comment 2•12 years ago
|
||
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+
Assignee | ||
Comment 3•12 years ago
|
||
try: https://tbpl.mozilla.org/?tree=Try&rev=f5d6a516380f https://tbpl.mozilla.org/?tree=Try&rev=2b4233a9c39a
Attachment #682289 -
Attachment is obsolete: true
Assignee | ||
Comment 4•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/314db04e1fa7
Comment 5•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/314db04e1fa7
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
Updated•12 years ago
|
blocking-basecamp: ? → +
Comment 6•12 years ago
|
||
https://hg.mozilla.org/releases/mozilla-beta/rev/9218c2841ead
status-firefox18:
--- → fixed
status-firefox19:
--- → fixed
Comment 7•12 years ago
|
||
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>&)'
Assignee | ||
Comment 8•12 years ago
|
||
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.
Description
•