cabana: simplifying endian conversion (#28601)

tools/cabana/binaryview.cc

@@ -1,7 +1,5 @@
 #include "tools/cabana/binaryview.h"
 
-#include <cmath>
-
 #include <QFontDatabase>
 #include <QHeaderView>
 #include <QMouseEvent>
@@ -11,16 +9,12 @@
 #include <QToolTip>
 
 #include "tools/cabana/commands.h"
-#include "tools/cabana/signalview.h"
 
 // BinaryView
 
 const int CELL_HEIGHT = 36;
 const int VERTICAL_HEADER_WIDTH = 30;
-
+inline int get_bit_pos(const QModelIndex &index) { return flipBitPos(index.row() * 8 + index.column()); }
-inline int get_bit_index(const QModelIndex &index, bool little_endian) {
-  return index.row() * 8 + (little_endian ? 7 - index.column() : index.column());
-}
 
 BinaryView::BinaryView(QWidget *parent) : QTableView(parent) {
   model = new BinaryViewModel(this);
@@ -66,7 +60,7 @@ void BinaryView::addShortcuts() {
   QObject::connect(shortcut_delete_backspace, &QShortcut::activated, shortcut_delete_x, &QShortcut::activated);
   QObject::connect(shortcut_delete_x, &QShortcut::activated, [=]{
     if (hovered_sig != nullptr) {
-      emit removeSignal(hovered_sig);
+      UndoStack::push(new RemoveSigCommand(model->msg_id, hovered_sig));
       hovered_sig = nullptr;
     }
   });
@@ -75,13 +69,9 @@ void BinaryView::addShortcuts() {
   QShortcut *shortcut_endian = new QShortcut(QKeySequence(Qt::Key_E), this);
   QObject::connect(shortcut_endian, &QShortcut::activated, [=]{
     if (hovered_sig != nullptr) {
-      const cabana::Signal *hovered_sig_prev = hovered_sig;
       cabana::Signal s = *hovered_sig;
       s.is_little_endian = !s.is_little_endian;
       emit editSignal(hovered_sig, s);
-
-      hovered_sig = nullptr;
-      highlight(hovered_sig_prev);
     }
   });
 
@@ -89,13 +79,9 @@ void BinaryView::addShortcuts() {
   QShortcut *shortcut_sign = new QShortcut(QKeySequence(Qt::Key_S), this);
   QObject::connect(shortcut_sign, &QShortcut::activated, [=]{
     if (hovered_sig != nullptr) {
-      const cabana::Signal *hovered_sig_prev = hovered_sig;
       cabana::Signal s = *hovered_sig;
       s.is_signed = !s.is_signed;
       emit editSignal(hovered_sig, s);
-
-      hovered_sig = nullptr;
-      highlight(hovered_sig_prev);
     }
   });
 
@@ -139,24 +125,24 @@ void BinaryView::setSelection(const QRect &rect, QItemSelectionModel::SelectionF
 
   QItemSelection selection;
   auto [start, size, is_lb] = getSelection(index);
-  for (int i = start; i < start + size; ++i) {
+  for (int i = 0; i < size; ++i) {
-    auto idx = model->bitIndex(i, is_lb);
+    int pos = is_lb ? flipBitPos(start + i) : flipBitPos(start) + i;
-    selection.merge({idx, idx}, flags);
+    selection << QItemSelectionRange{model->index(pos / 8, pos % 8)};
   }
   selectionModel()->select(selection, flags);
 }
 
 void BinaryView::mousePressEvent(QMouseEvent *event) {
-  delegate->selection_color = (palette().color(QPalette::Active, QPalette::Highlight));
+  resize_sig = nullptr;
   if (auto index = indexAt(event->pos()); index.isValid() && index.column() != 8) {
     anchor_index = index;
     auto item = (const BinaryViewModel::Item *)anchor_index.internalPointer();
-    int bit_idx = get_bit_index(anchor_index, true);
+    int bit_pos = get_bit_pos(anchor_index);
     for (auto s : item->sigs) {
-      if (bit_idx == s->lsb || bit_idx == s->msb) {
+      if (bit_pos == s->lsb || bit_pos == s->msb) {
-        anchor_index = model->bitIndex(bit_idx == s->lsb ? s->msb : s->lsb, true);
+        int idx = flipBitPos(bit_pos == s->lsb ? s->msb : s->lsb);
+        anchor_index = model->index(idx / 8, idx % 8);
         resize_sig = s;
-        delegate->selection_color = s->color;
         break;
       }
     }
@@ -183,9 +169,10 @@ void BinaryView::mouseReleaseEvent(QMouseEvent *event) {
   auto release_index = indexAt(event->pos());
   if (release_index.isValid() && anchor_index.isValid()) {
     if (selectionModel()->hasSelection()) {
-      auto [start_bit, size, is_lb] = getSelection(release_index);
+      auto sig = resize_sig ? *resize_sig : cabana::Signal{};
-      resize_sig ? emit resizeSignal(resize_sig, start_bit, size)
+      std::tie(sig.start_bit, sig.size, sig.is_little_endian) = getSelection(release_index);
-                 : emit addSignal(start_bit, size, is_lb);
+      resize_sig ? emit editSignal(resize_sig, sig)
+                 : UndoStack::push(new AddSigCommand(model->msg_id, sig));
     } else {
       auto item = (const BinaryViewModel::Item *)anchor_index.internalPointer();
       if (item && item->sigs.size() > 0)
@@ -246,10 +233,11 @@ std::tuple<int, int, bool> BinaryView::getSelection(QModelIndex index) {
     is_lb = false;
   }
 
-  int cur_bit_idx = get_bit_index(index, is_lb);
+  int cur_bit_pos = get_bit_pos(index);
-  int anchor_bit_idx = get_bit_index(anchor_index, is_lb);
+  int anchor_bit_pos = get_bit_pos(anchor_index);
-  auto [start_bit, end_bit] = std::minmax(cur_bit_idx, anchor_bit_idx);
+  int start_bit = is_lb ? std::min(cur_bit_pos, anchor_bit_pos) : get_bit_pos(std::min(index, anchor_index));
-  return {start_bit, end_bit - start_bit + 1, is_lb};
+  int size = is_lb ? std::abs(cur_bit_pos - anchor_bit_pos) + 1 : std::abs(flipBitPos(cur_bit_pos) - flipBitPos(anchor_bit_pos)) + 1;
+  return {start_bit, size, is_lb};
 }
 
 // BinaryViewModel
@@ -261,16 +249,15 @@ void BinaryViewModel::refresh() {
     row_count = dbc_msg->size;
     items.resize(row_count * column_count);
     for (auto sig : dbc_msg->getSignals()) {
-      auto [start, end] = getSignalRange(sig);
+      for (int j = 0; j < sig->size; ++j) {
-      for (int j = start; j <= end; ++j) {
+        int pos = sig->is_little_endian ? flipBitPos(sig->start_bit + j) : flipBitPos(sig->start_bit) + j;
-        int bit_index = sig->is_little_endian ? bigEndianBitIndex(j) : j;
+        int idx = column_count * (pos / 8) + pos % 8;
-        int idx = column_count * (bit_index / 8) + bit_index % 8;
         if (idx >= items.size()) {
           qWarning() << "signal " << sig->name << "out of bounds.start_bit:" << sig->start_bit << "size:" << sig->size;
           break;
         }
-        if (j == start) sig->is_little_endian ? items[idx].is_lsb = true : items[idx].is_msb = true;
+        if (j == 0) sig->is_little_endian ? items[idx].is_lsb = true : items[idx].is_msb = true;
-        if (j == end) sig->is_little_endian ? items[idx].is_msb = true : items[idx].is_lsb = true;
+        if (j == sig->size - 1) sig->is_little_endian ? items[idx].is_msb = true : items[idx].is_lsb = true;
 
         auto &sigs = items[idx].sigs;
         sigs.push_back(sig);
@@ -379,7 +366,8 @@ void BinaryItemDelegate::paint(QPainter *painter, const QStyleOptionViewItem &op
       painter->fillRect(option.rect, item->bg_color);
     }
   } else if (option.state & QStyle::State_Selected) {
-    painter->fillRect(option.rect, selection_color);
+    auto color = bin_view->resize_sig ? bin_view->resize_sig->color : option.palette.color(QPalette::Active, QPalette::Highlight);
+    painter->fillRect(option.rect, color);
     painter->setPen(option.palette.color(QPalette::BrightText));
   } else if (!bin_view->selectionModel()->hasSelection() || !item->sigs.contains(bin_view->resize_sig)) {  // not resizing
     if (item->sigs.size() > 0) {
@@ -441,7 +429,7 @@ void BinaryItemDelegate::drawSignalCell(QPainter *painter, const QStyleOptionVie
   QColor color = sig->color;
   color.setAlpha(item->bg_color.alpha());
   // Mixing the signal colour with the Base background color to fade it
-  painter->fillRect(rc, QApplication::palette().color(QPalette::Base));
+  painter->fillRect(rc, option.palette.color(QPalette::Base));
   painter->fillRect(rc, color);
 
   // Draw edges

tools/cabana/binaryview.h

@@ -1,6 +1,5 @@
 #pragma once
 
-#include <QApplication>
 #include <QList>
 #include <QSet>
 #include <QStyledItemDelegate>
@@ -13,12 +12,10 @@ class BinaryItemDelegate : public QStyledItemDelegate {
 public:
   BinaryItemDelegate(QObject *parent);
   void paint(QPainter *painter, const QStyleOptionViewItem &option, const QModelIndex &index) const override;
-  void setSelectionColor(const QColor &color) { selection_color = color; }
   bool hasSignal(const QModelIndex &index, int dx, int dy, const cabana::Signal *sig) const;
   void drawSignalCell(QPainter* painter, const QStyleOptionViewItem &option, const QModelIndex &index, const cabana::Signal *sig) const;
 
   QFont small_font, hex_font;
-  QColor selection_color;
 };
 
 class BinaryViewModel : public QAbstractTableModel {
@@ -31,7 +28,6 @@ public:
   QVariant data(const QModelIndex &index, int role = Qt::DisplayRole) const override;
   int rowCount(const QModelIndex &parent = QModelIndex()) const override { return row_count; }
   int columnCount(const QModelIndex &parent = QModelIndex()) const override { return column_count; }
-  inline QModelIndex bitIndex(int bit, bool is_lb) const { return index(bit / 8, is_lb ? (7 - bit % 8) : bit % 8); }
   QModelIndex index(int row, int column, const QModelIndex &parent = QModelIndex()) const override {
     return createIndex(row, column, (void *)&items[row * column_count + column]);
   }
@@ -68,9 +64,6 @@ public:
 signals:
   void signalClicked(const cabana::Signal *sig);
   void signalHovered(const cabana::Signal *sig);
-  void addSignal(int start_bit, int size, bool little_endian);
-  void resizeSignal(const cabana::Signal *sig, int from, int size);
-  void removeSignal(const cabana::Signal *sig);
   void editSignal(const cabana::Signal *origin_s, cabana::Signal &s);
   void showChart(const MessageId &id, const cabana::Signal *sig, bool show, bool merge);
 

tools/cabana/commands.cc

@@ -56,8 +56,20 @@ AddSigCommand::AddSigCommand(const MessageId &id, const cabana::Signal &sig, QUn
   setText(QObject::tr("add signal %1 to %2:%3").arg(sig.name).arg(msgName(id)).arg(id.address));
 }
 
-void AddSigCommand::undo() { dbc()->removeSignal(id, signal.name); }
+void AddSigCommand::undo() {
-void AddSigCommand::redo() { dbc()->addSignal(id, signal); }
+  dbc()->removeSignal(id, signal.name);
+  if (msg_created) dbc()->removeMsg(id);
+}
+
+void AddSigCommand::redo() {
+  if (auto msg = dbc()->msg(id); !msg) {
+    msg_created = true;
+    dbc()->updateMsg(id, dbc()->newMsgName(id), can->lastMessage(id).dat.size(), "");
+  }
+  signal.name = dbc()->newSignalName(id);
+  signal.max = std::pow(2, signal.size) - 1;
+  dbc()->addSignal(id, signal);
+}
 
 // RemoveSigCommand
 

tools/cabana/commands.h

@@ -37,6 +37,7 @@ public:
 
 private:
   const MessageId id;
+  bool msg_created = false;
   cabana::Signal signal = {};
 };
 

tools/cabana/dbc/dbc.cc

@@ -119,6 +119,8 @@ void cabana::Msg::update() {
 // cabana::Signal
 
 void cabana::Signal::update() {
+  updateMsbLsb(*this);
+
   float h = 19 * (float)lsb / 64.0;
   h = fmod(h, 1.0);
   size_t hash = qHash(name);
@@ -165,14 +167,6 @@ bool cabana::Signal::operator==(const cabana::Signal &other) const {
 
 // helper functions
 
-static QVector<int> BIG_ENDIAN_START_BITS = []() {
-  QVector<int> ret;
-  for (int i = 0; i < 64; i++)
-    for (int j = 7; j >= 0; j--)
-      ret.push_back(j + i * 8);
-  return ret;
-}();
-
 double get_raw_value(const uint8_t *data, size_t data_size, const cabana::Signal &sig) {
   int64_t val = 0;
 
@@ -195,23 +189,12 @@ double get_raw_value(const uint8_t *data, size_t data_size, const cabana::Signal
   return val * sig.factor + sig.offset;
 }
 
-int bigEndianStartBitsIndex(int start_bit) { return BIG_ENDIAN_START_BITS[start_bit]; }
+void updateMsbLsb(cabana::Signal &s) {
-int bigEndianBitIndex(int index) { return BIG_ENDIAN_START_BITS.indexOf(index); }
-
-void updateSigSizeParamsFromRange(cabana::Signal &s, int start_bit, int size) {
-  s.start_bit = s.is_little_endian ? start_bit : bigEndianBitIndex(start_bit);
-  s.size = size;
   if (s.is_little_endian) {
     s.lsb = s.start_bit;
     s.msb = s.start_bit + s.size - 1;
   } else {
-    s.lsb = bigEndianStartBitsIndex(bigEndianBitIndex(s.start_bit) + s.size - 1);
+    s.lsb = flipBitPos(flipBitPos(s.start_bit) + s.size - 1);
     s.msb = s.start_bit;
   }
 }
-
-std::pair<int, int> getSignalRange(const cabana::Signal *s) {
-  int from = s->is_little_endian ? s->start_bit : bigEndianBitIndex(s->start_bit);
-  int to = from + s->size - 1;
-  return {from, to};
-}

tools/cabana/dbc/dbc.h

@@ -66,7 +66,8 @@ public:
   Type type = Type::Normal;
   QString name;
   int start_bit, msb, lsb, size;
-  double factor, offset;
+  double factor = 1.0;
+  double offset = 0;
   bool is_signed;
   bool is_little_endian;
   double min, max;
@@ -110,9 +111,7 @@ public:
 
 // Helper functions
 double get_raw_value(const uint8_t *data, size_t data_size, const cabana::Signal &sig);
-int bigEndianStartBitsIndex(int start_bit);
+void updateMsbLsb(cabana::Signal &s);
-int bigEndianBitIndex(int index);
+inline int flipBitPos(int start_bit) { return 8 * (start_bit / 8) + 7 - start_bit % 8; }
-void updateSigSizeParamsFromRange(cabana::Signal &s, int start_bit, int size);
-std::pair<int, int> getSignalRange(const cabana::Signal *s);
 inline std::vector<std::string> allDBCNames() { return get_dbc_names(); }
 inline QString doubleToString(double value) { return QString::number(value, 'g', std::numeric_limits<double>::digits10); }

tools/cabana/dbc/dbcfile.cc

@@ -148,16 +148,10 @@ void DBCFile::parse(const QString &content) {
       s.is_signed = match.captured(offset + 5) == "-";
       s.factor = match.captured(offset + 6).toDouble();
       s.offset = match.captured(offset + 7).toDouble();
-      if (s.is_little_endian) {
-        s.lsb = s.start_bit;
-        s.msb = s.start_bit + s.size - 1;
-      } else {
-        s.lsb = bigEndianStartBitsIndex(bigEndianBitIndex(s.start_bit) + s.size - 1);
-        s.msb = s.start_bit;
-      }
       s.min = match.captured(8 + offset).toDouble();
       s.max = match.captured(9 + offset).toDouble();
       s.unit = match.captured(10 + offset);
+
       current_msg->sigs.push_back(new cabana::Signal(s));
     } else if (line.startsWith("VAL_ ")) {
       auto match = val_regexp.match(line);

tools/cabana/detailwidget.cc

@@ -64,12 +64,9 @@ DetailWidget::DetailWidget(ChartsWidget *charts, QWidget *parent) : charts(chart
 
   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);
   QObject::connect(binary_view, &BinaryView::signalClicked, [this](const cabana::Signal *s) { signal_view->selectSignal(s, true); });
   QObject::connect(binary_view, &BinaryView::editSignal, signal_view->model, &SignalModel::saveSignal);
-  QObject::connect(binary_view, &BinaryView::removeSignal, signal_view->model, &SignalModel::removeSignal);
   QObject::connect(binary_view, &BinaryView::showChart, charts, &ChartsWidget::showChart);
   QObject::connect(signal_view, &SignalView::showChart, charts, &ChartsWidget::showChart);
   QObject::connect(signal_view, &SignalView::highlight, binary_view, &BinaryView::highlight);

tools/cabana/signalview.cc

@@ -1,11 +1,9 @@
 #include "tools/cabana/signalview.h"
 
-#include <QApplication>
 #include <QCompleter>
 #include <QDialogButtonBox>
 #include <QHBoxLayout>
 #include <QHeaderView>
-#include <QHelpEvent>
 #include <QMessageBox>
 #include <QPainter>
 #include <QPainterPath>
@@ -214,53 +212,12 @@ bool SignalModel::saveSignal(const cabana::Signal *origin_s, cabana::Signal &s)
   }
 
   if (s.is_little_endian != origin_s->is_little_endian) {
-    int start = std::floor(s.start_bit / 8);
+    s.start_bit = flipBitPos(s.start_bit);
-    if (s.is_little_endian) {
-      int end = std::floor((s.start_bit - s.size + 1) / 8);
-      s.start_bit = start == end ? s.start_bit - s.size + 1 : bigEndianStartBitsIndex(s.start_bit);
-    } else {
-      int end = std::floor((s.start_bit + s.size - 1) / 8);
-      s.start_bit = start == end ? s.start_bit + s.size - 1 : bigEndianBitIndex(s.start_bit);
-    }
-  }
-  if (s.is_little_endian) {
-    s.lsb = s.start_bit;
-    s.msb = s.start_bit + s.size - 1;
-  } else {
-    s.lsb = bigEndianStartBitsIndex(bigEndianBitIndex(s.start_bit) + s.size - 1);
-    s.msb = s.start_bit;
   }
-
   UndoStack::push(new EditSignalCommand(msg_id, origin_s, s));
   return true;
 }
 
-void SignalModel::addSignal(int start_bit, int size, bool little_endian) {
-  auto msg = dbc()->msg(msg_id);
-  if (!msg) {
-    QString name = dbc()->newMsgName(msg_id);
-    UndoStack::push(new EditMsgCommand(msg_id, name, can->lastMessage(msg_id).dat.size(), ""));
-    msg = dbc()->msg(msg_id);
-  }
-
-  cabana::Signal sig = {.name = dbc()->newSignalName(msg_id), .is_little_endian = little_endian, .factor = 1, .min = 0, .max = std::pow(2, size) - 1};
-  updateSigSizeParamsFromRange(sig, start_bit, size);
-  UndoStack::push(new AddSigCommand(msg_id, sig));
-}
-
-void SignalModel::resizeSignal(const cabana::Signal *sig, int start_bit, int size) {
-  cabana::Signal s = *sig;
-  updateSigSizeParamsFromRange(s, start_bit, size);
-  saveSignal(sig, s);
-}
-
-void SignalModel::removeSignal(const cabana::Signal *sig) {
-  UndoStack::push(new RemoveSigCommand(msg_id, sig));
-  if (dbc()->signalCount(msg_id) == 0) {
-    UndoStack::push(new RemoveMsgCommand(msg_id));
-  }
-}
-
 void SignalModel::handleMsgChanged(MessageId id) {
   if (id.address == msg_id.address) {
     refresh();
@@ -324,7 +281,7 @@ QSize SignalItemDelegate::sizeHint(const QStyleOptionViewItem &option, const QMo
     }
     width = std::min<int>(option.widget->size().width() / 3.0, it.value() + spacing);
   }
-  return {width, QApplication::fontMetrics().height()};
+  return {width, option.fontMetrics.height()};
 }
 
 void SignalItemDelegate::updateEditorGeometry(QWidget *editor, const QStyleOptionViewItem &option, const QModelIndex &index) const {
@@ -565,7 +522,7 @@ void SignalView::rowsChanged() {
 
       tree->setIndexWidget(index, w);
       auto sig = model->getItem(index)->sig;
-      QObject::connect(remove_btn, &QToolButton::clicked, [=]() { model->removeSignal(sig); });
+      QObject::connect(remove_btn, &QToolButton::clicked, [=]() { UndoStack::push(new RemoveSigCommand(model->msg_id, sig)); });
       QObject::connect(plot_btn, &QToolButton::clicked, [=](bool checked) {
         emit showChart(model->msg_id, sig, checked, QGuiApplication::keyboardModifiers() & Qt::ShiftModifier);
       });

tools/cabana/signalview.h

@@ -41,10 +41,7 @@ public:
   bool setData(const QModelIndex &index, const QVariant &value, int role = Qt::EditRole) override;
   void setMessage(const MessageId &id);
   void setFilter(const QString &txt);
-  void addSignal(int start_bit, int size, bool little_endian);
   bool saveSignal(const cabana::Signal *origin_s, cabana::Signal &s);
-  void resizeSignal(const cabana::Signal *sig, int start_bit, int size);
-  void removeSignal(const cabana::Signal *sig);
   Item *getItem(const QModelIndex &index) const;
   int signalRow(const cabana::Signal *sig) const;
   void showExtraInfo(const QModelIndex &index);

tools/cabana/tools/findsignal.cc

@@ -231,7 +231,9 @@ void FindSignalDlg::setInitialSignals() {
         for (int size = min_size->value(); size <= max_size->value(); ++size) {
           for (int start = 0; start <= total_size - size; ++start) {
             FindSignalModel::SearchSignal s{.id = it.key(), .mono_time = first_time, .sig = sig};
-            updateSigSizeParamsFromRange(s.sig, start, size);
+            s.sig.start_bit = start;
+            s.sig.size = size;
+            updateMsbLsb(s.sig);
             s.value = get_raw_value((*e)->dat, (*e)->size, s.sig);
             model->initial_signals.push_back(s);
           }
@@ -258,12 +260,6 @@ void FindSignalDlg::customMenuRequested(const QPoint &pos) {
     menu.addAction(tr("Create Signal"));
     if (menu.exec(view->mapToGlobal(pos))) {
       auto &s = model->filtered_signals[index.row()];
-      auto msg = dbc()->msg(s.id);
-      if (!msg) {
-        UndoStack::push(new EditMsgCommand(s.id, dbc()->newMsgName(s.id), can->lastMessage(s.id).dat.size(), ""));
-        msg = dbc()->msg(s.id);
-      }
-      s.sig.name = dbc()->newSignalName(s.id);
       UndoStack::push(new AddSigCommand(s.id, s.sig));
       emit openMessage(s.id);
     }

tools/cabana/util.cc

@@ -2,11 +2,9 @@
 
 #include <QColor>
 #include <QFontDatabase>
-#include <QHelpEvent>
 #include <QLocale>
 #include <QPainter>
 #include <QPixmapCache>
-#include <QToolTip>
 
 #include "selfdrive/ui/qt/util.h"