cabana: cleanp code (#30666)

old-commit-hash: aa744e8437

tools/cabana/dbc/dbc.h

@@ -1,12 +1,10 @@
 #pragma once
 
 #include <limits>
-#include <string>
 #include <utility>
 #include <vector>
 
 #include <QColor>
-#include <QList>
 #include <QMetaType>
 #include <QString>
 
@@ -47,7 +45,7 @@ struct std::hash<MessageId> {
   std::size_t operator()(const MessageId &k) const noexcept { return qHash(k); }
 };
 
-typedef QList<std::pair<double, QString>> ValueDescription;
+typedef std::vector<std::pair<double, QString>> ValueDescription;
 
 namespace cabana {
 

tools/cabana/dbc/dbcfile.cc

@@ -52,8 +52,7 @@ void DBCFile::cleanupAutoSaveFile() {
 bool DBCFile::writeContents(const QString &fn) {
   QFile file(fn);
   if (file.open(QIODevice::WriteOnly)) {
-    file.write(generateDBC().toUtf8());
-    return true;
+    return file.write(generateDBC().toUtf8()) >= 0;
   }
   return false;
 }
@@ -77,10 +76,6 @@ cabana::Msg *DBCFile::msg(const QString &name) {
   return it != msgs.end() ? &(it->second) : nullptr;
 }
 
-int DBCFile::signalCount() {
-  return std::accumulate(msgs.cbegin(), msgs.cend(), 0, [](int &n, const auto &m) { return n + m.second.sigs.size(); });
-}
-
 void DBCFile::parse(const QString &content) {
   static QRegularExpression bo_regexp(R"(^BO_ (\w+) (\w+) *: (\w+) (\w+))");
   static QRegularExpression sg_regexp(R"(^SG_ (\w+) : (\d+)\|(\d+)@(\d+)([\+|\-]) \(([0-9.+\-eE]+),([0-9.+\-eE]+)\) \[([0-9.+\-eE]+)\|([0-9.+\-eE]+)\] \"(.*)\" (.*))");
@@ -226,7 +221,7 @@ QString DBCFile::generateDBC() {
       if (!sig->comment.isEmpty()) {
         signal_comment += QString("CM_ SG_ %1 %2 \"%3\";\n").arg(address).arg(sig->name).arg(sig->comment);
       }
-      if (!sig->val_desc.isEmpty()) {
+      if (!sig->val_desc.empty()) {
         QStringList text;
         for (auto &[val, desc] : sig->val_desc) {
           text << QString("%1 \"%2\"").arg(val).arg(desc);

tools/cabana/dbc/dbcfile.h

@@ -27,10 +27,8 @@ public:
   cabana::Msg *msg(const QString &name);
   inline cabana::Msg *msg(const MessageId &id) { return msg(id.address); }
 
-  int signalCount();
-  inline int msgCount() { return msgs.size(); }
-  inline QString name() { return name_.isEmpty() ? "untitled" : name_; }
-  inline bool isEmpty() { return (signalCount() == 0) && name_.isEmpty(); }
+  inline QString name() const { return name_.isEmpty() ? "untitled" : name_; }
+  inline bool isEmpty() const { return msgs.empty() && name_.isEmpty(); }
 
   QString filename;
 

tools/cabana/dbc/dbcmanager.cc

@@ -106,12 +106,6 @@ QString DBCManager::newSignalName(const MessageId &id) {
   return m ? m->newSignalName() : "";
 }
 
-const std::vector<uint8_t> &DBCManager::mask(const MessageId &id) {
-  static std::vector<uint8_t> empty_mask;
-  auto m = msg(id);
-  return m ? m->mask : empty_mask;
-}
-
 const std::map<uint32_t, cabana::Msg> &DBCManager::getMessages(uint8_t source) {
   static std::map<uint32_t, cabana::Msg> empty_msgs;
   auto dbc_file = findDBCFile(source);
@@ -143,25 +137,6 @@ QStringList DBCManager::signalNames() {
   return ret;
 }
 
-int DBCManager::signalCount(const MessageId &id) {
-  auto m = msg(id);
-  return m ? m->sigs.size() : 0;
-}
-
-int DBCManager::signalCount() {
-  auto files = allDBCFiles();
-  return std::accumulate(files.cbegin(), files.cend(), 0, [](int &n, auto &f) { return n + f->signalCount(); });
-}
-
-int DBCManager::msgCount() {
-  auto files = allDBCFiles();
-  return std::accumulate(files.cbegin(), files.cend(), 0, [](int &n, auto &f) { return n + f->msgCount(); });
-}
-
-int DBCManager::dbcCount() {
-  return allDBCFiles().size();
-}
-
 int DBCManager::nonEmptyDBCCount() {
   auto files = allDBCFiles();
   return std::count_if(files.cbegin(), files.cend(), [](auto &f) { return !f->isEmpty(); });

tools/cabana/dbc/dbcmanager.h

@@ -4,7 +4,6 @@
 #include <memory>
 #include <map>
 #include <set>
-#include <vector>
 
 #include "tools/cabana/dbc/dbcfile.h"
 
@@ -34,17 +33,13 @@ public:
 
   QString newMsgName(const MessageId &id);
   QString newSignalName(const MessageId &id);
-  const std::vector<uint8_t>& mask(const MessageId &id);
 
   const std::map<uint32_t, cabana::Msg> &getMessages(uint8_t source);
   cabana::Msg *msg(const MessageId &id);
   cabana::Msg* msg(uint8_t source, const QString &name);
 
   QStringList signalNames();
-  int signalCount(const MessageId &id);
-  int signalCount();
-  int msgCount();
-  int dbcCount();
+  inline int dbcCount() { return allDBCFiles().size(); }
   int nonEmptyDBCCount();
 
   const SourceSet sources(const DBCFile *dbc_file) const;

tools/cabana/mainwin.cc

@@ -46,7 +46,6 @@ MainWindow::MainWindow() : QMainWindow() {
   static auto static_main_win = this;
   qRegisterMetaType<uint64_t>("uint64_t");
   qRegisterMetaType<SourceSet>("SourceSet");
-  qRegisterMetaType<ReplyMsgType>("ReplyMsgType");
   installDownloadProgressHandler([](uint64_t cur, uint64_t total, bool success) {
     emit static_main_win->updateProgressBar(cur, total, success);
   });
@@ -54,9 +53,7 @@ MainWindow::MainWindow() : QMainWindow() {
     if (type == QtDebugMsg) std::cout << msg.toStdString() << std::endl;
     emit static_main_win->showMessage(msg, 2000);
   });
-  installMessageHandler([](ReplyMsgType type, const std::string msg) {
-    qInfo() << QString::fromStdString(msg);
-  });
+  installMessageHandler([](ReplyMsgType type, const std::string msg) { qInfo() << msg.c_str(); });
 
   setStyleSheet(QString(R"(QMainWindow::separator {
     width: %1px; /* when vertical */
@@ -329,7 +326,7 @@ void MainWindow::loadFromClipboard(SourceSet s, bool close_all) {
   QString dbc_str = QGuiApplication::clipboard()->text();
   QString error;
   bool ret = dbc()->open(s, "", dbc_str, &error);
-  if (ret && dbc()->msgCount() > 0) {
+  if (ret && dbc()->nonEmptyDBCCount() > 0) {
     QMessageBox::information(this, tr("Load From Clipboard"), tr("DBC Successfully Loaded!"));
   } else {
     QMessageBox msg_box(QMessageBox::Warning, tr("Failed to load DBC from clipboard"), tr("Make sure that you paste the text with correct format."));
@@ -356,7 +353,7 @@ void MainWindow::streamStarted() {
     video_splitter->setSizes({1, 1});
   }
   // Don't overwrite already loaded DBC
-  if (!dbc()->msgCount()) {
+  if (!dbc()->nonEmptyDBCCount()) {
     newFile();
   }
 
@@ -371,7 +368,7 @@ void MainWindow::eventsMerged() {
                                     .arg(can->routeName())
                                     .arg(car_fingerprint.isEmpty() ? tr("Unknown Car") : car_fingerprint));
     // Don't overwrite already loaded DBC
-    if (!dbc()->msgCount() && !car_fingerprint.isEmpty()) {
+    if (!dbc()->nonEmptyDBCCount() && !car_fingerprint.isEmpty()) {
       auto dbc_name = fingerprint_to_dbc[car_fingerprint];
       if (dbc_name != QJsonValue::Undefined) {
         // Prevent dialog that load autosaved file from blocking replay->start().
@@ -529,7 +526,6 @@ void MainWindow::updateRecentFiles(const QString &fn) {
 
 void MainWindow::updateRecentFileActions() {
   int num_recent_files = std::min<int>(settings.recent_files.size(), MAX_RECENT_FILES);
-
   for (int i = 0; i < num_recent_files; ++i) {
     QString text = tr("&%1 %2").arg(i + 1).arg(QFileInfo(settings.recent_files[i]).fileName());
     recent_files_acts[i]->setText(text);
@@ -543,15 +539,11 @@ void MainWindow::updateRecentFileActions() {
 }
 
 void MainWindow::remindSaveChanges() {
-  bool discard_changes = false;
-  while (!UndoStack::instance()->isClean() && !discard_changes) {
+  while (!UndoStack::instance()->isClean()) {
     QString text = tr("You have unsaved changes. Press ok to save them, cancel to discard.");
-    int ret = (QMessageBox::question(this, tr("Unsaved Changes"), text, QMessageBox::Ok | QMessageBox::Cancel));
-    if (ret == QMessageBox::Ok) {
+    int ret = QMessageBox::question(this, tr("Unsaved Changes"), text, QMessageBox::Ok | QMessageBox::Cancel);
+    if (ret != QMessageBox::Ok) break;
     save();
-    } else {
-      discard_changes = true;
-    }
   }
   UndoStack::instance()->clear();
 }
@@ -660,7 +652,7 @@ HelpOverlay::HelpOverlay(MainWindow *parent) : QWidget(parent) {
 void HelpOverlay::paintEvent(QPaintEvent *event) {
   QPainter painter(this);
   painter.fillRect(rect(), QColor(0, 0, 0, 50));
-  MainWindow *parent = (MainWindow *)parentWidget();
+  auto parent = parentWidget();
   drawHelpForWidget(painter, parent->findChild<MessagesWidget *>());
   drawHelpForWidget(painter, parent->findChild<BinaryView *>());
   drawHelpForWidget(painter, parent->findChild<SignalView *>());

tools/cabana/mainwin.h

@@ -101,7 +101,6 @@ protected:
   int prev_undostack_index = 0;
   int prev_undostack_count = 0;
   QByteArray default_state;
-  friend class OnlineHelp;
 };
 
 class HelpOverlay : public QWidget {

tools/cabana/tests/test_cabana.cc

@@ -3,9 +3,7 @@
 #include <QDir>
 
 #include "catch2/catch.hpp"
-#include "tools/replay/logreader.h"
 #include "tools/cabana/dbc/dbcmanager.h"
-#include "tools/cabana/streams/abstractstream.h"
 
 const std::string TEST_RLOG_URL = "https://commadataci.blob.core.windows.net/openpilotci/0c94aa1e1296d7c6/2021-05-05--19-48-37/0/rlog.bz2";
 
@@ -14,7 +12,7 @@ TEST_CASE("DBCFile::generateDBC") {
   DBCFile dbc_origin(fn);
   DBCFile dbc_from_generated("", dbc_origin.generateDBC());
 
-  REQUIRE(dbc_origin.msgCount() == dbc_from_generated.msgCount());
+  REQUIRE(dbc_origin.getMessages().size() == dbc_from_generated.getMessages().size());
   auto &msgs = dbc_origin.getMessages();
   auto &new_msgs = dbc_from_generated.getMessages();
   for (auto &[id, m] : msgs) {

tools/cabana/util.cc

@@ -12,7 +12,6 @@
 #include <QDateTime>
 #include <QFontDatabase>
 #include <QLocale>
-#include <QPainter>
 #include <QPixmapCache>
 
 #include "selfdrive/ui/qt/util.h"