From 2c057b631d2d3d452f19ce04aa4f393fea5ff1ec Mon Sep 17 00:00:00 2001 From: deeaitch Date: Thu, 1 Oct 2026 12:57:12 -0400 Subject: [PATCH] Ref #1 Fix motorola data format --- dbc_decoder.cpp | 21 +++---- tests/README.md | 27 +++++++++ tests/dbc_decoder_test.cpp | 97 ++++++++++++++++++++++++++++++ tests/dbc_decoder_test.pro | 7 +++ typefactory.h | 118 +++++++++++++++++++++++++++++++++++++ 5 files changed, 257 insertions(+), 13 deletions(-) create mode 100644 tests/README.md create mode 100644 tests/dbc_decoder_test.cpp create mode 100644 tests/dbc_decoder_test.pro create mode 100644 typefactory.h diff --git a/dbc_decoder.cpp b/dbc_decoder.cpp index 42297bc..1490152 100644 --- a/dbc_decoder.cpp +++ b/dbc_decoder.cpp @@ -111,31 +111,26 @@ bool DbcDecoder::ExtractMotorola (const std::vector &data, * - startBit points to the most significant bit of the signal * - inside a byte, bit numbering goes 7..0 * - crossing byte boundary moves to the next byte, bit 7 + * Example: a signal starting at 7 follows 7..0, 15..8, 23..16. */ value = 0U; - std::int32_t currentBit = static_cast (startBit); + std::size_t byteIndex = startBit / 8U; + std::uint32_t bitInByte = startBit % 8U; for (std::uint32_t bitIndex = 0U; bitIndex < length; ++bitIndex) { - if (currentBit < 0) - return false; - - const std::uint32_t absoluteBit = static_cast (currentBit); - const std::uint32_t byteIndex = absoluteBit / 8U; - const std::uint32_t bitFromMsb = absoluteBit % 8U; - const std::uint32_t bitInByte = 7U - bitFromMsb; - if (byteIndex >= data.size()) return false; value <<= 1U; value |= static_cast ((data[byteIndex] >> bitInByte) & 0x01U); - if ((absoluteBit % 8U) == 7U) - currentBit = static_cast ((byteIndex + 1U) * 8U); - else - --currentBit; + if (bitInByte == 0U) { + ++byteIndex; + bitInByte = 7U; + } else + --bitInByte; } return true; diff --git a/tests/README.md b/tests/README.md new file mode 100644 index 0000000..47fd998 --- /dev/null +++ b/tests/README.md @@ -0,0 +1,27 @@ +# Decoder regression tests + +Run from the repository root with a C++17 compiler: + +```sh +g++ -std=c++17 -Wall -Wextra -Wpedantic -I. \ + tests/dbc_decoder_test.cpp dbc_decoder.cpp -o /tmp/dbc_decoder_test +/tmp/dbc_decoder_test +``` + +Alternatively, build the standalone qmake project out of source: + +```sh +mkdir -p /tmp/dbc_decoder_test_build +cd /tmp/dbc_decoder_test_build +qmake /absolute/path/to/dbc/tests/dbc_decoder_test.pro +make +./dbc_decoder_test +``` + +The executable returns a nonzero exit code if any case fails. Checks remain +active in release builds and do not depend on `assert` or an external test framework. + +Coverage includes Motorola bit numbering within and across bytes, unaligned +signals, signed values, scaling, 64-bit extraction, invalid lengths, insufficient +payloads, and Intel regression cases. Frame validity means the definition was +found; insufficient payloads are checked through signal validity. diff --git a/tests/dbc_decoder_test.cpp b/tests/dbc_decoder_test.cpp new file mode 100644 index 0000000..51f8450 --- /dev/null +++ b/tests/dbc_decoder_test.cpp @@ -0,0 +1,97 @@ +#include "dbc_decoder.h" + +#include +#include + +struct DecodeCase { + const char *name; + ByteOrder byteOrder; + std::uint32_t startBit; + std::uint32_t length; + ValueType valueType; + std::vector data; + bool valid; + std::int64_t rawValue; + double factor; + double offset; + double physicalValue; +}; + +int main() { + const DecodeCase cases[] = { + {"Motorola reproduction", ByteOrder::Motorola, 7U, 16U, ValueType::Unsigned, + {0x12, 0x34}, true, 0x1234, 1.0, 0.0, 4660.0}, + {"Motorola within byte", ByteOrder::Motorola, 5U, 4U, ValueType::Unsigned, + {0xAC}, true, 11, 1.0, 0.0, 11.0}, + {"Motorola unaligned crossing", ByteOrder::Motorola, 3U, 12U, ValueType::Unsigned, + {0xAB, 0xCD}, true, 0xBCD, 1.0, 0.0, 3021.0}, + {"Motorola starts at bit zero", ByteOrder::Motorola, 0U, 2U, ValueType::Unsigned, + {0x01, 0x80}, true, 3, 1.0, 0.0, 3.0}, + {"Motorola second byte", ByteOrder::Motorola, 15U, 16U, ValueType::Unsigned, + {0xFF, 0x12, 0x34}, true, 0x1234, 1.0, 0.0, 4660.0}, + {"Motorola signed scaled", ByteOrder::Motorola, 7U, 16U, ValueType::Signed, + {0xFF, 0xFE}, true, -2, 0.5, 10.0, 9.0}, + {"Motorola 64 bits", ByteOrder::Motorola, 7U, 64U, ValueType::Unsigned, + {0, 0, 0, 0, 0, 0, 0x12, 0x34}, true, 0x1234, 1.0, 0.0, 4660.0}, + {"Motorola 64 bits high bit", ByteOrder::Motorola, 7U, 64U, ValueType::Unsigned, + {0x40, 0, 0, 0, 0, 0, 0, 0}, true, 0x4000000000000000LL, + 1.0, 0.0, 4611686018427387904.0}, + {"Motorola short payload", ByteOrder::Motorola, 7U, 16U, ValueType::Unsigned, + {0x12}, false, 0, 1.0, 0.0, 0.0}, + {"Motorola empty payload", ByteOrder::Motorola, 7U, 8U, ValueType::Unsigned, + {}, false, 0, 1.0, 0.0, 0.0}, + {"Motorola large start", ByteOrder::Motorola, + std::numeric_limits::max(), 8U, ValueType::Unsigned, + {0xFF}, false, 0, 1.0, 0.0, 0.0}, + {"Motorola zero length", ByteOrder::Motorola, 7U, 0U, ValueType::Unsigned, + {0xFF}, false, 0, 1.0, 0.0, 0.0}, + {"Motorola excessive length", ByteOrder::Motorola, 7U, 65U, ValueType::Unsigned, + std::vector (9U, 0xFF), false, 0, 1.0, 0.0, 0.0}, + {"Intel regression", ByteOrder::Intel, 0U, 16U, ValueType::Unsigned, + {0x12, 0x34}, true, 0x3412, 1.0, 0.0, 13330.0}, + {"Intel signed regression", ByteOrder::Intel, 0U, 16U, ValueType::Signed, + {0xFE, 0xFF}, true, -2, 0.5, 10.0, 9.0}, + {"Intel short payload", ByteOrder::Intel, 0U, 16U, ValueType::Unsigned, + {0x12}, false, 0, 1.0, 0.0, 0.0} + }; + + int failures = 0; + for (const DecodeCase &test : cases) { + DecodeSignal signal; + signal.name = test.name; + signal.byteOrder = test.byteOrder; + signal.startBit = test.startBit; + signal.length = test.length; + signal.valueType = test.valueType; + signal.factor = test.factor; + signal.offset = test.offset; + + DecodeFrame definition; + definition.canId = 0x123U; + definition.dlc = static_cast (test.data.size()); + definition.signals.push_back (signal); + DecodeDatabase database; + database.frames.push_back (definition); + database.frameIndexByKey.emplace (FrameKey (definition.canId, false), 0U); + + RawCanFrame frame; + frame.canId = definition.canId; + frame.data = test.data; + const DecodedFrameValue result = DbcDecoder().Decode (database, frame); + const bool passed = result.valid && + (result.definition == &database.frames[0]) && + (result.signals.size() == 1U) && + (result.signals[0].definition == &database.frames[0].signals[0]) && + (result.signals[0].valid == test.valid) && + (!test.valid || ((result.signals[0].rawValue == test.rawValue) && + (result.signals[0].physicalValue == test.physicalValue))); + if (!passed) { + std::cerr << "FAIL: " << test.name << '\n'; + ++failures; + } + } + if (failures != 0) + return 1; + std::cout << "All " << sizeof (cases) / sizeof (cases[0]) << " decoder cases passed.\n"; + return 0; +} diff --git a/tests/dbc_decoder_test.pro b/tests/dbc_decoder_test.pro new file mode 100644 index 0000000..b5e22b4 --- /dev/null +++ b/tests/dbc_decoder_test.pro @@ -0,0 +1,7 @@ +TEMPLATE = app +TARGET = dbc_decoder_test +CONFIG += console c++17 +CONFIG -= app_bundle qt +INCLUDEPATH += .. +SOURCES += dbc_decoder_test.cpp ../dbc_decoder.cpp +HEADERS += ../dbc_decoder.h ../decode_database.h diff --git a/typefactory.h b/typefactory.h new file mode 100644 index 0000000..4f8909f --- /dev/null +++ b/typefactory.h @@ -0,0 +1,118 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2026 Dim Himro + +#ifndef TYPEFACTORY_H +#define TYPEFACTORY_H + +#include +#include +#include +#include // std::move + +/** + * @brief Registry-based factory for creating objects by runtime identifier. + * + * Allows registering derived types and instantiating them using a ClassId key. + * + * Notes: + * - Not thread-safe. Register types during initialization phase only. + * - Re-registering the same id overwrites the previous entry ("last wins"). + * - ClassId must be hashable (std::hash specialization must exist). + * + * @tparam ClassId -Unique class identifier type (e.g. std::string, int, enum). + * @tparam BaseClass -Base type for all created objects. + * @tparam Arg - Constructor arguments forwarded to registered derived types (by value). + * + * Usage example: + * @code + * #include + * + * struct Base { + * virtual ~Base() = default; + * virtual int get() = 0; + * }; + * + * struct Derived1 : Base { + * explicit Derived1(int start) : m_start(start) {} + * int get() override { return m_start + 1; } + * int m_start = 0; + * }; + * + * struct Derived2 : Base { + * explicit Derived2(int start) : m_start(start) {} + * int get() override { return m_start + 2; } + * int m_start = 0; + * }; + * + * TypeFactory factory; + * factory.registerType("one"); + * factory.registerType("two"); + * + * auto a = factory.create("one", 10); + * auto b = factory.creator("two")(10); // advanced API: get creator function + * + * (void)a->get(); + * (void)b->get(); + * @endcode + */ +template +class TypeFactory { +public: + using BasePtr = std::shared_ptr; + using CreatorFn = BasePtr (*)(Args...); + + TypeFactory() = default; + + /** + * @brief Registers a Derived type under the given id. + * + * Requirements (checked at compile-time): + * - Derived must inherit from BaseClass + * - Derived must be constructible from Args... + * + * Re-registering the same id overwrites the previous entry ("last wins"). + */ + template + void registerType(const ClassId& id) { + static_assert(std::is_base_of_v, + "Derived must inherit from BaseClass"); + static_assert(std::is_constructible_v, + "Derived must be constructible from Args..."); + + classes.insert_or_assign(id, &instantiate); + } + + /** + * @brief Returns creator function for the given id. + * @throws std::out_of_range when id is not registered. + */ + [[nodiscard]] CreatorFn creator(const ClassId& id) const { + return classes.at(id); + } + + /** + * @brief Creates an instance for the given id using provided arguments. + * @throws std::out_of_range when id is not registered. + */ + [[nodiscard]] BasePtr create(const ClassId& id, Args... args) const { + return creator(id)(std::move(args)...); + } + + /** + * @brief Checks whether an id is registered. + */ + [[nodiscard]] bool contains(const ClassId& id) const { + return classes.find(id) != classes.end(); + } + +protected: + // Protected by design: derived factories may extend/inspect the registry. + std::unordered_map classes; + +private: + template + static BasePtr instantiate(Args... args) { + return std::make_shared(std::move(args)...); + } +}; +#endif // TYPEFACTORY_H