Bluetooth: macOS: Implementing read/write for descriptors

device/bluetooth/bluetooth_remote_gatt_descriptor_mac.mm

 // Copyright 2017 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_descriptor_mac.h"
 
+#include "base/bind.h"
+#import "base/mac/foundation_util.h"
 #include "base/strings/sys_string_conversions.h"
-#include "device/bluetooth/bluetooth_adapter_mac.h"
-#include "device/bluetooth/bluetooth_remote_gatt_characteristic_mac.h"
+#include "base/threading/thread_task_runner_handle.h"
+#import "device/bluetooth/bluetooth_adapter_mac.h"
+#import "device/bluetooth/bluetooth_remote_gatt_characteristic_mac.h"
+
+using base::mac::ObjCCast;
 
 namespace device {
 
+std::vector<uint8_t> VectorValueFromObjC(id objc_value) {
+  // According to
+  // https://developer.apple.com/reference/corebluetooth/cbdescriptor some
+  // descriptor values can be NSData, NSString or NSNumber.
+  std::vector<uint8_t> value;
+  NSData* data = ObjCCast<NSData>(objc_value);
+  NSString* as_string = ObjCCast<NSString>(objc_value);
+  NSNumber* as_number = ObjCCast<NSNumber>(objc_value);
+
+  if (!data && !as_string && as_number) {
+    unsigned short descriptor = [as_number shortValue];
+    data = [NSData dataWithBytes:&descriptor length:sizeof(descriptor)];
+  }
+
+  if (!data && as_string)
+    data = [as_string dataUsingEncoding:NSUTF8StringEncoding];
+
+  DCHECK(data) << "Unexpected value: "

[ortuno, 2017/04/10 01:52:11]

Can we log this?

[ortuno, 2017/04/10 23:36:39]

DCHECK usually signals that that case will never happen. So readers don't have
to worry about that case. Having the "if" right after the DCHECK is really
confusing to me.

I would remove the DCHECK and add an else statement to "if (data)":

if (data) {
  // ...
} else {
  // In case of new unhandled types.
  LOG(WARNING) << "Unexpected value: " << base::SysNSStringToUTF8(...);
}

[jlebel, 2017/04/11 18:48:18]

Done.

+               << base::SysNSStringToUTF8([objc_value description]);
+  if (data) {
+    value.resize([data length]);
+    [data getBytes:value.data() length:value.size()];
+  }
+  return value;
+}
+
 BluetoothRemoteGattDescriptorMac::BluetoothRemoteGattDescriptorMac(
     BluetoothRemoteGattCharacteristicMac* characteristic,
     CBDescriptor* descriptor)
     : gatt_characteristic_(characteristic),
-      cb_descriptor_(descriptor, base::scoped_policy::RETAIN) {
-  uuid_ =
-      BluetoothAdapterMac::BluetoothUUIDWithCBUUID([cb_descriptor_.get() UUID]);
+      cb_descriptor_(descriptor, base::scoped_policy::RETAIN),
+      value_read_or_write_in_progress_(false) {
+  uuid_ = BluetoothAdapterMac::BluetoothUUIDWithCBUUID([cb_descriptor_ UUID]);
   identifier_ = base::SysNSStringToUTF8(
       [NSString stringWithFormat:@"%s-%p", uuid_.canonical_value().c_str(),
                                  (void*)cb_descriptor_]);
 }
 
 std::string BluetoothRemoteGattDescriptorMac::GetIdentifier() const {
   return identifier_;
 }
 
 BluetoothUUID BluetoothRemoteGattDescriptorMac::GetUUID() const {
   return uuid_;
 }
 
 BluetoothGattCharacteristic::Permissions
 BluetoothRemoteGattDescriptorMac::GetPermissions() const {
   NOTIMPLEMENTED();
   return BluetoothGattCharacteristic::PERMISSION_NONE;
 }
 
 const std::vector<uint8_t>& BluetoothRemoteGattDescriptorMac::GetValue() const {
   return value_;
 }
 
-BluetoothRemoteGattDescriptorMac::~BluetoothRemoteGattDescriptorMac() {}
+BluetoothRemoteGattDescriptorMac::~BluetoothRemoteGattDescriptorMac() {
+  if (!read_value_callbacks_.first.is_null()) {
+    std::pair<ValueCallback, ErrorCallback> callbacks;
+    callbacks.swap(read_value_callbacks_);
+    callbacks.second.Run(BluetoothGattService::GATT_ERROR_FAILED);
+  }
+  if (!write_value_callbacks_.first.is_null()) {
+    std::pair<base::Closure, ErrorCallback> callbacks;
+    callbacks.swap(write_value_callbacks_);
+    callbacks.second.Run(BluetoothGattService::GATT_ERROR_FAILED);
+  }
+}
 
-// Returns a pointer to the GATT characteristic that this characteristic
-// descriptor belongs to.
 BluetoothRemoteGattCharacteristic*
 BluetoothRemoteGattDescriptorMac::GetCharacteristic() const {
   return static_cast<BluetoothRemoteGattCharacteristic*>(gatt_characteristic_);
 }
 
 // Sends a read request to a remote characteristic descriptor to read its
 // value. |callback| is called to return the read value on success and
 // |error_callback| is called for failures.
 void BluetoothRemoteGattDescriptorMac::ReadRemoteDescriptor(
     const ValueCallback& callback,
     const ErrorCallback& error_callback) {
-  NOTIMPLEMENTED();
+  if (value_read_or_write_in_progress_) {
+    VLOG(1) << *this << ": Read failed, already in progress.";
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE,
+        base::Bind(error_callback,
+                   BluetoothRemoteGattService::GATT_ERROR_IN_PROGRESS));
+    return;
+  }
+  VLOG(1) << *this << ": Read value.";
+  value_read_or_write_in_progress_ = true;
+  read_value_callbacks_ = std::make_pair(callback, error_callback);
+  [GetCBPeripheral() readValueForDescriptor:cb_descriptor_];
 }
 
-// Sends a write request to a remote characteristic descriptor, to modify the
-// value of the descriptor with the new value |new_value|. |callback| is
-// called to signal success and |error_callback| for failures. This method
-// only applies to remote descriptors and will fail for those that are locally
-// hosted.
 void BluetoothRemoteGattDescriptorMac::WriteRemoteDescriptor(
-    const std::vector<uint8_t>& new_value,
+    const std::vector<uint8_t>& value,
     const base::Closure& callback,
     const ErrorCallback& error_callback) {
-  NOTIMPLEMENTED();
+  if (value_read_or_write_in_progress_) {
+    VLOG(1) << *this << ": Write failed, already in progress.";
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE,
+        base::Bind(error_callback,
+                   BluetoothRemoteGattService::GATT_ERROR_IN_PROGRESS));
+    return;
+  }
+  VLOG(1) << *this << ": Write value.";
+  value_read_or_write_in_progress_ = true;
+  write_value_callbacks_ = std::make_pair(callback, error_callback);
+  base::scoped_nsobject<NSData> nsdata_value(
+      [[NSData alloc] initWithBytes:value.data() length:value.size()]);
+  [GetCBPeripheral() writeValue:nsdata_value forDescriptor:GetCBDescriptor()];
+}
+
+void BluetoothRemoteGattDescriptorMac::DidUpdateValueForDescriptor(
+    NSError* error) {
+  if (!value_read_or_write_in_progress_) {
+    VLOG(1) << *this << ": Value updated, no read in progress.";
+    return;
+  }
+  std::pair<ValueCallback, ErrorCallback> callbacks;
+  callbacks.swap(read_value_callbacks_);
+  value_read_or_write_in_progress_ = false;
+  if (error) {
+    BluetoothGattService::GattErrorCode error_code =
+        BluetoothDeviceMac::GetGattErrorCodeFromNSError(error);
+    VLOG(1) << *this << ": Read value failed with error: "
+            << BluetoothAdapterMac::String(error)
+            << ", converted to error code: " << error_code;
+    callbacks.second.Run(error_code);
+    return;
+  }
+  VLOG(1) << *this << ": Value read.";
+  value_ = VectorValueFromObjC([cb_descriptor_ value]);
+  callbacks.first.Run(value_);
+}
+
+void BluetoothRemoteGattDescriptorMac::DidWriteValueForDescriptor(
+    NSError* error) {
+  if (!value_read_or_write_in_progress_) {
+    VLOG(1) << *this << ": Value written, no write in progress.";
+    return;
+  }
+  std::pair<base::Closure, ErrorCallback> callbacks;
+  callbacks.swap(write_value_callbacks_);
+  value_read_or_write_in_progress_ = false;
+  if (error) {
+    BluetoothGattService::GattErrorCode error_code =
+        BluetoothDeviceMac::GetGattErrorCodeFromNSError(error);
+    VLOG(1) << *this << ": Write value failed with error: "
+            << BluetoothAdapterMac::String(error)
+            << ", converted to error code: " << error_code;
+    callbacks.second.Run(error_code);
+    return;
+  }
+  VLOG(1) << *this << ": Value written.";
+  callbacks.first.Run();
+}
+
+CBPeripheral* BluetoothRemoteGattDescriptorMac::GetCBPeripheral() const {
+  return gatt_characteristic_->GetCBPeripheral();
 }
 
 CBDescriptor* BluetoothRemoteGattDescriptorMac::GetCBDescriptor() const {
-  return cb_descriptor_.get();
+  return cb_descriptor_;
 }
 
 DEVICE_BLUETOOTH_EXPORT std::ostream& operator<<(
     std::ostream& out,
     const BluetoothRemoteGattDescriptorMac& descriptor) {
   const BluetoothRemoteGattCharacteristicMac* characteristic_mac =
       static_cast<const BluetoothRemoteGattCharacteristicMac*>(
           descriptor.GetCharacteristic());
   return out << "<BluetoothRemoteGattServiceMac "
              << descriptor.GetUUID().canonical_value() << "/" << &descriptor
              << ", characteristic: "
              << characteristic_mac->GetUUID().canonical_value() << "/"
              << characteristic_mac << ">";
 }
 
 }  // namespace device.