1050126 - [Bluetooth] Port bug 1038591 to bluetooth2

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Bluetooth, defect)

Comment 1

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Attachment: [01] Bug 1050126: Convert Bluetooth data types in |BluetoothInterface| (under bluetooth2/)

Comment 2

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Attachment: [02] Bug 1050126: Convert Bluetooth Socket data types in |BluetoothSocketInterface| (under bluetooth2/)

Comment 3

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Attachment: [03] Bug 1050126: Convert Bluetooth Handsfree data types in |BluetoothHandsfreeInterface| (under bluetooth2/)

Comment 4

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Attachment: [04] Bug 1050126: Convert Bluetooth A2DP data types in |BluetoothA2dpInterface| (under bluetooth2/)

Comment 5

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Attachment: [05] Bug 1050126: Convert Bluetooth AVRCP data types in |BluetoothAvrcpInterface| (under bluetooth2/)

Comment 6

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Attachment: [06] Bug 1050126: Convert Bluedroid status codes and error handlers (under bluetooth2/)

Comment 7

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Hi Ben,

These patches are the same as for bug 1038591 plus the build fix from bug 1050299.

Comment 8

[Ben Tian (inactive)]

Comment on attachment 8472919 [details] [diff] [review]
[01] Bug 1050126: Convert Bluetooth data types in |BluetoothInterface| (under bluetooth2/)

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

LGTM

Comment 9

[Ben Tian (inactive)]

Comment on attachment 8472920 [details] [diff] [review]
[02] Bug 1050126: Convert Bluetooth Socket data types in |BluetoothSocketInterface| (under bluetooth2/)

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

LGTM

Comment 10

[Ben Tian (inactive)]

Comment on attachment 8472921 [details] [diff] [review]
[03] Bug 1050126: Convert Bluetooth Handsfree data types in |BluetoothHandsfreeInterface| (under bluetooth2/)

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

r=me with comment addressed.

Also please have a followup bug (as mentioned in bug 1038591 comment 13) to integrate |BluetoothHfpManager::ConvertToBluetoothHandsfreeCallState| into |BluetoothInterface::Convert|. The reason is that call state transition has become: telephony call state -> BluetoothHandsfreeCallState -> bthf_call_state_t. I think we can leverage telephony call state in Handsfree manager and do telephony call state -> bthf_call_state_t transition only.

::: dom/bluetooth2/bluedroid/hfp/BluetoothHfpManager.cpp
@@ +1087,5 @@
>  
>    int service = (regState.EqualsLiteral("registered")) ? 1 : 0;
>    if (service != mService) {
>      // Notify BluetoothRilListener of service change
>      mListener->ServiceChanged(aClientId, service);

Declare |service| of type |BluetoothHandsfreeNetworkState| as well since we compare |service| and |mService|.

  BluetoothHandsfreeNetworkState service =
    regState.EqualsLiteral("registered") ? HFP_NETWORK_STATE_AVAILABLE :
                                           HFP_NETWORK_STATE_NOT_AVAILABLE;
  if (service != mService) {
    // Notify BluetoothRilListener of service change
    mListener->ServiceChanged(aClientId,
                              (service == HFP_NETWORK_STATE_AVAILABLE));
  }
  mService = service;

Comment 11

[Ben Tian (inactive)]

Comment on attachment 8472922 [details] [diff] [review]
[04] Bug 1050126: Convert Bluetooth A2DP data types in |BluetoothA2dpInterface| (under bluetooth2/)

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

LGTM

Comment 12

[Ben Tian (inactive)]

Comment on attachment 8472924 [details] [diff] [review]
[06] Bug 1050126: Convert Bluedroid status codes and error handlers (under bluetooth2/)

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

LGTM

Comment 13

[Ben Tian (inactive)]

Comment on attachment 8472923 [details] [diff] [review]
[05] Bug 1050126: Convert Bluetooth AVRCP data types in |BluetoothAvrcpInterface| (under bluetooth2/)

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

LGTM

Comment 14

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Hi

> r=me with comment addressed.
> 
> Also please have a followup bug (as mentioned in bug 1038591 comment 13) to
> integrate |BluetoothHfpManager::ConvertToBluetoothHandsfreeCallState| into
> |BluetoothInterface::Convert|. The reason is that call state transition has
> become: telephony call state -> BluetoothHandsfreeCallState ->
> bthf_call_state_t. I think we can leverage telephony call state in Handsfree
> manager and do telephony call state -> bthf_call_state_t transition only.

That was the first approach I tried with the original patches, but it doesn't work for several reasons. First of all there's no 1-to-1 conversion from Telephony states to HFP states in the HFP manager. That means that we'd be moving policy from the HFP manager to the Bluetooth backend, where it doesn't belong. The other reason is that we're coupling the BT backend to the Telephony module, from which it should be independent for design reasons.

Comment 15

[Ben Tian (inactive)]

Got it. Thanks for clarification.

(In reply to Thomas Zimmermann [:tzimmermann] [:tdz] from comment #14)
> That was the first approach I tried with the original patches, but it
> doesn't work for several reasons. First of all there's no 1-to-1 conversion
> from Telephony states to HFP states in the HFP manager. That means that we'd
> be moving policy from the HFP manager to the Bluetooth backend, where it
> doesn't belong. The other reason is that we're coupling the BT backend to
> the Telephony module, from which it should be independent for design reasons.

Comment 16

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Attachment: [03] Bug 1050126: Convert Bluetooth Handsfree data types in |BluetoothHandsfreeInterface| (under bluetooth2/) (v2)

Changes since v1:

  - cleaned up type of |service|

Comment 17

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Thanks for the review.

https://hg.mozilla.org/integration/b2g-inbound/rev/c1a0ba575d98
https://hg.mozilla.org/integration/b2g-inbound/rev/db1e95989992
https://hg.mozilla.org/integration/b2g-inbound/rev/241f302fcfbb
https://hg.mozilla.org/integration/b2g-inbound/rev/86d789bc9b8b
https://hg.mozilla.org/integration/b2g-inbound/rev/aff5d6abe96d
https://hg.mozilla.org/integration/b2g-inbound/rev/4e978cc4242c

https://tbpl.mozilla.org/?tree=B2g-Inbound&rev=4e978cc4242c

Comment 18

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/c1a0ba575d98
https://hg.mozilla.org/mozilla-central/rev/db1e95989992
https://hg.mozilla.org/mozilla-central/rev/241f302fcfbb
https://hg.mozilla.org/mozilla-central/rev/86d789bc9b8b
https://hg.mozilla.org/mozilla-central/rev/aff5d6abe96d
https://hg.mozilla.org/mozilla-central/rev/4e978cc4242c