From d058968e0eddeffb2074fb1a3f8e4b78a5fbf255 Mon Sep 17 00:00:00 2001 From: Viktor Kopp <vifactor@gmail.com> Date: Sun, 17 Nov 2024 22:08:51 +0100 Subject: [PATCH 1/4] Various code fixes - Fix couple of warnings - Minor cleanup of dead code - Const-correctness for a couple of funcs Signed-off-by: Viktor Kopp <vifactor@gmail.com> --- src/dltfileindexer.cpp | 10 +--------- src/dltfileindexer.h | 2 +- src/mainwindow.cpp | 25 ++++++++++++------------- src/mainwindow.h | 7 +++---- 4 files changed, 17 insertions(+), 27 deletions(-) diff --git a/src/dltfileindexer.cpp b/src/dltfileindexer.cpp index f85f9a93..9013eb97 100644 --- a/src/dltfileindexer.cpp +++ b/src/dltfileindexer.cpp @@ -151,7 +151,6 @@ bool DltFileIndexer::index(int num) quint8 version=1; qint64 lengthOffset=2; qint64 storageLength=0; - int iPercent =0; errors_in_file = 0; char *data = new char[DLT_FILE_INDEXER_SEG_SIZE]; @@ -378,7 +377,6 @@ bool DltFileIndexer::indexFilter(QStringList filenames) { QSharedPointer<QDltMsg> msg; QDltFilterList filterList; - QTime time; quint64 ix = 0; unsigned int iPercent = 0; @@ -429,12 +427,6 @@ bool DltFileIndexer::indexFilter(QStringList filenames) return true; } - unsigned int modvalue = dltFile->size()/20; - if (modvalue == 0) // avoid divison by zero - { - modvalue = 1; - } - // Initialise progress bar emit(progressText(QString("CFI %1/%2").arg(currentRun).arg(maxRun))); emit(progressMax(100)); @@ -470,7 +462,7 @@ bool DltFileIndexer::indexFilter(QStringList filenames) qDebug() << "Create filter index: Start"; /* init fileprogress */ - int progressCounter = 1; + unsigned int progressCounter = 1; emit progress(0); // Start reading messages diff --git a/src/dltfileindexer.h b/src/dltfileindexer.h index 3ba992bf..0951bfae 100644 --- a/src/dltfileindexer.h +++ b/src/dltfileindexer.h @@ -128,7 +128,7 @@ class DltFileIndexer : public QThread // get index of all messages QVector<qint64> getIndexAll() { return indexAllList; } QVector<qint64> getIndexFilters() { return indexFilterList; } - QList<int> getGetLogInfoList() { return getLogInfoList; } + const QList<int>& getGetLogInfoList() { return getLogInfoList; } // let worker thread append to getLogInfoList void appendToGetLogInfoList(int value); diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 100ee7da..f2cc36a6 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -685,8 +685,8 @@ void MainWindow::initFileHandling() ui->checkBoxSortByTimestamp->setEnabled(ui->filtersEnabled->isChecked()); ui->checkBoxSortByTimestamp->setChecked(QDltSettingsManager::getInstance()->value("startup/sortByTimestampEnabled", false).toBool()); ui->checkBoxFilterRange->setEnabled(ui->filtersEnabled->isChecked()); - ui->lineEditFilterStart->setEnabled(ui->checkBoxFilterRange->isChecked() & ui->filtersEnabled->isChecked()); - ui->lineEditFilterEnd->setEnabled(ui->checkBoxFilterRange->isChecked() & ui->filtersEnabled->isChecked()); + ui->lineEditFilterStart->setEnabled(ui->checkBoxFilterRange->isChecked() && ui->filtersEnabled->isChecked()); + ui->lineEditFilterEnd->setEnabled(ui->checkBoxFilterRange->isChecked() && ui->filtersEnabled->isChecked()); /* Process Project */ if(QDltOptManager::getInstance()->isProjectFile()) @@ -1895,7 +1895,7 @@ void MainWindow::on_action_menuFile_Clear_triggered() return; } -void MainWindow::contextLoadingFile(QDltMsg &msg) +void MainWindow::contextLoadingFile(const QDltMsg &msg) { /* analyse message, check if DLT control message response */ if ( (msg.getType()==QDltMsg::DltTypeControl) && (msg.getSubtype()==QDltMsg::DltControlResponse)) @@ -1931,7 +1931,7 @@ void MainWindow::contextLoadingFile(QDltMsg &msg) } - controlMessage_ReceiveControlMessage(ecuitemFound,msg); + controlMessage_ReceiveControlMessage(ecuitemFound, msg); } } @@ -2058,15 +2058,14 @@ void MainWindow::reloadLogFileFinishFilter() m_searchtableModel->modelChanged(); // process getLogInfoMessages - if(( dltIndexer->getMode() == DltFileIndexer::modeIndexAndFilter) && settings->updateContextLoadingFile) - { - QList<int> list = dltIndexer->getGetLogInfoList(); - QDltMsg msg; + if ((dltIndexer->getMode() == DltFileIndexer::modeIndexAndFilter) && + settings->updateContextLoadingFile) { + const QList<int> &msgIndexList = dltIndexer->getGetLogInfoList(); // FIXME: this is slow operation running in the main loop - for(int num=0;num<list.size();num++) - { - if(qfile.getMsg(list[num],msg)) + QDltMsg msg; + for (const auto msgIndex : msgIndexList) { + if (qfile.getMsg(msgIndex, msg)) contextLoadingFile(msg); } } @@ -4461,7 +4460,7 @@ void MainWindow::onSearchresultsTableSelectionChanged(const QItemSelection & sel } } -void MainWindow::controlMessage_ReceiveControlMessage(EcuItem *ecuitem, QDltMsg &msg) +void MainWindow::controlMessage_ReceiveControlMessage(EcuItem *ecuitem, const QDltMsg &msg) { const char *ptr; int32_t length; @@ -6399,7 +6398,7 @@ void MainWindow::updatePlugin(PluginItem *item) } } -void MainWindow::versionString(QDltMsg &msg) +void MainWindow::versionString(const QDltMsg &msg) { // get the version string from the version message // Skip the ServiceID, Status and Length bytes and start from the String containing the ECU Software Version diff --git a/src/mainwindow.h b/src/mainwindow.h index 91c62be5..4abd3cd2 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -273,7 +273,7 @@ class MainWindow : public QMainWindow void controlMessage_SetTimingPackets(EcuItem* ecuitem, bool enable); void controlMessage_GetSoftwareVersion(EcuItem* ecuitem); void controlMessage_GetLogInfo(EcuItem* ecuitem); - void controlMessage_ReceiveControlMessage(EcuItem *ecuitem,QDltMsg &msg); + void controlMessage_ReceiveControlMessage(EcuItem *ecuitem, const QDltMsg &msg); void controlMessage_SetContext(EcuItem *ecuitem, QString apid, QString ctid,QString ctdescription,int log_level,int trace_status); void controlMessage_SetApplication(EcuItem *ecuitem, QString apid, QString appdescription); void controlMessage_Marker(); @@ -286,8 +286,8 @@ class MainWindow : public QMainWindow void updatePluginsECUList(); void updatePlugins(); void updatePlugin(PluginItem *item); - void contextLoadingFile(QDltMsg &msg); - void versionString(QDltMsg &msg); + void contextLoadingFile(const QDltMsg &msg); + void versionString(const QDltMsg &msg); void pluginsAutoload(QString version); void connectECU(EcuItem *ecuitem,bool force = false); @@ -633,7 +633,6 @@ public slots: signals: void dltFileLoaded(const QStringList& paths); - }; #endif // MAINWINDOW_H From b174c2bd1b240a889ffa03cf069b55db87f71337 Mon Sep 17 00:00:00 2001 From: Viktor Kopp <vifactor@gmail.com> Date: Fri, 22 Nov 2024 21:23:08 +0100 Subject: [PATCH 2/4] Remove redundant check We are looping over messages which satisfy this condition since they were filtered in indexer, see DltFileIndexerThread::processMessage method Signed-off-by: Viktor Kopp <vifactor@gmail.com> --- src/mainwindow.cpp | 48 +++++++++++++++++++++------------------------- 1 file changed, 22 insertions(+), 26 deletions(-) diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index f2cc36a6..d355b17c 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -1897,42 +1897,38 @@ void MainWindow::on_action_menuFile_Clear_triggered() void MainWindow::contextLoadingFile(const QDltMsg &msg) { - /* analyse message, check if DLT control message response */ - if ( (msg.getType()==QDltMsg::DltTypeControl) && (msg.getSubtype()==QDltMsg::DltControlResponse)) + /* find ecu item */ + EcuItem *ecuitemFound = 0; + for(int num = 0; num < project.ecu->topLevelItemCount (); num++) { - /* find ecu item */ - EcuItem *ecuitemFound = 0; - for(int num = 0; num < project.ecu->topLevelItemCount (); num++) + EcuItem *ecuitem = (EcuItem*)project.ecu->topLevelItem(num); + if(ecuitem->id == msg.getEcuid()) { - EcuItem *ecuitem = (EcuItem*)project.ecu->topLevelItem(num); - if(ecuitem->id == msg.getEcuid()) - { - ecuitemFound = ecuitem; - break; - } + ecuitemFound = ecuitem; + break; } + } - if(!ecuitemFound) - { - /* no Ecuitem found, create a new one */ - ecuitemFound = new EcuItem(0); - - /* update ECU item */ - ecuitemFound->id = msg.getEcuid(); - ecuitemFound->update(); + if(!ecuitemFound) + { + /* no Ecuitem found, create a new one */ + ecuitemFound = new EcuItem(0); - /* add ECU to configuration */ - project.ecu->addTopLevelItem(ecuitemFound); + /* update ECU item */ + ecuitemFound->id = msg.getEcuid(); + ecuitemFound->update(); - /* Update the ECU list in control plugins */ - updatePluginsECUList(); + /* add ECU to configuration */ + project.ecu->addTopLevelItem(ecuitemFound); - pluginManager.stateChanged(project.ecu->indexOfTopLevelItem(ecuitemFound), QDltConnection::QDltConnectionOffline,ecuitemFound->getHostname()); + /* Update the ECU list in control plugins */ + updatePluginsECUList(); - } + pluginManager.stateChanged(project.ecu->indexOfTopLevelItem(ecuitemFound), QDltConnection::QDltConnectionOffline,ecuitemFound->getHostname()); - controlMessage_ReceiveControlMessage(ecuitemFound, msg); } + + controlMessage_ReceiveControlMessage(ecuitemFound, msg); } void MainWindow::reloadLogFileStop() From fccc90ab5d5f3124879d5b6b01bfe3b22bbb9202 Mon Sep 17 00:00:00 2001 From: Viktor Kopp <vifactor@gmail.com> Date: Fri, 22 Nov 2024 21:32:38 +0100 Subject: [PATCH 3/4] Prettify a bit controlMessage_ReceiveControlMessage method Signed-off-by: Viktor Kopp <vifactor@gmail.com> --- src/mainwindow.cpp | 55 ++++++++++------------------------------------ 1 file changed, 11 insertions(+), 44 deletions(-) diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index d355b17c..04e95953 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -4466,21 +4466,21 @@ void MainWindow::controlMessage_ReceiveControlMessage(EcuItem *ecuitem, const QD length = payload.size(); /* control message was received */ - uint32_t service_id=0, service_id_tmp=0; + uint32_t service_id_tmp=0; DLT_MSG_READ_VALUE(service_id_tmp,ptr,length,uint32_t); - service_id=DLT_ENDIAN_GET_32( ((msg.getEndianness()==QDltMsg::DltEndiannessBigEndian)?DLT_HTYP_MSBF:0), service_id_tmp); - - /* check if plugin autoload enabled and - * it is a version message and - version string not already parsed */ - if(service_id == DLT_SERVICE_ID_GET_SOFTWARE_VERSION && (false == autoloadPluginsVersionEcus.contains(msg.getEcuid()))) - { - versionString(msg); - autoloadPluginsVersionEcus.append(msg.getEcuid()); - } + uint32_t service_id=DLT_ENDIAN_GET_32( ((msg.getEndianness()==QDltMsg::DltEndiannessBigEndian)?DLT_HTYP_MSBF:0), service_id_tmp); switch (service_id) { + case DLT_SERVICE_ID_GET_SOFTWARE_VERSION: + { + // check if plugin autoload enabled and version string not already parsed + if(!autoloadPluginsVersionEcus.contains(msg.getEcuid())) + { + versionString(msg); + autoloadPluginsVersionEcus.append(msg.getEcuid()); + } + } case DLT_SERVICE_ID_GET_LOG_INFO: { /* Only status 1,2,6,7,8 is supported yet! */ @@ -4559,14 +4559,6 @@ void MainWindow::controlMessage_ReceiveControlMessage(EcuItem *ecuitem, const QD } } - char com_interface[DLT_ID_SIZE]; - DLT_MSG_READ_ID(com_interface,ptr,length); - - if (length<0) - { - // wxMessageBox(_("Control Message corrupted!"),_("Receive Control Message")); - } - break; } case DLT_SERVICE_ID_GET_DEFAULT_LOG_LEVEL: @@ -4603,31 +4595,6 @@ void MainWindow::controlMessage_ReceiveControlMessage(EcuItem *ecuitem, const QD } case DLT_SERVICE_ID_SET_LOG_LEVEL: { - uint8_t status=0; - DLT_MSG_READ_VALUE(status,ptr,length,uint8_t); /* No endian conversion necessary */ - - switch (status) - { - case 0: /* OK */ - { - //conitem->status = EcuItem::valid; - } - break; - case 1: /* NOT_SUPPORTED */ - { - //conitem->status = EcuItem::unknown; - } - break; - case 2: /* ERROR */ - { - //conitem->status = EcuItem::invalid; - } - break; - } - - /* update status*/ - //conitem->update(); - break; } case DLT_SERVICE_ID_TIMEZONE: From 5081dc50768e167db887aa19e2a39db6e2714475 Mon Sep 17 00:00:00 2001 From: Viktor Kopp <vifactor@gmail.com> Date: Mon, 25 Nov 2024 21:07:49 +0100 Subject: [PATCH 4/4] Introduce EcuItem::find method to accelerate search Use new EcuItem::find method where possible Signed-off-by: Viktor Kopp <vifactor@gmail.com> --- src/mainwindow.cpp | 116 ++++++++++++++++----------------------------- src/project.cpp | 9 ++++ src/project.h | 4 ++ 3 files changed, 53 insertions(+), 76 deletions(-) diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 04e95953..802978e7 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -5499,89 +5499,56 @@ void MainWindow::on_action_menuDLT_Send_Injection_triggered() void MainWindow::controlMessage_SetApplication(EcuItem *ecuitem, QString apid, QString appdescription) { - /* Try to find App */ - for(int numapp = 0; numapp < ecuitem->childCount(); numapp++) - { - ApplicationItem * appitem = (ApplicationItem *) ecuitem->child(numapp); - - if(appitem->id == apid) - { - appitem->description = appdescription; - appitem->update(); - return; - } + if (auto appitem = ecuitem->find(apid); appitem) { + appitem->description = appdescription; + appitem->update(); + } else { + appitem = new ApplicationItem(ecuitem); + appitem->id = apid; + appitem->description = appdescription; + appitem->update(); + ecuitem->addChild(appitem); } - - /* No app and no con found */ - ApplicationItem* appitem = new ApplicationItem(ecuitem); - appitem->id = apid; - appitem->description = appdescription; - appitem->update(); - ecuitem->addChild(appitem); - } void MainWindow::controlMessage_SetContext(EcuItem *ecuitem, QString apid, QString ctid,QString ctdescription,int log_level,int trace_status) { - /* First try to find existing context */ - //qDebug() << "New CTX for" << apid << ctid << ctdescription; - - for(int numapp = 0; numapp < ecuitem->childCount(); numapp++) - { - ApplicationItem * appitem = (ApplicationItem *) ecuitem->child(numapp); + if (auto appitem = ecuitem->find(apid); appitem) { - for(int numcontext = 0; numcontext < appitem->childCount(); numcontext++) - { - ContextItem * conitem = (ContextItem *) appitem->child(numcontext); - - if(appitem->id == apid && conitem->id == ctid) - { - /* set new log level and trace status */ - conitem->loglevel = log_level; - conitem->tracestatus = trace_status; - conitem->description = ctdescription; - conitem->status = ContextItem::valid; - conitem->update(); - return; + ContextItem *conitem = nullptr; + for (int numcontext = 0; numcontext < appitem->childCount(); numcontext++) { + ContextItem *currconitem = (ContextItem *)appitem->child(numcontext); + if (currconitem->id == ctid) { + conitem = currconitem; } } - } - - /* Try to find App */ - for(int numapp = 0; numapp < ecuitem->childCount(); numapp++) - { - ApplicationItem * appitem = (ApplicationItem *) ecuitem->child(numapp); - if(appitem->id == apid) - { - /* Add new context */ - ContextItem* conitem = new ContextItem(appitem); - conitem->id = ctid; - conitem->loglevel = log_level; - conitem->tracestatus = trace_status; - conitem->description = ctdescription; - conitem->status = ContextItem::valid; - conitem->update(); + if (!conitem) { + conitem = new ContextItem(appitem); appitem->addChild(conitem); - - return; } + conitem->id = ctid; + conitem->loglevel = log_level; + conitem->tracestatus = trace_status; + conitem->description = ctdescription; + conitem->status = ContextItem::valid; + conitem->update(); + } else { + appitem = new ApplicationItem(ecuitem); + appitem->id = apid; + appitem->description = ""; + appitem->update(); + ecuitem->addChild(appitem); + + ContextItem* conitem = new ContextItem(appitem); + conitem->id = ctid; + conitem->loglevel = log_level; + conitem->tracestatus = trace_status; + conitem->description = ctdescription; + conitem->status = ContextItem::valid; + conitem->update(); + appitem->addChild(conitem); } - - /* No app and no con found */ - ApplicationItem* appitem = new ApplicationItem(ecuitem); - appitem->id = apid; - appitem->description = QString(""); - appitem->update(); - ecuitem->addChild(appitem); - ContextItem* conitem = new ContextItem(appitem); - conitem->id = ctid; - conitem->loglevel = log_level; - conitem->tracestatus = trace_status; - conitem->description = ctdescription; - conitem->status = ContextItem::valid; - conitem->update(); - appitem->addChild(conitem); } void MainWindow::controlMessage_Timezone(int timezone, unsigned char dst) @@ -5614,15 +5581,12 @@ void MainWindow::controlMessage_UnregisterContext(QString ecuId,QString appId,QS return; /* First try to find existing context */ - for(int numapp = 0; numapp < ecuitemFound->childCount(); numapp++) + if(auto appitem = ecuitemFound->find(appId); appitem) { - ApplicationItem * appitem = (ApplicationItem *) ecuitemFound->child(numapp); - for(int numcontext = 0; numcontext < appitem->childCount(); numcontext++) { ContextItem * conitem = (ContextItem *) appitem->child(numcontext); - - if(appitem->id == appId && conitem->id == ctId) + if(conitem->id == ctId) { /* remove context */ delete conitem->parent()->takeChild(conitem->parent()->indexOfChild(conitem)); diff --git a/src/project.cpp b/src/project.cpp index cc4f2e98..38c37443 100644 --- a/src/project.cpp +++ b/src/project.cpp @@ -250,6 +250,15 @@ void EcuItem::updateAutoReconnectTimestamp() //qDebug() << "updateAutoReconnectTimestamp" << autoReconnectTimestamp; } +ApplicationItem *EcuItem::find(const QString &apid) const { + for (int numapp = 0; numapp < childCount(); numapp++) { + ApplicationItem *appitem = static_cast<ApplicationItem *>(child(numapp)); + if (appitem->id == apid) + return appitem; + } + return nullptr; +} + ApplicationItem::ApplicationItem(QTreeWidgetItem *parent) : QTreeWidgetItem(parent,application_type) { diff --git a/src/project.h b/src/project.h index 46842089..0c0b6dea 100644 --- a/src/project.h +++ b/src/project.h @@ -42,6 +42,8 @@ enum dlt_item_type { ecu_type = QTreeWidgetItem::UserType, application_type, context_type, filter_type, plugin_type }; +class ApplicationItem; + class EcuItem : public QTreeWidgetItem { public: @@ -99,6 +101,8 @@ class EcuItem : public QTreeWidgetItem bool isAutoReconnectTimeoutPassed(); void updateAutoReconnectTimestamp(); + ApplicationItem* find(const QString& apid) const; + private: QDateTime autoReconnectTimestamp; bool operator< ( const QTreeWidgetItem & other ) const;