cleanup code

Author: deanlee

tools/cabana/binaryview.cc

@@ -289,33 +289,29 @@ void BinaryViewModel::updateState() {
     items.resize(row_count * column_count);
     endInsertRows();
   }
+
+  double max_f = 255.0;
+  double factor = 0.25;
+  double scaler = max_f / log2(1.0 + factor);
   char hex[3] = {'\0'};
   for (int i = 0; i < binary.size(); ++i) {
     for (int j = 0; j < 8; ++j) {
-      items[i * column_count + j].val = ((binary[i] >> (7 - j)) & 1) != 0 ? '1' : '0';
-
+      auto &item = items[i * column_count + j];
+      item.val = ((binary[i] >> (7 - j)) & 1) != 0 ? '1' : '0';
       // Bit update frequency based highlighting
-      bool has_signal = items[i * column_count + j].sigs.size() > 0;
-      double offset = has_signal ? 50 : 0;
-
-      double min_f = last_msg.bit_change_counts[i][7 - j] == 0 ? offset : offset + 25;
-      double max_f = 255.0;
-
-      double factor = 0.25;
-      double scaler = max_f / log2(1.0 + factor);
-
-      double alpha = std::clamp(offset + log2(1.0 + factor * (double)last_msg.bit_change_counts[i][7 - j] / (double)last_msg.count) * scaler, min_f, max_f);
-      items[i * column_count + j].bg_color.setAlpha(alpha);
+      double offset = !item.sigs.empty() ? 50 : 0;
+      auto n = last_msg.bit_change_counts[i][7 - j];
+      double min_f = n == 0 ? offset : offset + 25;
+      double alpha = std::clamp(offset + log2(1.0 + factor * (double)n / (double)last_msg.count) * scaler, min_f, max_f);
+      item.bg_color.setAlpha(alpha);
     }
     hex[0] = toHex(binary[i] >> 4);
     hex[1] = toHex(binary[i] & 0xf);
     items[i * column_count + 8].val = hex;
     items[i * column_count + 8].bg_color = last_msg.colors[i];
   }
-  for (int i = binary.size(); i < row_count; ++i) {
-    for (int j = 0; j < column_count; ++j) {
-      items[i * column_count + j].val = "-";
-    }
+  for (int i = binary.size() * column_count; i < items.size(); ++i) {
+    items[i].val = "-";
   }
 
   for (int i = 0; i < items.size(); ++i) {

tools/cabana/chartswidget.cc

@@ -11,7 +11,6 @@
 #include <QRubberBand>
 #include <QPushButton>
 #include <QToolBar>
-#include <QToolButton>
 #include <QToolTip>
 #include <QtConcurrent>
 
@@ -333,18 +332,12 @@ ChartView::ChartView(QWidget *parent) : QChartView(nullptr, parent) {
   move_icon = new QGraphicsPixmapItem(utils::icon("grip-horizontal"), chart);
   move_icon->setToolTip(tr("Drag and drop to combine charts"));
 
-  QToolButton *remove_btn = new QToolButton();
-  remove_btn->setIcon(utils::icon("x"));
-  remove_btn->setAutoRaise(true);
-  remove_btn->setToolTip(tr("Remove Chart"));
+  QToolButton *remove_btn = toolButton("x", tr("Remove Chart"));
   close_btn_proxy = new QGraphicsProxyWidget(chart);
   close_btn_proxy->setWidget(remove_btn);
   close_btn_proxy->setZValue(chart->zValue() + 11);
 
-  QToolButton *manage_btn = new QToolButton();
-  manage_btn->setToolButtonStyle(Qt::ToolButtonIconOnly);
-  manage_btn->setIcon(utils::icon("list"));
-  manage_btn->setAutoRaise(true);
+  QToolButton *manage_btn = toolButton("list", "");
   QMenu *menu = new QMenu(this);
   line_series_action = menu->addAction(tr("Line"), [this]() { setSeriesType(QAbstractSeries::SeriesTypeLine); });
   line_series_action->setCheckable(true);

tools/cabana/detailwidget.cc

@@ -3,7 +3,6 @@
 #include <QFormLayout>
 #include <QMenu>
 #include <QMessageBox>
-#include <QToolButton>
 
 #include "tools/cabana/commands.h"
 
@@ -22,22 +21,22 @@ DetailWidget::DetailWidget(ChartsWidget *charts, QWidget *parent) : charts(chart
   main_layout->addWidget(tabbar);
 
   // message title
-  QToolBar *toolbar = new QToolBar(this);
-  toolbar->setIconSize({16, 16});
+  QHBoxLayout *title_layout = new QHBoxLayout();
+  title_layout->setContentsMargins(0, 6, 0, 0);
   time_label = new QLabel(this);
   time_label->setToolTip(tr("Current time"));
   time_label->setStyleSheet("QLabel{font-weight:bold;}");
-  toolbar->addWidget(time_label);
+  title_layout->addWidget(time_label);
   name_label = new ElidedLabel(this);
-  name_label->setContentsMargins(5, 0, 5, 0);
   name_label->setStyleSheet("QLabel{font-weight:bold;}");
   name_label->setAlignment(Qt::AlignCenter);
   name_label->setSizePolicy(QSizePolicy::Expanding, QSizePolicy::Preferred);
-  toolbar->addWidget(name_label);
-  toolbar->addAction(utils::icon("pencil"), "", this, &DetailWidget::editMsg)->setToolTip(tr("Edit Message"));
-  remove_msg_act = toolbar->addAction(utils::icon("x-lg"), "", this, &DetailWidget::removeMsg);
-  remove_msg_act->setToolTip(tr("Remove Message"));
-  main_layout->addWidget(toolbar);
+  title_layout->addWidget(name_label);
+  auto edit_btn = toolButton("pencil", tr("Edit Message"));
+  title_layout->addWidget(edit_btn);
+  remove_btn = toolButton("x-lg", tr("Remove Message"));
+  title_layout->addWidget(remove_btn);
+  main_layout->addLayout(title_layout);
 
   // warning
   warning_widget = new QWidget(this);
@@ -64,6 +63,8 @@ DetailWidget::DetailWidget(ChartsWidget *charts, QWidget *parent) : charts(chart
   tab_widget->addTab(history_log = new LogsWidget(this), utils::icon("stopwatch"), "&Logs");
   main_layout->addWidget(tab_widget);
 
+  QObject::connect(edit_btn, &QToolButton::clicked, this, &DetailWidget::editMsg);
+  QObject::connect(remove_btn, &QToolButton::clicked, this, &DetailWidget::removeMsg);
   QObject::connect(binary_view, &BinaryView::resizeSignal, signal_view->model, &SignalModel::resizeSignal);
   QObject::connect(binary_view, &BinaryView::addSignal, signal_view->model, &SignalModel::addSignal);
   QObject::connect(binary_view, &BinaryView::signalHovered, signal_view, &SignalView::signalHovered);
@@ -83,9 +84,7 @@ DetailWidget::DetailWidget(ChartsWidget *charts, QWidget *parent) : charts(chart
       setMessage(tabbar->tabData(index).value<MessageId>());
     }
   });
-  QObject::connect(tabbar, &QTabBar::tabCloseRequested, [this](int index) {
-    tabbar->removeTab(index);
-  });
+  QObject::connect(tabbar, &QTabBar::tabCloseRequested, tabbar, &QTabBar::removeTab);
   QObject::connect(charts, &ChartsWidget::seriesChanged, signal_view, &SignalView::updateChartState);
 }
 
@@ -141,7 +140,7 @@ void DetailWidget::refresh() {
   } else {
     warnings.push_back(tr("Drag-Select in binary view to create new signal."));
   }
-  remove_msg_act->setEnabled(msg != nullptr);
+  remove_btn->setEnabled(msg != nullptr);
   name_label->setText(msgName(msg_id));
 
   if (!warnings.isEmpty()) {

tools/cabana/detailwidget.h

@@ -3,7 +3,6 @@
 #include <QDialogButtonBox>
 #include <QSplitter>
 #include <QTabWidget>
-#include <QToolBar>
 
 #include "selfdrive/ui/qt/widgets/controls.h"
 #include "tools/cabana/binaryview.h"
@@ -44,7 +43,7 @@ class DetailWidget : public QWidget {
   QWidget *warning_widget;
   QTabBar *tabbar;
   QTabWidget *tab_widget;
-  QAction *remove_msg_act;
+  QToolButton *remove_btn;
   LogsWidget *history_log;
   BinaryView *binary_view;
   SignalView *signal_view;

tools/cabana/signaledit.cc

@@ -6,7 +6,6 @@
 #include <QHeaderView>
 #include <QMessageBox>
 #include <QPushButton>
-#include <QToolButton>
 #include <QVBoxLayout>
 
 #include "tools/cabana/commands.h"
@@ -93,9 +92,8 @@ Qt::ItemFlags SignalModel::flags(const QModelIndex &index) const {
 }
 
 int SignalModel::signalRow(const Signal *sig) const {
-  auto &children = root->children;
-  for (int i = 0; i < children.size(); ++i) {
-    if (children[i]->sig == sig) return i;
+  for (int i = 0; i < root->children.size(); ++i) {
+    if (root->children[i]->sig == sig) return i;
   }
   return -1;
 }
@@ -367,11 +365,8 @@ SignalView::SignalView(ChartsWidget *charts, QWidget *parent) : charts(charts),
   filter_edit->setPlaceholderText(tr("filter signals"));
   hl->addWidget(filter_edit);
   hl->addStretch(1);
-  auto collapse_btn = new QToolButton();
-  collapse_btn->setIcon(utils::icon("dash-square"));
+  auto collapse_btn = toolButton("dash-square", tr("Collapse All"));
   collapse_btn->setIconSize({12, 12});
-  collapse_btn->setAutoRaise(true);
-  collapse_btn->setToolTip(tr("Collapse All"));
   hl->addWidget(collapse_btn);
 
   // tree view
@@ -416,16 +411,6 @@ void SignalView::setMessage(const MessageId &id) {
 }
 
 void SignalView::rowsChanged() {
-  auto create_btn = [](const QString &id, const QString &tooltip) {
-    auto btn = new QToolButton();
-    btn->setIcon(utils::icon(id));
-    btn->setToolTip(tooltip);
-    btn->setAutoRaise(true);
-    return btn;
-  };
-
-  signal_count_lb->setText(tr("Signals: %1").arg(model->rowCount()));
-
   for (int i = 0; i < model->rowCount(); ++i) {
     auto index = model->index(i, 1);
     if (!tree->indexWidget(index)) {
@@ -434,8 +419,8 @@ void SignalView::rowsChanged() {
       h->setContentsMargins(0, 2, 0, 2);
       h->addStretch(1);
 
-      auto remove_btn = create_btn("x", tr("Remove signal"));
-      auto plot_btn = create_btn("graph-up", "");
+      auto remove_btn = toolButton("x", tr("Remove signal"));
+      auto plot_btn = toolButton("graph-up", "");
       plot_btn->setCheckable(true);
       h->addWidget(plot_btn);
       h->addWidget(remove_btn);
@@ -448,6 +433,7 @@ void SignalView::rowsChanged() {
       });
     }
   }
+  signal_count_lb->setText(tr("Signals: %1").arg(model->rowCount()));
   updateChartState();
 }
 
@@ -475,10 +461,12 @@ void SignalView::selectSignal(const Signal *sig, bool expand) {
 void SignalView::updateChartState() {
   int i = 0;
   for (auto item : model->root->children) {
-    auto plot_btn = tree->indexWidget(model->index(i, 1))->findChildren<QToolButton *>()[0];
     bool chart_opened = charts->hasSignal(msg_id, item->sig);
-    plot_btn->setChecked(chart_opened);
-    plot_btn->setToolTip(chart_opened ? tr("Close Plot") : tr("Show Plot\nSHIFT click to add to previous opened plot"));
+    auto buttons = tree->indexWidget(model->index(i, 1))->findChildren<QToolButton *>();
+    if (buttons.size() > 0) {
+      buttons[0]->setChecked(chart_opened);
+      buttons[0]->setToolTip(chart_opened ? tr("Close Plot") : tr("Show Plot\nSHIFT click to add to previous opened plot"));
+    }
     ++i;
   }
 }

tools/cabana/streams/abstractstream.cc

@@ -31,10 +31,12 @@ bool AbstractStream::updateEvent(const Event *event) {
       data.dat = QByteArray((char *)c.getDat().begin(), c.getDat().size());
       data.count = ++counters[id];
       data.freq = data.count / std::max(1.0, current_sec);
-      change_trackers[id].compute(data.dat, data.ts, data.freq);
-      data.colors = change_trackers[id].colors;
-      data.last_change_t = change_trackers[id].last_change_t;
-      data.bit_change_counts = change_trackers[id].bit_change_counts;
+
+      auto &tracker = change_trackers[id];
+      tracker.compute(data.dat, data.ts, data.freq);
+      data.colors = tracker.colors;
+      data.last_change_t = tracker.last_change_t;
+      data.bit_change_counts = tracker.bit_change_counts;
     }
 
     double ts = millis_since_boot();

tools/cabana/util.cc

@@ -130,3 +130,11 @@ QPixmap icon(const QString &id) {
   return pm;
 }
 }  // namespace utils
+
+QToolButton *toolButton(const QString &icon, const QString &tooltip) {
+  auto btn = new QToolButton();
+  btn->setIcon(utils::icon(icon));
+  btn->setToolTip(tooltip);
+  btn->setAutoRaise(true);
+  return btn;
+};

tools/cabana/util.h

@@ -8,6 +8,7 @@
 #include <QRegExpValidator>
 #include <QStringBuilder>
 #include <QStyledItemDelegate>
+#include <QToolButton>
 #include <QVector>
 
 #include "tools/cabana/dbcmanager.h"
@@ -53,3 +54,5 @@ class NameValidator : public QRegExpValidator {
 namespace utils {
 QPixmap icon(const QString &id);
 }
+
+QToolButton *toolButton(const QString &icon, const QString &tooltip);