From 768ebf9966fbdef2cbdf91cc31d9cf56677b74a2 Mon Sep 17 00:00:00 2001 From: Ludovic LANGE Date: Tue, 27 Sep 2022 21:04:19 +0200 Subject: [PATCH 01/12] replace `QString::SkipEmptyParts` with `Qt::SkipEmptyParts` (qt6) `QString::SkipEmptyParts` is [deprecated since around Qt5.14](https://doc.qt.io/qt-5/qstring-obsolete.html), and has been removed in Qt6. We replace it with `Qt::SkipEmptyParts` for Qt >= 5.14 --- framesenderwindow.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/framesenderwindow.cpp b/framesenderwindow.cpp index 7029fa0..4d66317 100644 --- a/framesenderwindow.cpp +++ b/framesenderwindow.cpp @@ -898,7 +898,11 @@ void FrameSenderWindow::processCellChange(int line, int col) case 6: //Data bytes for (int i = 0; i < 8; i++) sendingData[line].payload().data()[i] = 0; +#if QT_VERSION >= QT_VERSION_CHECK( 5, 14, 0 ) + tokens = ui->tableSender->item(line, 6)->text().split(" ", Qt::SkipEmptyParts); +#else tokens = ui->tableSender->item(line, 6)->text().split(" ", QString::SkipEmptyParts); +#endif arr.clear(); arr.reserve(tokens.count()); for (int j = 0; j < tokens.count(); j++) From bcbe3d38bd981569cc7d57a2c27126b66395861a Mon Sep 17 00:00:00 2001 From: Ludovic LANGE Date: Tue, 27 Sep 2022 21:29:03 +0200 Subject: [PATCH 02/12] Use enums from `Qt::ItemFlags` It seems that one my compiler we need to use an explicit enum instead of the constant it represents. It shouldn't cause any issue as the value is the same. --- re/sniffer/sniffermodel.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/re/sniffer/sniffermodel.cpp b/re/sniffer/sniffermodel.cpp index 55736b4..293a69d 100644 --- a/re/sniffer/sniffermodel.cpp +++ b/re/sniffer/sniffermodel.cpp @@ -137,7 +137,7 @@ QVariant SnifferModel::data(const QModelIndex &index, int role) const Qt::ItemFlags SnifferModel::flags(const QModelIndex &index) const { if (!index.isValid()) - return 0; + return Qt::NoItemFlags; return QAbstractItemModel::flags(index); } From 74388791bf3aada1c9b0b3a8336738e4be5d643c Mon Sep 17 00:00:00 2001 From: Ludovic LANGE Date: Tue, 27 Sep 2022 21:10:37 +0200 Subject: [PATCH 03/12] remove `QSerialPort` obsolete enum values (qt6) Three enums (`QSerialPort::ParityError`, `QSerialPort::FramingError`, `QSerialPort::BreakConditionError`) are [deprecated since Qt5.6](https://doc.qt.io/qt-5/qserialport.html) and removed from Qt6. We remove them, it's unfortunate but I don't know how to replace these - it seems that we need to handle those in an [OS-specific way](https://codereview.qt-project.org/c/qt/qtserialport/+/125517/). --- connections/gvretserial.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/connections/gvretserial.cpp b/connections/gvretserial.cpp index c307b44..d6a122c 100644 --- a/connections/gvretserial.cpp +++ b/connections/gvretserial.cpp @@ -485,6 +485,7 @@ void GVRetSerial::serialError(QSerialPort::SerialPortError err) killConnection = true; piStop(); break; +#if QT_VERSION <= QT_VERSION_CHECK( 6, 0, 0 ) case QSerialPort::ParityError: errMessage = "Parity error on serial port"; break; @@ -494,6 +495,7 @@ void GVRetSerial::serialError(QSerialPort::SerialPortError err) case QSerialPort::BreakConditionError: errMessage = "Break error on serial port"; break; +#endif case QSerialPort::WriteError: errMessage = "Write error on serial port"; piStop(); From 1af5cbacc6efa519ffec512b72da2a231671ece8 Mon Sep 17 00:00:00 2001 From: Ludovic LANGE Date: Tue, 27 Sep 2022 22:09:30 +0200 Subject: [PATCH 04/12] fix QMqtt ambiguous conversion Prevent error: ``` moc_qmqtt_client.cpp:459:53: error: conversion from 'QMQTT::ConnectionState' to 'QChar' is ambiguous ``` --- mqtt/qmqtt_client.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mqtt/qmqtt_client.h b/mqtt/qmqtt_client.h index da89f01..690319b 100644 --- a/mqtt/qmqtt_client.h +++ b/mqtt/qmqtt_client.h @@ -129,7 +129,7 @@ class Q_MQTT_EXPORT Client : public QObject Q_PROPERTY(quint8 _willQos READ willQos WRITE setWillQos) Q_PROPERTY(bool _willRetain READ willRetain WRITE setWillRetain) Q_PROPERTY(QByteArray _willMessage READ willMessage WRITE setWillMessage) - Q_PROPERTY(QString _connectionState READ connectionState) + Q_PROPERTY(ConnectionState _connectionState READ connectionState) #ifndef QT_NO_SSL Q_PROPERTY(QSslConfiguration _sslConfiguration READ sslConfiguration WRITE setSslConfiguration) #endif // QT_NO_SSL From 5002f3ebec8a8c35b87bf731e9149c7dcd4f4500 Mon Sep 17 00:00:00 2001 From: Ludovic LANGE Date: Tue, 27 Sep 2022 22:15:47 +0200 Subject: [PATCH 05/12] fix some static cast errors in QMqtt The following error occurs twice: ``` mqtt/qmqtt_ssl_socket.cpp:49:13: error: static_cast from 'QAbstractSocket::SocketError (QAbstractSocket::*)() const' to 'void (QSslSocket::*)(QAbstractSocket::SocketError)' is not allowed ``` It may not be the cleanest fix but the it is the only one I could come with... --- mqtt/qmqtt_socket.cpp | 4 ++-- mqtt/qmqtt_ssl_socket.cpp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/mqtt/qmqtt_socket.cpp b/mqtt/qmqtt_socket.cpp index f0628f0..5805f94 100644 --- a/mqtt/qmqtt_socket.cpp +++ b/mqtt/qmqtt_socket.cpp @@ -41,9 +41,9 @@ QMQTT::Socket::Socket(QObject* parent) connect(_socket.data(), &QTcpSocket::connected, this, &SocketInterface::connected); connect(_socket.data(), &QTcpSocket::disconnected, this, &SocketInterface::disconnected); connect(_socket.data(), - static_cast(&QTcpSocket::error), + SIGNAL(error(QTcpSocket::error)), this, - static_cast(&SocketInterface::error)); + SLOT(errorHandler(SocketInterface::error))); } QMQTT::Socket::~Socket() diff --git a/mqtt/qmqtt_ssl_socket.cpp b/mqtt/qmqtt_ssl_socket.cpp index 2d3a808..9f70434 100644 --- a/mqtt/qmqtt_ssl_socket.cpp +++ b/mqtt/qmqtt_ssl_socket.cpp @@ -46,9 +46,9 @@ QMQTT::SslSocket::SslSocket(const QSslConfiguration& config, QObject* parent) connect(_socket.data(), &QSslSocket::encrypted, this, &SocketInterface::connected); connect(_socket.data(), &QSslSocket::disconnected, this, &SocketInterface::disconnected); connect(_socket.data(), - static_cast(&QSslSocket::error), + SIGNAL(error(QSslSocket::error)), this, - static_cast(&SocketInterface::error)); + SLOT(errorHandler(SocketInterface::error))); connect(_socket.data(), static_cast&)>(&QSslSocket::sslErrors), this, From 88e51a4a326850577d5577aada2f44d6de345dac Mon Sep 17 00:00:00 2001 From: Ludovic LANGE Date: Tue, 27 Sep 2022 23:01:58 +0200 Subject: [PATCH 06/12] remove obsolete `QWheelEvent::delta()` (qt6) The function `QWheelEvent::delta()` is [deprecated in Qt5](https://doc.qt.io/qt-5/qwheelevent-obsolete.html#delta), and has been removed in Qt6. We port it to `QWheelEvent::angleDelta()`. --- jsedit.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/jsedit.cpp b/jsedit.cpp index 5a994d0..edb1eef 100644 --- a/jsedit.cpp +++ b/jsedit.cpp @@ -931,7 +931,8 @@ void JSEdit::resizeEvent(QResizeEvent *e) void JSEdit::wheelEvent(QWheelEvent *e) { if (e->modifiers() == Qt::ControlModifier) { - int steps = e->delta() / 20; + QPoint numDegrees = e->angleDelta(); + int steps = numDegrees.y() / 20; steps = qBound(-3, steps, 3); QFont textFont = font(); int pointSize = textFont.pointSize() + steps; From b8039f9b163f879b1bf2a4cb6e2b781ac858a66c Mon Sep 17 00:00:00 2001 From: bigoulours Date: Wed, 28 Sep 2022 15:42:19 +0200 Subject: [PATCH 07/12] Priorizing exact match over J1939 and GMLAN Hi Collin, I had a case where two ECUs were sending the same PGN (with different contents though), leading to SavvyCAN picking the first found PGN-Match. Hence my proposal: go over the list until an exact match is found, otherwise returning the best match (same PGN). --- dbc/dbchandler.cpp | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/dbc/dbchandler.cpp b/dbc/dbchandler.cpp index d60dd1b..8290c10 100644 --- a/dbc/dbchandler.cpp +++ b/dbc/dbchandler.cpp @@ -100,8 +100,15 @@ void DBCSignalHandler::sort() DBC_MESSAGE* DBCMessageHandler::findMsgByID(uint32_t id) { if (messages.count() == 0) return nullptr; + DBC_MESSAGE *bestMatch = nullptr; + for (int i = 0; i < messages.count(); i++) { + if ( messages[i].ID == id ) + { + return &messages[i]; + } + if (matchingCriteria == J1939) { // include data page and extended data page in the pgn @@ -112,7 +119,7 @@ DBC_MESSAGE* DBCMessageHandler::findMsgByID(uint32_t id) pgn &= 0x3FF00; if ((messages[i].ID & 0x3FF0000) == (pgn << 8)) { - return &messages[i]; + bestMatch = &messages[i]; } } else @@ -120,7 +127,7 @@ DBC_MESSAGE* DBCMessageHandler::findMsgByID(uint32_t id) // PDU2 format if ((messages[i].ID & 0x3FFFF00) == (pgn << 8)) { - return &messages[i]; + bestMatch = &messages[i]; } } } @@ -129,17 +136,10 @@ DBC_MESSAGE* DBCMessageHandler::findMsgByID(uint32_t id) // Match the bits 14-26 (Arbitration Id) of GMLAN 29bit header uint32_t arbId = id &0x3FFE000; if ( (arbId != 0) && (messages[i].ID & 0x3FFE000) == arbId ) - return &messages[i]; - } - else - { - if ( messages[i].ID == id ) - { - return &messages[i]; - } + bestMatch = &messages[i]; } } - return nullptr; + return bestMatch; } DBC_MESSAGE* DBCMessageHandler::findMsgByIdx(int idx) From bd8582b8e36626fdb1ee8df46e297b9799fc028c Mon Sep 17 00:00:00 2001 From: Andy Huska Date: Wed, 28 Sep 2022 20:02:27 -0500 Subject: [PATCH 08/12] Added combobox for nodes so msg combobox isnt so long Added disabled filenames in node combobox so you can understand what is what Added node column to table Made table columns sortable so you can sort by node name or message name Want to make table fixed width font capable Want to make table rearrangable Want to make table rows shorter to fix more info in a window Want to make it possible to have multiple signal viewer windows Want to add ability to click on signals and add them to graphs like the main window --- signalviewerwindow.cpp | 75 ++++++++++++++++++++++++++++++++++------ signalviewerwindow.h | 3 +- ui/signalviewerwindow.ui | 16 ++++++++- 3 files changed, 81 insertions(+), 13 deletions(-) diff --git a/signalviewerwindow.cpp b/signalviewerwindow.cpp index cb70364..7d20531 100644 --- a/signalviewerwindow.cpp +++ b/signalviewerwindow.cpp @@ -4,6 +4,9 @@ #include "mainwindow.h" #include +#define MSG_COL 1 +#define VALUE_COL 2 + SignalViewerWindow::SignalViewerWindow(const QVector *frames, QWidget *parent) : QDialog(parent), ui(new Ui::SignalViewerWindow) @@ -14,15 +17,16 @@ SignalViewerWindow::SignalViewerWindow(const QVector *frames, QWidget modelFrames = frames; QStringList headers; - headers << "Signal" << "Value"; + headers << "Node" << "Signal" << "Value"; ui->tableViewer->setHorizontalHeaderLabels(headers); - ui->tableViewer->setColumnWidth(0, 150); - ui->tableViewer->setColumnWidth(1, 300); + ui->tableViewer->setColumnWidth(0, 100); + ui->tableViewer->setColumnWidth(1, 150); QHeaderView *HorzHdr = ui->tableViewer->horizontalHeader(); HorzHdr->setStretchLastSection(true); //causes the data column to automatically fill the tableview dbcHandler = DBCHandler::getReference(); + connect(ui->cbNodes, SIGNAL(currentIndexChanged(int)), this, SLOT(loadMessages(int))); connect(ui->cbMessages, SIGNAL(currentIndexChanged(int)), this, SLOT(loadSignals(int))); connect(ui->btnAdd, SIGNAL(clicked(bool)), this, SLOT(addSignal())); connect(MainWindow::getReference(), SIGNAL(framesUpdated(int)), this, SLOT(updatedFrames(int))); @@ -32,7 +36,7 @@ SignalViewerWindow::SignalViewerWindow(const QVector *frames, QWidget connect(ui->btnAppend, SIGNAL(clicked(bool)), this, SLOT(appendSignalsFile())); connect(ui->btnClear, SIGNAL(clicked(bool)), this, SLOT(clearSignalsTable())); - loadMessages(); + loadNodes(); } SignalViewerWindow::~SignalViewerWindow() @@ -79,11 +83,11 @@ void SignalViewerWindow::processFrame(CANFrame &frame) { if (sig->processAsText(frame, sigString, false)) //if true we could interpret the signal so update it in the list { - QTableWidgetItem *item = ui->tableViewer->item(i, 1); + QTableWidgetItem *item = ui->tableViewer->item(i, VALUE_COL); if (!item) { item = new QTableWidgetItem(sigString); - ui->tableViewer->setItem(i, 1, item); + ui->tableViewer->setItem(i, VALUE_COL, item); } else item->setText(sigString); } @@ -99,20 +103,67 @@ void SignalViewerWindow::removeSelectedSignal() ui->tableViewer->removeRow(selRow); } -void SignalViewerWindow::loadMessages() +void SetComboBoxItemEnabled(QComboBox * comboBox, int index, bool enabled) +{ + auto * model = qobject_cast(comboBox->model()); + assert(model); + if(!model) return; + + auto * item = model->item(index); + assert(item); + if(!item) return; + item->setEnabled(enabled); +} + +void SignalViewerWindow::loadNodes() { int numFiles; - ui->cbMessages->clear(); + ui->cbNodes->clear(); if (dbcHandler == nullptr) return; if ((numFiles = dbcHandler->getFileCount()) == 0) return; qDebug() << numFiles; + for (int f = 0; f < numFiles; f++) + { + qDebug() << dbcHandler->getFileByIdx(f)->messageHandler->getCount(); + + QList names; + + for (int x = 0; x < dbcHandler->getFileByIdx(f)->dbc_nodes.count(); x++) + { + QString name = dbcHandler->getFileByIdx(f)->dbc_nodes[x].name; + if(name != "Vector__XXX") + names.append(name); + } + + if(names.count() > 0) + { + names.sort(); + ui->cbNodes->addItem("----" + dbcHandler->getFileByIdx(f)->getFilename()); + SetComboBoxItemEnabled(ui->cbNodes, ui->cbNodes->count() -1, false); + for(int i=0; icbNodes->addItem(names[i]); + } + } +} + +void SignalViewerWindow::loadMessages(int idx) +{ + int numFiles; + ui->cbMessages->clear(); + if (dbcHandler == nullptr) return; + if ((numFiles = dbcHandler->getFileCount()) == 0) return; + qDebug() << numFiles; + + QString nodeName = ui->cbNodes->itemText(idx); + for (int f = 0; f < numFiles; f++) { qDebug() << dbcHandler->getFileByIdx(f)->messageHandler->getCount(); for (int x = 0; x < dbcHandler->getFileByIdx(f)->messageHandler->getCount(); x++) { - ui->cbMessages->addItem(dbcHandler->getFileByIdx(f)->messageHandler->findMsgByIdx(x)->name); + if(dbcHandler->getFileByIdx(f)->messageHandler->findMsgByIdx(x)->sender->name == nodeName) + ui->cbMessages->addItem(dbcHandler->getFileByIdx(f)->messageHandler->findMsgByIdx(x)->name); } } } @@ -150,8 +201,10 @@ void SignalViewerWindow::addSignal(DBC_SIGNAL *sig) int rowIdx = ui->tableViewer->rowCount(); ui->tableViewer->insertRow(rowIdx); - QTableWidgetItem *item = new QTableWidgetItem(sig->parentMessage->sender->name + " - " + sig->name); - ui->tableViewer->setItem(rowIdx, 0, item); + QTableWidgetItem *nodeitem = new QTableWidgetItem(sig->parentMessage->sender->name); + ui->tableViewer->setItem(rowIdx, 0, nodeitem); + QTableWidgetItem *msgitem = new QTableWidgetItem(sig->name); + ui->tableViewer->setItem(rowIdx, 1, msgitem); } void SignalViewerWindow::saveSignalsFile() diff --git a/signalviewerwindow.h b/signalviewerwindow.h index fab979c..711fe2c 100644 --- a/signalviewerwindow.h +++ b/signalviewerwindow.h @@ -17,7 +17,8 @@ public: ~SignalViewerWindow(); private slots: - void loadMessages(); + void loadNodes(); + void loadMessages(int idx); void loadSignals(int idx); void addSignal(); void addSignal(DBC_SIGNAL *sig); diff --git a/ui/signalviewerwindow.ui b/ui/signalviewerwindow.ui index 0814757..8f1c75d 100644 --- a/ui/signalviewerwindow.ui +++ b/ui/signalviewerwindow.ui @@ -18,8 +18,11 @@ + + true + - 2 + 3 300 @@ -32,6 +35,7 @@ + @@ -45,6 +49,16 @@ + + + + Node + + + + + + From dc0b25bee8e569113ba5f942987656060cc79fa3 Mon Sep 17 00:00:00 2001 From: Andy Huska Date: Fri, 30 Sep 2022 16:33:07 -0500 Subject: [PATCH 09/12] Added setting for fixed width vs normal font in the data tables --- mainsettingsdialog.cpp | 3 +++ mainwindow.cpp | 8 +++++++- signalviewerwindow.cpp | 15 +++++++++++++++ ui/mainsettingsdialog.ui | 18 ++++++++++++++++-- 4 files changed, 41 insertions(+), 3 deletions(-) diff --git a/mainsettingsdialog.cpp b/mainsettingsdialog.cpp index 2da44ed..7cd1230 100644 --- a/mainsettingsdialog.cpp +++ b/mainsettingsdialog.cpp @@ -48,6 +48,7 @@ MainSettingsDialog::MainSettingsDialog(QWidget *parent) : ui->cbLoadConnections->setChecked(settings.value("Main/SaveRestoreConnections", false).toBool()); ui->spinFontSize->setValue(settings.value("Main/FontSize", ui->cbDisplayHex->font().pointSize()).toUInt()); + ui->cbFontFixedWidth->setChecked(settings.value("Main/FontFixedWidth", false).toBool()); bool secondsMode = settings.value("Main/TimeSeconds", false).toBool(); bool clockMode = settings.value("Main/TimeClock", false).toBool(); @@ -132,6 +133,7 @@ MainSettingsDialog::MainSettingsDialog(QWidget *parent) : connect(ui->cbHexGraphInfo, SIGNAL(toggled(bool)), this, SLOT(updateSettings())); connect(ui->cbIgnoreDBCColors, SIGNAL(toggled(bool)), this, SLOT(updateSettings())); connect(ui->spinMaximumFrames, SIGNAL(valueChanged(int)), this, SLOT(updateSettings())); + connect(ui->cbFontFixedWidth, SIGNAL(toggled(bool)), this, SLOT(updateSettings())); installEventFilter(this); } @@ -199,6 +201,7 @@ void MainSettingsDialog::updateSettings() settings.setValue("Main/FilterLabeling", ui->cbFilterLabeling->isChecked()); settings.setValue("Main/IgnoreDBCColors", ui->cbIgnoreDBCColors->isChecked()); settings.setValue("Main/MaximumFrames", ui->spinMaximumFrames->value()); + settings.setValue("Main/FontFixedWidth", ui->cbFontFixedWidth->isChecked()); settings.sync(); emit updatedSettings(); diff --git a/mainwindow.cpp b/mainwindow.cpp index e1184f1..e84da30 100644 --- a/mainwindow.cpp +++ b/mainwindow.cpp @@ -57,12 +57,18 @@ MainWindow::MainWindow(QWidget *parent) : verticalHeader->setSectionResizeMode(QHeaderView::Fixed); QSettings settings; int fontSize = settings.value("Main/FontSize", 9).toUInt(); - QFont sysFont = QFontDatabase::systemFont(QFontDatabase::FixedFont); //get default font + QFont sysFont; + if(settings.value("Main/FontFixedWidth", false).toBool()) + sysFont = QFontDatabase::systemFont(QFontDatabase::FixedFont); //get default fixed width font + else + sysFont = QFont(); //get default font sysFont.setPointSize(fontSize); verticalHeader->setDefaultSectionSize(sysFont.pixelSize()); + verticalHeader->setFont(QFont()); ui->canFramesView->setFont(sysFont); QHeaderView *HorzHdr = ui->canFramesView->horizontalHeader(); + HorzHdr->setFont(QFont()); HorzHdr->setStretchLastSection(true); //causes the data column to automatically fill the tableview connect(HorzHdr, SIGNAL(sectionClicked(int)), this, SLOT(headerClicked(int))); diff --git a/signalviewerwindow.cpp b/signalviewerwindow.cpp index 7d20531..c061074 100644 --- a/signalviewerwindow.cpp +++ b/signalviewerwindow.cpp @@ -21,8 +21,23 @@ SignalViewerWindow::SignalViewerWindow(const QVector *frames, QWidget ui->tableViewer->setHorizontalHeaderLabels(headers); ui->tableViewer->setColumnWidth(0, 100); ui->tableViewer->setColumnWidth(1, 150); + + QSettings settings; + QFont sysFont; + int fontSize = settings.value("Main/FontSize", 9).toUInt(); + if(settings.value("Main/FontFixedWidth", false).toBool()) + sysFont = QFontDatabase::systemFont(QFontDatabase::FixedFont); //get default fixed width font + else + sysFont = QFont(); //get default font + sysFont.setPointSize(fontSize); + ui->tableViewer->setFont(sysFont); + QHeaderView *HorzHdr = ui->tableViewer->horizontalHeader(); HorzHdr->setStretchLastSection(true); //causes the data column to automatically fill the tableview + HorzHdr->setFont(QFont()); + + QHeaderView *verticalHeader = ui->tableViewer->verticalHeader(); + verticalHeader->setFont(QFont()); dbcHandler = DBCHandler::getReference(); diff --git a/ui/mainsettingsdialog.ui b/ui/mainsettingsdialog.ui index d119b08..cb61d4d 100644 --- a/ui/mainsettingsdialog.ui +++ b/ui/mainsettingsdialog.ui @@ -7,7 +7,7 @@ 0 0 965 - 678 + 713 @@ -191,9 +191,23 @@ - Font Size + Font + + + + Use fixed-width font in tables + + + + + + + Size + + + From 08953a4004128c182a603819fd09b99313f419f7 Mon Sep 17 00:00:00 2001 From: Andy Huska Date: Mon, 3 Oct 2022 11:26:28 -0500 Subject: [PATCH 10/12] Improved socketcand input buffer handling to reduce lost data. No longer tossing partial frames, saving the data to concat with future data. On startup there can be some data loss, but after it gets rolling there is none. Added rough provision to ensure the buffer doesn't get filled with bad data, but the decodeFrames recursive calls do a pretty good job of getting rid of it so I haven't seen the buffer grow after millions of frames coming over a UDP-based VPN. --- connections/socketcand.cpp | 53 ++++++++++++++++++++++++++++++-------- connections/socketcand.h | 3 ++- 2 files changed, 44 insertions(+), 12 deletions(-) diff --git a/connections/socketcand.cpp b/connections/socketcand.cpp index 20f2ae8..90f2408 100644 --- a/connections/socketcand.cpp +++ b/connections/socketcand.cpp @@ -249,24 +249,38 @@ void SocketCANd::switchToRawMode(int busNum) QCoreApplication::processEvents(); } -void SocketCANd::decodeFrames(QString data, int busNum) +QString SocketCANd::decodeFrames(QString data, int busNum) { if (data.indexOf("< frame ") == -1) { - qDebug() << "Received datagramm doesn't contain any frame: " << data; - return; + //qDebug() << "Received datagramm doesn't contain any frame: " << data; + if (data.indexOf("<") == -1) + return ""; + else + return data; } else { - QString framePart = data.mid(data.indexOf("< frame "), data.length()); //remove starting beginning of payload if not < frame > + int firstIndex = data.indexOf("< frame "); + if(firstIndex > 0) + { + QString framePartial = data.left(firstIndex); + qDebug() << "Received datagramm that starts with fragment (missing '< frame'), this should only occur on startup, removing...: " << framePartial; + } + QString framePart = data.mid(firstIndex, data.length()); //remove starting beginning of payload if not < frame > const QString frameStrConst = framePart.left(framePart.indexOf(">")+1); QString frameStr = frameStrConst; QStringList frameParsed = (frameStr.remove(QRegExp("^<")).remove(QRegExp(">$"))).simplified().split(' '); if(frameParsed.length() < 2) { - qDebug() << "Received datagramm is an incomplete frame: " << data; - return; + //qDebug() << "Received datagramm is an incomplete frame: " << data; + + //ok great, need to leave it in the buffer in case it can be combined with what comes next + //but if there was a fragment that did not have a starting token then we don't want it so only return + //known good data...again this should only happen on startup, but just in case we need to remove it + //so the data buffer doesn't grow uncontrolled. + return framePart; } buildFrame.setFrameId(frameParsed[1].toUInt(nullptr, 16)); @@ -281,7 +295,7 @@ void SocketCANd::decodeFrames(QString data, int busNum) if(frameParsed.length() < 4) { qDebug() << "Received frame doesn't contain any data: " << data; - return; + return data; } int framelength = frameParsed[3].length() * 0.5; @@ -315,8 +329,12 @@ void SocketCANd::decodeFrames(QString data, int busNum) else qDebug() << "can't get a frame, capture suspended"; + //take out the data that we just processed and anything that is in front of it + //this should keep broken frames from accumulating at in the data buffer if (framePart.length() > frameStrConst.length()) - decodeFrames(framePart.right(framePart.length() - frameStrConst.length()), busNum); + return decodeFrames(framePart.right(framePart.length() - frameStrConst.length()), busNum); + + return ""; } } @@ -400,17 +418,30 @@ void SocketCANd::procRXData(QString data, int busNum) { qDebug() << "Ok found at start of compound message, switching to RAW and decoding immediately"; rx_state[busNum] = RAWMODE; - decodeFrames(data, busNum); + unprocessedData = decodeFrames(data, busNum); } else if(data.indexOf("< ok >", 0, Qt::CaseSensitivity::CaseInsensitive) > 0) { qDebug() << "Ok found at in middle of compound message, switching to RAW and decoding immediately"; rx_state[busNum] = RAWMODE; - decodeFrames(data, busNum); + unprocessedData = decodeFrames(data, busNum); } break; case RAWMODE: - decodeFrames(data, busNum); + if(!unprocessedData.isEmpty()) + { + //qDebug() << unprocessedData.length() << " bytes of unprocessedData: " << unprocessedData << " adding it to new data: " + data.left(50) + "..."; + } + unprocessedData = decodeFrames(unprocessedData + data, busNum); + + if(unprocessedData.length() > 128) + { + //the buffer has grown too much we need to clear it out, but what is good logic for that? + //the decodeFrames function strips out datat that doesn't have a '< frame' starting token, and in its + //recursive calling of itself it strips out data that preceedes valid frames, so this should never happen + qDebug() << unprocessedData.length() << " bytes in unprocessedData, something is wrong, clearing..."; + unprocessedData.clear(); + } break; case ISOTP: break; diff --git a/connections/socketcand.h b/connections/socketcand.h index b5c9376..bd86e33 100644 --- a/connections/socketcand.h +++ b/connections/socketcand.h @@ -57,7 +57,7 @@ private slots: void invokeReadTCPData(); void deviceConnected(int busNum); void switchToRawMode(int busNum); - void decodeFrames(QString, int busNum); + QString decodeFrames(QString, int busNum); private: void procRXData(QString, int busNum); @@ -76,6 +76,7 @@ protected: QByteArray buildData; QVarLengthArray rx_state; CANFrame buildFrame; + QString unprocessedData; }; From e52a74ac7a464e94fefaec44c0c5b1f94a95431e Mon Sep 17 00:00:00 2001 From: Andy Huska Date: Mon, 3 Oct 2022 16:36:30 -0500 Subject: [PATCH 11/12] Found a bug where complete frames being removed from the buffer were being shorted by 1 character Committing now with debug comments for future reference --- connections/socketcand.cpp | 160 ++++++++++++++++++------------------- connections/socketcand.h | 2 +- 2 files changed, 81 insertions(+), 81 deletions(-) diff --git a/connections/socketcand.cpp b/connections/socketcand.cpp index 90f2408..c290204 100644 --- a/connections/socketcand.cpp +++ b/connections/socketcand.cpp @@ -32,6 +32,7 @@ SocketCANd::SocketCANd(QString portName) : for (int i = 0; i < mNumBuses; i++) { rx_state.append(IDLE); + unprocessedData.append(""); } } @@ -251,91 +252,87 @@ void SocketCANd::switchToRawMode(int busNum) QString SocketCANd::decodeFrames(QString data, int busNum) { - if (data.indexOf("< frame ") == -1) + if (data.indexOf("<") == -1) + return ""; + else if(data.length() >= 8 && data.indexOf("< frame ") == -1) + return ""; + + int firstIndex = data.indexOf("< frame "); + if(firstIndex > 0) { - //qDebug() << "Received datagramm doesn't contain any frame: " << data; - if (data.indexOf("<") == -1) - return ""; - else - return data; + QString framePartial = data.left(firstIndex); + qDebug() << "Received datagramm that starts with fragment (missing '< frame'), this should only occur on startup, removing...: " << framePartial; } - else + QString framePart = data.mid(firstIndex); //remove starting beginning of payload if not < frame > + const QString frameStrConst = framePart.left(framePart.indexOf(">")+1); + QString frameStr = frameStrConst; + QStringList frameParsed = (frameStr.remove(QRegExp("^<")).remove(QRegExp(">$"))).simplified().split(' '); + + if(frameParsed.length() < 3) { - int firstIndex = data.indexOf("< frame "); - if(firstIndex > 0) - { - QString framePartial = data.left(firstIndex); - qDebug() << "Received datagramm that starts with fragment (missing '< frame'), this should only occur on startup, removing...: " << framePartial; - } - QString framePart = data.mid(firstIndex, data.length()); //remove starting beginning of payload if not < frame > - const QString frameStrConst = framePart.left(framePart.indexOf(">")+1); - QString frameStr = frameStrConst; - QStringList frameParsed = (frameStr.remove(QRegExp("^<")).remove(QRegExp(">$"))).simplified().split(' '); + //qDebug() << "Received datagramm is an incomplete frame: " << data; - if(frameParsed.length() < 2) - { - //qDebug() << "Received datagramm is an incomplete frame: " << data; + //ok great, need to leave it in the buffer in case it can be combined with what comes next + //but if there was a fragment that did not have a starting token then we don't want it so only return + //known good data...again this should only happen on startup, but just in case we need to remove it + //so the data buffer doesn't grow uncontrolled. + return framePart; + } - //ok great, need to leave it in the buffer in case it can be combined with what comes next - //but if there was a fragment that did not have a starting token then we don't want it so only return - //known good data...again this should only happen on startup, but just in case we need to remove it - //so the data buffer doesn't grow uncontrolled. - return framePart; - } + buildFrame.setFrameId(frameParsed[1].toUInt(nullptr, 16)); + buildFrame.bus = busNum; - buildFrame.setFrameId(frameParsed[1].toUInt(nullptr, 16)); - buildFrame.bus = busNum; + if (buildFrame.frameId() > 0x7FF) buildFrame.setExtendedFrameFormat(true); + else buildFrame.setExtendedFrameFormat(false); - if (buildFrame.frameId() > 0x7FF) buildFrame.setExtendedFrameFormat(true); - else buildFrame.setExtendedFrameFormat(false); + buildFrame.setTimeStamp(QCanBusFrame::TimeStamp(0, frameParsed[2].toDouble() * 1000000l)); + //buildFrame.len = frameParsed[3].length() * 0.5; - buildFrame.setTimeStamp(QCanBusFrame::TimeStamp(0, frameParsed[2].toDouble() * 1000000l)); - //buildFrame.len = frameParsed[3].length() * 0.5; + if(frameParsed.length() < 4) + { + qDebug() << "Received frame doesn't contain any data: " << data; + return data; + } - if(frameParsed.length() < 4) - { - qDebug() << "Received frame doesn't contain any data: " << data; - return data; - } + int framelength = frameParsed[3].length() * 0.5; - int framelength = frameParsed[3].length() * 0.5; + buildData.resize(framelength); - buildData.resize(framelength); - - int c; - for (c = 0; c < framelength; c++) - { - bool ok; - unsigned char byteVal = frameParsed[3].mid(c*2, 2).toUInt(&ok, 16); - buildData[c] = byteVal; - } - buildFrame.setPayload(buildData); + int c; + for (c = 0; c < framelength; c++) + { + bool ok; + unsigned char byteVal = frameParsed[3].mid(c*2, 2).toUInt(&ok, 16); + buildData[c] = byteVal; + } + buildFrame.setPayload(buildData); // buildFrame.isReceived = true; - if (!isCapSuspended()) - { - /* get frame from queue */ - CANFrame* frame_p = getQueue().get(); - if(frame_p) { - /* copy frame */ - *frame_p = buildFrame; - //frame_p->remote = false; - frame_p->setFrameType(QCanBusFrame::DataFrame); - checkTargettedFrame(buildFrame); - /* enqueue frame */ - getQueue().queue(); - } + if (!isCapSuspended()) + { + /* get frame from queue */ + CANFrame* frame_p = getQueue().get(); + if(frame_p) { + /* copy frame */ + *frame_p = buildFrame; + //frame_p->remote = false; + frame_p->setFrameType(QCanBusFrame::DataFrame); + checkTargettedFrame(buildFrame); + /* enqueue frame */ + getQueue().queue(); } - else - qDebug() << "can't get a frame, capture suspended"; - - //take out the data that we just processed and anything that is in front of it - //this should keep broken frames from accumulating at in the data buffer - if (framePart.length() > frameStrConst.length()) - return decodeFrames(framePart.right(framePart.length() - frameStrConst.length()), busNum); - - return ""; } + else + qDebug() << "can't get a frame, capture suspended"; + + //take out the data that we just processed and anything that is in front of it + //this should keep broken frames from accumulating at in the data buffer + if (framePart.length() > frameStrConst.length()) + { + return decodeFrames(framePart.right(framePart.length() - frameStrConst.length()), busNum); + } + + return ""; } void SocketCANd::disconnectDevice() { @@ -405,6 +402,7 @@ void SocketCANd::procRXData(QString data, int busNum) { switchToRawMode(busNum); rx_state[busNum] = SWITCHING2RAW; + unprocessedData[busNum].clear(); } else qInfo() << hostCanIDs[busNum] << ": Could not open bus. Host did not respond with ""< ok >"": " << data; break; @@ -418,29 +416,31 @@ void SocketCANd::procRXData(QString data, int busNum) { qDebug() << "Ok found at start of compound message, switching to RAW and decoding immediately"; rx_state[busNum] = RAWMODE; - unprocessedData = decodeFrames(data, busNum); + unprocessedData[busNum] = decodeFrames(data, busNum); } else if(data.indexOf("< ok >", 0, Qt::CaseSensitivity::CaseInsensitive) > 0) { qDebug() << "Ok found at in middle of compound message, switching to RAW and decoding immediately"; rx_state[busNum] = RAWMODE; - unprocessedData = decodeFrames(data, busNum); + unprocessedData[busNum] = decodeFrames(data, busNum); } break; case RAWMODE: - if(!unprocessedData.isEmpty()) - { - //qDebug() << unprocessedData.length() << " bytes of unprocessedData: " << unprocessedData << " adding it to new data: " + data.left(50) + "..."; - } - unprocessedData = decodeFrames(unprocessedData + data, busNum); + //if(!unprocessedData[busNum].isEmpty()) + //{ + // qDebug() << "busNum: " << busNum << "- " << unprocessedData[busNum].length() << " bytes of unprocessedData: " << unprocessedData[busNum] << " adding it to new data: " + data.left(50) + "..."; + // } + unprocessedData[busNum] = decodeFrames(unprocessedData[busNum] + data, busNum); + //if(unprocessedData[busNum].length() > 0) + // qDebug() << "busNum: " << busNum << " has data left over, what was at the end of the last packet?: " << data.right(20); - if(unprocessedData.length() > 128) + if(unprocessedData[busNum].length() > 128) { //the buffer has grown too much we need to clear it out, but what is good logic for that? //the decodeFrames function strips out datat that doesn't have a '< frame' starting token, and in its //recursive calling of itself it strips out data that preceedes valid frames, so this should never happen - qDebug() << unprocessedData.length() << " bytes in unprocessedData, something is wrong, clearing..."; - unprocessedData.clear(); + qDebug() << "busNum: " << busNum << "- " << unprocessedData[busNum].length() << " bytes in unprocessedData, something is wrong, clearing..."; + unprocessedData[busNum].clear(); } break; case ISOTP: diff --git a/connections/socketcand.h b/connections/socketcand.h index bd86e33..aa8510a 100644 --- a/connections/socketcand.h +++ b/connections/socketcand.h @@ -76,7 +76,7 @@ protected: QByteArray buildData; QVarLengthArray rx_state; CANFrame buildFrame; - QString unprocessedData; + QVarLengthArray unprocessedData; }; From 3c07785040461c2cddad0f691ada5ae0fb57d765 Mon Sep 17 00:00:00 2001 From: Andy Huska Date: Mon, 3 Oct 2022 17:02:13 -0500 Subject: [PATCH 12/12] Buffer for partially received and/or leftover frame fragments now working with multiple active busses. Removed commented out test code and put a fix in for for the filters list not having its capacity properly reserved in recalcOverwrite so multiple entries of the same message show up in the filtered view --- canframemodel.cpp | 1 + connections/socketcand.cpp | 6 ------ 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/canframemodel.cpp b/canframemodel.cpp index e061a57..5f84930 100644 --- a/canframemodel.cpp +++ b/canframemodel.cpp @@ -381,6 +381,7 @@ void CANFrameModel::recalcOverwrite() //Then replace the old list of frames with just the unique list frames.clear(); frames.append(overWriteFrames.values().toVector()); + frames.reserve(preallocSize); filteredFrames.clear(); filteredFrames.reserve(preallocSize); diff --git a/connections/socketcand.cpp b/connections/socketcand.cpp index c290204..8a6e5da 100644 --- a/connections/socketcand.cpp +++ b/connections/socketcand.cpp @@ -426,13 +426,7 @@ void SocketCANd::procRXData(QString data, int busNum) } break; case RAWMODE: - //if(!unprocessedData[busNum].isEmpty()) - //{ - // qDebug() << "busNum: " << busNum << "- " << unprocessedData[busNum].length() << " bytes of unprocessedData: " << unprocessedData[busNum] << " adding it to new data: " + data.left(50) + "..."; - // } unprocessedData[busNum] = decodeFrames(unprocessedData[busNum] + data, busNum); - //if(unprocessedData[busNum].length() > 0) - // qDebug() << "busNum: " << busNum << " has data left over, what was at the end of the last packet?: " << data.right(20); if(unprocessedData[busNum].length() > 128) {