This commit is contained in:
deeaitch committed 2026-10-14 10:32:44 -04:00
1 parent 0b05152bcf
commit d87c100398
9 files changed
+265 -65

No files matched your search

+20 -5
View File
@@ -614,14 +614,14 @@ if (!result.HasErrors()) {
| --- | --- |
| `Error` / `FileOpenError` | Input cannot be opened |
| `Error` / `FileReadError` | Input cannot be read completely |
| `Error` / `InvalidStatement` | Invalid frame/signal definition, duplicate, or payload violation |
| `Error` / `InvalidStatement` | Invalid frame/signal definition or comment, unresolved comment reference, duplicate, or payload violation |
| `Error` / `InvalidValueDescription` | Invalid `VAL_` syntax, duplicate value, or unresolved reference |
| `Error` / `InvalidMultiplexing` | Selector, branch range, or signal overlap validation failed |
| `Warning` / `UnsupportedStatement` | Unregistered statement skipped; decoding or metadata may be incomplete |
| `Error` / `UnsupportedStatement` | Extended multiplexing (`SG_MUL_VAL_`) rejected |
Parsing stops at the first error and returns an empty database, together with
any preceding warnings and the error. Deferred `VAL_` and multiplexing errors
any preceding warnings and the error. Deferred comment, `VAL_`, and multiplexing errors
retain their original source lines. Invalid signal markers use `InvalidStatement`.
`HasErrors()` checks severity; warnings alone do not make a parse fail.
@@ -637,8 +637,9 @@ indentation, even when they name unsupported syntax. Frame, signal, bus, and
node definitions or a line with arguments end the list. Actual unsupported
statements outside that list are reported.
Node and bus metadata are not stored. Other comment forms (`CM_`, `CM_ BU_`,
`CM_ EV_`) are ignored by the comment handler; comments only attach to already
parsed frames/signals. Supported statements currently require one physical line.
`CM_ EV_`) are ignored by the comment handler. Frame and signal comments may span
physical lines and resolve after definitions are read; other supported statements
require one physical line.
`ParseFile(path)` remains compatible: it returns the database, discards warnings,
and throws `std::runtime_error` on the first error with the existing `path:line:`
@@ -819,6 +820,19 @@ Stored in:
- `FrameInfo::comment`
- `SignalInfo::comment`
Comments accept spaces and tabs between fields, escaped quotes (`\"`) and
backslashes (`\\`), and text spanning multiple physical lines. Newlines, blank
lines, and indentation inside quoted text are preserved; CRLF becomes LF.
Comments may appear before their frame or signal definitions. References use the
same decimal CAN ID validation and normalization as `BO_`, including reserved-bit
and overflow checks and distinct standard/extended targets. Repeated comments
apply in statement order, so the last comment wins, including empty text.
Malformed supported comments (including invalid escapes, missing quotes or
semicolon, and trailing text) and missing frame/signal references produce an
`InvalidStatement` error at the comment's starting line. Failed parses return an
empty database; `ParseFile` throws with the same source location.
---
## CAN ID Normalization
@@ -1149,7 +1163,8 @@ Additional behavior to account for:
databases are also checked for ambiguous `VAL_` references during resolution
- payload lengths are checked against storage bounds (`0..255`), rather than
CAN or CAN FD protocol limits
- comment handling is limited to the supported forms and already parsed targets
- comment handling is limited to frame and signal comments; references resolve
after definitions are read
Motorola extraction uses DBC sawtooth numbering: a start bit of 7 traverses
`7..0, 15..8, 23..16`. Regression tests cover aligned and unaligned signals,
+10
View File
@@ -8,6 +8,16 @@ existing Git tag; subsequent changes are listed as unreleased.
### 2026-10-14
#### Fixed
- Issue #18: frame/signal comments share frame-definition CAN ID validation and
resolve after definitions are read. Support tabs, escaped quotes/backslashes,
and multiline text while preserving whitespace and starting source locations.
- Report malformed comments and missing references as `InvalidStatement` errors;
keep statement-order overwrites, including empty comments. Added regression
coverage and updated parser/test documentation. Verified with `qmake6`,
`make -j4`, and `make check`: all six regression suites passed.
#### Changed
- Validate duplicate frame keys before building the runtime lookup index, alongside
+44 -4
View File
@@ -19,6 +19,36 @@
#include <utility>
namespace {
// Read through the closing unescaped quote, preserving comment whitespace.
void ReadCommentLines(std::ifstream &input, std::string &statement,
std::size_t &lineNumber) {
bool inQuote = false;
bool escaped = false;
std::string part = statement;
for (;;) {
for (const char character: part) {
if (escaped) {
escaped = false;
} else if (inQuote && character == '\\') {
escaped = true;
} else if (character == '"') {
if (inQuote)
return;
inQuote = true;
}
}
if (!inQuote || !std::getline(input, part))
return;
++lineNumber;
if (!part.empty() && part.back() == '\r')
part.pop_back();
statement += '\n';
statement += part;
// A physical newline cannot escape the next line's first character.
escaped = false;
}
}
DbcDiagnosticCode StatementErrorCode(const std::string &keyword) {
if (keyword == "SG_MUL_VAL_")
return DbcDiagnosticCode::UnsupportedStatement;
@@ -59,6 +89,10 @@ DbcParseResult DbcParser::ParseFileWithDiagnostics(const std::string &filePath)
std::string line;
while (std::getline(input, line)) {
++lineNumber;
if (!line.empty() && line.back() == '\r')
line.pop_back();
const std::string rawLine = line;
const std::size_t statementLine = lineNumber;
line = dbc_detail::TrimText(line);
if (line.empty())
continue;
@@ -74,19 +108,24 @@ DbcParseResult DbcParser::ParseFileWithDiagnostics(const std::string &filePath)
continue; // NS_ lists syntax symbols; BO_, SG_, BS_ and BU_ end the list.
inNamespace = keyword == "NS_" || keyword == "NS_:";
if (keyword == "CM_") {
line = rawLine;
ReadCommentLines(input, line, lineNumber);
}
try {
context.lineNumber = lineNumber;
context.lineNumber = statementLine;
if (!dispatcher.Dispatch(line, context)) {
result.diagnostics.push_back({DbcDiagnosticSeverity::Warning,
DbcDiagnosticCode::UnsupportedStatement,
filePath, lineNumber,
filePath, statementLine,
"Unsupported statement '" + keyword +
"' is ignored; decoding or metadata may be incomplete."});
"' is ignored; decoding or metadata may be incomplete."});
}
} catch (const std::exception &error) {
result.diagnostics.push_back({DbcDiagnosticSeverity::Error,
StatementErrorCode(keyword), filePath,
lineNumber, error.what()});
statementLine, error.what()});
return result;
}
}
@@ -99,6 +138,7 @@ DbcParseResult DbcParser::ParseFileWithDiagnostics(const std::string &filePath)
}
try {
context.ResolveComments(filePath);
context.ResolveValueDescriptions(filePath);
context.ValidateMultiplexing(filePath);
} catch (const DbcDiagnosticError &error) {
+2
View File
@@ -27,6 +27,8 @@
* - basic M / mN multiplexing
* - VAL_ signal value descriptions
*
* Frame/signal comments support multiline quoted text and deferred references.
*
* Reports unsupported statements as warnings, except SG_MUL_VAL_ which is
* an error. VERSION, NS_, BS_, BU_ and NS_ symbol declarations stay quiet.
*/
+71 -50
View File
@@ -21,6 +21,15 @@ namespace {
return value;
}
std::uint32_t ParseCanId(const std::string &text) {
const std::uint32_t rawId = ParseDecimal(
text, std::numeric_limits<std::uint32_t>::max(), "CAN ID");
// Only bit 31 may accompany the actual 29-bit identifier.
if ((rawId & 0x60000000U) != 0U)
throw std::runtime_error("CAN ID exceeds the 29-bit identifier range.");
return rawId;
}
double ParsePhysicalValue(const std::string &text, const char *field) {
static const std::regex number(
R"(^[+-]?(?:[0-9]+(?:\.[0-9]*)?|\.[0-9]+)(?:[eE][+-]?[0-9]+)?$)");
@@ -193,11 +202,7 @@ FrameInfo DbcFrameHandler::ParseFrameLine(const std::string &line) {
if (!(stream >> keyword >> idText) || keyword != "BO_")
throw std::runtime_error("Missing CAN ID in frame definition.");
const std::uint32_t rawCanId = ParseDecimal(
idText, std::numeric_limits<std::uint32_t>::max(), "CAN ID");
// Only bit 31 may accompany the actual 29-bit identifier.
if ((rawCanId & 0x60000000U) != 0U)
throw std::runtime_error("CAN ID exceeds the 29-bit identifier range.");
const std::uint32_t rawCanId = ParseCanId(idText);
std::string nameText;
if (!std::getline(stream, nameText, ':') || stream.eof())
@@ -273,50 +278,6 @@ SignalInfo DbcSignalHandler::ParseSignalLine(const std::string &line) {
return signal;
}
void DbcCommentHandler::ParseCommentLine(const std::string &line, DbcDatabase &database) {
/*
* Examples:
* CM_ BO_ 256 "Frame comment";
* CM_ SG_ 256 EngineSpeed "Signal comment";
*/
std::istringstream stream(line);
std::string token;
stream >> token;
if (token != "CM_")
return;
stream >> token;
if (token != "BO_" && token != "SG_")
return;
std::uint32_t rawCanId = 0U;
std::uint32_t canId = 0U;
bool isExtended = false;
std::string signalName;
stream >> rawCanId;
if (token == "SG_")
stream >> signalName;
NormalizeCanId(rawCanId, canId, isExtended);
const std::string::size_type quoteBegin = line.find('"');
const std::string::size_type quoteEnd = line.rfind('"');
if (quoteBegin == std::string::npos || quoteEnd == std::string::npos || quoteEnd <= quoteBegin)
return;
FrameInfo *frame = FindFrameById(database, canId, isExtended);
if (frame == nullptr)
return;
const std::string comment = line.substr(quoteBegin + 1U, quoteEnd - quoteBegin - 1U);
if (token == "BO_") {
frame->comment = comment;
return;
}
SignalInfo *signal = FindSignalByName(*frame, signalName);
if (signal != nullptr)
signal->comment = comment;
}
namespace {
FrameInfo *FindFrameById(DbcDatabase &database,
@@ -365,7 +326,67 @@ void DbcSignalHandler::Handle(const std::string &line, DbcParseContext &context)
}
void DbcCommentHandler::Handle(const std::string &line, DbcParseContext &context) const {
ParseCommentLine(line, context.database);
std::istringstream stream(line);
std::string keyword;
std::string target;
stream >> keyword >> target;
if (keyword != "CM_" || (target != "BO_" && target != "SG_"))
return;
PendingComment pending;
pending.isSignal = target == "SG_";
pending.lineNumber = context.lineNumber;
std::string idText;
if (!(stream >> idText))
throw std::runtime_error("Missing CAN ID in CM_ statement.");
NormalizeCanId(ParseCanId(idText), pending.canId, pending.isExtended);
if (pending.isSignal && (!(stream >> pending.signalName) ||
pending.signalName.find_first_of("\";") != std::string::npos))
throw std::runtime_error("Missing or invalid signal name in CM_ statement.");
stream >> std::ws;
if (stream.get() != '"')
throw std::runtime_error("Missing opening quote in CM_ statement.");
bool closed = false;
char character;
while (stream.get(character)) {
if (character == '"') {
closed = true;
break;
}
if (character == '\\') {
if (!stream.get(character) || (character != '"' && character != '\\'))
throw std::runtime_error("Invalid escape in CM_ statement.");
}
pending.comment += character;
}
if (!closed)
throw std::runtime_error("Unterminated CM_ comment.");
stream >> std::ws;
if (stream.get() != ';')
throw std::runtime_error("Missing semicolon in CM_ statement.");
stream >> std::ws;
if (!stream.eof())
throw std::runtime_error("Unexpected trailing text in CM_ statement.");
context.pendingComments.push_back(std::move(pending));
}
void DbcParseContext::ResolveComments(const std::string &filePath) {
for (const PendingComment &pending: pendingComments) {
FrameInfo *frame = FindFrameById(database, pending.canId, pending.isExtended);
if (!frame)
throw DbcDiagnosticError(DbcDiagnosticCode::InvalidStatement, filePath,
pending.lineNumber, "Unresolved CM_ frame reference.");
if (pending.isSignal) {
SignalInfo *signal = FindSignalByName(*frame, pending.signalName);
if (!signal)
throw DbcDiagnosticError(DbcDiagnosticCode::InvalidStatement, filePath,
pending.lineNumber, "Unresolved CM_ signal reference: " + pending.signalName);
signal->comment = pending.comment;
} else {
frame->comment = pending.comment;
}
}
}
DbcStatementDispatcher::DbcStatementDispatcher() {
+12 -2
View File
@@ -17,6 +17,15 @@ struct PendingValueDescriptions {
std::size_t lineNumber;
};
struct PendingComment {
std::uint32_t canId;
bool isExtended;
bool isSignal;
std::string signalName;
std::string comment;
std::size_t lineNumber;
};
/** Carries deferred validation locations without parsing exception text. */
class DbcDiagnosticError : public std::runtime_error {
public:
@@ -38,6 +47,9 @@ struct DbcParseContext {
std::vector<std::vector<std::size_t>> signalLines = {};
void ValidateMultiplexing(const std::string &filePath) const;
std::vector<PendingComment> pendingComments = {};
void ResolveComments(const std::string &filePath);
void ResolveValueDescriptions(const std::string &filePath);
};
@@ -67,8 +79,6 @@ class DbcCommentHandler : public DbcStatementHandler {
public:
void Handle(const std::string &line, DbcParseContext &context) const override;
private:
static void ParseCommentLine(const std::string &line, DbcDatabase &database);
};
class DbcValueDescriptionHandler : public DbcStatementHandler {
+5
View File
@@ -75,6 +75,11 @@ On Linux, reading a directory exercises the distinct `FileReadError` path.
Parser tests cover standard and extended CAN IDs, whitespace, payload storage
boundaries, missing fields, malformed and overflowing numbers, reserved ID bits,
error paths and line numbers, and integration with signals and comments.
Comment regressions cover tabs, escaped quotes/backslashes, multiline text with
blank lines and indentation, CRLF, missing final newlines, comments preceding
frame/signal definitions, last-wins and empty overwrites, normalized ID boundaries,
standard/extended separation, malformed syntax/IDs, and missing targets. Errors
verify both structured diagnostics and the throwing API at the starting line.
Duplicate definitions are rejected at their source lines, including equivalent
extended ID encodings and repeated signal names within a frame. Standard and
extended frames may share a numeric ID, and separate frames may reuse signal names.
+1 -4
View File
@@ -312,10 +312,7 @@ void CoverageTest::comments_data() {
QTest::newRow("tabs") << QString::fromUtf8("") << QString::fromUtf8("CM_\tBO_ 1 \"Frame\";\nCM_\tSG_ 1 State \"Signal\";\n") << QString::fromUtf8("Frame") << QString::fromUtf8("Signal");
QTest::newRow("empty-overwrite") << QString::fromUtf8("") << QString::fromUtf8("CM_ BO_ 1 \"Old\";\nCM_ BO_ 1 \"\";\nCM_ SG_ 1 State \"Old\";\nCM_ SG_ 1 State \"\";\n") << QString::fromUtf8("") << QString::fromUtf8("");
QTest::newRow("last-wins") << QString::fromUtf8("") << QString::fromUtf8("CM_ BO_ 1 \"First\";\nCM_ BO_ 1 \"Last\";\nCM_ SG_ 1 State \"First\";\nCM_ SG_ 1 State \"Last\";\n") << QString::fromUtf8("Last") << QString::fromUtf8("Last");
QTest::newRow("missing-targets") << QString::fromUtf8("") << QString::fromUtf8("CM_ BO_ 2 \"Unknown\";\nCM_ SG_ 2 State \"Unknown\";\nCM_ SG_ 1 Missing \"Unknown\";\n") << QString::fromUtf8("") << QString::fromUtf8("");
// Current documented limitation: comments resolve only already parsed targets.
QTest::newRow("before-definitions") << QString::fromUtf8("CM_ BO_ 1 \"Early\";\nCM_ SG_ 1 State \"Early\";\n") << QString::fromUtf8("") << QString::fromUtf8("") << QString::fromUtf8("");
QTest::newRow("missing-quotes") << QString::fromUtf8("") << QString::fromUtf8("CM_ BO_ 1 Missing;\nCM_ SG_ 1 State \"Unclosed;\n") << QString::fromUtf8("") << QString::fromUtf8("");
QTest::newRow("before-definitions") << QString::fromUtf8("CM_ BO_ 1 \"Early\";\nCM_ SG_ 1 State \"Early\";\n") << QString::fromUtf8("") << QString::fromUtf8("Early") << QString::fromUtf8("Early");
}
void CoverageTest::comments() {
+100
View File
@@ -75,6 +75,9 @@ private Q_SLOTS:
void duplicateFrames();
void duplicateSignals();
void distinctDefinitions();
void comments();
void invalidComments_data();
void invalidComments();
void integration();
void invalidSignals();
void validSignals();
@@ -489,6 +492,103 @@ void DbcParserTest::distinctDefinitions() {
QCOMPARE(extended.definition->comment, std::string("Extended"));
}
void DbcParserTest::comments() {
Fixture fixture;
fixture.Write(
"CM_\tBO_\t1\t\"First\";\r\n"
"CM_\tSG_\t1\tState\t\" Line one \r\n"
"\r\n"
"\tBO_ is text; \\\"quoted\\\" and \\\\slash\"\t;\r\n"
"CM_ BO_ 2147483649 \"Extended\";\r\n"
"CM_ SG_ 2147483649 State \"Extended signal\";\r\n"
"BO_ 1 Standard: 1 ECU\r\n"
"CM_ BO_ 1 \"Last\";\r\n"
"SG_ State : 0|8@1+ (1,0) [0|255] \"\" ECU\r\n"
"BO_ 2147483649 Extended: 1 ECU\r\n"
"SG_ State : 0|8@1+ (1,0) [0|255] \"\" ECU");
const DbcParseResult result = DbcParser().ParseFileWithDiagnostics(fixture.path.string());
QVERIFY(!result.HasErrors());
QVERIFY(result.diagnostics.empty());
QCOMPARE(result.database.frames.size(), std::size_t(2));
QCOMPARE(result.database.frames[0].comment, std::string("Last"));
QCOMPARE(result.database.frames[0].signals[0].comment,
std::string(" Line one \n\n\tBO_ is text; \"quoted\" and \\slash"));
QCOMPARE(result.database.frames[1].comment, std::string("Extended"));
QCOMPARE(result.database.frames[1].signals[0].comment, std::string("Extended signal"));
const std::string ids[][2] = {
{"0", "0"},
{"2047", "2047"},
{"2048", "2147485696"},
{"536870911", "2684354559"},
{"2684354559", "536870911"}};
for (const std::string(&id)[2]: ids) {
fixture.Write("CM_ BO_ " + id[0] + " \"Frame\";\nCM_ SG_ " + id[0] +
" State \"Signal\";\nBO_ " + id[1] + " Frame: 1 ECU\n"
"SG_ State : 0|8@1+ (1,0) [0|255] \"\" ECU\n");
const DbcDatabase database = DbcParser().ParseFile(fixture.path.string());
QCOMPARE(database.frames[0].comment, std::string("Frame"));
QCOMPARE(database.frames[0].signals[0].comment, std::string("Signal"));
}
fixture.Write("CM_ BO_ 1 \"Line one\nLine two\";\n"
"BO_ 1 Frame: 0 ECU\nUNKNOWN_ value\nBO_ bad Frame: 0 ECU\n");
const DbcParseResult following = DbcParser().ParseFileWithDiagnostics(fixture.path.string());
QVERIFY(following.HasErrors());
QCOMPARE(following.diagnostics.size(), std::size_t(2));
QCOMPARE(following.diagnostics[0].lineNumber, std::size_t(4));
QCOMPARE(following.diagnostics[1].lineNumber, std::size_t(5));
}
void DbcParserTest::invalidComments_data() {
QTest::addColumn<QString>("statement");
const char *ids[] = {"-1", "+1", "1x", "0x1", "4294967296", "536870912",
"1073741824", "2684354560", "4294967295"};
for (const char *id: ids) {
const std::string frameRow = std::string("frame-id-") + id;
const std::string signalRow = std::string("signal-id-") + id;
QTest::newRow(frameRow.c_str()) << QString::fromStdString("CM_ BO_ " + std::string(id) + " \"Text\";");
QTest::newRow(signalRow.c_str()) << QString::fromStdString("CM_ SG_ " + std::string(id) + " State \"Text\";");
}
QTest::newRow("missing-id") << QStringLiteral("CM_ BO_");
QTest::newRow("missing-signal") << QStringLiteral("CM_ SG_ 1 \"Text\";");
QTest::newRow("missing-quote") << QStringLiteral("CM_ BO_ 1 Text;");
QTest::newRow("unclosed") << QStringLiteral("CM_ SG_ 1 State \"Text\n More");
QTest::newRow("invalid-escape") << QStringLiteral("CM_ BO_ 1 \"bad\\q\";");
QTest::newRow("dangling-escape") << QStringLiteral("CM_ BO_ 1 \"bad\\");
QTest::newRow("missing-semicolon") << QStringLiteral("CM_ SG_ 1 State \"Text\"");
QTest::newRow("trailing-text") << QStringLiteral("CM_ BO_ 1 \"Text\"; garbage");
QTest::newRow("extra-header") << QStringLiteral("CM_ BO_ 1 extra \"Text\";");
QTest::newRow("extra-string") << QStringLiteral("CM_ BO_ 1 \"Text\" \"More\";");
QTest::newRow("missing-frame") << QStringLiteral("CM_ BO_ 2 \"Text\";");
QTest::newRow("missing-signal-frame") << QStringLiteral("CM_ SG_ 2 State \"Text\";");
QTest::newRow("missing-signal-reference") << QStringLiteral("CM_ SG_ 1 Missing \"Text\";");
QTest::newRow("different-frame-type") << QStringLiteral("CM_ BO_ 2147483649 \"Text\";");
QTest::newRow("missing-multiline-reference") << QStringLiteral("CM_ SG_ 1 Missing \"Text\nMore\";");
}
void DbcParserTest::invalidComments() {
QFETCH(QString, statement);
Fixture fixture;
fixture.Write("BO_ 1 Frame: 1 ECU\nSG_ State : 0|8@1+ (1,0) [0|255] \"\" ECU\n\n" +
statement.toStdString());
const DbcParseResult result = DbcParser().ParseFileWithDiagnostics(fixture.path.string());
QVERIFY(result.HasErrors());
QVERIFY(result.database.frames.empty());
QCOMPARE(result.diagnostics.size(), std::size_t(1));
const DbcDiagnostic &error = result.diagnostics[0];
QCOMPARE(error.severity, DbcDiagnosticSeverity::Error);
QCOMPARE(error.code, DbcDiagnosticCode::InvalidStatement);
QCOMPARE(error.filePath, fixture.path.string());
QCOMPARE(error.lineNumber, std::size_t(4));
QVERIFY(!error.message.empty());
try {
DbcParser().ParseFile(fixture.path.string());
QFAIL("Malformed or unresolved comment must throw.");
} catch (const std::runtime_error &exception) {
QVERIFY(std::string(exception.what()).find(fixture.path.string() + ":4: ") == 0U);
}
}
void DbcParserTest::integration() {
Fixture fixture;
fixture.Write(