refactor httprequest: emit single signals (#23039)

* refactor httprequest

* Trigger Build

selfdrive/ui/qt/api.cc

@@ -70,10 +70,14 @@ HttpRequest::HttpRequest(QObject *parent, bool create_jwt, int timeout) : create
   connect(networkTimer, &QTimer::timeout, this, &HttpRequest::requestTimeout);
 }
 
-bool HttpRequest::active() {
+bool HttpRequest::active() const {
   return reply != nullptr;
 }
 
+bool HttpRequest::timeout() const {
+  return reply && reply->error() == QNetworkReply::OperationCanceledError;
+}
+
 void HttpRequest::sendRequest(const QString &requestURL, const HttpRequest::Method method) {
   if (active()) {
     qDebug() << "HttpRequest is active";
@@ -110,29 +114,26 @@ void HttpRequest::requestTimeout() {
   reply->abort();
 }
 
-// This function should always emit something
 void HttpRequest::requestFinished() {
-  bool success = false;
+  networkTimer->stop();
-  if (reply->error() != QNetworkReply::OperationCanceledError) {
-    networkTimer->stop();
-    QString response = reply->readAll();
-
-    if (reply->error() == QNetworkReply::NoError) {
-      success = true;
-      emit receivedResponse(response);
-    } else {
-      emit failedResponse(reply->errorString());
 
+  if (reply->error() == QNetworkReply::NoError) {
+    emit requestDone(reply->readAll(), true);
+  } else {
+    QString error;
+    if (reply->error() == QNetworkReply::OperationCanceledError) {
+      networkAccessManager->clearAccessCache();
+      networkAccessManager->clearConnectionCache();
+      error = "Request timed out";
+    } else {
       if (reply->error() == QNetworkReply::ContentAccessDenied || reply->error() == QNetworkReply::AuthenticationRequiredError) {
         qWarning() << ">>  Unauthorized. Authenticate with tools/lib/auth.py  <<";
       }
+      error = reply->errorString();
     }
-  } else {
+    emit requestDone(error, false);
-    networkAccessManager->clearAccessCache();
-    networkAccessManager->clearConnectionCache();
-    emit timeoutResponse("timeout");
   }
-  emit requestDone(success);
+
   reply->deleteLater();
   reply = nullptr;
 }

selfdrive/ui/qt/api.h

@@ -27,7 +27,11 @@ public:
 
   explicit HttpRequest(QObject* parent, bool create_jwt = true, int timeout = 20000);
   void sendRequest(const QString &requestURL, const Method method = Method::GET);
-  bool active();
+  bool active() const;
+  bool timeout() const;
+
+signals:
+  void requestDone(const QString &response, bool success);
 
 protected:
   QNetworkReply *reply = nullptr;
@@ -40,10 +44,4 @@ private:
 private slots:
   void requestTimeout();
   void requestFinished();
-
-signals:
-  void requestDone(bool success);
-  void receivedResponse(const QString &response);
-  void failedResponse(const QString &errorString);
-  void timeoutResponse(const QString &errorString);
 };

selfdrive/ui/qt/maps/map_settings.cc

@@ -127,8 +127,7 @@ MapPanel::MapPanel(QWidget* parent) : QWidget(parent) {
     {
       QString url = CommaApi::BASE_URL + "/v1/navigation/" + *dongle_id + "/locations";
       RequestRepeater* repeater = new RequestRepeater(this, url, "ApiCache_NavDestinations", 30, true);
-      QObject::connect(repeater, &RequestRepeater::receivedResponse, this, &MapPanel::parseResponse);
+      QObject::connect(repeater, &RequestRepeater::requestDone, this, &MapPanel::parseResponse);
-      QObject::connect(repeater, &RequestRepeater::failedResponse, this, &MapPanel::failedResponse);
     }
 
     // Destination set while offline
@@ -137,8 +136,8 @@ MapPanel::MapPanel(QWidget* parent) : QWidget(parent) {
       RequestRepeater* repeater = new RequestRepeater(this, url, "", 10, true);
       HttpRequest* deleter = new HttpRequest(this);
 
-      QObject::connect(repeater, &RequestRepeater::receivedResponse, [=](QString resp) {
+      QObject::connect(repeater, &RequestRepeater::requestDone, [=](const QString &resp, bool success) {
-        if (resp != "null") {
+        if (success && resp != "null") {
           if (params.get("NavDestination").empty()) {
             qWarning() << "Setting NavDestination from /next" << resp;
             params.put("NavDestination", resp.toStdString());
@@ -183,7 +182,12 @@ void MapPanel::updateCurrentRoute() {
   current_widget->setVisible(dest.size() && !doc.isNull());
 }
 
-void MapPanel::parseResponse(const QString &response) {
+void MapPanel::parseResponse(const QString &response, bool success) {
+  if (!success) {
+    stack->setCurrentIndex(1);
+    return;
+  }
+
   QJsonDocument doc = QJsonDocument::fromJson(response.trimmed().toUtf8());
   if (doc.isNull()) {
     qDebug() << "JSON Parse failed on navigation locations";
@@ -283,10 +287,6 @@ void MapPanel::parseResponse(const QString &response) {
   repaint();
 }
 
-void MapPanel::failedResponse(const QString &response) {
-  stack->setCurrentIndex(1);
-}
-
 void MapPanel::navigateTo(const QJsonObject &place) {
   QJsonDocument doc(place);
   params.put("NavDestination", doc.toJson().toStdString());

selfdrive/ui/qt/maps/map_settings.h

@@ -17,8 +17,7 @@ public:
   explicit MapPanel(QWidget* parent = nullptr);
 
   void navigateTo(const QJsonObject &place);
-  void parseResponse(const QString &response);
+  void parseResponse(const QString &response, bool success);
-  void failedResponse(const QString &response);
   void updateCurrentRoute();
   void clear();
 

selfdrive/ui/qt/request_repeater.cc

@@ -15,10 +15,10 @@ RequestRepeater::RequestRepeater(QObject *parent, const QString &requestURL, con
   if (!cacheKey.isEmpty()) {
     prevResp = QString::fromStdString(params.get(cacheKey.toStdString()));
     if (!prevResp.isEmpty()) {
-      QTimer::singleShot(500, [=]() { emit receivedResponse(prevResp); });
+      QTimer::singleShot(500, [=]() { emit requestDone(prevResp, true); });
     }
-    QObject::connect(this, &HttpRequest::receivedResponse, [=](const QString &resp) {
+    QObject::connect(this, &HttpRequest::requestDone, [=](const QString &resp, bool success) {
-      if (resp != prevResp) {
+      if (success && resp != prevResp) {
         params.put(cacheKey.toStdString(), resp.toStdString());
         prevResp = resp;
       }

selfdrive/ui/qt/setup/setup.cc

@@ -169,7 +169,7 @@ QWidget * Setup::network_setup() {
 
   // setup timer for testing internet connection
   HttpRequest *request = new HttpRequest(this, false, 2500);
-  QObject::connect(request, &HttpRequest::requestDone, [=](bool success) {
+  QObject::connect(request, &HttpRequest::requestDone, [=](const QString &, bool success) {
     cont->setEnabled(success);
     if (success) {
       const bool cell = networking->wifi->currentNetworkType() == NetworkType::CELL;

selfdrive/ui/qt/widgets/drive_stats.cc

@@ -48,7 +48,7 @@ DriveStats::DriveStats(QWidget* parent) : QFrame(parent) {
   if (auto dongleId = getDongleId()) {
     QString url = CommaApi::BASE_URL + "/v1.1/devices/" + *dongleId + "/stats";
     RequestRepeater* repeater = new RequestRepeater(this, url, "ApiCache_DriveStats", 30);
-    QObject::connect(repeater, &RequestRepeater::receivedResponse, this, &DriveStats::parseResponse);
+    QObject::connect(repeater, &RequestRepeater::requestDone, this, &DriveStats::parseResponse);
   }
 
   setStyleSheet(R"(
@@ -76,7 +76,9 @@ void DriveStats::updateStats() {
   update(json["week"].toObject(), week_);
 }
 
-void DriveStats::parseResponse(const QString& response) {
+void DriveStats::parseResponse(const QString& response, bool success) {
+  if (!success) return;
+
   QJsonDocument doc = QJsonDocument::fromJson(response.trimmed().toUtf8());
   if (doc.isNull()) {
     qDebug() << "JSON Parse failed on getting past drives statistics";

selfdrive/ui/qt/widgets/drive_stats.h

@@ -21,5 +21,5 @@ private:
   } all_, week_;
 
 private slots:
-  void parseResponse(const QString &response);
+  void parseResponse(const QString &response, bool success);
 };

selfdrive/ui/qt/widgets/prime.cc

@@ -161,7 +161,7 @@ PrimeUserWidget::PrimeUserWidget(QWidget* parent) : QWidget(parent) {
   if (auto dongleId = getDongleId()) {
     QString url = CommaApi::BASE_URL + "/v1/devices/" + *dongleId + "/owner";
     RequestRepeater *repeater = new RequestRepeater(this, url, "ApiCache_Owner", 6);
-    QObject::connect(repeater, &RequestRepeater::receivedResponse, this, &PrimeUserWidget::replyFinished);
+    QObject::connect(repeater, &RequestRepeater::requestDone, this, &PrimeUserWidget::replyFinished);
   }
 }
 
@@ -291,14 +291,14 @@ SetupWidget::SetupWidget(QWidget* parent) : QFrame(parent) {
     QString url = CommaApi::BASE_URL + "/v1.1/devices/" + *dongleId + "/";
     RequestRepeater* repeater = new RequestRepeater(this, url, "ApiCache_Device", 5);
 
-    QObject::connect(repeater, &RequestRepeater::failedResponse, this, &SetupWidget::show);
+    QObject::connect(repeater, &RequestRepeater::requestDone, this, &SetupWidget::replyFinished);
-    QObject::connect(repeater, &RequestRepeater::receivedResponse, this, &SetupWidget::replyFinished);
   }
   hide(); // Only show when first request comes back
 }
 
-void SetupWidget::replyFinished(const QString &response) {
+void SetupWidget::replyFinished(const QString &response, bool success) {
   show();
+  if (!success) return;
 
   QJsonDocument doc = QJsonDocument::fromJson(response.toUtf8());
   if (doc.isNull()) {

selfdrive/ui/qt/widgets/prime.h

@@ -68,5 +68,5 @@ private:
   PrimeUserWidget *primeUser;
 
 private slots:
-  void replyFinished(const QString &response);
+  void replyFinished(const QString &response, bool success);
 };

selfdrive/ui/qt/widgets/ssh_keys.cc

@@ -41,23 +41,22 @@ void SshControl::refresh() {
 
 void SshControl::getUserKeys(const QString &username) {
   HttpRequest *request = new HttpRequest(this, false);
-  QObject::connect(request, &HttpRequest::receivedResponse, [=](const QString &resp) {
+  QObject::connect(request, &HttpRequest::requestDone, [=](const QString &resp, bool success) {
-    if (!resp.isEmpty()) {
+    if (success) {
-      params.put("GithubUsername", username.toStdString());
+      if (!resp.isEmpty()) {
-      params.put("GithubSshKeys", resp.toStdString());
+        params.put("GithubUsername", username.toStdString());
+        params.put("GithubSshKeys", resp.toStdString());
+      } else {
+        ConfirmationDialog::alert(QString("Username '%1' has no keys on GitHub").arg(username), this);
+      }
     } else {
-      ConfirmationDialog::alert(QString("Username '%1' has no keys on GitHub").arg(username), this);
+      if (request->timeout()) {
+        ConfirmationDialog::alert("Request timed out", this);
+      } else {
+        ConfirmationDialog::alert(QString("Username '%1' doesn't exist on GitHub").arg(username), this);
+      }
     }
-    refresh();
+
-    request->deleteLater();
-  });
-  QObject::connect(request, &HttpRequest::failedResponse, [=] {
-    ConfirmationDialog::alert(QString("Username '%1' doesn't exist on GitHub").arg(username), this);
-    refresh();
-    request->deleteLater();
-  });
-  QObject::connect(request, &HttpRequest::timeoutResponse, [=] {
-    ConfirmationDialog::alert("Request timed out", this);
     refresh();
     request->deleteLater();
   });

selfdrive/ui/replay/route.cc

@@ -35,10 +35,8 @@ bool Route::load() {
 bool Route::loadFromServer() {
   QEventLoop loop;
   HttpRequest http(nullptr, !Hardware::PC());
-  QObject::connect(&http, &HttpRequest::failedResponse, [&] { loop.exit(0); });
+  QObject::connect(&http, &HttpRequest::requestDone, [&](const QString &json, bool success) {
-  QObject::connect(&http, &HttpRequest::timeoutResponse, [&] { loop.exit(0); });
+    loop.exit(success ? loadFromJson(json) : 0);
-  QObject::connect(&http, &HttpRequest::receivedResponse, [&](const QString &json) {
-    loop.exit(loadFromJson(json));
   });
   http.sendRequest("https://api.commadotai.com/v1/route/" + route_.str + "/files");
   return loop.exec();