arc: bluetooth: Implement Gatt notify

components/arc/bluetooth/arc_bluetooth_bridge.cc

 // 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 "components/arc/bluetooth/arc_bluetooth_bridge.h"
 
 #include <fcntl.h>
 #include <stddef.h>
 
 #include <iomanip>
@@ skipping 28 matching lines @@
 using device::BluetoothGattCharacteristic;
 using device::BluetoothGattDescriptor;
 using device::BluetoothGattService;
 using device::BluetoothRemoteGattCharacteristic;
 using device::BluetoothRemoteGattDescriptor;
 using device::BluetoothRemoteGattService;
 using device::BluetoothUUID;
 
 namespace {
 const int kMinBtleVersion = 1;
+const int kMinBtleNotifyVersion = 2;

[ortuno, 2016/06/21 23:23:06]

enum { kMinBtleNotifyVersion = 2 };

Context:
https://groups.google.com/a/chromium.org/forum/#!msg/chromium-dev/hDqJ4KBVqog/

[puthik_chromium, 2016/06/23 02:53:34]

Done.

 const uint32_t kGattReadPermission =
     BluetoothGattCharacteristic::Permission::PERMISSION_READ |
     BluetoothGattCharacteristic::Permission::PERMISSION_READ_ENCRYPTED |
     BluetoothGattCharacteristic::Permission::
         PERMISSION_READ_ENCRYPTED_AUTHENTICATED;
 const uint32_t kGattWritePermission =
     BluetoothGattCharacteristic::Permission::PERMISSION_WRITE |
     BluetoothGattCharacteristic::Permission::PERMISSION_WRITE_ENCRYPTED |
     BluetoothGattCharacteristic::Permission::
         PERMISSION_WRITE_ENCRYPTED_AUTHENTICATED;
@@ skipping 187 matching lines @@
     BluetoothRemoteGattDescriptor* descriptor) {
   // Placeholder for GATT client functionality
 }
 
 void ArcBluetoothBridge::GattDescriptorRemoved(
     BluetoothAdapter* adapter,
     BluetoothRemoteGattDescriptor* descriptor) {
   // Placeholder for GATT client functionality
 }
 
 void ArcBluetoothBridge::GattCharacteristicValueChanged(

[ortuno, 2016/06/21 20:21:29]

FYI: this function gets called when reading values as well. Is the android side
expecting OnGattNotify to be called when reading values?

[puthik_chromium, 2016/06/23 02:53:34]

Android side is not expect that. But it should be fine anyway.

     BluetoothAdapter* adapter,
     BluetoothRemoteGattCharacteristic* characteristic,
     const std::vector<uint8_t>& value) {
-  // Placeholder for GATT client functionality
+  if (!HasBluetoothInstance())
+    return;
+
+  if (arc_bridge_service()->bluetooth_version() < kMinBtleNotifyVersion) {
+    LOG(WARNING) << "Bluetooth instance is too old and does not support notify";
+    return;
+  }
+  BluetoothRemoteGattService* service = characteristic->GetService();
+  BluetoothDevice* device = service->GetDevice();
+  mojom::BluetoothAddressPtr address =
+      mojom::BluetoothAddress::From(device->GetAddress());
+  mojom::BluetoothGattServiceIDPtr service_id =
+      mojom::BluetoothGattServiceID::New();
+  service_id->is_primary = service->IsPrimary();
+  service_id->id = mojom::BluetoothGattID::New();
+  service_id->id->inst_id = ConvertGattIdentifierToId(service->GetIdentifier());
+  service_id->id->uuid = mojom::BluetoothUUID::From(service->GetUUID());
+
+  mojom::BluetoothGattIDPtr char_id = mojom::BluetoothGattID::New();
+  char_id->inst_id = ConvertGattIdentifierToId(characteristic->GetIdentifier());
+  char_id->uuid = mojom::BluetoothUUID::From(characteristic->GetUUID());
+
+  arc_bridge_service()->bluetooth_instance()->OnGattNotify(
+      std::move(address), std::move(service_id), std::move(char_id),
+      true /* is_notify */, mojo::Array<uint8_t>::From(value));
 }
 
 void ArcBluetoothBridge::GattDescriptorValueChanged(
     BluetoothAdapter* adapter,
     BluetoothRemoteGattDescriptor* descriptor,
     const std::vector<uint8_t>& value) {
   // Placeholder for GATT client functionality
 }
 
 void ArcBluetoothBridge::EnableAdapter(const EnableAdapterCallback& callback) {
@@ skipping 651 matching lines @@
     const RegisterForGattNotificationCallback& callback,
     BluetoothGattService::GattErrorCode error_code) const {
   callback.Run(mojo::ConvertTo<mojom::BluetoothGattStatus>(error_code));
 }
 
 void ArcBluetoothBridge::OnGattNotifyStopDone(
     const DeregisterForGattNotificationCallback& callback) const {
   callback.Run(mojom::BluetoothGattStatus::GATT_SUCCESS);
 }
 
 void ArcBluetoothBridge::RegisterForGattNotification(

[ortuno, 2016/06/21 23:23:06]

Does calling setCharacteristicNotification()[1] result in this function getting
called? If so there are a couple of problems with this implementation:

On a real android device calling setCharacteristicNotification() does *not*
write to the CCC descriptor. If you call StartNotifySession then you will write
to the descriptor. Which means the behavior on Arc++ will be different than the
behavior in Android.

All BLE Android code that wants to use notifications writes to the CCC
Descriptor[2]. Bluez blocks access to this descriptor so calls to write to the
descriptor will fail. This could result in Apps not being able to use
notifications in Arc++.

Have you thought about these problems?

[1] https://developer.android.com/reference/android/bluetooth/BluetoothGatt.html
[2]
https://developer.android.com/guide/topics/connectivity/bluetooth-le.html#not...

[puthik_chromium, 2016/06/23 02:53:34]

Thank you for pointing this out.
Since the CCC is only used for start notification, I think the best work-around
is just always return write success when trying to write to CCC without doing
anything.

[ortuno, 2016/06/23 14:29:38]

Developers will write to the CCC to stop notifications and if the implementation
does nothing, notifications will continue to arrive. This seems very unexpected.

Also a question:

If setCharacteristicNotification, which is a synchronous function, ends up
calling this function which is an async function, how do you notify of
success/failure if for example the characteristic doesn't support notifications
or there was an error when writing to the descriptor?

[puthik_chromium, 2016/06/23 18:36:30]

Q1 Developers will write to the CCC to stop notifications...
From the app I tested[1], they will write CCC to disable and then call the
deregister_for_notification. And it worked as expected.

Q2 how do you notify of success/failure for notify
Chrome side has OnGattNotifyStartDone/OnGattNotifyStartError callback to Android
side.
Android side use Future class to make it synchronous [2].

[1]
https://play.google.com/store/apps/details?id=no.nordicsemi.android.mcp&hl=en
This app also provide library for other developers. 
[2]
https://googleplex-android.googlesource.com/device/google/cheets2/+/591fc94de...
    Future<BluetoothGattStatus> status;
    mBluetoothHost->RegisterForGattNotification(std::move(remote_addr),
std::move(service_id),
                                                std::move(char_id),
                                               
FutureCallback<BluetoothGattStatus>(&status));
    return status.get();

[ortuno, 2016/06/23 19:13:54]

Does this mean a call to setCharacteristicNotification will block until we've
written the descriptor?

     mojom::BluetoothAddressPtr remote_addr,
     mojom::BluetoothGattServiceIDPtr service_id,
     mojom::BluetoothGattIDPtr char_id,
     const RegisterForGattNotificationCallback& callback) {
   BluetoothRemoteGattCharacteristic* characteristic = FindGattCharacteristic(
       std::move(remote_addr), std::move(service_id), std::move(char_id));
 
   if (!characteristic) {
     LOG(WARNING) << __func__ << " Characteristic is not existed.";
     return;
@@ skipping 369 matching lines @@
     // to
     // make sure the bond state machine on Android is ready to take the
     // pair-done event. Otherwise the pair-done event will be dropped as an
     // invalid change of paired status.
     OnPairing(addr->Clone());
     OnPairedDone(std::move(addr));
   }
 }
 
 }  // namespace arc