[chromecast] Some fixes for Web BT

- Automatically deduce WriteType based on properties - Set initial connected State on RemoteDevice BUG=internal b/75967216 TEST=cast_bluetooth_unittests. Manual Change-Id: I7b06f2cfaae180b4e76322675a02712fdff69d04 Reviewed-on: https://chromium-review.googlesource.com/1038754 Commit-Queue: Bailey Forrest <bcf@chromium.org> Reviewed-by: Stephen Lanham <slan@chromium.org> Cr-Commit-Position: refs/heads/master@{#555601}
FairyWorld · May 2, 2018 · e4c2ba3 · e4c2ba3
1 parent 73b18e3
commit e4c2ba3
Show file tree

Hide file tree

Showing 8 changed files with 99 additions and 27 deletions.
diff --git a/chromecast/device/bluetooth/le/gatt_client_manager_impl_test.cc b/chromecast/device/bluetooth/le/gatt_client_manager_impl_test.cc
@@ -289,13 +289,6 @@ TEST_F(GattClientManagerTest, RemoteDeviceServices) {
   bluetooth_v2_shlib::Gatt::Client::Delegate* delegate =
       gatt_client_->delegate();
   delegate->OnServicesAdded(kTestAddr1, kServices);
-
-  device->GetServices(base::BindOnce(
-      [](std::vector<scoped_refptr<RemoteService>>* pservices,
-         std::vector<scoped_refptr<RemoteService>> services) {
-        *pservices = services;
-      },
-      &services));
   base::RunLoop().RunUntilIdle();
 
   EXPECT_EQ(kServices.size(), GetServices(device.get()).size());
@@ -528,5 +521,74 @@ TEST_F(GattClientManagerTest, FakeCccd) {
   }
 }
 
+TEST_F(GattClientManagerTest, WriteType) {
+  const std::vector<uint8_t> kTestData1 = {0x1, 0x2, 0x3};
+
+  bluetooth_v2_shlib::Gatt::Service service;
+  bluetooth_v2_shlib::Gatt::Characteristic characteristic;
+
+  service.uuid = {{0x1}};
+  service.handle = 0x1;
+  service.primary = true;
+
+  characteristic.uuid = {{0x1, 0x1}};
+  characteristic.handle = 0x2;
+  characteristic.permissions = bluetooth_v2_shlib::Gatt::PERMISSION_WRITE;
+  characteristic.properties = bluetooth_v2_shlib::Gatt::PROPERTY_WRITE;
+  service.characteristics.push_back(characteristic);
+
+  characteristic.uuid = {{0x1, 0x2}};
+  characteristic.handle = 0x3;
+  characteristic.permissions = bluetooth_v2_shlib::Gatt::PERMISSION_WRITE;
+  characteristic.properties =
+      bluetooth_v2_shlib::Gatt::PROPERTY_WRITE_NO_RESPONSE;
+  service.characteristics.push_back(characteristic);
+
+  characteristic.uuid = {{0x1, 0x3}};
+  characteristic.handle = 0x4;
+  characteristic.permissions = bluetooth_v2_shlib::Gatt::PERMISSION_WRITE;
+  characteristic.properties = bluetooth_v2_shlib::Gatt::PROPERTY_SIGNED_WRITE;
+  service.characteristics.push_back(characteristic);
+
+  Connect(kTestAddr1);
+  bluetooth_v2_shlib::Gatt::Client::Delegate* delegate =
+      gatt_client_->delegate();
+  delegate->OnServicesAdded(kTestAddr1, {service});
+
+  scoped_refptr<RemoteDevice> device = GetDevice(kTestAddr1);
+
+  std::vector<scoped_refptr<RemoteService>> services =
+      GetServices(device.get());
+  ASSERT_EQ(1u, services.size());
+
+  std::vector<scoped_refptr<RemoteCharacteristic>> characteristics =
+      services[0]->GetCharacteristics();
+  ASSERT_EQ(3u, characteristics.size());
+
+  using WriteType = bluetooth_v2_shlib::Gatt::WriteType;
+
+  // The current implementation of RemoteDevice will put the characteristics in
+  // the order reported by libcast_bluetooth.
+  const WriteType kWriteTypes[] = {WriteType::WRITE_TYPE_DEFAULT,
+                                   WriteType::WRITE_TYPE_NO_RESPONSE,
+                                   WriteType::WRITE_TYPE_SIGNED};
+
+  for (size_t i = 0; i < characteristics.size(); ++i) {
+    const auto& characteristic = characteristics[i];
+    EXPECT_CALL(
+        *gatt_client_,
+        WriteCharacteristic(kTestAddr1, characteristic->characteristic(),
+                            bluetooth_v2_shlib::Gatt::Client::AUTH_REQ_NONE,
+                            kWriteTypes[i], kTestData1))
+        .WillOnce(Return(true));
+
+    base::MockCallback<RemoteCharacteristic::StatusCallback> write_cb;
+    EXPECT_CALL(write_cb, Run(true));
+    characteristic->Write(kTestData1, write_cb.Get());
+    delegate->OnCharacteristicWriteResponse(kTestAddr1, true,
+                                            characteristic->handle());
+  }
+}
+
 }  // namespace bluetooth
 }  // namespace chromecast
diff --git a/chromecast/device/bluetooth/le/mock_remote_characteristic.h b/chromecast/device/bluetooth/le/mock_remote_characteristic.h
@@ -44,13 +44,10 @@ class MockRemoteCharacteristic : public RemoteCharacteristic {
                  const std::vector<uint8_t>& value,
                  StatusCallback callback) override {}
 
-  MOCK_METHOD2(Write,
-               bool(bluetooth_v2_shlib::Gatt::WriteType write_type,
-                    const std::vector<uint8_t>& value));
-  void Write(bluetooth_v2_shlib::Gatt::WriteType write_type,
-             const std::vector<uint8_t>& value,
+  MOCK_METHOD1(Write, bool(const std::vector<uint8_t>& value));
+  void Write(const std::vector<uint8_t>& value,
              StatusCallback callback) override {
-    std::move(callback).Run(Write(write_type, value));
+    std::move(callback).Run(Write(value));
   }
 
   MOCK_METHOD0(NotificationEnabled, bool());

diff --git a/chromecast/device/bluetooth/le/remote_characteristic.h b/chromecast/device/bluetooth/le/remote_characteristic.h
@@ -61,10 +61,10 @@ class RemoteCharacteristic
                          const std::vector<uint8_t>& value,
                          StatusCallback callback) = 0;
 
-  // Write |value| to the characteristic with |write_type|. Will retry if
-  // auth_req isn't met. When completed, |callback| will be called.
-  virtual void Write(bluetooth_v2_shlib::Gatt::WriteType write_type,
-                     const std::vector<uint8_t>& value,
+  // Write |value| to the characteristic inferring write_type from
+  // |permissions()|. Will retry if auth_req isn't met. When completed,
+  // |callback| will be called.
+  virtual void Write(const std::vector<uint8_t>& value,
                      StatusCallback callback) = 0;
 
   // Returns true if notifications are enabled.

diff --git a/chromecast/device/bluetooth/le/remote_characteristic_impl.cc b/chromecast/device/bluetooth/le/remote_characteristic_impl.cc
@@ -234,12 +234,26 @@ void RemoteCharacteristicImpl::WriteAuth(
   write_callback_ = std::move(callback);
 }
 
-void RemoteCharacteristicImpl::Write(
-    bluetooth_v2_shlib::Gatt::WriteType write_type,
-    const std::vector<uint8_t>& value,
-    StatusCallback callback) {
-  return WriteAuth(bluetooth_v2_shlib::Gatt::Client::AUTH_REQ_NONE, write_type,
-                   value, std::move(callback));
+void RemoteCharacteristicImpl::Write(const std::vector<uint8_t>& value,
+                                     StatusCallback callback) {
+  using WriteType = bluetooth_v2_shlib::Gatt::WriteType;
+  using Properties = bluetooth_v2_shlib::Gatt::Properties;
+
+  WriteType write_type = WriteType::WRITE_TYPE_NONE;
+  if (properties() & Properties::PROPERTY_WRITE) {
+    write_type = WriteType::WRITE_TYPE_DEFAULT;
+  } else if (properties() & Properties::PROPERTY_WRITE_NO_RESPONSE) {
+    write_type = WriteType::WRITE_TYPE_NO_RESPONSE;
+  } else if (properties() & Properties::PROPERTY_SIGNED_WRITE) {
+    write_type = WriteType::WRITE_TYPE_SIGNED;
+  } else {
+    LOG(ERROR) << "Write not supported. Properties: "
+               << static_cast<int>(properties());
+    EXEC_CB_AND_RET(callback, false);
+  }
+
+  WriteAuth(bluetooth_v2_shlib::Gatt::Client::AUTH_REQ_NONE, write_type, value,
+            std::move(callback));
 }
 
 bool RemoteCharacteristicImpl::NotificationEnabled() {

diff --git a/chromecast/device/bluetooth/le/remote_characteristic_impl.h b/chromecast/device/bluetooth/le/remote_characteristic_impl.h
@@ -37,8 +37,7 @@ class RemoteCharacteristicImpl : public RemoteCharacteristic {
                  bluetooth_v2_shlib::Gatt::WriteType write_type,
                  const std::vector<uint8_t>& value,
                  StatusCallback callback) override;
-  void Write(bluetooth_v2_shlib::Gatt::WriteType write_type,
-             const std::vector<uint8_t>& value,
+  void Write(const std::vector<uint8_t>& value,
              StatusCallback callback) override;
   bool NotificationEnabled() override;
   const bluetooth_v2_shlib::Gatt::Characteristic& characteristic()

diff --git a/device/bluetooth/cast/bluetooth_device_cast.cc b/device/bluetooth/cast/bluetooth_device_cast.cc
@@ -57,6 +57,7 @@ BluetoothDeviceCast::BluetoothDeviceCast(
     BluetoothAdapter* adapter,
     scoped_refptr<chromecast::bluetooth::RemoteDevice> device)
     : BluetoothDevice(adapter),
+      connected_(device->IsConnected()),
       remote_device_(std::move(device)),
       address_(GetCanonicalBluetoothAddress(remote_device_->addr())),
       weak_factory_(this) {}

diff --git a/device/bluetooth/cast/bluetooth_device_cast.h b/device/bluetooth/cast/bluetooth_device_cast.h
@@ -132,7 +132,7 @@ class BluetoothDeviceCast : public BluetoothDevice {
   // Called back from disconnect requests.
   void OnDisconnect(bool success);
 
-  bool connected_ = false;
+  bool connected_;
   bool pending_connect_ = false;
   bool pending_disconnect_ = false;
 

diff --git a/device/bluetooth/cast/bluetooth_remote_gatt_characteristic_cast.cc b/device/bluetooth/cast/bluetooth_remote_gatt_characteristic_cast.cc
@@ -164,7 +164,6 @@ void BluetoothRemoteGattCharacteristicCast::WriteRemoteCharacteristic(
     const base::Closure& callback,
     const ErrorCallback& error_callback) {
   remote_characteristic_->Write(
-      chromecast::bluetooth_v2_shlib::Gatt::WriteType::WRITE_TYPE_DEFAULT,
       value,
       base::BindOnce(
           &BluetoothRemoteGattCharacteristicCast::OnWriteRemoteCharacteristic,