bluetooth: mac: Initial BluetoothRemoteGattCharacteristicMac implementation

device/bluetooth/bluetooth_remote_gatt_service_mac.mm

Index: device/bluetooth/bluetooth_remote_gatt_service_mac.mm
diff --git a/device/bluetooth/bluetooth_remote_gatt_service_mac.mm b/device/bluetooth/bluetooth_remote_gatt_service_mac.mm
index 2e840703e40d32391166aa9f42ca0fcaa277b2ab..baef55e2c3daba4bfee1ec523112f34469b06fd1 100644
--- a/device/bluetooth/bluetooth_remote_gatt_service_mac.mm
+++ b/device/bluetooth/bluetooth_remote_gatt_service_mac.mm
@@ -8,8 +8,10 @@
 #include <vector>
 
 #include "base/logging.h"
+#include "base/memory/ptr_util.h"
 #include "device/bluetooth/bluetooth_adapter_mac.h"
 #include "device/bluetooth/bluetooth_low_energy_device_mac.h"
+#include "device/bluetooth/bluetooth_remote_gatt_characteristic_mac.h"
 #include "device/bluetooth/bluetooth_uuid.h"
 
 namespace device {
@@ -20,7 +22,8 @@ BluetoothRemoteGattServiceMac::BluetoothRemoteGattServiceMac(
     bool is_primary)
     : bluetooth_device_mac_(bluetooth_device_mac),
       service_(service, base::scoped_policy::RETAIN),
-      is_primary_(is_primary) {
+      is_primary_(is_primary),
+      is_discovery_complete_(false) {
   uuid_ = BluetoothAdapterMac::BluetoothUUIDWithCBUUID([service_.get() UUID]);
   identifier_ =
       [NSString stringWithFormat:@"%s-%p", uuid_.canonical_value().c_str(),
@@ -48,8 +51,13 @@ BluetoothDevice* BluetoothRemoteGattServiceMac::GetDevice() const {
 
 std::vector<BluetoothRemoteGattCharacteristic*>
 BluetoothRemoteGattServiceMac::GetCharacteristics() const {
-  NOTIMPLEMENTED();
-  return std::vector<BluetoothRemoteGattCharacteristic*>();
+  std::vector<BluetoothRemoteGattCharacteristic*> gatt_characteristics;
+  for (const auto& iter : gatt_characteristic_macs_) {
+    BluetoothRemoteGattCharacteristic* gatt_characteristic =
+        static_cast<BluetoothRemoteGattCharacteristic*>(iter.second);
+    gatt_characteristics.push_back(gatt_characteristic);
+  }
+  return gatt_characteristics;
 }
 
 std::vector<BluetoothRemoteGattService*>
@@ -61,8 +69,33 @@ BluetoothRemoteGattServiceMac::GetIncludedServices() const {
 BluetoothRemoteGattCharacteristic*
 BluetoothRemoteGattServiceMac::GetCharacteristic(
     const std::string& identifier) const {
-  NOTIMPLEMENTED();
-  return nullptr;
+  return (BluetoothRemoteGattCharacteristic*)(gatt_characteristic_macs_.get(
+      identifier));
+}
+
+void BluetoothRemoteGattServiceMac::DiscoverCharacteristics() {
+  is_discovery_complete_ = false;
+  gatt_characteristic_macs_.clear();

[ortuno, 2016/06/13 20:51:19]

Why do you need to clear the map of characteristics?

[jlebel, 2016/06/15 16:16:37]

To remove the characteristics each time we do discovery. I guess we have to send
notification, we have to recycle characteristics.

+  [GetPeripheral() discoverCharacteristics:nil forService:GetService()];

[ortuno, 2016/06/13 20:51:19]

Does OSX queue calls to discoverCharacteristics? Is it possible that calling
this function twice consecutively results in an error? For example in Android if
you call BluetoothGattCharacteristic.readValue twice consecutively the second
call fails with an IN_PROGRESS error.

[jlebel, 2016/06/15 16:16:37]

OS X doesn't care. It will continue connecting to the peripheral.

+}
+
+void BluetoothRemoteGattServiceMac::DidDiscoverCharacteristics() {
+  DCHECK(!is_discovery_complete_);
+  for (CBCharacteristic* cb_characteristic in GetService().characteristics) {
+    BluetoothRemoteGattCharacteristicMac* gatt_characteristic_mac =
+        new BluetoothRemoteGattCharacteristicMac(this, cb_characteristic);
+    gatt_characteristic_macs_.add(gatt_characteristic_mac->GetIdentifier(),

[ortuno, 2016/06/13 20:51:19]

You also need to call GattCharacteristicAdded. I'm surprised we don't have a
test for it D: Could you add one? There is one for services here:

https://cs.chromium.org/chromium/src/device/bluetooth/bluetooth_device_unitte...

[jlebel, 2016/06/15 16:16:37]

https://codereview.chromium.org/2068203002/

Done.

+                                  base::WrapUnique(gatt_characteristic_mac));

[ortuno, 2016/06/13 20:51:19]

It's a bit dangerous to call functions on the object that you are passing
because some compilers evaluate the arguments in different orders. For example
clang would call base::WrapUnique first and then GetIdentifier, but gcc would
call GetIdentifier first and then base::WrapUnique. Consider saving copying the
value of GetIdentifier first.

[jlebel, 2016/06/15 16:16:37]

Done.

+  }
+  is_discovery_complete_ = true;
+}
+
+bool BluetoothRemoteGattServiceMac::IsDiscoveryComplete() {
+  return is_discovery_complete_;
+}
+
+CBPeripheral* BluetoothRemoteGattServiceMac::GetPeripheral() const {
+  return bluetooth_device_mac_->GetPeripheral();
 }
 
 CBService* BluetoothRemoteGattServiceMac::GetService() const {