bluetooth: mac: Initial BluetoothRemoteGattCharacteristicMac implementation

device/bluetooth/bluetooth_remote_gatt_service_mac.mm

 // Copyright 2016 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_remote_gatt_service_mac.h"
 
 #import <CoreBluetooth/CoreBluetooth.h>
 #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 {
 
 BluetoothRemoteGattServiceMac::BluetoothRemoteGattServiceMac(
     BluetoothLowEnergyDeviceMac* bluetooth_device_mac,
     CBService* service,
     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(),
                                  (void*)service_]
           .UTF8String;
 }
 
 BluetoothRemoteGattServiceMac::~BluetoothRemoteGattServiceMac() {}

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

Also you need to call NotifyGattCharacteristicRemoved when destroying the
service.

 
 std::string BluetoothRemoteGattServiceMac::GetIdentifier() const {
   return identifier_;
 }
 
 BluetoothUUID BluetoothRemoteGattServiceMac::GetUUID() const {
   return uuid_;
 }
 
 bool BluetoothRemoteGattServiceMac::IsPrimary() const {
   return is_primary_;
 }
 
 BluetoothDevice* BluetoothRemoteGattServiceMac::GetDevice() const {
   return bluetooth_device_mac_;
 }
 
 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*>
 BluetoothRemoteGattServiceMac::GetIncludedServices() const {
   NOTIMPLEMENTED();
   return std::vector<BluetoothRemoteGattService*>();
 }
 
 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 {
   return service_.get();
 }
 
 }  // namespace device