1154136 - [NFC][Bluetooth] NFC headset auto connect always failed when user manually disconnect already connected NFC headset

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Bluetooth, defect)

Description

[Alison Shiue]

+++ This bug was initially created as a clone of Bug #1146744 +++

Tried with following build, and found that NFC headset still cannot auto connect when user manually disconnect already connected it.

Build ID               20150412162502
Gaia Revision          cec00d643f517ffd96cde559cd3bbd43ab85816c
Gaia Date              2015-04-10 21:41:12
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/5005522fd68e
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150412.200029
Firmware Date          Sun Apr 12 20:00:40 EDT 2015
Bootloader             L1TC000118D0

Comment 1

[Jamin Liu [:jaliu][:Jamin]]

The patch in Bug 1146744 use the attribute 'BluetoothDevice.connected' as the if-condition to determine whether to connect again. https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/nfc_handover_manager.js#L643 However, the value of 'BluetoothDevice.connected' isn't correct with BlueDroid stack. Therefore, gaia wouldn't trigger the connection process since it think its already connect.

Comment 2

[Jamin Liu [:jaliu][:Jamin]]

Attachment: Update Bluetooth connection state when BT stack updates device properties. (v1)

Comment 3

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

Comment on attachment 8592099 [details] [diff] [review]
Update Bluetooth connection state when BT stack updates device properties. (v1)

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

LGTM. I think we shall notice gaia guys that V2 API BluetoothDevice.IsConnected no longer been used?

::: dom/bluetooth/bluedroid/BluetoothServiceBluedroid.cpp
@@ +2010,5 @@
> +      return true;
> +    }
> +  }
> +
> +  // P.S. HID profile isn't support by BT API v1

I think you don't need to add this comment. I shall add it when we implement new profile.

@@ +2526,5 @@
>  
> +  // BlueDroid wouldn't notify the status of connection, query the connection
> +  // state and append to properties array
> +  bool isConnected = IsConnected(aBdAddr);
> +  BT_APPEND_NAMED_VALUE(props, "Connected", isConnected);

I think you can call IsConnected(aBdAddr) directly.

Comment 4

[Jamin Liu [:jaliu][:Jamin]]

Attachment: Update Bluetooth connection state when BT stack updates device properties. (v2), r=shuang

Thank Shawn for reviewing the patch.

Comment 5

[Jamin Liu [:jaliu][:Jamin]]

(In reply to Shawn Huang [:shawnjohnjr] from comment #3)
> Comment on attachment 8592099 [details] [diff] [review]
> LGTM. I think we shall notice gaia guys that V2 API
> BluetoothDevice.IsConnected no longer been used?
Sure, I will leave a comment on Bug 1090799.

> ::: dom/bluetooth/bluedroid/BluetoothServiceBluedroid.cpp
> I think you don't need to add this comment.

> @@ +2526,5 @@
> I think you can call IsConnected(aBdAddr) directly.
Got it. :)

Comment 6

[Jamin Liu [:jaliu][:Jamin]]

Treeherder:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c9a8fb1fff56

Comment 7

[Jamin Liu [:jaliu][:Jamin]]

https://hg.mozilla.org/integration/b2g-inbound/rev/8aaa0ee014a4

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8aaa0ee014a4

Comment 9

[Alison Shiue]

Verified on 3.0
Build ID               20150415010203
Gaia Revision          8e28588496f82f8f069c171c65842d622b9d8d7d
Gaia Date              2015-04-14 18:43:50
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/de27ac2ab94f
Gecko Version          40.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150415.044739
Firmware Date          Wed Apr 15 04:47:48 EDT 2015
Bootloader             L1TC000118D0

Hi Jamin, could you please uplift this patch to v2.2? Thanks!

Comment 10

[Jamin Liu [:jaliu][:Jamin]]

Attachment: (for 2.2) Update Bluetooth connection state when BT stack updates device properties. (v2), r=shuang

Comment 11

[Aaron Wu]

triage: move to 2.2+

Comment 12

[Jamin Liu [:jaliu][:Jamin]]

Comment on attachment 8592736 [details] [diff] [review]
(for 2.2) Update Bluetooth connection state when BT stack updates device properties. (v2), r=shuang

[Approval Request Comment]
> Bug caused by (feature/regressing bug #): 
Bug 1146744

> User impact if declined: 
User can't use NFC to connect a Bluetooth headset if it's been manually disconnected.

> Testing completed: 
Manually test on v2.2 and master branch by following the STR.

> Risk to taking this patch (and alternatives if risky): 
Currently, Bluetooth component supports HFP/HSP, A2DP/AVRCP and OPP profiles. 
When Gecko support a new profile in future, the developer must remember to check the connection state of that profile.

> String or UUID changes made by this patch:
no

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/f928e50b4601

Comment 14

[Alison Shiue]

Verified on 2.2 

Build ID               20150419162502
Gaia Revision          c15a2b6d3a783813959c2b3bffd2a131f4270b9e
Gaia Date              2015-04-17 17:49:32
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/cc02ee38b252
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150419.201820
Firmware Date          Sun Apr 19 20:18:31 EDT 2015
Bootloader             L1TC100118D0

Comment 15

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

Thanks Alison and Jamin.