device/bluetooth/public/interfaces/bluetooth.mojom - Issue 2357383002: bluetooth: Add device list retrieval for chrome://bluetooth-internals

Side by Side Diff: device/bluetooth/public/interfaces/bluetooth.mojom

Issue 2357383002: bluetooth: Add device list retrieval for chrome://bluetooth-internals (Closed)

Patch Set: Created 4 years, 2 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

OLD	NEW
(Empty)
	1 // Copyright 2016 The Chromium Authors. All rights reserved.

	2
	ortuno 2016/09/22 08:32:54 nit: remove new line nit: remove new line mbrunson 2016/09/24 01:05:47 Done. Show quoted text On 2016/09/22 08:32:54, ortuno wrote: > nit: remove new line Done.
	3 // Use of this source code is governed by a BSD-style license that can be

	4 // found in the LICENSE file.

	5

	6 module bluetooth;
	ortuno 2016/09/22 08:32:54 scheib: Should we add a note mentioning that this scheib: Should we add a note mentioning that this is experimental and shouldn't be used by any clients that what a stable API? scheib 2016/09/23 20:53:51 Use: module bluetooth.mojom I think we should use Show quoted text On 2016/09/22 08:32:54, ortuno wrote: > scheib: Should we add a note mentioning that this is experimental and shouldn't > be used by any clients that what a stable API? Use: module bluetooth.mojom I think we should use gn's visibility to restrict access for now: https://chromium.googlesource.com/chromium/src/+/master/tools/gn/docs/check.md
	7

	8 struct DeviceInfo {

	9 string? name;

	10 string name_for_display;

	11 string id;

	12 string address;

	13 uint16 device_id;
	ortuno 2016/09/22 08:32:54 nit: there was a comment about this on the design nit: there was a comment about this on the design doc. I'm OK with landing this and then removing depending on the outcome of that discussion but if you can get a resolution sooner it would be better. mbrunson 2016/09/24 01:05:47 I think just removing it entirely for now would be Show quoted text On 2016/09/22 08:32:54, ortuno wrote: > nit: there was a comment about this on the design doc. I'm OK with landing this > and then removing depending on the outcome of that discussion but if you can get > a resolution sooner it would be better. I think just removing it entirely for now would be better. Many, many things will be added to this file later so I think it's OK to omit things we're not sure about for now unless it hinders manual testing.
	14 uint16 product_id;

	15 uint16 vendor_id;

	16 };

	17

	18 interface Adapter {
	scheib 2016/09/23 20:53:51 ortuno: what do you think here, I think that we sh ortuno: what do you think here, I think that we should keep related interfaces in individual files. e.g. this file should be renamed to adapter.mojom, and the internals interface moved out to chrome/browser/ui/webui/bluetooth/bluetooth_internals.mojom mbrunson 2016/09/24 01:05:47 The internals interface was not supposed to be in Show quoted text On 2016/09/23 20:53:51, scheib wrote: > ortuno: what do you think here, > I think that we should keep related interfaces in individual files. e.g. this > file should be renamed to adapter.mojom, and the internals interface moved out > to > chrome/browser/ui/webui/bluetooth/bluetooth_internals.mojom The internals interface was not supposed to be in here to begin with. I must have left it in here when I copied this file to make the bluetooth_internals.mojom. ortuno 2016/09/26 01:57:56 Separating in the interfaces in files sounds good. Show quoted text On 2016/09/24 at 01:05:47, mbrunson wrote: > On 2016/09/23 20:53:51, scheib wrote: > > ortuno: what do you think here, > > I think that we should keep related interfaces in individual files. e.g. this > > file should be renamed to adapter.mojom, and the internals interface moved out > > to > > chrome/browser/ui/webui/bluetooth/bluetooth_internals.mojom > > The internals interface was not supposed to be in here to begin with. I must have left it in here when I copied this file to make the bluetooth_internals.mojom. Separating in the interfaces in files sounds good.
	19 // Retrieves the list of devices that can be detected
	ortuno 2016/09/22 08:32:54 I want to get rid of this function but since that I want to get rid of this function but since that discussion is still ongoing I think it's OK to land this but we should add more detail regarding what the function returns i.e. Retrieves the list of the devices known by the adapter. This includes Connected Devices, GATT Connected Devices, Paired Devices and Devices discovered by during a classic of low-energy scan. nit: Period at the end of sentence. mbrunson 2016/09/24 01:05:47 Done. Show quoted text On 2016/09/22 08:32:54, ortuno wrote: > I want to get rid of this function but since that discussion is still ongoing I > think it's OK to land this but we should add more detail regarding what the > function returns i.e. Retrieves the list of the devices known by the adapter. > This includes Connected Devices, GATT Connected Devices, Paired Devices and > Devices discovered by during a classic of low-energy scan. > > nit: Period at the end of sentence. Done.
	20 GetDevices(int8 index, int8 count) => (array<DeviceInfo> devices);
	ortuno 2016/09/22 08:32:54 Could you mention the purpose of the two arguments Could you mention the purpose of the two arguments in the comment? Also question: What's the use case for them? ortuno 2016/09/22 08:32:54 Could you mention the purpose of the two arguments Could you mention the purpose of the two arguments in the comment? Also question: What's the use case for them? mbrunson 2016/09/24 01:05:47 These should probably be removed. The original use Show quoted text On 2016/09/22 08:32:54, ortuno wrote: > Could you mention the purpose of the two arguments in the comment? > > Also question: What's the use case for them? These should probably be removed. The original use case was for pagination on the internals page but with the move to a more generic interface that seems like something that should be handled by the consumer of this.
	21 };

	22

	23 interface AdapterClient {

	24 // Called when a device has been detected
	ortuno 2016/09/22 08:32:54 nit: Called the first time a device is discovered. nit: Called the first time a device is discovered. Also period at the end of sentence. mbrunson 2016/09/24 01:05:47 Done. Show quoted text On 2016/09/22 08:32:54, ortuno wrote: > nit: Called the first time a device is discovered. > > Also period at the end of sentence. Done.
	25 DeviceAdded(DeviceInfo device);

	26

	27 // Called when a device can no longer be detected

	28 DeviceRemoved(DeviceInfo device);
	ortuno 2016/09/22 08:32:54 nit: Called after the device hasn't been seen for nit: Called after the device hasn't been seen for 3 minutes. Also period at the end of sentence. mbrunson 2016/09/24 01:05:47 Done. Show quoted text On 2016/09/22 08:32:54, ortuno wrote: > nit: Called after the device hasn't been seen for 3 minutes. > > Also period at the end of sentence. Done.
	29 };

	30

	31 interface InternalsPageHandler {
	scheib 2016/09/23 20:53:51 This interface shouldn't be in device/bluetooth This interface shouldn't be in device/bluetooth mbrunson 2016/09/24 01:05:47 Done. Show quoted text On 2016/09/23 20:53:51, scheib wrote: > This interface shouldn't be in device/bluetooth Done.
	32 // Creates pipe for Bluetooth Adapter between \|client\| and \|adapter\|
	ortuno 2016/09/22 08:32:54 nit: Period at the end of sentence. nit: Period at the end of sentence. mbrunson 2016/09/24 01:05:47 Done. Show quoted text On 2016/09/22 08:32:54, ortuno wrote: > nit: Period at the end of sentence. Done.
	33 GetAdapterService(Adapter& adapter,

	34 AdapterClient client);

	35 };

OLD	NEW

« device/bluetooth/public/interfaces/BUILD.gn ('K') | « device/bluetooth/public/interfaces/BUILD.gn ('k') | mojo/public/js/connection.js » ('j') | mojo/public/js/connection.js » ('J')