device/bluetooth:Implement Register() for BluetoothAudioSinkChromeOS.

device/bluetooth/bluetooth_audio_sink_chromeos.cc

 // Copyright 2015 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_audio_sink_chromeos.h"
 
 #include <sstream>
 
 #include "base/logging.h"
+#include "chromeos/dbus/dbus_thread_manager.h"
+
+namespace {
+
+// TODO(mcchou): Add the constant to dbus/service_constants.h.
+const char kBluetoothAudioSinkServicePath[] = "/org/chromium/AudioSink";
+const char kBluetoothAudioSinkUUID[] = "0000110b-0000-1000-8000-00805f9b34fb";
+
+dbus::ObjectPath GenerateEndpointPath() {
+  static unsigned int sequence_number = 0;
+  ++sequence_number;
+  std::stringstream path;
+  path << kBluetoothAudioSinkServicePath << "/endpoint" << sequence_number;
+  return dbus::ObjectPath(path.str());
+}
+
+}  // namespace
 
 namespace chromeos {
 
 BluetoothAudioSinkChromeOS::BluetoothAudioSinkChromeOS(
     BluetoothAdapterChromeOS* adapter)
     : state_(device::BluetoothAudioSink::STATE_INVALID),
       present_(false),
       powered_(false),
       volume_(0),
       read_mtu_(0),
       write_mtu_(0),
       adapter_(adapter),
       weak_ptr_factory_(this) {
   DCHECK(adapter_);
 
   present_ = adapter_->IsPresent();
   powered_ = adapter_->IsPowered();
   if (present_ && powered_)
-    state_ = device::BluetoothAudioSink::STATE_DISCONNECTED;
+    StateChanged(device::BluetoothAudioSink::STATE_DISCONNECTED);
   adapter_->AddObserver(this);
 }
 
 BluetoothAudioSinkChromeOS::~BluetoothAudioSinkChromeOS() {
   DCHECK(adapter_);
   adapter_->RemoveObserver(this);
 }
 
 void BluetoothAudioSinkChromeOS::AddObserver(
     device::BluetoothAudioSink::Observer* observer) {
@@ skipping 11 matching lines @@
   return state_;
 }
 
 uint16_t BluetoothAudioSinkChromeOS::GetVolume() const {
   return volume_;
 }
 
 void BluetoothAudioSinkChromeOS::AdapterPresentChanged(
     device::BluetoothAdapter* adapter,
     bool present) {
-  // TODO(mcchou): BUG=441581
-  // If |persent| is true, change state to |STATE_DISCONNECTED| and call
-  // StateChanged(). Otherwise, change state to |STATE_INVALID| and call
-  // StateChanged.
+  if (present == present_) return;

[armansito, 2015/01/27 04:38:06]

nit: Put return in its own line.

[armansito, 2015/01/27 04:38:06]

This check isn't really necessary, since the adapter won't call this if the
value hasn't changed.

[Miao, 2015/01/28 02:05:01]

Removed.

+  VLOG(1) << "Bluetooth audio sink: Bluetooth adapter present changed: "
+          << present;
+  present_ = present;

[armansito, 2015/01/27 04:38:06]

I don't think that |present_| and |powered_| are necessary member variables
since you can obtain these from the adapter by calling adapter_->IsPresent() and
adapter_->IsPowered(). So I would remove them from the class.

[Miao, 2015/01/28 02:05:01]

Removed.

+  if (present_ && adapter->IsPowered()) {
+    StateChanged(device::BluetoothAudioSink::STATE_DISCONNECTED);
+  } else {

[armansito, 2015/01/27 04:38:06]

This means that the state will become INVALID if |present_| is true and
IsPowered is false, which is probably not what we want. I would simply mark the
state as invalid if |present_| is false. Otherwise, mark the state as
DISCONNECTED if no media transport is currently assigned.

[Miao, 2015/01/28 02:05:01]

Will remove AdapterPoweredChanged and let MediaTransportRemoved change the state
to STATE_DISCONNECTED. The MediaTransport will be destroyed if powered is down.

+    StateChanged(device::BluetoothAudioSink::STATE_INVALID);
+  }
 }
 
 void BluetoothAudioSinkChromeOS::AdapterPoweredChanged(
     device::BluetoothAdapter* adapter,
     bool powered) {
-  // TODO(mcchou): BUG=441581
-  // If |powered| is true, change state to |STATE_DISCONNECTED| and call
-  // StateChanged(). Otherwise, change state to |STATE_INVALID| and call
-  // StateChanged.
+  if (powered == powered_) return;

[armansito, 2015/01/27 04:38:05]

Same as above.

[Miao, 2015/01/28 02:05:01]

Removed.

+  VLOG(1) << "Bluetooth audio sink: Bluetooth adapter powered changed: "
+          << powered;
+  powered_ = powered;
+  if (powered_ && adapter->IsPresent()) {
+    StateChanged(device::BluetoothAudioSink::STATE_DISCONNECTED);
+  } else {
+    StateChanged(device::BluetoothAudioSink::STATE_INVALID);

[armansito, 2015/01/27 04:38:05]

Same logic as above. Besides this code is essentially identical, so I would put
the state determination into a single method.

[Miao, 2015/01/28 02:05:01]

Removed.

+  }
 }
 
 void BluetoothAudioSinkChromeOS::MediaRemoved(
     const dbus::ObjectPath& object_path) {
   // TODO(mcchou): BUG=441581
   // Check if |object_path| equals to |media_path_|. If true, change the state
   // of the audio sink, call StateChanged and reset the audio sink.
 }
 
 void BluetoothAudioSinkChromeOS::MediaTransportRemoved(
@@ skipping 16 matching lines @@
     const dbus::MessageReader& properties) {
   // TODO(mcchou): BUG=441581
   // Update |transport_path_| and store properties if needed.
 }
 
 void BluetoothAudioSinkChromeOS::SelectConfiguration(
     const std::vector<uint8_t>& capabilities,
     const SelectConfigurationCallback& callback) {
   // TODO(mcchou): BUG=441581
   // Use SelectConfigurationCallback to return the agreed capabilities.
+  VLOG(1) << "Bluetooth audio sink: SelectConfiguration called";
+  callback.Run(options_.capabilities);
 }
 
 void BluetoothAudioSinkChromeOS::ClearConfiguration(
     const dbus::ObjectPath& transport_path) {
   // TODO(mcchou): BUG=441581
   // Reset the configuration to the default one and close IOBuffer.
 }
 
 void BluetoothAudioSinkChromeOS::Release() {
   // TODO(mcchou): BUG=441581
   // Let the audio sink does the clean-up and do nothing here.
 }
 
 void BluetoothAudioSinkChromeOS::Register(
     const device::BluetoothAudioSink::Options& options,
     const base::Closure& callback,
     const device::BluetoothAudioSink::ErrorCallback& error_callback) {
   // TODO(mcchou): BUG=441581
   // Get Media object, initiate an Media Endpoint with options, and return the
   // audio sink via callback. Add the audio sink as observer of both Media and
   // Media Transport.
+  DCHECK(adapter_);
+  DCHECK_EQ(state_, device::BluetoothAudioSink::STATE_DISCONNECTED);
+
+  // Gets system bus.
+  dbus::Bus* system_bus = DBusThreadManager::Get()->GetSystemBus();
+
+  // Creates a Media Endpoint with newly-generated path.
+  dbus::ObjectPath endpoint_path_ = GenerateEndpointPath();

[armansito, 2015/01/27 04:38:05]

This code shouldn't compile, since you're redeclaring |endpoint_path_| (which
appears to be a member variable).

[Miao, 2015/01/28 02:05:01]

The compilation will pass, since it will be a local variable hiding the
class-wide one, but I shouldn't redeclare endpoint_point here anyway.

+  media_endpoint_.reset(
+      BluetoothMediaEndpointServiceProvider::Create(
+          system_bus, endpoint_path_, this));
+
+  DCHECK(media_endpoint_.get());
+
+  // Creates endpoint properties with |options|.
+  options_ = options;
+  chromeos::BluetoothMediaClient::EndpointProperties endpoint_properties;
+  endpoint_properties.uuid = std::string(kBluetoothAudioSinkUUID);
+  endpoint_properties.codec = options_.codec;
+  endpoint_properties.capabilities = options_.capabilities;
+
+  // Gets Media object exported by D-Bus and registers the endpoint.
+  chromeos::BluetoothMediaClient* media =
+      DBusThreadManager::Get()->GetBluetoothMediaClient();
+  media->AddObserver(this);
+  media_path_ = adapter_->object_path();
+  media->RegisterEndpoint(
+      media_path_,
+      endpoint_path_,
+      endpoint_properties,
+      base::Bind(&BluetoothAudioSinkChromeOS::OnRegisterSucceeded,
+                 weak_ptr_factory_.GetWeakPtr(), callback),
+      base::Bind(&BluetoothAudioSinkChromeOS::OnRegisterFailed,
+                 weak_ptr_factory_.GetWeakPtr(), error_callback));
 }
 
 void BluetoothAudioSinkChromeOS::Unregister(
     const base::Closure& callback,
     const device::BluetoothAudioSink::ErrorCallback& error_callback) {
   // TODO(mcchou): BUG=441581
-  // Clean |observers_| and |transport_path_| and reset |state_| and |volume_|.
+  // Call UnregisterEndpoint on the media object with |media_path_| and clean
+  // |observers_| and transport_path_ and reset state_ and volume.
 }
 
 void BluetoothAudioSinkChromeOS::StateChanged(
     device::BluetoothAudioSink::State state) {
-  DCHECK_NE(state, state_);
+  if (state == state_) return;

[armansito, 2015/01/27 04:38:06]

nit: return; in its own line.

[Miao, 2015/01/28 02:05:01]

Done.

   VLOG(1) << "Bluetooth audio sink state changed: " << state;
+  switch (state) {
+    case device::BluetoothAudioSink::STATE_INVALID: {
+      // TODO(mcchou): BUG=441581
+      // Clean media, media transport and media endpoint.
+      break;
+    }
+    case device::BluetoothAudioSink::STATE_DISCONNECTED: {
+      // TODO(mcchou): BUG=441581
+      // Clean media transport.
+      break;
+    }
+    case device::BluetoothAudioSink::STATE_IDLE: {
+      // TODO(mcchou): BUG=441581
+      // Triggered by MediaTransportPropertyChanged. Stop watching on file
+      // descriptor if there is one.
+      break;
+    }
+    case device::BluetoothAudioSink::STATE_PENDING: {
+      // TODO(mcchou): BUG=441581
+      // Call BluetoothMediaTransportClient::Acquire() to get fd and mtus.
+      break;
+    }
+    case device::BluetoothAudioSink::STATE_ACTIVE: {
+      // TODO(mcchou): BUG=441581
+      // Read from fd and call DataAvailable.
+      ReadFromFD();
+      break;
+    }
+    default: {

[armansito, 2015/01/27 04:38:05]

No need for empty scope here.

[Miao, 2015/01/28 02:05:01]

Done.

+    }
+  }
+
   state_ = state;
   FOR_EACH_OBSERVER(device::BluetoothAudioSink::Observer, observers_,
                     BluetoothAudioSinkStateChanged(this, state_));
 }
 
 void BluetoothAudioSinkChromeOS::VolumeChanged(uint16_t volume) {
   DCHECK_NE(volume, volume_);
   VLOG(1) << "Bluetooth audio sink volume changed: " << volume;
   volume_ = volume;
   FOR_EACH_OBSERVER(device::BluetoothAudioSink::Observer, observers_,
                     BluetoothAudioSinkVolumeChanged(this, volume_));
 }
 
+void BluetoothAudioSinkChromeOS::OnRegisterSucceeded(
+    const base::Closure& callback) {
+  VLOG(1) << "Bluetooth audio sink registerd";
+
+  StateChanged(device::BluetoothAudioSink::STATE_DISCONNECTED);
+  callback.Run();
+}
+
+void BluetoothAudioSinkChromeOS::OnRegisterFailed(
+    const BluetoothAudioSink::ErrorCallback& error_back,
+    const std::string& error_name,
+    const std::string& error_message) {
+  DCHECK(media_endpoint_.get());
+  VLOG(1) << "Bluetooth audio sink: " << error_name << ": " <<  error_message;
+
+  endpoint_path_ = dbus::ObjectPath("");
+  media_endpoint_ = nullptr;
+  error_back.Run(device::BluetoothAudioSink::ERROR_NOT_REGISTERED);
+}
+
 void BluetoothAudioSinkChromeOS::ReadFromFD() {
   DCHECK_GE(fd_.value(), 0);
 
   // TODO(mcchou): BUG=441581
   // Read from file descriptor using watcher and create a buffer to contain the
   // data. Notify |Observers_| while there is audio data available.
 }
 
 }  // namespace chromeos