search

openxc / bitfield-c

Bit array parsing and encoding utility library in C
BSD 3-Clause "New" or "Revised" License
92 stars 51 forks source link

Fixes encoder bug with overloaded call to buildMessage #2

Closed kyle-kelly closed 5 years ago

kyle-kelly commented 8 years ago

Encoded value is changed from 32-bit to 64-bit before being written to the CAN.

peplin commented 8 years ago

Thanks! It would be awesome if you could add a regression test to make sure this bug stays fixed - it should be sufficient call each of those helper functions and check the return value to make sure it isn't truncated.

kyle-kelly commented 8 years ago

I'm having a bit of trouble with the test suite. There appears to be some kind of rounding error that adds 1 in the conversion from a float to an int. It occurs at least once in float_to_fixed_point but may occur in another place as well. I can still check for truncation without resolving this but I wanted you to be aware of the issue.

For example, the test:

START_TEST (test_float_to_fixed_point) { uint64_t value = 0x0123456789ABCDEF; uint64_t encoded_value = float_to_fixed_point(value, 1, 0); ck_assert_int_eq(encoded_value, value); } END_TEST

Produces this:

tests/truncation_tests.c:33:F:core:test_float_to_fixed_point:0: Assertion 'encoded_value == value' failed: encoded_value == 81985529216486896, value == 81985529216486895

I've tried removing the raw += 0.5 (among other things) but there was no change. I think it has something to do with the compiler. Let me know if you have any thoughts, I'll come back to this later in the week.