From 6d18706c7ceb09b5ec5bc57dbde508c8329c1785 Mon Sep 17 00:00:00 2001 From: Alex Blasche Date: Mon, 2 Mar 2020 13:42:43 +0100 Subject: Support characteristic Write Without Response on BlueZ 5.50+ BlueZ 5.50 adds support for this feature. This patch was provided by David Lechner via QTBUG-81696. Fixes: QTBUG-81696 Change-Id: I2a73b173821e36534b3848f7d0e35df16f118011 Reviewed-by: Konstantin Ritt Reviewed-by: Oliver Wolff Reviewed-by: Timur Pocheptsov --- src/bluetooth/qlowenergycontroller_bluezdbus.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/bluetooth/qlowenergycontroller_bluezdbus.cpp b/src/bluetooth/qlowenergycontroller_bluezdbus.cpp index 5dde8774..d69fb6cd 100644 --- a/src/bluetooth/qlowenergycontroller_bluezdbus.cpp +++ b/src/bluetooth/qlowenergycontroller_bluezdbus.cpp @@ -851,7 +851,7 @@ void QLowEnergyControllerPrivateBluezDBus::onCharWriteFinished(QDBusPendingCallW updateValueOfCharacteristic(nextJob.handle, nextJob.value, false); QLowEnergyCharacteristic ch(service, nextJob.handle); - // write without respone implies zero feedback + // write without response implies zero feedback if (nextJob.writeMode == QLowEnergyService::WriteWithResponse) { qCDebug(QT_BT_BLUEZ) << "Written Char:" << charData.uuid << nextJob.value.toHex(); emit service->characteristicWritten(ch, nextJob.value); @@ -971,7 +971,12 @@ void QLowEnergyControllerPrivateBluezDBus::scheduleNextJob() if (charData.uuid != QBluetoothUuid(gattChar.characteristic->uUID())) continue; - QDBusPendingReply<> reply = gattChar.characteristic->WriteValue(nextJob.value, QVariantMap()); + QVariantMap options; + // The "type" option only works with BlueZ >= 5.50, older versions always write with response + options[QStringLiteral("type")] = nextJob.writeMode == QLowEnergyService::WriteWithoutResponse ? + QStringLiteral("command") : QStringLiteral("request"); + QDBusPendingReply<> reply = gattChar.characteristic->WriteValue(nextJob.value, options); + QDBusPendingCallWatcher* watcher = new QDBusPendingCallWatcher(reply, this); connect(watcher, &QDBusPendingCallWatcher::finished, this, &QLowEnergyControllerPrivateBluezDBus::onCharWriteFinished); -- cgit v1.2.3 From 9931a174a699411a0ebccd0be496d8de26713ca6 Mon Sep 17 00:00:00 2001 From: Marius Kittler Date: Tue, 24 Dec 2019 13:06:11 +0100 Subject: Fix case of setupapi.h for mingw-w64 Change-Id: I9ddb290a10a620334abaf0500dd7b280e86b3a03 Reviewed-by: Thiago Macieira --- src/bluetooth/qlowenergycontroller_win.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bluetooth/qlowenergycontroller_win.cpp b/src/bluetooth/qlowenergycontroller_win.cpp index ced69685..7a30be9a 100644 --- a/src/bluetooth/qlowenergycontroller_win.cpp +++ b/src/bluetooth/qlowenergycontroller_win.cpp @@ -51,7 +51,7 @@ #include // for std::max -#include +#include QT_BEGIN_NAMESPACE -- cgit v1.2.3 From 962f282d21e62b8ddc170cd62cbd918f56493ab8 Mon Sep 17 00:00:00 2001 From: Thiemo van Engelen Date: Fri, 6 Dec 2019 11:29:54 +0100 Subject: Fix a race condition between a write and an incoming change The Android API that is available to write to a characteristic requires the user to first set the new value of the characteristic and then tell Android to write the value to the device. If the phone processes an incoming change for this same characteristic in between the setting of the value and the actual write, it could happen that the characteristic gets the incoming value and that incoming value is then sent to the connected device. Something similar could occur for the incoming change as the received value is not passed in the callback but it is set on the characteristic and the user is expected to read the received value from the characteristic. To prevent these problems, a new connectGatt function that takes a Handler as extra argument was added to SDK v26. The callbacks are then always executed via this handler and thus on the associated handler thread. If the user also performs all writes on this thread, the described race condition can no longer occur. This could have solved the problems, but Android 8.0 contained a race condition in the callback part because it set the value of the characteristic before passing the callback to the handler, still allowing the same race condition for incoming changed. This was fixed in Android 8.1. This commit causes the code to use a Handler and the new connectGatt function if it is running on SDK >= 27, which is associated with Android 8.1. For older SDK versions, a different solution is implemented. In this case, a temporary BluetoothGattCharacteristic is instantiated that can hold the value to be written. This way, the write can no longer interfere with changes that are being received. To instantiate the BluetoothGattCharacteristic, a private constructor is invoked using reflection. This private constructor is needed as only this constructor allows to create an instance that has the required information for Android to be able to write to the charatceristic (such as the associated service and instanceId). This is also the reason why this solution cannot be used on newer SDK's as this constructor is blacklisted and a warning box will be shown when reflection is used to get a reference to it. Because a temporary BluetoothGattCharacteristic is instantiated that holds the written value, it is necessary to store the value that was written and pass that in the callback. More information on these Android issues and solution direction can be found here: https://stackoverflow.com/questions/38922639/how-could-i-achieve-maximum-thread-safety-with-a-read-write-ble-gatt-characteris https://medium.com/@martijn.van.welie/making-android-ble-work-part-3-117d3a8aee23 Change-Id: I5a9eda501a20933e37f758a3114bf71ec3e7cfd4 Reviewed-by: Thiemo van Engelen Reviewed-by: Konstantin Ritt Reviewed-by: Oliver Wolff --- .../qt5/android/bluetooth/QtBluetoothLE.java | 178 ++++++++++++++++----- 1 file changed, 142 insertions(+), 36 deletions(-) diff --git a/src/android/bluetooth/src/org/qtproject/qt5/android/bluetooth/QtBluetoothLE.java b/src/android/bluetooth/src/org/qtproject/qt5/android/bluetooth/QtBluetoothLE.java index f70ee8a4..3e602186 100644 --- a/src/android/bluetooth/src/org/qtproject/qt5/android/bluetooth/QtBluetoothLE.java +++ b/src/android/bluetooth/src/org/qtproject/qt5/android/bluetooth/QtBluetoothLE.java @@ -55,11 +55,12 @@ import android.bluetooth.le.ScanSettings; import android.content.Context; import android.os.Build; import android.os.Handler; +import android.os.HandlerThread; import android.os.Looper; import android.util.Log; +import java.lang.reflect.Constructor; import java.lang.reflect.Method; import java.util.concurrent.atomic.AtomicInteger; -import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Hashtable; @@ -75,6 +76,9 @@ public class QtBluetoothLE { private boolean mLeScanRunning = false; private BluetoothGatt mBluetoothGatt = null; + private HandlerThread mHandlerThread = null; + private Handler mHandler = null; + private Constructor mCharacteristicConstructor = null; private String mRemoteGattAddress; private final UUID clientCharacteristicUuid = UUID.fromString("00002902-0000-1000-8000-00805f9b34fb"); private final int MAX_MTU = 512; @@ -219,8 +223,13 @@ public class QtBluetoothLE { resetData(); // reset mBluetoothGatt, reusing same object is not very reliable // sometimes it reconnects and sometimes it does not. - if (mBluetoothGatt != null) + if (mBluetoothGatt != null) { mBluetoothGatt.close(); + if (mHandler != null) { + mHandler.getLooper().quitSafely(); + mHandler = null; + } + } mBluetoothGatt = null; break; case BluetoothProfile.STATE_CONNECTED: @@ -377,10 +386,11 @@ public class QtBluetoothLE { errorCode = 2; break; // CharacteristicWriteError } + byte[] value = pendingJob.newValue; synchronized (readWriteQueue) { ioJobPending = false; } - leCharacteristicWritten(qtObject, handle+1, characteristic.getValue(), errorCode); + leCharacteristicWritten(qtObject, handle+1, value, errorCode); performNextIO(); } @@ -572,29 +582,86 @@ public class QtBluetoothLE { return false; } - try { - // BluetoothDevice.connectGatt(Context, boolean, BluetoothGattCallback, int) was - // officially introduced by Android API v23. Earlier Android versions have a private - // implementation already though. Let's check at runtime and use it if possible. - // - // In general the new connectGatt() seems to be much more reliable than the function - // that doesn't specify the transport layer. - - Class[] args = new Class[4]; + /* The required connectGatt function is already available in SDK v26, but Android 8.0 + * contains a race condition in the Changed callback such that it can return the value that + * was written. This is fixed in Android 8.1, which matches SDK v27. */ + if (Build.VERSION.SDK_INT >= 27) { + HandlerThread handlerThread = new HandlerThread("QtBluetoothLEHandlerThread"); + handlerThread.start(); + mHandler = new Handler(handlerThread.getLooper()); + + Class[] args = new Class[6]; args[0] = android.content.Context.class; args[1] = boolean.class; args[2] = android.bluetooth.BluetoothGattCallback.class; args[3] = int.class; - Method connectMethod = mRemoteGattDevice.getClass().getDeclaredMethod("connectGatt", args); - if (connectMethod != null) { - mBluetoothGatt = (BluetoothGatt) connectMethod.invoke(mRemoteGattDevice, qtContext, - false, gattCallback, - 2 /*TRANSPORT_LE*/); - Log.w(TAG, "Using Android v23 BluetoothDevice.connectGatt()"); + args[4] = int.class; + args[5] = android.os.Handler.class; + + try { + Method connectMethod = mRemoteGattDevice.getClass().getDeclaredMethod("connectGatt", args); + if (connectMethod != null) { + mBluetoothGatt = (BluetoothGatt) connectMethod.invoke(mRemoteGattDevice, qtContext, false, + gattCallback, 2 /* TRANSPORT_LE */, 1 /*BluetoothDevice.PHY_LE_1M*/, mHandler); + Log.w(TAG, "Using Android v26 BluetoothDevice.connectGatt()"); + } + } catch (Exception ex) { + Log.w(TAG, "connectGatt() v26 not available"); + ex.printStackTrace(); + } + + if (mBluetoothGatt == null) { + mHandler.getLooper().quitSafely(); + mHandler = null; + } + } + + if (mBluetoothGatt == null) { + try { + //This API element is currently: greylist-max-o, reflection, allowed + //It may change in the future + Class[] constr_args = new Class[5]; + constr_args[0] = android.bluetooth.BluetoothGattService.class; + constr_args[1] = java.util.UUID.class; + constr_args[2] = int.class; + constr_args[3] = int.class; + constr_args[4] = int.class; + mCharacteristicConstructor = BluetoothGattCharacteristic.class.getDeclaredConstructor(constr_args); + mCharacteristicConstructor.setAccessible(true); + } catch (NoSuchMethodException ex) { + Log.w(TAG, "Unable get characteristic constructor. Buffer race condition are possible"); + /* For some reason we don't get the private BluetoothGattCharacteristic ctor. + This means that we cannot protect ourselves from issues where concurrent + read and write operations on the same char can overwrite each others buffer. + Nevertheless we continue with best effort. + */ + } + + try { + // BluetoothDevice.connectGatt(Context, boolean, BluetoothGattCallback, int) was + // officially introduced by Android API v23. Earlier Android versions have a + // private + // implementation already though. Let's check at runtime and use it if possible. + // + // In general the new connectGatt() seems to be much more reliable than the + // function + // that doesn't specify the transport layer. + + Class[] args = new Class[4]; + args[0] = android.content.Context.class; + args[1] = boolean.class; + args[2] = android.bluetooth.BluetoothGattCallback.class; + args[3] = int.class; + Method connectMethod = mRemoteGattDevice.getClass().getDeclaredMethod("connectGatt", args); + if (connectMethod != null) { + mBluetoothGatt = (BluetoothGatt) connectMethod.invoke(mRemoteGattDevice, qtContext, false, + gattCallback, 2 /* TRANSPORT_LE */); + Log.w(TAG, "Using Android v23 BluetoothDevice.connectGatt()"); + } + } catch (Exception ex) { + // fallback to less reliable API 18 version + mBluetoothGatt = mRemoteGattDevice.connectGatt(qtContext, false, gattCallback); } - } catch (Exception ex) { - // fallback to less reliable API 18 version - mBluetoothGatt = mRemoteGattDevice.connectGatt(qtContext, false, gattCallback); } return mBluetoothGatt != null; @@ -658,6 +725,7 @@ public class QtBluetoothLE { private final LinkedList readWriteQueue = new LinkedList(); private boolean ioJobPending; + private ReadWriteJob pendingJob; /* Internal helper function @@ -864,8 +932,7 @@ public class QtBluetoothLE { servicesToBeDiscovered.add(serviceHandle); scheduleServiceDetailDiscovery(serviceHandle); - performNextIO(); - + performNextIOThreaded(); } catch (Exception ex) { ex.printStackTrace(); return false; @@ -949,8 +1016,11 @@ public class QtBluetoothLE { return true; } - private void scheduleMtuExchange() - { + /* + * Already executed in GattCallback so executed by the HandlerThread. No need to + * post it to the Hander. + */ + private void scheduleMtuExchange() { ReadWriteJob newJob = new ReadWriteJob(); newJob.jobType = IoJobType.Mtu; newJob.entry = null; @@ -1063,7 +1133,7 @@ public class QtBluetoothLE { return false; } - performNextIO(); + performNextIOThreaded(); return true; } @@ -1100,7 +1170,7 @@ public class QtBluetoothLE { return false; } - performNextIO(); + performNextIOThreaded(); return true; } @@ -1135,7 +1205,7 @@ public class QtBluetoothLE { return false; } - performNextIO(); + performNextIOThreaded(); return true; } @@ -1166,7 +1236,7 @@ public class QtBluetoothLE { return false; } - performNextIO(); + performNextIOThreaded(); return true; } @@ -1180,7 +1250,7 @@ public class QtBluetoothLE { ioJobPending = false; } - performNextIO(); + performNextIOThreaded(); if (handle == HANDLE_FOR_MTU_EXCHANGE) return; @@ -1205,6 +1275,24 @@ public class QtBluetoothLE { } } + /* + Wrapper around performNextIO() ensuring that performNextIO() is executed inside + the mHandler/mHandlerThread if it exists. + */ + private void performNextIOThreaded() + { + if (mHandler != null) { + mHandler.post(new Runnable() { + @Override + public void run() { + performNextIO(); + } + }); + } else { + performNextIO(); + } + } + /* The queuing is required because two writeCharacteristic/writeDescriptor calls cannot execute at the same time. The second write must happen after the @@ -1265,6 +1353,7 @@ public class QtBluetoothLE { handleForTimeout.set(HANDLE_FOR_RESET); // not a pending call -> release atomic } else { ioJobPending = true; + pendingJob = nextJob; timeoutHandler.postDelayed(new TimeoutRunnable( modifiedReadWriteHandle(handle, nextJob.jobType)), RUNNABLE_TIMEOUT); } @@ -1353,6 +1442,16 @@ public class QtBluetoothLE { } } + private BluetoothGattCharacteristic cloneChararacteristic(BluetoothGattCharacteristic other) { + try { + return (BluetoothGattCharacteristic) mCharacteristicConstructor.newInstance(other.getService(), + other.getUuid(), other.getInstanceId(), other.getProperties(), other.getPermissions()); + } catch (Exception ex) { + Log.w(TAG, "Cloning characteristic failed!" + ex); + return null; + } + } + // Runs inside the Mutex on readWriteQueue. // Returns true if nextJob should be skipped. private boolean executeWriteJob(ReadWriteJob nextJob) @@ -1360,13 +1459,20 @@ public class QtBluetoothLE { boolean result; switch (nextJob.entry.type) { case Characteristic: - if (nextJob.entry.characteristic.getWriteType() != nextJob.requestedWriteType) { - nextJob.entry.characteristic.setWriteType(nextJob.requestedWriteType); + if (mHandler != null || mCharacteristicConstructor == null) { + if (nextJob.entry.characteristic.getWriteType() != nextJob.requestedWriteType) { + nextJob.entry.characteristic.setWriteType(nextJob.requestedWriteType); + } + result = nextJob.entry.characteristic.setValue(nextJob.newValue); + return !result || !mBluetoothGatt.writeCharacteristic(nextJob.entry.characteristic); + } else { + BluetoothGattCharacteristic orig = nextJob.entry.characteristic; + BluetoothGattCharacteristic tmp = cloneChararacteristic(orig); + if (tmp == null) + return true; + tmp.setWriteType(nextJob.requestedWriteType); + return !tmp.setValue(nextJob.newValue) || !mBluetoothGatt.writeCharacteristic(tmp); } - result = nextJob.entry.characteristic.setValue(nextJob.newValue); - if (!result || !mBluetoothGatt.writeCharacteristic(nextJob.entry.characteristic)) - return true; - break; case Descriptor: if (nextJob.entry.descriptor.getUuid().compareTo(clientCharacteristicUuid) == 0) { /* -- cgit v1.2.3 From 539fce5bf6bb5b1e3d599ea6e41354c80a79500f Mon Sep 17 00:00:00 2001 From: Lars Schmertmann Date: Mon, 24 Feb 2020 14:34:20 +0100 Subject: Also add a response for a request if reportError is called When using QNearFieldTarget::sendCommand on Android an exception can occur. In this case reportError is called and handleResponse is omitted. A call to QNearFieldTarget::waitForRequestCompleted will result in a full usage of the specified timeout, even if the exception occurred. To avoid this behavior also add an (invalid) response in reportError. Change-Id: Id80b7a97da5628d229b942803ec1a2fb13536d99 Reviewed-by: Alex Blasche --- src/nfc/qnearfieldtarget.cpp | 1 + .../qnearfieldtagtype1/tst_qnearfieldtagtype1.cpp | 19 ++++++++++--------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/nfc/qnearfieldtarget.cpp b/src/nfc/qnearfieldtarget.cpp index 2905c380..814747a4 100644 --- a/src/nfc/qnearfieldtarget.cpp +++ b/src/nfc/qnearfieldtarget.cpp @@ -544,6 +544,7 @@ bool QNearFieldTarget::handleResponse(const QNearFieldTarget::RequestId &id, void QNearFieldTarget::reportError(QNearFieldTarget::Error error, const QNearFieldTarget::RequestId &id) { + setResponseForRequest(id, QVariant(), false); QMetaObject::invokeMethod(this, [this, error, id]() { Q_EMIT this->error(error, id); }, Qt::QueuedConnection); diff --git a/tests/auto/qnearfieldtagtype1/tst_qnearfieldtagtype1.cpp b/tests/auto/qnearfieldtagtype1/tst_qnearfieldtagtype1.cpp index fb7aa7ae..2d1f052e 100644 --- a/tests/auto/qnearfieldtagtype1/tst_qnearfieldtagtype1.cpp +++ b/tests/auto/qnearfieldtagtype1/tst_qnearfieldtagtype1.cpp @@ -174,8 +174,7 @@ void tst_QNearFieldTagType1::staticMemoryModel() quint8 byte = target->requestResponse(id).toUInt(); id = target->writeByte(i, 0x55); - QVERIFY(!target->waitForRequestCompleted(id,50)); - + QVERIFY(target->waitForRequestCompleted(id)); QVERIFY(!target->requestResponse(id).isValid()); id = target->readByte(i); @@ -224,8 +223,7 @@ void tst_QNearFieldTagType1::staticMemoryModel() quint8 byte = target->requestResponse(id).toUInt(); id = target->writeByte(i, 0x55); - QVERIFY(!target->waitForRequestCompleted(id,50)); - + QVERIFY(target->waitForRequestCompleted(id)); QVERIFY(!target->requestResponse(id).isValid()); id = target->readByte(i); @@ -272,7 +270,7 @@ void tst_QNearFieldTagType1::dynamicMemoryModel() QCOMPARE(quint8(block.at(7)), quint8(0x00)); id = target->writeBlock(0x00, QByteArray(8, quint8(0x55))); - QVERIFY(!target->waitForRequestCompleted(id,50)); + QVERIFY(target->waitForRequestCompleted(id)); QVERIFY(!target->requestResponse(id).isValid()); QCOMPARE(target->uid(), block.left(7)); @@ -338,7 +336,7 @@ void tst_QNearFieldTagType1::dynamicMemoryModel() QByteArray block = target->requestResponse(id).toByteArray(); id = target->writeBlock(i, QByteArray(8, quint8(0x55))); - QVERIFY(!target->waitForRequestCompleted(id,50)); + QVERIFY(target->waitForRequestCompleted(id)); QVERIFY(!target->requestResponse(id).isValid()); id = target->readBlock(i); @@ -350,14 +348,17 @@ void tst_QNearFieldTagType1::dynamicMemoryModel() for (int i = 0; i < 256; ++i) { QNearFieldTarget::RequestId id = target->readBlock(i); - QVERIFY(!target->waitForRequestCompleted(id,50)); + QVERIFY(target->waitForRequestCompleted(id)); + QVERIFY(!target->requestResponse(id).isValid()); id = target->writeBlock(i, QByteArray(8, quint8(0x55))); - QVERIFY(!target->waitForRequestCompleted(id,50)); + QVERIFY(target->waitForRequestCompleted(id)); + QVERIFY(!target->requestResponse(id).isValid()); } for (int i = 0; i < 16; ++i) { QNearFieldTarget::RequestId id = target->readSegment(i); - QVERIFY(!target->waitForRequestCompleted(id,50)); + QVERIFY(target->waitForRequestCompleted(id)); + QVERIFY(!target->requestResponse(id).isValid()); } } } -- cgit v1.2.3 From 58e0224a6677667b1102b34ad5b35a787bd74077 Mon Sep 17 00:00:00 2001 From: Denis Pronin Date: Wed, 4 Mar 2020 00:59:07 +0300 Subject: support building with clang and libc++ fixed undefined errno when compiling with clang++ and libc++ Change-Id: I30be6e2da36f9189261535f103ce689462634f49 Reviewed-by: Alex Blasche --- src/bluetooth/bluez/bluetoothmanagement.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/bluetooth/bluez/bluetoothmanagement.cpp b/src/bluetooth/bluez/bluetoothmanagement.cpp index 31d3dc02..b6ad93e5 100644 --- a/src/bluetooth/bluez/bluetoothmanagement.cpp +++ b/src/bluetooth/bluez/bluetoothmanagement.cpp @@ -51,6 +51,7 @@ #include #include +#include QT_BEGIN_NAMESPACE -- cgit v1.2.3