From c2e86afc8ca2a7f96c0f1e1d93ec64805226d69c Mon Sep 17 00:00:00 2001 From: Dean Lee Date: Fri, 26 May 2023 04:38:58 +0800 Subject: [PATCH] cabana: support editing comments for messages (#28299) old-commit-hash: 54506774f05a867ff0f6ab4ab494eeec792f838f --- tools/cabana/commands.cc | 11 ++++++----- tools/cabana/commands.h | 4 ++-- tools/cabana/dbc/dbc.h | 1 + tools/cabana/dbc/dbcfile.cc | 19 +++++++++++++++---- tools/cabana/dbc/dbcfile.h | 2 +- tools/cabana/dbc/dbcmanager.cc | 4 ++-- tools/cabana/dbc/dbcmanager.h | 2 +- tools/cabana/detailwidget.cc | 7 ++++++- tools/cabana/detailwidget.h | 2 ++ tools/cabana/messageswidget.cc | 5 +++++ tools/cabana/signalview.cc | 2 +- tools/cabana/tools/findsignal.cc | 2 +- tools/cabana/util.cc | 11 ----------- tools/cabana/util.h | 1 - 14 files changed, 43 insertions(+), 30 deletions(-) diff --git a/tools/cabana/commands.cc b/tools/cabana/commands.cc index 337de1c702..d64e173338 100644 --- a/tools/cabana/commands.cc +++ b/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); } diff --git a/tools/cabana/commands.h b/tools/cabana/commands.h index 84d1a6e2a1..2552115229 100644 --- a/tools/cabana/commands.h +++ b/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; }; diff --git a/tools/cabana/dbc/dbc.h b/tools/cabana/dbc/dbc.h index d074987f50..b43d9cdecc 100644 --- a/tools/cabana/dbc/dbc.h +++ b/tools/cabana/dbc/dbc.h @@ -64,6 +64,7 @@ namespace cabana { uint32_t address; QString name; uint32_t size; + QString comment; QList sigs; QList mask; diff --git a/tools/cabana/dbc/dbcfile.cc b/tools/cabana/dbc/dbcfile.cc index 2b270fc30f..4679831ddc 100644 --- a/tools/cabana/dbc/dbcfile.cc +++ b/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; } diff --git a/tools/cabana/dbc/dbcfile.h b/tools/cabana/dbc/dbcfile.h index 9f140f44d9..1ed9e9cd0a 100644 --- a/tools/cabana/dbc/dbcfile.h +++ b/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); diff --git a/tools/cabana/dbc/dbcmanager.cc b/tools/cabana/dbc/dbcmanager.cc index 000035bd2c..2176635caa 100644 --- a/tools/cabana/dbc/dbcmanager.cc +++ b/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}); diff --git a/tools/cabana/dbc/dbcmanager.h b/tools/cabana/dbc/dbcmanager.h index 0d7055588b..ba590ee56e 100644 --- a/tools/cabana/dbc/dbcmanager.h +++ b/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); diff --git a/tools/cabana/detailwidget.cc b/tools/cabana/detailwidget.cc index c05b76c878..92851761f9 100644 --- a/tools/cabana/detailwidget.cc +++ b/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); diff --git a/tools/cabana/detailwidget.h b/tools/cabana/detailwidget.h index c4406bd39a..10c6a6c46e 100644 --- a/tools/cabana/detailwidget.h +++ b/tools/cabana/detailwidget.h @@ -3,6 +3,7 @@ #include #include #include +#include #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; }; diff --git a/tools/cabana/messageswidget.cc b/tools/cabana/messageswidget.cc index 1d8499fa41..2b591a26d7 100644 --- a/tools/cabana/messageswidget.cc +++ b/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 += "
" + msg->comment + ""; + return tooltip; } return {}; } diff --git a/tools/cabana/signalview.cc b/tools/cabana/signalview.cc index 0b74455d7c..b501f14a7c 100644 --- a/tools/cabana/signalview.cc +++ b/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); } diff --git a/tools/cabana/tools/findsignal.cc b/tools/cabana/tools/findsignal.cc index bc969356d4..7b4a5633f6 100644 --- a/tools/cabana/tools/findsignal.cc +++ b/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); diff --git a/tools/cabana/util.cc b/tools/cabana/util.cc index d95d6ac525..a75d10139d 100644 --- a/tools/cabana/util.cc +++ b/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()) { diff --git a/tools/cabana/util.h b/tools/cabana/util.h index dcf1ac0cbf..3955ab82c5 100644 --- a/tools/cabana/util.h +++ b/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; }