bluetooth: Require frame to be connected for characteristic.readValue to succeed

bluetooth: Require frame to be connected for characteristic.readValue to succeed

Rejects characteristic.readValue if the frame is not connected or if
the frame disconnects during the operation.

The new adapter has a Health Thermometer service with a measurement interval
characteristic. We use this characteristic because it the only standard
characteristic that supports Read, Write and Notify.

Reconnection tests in follow up patch: http://crrev.com/2438963002

BUG=608538
Committed: https://crrev.com/0d1db684a6570d46b61fa141bc37ac95f3a6876a
Cr-Commit-Position: refs/heads/master@{#427007}

[ortuno, 2016-10-21 05:24:38]

Description was changed from

==========
bluetooth: Require frame to be connected for characteristic.readValue to succeed

Rejects characteristic.readValue if the frame is not connected or if
the frame disconnects during the operation.

Reconnection tests in follow up patch: http://crrev.com/2438963002

BUG=608538
==========

to

==========
bluetooth: Require frame to be connected for characteristic.readValue to succeed

Rejects characteristic.readValue if the frame is not connected or if
the frame disconnects during the operation.

The new adapter has a Health Thermometer service with a measurement interval 
characteristic. We use this characteristic because it the only standard
characteristic that supports Read, Write and Notify.

Reconnection tests in follow up patch: http://crrev.com/2438963002

BUG=608538
==========

[ortuno, 2016-10-21 05:32:09]

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

[commit-bot: I haz the power, 2016-10-21 05:32:23]

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

[ortuno, 2016-10-21 05:48:52]

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

[ortuno, 2016-10-21 05:48:53]

jyasskin: PTAL.

[commit-bot: I haz the power, 2016-10-21 05:57:23]

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

[commit-bot: I haz the power, 2016-10-21 05:57:23]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[Jeffrey Yasskin, 2016-10-21 22:40:15]

https://codereview.chromium.org/2443473003/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/bluetooth/script-tests/gatt-op-device-disconnects-before.js
(right):

https://codereview.chromium.org/2443473003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/bluetooth/script-tests/gatt-op-device-disconnects-before.js:22:
measurement_interval.CALLS([readValue()]),
I don't think we should generate tests when doing it doesn't save lines of
human-written code. I'd like to generate all 6 of these tests from a single
template, but as you've said it's hard.

[blink-reviews, 2016-10-21 22:56:33]

I wrote these tests with only a readValue call to keep the patch size
small. In follow up patches writeValue, startNotifications and
stopNotifications will use this same test. So we are cutting down the lines
human written test code by 1/3.

On Sat, Oct 22, 2016, 9:40 AM <jyasskin@chromium.org> wrote:

>
>
>
https://codereview.chromium.org/2443473003/diff/20001/third_party/WebKit/Layo...
> File
>
>
third_party/WebKit/LayoutTests/bluetooth/script-tests/gatt-op-device-disconnects-before.js
> (right):
>
>
>
https://codereview.chromium.org/2443473003/diff/20001/third_party/WebKit/Layo...
>
>
third_party/WebKit/LayoutTests/bluetooth/script-tests/gatt-op-device-disconnects-before.js:22:
> measurement_interval.CALLS([readValue()]),
> I don't think we should generate tests when doing it doesn't save lines
> of human-written code. I'd like to generate all 6 of these tests from a
> single template, but as you've said it's hard.
>
> https://codereview.chromium.org/2443473003/
>

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[chromium-reviews, 2016-10-21 22:56:33]

I wrote these tests with only a readValue call to keep the patch size
small. In follow up patches writeValue, startNotifications and
stopNotifications will use this same test. So we are cutting down the lines
human written test code by 1/3.

On Sat, Oct 22, 2016, 9:40 AM <jyasskin@chromium.org> wrote:

>
>
>
https://codereview.chromium.org/2443473003/diff/20001/third_party/WebKit/Layo...
> File
>
>
third_party/WebKit/LayoutTests/bluetooth/script-tests/gatt-op-device-disconnects-before.js
> (right):
>
>
>
https://codereview.chromium.org/2443473003/diff/20001/third_party/WebKit/Layo...
>
>
third_party/WebKit/LayoutTests/bluetooth/script-tests/gatt-op-device-disconnects-before.js:22:
> measurement_interval.CALLS([readValue()]),
> I don't think we should generate tests when doing it doesn't save lines
> of human-written code. I'd like to generate all 6 of these tests from a
> single template, but as you've said it's hard.
>
> https://codereview.chromium.org/2443473003/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Jeffrey Yasskin, 2016-10-21 23:06:23]

'k, LGTM then.

[ortuno, 2016-10-23 23:22:23]

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

[commit-bot: I haz the power, 2016-10-23 23:22:35]

Dry run: 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-10-24 00:49:49]

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

[commit-bot: I haz the power, 2016-10-24 00:49:50]

Dry run: This issue passed the CQ dry run.

[ortuno, 2016-10-24 01:39:05]

The CQ bit was checked by ortuno@chromium.org

[ortuno, 2016-10-24 01:39:06]

The patchset sent to the CQ was uploaded after l-g-t-m from
jyasskin@chromium.org
Link to the patchset: https://codereview.chromium.org/2443473003/#ps40001
(title: "Fix typo")

[commit-bot: I haz the power, 2016-10-24 01:39:13]

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-10-24 01:51:43]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-10-24 01:54:46]

Description was changed from

==========
bluetooth: Require frame to be connected for characteristic.readValue to succeed

Rejects characteristic.readValue if the frame is not connected or if
the frame disconnects during the operation.

The new adapter has a Health Thermometer service with a measurement interval 
characteristic. We use this characteristic because it the only standard
characteristic that supports Read, Write and Notify.

Reconnection tests in follow up patch: http://crrev.com/2438963002

BUG=608538
==========

to

==========
bluetooth: Require frame to be connected for characteristic.readValue to succeed

Rejects characteristic.readValue if the frame is not connected or if
the frame disconnects during the operation.

The new adapter has a Health Thermometer service with a measurement interval
characteristic. We use this characteristic because it the only standard
characteristic that supports Read, Write and Notify.

Reconnection tests in follow up patch: http://crrev.com/2438963002

BUG=608538

Committed: https://crrev.com/0d1db684a6570d46b61fa141bc37ac95f3a6876a
Cr-Commit-Position: refs/heads/master@{#427007}
==========

[commit-bot: I haz the power, 2016-10-24 01:54:47]

Patchset 3 (id:??) landed as
https://crrev.com/0d1db684a6570d46b61fa141bc37ac95f3a6876a
Cr-Commit-Position: refs/heads/master@{#427007}