Bluetooth: macOS: Adding logs

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 <stddef.h>
 
@@ skipping 18 matching lines @@
 #include "device/bluetooth/bluetooth_discovery_session.h"
 #include "device/bluetooth/bluetooth_discovery_session_outcome.h"
 #include "device/bluetooth/bluetooth_low_energy_central_manager_delegate.h"
 #include "device/bluetooth/bluetooth_socket_mac.h"
 
 namespace {
 
 // The frequency with which to poll the adapter for updates.
 const int kPollIntervalMs = 500;
 
+std::string GetCentralManagerStateString(CBCentralManagerState state) {

[ortuno, 2017/03/14 00:34:33]

I feel bad for adding these strings to the binary and increasing its size. It
doesn't seem to hard to just look up the error number. What do you think?

[jlebel, 2017/03/15 01:08:40]

This should not be a big source of problems, so we should not need read those
values often.
I guess I can remove that function.

+  switch (state) {
+    case CBCentralManagerStateUnknown:
+      return "CBCentralManagerStateUnknown";
+    case CBCentralManagerStateResetting:
+      return "CBCentralManagerStateResetting";
+    case CBCentralManagerStateUnsupported:
+      return "CBCentralManagerStateUnsupported";
+    case CBCentralManagerStateUnauthorized:
+      return "CBCentralManagerStateUnauthorized";
+    case CBCentralManagerStatePoweredOff:
+      return "CBCentralManagerStatePoweredOff";
+    case CBCentralManagerStatePoweredOn:
+      return "CBCentralManagerStatePoweredOn";
+  }
+  DCHECK(false);
+  return std::string("Unknown central manager state ") + std::to_string(state);
+}
+
 }  // namespace
 
 namespace device {
 
 // static
 base::WeakPtr<BluetoothAdapter> BluetoothAdapter::CreateAdapter(
     const InitCallback& init_callback) {
   return BluetoothAdapterMac::CreateAdapter();
 }
 
@@ skipping 463 matching lines @@
 void BluetoothAdapterMac::LowEnergyDeviceUpdated(
     CBPeripheral* peripheral,
     NSDictionary* advertisement_data,
     int rssi) {
   BluetoothLowEnergyDeviceMac* device_mac =
       GetBluetoothLowEnergyDeviceMac(peripheral);
   // If has no entry in the map, create new device and insert into |devices_|,
   // otherwise update the existing device.
   const bool is_new_device = device_mac == nullptr;
   if (is_new_device) {
-    VLOG(1) << "LowEnergyDeviceUpdated new device";
     // A new device has been found.
     device_mac = new BluetoothLowEnergyDeviceMac(this, peripheral);
+    VLOG(1) << device_mac->ToString() << ": New Device.";
   } else if (DoesCollideWithKnownDevice(peripheral, device_mac)) {
     return;
   }
 
   DCHECK(device_mac);
+  VLOG(1) << device_mac->ToString() << ": Device updated with "

[ortuno, 2017/03/14 00:34:33]

Make this a VLOG(3). We are going to get tens of these per second.

Also remove the VLOG at
https://cs.chromium.org/chromium/src/device/bluetooth/bluetooth_low_energy_di...

[jlebel, 2017/03/15 01:08:40]

Done.

+          << base::SysNSStringToUTF8([advertisement_data description]);
 
   // 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];
@@ skipping 29 matching lines @@
     devices_[device_address] = base::WrapUnique(device_mac);
     for (auto& observer : observers_)
       observer.DeviceAdded(this, device_mac);
   } else {
     for (auto& observer : observers_)
       observer.DeviceChanged(this, device_mac);
   }
 }
 
 // TODO(krstnmnlsn): Implement. crbug.com/511025
-void BluetoothAdapterMac::LowEnergyCentralManagerUpdatedState() {}
+void BluetoothAdapterMac::LowEnergyCentralManagerUpdatedState() {
+  VLOG(1) << "Central manager state updated "
+          << GetCentralManagerStateString([low_energy_central_manager_ state]);
+}
 
 void BluetoothAdapterMac::AddPairedDevices() {
   // Add any new paired devices.
   for (IOBluetoothDevice* device in [IOBluetoothDevice pairedDevices]) {
     // pairedDevices sometimes includes unknown devices that are not paired.
     // Radar issue with id 2282763004 has been filed about it.
     if ([device isPaired]) {
       ClassicDeviceAdded(device);
     }
   }
 }
 
 std::vector<BluetoothDevice*>
 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] ];
   }
+  VLOG(1) << "RetrieveGattConnectedDevicesWithService " << uuid->value();

[ortuno, 2017/03/14 00:34:33]

uuid could be null so let's do the logging inside the if/else:

if (!uuid) {
  VLOG(1) << "Retrieving all connected devices";
  // ...
} else {
  VLOG(1) << "Retrieving connected devices with UUID: " <<
uuid->canonical_value();
  // ...
}

[jlebel, 2017/03/15 01:08:40]

Done.

   NSArray* peripherals = [low_energy_central_manager_
       retrieveConnectedPeripheralsWithServices:cbUUIDs];
   std::vector<BluetoothDevice*> 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_[device_address] = base::WrapUnique(device_mac);
       for (auto& observer : observers_) {
         observer.DeviceAdded(this, device_mac);
       }
     }
     connected_devices.push_back(device_mac);
+    VLOG(1) << "  " << device_mac->ToString();

[ortuno, 2017/03/14 00:34:33]

Please follow the same pattern for all logs:

VLOG(1) << device_mac->ToString() << ": New Connected Device.";

[jlebel, 2017/03/15 01:08:40]

Done.

   }
   return connected_devices;
 }
 
 void BluetoothAdapterMac::CreateGattConnection(
     BluetoothLowEnergyDeviceMac* device_mac) {
   [low_energy_central_manager_ connectPeripheral:device_mac->peripheral_
                                          options:nil];
 }
 
@@ skipping 15 matching lines @@
 }
 
 void BluetoothAdapterMac::DidFailToConnectPeripheral(CBPeripheral* peripheral,
                                                      NSError* error) {
   BluetoothLowEnergyDeviceMac* device_mac =
       GetBluetoothLowEnergyDeviceMac(peripheral);
   if (!device_mac) {
     [low_energy_central_manager_ cancelPeripheralConnection:peripheral];
     return;
   }
-  VLOG(1) << "Failed to connect to peripheral";
+  VLOG(1) << device_mac->ToString() << ": Failed to connect to peripheral.";
   BluetoothDevice::ConnectErrorCode error_code =
       BluetoothDevice::ConnectErrorCode::ERROR_UNKNOWN;
   if (error) {
     error_code = BluetoothDeviceMac::GetConnectErrorCodeFromNSError(error);
-    VLOG(1) << "Bluetooth error, domain: " << error.domain.UTF8String
+    VLOG(1) << device_mac->ToString()
+            << ": Bluetooth error, domain: " << error.domain.UTF8String
             << ", error code: " << error.code
             << ", converted into: " << error_code;
   }
   device_mac->DidFailToConnectGatt(error_code);
 }
 
 void BluetoothAdapterMac::DidDisconnectPeripheral(CBPeripheral* peripheral,
                                                   NSError* error) {
   BluetoothLowEnergyDeviceMac* device_mac =
       GetBluetoothLowEnergyDeviceMac(peripheral);
   if (!device_mac) {
     [low_energy_central_manager_ cancelPeripheralConnection:peripheral];
     return;
   }
-  VLOG(1) << "Disconnected from peripheral.";
+  VLOG(1) << device_mac->ToString() << ": Disconnected from peripheral.";
   if (error) {
-    VLOG(1) << "Bluetooth error, domain: " << error.domain.UTF8String
+    VLOG(1) << device_mac->ToString()
+            << ": Bluetooth error, domain: " << error.domain.UTF8String
             << ", error code: " << error.code;
   }
   device_mac->DidDisconnectPeripheral(error);
 }
 
 BluetoothLowEnergyDeviceMac*
 BluetoothAdapterMac::GetBluetoothLowEnergyDeviceMac(CBPeripheral* peripheral) {
   std::string device_address =
       BluetoothLowEnergyDeviceMac::GetPeripheralHashAddress(peripheral);
   auto iter = devices_.find(device_address);
@@ skipping 19 matching lines @@
     // 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