Issue 1915803002: Bluetooth class changes for implementing local GATT attributes.

rkc

4 years, 7 months ago (2016-04-25 00:31:16 UTC) #1

rkc

The CQ bit was checked by rkc@chromium.org to run a CQ dry run

4 years, 7 months ago (2016-04-25 00:50:09 UTC) #2

commit-bot: I haz the power

Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1915803002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1915803002/1

4 years, 7 months ago (2016-04-25 00:50:21 UTC) #3

commit-bot: I haz the power

The CQ bit was unchecked by commit-bot@chromium.org

4 years, 7 months ago (2016-04-25 01:30:50 UTC) #4

commit-bot: I haz the power

Dry run: Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_ozone_rel_ng/builds/159695)

4 years, 7 months ago (2016-04-25 01:30:51 UTC) #5

rkc

The CQ bit was checked by rkc@chromium.org to run a CQ dry run

4 years, 7 months ago (2016-04-25 19:10:21 UTC) #6

commit-bot: I haz the power

Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1915803002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1915803002/40001

4 years, 7 months ago (2016-04-25 19:10:44 UTC) #7

commit-bot: I haz the power

The CQ bit was unchecked by commit-bot@chromium.org

4 years, 7 months ago (2016-04-25 20:57:13 UTC) #8

commit-bot: I haz the power

Dry run: This issue passed the CQ dry run.

4 years, 7 months ago (2016-04-25 20:57:14 UTC) #9

scheib

There's too much going on in this patch and no tests, please keep changes more ...

4 years, 7 months ago (2016-04-26 06:03:33 UTC) #10

rkc

re: tests I am trying to follow two things here. a.) Break the CLs down ...

4 years, 7 months ago (2016-04-26 18:23:59 UTC) #11

re: tests
I am trying to follow two things here.
a.) Break the CLs down the way you asked me to.
b.) Write tests using the cross-platform framework.

I am not sure how to do both in every single CL I split out. If you have any
suggestion on how to do this, I'll be glad to move tests into this CL instead of
the final CL, where they are currently.

https://codereview.chromium.org/1915803002/diff/40001/device/bluetooth/blueto...
File device/bluetooth/bluetooth_gatt_service.h (right):

https://codereview.chromium.org/1915803002/diff/40001/device/bluetooth/blueto...
device/bluetooth/bluetooth_gatt_service.h:25: // TODO(rkc): Update this list.
On 2016/04/26 06:03:32, scheib wrote:
> If a following patch updates this, link the patch. If it is longer term than
> that use an issue #.

The comment is outdated. This list is pretty comprehensive.
TODO removed.

https://codereview.chromium.org/1915803002/diff/40001/device/bluetooth/blueto...
File device/bluetooth/bluetooth_local_gatt_characteristic.cc (right):

https://codereview.chromium.org/1915803002/diff/40001/device/bluetooth/blueto...
device/bluetooth/bluetooth_local_gatt_characteristic.cc:27: return
bluez::BluetoothLocalGattCharacteristicBlueZ::Create(
On 2016/04/26 06:03:32, scheib wrote:
> Place an impl of BluetoothLocalGattCharacteristic::Create in the
> bluetooth_local_gatt_characteristic_bluez.cc file.
> 
> #if !defined(OS_CHROMEOS)... this generic impl. 
> 
> This is the pattern I see used most other places, incl device/bluetooth. It
> avoids the #if defined wrapper around include lines, and once all platforms
have
> impls then this default impl goes away entirely. 

Done.

https://codereview.chromium.org/1915803002/diff/40001/device/bluetooth/bluez/...
File device/bluetooth/bluez/bluetooth_gatt_service_bluez.h (right):

https://codereview.chromium.org/1915803002/diff/40001/device/bluetooth/bluez/...
device/bluetooth/bluez/bluetooth_gatt_service_bluez.h:28: class
AttributeValueDelegate {
On 2016/04/26 06:03:32, scheib wrote:
> Usually the delegate interface is defined on the class that needs to use the
> delegate interface (BluetoothGattCharacteristicServiceProvider)? Is this a
> better home for a reason I don't see yet?

This delegate is used by both characteristics and descriptors and forwards calls
to the delegate on the local GATT service, hence it made sense to keep it in
BluetoothLocalGattServiceBlueZ.

Even though the references to it use BluetoothLocalGattServiceBlueZ, I am not
sure how this ended up here. It is supposed to be in the local GATT service
bluez class. Moved.

https://codereview.chromium.org/1915803002/diff/40001/device/bluetooth/bluez/...
File device/bluetooth/bluez/bluetooth_local_gatt_characteristic_bluez.cc
(right):

https://codereview.chromium.org/1915803002/diff/40001/device/bluetooth/bluez/...
device/bluetooth/bluez/bluetooth_local_gatt_characteristic_bluez.cc:21: return
Properties();
On 2016/04/26 06:03:32, scheib wrote:
> NOTIMPL, or... impl by using the passed in properties and permissions.

I am not quite sure why this wasn't implemented in the first place. I plan on
investigating it and then implementing these. Till then, the code for properties
and permissions gets called from all over the place. I don't want to break
things by putting in NOTIMPLEMENTED everywhere.

Instead of putting TODOs in every single place where properties and permissions
are being used, I've created a bug to track this work.

https://codereview.chromium.org/1915803002/diff/40001/device/bluetooth/bluez/...
File device/bluetooth/bluez/bluetooth_local_gatt_characteristic_bluez.h (right):

https://codereview.chromium.org/1915803002/diff/40001/device/bluetooth/bluez/...
device/bluetooth/bluez/bluetooth_local_gatt_characteristic_bluez.h:34: static
base::WeakPtr<device::BluetoothLocalGattCharacteristic> Create(
On 2016/04/26 06:03:32, scheib wrote:
> Usually Create / constructors before overrides of base class interfaces.

Done.

rkc

The CQ bit was checked by rkc@chromium.org to run a CQ dry run

4 years, 7 months ago (2016-04-26 19:00:32 UTC) #12

commit-bot: I haz the power

Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1915803002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1915803002/60001

4 years, 7 months ago (2016-04-26 19:01:58 UTC) #13

commit-bot: I haz the power

The CQ bit was unchecked by commit-bot@chromium.org

4 years, 7 months ago (2016-04-26 21:04:33 UTC) #14

commit-bot: I haz the power

Dry run: This issue passed the CQ dry run.

4 years, 7 months ago (2016-04-26 21:04:33 UTC) #15

scheib

On 2016/04/26 18:23:59, Rahul Chaturvedi wrote: > re: tests > I am trying to follow ...

4 years, 7 months ago (2016-04-27 05:13:00 UTC) #16

On 2016/04/26 18:23:59, Rahul Chaturvedi wrote:
> re: tests
> I am trying to follow two things here.
> a.) Break the CLs down the way you asked me to.
> b.) Write tests using the cross-platform framework.
> 
> I am not sure how to do both in every single CL I split out. If you have any
> suggestion on how to do this, I'll be glad to move tests into this CL instead
of
> the final CL, where they are currently.

Appreciated. I think part of the challenge is not only having tests, but in
trying to do too many things at once.

When possible, patches like this break steps down and include tests as they go:
https://codereview.chromium.org/1150833002
https://codereview.chromium.org/1215303006
https://codereview.chromium.org/1394973003
https://codereview.chromium.org/1395783005

Sometimes several steps are needed before getting to a reasonable point to add
tests, https://codereview.chromium.org/1287753002 is an example. I suggest
updating this patch series to have the summary and cross link to other patches
similar to that one. Maybe some early patches just aren't at a testable state.

"Cleaning up of the classes and implementing all missing parts" is probably too
much. The goal is, can we construct the most basic objects and then create a
unit test that uses them. Early unit tests may only construct and free.

Then progress by building up more of the class details. If there are too many
details already they should be NOTIMPLEMENTED with TODO(issuenumber).

Why is "Addition of an AttributeDelegate to handle calls from DBus and wrappers"
in this patch? Seems we can bring up unit tests ability before we need to bring
up any dbus impl detail.

https://codereview.chromium.org/1915803002/diff/40001/device/bluetooth/bluez/...
File device/bluetooth/bluez/bluetooth_gatt_service_bluez.h (right):

https://codereview.chromium.org/1915803002/diff/40001/device/bluetooth/bluez/...
device/bluetooth/bluez/bluetooth_gatt_service_bluez.h:28: class
AttributeValueDelegate {
On 2016/04/26 18:23:59, Rahul Chaturvedi wrote:
> On 2016/04/26 06:03:32, scheib wrote:
> > Usually the delegate interface is defined on the class that needs to use the
> > delegate interface (BluetoothGattCharacteristicServiceProvider)? Is this a
> > better home for a reason I don't see yet?
> 
> This delegate is used by both characteristics and descriptors and forwards
calls
> to the delegate on the local GATT service, hence it made sense to keep it in
> BluetoothLocalGattServiceBlueZ.
> 
> Even though the references to it use BluetoothLocalGattServiceBlueZ, I am not
> sure how this ended up here. It is supposed to be in the local GATT service
> bluez class. Moved.

I see that it is a delegate used by two dbus classes. Perhaps it should live in
a stand alone file in dbus subdir. Or, just place it in
BluetoothGattCharacteristicServiceProvider and reuse it from Descriptor.

BluetoothLocalGattServiceBlueZ seems a strange place, though I understand the
implementation of the delegate results in calling into
BluetoothLocalGattServiceBlueZ and then onwards via the
BluetoothLocalGattService::Delegate. 

I think that's all the more justification to use the Delegate pattern more
consistently, with the Delegate interface being declared at the location code
needs to reach out elsewhere. Having 2 delegates defined on
BluetoothLocalGattService* confuse the delegating directions. (in other words,
BluetoothLocalGattServiceBlueZ is not using AttributeValueDelegate to delegate
work elsewhere.)

https://codereview.chromium.org/1915803002/diff/40001/device/bluetooth/bluez/...
File device/bluetooth/bluez/bluetooth_local_gatt_characteristic_bluez.cc
(right):

https://codereview.chromium.org/1915803002/diff/40001/device/bluetooth/bluez/...
device/bluetooth/bluez/bluetooth_local_gatt_characteristic_bluez.cc:21: return
Properties();
On 2016/04/26 18:23:59, Rahul Chaturvedi wrote:
> On 2016/04/26 06:03:32, scheib wrote:
> > NOTIMPL, or... impl by using the passed in properties and permissions.
> 
> I am not quite sure why this wasn't implemented in the first place. I plan on
> investigating it and then implementing these. Till then, the code for
properties
> and permissions gets called from all over the place. I don't want to break
> things by putting in NOTIMPLEMENTED everywhere.
> 
> Instead of putting TODOs in every single place where properties and
permissions
> are being used, I've created a bug to track this work.

The local objects aren't created anywhere yet though, are they? I'm not clear on
how these are being called.

https://codereview.chromium.org/1915803002/diff/40001/device/bluetooth/bluez/...
File device/bluetooth/bluez/bluetooth_local_gatt_characteristic_bluez.h (right):

https://codereview.chromium.org/1915803002/diff/40001/device/bluetooth/bluez/...
device/bluetooth/bluez/bluetooth_local_gatt_characteristic_bluez.h:34: static
base::WeakPtr<device::BluetoothLocalGattCharacteristic> Create(
On 2016/04/26 18:23:59, Rahul Chaturvedi wrote:
> On 2016/04/26 06:03:32, scheib wrote:
> > Usually Create / constructors before overrides of base class interfaces.
> 
> Done.

Here and BluetoothLocalGattDescriptorBlueZ are not done.

https://codereview.chromium.org/1915803002/diff/60001/device/bluetooth/bluez/...
File device/bluetooth/bluez/bluetooth_gatt_service_bluez.h (right):

https://codereview.chromium.org/1915803002/diff/60001/device/bluetooth/bluez/...
device/bluetooth/bluez/bluetooth_gatt_service_bluez.h:8: #include <cstdint>
Include changes do not seem needed in this file.

rkc

https://codereview.chromium.org/1915803002/diff/40001/device/bluetooth/bluez/bluetooth_gatt_service_bluez.h File device/bluetooth/bluez/bluetooth_gatt_service_bluez.h (right): https://codereview.chromium.org/1915803002/diff/40001/device/bluetooth/bluez/bluetooth_gatt_service_bluez.h#newcode28 device/bluetooth/bluez/bluetooth_gatt_service_bluez.h:28: class AttributeValueDelegate { On 2016/04/27 05:12:59, scheib wrote: > ...

4 years, 7 months ago (2016-04-27 19:41:06 UTC) #17

https://codereview.chromium.org/1915803002/diff/40001/device/bluetooth/bluez/...
File device/bluetooth/bluez/bluetooth_gatt_service_bluez.h (right):

https://codereview.chromium.org/1915803002/diff/40001/device/bluetooth/bluez/...
device/bluetooth/bluez/bluetooth_gatt_service_bluez.h:28: class
AttributeValueDelegate {
On 2016/04/27 05:12:59, scheib wrote:
> On 2016/04/26 18:23:59, Rahul Chaturvedi wrote:
> > On 2016/04/26 06:03:32, scheib wrote:
> > > Usually the delegate interface is defined on the class that needs to use
the
> > > delegate interface (BluetoothGattCharacteristicServiceProvider)? Is this a
> > > better home for a reason I don't see yet?
> > 
> > This delegate is used by both characteristics and descriptors and forwards
> calls
> > to the delegate on the local GATT service, hence it made sense to keep it in
> > BluetoothLocalGattServiceBlueZ.
> > 
> > Even though the references to it use BluetoothLocalGattServiceBlueZ, I am
not
> > sure how this ended up here. It is supposed to be in the local GATT service
> > bluez class. Moved.
> 
> I see that it is a delegate used by two dbus classes. Perhaps it should live
in
> a stand alone file in dbus subdir. Or, just place it in
> BluetoothGattCharacteristicServiceProvider and reuse it from Descriptor.
> 
> BluetoothLocalGattServiceBlueZ seems a strange place, though I understand the
> implementation of the delegate results in calling into
> BluetoothLocalGattServiceBlueZ and then onwards via the
> BluetoothLocalGattService::Delegate. 
> 
> I think that's all the more justification to use the Delegate pattern more
> consistently, with the Delegate interface being declared at the location code
> needs to reach out elsewhere. Having 2 delegates defined on
> BluetoothLocalGattService* confuse the delegating directions. (in other words,
> BluetoothLocalGattServiceBlueZ is not using AttributeValueDelegate to delegate
> work elsewhere.)

Moving to its own class in bluez/dbus.
Done.

https://codereview.chromium.org/1915803002/diff/40001/device/bluetooth/bluez/...
File device/bluetooth/bluez/bluetooth_local_gatt_characteristic_bluez.cc
(right):

https://codereview.chromium.org/1915803002/diff/40001/device/bluetooth/bluez/...
device/bluetooth/bluez/bluetooth_local_gatt_characteristic_bluez.cc:21: return
Properties();
On 2016/04/27 05:12:59, scheib wrote:
> On 2016/04/26 18:23:59, Rahul Chaturvedi wrote:
> > On 2016/04/26 06:03:32, scheib wrote:
> > > NOTIMPL, or... impl by using the passed in properties and permissions.
> > 
> > I am not quite sure why this wasn't implemented in the first place. I plan
on
> > investigating it and then implementing these. Till then, the code for
> properties
> > and permissions gets called from all over the place. I don't want to break
> > things by putting in NOTIMPLEMENTED everywhere.
> > 
> > Instead of putting TODOs in every single place where properties and
> permissions
> > are being used, I've created a bug to track this work.
> 
> The local objects aren't created anywhere yet though, are they? I'm not clear
on
> how these are being called.

You're right. This can be marked NOTIMPLEMENTED. I was confusing this with some
of the code on remote which does get called.
Done.

https://codereview.chromium.org/1915803002/diff/40001/device/bluetooth/bluez/...
File device/bluetooth/bluez/bluetooth_local_gatt_characteristic_bluez.h (right):

https://codereview.chromium.org/1915803002/diff/40001/device/bluetooth/bluez/...
device/bluetooth/bluez/bluetooth_local_gatt_characteristic_bluez.h:34: static
base::WeakPtr<device::BluetoothLocalGattCharacteristic> Create(
On 2016/04/27 05:13:00, scheib wrote:
> On 2016/04/26 18:23:59, Rahul Chaturvedi wrote:
> > On 2016/04/26 06:03:32, scheib wrote:
> > > Usually Create / constructors before overrides of base class interfaces.
> > 
> > Done.
> 
> Here and BluetoothLocalGattDescriptorBlueZ are not done.

Whoops. Done and matched order of functions in all three.
Done.

https://codereview.chromium.org/1915803002/diff/60001/device/bluetooth/bluez/...
File device/bluetooth/bluez/bluetooth_gatt_service_bluez.h (right):

https://codereview.chromium.org/1915803002/diff/60001/device/bluetooth/bluez/...
device/bluetooth/bluez/bluetooth_gatt_service_bluez.h:8: #include <cstdint>
On 2016/04/27 05:13:00, scheib wrote:
> Include changes do not seem needed in this file.

Removed this and vector.
Done.

rkc

Description was changed from ========== Bluetooth class changes for implementing local GATT attributes. This CL ...

4 years, 7 months ago (2016-04-27 20:11:26 UTC) #18

scheib

The CQ bit was checked by scheib@chromium.org to run a CQ dry run

4 years, 7 months ago (2016-04-28 00:32:38 UTC) #19

commit-bot: I haz the power

Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1915803002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1915803002/80001

4 years, 7 months ago (2016-04-28 00:33:00 UTC) #20

scheib

https://codereview.chromium.org/1915803002/diff/80001/device/bluetooth/bluetooth_local_gatt_service.h File device/bluetooth/bluetooth_local_gatt_service.h (right): https://codereview.chromium.org/1915803002/diff/80001/device/bluetooth/bluetooth_local_gatt_service.h#newcode77 device/bluetooth/bluetooth_local_gatt_service.h:77: // |callback| with the new value of the characteristic. ...

4 years, 7 months ago (2016-04-28 01:06:24 UTC) #21

rkc

https://codereview.chromium.org/1915803002/diff/80001/device/bluetooth/bluetooth_local_gatt_service.h File device/bluetooth/bluetooth_local_gatt_service.h (right): https://codereview.chromium.org/1915803002/diff/80001/device/bluetooth/bluetooth_local_gatt_service.h#newcode77 device/bluetooth/bluetooth_local_gatt_service.h:77: // |callback| with the new value of the characteristic. ...

4 years, 7 months ago (2016-04-28 02:06:58 UTC) #22

https://codereview.chromium.org/1915803002/diff/80001/device/bluetooth/blueto...
File device/bluetooth/bluetooth_local_gatt_service.h (right):

https://codereview.chromium.org/1915803002/diff/80001/device/bluetooth/blueto...
device/bluetooth/bluetooth_local_gatt_service.h:77: // |callback| with the new
value of the characteristic. Doing so will
On 2016/04/28 01:06:24, scheib wrote:
> You've changed the callback to Closure instead of ValueCallback. I don't see
> why, but if it was intended then the comments need to be updated. In the later
> patches the tests are using ValueCallback.

Fixed the comments. Nothing should be using ValueCallback here, the code won't
compile. I looked but all write delegate functions seem to be using a closure.

If you see any further code using a ValueCallback for write functions, please do
let me know. That needs to change.

https://codereview.chromium.org/1915803002/diff/80001/device/bluetooth/bluez/...
File device/bluetooth/bluez/bluetooth_gatt_characteristic_bluez.h (right):

https://codereview.chromium.org/1915803002/diff/80001/device/bluetooth/bluez/...
device/bluetooth/bluez/bluetooth_gatt_characteristic_bluez.h:30:
BluetoothGattCharacteristicBlueZ();
On 2016/04/28 01:06:24, scheib wrote:
> I mentioned in the earlier patch (before splitting to this) that it seems odd
to
> move the members that are assigned during construction up to be protected. At
> the time I suggested having virtual implementations of e.g. object_path.
> 
> You argued against virtual impl of the accessors, which is OK. But, I still
> don't like the uncertain constructed state of these members.
> 
> object_path_ can remain a constructor argument and the member can be
initialized
> programmatically in the constructor initializer list. E.g. below 
>   object_path_(BluetoothLocalGattServiceBlueZ::AddGuidToObjectPath(
>     service->object_path().value() + "/characteristic"))
> 
> This makes it clear in the BluetoothGattCharacteristicBlueZ declaration that
> object_path_ will always be set upon construction by any subclass.

Done.

https://codereview.chromium.org/1915803002/diff/80001/device/bluetooth/bluez/...
File device/bluetooth/bluez/bluetooth_local_gatt_characteristic_bluez.cc
(right):

https://codereview.chromium.org/1915803002/diff/80001/device/bluetooth/bluez/...
device/bluetooth/bluez/bluetooth_local_gatt_characteristic_bluez.cc:31:
BluetoothLocalGattCharacteristicBlueZ::Create(
On 2016/04/28 01:06:24, scheib wrote:
> Two ::Create methods are not needed, the ___BlueZ::Create can be removed and
> code moved to the ___::Create method. (ditto descriptor)

Done.

https://codereview.chromium.org/1915803002/diff/80001/device/bluetooth/bluez/...
File device/bluetooth/bluez/bluetooth_local_gatt_characteristic_bluez.h (right):

https://codereview.chromium.org/1915803002/diff/80001/device/bluetooth/bluez/...
device/bluetooth/bluez/bluetooth_local_gatt_characteristic_bluez.h:41:
device::BluetoothUUID GetUUID() const override;
On 2016/04/28 01:06:24, scheib wrote:
> GetUUID: either make not implemented, test in this patch, or test in
>
https://codereview.chromium.org/1919683002/diff/80001/device/bluetooth/blueto...

Added code to test this for characteristics and descriptors in the third patch.

https://codereview.chromium.org/1915803002/diff/80001/device/bluetooth/bluez/...
File device/bluetooth/bluez/bluetooth_local_gatt_service_bluez.cc (right):

https://codereview.chromium.org/1915803002/diff/80001/device/bluetooth/bluez/...
device/bluetooth/bluez/bluetooth_local_gatt_service_bluez.cc:57: // TODO(rkc):
Get base application path from adapter and prefix it here.
On 2016/04/28 01:06:24, scheib wrote:
> Use issue numbers for TODOs.

This is just a remnant of the split to make the code compile. The CL that adds
the adapter code in removes this.

rkc

The CQ bit was checked by rkc@chromium.org to run a CQ dry run

4 years, 7 months ago (2016-04-28 03:09:56 UTC) #23

commit-bot: I haz the power

Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1915803002/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1915803002/100001

4 years, 7 months ago (2016-04-28 03:10:08 UTC) #24

commit-bot: I haz the power

The CQ bit was unchecked by commit-bot@chromium.org

4 years, 7 months ago (2016-04-28 04:12:44 UTC) #25

commit-bot: I haz the power

Dry run: This issue passed the CQ dry run.

4 years, 7 months ago (2016-04-28 04:12:45 UTC) #26

scheib

LGTM either with friends removed or comments in code to explain why. https://codereview.chromium.org/1915803002/diff/80001/device/bluetooth/bluez/bluetooth_local_gatt_service_bluez.cc File device/bluetooth/bluez/bluetooth_local_gatt_service_bluez.cc ...

4 years, 7 months ago (2016-04-28 04:34:08 UTC) #27

rkc

https://codereview.chromium.org/1915803002/diff/80001/device/bluetooth/bluez/bluetooth_local_gatt_service_bluez.cc File device/bluetooth/bluez/bluetooth_local_gatt_service_bluez.cc (right): https://codereview.chromium.org/1915803002/diff/80001/device/bluetooth/bluez/bluetooth_local_gatt_service_bluez.cc#newcode57 device/bluetooth/bluez/bluetooth_local_gatt_service_bluez.cc:57: // TODO(rkc): Get base application path from adapter and ...

4 years, 7 months ago (2016-04-28 05:07:23 UTC) #28

rkc

The patchset sent to the CQ was uploaded after l-g-t-m from scheib@chromium.org Link to the ...

4 years, 7 months ago (2016-04-28 05:07:51 UTC) #30

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1915803002/120001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1915803002/120001

4 years, 7 months ago (2016-04-28 05:08:08 UTC) #31

commit-bot: I haz the power

Description was changed from ========== Bluetooth class changes for implementing local GATT attributes. In this ...

4 years, 7 months ago (2016-04-28 06:56:49 UTC) #32

Message was sent while issue was closed.

Description was changed from

==========
Bluetooth class changes for implementing local GATT attributes.

In this CL, we implement the changes to the platform independent and BlueZ
specific class changes to support local GATT services. The major changes
from the code in trunk are,

.) Cleaning up of the classes and implementing all missing parts.
.) Adding code to properly take ownership of attributes of all three kinds.
.) Adding structure to the attributes to correctly setup the hierarchy
expected (services <-- characteristics <-- descriptors).
.) Addition of an AttributeDelegate to handle calls from DBus and wrappers
to translate them to the BluetoothLocalGattService::Delegate.
.) Cleanup in all the code touched.


Part 1 of a 3 patch series:
https://crrev.com/1915803002 Bluetooth class changes <<<
https://crrev.com/1914893002 DBus class changes
https://crrev.com/1919683002 Adapter changes + tests


R=ortuno@chromium.org, scheib@chromium.org
BUG=601935
==========

to

==========
Bluetooth class changes for implementing local GATT attributes.

In this CL, we implement the changes to the platform independent and BlueZ
specific class changes to support local GATT services. The major changes
from the code in trunk are,

.) Cleaning up of the classes and implementing all missing parts.
.) Adding code to properly take ownership of attributes of all three kinds.
.) Adding structure to the attributes to correctly setup the hierarchy
expected (services <-- characteristics <-- descriptors).
.) Addition of an AttributeDelegate to handle calls from DBus and wrappers
to translate them to the BluetoothLocalGattService::Delegate.
.) Cleanup in all the code touched.


Part 1 of a 3 patch series:
https://crrev.com/1915803002 Bluetooth class changes <<<
https://crrev.com/1914893002 DBus class changes
https://crrev.com/1919683002 Adapter changes + tests


R=ortuno@chromium.org, scheib@chromium.org
BUG=601935
==========

commit-bot: I haz the power

4 years, 7 months ago (2016-04-30 17:16:38 UTC) #34

Message was sent while issue was closed.

Patchset 7 (id:??) landed as
https://crrev.com/a65ced97ca5fe61b342eeda935248175e1a6ab74
Cr-Commit-Position: refs/heads/master@{#390315}

commit-bot: I haz the power

4 years, 7 months ago (2016-04-30 17:54:28 UTC) #35

Message was sent while issue was closed.

Description was changed from

==========
Bluetooth class changes for implementing local GATT attributes.

In this CL, we implement the changes to the platform independent and BlueZ
specific class changes to support local GATT services. The major changes
from the code in trunk are,

.) Cleaning up of the classes and implementing all missing parts.
.) Adding code to properly take ownership of attributes of all three kinds.
.) Adding structure to the attributes to correctly setup the hierarchy
expected (services <-- characteristics <-- descriptors).
.) Addition of an AttributeDelegate to handle calls from DBus and wrappers
to translate them to the BluetoothLocalGattService::Delegate.
.) Cleanup in all the code touched.


Part 1 of a 3 patch series:
https://crrev.com/1915803002 Bluetooth class changes <<<
https://crrev.com/1914893002 DBus class changes
https://crrev.com/1919683002 Adapter changes + tests


R=ortuno@chromium.org, scheib@chromium.org
BUG=601935
==========

to

==========
Bluetooth class changes for implementing local GATT attributes.

In this CL, we implement the changes to the platform independent and BlueZ
specific class changes to support local GATT services. The major changes
from the code in trunk are,

.) Cleaning up of the classes and implementing all missing parts.
.) Adding code to properly take ownership of attributes of all three kinds.
.) Adding structure to the attributes to correctly setup the hierarchy
expected (services <-- characteristics <-- descriptors).
.) Addition of an AttributeDelegate to handle calls from DBus and wrappers
to translate them to the BluetoothLocalGattService::Delegate.
.) Cleanup in all the code touched.

Part 1 of a 3 patch series:
https://crrev.com/1915803002 Bluetooth class changes <<<
https://crrev.com/1914893002 DBus class changes
https://crrev.com/1919683002 Adapter changes + tests

R=ortuno@chromium.org, scheib@chromium.org
BUG=601935

Committed: https://crrev.com/a65ced97ca5fe61b342eeda935248175e1a6ab74
Cr-Commit-Position: refs/heads/master@{#390315}
==========

Issue 1915803002: Bluetooth class changes for implementing local GATT attributes. (Closed)

Description

Patch Set 1 #

Patch Set 2 : object paths fix #

Patch Set 3 : magic values fix #

Patch Set 4 : #

Patch Set 5 : #

Patch Set 6 : #

Patch Set 7 : #

Messages