From 9fcdbafda4dbb780710cb024638cc4fdaf8ba402 Mon Sep 17 00:00:00 2001 From: Michael Zanetti Date: Fri, 17 Dec 2021 01:17:34 +0100 Subject: [PATCH] Tune reconnecting after deep sleep --- libnymea-app/connection/nymeaconnection.cpp | 48 ++++++++++--------- .../connection/tcpsockettransport.cpp | 14 ++++-- libnymea-app/jsonrpc/jsonrpcclient.cpp | 12 ++++- 3 files changed, 45 insertions(+), 29 deletions(-) diff --git a/libnymea-app/connection/nymeaconnection.cpp b/libnymea-app/connection/nymeaconnection.cpp index 8a665ddf..7533afca 100644 --- a/libnymea-app/connection/nymeaconnection.cpp +++ b/libnymea-app/connection/nymeaconnection.cpp @@ -64,14 +64,28 @@ NymeaConnection::NymeaConnection(QObject *parent) : QObject(parent) }); QGuiApplication *app = static_cast(QGuiApplication::instance()); - QObject::connect(app, &QGuiApplication::applicationStateChanged, this, [this](Qt::ApplicationState state) { + QObject::connect(app, &QGuiApplication::applicationStateChanged, this, [app, this](Qt::ApplicationState state) { qCDebug(dcNymeaConnection()) << "Application state changed to:" << state; + + // On android, when the device is suspended with display off for a while, it happens that the app is still woken up + // occationally and it would try to reconnect, but the WiFi is in deep sleep and the connection attempts time out in 5 seconds + // or so. So it happens that the device is woken up and the app would try to reconnect, but there are still ongoing + // connection attempts on all the transports. In order to not wait for them time out, let's abort all the currently + // pending attempts so we try again immediately, now that wifi should be up again. + if (app->applicationState() == Qt::ApplicationActive) { + foreach (NymeaTransportInterface *transport, m_transportCandidates.keys()) { + if (transport != m_currentTransport) { + transport->disconnect(); + } + } + } + updateActiveBearers(); }); updateActiveBearers(); - m_reconnectTimer.setInterval(500); + m_reconnectTimer.setInterval(100); m_reconnectTimer.setSingleShot(true); connect(&m_reconnectTimer, &QTimer::timeout, this, [this](){ if (m_currentHost && !m_currentTransport) { @@ -309,25 +323,18 @@ void NymeaConnection::onDisconnected() } t->deleteLater(); - qCInfo(dcNymeaConnection()) << "Current transport:" << m_currentTransport << "Remaining connections:" << m_transportCandidates.count() << "Current host:" << m_currentHost; + qCDebug(dcNymeaConnection()) << "Current transport:" << m_currentTransport << "Remaining connections:" << m_transportCandidates.count() << "Current host:" << m_currentHost; if (!m_currentTransport && m_transportCandidates.isEmpty()) { - qCWarning(dcNymeaConnection()) << "Last connection dropped."; - QTimer::singleShot(1000, this, [this](){ - if (m_currentHost && m_connectionStatus != ConnectionStatusSslUntrusted) { - qCInfo(dcNymeaConnection()) << "Trying to reconnect.."; - connectInternal(m_currentHost); - } - }); + qCInfo(dcNymeaConnection()) << "Last connection dropped."; + if (!m_reconnectTimer.isActive()) { + m_reconnectTimer.start(); + } } return; } m_transportCandidates.remove(m_currentTransport); - disconnect(m_currentTransport); - disconnect(m_currentTransport, nullptr, nullptr, nullptr); - // FIXME: directly deleting crashes with a stack trace that never passes any of our code. - // Seems to happen for both, Tcp and WebSocket m_currentTransport->deleteLater(); m_currentTransport = nullptr; @@ -352,13 +359,9 @@ void NymeaConnection::onDisconnected() } // Try to reconnect, only if we're not waiting for SSL certs to be trusted. - if (m_connectionStatus != ConnectionStatusSslUntrusted) { - QTimer::singleShot(1000, this, [this](){ - if (m_currentHost) { - qCInfo(dcNymeaConnection()) << "Trying to reconnect after disconnect..."; - connectInternal(m_currentHost); - } - }); + if (m_connectionStatus != ConnectionStatusSslUntrusted && !m_reconnectTimer.isActive()) { + qCInfo(dcNymeaConnection()) << "Trying to reconnect after disconnect..."; + m_reconnectTimer.start(); } } @@ -366,6 +369,7 @@ void NymeaConnection::onDataAvailable(const QByteArray &data) { NymeaTransportInterface *t = static_cast(sender()); if (t == m_currentTransport) { +// qCDebug(dcNymeaConnection()) << "Data available"; emit dataAvailable(data); } else { qCDebug(dcNymeaConnection()) << "Received data from a transport that is not the current one:" << t->url(); @@ -534,7 +538,7 @@ bool NymeaConnection::connectInternal(Connection *connection) QObject::connect(newTransport, &NymeaTransportInterface::error, this, &NymeaConnection::onError); QObject::connect(newTransport, &NymeaTransportInterface::connected, this, &NymeaConnection::onConnected); QObject::connect(newTransport, &NymeaTransportInterface::disconnected, this, &NymeaConnection::onDisconnected); - QObject::connect(newTransport, &NymeaTransportInterface::dataReady, this, &NymeaConnection::onDataAvailable); + QObject::connect(newTransport, &NymeaTransportInterface::dataReady, this, &NymeaConnection::onDataAvailable, Qt::QueuedConnection); // // Load any certificate we might have for this url // QByteArray pem; diff --git a/libnymea-app/connection/tcpsockettransport.cpp b/libnymea-app/connection/tcpsockettransport.cpp index 0a93c7e4..df154a48 100644 --- a/libnymea-app/connection/tcpsockettransport.cpp +++ b/libnymea-app/connection/tcpsockettransport.cpp @@ -33,6 +33,10 @@ #include #include +#include "logging.h" + +NYMEA_LOGGING_CATEGORY(dcTcpTransport, "TcpTransport") + TcpSocketTransport::TcpSocketTransport(QObject *parent) : NymeaTransportInterface(parent) { QObject::connect(&m_socket, &QSslSocket::connected, this, &TcpSocketTransport::onConnected); @@ -72,7 +76,7 @@ QSslCertificate TcpSocketTransport::serverCertificate() const void TcpSocketTransport::onConnected() { if (m_url.scheme() == "nymea") { - qDebug() << "TCP socket connected"; + qCDebug(dcTcpTransport()) << "TCP socket connected"; emit connected(); } } @@ -87,14 +91,14 @@ bool TcpSocketTransport::connect(const QUrl &url) { m_url = url; if (url.scheme() == "nymeas") { - qDebug() << "TCP socket connecting to" << url.host() << url.port(); + qCDebug(dcTcpTransport()) << "TCP socket connecting to" << url.host() << url.port(); m_socket.connectToHostEncrypted(url.host(), static_cast(url.port())); return true; } else if (url.scheme() == "nymea") { m_socket.connectToHost(url.host(), static_cast(url.port())); return true; } - qWarning() << "TCP socket: Unsupported scheme"; + qCWarning(dcTcpTransport()) << "TCP socket: Unsupported scheme"; return false; } @@ -118,7 +122,7 @@ NymeaTransportInterface::ConnectionState TcpSocketTransport::connectionState() c void TcpSocketTransport::disconnect() { - qDebug() << "closing socket"; + qCDebug(dcTcpTransport()) << "closing socket"; m_socket.disconnectFromHost(); m_socket.close(); // QTcpSocket might endlessly wait for a timeout if we call connectToHost() for an IP which isn't @@ -135,7 +139,7 @@ void TcpSocketTransport::socketReadyRead() void TcpSocketTransport::onSocketStateChanged(const QAbstractSocket::SocketState &state) { - qDebug() << "Socket state changed -->" << state; + qCDebug(dcTcpTransport()) << "Socket state changed -->" << state; if (state == QAbstractSocket::UnconnectedState) { emit disconnected(); } diff --git a/libnymea-app/jsonrpc/jsonrpcclient.cpp b/libnymea-app/jsonrpc/jsonrpcclient.cpp index c46c1dc2..3487d2da 100644 --- a/libnymea-app/jsonrpc/jsonrpcclient.cpp +++ b/libnymea-app/jsonrpc/jsonrpcclient.cpp @@ -67,7 +67,9 @@ JsonRpcClient::JsonRpcClient(QObject *parent) : connect(m_connection, &NymeaConnection::connectedChanged, this, &JsonRpcClient::onInterfaceConnectedChanged); connect(m_connection, &NymeaConnection::currentHostChanged, this, &JsonRpcClient:: currentHostChanged); connect(m_connection, &NymeaConnection::currentConnectionChanged, this, &JsonRpcClient:: currentConnectionChanged); - connect(m_connection, &NymeaConnection::dataAvailable, this, &JsonRpcClient::dataReceived); + // We'll connect this Queued, because in case of a disconnect we'll want to react on that ASAP instead of processing a queue that may be left in buffers + // Especially on mobile platforms (hello Android) we get a huge queue of buffers upon resume from suspend just to get a disconnect after that. + connect(m_connection, &NymeaConnection::dataAvailable, this, &JsonRpcClient::dataReceived, Qt::QueuedConnection); registerNotificationHandler(this, QStringLiteral("JSONRPC"), "notificationReceived"); } @@ -538,10 +540,11 @@ void JsonRpcClient::onInterfaceConnectedChanged(bool connected) { if (!connected) { - qCDebug(dcJsonRpc()) << "JsonRpcClient: Transport disconnected."; + qCInfo(dcJsonRpc()) << "JsonRpcClient: Transport disconnected."; m_initialSetupRequired = false; m_authenticationRequired = false; m_authenticated = false; + m_receiveBuffer.clear(); m_serverQtVersion.clear(); m_serverQtBuildVersion.clear(); if (m_connected) { @@ -560,6 +563,11 @@ void JsonRpcClient::onInterfaceConnectedChanged(bool connected) void JsonRpcClient::dataReceived(const QByteArray &data) { + if (!m_connection->connected()) { + // Given this slot is invoked with QueuedConnection, we might still get pending data packages after a disconnected event + // In that case we can discard all pending packages as we'll have to reconnect anyways. + return; + } // qDebug() << "JsonRpcClient: received data:" << qUtf8Printable(data); m_receiveBuffer.append(data);