-
Notifications
You must be signed in to change notification settings - Fork 207
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CRC calculation does not match expected values in functional test #2449
Comments
Worth noting that this API is designed to allow different CRC types, but no such feature was ever implemented nor asked for by any users. Only a single implementation - which is CRC-16/ARC - was ever implemented, and nothing else is used anywhere in CFE/CFS. Going forward - suggesting we consider a simpler declaration for ease-of-use:
This would use the default CRC algorithm, and always start from an initial value of 0, which is the way this is used in most cases. The CS app is the exception that computes a CRC over several cycles, and thus needs to pass in an initial value that is not 0. To support this case as well as to preserve backward compatibility, the existing API will not change. |
Move the current CRC-16 algorithm to a separate source file and better structure the code to support future CRC algorithm alternatives. Improve documentation to better indicate what the current algorithm is and what to expect going forward. Also corrects for issues in the CRC functional test: - Use the standard check input and compare against the standard check value for CRC-16/ARC. - Change cases from MIR to a normal test case - given a specific algorithm with specific input, the return value should be the same.
Move the current CRC-16 algorithm to a separate source file and better structure the code to support future CRC algorithm alternatives. Improve documentation to better indicate what the current algorithm is and what to expect going forward. Also corrects for issues in the CRC functional test: - Use the standard check input and compare against the standard check value for CRC-16/ARC. - Change cases from MIR to a normal test case - given a specific algorithm with specific input, the return value should be the same.
Describe the bug
Functional test reports the following results:
However, manual inspection - via a 3rd party implementation of CRC-16/ARC (the algorithm used in ES) indicates these values are not correct. The CRC of "Random Stuff" (12-octet) string should be 0x11E3 (decimal 4579) not 0x5158 (decimal 20824).
To Reproduce
Run unit tests and inspect the test results labeled "MIR".
Expected behavior
Values should match a 3rd party implementation.
System observed on:
Debian
Additional context
The reference documentation for "official" CRC implementations usually cite a check value which is the correct computation of a CRC over the string
123456789
. For CRC-16/ARC, this check value is 0xBB3D (see here: https://reveng.sourceforge.io/crc-catalogue/16.htm).The functional test should add a test case for this string using this check value, and furthermore it shouldn't be "MIR" -- there is only one correct/acceptable answer for a CRC-16/ARC on a known input. Currently, CFE ES does not allow for different algorithms/polynomials - there is just one defined - so there is just one correct result.
Reporter Info
Joseph Hickey, Vantage Systems, Inc.
The text was updated successfully, but these errors were encountered: