From 5f99248fd2d5a342cf378cfa46e61e212528b440 Mon Sep 17 00:00:00 2001 From: Michael Zanetti Date: Thu, 7 Feb 2019 13:12:57 +0100 Subject: [PATCH] cleanup --- .../discovery/zeroconfdiscovery.cpp | 17 ++-- .../connection/nymeaconnection.cpp | 77 +++++++++++++++---- .../connection/nymeaconnection.h | 3 +- libnymea-app-core/connection/nymeahost.cpp | 11 ++- libnymea-app-core/connection/nymeahosts.cpp | 2 +- nymea-app/ui/RootItem.qml | 2 +- nymea-app/ui/appsettings/CloudLoginPage.qml | 10 +-- nymea-app/ui/connection/ConnectPage.qml | 41 +++------- 8 files changed, 95 insertions(+), 68 deletions(-) diff --git a/libnymea-app-core/connection/discovery/zeroconfdiscovery.cpp b/libnymea-app-core/connection/discovery/zeroconfdiscovery.cpp index 836e75d6..baaf3c8e 100644 --- a/libnymea-app-core/connection/discovery/zeroconfdiscovery.cpp +++ b/libnymea-app-core/connection/discovery/zeroconfdiscovery.cpp @@ -122,12 +122,16 @@ void ZeroconfDiscovery::serviceEntryAdded(const QZeroConfService &entry) } url.setHost(!entry.ip().isNull() ? entry.ip().toString() : entry.ipv6().toString()); url.setPort(entry.port()); - if (!host->connections()->find(url)){ + Connection *connection = host->connections()->find(url); + if (!connection) { qDebug() << "Zeroconf: Adding new connection to host:" << host->name() << url.toString(); QString displayName = QString("%1:%2").arg(url.host()).arg(url.port()); - Connection *connection = new Connection(url, Connection::BearerTypeWifi, sslEnabled, displayName); + connection = new Connection(url, Connection::BearerTypeWifi, sslEnabled, displayName); connection->setOnline(true); host->connections()->addConnection(connection); + } else { + qDebug() << "Zeroconf: Setting connection online:" << host->name() << url.toString(); + connection->setOnline(true); } } @@ -179,14 +183,7 @@ void ZeroconfDiscovery::serviceEntryRemoved(const QZeroConfService &entry) return; } - // Ok, now we need to remove it -// host->connections()->removeConnection(connection); + qDebug() << "Zeroconf: Setting connection offline:" << host->name() << url.toString(); connection->setOnline(false); - - // And if there aren't any connections left, remove the entire device -// if (host->connections()->rowCount() == 0) { -// qDebug() << "Zeroconf: Removing connection from host:" << host->name() << url.toString(); -// m_nymeaHosts->removeHost(host); -// } } #endif diff --git a/libnymea-app-core/connection/nymeaconnection.cpp b/libnymea-app-core/connection/nymeaconnection.cpp index ed769e3b..63e8efea 100644 --- a/libnymea-app-core/connection/nymeaconnection.cpp +++ b/libnymea-app-core/connection/nymeaconnection.cpp @@ -278,18 +278,27 @@ void NymeaConnection::onConnected() if (m_currentTransport != newTransport) { qDebug() << "Alternative connection established:" << newTransport->url(); - Connection *existingConnection = m_transportCandidates.value(m_currentTransport); - Connection *alternativeConnection = m_transportCandidates.value(newTransport); - if (alternativeConnection->priority() > existingConnection->priority()) { - qDebug() << "New connection has higher priority! Roaming from" << existingConnection->url() << existingConnection->priority() << "to" << alternativeConnection->url() << alternativeConnection->priority(); + + // In theory, we could roam from one connection to another. + // However, in practice it turns out there are too many issues for this to be reliable + // So lets just tear down any alternative connection that comes up again. + + m_transportCandidates.remove(newTransport); + newTransport->deleteLater(); + + +// Connection *existingConnection = m_transportCandidates.value(m_currentTransport); +// Connection *alternativeConnection = m_transportCandidates.value(newTransport); +// if (alternativeConnection->priority() > existingConnection->priority()) { +// qDebug() << "New connection has higher priority! Roaming from" << existingConnection->url() << existingConnection->priority() << "to" << alternativeConnection->url() << alternativeConnection->priority(); // m_transportCandidates.remove(m_currentTransport); // m_currentTransport->deleteLater(); - m_currentTransport = newTransport; - } else { - qDebug() << "Connection" << alternativeConnection->url() << alternativeConnection->priority() << "has lower priority than existing" << existingConnection->url() << existingConnection->priority(); - m_transportCandidates.remove(newTransport); - newTransport->deleteLater(); - } +// m_currentTransport = newTransport; +// } else { +// qDebug() << "Connection" << alternativeConnection->url() << alternativeConnection->priority() << "has lower priority than existing" << existingConnection->url() << existingConnection->priority(); +// m_transportCandidates.remove(newTransport); +// newTransport->deleteLater(); +// } return; } } @@ -308,10 +317,21 @@ void NymeaConnection::onDisconnected() m_transportCandidates.remove(m_currentTransport); m_currentTransport->deleteLater(); m_currentTransport = nullptr; + + foreach (NymeaTransportInterface *candidate, m_transportCandidates.keys()) { + if (candidate->connectionState() == NymeaTransportInterface::ConnectionStateConnected) { + qDebug() << "Alternative connection is still up. Roaming to:" << candidate->url(); + m_currentTransport = candidate; + break; + } + } + emit currentConnectionChanged(); - qDebug() << "NymeaConnection: disconnected."; - emit connectedChanged(false); + if (!m_currentTransport) { + qDebug() << "NymeaConnection: disconnected."; + emit connectedChanged(false); + } // Try to reconnect, only if we're not waiting for SSL certs to be trusted. if (m_connectionStatus != ConnectionStatusSslUntrusted) { @@ -345,6 +365,16 @@ void NymeaConnection::updateActiveBearers() qDebug() << "No current host... Nothing to do..."; return; } + + // In theory we could try to connect via any different/new bearers now. However, in practice + // I have observed the following issues: + // - When roaming from WiFi to mobile data, we've already lost WiFi at this point + // (Unless aggressive WiFi to mobile handover is enabled on the phone) + // - When roaming from mobile to Wifi, for some reason, any new connection attempts + // fail as long as the mobile data isn't shut down by the OS. + // Those issues prevent roaming from working properly, so let's just not do anything at + // this point if there already is a connected channel, try reconnecting otherwise. + if (!m_currentTransport) { // There's a host but no connection. Try connecting now... qDebug() << "There's a host but no connection. Trying to connect now..."; @@ -414,8 +444,22 @@ void NymeaConnection::registerTransport(NymeaTransportInterfaceFactory *transpor } } -void NymeaConnection::connect(NymeaHost *nymeaHost) +void NymeaConnection::connect(NymeaHost *nymeaHost, Connection *connection) { + if (!nymeaHost) { + return; + } + + m_preferredConnection = nullptr; + if (connection) { + if (nymeaHost->connections()->find(connection->url())) { + qDebug() << "Setting preferred connection to" << connection->url(); + m_preferredConnection = connection; + } else { + qWarning() << "Connection" << connection << "is not a candidate for" << nymeaHost->name() << "Not setting preferred connection."; + } + } + setCurrentHost(nymeaHost); } @@ -427,6 +471,13 @@ void NymeaConnection::connectInternal(NymeaHost *host) emit connectionStatusChanged(); return; } + + if (m_preferredConnection) { + qDebug() << "Preferred connection is set. Using" << m_preferredConnection->url(); + connectInternal(m_preferredConnection); + return; + } + if (m_availableBearerTypes.testFlag(Connection::BearerTypeWifi) || m_availableBearerTypes.testFlag(Connection::BearerTypeEthernet)) { Connection* lanConnection = host->connections()->bestMatch(Connection::BearerTypeWifi | Connection::BearerTypeEthernet); if (lanConnection) { diff --git a/libnymea-app-core/connection/nymeaconnection.h b/libnymea-app-core/connection/nymeaconnection.h index faeba0a4..ad7c69a3 100644 --- a/libnymea-app-core/connection/nymeaconnection.h +++ b/libnymea-app-core/connection/nymeaconnection.h @@ -43,7 +43,7 @@ public: void registerTransport(NymeaTransportInterfaceFactory *transportFactory); - Q_INVOKABLE void connect(NymeaHost* nymeaHost); + Q_INVOKABLE void connect(NymeaHost* nymeaHost, Connection *connection = nullptr); Q_INVOKABLE void disconnect(); Q_INVOKABLE void acceptCertificate(const QString &url, const QByteArray &pem); Q_INVOKABLE bool isTrusted(const QString &url); @@ -95,6 +95,7 @@ private: QHash m_transportCandidates; NymeaTransportInterface *m_currentTransport = nullptr; NymeaHost *m_currentHost = nullptr; + Connection *m_preferredConnection = nullptr; }; #endif // NYMEACONNECTION_H diff --git a/libnymea-app-core/connection/nymeahost.cpp b/libnymea-app-core/connection/nymeahost.cpp index 3c0b4f00..3e59ed3a 100644 --- a/libnymea-app-core/connection/nymeahost.cpp +++ b/libnymea-app-core/connection/nymeahost.cpp @@ -238,12 +238,17 @@ void Connection::setOnline(bool online) if (m_online != online) { m_online = online; emit onlineChanged(); + emit priorityChanged(); } } int Connection::priority() const { int prio = 0; + if (m_online) { + prio += 1000; + } + switch(m_bearerType) { case BearerTypeEthernet: prio += 400; @@ -263,8 +268,8 @@ int Connection::priority() const if (m_secure) { prio += 10; } -// if (m_url.scheme().startsWith("nymea")) { -// prio += 5; -// } + if (m_url.scheme().startsWith("nymea")) { + prio += 1; + } return prio; } diff --git a/libnymea-app-core/connection/nymeahosts.cpp b/libnymea-app-core/connection/nymeahosts.cpp index b00f5e2d..18b6987e 100644 --- a/libnymea-app-core/connection/nymeahosts.cpp +++ b/libnymea-app-core/connection/nymeahosts.cpp @@ -206,7 +206,7 @@ bool NymeaHostsFilterModel::filterAcceptsRow(int sourceRow, const QModelIndex &s if (m_nymeaConnection && !m_showUneachableBearers) { bool hasReachableConnection = false; for (int i = 0; i < host->connections()->rowCount(); i++) { - qDebug() << "checking host for available bearer" << host->name() << host->connections()->get(i)->url() << "available bearer types:" << m_nymeaConnection->availableBearerTypes() << "hosts bearer types" << host->connections()->get(i)->bearerType(); +// qDebug() << "checking host for available bearer" << host->name() << host->connections()->get(i)->url() << "available bearer types:" << m_nymeaConnection->availableBearerTypes() << "hosts bearer types" << host->connections()->get(i)->bearerType(); if (m_nymeaConnection->availableBearerTypes().testFlag(host->connections()->get(i)->bearerType())) { hasReachableConnection = true; break; diff --git a/nymea-app/ui/RootItem.qml b/nymea-app/ui/RootItem.qml index 37bd0539..8d87e6d4 100644 --- a/nymea-app/ui/RootItem.qml +++ b/nymea-app/ui/RootItem.qml @@ -111,6 +111,7 @@ Item { pageStack.push(Qt.resolvedUrl("connection/ConnectPage.qml"), StackView.Immediate) } + Timer { running: true; repeat: false; interval: 3000; onTriggered: PlatformHelper.hideSplashScreen(); } function init() { print("calling init. Auth required:", engine.jsonRpcClient.authenticationRequired, "initial setup required:", engine.jsonRpcClient.initialSetupRequired, "jsonrpc connected:", engine.jsonRpcClient.connected, "Current host:", engine.connection.currentHost) @@ -151,7 +152,6 @@ Item { } print("pushing ConnectingPage") - PlatformHelper.hideSplashScreen(); var page = pageStack.push(Qt.resolvedUrl("connection/ConnectingPage.qml")); page.cancel.connect(function(){ engine.connection.disconnect(); diff --git a/nymea-app/ui/appsettings/CloudLoginPage.qml b/nymea-app/ui/appsettings/CloudLoginPage.qml index 742227ac..e6f827a7 100644 --- a/nymea-app/ui/appsettings/CloudLoginPage.qml +++ b/nymea-app/ui/appsettings/CloudLoginPage.qml @@ -86,14 +86,10 @@ Page { secondaryIconName: !model.online ? "../images/cloud-error.svg" : "" onClicked: { + print("clicked, connected:", engine.connection.connected, model.id) if (!engine.connection.connected) { - var page = pageStack.push(Qt.resolvedUrl("../connection/ConnectingPage.qml")) - page.cancel.connect(function() { - engine.connection.disconnect() - pageStack.pop(root, StackView.Immediate); - pageStack.push(discoveryPage) - }) - engine.connection.connect("cloud://" + model.id) + var host = discovery.nymeaHosts.find(model.id) + engine.connection.connect(host); } } diff --git a/nymea-app/ui/connection/ConnectPage.qml b/nymea-app/ui/connection/ConnectPage.qml index b3b627c9..19f11b58 100644 --- a/nymea-app/ui/connection/ConnectPage.qml +++ b/nymea-app/ui/connection/ConnectPage.qml @@ -130,39 +130,16 @@ Page { objectName: "discoveryDelegate" + index property var nymeaHost: hostsProxy.get(index) property string defaultConnectionIndex: { - var usedConfigIndex = 0; - for (var i = 1; i < nymeaHost.connections.count; i++) { - var oldConfig = nymeaHost.connections.get(usedConfigIndex); - var newConfig = nymeaHost.connections.get(i); - - // Preference of bearerType - var bearerPreference = [Connection.BearerTypeEthernet, Connection.BearerTypeWifi, Connection.BearerTypeBluetooth, Connection.BearerTypeCloud] - var oldBearerPriority = bearerPreference.indexOf(oldConfig.bearerType); - var newBearerPriority = bearerPreference.indexOf(newConfig.bearerType); - if (newBearerPriority < oldBearerPriority) { - print(nymeaHost.name, "switching to preferred index", i, "of bearer type", newConfig.bearerType, "from", oldConfig.bearerType, "new prio:", newBearerPriority, "old:", oldBearerPriority) - usedConfigIndex = i; - continue; - } - if (oldBearerPriority < newBearerPriority) { - continue; // discard new one the one we have is on a better bearer type - } - - // prefer secure over insecure - if (!oldConfig.secure && newConfig.secure) { - usedConfigIndex = i; - continue; - } - if (oldConfig.secure && !newConfig.secure) { - continue; // discard new one as the one we already have is more secure - } - - // both options are now on the same bearer and either secure or insecure, prefer nymearpc over websocket for less overhead - if (oldConfig.url.toString().startsWith("ws") && newConfig.url.toString().startsWith("nymea")) { - usedConfigIndex = i; + var bestIndex = -1 + var bestPriority = 0; + for (var i = 0; i < nymeaHost.connections.count; i++) { + var connection = nymeaHost.connections.get(i); + if (bestIndex === -1 || connection.priority > bestPriority) { + bestIndex = i; + bestPriority = connection.priority; } } - return usedConfigIndex + return bestIndex; } iconName: { @@ -405,8 +382,8 @@ Page { secondaryIconColor: "red" onClicked: { - root.connectToHost2(dialog.nymeaHost.connections.get(index)) dialog.close() + engine.connection.connect(dialog.nymeaHost, dialog.nymeaHost.connections.get(index)) } } }