Cabana: complete edit functions (#26097)

complete forms
old-commit-hash: e25ea8529698053f4c1b915b7e014ebdc499e56a

tools/cabana/chartswidget.cc

@@ -1,7 +1,6 @@
 #include "tools/cabana/chartswidget.h"
 
 #include <QGraphicsLayout>
-#include <QLabel>
 #include <QRubberBand>
 #include <QtCharts/QLineSeries>
 #include <QtCharts/QValueAxis>
@@ -14,6 +13,7 @@ ChartsWidget::ChartsWidget(QWidget *parent) : QWidget(parent) {
 
   // title bar
   title_bar = new QWidget(this);
+  title_bar->setVisible(false);
   QHBoxLayout *title_layout = new QHBoxLayout(title_bar);
   title_layout->setContentsMargins(0, 0, 0, 0);
   title_label = new QLabel(tr("Charts"));
@@ -25,13 +25,11 @@ ChartsWidget::ChartsWidget(QWidget *parent) : QWidget(parent) {
   title_layout->addWidget(range_label);
 
   reset_zoom_btn = new QPushButton("⟲", this);
-  reset_zoom_btn->setVisible(false);
   reset_zoom_btn->setFixedSize(30, 30);
   reset_zoom_btn->setToolTip(tr("Reset zoom (drag on chart to zoom X-Axis)"));
   title_layout->addWidget(reset_zoom_btn);
 
   remove_all_btn = new QPushButton("✖", this);
-  remove_all_btn->setVisible(false);
   remove_all_btn->setToolTip(tr("Remove all charts"));
   remove_all_btn->setFixedSize(30, 30);
   title_layout->addWidget(remove_all_btn);
@@ -56,10 +54,20 @@ ChartsWidget::ChartsWidget(QWidget *parent) : QWidget(parent) {
 
   main_layout->addWidget(charts_scroll);
 
-  updateTitleBar();
-
-  QObject::connect(dbc(), &DBCManager::signalRemoved, this, &ChartsWidget::removeChart);
   QObject::connect(dbc(), &DBCManager::DBCFileChanged, this, &ChartsWidget::removeAll);
+  QObject::connect(dbc(), &DBCManager::signalRemoved, this, &ChartsWidget::removeChart);
+  QObject::connect(dbc(), &DBCManager::signalUpdated, [this](const Signal *sig) {
+    if (auto it = charts.find(sig); it != charts.end()) {
+      it.value()->chart_view->updateSeries();
+    }
+  });
+  QObject::connect(dbc(), &DBCManager::msgUpdated, [this](const QString &id) {
+    for (auto chart : charts) {
+      if (chart->id == id)
+        chart->updateTitle();
+    }
+  });
+
   QObject::connect(can, &CANMessages::rangeChanged, [this]() { updateTitleBar(); });
   QObject::connect(reset_zoom_btn, &QPushButton::clicked, can, &CANMessages::resetRange);
   QObject::connect(remove_all_btn, &QPushButton::clicked, this, &ChartsWidget::removeAll);
@@ -71,54 +79,43 @@ ChartsWidget::ChartsWidget(QWidget *parent) : QWidget(parent) {
 }
 
 void ChartsWidget::updateTitleBar() {
-  if (!charts.size()) {
-    title_bar->setVisible(false);
-    return;
-  }
-
-  title_label->setText(tr("Charts (%1)").arg(charts.size()));
+  title_bar->setVisible(!charts.isEmpty());
+  if (charts.isEmpty()) return;
 
   // show select range
+  range_label->setVisible(can->isZoomed());
+  reset_zoom_btn->setEnabled(can->isZoomed());
   if (can->isZoomed()) {
     auto [min, max] = can->range();
     range_label->setText(tr("%1 - %2").arg(min, 0, 'f', 2).arg(max, 0, 'f', 2));
-    range_label->setVisible(true);
-    reset_zoom_btn->setEnabled(true);
-  } else {
-    reset_zoom_btn->setEnabled(false);
-    range_label->setVisible(false);
   }
 
+  title_label->setText(tr("Charts (%1)").arg(charts.size()));
   dock_btn->setText(docking ? "⬈" : "⬋");
   dock_btn->setToolTip(docking ? tr("Undock charts") : tr("Dock charts"));
-  remove_all_btn->setVisible(!charts.empty());
-  reset_zoom_btn->setVisible(!charts.empty());
-  title_bar->setVisible(true);
 }
 
-void ChartsWidget::addChart(const QString &id, const QString &sig_name) {
-  const QString char_name = id + ":" + sig_name;
-  if (charts.find(char_name) == charts.end()) {
-    auto chart = new ChartWidget(id, sig_name, this);
-    QObject::connect(chart, &ChartWidget::remove, [=]() {
-      removeChart(id, sig_name);
-    });
+void ChartsWidget::addChart(const QString &id, const Signal *sig) {
+  if (!charts.contains(sig)) {
+    auto chart = new ChartWidget(id, sig, this);
+    QObject::connect(chart, &ChartWidget::remove, [=]() { removeChart(sig); });
     charts_layout->insertWidget(0, chart);
-    charts[char_name] = chart;
+    charts.insert(sig, chart);
   }
   updateTitleBar();
 }
 
-void ChartsWidget::removeChart(const QString &id, const QString &sig_name) {
-  if (auto it = charts.find(id + ":" + sig_name); it != charts.end()) {
-    it->second->deleteLater();
-    charts.erase(it);
+void ChartsWidget::removeChart(const Signal *sig) {
+  auto it = charts.find(sig);
+  if (it != charts.end()) {
+    it.value()->deleteLater();
+    charts.remove(sig);
   }
   updateTitleBar();
 }
 
 void ChartsWidget::removeAll() {
-  for (auto [_, chart] : charts)
+  for (auto chart : charts)
     chart->deleteLater();
   charts.clear();
   updateTitleBar();
@@ -134,19 +131,16 @@ bool ChartsWidget::eventFilter(QObject *obj, QEvent *event) {
 
 // ChartWidget
 
-ChartWidget::ChartWidget(const QString &id, const QString &sig_name, QWidget *parent) : id(id), sig_name(sig_name), QWidget(parent) {
+ChartWidget::ChartWidget(const QString &id, const Signal *sig, QWidget *parent) : id(id), signal(sig), QWidget(parent) {
   QVBoxLayout *main_layout = new QVBoxLayout(this);
-
-  QWidget *chart_widget = new QWidget(this);
-  QVBoxLayout *chart_layout = new QVBoxLayout(chart_widget);
-  chart_layout->setSpacing(0);
-  chart_layout->setContentsMargins(0, 0, 0, 0);
+  main_layout->setSpacing(0);
+  main_layout->setContentsMargins(0, 0, 0, 0);
 
   QWidget *header = new QWidget(this);
   header->setStyleSheet("background-color:white");
   QHBoxLayout *header_layout = new QHBoxLayout(header);
   header_layout->setContentsMargins(11, 11, 11, 0);
-  QLabel *title = new QLabel(tr("%1 %2").arg(dbc()->msg(id)->name.c_str()).arg(id));
+  title = new QLabel(tr("%1 %2").arg(dbc()->msg(id)->name.c_str()).arg(id));
   header_layout->addWidget(title);
   header_layout->addStretch();
 
@@ -155,26 +149,28 @@ ChartWidget::ChartWidget(const QString &id, const QString &sig_name, QWidget *pa
   remove_btn->setToolTip(tr("Remove chart"));
   QObject::connect(remove_btn, &QPushButton::clicked, this, &ChartWidget::remove);
   header_layout->addWidget(remove_btn);
-  chart_layout->addWidget(header);
+  main_layout->addWidget(header);
 
-  chart_view = new ChartView(id, sig_name, this);
+  chart_view = new ChartView(id, sig, this);
   chart_view->setFixedHeight(300);
-  chart_layout->addWidget(chart_view);
-  chart_layout->addStretch();
-
-  main_layout->addWidget(chart_widget);
+  main_layout->addWidget(chart_view);
+  main_layout->addStretch();
 
   setSizePolicy(QSizePolicy::Expanding, QSizePolicy::Fixed);
 }
 
+void ChartWidget::updateTitle() {
+  title->setText(tr("%1 %2").arg(dbc()->msg(id)->name.c_str()).arg(id));
+}
+
 // ChartView
 
-ChartView::ChartView(const QString &id, const QString &sig_name, QWidget *parent)
-    : id(id), sig_name(sig_name), QChartView(nullptr, parent) {
+ChartView::ChartView(const QString &id, const Signal *sig, QWidget *parent)
+    : id(id), signal(sig), QChartView(nullptr, parent) {
   QLineSeries *series = new QLineSeries();
   series->setUseOpenGL(true);
   QChart *chart = new QChart();
-  chart->setTitle(sig_name);
+  chart->setTitle(sig->name.c_str());
   chart->addSeries(series);
   chart->createDefaultAxes();
   chart->legend()->hide();
@@ -205,10 +201,7 @@ ChartView::ChartView(const QString &id, const QString &sig_name, QWidget *parent
   QObject::connect(can, &CANMessages::rangeChanged, this, &ChartView::rangeChanged);
   QObject::connect(can, &CANMessages::eventsMerged, this, &ChartView::updateSeries);
   QObject::connect(dynamic_cast<QValueAxis *>(chart->axisX()), &QValueAxis::rangeChanged, can, &CANMessages::setRange);
-  QObject::connect(dbc(), &DBCManager::signalUpdated, [this](const QString &msg_id, const QString &sig_name) {
-    if (this->id == msg_id && this->sig_name == sig_name)
-      updateSeries();
-  });
+
   updateSeries();
 }
 
@@ -219,9 +212,9 @@ void ChartView::updateState() {
 }
 
 void ChartView::updateSeries() {
-  const Signal *sig = dbc()->signal(id, sig_name);
+  chart()->setTitle(signal->name.c_str());
   auto events = can->events();
-  if (!sig || !events) return;
+  if (!events) return;
 
   auto l = id.split(':');
   int bus = l[0].toInt();
@@ -235,7 +228,7 @@ void ChartView::updateSeries() {
       for (auto c : evt->event.getCan()) {
         if (bus == c.getSrc() && address == c.getAddress()) {
           auto dat = c.getDat();
-          double value = get_raw_value((uint8_t *)dat.begin(), dat.size(), *sig);
+          double value = get_raw_value((uint8_t *)dat.begin(), dat.size(), *signal);
           double ts = (evt->mono_time / (double)1e9) - route_start_time;  // seconds
           vals.push_back({ts, value});
         }

tools/cabana/chartswidget.h

@@ -9,7 +9,6 @@
 #include <QVBoxLayout>
 #include <QWidget>
 #include <QtCharts/QChartView>
-#include <QtCharts/QLineSeries>
 
 #include "tools/cabana/canmessages.h"
 #include "tools/cabana/dbcmanager.h"
@@ -20,7 +19,8 @@ class ChartView : public QChartView {
   Q_OBJECT
 
 public:
-  ChartView(const QString &id, const QString &sig_name, QWidget *parent = nullptr);
+  ChartView(const QString &id, const Signal *sig, QWidget *parent = nullptr);
+  void updateSeries();
 
 private:
   void mouseReleaseEvent(QMouseEvent *event) override;
@@ -28,7 +28,6 @@ private:
   void enterEvent(QEvent *event) override;
   void leaveEvent(QEvent *event) override;
 
-  void updateSeries();
   void rangeChanged(qreal min, qreal max);
   void updateAxisY();
   void updateState();
@@ -38,22 +37,23 @@ private:
   QGraphicsLineItem *line_marker;
   QList<QPointF> vals;
   QString id;
-  QString sig_name;
+  const Signal *signal;
 };
 
 class ChartWidget : public QWidget {
 Q_OBJECT
 
 public:
-  ChartWidget(const QString &id, const QString &sig_name, QWidget *parent);
-  inline QChart *chart() const { return chart_view->chart(); }
+  ChartWidget(const QString &id, const Signal *sig, QWidget *parent);
+  void updateTitle();
 
 signals:
   void remove();
 
-protected:
+public:
   QString id;
-  QString sig_name;
+  const Signal *signal;
+  QLabel *title;
   ChartView *chart_view = nullptr;
 };
 
@@ -62,11 +62,8 @@ class ChartsWidget : public QWidget {
 
 public:
   ChartsWidget(QWidget *parent = nullptr);
-  void addChart(const QString &id, const QString &sig_name);
-  void removeChart(const QString &id, const QString &sig_name);
-  inline bool hasChart(const QString &id, const QString &sig_name) {
-    return charts.find(id + sig_name) != charts.end();
-  }
+  void addChart(const QString &id, const Signal *sig);
+  void removeChart(const Signal *sig);
 
 signals:
   void dock(bool floating);
@@ -85,5 +82,5 @@ private:
   QPushButton *reset_zoom_btn;
   QPushButton *remove_all_btn;
   QVBoxLayout *charts_layout;
-  std::map<QString, ChartWidget *> charts;
+  QHash<const Signal *, ChartWidget *> charts;
 };

tools/cabana/dbcmanager.cc

@@ -36,14 +36,17 @@ void DBCManager::updateMsg(const QString &id, const QString &name, uint32_t size
 void DBCManager::addSignal(const QString &id, const Signal &sig) {
   if (Msg *m = const_cast<Msg *>(msg(id))) {
     m->sigs.push_back(sig);
-    emit signalAdded(id, QString::fromStdString(sig.name));
+    emit signalAdded(&m->sigs.back());
   }
 }
 
 void DBCManager::updateSignal(const QString &id, const QString &sig_name, const Signal &sig) {
-  if (Signal *s = const_cast<Signal *>(signal(id, sig_name))) {
-    *s = sig;
-    emit signalUpdated(id, sig_name);
+  if (Msg *m = const_cast<Msg *>(msg(id))) {
+    auto it = std::find_if(m->sigs.begin(), m->sigs.end(), [=](auto &sig) { return sig_name == sig.name.c_str(); });
+    if (it != m->sigs.end()) {
+      *it = sig;
+      emit signalUpdated(&(*it));
+    }
   }
 }
 
@@ -51,21 +54,12 @@ void DBCManager::removeSignal(const QString &id, const QString &sig_name) {
   if (Msg *m = const_cast<Msg *>(msg(id))) {
     auto it = std::find_if(m->sigs.begin(), m->sigs.end(), [=](auto &sig) { return sig_name == sig.name.c_str(); });
     if (it != m->sigs.end()) {
+      emit signalRemoved(&(*it));
       m->sigs.erase(it);
-      emit signalRemoved(id, sig_name);
     }
   }
 }
 
-const Signal *DBCManager::signal(const QString &id, const QString &sig_name) const {
-  if (auto m = msg(id)) {
-    auto it = std::find_if(m->sigs.begin(), m->sigs.end(), [&](auto &s) { return sig_name == s.name.c_str(); });
-    if (it != m->sigs.end())
-      return &(*it);
-  }
-  return nullptr;
-}
-
 uint32_t DBCManager::addressFromId(const QString &id) {
   return id.mid(id.indexOf(':') + 1).toUInt(nullptr, 16);
 }

tools/cabana/dbcmanager.h

@@ -14,7 +14,6 @@ public:
   void open(const QString &dbc_file_name);
   void save(const QString &dbc_file_name);
 
-  const Signal *signal(const QString &id, const QString &sig_name) const;
   void addSignal(const QString &id, const Signal &sig);
   void updateSignal(const QString &id, const QString &sig_name, const Signal &sig);
   void removeSignal(const QString &id, const QString &sig_name);
@@ -31,9 +30,9 @@ public:
   }
 
 signals:
-  void signalAdded(const QString &id, const QString &sig_name);
-  void signalRemoved(const QString &id, const QString &sig_name);
-  void signalUpdated(const QString &id, const QString &sig_name);
+  void signalAdded(const Signal *sig);
+  void signalRemoved(const Signal *sig);
+  void signalUpdated(const Signal *sig);
   void msgUpdated(const QString &id);
   void DBCFileChanged();
 

tools/cabana/detailwidget.cc

@@ -3,9 +3,9 @@
 #include <QDialogButtonBox>
 #include <QFormLayout>
 #include <QHeaderView>
+#include <QMessageBox>
 #include <QScrollBar>
 #include <QTimer>
-#include <QVBoxLayout>
 
 // DetailWidget
 
@@ -32,60 +32,52 @@ DetailWidget::DetailWidget(QWidget *parent) : QWidget(parent) {
   binary_view = new BinaryView(this);
   main_layout->addWidget(binary_view, 0, Qt::AlignTop);
 
-  // signal header
-  signals_header = new QWidget(this);
-  QHBoxLayout *signals_header_layout = new QHBoxLayout(signals_header);
-  signals_header_layout->addWidget(new QLabel(tr("Signals")));
-  signals_header_layout->addStretch();
-  QPushButton *add_sig_btn = new QPushButton(tr("Add signal"), this);
-  signals_header_layout->addWidget(add_sig_btn);
-  signals_header->setVisible(false);
-  main_layout->addWidget(signals_header);
+  // signals
+  signals_container = new QWidget(this);
+  signals_container->setLayout(new QVBoxLayout);
+  signals_container->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Maximum);
 
-  // scroll area
   scroll = new ScrollArea(this);
-  QWidget *container = new QWidget(this);
-  container->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Maximum);
-  QVBoxLayout *container_layout = new QVBoxLayout(container);
-  signal_edit_layout = new QVBoxLayout();
-  signal_edit_layout->setSpacing(2);
-  container_layout->addLayout(signal_edit_layout);
-
-  scroll->setWidget(container);
+  scroll->setWidget(signals_container);
   scroll->setWidgetResizable(true);
   scroll->setHorizontalScrollBarPolicy(Qt::ScrollBarAlwaysOff);
   main_layout->addWidget(scroll);
 
+  // history log
   history_log = new HistoryLog(this);
   main_layout->addWidget(history_log);
 
-  QObject::connect(add_sig_btn, &QPushButton::clicked, this, &DetailWidget::addSignal);
   QObject::connect(edit_btn, &QPushButton::clicked, this, &DetailWidget::editMsg);
+  QObject::connect(binary_view, &BinaryView::cellsSelected, this, &DetailWidget::addSignal);
   QObject::connect(can, &CANMessages::updated, this, &DetailWidget::updateState);
+  QObject::connect(dbc(), &DBCManager::DBCFileChanged, this, &DetailWidget::dbcMsgChanged);
 }
 
 void DetailWidget::setMessage(const QString &message_id) {
-  msg_id = message_id;
-  for (auto f : signal_forms) {
-    f->deleteLater();
+  if (msg_id != message_id) {
+    msg_id = message_id;
+    dbcMsgChanged();
   }
-  signal_forms.clear();
+}
 
+void DetailWidget::dbcMsgChanged() {
+  if (msg_id.isEmpty()) return;
+
+  qDeleteAll(signals_container->findChildren<SignalEdit *>());
+  QString msg_name = tr("untitled");
   if (auto msg = dbc()->msg(msg_id)) {
     for (int i = 0; i < msg->sigs.size(); ++i) {
-      auto form = new SignalEdit(i, msg_id, msg->sigs[i], getColor(i));
-      signal_edit_layout->addWidget(form);
-      QObject::connect(form, &SignalEdit::showChart, this, &DetailWidget::showChart);
+      auto form = new SignalEdit(i, msg_id, msg->sigs[i]);
+      signals_container->layout()->addWidget(form);
+      QObject::connect(form, &SignalEdit::showChart, [this, sig = &msg->sigs[i]]() { emit showChart(msg_id, sig); });
       QObject::connect(form, &SignalEdit::showFormClicked, this, &DetailWidget::showForm);
-      signal_forms.push_back(form);
+      QObject::connect(form, &SignalEdit::remove, this, &DetailWidget::removeSignal);
+      QObject::connect(form, &SignalEdit::save, this, &DetailWidget::saveSignal);
     }
-    name_label->setText(msg->name.c_str());
-    signals_header->setVisible(true);
-  } else {
-    name_label->setText(tr("untitled"));
-    signals_header->setVisible(false);
+    msg_name = msg->name.c_str();
   }
   edit_btn->setVisible(true);
+  name_label->setText(msg_name);
 
   binary_view->setMessage(msg_id);
   history_log->setMessage(msg_id);
@@ -99,60 +91,90 @@ void DetailWidget::updateState() {
   history_log->updateState();
 }
 
-void DetailWidget::editMsg() {
-  EditMessageDialog dlg(msg_id, this);
-  if (dlg.exec()) {
-    setMessage(msg_id);
-  }
-}
-
-void DetailWidget::addSignal() {
-  AddSignalDialog dlg(msg_id, this);
-  if (dlg.exec()) {
-    setMessage(msg_id);
-  }
-}
-
 void DetailWidget::showForm() {
   SignalEdit *sender = qobject_cast<SignalEdit *>(QObject::sender());
-  if (sender->isFormVisible()) {
-    sender->setFormVisible(false);
-  } else {
-    for (auto f : signal_forms) {
-      f->setFormVisible(f == sender);
-      if (f == sender) {
-        // scroll to header
-        QTimer::singleShot(0, [=]() {
-          const QPoint p = f->mapTo(scroll, QPoint(0, 0));
-          scroll->verticalScrollBar()->setValue(p.y() + scroll->verticalScrollBar()->value());
-        });
-      }
+  for (auto f : signals_container->findChildren<SignalEdit *>()) {
+    f->setFormVisible(f == sender && !f->isFormVisible());
+    if (f == sender) {
+      QTimer::singleShot(0, [=]() { scroll->ensureWidgetVisible(f); });
     }
   }
 }
 
+void DetailWidget::editMsg() {
+  auto msg = dbc()->msg(msg_id);
+  QString name = msg ? msg->name.c_str() : "untitled";
+  int size = msg ? msg->size : can->lastMessage(msg_id).dat.size();
+  EditMessageDialog dlg(msg_id, name, size, this);
+  if (dlg.exec()) {
+    dbc()->updateMsg(msg_id, dlg.name_edit->text(), dlg.size_spin->value());
+    dbcMsgChanged();
+  }
+}
+
+void DetailWidget::addSignal(int start_bit, int size) {
+  if (dbc()->msg(msg_id)) {
+    AddSignalDialog dlg(msg_id, start_bit, size, this);
+    if (dlg.exec()) {
+      dbc()->addSignal(msg_id, dlg.form->getSignal());
+      dbcMsgChanged();
+    }
+  }
+}
+
+void DetailWidget::saveSignal() {
+  SignalEdit *sig_form = qobject_cast<SignalEdit *>(QObject::sender());
+  auto s = sig_form->form->getSignal();
+  dbc()->updateSignal(msg_id, sig_form->sig_name, s);
+  // update binary view and history log
+  binary_view->setMessage(msg_id);
+  history_log->setMessage(msg_id);
+}
+
+void DetailWidget::removeSignal() {
+  SignalEdit *sig_form = qobject_cast<SignalEdit *>(QObject::sender());
+  QString text = tr("Are you sure you want to remove signal '%1'").arg(sig_form->sig_name);
+  if (QMessageBox::Yes == QMessageBox::question(this, tr("Remove signal"), text)) {
+    dbc()->removeSignal(msg_id, sig_form->sig_name);
+    dbcMsgChanged();
+  }
+}
+
 // BinaryView
 
-BinaryView::BinaryView(QWidget *parent) {
-  QVBoxLayout *main_layout = new QVBoxLayout(this);
-  main_layout->setContentsMargins(0, 0, 0, 0);
-  table = new QTableWidget(this);
-  table->verticalHeader()->setSectionResizeMode(QHeaderView::Stretch);
-  table->horizontalHeader()->setSectionResizeMode(QHeaderView::Stretch);
-  table->horizontalHeader()->hide();
-  table->setHorizontalScrollBarPolicy(Qt::ScrollBarAlwaysOff);
-  main_layout->addWidget(table);
-  table->setColumnCount(9);
+BinaryView::BinaryView(QWidget *parent) : QTableWidget(parent) {
+  horizontalHeader()->setSectionResizeMode(QHeaderView::Stretch);
+  verticalHeader()->setSectionResizeMode(QHeaderView::Stretch);
+  horizontalHeader()->hide();
+  setHorizontalScrollBarPolicy(Qt::ScrollBarAlwaysOff);
+  setColumnCount(9);
+
+  // replace selection model
+  auto old_model = selectionModel();
+  setSelectionModel(new BinarySelectionModel(model()));
+  delete old_model;
+}
+
+void BinaryView::mouseReleaseEvent(QMouseEvent *event) {
+  QTableWidget::mouseReleaseEvent(event);
+
+  if (auto items = selectedItems(); !items.isEmpty()) {
+    int start_bit = items.first()->row() * 8 + items.first()->column();
+    int size = items.back()->row() * 8 + items.back()->column() - start_bit + 1;
+    emit cellsSelected(start_bit, size);
+  }
 }
 
 void BinaryView::setMessage(const QString &message_id) {
   msg_id = message_id;
+  if (msg_id.isEmpty()) return;
+
   const Msg *msg = dbc()->msg(msg_id);
-  const int row_count = msg ? msg->size : can->lastMessage(msg_id).dat.size();
-  table->setRowCount(row_count);
-  table->setColumnCount(9);
-  for (int i = 0; i < table->rowCount(); ++i) {
-    for (int j = 0; j < table->columnCount(); ++j) {
+  int row_count = msg ? msg->size : can->lastMessage(msg_id).dat.size();
+  setRowCount(row_count);
+  setColumnCount(9);
+  for (int i = 0; i < rowCount(); ++i) {
+    for (int j = 0; j < columnCount(); ++j) {
       auto item = new QTableWidgetItem();
       item->setFlags(item->flags() ^ Qt::ItemIsEditable);
       item->setTextAlignment(Qt::AlignCenter);
@@ -160,8 +182,9 @@ void BinaryView::setMessage(const QString &message_id) {
         QFont font;
         font.setBold(true);
         item->setFont(font);
+        item->setFlags(item->flags() ^ Qt::ItemIsSelectable);
       }
-      table->setItem(i, j, item);
+      setItem(i, j, item);
     }
   }
 
@@ -170,71 +193,73 @@ void BinaryView::setMessage(const QString &message_id) {
     for (int i = 0; i < msg->sigs.size(); ++i) {
       const auto &sig = msg->sigs[i];
       int start = sig.is_little_endian ? sig.start_bit : bigEndianBitIndex(sig.start_bit);
-      for (int j = start; j <= start + sig.size - 1; ++j) {
-        table->item(j / 8, j % 8)->setBackground(QColor(getColor(i)));
+      for (int j = start; j <= std::min(start + sig.size - 1, rowCount() * columnCount() - 1); ++j) {
+        item(j / 8, j % 8)->setBackground(QColor(getColor(i)));
       }
     }
   }
-  table->setFixedHeight(table->rowHeight(0) * std::min(row_count, 8) + 2);
+
+  setFixedHeight(rowHeight(0) * std::min(row_count, 8) + 2);
+  clearSelection();
   updateState();
 }
 
 void BinaryView::updateState() {
-  if (msg_id.isEmpty()) return;
-
   const auto &binary = can->lastMessage(msg_id).dat;
-
   setUpdatesEnabled(false);
   char hex[3] = {'\0'};
   for (int i = 0; i < binary.size(); ++i) {
     for (int j = 0; j < 8; ++j) {
-      table->item(i, j)->setText(QChar((binary[i] >> (7 - j)) & 1 ? '1' : '0'));
+      item(i, j)->setText(QChar((binary[i] >> (7 - j)) & 1 ? '1' : '0'));
     }
     hex[0] = toHex(binary[i] >> 4);
     hex[1] = toHex(binary[i] & 0xf);
-    table->item(i, 8)->setText(hex);
+    item(i, 8)->setText(hex);
   }
   setUpdatesEnabled(true);
 }
 
+void BinarySelectionModel::select(const QItemSelection &selection, QItemSelectionModel::SelectionFlags command) {
+  QItemSelection new_selection = selection;
+  if (auto indexes = selection.indexes(); !indexes.isEmpty()) {
+    auto [begin_idx, end_idx] = (QModelIndex[]){indexes.first(), indexes.back()};
+    for (int row = begin_idx.row(); row <= end_idx.row(); ++row) {
+      int left_col = (row == begin_idx.row()) ? begin_idx.column() : 0;
+      int right_col = (row == end_idx.row()) ? end_idx.column() : 7;
+      new_selection.merge({model()->index(row, left_col), model()->index(row, right_col)}, command);
+    }
+  }
+  QItemSelectionModel::select(new_selection, command);
+}
+
 // EditMessageDialog
 
-EditMessageDialog::EditMessageDialog(const QString &msg_id, QWidget *parent) : msg_id(msg_id), QDialog(parent) {
+EditMessageDialog::EditMessageDialog(const QString &msg_id, const QString &title, int size, QWidget *parent) : QDialog(parent) {
   setWindowTitle(tr("Edit message"));
   QVBoxLayout *main_layout = new QVBoxLayout(this);
 
   QFormLayout *form_layout = new QFormLayout();
   form_layout->addRow("ID", new QLabel(msg_id));
 
-  const auto msg = dbc()->msg(msg_id);
-  name_edit = new QLineEdit(this);
-  name_edit->setText(msg ? msg->name.c_str() : "untitled");
+  name_edit = new QLineEdit(title, this);
   form_layout->addRow(tr("Name"), name_edit);
 
   size_spin = new QSpinBox(this);
-  size_spin->setValue(msg ? msg->size : can->lastMessage(msg_id).dat.size());
+  // TODO: limit the maximum?
+  size_spin->setMinimum(1);
+  size_spin->setValue(size);
   form_layout->addRow(tr("Size"), size_spin);
 
   main_layout->addLayout(form_layout);
 
   auto buttonBox = new QDialogButtonBox(QDialogButtonBox::Ok | QDialogButtonBox::Cancel);
   main_layout->addWidget(buttonBox);
-
   setFixedWidth(parent->width() * 0.9);
 
-  connect(buttonBox, &QDialogButtonBox::accepted, this, &EditMessageDialog::save);
+  connect(buttonBox, &QDialogButtonBox::accepted, this, &QDialog::accept);
   connect(buttonBox, &QDialogButtonBox::rejected, this, &QDialog::reject);
 }
 
-void EditMessageDialog::save() {
-  const QString name = name_edit->text();
-  if (size_spin->value() <= 0 || name_edit->text().isEmpty() || name == tr("untitled"))
-    return;
-
-  dbc()->updateMsg(msg_id, name, size_spin->value());
-  QDialog::accept();
-}
-
 // ScrollArea
 
 bool ScrollArea::eventFilter(QObject *obj, QEvent *ev) {

tools/cabana/detailwidget.h

@@ -1,12 +1,7 @@
 #pragma once
 
-#include <QDialog>
-#include <QLabel>
-#include <QPushButton>
 #include <QScrollArea>
 #include <QTableWidget>
-#include <QVBoxLayout>
-#include <QWidget>
 
 #include "opendbc/can/common.h"
 #include "opendbc/can/common_dbc.h"
@@ -15,29 +10,32 @@
 #include "tools/cabana/historylog.h"
 #include "tools/cabana/signaledit.h"
 
-class BinaryView : public QWidget {
-  Q_OBJECT
-
+class BinarySelectionModel : public QItemSelectionModel {
 public:
-  BinaryView(QWidget *parent);
+  BinarySelectionModel(QAbstractItemModel *model = nullptr) : QItemSelectionModel(model) {}
+  void select(const QItemSelection &selection, QItemSelectionModel::SelectionFlags command) override;
+};
+
+class BinaryView : public QTableWidget {
+  Q_OBJECT
+public:
+  BinaryView(QWidget *parent = nullptr);
+  void mouseReleaseEvent(QMouseEvent *event) override;
   void setMessage(const QString &message_id);
   void updateState();
+signals:
+  void cellsSelected(int start_bit, int size);
 
 private:
   QString msg_id;
-  QTableWidget *table;
 };
 
 class EditMessageDialog : public QDialog {
   Q_OBJECT
 
 public:
-  EditMessageDialog(const QString &msg_id, QWidget *parent);
+  EditMessageDialog(const QString &msg_id, const QString &title, int size, QWidget *parent);
 
-protected:
-  void save();
-
-  QString msg_id;
   QLineEdit *name_edit;
   QSpinBox *size_spin;
 };
@@ -57,24 +55,24 @@ class DetailWidget : public QWidget {
 public:
   DetailWidget(QWidget *parent);
   void setMessage(const QString &message_id);
+  void dbcMsgChanged();
 
 signals:
-  void showChart(const QString &msg_id, const QString &sig_name);
-
-private slots:
-  void showForm();
+  void showChart(const QString &msg_id, const Signal *sig);
+  void removeChart(const Signal *sig);
 
 private:
-  void addSignal();
+  void addSignal(int start_bit, int size);
+  void saveSignal();
+  void removeSignal();
   void editMsg();
+  void showForm();
   void updateState();
 
   QString msg_id;
   QLabel *name_label, *time_label;
   QPushButton *edit_btn;
-  QVBoxLayout *signal_edit_layout;
-  QWidget *signals_header;
-  QList<SignalEdit *> signal_forms;
+  QWidget *signals_container;
   HistoryLog *history_log;
   BinaryView *binary_view;
   ScrollArea *scroll;

tools/cabana/historylog.cc

@@ -52,9 +52,8 @@ QVariant HistoryLogModel::headerData(int section, Qt::Orientation orientation, i
 void HistoryLogModel::updateState() {
   if (msg_id.isEmpty()) return;
 
-  const auto &can_msgs = can->messages(msg_id);
   int prev_row_count = row_count;
-  row_count = can_msgs.size();
+  row_count = can->messages(msg_id).size();
   int delta = row_count - prev_row_count;
   if (delta > 0) {
     beginInsertRows({}, prev_row_count, row_count - 1);
@@ -64,7 +63,7 @@ void HistoryLogModel::updateState() {
     endRemoveRows();
   }
   if (row_count > 0) {
-    emit dataChanged(index(0, 0), index(row_count - 1, column_count - 1));
+    emit dataChanged(index(0, 0), index(row_count - 1, column_count - 1), {Qt::DisplayRole});
     emit headerDataChanged(Qt::Vertical, 0, row_count - 1);
   }
 }

tools/cabana/messageswidget.cc

@@ -68,7 +68,7 @@ MessagesWidget::MessagesWidget(QWidget *parent) : QWidget(parent) {
   });
   QObject::connect(table_widget->selectionModel(), &QItemSelectionModel::currentChanged, [=](const QModelIndex &current, const QModelIndex &previous) {
     if (current.isValid()) {
-      emit msgSelectionChanged(table_widget->model()->data(current, Qt::UserRole).toString());
+      emit msgSelectionChanged(current.data(Qt::UserRole).toString());
     }
   });
 
@@ -78,11 +78,8 @@ MessagesWidget::MessagesWidget(QWidget *parent) : QWidget(parent) {
 
 void MessagesWidget::dbcSelectionChanged(const QString &dbc_file) {
   dbc()->open(dbc_file);
-  // update detailwidget
-  auto current = table_widget->selectionModel()->currentIndex();
-  if (current.isValid()) {
-    emit msgSelectionChanged(table_widget->model()->data(current, Qt::UserRole).toString());
-  }
+  // TODO: reset model?
+  table_widget->sortByColumn(0, Qt::AscendingOrder);
 }
 
 // MessageListModel
@@ -90,9 +87,6 @@ void MessagesWidget::dbcSelectionChanged(const QString &dbc_file) {
 QVariant MessageListModel::headerData(int section, Qt::Orientation orientation, int role) const {
   if (orientation == Qt::Horizontal && role == Qt::DisplayRole)
     return (QString[]){"Name", "ID", "Count", "Bytes"}[section];
-  else if (orientation == Qt::Vertical && role == Qt::DisplayRole) {
-    // return QString::number(section);
-  }
   return {};
 }
 
@@ -100,17 +94,15 @@ QVariant MessageListModel::data(const QModelIndex &index, int role) const {
   if (role == Qt::DisplayRole) {
     auto it = std::next(can->can_msgs.begin(), index.row());
     if (it != can->can_msgs.end() && !it.value().empty()) {
-      const auto &d = it.value().front();
       const QString &msg_id = it.key();
       switch (index.column()) {
         case 0: {
           auto msg = dbc()->msg(msg_id);
-          QString name = msg ? msg->name.c_str() : "untitled";
-          return name;
+          return msg ? msg->name.c_str() : "untitled";
         }
         case 1: return msg_id;
         case 2: return can->counters[msg_id];
-        case 3: return toHex(d.dat);
+        case 3: return toHex(it.value().front().dat);
       }
     }
   } else if (role == Qt::UserRole) {
@@ -132,6 +124,6 @@ void MessageListModel::updateState() {
   }
 
   if (row_count > 0) {
-    emit dataChanged(index(0, 0), index(row_count - 1, 3));
+    emit dataChanged(index(0, 0), index(row_count - 1, 3), {Qt::DisplayRole});
   }
 }

tools/cabana/signaledit.cc

@@ -3,7 +3,6 @@
 #include <QDialogButtonBox>
 #include <QFormLayout>
 #include <QHBoxLayout>
-#include <QMessageBox>
 #include <QVBoxLayout>
 
 // SignalForm
@@ -15,13 +14,10 @@ SignalForm::SignalForm(const Signal &sig, QWidget *parent) : start_bit(sig.start
   form_layout->addRow(tr("Name"), name);
 
   size = new QSpinBox();
+  size->setMinimum(1);
   size->setValue(sig.size);
   form_layout->addRow(tr("Size"), size);
 
-  msb = new QSpinBox();
-  msb->setValue(sig.msb);
-  form_layout->addRow(tr("Most significant bit"), msb);
-
   endianness = new QComboBox();
   endianness->addItems({"Little", "Big"});
   endianness->setCurrentIndex(sig.is_little_endian ? 0 : 1);
@@ -56,7 +52,8 @@ SignalForm::SignalForm(const Signal &sig, QWidget *parent) : start_bit(sig.start
   form_layout->addRow(tr("Value descriptions"), val_desc);
 }
 
-std::optional<Signal> SignalForm::getSignal() {
+Signal SignalForm::getSignal() {
+  // TODO: Check if the size is valid, and no duplicate name
   Signal sig = {};
   sig.start_bit = start_bit;
   sig.name = name->text().toStdString();
@@ -72,17 +69,17 @@ std::optional<Signal> SignalForm::getSignal() {
     sig.lsb = bigEndianStartBitsIndex(bigEndianBitIndex(sig.start_bit) + sig.size - 1);
     sig.msb = sig.start_bit;
   }
-  return (sig.name.empty() || sig.size <= 0) ? std::nullopt : std::optional(sig);
+  return sig;
 }
 
 // SignalEdit
 
-SignalEdit::SignalEdit(int index, const QString &id, const Signal &sig, const QString &color, QWidget *parent)
-    : id(id), name_(sig.name.c_str()), QWidget(parent) {
+SignalEdit::SignalEdit(int index, const QString &msg_id, const Signal &sig, QWidget *parent)
+    : sig_name(sig.name.c_str()), QWidget(parent) {
   QVBoxLayout *main_layout = new QVBoxLayout(this);
   main_layout->setContentsMargins(0, 0, 0, 0);
 
-  // title
+  // title bar
   QHBoxLayout *title_layout = new QHBoxLayout();
   icon = new QLabel(">");
   icon->setFixedSize(15, 30);
@@ -90,24 +87,25 @@ SignalEdit::SignalEdit(int index, const QString &id, const Signal &sig, const QS
   title_layout->addWidget(icon);
   title = new ElidedLabel(this);
   title->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::MinimumExpanding);
-  title->setText(QString("%1. %2").arg(index + 1).arg(sig.name.c_str()));
-  title->setStyleSheet(QString("font-weight:bold; color:%1").arg(color));
+  title->setText(QString("%1. %2").arg(index + 1).arg(sig_name));
+  title->setStyleSheet(QString("font-weight:bold; color:%1").arg(getColor(index)));
   title_layout->addWidget(title);
 
-  plot_btn = new QPushButton("📈");
+  QPushButton *plot_btn = new QPushButton("📈");
   plot_btn->setToolTip(tr("Show Plot"));
   plot_btn->setFixedSize(30, 30);
-  QObject::connect(plot_btn, &QPushButton::clicked, [=]() { emit showChart(id, name_); });
+  QObject::connect(plot_btn, &QPushButton::clicked, this, &SignalEdit::showChart);
   title_layout->addWidget(plot_btn);
   main_layout->addLayout(title_layout);
 
+  // signal form
   form_container = new QWidget(this);
   QVBoxLayout *v_layout = new QVBoxLayout(form_container);
   form = new SignalForm(sig, this);
   v_layout->addWidget(form);
 
   QHBoxLayout *h = new QHBoxLayout();
-  remove_btn = new QPushButton(tr("Remove Signal"));
+  QPushButton *remove_btn = new QPushButton(tr("Remove Signal"));
   h->addWidget(remove_btn);
   h->addStretch();
   QPushButton *save_btn = new QPushButton(tr("Save"));
@@ -117,13 +115,19 @@ SignalEdit::SignalEdit(int index, const QString &id, const Signal &sig, const QS
   form_container->setVisible(false);
   main_layout->addWidget(form_container);
 
-  QFrame* hline = new QFrame();
+  // bottom line
+  QFrame *hline = new QFrame();
   hline->setFrameShape(QFrame::HLine);
   hline->setFrameShadow(QFrame::Sunken);
   main_layout->addWidget(hline);
 
   QObject::connect(remove_btn, &QPushButton::clicked, this, &SignalEdit::remove);
-  QObject::connect(save_btn, &QPushButton::clicked, this, &SignalEdit::save);
+  QObject::connect(save_btn, &QPushButton::clicked, [=]() {
+    QString new_name = form->getSignal().name.c_str();
+    title->setText(QString("%1. %2").arg(index + 1).arg(new_name));
+    emit save();
+    sig_name = new_name;
+  });
   QObject::connect(title, &ElidedLabel::clicked, this, &SignalEdit::showFormClicked);
 }
 
@@ -132,40 +136,24 @@ void SignalEdit::setFormVisible(bool visible) {
   icon->setText(visible ? "▼" : ">");
 }
 
-void SignalEdit::save() {
-  if (auto s = form->getSignal())
-    dbc()->updateSignal(id, name_, *s);
-}
-
-void SignalEdit::remove() {
-  QMessageBox msgbox;
-  msgbox.setText(tr("Remove signal"));
-  msgbox.setInformativeText(tr("Are you sure you want to remove signal '%1'").arg(name_));
-  msgbox.setStandardButtons(QMessageBox::Ok | QMessageBox::Cancel);
-  msgbox.setDefaultButton(QMessageBox::Cancel);
-  if (msgbox.exec()) {
-    dbc()->removeSignal(id, name_);
-    deleteLater();
-  }
-}
-
 // AddSignalDialog
 
-AddSignalDialog::AddSignalDialog(const QString &id, QWidget *parent) : QDialog(parent) {
+AddSignalDialog::AddSignalDialog(const QString &id, int start_bit, int size, QWidget *parent) : QDialog(parent) {
   setWindowTitle(tr("Add signal to %1").arg(dbc()->msg(id)->name.c_str()));
   QVBoxLayout *main_layout = new QVBoxLayout(this);
-  Signal sig = {.name = "untitled"};
-  auto form = new SignalForm(sig, this);
+
+  Signal sig = {
+    .name = "untitled",
+    .start_bit = bigEndianBitIndex(start_bit),
+    .is_little_endian = false,
+    .size = size,
+  };
+  form = new SignalForm(sig, this);
   main_layout->addWidget(form);
   auto buttonBox = new QDialogButtonBox(QDialogButtonBox::Ok | QDialogButtonBox::Cancel);
   main_layout->addWidget(buttonBox);
   setFixedWidth(parent->width() * 0.9);
 
   connect(buttonBox, &QDialogButtonBox::rejected, this, &QDialog::reject);
-  connect(buttonBox, &QDialogButtonBox::accepted, [=]() {
-    if (auto signal = form->getSignal()) {
-      dbc()->addSignal(id, *signal);
-    }
-    QDialog::accept();
-  });
+  connect(buttonBox, &QDialogButtonBox::accepted, this, &QDialog::accept);
 }

tools/cabana/signaledit.h

@@ -1,7 +1,5 @@
 #pragma once
 
-#include <optional>
-
 #include <QComboBox>
 #include <QDialog>
 #include <QLabel>
@@ -15,14 +13,12 @@
 #include "tools/cabana/dbcmanager.h"
 
 class SignalForm : public QWidget {
-  Q_OBJECT
-
 public:
   SignalForm(const Signal &sig, QWidget *parent);
-  std::optional<Signal> getSignal();
+  Signal getSignal();
 
   QLineEdit *name, *unit, *comment, *val_desc;
-  QSpinBox *size, *msb, *lsb, *offset;
+  QSpinBox *size, *offset;
   QDoubleSpinBox *factor, *min_val, *max_val;
   QComboBox *sign, *endianness;
   int start_bit = 0;
@@ -32,31 +28,26 @@ class SignalEdit : public QWidget {
   Q_OBJECT
 
 public:
-  SignalEdit(int index, const QString &id, const Signal &sig, const QString &color, QWidget *parent = nullptr);
+  SignalEdit(int index, const QString &msg_id, const Signal &sig, QWidget *parent = nullptr);
   void setFormVisible(bool show);
   inline bool isFormVisible() const { return form_container->isVisible(); }
-  void save();
+  QString sig_name;
+  SignalForm *form;
 
 signals:
-  void showChart(const QString &msg_id, const QString &sig_name);
+  void showChart();
   void showFormClicked();
+  void remove();
+  void save();
 
 protected:
-  void remove();
-
-  QString id;
-  QString name_;
-  QPushButton *plot_btn;
   ElidedLabel *title;
-  SignalForm *form;
   QWidget *form_container;
-  QPushButton *remove_btn;
   QLabel *icon;
 };
 
 class AddSignalDialog : public QDialog {
-  Q_OBJECT
-
 public:
-  AddSignalDialog(const QString &id, QWidget *parent);
+  AddSignalDialog(const QString &id, int start_bit, int size, QWidget *parent);
+  SignalForm *form;
 };