[b2g-bluetooth] Readd reserved service reg/unreg for bluetooth

RESOLVED FIXED in mozilla18

Status

()

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

People

(Reporter: qdot, Assigned: ericchou)

Tracking

Trunk
mozilla18
x86_64
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking-basecamp:+)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Reserved service registration was added as part of bug 790136, patch 3. Unfortunately, it fails in both directions.

- Registration of reserved services happens on GetDefaultAdapter. However, we need to register services the second we complete a successful enabling, not whenever a user asks for the adapter for the first time. Otherwise outside devices will never be able to connect until we bring up an adapter.

- Unregistration of reserved services happens on disabling, but depends on a sDefaultAdapterPath variable, which is only set during enabling /when the device has not been already enabled/. This means that is the b2g process cycles and we enable, we never set this path, and we crash (one of the causes of bug 791189).

For registering, we need two cases:
- If firmware isn't up yet, register services when AdapterAdded is called
- If firmware is up already, get the default adapter (We'll just assume its existence?) and register services

For unregistering, we should be able to work the same way we were since we'll have gotten the adapter one way or another while registering with this new solution.
Another, possibly MUCH simpler idea: If we try to enable and bluetooth is already up, we disable then continue with enabling. This means we fire all the signals we expect, and it'll also do things like bouncing any live connections we might have left over (assuming that's even a problem with bluetooth).
(Assignee)

Comment 2

6 years ago
Created attachment 661561 [details] [diff] [review]
v1: Poll for adapter path

The reason why I called function AddReservedServices() when AdapterAdded is received is because some actions, such as RegisterAgent() and 
AddReservedServices(), need adapter path. Instead of waiting for AdapterAdded event, I suggest that we poll for it.

Reference: http://androidxref.com/4.0.4/xref/frameworks/base/core/jni/android_server_BluetoothEventLoop.cpp#314
Attachment #661561 - Flags: feedback?(kyle)
(Assignee)

Comment 3

6 years ago
Created attachment 661583 [details] [diff] [review]
v2: Revision of register/unregister services, followed comment 1
Attachment #661561 - Attachment is obsolete: true
Attachment #661561 - Flags: feedback?(kyle)
Attachment #661583 - Flags: review?(kyle)
Comment on attachment 661583 [details] [diff] [review]
v2: Revision of register/unregister services, followed comment 1

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

This still has the same problem as before: When we bring up an already enabled adapter, we only register services the first time it's retrieved. That's why I said if we run StartInternal and bluetoothd is already running, we should run StopInternal then restart everything.
Attachment #661583 - Flags: review?(kyle) → review-
(Assignee)

Comment 5

6 years ago
A problem is that when code is running into StartInternal(), bluetoothd should be already running.

For BluetoothGonkService, we call StartStopGonkBluetooth(true) before calling BluetoothDBusService::StartInternal(), which means IsEnabled() should always return true in StartInternal(). If it's the case, when should I check if bluetoothd is running and restart?

I think we could only register services when we get AdapterAdded event. If bluetoothd has been already up, it means those services should have been registered last time received AdapterAdded, and also means that they would not have been cleared. 

However, even if we do so, the static variable sDefaultAdapterPath & sServiceHandles will still be empty. That will be a problem for unregistering. We can check if these two variables are empty or not, or just turn Bluetooth off without unregistration would be fine for now.
(Assignee)

Comment 6

6 years ago
Created attachment 661674 [details] [diff] [review]
v3: Register services when get AdapterAdded event, but not unregister.

Please see comment above.
Attachment #661583 - Attachment is obsolete: true
Attachment #661674 - Flags: review?(kyle)
Comment on attachment 661674 [details] [diff] [review]
v3: Register services when get AdapterAdded event, but not unregister.

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

Ok, if unregistering isn't that big of a deal, then this works I suppose. :)
Attachment #661674 - Flags: review?(kyle) → review+
blocking-basecamp: ? → +

Comment 9

6 years ago
https://hg.mozilla.org/mozilla-central/rev/0394f4ace522
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in before you can comment on or make changes to this bug.