852583 - B2G Emulator: support bluetooth emulation

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Vicamo Yang [:vicamo][:vyang]]

Attachment: Bring up Bluetooth in emulator sucessfully

Current WIP branches:
https://github.com/vicamo/b2g_platform_build/tree/bluetooth
https://github.com/vicamo/b2g_platform_system_core/tree/bluetooth
https://github.com/vicamo/b2g_platform_external_qemu/tree/bluetooth
https://github.com/vicamo/b2g_kernel_goldfish/tree/bluetooth

Comment 1

[Vicamo Yang [:vicamo][:vyang]]

Attachment: prebuilts/qemu-kernel/arm/kernel-qemu-armv7

Comment 2

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 726741 [details]
prebuilts/qemu-kernel/arm/kernel-qemu-armv7

Kernel images are too big to be attached in bugzilla. Here is the build instructions:

  1) cd B2G_SOURCE_TOP
  2) . build/envsetup.sh
  3) lunch 1
  4) cd KENER_SOURCE_TOP
  5) ${ANDROID_BUILD_TOP}/external/qemu/distrib/build-kernel.sh \
     --out=${ANDROID_BUILD_TOP}/prebuilts/qemu-kernel/arm \
     --armv7 --verbose

To build goldfish kernel (not goldfish_armv7):

  5) ${ANDROID_BUILD_TOP}/external/qemu/distrib/build-kernel.sh \
     --out=${ANDROID_BUILD_TOP}/prebuilts/qemu-kernel/arm \
     --config=goldfish --verbose

You may omit --verbose to disable the noises. And to config the kernel based on either goldfish_defconfig or goldfish_armv7_defconfig, append --menuconfig like:

  5) ${ANDROID_BUILD_TOP}/external/qemu/distrib/build-kernel.sh \
     --out=${ANDROID_BUILD_TOP}/prebuilts/qemu-kernel/arm \
     --armv7 --menuconfig

Note that you may need libncurse5-dev:amd64 for kernel menuconfig, however B2G needs libncurse5-dev:i386 and they can't co-exist on debian-based distros.

Comment 3

[cmtalbert]

Is this actually putting the full bluetooth stack into the emulator? Could you put the pre-built kernel up on people.m.o?

Comment 4

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Clint Talbert ( :ctalbert ) from comment #3)
> Is this actually putting the full bluetooth stack into the emulator?

Yes, the emulator will then have full bluetooth support just like a real device.

> Could you put the pre-built kernel up on people.m.o?

Will, but don't know how right now ;)

Comment 5

[Vicamo Yang [:vicamo][:vyang]]

Attachment: run-emulator.sh: add an emulated hci device to target

You'll need this patch to insert a emulated hci device to target for now. Will make it a default in updates later.

Comment 6

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #0)
> Bring up Bluetooth in emulator sucessfully

Current status:

1) target kernel uart device done, can bind hci h4 protocol with success
2) target kernel dummy rfkill driver installed, always returns success
3) emulator hci device creation done, supports multiple instances as well
4) hci traffic between target and host works
5) hciattach service and misc bluetooth related settings in init.goldfish.rc

TODOs:

1) link target rfkill driver with emulator host side
2) emulator hci traffic handling
3) make bluetooth a default enabled hw config, obsoletes attachment 726783 [details] [diff] [review]
4) more bluetooth related init services, settings: bluetoothd, dbus, etc.
5) emulator console commands for automation tests

Comment 7

[Vicamo Yang [:vicamo][:vyang]]

Github WIP branch for kernel images:
https://github.com/vicamo/b2g_platform_prebuilts_qemu-kernel/tree/bluetooth

Kernel images for 2.6 ARMv7
https://github.com/vicamo/b2g_platform_prebuilts_qemu-kernel/raw/bluetooth/arm/kernel-qemu-armv7
https://github.com/vicamo/b2g_platform_prebuilts_qemu-kernel/raw/bluetooth/arm/vmlinux-qemu-armv7

Kernel images for 2.6 ARMv5
https://github.com/vicamo/b2g_platform_prebuilts_qemu-kernel/raw/bluetooth/arm/kernel-qemu
https://github.com/vicamo/b2g_platform_prebuilts_qemu-kernel/raw/bluetooth/arm/vmlinux-qemu

You don't need libncurse5-dev:amd64 if you don't want to re-configure kernel, and in my opinion, you don't really have to config kernel again. Just rebuild the kernel images locally or download them from above links. Or, you can create two chroots, one for building B2G and another for configuring the kernel.

Comment 8

[Vicamo Yang [:vicamo][:vyang]]

TODOs 1), 3) done, a few more bits for 2).

Comment 9

[Vicamo Yang [:vicamo][:vyang]]

TODOs 5): kernel rfkill driver officially supports hardware block. Emulator console commands for testing rfkill are landed as well. Now we can write our first bluetooth test case based on Marionette.

Links in comment #7 are still valid. All binaries are just updated.

Comment 10

[Vicamo Yang [:vicamo][:vyang]]

It seems qemu has already a working virtual hci device implementation[1]. If we can take advantage of it, we might have a fully functional emulated bluetooth device in target os in a couple of days. :D

[1]: https://github.com/vicamo/b2g_platform_external_qemu/blob/bluetooth/hw/bt-hci.c#L2144

Comment 11

[Vicamo Yang [:vicamo][:vyang]]

All TODOs listed in comment 6 are done, commits pushed. Emulator BT is now functional :) However, there seems to be two problems: BlueZ version compatibility and Gaia timing issue.

1) The BlueZ shipped in Unagi is from some proprietary source and Mozilla's WebBluetooth implementation depends on some of the proprietary dbus methods like DisconnectAllConnections. We reset to the same commit in external/bluetooth/bluez and external/bluetooth/glib to avoid UnknownMethod errors. Also an assertion in dbus was observed.

2) Gaia issues bluetooth requests even before bluetoothd is ready and results in dbus UnknownService errors and UI lock-up. Need more info.

Comment 12

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

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #11)
> All TODOs listed in comment 6 are done, commits pushed. Emulator BT is now
> functional :) However, there seems to be two problems: BlueZ version
> compatibility and Gaia timing issue.
> 
> 1) The BlueZ shipped in Unagi is from some proprietary source and Mozilla's
> WebBluetooth implementation depends on some of the proprietary dbus methods
> like DisconnectAllConnections. We reset to the same commit in
> external/bluetooth/bluez and external/bluetooth/glib to avoid UnknownMethod
> errors. Also an assertion in dbus was observed.

Welcome to android. :3

Google implemented all sorts of convenience functions inside android bluez because why the hell not. None of the functions that push around XML exist, all service channels are hardcoded, etc...

We have bug 793977 to implement the linux desktop specific backends for this, but what you're seeing now is part of why google never implemented BT in the emulator. We may just be out of luck for testing some of those things.

> 2) Gaia issues bluetooth requests even before bluetoothd is ready and
> results in dbus UnknownService errors and UI lock-up. Need more info.

Which requests is it issuing, exactly?

Comment 13

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

DisconnectAllConnections added by qualcomm so this is why we don't see code in aosp.

Comment 14

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

Wow, didn't realize there was QC additions on top of the google additions. :(

Comment 15

[Michael Wu [:mwu]]

Most people don't realize it. And then their code breaks on pandaboard or emulator.

Comment 16

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

Hmmm. As much as I hate to ask, do we have any QC specific ifdefs? BTW, this came in with bug 834816.

Comment 17

[Michael Wu [:mwu]]

There are none as far as I know.

Comment 18

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

QC specific code from codeaurora fixed some of donez compatibility problems and extends LE part. During Bluetooth turn off process disconnecting all ACL connections will be initiated. If we don't use extend bluez api (AOSP version), we still need to do disconnection gracefully one profile by profile, otherwise we could suffer many bugs if the ACL link Link Timeout value is large.
If we disconnect per profile (currently only HFP profile), the time of turning off Bluetooth could take much longer.

Comment 19

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

So can we just put in a fallback for the functionality to do it the slow way if DisconnectAllConnections doesn't exist? I'm mainly worried about when we start trying to do pandaboard or non-qc-chipset testing.

Comment 20

[Vicamo Yang [:vicamo][:vyang]]

Attachment: gaia-lockup-after-enable-bt.txt

The last hci command is OGF_HOST_CTL:OCF_READ_SCAN_ENABLE in line 1093, and we have response {status: 0, enable: SCAN_PAGE} in line 1128 of the log file. And in bluez/src/adapter.c, function adapter_mode_changed, it seems that we're going to emit a property signal, but somehow Gaia gets a UnknownService DbusError first, so that signal is never delivered to Gaia and results in UI lock-up.

Comment 21

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

If DisconnectAllConnections doesn't exist, i think dbus just prints Unknown method error but turning-off thing shall still can work. DisconnectAllConnections in bluez does not do vendor specific HCI command but just tear down ACL link using standard hci command, it won't behave differntly with other Bluetooth wireless chipsets. And yes, it's nice to have centralized graceful shutdown process in gecko. Eric, please correct me if i'm wrong.

Comment 22

[John Lin [:jhlin][:jolin]]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #20)
> Created attachment 729632 [details]
> gaia-lockup-after-enable-bt.txt
> 
> The last hci command is OGF_HOST_CTL:OCF_READ_SCAN_ENABLE in line 1093, and
> we have response {status: 0, enable: SCAN_PAGE} in line 1128 of the log
> file. And in bluez/src/adapter.c, function adapter_mode_changed, it seems
> that we're going to emit a property signal, but somehow Gaia gets a
> UnknownService DbusError first, so that signal is never delivered to Gaia
> and results in UI lock-up.
Also got UnknownService DbusError here after applying your latest patches and run b2g emulator. Logcat shows bluetoothd crashes in some bluez sdp function. (Attachment later)

Comment 23

[John Lin [:jhlin][:jolin]]

Attachment: Emulator bluetoothd crash log

Comment 24

[Vicamo Yang [:vicamo][:vyang]]

(In reply to John Lin[:jolin][:jhlin] from comment #22)
> Also got UnknownService DbusError here after applying your latest patches
> and run b2g emulator. Logcat shows bluetoothd crashes in some bluez sdp
> function. (Attachment later)

Great! Maybe that's why Gaia can't get a service.

Comment 25

[Vicamo Yang [:vicamo][:vyang]]

(In reply to John Lin[:jolin][:jhlin] from comment #22)
> Also got UnknownService DbusError here after applying your latest patches
> and run b2g emulator. Logcat shows bluetoothd crashes in some bluez sdp
> function. (Attachment later)

No much time to work on this recently. But I haven't found a bluetoothd/dbus-daemon crash yet. And bluetoothd had registered itself to dbus, or it should have exited with errors.

Comment 26

[Vicamo Yang [:vicamo][:vyang]]

Attachment: [gaia-lockup-after-enable-bt] success

Gaia installs onadatperadded callback in line 180. That callback is then called in line 279.

Comment 27

[Vicamo Yang [:vicamo][:vyang]]

Attachment: [gaia-lockup-after-enable-bt] failed

Gaia installs onadatperadded callback in line 174. That callback is never called near line 278 even BluetoothManager dispatches that signal!

Comment 28

[Vicamo Yang [:vicamo][:vyang]]

Thanks GinaYeh, the Gaia lock-up problem is now proven bug 847851. I'll then do some more clean-ups and file pull requests for basic Bluetooth emulation support :)

Comment 29

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 731930 [details]
[gaia-lockup-after-enable-bt] success

Bug 847851 is already landed in master branch.

Comment 30

[Vicamo Yang [:vicamo][:vyang]]

(In reply to John Lin[:jolin][:jhlin] from comment #23)
> Emulator bluetoothd crash log

I still have no luck to reproduce such crash. Let's file a new bug for it later.

Comment 31

[Vicamo Yang [:vicamo][:vyang]]

Attachment: Github PR for platform/build

Comment 32

[Vicamo Yang [:vicamo][:vyang]]

Attachment: Github PR for platform/system/core

platform/system/core is not yet a repository in git.mozilla.org/b2g. Its master branch HEAD is actually aosp/ics-plus-aosp.

Comment 33

[Vicamo Yang [:vicamo][:vyang]]

Attachment: Github PR for platform/external/qemu

Comment 34

[Vicamo Yang [:vicamo][:vyang]]

Attachment: Github PR for manifest

Comment 35

[Vicamo Yang [:vicamo][:vyang]]

Push kernel commits to Github takes a while. I'll file PRs for both kernel source and prebuilt kernel images tomorrow.

Comment 36

[John Lin [:jhlin][:jolin]]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #30)
> (In reply to John Lin[:jolin][:jhlin] from comment #23)
> > Emulator bluetoothd crash log
> 
> I still have no luck to reproduce such crash. Let's file a new bug for it
> later.
Looks like that's a false alarm. After a clean rebuild the crash went away.

Comment 37

[Vicamo Yang [:vicamo][:vyang]]

Attachment: Github PR for kernel/goldfish

Comment 38

[Vicamo Yang [:vicamo][:vyang]]

Attachment: Github PR for platform/prebuilts/qemu-kernel

Comment 39

[Thomas Zimmermann [:tzimmermann] [:tdz]]

I'm currently traveling. I won't get to the review until early net week.

Comment 40

[Michael Wu [:mwu]]

One thing I forgot to mention - for any new files you add with a copyright, make sure it's copyright Mozilla Foundation.

Comment 41

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 732219 [details]
Github PR for platform/build

Merged in Github: https://github.com/mozilla-b2g/platform_build/commit/a56a09f9b3342e5d5cb05699288913035f4f6e48

Comment 42

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Vicamo Yang (@Telefonica 6F) [:vicamo][:vyang] from comment #32)
> Created attachment 732228 [details]
> Github PR for platform/system/core
> 
> platform/system/core is not yet a repository in git.mozilla.org/b2g. Its
> master branch HEAD is actually aosp/ics-plus-aosp.

Will fork device/generic/goldfish instead.

Comment 43

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Comment on attachment 732655 [details]
Github PR for kernel/goldfish

r-'ing this for now.

Comment 44

[Vicamo Yang [:vicamo][:vyang]]

Attachment: Github PR for device/generic/goldfish

Deprecates PR for platform/system/core to keep device specific changes in device/.

Comment 45

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Comment on attachment 732234 [details]
Github PR for platform/external/qemu

Hi Vicamo,

Sorry for the late response. I just glanced through your patches which look cool, but I feel like I am not confident enough to review them. Do you think you can find another reviewer for your patches?

Comment 46

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 732655 [details]
Github PR for kernel/goldfish

Updated on GitHub.

Comment 47

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 732234 [details]
Github PR for platform/external/qemu

May I have your review?

Comment 48

[Michael Wu [:mwu]]

Comment on attachment 732252 [details]
Github PR for manifest

You may need to talk to jgriffin about deploying emulator changes when this is ready.

Comment 49

[Dave Hylands [:dhylands]]

Comment on attachment 732234 [details]
Github PR for platform/external/qemu

I'm not famailiar with this script or this tree, but the changes seem reasonable to me, provided we're not planning on trying to upstream the changes. I made a couple of nit comments on some of the echos.

Comment 50

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #44)
> Created attachment 745085 [details]
> Github PR for device/generic/goldfish

Merged on Github https://github.com/mozilla-b2g/device_generic_goldfish/commit/b1f212165a53315155cdd942d1ee36d4763989ed

Comment 51

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Michael Wu [:mwu] from comment #48)
> Comment on attachment 732252 [details]
> Github PR for manifest
> 
> You may need to talk to jgriffin about deploying emulator changes when this
> is ready.

This pull request doesn't depend on other PRs, and even some of other PRs are merged, we still get a working emulator.  Only when all PRs listed are merged, we have bluetooth emulation.  But, for sure, I'll ask jgriffin for deploying these changes after everything is done here.

Comment 52

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Comment on attachment 732234 [details]
Github PR for platform/external/qemu

I reviewed the changes and left some suggestions for improving the commits at

>  https://github.com/vicamo/b2g_platform_external_qemu/commit/a45f6219e86ef74a41046d17fbb4f6ebfc8cf388

and

>  https://github.com/vicamo/b2g_platform_external_qemu/commit/08177526b4376a8b10c37ac44eb3b50435a55a8e

Comment 53

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Comment on attachment 732656 [details]
Github PR for platform/prebuilts/qemu-kernel

Depends on attachment 732655 [details].

Comment 54

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Thomas Zimmermann [:tzimmermann] from comment #52)
> Comment on attachment 732234 [details]
> Github PR for platform/external/qemu

Merged on GitHub https://github.com/mozilla-b2g/platform_external_qemu/commit/64ed2dc60e08783c500b69d8d80a659cc58cad29

Comment 55

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #38)
> Created attachment 732656 [details]
> Github PR for platform/prebuilts/qemu-kernel

Merged on GitHub https://github.com/mozilla-b2g/platform_prebuilts_qemu-kernel/commit/1c1aedebe8b5c854df5fa3bd3690b67859e44388

Comment 56

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #46)
> Comment on attachment 732655 [details]
> Github PR for kernel/goldfish

Merged on GitHub, branch b2g-goldfish-2.6.29 (not part of emulator build repos) https://github.com/mozilla-b2g/kernel_goldfish/commit/5bfbccc708adbd98faae978aed59b5b15351c343

Comment 57

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 732252 [details]
Github PR for manifest

Hi, all necessary parts are now merged, but still need this manifest change to take effects.  Without this manifest, only emulator parts are built.  You should already find an additional goldfish_tty device and a new goldfish_rfkill device.

To land this change, do you think we should have device/generic/goldfish and prebuilts/qemu-kernel forked on git.mozilla.org instead?  Or do you have any other comments?  Thank you.

Comment 58

[Jonathan Griffin (:jgriffin)]

Comment on attachment 732252 [details]
Github PR for manifest

I think it's OK as-is.

Comment 59

[Vicamo Yang [:vicamo][:vyang]]

Merged on GitHub https://github.com/mozilla-b2g/b2g-manifest/commit/abcffdac12c1c2c79e13f76405c47887308b3942

Comment 60

[Vicamo Yang [:vicamo][:vyang]]

All PRs are merged.

Comment 61

[Aki Sasaki (not active)]

This appears to have busted builds, since the manifest repos may not be synced to git.m.o yet.

Comment 62

[John Ford [:jhford] CET/CEST Berlin Time]

The manifest change was backed out because the vcs syncing tool is not currently able to handle new repositories being added to the manifest without releng/it setup.

There is nothing that I know of that points to this code being a problem or broken, this is an infrastructural need to back out.

Backout commit: aa03bc4a8c53d4a11c2a1b460a69afdccba64d10

Comment 63

[John Ford [:jhford] CET/CEST Berlin Time]

Since 877290 has been resolved, I've relanded the b2g-manifest change as cb7f3fe24e4093aec411cb3b1fc8a05fe5f7004b

Comment 64

[Vicamo Yang [:vicamo][:vyang]]

Manifest has been re-landed.