804436 - [b2g-bluetooth] Failed to connect from a headset again after turn off and turn on the headset

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

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

When we connect with a bluetooth headset, Sco will be created while there's a phone call, and user can initiate audio transfer to close Sco socket. At this time, the audio route should be set back to headphone.

However, during the phone call, if user want to make the audio transfer again (which means set audio route to back Sco), we don't have listen socket and we can't accomplish the request.

Repro:
- Connect with a bluetooth headset
- Turn off the headset (and the socket will be closed)
- Turn on the headset again (some devices will automatically try to connect with the device connected with it last time, pressing button will trigger this, too)

Expected Behaviour:
- Have a socket and listen for connection requests
- The headset successfully connects to a B2G phone

Actual Behaviour:
- No socket listen for connection requests
- Failed to connect from a headset again, but we can connect with it from our B2G

Comment 1

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

Attachment: Patch 1(v1): Call Listen() in OnDisconnect()

SocketType::LISTENING means having a server socket and waiting for connection request from headsets.

SocketType::CONNECTING means having a connection with remotes. When it is disconnected, we should notify settings app and restart listening socket.

Comment 2

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

Comment on attachment 674574 [details] [diff] [review]
Patch 1(v1): Call Listen() in OnDisconnect()

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

Ok, I get what you were looking for now, and I don't think this version of it is correct. I just need to add a SOCKET_LISTENING status to SocketConnectionTypes, and then you can store off the previous socket connection type locally. There's no reason for you to make your own enum when you're provided one from UnixSocket. I'll add my patch for UnixSocket here, then you can update to just store off SocketConnectionType.

::: dom/bluetooth/BluetoothHfpManager.cpp
@@ +85,5 @@
> +enum SocketType {
> +  UNKNOWN = 0,
> +  CONNECTING = 1,
> +  LISTENING = 2
> +};

Unneeded, I'll just add SOCKET_LISTENING to SocketConnectionStatus

@@ +226,5 @@
>  
>  BluetoothHfpManager::BluetoothHfpManager()
>    : mCurrentVgs(-1)
>    , mCurrentCallIndex(0)
> +  , mSocketType(SocketType::UNKNOWN)

Unneeded, mConnectionStatus handles this.

::: dom/bluetooth/BluetoothHfpManager.h
@@ +52,5 @@
>  
>    int mCurrentVgs;
>    int mCurrentCallIndex;
>    int mCurrentCallState;
> +  int mSocketType;

Just use SocketConnectionType. And give this a better name, we already have a socket type for bluetooth (RFCOMM/L2CAP/etc...), so this is confusing. After I get Listening status in, we'll want this to be a store of the PREVIOUS state, since we'll be checking it in the callbacks, which fire after the internals of UnixSocket change.

Comment 3

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

Attachment: Patch 0 (v1) - Add listening status to UnixSocket

Comment 4

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

Comment on attachment 674907 [details] [diff] [review]
Patch 0 (v1) - Add listening status to UnixSocket

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

Looks good. I can also simplify my implementation in BluetoothHfpManager.

Comment 5

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

Attachment: Patch 1(v2): Call Listen() in OnDisconnect()

Comment 6

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

Comment on attachment 674972 [details] [diff] [review]
Patch 1(v2): Call Listen() in OnDisconnect()

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

r=me with nits picked.

::: dom/bluetooth/BluetoothHfpManager.h
@@ +52,5 @@
>  
>    int mCurrentVgs;
>    int mCurrentCallIndex;
>    int mCurrentCallState;
> +  int mSocketStatus;

Nit: That's a SocketConnectionStatus, not an int. :)

Comment 7

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

Attachment: Patch 1: Add listening status to UnixSocket, r=gyeh

Comment 8

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

Attachment: Patch 2: Failed to connect from a headset again after turn off and turn on the headset, r=qdot

Comment 9

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

Attachment: Patch 2: Failed to connect from a headset again after turn off and turn on the headset, r=qdot

Comment 10

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

try server: 
https://tbpl.mozilla.org/?tree=Try&rev=10dfe59dc433
https://tbpl.mozilla.org/?tree=Try&rev=6b22103a1603

Comment 11

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/56209d235ff5
https://hg.mozilla.org/integration/mozilla-inbound/rev/3bd24121c80d

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/56209d235ff5
https://hg.mozilla.org/mozilla-central/rev/3bd24121c80d

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/5c200eb5ddf7
https://hg.mozilla.org/releases/mozilla-aurora/rev/b471cb0a5638