cabana: support editing comments for messages (#28299)

old-commit-hash: 54506774f05a867ff0f6ab4ab494eeec792f838f

tools/cabana/commands.cc

@@ -4,11 +4,12 @@
 
 // EditMsgCommand
 
-EditMsgCommand::EditMsgCommand(const MessageId &id, const QString &name, int size, QUndoCommand *parent)
-    : id(id), new_name(name), new_size(size), QUndoCommand(parent) {
+EditMsgCommand::EditMsgCommand(const MessageId &id, const QString &name, int size, const QString &comment, QUndoCommand *parent)
+    : id(id), new_name(name), new_size(size), new_comment(comment), QUndoCommand(parent) {
   if (auto msg = dbc()->msg(id)) {
     old_name = msg->name;
     old_size = msg->size;
+    old_comment = msg->comment;
     setText(QObject::tr("edit message %1:%2").arg(name).arg(id.address));
   } else {
     setText(QObject::tr("new message %1:%2").arg(name).arg(id.address));
@@ -19,11 +20,11 @@ void EditMsgCommand::undo() {
   if (old_name.isEmpty())
     dbc()->removeMsg(id);
   else
-    dbc()->updateMsg(id, old_name, old_size);
+    dbc()->updateMsg(id, old_name, old_size, old_comment);
 }
 
 void EditMsgCommand::redo() {
-  dbc()->updateMsg(id, new_name, new_size);
+  dbc()->updateMsg(id, new_name, new_size, new_comment);
 }
 
 // RemoveMsgCommand
@@ -37,7 +38,7 @@ RemoveMsgCommand::RemoveMsgCommand(const MessageId &id, QUndoCommand *parent) :
 
 void RemoveMsgCommand::undo() {
   if (!message.name.isEmpty()) {
-    dbc()->updateMsg(id, message.name, message.size);
+    dbc()->updateMsg(id, message.name, message.size, message.comment);
     for (auto s : message.getSignals())
       dbc()->addSignal(id, *s);
   }

tools/cabana/commands.h

@@ -8,13 +8,13 @@
 
 class EditMsgCommand : public QUndoCommand {
 public:
-  EditMsgCommand(const MessageId &id, const QString &name, int size, QUndoCommand *parent = nullptr);
+  EditMsgCommand(const MessageId &id, const QString &name, int size, const QString &comment, QUndoCommand *parent = nullptr);
   void undo() override;
   void redo() override;
 
 private:
   const MessageId id;
-  QString old_name, new_name;
+  QString old_name, new_name, old_comment, new_comment;
   int old_size = 0, new_size = 0;
 };
 

tools/cabana/dbc/dbc.h

@@ -64,6 +64,7 @@ namespace cabana {
     uint32_t address;
     QString name;
     uint32_t size;
+    QString comment;
     QList<cabana::Signal> sigs;
 
     QList<uint8_t> mask;

tools/cabana/dbc/dbcfile.cc

@@ -125,11 +125,12 @@ void DBCFile::removeSignal(const MessageId &id, const QString &sig_name) {
   }
 }
 
-void DBCFile::updateMsg(const MessageId &id, const QString &name, uint32_t size) {
+void DBCFile::updateMsg(const MessageId &id, const QString &name, uint32_t size, const QString &comment) {
   auto &m = msgs[id.address];
   m.address = id.address;
   m.name = name;
   m.size = size;
+  m.comment = comment;
 }
 
 void DBCFile::removeMsg(const MessageId &id) {
@@ -193,6 +194,7 @@ void DBCFile::parseExtraInfo(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]+)\] \"(.*)\" (.*))");
   static QRegularExpression sgm_regexp(R"(^SG_ (\w+) (\w+) *: (\d+)\|(\d+)@(\d+)([\+|\-]) \(([0-9.+\-eE]+),([0-9.+\-eE]+)\) \[([0-9.+\-eE]+)\|([0-9.+\-eE]+)\] \"(.*)\" (.*))");
+  static QRegularExpression msg_comment_regexp(R"(^CM_ BO_ *(\w+) *\"(.*)\";)");
   static QRegularExpression sg_comment_regexp(R"(^CM_ SG_ *(\w+) *(\w+) *\"(.*)\";)");
   static QRegularExpression val_regexp(R"(VAL_ (\w+) (\w+) (.*);)");
   auto get_sig = [this](uint32_t address, const QString &name) -> cabana::Signal * {
@@ -235,6 +237,12 @@ void DBCFile::parseExtraInfo(const QString &content) {
           }
         }
       }
+    } else if (line.startsWith("CM_ BO_")) {
+      if (auto match = msg_comment_regexp.match(line); match.hasMatch()) {
+        if (auto m = (cabana::Msg *)msg(match.captured(1).toUInt())) {
+          m->comment = match.captured(2).trimmed();
+        }
+      }
     } else if (line.startsWith("CM_ SG_ ")) {
       if (auto match = sg_comment_regexp.match(line); match.hasMatch()) {
         if (auto s = get_sig(match.captured(1).toUInt(), match.captured(2))) {
@@ -246,9 +254,12 @@ void DBCFile::parseExtraInfo(const QString &content) {
 }
 
 QString DBCFile::generateDBC() {
-  QString dbc_string, signal_comment, val_desc;
-  for (auto &[address, m] : msgs) {
+  QString dbc_string, signal_comment, message_comment, val_desc;
+  for (const auto &[address, m] : msgs) {
     dbc_string += QString("BO_ %1 %2: %3 XXX\n").arg(address).arg(m.name).arg(m.size);
+    if (!m.comment.isEmpty()) {
+      message_comment += QString("CM_ BO_ %1 \"%2\";\n").arg(address).arg(m.comment);
+    }
     for (auto sig : m.getSignals()) {
       dbc_string += QString(" SG_ %1 : %2|%3@%4%5 (%6,%7) [%8|%9] \"%10\" XXX\n")
                         .arg(sig->name)
@@ -274,5 +285,5 @@ QString DBCFile::generateDBC() {
     }
     dbc_string += "\n";
   }
-  return dbc_string + signal_comment + val_desc;
+  return dbc_string + message_comment + signal_comment + val_desc;
 }

tools/cabana/dbc/dbcfile.h

@@ -31,7 +31,7 @@ public:
   cabana::Signal *getSignal(const MessageId &id, const QString &sig_name);
   void removeSignal(const MessageId &id, const QString &sig_name);
 
-  void updateMsg(const MessageId &id, const QString &name, uint32_t size);
+  void updateMsg(const MessageId &id, const QString &name, uint32_t size, const QString &comment);
   void removeMsg(const MessageId &id);
 
   QString newMsgName(const MessageId &id);

tools/cabana/dbc/dbcmanager.cc

@@ -144,12 +144,12 @@ void DBCManager::removeSignal(const MessageId &id, const QString &sig_name) {
   }
 }
 
-void DBCManager::updateMsg(const MessageId &id, const QString &name, uint32_t size) {
+void DBCManager::updateMsg(const MessageId &id, const QString &name, uint32_t size, const QString &comment) {
   auto sources_dbc_file = findDBCFile(id);
   assert(sources_dbc_file); // This should be impossible
   auto [dbc_sources, dbc_file] = *sources_dbc_file;
 
-  dbc_file->updateMsg(id, name, size);
+  dbc_file->updateMsg(id, name, size, comment);
 
   for (uint8_t source : dbc_sources) {
     emit msgUpdated({.source = source, .address = id.address});

tools/cabana/dbc/dbcmanager.h

@@ -33,7 +33,7 @@ public:
   void updateSignal(const MessageId &id, const QString &sig_name, const cabana::Signal &sig);
   void removeSignal(const MessageId &id, const QString &sig_name);
 
-  void updateMsg(const MessageId &id, const QString &name, uint32_t size);
+  void updateMsg(const MessageId &id, const QString &name, uint32_t size, const QString &comment);
   void removeMsg(const MessageId &id);
 
   QString newMsgName(const MessageId &id);

tools/cabana/detailwidget.cc

@@ -167,7 +167,7 @@ void DetailWidget::editMsg() {
   int size = msg ? msg->size : can->lastMessage(msg_id).dat.size();
   EditMessageDialog dlg(msg_id, msgName(msg_id), size, this);
   if (dlg.exec()) {
-    UndoStack::push(new EditMsgCommand(msg_id, dlg.name_edit->text(), dlg.size_spin->value()));
+    UndoStack::push(new EditMsgCommand(msg_id, dlg.name_edit->text().trimmed(), dlg.size_spin->value(), dlg.comment_edit->toPlainText().trimmed()));
   }
 }
 
@@ -194,6 +194,11 @@ EditMessageDialog::EditMessageDialog(const MessageId &msg_id, const QString &tit
   size_spin->setValue(size);
   form_layout->addRow(tr("Size"), size_spin);
 
+  form_layout->addRow(tr("Comment"), comment_edit = new QTextEdit(this));
+  if (auto msg = dbc()->msg(msg_id)) {
+    comment_edit->setText(msg->comment);
+  }
+
   btn_box = new QDialogButtonBox(QDialogButtonBox::Ok | QDialogButtonBox::Cancel);
   validateName(name_edit->text());
   form_layout->addRow(btn_box);

tools/cabana/detailwidget.h

@@ -3,6 +3,7 @@
 #include <QDialogButtonBox>
 #include <QSplitter>
 #include <QTabWidget>
+#include <QTextEdit>
 
 #include "selfdrive/ui/qt/widgets/controls.h"
 #include "tools/cabana/binaryview.h"
@@ -20,6 +21,7 @@ public:
   QString original_name;
   QDialogButtonBox *btn_box;
   QLineEdit *name_edit;
+  QTextEdit *comment_edit;
   QLabel *error_label;
   QSpinBox *size_spin;
 };

tools/cabana/messageswidget.cc

@@ -192,6 +192,11 @@ QVariant MessageListModel::data(const QModelIndex &index, int role) const {
     return QVariant::fromValue(colors);
   } else if (role == BytesRole && index.column() == Column::DATA && id.source != INVALID_SOURCE) {
     return can_data.dat;
+  } else if (role == Qt::ToolTipRole && index.column() == Column::NAME) {
+    auto msg = dbc()->msg(id);
+    auto tooltip = msg ? msg->name : UNTITLED;
+    if (msg && !msg->comment.isEmpty()) tooltip += "<br /><span style=\"color:gray;\">" + msg->comment + "</span>";
+    return tooltip;
   }
   return {};
 }

tools/cabana/signalview.cc

@@ -226,7 +226,7 @@ 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()));
+    UndoStack::push(new EditMsgCommand(msg_id, name, can->lastMessage(msg_id).dat.size(), ""));
     msg = dbc()->msg(msg_id);
   }
 

tools/cabana/tools/findsignal.cc

@@ -262,7 +262,7 @@ void FindSignalDlg::customMenuRequested(const QPoint &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()));
+        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);

tools/cabana/util.cc

@@ -82,17 +82,6 @@ QSize MessageBytesDelegate::sizeHint(const QStyleOptionViewItem &option, const Q
   return size;
 }
 
-bool MessageBytesDelegate::helpEvent(QHelpEvent *e, QAbstractItemView *view, const QStyleOptionViewItem &option, const QModelIndex &index) {
-  if (e->type() == QEvent::ToolTip && index.column() == 0) {
-    if (view->visualRect(index).width() < QStyledItemDelegate::sizeHint(option, index).width()) {
-      QToolTip::showText(e->globalPos(), index.data(Qt::DisplayRole).toString(), view);
-      return true;
-    }
-  }
-  QToolTip::hideText();
-  return false;
-}
-
 void MessageBytesDelegate::paint(QPainter *painter, const QStyleOptionViewItem &option, const QModelIndex &index) const {
   auto data = index.data(BytesRole);
   if (!data.isValid()) {

tools/cabana/util.h

@@ -67,7 +67,6 @@ public:
   MessageBytesDelegate(QObject *parent, bool multiple_lines = false);
   void paint(QPainter *painter, const QStyleOptionViewItem &option, const QModelIndex &index) const override;
   QSize sizeHint(const QStyleOptionViewItem &option, const QModelIndex &index) const override;
-  bool helpEvent(QHelpEvent *event, QAbstractItemView *view, const QStyleOptionViewItem &option, const QModelIndex &index) override;
   void setMultipleLines(bool v);
   int widthForBytes(int n) const;
   bool multipleLines() const { return multiple_lines; }