On Dienstag, 24. Oktober 2017 21:35:24 CEST Berthold Stoeger wrote:
> Dear all,
>
> in a previous email I reported leaking resources in qt-ble.c. Here is my
> attempt at a fix. I reckon this ideally has no user-visible effect (i.e. is
> gratuitous), yet is risky. Therefore some prudence is in order.
>
> There are three patches with a decreasing degree of obviousness.
Damn, forgot a debugging #include in last patch. :(
>From 350fc2c17bbc77dfc36ca304522cfda257ce9853 Mon Sep 17 00:00:00 2001
From: Berthold Stoeger <bstoe...@mail.tuwien.ac.at>
Date: Tue, 24 Oct 2017 19:00:35 +0200
Subject: [PATCH 3/3] Move creation of BLE controller into BLEObject
Instead of taking ownership of the BLE controller, generate the controller
in BLEObject. Thus avoid ownership subtleties.
Signed-off-by: Berthold Stoeger <bstoe...@mail.tuwien.ac.at>
---
core/qt-ble.cpp | 161 ++++++++++++++++++++++++++++++--------------------------
core/qt-ble.h | 3 +-
2 files changed, 87 insertions(+), 77 deletions(-)
diff --git a/core/qt-ble.cpp b/core/qt-ble.cpp
index 678296b2..dcca5f3f 100644
--- a/core/qt-ble.cpp
+++ b/core/qt-ble.cpp
@@ -130,11 +130,11 @@ void BLEObject::addService(const QBluetoothUuid &newService)
}
}
-BLEObject::BLEObject(QLowEnergyController *c, dc_user_device_t *d)
+BLEObject::BLEObject(dc_user_device_t *d)
{
- controller = c;
device = d;
debugCounter = 0;
+ controller = nullptr;
}
BLEObject::~BLEObject()
@@ -144,7 +144,85 @@ BLEObject::~BLEObject()
foreach (QLowEnergyService *service, services)
delete service;
- delete controller;
+ if(controller)
+ delete controller;
+}
+
+dc_status_t BLEObject::connect_and_discover_services(const char *devaddr)
+{
+#if defined(Q_OS_MACOS) || defined(Q_OS_IOS)
+ QBluetoothDeviceInfo remoteDevice = getBtDeviceInfo(devaddr);
+ controller = QLowEnergyController::createCentral(remoteDevice);
+#else
+ // this is deprecated but given that we don't use Qt to scan for
+ // devices on Android, we don't have QBluetoothDeviceInfo for the
+ // paired devices and therefore cannot use the newer interfaces
+ // that are preferred starting with Qt 5.7
+ QBluetoothAddress remoteDeviceAddress(devaddr);
+ controller = new QLowEnergyController(remoteDeviceAddress);
+#endif
+
+ if (IS_SHEARWATER(device))
+ controller->setRemoteAddressType(QLowEnergyController::RandomAddress);
+
+ // Try to connect to the device
+ controller->connectToDevice();
+
+ // Create a timer. If the connection doesn't succeed after five seconds or no error occurs then stop the opening step
+ int msec = BLE_TIMEOUT;
+ while (msec > 0 && controller->state() == QLowEnergyController::ConnectingState) {
+ waitFor(100);
+ msec -= 100;
+ };
+
+ switch (controller->state()) {
+ case QLowEnergyController::ConnectedState:
+ qDebug() << "connected to the controller for device" << devaddr;
+ break;
+ default:
+ qDebug() << "failed to connect to the controller " << devaddr << "with error" << controller->errorString();
+ report_error("Failed to connect to %s: '%s'", devaddr, controller->errorString().toUtf8().data());
+ controller->disconnectFromDevice();
+ return DC_STATUS_IO;
+ }
+
+ // Receive change notifications, etc.
+ connect(controller, SIGNAL(serviceDiscovered(QBluetoothUuid)), SLOT(addService(QBluetoothUuid)));
+
+ // We need to discover services etc here!
+ qDebug() << " .. discovering services";
+
+ controller->discoverServices();
+
+ msec = BLE_TIMEOUT;
+ while (msec > 0 && controller->state() == QLowEnergyController::DiscoveringState) {
+ waitFor(100);
+ msec -= 100;
+ };
+
+ qDebug() << " .. done discovering services";
+ if (services.isEmpty()) {
+ qDebug() << "failed to find suitable service on" << devaddr;
+ report_error("Failed to find suitable service on '%s'", devaddr);
+ controller->disconnectFromDevice();
+ return DC_STATUS_IO;
+ }
+
+ qDebug() << " .. discovering details";
+ msec = BLE_TIMEOUT;
+ while (msec > 0 && preferredService()->state() == QLowEnergyService::DiscoveringServices) {
+ waitFor(100);
+ msec -= 100;
+ };
+
+ if (preferredService()->state() != QLowEnergyService::ServiceDiscovered) {
+ qDebug() << "failed to find suitable service on" << devaddr;
+ report_error("Failed to find suitable service on '%s'", devaddr);
+ controller->disconnectFromDevice();
+ return DC_STATUS_IO;
+ }
+
+ return DC_STATUS_SUCCESS;
}
dc_status_t BLEObject::write(const void *data, size_t size, size_t *actual)
@@ -296,85 +374,16 @@ dc_status_t qt_ble_open(dc_custom_io_t *io, dc_context_t *context, const char *d
// HACK ALERT! Qt 5.9 needs this for proper Bluez operation
qputenv("QT_DEFAULT_CENTRAL_SERVICES", "1");
-#if defined(Q_OS_MACOS) || defined(Q_OS_IOS)
- QBluetoothDeviceInfo remoteDevice = getBtDeviceInfo(devaddr);
- QLowEnergyController *controller = QLowEnergyController::createCentral(remoteDevice);
-#else
- // this is deprecated but given that we don't use Qt to scan for
- // devices on Android, we don't have QBluetoothDeviceInfo for the
- // paired devices and therefore cannot use the newer interfaces
- // that are preferred starting with Qt 5.7
- QBluetoothAddress remoteDeviceAddress(devaddr);
- QLowEnergyController *controller = new QLowEnergyController(remoteDeviceAddress);
-#endif
qDebug() << "qt_ble_open(" << devaddr << ")";
- if (IS_SHEARWATER(io->user_device))
- controller->setRemoteAddressType(QLowEnergyController::RandomAddress);
-
- // Try to connect to the device
- controller->connectToDevice();
-
- // Create a timer. If the connection doesn't succeed after five seconds or no error occurs then stop the opening step
- int msec = BLE_TIMEOUT;
- while (msec > 0 && controller->state() == QLowEnergyController::ConnectingState) {
- waitFor(100);
- msec -= 100;
- };
-
- switch (controller->state()) {
- case QLowEnergyController::ConnectedState:
- qDebug() << "connected to the controller for device" << devaddr;
- break;
- default:
- qDebug() << "failed to connect to the controller " << devaddr << "with error" << controller->errorString();
- report_error("Failed to connect to %s: '%s'", devaddr, controller->errorString().toUtf8().data());
- controller->disconnectFromDevice();
- delete controller;
- return DC_STATUS_IO;
- }
-
- // We need to discover services etc here!
- // Note that ble takes ownership of controller and henceforth deleting ble will
- // take care of deleting controller.
- BLEObject *ble = new BLEObject(controller, io->user_device);
- ble->connect(controller, SIGNAL(serviceDiscovered(QBluetoothUuid)), SLOT(addService(QBluetoothUuid)));
+ BLEObject *ble = new BLEObject(io->user_device);
- qDebug() << " .. discovering services";
-
- controller->discoverServices();
-
- msec = BLE_TIMEOUT;
- while (msec > 0 && controller->state() == QLowEnergyController::DiscoveringState) {
- waitFor(100);
- msec -= 100;
- };
-
- qDebug() << " .. done discovering services";
- if (ble->preferredService() == nullptr) {
- qDebug() << "failed to find suitable service on" << devaddr;
- report_error("Failed to find suitable service on '%s'", devaddr);
- controller->disconnectFromDevice();
- delete ble;
- return DC_STATUS_IO;
- }
-
- qDebug() << " .. discovering details";
- msec = BLE_TIMEOUT;
- while (msec > 0 && ble->preferredService()->state() == QLowEnergyService::DiscoveringServices) {
- waitFor(100);
- msec -= 100;
- };
-
- if (ble->preferredService()->state() != QLowEnergyService::ServiceDiscovered) {
- qDebug() << "failed to find suitable service on" << devaddr;
- report_error("Failed to find suitable service on '%s'", devaddr);
- controller->disconnectFromDevice();
+ dc_status_t r = ble->connect_and_discover_services(devaddr);
+ if (r != DC_STATUS_SUCCESS) {
delete ble;
- return DC_STATUS_IO;
+ return r;
}
-
qDebug() << " .. enabling notifications";
/* Enable notifications */
diff --git a/core/qt-ble.h b/core/qt-ble.h
index f366afe8..6aaa6416 100644
--- a/core/qt-ble.h
+++ b/core/qt-ble.h
@@ -18,8 +18,9 @@ class BLEObject : public QObject
Q_OBJECT
public:
- BLEObject(QLowEnergyController *c, dc_user_device_t *);
+ BLEObject(dc_user_device_t *);
~BLEObject();
+ dc_status_t connect_and_discover_services(const char *devaddr);
dc_status_t write(const void* data, size_t size, size_t *actual);
dc_status_t read(void* data, size_t size, size_t *actual);
--
2.14.1
_______________________________________________
subsurface mailing list
subsurface@subsurface-divelog.org
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
