Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2230703002:
bluetooth: android: Record GATT status codes UMA. (Closed)
|
Created:
4 years, 4 months ago by scheib Modified:
4 years, 4 months ago CC:
chromium-reviews, scheib+watch_chromium.org, ortuno+watch_chromium.org, asvitkine+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Descriptionbluetooth: android: Record GATT status codes UMA.
Android's API is underdocumented, and to help understand what errors
are actually being encountered in the wild we are adding UMA to capture
the error code values for results of GATT operations.
BUG=602060
Committed: https://crrev.com/d5980418932d12bfa1f6884473247961d1e4151b
Cr-Commit-Position: refs/heads/master@{#411878}
Patch Set 1 : #
Total comments: 4
Patch Set 2 : Add more status measurements #Patch Set 3 : Add more status reports and GATT codes #Patch Set 4 : Add more status reports and GATT codes++ #
Messages
Total messages: 43 (35 generated)
The CQ bit was checked by scheib@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by scheib@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Patchset #1 (id:1) has been deleted
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by scheib@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by scheib@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
scheib@chromium.org changed reviewers: + jyasskin@chromium.org
lgtm https://codereview.chromium.org/2230703002/diff/20001/device/bluetooth/androi... File device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothDevice.java (right): https://codereview.chromium.org/2230703002/diff/20001/device/bluetooth/androi... device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothDevice.java:151: public void onConnectionStateChange(final int status, final int newState) { Should we record the other status variables too? What distinguishes the ServicesDiscovered status from the ones you're not histogramming? https://codereview.chromium.org/2230703002/diff/20001/device/bluetooth/androi... device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothDevice.java:183: // When the device disconnects is deletes s/is/it/?
The CQ bit was checked by scheib@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
The CQ bit was checked by scheib@chromium.org to run a CQ dry run
Description was changed from ========== bluetooth: android: Record status codes of onServicesDiscovered Android's API is underdocumented, and to help understand what errors are actually being encountered in the wild we are adding UMA to capture the error code values. BUG=602060 ========== to ========== bluetooth: android: Record GATT status codes UMA. Android's API is underdocumented, and to help understand what errors are actually being encountered in the wild we are adding UMA to capture the error code values for results of GATT operations. BUG=602060 ==========
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
scheib@chromium.org changed reviewers: + holte@chromium.org
holte for metrics OWNER. https://codereview.chromium.org/2230703002/diff/20001/device/bluetooth/androi... File device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothDevice.java (right): https://codereview.chromium.org/2230703002/diff/20001/device/bluetooth/androi... device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothDevice.java:151: public void onConnectionStateChange(final int status, final int newState) { On 2016/08/11 21:12:43, Jeffrey Yasskin wrote: > Should we record the other status variables too? What distinguishes the > ServicesDiscovered status from the ones you're not histogramming? Done. onServicesDiscovered just drew attention early on as it prompted the question of what to do in the event of error. But it would be good to know all around. https://codereview.chromium.org/2230703002/diff/20001/device/bluetooth/androi... device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothDevice.java:183: // When the device disconnects is deletes On 2016/08/11 21:12:43, Jeffrey Yasskin wrote: > s/is/it/? Done.
The CQ bit was checked by scheib@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Patchset #4 (id:80001) has been deleted
The CQ bit was checked by scheib@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2016/08/11 22:35:51, scheib wrote: > holte for metrics OWNER. > > https://codereview.chromium.org/2230703002/diff/20001/device/bluetooth/androi... > File > device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothDevice.java > (right): > > https://codereview.chromium.org/2230703002/diff/20001/device/bluetooth/androi... > device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothDevice.java:151: > public void onConnectionStateChange(final int status, final int newState) { > On 2016/08/11 21:12:43, Jeffrey Yasskin wrote: > > Should we record the other status variables too? What distinguishes the > > ServicesDiscovered status from the ones you're not histogramming? > > Done. onServicesDiscovered just drew attention early on as it prompted the > question of what to do in the event of error. But it would be good to know all > around. > > https://codereview.chromium.org/2230703002/diff/20001/device/bluetooth/androi... > device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothDevice.java:183: > // When the device disconnects is deletes > On 2016/08/11 21:12:43, Jeffrey Yasskin wrote: > > s/is/it/? > > Done. lgtm, but this is probably a good use case for using histogram_suffixes for the histogram descriptions.
Thanks, I didn't know about them -- and will follow up with a patch to clean up to use them.
The CQ bit was checked by scheib@chromium.org
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/2230703002/#ps100001 (title: "Add more status reports and GATT codes++")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== bluetooth: android: Record GATT status codes UMA. Android's API is underdocumented, and to help understand what errors are actually being encountered in the wild we are adding UMA to capture the error code values for results of GATT operations. BUG=602060 ========== to ========== bluetooth: android: Record GATT status codes UMA. Android's API is underdocumented, and to help understand what errors are actually being encountered in the wild we are adding UMA to capture the error code values for results of GATT operations. BUG=602060 ==========
Message was sent while issue was closed.
Committed patchset #4 (id:100001)
Message was sent while issue was closed.
Description was changed from ========== bluetooth: android: Record GATT status codes UMA. Android's API is underdocumented, and to help understand what errors are actually being encountered in the wild we are adding UMA to capture the error code values for results of GATT operations. BUG=602060 ========== to ========== bluetooth: android: Record GATT status codes UMA. Android's API is underdocumented, and to help understand what errors are actually being encountered in the wild we are adding UMA to capture the error code values for results of GATT operations. BUG=602060 Committed: https://crrev.com/d5980418932d12bfa1f6884473247961d1e4151b Cr-Commit-Position: refs/heads/master@{#411878} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/d5980418932d12bfa1f6884473247961d1e4151b Cr-Commit-Position: refs/heads/master@{#411878} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
