Closed Bug 945149 Opened 6 years ago Closed 6 years ago

[fugu] No event AdapterAdded is received after turning on Bluetooth

Categories

(Firefox OS Graveyard :: Bluetooth, defect)

defect
Not set

Tracking

(blocking-b2g:fugu+)

RESOLVED FIXED
1.3 Sprint 6 - 12/6
blocking-b2g fugu+

People

(Reporter: xinhe.yan, Assigned: gyeh)

References

Details

(Whiteboard: [Fugu] [v1.2f-uplift-needed])

Attachments

(2 files, 2 obsolete files)

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/535.19 (KHTML, like Gecko) Ubuntu/11.04 Chromium/18.0.1025.151 Chrome/18.0.1025.151 Safari/535.19
Hi Gina,

After Bug 932914 code merged, I found fugu bluetooth can not work.

Now adapter only be add when receive PropertyChanged signal(Property Name).
In fugu, there is no PropertyChanged signal for Property Name. So there is no channce to add adater.

I guess when AdapterAdded is received, Query all properties of the adapter, adapter Property Name is not null.

Maybe the correct flow is add adapter when receive AdapterAdded, and update adapter name when got PropertyChanged signal(Property Name).
Or check adatpter name when receive AdapterAdded. If null, hold; if not null, add adpter.
Thanks for your reporting. We'll figure out a solution soon. At the same time, I'd like to know more details. 

Without the patch of Bug 932914:
1. Enable Bluetooth
2. Event 'AdapterAdded' is received
3. Get an instance of BluetoothAdapter by calling getDefaultAdapter()
4. Check the adapter name

For device fugu with your blueZ stack, I'd like to know whether the adapter name is null or not. This information would be helpful for us. Thank you in advance.
Flags: needinfo?(xinhe.yan)
the adapter name is not null.

E/GeckoBluetooth(  738): [M] Notify: AdapterAdded
E/GeckoBlue(  738): BluetoothManager::GetDefaultAdapter
E/GeckoBlue(  738): linux/BluetoothDBusService.cpp DBUS_ADAPTER_IFACE PropertyChanged name Powered
E/GeckoBlue(  738): linux/BluetoothDBusService.cpp DBUS_ADAPTER_IFACE PropertyChanged name Pairable
E/GeckoBlue(  738): linux/BluetoothDBusService.cpp DBUS_ADAPTER_IFACE PropertyChanged name UUIDs

E/GeckoBlue(  738): GetAdapterTask ParseSuccessfulReply
E/GeckoBlue(  738): BluetoothAdapter::SetPropertyByValue name SP7710

E/GeckoBlue(  738): linux/BluetoothDBusService.cpp DBUS_ADAPTER_IFACE PropertyChanged name Class
Flags: needinfo?(xinhe.yan)
blocking-b2g: --- → fugu?
Thanks, Xinhe. The patch is almost ready to review.
Summary: [fugu]BlutoothAdapter can not be add because no PropertyChanged signal for Property Name → [fugu] No event AdapterAdded is received after turning on Bluetooth
Attached patch Patch 1(v1): Set timeout (obsolete) — Splinter Review
Assignee: nobody → gyeh
Attachment #8341490 - Flags: review?(echou)
Xinhe, can you help to apply this patch and verify? I'd like to know if the patch fix the problem. Thanks.
Flags: needinfo?(xinhe.yan)
I am compile code to verify this patch on master branch.
This bug is a block problem for fugu in v1.2 branch. Can you give a patch for v1.2 branch?
Flags: needinfo?(xinhe.yan)
Sure! Let me upload patch for v1.2 branch later.
Depends on: 942712
Note that this issue is a regression of bug 932914, but it cannot be reproducible on unagi/hamachi/leo...

It can only reproduce with device fugu since the behaviour of BlueZ is slightly different.
Duplicate of this bug: 945138
triage: fugu+ for impacting fugu
blocking-b2g: fugu? → fugu+
Attached patch [v1.2] Patch 1: Set Timeout (obsolete) — Splinter Review
inhe, please verify with this patch. Thanks.
Flags: needinfo?(xinhe.yan)
Comment on attachment 8341490 [details] [diff] [review]
Patch 1(v1): Set timeout

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

r=me with nits addressed.

Our code is getting complicated because of this kind of timing issue which happened while enabling Bluetooth. We should really start to consider refactoring BluetoothDBusService.cpp.

::: dom/bluetooth/bluez/linux/BluetoothDBusService.cpp
@@ +177,3 @@
>  static nsTArray<uint32_t> sAuthorizedServiceClass;
> +
> +// The object path of adpater which is invalid after switching Bluetooth.

The description seems a little misleading. sAdapterPath will have valid value again once it's been updated. Could you describe more?
Attachment #8341490 - Flags: review?(echou) → review+
Comment on attachment 8341596 [details] [diff] [review]
[v1.2] Patch 1: Set Timeout

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

dom/bluetooth/BluetoothService.cpp
@@ -533,10 +535,8 @@ BluetoothService::SetEnabled(bool aEnabled)
                 or the toggling is failed.");
   }   
 
   mEnabled = aEnabled;
-
-  gToggleInProgress = false;
 }

This will make bluetoothd can not start. After recover this line back, Bluetooth work fine.
Whiteboard: [dsds_US_test]
Blocks: 926342
Patch for master branch work good. Thanks.
Flags: needinfo?(xinhe.yan)
(In reply to Xinhe Yan from comment #15)
>    mEnabled = aEnabled;
> -
> -  gToggleInProgress = false;
>  }
> 
> This will make bluetoothd can not start. After recover this line back,
> Bluetooth work fine.

I think something important is missing in the patch. Please let me upload a new one for v1.2 branch. Thanks.
I've tested this patch with fugu. It works well. Please verify it again, thanks.
Attachment #8341596 - Attachment is obsolete: true
Flags: needinfo?(xinhe.yan)
(In reply to Gina Yeh [:gyeh] [:ginayeh] from comment #19)
> Created attachment 8342109 [details] [diff] [review]
> [v1.2] Set timeout, r=echou, a=fugu+
> 
> I've tested this patch with fugu. It works well. Please verify it again,
> thanks.

This patch work good. Thanks.
Flags: needinfo?(xinhe.yan)
Thanks for your help, Xinhe. The patches are ready. Let's wait for the Sheriff to merge them into both m-c and v1.2f.
https://hg.mozilla.org/mozilla-central/rev/263f538a5509
Status: UNCONFIRMED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.3 Sprint 6 - 12/6
Verified on master build. Passed.
Gaia      def4baa9ad44843ea214b6f94d04c29613cbfadb
Gecko     8c89455659d2ab56e9e56b7591f0982bed638e0e
BuildID   20131205103217
Version   28.0a1
Thanks, Enpei.
No longer blocks: 926342
Whiteboard: [dsds_US_test] → [dsds_US_test] [Fugu] [v1.2f-uplift-needed]
Whiteboard: [dsds_US_test] [Fugu] [v1.2f-uplift-needed] → [Fugu] [v1.2f-uplift-needed]
You need to log in before you can comment on or make changes to this bug.