From e555ebb5717c5b5846a5298f5944bc46025c6c3f Mon Sep 17 00:00:00 2001 From: deeaitch Date: Tue, 20 Oct 2026 13:12:30 -0400 Subject: [PATCH] Closes #24 --- README.md | 62 ++++++++++--- changelog.md | 15 +++ core/core_files.pri | 1 + core/dbc_decoder.cpp | 22 ++++- core/dbc_decoder.h | 15 ++- core/dbc_parser.cpp | 1 + core/dbc_parser.h | 9 ++ core/dbc_statement_handler.cpp | 31 +------ core/dbc_statement_handler.h | 1 + core/decode_database.cpp | 22 +++++ core/decode_database.h | 12 ++- core/frame_info.h | 2 +- core/j1939.h | 17 ++++ tests/README.md | 12 +++ tests/dbc_coverage_test.cpp | 165 ++++++++++++++++++++++++++++++++- tests/dbc_pipeline_test.cpp | 2 +- 16 files changed, 337 insertions(+), 52 deletions(-) create mode 100644 core/j1939.h diff --git a/README.md b/README.md index ba9e2a7..ce2726b 100644 --- a/README.md +++ b/README.md @@ -193,7 +193,7 @@ Notes: - `signals` is a list of `SignalInfo` - `isExtended` is determined during CAN ID normalization -- `pgn` is derived using simplified J1939 logic when applicable +- `pgn` is derived for extended frames only with explicit J1939 opt-in (see below) --- @@ -476,14 +476,16 @@ Fields: `valid` means a matching frame definition was found. Check each signal's `valid` separately: insufficient payload data can leave individual signals -invalid even when the frame is valid. An unknown frame returns `valid == false`, -a null definition, its requested key, and an empty signal list. +invalid even when the frame is valid. An unknown or ambiguous frame returns `valid == false`, +a null definition, its requested key, and an empty signal list. A successful PGN +match uses the DBC definition key for the frame and its signals. `FrameDecodeStatus` summarizes the result: | Status | Meaning | | --- | --- | -| `UnknownFrame` | No definition matches both CAN ID and standard/extended type | +| `UnknownFrame` | No exact CAN ID/type match or eligible PGN match | +| `AmbiguousFrame` | No exact match and multiple J1939 definitions share the PGN | | `InsufficientPayload` | Actual byte count is less than the declared `dlc` | | `SignalErrors` | Payload is long enough, but at least one signal failed | | `Valid` | Payload is long enough and every signal is valid or inactive | @@ -875,17 +877,55 @@ This is important both for correct lookup and for future interoperability with l --- -## PGN Extraction +## J1939 Opt-in and PGN Lookup -PGN is derived only when the frame is treated as extended. +Extended CAN identifiers do not imply J1939. The default parser leaves `hasPgn` +false and `pgn` zero, and decoding uses exact normalized CAN ID and frame type. +Enable J1939 interpretation explicitly for a file: -The current logic is simplified J1939 extraction: +```cpp +DbcParserOptions options; +options.enableJ1939 = true; +const DbcDatabase parsed = DbcParser(options).ParseFile("vehicle.dbc"); +const DecodeDatabase runtime = DbcDecodeBuilder().Build(parsed); +``` -- `pf` -- `ps` -- `dp` +Both parser entry points honor this option. Standard frames remain ordinary CAN. +For mixed-protocol files, parse with the default options, then set `hasPgn = true` +and `pgn = ExtractJ1939Pgn(frame.canId)` (from `j1939.h`) only on known J1939 +extended definitions before building the runtime snapshot. The same explicit +metadata enables J1939 for manually constructed `DecodeFrame` definitions. +Runtime construction rejects `hasPgn` on standard frames or an inconsistent PGN. +No DBC attribute automatically enables J1939 interpretation. -This is enough for a practical start but should not be treated as full J1939 validation. +PGN extraction retains both data-page bits (CAN ID bits 25–24) and PF: + +- PDU1 (`PF < 240`): PS is the destination address and is zeroed in the PGN. +- PDU2 (`PF >= 240`): PS is the group extension and remains part of the PGN. +- Priority and source address are excluded in both cases. This follows the + [Linux J1939 PGN layout](https://docs.kernel.org/networking/j1939.html). + +`DecodeDatabase::FindFrame(FrameKey)` stays an exact lookup. +`FindFrameByPgn(pgn)` returns a `PgnLookupResult` with an owning `definition` +handle for one eligible definition. No match returns a null handle with +`ambiguous == false`; multiple definitions return a null handle with +`ambiguous == true`. Copies and returned handles retain the immutable snapshot. + +`DbcDecoder::FindFrame` and both `Decode` overloads first try the exact ID/type. +Only when that fails for a valid normalized extended ID do they try PGN lookup +among definitions with `hasPgn == true`. A unique PGN can therefore match a +changed source address or priority, and for PDU1 a changed destination address. +A changed PDU2 group extension does not match. Exact matches win even when the +PGN is ambiguous, including exact matches to ordinary extended CAN definitions. +Multiple PGN candidates are never resolved by order, source, or payload: +`FindFrame` returns null and `Decode` returns `AmbiguousFrame` without decoding +signals or validating the selection. Missing candidates return `UnknownFrame`. + +On success, decoded frame/signal keys belong to the matched DBC definition; +selected-signal decoding also expects those definition keys. Keep the input +`RawCanFrame` if the received address/ID is needed separately. This feature does +not implement transport-protocol reassembly, address claiming, or full J1939 +validation. --- diff --git a/changelog.md b/changelog.md index 931e187..62762f5 100644 --- a/changelog.md +++ b/changelog.md @@ -8,6 +8,21 @@ existing Git tag; subsequent changes are listed as unreleased. ### 2026-10-20 +#### Changed + +- J1939 interpretation now requires `DbcParserOptions::enableJ1939` (default off) + or explicit per-frame `hasPgn` metadata. Extended CAN alone no longer derives + PGN metadata. Runtime construction validates opted-in PGNs against frame IDs. +- Add indexed PGN lookup with PDU1 destination exclusion, PDU2 group extension, + and both data-page bits. Decoder exact-ID matches take precedence; otherwise + one J1939 PGN candidate is accepted and multiple candidates return + `AmbiguousFrame`. Result and selection keys identify the DBC definition. +- Add opt-in, PDU1/PDU2, address/priority changes, ambiguity, exact precedence, + metadata validation, selected decode, and snapshot-lifetime regressions. + Verified with `qmake6 ../dbc.pro`, `make -j4`, and `make check` in `build/`: + all six QtTest suites and the Qt-free tree target passed; the existing named + comma-decimal C locale test skipped because that locale is not installed. + #### Added - Expose stable frame and signal keys from Qt-free tree nodes using the existing diff --git a/core/core_files.pri b/core/core_files.pri index 0ccf7c1..d93b7da 100644 --- a/core/core_files.pri +++ b/core/core_files.pri @@ -8,6 +8,7 @@ SOURCES += \ $$PWD/tree_node.cpp HEADERS += \ + $$PWD/j1939.h \ $$PWD/signal_layout.h \ $$PWD/definition_key.h \ $$PWD/dbc_diagnostic.h \ diff --git a/core/dbc_decoder.cpp b/core/dbc_decoder.cpp index b97e30c..cc0bea9 100644 --- a/core/dbc_decoder.cpp +++ b/core/dbc_decoder.cpp @@ -10,6 +10,7 @@ */ #include "dbc_decoder.h" +#include "j1939.h" #include "signal_layout.h" #include #include @@ -19,7 +20,17 @@ std::shared_ptr DbcDecoder::FindFrame(const DecodeDatabase &database, std::uint32_t canId, bool isExtended) const { - return database.FindFrame(FrameKey(canId, isExtended)); + return ResolveFrame(database, canId, isExtended).definition; +} + +PgnLookupResult DbcDecoder::ResolveFrame(const DecodeDatabase &database, + std::uint32_t canId, bool isExtended) { + const std::shared_ptr exact = database.FindFrame(FrameKey(canId, isExtended)); + if (exact) + return {exact, false}; + if (!isExtended || canId > 0x1FFFFFFFU) + return {}; + return database.FindFrameByPgn(ExtractJ1939Pgn(canId)); } DecodedFrameValue DbcDecoder::Decode(const DecodeDatabase &database, @@ -38,10 +49,15 @@ DecodedFrameValue DbcDecoder::DecodeImpl(const DecodeDatabase &database, const std::vector *selection) const { DecodedFrameValue result; result.key = FrameKey(frame.canId, frame.isExtended); - const std::shared_ptr definition = FindFrame(database, frame.canId, frame.isExtended); - + const PgnLookupResult match = ResolveFrame(database, frame.canId, frame.isExtended); + if (match.ambiguous) { + result.status = FrameDecodeStatus::AmbiguousFrame; + return result; + } + const std::shared_ptr definition = match.definition; if (definition == nullptr) return result; + result.key = definition->GetKey(); std::vector selected; if (selection != nullptr) { diff --git a/core/dbc_decoder.h b/core/dbc_decoder.h index a5d6655..9165cdb 100644 --- a/core/dbc_decoder.h +++ b/core/dbc_decoder.h @@ -43,7 +43,8 @@ enum class FrameDecodeStatus { UnknownFrame, InsufficientPayload, SignalErrors, - Valid + Valid, + AmbiguousFrame }; /** @@ -70,7 +71,7 @@ struct DecodedSignalValue { */ struct DecodedFrameValue { std::shared_ptr definition; /**< Owning immutable definition. */ - FrameKey key; /**< Stable application identifier. */ + FrameKey key; /**< Matched definition key; received key if unmatched. */ std::vector signals; /**< Decoded signal values. */ bool valid = false; /**< true if frame was matched. */ FrameDecodeStatus status = FrameDecodeStatus::UnknownFrame; /**< Inspect signals for individual failures. */ @@ -84,11 +85,11 @@ struct DecodedFrameValue { class DbcDecoder { public: /** - * @brief Find frame definition by CAN ID. + * @brief Find exact CAN ID first, then unique PGN among explicitly J1939 definitions. * @param database Runtime decode database. * @param canId Normalized CAN ID. * @param isExtended true for extended frame. - * @return Owning immutable frame handle or nullptr. Safe across reload. + * @return Owning immutable handle, or nullptr for missing/ambiguous matches. Safe across reload. */ std::shared_ptr FindFrame(const DecodeDatabase &database, std::uint32_t canId, @@ -108,7 +109,8 @@ public: * @brief Decode selected signals in selection order using stable keys. * An empty selection returns no signals. Multiplex selectors are read internally. * Frame status summarizes selected signals; short payload still takes precedence. - * Unknown frames return UnknownFrame before checking the selection. + * Unknown/ambiguous frames return before checking the selection. + * For PGN matches, selection keys must belong to the matched DBC definition. * @throws std::invalid_argument for a foreign frame key, unknown signal, or duplicate key. */ DecodedFrameValue Decode(const DecodeDatabase &database, @@ -116,6 +118,9 @@ public: const std::vector &selection) const; private: + static PgnLookupResult ResolveFrame(const DecodeDatabase &database, + std::uint32_t canId, bool isExtended); + DecodedFrameValue DecodeImpl(const DecodeDatabase &database, const RawCanFrame &frame, const std::vector *selection) const; diff --git a/core/dbc_parser.cpp b/core/dbc_parser.cpp index 000ccb7..f729ad0 100644 --- a/core/dbc_parser.cpp +++ b/core/dbc_parser.cpp @@ -83,6 +83,7 @@ DbcParseResult DbcParser::ParseFileWithDiagnostics(const std::string &filePath) DbcDatabase database; DbcParseContext context{database, nullptr}; + context.enableJ1939 = options.enableJ1939; const DbcStatementDispatcher dispatcher; bool inNamespace = false; std::size_t lineNumber = 0U; diff --git a/core/dbc_parser.h b/core/dbc_parser.h index 4d4c62e..f606f5c 100644 --- a/core/dbc_parser.h +++ b/core/dbc_parser.h @@ -16,6 +16,11 @@ #include "dbc_diagnostic.h" +/** Explicit interpretation applied to extended frames in this file. */ +struct DbcParserOptions { + bool enableJ1939 = false; +}; + /** * @brief Minimal DBC parser. * @@ -34,6 +39,8 @@ */ class DbcParser { public: + explicit DbcParser(DbcParserOptions options = {}) : options(options) {} + /** * @brief Parse DBC file. * @param filePath Path to input file. @@ -53,6 +60,8 @@ public: */ DbcParseResult ParseFileWithDiagnostics(const std::string &filePath) const; +private: + DbcParserOptions options; }; #endif /* DBC_PARSER_H */ diff --git a/core/dbc_statement_handler.cpp b/core/dbc_statement_handler.cpp index 919a943..47e4723 100644 --- a/core/dbc_statement_handler.cpp +++ b/core/dbc_statement_handler.cpp @@ -1,5 +1,6 @@ // SPDX-License-Identifier: MIT #include "dbc_statement_handler.h" +#include "j1939.h" #include "text_utils.h" #include "signal_layout.h" #include @@ -167,31 +168,6 @@ namespace { normalizedCanId = rawCanId; } } - - std::uint32_t TryExtractPgn(std::uint32_t canId, bool isExtended, bool &hasPgn) { - hasPgn = false; - - if (!isExtended) - return 0U; - - if ((canId & 0x1FFFFFFFU) != canId) - return 0U; - - const std::uint32_t pf = (canId >> 16U) & 0xFFU; - const std::uint32_t ps = (canId >> 8U) & 0xFFU; - const std::uint32_t dp = (canId >> 24U) & 0x01U; - - std::uint32_t pgn = 0U; - - if (pf < 240U) - pgn = (dp << 16U) | (pf << 8U); - else - pgn = (dp << 16U) | (pf << 8U) | ps; - - hasPgn = true; - return pgn; - } - } // namespace FrameInfo DbcFrameHandler::ParseFrameLine(const std::string &line) { @@ -228,7 +204,6 @@ FrameInfo DbcFrameHandler::ParseFrameLine(const std::string &line) { throw std::runtime_error("Unexpected trailing field in frame definition: " + extra); NormalizeCanId(rawCanId, frame.canId, frame.isExtended); - frame.pgn = TryExtractPgn(frame.canId, frame.isExtended, frame.hasPgn); return frame; } @@ -307,6 +282,10 @@ namespace { void DbcFrameHandler::Handle(const std::string &line, DbcParseContext &context) const { FrameInfo frame = ParseFrameLine(line); + if (context.enableJ1939 && frame.isExtended) { + frame.hasPgn = true; + frame.pgn = ExtractJ1939Pgn(frame.canId); + } 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)); diff --git a/core/dbc_statement_handler.h b/core/dbc_statement_handler.h index e1e116e..7cab1e4 100644 --- a/core/dbc_statement_handler.h +++ b/core/dbc_statement_handler.h @@ -41,6 +41,7 @@ public: struct DbcParseContext { DbcDatabase &database; FrameInfo *currentFrame = nullptr; + bool enableJ1939 = false; std::size_t lineNumber = 0U; std::vector pendingValues = {}; diff --git a/core/decode_database.cpp b/core/decode_database.cpp index 9956e43..58e02dd 100644 --- a/core/decode_database.cpp +++ b/core/decode_database.cpp @@ -2,6 +2,7 @@ // Copyright (c) 2026 Dim Himro #include "decode_database.h" +#include "j1939.h" #include "signal_layout.h" #include #include @@ -55,6 +56,9 @@ namespace { if (frame.canId > maximumId) throw std::invalid_argument("Invalid normalized CAN ID: " + std::to_string(frame.canId)); + if (frame.hasPgn && (!frame.isExtended || frame.pgn != ExtractJ1939Pgn(frame.canId))) + throw std::invalid_argument("Invalid J1939 PGN metadata: " + frame.name); + std::unordered_set names; const DecodeSignal *selector = nullptr; for (const DecodeSignal &signal: frame.signals) { @@ -99,6 +103,8 @@ DecodeDatabase::Storage::Storage(const std::vector &definitions) frameIndexByKey.reserve(frames.size()); for (std::size_t index = 0U; index < frames.size(); ++index) { frameIndexByKey.emplace(frames[index].GetKey(), index); + if (frames[index].hasPgn) + frameIndicesByPgn[frames[index].pgn].push_back(index); } } @@ -121,6 +127,22 @@ std::shared_ptr DecodeDatabase::FindFrame(const FrameKey &key return std::shared_ptr(storage, &storage->frames[it->second]); } +PgnLookupResult DecodeDatabase::FindFrameByPgn(std::uint32_t pgn) const { + PgnLookupResult result; + if (!storage) + return result; + const std::unordered_map>::const_iterator it = + storage->frameIndicesByPgn.find(pgn); + if (it == storage->frameIndicesByPgn.end()) + return result; + if (it->second.size() != 1U) { + result.ambiguous = true; + return result; + } + result.definition = std::shared_ptr(storage, &storage->frames[it->second[0]]); + return result; +} + std::shared_ptr DecodeDatabase::FindSignal(const SignalKey &key) const { const std::shared_ptr frame = FindFrame(key.frame); if (!frame) diff --git a/core/decode_database.h b/core/decode_database.h index 6d8eb82..3c85847 100644 --- a/core/decode_database.h +++ b/core/decode_database.h @@ -68,7 +68,7 @@ struct DecodeFrame { bool isExtended = false; /**< true for extended frame. */ std::uint8_t dlc = 0U; /**< Payload length. */ std::uint32_t pgn = 0U; /**< PGN if available. */ - bool hasPgn = false; /**< true if PGN is valid. */ + bool hasPgn = false; /**< Explicit J1939 opt-in; pgn must match canId. */ std::string transmitter; /**< Transmitter ECU. */ std::string comment; /**< Frame comment. */ std::vector signals; /**< Signal definitions. */ @@ -80,6 +80,12 @@ struct DecodeFrame { } }; +/** A PGN lookup rejects multiple definitions rather than choosing by order. */ +struct PgnLookupResult { + std::shared_ptr definition; + bool ambiguous = false; +}; + /** * @brief Immutable runtime snapshot. Copies share storage; reload replaces a snapshot. * Owning lookup handles and decoded results keep old definitions alive. @@ -98,6 +104,8 @@ public: const std::vector &GetFrames() const; /** Owning handles retain the entire snapshot across reload or destruction. */ std::shared_ptr FindFrame(const FrameKey &key) const; + /** Only explicitly marked hasPgn frames participate; ambiguous results have no definition. */ + PgnLookupResult FindFrameByPgn(std::uint32_t pgn) const; std::shared_ptr FindSignal(const SignalKey &key) const; private: @@ -105,6 +113,8 @@ private: std::vector frames; std::unordered_map frameIndexByKey; + std::unordered_map> frameIndicesByPgn; + explicit Storage(const std::vector &definitions); }; diff --git a/core/frame_info.h b/core/frame_info.h index 63da875..c4ee869 100644 --- a/core/frame_info.h +++ b/core/frame_info.h @@ -26,7 +26,7 @@ struct FrameInfo { std::uint32_t canId = 0U; /**< Normalized CAN identifier. */ bool isExtended = false; /**< true for extended frame. */ std::uint32_t pgn = 0U; /**< J1939 PGN if applicable. */ - bool hasPgn = false; /**< true if PGN was derived. */ + bool hasPgn = false; /**< Explicit J1939 opt-in; pgn is derived from canId. */ std::uint8_t dlc = 0U; /**< Frame payload length. */ std::string transmitter; /**< Transmitter ECU name. */ std::string comment; /**< Optional frame comment. */ diff --git a/core/j1939.h b/core/j1939.h new file mode 100644 index 0000000..5b8826c --- /dev/null +++ b/core/j1939.h @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: MIT +#ifndef J1939_H +#define J1939_H + +#include + +/** Extract an 18-bit PGN from a normalized 29-bit ID explicitly interpreted as J1939. + * Priority and source address are excluded. PDU1 also excludes destination address. + * Both data-page bits are retained; this does not validate the J1939 protocol. + */ +inline std::uint32_t ExtractJ1939Pgn(std::uint32_t canId) { + const std::uint32_t pf = (canId >> 16U) & 0xFFU; + const std::uint32_t pgn = (canId >> 8U) & 0x3FFFFU; + return pf < 240U ? pgn & 0x3FF00U : pgn; +} + +#endif /* J1939_H */ diff --git a/tests/README.md b/tests/README.md index c16da32..05cb630 100644 --- a/tests/README.md +++ b/tests/README.md @@ -152,6 +152,18 @@ and source lifetime, physical conversion, and TypeFactory registration/creation. Direct resolver tests construct ambiguous databases through the public metadata API, verify source locations, and check that ambiguous targets receive no values. +J1939 regressions in `dbc_coverage_test.cpp` verify explicit parser opt-in and +ordinary extended CAN defaults, PF 239/240 and all data-page combinations, PGN +zero, PDU1 destination exclusion, PDU2 group-extension matching, changed source +and priority, standard/invalid-ID exclusion, and inconsistent manual metadata. +They also cover exact-ID precedence, order-independent ambiguous PGNs for both +PDU formats, mixed ordinary/J1939 definitions, selected-signal definition keys, +raw/physical values, and owning PGN handles across snapshot replacement. +The pipeline fixture explicitly enables J1939 through the diagnostic parser API. +Verified in `build/` with `qmake6 ../dbc.pro`, `make -j4`, and `make check`: +all six QtTest suites and the Qt-free tree target passed. The existing named +comma-decimal C locale regression skipped because that locale is not installed. + All QtTest targets use Qt 6.8.2 and `QVERIFY_THROWS_EXCEPTION`. Exception tests explicitly discard `nodiscard` results when only the exception is being checked. The standalone tree test requires only a C++17 compiler. diff --git a/tests/dbc_coverage_test.cpp b/tests/dbc_coverage_test.cpp index 4a5191e..9f42fe9 100644 --- a/tests/dbc_coverage_test.cpp +++ b/tests/dbc_coverage_test.cpp @@ -12,12 +12,12 @@ #include namespace { - DbcDatabase ParseText(const std::string &text) { + DbcDatabase ParseText(const std::string &text, bool enableJ1939 = false) { QTemporaryFile file; if (!file.open()) throw std::runtime_error("Cannot create DBC fixture"); if (file.write(text.data(), static_cast(text.size())) != static_cast(text.size()) || !file.flush()) throw std::runtime_error("Cannot write DBC fixture"); - return DbcParser().ParseFile(file.fileName().toStdString()); + return DbcParser(DbcParserOptions{enableJ1939}).ParseFile(file.fileName().toStdString()); } DecodeDatabase SingleSignal(const DecodeSignal &signal) { @@ -73,6 +73,11 @@ private Q_SLOTS: void malformedRuntimeMultiplexing(); void pgn_data(); void pgn(); + void j1939Lookup_data(); + void j1939Lookup(); + void j1939Ambiguity_data(); + void j1939Ambiguity(); + void j1939MetadataValidation(); void comments_data(); void comments(); void ignoredCommentStatements_data(); @@ -285,7 +290,7 @@ void CoverageTest::pgn_data() { QTest::addColumn("expected"); QTest::addColumn("hasPgn"); QTest::newRow("standard") << 0x7FFU << 0U << false; - for (unsigned dp: {0U, 1U}) + for (unsigned dp: {0U, 1U, 2U, 3U}) for (unsigned pf: {239U, 240U}) for (unsigned ps: {0U, 0xABU}) { const unsigned id = 0x80000000U | (dp << 24U) | (pf << 16U) | (ps << 8U) | 0x42U; @@ -299,11 +304,163 @@ void CoverageTest::pgn() { QFETCH(unsigned, id); QFETCH(unsigned, expected); QFETCH(bool, hasPgn); - const DbcDatabase parsed = ParseText("BO_ " + std::to_string(id) + " Frame: 8 ECU\n"); + const std::string text = "BO_ " + std::to_string(id) + " Frame: 8 ECU\n"; + const DbcDatabase generic = ParseText(text); + QVERIFY(!generic.frames[0].hasPgn); + QCOMPARE(generic.frames[0].pgn, 0U); + const DbcDatabase parsed = ParseText(text, true); QCOMPARE(parsed.frames[0].hasPgn, hasPgn); QCOMPARE(parsed.frames[0].pgn, expected); } +void CoverageTest::j1939Lookup_data() { + QTest::addColumn("definitionId"); + QTest::addColumn("receivedId"); + QTest::addColumn("pgn"); + QTest::addColumn("matches"); + QTest::newRow("pdu1-source") << 0x18EF1242U << 0x18EF1299U << 0xEF00U << true; + QTest::newRow("pdu1-destination-priority") << 0x18EF1242U << 0x0CEF3499U << 0xEF00U << true; + QTest::newRow("pdu2-source-priority") << 0x18F00442U << 0x0CF00499U << 0xF004U << true; + QTest::newRow("pdu2-group-extension") << 0x18F00442U << 0x18F00599U << 0xF004U << false; + QTest::newRow("data-page") << 0x18F00442U << 0x19F00499U << 0xF004U << false; + QTest::newRow("extended-data-page") << 0x18F00442U << 0x1AF00499U << 0xF004U << false; + QTest::newRow("pgn-zero") << 0x80000123U << 0x00000456U << 0U << true; +} + +void CoverageTest::j1939Lookup() { + QFETCH(unsigned, definitionId); + QFETCH(unsigned, receivedId); + QFETCH(unsigned, pgn); + QFETCH(bool, matches); + const std::string text = "BO_ " + std::to_string(definitionId) + " Frame: 1 ECU\n" + "SG_ Value : 0|8@1+ (2,1) [0|511] \"\" ECU\n"; + const DbcDecoder decoder; + RawCanFrame raw; + raw.canId = receivedId; + raw.isExtended = true; + raw.data = {7U}; + const DecodeDatabase generic = DbcDecodeBuilder().Build(ParseText(text)); + QVERIFY(!decoder.FindFrame(generic, receivedId, true)); + QCOMPARE(decoder.Decode(generic, raw).status, FrameDecodeStatus::UnknownFrame); + QVERIFY(!generic.FindFrameByPgn(pgn).definition); + + DecodeDatabase database = DbcDecodeBuilder().Build(ParseText(text, true)); + const FrameKey key(definitionId & 0x1FFFFFFFU, true); + const PgnLookupResult lookup = database.FindFrameByPgn(pgn); + QVERIFY(!lookup.ambiguous); + QVERIFY(lookup.definition != nullptr); + QCOMPARE(lookup.definition.get(), database.FindFrame(key).get()); + const DecodedFrameValue result = decoder.Decode(database, raw); + QCOMPARE(result.valid, matches); + QCOMPARE(result.status, matches ? FrameDecodeStatus::Valid : FrameDecodeStatus::UnknownFrame); + QCOMPARE(bool(decoder.FindFrame(database, receivedId, true)), matches); + if (matches) { + QVERIFY(result.key == key); + QCOMPARE(result.definition.get(), lookup.definition.get()); + QCOMPARE(result.signals.size(), std::size_t(1)); + QVERIFY(result.signals[0].key == SignalKey(key, "Value")); + QCOMPARE(std::get(result.signals[0].rawValue), std::uint64_t(7)); + QCOMPARE(result.signals[0].physicalValue, 15.0); + const DecodedFrameValue selected = decoder.Decode(database, raw, {SignalKey(key, "Value")}); + QCOMPARE(selected.status, FrameDecodeStatus::Valid); + QVERIFY(selected.signals[0].key == result.signals[0].key); + QVERIFY_THROWS_EXCEPTION(std::invalid_argument, + decoder.Decode(database, raw, {SignalKey(FrameKey(receivedId, true), "Value")})); + } else { + QVERIFY(result.key == FrameKey(receivedId, true)); + QVERIFY(!result.definition); + QVERIFY(result.signals.empty()); + } + raw.isExtended = false; + QCOMPARE(decoder.Decode(database, raw).status, FrameDecodeStatus::UnknownFrame); + raw.isExtended = true; + raw.canId = receivedId | 0x80000000U; + QCOMPARE(decoder.Decode(database, raw).status, FrameDecodeStatus::UnknownFrame); + + database = DecodeDatabase(); + QCOMPARE(lookup.definition->pgn, pgn); // PGN handles own the old snapshot. + QVERIFY(!database.FindFrameByPgn(pgn).definition); + QVERIFY(!database.FindFrameByPgn(pgn).ambiguous); +} + +void CoverageTest::j1939Ambiguity_data() { + QTest::addColumn("firstId"); + QTest::addColumn("secondId"); + QTest::addColumn("receivedId"); + QTest::addColumn("pgn"); + QTest::newRow("pdu1") << 0x18EF1242U << 0x18EF3443U << 0x18EF5699U << 0xEF00U; + QTest::newRow("pdu2") << 0x18F00442U << 0x0CF00443U << 0x18F00499U << 0xF004U; +} + +void CoverageTest::j1939Ambiguity() { + QFETCH(unsigned, firstId); + QFETCH(unsigned, secondId); + QFETCH(unsigned, receivedId); + QFETCH(unsigned, pgn); + DecodeFrame first; + first.canId = firstId; + first.isExtended = true; + first.hasPgn = true; + first.pgn = pgn; + DecodeFrame second = first; + second.canId = secondId; + RawCanFrame raw; + raw.canId = receivedId; + raw.isExtended = true; + const DbcDecoder decoder; + for (const std::vector &frames: {std::vector{first, second}, + std::vector{second, first}}) { + const DecodeDatabase database(frames); + const PgnLookupResult lookup = database.FindFrameByPgn(pgn); + QVERIFY(lookup.ambiguous); + QVERIFY(!lookup.definition); + QVERIFY(!decoder.FindFrame(database, receivedId, true)); + const DecodedFrameValue result = decoder.Decode(database, raw); + QCOMPARE(result.status, FrameDecodeStatus::AmbiguousFrame); + QVERIFY(!result.valid); + QVERIFY(!result.definition); + QVERIFY(result.signals.empty()); + QVERIFY(result.key == FrameKey(receivedId, true)); + QCOMPARE(decoder.Decode(database, raw, {SignalKey(first.GetKey(), "Missing")}).status, + FrameDecodeStatus::AmbiguousFrame); + raw.canId = firstId; + const DecodedFrameValue exact = decoder.Decode(database, raw); + QCOMPARE(exact.status, FrameDecodeStatus::Valid); + QVERIFY(exact.key == first.GetKey()); + raw.canId = receivedId; + } + // A generic extended definition neither creates ambiguity nor loses exact-ID precedence. + second.hasPgn = false; + second.canId = receivedId; + const DecodeDatabase mixed({first, second}); + QVERIFY(!mixed.FindFrameByPgn(pgn).ambiguous); + QCOMPARE(mixed.FindFrameByPgn(pgn).definition->canId, firstId); + QCOMPARE(decoder.Decode(mixed, raw).definition->canId, receivedId); + raw.canId = receivedId ^ 1U; + QCOMPARE(decoder.Decode(mixed, raw).definition->canId, firstId); +} + +void CoverageTest::j1939MetadataValidation() { + DecodeFrame frame; + frame.canId = 0x18F00442U; + frame.isExtended = true; + frame.hasPgn = true; + frame.pgn = 0xF005U; + QVERIFY_THROWS_EXCEPTION(std::invalid_argument, (void) DecodeDatabase({frame})); + DbcDatabase source; + FrameInfo metadata; + metadata.canId = frame.canId; + metadata.isExtended = true; + metadata.hasPgn = true; + metadata.pgn = frame.pgn; + source.frames.push_back(metadata); + QVERIFY_THROWS_EXCEPTION(std::invalid_argument, DbcDecodeBuilder().Build(source)); + frame.canId = 1U; + frame.isExtended = false; + frame.pgn = 0U; + QVERIFY_THROWS_EXCEPTION(std::invalid_argument, (void) DecodeDatabase({frame})); +} + void CoverageTest::comments_data() { QTest::addColumn("before"); QTest::addColumn("after"); diff --git a/tests/dbc_pipeline_test.cpp b/tests/dbc_pipeline_test.cpp index 775bdb0..e70563f 100644 --- a/tests/dbc_pipeline_test.cpp +++ b/tests/dbc_pipeline_test.cpp @@ -29,7 +29,7 @@ VAL_ 2147483939 State -2 "Extended minus two"; if (!file.open()) throw std::runtime_error("Cannot create DBC fixture"); if (file.write(text.data(), static_cast(text.size())) != static_cast(text.size()) || !file.flush()) throw std::runtime_error("Cannot write DBC fixture"); - return DbcParser().ParseFileWithDiagnostics(file.fileName().toStdString()); + return DbcParser(DbcParserOptions{true}).ParseFileWithDiagnostics(file.fileName().toStdString()); } // Independent expectations ensure a parser regression cannot be hidden by