Closed Bug 1006316 Opened 6 years ago Closed 6 years ago

Modify WebAPI for BT on/off

Categories

(Firefox OS Graveyard :: Bluetooth, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
2.0 S3 (6june)

People

(Reporter: ben.tian, Assigned: yrliou)

References

Details

(Whiteboard: [webbt-api])

Attachments

(2 files, 5 obsolete files)

Modify dom/webidl/BluetoothAdapter.webidl based on bug 1006308 comment 0. This bug should request a super review for WebAPI change.
Whiteboard: [webbt-api]
Assignee: nobody → joliu
Hi Ben,

I have done two patches for the BluetoothAdapter WebAPI change.
Please let me know if you have any feedbacks.

Thanks,
Jocelyn
Attachment #8419236 - Flags: feedback?(btian)
Attachment #8419236 - Flags: feedback?(btian)
Attachment #8419234 - Flags: feedback?(btian)
Revise BluetoothAdapter2.webidl:
    * BluetoothAdapterState enum
    * BluetoothAdapterAttribute enum
    * Modify BluetoothAdapter interface for refined WebBluetooth API
Attachment #8419234 - Attachment is obsolete: true
Attachment #8428484 - Flags: feedback?(btian)
Revise bluetooth2/BluetoothAdapter based on patch1.
Attachment #8419236 - Attachment is obsolete: true
Attachment #8428486 - Flags: feedback?(btian)
Attachment #8428486 - Attachment description: Modify bluetooth2/BluetoothAdapter to support WebAPI change → Patch2: Modify bluetooth2/BluetoothAdapter to support WebAPI change
Comment on attachment 8428484 [details] [diff] [review]
Patch1: Revise BluetoothAdapter2.webidl for new WebBluetooth API

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

Please remove |discoverableTimeout| related attribute and function since they are not used in new WebBluetooth API.

::: dom/webidl/BluetoothAdapter2.webidl
@@ +54,5 @@
>  interface BluetoothAdapter : EventTarget {
> +  readonly attribute BluetoothAdapterState  state;
> +  readonly attribute DOMString              address;
> +  readonly attribute unsigned long          class;
> +  readonly attribute boolean                discovering;

nit: Put |discovering| after |discoverable| as on wiki: https://wiki.mozilla.org/B2G/Bluetooth/WebBluetooth-v2/BluetoothAdapter#BluetoothAdapter

@@ +57,5 @@
> +  readonly attribute unsigned long          class;
> +  readonly attribute boolean                discovering;
> +  readonly attribute DOMString              name;
> +  readonly attribute boolean                discoverable;
> +  readonly attribute unsigned long          discoverableTimeout; // in seconds

Remove |discoverableTimeout| and |setDiscoverableTimeout| if no gaia application require them.

@@ +92,5 @@
>    DOMRequest setName(DOMString name);
>    [NewObject, Throws]
>    DOMRequest setDiscoverable(boolean discoverable);
>    [NewObject, Throws]
>    DOMRequest setDiscoverableTimeout(unsigned long timeout);

Ditto.
Hi Ben,

Thanks for the feedback, I have updated the patch according to the wiki page.
Please see the updated patch for webidl.
They will also be removed in implementation level later by Bug 1006308.

Thanks,
Jocelyn
Attachment #8428484 - Attachment is obsolete: true
Attachment #8428484 - Flags: feedback?(btian)
Attachment #8428560 - Flags: feedback?(btian)
Comment on attachment 8428560 [details] [diff] [review]
Patch1(v2): Revise BluetoothAdapter2.webidl for new WebBluetooth API

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

f=me. Please request :bz review IDL change.
Attachment #8428560 - Flags: feedback?(btian) → feedback+
Comment on attachment 8428486 [details] [diff] [review]
Patch2: Modify bluetooth2/BluetoothAdapter to support WebAPI change

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

r=me with comment addressed. Thanks.

::: dom/bluetooth2/BluetoothAdapter.cpp
@@ +670,5 @@
>  }
>  
> +/*
> +  TODO: Implement Enable/Disable functions
> +*/

nit: format comment as following.

/**
 * TODO: Implement Enable/Disable functions
 */

::: dom/bluetooth2/BluetoothAdapter.h
@@ +187,5 @@
>      PairUnpair(bool aPair, const nsAString& aDeviceAddress, ErrorResult& aRv);
>  
>    JS::Heap<JSObject*> mJsUuids;
>    JS::Heap<JSObject*> mJsDeviceAddresses;
> +  BluetoothAdapterState mState;

Assign initial value of mState in constructor.
Attachment #8428486 - Flags: feedback?(btian) → review+
Comment on attachment 8428560 [details] [diff] [review]
Patch1(v2): Revise BluetoothAdapter2.webidl for new WebBluetooth API

Hi Boris,

  I had revised BluetoothAdapter2.webidl for our new WebBluetooth API.
  (spec: https://wiki.mozilla.org/B2G/Bluetooth/WebBluetooth-v2/BluetoothAdapter#BluetoothAdapter)
  Would you mind to review this webidl change?

Thanks,
Jocelyn
Attachment #8428560 - Flags: review?(bzbarsky)
Comment on attachment 8428560 [details] [diff] [review]
Patch1(v2): Revise BluetoothAdapter2.webidl for new WebBluetooth API

r=me
Attachment #8428560 - Flags: review?(bzbarsky) → review+
Replace [Func="Navigator::HasBluetoothSupport"] with [CheckPermissions="bluetooth"], see bug 952486.
Attachment #8428560 - Attachment is obsolete: true
Attachment #8428486 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/4e836dee7dc6
https://hg.mozilla.org/mozilla-central/rev/411f2bef5560
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.0 S3 (6june)
No longer blocks: webbt-test-onoff
You need to log in before you can comment on or make changes to this bug.