bluetooth: Call GattCharacteristicValueChanged after a read succeeds

bluetooth: Call GattCharacteristicValueChanged after a read succeeds

Adds a call to NotifyGattCharacteristicValueChanged before
calling the callback for ReadRemoteCharacteristic.

BUG=622217
Committed: https://crrev.com/32321de73aacae1e87fca09045658aeeb6858575
Cr-Commit-Position: refs/heads/master@{#401911}

[ortuno, 2016-06-22 15:22:29]

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

[commit-bot: I haz the power, 2016-06-22 15:22:56]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2085293002/20001

[ortuno, 2016-06-22 15:27:27]

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

[commit-bot: I haz the power, 2016-06-22 15:27:49]

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

[ortuno, 2016-06-22 16:37:28]

ortuno@chromium.org changed reviewers:
+ scheib@chromium.org

[ortuno, 2016-06-22 16:37:28]

scheib: PTAL

[commit-bot: I haz the power, 2016-06-22 16:55:36]

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

[commit-bot: I haz the power, 2016-06-22 16:55:37]

Dry run: This issue passed the CQ dry run.

[scheib, 2016-06-23 20:36:49]

LGTM

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

https://codereview.chromium.org/2085293002/diff/40001/device/bluetooth/blueto...
device/bluetooth/bluetooth_remote_gatt_characteristic_unittest.cc:416: //
TODO(http://crbug.com/580406): Make sure GattCharacteristicValueChanged
You can probably do this with not so much extra work by creating a Lambda
Expression that captures 'observer' and a
'GetGattErrorCallback(Call::NOT_EXPECTED)' that checks this expectation.  OK to
defer if you think it's too much.

https://codereview.chromium.org/2085293002/diff/40001/device/bluetooth/blueto...
device/bluetooth/bluetooth_remote_gatt_characteristic_unittest.cc:420:
EXPECT_EQ(test_vector, last_read_value_);
Can simplify this test to just the
observer.gatt_characteristic_value_changed_count by removing these last two
lines. The test vector could also be empty.

[ortuno, 2016-06-23 22:57:41]

Thanks!

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

https://codereview.chromium.org/2085293002/diff/40001/device/bluetooth/blueto...
device/bluetooth/bluetooth_remote_gatt_characteristic_unittest.cc:416: //
TODO(http://crbug.com/580406): Make sure GattCharacteristicValueChanged
On 2016/06/23 at 20:36:48, scheib wrote:
> You can probably do this with not so much extra work by creating a Lambda
Expression that captures 'observer' and a
'GetGattErrorCallback(Call::NOT_EXPECTED)' that checks this expectation.  OK to
defer if you think it's too much.

I tried using a lambda for the callback but it fails to compile? Did you mean
something different?

https://codereview.chromium.org/2085293002/diff/40001/device/bluetooth/blueto...
device/bluetooth/bluetooth_remote_gatt_characteristic_unittest.cc:420:
EXPECT_EQ(test_vector, last_read_value_);
On 2016/06/23 at 20:36:48, scheib wrote:
> Can simplify this test to just the
observer.gatt_characteristic_value_changed_count by removing these last two
lines. The test vector could also be empty.

Changed to more appropriate checks.

[scheib, 2016-06-24 00:06:42]

LGTM still, 

Too bad about the lambda.

[ortuno, 2016-06-24 15:50:24]

The CQ bit was checked by ortuno@chromium.org

[ortuno, 2016-06-24 15:50:25]

TIL: you can use a labmda as a callback if you cast it to a function pointer
first. There is some discussion to change base::Bind to accept callbacks
directly[1]. I will change the cast once that discussion concludes.

[1]
https://groups.google.com/a/chromium.org/forum/?utm_medium=email&utm_source=f...

[ortuno, 2016-06-24 15:50:25]

The patchset sent to the CQ was uploaded after l-g-t-m from scheib@chromium.org
Link to the patchset: https://codereview.chromium.org/2085293002/#ps80001
(title: "Use a lambda to check that GattCharacteristicValueChanged has been
called")

[commit-bot: I haz the power, 2016-06-24 15:51:03]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-24 18:35:49]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-06-24 18:36:55]

Description was changed from

==========
bluetooth: Call GattCharacteristicValueChanged after a read succeeds

Adds a call to NotifyGattCharacteristicValueChanged before
calling the callback for ReadRemoteCharacteristic.

BUG=622217
==========

to

==========
bluetooth: Call GattCharacteristicValueChanged after a read succeeds

Adds a call to NotifyGattCharacteristicValueChanged before
calling the callback for ReadRemoteCharacteristic.

BUG=622217

Committed: https://crrev.com/32321de73aacae1e87fca09045658aeeb6858575
Cr-Commit-Position: refs/heads/master@{#401911}
==========

[commit-bot: I haz the power, 2016-06-24 18:36:56]

Patchset 5 (id:??) landed as
https://crrev.com/32321de73aacae1e87fca09045658aeeb6858575
Cr-Commit-Position: refs/heads/master@{#401911}