B2G Bluetooth Boiler Plate

RESOLVED FIXED in mozilla13

Status

()

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

People

(Reporter: qdot, Assigned: qdot)

Tracking

Trunk
mozilla13
ARM
Gonk (Firefox OS)
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Metabug for B2G Bluetooth tasks
Assignee: nobody → kyle
Alias: b2g-bluetooth
Version: unspecified → Trunk
There's no "b2g bluetooth" ... there's the general DOM API, and the gonk impl of that API.  Do you mean the latter here?
Ah, yeah, mean gonk impl of API. I'll clear up the name of the bug a bit.
Summary: [meta] B2G Bluetooth → [meta] Bluetooth Implementation on B2G
Main idea is to make a python-bluez like interface in js. Starting with looking at http://code.google.com/p/pybluez/
Depends on: 674737
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++?
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.
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.
Depends on: 713116
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.
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.
Blocks: 715782

Comment 9

6 years ago
(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
Created attachment 589688 [details] [diff] [review]
Patch 1: Boilerplate code for creating navigator.mozBluetooth object
Attachment #589688 - Flags: review?(bent.mozilla)
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.
Attachment #589688 - Flags: review?(bent.mozilla) → review+
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.
Attachment #589688 - Attachment is obsolete: true
Try Results:

https://tbpl.mozilla.org/?tree=Try&rev=df2ec5e17dc9
Whiteboard: [Leave open after merge]
Morphing this bug to what landed: just the boilerplate. Going to open a new bug for trackign.
Alias: b2g-bluetooth
No longer depends on: 674737, 713116
Summary: [meta] Bluetooth Implementation on B2G → B2G Bluetooth Boiler Plate
Whiteboard: [Leave open after merge]
Blocks: 727618
https://hg.mozilla.org/integration/mozilla-inbound/rev/50a3c34ea8ae
https://hg.mozilla.org/mozilla-central/rev/4c8aa2b0ca1c
https://hg.mozilla.org/mozilla-central/rev/50a3c34ea8ae
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
Depends on: 728389
Reopened due to qemu breakage

https://hg.mozilla.org/mozilla-central/rev/a3b93f3949fe
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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.
Attachment #595902 - Attachment is obsolete: true
Attachment #598859 - Flags: review?(bent.mozilla)
Attachment #598859 - Flags: review?(bent.mozilla) → review+
https://hg.mozilla.org/mozilla-central/rev/13ff3e5b66ea
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.