Add timeouts for GATT connection steps |
|||||||||||||||
Issue descriptionThere are several steps to forming a GATT connection: (1) Set connection latency. (2) Create GATT connection. (3) Find GATT characteristics. (4) Start notify session. Each of these steps comes with a callback, but sometimes these callbacks are never invoked. I've filed bugs against Bluetooth about fixing these issues, but we should also code defensively here as well and ensure that we do not get stuck, even if the Bluetooth APIs fail to work.
,
Sep 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b3e3134cdb044164ad384a87b56c670013f9284f commit b3e3134cdb044164ad384a87b56c670013f9284f Author: Kyle Horimoto <khorimoto@google.com> Date: Tue Sep 12 21:55:26 2017 [CrOS Tether] Add timeouts for each step in creating Weave connection. There are several steps of the Bluetooth connection flow which call into Bluetooth APIs and wait for an asynchronous response. These asynchronous callbacks *should* be getting invoked in a timely manner by Bluetooth, but this is not happening in practice. This CL adds timeouts for each step of the connection process in which we wait for an asynchronous reply. Bug: 763176 , 672263 Change-Id: Ia7a538e0de8280346a3c113c7dccd5c18cc7086d Reviewed-on: https://chromium-review.googlesource.com/657811 Commit-Queue: Kyle Horimoto <khorimoto@chromium.org> Reviewed-by: Tim Song <tengs@chromium.org> Cr-Commit-Position: refs/heads/master@{#501418} [modify] https://crrev.com/b3e3134cdb044164ad384a87b56c670013f9284f/components/cryptauth/ble/bluetooth_low_energy_weave_client_connection.cc [modify] https://crrev.com/b3e3134cdb044164ad384a87b56c670013f9284f/components/cryptauth/ble/bluetooth_low_energy_weave_client_connection.h [modify] https://crrev.com/b3e3134cdb044164ad384a87b56c670013f9284f/components/cryptauth/ble/bluetooth_low_energy_weave_client_connection_unittest.cc
,
Sep 12 2017
,
Sep 12 2017
This bug requires manual review: Request affecting a post-stable build Please contact the milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), ketakid@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Sep 12 2017
Pls apply appropriate OSs label. Thank you.
,
Sep 12 2017
,
Sep 13 2017
Approving merge to M61 and M62.
,
Sep 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7b954f127154ef455795bed22c2fa89e2beed957 commit 7b954f127154ef455795bed22c2fa89e2beed957 Author: Kyle Horimoto <khorimoto@google.com> Date: Wed Sep 13 01:43:27 2017 [CrOS Tether] Add timeouts for each step in creating Weave connection. There are several steps of the Bluetooth connection flow which call into Bluetooth APIs and wait for an asynchronous response. These asynchronous callbacks *should* be getting invoked in a timely manner by Bluetooth, but this is not happening in practice. This CL adds timeouts for each step of the connection process in which we wait for an asynchronous reply. TBR=khorimoto@google.com (cherry picked from commit b3e3134cdb044164ad384a87b56c670013f9284f) Bug: 763176 , 672263 Change-Id: Ia7a538e0de8280346a3c113c7dccd5c18cc7086d Reviewed-on: https://chromium-review.googlesource.com/657811 Commit-Queue: Kyle Horimoto <khorimoto@chromium.org> Reviewed-by: Tim Song <tengs@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#501418} Reviewed-on: https://chromium-review.googlesource.com/664181 Reviewed-by: Kyle Horimoto <khorimoto@chromium.org> Cr-Commit-Position: refs/branch-heads/3202@{#192} Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098} [modify] https://crrev.com/7b954f127154ef455795bed22c2fa89e2beed957/components/cryptauth/ble/bluetooth_low_energy_weave_client_connection.cc [modify] https://crrev.com/7b954f127154ef455795bed22c2fa89e2beed957/components/cryptauth/ble/bluetooth_low_energy_weave_client_connection.h [modify] https://crrev.com/7b954f127154ef455795bed22c2fa89e2beed957/components/cryptauth/ble/bluetooth_low_energy_weave_client_connection_unittest.cc
,
Sep 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9fd97e861f49add5af6ded0f3f01de542c30497f commit 9fd97e861f49add5af6ded0f3f01de542c30497f Author: Kyle Horimoto <khorimoto@google.com> Date: Wed Sep 13 01:48:30 2017 [CrOS Tether] Add timeouts for each step in creating Weave connection. There are several steps of the Bluetooth connection flow which call into Bluetooth APIs and wait for an asynchronous response. These asynchronous callbacks *should* be getting invoked in a timely manner by Bluetooth, but this is not happening in practice. This CL adds timeouts for each step of the connection process in which we wait for an asynchronous reply. TBR=khorimoto@google.com (cherry picked from commit b3e3134cdb044164ad384a87b56c670013f9284f) Bug: 763176 , 672263 Change-Id: Ia7a538e0de8280346a3c113c7dccd5c18cc7086d Reviewed-on: https://chromium-review.googlesource.com/657811 Commit-Queue: Kyle Horimoto <khorimoto@chromium.org> Reviewed-by: Tim Song <tengs@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#501418} Reviewed-on: https://chromium-review.googlesource.com/663981 Reviewed-by: Kyle Horimoto <khorimoto@chromium.org> Cr-Commit-Position: refs/branch-heads/3163@{#1182} Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528} [modify] https://crrev.com/9fd97e861f49add5af6ded0f3f01de542c30497f/components/cryptauth/ble/bluetooth_low_energy_weave_client_connection.cc [modify] https://crrev.com/9fd97e861f49add5af6ded0f3f01de542c30497f/components/cryptauth/ble/bluetooth_low_energy_weave_client_connection.h [modify] https://crrev.com/9fd97e861f49add5af6ded0f3f01de542c30497f/components/cryptauth/ble/bluetooth_low_energy_weave_client_connection_unittest.cc
,
Sep 13 2017
,
Sep 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/33b8a988b1dbfe4fce909bedd1e02b907865283c commit 33b8a988b1dbfe4fce909bedd1e02b907865283c Author: Kyle Horimoto <khorimoto@google.com> Date: Wed Sep 13 23:54:24 2017 [CrOS Tether] Fix timeout values for GATT connections. A previous CL accidentally used the wrong timeout values. Bug: 763176 , 672263 Change-Id: I64853429fe83fb05830d8032692629c6b0d99d4f Reviewed-on: https://chromium-review.googlesource.com/665513 Reviewed-by: Tim Song <tengs@chromium.org> Commit-Queue: Kyle Horimoto <khorimoto@chromium.org> Cr-Commit-Position: refs/heads/master@{#501807} [modify] https://crrev.com/33b8a988b1dbfe4fce909bedd1e02b907865283c/components/cryptauth/ble/bluetooth_low_energy_weave_client_connection.cc
,
Sep 14 2017
I had to land an additional CL to fix these timeout values. Requesting merge of this new CL to M61/M62.
,
Sep 14 2017
This bug requires manual review: Request affecting a post-stable build Please contact the milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), ketakid@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Sep 14 2017
Merge approved to 61 and 62.
,
Sep 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1fa1c4d32ad6decfb1a3fe6a5668b5b4ea29a3cb commit 1fa1c4d32ad6decfb1a3fe6a5668b5b4ea29a3cb Author: Kyle Horimoto <khorimoto@google.com> Date: Thu Sep 14 22:27:57 2017 [CrOS Tether] Fix timeout values for GATT connections. A previous CL accidentally used the wrong timeout values. TBR=khorimoto@google.com (cherry picked from commit 33b8a988b1dbfe4fce909bedd1e02b907865283c) Bug: 763176 , 672263 Change-Id: I64853429fe83fb05830d8032692629c6b0d99d4f Reviewed-on: https://chromium-review.googlesource.com/665513 Reviewed-by: Tim Song <tengs@chromium.org> Commit-Queue: Kyle Horimoto <khorimoto@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#501807} Reviewed-on: https://chromium-review.googlesource.com/666042 Reviewed-by: Kyle Horimoto <khorimoto@chromium.org> Cr-Commit-Position: refs/branch-heads/3202@{#236} Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098} [modify] https://crrev.com/1fa1c4d32ad6decfb1a3fe6a5668b5b4ea29a3cb/components/cryptauth/ble/bluetooth_low_energy_weave_client_connection.cc
,
Sep 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/09df30ff1bc33e9377397ca568fcabd5778010df commit 09df30ff1bc33e9377397ca568fcabd5778010df Author: Kyle Horimoto <khorimoto@google.com> Date: Thu Sep 14 22:29:04 2017 [CrOS Tether] Fix timeout values for GATT connections. A previous CL accidentally used the wrong timeout values. TBR=khorimoto@google.com (cherry picked from commit 33b8a988b1dbfe4fce909bedd1e02b907865283c) Bug: 763176 , 672263 Change-Id: I64853429fe83fb05830d8032692629c6b0d99d4f Reviewed-on: https://chromium-review.googlesource.com/665513 Reviewed-by: Tim Song <tengs@chromium.org> Commit-Queue: Kyle Horimoto <khorimoto@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#501807} Reviewed-on: https://chromium-review.googlesource.com/665887 Reviewed-by: Kyle Horimoto <khorimoto@chromium.org> Cr-Commit-Position: refs/branch-heads/3163@{#1200} Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528} [modify] https://crrev.com/09df30ff1bc33e9377397ca568fcabd5778010df/components/cryptauth/ble/bluetooth_low_energy_weave_client_connection.cc
,
Jan 22 2018
,
Jan 23 2018
|
|||||||||||||||
►
Sign in to add a comment |
|||||||||||||||
Comment 1 by khorimoto@chromium.org
, Sep 8 2017