Merge PR #277: Tado Plug-In: Fixed crash during thing pairing

tado/integrationplugintado.cpp

@@ -48,6 +48,7 @@ void IntegrationPluginTado::startPairing(ThingPairingInfo *info)
     // Checking the internet connection
     NetworkAccessManager *network = hardwareManager()->networkManager();
     QNetworkReply *reply = network->get(QNetworkRequest(QUrl("https://my.tado.com/api/v2")));
+    connect(reply, &QNetworkReply::finished, reply, &QNetworkReply::deleteLater);
     connect(reply, &QNetworkReply::finished, info, [reply, info] {
 
         if (reply->error() == QNetworkReply::NetworkError::HostNotFoundError) {
@@ -56,13 +57,12 @@ void IntegrationPluginTado::startPairing(ThingPairingInfo *info)
             info->finish(Thing::ThingErrorNoError, QT_TR_NOOP("Please enter the login credentials for your Tado account."));
         }
     });
-    connect(reply, &QNetworkReply::finished, this, &QNetworkReply::deleteLater);
 }
 
 void IntegrationPluginTado::confirmPairing(ThingPairingInfo *info, const QString &username, const QString &password)
 {
+    qCDebug(dcTado()) << "Confirm pairing" << username << "Network manager available" << hardwareManager()->networkManager()->available();
     Tado *tado = new Tado(hardwareManager()->networkManager(), username, this);
-    connect(tado, &Tado::tokenReceived, this, &IntegrationPluginTado::onTokenReceived);
     connect(tado, &Tado::authenticationStatusChanged, this, &IntegrationPluginTado::onAuthenticationStatusChanged);
     connect(tado, &Tado::requestExecuted, this, &IntegrationPluginTado::onRequestExecuted);
     connect(tado, &Tado::connectionChanged, this, &IntegrationPluginTado::onConnectionChanged);
@@ -71,13 +71,31 @@ void IntegrationPluginTado::confirmPairing(ThingPairingInfo *info, const QString
     connect(tado, &Tado::zoneStateReceived, this, &IntegrationPluginTado::onZoneStateReceived);
     connect(tado, &Tado::overlayReceived, this, &IntegrationPluginTado::onOverlayReceived);
     m_unfinishedTadoAccounts.insert(info->thingId(), tado);
-    m_unfinishedDevicePairings.insert(info->thingId(), info);
-    tado->getToken(password);
 
-    pluginStorage()->beginGroup(info->thingId().toString());
+    connect(info, &ThingPairingInfo::aborted, this, [info, tado, this]() {
-    pluginStorage()->setValue("username", username);
+        qCWarning(dcTado()) << "Thing pairing has been aborted, going to clean-up";
-    pluginStorage()->setValue("password", password);
+        m_unfinishedTadoAccounts.remove(info->thingId());
-    pluginStorage()->endGroup();
+        tado->deleteLater();
+    });
+
+    connect(tado, &Tado::connectionError, info, [this, info] (QNetworkReply::NetworkError error){
+        if (error != QNetworkReply::NetworkError::NoError){
+            info->finish(Thing::ThingErrorSetupFailed);
+        }
+        // info->finish(success) will be called after the token has been received
+    });
+
+    connect(tado, &Tado::tokenReceived, info, [this, info, username, password](Tado::Token token) {
+        Q_UNUSED(token)
+
+        pluginStorage()->beginGroup(info->thingId().toString());
+        pluginStorage()->setValue("username", username);
+        pluginStorage()->setValue("password", password);
+        pluginStorage()->endGroup();
+
+        info->finish(Thing::ThingErrorNoError);
+    });
+    tado->getToken(password);
 }
 
 void IntegrationPluginTado::setupThing(ThingSetupInfo *info)
@@ -87,19 +105,18 @@ void IntegrationPluginTado::setupThing(ThingSetupInfo *info)
     if (thing->thingClassId() == tadoConnectionThingClassId) {
 
         qCDebug(dcTado) << "Setup tado connection" << thing->name() << thing->params();
-        pluginStorage()->beginGroup(thing->id().toString());
-        QString username = pluginStorage()->value("username").toString();
-        QString password = pluginStorage()->value("password").toString();
-        pluginStorage()->endGroup();
-
         Tado *tado;
         if (m_unfinishedTadoAccounts.contains(thing->id())) {
             tado = m_unfinishedTadoAccounts.take(thing->id());
             m_tadoAccounts.insert(thing->id(), tado);
             return info->finish(Thing::ThingErrorNoError);
         } else {
+            pluginStorage()->beginGroup(thing->id().toString());
+            QString username = pluginStorage()->value("username").toString();
+            QString password = pluginStorage()->value("password").toString();
+            pluginStorage()->endGroup();
+
             tado = new Tado(hardwareManager()->networkManager(), username, this);
-            connect(tado, &Tado::tokenReceived, this, &IntegrationPluginTado::onTokenReceived);
             connect(tado, &Tado::authenticationStatusChanged, this, &IntegrationPluginTado::onAuthenticationStatusChanged);
             connect(tado, &Tado::requestExecuted, this, &IntegrationPluginTado::onRequestExecuted);
             connect(tado, &Tado::connectionChanged, this, &IntegrationPluginTado::onConnectionChanged);
@@ -107,14 +124,43 @@ void IntegrationPluginTado::setupThing(ThingSetupInfo *info)
             connect(tado, &Tado::zonesReceived, this, &IntegrationPluginTado::onZonesReceived);
             connect(tado, &Tado::zoneStateReceived, this, &IntegrationPluginTado::onZoneStateReceived);
             connect(tado, &Tado::overlayReceived, this, &IntegrationPluginTado::onOverlayReceived);
-            tado->getToken(password);
+
             m_tadoAccounts.insert(thing->id(), tado);
-            m_asyncDeviceSetup.insert(tado, info);
             connect(info, &ThingSetupInfo::aborted, [info, this] {
-                Tado *tado = m_tadoAccounts.take(info->thing()->id());
+                if (m_tadoAccounts.contains(info->thing()->id())) {
-                m_asyncDeviceSetup.remove(tado);
+                    Tado *tado = m_tadoAccounts.take(info->thing()->id());
-                tado->deleteLater();
+                    tado->deleteLater();
+                }
             });
+
+            connect(tado, &Tado::tokenReceived, info, [this, info, tado](Tado::Token token) {
+                Q_UNUSED(token)
+
+                qCDebug(dcTado()) << "Token received, account setup successfull";
+                info->finish(Thing::ThingErrorNoError);
+            });
+
+            connect(tado, &Tado::connectionError, info, [this, info] (QNetworkReply::NetworkError error){
+                if (error == QNetworkReply::NetworkError::HostNotFoundError) {
+                    QTimer::singleShot(2000, info, [info, this] {
+
+                        pluginStorage()->beginGroup(info->thing()->id().toString());
+                        QString password = pluginStorage()->value("password").toString();
+                        pluginStorage()->endGroup();
+                        if (m_tadoAccounts.contains(info->thing()->id())) {
+                            Tado *tado = m_tadoAccounts.take(info->thing()->id());
+                            tado->getToken(password);
+                        }
+                    });
+                } else if (error != QNetworkReply::NetworkError::NoError){
+                    if (m_tadoAccounts.contains(info->thing()->id())) {
+                        Tado *tado = m_tadoAccounts.take(info->thing()->id());
+                        tado->deleteLater();
+                    }
+                    info->finish(Thing::ThingErrorSetupFailed);
+                }
+            });
+            tado->getToken(password);
         }
 
     } else if (thing->thingClassId() == zoneThingClassId) {
@@ -226,8 +272,10 @@ void IntegrationPluginTado::onPluginTimer()
 {
     foreach (Thing *thing, myThings().filterByThingClassId(zoneThingClassId)) {
         Tado *tado = m_tadoAccounts.value(thing->parentId());
-        if (!tado)
+        if (!tado){
+            qCWarning(dcTado()) << "Could not find any Tado connection to Zone" << thing->name();
             continue;
+        }
 
         QString homeId = thing->paramValue(zoneThingHomeIdParamTypeId).toString();
         QString zoneId = thing->paramValue(zoneThingZoneIdParamTypeId).toString();
@@ -239,20 +287,6 @@ void IntegrationPluginTado::onConnectionChanged(bool connected)
 {
     Tado *tado = static_cast<Tado*>(sender());
 
-    if (m_asyncDeviceSetup.contains(tado)) {
-        //Thing setup failed, try as long as ThingSetupInfo is valid.
-        if (!connected) {
-            QTimer::singleShot(2000, tado, [tado, this]{
-                if(m_asyncDeviceSetup.contains(tado)){
-                    //Check once more if the ThingSetupInfo is still valid
-                    pluginStorage()->beginGroup(m_asyncDeviceSetup.value(tado)->thing()->id().toString());
-                    QString password = pluginStorage()->value("password").toString();
-                    pluginStorage()->endGroup();
-                    tado->getToken(password);
-                }
-            });
-        }
-    }
     if (m_tadoAccounts.values().contains(tado)){
         Thing *thing = myThings().findById(m_tadoAccounts.key(tado));
         if (!thing)
@@ -269,17 +303,12 @@ void IntegrationPluginTado::onAuthenticationStatusChanged(bool authenticated)
 {
     Tado *tado = static_cast<Tado*>(sender());
 
-    if (m_unfinishedTadoAccounts.values().contains(tado) && !authenticated){
-        ThingId id = m_unfinishedTadoAccounts.key(tado);
-        m_unfinishedTadoAccounts.remove(id);
-        ThingPairingInfo *info = m_unfinishedDevicePairings.take(id);
-        info->finish(Thing::ThingErrorSetupFailed);
-    }
-
     if (m_tadoAccounts.values().contains(tado)){
         Thing *thing = myThings().findById(m_tadoAccounts.key(tado));
-        if (!thing)
+        if (!thing){
+            qCWarning(dcTado()) << "OnAuthenticationChanged no thing found by ID" << m_tadoAccounts.key(tado);
             return;
+        }
         thing->setStateValue(tadoConnectionLoggedInStateTypeId, authenticated);
 
         if (!authenticated) {
@@ -307,25 +336,6 @@ void IntegrationPluginTado::onRequestExecuted(QUuid requestId, bool success)
     }
 }
 
-void IntegrationPluginTado::onTokenReceived(Tado::Token token)
-{
-    Q_UNUSED(token)
-
-    qCDebug(dcTado()) << "Token received";
-    Tado *tado = static_cast<Tado*>(sender());
-
-    if (m_asyncDeviceSetup.contains(tado)) {
-        ThingSetupInfo *info = m_asyncDeviceSetup.take(tado);
-        info->finish(Thing::ThingErrorNoError);
-    }
-
-    if (m_unfinishedTadoAccounts.values().contains(tado)) {
-        ThingId id = m_unfinishedTadoAccounts.key(tado);
-        ThingPairingInfo *info = m_unfinishedDevicePairings.take(id);
-        info->finish(Thing::ThingErrorNoError);
-    }
-}
-
 void IntegrationPluginTado::onHomesReceived(QList<Tado::Home> homes)
 {
     qCDebug(dcTado()) << "Homes received";

tado/integrationplugintado.h

@@ -57,10 +57,8 @@ public:
 private:
     PluginTimer *m_pluginTimer = nullptr;
     QHash<ThingId, Tado*> m_unfinishedTadoAccounts;
-    QHash<ThingId, ThingPairingInfo *> m_unfinishedDevicePairings;
 
     QHash<ThingId, Tado*> m_tadoAccounts;
-    QHash<Tado *, ThingSetupInfo *> m_asyncDeviceSetup;
     QHash<QUuid, ThingActionInfo *> m_asyncActions;
 
 private slots:
@@ -69,7 +67,6 @@ private slots:
     void onConnectionChanged(bool connected);
     void onAuthenticationStatusChanged(bool authenticated);
     void onRequestExecuted(QUuid requestId, bool success);
-    void onTokenReceived(Tado::Token token);
     void onHomesReceived(QList<Tado::Home> homes);
     void onZonesReceived(const QString &homeId, QList<Tado::Zone> zones);
     void onZoneStateReceived(const QString &homeId,const QString &zoneId, Tado::ZoneState sate);

tado/tado.cpp

@@ -81,12 +81,13 @@ void Tado::getToken(const QString &password)
 
     QNetworkReply *reply = m_networkManager->post(request, body);
     //qCDebug(dcTado()) << "Sending request" << request.url() << body;
+    connect(reply, &QNetworkReply::finished, reply, &QNetworkReply::deleteLater);
     connect(reply, &QNetworkReply::finished, this, [reply, this] {
-        reply->deleteLater();
-        int status = reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
 
+        int status = reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
         // Check HTTP status code
         if (status != 200 || reply->error() != QNetworkReply::NoError) {
+            emit connectionError(reply->error());
             if (reply->error() == QNetworkReply::HostNotFoundError) {
                 setConnectionStatus(false);
             }
@@ -136,18 +137,24 @@ void Tado::getToken(const QString &password)
 
 void Tado::getHomes()
 {
+    if(m_accessToken.isEmpty()) {
+        qCWarning(dcTado()) << "Not sending request, get the access token first";
+        return;
+    }
     QNetworkRequest request;
     request.setUrl(QUrl(m_baseControlUrl + "/me"));
     request.setHeader(QNetworkRequest::KnownHeaders::ContentTypeHeader, "application/x-www-form-urlencoded");
     request.setRawHeader("Authorization", "Bearer " + m_accessToken.toLocal8Bit());
     QNetworkReply *reply = m_networkManager->get(request);
     //qDebug(dcTado()) << "Sending request" << request.url() << request.rawHeaderList();
+    connect(reply, &QNetworkReply::finished, reply, &QNetworkReply::deleteLater);
     connect(reply, &QNetworkReply::finished, this, [reply, this] {
-        reply->deleteLater();
+
         int status = reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
 
         // Check HTTP status code
         if (status != 200 || reply->error() != QNetworkReply::NoError) {
+            emit connectionError(reply->error());
             if (reply->error() == QNetworkReply::HostNotFoundError) {
                 setConnectionStatus(false);
             }
@@ -187,8 +194,9 @@ void Tado::getZones(const QString &homeId)
     request.setRawHeader("Authorization", "Bearer " + m_accessToken.toLocal8Bit());
     QNetworkReply *reply = m_networkManager->get(request);
     //qDebug(dcTado()) << "Sending request" << request.url() << request.rawHeaderList();
+    connect(reply, &QNetworkReply::finished, reply, &QNetworkReply::deleteLater);
     connect(reply, &QNetworkReply::finished, this, [reply, homeId, this] {
-        reply->deleteLater();
+
         int status = reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
 
         // Check HTTP status code
@@ -227,18 +235,25 @@ void Tado::getZones(const QString &homeId)
 
 void Tado::getZoneState(const QString &homeId, const QString &zoneId)
 {
+    if(m_accessToken.isEmpty()) {
+        qCWarning(dcTado()) << "Not sending request, get the access token first";
+        return;
+    }
     QNetworkRequest request;
     request.setUrl(QUrl(m_baseControlUrl+"/homes/"+homeId+"/zones/"+zoneId+"/state"));
     request.setHeader(QNetworkRequest::KnownHeaders::ContentTypeHeader, "application/x-www-form-urlencoded");
     request.setRawHeader("Authorization", "Bearer " + m_accessToken.toLocal8Bit());
     QNetworkReply *reply = m_networkManager->get(request);
     //qDebug(dcTado()) << "Sending request" << request.url() << request.rawHeaderList();
+    connect(reply, &QNetworkReply::finished, reply, &QNetworkReply::deleteLater);
     connect(reply, &QNetworkReply::finished, this, [reply, homeId, zoneId, this] {
-        reply->deleteLater();
+
         int status = reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
 
         // Check HTTP status code
         if (status != 200 || reply->error() != QNetworkReply::NoError) {
+            emit connectionError(reply->error());
+
             if (reply->error() == QNetworkReply::HostNotFoundError) {
                 setConnectionStatus(false);
             }
@@ -292,6 +307,10 @@ void Tado::getZoneState(const QString &homeId, const QString &zoneId)
 
 QUuid Tado::setOverlay(const QString &homeId, const QString &zoneId, bool power, double targetTemperature)
 {
+    if(m_accessToken.isEmpty()) {
+        qCWarning(dcTado()) << "Not sending request, get the access token first";
+        return "";
+    }
     QUuid requestId = QUuid::createUuid();
     QNetworkRequest request;
     request.setUrl(QUrl(m_baseControlUrl+"/homes/"+homeId+"/zones/"+zoneId+"/overlay"));
@@ -308,13 +327,15 @@ QUuid Tado::setOverlay(const QString &homeId, const QString &zoneId, bool power,
     body.append("{\"setting\":{\"type\":\"HEATING\",\"power\":\""+ powerString + "\",\"temperature\":{\"celsius\":" + QVariant(targetTemperature).toByteArray() + "}},\"termination\":{\"type\":\"MANUAL\"}}");
     //qCDebug(dcTado()) << "Sending request" << body;
     QNetworkReply *reply = m_networkManager->put(request, body);
+    connect(reply, &QNetworkReply::finished, reply, &QNetworkReply::deleteLater);
     connect(reply, &QNetworkReply::finished, this, [homeId, zoneId, requestId, reply, this] {
-        reply->deleteLater();
 
         int status = reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
         // Check HTTP status code
         if (status != 200 || reply->error() != QNetworkReply::NoError) {
             emit requestExecuted(requestId, false);
+            emit connectionError(reply->error());
+
             if (reply->error() == QNetworkReply::HostNotFoundError) {
                 setConnectionStatus(false);
             }
@@ -355,18 +376,24 @@ QUuid Tado::setOverlay(const QString &homeId, const QString &zoneId, bool power,
 
 QUuid Tado::deleteOverlay(const QString &homeId, const QString &zoneId)
 {
+    if(m_accessToken.isEmpty()) {
+        qCWarning(dcTado()) << "Not sending request, get the access token first";
+        return "";
+    }
     QUuid requestId = QUuid::createUuid();
     QNetworkRequest request;
     request.setUrl(QUrl(m_baseControlUrl+"/homes/"+homeId+"/zones/"+zoneId+"/overlay"));
     request.setRawHeader("Authorization", "Bearer " + m_accessToken.toLocal8Bit());
     QNetworkReply *reply = m_networkManager->deleteResource(request);
+    connect(reply, &QNetworkReply::finished, reply, &QNetworkReply::deleteLater);
     connect(reply, &QNetworkReply::finished, this, [homeId, zoneId, requestId, reply, this] {
-        reply->deleteLater();
+
         int status = reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
 
         // Check HTTP status code
         if (status < 200 || status > 210 || reply->error() != QNetworkReply::NoError) {
             emit requestExecuted(requestId ,false);
+            emit connectionError(reply->error());
             if (reply->error() == QNetworkReply::HostNotFoundError) {
                 setConnectionStatus(false);
             }
@@ -428,6 +455,11 @@ void Tado::setConnectionStatus(bool status)
 
 void Tado::onRefreshTimer()
 {
+    if(m_refreshToken.isEmpty()) {
+        qCWarning(dcTado()) << "Not sending request, get the access token first";
+        return;
+    }
+
     QNetworkRequest request;
     request.setUrl(QUrl(m_baseAuthorizationUrl));
     request.setHeader(QNetworkRequest::KnownHeaders::ContentTypeHeader, "application/x-www-form-urlencoded");
@@ -439,12 +471,14 @@ void Tado::onRefreshTimer()
     body.append("&scope=home.user");
 
     QNetworkReply *reply = m_networkManager->post(request, body);
+    connect(reply, &QNetworkReply::finished, reply, &QNetworkReply::deleteLater);
     connect(reply, &QNetworkReply::finished, this, [reply, this] {
-        reply->deleteLater();
+
         int status = reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
 
         // Check HTTP status code
         if (status != 200 || reply->error() != QNetworkReply::NoError) {
+            emit connectionError(reply->error());
             if (reply->error() == QNetworkReply::HostNotFoundError) {
                 setConnectionStatus(false);
             }

tado/tado.h

@@ -42,6 +42,7 @@ class Tado : public QObject
 {
     Q_OBJECT
 public:
+
     struct Token {
         QString accesToken;
         QString tokenType;
@@ -65,7 +66,6 @@ public:
       QString tadoMode;
     };
 
-
     struct ZoneState {
         bool connected;
         bool power;
@@ -131,6 +131,7 @@ signals:
     void zonesReceived(const QString &homeId, QList<Zone> zones);
     void zoneStateReceived(const QString &homeId,const QString &zoneId, ZoneState sate);
     void overlayReceived(const QString &homeId, const QString &zoneId, const Overlay &overlay);
+    void connectionError(QNetworkReply::NetworkError error);
 
 private slots:
     void onRefreshTimer();