From 27b5d156b7f7672c818b95c9d7128e14cd3595e2 Mon Sep 17 00:00:00 2001 From: Collin Kidder Date: Tue, 17 Mar 2020 21:26:17 -0400 Subject: [PATCH] Bug fixes for ISOTP and UDS interpretation. --- bus_protocols/isotp_handler.cpp | 39 +++++++++++++-------------------- bus_protocols/isotp_handler.h | 2 +- bus_protocols/uds_handler.cpp | 20 ++++++++--------- re/isotp_interpreterwindow.cpp | 2 +- 4 files changed, 27 insertions(+), 36 deletions(-) diff --git a/bus_protocols/isotp_handler.cpp b/bus_protocols/isotp_handler.cpp index c589cdc..c741866 100644 --- a/bus_protocols/isotp_handler.cpp +++ b/bus_protocols/isotp_handler.cpp @@ -210,6 +210,7 @@ void ISOTP_HANDLER::processFrame(const CANFrame &frame) } } qDebug() << "Emitting single frame ISOTP message"; + msg.setPayload(dataBytes); emit newISOMessage(msg); break; case 1: //first frame of a multi-frame message @@ -239,7 +240,7 @@ void ISOTP_HANDLER::processFrame(const CANFrame &frame) } msg.lastSequence = -1; msg.setPayload(dataBytes); - messageBuffer.append(msg); + messageBuffer.insert(msg.frameId(), msg); //The sending ID is set to the last ID we used to send from this class which is //very likely to be correct. But, caution, there is a chance that it isn't. Beware. if (issueFlowMsgs && lastSenderID > 0) @@ -258,13 +259,9 @@ void ISOTP_HANDLER::processFrame(const CANFrame &frame) break; case 2: //subsequent frames for multi-frame messages pMsg = nullptr; - for (int i = 0; i < messageBuffer.length(); i++) + if (messageBuffer.contains(ID)) { - if (messageBuffer[i].frameId() == ID) - { - pMsg = &messageBuffer[i]; - break; - } + pMsg = &messageBuffer[ID]; } if (!pMsg) return; if (!pMsg->isMultiframe) return; //if we didn't get a frame type 1 (start of multiframe) first then ignore this frame. @@ -319,26 +316,20 @@ void ISOTP_HANDLER::processFrame(const CANFrame &frame) void ISOTP_HANDLER::checkNeedFlush(uint64_t ID) { - for (int i = 0; i < messageBuffer.length(); i++) + ISOTP_MESSAGE *msg; + if (messageBuffer.contains(ID)) { - if (messageBuffer[i].frameId() == ID) + msg = &messageBuffer[ID]; + if (msg->reportedLength <= msg->payload().count()) { - //used to pass by reference but now newISOMessage should pass by value which makes it easier to use cross thread - if (messageBuffer[i].frameId() > 0x600 && messageBuffer[i].frameId() < 0x630) - { - if (messageBuffer[i].reportedLength <= messageBuffer[i].payload().count()) - { - qDebug() << "Flushing full frame" << QString::number(messageBuffer[i].frameId(), 16) << " " << messageBuffer[i].reportedLength << " " << messageBuffer[i].payload().count(); - } - else - { - qDebug() << "Flushing a partial frame " << QString::number(messageBuffer[i].frameId(), 16) << " " << messageBuffer[i].reportedLength << " " << messageBuffer[i].payload().count(); - } - } - if (messageBuffer[i].reportedLength > 0) emit newISOMessage(messageBuffer[i]); - messageBuffer.removeAt(i); - return; + qDebug() << "Flushing full frame" << QString::number(msg->frameId(), 16) << " " << msg->reportedLength << " " << msg->payload().count(); } + else + { + qDebug() << "Flushing a partial frame " << QString::number(msg->frameId(), 16) << " " << msg->reportedLength << " " << msg->payload().count(); + } + if (msg->reportedLength > 0) emit newISOMessage(*msg); + messageBuffer.remove(ID); } } diff --git a/bus_protocols/isotp_handler.h b/bus_protocols/isotp_handler.h index f45e0ba..68d49b1 100644 --- a/bus_protocols/isotp_handler.h +++ b/bus_protocols/isotp_handler.h @@ -35,7 +35,7 @@ signals: void newISOMessage(ISOTP_MESSAGE msg); private: - QList messageBuffer; + QHash messageBuffer; QList sendingFrames; QList filters; const QVector *modelFrames; diff --git a/bus_protocols/uds_handler.cpp b/bus_protocols/uds_handler.cpp index d218c5d..48f2d99 100644 --- a/bus_protocols/uds_handler.cpp +++ b/bus_protocols/uds_handler.cpp @@ -409,7 +409,7 @@ QString UDS_HANDLER::getDetailedMessageAnalysis(const UDS_MESSAGE &msg) if (dataLen > 1) { buildString.append("Data payload: "); - for (int j = 1; j < dataLen; j++) buildString.append(Utility::formatHexNum(data[j]) + " "); + for (int j = 2; j < dataLen; j++) buildString.append(Utility::formatHexNum(data[j]) + " "); } } else @@ -417,8 +417,8 @@ QString UDS_HANDLER::getDetailedMessageAnalysis(const UDS_MESSAGE &msg) buildString.append("Key sending for security level: " + QString::number(msg.subFunc - 1)); if (dataLen > 1) //and it sure as hell should be! { - buildString.append("KEY: "); - for (int j = 1; j < dataLen; j++) buildString.append(Utility::formatHexNum(data[j]) + " "); + buildString.append(" KEY: "); + for (int j = 2; j < dataLen; j++) buildString.append(Utility::formatHexNum(data[j]) + " "); } } break; @@ -429,7 +429,7 @@ QString UDS_HANDLER::getDetailedMessageAnalysis(const UDS_MESSAGE &msg) if (dataLen > 1) //be kinda pointless if it weren't { buildString.append("SEED: "); - for (int j = 1; j < dataLen; j++) buildString.append(Utility::formatHexNum(data[j]) + " "); + for (int j = 2; j < dataLen; j++) buildString.append(Utility::formatHexNum(data[j]) + " "); } } else @@ -484,13 +484,13 @@ QString UDS_HANDLER::getDetailedMessageAnalysis(const UDS_MESSAGE &msg) break; case UDS_SERVICES::ROUTINE_CTRL: buildString.append("Routine Control: " + getLongDesc(UDS_ROUTINE_SUB, msg.subFunc)); - if (dataLen > 2) + if (dataLen > 3) { int routineID; - routineID = (data[1] * 256 + data[2]); + routineID = (data[2] * 256 + data[3]); buildString.append("\nRoutine ID: " + Utility::formatHexNum(routineID)); } - if (dataLen > 3) + if (dataLen > 4) { buildString.append("\nParameter bytes to routine: "); for (int i = 4; i < dataLen; i++) @@ -501,13 +501,13 @@ QString UDS_HANDLER::getDetailedMessageAnalysis(const UDS_MESSAGE &msg) break; case UDS_SERVICES::ROUTINE_CTRL + 0x40: buildString.append("Routine Control: " + getLongDesc(UDS_ROUTINE_SUB, msg.subFunc)); - if (dataLen > 2) + if (dataLen > 3) { int routineID; - routineID = (data[1] * 256 + data[2]); + routineID = (data[2] * 256 + data[3]); buildString.append("\nRoutine ID: " + Utility::formatHexNum(routineID)); } - if (dataLen > 3) + if (dataLen > 4) { buildString.append("\nBytes returned by routine: "); for (int i = 4; i < dataLen; i++) diff --git a/re/isotp_interpreterwindow.cpp b/re/isotp_interpreterwindow.cpp index 50ff657..d8a15cf 100644 --- a/re/isotp_interpreterwindow.cpp +++ b/re/isotp_interpreterwindow.cpp @@ -73,7 +73,7 @@ void ISOTP_InterpreterWindow::showEvent(QShowEvent* event) qApp->processEvents(); - decoder->updatedFrames(-2); + decoder->rapidFrames(nullptr, *modelFrames); progress.cancel();