13

I am communicating with a BLE device that sends me lots of data via one characteristic. The same Characteristic is used to send data to the device.

Inside Androids BluetoothGattCharacteristic there are the methods

public byte[] getValue() {
    return mValue;
}

public boolean setValue(byte[] value) {
    mValue = value;
    return true;
}

However, the execution happens from different threads. Android runs about 5 different binder-threads and they call

onCharacteristicChanged(BluetoothGatt gatt, BluetoothGattCharacteristic characteristic)

I now try to grab the array as first operation in the callback, but it is NOT guaranteed that another thread (not under my control) is setting the array at the same time.

While above seems to do the trick, a more complicated matter is sending data 'against an incoming stream of data'.

I have to use the same Characteristic to send data down to the device, so I setValue() and then BluetoothGatt.writeCharacteristic.

public boolean writeCharacteristic(BluetoothGattCharacteristic characteristic) {
// some null checks etc

//now it locks the device
synchronized(mDeviceBusy) {
    if (mDeviceBusy) return false;
    mDeviceBusy = true;
}

//the actual execution

return true;
}

I will then at some point receive a callback from some thread

onCharacteristicWrite(BluetoothGatt gatt, BluetoothGattCharacteristic characteristic, int status)

However, when I grab the value and try to check if it is what I wanted to send, sometimes it already is a received package that has just been updated from some other thread.

How could i make this more thread-safe without having access to Androids BLE API or the stack etc ?

Alex Bravo
  • 1,601
  • 2
  • 24
  • 40
NikkyD
  • 2,209
  • 1
  • 16
  • 31
  • Very good question. I can only tell that when I tested this a few years ago (but only in one direction: notifications) I tested to do a thread sleep in the onCharacteristicChanged handler and found out that it actually only did one callback at a time, i.e. it waited for the completion until the next was called. Even though I noted that the callbacks always were on different threads. – Emil Aug 12 '16 at 22:51
  • as far as i can see in the BluetoothGatt source, the onNotify, which leads to onCharacteristicChanged does not look at the mDeviceBusy lock and simply overwrites the value of the characteristic removing any full-duplex capability – NikkyD Aug 16 '16 at 11:11
  • 1
    I found out that the "Binder" mechanism makes sure that only one callback will be executed at a time per BluetoothGatt object. That means there is no issue of getting a stream of notifications. However there is still a race condition if a write is done from another thread and an notification comes in at the same time on the same characteristic. – Emil Sep 19 '17 at 20:42
  • and it is that race condition that is dangerous. I just can't be sure if what i sent/received is true or if it's what i just wrote to the buffer at the time i received something. – NikkyD Sep 22 '17 at 16:39
  • Did anyone solve this issue? – Jakkra Sep 21 '18 at 15:08
  • AFAIK there has been no change but i only get lots of data from one side, so the issue isn't hurting me that much – NikkyD Oct 05 '18 at 10:26

2 Answers2

3

In SDK27, the onNotify() callback function in BluetoothGatt.java was updated to call BOTH BluetoothGattCharacteristic.setValue() and BluetoothGattCallback.onCharacteristicChanged() in the Runnable's run().

This change allows us to force all calls to BluetoothGattCharacteristic.setValue() - both for our outbound writing to the characteristic and the inbound notifications - onto the same thread, which eliminates the race condition corrupting the BluetoothGattCharacteristic.mValue;

  1. Create a HandlerThread
  2. Create a Handler attached to your HandlerThread
  3. Pass your Handler into BluetoothDevice.connectGatt() - congratulations, when a notify is received the setValue() and onCharacteristicChanged() will be called on your HandlerThread.
  4. When you want to write to the characteristic, post your setValue() and writeCharacteristic() to your HandlerThread via your Handler

Now all the function calls that were involved in the race condition are being executed on the same thread, eliminating the race condition.

Emil
  • 16,784
  • 2
  • 41
  • 52
Michael Fomin
  • 31
  • 4
2

Before SDK27, it appears to be impossible to have reliable full duplex communication over a single characteristic - using the public API.

Quite vexing.

There appears to be a way, though, if you're willing to cheat a little. (There is probably more than one way to cheat; the one described below is fairly low-impact.)

The problem is the use of the mValue field in BluetoothGattCharacteristic for two conflicting purposes - send and receive. It is fairly bad API design; one API-level fix would have been to introduce another field, so that there would be one for each direction. Another would have been to not use the setValue() method when sending data, but instead provided the data in question as a parameter to writeCharacteristic() (though this is complicated by the fact that the value field may be encoded during get/set, supporting several data types). However, it appears that the API maintainers have chosen a different path.

Anyway - to fix the problem, ensure that the received value and the value-to-be-sent are stored in different locations. More specifically, in the value fields of two different instances of BluetoothGattCharacteristic.

Now, cloning a BGC is not possible using the official API. Using the public constructor, you can get an instance which has all fields set except service and instanceId. These have public getters - to set them, use reflection to access setService() and setInstanceId() - this is the cheaty part.

I have tested this approach[1], and it appears to work as desired. :-)

[1] A variation of this approach, in fact; in newer SDK versions, the class in question is Parcelable, so when possible I clone the object through Parcel-serialization, just in case future implementations add more fields. This takes care of everything except the service field, which you still need to set using reflection.

eriksoe
  • 252
  • 1
  • 3