From 4789deec2db226a41f60061666c1e5b36a16abd9 Mon Sep 17 00:00:00 2001 From: Andreas Buhr Date: Wed, 24 Feb 2021 13:07:11 +0100 Subject: Correct error handling in QLowEnergyControllerPrivateBluezDBUS When a DBUS interface was removed for a device, error handling just assumed it was the org.bluez.Device1 interface, without checking. This lead to errors on disconnection if the device has a battery service: The code sends a disconnection request, then an InterfaceRemoved event for org.bluez.Battery1 occurs. This was interpreted as a removal of org.bluez.Device1 which in turn led to the controller going into UnknownRemoteDeviceError error state. This patch adds a check whether it is really org.bluez.Device1 which is removed. Change-Id: I449b29cb9528cda23ce972f36f716a8774f01fe5 Reviewed-by: Alex Blasche (cherry picked from commit af5f801c6b8de0bf0d21798bddef54cb14759ecd) Reviewed-by: Qt Cherry-pick Bot --- src/bluetooth/qlowenergycontroller_bluezdbus.cpp | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/bluetooth/qlowenergycontroller_bluezdbus.cpp b/src/bluetooth/qlowenergycontroller_bluezdbus.cpp index bacd2c62..8720c549 100644 --- a/src/bluetooth/qlowenergycontroller_bluezdbus.cpp +++ b/src/bluetooth/qlowenergycontroller_bluezdbus.cpp @@ -208,12 +208,17 @@ void QLowEnergyControllerPrivateBluezDBus::characteristicPropertiesChanged( emit service->characteristicChanged(changedChar, newValue); } -void QLowEnergyControllerPrivateBluezDBus::interfacesRemoved( - const QDBusObjectPath &objectPath, const QStringList &/*interfaces*/) +void QLowEnergyControllerPrivateBluezDBus::interfacesRemoved(const QDBusObjectPath &objectPath, + const QStringList &interfaces) { if (objectPath.path() == device->path()) { - qCWarning(QT_BT_BLUEZ) << "DBus Device1 was removed"; - executeClose(QLowEnergyController::UnknownRemoteDeviceError); + if (interfaces.contains(QStringLiteral("org.bluez.Device1"))) { + qCWarning(QT_BT_BLUEZ) << "DBus Device1 was removed"; + executeClose(QLowEnergyController::UnknownRemoteDeviceError); + } else { + qCDebug(QT_BT_BLUEZ) << "DBus interfaces" << interfaces << "were removed from" + << objectPath.path(); + } } else if (objectPath.path() == adapter->path()) { qCWarning(QT_BT_BLUEZ) << "DBus Adapter was removed"; executeClose(QLowEnergyController::InvalidBluetoothAdapterError); -- cgit v1.2.3 From 51c341ce5fd9682cd38f1a76ddc72d9e14144392 Mon Sep 17 00:00:00 2001 From: Andreas Buhr Date: Wed, 24 Feb 2021 20:38:36 +0100 Subject: Change tst_qlowenergycontroller to wait for disconnects Many occurrences in tst_qlowenergycontroller exist where disconnectFromDevice() is called but the code did not wait for the disconnect to happen. This patch changes this. Change-Id: I1df4e68136b8a83640af1fc50298e559d983cc9a Reviewed-by: Alex Blasche (cherry picked from commit 180f895023f136a694738667a73674968964bafc) Reviewed-by: Andreas Buhr --- src/bluetooth/qlowenergycontroller_bluezdbus.cpp | 5 +++++ src/bluetooth/qlowenergycontrollerbase.cpp | 1 + .../tst_qlowenergycontroller.cpp | 26 ++++++++++++++++++++-- 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/bluetooth/qlowenergycontroller_bluezdbus.cpp b/src/bluetooth/qlowenergycontroller_bluezdbus.cpp index 8720c549..9849e3f8 100644 --- a/src/bluetooth/qlowenergycontroller_bluezdbus.cpp +++ b/src/bluetooth/qlowenergycontroller_bluezdbus.cpp @@ -60,6 +60,9 @@ QLowEnergyControllerPrivateBluezDBus::QLowEnergyControllerPrivateBluezDBus() QLowEnergyControllerPrivateBluezDBus::~QLowEnergyControllerPrivateBluezDBus() { + if (state != QLowEnergyController::UnconnectedState) { + qCWarning(QT_BT_BLUEZ) << "Low Energy Controller deleted while connected."; + } } void QLowEnergyControllerPrivateBluezDBus::init() @@ -390,6 +393,8 @@ void QLowEnergyControllerPrivateBluezDBus::disconnectFromDevice() qCDebug(QT_BT_BLUEZ) << "BTLE_DBUS::disconnect() failed" << reply.reply().errorName() << reply.reply().errorMessage(); + executeClose(QLowEnergyController::UnknownError); + } else { executeClose(QLowEnergyController::NoError); } call->deleteLater(); diff --git a/src/bluetooth/qlowenergycontrollerbase.cpp b/src/bluetooth/qlowenergycontrollerbase.cpp index 841de3e3..07c3b44b 100644 --- a/src/bluetooth/qlowenergycontrollerbase.cpp +++ b/src/bluetooth/qlowenergycontrollerbase.cpp @@ -123,6 +123,7 @@ void QLowEnergyControllerPrivate::setError( void QLowEnergyControllerPrivate::setState( QLowEnergyController::ControllerState newState) { + qCDebug(QT_BT) << "QLowEnergyControllerPrivate setting state to" << newState; Q_Q(QLowEnergyController); if (state == newState) return; diff --git a/tests/auto/qlowenergycontroller/tst_qlowenergycontroller.cpp b/tests/auto/qlowenergycontroller/tst_qlowenergycontroller.cpp index ab393210..bc74c693 100644 --- a/tests/auto/qlowenergycontroller/tst_qlowenergycontroller.cpp +++ b/tests/auto/qlowenergycontroller/tst_qlowenergycontroller.cpp @@ -148,6 +148,8 @@ void tst_QLowEnergyController::initTestCase() } #endif + // QLoggingCategory::setFilterRules(QStringLiteral("qt.bluetooth* = true")); + devAgent = new QBluetoothDeviceDiscoveryAgent(this); devAgent->setLowEnergyDiscoveryTimeout(5000); @@ -489,10 +491,12 @@ void tst_QLowEnergyController::tst_concurrentDiscovery() } else { QCOMPARE(control.state(), QLowEnergyController::ConnectedState); QCOMPARE(control2.state(), QLowEnergyController::ConnectedState); + QTRY_COMPARE(control2.error(), QLowEnergyController::NoError); control2.disconnectFromDevice(); QTRY_COMPARE(control2.state(), QLowEnergyController::UnconnectedState); QTRY_COMPARE(control2.error(), QLowEnergyController::NoError); QTRY_COMPARE(control.state(), QLowEnergyController::UnconnectedState); + QTRY_COMPARE(control.error(), QLowEnergyController::NoError); // reconnect control control.connectToDevice(); @@ -627,6 +631,8 @@ void tst_QLowEnergyController::tst_concurrentDiscovery() } control.disconnectFromDevice(); + QTRY_COMPARE(control.state(), QLowEnergyController::UnconnectedState); + QCOMPARE(control.error(), QLowEnergyController::NoError); } void tst_QLowEnergyController::verifyServiceProperties( @@ -1738,7 +1744,7 @@ void tst_QLowEnergyController::tst_writeCharacteristic() QSKIP("Cannot connect to remote device"); } - QCOMPARE(control.state(), QLowEnergyController::ConnectedState); + QTRY_VERIFY_WITH_TIMEOUT(control.state() == QLowEnergyController::ConnectedState, 20000); QSignalSpy discoveryFinishedSpy(&control, SIGNAL(discoveryFinished())); QSignalSpy stateSpy(&control, SIGNAL(stateChanged(QLowEnergyController::ControllerState))); control.discoverServices(); @@ -1866,6 +1872,8 @@ void tst_QLowEnergyController::tst_writeCharacteristic() control.disconnectFromDevice(); + QTRY_COMPARE(control.state(), QLowEnergyController::UnconnectedState); + QCOMPARE(control.error(), QLowEnergyController::NoError); // ******************************************* // write value while disconnected -> error errorSpy.clear(); @@ -1945,6 +1953,8 @@ void tst_QLowEnergyController::tst_readWriteDescriptor() if (!tempData.isValid()) { delete service; control.disconnectFromDevice(); + QTRY_COMPARE(control.state(), QLowEnergyController::UnconnectedState); + QCOMPARE(control.error(), QLowEnergyController::NoError); QSKIP("Cannot find temperature data characteristic of TI Sensor"); } @@ -1955,6 +1965,8 @@ void tst_QLowEnergyController::tst_readWriteDescriptor() if (!notification.isValid()) { delete service; control.disconnectFromDevice(); + QTRY_COMPARE(control.state(), QLowEnergyController::UnconnectedState); + QCOMPARE(control.error(), QLowEnergyController::NoError); QSKIP("Cannot find temperature data notification of TI Sensor"); } @@ -2131,6 +2143,8 @@ void tst_QLowEnergyController::tst_readWriteDescriptor() QCOMPARE(notification.value(), QByteArray::fromHex("0000")); control.disconnectFromDevice(); + QTRY_COMPARE(control.state(), QLowEnergyController::UnconnectedState); + QCOMPARE(control.error(), QLowEnergyController::NoError); // ******************************************* // write value while disconnected -> error @@ -2319,6 +2333,8 @@ void tst_QLowEnergyController::tst_customProgrammableDevice() delete service; control.disconnectFromDevice(); + QTRY_COMPARE(control.state(), QLowEnergyController::UnconnectedState); + QCOMPARE(control.error(), QLowEnergyController::NoError); } @@ -2544,6 +2560,8 @@ void tst_QLowEnergyController::tst_errorCases() delete irService; delete oadService; control.disconnectFromDevice(); + QTRY_COMPARE(control.state(), QLowEnergyController::UnconnectedState); + QCOMPARE(control.error(), QLowEnergyController::NoError); } /* @@ -2576,7 +2594,7 @@ void tst_QLowEnergyController::tst_writeCharacteristicNoResponse() QSKIP("Cannot connect to remote device"); } - QCOMPARE(control.state(), QLowEnergyController::ConnectedState); + QTRY_VERIFY_WITH_TIMEOUT(control.state() == QLowEnergyController::ConnectedState, 20000); QSignalSpy discoveryFinishedSpy(&control, SIGNAL(discoveryFinished())); QSignalSpy stateSpy(&control, SIGNAL(stateChanged(QLowEnergyController::ControllerState))); control.discoverServices(); @@ -2620,6 +2638,8 @@ void tst_QLowEnergyController::tst_writeCharacteristicNoResponse() || !imageIdentityChar.isValid()) { delete service; control.disconnectFromDevice(); + QTRY_COMPARE(control.state(), QLowEnergyController::UnconnectedState); + QCOMPARE(control.error(), QLowEnergyController::NoError); QSKIP("Cannot find OAD char/notification"); } @@ -2829,6 +2849,8 @@ void tst_QLowEnergyController::tst_writeCharacteristicNoResponse() delete service; control.disconnectFromDevice(); + QTRY_COMPARE(control.state(), QLowEnergyController::UnconnectedState); + QCOMPARE(control.error(), QLowEnergyController::NoError); } QTEST_MAIN(tst_QLowEnergyController) -- cgit v1.2.3 From fbda5f09c7317de47e83fa2d643d6babe10a06db Mon Sep 17 00:00:00 2001 From: Jani Heikkinen Date: Wed, 3 Mar 2021 15:06:13 +0200 Subject: Bump version Change-Id: I12f3b1fcc0adb4a5243717647cba463904676a15 --- .qmake.conf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.qmake.conf b/.qmake.conf index 5fb75cff..163ffef2 100644 --- a/.qmake.conf +++ b/.qmake.conf @@ -2,4 +2,4 @@ load(qt_build_config) DEFINES += QT_NO_FOREACH QT_NO_JAVA_STYLE_ITERATORS QT_NO_LINKED_LIST -MODULE_VERSION = 5.15.3 +MODULE_VERSION = 5.15.4 -- cgit v1.2.3 From 1b07519f6ca912c0c6334b96d26c61fd8b216a6e Mon Sep 17 00:00:00 2001 From: Andreas Buhr Date: Tue, 16 Mar 2021 17:42:43 +0100 Subject: Fix bug: Let QBluetoothsocket::close emit disconnected only once QBluetoothSocket::close emitted "disconnected" twice on macOS. The first emit is by setSocketState(UnconnectedState). This patch fixes the behavior to only emit it once. Fixes: QTBUG-91164 Change-Id: I7147a28b40c49db947ee97a4f1a14f319da341d1 Reviewed-by: Qt CI Bot Reviewed-by: Oliver Wolff (cherry picked from commit 2a6ab855c2a4ffc5253126d53bc667b0076dd9c9) Reviewed-by: Andreas Buhr Reviewed-by: Alex Blasche --- src/bluetooth/qbluetoothsocket_osx.mm | 1 - 1 file changed, 1 deletion(-) diff --git a/src/bluetooth/qbluetoothsocket_osx.mm b/src/bluetooth/qbluetoothsocket_osx.mm index 70ed87ca..7259aeb5 100644 --- a/src/bluetooth/qbluetoothsocket_osx.mm +++ b/src/bluetooth/qbluetoothsocket_osx.mm @@ -540,7 +540,6 @@ void QBluetoothSocketPrivate::channelClosed() q_ptr->setSocketState(QBluetoothSocket::UnconnectedState); q_ptr->setOpenMode(QIODevice::NotOpen); emit q_ptr->readChannelFinished(); - emit q_ptr->disconnected(); } else { state = QBluetoothSocket::UnconnectedState; // We are still in connectToService and do not want -- cgit v1.2.3 From 5466df6f9b9237caba71a3ae323b931de1f7eda9 Mon Sep 17 00:00:00 2001 From: Andreas Buhr Date: Mon, 22 Mar 2021 14:09:20 +0100 Subject: Always call setOpenMode before setSocketState setSocketState emits signals and should thus be called last. I most code, setOpenMode is called before setSocketState. But in some occasions, setSocketState was called before setOpenMode. This patch introduces a consistent call order: setOpenMode before setSocketState. Change-Id: I07f33511c76fbd08c79050a3fcbc1e1dd72fff04 Reviewed-by: Alex Blasche (cherry picked from commit 62da7abae76b78fdc9385a0d997f3483057cf37c) --- src/bluetooth/qbluetoothsocket_bluez.cpp | 2 +- src/bluetooth/qbluetoothsocket_osx.mm | 4 ++-- src/bluetooth/qbluetoothsocket_win.cpp | 8 +++++--- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/bluetooth/qbluetoothsocket_bluez.cpp b/src/bluetooth/qbluetoothsocket_bluez.cpp index f81580a6..2a0d77f2 100644 --- a/src/bluetooth/qbluetoothsocket_bluez.cpp +++ b/src/bluetooth/qbluetoothsocket_bluez.cpp @@ -689,8 +689,8 @@ bool QBluetoothSocketPrivateBluez::setSocketDescriptor(int socketDescriptor, QBl connectWriteNotifier = new QSocketNotifier(socket, QSocketNotifier::Write, q); QObject::connect(connectWriteNotifier, SIGNAL(activated(QSocketDescriptor)), this, SLOT(_q_writeNotify())); - q->setSocketState(socketState); q->setOpenMode(openMode); + q->setSocketState(socketState); return true; } diff --git a/src/bluetooth/qbluetoothsocket_osx.mm b/src/bluetooth/qbluetoothsocket_osx.mm index 7259aeb5..60825048 100644 --- a/src/bluetooth/qbluetoothsocket_osx.mm +++ b/src/bluetooth/qbluetoothsocket_osx.mm @@ -519,8 +519,8 @@ void QBluetoothSocketPrivate::channelOpenComplete() Q_ASSERT_X(q_ptr, Q_FUNC_INFO, "invalid q_ptr (null)"); if (!isConnecting) { - q_ptr->setSocketState(QBluetoothSocket::ConnectedState); q_ptr->setOpenMode(openMode); + q_ptr->setSocketState(QBluetoothSocket::ConnectedState); emit q_ptr->connected(); } else { state = QBluetoothSocket::ConnectedState; @@ -537,8 +537,8 @@ void QBluetoothSocketPrivate::channelClosed() // (thus close/abort probably will not work). if (!isConnecting) { - q_ptr->setSocketState(QBluetoothSocket::UnconnectedState); q_ptr->setOpenMode(QIODevice::NotOpen); + q_ptr->setSocketState(QBluetoothSocket::UnconnectedState); emit q_ptr->readChannelFinished(); } else { state = QBluetoothSocket::UnconnectedState; diff --git a/src/bluetooth/qbluetoothsocket_win.cpp b/src/bluetooth/qbluetoothsocket_win.cpp index 713fcbf8..12ad8094 100644 --- a/src/bluetooth/qbluetoothsocket_win.cpp +++ b/src/bluetooth/qbluetoothsocket_win.cpp @@ -133,8 +133,8 @@ void QBluetoothSocketPrivateWin::connectToServiceHelper(const QBluetoothAddress const int error = ::WSAGetLastError(); if (result != SOCKET_ERROR || error == WSAEWOULDBLOCK) { - q->setSocketState(QBluetoothSocket::ConnectingState); q->setOpenMode(openMode); + q->setSocketState(QBluetoothSocket::ConnectingState); } else { errorString = qt_error_string(error); q->setSocketError(QBluetoothSocket::UnknownSocketError); @@ -341,10 +341,12 @@ void QBluetoothSocketPrivateWin::abort() Q_Q(QBluetoothSocket); const bool wasConnected = q->state() == QBluetoothSocket::ConnectedState; - q->setSocketState(QBluetoothSocket::UnconnectedState); if (wasConnected) { q->setOpenMode(QIODevice::NotOpen); + q->setSocketState(QBluetoothSocket::UnconnectedState); emit q->readChannelFinished(); + } else { + q->setSocketState(QBluetoothSocket::UnconnectedState); } } @@ -509,8 +511,8 @@ bool QBluetoothSocketPrivateWin::setSocketDescriptor(int socketDescriptor, if (!createNotifiers()) return false; - q->setSocketState(socketState); q->setOpenMode(openMode); + q->setSocketState(socketState); if (socketState == QBluetoothSocket::ConnectedState) { connectWriteNotifier->setEnabled(true); readNotifier->setEnabled(true); -- cgit v1.2.3