cleanup

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
+    qDebug() << "Zeroconf: Setting connection offline:" << host->name() << url.toString();
-//    host->connections()->removeConnection(connection);
     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

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);
+        // In theory, we could roam from one connection to another.
-        if (alternativeConnection->priority() > existingConnection->priority()) {
+        // However, in practice it turns out there are too many issues for this to be reliable
-            qDebug() << "New connection has higher priority! Roaming from" << existingConnection->url() << existingConnection->priority() << "to" << alternativeConnection->url() << alternativeConnection->priority();
+        // 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;
+//            m_currentTransport = newTransport;
-        } else {
+//        } else {
-            qDebug() << "Connection" << alternativeConnection->url() << alternativeConnection->priority() << "has lower priority than existing" << existingConnection->url() << existingConnection->priority();
+//            qDebug() << "Connection" << alternativeConnection->url() << alternativeConnection->priority() << "has lower priority than existing" << existingConnection->url() << existingConnection->priority();
-            m_transportCandidates.remove(newTransport);
+//            m_transportCandidates.remove(newTransport);
-            newTransport->deleteLater();
+//            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.";
+    if (!m_currentTransport) {
-    emit connectedChanged(false);
+        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) {

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<NymeaTransportInterface*, Connection*> m_transportCandidates;
     NymeaTransportInterface *m_currentTransport = nullptr;
     NymeaHost *m_currentHost = nullptr;
+    Connection *m_preferredConnection = nullptr;
 };
 
 #endif // NYMEACONNECTION_H

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")) {
+    if (m_url.scheme().startsWith("nymea")) {
-//        prio += 5;
+        prio += 1;
-//    }
+    }
     return prio;
 }

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;

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();

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"))
+                        var host = discovery.nymeaHosts.find(model.id)
-                        page.cancel.connect(function() {
+                        engine.connection.connect(host);
-                            engine.connection.disconnect()
-                            pageStack.pop(root, StackView.Immediate);
-                            pageStack.push(discoveryPage)
-                        })
-                        engine.connection.connect("cloud://" + model.id)
                     }
                 }
 

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;
+                            var bestIndex = -1
-                            for (var i = 1; i < nymeaHost.connections.count; i++) {
+                            var bestPriority = 0;
-                                var oldConfig = nymeaHost.connections.get(usedConfigIndex);
+                            for (var i = 0; i < nymeaHost.connections.count; i++) {
-                                var newConfig = nymeaHost.connections.get(i);
+                                var connection = nymeaHost.connections.get(i);
-
+                                if (bestIndex === -1 || connection.priority > bestPriority) {
-                                // Preference of bearerType
+                                    bestIndex = i;
-                                var bearerPreference = [Connection.BearerTypeEthernet, Connection.BearerTypeWifi, Connection.BearerTypeBluetooth, Connection.BearerTypeCloud]
+                                    bestPriority = connection.priority;
-                                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;
                                 }
                             }
-                            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))
                                 }
                             }
                         }