Adding support for service scan on OS X

device/bluetooth/bluetooth_remote_gatt_service_mac.mm

+// 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.
+

[ortuno, 2016/05/06 16:03:34]

nit: #include <vector>

[jlebel, 2016/05/07 00:16:12]

Done.

+#include "device/bluetooth/bluetooth_remote_gatt_service_mac.h"
+
+#import <CoreBluetooth/CoreBluetooth.h>
+
+#include "base/logging.h"
+#include "device/bluetooth/bluetooth_adapter_mac.h"
+#include "device/bluetooth/bluetooth_low_energy_device_mac.h"
+#include "device/bluetooth/bluetooth_uuid.h"
+
+namespace device {
+
+BluetoothRemoteGattServiceMac::BluetoothRemoteGattServiceMac(
+    BluetoothLowEnergyDeviceMac* bluetooth_device_mac,
+    CBService* service,
+    bool primary)

[ortuno, 2016/05/06 16:03:34]

nit: is_primary

[jlebel, 2016/05/07 00:16:11]

Done.

+    : bluetooth_device_mac_(bluetooth_device_mac),
+      service_(service, base::scoped_policy::RETAIN),

[ortuno, 2016/05/06 16:03:34]

This might be my Objective-C ignorance showing, but is passing ownership of
CBService the right thing to do? You get the CBService* from
CBPeripheral:services and I don't see anywhere in the documentation that says
that you are getting ownership of these pointers.

[jlebel, 2016/05/07 00:16:11]

BluetoothRemoteGattServiceMac needs CBService instance. Therefore it has to
retain it. It is not an ownership. BluetoothRemoteGattServiceMac instance has
also to find out when the CBService instance should not be used but this is
another question.

[ortuno, 2016/05/09 19:54:00]

Ah I see. Thanks for the explanation.

+      is_primary_(primary) {}
+
+BluetoothRemoteGattServiceMac::~BluetoothRemoteGattServiceMac() {
+  bluetooth_device_mac_->GetAdapter()->NotifyGattServiceRemoved(this);
+}
+
+std::string BluetoothRemoteGattServiceMac::GetIdentifier() const {
+  const std::string string_uuid(

[ortuno, 2016/05/06 16:03:33]

I think we should create and store the identifier when we construct the object.
GetIdentifier is used a lot and we don't want to be creating and copying a
string every time someone calls GetIdentifier.

[jlebel, 2016/05/07 00:16:12]

Done.

+      BluetoothAdapterMac::StringWithCBUUID([service_.get() UUID]));
+  return
+      [NSString stringWithFormat:@"%s-%p", string_uuid.c_str(), (void*)service_]
+          .UTF8String;
+}
+
+BluetoothUUID BluetoothRemoteGattServiceMac::GetUUID() const {
+  return BluetoothAdapterMac::BluetoothUUIDWithCBUUID([service_.get() UUID]);

[ortuno, 2016/05/06 16:03:34]

I think we should also save the UUID.

[jlebel, 2016/05/07 00:16:12]

Done.

+}
+
+bool BluetoothRemoteGattServiceMac::IsPrimary() const {
+  return is_primary_;
+}
+
+BluetoothDevice* BluetoothRemoteGattServiceMac::GetDevice() const {
+  return bluetooth_device_mac_;
+}
+
+std::vector<BluetoothRemoteGattCharacteristic*>
+BluetoothRemoteGattServiceMac::GetCharacteristics() const {
+  NOTIMPLEMENTED();
+  std::vector<BluetoothRemoteGattCharacteristic*> characteristics;
+  return characteristics;

[ortuno, 2016/05/06 16:03:34]

nit: I think you can just do:

return std::vector<BluetoothRemoteGattCharacteristic*>();

[jlebel, 2016/05/07 00:16:11]

Done.

+}
+
+std::vector<BluetoothRemoteGattService*>
+BluetoothRemoteGattServiceMac::GetIncludedServices() const {
+  NOTIMPLEMENTED();
+  std::vector<BluetoothRemoteGattService*> included_services;
+  return included_services;

[ortuno, 2016/05/06 16:03:34]

Same here.

[jlebel, 2016/05/07 00:16:12]

Done.

+}
+
+BluetoothRemoteGattCharacteristic*
+BluetoothRemoteGattServiceMac::GetCharacteristic(
+    const std::string& identifier) const {
+  NOTIMPLEMENTED();
+  return nil;

[ortuno, 2016/05/06 16:03:34]

nit: return nullptr;

[jlebel, 2016/05/07 00:16:11]

Done.

+}
+
+void BluetoothRemoteGattServiceMac::Register(
+    const base::Closure& callback,
+    const ErrorCallback& error_callback) {
+  NOTIMPLEMENTED();
+}
+void BluetoothRemoteGattServiceMac::Unregister(

[ortuno, 2016/05/06 16:03:33]

As mentioned in the .h, you no longer need these two functions.

[jlebel, 2016/05/07 00:16:12]

Done.

+    const base::Closure& callback,
+    const ErrorCallback& error_callback) {
+  NOTIMPLEMENTED();
+}
+
+CBService* BluetoothRemoteGattServiceMac::GetService() const {
+  return service_.get();
+}
+
+}  // namespace device