Dear all,

I tried to modify the behavior of the Bluetooth (BT) device selection dialog 
and would need some feedback. Unfortunately, BT is highly platform dependent 
and I have no idea how this behaves with non-Bluez 5 backends.

The idea was that when I activate the BT device selection dialog I certainly 
don't want an empty list of devices. Because that means that I have to click 
"Scan" in all cases. Ideally, I'd immediately get the BT devices known to the 
OS (especially the paired ones). It seems that Qt does not provide such a list 
(an API defect?). But looking into the Qt Bluez5 backend of 
QBluetoothDeviceDiscoveryAgent, these devices are collected synchronously in 
the start() method.

The first patch therefore simply does a start()/stop() pair, which is of course 
horrible because it uses undocumented behavior, but has precisely the intended 
effect. I didn't bother to figure out the Objective-C MacOS code - perhaps 
there you would get the same effect with a short BLE timeout?

The second patch is a trivial code clean up and independent of the others.

The third patch adds "Aladin" to the list of recognized BT names and exports 
the getDeviceType() function. This is a preparation for the next patch.

The fourth patch came from the idea that I'd want the previous selection 
retained. So what it does is preselecting a device according to priority:
 1st: previously selected device
 2nd: a known dive computer name
 3rd: paired device
Non-paired devices are never selected.
I'm not really happy about the code. It has also to be noted that this patch 
might lead to the undesired behavior that a user selects a device and the UI 
overrules the user's selection if a higher-priority device is detected later 
on. This situation seems quite unlikely but nevertheless the patch might need 
some refinement.

The fifth patch makes the Save button the "default button" of the dialog. This 
has the effect that pressing enter does not discard the selection of the user. 
This patch might also be done with QDialogButtonBox(?), but here I opt for the 
less intrusive change.

Thank you,

Berthold
>From 2a2c69c13d926ddd0b9a95d3b53baf82c18da221 Mon Sep 17 00:00:00 2001
From: Berthold Stoeger <bstoe...@mail.tuwien.ac.at>
Date: Fri, 3 Nov 2017 09:05:25 +0100
Subject: [PATCH 1/5] Initialize Bluetooth device list

On creating the Bluetooth device selection widget, perform a
 QBluetoothDeviceDiscoveryAgent::start()
 QBluetoothDeviceDiscoveryAgent::stop()
pair. Bluez5 backends synchronously discover the devices known to
the OS in the start() method. Thus, the device list is filled with
the devices known to the OS. For non-Bluez5 backends this is probably
a NOP.

Signed-off-by: Berthold Stoeger <bstoe...@mail.tuwien.ac.at>
---
 desktop-widgets/btdeviceselectiondialog.cpp | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/desktop-widgets/btdeviceselectiondialog.cpp b/desktop-widgets/btdeviceselectiondialog.cpp
index cf70daa3..b2748008 100644
--- a/desktop-widgets/btdeviceselectiondialog.cpp
+++ b/desktop-widgets/btdeviceselectiondialog.cpp
@@ -539,6 +539,13 @@ void BtDeviceSelectionDialog::initializeDeviceDiscoveryAgent()
 		this, SLOT(remoteDeviceScanFinished()));
 	connect(remoteDeviceDiscoveryAgent, SIGNAL(error(QBluetoothDeviceDiscoveryAgent::Error)),
 		this, SLOT(deviceDiscoveryError(QBluetoothDeviceDiscoveryAgent::Error)));
+
+#if !defined(Q_IS_WIN)
+	// With a bluez5 backend a call to QBluetoothDeviceDiscoveryAgent::start()
+	// returns only after having synchronously discovered the devices known to the OS.
+	remoteDeviceDiscoveryAgent->start();
+	remoteDeviceDiscoveryAgent->stop();
+#endif
 }
 
 #if defined(Q_OS_WIN)
-- 
2.14.1

>From ef48c1e84c289a6735a96d5409bffae9d9fc591b Mon Sep 17 00:00:00 2001
From: Berthold Stoeger <bstoe...@mail.tuwien.ac.at>
Date: Sat, 4 Nov 2017 19:18:43 +0100
Subject: [PATCH 2/5] Remove unnecessary == NULL test.

Test not necessary, because the QString in question is not a pointer
and the string is tested for emptiness (which also flags null-strings).

Signed-off-by: Berthold Stoeger <bstoe...@mail.tuwien.ac.at>
---
 desktop-widgets/downloadfromdivecomputer.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/desktop-widgets/downloadfromdivecomputer.cpp b/desktop-widgets/downloadfromdivecomputer.cpp
index 3b551ede..f804f811 100644
--- a/desktop-widgets/downloadfromdivecomputer.cpp
+++ b/desktop-widgets/downloadfromdivecomputer.cpp
@@ -555,7 +555,7 @@ void DownloadFromDCWidget::bluetoothSelectionDialogIsFinished(int result)
 		/* Make the selected Bluetooth device default */
 		QString selectedDeviceName = btDeviceSelectionDialog->getSelectedDeviceName();
 
-		if (selectedDeviceName == NULL || selectedDeviceName.isEmpty()) {
+		if (selectedDeviceName.isEmpty()) {
 			ui.device->setCurrentText(btDeviceSelectionDialog->getSelectedDeviceAddress());
 		} else {
 			ui.device->setCurrentText(selectedDeviceName);
-- 
2.14.1

>From 0078049802af2fac41abbe370dd879375fd434f1 Mon Sep 17 00:00:00 2001
From: Berthold Stoeger <bstoe...@mail.tuwien.ac.at>
Date: Sun, 5 Nov 2017 11:47:36 +0100
Subject: [PATCH 3/5] Add "Aladin" to the list of recognized BT names.

Recognize Aladin as the Bluetooth name of the Scubapro Aladin Sport
Matrix. To my knowledge, this is the only dive computer of the Aladin
series that supports BT.

Moreover, export the getDeviceType function to make it accessible from
other translation units.

Signed-off-by: Berthold Stoeger <bstoe...@mail.tuwien.ac.at>
---
 core/btdiscovery.cpp | 7 ++++---
 core/btdiscovery.h   | 1 +
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/core/btdiscovery.cpp b/core/btdiscovery.cpp
index 731c4ca4..6833ae22 100644
--- a/core/btdiscovery.cpp
+++ b/core/btdiscovery.cpp
@@ -10,7 +10,7 @@ extern QMap<QString, dc_descriptor_t *> descriptorLookup;
 
 BTDiscovery *BTDiscovery::m_instance = NULL;
 
-static dc_descriptor_t *getDeviceType(QString btName)
+dc_descriptor_t *getDeviceType(QString btName)
 // central function to convert a BT name to a Subsurface known vendor/model pair
 {
 	QString vendor, product;
@@ -40,9 +40,10 @@ static dc_descriptor_t *getDeviceType(QString btName)
 		product = "EON Steel";
 	}
 
-	if (btName.startsWith("G2")) {
+	if (btName.startsWith("G2")  || btName.startsWith("Aladin")) {
 		vendor = "Scubapro";
-		product = "G2";
+		if (btName.startsWith("G2")) product = "G2";
+		if (btName.startsWith("Aladin")) product = "Aladin Sport Matrix";
 	}
 
 	if (!vendor.isEmpty() && !product.isEmpty())
diff --git a/core/btdiscovery.h b/core/btdiscovery.h
index 7a9e9c65..fa392533 100644
--- a/core/btdiscovery.h
+++ b/core/btdiscovery.h
@@ -18,6 +18,7 @@
 
 void saveBtDeviceInfo(const char* devaddr, QBluetoothDeviceInfo deviceInfo);
 QBluetoothDeviceInfo getBtDeviceInfo(const char* devaddr);
+dc_descriptor_t *getDeviceType(QString btName);
 
 class BTDiscovery : public QObject {
 	Q_OBJECT
-- 
2.14.1

>From 979b77865c013fbd4d06fcfa5b82cd72caae4b27 Mon Sep 17 00:00:00 2001
From: Berthold Stoeger <bstoe...@mail.tuwien.ac.at>
Date: Sun, 5 Nov 2017 13:43:36 +0100
Subject: [PATCH 4/5] Automatically select item in Bluetooth device selection
 dialog

On initialization or re-scanning, a device is automatically selected.
The highest priority is given to the previously selected device,
whose address therefore has to be passed to the dialog.
The second highest priority is given to devices which are recognized
as Bluetooth-enabled dive computers.
Lowest priority is given to paired devices.
Unpaired devices are never selected.

In rare circumstances this might lead to a "fight" between user and UI
over the selected item: User starts scan and selects an item. A
higher-prioritized device is found later. Thus, this behaviour might
need some refinement.

Signed-off-by: Berthold Stoeger <bstoe...@mail.tuwien.ac.at>
---
 desktop-widgets/btdeviceselectiondialog.cpp     | 43 +++++++++++++++++++++++--
 desktop-widgets/btdeviceselectiondialog.h       |  6 +++-
 desktop-widgets/configuredivecomputerdialog.cpp |  2 +-
 desktop-widgets/downloadfromdivecomputer.cpp    |  2 +-
 4 files changed, 48 insertions(+), 5 deletions(-)

diff --git a/desktop-widgets/btdeviceselectiondialog.cpp b/desktop-widgets/btdeviceselectiondialog.cpp
index b2748008..c7b74c0a 100644
--- a/desktop-widgets/btdeviceselectiondialog.cpp
+++ b/desktop-widgets/btdeviceselectiondialog.cpp
@@ -17,10 +17,12 @@ Q_DECLARE_METATYPE(QBluetoothDeviceDiscoveryAgent::Error)
 Q_DECLARE_METATYPE(QBluetoothDeviceInfo)
 #endif
 
-BtDeviceSelectionDialog::BtDeviceSelectionDialog(QWidget *parent) :
+BtDeviceSelectionDialog::BtDeviceSelectionDialog(const QString &previous, QWidget *parent) :
 	QDialog(parent),
 	ui(new Ui::BtDeviceSelectionDialog),
-	remoteDeviceDiscoveryAgent(0)
+	remoteDeviceDiscoveryAgent(0),
+	previousDevice(previous),
+	maxPriority(0)
 {
 	ui->setupUi(this);
 
@@ -49,6 +51,10 @@ BtDeviceSelectionDialog::BtDeviceSelectionDialog(QWidget *parent) :
 	connect(ui->discoveredDevicesList, SIGNAL(itemClicked(QListWidgetItem*)),
 		this, SLOT(itemClicked(QListWidgetItem*)));
 
+	// Remove BLE marker from address
+	if (previousDevice.startsWith("LE:"))
+		previousDevice = previousDevice.mid(3);
+
 #if defined(Q_OS_WIN)
 	ULONG       ulRetCode = SUCCESS;
 	WSADATA     WSAData = { 0 };
@@ -155,6 +161,7 @@ void BtDeviceSelectionDialog::on_save_clicked()
 	QString address = remoteDeviceInfo.address().isNull() ? remoteDeviceInfo.deviceUuid().toString() :
 								remoteDeviceInfo.address().toString();
 	saveBtDeviceInfo(address.toUtf8().constData(), remoteDeviceInfo);
+	previousDevice = address;
 	if (remoteDeviceDiscoveryAgent->isActive()) {
 		// Stop the SDP agent if the clear button is pressed and enable the Scan button
 		remoteDeviceDiscoveryAgent->stop();
@@ -172,6 +179,7 @@ void BtDeviceSelectionDialog::on_clear_clicked()
 {
 	ui->dialogStatus->setText(tr("Remote devices list was cleared."));
 	ui->discoveredDevicesList->clear();
+	maxPriority = 0;
 	ui->save->setEnabled(false);
 
 	if (remoteDeviceDiscoveryAgent->isActive()) {
@@ -188,6 +196,7 @@ void BtDeviceSelectionDialog::on_scan_clicked()
 {
 	ui->dialogStatus->setText(tr("Scanning for remote devices..."));
 	ui->discoveredDevicesList->clear();
+	maxPriority = 0;
 	remoteDeviceDiscoveryAgent->start();
 	ui->scan->setEnabled(false);
 }
@@ -218,10 +227,28 @@ void BtDeviceSelectionDialog::hostModeStateChanged(QBluetoothLocalDevice::HostMo
 #endif
 }
 
+int BtDeviceSelectionDialog::getPriority(bool connectable, const QBluetoothDeviceInfo &remoteDeviceInfo)
+{
+	if (!connectable)
+		return 0;
+
+	QString address = remoteDeviceInfo.address().isNull() ?
+		remoteDeviceInfo.deviceUuid().toString() :
+		remoteDeviceInfo.address().toString();
+	if (address == previousDevice)
+		return 3;
+
+	if (getDeviceType(remoteDeviceInfo.name()))
+		return 2;
+
+	return 1;
+}
+
 void BtDeviceSelectionDialog::addRemoteDevice(const QBluetoothDeviceInfo &remoteDeviceInfo)
 {
 #if defined(Q_OS_WIN)
 	// On Windows we cannot obtain the pairing status so we set only the name and the address of the device
+	bool connectable = true;
 	QString deviceLabel = QString("%1 (%2)").arg(remoteDeviceInfo.name(),
 						     remoteDeviceInfo.address().toString());
 	QColor pairingColor = QColor(Qt::white);
@@ -231,12 +258,15 @@ void BtDeviceSelectionDialog::addRemoteDevice(const QBluetoothDeviceInfo &remote
 	QString pairingStatusLabel = tr("UNPAIRED");
 	QBluetoothLocalDevice::Pairing pairingStatus = localDevice->pairingStatus(remoteDeviceInfo.address());
 
+	bool connectable = false;
 	if (pairingStatus == QBluetoothLocalDevice::Paired) {
 		pairingStatusLabel = tr("PAIRED");
 		pairingColor = QColor(Qt::gray);
+		connectable = true;
 	} else if (pairingStatus == QBluetoothLocalDevice::AuthorizedPaired) {
 		pairingStatusLabel = tr("AUTHORIZED_PAIRED");
 		pairingColor = QColor("#89C4F4");
+		connectable = true;
 	}
 	if (remoteDeviceInfo.address().isNull())
 		pairingColor = QColor(Qt::gray);
@@ -248,6 +278,7 @@ void BtDeviceSelectionDialog::addRemoteDevice(const QBluetoothDeviceInfo &remote
 		// we have only a Uuid, no address, so show that and reset the pairing color
 		deviceLabel = QString("%1 (%2)").arg(remoteDeviceInfo.name(),remoteDeviceInfo.deviceUuid().toString());
 		pairingColor = QColor(Qt::white);
+		connectable = true;
 	} else
 #endif
 	deviceLabel = tr("%1 (%2)   [State: %3]").arg(remoteDeviceInfo.name(),
@@ -261,6 +292,14 @@ void BtDeviceSelectionDialog::addRemoteDevice(const QBluetoothDeviceInfo &remote
 	item->setBackgroundColor(pairingColor);
 
 	ui->discoveredDevicesList->addItem(item);
+	int priority = getPriority(connectable, remoteDeviceInfo);
+	if(priority > maxPriority) {
+		maxPriority = priority;
+		//item->setSelected(true);
+		ui->discoveredDevicesList->setCurrentItem(item);
+		ui->save->setEnabled(true);
+		ui->save->setFocus();
+	}
 }
 
 void BtDeviceSelectionDialog::itemClicked(QListWidgetItem *item)
diff --git a/desktop-widgets/btdeviceselectiondialog.h b/desktop-widgets/btdeviceselectiondialog.h
index f3cfa341..f56edb17 100644
--- a/desktop-widgets/btdeviceselectiondialog.h
+++ b/desktop-widgets/btdeviceselectiondialog.h
@@ -55,7 +55,7 @@ class BtDeviceSelectionDialog : public QDialog {
 	Q_OBJECT
 
 public:
-	explicit BtDeviceSelectionDialog(QWidget *parent = 0);
+	explicit BtDeviceSelectionDialog(const QString &previous, QWidget *parent = 0);
 	~BtDeviceSelectionDialog();
 	QString getSelectedDeviceAddress();
 	QString getSelectedDeviceName();
@@ -77,6 +77,8 @@ private slots:
 
 private:
 	Ui::BtDeviceSelectionDialog *ui;
+	QString previousDevice;
+	int maxPriority;
 #if defined(Q_OS_WIN)
 	WinBluetoothDeviceDiscoveryAgent *remoteDeviceDiscoveryAgent;
 #else
@@ -87,6 +89,8 @@ private:
 
 	void updateLocalDeviceInformation();
 	void initializeDeviceDiscoveryAgent();
+
+	int getPriority(bool connectable, const QBluetoothDeviceInfo &remoteDeviceInfo);
 };
 
 #endif // BTDEVICESELECTIONDIALOG_H
diff --git a/desktop-widgets/configuredivecomputerdialog.cpp b/desktop-widgets/configuredivecomputerdialog.cpp
index 0a601730..c21c8d78 100644
--- a/desktop-widgets/configuredivecomputerdialog.cpp
+++ b/desktop-widgets/configuredivecomputerdialog.cpp
@@ -1477,7 +1477,7 @@ void ConfigureDiveComputerDialog::pickLogFile()
 void ConfigureDiveComputerDialog::selectRemoteBluetoothDevice()
 {
 	if (!btDeviceSelectionDialog) {
-		btDeviceSelectionDialog = new BtDeviceSelectionDialog(this);
+		btDeviceSelectionDialog = new BtDeviceSelectionDialog(ui.device->currentText(), this);
 		connect(btDeviceSelectionDialog, SIGNAL(finished(int)),
 			this, SLOT(bluetoothSelectionDialogIsFinished(int)));
 	}
diff --git a/desktop-widgets/downloadfromdivecomputer.cpp b/desktop-widgets/downloadfromdivecomputer.cpp
index f804f811..19d38cbd 100644
--- a/desktop-widgets/downloadfromdivecomputer.cpp
+++ b/desktop-widgets/downloadfromdivecomputer.cpp
@@ -541,7 +541,7 @@ void DownloadFromDCWidget::markChildrenAsEnabled()
 void DownloadFromDCWidget::selectRemoteBluetoothDevice()
 {
 	if (!btDeviceSelectionDialog) {
-		btDeviceSelectionDialog = new BtDeviceSelectionDialog(this);
+		btDeviceSelectionDialog = new BtDeviceSelectionDialog(ui.device->currentText(), this);
 		connect(btDeviceSelectionDialog, SIGNAL(finished(int)),
 			this, SLOT(bluetoothSelectionDialogIsFinished(int)));
 	}
-- 
2.14.1

>From 5ec3294b46798ac48178465e5366abc0a64e3621 Mon Sep 17 00:00:00 2001
From: Berthold Stoeger <bstoe...@mail.tuwien.ac.at>
Date: Sun, 5 Nov 2017 18:16:46 +0100
Subject: [PATCH 5/5] BT device selection dialog: make Save the default button

When a user presses enter, they probably want their selection saved,
not discarded.

Signed-off-by: Berthold Stoeger <bstoe...@mail.tuwien.ac.at>
---
 desktop-widgets/btdeviceselectiondialog.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/desktop-widgets/btdeviceselectiondialog.cpp b/desktop-widgets/btdeviceselectiondialog.cpp
index c7b74c0a..6dcc22d2 100644
--- a/desktop-widgets/btdeviceselectiondialog.cpp
+++ b/desktop-widgets/btdeviceselectiondialog.cpp
@@ -42,6 +42,7 @@ BtDeviceSelectionDialog::BtDeviceSelectionDialog(const QString &previous, QWidge
 	ui->scan->setText(tr("Scan"));
 	ui->clear->setText(tr("Clear"));
 	ui->save->setText(tr("Save"));
+	ui->save->setDefault(true);
 	ui->quit->setText(tr("Quit"));
 
 	// Disable the save button because there is no device selected
-- 
2.14.1

_______________________________________________
subsurface mailing list
subsurface@subsurface-divelog.org
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface

Reply via email to