Adding support for Low Energy device discovery to BluetoothAdapterMac

device/bluetooth/bluetooth_adapter_mac.mm

 // Copyright 2013 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_adapter_mac.h"
 
 #import <IOBluetooth/objc/IOBluetoothDevice.h>
 #import <IOBluetooth/objc/IOBluetoothHostController.h>
 
 #include <string>
 
 #include "base/bind.h"
 #include "base/compiler_specific.h"
 #include "base/containers/hash_tables.h"
 #include "base/location.h"
 #include "base/mac/sdk_forward_declarations.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/profiler/scoped_tracker.h"
 #include "base/sequenced_task_runner.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/sys_string_conversions.h"
 #include "base/thread_task_runner_handle.h"
 #include "base/time/time.h"
 #include "device/bluetooth/bluetooth_device_mac.h"
+#include "device/bluetooth/bluetooth_discovery_session.h"
 #include "device/bluetooth/bluetooth_socket_mac.h"
 #include "device/bluetooth/bluetooth_uuid.h"
 
 namespace {
 
 // The frequency with which to poll the adapter for updates.
 const int kPollIntervalMs = 500;
 
 // The length of time that must elapse since the last Inquiry response before a
 // discovered Classic device is considered to be no longer available.
@@ skipping 15 matching lines @@
   adapter->Init();
   return adapter->weak_ptr_factory_.GetWeakPtr();
 }
 
 BluetoothAdapterMac::BluetoothAdapterMac()
     : BluetoothAdapter(),
       powered_(false),
       num_discovery_sessions_(0),
       classic_discovery_manager_(
           BluetoothDiscoveryManagerMac::CreateClassic(this)),
+      low_energy_discovery_manager_(
+          BluetoothLowEnergyDiscoveryManagerMac::Create(this)),
       weak_ptr_factory_(this) {
   DCHECK(classic_discovery_manager_.get());
 }
 
 BluetoothAdapterMac::~BluetoothAdapterMac() {
 }
 
 std::string BluetoothAdapterMac::GetAddress() const {
   return address_;
 }
@@ skipping 32 matching lines @@
 }
 
 void BluetoothAdapterMac::SetDiscoverable(
     bool discoverable,
     const base::Closure& callback,
     const ErrorCallback& error_callback) {
   NOTIMPLEMENTED();
 }
 
 bool BluetoothAdapterMac::IsDiscovering() const {
-  return classic_discovery_manager_->IsDiscovering();
+  return (classic_discovery_manager_->IsDiscovering() ||
+          low_energy_discovery_manager_->IsDiscovering());
 }
 
 void BluetoothAdapterMac::CreateRfcommService(
     const BluetoothUUID& uuid,
     const ServiceOptions& options,
     const CreateServiceCallback& callback,
     const CreateServiceErrorCallback& error_callback) {
   scoped_refptr<BluetoothSocketMac> socket = BluetoothSocketMac::CreateSocket();
   socket->ListenUsingRfcomm(
       this, uuid, options, base::Bind(callback, socket), error_callback);
@@ skipping 18 matching lines @@
 }
 
 void BluetoothAdapterMac::RegisterAdvertisement(
     scoped_ptr<BluetoothAdvertisement::Data> advertisement_data,
     const CreateAdvertisementCallback& callback,
     const CreateAdvertisementErrorCallback& error_callback) {
   NOTIMPLEMENTED();
   error_callback.Run(BluetoothAdvertisement::ERROR_UNSUPPORTED_PLATFORM);
 }
 
-void BluetoothAdapterMac::DeviceFound(IOBluetoothDevice* device) {
-  DeviceAdded(device);
+void BluetoothAdapterMac::ClassicDeviceFound(IOBluetoothDevice* device) {
+  ClassicDeviceAdded(device);
 }
 
-void BluetoothAdapterMac::DiscoveryStopped(bool unexpected) {
+void BluetoothAdapterMac::ClassicDiscoveryStopped(bool unexpected) {
   if (unexpected) {
     DVLOG(1) << "Discovery stopped unexpectedly";
     num_discovery_sessions_ = 0;
     MarkDiscoverySessionsAsInactive();
   }
   FOR_EACH_OBSERVER(BluetoothAdapter::Observer,
                     observers_,
                     AdapterDiscoveringChanged(this, false));
 }
 
 void BluetoothAdapterMac::DeviceConnected(IOBluetoothDevice* device) {
   // TODO(isherman): Investigate whether this method can be replaced with a call
   // to +registerForConnectNotifications:selector:.
   DVLOG(1) << "Adapter registered a new connection from device with address: "
            << BluetoothDeviceMac::GetDeviceAddress(device);
-  DeviceAdded(device);
+  ClassicDeviceAdded(device);
+}
+
+bool BluetoothAdapterMac::StartDiscovery(

[scheib, 2015/06/05 18:10:20]

Move to after SetDiscoveryFilter. "Method definitions in the corresponding .cc
file should be the same as the declaration order, as much as possible."
http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Declaration_O...

[krstnmnlsn, 2015/06/09 01:08:10]

Right.  I went ahead and moved RemovePairingDelegateInteral as well.

+    BluetoothDiscoveryFilter* discovery_filter) {
+  // Default to dual discovery if |discovery_filter| is NULL.
+  BluetoothDiscoveryFilter::TransportMask transport =
+      BluetoothDiscoveryFilter::Transport::TRANSPORT_DUAL;
+  if (discovery_filter)
+    transport = discovery_filter->GetTransport();
+
+  if ((transport & BluetoothDiscoveryFilter::Transport::TRANSPORT_CLASSIC) &&

[scheib, 2015/06/05 18:10:20]

We should find a mix of classic and LE devices and test that starting discovery
on both APIs at the same time will work. I can help with that a bit. 

Or, if there's more info already known about this we should document it here in
comments.

[krstnmnlsn, 2015/06/09 01:08:10]

Manually tested and added a short comment (cannot find any information online).

+      !classic_discovery_manager_->IsDiscovering()) {
+    // TODO(krstnmnlsn): If a classic discovery session is already running then
+    // we should update its filter.
+    if (!classic_discovery_manager_->StartDiscovery()) {
+      DVLOG(1) << "Failed to add a classic discovery session";
+      return false;
+    }
+  }
+  if (transport & BluetoothDiscoveryFilter::Transport::TRANSPORT_LE) {
+    // Begin a low energy discovery session or update it if one is already
+    // running.
+    low_energy_discovery_manager_->StartDiscovery(BluetoothDevice::UUIDList());
+  }
+  return true;
 }
 
 void BluetoothAdapterMac::AddDiscoverySession(
     BluetoothDiscoveryFilter* discovery_filter,
     const base::Closure& callback,
     const ErrorCallback& error_callback) {
   DVLOG(1) << __func__;
   if (num_discovery_sessions_ > 0) {
     DCHECK(IsDiscovering());
     num_discovery_sessions_++;
+    // We are already running a discovery session, notify the system if the
+    // filter has changed.
+    if (!StartDiscovery(discovery_filter)) {

[scheib, 2015/06/06 05:09:48]

Without seeing the plans for how differing filters will be handled I'm uncertain
about this. Previously if multiple AddDiscoverSessions happen the system was
only asked to scan once. I don't know if the OSX APIs care, but I also don't see
a need to call them more than once unless we have a reason to.

Adding Classic and LE simultaneously gives us a reason to do something if first
a discovery session requested only one, but another AddDiscoverySession
requested another one. Perhaps num_discovery_sessions_ should be duplicated for
both Classic and LE counts?

If filters later try to support scanning for differing sets of service UUIDs in
LE, we'll need e.g. merging logic for the filer, or postponing or failing a new
discovery session if one is already under way.

After clearing up the design intent (put it in the design doc, and add comments
here explaining anything (especially the 'why' that isn't obvious from code))
Then add tests for the different scenarios of multiple AddDiscoverySessions.

[krstnmnlsn, 2015/06/09 01:08:09]

Recap/continuing of offline discussion: 
The plans for handling of differing filters have mostly already been made in the
BluetoothAdapter class (see: GetMergedDiscoveryFilter).  We can handle merging
(and editing) the set of filters, and notifying the system of a filter change
will be consistent with Chromeos and what (I was interpreting to be) the desired
cross-platform behavior.

The LE API does not mind repeated calls to start bluetooth/peripheral discovery,
the classic API might.  That is why the classic part of StartDiscovery currently
does nothing on a repeated call (this was the previous behavior as well). 
Issue: crbug.com/498056

+      error_callback.Run();
+      return;
+    }
     callback.Run();
     return;
   }
 
   DCHECK_EQ(0, num_discovery_sessions_);
 
-  if (!classic_discovery_manager_->StartDiscovery()) {
-    DVLOG(1) << "Failed to add a discovery session";
+  if (!StartDiscovery(discovery_filter)) {
     error_callback.Run();
     return;
   }
 
   DVLOG(1) << "Added a discovery session";
   num_discovery_sessions_++;
   FOR_EACH_OBSERVER(BluetoothAdapter::Observer,
                     observers_,
                     AdapterDiscoveringChanged(this, true));
   callback.Run();
@@ skipping 12 matching lines @@
     callback.Run();
     return;
   }
 
   if (num_discovery_sessions_ == 0) {
     DVLOG(1) << "No active discovery sessions. Returning error.";
     error_callback.Run();
     return;
   }
 
-  if (!classic_discovery_manager_->StopDiscovery()) {
-    DVLOG(1) << "Failed to stop discovery";
-    error_callback.Run();
-    return;
+  // Default to dual discovery if |discovery_filter| is NULL.
+  BluetoothDiscoveryFilter::TransportMask transport =
+      BluetoothDiscoveryFilter::Transport::TRANSPORT_DUAL;
+  if (discovery_filter)
+    transport = discovery_filter->GetTransport();
+
+  if (transport & BluetoothDiscoveryFilter::Transport::TRANSPORT_CLASSIC) {
+    if (!classic_discovery_manager_->StopDiscovery()) {
+      DVLOG(1) << "Failed to stop classic discovery";
+      error_callback.Run();
+      return;
+    }
+  }
+  if (transport & BluetoothDiscoveryFilter::Transport::TRANSPORT_LE) {
+    low_energy_discovery_manager_->StopDiscovery();
   }
 
   DVLOG(1) << "Discovery stopped";
   num_discovery_sessions_--;
   callback.Run();
 }
 
 void BluetoothAdapterMac::SetDiscoveryFilter(
     scoped_ptr<BluetoothDiscoveryFilter> discovery_filter,
     const base::Closure& callback,
@@ skipping 80 matching lines @@
           "461181 BluetoothAdapterMac::PollAdapter::UpdateDevices"));
   UpdateDevices();
 
   ui_task_runner_->PostDelayedTask(
       FROM_HERE,
       base::Bind(&BluetoothAdapterMac::PollAdapter,
                  weak_ptr_factory_.GetWeakPtr()),
       base::TimeDelta::FromMilliseconds(kPollIntervalMs));
 }
 
-void BluetoothAdapterMac::DeviceAdded(IOBluetoothDevice* device) {
+void BluetoothAdapterMac::ClassicDeviceAdded(IOBluetoothDevice* device) {
   std::string device_address = BluetoothDeviceMac::GetDeviceAddress(device);
 
   // Only notify observers once per device.
   if (devices_.count(device_address))
     return;
 
   devices_[device_address] = new BluetoothDeviceMac(device);
   FOR_EACH_OBSERVER(BluetoothAdapter::Observer,
                     observers_,
                     DeviceAdded(this, devices_[device_address]));
 }
 
+// TODO(krstnmnlsn): This method to be implemented as soon as UpdateDevices can
+// handle instances of LowEnergyBluetoothDevice in |devices_|.

[scheib, 2015/06/05 18:10:20]

Add a reference to the issue number to do that work. Just a crbug.com/000000
after the sentence. 

[krstnmnlsn, 2015/06/09 01:08:09]

Done.

+void BluetoothAdapterMac::LowEnergyDeviceUpdated(
+    CBPeripheral* peripheral,
+    NSDictionary* advertisementData,
+    int rssi) {
+  NOTIMPLEMENTED();

[scheib, 2015/06/05 18:10:20]

We expect LE sessions will be started, so we can't throw an error here because
we haven't implemented this yet. Just remove NOTIMPLEMENTED for now.

[krstnmnlsn, 2015/06/09 01:08:09]

Done.

+}
+
+// TODO(krstnmnlsn): This method assumes all BluetoothDevices in devices_ are
+// instances of BluetoothDeviceMac.  Add support for low energy devices.

[scheib, 2015/06/05 18:10:20]

issue number.

[krstnmnlsn, 2015/06/09 01:08:09]

Done.

 void BluetoothAdapterMac::UpdateDevices() {
   // Notify observers if any previously seen devices are no longer available,
   // i.e. if they are no longer paired, connected, nor recently discovered via
   // an inquiry.
   std::set<std::string> removed_devices;
   for (DevicesMap::iterator it = devices_.begin(); it != devices_.end(); ++it) {
     BluetoothDevice* device = it->second;
     if (device->IsPaired() || device->IsConnected())
       continue;
 
     NSDate* last_inquiry_update =
         static_cast<BluetoothDeviceMac*>(device)->GetLastInquiryUpdate();
     if (last_inquiry_update &&
         -[last_inquiry_update timeIntervalSinceNow] < kDiscoveryTimeoutSec)
       continue;
 
     FOR_EACH_OBSERVER(
         BluetoothAdapter::Observer, observers_, DeviceRemoved(this, device));
     delete device;
     removed_devices.insert(it->first);
     // The device will be erased from the map in the loop immediately below.
   }
   for (const std::string& device_address : removed_devices) {
     size_t num_removed = devices_.erase(device_address);
     DCHECK_EQ(num_removed, 1U);
   }
 
   // Add any new paired devices.
   for (IOBluetoothDevice* device in [IOBluetoothDevice pairedDevices]) {
-    DeviceAdded(device);
+    ClassicDeviceAdded(device);
   }
 }
 
 }  // namespace device