Closed Bug 979160 Opened 6 years ago Closed 6 years ago

[bluedroid]Outgoing connection failure shall trigger OnConnect instead of OnDisconnect

Categories

(Firefox OS Graveyard :: Bluetooth, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: shawnjohnjr, Assigned: shawnjohnjr)

References

Details

Attachments

(1 file, 3 obsolete files)

I found that if the remote headset turns off, HFP profile cannot connect to the remote side, however OnDisconnect get called without any error string, which is not expected here. The original design that we shall expect OnConnect with error string returned. Somehow, we try to map bluedroid HFP state-change (from connecting to disconnected).
Since mSuccess = true, even if A2DP also fail to connect with the headset,
mSuccess will not be updated.
https://github.com/mozilla/gecko-dev/blob/master/dom/bluetooth/
BluetoothProfileController.cpp#L242
This leads gecko reply success to Gaia.
Summary: [bluedroid]Handle onConnect with Error if fail to connect HFP profile → [bluedroid]Handle onConnect with errror if fail to connect HFP profile
OS: Linux → Gonk (Firefox OS)
Hardware: x86_64 → ARM
Assignee: nobody → shuang
Summary: [bluedroid]Handle onConnect with errror if fail to connect HFP profile → [bluedroid]Outgoing connection failure shall trigger OnConnect instead of OnDisconnect
Attachment #8385974 - Attachment is obsolete: true
Attachment #8385974 - Flags: review?(echou)
Attachment #8385977 - Attachment is obsolete: true
Attachment #8385977 - Flags: review?(echou)
Comment on attachment 8386574 [details] [diff] [review]
Bug 979160 - Outgoing connection failure shall trigger OnConnect instead of OnDisconnect

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

r=me with nits addressed.

Just realized that some member variables may be accessed at different threads without protection. We may need file a bug for that but should not block this issue.

::: dom/bluetooth/bluedroid/hfp/BluetoothHfpManager.cpp
@@ +756,5 @@
>      } else if (mConnectionState == BTHF_CONNECTION_STATE_DISCONNECTED) {
>        mDeviceAddress.AssignLiteral(BLUETOOTH_ADDRESS_NONE);
> +      if (mPrevConnectionState == BTHF_CONNECTION_STATE_DISCONNECTED) {
> +        // Bug 979160: This implies the outgoing connection failure.
> +        // When the outoging hfp connection fails, state changes to disconnected

typo: outoging

@@ +757,5 @@
>        mDeviceAddress.AssignLiteral(BLUETOOTH_ADDRESS_NONE);
> +      if (mPrevConnectionState == BTHF_CONNECTION_STATE_DISCONNECTED) {
> +        // Bug 979160: This implies the outgoing connection failure.
> +        // When the outoging hfp connection fails, state changes to disconnected
> +        // state. Since bluedroid would not report connecting state, but Only

Only => only

@@ +759,5 @@
> +        // Bug 979160: This implies the outgoing connection failure.
> +        // When the outoging hfp connection fails, state changes to disconnected
> +        // state. Since bluedroid would not report connecting state, but Only
> +        // report connected/disconnected.
> +        BT_WARNING("Outgoing HFP connection failure");

In this case a warning message will be seen when you try to connect to a bt-disabled device, and I think it may be too harsh.
Attachment #8386574 - Flags: review?(echou) → review+
https://hg.mozilla.org/mozilla-central/rev/09ad3e796ebf
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.