Index: device/bluetooth/bluetooth_remote_gatt_characteristic_android.cc |
diff --git a/device/bluetooth/bluetooth_remote_gatt_characteristic_android.cc b/device/bluetooth/bluetooth_remote_gatt_characteristic_android.cc |
index d8d029900367defb7a0fc01af7d8895132b876fe..bb7e6696e7e55bfd7a930321243befbdd1541970 100644 |
--- a/device/bluetooth/bluetooth_remote_gatt_characteristic_android.cc |
+++ b/device/bluetooth/bluetooth_remote_gatt_characteristic_android.cc |
@@ -136,20 +136,60 @@ bool BluetoothRemoteGattCharacteristicAndroid::UpdateValue( |
void BluetoothRemoteGattCharacteristicAndroid::StartNotifySession( |
const NotifySessionCallback& callback, |
const ErrorCallback& error_callback) { |
- if (Java_ChromeBluetoothRemoteGattCharacteristic_startNotifySession( |
- AttachCurrentThread(), j_characteristic_.obj())) { |
- // TODO(crbug.com/569664): Wait until descriptor write completes before |
- // reporting success via calling |callback|. |
- scoped_ptr<device::BluetoothGattNotifySession> notify_session( |
- new BluetoothGattNotifySessionAndroid(instance_id_)); |
+ if (!pending_start_notify_calls_.empty()) { |
ortuno
2016/03/04 17:39:47
Why are we allowing multiple notify session calls?
scheib
2016/03/11 03:17:31
Unlike a read or write that must happen serially,
|
+ pending_start_notify_calls_.push(std::make_pair(callback, error_callback)); |
+ return; |
+ } |
+ |
+ Properties properties = GetProperties(); |
+ |
+ bool hasNotify = properties & PROPERTY_NOTIFY; |
+ bool hasIndicate = properties & PROPERTY_INDICATE; |
+ |
+ if (!hasNotify && !hasIndicate) { |
+ LOG(ERROR) << "Characteristic needs NOTIFY or INDICATE"; |
+ base::MessageLoop::current()->PostTask( |
+ FROM_HERE, |
+ base::Bind(error_callback, |
+ BluetoothRemoteGattServiceAndroid::GATT_ERROR_FAILED)); |
ortuno
2016/03/04 17:39:47
I think this should be GATT_ERROR_NOT_SUPPORTED.
scheib
2016/03/11 03:17:31
Done.
|
+ return; |
+ } |
+ |
+ BluetoothGattDescriptor* descriptor = GetDescriptorForUUID( |
+ BluetoothGattDescriptor::ClientCharacteristicConfigurationUuid()); |
+ |
+ if (!descriptor) { |
+ LOG(ERROR) |
+ << "Could not find client characteristic configuration descriptor"; |
base::MessageLoop::current()->PostTask( |
- FROM_HERE, base::Bind(callback, base::Passed(¬ify_session))); |
- } else { |
+ FROM_HERE, |
+ base::Bind(error_callback, |
+ BluetoothRemoteGattServiceAndroid::GATT_ERROR_FAILED)); |
ortuno
2016/03/04 17:39:47
Same here. I think GATT_ERROR_NOT_SUPPORTED is mor
scheib
2016/03/11 03:17:31
Done.
|
+ return; |
+ } |
+ |
+ if (!Java_ChromeBluetoothRemoteGattCharacteristic_setCharacteristicNotification( |
+ AttachCurrentThread(), j_characteristic_.obj(), true)) { |
+ LOG(ERROR) << "Error enabling characteristic notification"; |
base::MessageLoop::current()->PostTask( |
FROM_HERE, |
base::Bind(error_callback, |
BluetoothRemoteGattServiceAndroid::GATT_ERROR_FAILED)); |
+ return; |
} |
+ |
+ std::vector<uint8_t> value; |
ortuno
2016/03/04 17:39:47
nit: value(2) // The Client Characteristic Configu
scheib
2016/03/11 03:17:31
Done.
|
+ value.push_back(hasNotify ? 1 : 2); |
+ value.push_back(0); |
+ |
+ pending_start_notify_calls_.push(std::make_pair(callback, error_callback)); |
+ descriptor->WriteRemoteDescriptor( |
+ value, base::Bind(&BluetoothRemoteGattCharacteristicAndroid:: |
+ OnStartNotifySessionSuccess, |
+ base::Unretained(this)), |
ortuno
2016/03/04 17:39:47
Is this safe? I guess it is since the characterist
scheib
2016/03/11 03:17:31
Acknowledged.
|
+ base::Bind( |
+ &BluetoothRemoteGattCharacteristicAndroid::OnStartNotifySessionError, |
+ base::Unretained(this))); |
} |
void BluetoothRemoteGattCharacteristicAndroid::ReadRemoteCharacteristic( |
@@ -211,6 +251,25 @@ void BluetoothRemoteGattCharacteristicAndroid::OnChanged( |
adapter_->NotifyGattCharacteristicValueChanged(this, value_); |
} |
+void BluetoothRemoteGattCharacteristicAndroid::OnStartNotifySessionSuccess() { |
+ while (!pending_start_notify_calls_.empty()) { |
scheib
2016/03/02 06:15:20
Protect against reentrant code by swapping the mem
ortuno
2016/03/04 17:39:47
I just realized how easy it is to miss this. Do we
scheib
2016/03/11 03:17:31
Not yet -- we should. Here's a new issue: https://
|
+ PendingStartNotifyCall callbacks = pending_start_notify_calls_.front(); |
+ pending_start_notify_calls_.pop(); |
+ scoped_ptr<device::BluetoothGattNotifySession> notify_session( |
+ new BluetoothGattNotifySessionAndroid(instance_id_)); |
+ callbacks.first.Run(std::move(notify_session)); |
+ } |
+} |
+ |
+void BluetoothRemoteGattCharacteristicAndroid::OnStartNotifySessionError( |
+ BluetoothGattService::GattErrorCode error) { |
+ while (!pending_start_notify_calls_.empty()) { |
+ PendingStartNotifyCall callbacks = pending_start_notify_calls_.front(); |
+ pending_start_notify_calls_.pop(); |
+ callbacks.second.Run(error); |
+ } |
+} |
+ |
void BluetoothRemoteGattCharacteristicAndroid::OnRead( |
JNIEnv* env, |
const JavaParamRef<jobject>& jcaller, |