[b2g-bluetooth] BluetoothHfpManager prototype

RESOLVED FIXED in mozilla18

Status

()

Core
DOM: Device Interfaces
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: ericchou, Assigned: ericchou)

Tracking

unspecified
mozilla18
ARM
Gonk (Firefox OS)
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(blocking-basecamp:+)

Details

(Whiteboard: [LOE:S])

Attachments

(1 attachment, 8 obsolete attachments)

(Assignee)

Description

6 years ago
This is the beginning of a series of bugs for Bluetooth Handsfree prorile implementation. The meta bug is Bug 788300.
(Assignee)

Updated

6 years ago
Assignee: nobody → echou
Blocks: 788300
(Assignee)

Comment 1

6 years ago
Created attachment 661170 [details] [diff] [review]
patch 1: WIP: Added BluetoothHfpManager.cpp & .h
(Assignee)

Comment 2

6 years ago
Created attachment 661172 [details] [diff] [review]
patch 1: WIP: Added BluetoothHfpManager.cpp & .h

In current codebase, we haven't had class SocketConsumer & SocketRawData under namespace mozilla::ipc yet, waiting for Kyle's implementation.
Attachment #661170 - Attachment is obsolete: true
(Assignee)

Comment 3

6 years ago
Created attachment 661173 [details] [diff] [review]
patch 2: WIP: Added Connect()
(Assignee)

Comment 4

6 years ago
Created attachment 661174 [details] [diff] [review]
patch 3: WIP: Added Disconnect()
(Assignee)

Updated

6 years ago
blocking-basecamp: --- → ?
blocking-basecamp: ? → +
(Assignee)

Updated

6 years ago
Whiteboard: [LOE:S]
(Assignee)

Comment 5

6 years ago
Created attachment 662110 [details] [diff] [review]
v1: Essential functions of BluetoothHfpManager

Implemented basic functions (ctor/dtor/Connect/Disconnect/Message handler) of BluetoothHfpManager, assumed we've already had SocketConsumer here.
Attachment #661172 - Attachment is obsolete: true
Attachment #661173 - Attachment is obsolete: true
Attachment #661174 - Attachment is obsolete: true
Attachment #662110 - Flags: review?(kyle)
(Assignee)

Updated

6 years ago
Blocks: 792002
Comment on attachment 662110 [details] [diff] [review]
v1: Essential functions of BluetoothHfpManager

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

Doesn't this need to be added to the makefile? :)

Other than that, looks good. Of course, still completely dependent on Bug 776182. From the review I got from cjones yesterday, it looks like the core API is sound, but there may be quite a bit of changes underneath. Nothing that should drastically affect this, but enough that this should wait until I'm done. So, marking r? until then but assumed r+.

::: dom/bluetooth/BluetoothHfpManager.cpp
@@ +55,5 @@
> +    NS_ConvertUTF8toUTF16(mozilla::dom::bluetooth::BluetoothServiceUuidStr::Handsfree);
> +
> +  nsRefPtr<BluetoothReplyRunnable> runnable = aRunnable;
> +
> +  nsresult rv = bs->GetSocketViaService(aDeviceObjectPath,

Note: This was an nsresult because it was going to be exposed to DOM. Since it's going to be internal only I'll probably change it to a bool.

@@ +80,5 @@
> +  }
> +
> +  nsRefPtr<BluetoothReplyRunnable> runnable = aRunnable;
> +
> +  nsresult rv = bs->CloseSocket(this, runnable);

Note: Same deal as above.
(Assignee)

Comment 7

6 years ago
> Doesn't this need to be added to the makefile? :)
Definitely. :)
(Assignee)

Updated

6 years ago
Depends on: 776182
(Assignee)

Comment 8

6 years ago
Created attachment 662391 [details] [diff] [review]
v2: Essential functions of BluetoothHfpManager

Added BluetoothHfpManager to Makefile
Attachment #662110 - Attachment is obsolete: true
Attachment #662110 - Flags: review?(kyle)
(Assignee)

Comment 9

6 years ago
Created attachment 664788 [details] [diff] [review]
v3: BluetoothHfpManager prototype

* Used nsRefPtr to hold sInstance
* Changed class name and little code for new unixsocket API
Attachment #662391 - Attachment is obsolete: true
Attachment #664788 - Flags: review?(kyle)
Comment on attachment 664788 [details] [diff] [review]
v3: BluetoothHfpManager prototype

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

::: dom/bluetooth/BluetoothHfpManager.cpp
@@ +12,5 @@
> +
> +USING_BLUETOOTH_NAMESPACE
> +using namespace mozilla::ipc;
> +
> +nsRefPtr<BluetoothHfpManager> sInstance = nullptr;

We should probably have a shutdown observer somewhere to actually kill this, but for now that can be filed in a followup, since B2G never rightly shuts down.

@@ +25,5 @@
> +
> +//static
> +BluetoothHfpManager*
> +BluetoothHfpManager::Get()
> +{

Assert for main thread only.

@@ +36,5 @@
> +
> +// Virtual function of class SocketConsumer
> +void
> +BluetoothHfpManager::ReceiveSocketData(UnixSocketRawData* aMessage)
> +{

Assert for main thread only.
Attachment #664788 - Flags: review?(kyle) → review+
(Assignee)

Comment 11

6 years ago
Created attachment 664806 [details] [diff] [review]
Final: Essential functions of BluetoothHfpManager, r=qdot

I'll file a follow-up bug for Kyle's first comment later.
Attachment #664788 - Attachment is obsolete: true
(Assignee)

Comment 13

6 years ago
Created attachment 664876 [details] [diff] [review]
Final: Essential functions of BluetoothHfpManager, r=qdot

added "static" keyword.
Attachment #664806 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/4dfa1754f818
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in before you can comment on or make changes to this bug.