Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2425813003:
arc: bluetooth: Send missing advertise data to Android. (Closed)
|
Created:
4 years, 2 months ago by puthik_chromium Modified:
4 years, 1 month ago Reviewers:
Luis Héctor Chávez CC:
chromium-reviews, elijahtaylor+arcwatch_chromium.org, hidehiko+watch_chromium.org, lhchavez+watch_chromium.org, yusukes+watch_chromium.org, Rahul Chaturvedi Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Descriptionarc: bluetooth: Send missing advertise data to Android
This CL sends Manufacturer Data, Advertising Data Flags and
Tx power level to Android when discovering for new devices.
BUG=646584, 618442
TEST=nRF Connect App in minnie can see all advertise data
Committed: https://crrev.com/d922bbb7db1751071dc8fcfb819de56a76aeaf27
Cr-Commit-Position: refs/heads/master@{#430001}
Patch Set 1 #Patch Set 2 : rebase #
Total comments: 7
Patch Set 3 : Fix lhchavez comment #
Depends on Patchset: Messages
Total messages: 17 (10 generated)
puthik@chromium.org changed reviewers: + lhchavez@chromium.org
PTAL Note that this patch depends on http://crrev.com/2421713002
Description was changed from ========== arc: bluetooth: Send missing advertise data to Android. This CL sends Manufacturer Data and Advertising Data Flags to Android while discovering for new devices. BUG=646584, 618442 TEST=nRF Connect App in minnie can see all advertise data ========== to ========== arc: bluetooth: Send missing advertise data to Android This CL sends Manufacturer Data and Advertising Data Flags to Android while discovering for new devices. BUG=646584, 618442 TEST=nRF Connect App in minnie can see all advertise data ==========
Description was changed from ========== arc: bluetooth: Send missing advertise data to Android This CL sends Manufacturer Data and Advertising Data Flags to Android while discovering for new devices. BUG=646584, 618442 TEST=nRF Connect App in minnie can see all advertise data ========== to ========== arc: bluetooth: Send missing advertise data to Android This CL sends Manufacturer Data, Advertising Data Flags and Tx power level to Android when discovering for new devices. BUG=646584, 618442 TEST=nRF Connect App in minnie can see all advertise data ==========
https://codereview.chromium.org/2425813003/diff/20001/components/arc/bluetoot... File components/arc/bluetooth/arc_bluetooth_bridge.cc (right): https://codereview.chromium.org/2425813003/diff/20001/components/arc/bluetoot... components/arc/bluetooth/arc_bluetooth_bridge.cc:2011: // Local Name, Service UUIDs, Tx Power Level, Service Data and Manufacturer nit: Service Data, and Manufacturer Data. https://en.wikipedia.org/wiki/Serial_comma https://codereview.chromium.org/2425813003/diff/20001/components/arc/bluetoot... components/arc/bluetooth/arc_bluetooth_bridge.cc:2012: // Data. Note that we need to use UUID 16 bits in Service Data section because nit: 16-bit UUID, 128-bit UUID? https://codereview.chromium.org/2425813003/diff/20001/components/arc/bluetoot... components/arc/bluetooth/arc_bluetooth_bridge.cc:2072: // Manufacture Data nit: Manufacturer. Same below (in manufacture_data). https://codereview.chromium.org/2425813003/diff/20001/components/arc/bluetoot... components/arc/bluetooth/arc_bluetooth_bridge.cc:2074: for (const auto& pair : device->GetManufacturerData()) { maybe: if (!device->GetManufacturerData().empty()) { std::vector<uint8_t> ........ } that way you can skip the if on L2082.
https://codereview.chromium.org/2425813003/diff/20001/components/arc/bluetoot... File components/arc/bluetooth/arc_bluetooth_bridge.cc (right): https://codereview.chromium.org/2425813003/diff/20001/components/arc/bluetoot... components/arc/bluetooth/arc_bluetooth_bridge.cc:2012: // Data. Note that we need to use UUID 16 bits in Service Data section because On 2016/11/03 21:17:12, Luis Héctor Chávez wrote: > nit: 16-bit UUID, 128-bit UUID? Done. https://codereview.chromium.org/2425813003/diff/20001/components/arc/bluetoot... components/arc/bluetooth/arc_bluetooth_bridge.cc:2072: // Manufacture Data On 2016/11/03 21:17:12, Luis Héctor Chávez wrote: > nit: Manufacturer. Same below (in manufacture_data). Done. https://codereview.chromium.org/2425813003/diff/20001/components/arc/bluetoot... components/arc/bluetooth/arc_bluetooth_bridge.cc:2074: for (const auto& pair : device->GetManufacturerData()) { On 2016/11/03 21:17:11, Luis Héctor Chávez wrote: > maybe: > > if (!device->GetManufacturerData().empty()) { > std::vector<uint8_t> ........ > } > > that way you can skip the if on L2082. Done.
The CQ bit was checked by puthik@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...
lgtm
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 puthik@chromium.org
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 ========== arc: bluetooth: Send missing advertise data to Android This CL sends Manufacturer Data, Advertising Data Flags and Tx power level to Android when discovering for new devices. BUG=646584, 618442 TEST=nRF Connect App in minnie can see all advertise data ========== to ========== arc: bluetooth: Send missing advertise data to Android This CL sends Manufacturer Data, Advertising Data Flags and Tx power level to Android when discovering for new devices. BUG=646584, 618442 TEST=nRF Connect App in minnie can see all advertise data ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== arc: bluetooth: Send missing advertise data to Android This CL sends Manufacturer Data, Advertising Data Flags and Tx power level to Android when discovering for new devices. BUG=646584, 618442 TEST=nRF Connect App in minnie can see all advertise data ========== to ========== arc: bluetooth: Send missing advertise data to Android This CL sends Manufacturer Data, Advertising Data Flags and Tx power level to Android when discovering for new devices. BUG=646584, 618442 TEST=nRF Connect App in minnie can see all advertise data Committed: https://crrev.com/d922bbb7db1751071dc8fcfb819de56a76aeaf27 Cr-Commit-Position: refs/heads/master@{#430001} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/d922bbb7db1751071dc8fcfb819de56a76aeaf27 Cr-Commit-Position: refs/heads/master@{#430001} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
