bluetooth: mac: add connected LE devices to chooser

device/bluetooth/bluetooth_adapter_mac.mm

Index: device/bluetooth/bluetooth_adapter_mac.mm
diff --git a/device/bluetooth/bluetooth_adapter_mac.mm b/device/bluetooth/bluetooth_adapter_mac.mm
index 16a876ead63e37671862e462b1f3703d8f33f000..295d5e12db39bc46f031322ebd19da17fe5220c0 100644
--- a/device/bluetooth/bluetooth_adapter_mac.mm
+++ b/device/bluetooth/bluetooth_adapter_mac.mm
@@ -186,6 +186,34 @@ bool BluetoothAdapterMac::IsDiscovering() const {
   return is_discovering;
 }
 
+std::unordered_map<BluetoothDevice*, BluetoothDevice::UUIDSet>
+BluetoothAdapterMac::RetrieveGattConnectedDevicesWithDiscoveryFilter(
+    const BluetoothDiscoveryFilter* discovery_filter) {
+  DCHECK(discovery_filter);

[ortuno, 2016/10/31 04:30:51]

Any reason why this is a pointer rather than a const reference? If you are
DCHECKing might as well make it a const reference.

[jlebel, 2016/11/07 01:43:17]

Done.

+  std::unordered_map<BluetoothDevice*, BluetoothDevice::UUIDSet>
+      connected_devices;
+  std::set<device::BluetoothUUID> uuids;
+  discovery_filter->GetUUIDs(uuids);
+  if (uuids.empty()) {

[ortuno, 2016/10/31 04:30:51]

Assuming you change RetrieveGattConnectedDevicesWithService to just return
devices.

if (uuids.empty()) {
  for (BluetoothDevice* device :
RetrieveGattConnectedDevicesWithService(nullptr)) {
    connected_devices[device] = BluetoothDevice::UUIDSet();
  }
  return connected_devices;
}

[jlebel, 2016/11/07 01:43:17]

Done.

+    connected_devices = RetrieveGattConnectedDevicesWithService(nullptr);
+    return connected_devices;
+  }
+  for (const BluetoothUUID& uuid : uuids) {
+    std::unordered_map<BluetoothDevice*, BluetoothDevice::UUIDSet> devices =
+        RetrieveGattConnectedDevicesWithService(&uuid);
+    for (auto iterator = devices.begin(); iterator != devices.end();

[ortuno, 2016/10/31 04:30:51]

Assuming you change RetrieveGattConnectedDevicesWithService to just return
devices:


for (const BluetoothUUID& uuid : uuids) {
  for (BluetoothDevice* device : RetrieveGattConnectedDevicesWithService(&uuid))
{
    connected_devices[device].insert(uuid);
  }
}

[jlebel, 2016/11/07 01:43:17]

Done.

+         ++iterator) {
+      auto find = connected_devices.find(iterator->first);
+      if (find == connected_devices.end()) {
+        connected_devices[iterator->first] = iterator->second;
+      } else {
+        find->second.insert(iterator->second.begin(), iterator->second.end());
+      }
+    }
+  }
+  return connected_devices;
+}
+
 BluetoothAdapter::UUIDList BluetoothAdapterMac::GetUUIDs() const {
   NOTIMPLEMENTED();
   return UUIDList();
@@ -499,62 +527,48 @@ void BluetoothAdapterMac::LowEnergyDeviceUpdated(
     VLOG(1) << "LowEnergyDeviceUpdated new device";
     // A new device has been found.
     device_mac = new BluetoothLowEnergyDeviceMac(this, peripheral);
-  } else {
-    // Check that there are no collisions.
-    std::string stored_device_id = device_mac->GetIdentifier();
-    std::string updated_device_id =
-        BluetoothLowEnergyDeviceMac::GetPeripheralIdentifier(peripheral);
-    if (stored_device_id != updated_device_id) {
-      VLOG(1)
-          << "LowEnergyDeviceUpdated stored_device_id != updated_device_id: "
-          << std::endl
-          << "  " << stored_device_id << std::endl
-          << "  " << updated_device_id;
-      // Collision, two identifiers map to the same hash address.  With a 48 bit
-      // hash the probability of this occuring with 10,000 devices
-      // simultaneously present is 1e-6 (see
-      // https://en.wikipedia.org/wiki/Birthday_problem#Probability_table).  We
-      // ignore the second device by returning.
-      return;
-    }
+  } else if (DoesCollideWithKnownDevice(peripheral, device_mac)) {
+    return;
   }
 
   DCHECK(device_mac);
 
-  // Get Advertised UUIDs
-  BluetoothDevice::UUIDList advertised_uuids;
-  NSArray* service_uuids =
-      [advertisement_data objectForKey:CBAdvertisementDataServiceUUIDsKey];
-  for (CBUUID* uuid in service_uuids) {
-    advertised_uuids.push_back(BluetoothUUID([[uuid UUIDString] UTF8String]));
-  }
-  NSArray* overflow_service_uuids = [advertisement_data
-      objectForKey:CBAdvertisementDataOverflowServiceUUIDsKey];
-  for (CBUUID* uuid in overflow_service_uuids) {
-    advertised_uuids.push_back(BluetoothUUID([[uuid UUIDString] UTF8String]));
-  }
-
-  // Get Service Data.
-  BluetoothDevice::ServiceDataMap service_data_map;
-  NSDictionary* service_data =
-      [advertisement_data objectForKey:CBAdvertisementDataServiceDataKey];
-  for (CBUUID* uuid in service_data) {
-    NSData* data = [service_data objectForKey:uuid];
-    const uint8_t* bytes = static_cast<const uint8_t*>([data bytes]);
-    size_t length = [data length];
-    service_data_map.emplace(BluetoothUUID([[uuid UUIDString] UTF8String]),
-                             std::vector<uint8_t>(bytes, bytes + length));
-  }
-
-  // Get Tx Power.
-  NSNumber* tx_power =
-      [advertisement_data objectForKey:CBAdvertisementDataTxPowerLevelKey];
-  int8_t clamped_tx_power = BluetoothDevice::ClampPower([tx_power intValue]);
-
-  device_mac->UpdateAdvertisementData(
-      BluetoothDevice::ClampPower(rssi), std::move(advertised_uuids),
-      std::move(service_data_map),
-      tx_power == nil ? nullptr : &clamped_tx_power);
+  if (advertisement_data) {

[ortuno, 2016/10/31 04:30:51]

I don't think you need this if statement anymore.

[jlebel, 2016/11/07 01:43:17]

Done.

+    // Get Advertised UUIDs
+    BluetoothDevice::UUIDList advertised_uuids;
+    NSArray* service_uuids =
+        [advertisement_data objectForKey:CBAdvertisementDataServiceUUIDsKey];
+    for (CBUUID* uuid in service_uuids) {
+      advertised_uuids.push_back(BluetoothUUID([[uuid UUIDString] UTF8String]));
+    }
+    NSArray* overflow_service_uuids = [advertisement_data
+        objectForKey:CBAdvertisementDataOverflowServiceUUIDsKey];
+    for (CBUUID* uuid in overflow_service_uuids) {
+      advertised_uuids.push_back(BluetoothUUID([[uuid UUIDString] UTF8String]));
+    }
+
+    // Get Service Data.
+    BluetoothDevice::ServiceDataMap service_data_map;
+    NSDictionary* service_data =
+        [advertisement_data objectForKey:CBAdvertisementDataServiceDataKey];
+    for (CBUUID* uuid in service_data) {
+      NSData* data = [service_data objectForKey:uuid];
+      const uint8_t* bytes = static_cast<const uint8_t*>([data bytes]);
+      size_t length = [data length];
+      service_data_map.emplace(BluetoothUUID([[uuid UUIDString] UTF8String]),
+                               std::vector<uint8_t>(bytes, bytes + length));
+    }
+
+    // Get Tx Power.
+    NSNumber* tx_power =
+        [advertisement_data objectForKey:CBAdvertisementDataTxPowerLevelKey];
+    int8_t clamped_tx_power = BluetoothDevice::ClampPower([tx_power intValue]);
+
+    device_mac->UpdateAdvertisementData(
+        BluetoothDevice::ClampPower(rssi), std::move(advertised_uuids),
+        std::move(service_data_map),
+        tx_power == nil ? nullptr : &clamped_tx_power);
+  }
 
   if (is_new_device) {
     std::string device_address =
@@ -582,6 +596,59 @@ void BluetoothAdapterMac::AddPairedDevices() {
   }
 }
 
+std::unordered_map<BluetoothDevice*, BluetoothDevice::UUIDSet>

[ortuno, 2016/10/31 04:30:51]

Why does this method return a map of devices to the UUID that we passed to this
method? I don't think you need to return the UUID. An array of devices should be
enough.

[jlebel, 2016/11/07 01:43:17]

Done.

+BluetoothAdapterMac::RetrieveGattConnectedDevicesWithService(
+    const BluetoothUUID* uuid) {
+  NSArray* cbUUIDs = nil;
+  if (!uuid) {
+    // It is not possible to ask for all connected peripherals with
+    // -[CBCentralManager retrieveConnectedPeripheralsWithServices:] by passing
+    // nil. To try to get most of the peripherals, the search is done with
+    // Generic Access service.
+    CBUUID* genericAccessServiceUUID = [CBUUID UUIDWithString:@"1800"];
+    cbUUIDs = @[ genericAccessServiceUUID ];
+  } else {
+    NSString* uuidString =
+        base::SysUTF8ToNSString(uuid->canonical_value().c_str());
+    cbUUIDs = @[ [CBUUID UUIDWithString:uuidString] ];
+  }
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wpartial-availability"
+  // Can remove ignore -Wpartial-availability when 10.8 will not be supported
+  // anymore.
+  // TODO(crbug.com/653056)
+  NSArray* peripherals = [low_energy_central_manager_
+      retrieveConnectedPeripheralsWithServices:cbUUIDs];
+#pragma clang diagnostic pop
+  std::unordered_map<BluetoothDevice*, BluetoothDevice::UUIDSet>
+      connected_devices;
+  for (CBPeripheral* peripheral in peripherals) {
+    BluetoothLowEnergyDeviceMac* device_mac =
+        GetBluetoothLowEnergyDeviceMac(peripheral);
+    const bool is_new_device = device_mac == nullptr;
+
+    if (!is_new_device && DoesCollideWithKnownDevice(peripheral, device_mac)) {
+      continue;
+    }
+    if (is_new_device) {
+      device_mac = new BluetoothLowEnergyDeviceMac(this, peripheral);
+      std::string device_address =
+          BluetoothLowEnergyDeviceMac::GetPeripheralHashAddress(peripheral);
+      devices_.add(device_address,
+                   std::unique_ptr<BluetoothDevice>(device_mac));
+      for (auto& observer : observers_) {
+        observer.DeviceAdded(this, device_mac);
+      }
+    }
+    BluetoothDevice::UUIDSet uuids;
+    if (uuid) {
+      uuids.insert(*uuid);
+    }
+    connected_devices[device_mac] = uuids;
+  }
+  return connected_devices;
+}
+
 void BluetoothAdapterMac::CreateGattConnection(
     BluetoothLowEnergyDeviceMac* device_mac) {
   [low_energy_central_manager_ connectPeripheral:device_mac->peripheral_
@@ -652,4 +719,26 @@ BluetoothAdapterMac::GetBluetoothLowEnergyDeviceMac(CBPeripheral* peripheral) {
   return static_cast<BluetoothLowEnergyDeviceMac*>(iter->second);
 }
 
+bool BluetoothAdapterMac::DoesCollideWithKnownDevice(
+    CBPeripheral* peripheral,
+    BluetoothLowEnergyDeviceMac* device_mac) {
+  // Check that there are no collisions.
+  std::string stored_device_id = device_mac->GetIdentifier();
+  std::string updated_device_id =
+      BluetoothLowEnergyDeviceMac::GetPeripheralIdentifier(peripheral);
+  if (stored_device_id != updated_device_id) {
+    VLOG(1) << "LowEnergyDeviceUpdated stored_device_id != updated_device_id: "
+            << std::endl
+            << "  " << stored_device_id << std::endl
+            << "  " << updated_device_id;
+    // Collision, two identifiers map to the same hash address.  With a 48 bit
+    // hash the probability of this occuring with 10,000 devices
+    // simultaneously present is 1e-6 (see
+    // https://en.wikipedia.org/wiki/Birthday_problem#Probability_table).  We
+    // ignore the second device by returning.
+    return true;
+  }
+  return false;
+}
+
 }  // namespace device