Fix #2 Preserve unsigned 64-bit decoded values
This commit is contained in:
1 parent
2c057b631d
commit
69074e3321
6 files changed
+112
-15
No files matched your search
@@ -414,6 +414,34 @@ Fields:
|
|||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
### Raw value representation and precision
|
||||||
|
|
||||||
|
`DecodedSignalValue::rawValue` is a C++17
|
||||||
|
`std::variant<std::int64_t, std::uint64_t>`. For a valid signal, signed definitions
|
||||||
|
produce the `int64_t` alternative and unsigned definitions produce the `uint64_t`
|
||||||
|
alternative. Raw values remain exact across their full 64-bit ranges. Check
|
||||||
|
`valid` before using the value; an invalid result's default value is not decoded data.
|
||||||
|
|
||||||
|
Use `std::get` when the numeric type is known, or `std::visit` to handle both:
|
||||||
|
|
||||||
|
```cpp
|
||||||
|
std::visit([] (auto raw) { std::cout << raw; }, decodedSignal.rawValue);
|
||||||
|
```
|
||||||
|
|
||||||
|
This replaces the previous `int64_t` field: consumers that read or print
|
||||||
|
`rawValue` directly must handle the variant.
|
||||||
|
|
||||||
|
Physical conversion uses the active numeric type:
|
||||||
|
`physicalValue = static_cast<double>(raw) * factor + offset`.
|
||||||
|
`physicalValue` remains a `double` and is approximate for large integers.
|
||||||
|
On usual IEEE 754 binary64 implementations, integers are exactly representable
|
||||||
|
through 2^53; above that, some integers round to the same floating-point value.
|
||||||
|
For example, unsigned `UINT64_MAX` remains exact in `rawValue`, but converts to
|
||||||
|
approximately 18446744073709551616 in binary64 before scaling.
|
||||||
|
Use `rawValue` whenever exact integer values are required.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
## `DecodedFrameValue`
|
## `DecodedFrameValue`
|
||||||
|
|
||||||
Represents one decoded frame result.
|
Represents one decoded frame result.
|
||||||
@@ -839,3 +867,9 @@ DBC parser → DbcDatabase → DecodeDatabase → DbcDecoder
|
|||||||
```
|
```
|
||||||
|
|
||||||
This gives FrameTap a much better foundation for real use, because both browsing and decoding are supported without forcing one representation to do the other's job.
|
This gives FrameTap a much better foundation for real use, because both browsing and decoding are supported without forcing one representation to do the other's job.
|
||||||
|
|
||||||
|
## Regression tests
|
||||||
|
|
||||||
|
See [tests/README.md](tests/README.md) for compiler and qmake commands.
|
||||||
|
Tests cover Intel and Motorola extraction, signed and unsigned 64-bit boundaries,
|
||||||
|
physical conversion, numeric output, and invalid signal payloads.
|
||||||
+11
-8
@@ -55,10 +55,12 @@ DecodedFrameValue DbcDecoder::Decode (const DecodeDatabase &database,
|
|||||||
if (signal.valueType == ValueType::Signed)
|
if (signal.valueType == ValueType::Signed)
|
||||||
decoded.rawValue = SignExtend (unsignedValue, signal.length);
|
decoded.rawValue = SignExtend (unsignedValue, signal.length);
|
||||||
else
|
else
|
||||||
decoded.rawValue = static_cast<std::int64_t> (unsignedValue);
|
decoded.rawValue = unsignedValue;
|
||||||
|
|
||||||
decoded.physicalValue =
|
decoded.physicalValue =
|
||||||
(static_cast<double> (decoded.rawValue) * signal.factor) + signal.offset;
|
std::visit ([&signal] (auto rawValue) {
|
||||||
|
return (static_cast<double> (rawValue) * signal.factor) + signal.offset;
|
||||||
|
}, decoded.rawValue);
|
||||||
decoded.valid = true;
|
decoded.valid = true;
|
||||||
|
|
||||||
result.signals.push_back (decoded);
|
result.signals.push_back (decoded);
|
||||||
@@ -137,16 +139,17 @@ bool DbcDecoder::ExtractMotorola (const std::vector<std::uint8_t> &data,
|
|||||||
}
|
}
|
||||||
|
|
||||||
std::int64_t DbcDecoder::SignExtend (std::uint64_t value, std::uint32_t bitLength) {
|
std::int64_t DbcDecoder::SignExtend (std::uint64_t value, std::uint32_t bitLength) {
|
||||||
if ((bitLength == 0U) || (bitLength >= 64U))
|
if ((bitLength == 0U) || (bitLength > 64U))
|
||||||
return static_cast<std::int64_t> (value);
|
return 0;
|
||||||
|
|
||||||
const std::uint64_t signMask = (static_cast<std::uint64_t> (1U) << (bitLength - 1U));
|
|
||||||
const std::uint64_t valueMask = (static_cast<std::uint64_t> (1U) << bitLength) - 1U;
|
|
||||||
|
|
||||||
|
const std::uint64_t signMask = std::uint64_t (1U) << (bitLength - 1U);
|
||||||
|
const std::uint64_t valueMask = (bitLength == 64U)
|
||||||
|
? ~std::uint64_t (0U) : (std::uint64_t (1U) << bitLength) - 1U;
|
||||||
value &= valueMask;
|
value &= valueMask;
|
||||||
|
|
||||||
if ((value & signMask) == 0U)
|
if ((value & signMask) == 0U)
|
||||||
return static_cast<std::int64_t> (value);
|
return static_cast<std::int64_t> (value);
|
||||||
|
|
||||||
return static_cast<std::int64_t> (value | (~valueMask));
|
// The complemented magnitude fits int64_t, including for INT64_MIN.
|
||||||
|
return -1 - static_cast<std::int64_t> ((~value) & valueMask);
|
||||||
}
|
}
|
||||||
+3
-2
@@ -14,6 +14,7 @@
|
|||||||
|
|
||||||
#include <vector>
|
#include <vector>
|
||||||
#include <cstdint>
|
#include <cstdint>
|
||||||
|
#include <variant>
|
||||||
|
|
||||||
#include "decode_database.h"
|
#include "decode_database.h"
|
||||||
|
|
||||||
@@ -37,13 +38,13 @@ struct RawCanFrame {
|
|||||||
*/
|
*/
|
||||||
struct DecodedSignalValue {
|
struct DecodedSignalValue {
|
||||||
const DecodeSignal *definition; /**< Signal definition. */
|
const DecodeSignal *definition; /**< Signal definition. */
|
||||||
std::int64_t rawValue; /**< Extracted raw integer value. */
|
std::variant<std::int64_t, std::uint64_t> rawValue; /**< Exact signed or unsigned raw value. */
|
||||||
double physicalValue; /**< Converted physical value. */
|
double physicalValue; /**< Converted physical value. */
|
||||||
bool valid; /**< true if decoding succeeded. */
|
bool valid; /**< true if decoding succeeded. */
|
||||||
|
|
||||||
DecodedSignalValue()
|
DecodedSignalValue()
|
||||||
: definition (nullptr)
|
: definition (nullptr)
|
||||||
, rawValue (0)
|
, rawValue (std::uint64_t (0U))
|
||||||
, physicalValue (0.0)
|
, physicalValue (0.0)
|
||||||
, valid (false) {
|
, valid (false) {
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -87,8 +87,9 @@ static void PrintDecodedFrame (const DecodedFrameValue &decoded) {
|
|||||||
continue;
|
continue;
|
||||||
|
|
||||||
std::cout << " " << signal.definition->name
|
std::cout << " " << signal.definition->name
|
||||||
<< " raw=" << signal.rawValue
|
<< " raw=";
|
||||||
<< " physical=" << signal.physicalValue
|
std::visit ([] (auto rawValue) { std::cout << rawValue; }, signal.rawValue);
|
||||||
|
std::cout << " physical=" << signal.physicalValue
|
||||||
<< " " << signal.definition->unit
|
<< " " << signal.definition->unit
|
||||||
<< "\n";
|
<< "\n";
|
||||||
}
|
}
|
||||||
|
|||||||
+2
-1
@@ -22,6 +22,7 @@ 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.
|
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
|
Coverage includes Motorola bit numbering within and across bytes, unaligned
|
||||||
signals, signed values, scaling, 64-bit extraction, invalid lengths, insufficient
|
signals, signed values, scaling, 64-bit extraction, signed/unsigned boundary values
|
||||||
|
(including `INT64_MIN`, `INT64_MAX`, and `UINT64_MAX`), numeric output, invalid lengths, insufficient
|
||||||
payloads, and Intel regression cases. Frame validity means the definition was
|
payloads, and Intel regression cases. Frame validity means the definition was
|
||||||
found; insufficient payloads are checked through signal validity.
|
found; insufficient payloads are checked through signal validity.
|
||||||
@@ -2,6 +2,7 @@
|
|||||||
|
|
||||||
#include <iostream>
|
#include <iostream>
|
||||||
#include <limits>
|
#include <limits>
|
||||||
|
#include <sstream>
|
||||||
|
|
||||||
struct DecodeCase {
|
struct DecodeCase {
|
||||||
const char *name;
|
const char *name;
|
||||||
@@ -83,15 +84,71 @@ int main() {
|
|||||||
(result.signals.size() == 1U) &&
|
(result.signals.size() == 1U) &&
|
||||||
(result.signals[0].definition == &database.frames[0].signals[0]) &&
|
(result.signals[0].definition == &database.frames[0].signals[0]) &&
|
||||||
(result.signals[0].valid == test.valid) &&
|
(result.signals[0].valid == test.valid) &&
|
||||||
(!test.valid || ((result.signals[0].rawValue == test.rawValue) &&
|
(!test.valid || (((test.valueType == ValueType::Signed)
|
||||||
|
? (std::get_if<std::int64_t> (&result.signals[0].rawValue) &&
|
||||||
|
std::get<std::int64_t> (result.signals[0].rawValue) == test.rawValue)
|
||||||
|
: (std::get_if<std::uint64_t> (&result.signals[0].rawValue) &&
|
||||||
|
std::get<std::uint64_t> (result.signals[0].rawValue) ==
|
||||||
|
static_cast<std::uint64_t> (test.rawValue))) &&
|
||||||
(result.signals[0].physicalValue == test.physicalValue)));
|
(result.signals[0].physicalValue == test.physicalValue)));
|
||||||
if (!passed) {
|
if (!passed) {
|
||||||
std::cerr << "FAIL: " << test.name << '\n';
|
std::cerr << "FAIL: " << test.name << '\n';
|
||||||
++failures;
|
++failures;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
std::size_t boundaryCases = 0U;
|
||||||
|
for (ByteOrder order : {ByteOrder::Intel, ByteOrder::Motorola}) {
|
||||||
|
for (ValueType type : {ValueType::Unsigned, ValueType::Signed}) {
|
||||||
|
for (std::uint64_t bits : {std::uint64_t (0U), std::uint64_t (0x7FFFFFFFFFFFFFFFULL),
|
||||||
|
std::uint64_t (0x8000000000000000ULL),
|
||||||
|
std::uint64_t (0x8000000000000001ULL),
|
||||||
|
std::uint64_t (0xFFFFFFFFFFFFFFFFULL)}) {
|
||||||
|
DecodeDatabase database;
|
||||||
|
DecodeFrame definition;
|
||||||
|
DecodeSignal signal;
|
||||||
|
signal.startBit = order == ByteOrder::Intel ? 0U : 7U;
|
||||||
|
signal.length = 64U;
|
||||||
|
signal.byteOrder = order;
|
||||||
|
signal.valueType = type;
|
||||||
|
signal.factor = 0.5;
|
||||||
|
signal.offset = 10.0;
|
||||||
|
definition.signals.push_back (signal);
|
||||||
|
database.frames.push_back (definition);
|
||||||
|
database.frameIndexByKey.emplace (FrameKey (0U, false), 0U);
|
||||||
|
RawCanFrame frame;
|
||||||
|
for (unsigned byte = 0U; byte < 8U; ++byte) {
|
||||||
|
const unsigned shift = (order == ByteOrder::Intel ? byte : 7U - byte) * 8U;
|
||||||
|
frame.data.push_back (static_cast<std::uint8_t> (bits >> shift));
|
||||||
|
}
|
||||||
|
const auto result = DbcDecoder().Decode (database, frame);
|
||||||
|
const std::int64_t signedExpected = bits <= 0x7FFFFFFFFFFFFFFFULL
|
||||||
|
? static_cast<std::int64_t> (bits)
|
||||||
|
: std::numeric_limits<std::int64_t>::min() +
|
||||||
|
static_cast<std::int64_t> (bits - 0x8000000000000000ULL);
|
||||||
|
const auto &decoded = result.signals.at (0U);
|
||||||
|
const auto expected = type == ValueType::Unsigned
|
||||||
|
? std::variant<std::int64_t, std::uint64_t> (bits)
|
||||||
|
: std::variant<std::int64_t, std::uint64_t> (signedExpected);
|
||||||
|
const double physical = (type == ValueType::Unsigned
|
||||||
|
? static_cast<double> (bits) : static_cast<double> (signedExpected)) * 0.5 + 10.0;
|
||||||
|
std::ostringstream actualText;
|
||||||
|
std::ostringstream expectedText;
|
||||||
|
std::visit ([&actualText] (auto raw) { actualText << raw; }, decoded.rawValue);
|
||||||
|
if (type == ValueType::Unsigned)
|
||||||
|
expectedText << bits;
|
||||||
|
else
|
||||||
|
expectedText << signedExpected;
|
||||||
|
if (!decoded.valid || decoded.rawValue != expected ||
|
||||||
|
decoded.physicalValue != physical || actualText.str() != expectedText.str()) {
|
||||||
|
std::cerr << "FAIL: 64-bit boundary " << bits << '\n';
|
||||||
|
++failures;
|
||||||
|
}
|
||||||
|
++boundaryCases;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
if (failures != 0)
|
if (failures != 0)
|
||||||
return 1;
|
return 1;
|
||||||
std::cout << "All " << sizeof (cases) / sizeof (cases[0]) << " decoder cases passed.\n";
|
std::cout << "All " << sizeof (cases) / sizeof (cases[0]) + boundaryCases << " decoder cases passed.\n";
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
Reference in new issue
Block a user