Skip to content

Commit

Permalink
device/bluetooth: Implement BluetoothGattConnection on Chrome OS.
Browse files Browse the repository at this point in the history
This patch adds BluetoothGattConnectionChromeOS and implements
BluetoothDeviceChromeOS::CreateGattConnection. At this point, creating
a connection simply calls org.bluez.Device1.Connect without pairing.
BluetoothGattConnectionChromeOS::Disconnect currently doesn't do anything,
at least until there is a system in bluetoothd for managing GATT connections
properly between Chrome and its profiles and plugins.

BUG=384620,381305
TEST=device_unittests

Review URL: https://codereview.chromium.org/333983003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@278595 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
armansito@chromium.org committed Jun 20, 2014
1 parent 05adc15 commit aa2ec2a
Show file tree
Hide file tree
Showing 9 changed files with 328 additions and 3 deletions.
3 changes: 2 additions & 1 deletion chromeos/dbus/fake_bluetooth_device_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,8 @@ void FakeBluetoothDeviceClient::Connect(
}

if (properties->paired.value() != true &&
object_path != dbus::ObjectPath(kConnectUnpairablePath)) {
object_path != dbus::ObjectPath(kConnectUnpairablePath) &&
object_path != dbus::ObjectPath(kLowEnergyPath)) {
// Must be paired.
error_callback.Run(bluetooth_device::kErrorFailed, "Not paired");
return;
Expand Down
4 changes: 4 additions & 0 deletions device/bluetooth/bluetooth.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@
'bluetooth_gatt_characteristic.h',
'bluetooth_gatt_connection.cc',
'bluetooth_gatt_connection.h',
'bluetooth_gatt_connection_chromeos.cc',
'bluetooth_gatt_connection_chromeos.h',
'bluetooth_gatt_descriptor.cc',
'bluetooth_gatt_descriptor.h',
'bluetooth_gatt_service.cc',
Expand Down Expand Up @@ -135,6 +137,8 @@
'test/mock_bluetooth_discovery_session.h',
'test/mock_bluetooth_gatt_characteristic.cc',
'test/mock_bluetooth_gatt_characteristic.h',
'test/mock_bluetooth_gatt_connection.cc',
'test/mock_bluetooth_gatt_connection.h',
'test/mock_bluetooth_gatt_descriptor.cc',
'test/mock_bluetooth_gatt_descriptor.h',
'test/mock_bluetooth_gatt_service.cc',
Expand Down
18 changes: 16 additions & 2 deletions device/bluetooth/bluetooth_device_chromeos.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "chromeos/dbus/dbus_thread_manager.h"
#include "dbus/bus.h"
#include "device/bluetooth/bluetooth_adapter_chromeos.h"
#include "device/bluetooth/bluetooth_gatt_connection_chromeos.h"
#include "device/bluetooth/bluetooth_pairing_chromeos.h"
#include "device/bluetooth/bluetooth_remote_gatt_service_chromeos.h"
#include "device/bluetooth/bluetooth_socket.h"
Expand Down Expand Up @@ -453,8 +454,13 @@ void BluetoothDeviceChromeOS::ConnectToService(
void BluetoothDeviceChromeOS::CreateGattConnection(
const GattConnectionCallback& callback,
const ConnectErrorCallback& error_callback) {
// TODO(armansito): Implement.
error_callback.Run(ERROR_UNSUPPORTED_DEVICE);
// TODO(armansito): Until there is a way to create a reference counted GATT
// connection in bluetoothd, simply do a regular connect.
Connect(NULL,
base::Bind(&BluetoothDeviceChromeOS::OnCreateGattConnection,
weak_ptr_factory_.GetWeakPtr(),
callback),
error_callback);
}

void BluetoothDeviceChromeOS::StartConnectionMonitor(
Expand Down Expand Up @@ -568,6 +574,14 @@ void BluetoothDeviceChromeOS::OnConnect(bool after_pairing,
callback.Run();
}

void BluetoothDeviceChromeOS::OnCreateGattConnection(
const GattConnectionCallback& callback) {
scoped_ptr<device::BluetoothGattConnection> conn(
new BluetoothGattConnectionChromeOS(
adapter_, GetAddress(), object_path_));
callback.Run(conn.Pass());
}

void BluetoothDeviceChromeOS::OnConnectError(
bool after_pairing,
const ConnectErrorCallback& error_callback,
Expand Down
1 change: 1 addition & 0 deletions device/bluetooth/bluetooth_device_chromeos.h
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ class BluetoothDeviceChromeOS
const ConnectErrorCallback& error_callback);
void OnConnect(bool after_pairing,
const base::Closure& callback);
void OnCreateGattConnection(const GattConnectionCallback& callback);
void OnConnectError(bool after_pairing,
const ConnectErrorCallback& error_callback,
const std::string& error_name,
Expand Down
88 changes: 88 additions & 0 deletions device/bluetooth/bluetooth_gatt_chromeos_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "device/bluetooth/bluetooth_adapter_factory.h"
#include "device/bluetooth/bluetooth_device.h"
#include "device/bluetooth/bluetooth_gatt_characteristic.h"
#include "device/bluetooth/bluetooth_gatt_connection.h"
#include "device/bluetooth/bluetooth_gatt_descriptor.h"
#include "device/bluetooth/bluetooth_gatt_service.h"
#include "device/bluetooth/bluetooth_uuid.h"
Expand All @@ -25,6 +26,7 @@
using device::BluetoothAdapter;
using device::BluetoothDevice;
using device::BluetoothGattCharacteristic;
using device::BluetoothGattConnection;
using device::BluetoothGattDescriptor;
using device::BluetoothGattService;
using device::BluetoothUUID;
Expand Down Expand Up @@ -341,6 +343,7 @@ class BluetoothGattChromeOSTest : public testing::Test {

virtual void TearDown() {
adapter_ = NULL;
gatt_conn_.reset();
DBusThreadManager::Shutdown();
}

Expand All @@ -366,10 +369,19 @@ class BluetoothGattChromeOSTest : public testing::Test {
last_read_value_ = value;
}

void GattConnectionCallback(scoped_ptr<BluetoothGattConnection> conn) {
++success_callback_count_;
gatt_conn_ = conn.Pass();
}

void ErrorCallback() {
++error_callback_count_;
}

void ConnectErrorCallback(BluetoothDevice::ConnectErrorCode error) {
++error_callback_count_;
}

protected:
base::MessageLoop message_loop_;

Expand All @@ -378,13 +390,89 @@ class BluetoothGattChromeOSTest : public testing::Test {
FakeBluetoothGattCharacteristicClient*
fake_bluetooth_gatt_characteristic_client_;
FakeBluetoothGattDescriptorClient* fake_bluetooth_gatt_descriptor_client_;
scoped_ptr<device::BluetoothGattConnection> gatt_conn_;
scoped_refptr<BluetoothAdapter> adapter_;

int success_callback_count_;
int error_callback_count_;
std::vector<uint8> last_read_value_;
};

TEST_F(BluetoothGattChromeOSTest, GattConnection) {
fake_bluetooth_device_client_->CreateDevice(
dbus::ObjectPath(FakeBluetoothAdapterClient::kAdapterPath),
dbus::ObjectPath(FakeBluetoothDeviceClient::kLowEnergyPath));
BluetoothDevice* device = adapter_->GetDevice(
FakeBluetoothDeviceClient::kLowEnergyAddress);
ASSERT_TRUE(device);
ASSERT_FALSE(device->IsConnected());
ASSERT_FALSE(gatt_conn_.get());
ASSERT_EQ(0, success_callback_count_);
ASSERT_EQ(0, error_callback_count_);

device->CreateGattConnection(
base::Bind(&BluetoothGattChromeOSTest::GattConnectionCallback,
base::Unretained(this)),
base::Bind(&BluetoothGattChromeOSTest::ConnectErrorCallback,
base::Unretained(this)));

EXPECT_EQ(1, success_callback_count_);
EXPECT_EQ(0, error_callback_count_);
EXPECT_TRUE(device->IsConnected());
ASSERT_TRUE(gatt_conn_.get());
EXPECT_TRUE(gatt_conn_->IsConnected());
EXPECT_EQ(FakeBluetoothDeviceClient::kLowEnergyAddress,
gatt_conn_->GetDeviceAddress());

gatt_conn_->Disconnect(
base::Bind(&BluetoothGattChromeOSTest::SuccessCallback,
base::Unretained(this)));
EXPECT_EQ(2, success_callback_count_);
EXPECT_EQ(0, error_callback_count_);
EXPECT_TRUE(device->IsConnected());
EXPECT_FALSE(gatt_conn_->IsConnected());

device->CreateGattConnection(
base::Bind(&BluetoothGattChromeOSTest::GattConnectionCallback,
base::Unretained(this)),
base::Bind(&BluetoothGattChromeOSTest::ConnectErrorCallback,
base::Unretained(this)));

EXPECT_EQ(3, success_callback_count_);
EXPECT_EQ(0, error_callback_count_);
EXPECT_TRUE(device->IsConnected());
ASSERT_TRUE(gatt_conn_.get());
EXPECT_TRUE(gatt_conn_->IsConnected());

device->Disconnect(
base::Bind(&BluetoothGattChromeOSTest::SuccessCallback,
base::Unretained(this)),
base::Bind(&BluetoothGattChromeOSTest::ErrorCallback,
base::Unretained(this)));

EXPECT_EQ(4, success_callback_count_);
EXPECT_EQ(0, error_callback_count_);
ASSERT_TRUE(gatt_conn_.get());
EXPECT_FALSE(gatt_conn_->IsConnected());

device->CreateGattConnection(
base::Bind(&BluetoothGattChromeOSTest::GattConnectionCallback,
base::Unretained(this)),
base::Bind(&BluetoothGattChromeOSTest::ConnectErrorCallback,
base::Unretained(this)));

EXPECT_EQ(5, success_callback_count_);
EXPECT_EQ(0, error_callback_count_);
EXPECT_TRUE(device->IsConnected());
EXPECT_TRUE(gatt_conn_->IsConnected());

fake_bluetooth_device_client_->RemoveDevice(
dbus::ObjectPath(FakeBluetoothAdapterClient::kAdapterPath),
dbus::ObjectPath(FakeBluetoothDeviceClient::kLowEnergyPath));
ASSERT_TRUE(gatt_conn_.get());
EXPECT_FALSE(gatt_conn_->IsConnected());
}

TEST_F(BluetoothGattChromeOSTest, GattServiceAddedAndRemoved) {
// Create a fake LE device. We store the device pointer here because this is a
// test. It's unsafe to do this in production as the device might get deleted.
Expand Down
107 changes: 107 additions & 0 deletions device/bluetooth/bluetooth_gatt_connection_chromeos.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
// Copyright 2014 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "device/bluetooth/bluetooth_gatt_connection_chromeos.h"

#include "base/bind.h"
#include "base/logging.h"
#include "chromeos/dbus/dbus_thread_manager.h"
#include "device/bluetooth/bluetooth_adapter.h"
#include "device/bluetooth/bluetooth_device.h"

namespace chromeos {

BluetoothGattConnectionChromeOS::BluetoothGattConnectionChromeOS(
scoped_refptr<device::BluetoothAdapter> adapter,
const std::string& device_address,
const dbus::ObjectPath& object_path)
: connected_(true),
adapter_(adapter),
device_address_(device_address),
object_path_(object_path) {
DCHECK(adapter_.get());
DCHECK(!device_address_.empty());
DCHECK(object_path_.IsValid());

DBusThreadManager::Get()->GetBluetoothDeviceClient()->AddObserver(this);
}

BluetoothGattConnectionChromeOS::~BluetoothGattConnectionChromeOS() {
DBusThreadManager::Get()->GetBluetoothDeviceClient()->RemoveObserver(this);
Disconnect(base::Bind(&base::DoNothing));
}

std::string BluetoothGattConnectionChromeOS::GetDeviceAddress() const {
return device_address_;
}

bool BluetoothGattConnectionChromeOS::IsConnected() {
// Lazily determine the activity state of the connection. If already
// marked as inactive, then return false. Otherwise, explicitly mark
// |connected_| as false if the device is removed or disconnected. We do this,
// so that if this method is called during a call to DeviceRemoved or
// DeviceChanged somewhere else, it returns the correct status.
if (!connected_)
return false;

BluetoothDeviceClient::Properties* properties =
DBusThreadManager::Get()->GetBluetoothDeviceClient()->
GetProperties(object_path_);
if (!properties || !properties->connected.value())
connected_ = false;

return connected_;
}

void BluetoothGattConnectionChromeOS::Disconnect(
const base::Closure& callback) {
if (!connected_) {
VLOG(1) << "Connection already inactive.";
callback.Run();
return;
}

// TODO(armansito): There isn't currently a good way to manage the ownership
// of a connection between Chrome and bluetoothd plugins/profiles. Until
// a proper reference count is kept by bluetoothd, we might unwittingly kill
// a connection that is managed by the daemon (e.g. HoG). For now, just return
// success to indicate that this BluetoothGattConnection is no longer active,
// even though the underlying connection won't actually be disconnected. This
// technically doesn't violate the contract put forth by this API.
connected_ = false;
callback.Run();
}

void BluetoothGattConnectionChromeOS::DeviceRemoved(
const dbus::ObjectPath& object_path) {
if (object_path != object_path_)
return;

connected_ = false;
}

void BluetoothGattConnectionChromeOS::DevicePropertyChanged(
const dbus::ObjectPath& object_path,
const std::string& property_name) {
if (object_path != object_path_)
return;

if (!connected_)
return;

BluetoothDeviceClient::Properties* properties =
DBusThreadManager::Get()->GetBluetoothDeviceClient()->
GetProperties(object_path_);

if (!properties) {
connected_ = false;
return;
}

if (property_name == properties->connected.name() &&
!properties->connected.value())
connected_ = false;
}

} // namespace chromeos
66 changes: 66 additions & 0 deletions device/bluetooth/bluetooth_gatt_connection_chromeos.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
// Copyright 2014 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef DEVICE_BLUETOOTH_BLUETOOTH_GATT_CONNECTION_CHROMEOS_H_
#define DEVICE_BLUETOOTH_BLUETOOTH_GATT_CONNECTION_CHROMEOS_H_

#include <string>

#include "base/callback.h"
#include "base/memory/weak_ptr.h"
#include "chromeos/dbus/bluetooth_device_client.h"
#include "dbus/object_path.h"
#include "device/bluetooth/bluetooth_gatt_connection.h"

namespace device {

class BluetoothAdapter;

} // namespace device

namespace chromeos {

// BluetoothGattConnectionChromeOS implements BluetoothGattConnection for the
// Chrome OS platform.
class BluetoothGattConnectionChromeOS
: public device::BluetoothGattConnection,
public BluetoothDeviceClient::Observer {
public:
explicit BluetoothGattConnectionChromeOS(
scoped_refptr<device::BluetoothAdapter> adapter,
const std::string& device_address,
const dbus::ObjectPath& object_path);
virtual ~BluetoothGattConnectionChromeOS();

// BluetoothGattConnection overrides.
virtual std::string GetDeviceAddress() const OVERRIDE;
virtual bool IsConnected() OVERRIDE;
virtual void Disconnect(const base::Closure& callback) OVERRIDE;

private:

// chromeos::BluetoothDeviceClient::Observer overrides.
virtual void DeviceRemoved(const dbus::ObjectPath& object_path) OVERRIDE;
virtual void DevicePropertyChanged(const dbus::ObjectPath& object_path,
const std::string& property_name) OVERRIDE;

// True, if the connection is currently active.
bool connected_;

// The Bluetooth adapter that this connection is associated with.
scoped_refptr<device::BluetoothAdapter> adapter_;

// Bluetooth address of the underlying device.
std::string device_address_;

// D-Bus object path of the underlying device. This is used to filter observer
// events.
dbus::ObjectPath object_path_;

DISALLOW_COPY_AND_ASSIGN(BluetoothGattConnectionChromeOS);
};

} // namespace chromeos

#endif // DEVICE_BLUETOOTH_BLUETOOTH_GATT_CONNECTION_CHROMEOS_H_
Loading

0 comments on commit aa2ec2a

Please sign in to comment.