Skip to content

Commit

Permalink
Comms: Create Sockets on New Thread
Browse files Browse the repository at this point in the history
  • Loading branch information
@HTRamsey
HTRamsey committed Dec 17, 2024
1 parent a21fee2 commit bdfe3d0
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 41 deletions.
64 changes: 41 additions & 23 deletions src/Comms/BluetoothLink.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,6 @@
#include "DeviceInfo.h"
#include "QGCLoggingCategory.h"

#include <QtBluetooth/QBluetoothDeviceDiscoveryAgent>
#include <QtBluetooth/QBluetoothServiceInfo>
#include <QtBluetooth/QBluetoothSocket>
#ifdef Q_OS_IOS
#include <QtBluetooth/QBluetoothServiceDiscoveryAgent>
#else
#include <QtBluetooth/QBluetoothUuid>
#endif

QGC_LOGGING_CATEGORY(BluetoothLinkLog, "qgc.comms.bluetoothlink")

/*===========================================================================*/
Expand Down Expand Up @@ -56,15 +47,15 @@ BluetoothConfiguration::~BluetoothConfiguration()
void BluetoothConfiguration::_initDeviceDiscoveryAgent()
{
(void) connect(_deviceDiscoveryAgent, &QBluetoothDeviceDiscoveryAgent::deviceDiscovered, this, &BluetoothConfiguration::_deviceDiscovered);
(void) connect(_deviceDiscoveryAgent, &QBluetoothDeviceDiscoveryAgent::deviceUpdated, this, [this](const QBluetoothDeviceInfo &info, QBluetoothDeviceInfo::Fields updatedFields) {
qCDebug(BluetoothLinkLog) << "Device Updated";
});
(void) connect(_deviceDiscoveryAgent, &QBluetoothDeviceDiscoveryAgent::canceled, this, &BluetoothConfiguration::scanningChanged);
(void) connect(_deviceDiscoveryAgent, &QBluetoothDeviceDiscoveryAgent::finished, this, &BluetoothConfiguration::scanningChanged);
(void) connect(_deviceDiscoveryAgent, &QBluetoothDeviceDiscoveryAgent::errorOccurred, this, [this](QBluetoothDeviceDiscoveryAgent::Error error) {
// TODO: Signal this error
qCWarning(BluetoothLinkLog) << "Bluetooth Configuration error:" << error << _deviceDiscoveryAgent->errorString();
});
(void) connect(_deviceDiscoveryAgent, &QBluetoothDeviceDiscoveryAgent::errorOccurred, this, &BluetoothConfiguration::_onSocketErrorOccurred);

if (BluetoothLinkLog().isDebugEnabled()) {
(void) connect(_deviceDiscoveryAgent, &QBluetoothDeviceDiscoveryAgent::deviceUpdated, this, [this](const QBluetoothDeviceInfo &info, QBluetoothDeviceInfo::Fields updatedFields) {
qCDebug(BluetoothLinkLog) << "Device Updated";
});
}
}

void BluetoothConfiguration::copyFrom(const LinkConfiguration *source)
Expand Down Expand Up @@ -169,6 +160,13 @@ void BluetoothConfiguration::_deviceDiscovered(const QBluetoothDeviceInfo &info)
}
}

void BluetoothConfiguration::_onSocketErrorOccurred(QBluetoothDeviceDiscoveryAgent::Error error)
{
const QString errorString = _deviceDiscoveryAgent->errorString();
qCWarning(BluetoothLinkLog) << "Bluetooth Configuration error:" << error << errorString;
emit errorOccurred(errorString);
}

/*===========================================================================*/

BluetoothWorker::BluetoothWorker(const BluetoothConfiguration *config, QObject *parent)
Expand All @@ -187,7 +185,7 @@ BluetoothWorker::~BluetoothWorker()

bool BluetoothWorker::isConnected() const
{
return (_socket->isOpen() && (_socket->state() == QBluetoothSocket::SocketState::ConnectedState));
return (_socket && _socket->isOpen() && (_socket->state() == QBluetoothSocket::SocketState::ConnectedState));
}

void BluetoothWorker::setupSocket()
Expand All @@ -203,16 +201,18 @@ void BluetoothWorker::setupSocket()
(void) connect(_socket, &QBluetoothSocket::connected, this, &BluetoothWorker::_onSocketConnected);
(void) connect(_socket, &QBluetoothSocket::disconnected, this, &BluetoothWorker::_onSocketDisconnected);
(void) connect(_socket, &QBluetoothSocket::readyRead, this, &BluetoothWorker::_onSocketReadyRead);
// (void) connect(_socket, &QBluetoothSocket::bytesWritten, this, &BluetoothWorker::_onSocketBytesWritten);
(void) connect(_socket, &QBluetoothSocket::errorOccurred, this, &BluetoothWorker::_onSocketErrorOccurred);

#ifdef Q_OS_IOS
(void) QObject::connect(_serviceDiscoveryAgent, &QBluetoothServiceDiscoveryAgent::serviceDiscovered, this, &BluetoothLink::_serviceDiscovered);
(void) QObject::connect(_serviceDiscoveryAgent, &QBluetoothServiceDiscoveryAgent::finished, this, &BluetoothLink::_discoveryFinished);
(void) QObject::connect(_serviceDiscoveryAgent, &QBluetoothServiceDiscoveryAgent::canceled, this, &BluetoothLink::_discoveryFinished);
(void) connect(_serviceDiscoveryAgent, &QBluetoothServiceDiscoveryAgent::serviceDiscovered, this, &BluetoothWorker::_serviceDiscovered);
(void) connect(_serviceDiscoveryAgent, &QBluetoothServiceDiscoveryAgent::finished, this, &BluetoothWorker::_discoveryFinished);
(void) connect(_serviceDiscoveryAgent, &QBluetoothServiceDiscoveryAgent::canceled, this, &BluetoothWorker::_discoveryFinished);
(void) connect(_serviceDiscoveryAgent, &QBluetoothServiceDiscoveryAgent::errorOccurred, this, &BluetoothWorker::_onServiceErrorOccurred);
#endif

if (BluetoothLinkLog().isDebugEnabled()) {
// (void) connect(_socket, &QBluetoothSocket::bytesWritten, this, &BluetoothWorker::_onSocketBytesWritten);

(void) QObject::connect(_socket, &QBluetoothSocket::stateChanged, this, [](QBluetoothSocket::SocketState state) {
qCDebug(BluetoothLinkLog) << "Bluetooth State Changed:" << state;
});
Expand All @@ -229,7 +229,9 @@ void BluetoothWorker::connectLink()
qCDebug(BluetoothLinkLog) << "Attempting to connect to" << _config->device().name;

#ifdef Q_OS_IOS
_serviceDiscoveryAgent->start();
if (_serviceDiscoveryAgent && _serviceDiscoveryAgent->isActive()) {
_serviceDiscoveryAgent->start();
}
#else
static constexpr QBluetoothUuid uuid = QBluetoothUuid(QBluetoothUuid::ServiceClassUuid::SerialPort);
_socket->connectToService(_config->device().address, uuid);
Expand All @@ -246,7 +248,9 @@ void BluetoothWorker::disconnectLink()
qCDebug(BluetoothLinkLog) << "Attempting to disconnect from device:" << _config->device().name;

#ifdef Q_OS_IOS
_serviceDiscoveryAgent->stop();
if (_serviceDiscoveryAgent && _serviceDiscoveryAgent->isActive()) {
_serviceDiscoveryAgent->stop();
}
#endif
_socket->disconnectFromService();
}
Expand Down Expand Up @@ -319,8 +323,20 @@ void BluetoothWorker::_onSocketErrorOccurred(QBluetoothSocket::SocketError socke
}

#ifdef Q_OS_IOS
void BluetoothWorker::_onServiceErrorOccurred(QBluetoothServiceDiscoveryAgent::Error error)
{
const QString errorString = _serviceDiscoveryAgent->errorString();
qCWarning(BluetoothLinkLog) << "Socket error:" << error << errorString;
emit errorOccurred(errorString);
}

void BluetoothWorker::_serviceDiscovered(const QBluetoothServiceInfo &info)
{
if (isConnected()) {
qCWarning(BluetoothLinkLog) << "Already connected to" << _config->device().name;
return;
}

if (info.device().name().isEmpty() || !info.isValid()) {
return;
}
Expand Down Expand Up @@ -361,6 +377,8 @@ BluetoothLink::BluetoothLink(SharedLinkConfigurationPtr &config, QObject *parent
(void) connect(_worker, &BluetoothWorker::dataReceived, this, &BluetoothLink::_onDataReceived, Qt::QueuedConnection);
(void) connect(_worker, &BluetoothWorker::dataSent, this, &BluetoothLink::_onDataSent, Qt::QueuedConnection);

(void) connect(_bluetoothConfig, &BluetoothConfiguration::errorOccurred, this, &BluetoothLink::_onErrorOccurred);

_workerThread->start();
}

Expand Down
16 changes: 9 additions & 7 deletions src/Comms/BluetoothLink.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,22 +10,21 @@
#pragma once

#include <QtBluetooth/QBluetoothAddress>
#include <QtBluetooth/QBluetoothDeviceDiscoveryAgent>
#include <QtBluetooth/QBluetoothDeviceInfo>
#ifdef Q_OS_IOS
#include <QtBluetooth/QBluetoothServiceDiscoveryAgent>
#endif
#include <QtBluetooth/QBluetoothServiceInfo>
#include <QtBluetooth/QBluetoothSocket>
#include <QtBluetooth/QBluetoothUuid>
#include <QtCore/QList>
#include <QtCore/QLoggingCategory>
#include <QtCore/QString>
#include <QtNetwork/QAbstractSocket>

#include "LinkConfiguration.h"
#include "LinkInterface.h"

#ifdef Q_OS_IOS
class QBluetoothServiceInfo;
class QBluetoothServiceDiscoveryAgent;
#endif
class QBluetoothDeviceDiscoveryAgent;
class QThread;

Q_DECLARE_LOGGING_CATEGORY(BluetoothLinkLog)
Expand Down Expand Up @@ -119,9 +118,11 @@ class BluetoothConfiguration : public LinkConfiguration
void deviceChanged();
void nameListChanged();
void scanningChanged();
void errorOccurred(const QString &errorString);

private slots:
void _deviceDiscovered(const QBluetoothDeviceInfo &info);
void _onSocketErrorOccurred(QBluetoothDeviceDiscoveryAgent::Error error);

private:
void _initDeviceDiscoveryAgent();
Expand Down Expand Up @@ -164,8 +165,9 @@ private slots:
void _onSocketBytesWritten(qint64 bytes);
void _onSocketErrorOccurred(QBluetoothSocket::SocketError socketError);
#ifdef Q_OS_IOS
void _discoveryFinished();
void _onServiceErrorOccurred(QBluetoothServiceDiscoveryAgent::Error error);
void _serviceDiscovered(const QBluetoothServiceInfo &info);
void _discoveryFinished();
#endif

private:
Expand Down
15 changes: 11 additions & 4 deletions src/Comms/SerialLink.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,6 @@ QString SerialConfiguration::cleanPortDisplayName(const QString &name)
SerialWorker::SerialWorker(const SerialConfiguration *config, QObject *parent)
: QObject(parent)
, _config(config)
, _port(new QSerialPort(this))
, _timer(new QTimer(this))
{
// qCDebug(SerialLinkLog) << Q_FUNC_INFO << this;

Expand All @@ -156,16 +154,25 @@ SerialWorker::~SerialWorker()

bool SerialWorker::isConnected() const
{
return _port->isOpen();
return (_port && _port->isOpen());
}

void SerialWorker::setupPort()
{
Q_ASSERT(!_port);
_port = new QSerialPort(this);

Q_ASSERT(!_timer);
_timer = new QTimer(this);

(void) connect(_port, &QSerialPort::aboutToClose, this, &SerialWorker::_onPortDisconnected);
(void) connect(_port, &QSerialPort::readyRead, this, &SerialWorker::_onPortReadyRead);
// (void) connect(_port, &QSerialPort::bytesWritten, this, &SerialWorker::_onPortBytesWritten);
(void) connect(_port, &QSerialPort::errorOccurred, this, &SerialWorker::_onPortErrorOccurred);

/* if (SerialLinkLog().isDebugEnabled()) {
(void) connect(_port, &QSerialPort::bytesWritten, this, &SerialWorker::_onPortBytesWritten);
} */

(void) connect(_timer, &QTimer::timeout, this, &SerialWorker::_checkPortAvailability);
_timer->start(CONNECT_TIMEOUT_MS);
}
Expand Down
13 changes: 8 additions & 5 deletions src/Comms/TCPLink.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ void TCPConfiguration::saveSettings(QSettings &settings, const QString &root)
TCPWorker::TCPWorker(const TCPConfiguration *config, QObject *parent)
: QObject(parent)
, _config(config)
, _socket(new QTcpSocket(this))
{
// qCDebug(TCPLinkLog) << Q_FUNC_INFO << this;
}
Expand All @@ -94,27 +93,31 @@ TCPWorker::~TCPWorker()

bool TCPWorker::isConnected() const
{
return (_socket->isOpen() && (_socket->state() == QAbstractSocket::ConnectedState));
return (_socket && _socket->isOpen() && (_socket->state() == QAbstractSocket::ConnectedState));
}

void TCPWorker::setupSocket()
{
Q_ASSERT(!_socket);
_socket = new QTcpSocket(this);

_socket->setSocketOption(QAbstractSocket::LowDelayOption, 1);
_socket->setSocketOption(QAbstractSocket::KeepAliveOption, 1);
_socket->setSocketOption(QAbstractSocket::TypeOfServiceOption, TYPE_OF_SERVICE);

(void) connect(_socket, &QTcpSocket::connected, this, &TCPWorker::_onSocketConnected);
(void) connect(_socket, &QTcpSocket::disconnected, this, &TCPWorker::_onSocketDisconnected);
(void) connect(_socket, &QTcpSocket::readyRead, this, &TCPWorker::_onSocketReadyRead);
// (void) connect(_socket, &QTcpSocket::bytesWritten, this, &TCPWorker::_onSocketBytesWritten);
(void) connect(_socket, &QTcpSocket::errorOccurred, this, &TCPWorker::_onSocketErrorOccurred);

if (TCPLinkLog().isDebugEnabled()) {
(void) QObject::connect(_socket, &QTcpSocket::stateChanged, this, [](QTcpSocket::SocketState state) {
// (void) connect(_socket, &QTcpSocket::bytesWritten, this, &TCPWorker::_onSocketBytesWritten);

(void) connect(_socket, &QTcpSocket::stateChanged, this, [](QTcpSocket::SocketState state) {
qCDebug(TCPLinkLog) << "TCP State Changed:" << state;
});

(void) QObject::connect(_socket, &QTcpSocket::hostFound, this, []() {
(void) connect(_socket, &QTcpSocket::hostFound, this, []() {
qCDebug(TCPLinkLog) << "TCP Host Found";
});
}
Expand Down
6 changes: 4 additions & 2 deletions src/Comms/UDPLink.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,6 @@ const QHostAddress UDPWorker::_multicastGroup = QHostAddress(QStringLiteral("224
UDPWorker::UDPWorker(const UDPConfiguration *config, QObject *parent)
: QObject(parent)
, _udpConfig(config)
, _socket(new QUdpSocket(this))
{
// qCDebug(UDPLinkLog) << Q_FUNC_INFO << this;
}
Expand All @@ -271,11 +270,14 @@ UDPWorker::~UDPWorker()

bool UDPWorker::isConnected() const
{
return (_socket->isValid() && _isConnected);
return (_socket && _socket->isValid() && _isConnected);
}

void UDPWorker::setupSocket()
{
Q_ASSERT(!_socket);
_socket = new QUdpSocket(this);

const QList<QHostAddress> localAddresses = QNetworkInterface::allAddresses();
_localAddresses = QSet(localAddresses.constBegin(), localAddresses.constEnd());

Expand Down

4 comments on commit bdfe3d0

@rubenp02
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this makes link error messages (such as when trying to connect to a bound UDP port) appear over and over again in such a way that the app cannot be used. Not 100% sure if it's this or another recent related commit

@HTRamsey
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably I emit an error signal somewhere in the UDP Link for an error that was previously ignored. I had the same issue with failing to set the multicast group, so I had to not emit an error for that. It's probably from using autoconnect which would generate an error over and over. I'll check it out

@HTRamsey
Copy link
Collaborator Author

@HTRamsey HTRamsey commented on bdfe3d0 Dec 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it the "Failed to bind UDP socket to port" message?

Edit: Nevermind, I got it

@HTRamsey
Copy link
Collaborator Author

@HTRamsey HTRamsey commented on bdfe3d0 Dec 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So basically you have to not disconnect the socket on a critical error during setup, so I decided to only disconnect links that aren't autoconnect which will prevent trying to reopen continuously

Please sign in to comment.