Last Comment Bug 711601 - B2G Bluetooth Boiler Plate
: B2G Bluetooth Boiler Plate
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM: Device Interfaces (show other bugs)
: Trunk
: ARM Gonk (Firefox OS)
: -- normal (vote)
: mozilla13
Assigned To: Kyle Machulis [:kmachulis] [:qdot]
:
Mentors:
Depends on: 728389
Blocks: b2g-demo-phone b2g-bluetooth
  Show dependency treegraph
 
Reported: 2011-12-16 13:43 PST by Kyle Machulis [:kmachulis] [:qdot]
Modified: 2012-02-21 08:53 PST (History)
10 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch 1: Boilerplate code for creating navigator.mozBluetooth object (25.09 KB, patch)
2012-01-18 15:57 PST, Kyle Machulis [:kmachulis] [:qdot]
bent.mozilla: review+
Details | Diff | Review
Patch 1: Boilerplate code for creating navigator.mozBluetooth object (14.70 KB, patch)
2012-02-09 16:26 PST, Kyle Machulis [:kmachulis] [:qdot]
no flags Details | Diff | Review
Patch 1: Boilerplate code for creating navigator.mozBluetooth object (14.57 KB, patch)
2012-02-20 07:09 PST, Kyle Machulis [:kmachulis] [:qdot]
bent.mozilla: review+
Details | Diff | Review

Description Kyle Machulis [:kmachulis] [:qdot] 2011-12-16 13:43:35 PST
Metabug for B2G Bluetooth tasks
Comment 1 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-12-16 13:48:54 PST
There's no "b2g bluetooth" ... there's the general DOM API, and the gonk impl of that API.  Do you mean the latter here?
Comment 2 Kyle Machulis [:kmachulis] [:qdot] 2011-12-16 16:10:08 PST
Ah, yeah, mean gonk impl of API. I'll clear up the name of the bug a bit.
Comment 3 Kyle Machulis [:kmachulis] [:qdot] 2011-12-16 16:22:54 PST
Main idea is to make a python-bluez like interface in js. Starting with looking at http://code.google.com/p/pybluez/
Comment 4 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-12-16 23:24:25 PST
Why do we need to bind bluez to JS?  I'm not sure what we gain from that.  Can't the impl of the bluetooth interface live in C++?
Comment 5 Kyle Machulis [:kmachulis] [:qdot] 2011-12-21 16:44:00 PST
The android bluetooth implementation diagram, henceforth referred to as the "diagram of sadness":

https://sites.google.com/a/android.com/opensource/projects/bluetooth-faq/android_bluetooth_architecture_10.jpg

So, if we want to cover everything android covers, we'll need bindings to dbus (doable thru nsDBusService), sockets (more ipc/libevent threads like RIL?), and sysfs (for power). I think we have pieces for most of this already, but we'll have to figure out what's going to connect where, and how. First goal is still just to do discovery and bonding.
Comment 6 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-12-21 16:57:22 PST
Let's focus on the highest-impact work first.  That's bluetooth headsets, right?  That part of the code looks like it can pretty self-contained.
Comment 7 Kyle Machulis [:kmachulis] [:qdot] 2012-01-04 15:59:58 PST
Starting up on device discovery, most of the work is going to be translating glue/gonk/frameworks/base/core/jni/android_server_BluetoothService.cpp into Gecko and the bt DOM. Obviously there's going to be lots of async happening here (requesting power enable/disable, waiting for updates from discovery, etc...), not quite sure how to handle that yet.
Comment 8 Kyle Machulis [:kmachulis] [:qdot] 2012-01-04 16:38:08 PST
I should note that android_server_BluetoothService isn't doing anything particularly interesting or out of the ordinary for bluez dbus communication, it's just a good TODO list of functions for the low level DOM queries.
Comment 9 Eddie Maddox 2012-01-16 19:01:49 PST
(In reply to Chris Jones [:cjones] [:warhammer] from comment #6)
> Let's focus on the highest-impact work first.
> That's bluetooth headsets, right? ...

Please also consider Human Interface Device (HID) support a "high-impact work".

I bought my first (lobotimized) "smart"phone 8 NOV 2011 from a dealer.
They had a stack of US$100 Motorola bluetooth keyboards that had come in that day,
"for Motorola ...".
My rep could pair one of those with my new Casio Commando Android device,
but not do any text entry with it.
He had no trouble pairing and using it on his Motorola device.

I later paid US$60 for a Targus Bluetooth Wireless Keyboard,
"for Tablets".
I can pair it, but not do anything else with it.

I do not know, and do not know how to find out,
whether Android 4.0, by Google or Cyanogenmod,
include bluetooth HID support for __all__ devices,
not just for Motorola and tablet devices.

So, please consider Human Interface Device (HID) support a "high-impact work".

Thank you,
Eddie Maddox
greatnessguru@gmail.com
Comment 10 Kyle Machulis [:kmachulis] [:qdot] 2012-01-18 15:57:58 PST
Created attachment 589688 [details] [diff] [review]
Patch 1: Boilerplate code for creating navigator.mozBluetooth object
Comment 11 Ben Turner (not reading bugmail, use the needinfo flag!) 2012-02-09 13:36:48 PST
Comment on attachment 589688 [details] [diff] [review]
Patch 1: Boilerplate code for creating navigator.mozBluetooth object

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

Looks good! Just a couple little things below:

::: dom/base/Navigator.cpp
@@ +1086,5 @@
> +    NS_ENSURE_TRUE(window, NS_ERROR_FAILURE);
> +
> +    mBluetooth = new bluetooth::BluetoothAdapter();
> +
> +    // mBluetoothAdapter may be null here!

It had better not be! Remove that comment ;)

::: dom/bluetooth/BluetoothAdapter.cpp
@@ +1,4 @@
> +/* -*- Mode: c++; c-basic-offset: 2; indent-tabs-mode: nil; tab-width: 40 -*- */
> +/* vim: set ts=2 et sw=2 tw=40: */
> +/* ***** BEGIN LICENSE BLOCK *****
> + * Version: MPL 1.1/GPL 2.0/LGPL 2.1

MPL2 now for all new code :)

@@ +50,5 @@
> +
> +NS_INTERFACE_MAP_BEGIN(BluetoothAdapter)
> +NS_INTERFACE_MAP_ENTRY(nsIDOMBluetoothAdapter)
> +NS_INTERFACE_MAP_ENTRY(nsISupports)
> +NS_DOM_INTERFACE_MAP_ENTRY_CLASSINFO(BluetoothAdapter)

Nit: We usually indent these.

::: dom/bluetooth/BluetoothCommon.h
@@ +50,5 @@
> +class nsIDOMBluetooth;
> +
> +BEGIN_BLUETOOTH_NAMESPACE
> +
> +class Adapter;

This has changed to BluetoothAdapter

::: dom/bluetooth/Makefile.in
@@ +50,5 @@
> +
> +include $(topsrcdir)/dom/dom-config.mk
> +
> +LOCAL_INCLUDES  += $(MOZ_DBUS_CFLAGS) \
> +                   $(NULL)

Nit: No need for the extra $(NULL) if it's only one

::: dom/dom-config.mk
@@ +31,5 @@
>  
> +ifdef MOZ_B2G_BT
> +DOM_SRCDIRS += \
> +  dom/bluetooth \
> +  $(NULL)

Nit: same here

::: dom/interfaces/base/nsIDOMNavigator.idl
@@ +39,4 @@
>  
>  #include "domstubs.idl"
>  
> +[scriptable, uuid(aafd8c5a-91c8-4c90-af2a-8c66b4d6773f)]

This looks unnecessary, you're not actually changing the interface.

::: layout/build/Makefile.in
@@ +242,4 @@
>  		   -I$(srcdir)/../xul/base/src \
>  		   -I$(srcdir)/../mathml \
>  		   -I$(topsrcdir)/content/base/src \
> +		   -I$(topsrcdir)/content/canvas/src \

Unrelated whitespace change?

::: toolkit/library/Makefile.in
@@ +427,5 @@
>    $(NULL)
> +ifdef MOZ_B2G_BT
> +OS_LIBS += \
> +  -lbluedroid \
> +  $(NULL)

Nit: single line here too.
Comment 12 Kyle Machulis [:kmachulis] [:qdot] 2012-02-09 16:26:55 PST
Created attachment 595902 [details] [diff] [review]
Patch 1: Boilerplate code for creating navigator.mozBluetooth object

Updated to fix nits and remove currently unneeded dbus inclusions.
Comment 13 Kyle Machulis [:kmachulis] [:qdot] 2012-02-13 19:13:40 PST
Try Results:

https://tbpl.mozilla.org/?tree=Try&rev=df2ec5e17dc9
Comment 14 Philipp von Weitershausen [:philikon] 2012-02-15 14:49:25 PST
Morphing this bug to what landed: just the boilerplate. Going to open a new bug for trackign.
Comment 15 Philipp von Weitershausen [:philikon] 2012-02-15 14:52:42 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/50a3c34ea8ae
Comment 17 Kyle Machulis [:kmachulis] [:qdot] 2012-02-17 15:57:21 PST
Reopened due to qemu breakage

https://hg.mozilla.org/mozilla-central/rev/a3b93f3949fe
Comment 18 Kyle Machulis [:kmachulis] [:qdot] 2012-02-20 07:09:43 PST
Created attachment 598859 [details] [diff] [review]
Patch 1: Boilerplate code for creating navigator.mozBluetooth object

Updated patch to remove inclusion of BT in gonk by default, until we can figure out how we want to deal with the emulator.
Comment 19 Ed Morley [:emorley] 2012-02-21 08:53:56 PST
https://hg.mozilla.org/mozilla-central/rev/13ff3e5b66ea

Note You need to log in before you can comment on or make changes to this bug.