Closed
Bug 740741
Opened 13 years ago
Closed 12 years ago
B2G Bluetooth: StartDiscovery/StopDiscovery
Categories
(Core :: DOM: Device Interfaces, defect)
Tracking
()
RESOLVED
DUPLICATE
of bug 740744
People
(Reporter: echou, Assigned: echou)
References
Details
Attachments
(2 files, 3 obsolete files)
6.69 KB,
patch
|
Details | Diff | Splinter Review | |
5.72 KB,
patch
|
qdot
:
feedback+
|
Details | Diff | Splinter Review |
One of major functions of Bluetooth is Device Discovering. We need to implement function 'StartDiscovery'/'StopDiscovery' of nsIDOMBluetoothAdapter to let user do/cancel device discovering routine.
Assignee | ||
Updated•13 years ago
|
Blocks: b2g-bluetooth
Assignee | ||
Updated•13 years ago
|
Assignee: nobody → echou
Assignee | ||
Comment 1•13 years ago
|
||
I added two functions dbus_func_args_timeout_valist() and dbus_func_args() to RawDBusConnection.cpp, may be used in BluetoothAdapter many times for DBus communications. However, those should be implemented in Bug 730942. That's why this issue depends on that one. So we should focus on StartDiscovery() and StopDiscovery().
While discovering, DeviceFound events would be created whenever a device has been discovered. For more information, please check Bug 740744.
Attachment #614333 -
Flags: feedback?(kyle)
Comment 2•13 years ago
|
||
Comment on attachment 614333 [details] [diff] [review]
WIP: StartDiscovery/StopDiscovery
We should actually throw this in their own file as free functions, since they'll be under the android license. I'll do this over in 730942 real quick.
Attachment #614333 -
Flags: feedback?(kyle) → feedback-
Assignee | ||
Updated•13 years ago
|
Summary: Implement 'StartDiscovery' and 'StopDiscovery' functions of Bluetooth → B2G Bluetooth: StartDiscovery/StopDiscovery
Assignee | ||
Comment 3•13 years ago
|
||
WIP. Functions called in StartDiscovery and StopDiscovery are not actually implemented in current code base, so implemetation will still be modified, but interface won't.
Attachment #614333 -
Attachment is obsolete: true
Assignee | ||
Comment 4•13 years ago
|
||
WIP. Functions called in StartDiscovery and StopDiscovery are not actually implemented in current code base, so implemetation will still be modified, but interface won't.
Attachment #626393 -
Attachment is obsolete: true
Assignee | ||
Comment 5•13 years ago
|
||
1. BluetoothService will handle DBus related functions. BluetoothAdapter & BluetoothDevice should not call those functions directly, or it will be hard to read. Furthermore, there will be several functions used by both BluetoothAdapter & BluetoothDevice, so we place them to BluetoothService.
2. We use Android apache license because in BluetoothService because most functions will be modified from android_server_BluetoothService.cpp
Attachment #626395 -
Attachment is obsolete: true
Attachment #627093 -
Flags: feedback?(kyle)
Comment 6•13 years ago
|
||
Comment on attachment 627093 [details] [diff] [review]
WIP v3: needs feedback
Review of attachment 627093 [details] [diff] [review]:
-----------------------------------------------------------------
Service probably isn't the best name for this file. We expect anything named "Service" in gecko to be a sort of Singleton accessible to the universe for some certain function (i.e. SettingsService, etc...). Maybe Utils?
Also, why does Start/StopDiscovery need to be there versus being methods of BluetoothAdapter? The only thing that can discover is an adapter, so there's no reason that I can tell to put those outside of the class? I can certainly see some of the other DBus-ish functions going in here, but Discovery seems pretty Adapter specific?
Attachment #627093 -
Flags: feedback?(kyle)
Assignee | ||
Comment 7•13 years ago
|
||
I know what a service is. I'll name it BluetoothUtils. :)
Although Start/Stop Discovery will only be called in BluetoothAdapter, the reason I put them outside of BluetoothAdapter is to make code more clean. You will not see dbus_func_args_async , dbus_set_error_from_message or dbus_message_iter_init_append is called in BluetoothAdapter or BluetoothDevice, sure we can just call them if you think that would be better.
Comment 8•13 years ago
|
||
Wow, yeah, I totally flaked on that "maybe we want to do this on other platforms someday" thing that I used to talk about so much, heh. Been totally focused on just getting this done for B2G. So I guess moving the implementations out to platform specific files may be a good idea. Just need to rethink how I'm going to deal with that elsewhere. Honestly, if it does take to long, we can throw them in the Bluetooth* classes to start and then refactor later, since we are just shipping this on B2G for the foreseeable future.
Assignee | ||
Comment 9•13 years ago
|
||
So, I'll make the implementation stay at BluetoothUtils, because that's what I've done in my branch now. Make sense? If so, I'm going to send the patch again.
Assignee | ||
Comment 10•13 years ago
|
||
Move to BluetoothUtils.cpp, needs more feedback.
Attachment #627146 -
Flags: feedback?(kyle)
Comment 11•13 years ago
|
||
Comment on attachment 627146 [details] [diff] [review]
WIP v4: StartDiscovery/StopDiscovery in BluetoothUtils
Review of attachment 627146 [details] [diff] [review]:
-----------------------------------------------------------------
Ok, this all looks good. I should change my mName to mPath to match your usage. Also, I know I already said we should change this once, but... If we're going to keep all of the DBusish stuff in BluetoothUtils (which brings up a really interesting question of what we want to do with the RawDBusConnection base class I had...), we should probably rename it to BluetoothDBusUtils. That way we can also have BluetoothOSXUtils/BluetoothWindowsUtils when the time comes.
Attachment #627146 -
Flags: feedback?(kyle) → feedback+
Assignee | ||
Comment 12•13 years ago
|
||
Actually, I'm not quite sure how to use RawDBusConnection, should I inherit it, or should I put a member object in BluetoothAdapter and use it as a utility class? I don't even know what functions should be in RawDBusConnection and what should be in BluetoothUtils or the other files.
And, we can rename it since functions in this file are mostly composed of DBus-ish stuff.
Comment 13•13 years ago
|
||
I originally saw it as an inheritance sort of thing, to give classes the ability to have their own DBusConnection, and probably where some of the more general functions would go (dbus_func_* from android, etc...). However, I wasn't really thinking much about the whole cross platform issue when I came up with that, so... I suppose things could continue to go in Utils for the moment. I don't really know if the idea of exposing DBusThread's dbus connection as the only one will work, because of the thread conflicts we'll have between the IOThread Event Loop and other things trying to make calls.
Comment 14•12 years ago
|
||
This got wrapped into the snowball that is bug 740744.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → DUPLICATE
You need to log in
before you can comment on or make changes to this bug.
Description
•