device/blueooth: Add BlueZ unit test for BluetoothDevice::SetConnectionInterval.

device/bluetooth/dbus/fake_bluetooth_device_client.cc

Index: device/bluetooth/dbus/fake_bluetooth_device_client.cc
diff --git a/device/bluetooth/dbus/fake_bluetooth_device_client.cc b/device/bluetooth/dbus/fake_bluetooth_device_client.cc
index cc4eaefd168133e2254bf8b58a50028a5a22380d..8eef208ca96917165c4cd330034c3cfaaa4a7e87 100644
--- a/device/bluetooth/dbus/fake_bluetooth_device_client.cc
+++ b/device/bluetooth/dbus/fake_bluetooth_device_client.cc
@@ -601,8 +601,9 @@ void FakeBluetoothDeviceClient::SetLEConnectionParameters(
     const base::Closure& callback,
     const ErrorCallback& error_callback) {
   Properties* properties = GetProperties(object_path);
-  if (!properties->connected.value()) {
-    error_callback.Run(bluetooth_device::kErrorNotConnected, "Not Connected");
+  if (properties->type.value() == kTypeBredr) {

[ortuno, 2017/05/26 16:54:36]

I'm not that familiar with type as is not in upstream bluez.

Do we know if it's always going to be valid? What should we return if it isn't?

[Tim Song, 2017/05/26 19:56:18]

This property is exposed by the org.bluez.Device1 API:
https://cs.corp.google.com/chromeos_public/src/third_party/bluez/doc/device-a...

So I think it should be safe to assume this property is valid.

[ortuno, 2017/05/26 19:59:50]

It's optional though so it could still be invalid.

mcchou do you have any insight regarding 'type'?

[Miao, 2017/05/31 20:14:06]

Device Type is a CHROMIUM specific patch that upstream didn't adopt in my last
attempt, so it's not in BlueZ upstream.

The existence of property Type depends on the value of device->bredr and
device->le where their values are set during the creation of a device [1]. These
values are set initially during the device creation and updated later when
connecting to the device for the first time, searching the device based on
address type, or updating the address of the device. Theoretically, the device
can be invalid if none of these two value is set to true. However if looking
closer in BlueZ, you will find that at least one is set, and there is no method
to invalidate these two values during the lifetime of a device.

In Tim's previous patch of adding SetLEConnectionParameters method [2], the
underlying check on the type of a given device is based on the address type of
the device, but since the address type is not presented to upper layer. Checking
property Type is the closest way to mimic the actual check, and the value of
property Type also depends the address type. However, I agree with ortuno on the
validity of this property, so a check on the validity should be added here.

[1]
https://cs.corp.google.com/chromeos_public/src/third_party/bluez/src/device.c...
[2] https://chromium-review.googlesource.com/c/503432/

[Tim Song, 2017/06/01 18:06:03]

Done. I added the validity check.

+    error_callback.Run(bluetooth_device::kErrorFailed,
+                       "BR/EDR devices not supported");

[ortuno, 2017/05/26 16:54:36]

Should the call still work for non-connected devices?

[Tim Song, 2017/05/26 19:56:18]

Yes. One of the principle use cases is to set the connection parameters before
connecting.

I just screwed up implementing this fake method the first time around =p

[Miao, 2017/05/31 20:14:06]

The connection parameters can be set BEFORE or DURING the connection, so the
check on connected status is not needed.

[Tim Song, 2017/06/01 18:06:02]

Acknowledged.

     return;
   }