1027504 - Implement BluetoothClassOfDevice

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Bluetooth, defect)

Description

[Ben Tian (inactive)]

BluetoothClassOfDevice: https://wiki.mozilla.org/B2G/Bluetooth/WebBluetooth-v2/BluetoothClassOfDevice

Comment 1

[Ben Tian (inactive)]

Attachment: Patch 1/2: BluetoothClassOfDevice.webidl

Comment 2

[Ben Tian (inactive)]

Attachment: Patch 2/2: BluetoothClassOfDevice implementation

Comment 3

[Ben Tian (inactive)]

Attachment: Patch 2/2 (v2): BluetoothClassOfDevice implementation

Comment 4

[Ben Tian (inactive)]

Attachment: Dev patch v1: CoD sample code in device

Comment 5

[Ben Tian (inactive)]

Attachment: Patch 1/2 (v2): BluetoothClassOfDevice.webidl

Revise webidl comment.

Comment 6

[Jocelyn Liu [:jocelyn] [:joliu]]

Attachment: [WIP] Bug 1027504 - Get uint32_t cod value from CoD class.

Hi Ben,

It's a minor patch to get uint32_t cod value from CoD class.
It can be removed after we replace all the code using uint32_t value.

Comment 7

[Ben Tian (inactive)]

Attachment: Patch 1/2 (v3): Add BluetoothClassOfDevice.webidl

Comment 8

[Ben Tian (inactive)]

Attachment: Patch 2/2 (v3): Implement BluetoothClassOfDevice

Integrate Jocelyn's change.

Comment 9

[Ben Tian (inactive)]

Comment on attachment 8449200 [details] [diff] [review]
Patch 1/2 (v3): Add BluetoothClassOfDevice.webidl

Boris, please help review patches of BluetoothClassOfDevice [1] implementation. BluetoothClassOfDevice is used to provide information of remote device's capabilities [2].

2 patches in total:
1) IDL changes, including new BluetoothClassOfDevice.webidl.
2) BluetoothClassOfDevice implementation

[1] https://wiki.mozilla.org/B2G/Bluetooth/WebBluetooth-v2/BluetoothClassOfDevice
[2] https://wiki.mozilla.org/B2G/Bluetooth/WebBluetooth-v2/BluetoothDevice#cod

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8449200 [details] [diff] [review]
Patch 1/2 (v3): Add BluetoothClassOfDevice.webidl

Please make sure to qfold/squash this with the second patch before landing, since it doesn't compile on its own!

r=me

Comment 11

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8449202 [details] [diff] [review]
Patch 2/2 (v3): Implement BluetoothClassOfDevice

>+#define GET_MAJOR_SERVICE_CLASS(cod) ((cod & 0xffe000) >> 13)

Parens about "cod" please:

  #define GET_MAJOR_SERVICE_CLASS(cod) (((cod) & 0xffe000) >> 13)

and similar for he other macros.

You need to hold a strong reference to mOwnerWindow and cycle collect it.

r=me with that fixed.

Comment 12

[Ben Tian (inactive)]

Attachment: Patch 2/2 (v4): Implement BluetoothClassOfDevice, r=bz

Comment 13

[Ben Tian (inactive)]

(In reply to Ben Tian [:btian] from comment #12)
> Created attachment 8451453 [details] [diff] [review]
> Patch 2/2 (v4): Implement BluetoothClassOfDevice, r=bz

Also revised macros in BluetoothProfileController based on bz's comment 11.

Comment 14

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

Comment on attachment 8451453 [details] [diff] [review]
Patch 2/2 (v4): Implement BluetoothClassOfDevice, r=bz

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

r=me with nits addressed. Thanks.

::: dom/bluetooth2/BluetoothClassOfDevice.cpp
@@ +31,5 @@
> + * https://www.bluetooth.org/en-us/specification/assigned-numbers/baseband
> + */
> +
> +// Bit 23 ~ Bit 13: Major service class
> +#define GET_MAJOR_SERVICE_CLASS(cod) (((cod) & 0xffe000) >> 13)

These macros are defined in both BluetoothClassOfDevice.cpp and BluetoothProfileController.h. Maybe we should put them somewhere such as BluetoothCommon.h or provide a static utility function in class BluetoothClassOfDevice for BluetoothProfileController to use.

::: dom/bluetooth2/BluetoothClassOfDevice.h
@@ +53,5 @@
> +  /**
> +   * Convert CoD to uint32_t CoD value.
> +   *
> +   * TODO: Remove this function once we replace uint32_t cod value with
> +           BluetoothClassOfDevice in BluetoothProfileController.

super-nit: I would add a '*' which aligns with other '*' at the beginning of this line

Comment 15

[Ben Tian (inactive)]

Attachment: [final] Patch 2/2: Implement BluetoothClassOfDevice, r=bz, r=echou

Fix nit addressed by reviewer. For the macros, I prefer to reorganize them after we decide how to handle profile-related API.

Comment 16

[Ben Tian (inactive)]

Land qfold-ed patch onto b2g-inbound.

https://hg.mozilla.org/integration/b2g-inbound/rev/cff18929a775

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/cff18929a775