Closes #12

changelog.md

@@ -24,6 +24,14 @@ existing Git tag; subsequent changes are listed as unreleased.
 
 - Overlapping unconditional signals are now rejected during parsing.
 
+#### Fixed
+
+- Reject duplicate frames by normalized CAN ID and frame type, including
+  equivalent extended ID encodings, and duplicate signal names within a frame.
+  Errors report the file path and line of the repeated definition. Standard and
+  extended frames may share a numeric ID; separate frames may reuse signal names.
+  Added regression coverage and updated parser documentation.
+
 ### 2026-10-06
 
 #### Added

core/dbc_parser.h

@@ -39,6 +39,8 @@ public:
      * @return Parsed database.
      * @throws std::runtime_error on file or parse errors.
      * Line parse errors include the file path and one-based line number.
+     * Duplicate normalized CAN IDs of the same frame type and duplicate signal
+     * names within a frame are rejected at the repeated definition's line.
      */
     DbcDatabase ParseFile(const std::string &filePath) const;
 

core/dbc_statement_handler.cpp

@@ -340,7 +340,11 @@ namespace {
 }// namespace
 
 void DbcFrameHandler::Handle(const std::string &line, DbcParseContext &context) const {
-    context.database.frames.push_back(ParseFrameLine(line));
+    FrameInfo frame = ParseFrameLine(line);
+    if (FindFrameById(context.database, frame.canId, frame.isExtended) != nullptr)
+        throw std::runtime_error("Duplicate " + std::string(frame.isExtended ? "extended" : "standard") +
+                                 " frame CAN ID: " + std::to_string(frame.canId));
+    context.database.frames.push_back(std::move(frame));
     context.currentFrame = &context.database.frames.back();
     context.signalLines.emplace_back();
 }
@@ -349,6 +353,9 @@ void DbcSignalHandler::Handle(const std::string &line, DbcParseContext &context)
     if (context.currentFrame == nullptr)
         throw std::runtime_error("Signal found before any frame definition.");
     SignalInfo signal = ParseSignalLine(line);
+    if (FindSignalByName(*context.currentFrame, signal.name) != nullptr)
+        throw std::runtime_error("Duplicate signal name '" + signal.name +
+                                 "' in frame '" + context.currentFrame->name + "'.");
     const std::uint64_t payloadBits = static_cast<std::uint64_t>(context.currentFrame->dlc) * 8U;
     std::uint64_t bit = signal.startBit;
     for (std::uint32_t index = 0U; index < signal.length; ++index) {

tests/README.md

@@ -47,6 +47,9 @@ found; insufficient payloads are checked through signal validity.
 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.
+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.
 Signal tests cover complete syntax and numeric validation, scientific notation,
 finite physical parameters and ordered ranges, spaces and tabs, receiver lists,
 unsupported multiplexing, and Intel/Motorola declared-payload boundaries.

tests/dbc_parser_test.cpp

@@ -42,8 +42,8 @@ public:
 std::string SignalLine(const std::string &start, const std::string &length,
                        const std::string &order, const std::string &factor,
                        const std::string &offset, const std::string &minimum,
-                       const std::string &maximum) {
+                       const std::string &maximum, const std::string &name = "Value") {
-    return "SG_ Value : " + start + "|" + length + "@" + order +
+    return "SG_ " + name + " : " + start + "|" + length + "@" + order +
            " (" + factor + "," + offset + ") [" + minimum + "|" + maximum + "] \"\" ECU";
 }
 
@@ -67,6 +67,9 @@ private Q_SLOTS:
     void multiplexingBoundaries();
     void validFrames();
     void invalidFrames();
+    void duplicateFrames();
+    void duplicateSignals();
+    void distinctDefinitions();
     void integration();
     void invalidSignals();
     void validSignals();
@@ -134,6 +137,93 @@ void DbcParserTest::invalidFrames() {
     }
 }
 
+void DbcParserTest::duplicateFrames() {
+    Fixture fixture;
+    struct DuplicateCase {
+        const char *firstId;
+        const char *duplicateId;
+        const char *message;
+    };
+    const DuplicateCase cases[] = {
+        {"1", "1", "Duplicate standard frame CAN ID: 1"},
+        {"2147483649", "2147483649", "Duplicate extended frame CAN ID: 1"},
+        {"2048", "2147485696", "Duplicate extended frame CAN ID: 2048"},
+        {"2147485696", "2048", "Duplicate extended frame CAN ID: 2048"},
+        {"536870911", "2684354559", "Duplicate extended frame CAN ID: 536870911"}
+    };
+    const DbcParser parser;
+    for (const DuplicateCase &test : cases) {
+        fixture.Write("BO_ " + std::string(test.firstId) + " First: 8 ECU\n"
+                      "BO_ 2 Other: 8 ECU\n\n"
+                      "  BO_\t" + test.duplicateId + "\tSecond: 8 ECU\n");
+        try {
+            parser.ParseFile(fixture.path.string());
+            CHECK(false, "Duplicate frame must be rejected");
+        } catch (const std::runtime_error &error) {
+            QCOMPARE(std::string(error.what()), fixture.path.string() + ":4: " + test.message);
+        }
+    }
+    fixture.Write("BO_ 1 First: 8 ECU\n");
+    QCOMPARE(parser.ParseFile(fixture.path.string()).frames.size(), std::size_t(1U));
+}
+
+void DbcParserTest::duplicateSignals() {
+    Fixture fixture;
+    const std::string first = "SG_ State : 0|8@1+ (1,0) [0|255] \"\" ECU\n";
+    const std::string duplicates[] = {
+        first,
+        "  SG_\tState : 16|8@1+ (2,1) [0|511] \"V\" ECU\n",
+        "SG_ State m1 : 16|8@1+ (1,0) [0|255] \"\" ECU\n"
+    };
+    for (const std::string &duplicate : duplicates) {
+        fixture.Write("BO_ 1 Frame: 8 ECU\n" + first +
+                      "SG_ Other : 8|8@1+ (1,0) [0|255] \"\" ECU\n\n" + duplicate);
+        try {
+            DbcParser().ParseFile(fixture.path.string());
+            CHECK(false, "Duplicate signal must be rejected");
+        } catch (const std::runtime_error &error) {
+            QCOMPARE(std::string(error.what()), fixture.path.string() +
+                     ":5: Duplicate signal name 'State' in frame 'Frame'.");
+        }
+    }
+}
+
+void DbcParserTest::distinctDefinitions() {
+    Fixture fixture;
+    fixture.Write("BO_ 1 Shared: 1 ECU\n"
+                  "SG_ State : 0|8@1+ (1,0) [0|255] \"\" ECU\n"
+                  "BO_ 2147483649 Shared: 1 ECU\n"
+                  "SG_ State : 0|8@1+ (1,0) [0|255] \"\" ECU\n"
+                  "BO_ 2 Shared: 2 ECU\n"
+                  "SG_ State : 0|8@1+ (1,0) [0|255] \"\" ECU\n"
+                  "SG_ state : 8|8@1+ (1,0) [0|255] \"\" ECU\n"
+                  "CM_ BO_ 1 \"Standard\";\n"
+                  "CM_ BO_ 2147483649 \"Extended\";\n"
+                  "CM_ SG_ 1 State \"Standard state\";\n"
+                  "CM_ SG_ 2147483649 State \"Extended state\";\n");
+    const DbcDatabase parsed = DbcParser().ParseFile(fixture.path.string());
+    QCOMPARE(parsed.frames.size(), std::size_t(3U));
+    QCOMPARE(parsed.frames[0].canId, parsed.frames[1].canId);
+    CHECK(!parsed.frames[0].isExtended && parsed.frames[1].isExtended,
+          "Standard and extended frames with the same numeric ID remain distinct");
+    QCOMPARE(parsed.frames[0].comment, std::string("Standard"));
+    QCOMPARE(parsed.frames[1].comment, std::string("Extended"));
+    QCOMPARE(parsed.frames[0].signals[0].comment, std::string("Standard state"));
+    QCOMPARE(parsed.frames[1].signals[0].comment, std::string("Extended state"));
+    QCOMPARE(parsed.frames[2].signals.size(), std::size_t(2U));
+    const DecodeDatabase runtime = DbcDecodeBuilder().Build(parsed);
+    RawCanFrame raw;
+    raw.canId = 1U;
+    raw.data = {7U};
+    const DecodedFrameValue standard = DbcDecoder().Decode(runtime, raw);
+    QVERIFY(standard.valid);
+    QCOMPARE(standard.definition->comment, std::string("Standard"));
+    raw.isExtended = true;
+    const DecodedFrameValue extended = DbcDecoder().Decode(runtime, raw);
+    QVERIFY(extended.valid);
+    QCOMPARE(extended.definition->comment, std::string("Extended"));
+}
+
 void DbcParserTest::integration() {
     Fixture fixture;
     fixture.Write(
@@ -324,7 +414,7 @@ void DbcParserTest::validSignals() {
     const DbcParser signalParser;
     fixture.Write("BO_ 1 Frame: 8 ECU\n" +
                   SignalLine("0", "8", "1+", "0", "0", "0", "0") + "\n" +
-                  SignalLine("8", "8", "1-", "-1", "0", "-255", "0") + "\n");
+                  SignalLine("8", "8", "1-", "-1", "0", "-255", "0", "Negative") + "\n");
     const DbcDatabase factors = signalParser.ParseFile(fixture.path.string());
     CHECK(factors.frames[0].signals.size() == 2U &&
                   factors.frames[0].signals[0].factor == 0.0 &&
@@ -506,17 +596,18 @@ void DbcParserTest::invalidValueDescriptions() {
         }
     }
     for (const std::string suffix: {
-                 "BO_ 1 Other: 8 ECU\n",                       // Ambiguous normalized frame reference.
+                 "BO_ 1 Other: 8 ECU\n",                       // Duplicate frame definition.
-                 "SG_ State : 0|8@1+ (1,0) [0|255] \"\" ECU\n",// Ambiguous signal reference.
+                 "SG_ State : 0|8@1+ (1,0) [0|255] \"\" ECU\n",// Duplicate signal definition.
                  ""}) {
         fixture.Write("VAL_ 1 " + std::string(suffix.empty() ? "Missing" : "State") +
                       " 0 \"Description\";\n" + definitions + suffix);
         try {
             DbcParser().ParseFile(fixture.path.string());
-            CHECK(false, "Deferred missing or ambiguous reference");
+            CHECK(false, "Missing reference or duplicate definition");
         } catch (const std::runtime_error &error) {
-            CHECK(std::string(error.what()).find(fixture.path.string() + ":1: ") == 0,
+            const std::string line = suffix.empty() ? "1" : "4";
-                  "Deferred reference retains source location");
+            CHECK(std::string(error.what()).find(fixture.path.string() + ":" + line + ": ") == 0,
+                  "Error retains the reference or duplicate definition source location");
         }
     }
     for (const std::string number: {"+1", "1x", "0x10", "1.5", "1e2", "--1",