857896 - [Leo] [Bluetooth] Convert device property "Connected" into a boolean if bluez returns a two-byte array

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Bluetooth, defect)

Description

[Tony Chung [:tchung]]

Leo is unsuccessful when pairing BT with a laptop.   Laptop says can't pair, and then Device will crap out and turn white screen.

Logcat:
04-03 20:44:09.204: I/qcom-bluetooth(3403): /system/etc/init.qcom.bt.sh: Transport :  
04-03 20:44:09.214: I/qcom-bluetooth(3404): /system/etc/init.qcom.bt.sh: Power Class: Ignored. Default(1) used (1-CLASS1/2-CLASS2/3-CUSTOM) 
04-03 20:44:09.214: I/qcom-bluetooth(3405): /system/etc/init.qcom.bt.sh: Power Class: To override, Before turning BT ON; setprop qcom.bt.dev_power_class <1 or 2 or 3> 
04-03 20:44:09.224: I/qcom-bluetooth(3406): /system/etc/init.qcom.bt.sh: LE Power Class: Ignored. Default(2) used (1-CLASS1/2-CLASS2/3-CUSTOM) 
04-03 20:44:09.224: I/qcom-bluetooth(3407): /system/etc/init.qcom.bt.sh: LE Power Class: To override, Before turning BT ON; setprop qcom.bt.le_dev_pwr_class <1 or 2 or 3> 
04-03 20:44:10.554: I/qcom-bluetooth(3412): /system/etc/init.qcom.bt.sh: Bluetooth QSoC firmware download succeeded, /dev/ttyHS0 qualcomm-ibs 3000000 00:00:00:99:81:24 
04-03 20:44:10.554: I/qcom-bluetooth(3413): /system/etc/init.qcom.bt.sh: start hciattach 
04-03 20:44:10.564: I/qcom-bluetooth(3415): /system/etc/init.qcom.bt.sh: start_hciattach: pid = 3414 
04-03 20:44:10.764: I/Gonk(133): log_and_free_dbus_error: D-Bus error: org.bluez.Error.NoSuchAdapter (No such adapter)
04-03 20:44:10.934: I/qcom-bt-wlan-coex(3432): /system/etc/init.qcom.coex.sh: btwlancoex/abtfilt not available 

Repro:
1) install nightly build on Leo device.  Using Moz RIL.
Gecko  http://hg.mozilla.org/releases/mozilla-b2g18/rev/d467369d1b0c
Gaia   06e0e5ce42bdfb62bdbe38271de6b5b2d9e40e75
BuildID 20130403070204
Version 18.0
2) settings > enable bluetooth.  make sure you pair a laptop with BT on
3) find the BT, and hit Pair
4) Verify BT pairing error, and device just turns whitescreen.

Expected:
- BT pairs to laptop

Actual:
- BT fails to pair, device goes whitescreen.

Comment 1

[Alex Keybl [:akeybl]]

Possibly related to bug 838421?

Comment 2

[Gina Yeh [:gyeh] [:ginayeh]]

Hi Tony, I'd like to know more details and need you help to get hcidump, thanks.

Here are some information for getting hcidump:
https://wiki.mozilla.org/B2G/Bluetooth#Helpful_Debugging_Information

Please attach the full hcidump if you can reproduce it. Besides, I'd like to know whether it is always reproducible for you? If yes, please also provide some information about the PC. Thanks. I'll take some time on this.

Comment 3

[Tony Chung [:tchung]]

Yep, 100% reproducible.

Attempting to pair to: Mac book Air, OSX 10.8.2, 1.8 Ghz Intel i7, 4Gb Mem.

logcat snippet again:
04-08 11:13:30.960: I/qcom-bluetooth(9852): /system/etc/init.qcom.bt.sh: Transport :  
04-08 11:13:30.970: I/qcom-bluetooth(9853): /system/etc/init.qcom.bt.sh: Power Class: Ignored. Default(1) used (1-CLASS1/2-CLASS2/3-CUSTOM) 
04-08 11:13:30.970: I/qcom-bluetooth(9854): /system/etc/init.qcom.bt.sh: Power Class: To override, Before turning BT ON; setprop qcom.bt.dev_power_class <1 or 2 or 3> 
04-08 11:13:30.980: I/qcom-bluetooth(9855): /system/etc/init.qcom.bt.sh: LE Power Class: Ignored. Default(2) used (1-CLASS1/2-CLASS2/3-CUSTOM) 
04-08 11:13:30.980: I/qcom-bluetooth(9856): /system/etc/init.qcom.bt.sh: LE Power Class: To override, Before turning BT ON; setprop qcom.bt.le_dev_pwr_class <1 or 2 or 3> 
04-08 11:13:32.250: I/Diag_Lib(137): rpc_handle_rpc_call: for Xid: 5065, Prog: 31000000, Vers: d17ed9ea, Proc: 00000014
04-08 11:13:32.250: D/QCRIL_RPC(137): Enter qcril_cm_srvsys_event_callback
04-08 11:13:32.250: D/QCRIL_RPC(137): Exit qcril_cm_srvsys_event_callback
04-08 11:13:32.250: I/Diag_Lib(137): rpc_handle_rpc_call: Find Status: 0 Xid: 5065
04-08 11:13:32.250: I/ONCRPC(137): oncrpc_proxy_handle_cmd_rpc_call: Dispatching xid: 5065
04-08 11:13:32.250: I/ONCRPC(137): oncrpc_xdr_reply_msg_start: Prog: 00000000, Ver: 00000000, Proc: 00000000 Xid: 00005065
04-08 11:13:32.250: I/ONCRPC(137): oncrpc_msg_reply: Prog: 00000000, Ver: 00000000, Proc: 00000000 Xid: 00005065
04-08 11:13:32.250: I/ONCRPC(137): oncrpc_proxy_handle_cmd_rpc_call: Dispatch returned for xid: 5065
04-08 11:13:32.300: I/qcom-bluetooth(9860): /system/etc/init.qcom.bt.sh: Bluetooth QSoC firmware download succeeded, /dev/ttyHS0 qualcomm-ibs 3000000 00:00:00:99:81:24 
04-08 11:13:32.320: I/qcom-bluetooth(9861): /system/etc/init.qcom.bt.sh: start hciattach 
04-08 11:13:32.340: I/qcom-bluetooth(9863): /system/etc/init.qcom.bt.sh: start_hciattach: pid = 9862 
04-08 11:13:32.450: I/Gonk(133): log_and_free_dbus_error: D-Bus error: org.freedesktop.DBus.Error.ServiceUnknown (The name org.bluez was not provided by any .service files)
04-08 11:13:32.560: I/qcom-bt-wlan-coex(9874): /system/etc/init.qcom.coex.sh: btwlancoex/abtfilt not available 

HCI Dump output:
/system/bin/sh: hcidump: not found

I'll look into this further, and attach the hcidump file.

Comment 4

[Gina Yeh [:gyeh] [:ginayeh]]

Thanks, Tony. I've got a leo device, and I'll take a look at this issue soon.

Comment 5

[Gina Yeh [:gyeh] [:ginayeh]]

Here are what I got from logcat:

> I/GonkDBus(  133): [B] GetProperty
> I/Gecko   (  133): [Parent 133] WARNING: Iterator not type we expect!: file /mozilla-central/dom/bluetooth/linux/BluetoothDBusService.cpp, line 1093
> I/Gecko   (  133): [Parent 133] WARNING: Property Name: ;Connected Property Type Expected: ;b Property Type Received: a: file /mozilla-central/dom/bluetooth/linux/BluetoothDBusService.cpp, line 1101
> I/Gecko   (  133): [Parent 133] WARNING: Can't get property!: file /mozilla-central/dom/bluetooth/linux/BluetoothDBusService.cpp, line 1319
> I/Gecko   (  133): [Parent 133] ###!!! ABORT: unexpected type tag: '(mType) == (aType)', file ../../ipc/ipdl/_ipdlheaders/mozilla/dom/bluetooth/BluetoothTypes.h, line 141
> E/Gecko   (  133): mozalloc_abort: [Parent 133] ###!!! ABORT: unexpected type tag: '(mType) == (aType)', file ../../ipc/ipdl/_ipdlheaders/mozilla/dom/bluetooth/BluetoothTypes.h, line 141
> F/libc    (  133): Fatal signal 11 (SIGSEGV) at 0x00000000 (code=1)
> I/DEBUG   (  136): debuggerd committing suicide to free the zombie!
> I/Gecko   (  437): [Child 437] WARNING: shutting down early because of crash!: file /home/gina/workspace/mozilla-central/dom/ipc/ContentChild.cpp, line 981
> I/Gecko   (  427): [Child 427] WARNING: shutting down early because of crash!: file /home/gina/workspace/mozilla-central/dom/ipc/ContentChild.cpp, line 981
> I/Gecko   (  525): [Child 525] WARNING: pipe error (17): Connection reset by peer: file /home/gina/workspace/mozilla-central/ipc/chromium/src/chrome/common/ipc_channel_posix.cc, line 432
> I/Gecko   (  525): 
> I/Gecko   (  525): ###!!! [Child][SyncChannel] Error: Channel error: cannot send/recv
> I/Gecko   (  525): 
> I/Gecko   (  525): [Child 525] ###!!! ABORT: unexpected type tag: '(mType) == (aType)', file ../../ipc/ipdl/_ipdlheaders/mozilla/layers/LayersSurfaces.h, line 83
> E/Gecko   (  525): mozalloc_abort: [Child 525] ###!!! ABORT: unexpected type tag: '(mType) == (aType)', file ../../ipc/ipdl/_ipdlheaders/mozilla/layers/LayersSurfaces.h, line 83
> F/libc    (  525): Fatal signal 11 (SIGSEGV) at 0x00000000 (code=1)
> I/Gecko   (  427): [Child 427] WARNING: content process _exit()ing: file /home/gina/workspace/mozilla-central/dom/ipc/ContentChild.cpp, line 1030
> I/Gecko   (  437): [Child 437] WARNING: content process _exit()ing: file /home/gina/workspace/mozilla-central/dom/ipc/ContentChild.cpp, line 1030

Comment 6

[Gina Yeh [:gyeh] [:ginayeh]]

Ok, here's the problem! When we get device property of 'Connected', we will check a system property named 'org.bluez.device.conn.type' first and then decide whether to replace the device property type.

For the case of leo device, the device property type is a 2-byte array. But we failed to recognize that and replace them with a boolean value because the system property is not set corrected. I'm thinking that, instead of relying on the system property to verify the type of device property, maybe we can check dbus message arg type directly. Kyle, does it make sense to you? If yes, I will generate a patch and send review request to you.

Comment 7

[Gina Yeh [:gyeh] [:ginayeh]]

BTW, we don't have this issue on unagi and otoro. Since we have different bluez revisions between unagi/otoro, and leo. For unagi and otoro, the value type of device property 'Connected' is boolean, rather than a 2-byte array.

Comment 8

[Tony Chung [:tchung]]

(In reply to Gina Yeh [:gyeh] [:ginayeh] from comment #7)
> BTW, we don't have this issue on unagi and otoro. Since we have different
> bluez revisions between unagi/otoro, and leo. For unagi and otoro, the value
> type of device property 'Connected' is boolean, rather than a 2-byte array.

Thanks for reproducing this.   look forward to a fix soon.

Comment 9

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

Oh god, not this again. :( This was originally covered in bug 802428, and I thought we were already doing property type checking based on that? Are we using going across strawberry/chocolate kernels again?

Comment 11

[Gina Yeh [:gyeh] [:ginayeh]]

Here's the commit information from git://codeaurora.org/platform/external/bluetooth/bluez.

commit 7dd3e509bfc2864fab1c36035e3e2e2a979a168b
Author: Archana Ramachandran <[e-mail]>
Date:   Tue Aug 14 14:18:31 2012 -0700

    Send disconnection reason in the property change dbus signal
    
    Added device disconnection reason in the "connected" device
    property and emit property change signal with reason parameter
    on receiving the device disconnected management event.
    
    CRs-fixed: 378240
    (cherry picked from commit 53c9c3501300fe35d0f42476e181b1397236333c)







As description in commit message, device property of "connected" is changed into a 2-byte array. The first byte is the value of property; the second byte is used for storing disconnection reason.

Comment 12

[Gina Yeh [:gyeh] [:ginayeh]]

Attachment: Patch 1(v1): Convert 2-byte array into a boolean value for device property "Connected"

Please help to review this patch.

Comment 13

[Robert Kaiser]

If that one is a dupe, let's add the crash identifiers from bug 859034 here as well.

Comment 14

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

Comment on attachment 736746 [details] [diff] [review]
Patch 1(v1): Convert 2-byte array into a boolean value for device property "Connected"

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

r=me with nits addressed. Even though I'm still sad we have to do this. :(

::: dom/bluetooth/linux/BluetoothDBusService.cpp
@@ +1085,5 @@
> +   * Bug 857896. Since device property "Connected" could be a boolean value or
> +   * an 2-byte array, we need to check the value type here and convert the
> +   * first byte into a boolean manually.
> +   */
> +  bool convert = ShouldConvertConnectedType(propertyName, receivedType);

Why make this its own function? Are we calling it anywhere else?

Comment 15

[Eric Chou [:ericchou] [:echou]]

Comment on attachment 736746 [details] [diff] [review]
Patch 1(v1): Convert 2-byte array into a boolean value for device property "Connected"

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

r+ with nits addressed.

::: dom/bluetooth/linux/BluetoothDBusService.cpp
@@ +1092,2 @@
>      NS_WARNING("Iterator not type we expect!");
>      nsAutoCString str;

nit: str would contain more than 64 code units, therefore using nsCString could save one time memory allocation on heap.

@@ +1159,5 @@
> +    MOZ_ASSERT(propertyValue.type() == BluetoothValue::TArrayOfuint8_t);
> +
> +    bool b = propertyValue.get_ArrayOfuint8_t()[0];
> +    BluetoothValue newValue(b);
> +    propertyValue = newValue;

nit: newValue is not neccesary. You can directly write |propertyValue = BluetoothValue(b);|

Comment 16

[Gina Yeh [:gyeh] [:ginayeh]]

Attachment: Final patch, r=echou,qdot

Will push to try server and then birch branch later

Comment 17

[Gina Yeh [:gyeh] [:ginayeh]]

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

Comment 18

[Gina Yeh [:gyeh] [:ginayeh]]

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

Comment 19

[Gina Yeh [:gyeh] [:ginayeh]]

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

Comment 20

[Gina Yeh [:gyeh] [:ginayeh]]

Pushing to birch:
https://hg.mozilla.org/projects/birch/rev/7948de900426

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7948de900426

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/2cc6a1c89a97

Comment 24

[Jason Smith [:jsmith]]

https://bugzilla.mozilla.org/show_bug.cgi?id=859034#c8 states to tef? if this is reproducible on 1.01, but the dupe just found on bug 877425 confirms this be reproducible on 1.01 as an OS restart crash.

Comment 25

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/267e96f9e90c

Comment 26

[Chris Kreinbring [:CKreinbring]]

Tested on the Buri 1.0.1 commercial RIL, and was able to connect via Bluetooth successfully.