From e64e84d6fa78b680cd98b75fcf2cd3c2d3bb7220 Mon Sep 17 00:00:00 2001 From: Collin Kidder Date: Wed, 28 Apr 2021 21:41:33 -0400 Subject: [PATCH] Bug fixes for extended IDs in DBC files, fix one off error in a few places (Corrects fuzzing window ID range), re-enabled overwrite mode showing all signals in a message --- canframemodel.cpp | 20 +++++++++++----- dbc/dbc_classes.cpp | 10 +++++--- dbc/dbc_classes.h | 1 + dbc/dbchandler.cpp | 50 +++++++++++++++++++++++++++++----------- dbc/dbcmessageeditor.cpp | 4 ++-- framefileio.cpp | 4 ++-- mainsettingsdialog.cpp | 1 + re/fuzzingwindow.cpp | 4 ++-- 8 files changed, 66 insertions(+), 28 deletions(-) diff --git a/canframemodel.cpp b/canframemodel.cpp index 7724912..baebc78 100644 --- a/canframemodel.cpp +++ b/canframemodel.cpp @@ -135,6 +135,7 @@ void CANFrameModel::normalizeTiming() mutex.lock(); if (frames.count() == 0) return; timeOffset = frames[0].timeStamp().microSeconds(); + qint64 prevStamp = 0; //find the absolute lowest timestamp in the whole time. Needed because maybe timestamp was reset in the middle. for (int j = 0; j < frames.count(); j++) @@ -144,7 +145,12 @@ void CANFrameModel::normalizeTiming() for (int i = 0; i < frames.count(); i++) { - frames[i].setTimeStamp(QCanBusFrame::TimeStamp(0, frames[i].timeStamp().microSeconds() - timeOffset)); + qint64 thisStamp = frames[i].timeStamp().microSeconds() - timeOffset; + if (thisStamp <= prevStamp) + { + timeOffset -= prevStamp; + } + frames[i].setTimeStamp(QCanBusFrame::TimeStamp(0, thisStamp)); } this->beginResetModel(); @@ -522,11 +528,13 @@ QVariant CANFrameModel::data(const QModelIndex &index, int role) const tempString.append(sig->processSignalTree(thisFrame)); } } - //else if (sig->isMultiplexed && overwriteDups) //wasn't in this exact frame but is in the message. Use cached value - //{ - // tempString.append(sig->makePrettyOutput(sig->cachedValue.toDouble(), sig->cachedValue.toLongLong())); - // tempString.append("\n"); - //} + else if (sig->isMultiplexed && overwriteDups) //wasn't in this exact frame but is in the message. Use cached value + { + bool isInteger = false; + if (sig->valType == UNSIGNED_INT || sig->valType == SIGNED_INT) isInteger = true; + tempString.append(sig->makePrettyOutput(sig->cachedValue.toDouble(), sig->cachedValue.toLongLong(), true, isInteger)); + tempString.append("\n"); + } } } } diff --git a/dbc/dbc_classes.cpp b/dbc/dbc_classes.cpp index 2ae9506..b7840a2 100644 --- a/dbc/dbc_classes.cpp +++ b/dbc/dbc_classes.cpp @@ -65,7 +65,11 @@ QString DBC_SIGNAL::processSignalTree(const CANFrame &frame) { QString build; int val; - if (!this->processAsInt(frame, val)) return build; + if (!this->processAsInt(frame, val)) + { + qDebug() << "Could not process multiplexor as an integer."; + return build; + } qDebug() << val; foreach (DBC_SIGNAL *sig, multiplexedChildren) @@ -222,11 +226,11 @@ bool DBC_SIGNAL::processAsInt(const CANFrame &frame, int32_t &outValue) //if (!isSignalInMessage(frame)) return false; if (valType == SIGNED_INT) isSigned = true; - if ( static_cast(frame.payload().length() * 8) < (startBit + signalSize) ) + /*if ( static_cast(frame.payload().length() * 8) <= (startBit + signalSize) ) { result = 0; return false; - } + }*/ result = static_cast(Utility::processIntegerSignal(frame.payload(), startBit, signalSize, intelByteOrder, isSigned)); diff --git a/dbc/dbc_classes.h b/dbc/dbc_classes.h index 1054a42..acb1f95 100644 --- a/dbc/dbc_classes.h +++ b/dbc/dbc_classes.h @@ -137,6 +137,7 @@ public: DBC_MESSAGE(); uint32_t ID; + bool extendedID; QString name; QString comment; unsigned int len; diff --git a/dbc/dbchandler.cpp b/dbc/dbchandler.cpp index 0f09628..fd6b046 100644 --- a/dbc/dbchandler.cpp +++ b/dbc/dbchandler.cpp @@ -431,7 +431,9 @@ DBC_MESSAGE* DBCFile::parseMessageLine(QString line) if (match.hasMatch()) { DBC_MESSAGE msg; - msg.ID = match.captured(1).toULong() & 0x7FFFFFFFul; //the ID is always stored in decimal format + uint32_t ID = match.captured(1).toULong(); //the ID is always stored in decimal format + msg.ID = ID & 0x1FFFFFFFul; + msg.extendedID = (ID & 80000000ul) ? true : false; msg.name = match.captured(2); msg.len = match.captured(3).toUInt(); msg.sender = findNodeByName(match.captured(4)); @@ -598,7 +600,7 @@ bool DBCFile::parseSignalMultiplexValueLine(QString line) //captured 5 is the upper bound if (match.hasMatch()) { - DBC_MESSAGE *msg = messageHandler->findMsgByID(match.captured(1).toUInt()); + DBC_MESSAGE *msg = messageHandler->findMsgByID(match.captured(1).toULong() & 0x1FFFFFFFUL); if (msg != nullptr) { DBC_SIGNAL *thisSignal = msg->sigHandler->findSignalByName(match.captured(2)); @@ -632,7 +634,7 @@ bool DBCFile::parseValueLine(QString line) if (match.hasMatch()) { //qDebug() << "Data was: " << match.captured(3); - DBC_MESSAGE *msg = messageHandler->findMsgByID(match.captured(1).toUInt()); + DBC_MESSAGE *msg = messageHandler->findMsgByID(match.captured(1).toULong() & 0x1FFFFFFFul); if (msg != nullptr) { DBC_SIGNAL *sig = msg->sigHandler->findSignalByName(match.captured(2)); @@ -646,7 +648,7 @@ bool DBCFile::parseValueLine(QString line) match = regex.match(tokenString); if (match.hasMatch()) { - val.value = match.captured(1).toInt(); + val.value = match.captured(1).toULong() & 0x1FFFFFFFul; val.descript = match.captured(2); //qDebug() << "sig val " << val.value << " desc " <valList.append(val); @@ -681,7 +683,7 @@ bool DBCFile::parseAttributeLine(QString line) if (foundAttr) { qDebug() << "That attribute does exist"; - DBC_MESSAGE *foundMsg = messageHandler->findMsgByID(match.captured(2).toUInt()); + DBC_MESSAGE *foundMsg = messageHandler->findMsgByID(match.captured(2).toUInt() & 0x1FFFFFFFul); if (foundMsg) { qDebug() << "It references a valid, registered message"; @@ -713,7 +715,7 @@ bool DBCFile::parseAttributeLine(QString line) if (foundAttr) { qDebug() << "That attribute does exist"; - DBC_MESSAGE *foundMsg = messageHandler->findMsgByID(match.captured(2).toUInt()); + DBC_MESSAGE *foundMsg = messageHandler->findMsgByID(match.captured(2).toUInt() & 0x1FFFFFFFUL); if (foundMsg) { qDebug() << "It references a valid, registered message"; @@ -1343,20 +1345,20 @@ bool DBCFile::saveFile(QString fileName) } uint32_t ID = msg->ID; - if (msg->ID > 0x7FF) msg->ID += 0x80000000ul; //set bit 31 if this ID is extended. + if (msg->ID > 0x7FF || msg->extendedID) msg->ID += 0x80000000ul; //set bit 31 if this ID is extended. msgOutput.append("BO_ " + QString::number(ID) + " " + msg->name + ": " + QString::number(msg->len) + " " + msg->sender->name + "\n"); if (msg->comment.length() > 0) { - commentsOutput.append("CM_ BO_ " + QString::number(msg->ID) + " \"" + msg->comment + "\";\n"); + commentsOutput.append("CM_ BO_ " + QString::number(ID) + " \"" + msg->comment + "\";\n"); } //If this message has attributes then compile them into attributes list to output later on. if (msg->attributes.count() > 0) { foreach (DBC_ATTRIBUTE_VALUE val, msg->attributes) { - attrValOutput.append("BA_ \"" + val.attrName + "\" BO_ " + QString::number(msg->ID) + " "); + attrValOutput.append("BA_ \"" + val.attrName + "\" BO_ " + QString::number(ID) + " "); switch (val.value.type()) { case QMetaType::QString: @@ -1426,14 +1428,14 @@ bool DBCFile::saveFile(QString fileName) + "\" " + sig->receiver->name + "\n"); if (sig->comment.length() > 0) { - commentsOutput.append("CM_ SG_ " + QString::number(msg->ID) + " " + sig->name + " \"" + sig->comment + "\";\n"); + commentsOutput.append("CM_ SG_ " + QString::number(ID) + " " + sig->name + " \"" + sig->comment + "\";\n"); } //if this signal has attributes then compile them in a special list of attributes if (sig->attributes.count() > 0) { foreach (DBC_ATTRIBUTE_VALUE val, sig->attributes) { - attrValOutput.append("BA_ \"" + val.attrName + "\" SG_ " + QString::number(msg->ID) + " " + sig->name + " "); + attrValOutput.append("BA_ \"" + val.attrName + "\" SG_ " + QString::number(ID) + " " + sig->name + " "); switch (val.value.type()) { case QMetaType::QString: @@ -1448,7 +1450,7 @@ bool DBCFile::saveFile(QString fileName) if (sig->valList.count() > 0) { - valuesOutput.append("VAL_ " + QString::number(msg->ID) + " " + sig->name); + valuesOutput.append("VAL_ " + QString::number(ID) + " " + sig->name); for (int v = 0; v < sig->valList.count(); v++) { DBC_VAL_ENUM_ENTRY val = sig->valList[v]; @@ -1538,13 +1540,16 @@ bool DBCFile::saveFile(QString fileName) { DBC_MESSAGE *msg = messageHandler->findMsgByIdx(x); + uint32_t ID = msg->ID; + if (msg->ID > 0x7FF || msg->extendedID) msg->ID += 0x80000000ul; //set bit 31 if this ID is extended. + for (int s = 0; s < msg->sigHandler->getCount(); s++) { DBC_SIGNAL *sig = msg->sigHandler->findSignalByIdx(s); if (sig->isMultiplexed) { - msgOutput.append("SG_MUL_VAL_ " + QString::number(msg->ID) + " "); + msgOutput.append("SG_MUL_VAL_ " + QString::number(ID) + " "); msgOutput.append(sig->name + " " + sig->parentMessage->name + " "); msgOutput.append(QString::number(sig->multiplexLowValue) + "-" + QString::number(sig->multiplexHighValue) + ";"); msgOutput.append("\n"); @@ -1859,6 +1864,25 @@ DBCFile* DBCHandler::loadJSONFile(QString filename) } } } + + for (int x = 0; x < thisFile->messageHandler->getCount(); x++) + { + DBC_MESSAGE *msg = thisFile->messageHandler->findMsgByIdx(x); + for (int y = 0; y < msg->sigHandler->getCount(); y++) + { + DBC_SIGNAL *sig = msg->sigHandler->findSignalByIdx(y); + //if this doesn't have a multiplex parent set but is multiplexed then it must have used + //simple multiplexing instead of any extended specification. So, fill in the multiplexor signal here + //and also write the extended entry for it too. + if (sig->isMultiplexed && (sig->multiplexParent == nullptr) ) + { + sig->multiplexParent = msg->multiplexorSignal; + msg->multiplexorSignal->multiplexedChildren.append(sig); + } + } + } + + thisFile->setDirtyFlag(); return thisFile; } diff --git a/dbc/dbcmessageeditor.cpp b/dbc/dbcmessageeditor.cpp index 6502f44..41fdbfa 100644 --- a/dbc/dbcmessageeditor.cpp +++ b/dbc/dbcmessageeditor.cpp @@ -34,7 +34,7 @@ DBCMessageEditor::DBCMessageEditor(QWidget *parent) : { if (dbcMessage == nullptr) return; if (suppressEditCallbacks) return; - if (dbcMessage->ID != Utility::ParseStringToNum(ui->lineFrameID->text())) dbcFile->setDirtyFlag(); + if ((dbcMessage->ID & 0x1FFFFFFFul) != Utility::ParseStringToNum(ui->lineFrameID->text())) dbcFile->setDirtyFlag(); dbcMessage->ID = Utility::ParseStringToNum(ui->lineFrameID->text()); emit updatedTreeInfo(dbcMessage); }); @@ -220,7 +220,7 @@ void DBCMessageEditor::refreshView() suppressEditCallbacks = true; ui->lineComment->setText(dbcMessage->comment); - ui->lineFrameID->setText(Utility::formatCANID(dbcMessage->ID)); + ui->lineFrameID->setText(Utility::formatCANID(dbcMessage->ID & 0x1FFFFFFFul)); ui->lineMsgName->setText(dbcMessage->name); ui->lineFrameLen->setText(QString::number(dbcMessage->len)); for (int i = 0; i < ui->comboSender->count(); i++) diff --git a/framefileio.cpp b/framefileio.cpp index 09fb624..4a79db1 100644 --- a/framefileio.cpp +++ b/framefileio.cpp @@ -1867,7 +1867,7 @@ bool FrameFileIO::loadNativeCSVFile(QString filename, QVector* frames) if (lng < 0) lng = 0; if (lng + 5 > tokens.length()) lng = tokens.length() - 5; QByteArray bytes(lng, 0); - for (int c = 0; c < 8; c++) bytes[c] = 0; + for (int c = 0; c < lng; c++) bytes[c] = 0; for (int d = 0; d < lng; d++) bytes[d] = static_cast(tokens[5 + d].toInt(nullptr, 16)); thisFrame.setPayload(bytes); @@ -1882,7 +1882,7 @@ bool FrameFileIO::loadNativeCSVFile(QString filename, QVector* frames) if (lng < 0) lng = 0; if (lng + 6 > tokens.length()) lng = tokens.length() - 6; QByteArray bytes(lng, 0); - for (int c = 0; c < 8; c++) bytes[c] = 0; + for (int c = 0; c < lng; c++) bytes[c] = 0; for (int d = 0; d < lng; d++) bytes[d] = static_cast(tokens[6 + d].toInt(nullptr, 16)); thisFrame.setPayload(bytes); diff --git a/mainsettingsdialog.cpp b/mainsettingsdialog.cpp index dd1764f..ba541cb 100644 --- a/mainsettingsdialog.cpp +++ b/mainsettingsdialog.cpp @@ -2,6 +2,7 @@ #include "ui_mainsettingsdialog.h" #include "helpwindow.h" #include +#include #include "simplecrypt.h" //using this simple encryption library to obfuscate stored password a bit. It's not super secure but better than diff --git a/re/fuzzingwindow.cpp b/re/fuzzingwindow.cpp index f510f7a..65df3f0 100644 --- a/re/fuzzingwindow.cpp +++ b/re/fuzzingwindow.cpp @@ -250,8 +250,8 @@ void FuzzingWindow::calcNextID() { if (rangeIDSelect) { - int range = endID - startID; - if (range != 0) currentID = startID + QRandomGenerator::global()->bounded(range); + int range = endID - startID + 1; + if (range != 1) currentID = startID + QRandomGenerator::global()->bounded(range); else currentID = startID; } else //IDs by filter so pick a random selected ID from the filter list