Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(10)

Issues Search
Open Issues | Closed Issues | All Issues | Sign in with your Google Account to create issues and add comments

Issue 1153343002: bluetooth: Add BluetoothGATTCharacteristic IDL and Blink interface (Closed)

Created:
4 years, 11 months ago by ortuno
Modified:
4 years, 10 months ago
Reviewers:
scheib, tasak
CC:
blink-reviews, dglazkov+blink, scheib+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/blink.git@bluetooth-get-primary-service-implementation
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

bluetooth: Add BluetoothGATTCharacteristic IDL and Blink interface This is the first of a set of three patches to implement getCharacteristic: [1] This patch. [2] http://crrev.com/1156573005 [3] http://crrev.com/1146163005 BUG=483344 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=196515

Patch Set 1 : BluetoothGATTCharacteristic Interface #

Total comments: 4

Patch Set 2 : Address scheib's comments #

Patch Set 3 : Added BluetoothGATTCharacteristic to global interface test #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+104 lines, -1 line) Patch
M LayoutTests/webexposed/global-interface-listing-expected.txt View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
A Source/modules/bluetooth/BluetoothGATTCharacteristic.h View 1 chunk +27 lines, -0 lines 0 comments Download
A Source/modules/bluetooth/BluetoothGATTCharacteristic.idl View 1 chunk +24 lines, -0 lines 0 comments Download
M Source/modules/modules.gypi View 2 chunks +2 lines, -0 lines 0 comments Download
M public/platform/modules/bluetooth/WebBluetooth.h View 1 2 chunks +14 lines, -1 line 0 comments Download
A public/platform/modules/bluetooth/WebBluetoothGATTCharacteristic.h View 1 1 chunk +35 lines, -0 lines 2 comments Download

Messages

Total messages: 29 (13 generated)
ortuno
@scheib: PTAL
4 years, 11 months ago (2015-06-01 23:59:23 UTC) #5
scheib
LGTM with small fixes: https://codereview.chromium.org/1153343002/diff/60001/public/platform/modules/bluetooth/WebBluetooth.h File public/platform/modules/bluetooth/WebBluetooth.h (right): https://codereview.chromium.org/1153343002/diff/60001/public/platform/modules/bluetooth/WebBluetooth.h#newcode29 public/platform/modules/bluetooth/WebBluetooth.h:29: // Success and failure callbacks ...
4 years, 11 months ago (2015-06-03 02:52:19 UTC) #6
ortuno
Thanks! https://codereview.chromium.org/1153343002/diff/60001/public/platform/modules/bluetooth/WebBluetooth.h File public/platform/modules/bluetooth/WebBluetooth.h (right): https://codereview.chromium.org/1153343002/diff/60001/public/platform/modules/bluetooth/WebBluetooth.h#newcode29 public/platform/modules/bluetooth/WebBluetooth.h:29: // Success and failure callbacks for getCharacteristic. On ...
4 years, 11 months ago (2015-06-03 19:30:46 UTC) #7
scheib
LGTM
4 years, 11 months ago (2015-06-03 20:01:21 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1153343002/80001
4 years, 11 months ago (2015-06-03 20:04:02 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1153343002/100001
4 years, 11 months ago (2015-06-03 22:07:05 UTC) #14
commit-bot: I haz the power
Try jobs failed on following builders: win_blink_rel on tryserver.blink (JOB_FAILED, http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/65143)
4 years, 11 months ago (2015-06-03 22:59:46 UTC) #16
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1153343002/100001
4 years, 11 months ago (2015-06-03 23:13:45 UTC) #18
commit-bot: I haz the power
Try jobs failed on following builders: win_blink_rel on tryserver.blink (JOB_FAILED, http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/65162)
4 years, 11 months ago (2015-06-04 00:02:43 UTC) #20
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1153343002/100001
4 years, 11 months ago (2015-06-04 16:11:25 UTC) #22
commit-bot: I haz the power
Committed patchset #3 (id:100001) as https://src.chromium.org/viewvc/blink?view=rev&revision=196515
4 years, 11 months ago (2015-06-04 17:05:53 UTC) #23
tasak
https://codereview.chromium.org/1153343002/diff/100001/public/platform/modules/bluetooth/WebBluetoothGATTCharacteristic.h File public/platform/modules/bluetooth/WebBluetoothGATTCharacteristic.h (right): https://codereview.chromium.org/1153343002/diff/100001/public/platform/modules/bluetooth/WebBluetoothGATTCharacteristic.h#newcode30 public/platform/modules/bluetooth/WebBluetoothGATTCharacteristic.h:30: const WebArrayBuffer value; WebArrayBuffer is placed under public/web, but ...
4 years, 10 months ago (2015-06-24 05:36:04 UTC) #25
haraken
On 2015/06/24 05:36:04, tasak wrote: > https://codereview.chromium.org/1153343002/diff/100001/public/platform/modules/bluetooth/WebBluetoothGATTCharacteristic.h > File public/platform/modules/bluetooth/WebBluetoothGATTCharacteristic.h (right): > > https://codereview.chromium.org/1153343002/diff/100001/public/platform/modules/bluetooth/WebBluetoothGATTCharacteristic.h#newcode30 > ...
4 years, 10 months ago (2015-06-24 07:42:25 UTC) #26
ortuno
https://codereview.chromium.org/1153343002/diff/100001/public/platform/modules/bluetooth/WebBluetoothGATTCharacteristic.h File public/platform/modules/bluetooth/WebBluetoothGATTCharacteristic.h (right): https://codereview.chromium.org/1153343002/diff/100001/public/platform/modules/bluetooth/WebBluetoothGATTCharacteristic.h#newcode30 public/platform/modules/bluetooth/WebBluetoothGATTCharacteristic.h:30: const WebArrayBuffer value; On 2015/06/24 at 05:36:04, tasak wrote: ...
4 years, 10 months ago (2015-06-24 16:48:13 UTC) #27
tasak
On 2015/06/24 16:48:13, ortuno wrote: > https://codereview.chromium.org/1153343002/diff/100001/public/platform/modules/bluetooth/WebBluetoothGATTCharacteristic.h > File public/platform/modules/bluetooth/WebBluetoothGATTCharacteristic.h (right): > > https://codereview.chromium.org/1153343002/diff/100001/public/platform/modules/bluetooth/WebBluetoothGATTCharacteristic.h#newcode30 > ...
4 years, 10 months ago (2015-06-25 06:32:29 UTC) #28
ortuno
4 years, 10 months ago (2015-06-25 16:25:29 UTC) #29
Message was sent while issue was closed. 
On 2015/06/25 at 06:32:29, tasak wrote:
> On 2015/06/24 16:48:13, ortuno wrote:
> >
https://codereview.chromium.org/1153343002/diff/100001/public/platform/module...
> > File public/platform/modules/bluetooth/WebBluetoothGATTCharacteristic.h
(right):
> > 
> >
https://codereview.chromium.org/1153343002/diff/100001/public/platform/module...
> > public/platform/modules/bluetooth/WebBluetoothGATTCharacteristic.h:30: const
> > WebArrayBuffer value;
> > On 2015/06/24 at 05:36:04, tasak wrote:
> > > WebArrayBuffer is placed under public/web, but this file is placed under
> > public/platform.
> > > This is not correct.
> > 
> > Changing it to WebVector wouldn't be too hard. But there really should be a
DEPS
> > rule or a presubmit check if this is not supposed to be allowed.
> 
> I missed "-public/web" and "-web" to public/platform/DEPS.
> I will update public/platform/DEPS.

Thanks! I will change it to WebVector ASAP.

Powered by Google App Engine
This is Rietveld 408576698