Cabana: improve message sorting,filtering and updating. (#26396)

* optimize sort/filter/update

* helper function msgName

* cleanup

tools/cabana/canmessages.cc

@@ -62,8 +62,9 @@ void CANMessages::process(QHash<QString, CanData> *messages) {
   for (auto it = messages->begin(); it != messages->end(); ++it) {
     can_msgs[it.key()] = it.value();
   }
-  delete messages;
   emit updated();
+  emit msgsReceived(messages);
+  delete messages;
 }
 
 bool CANMessages::eventFilter(const Event *event) {

tools/cabana/canmessages.h

@@ -50,6 +50,7 @@ signals:
   void streamStarted();
   void eventsMerged();
   void updated();
+  void msgsReceived(const QHash<QString, CanData> *);
   void received(QHash<QString, CanData> *);
 
 public:

tools/cabana/dbcmanager.h

@@ -48,5 +48,8 @@ int bigEndianStartBitsIndex(int start_bit);
 int bigEndianBitIndex(int index);
 void updateSigSizeParamsFromRange(Signal &s, int from, int to);
 std::pair<int, int> getSignalRange(const Signal *s);
-
 DBCManager *dbc();
+inline QString msgName(const QString &id, const char *def = "untitled") {
+  auto msg = dbc()->msg(id);
+  return msg ? msg->name.c_str() : def;
+}

tools/cabana/detailwidget.cc

@@ -132,8 +132,7 @@ void DetailWidget::setMessage(const QString &message_id) {
   }
   if (index == -1) {
     index = tabbar->addTab(message_id);
-    auto msg = dbc()->msg(message_id);
-    tabbar->setTabToolTip(index, msg ? msg->name.c_str() : "untitled");
+    tabbar->setTabToolTip(index, msgName(message_id));
   }
   tabbar->setCurrentIndex(index);
   msg_id = message_id;
@@ -173,7 +172,7 @@ void DetailWidget::dbcMsgChanged(int show_form_idx) {
   }
 
   edit_btn->setVisible(true);
-  name_label->setText(msg ? msg->name.c_str() : "untitled");
+  name_label->setText(msgName(msg_id));
 
   binary_view->setMessage(msg_id);
   history_log->setMessage(msg_id);
@@ -212,7 +211,7 @@ void DetailWidget::updateChartState(const QString &id, const Signal *sig, bool o
 
 void DetailWidget::editMsg() {
   auto msg = dbc()->msg(msg_id);
-  QString name = msg ? msg->name.c_str() : "untitled";
+  QString name = msgName(msg_id);
   int size = msg ? msg->size : can->lastMessage(msg_id).dat.size();
   EditMessageDialog dlg(msg_id, name, size, this);
   if (dlg.exec()) {

tools/cabana/messageswidget.cc

@@ -35,13 +35,19 @@ MessagesWidget::MessagesWidget(QWidget *parent) : QWidget(parent) {
 
   // signals/slots
   QObject::connect(filter, &QLineEdit::textChanged, model, &MessageListModel::setFilterString);
-  QObject::connect(can, &CANMessages::updated, [this]() { model->updateState(); });
-  QObject::connect(dbc(), &DBCManager::DBCFileChanged, [this]() { model->updateState(true); });
+  QObject::connect(can, &CANMessages::msgsReceived, model, &MessageListModel::msgsReceived);
+  QObject::connect(dbc(), &DBCManager::DBCFileChanged, model, &MessageListModel::sortMessages);
+  QObject::connect(dbc(), &DBCManager::msgUpdated, model, &MessageListModel::sortMessages);
   QObject::connect(table_widget->selectionModel(), &QItemSelectionModel::currentChanged, [=](const QModelIndex &current, const QModelIndex &previous) {
-    if (current.isValid()) {
-      emit msgSelectionChanged(current.data(Qt::UserRole).toString());
+    if (current.isValid() && current.row() < model->msgs.size()) {
+      current_msg_id = model->msgs[current.row()];
+      emit msgSelectionChanged(current_msg_id);
     }
   });
+  QObject::connect(model, &MessageListModel::modelReset, [this]() {
+    if (int row = model->msgs.indexOf(current_msg_id); row != -1)
+      table_widget->selectionModel()->select(model->index(row, 0), QItemSelectionModel::Rows | QItemSelectionModel::ClearAndSelect);
+  });
 }
 
 // MessageListModel
@@ -54,103 +60,71 @@ QVariant MessageListModel::headerData(int section, Qt::Orientation orientation,
 
 QVariant MessageListModel::data(const QModelIndex &index, int role) const {
   if (role == Qt::DisplayRole) {
-    const auto &m = msgs[index.row()];
-    auto &can_data = can->lastMessage(m->id);
+    const auto &id = msgs[index.row()];
+    auto &can_data = can->lastMessage(id);
     switch (index.column()) {
-      case 0: return m->name;
-      case 1: return m->id;
+      case 0: return msgName(id);
+      case 1: return id;
       case 2: return can_data.freq;
       case 3: return can_data.count;
       case 4: return toHex(can_data.dat);
     }
-  } else if (role == Qt::UserRole) {
-    return msgs[index.row()]->id;
-  } else if (role == Qt::FontRole) {
-    if (index.column() == columnCount() - 1) {
+  } else if (role == Qt::FontRole && index.column() == columnCount() - 1) {
     return QFontDatabase::systemFont(QFontDatabase::FixedFont);
   }
-  }
   return {};
 }
 
 void MessageListModel::setFilterString(const QString &string) {
   filter_str = string;
-  updateState(true);
-}
-
-bool MessageListModel::updateMessages(bool sort) {
-  if (msgs.size() == can->can_msgs.size() && filter_str.isEmpty() && !sort)
-    return false;
-
-  // update message list
-  int i = 0;
   bool search_id = filter_str.contains(':');
+  msgs.clear();
   for (auto it = can->can_msgs.begin(); it != can->can_msgs.end(); ++it) {
-    const Msg *msg = dbc()->msg(it.key());
-    QString msg_name = msg ? msg->name.c_str() : "untitled";
-    if (!filter_str.isEmpty() && !(search_id ? it.key() : msg_name).contains(filter_str, Qt::CaseInsensitive))
-      continue;
-    auto &m = i < msgs.size() ? msgs[i] : msgs.emplace_back(new Message);
-    m->id = it.key();
-    m->name = msg_name;
-    ++i;
+    if ((search_id ? it.key() : msgName(it.key())).contains(filter_str, Qt::CaseInsensitive))
+      msgs.push_back(it.key());
   }
-  msgs.resize(i);
+  sortMessages();
+}
 
+void MessageListModel::sortMessages() {
+  beginResetModel();
   if (sort_column == 0) {
     std::sort(msgs.begin(), msgs.end(), [this](auto &l, auto &r) {
-      bool ret = l->name < r->name || (l->name == r->name && l->id < r->id);
+      bool ret = std::pair{msgName(l), l} < std::pair{msgName(r), r};
       return sort_order == Qt::AscendingOrder ? ret : !ret;
     });
   } else if (sort_column == 1) {
     std::sort(msgs.begin(), msgs.end(), [this](auto &l, auto &r) {
-      return sort_order == Qt::AscendingOrder ? l->id < r->id : l->id > r->id;
+      return sort_order == Qt::AscendingOrder ? l < r : l > r;
     });
   } else if (sort_column == 2) {
-    // sort by frequency
     std::sort(msgs.begin(), msgs.end(), [this](auto &l, auto &r) {
-      uint32_t lfreq = can->lastMessage(l->id).freq;
-      uint32_t rfreq = can->lastMessage(r->id).freq;
-      bool ret = lfreq < rfreq || (lfreq == rfreq && l->id < r->id);
+      bool ret = std::pair{can->lastMessage(l).freq, l} < std::pair{can->lastMessage(r).freq, r};
       return sort_order == Qt::AscendingOrder ? ret : !ret;
     });
   } else if (sort_column == 3) {
-    // sort by count
     std::sort(msgs.begin(), msgs.end(), [this](auto &l, auto &r) {
-      uint32_t lcount = can->lastMessage(l->id).count;
-      uint32_t rcount = can->lastMessage(r->id).count;
-      bool ret = lcount < rcount || (lcount == rcount && l->id < r->id);
+      bool ret = std::pair{can->lastMessage(l).count, l} < std::pair{can->lastMessage(r).count, r};
       return sort_order == Qt::AscendingOrder ? ret : !ret;
     });
   }
-  return true;
+  endResetModel();
 }
 
-void MessageListModel::updateState(bool sort) {
+void MessageListModel::msgsReceived(const QHash<QString, CanData> *new_msgs) {
   int prev_row_count = msgs.size();
-  auto prev_idx = persistentIndexList();
-  QString selected_msg_id = prev_idx.empty() ? "" : prev_idx[0].data(Qt::UserRole).toString();
-
-  bool msg_updated = updateMessages(sort);
-  int delta = msgs.size() - prev_row_count;
-  if (delta > 0) {
-    beginInsertRows({}, prev_row_count, msgs.size() - 1);
-    endInsertRows();
-  } else if (delta < 0) {
-    beginRemoveRows({}, msgs.size(), prev_row_count - 1);
-    endRemoveRows();
+  if (filter_str.isEmpty() && msgs.size() != can->can_msgs.size()) {
+    msgs = can->can_msgs.keys();
   }
-
-  if (!msgs.empty()) {
-    if (msg_updated && !prev_idx.isEmpty()) {
-      // keep selection
-      auto it = std::find_if(msgs.begin(), msgs.end(), [&](auto &m) { return m->id == selected_msg_id; });
-      if (it != msgs.end()) {
-        for (auto &idx : prev_idx)
-          changePersistentIndex(idx, index(std::distance(msgs.begin(), it), idx.column()));
+  if (msgs.size() != prev_row_count) {
+    sortMessages();
+    return;
   }
+  for (int i = 0; i < msgs.size(); ++i) {
+    if (new_msgs->contains(msgs[i])) {
+      for (int col = 2; col < columnCount(); ++col)
+        emit dataChanged(index(i, col), index(i, col), {Qt::DisplayRole});
     }
-    emit dataChanged(index(0, 0), index(msgs.size() - 1, columnCount() - 1), {Qt::DisplayRole});
   }
 }
 
@@ -158,6 +132,6 @@ void MessageListModel::sort(int column, Qt::SortOrder order) {
   if (column != columnCount() - 1) {
     sort_column = column;
     sort_order = order;
-    updateState(true);
+    sortMessages();
   }
 }

tools/cabana/messageswidget.h

@@ -4,6 +4,7 @@
 #include <QTableView>
 
 #include "tools/cabana/canmessages.h"
+
 class MessageListModel : public QAbstractTableModel {
 Q_OBJECT
 
@@ -14,16 +15,12 @@ public:
   QVariant data(const QModelIndex &index, int role = Qt::DisplayRole) const;
   int rowCount(const QModelIndex &parent = QModelIndex()) const override { return msgs.size(); }
   void sort(int column, Qt::SortOrder order = Qt::AscendingOrder) override;
-  void updateState(bool sort = false);
   void setFilterString(const QString &string);
+  void msgsReceived(const QHash<QString, CanData> *new_msgs = nullptr);
+  void sortMessages();
+  QStringList msgs;
 
 private:
-  bool updateMessages(bool sort);
-
-  struct Message {
-    QString id, name;
-  };
-  std::vector<std::unique_ptr<Message>> msgs;
   QString filter_str;
   int sort_column = 0;
   Qt::SortOrder sort_order = Qt::AscendingOrder;
@@ -34,11 +31,11 @@ class MessagesWidget : public QWidget {
 
 public:
   MessagesWidget(QWidget *parent);
-
 signals:
   void msgSelectionChanged(const QString &message_id);
 
 protected:
   QTableView *table_widget;
+  QString current_msg_id;
   MessageListModel *model;
 };