711601 - B2G Bluetooth Boiler Plate

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Metabug for B2G Bluetooth tasks

Comment 1

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

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 [:qdot] [:kmachulis] (INACTIVE)]

Ah, yeah, mean gonk impl of API. I'll clear up the name of the bug a bit.

Comment 3

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

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]

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 [:qdot] [:kmachulis] (INACTIVE)]

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]

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 [:qdot] [:kmachulis] (INACTIVE)]

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 [:qdot] [:kmachulis] (INACTIVE)]

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]

(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
[e-mail]

Comment 10

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Attachment: Patch 1: Boilerplate code for creating navigator.mozBluetooth object

Comment 11

[Ben Turner (not reading bugmail, use the needinfo flag!)]

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 [:qdot] [:kmachulis] (INACTIVE)]

Attachment: Patch 1: Boilerplate code for creating navigator.mozBluetooth object

Updated to fix nits and remove currently unneeded dbus inclusions.

Comment 13

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Try Results:

https://tbpl.mozilla.org/?tree=Try&rev=df2ec5e17dc9

Comment 14

[Philipp von Weitershausen [:philikon]]

Morphing this bug to what landed: just the boilerplate. Going to open a new bug for trackign.

Comment 15

[Philipp von Weitershausen [:philikon]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/50a3c34ea8ae

Comment 16

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/4c8aa2b0ca1c
https://hg.mozilla.org/mozilla-central/rev/50a3c34ea8ae

Comment 17

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Reopened due to qemu breakage

https://hg.mozilla.org/mozilla-central/rev/a3b93f3949fe

Comment 18

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Attachment: 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]]

https://hg.mozilla.org/mozilla-central/rev/13ff3e5b66ea