From: =?utf-8?b?IlNaIExpbiAo5p6X5LiK5pm6KSI=?= Date: Wed, 19 Dec 2018 10:24:47 +0800 Subject: Fix float endianness issue on big endian arch It converts float values depending on what order they come in. This patch was modified from rm5248 [1] [1] https://github.com/synexxus/libmodbus/commit/a511768e7fe7ec52d7bae1d9ae04e33f87a59627 --- src/modbus-data.c | 110 ++++++++++++++++++++++++++++++++++++++--------- tests/unit-test-client.c | 22 ++++++---- tests/unit-test.h.in | 41 ++++++++++++++++-- 3 files changed, 141 insertions(+), 32 deletions(-) diff --git a/src/modbus-data.c b/src/modbus-data.c index 902b8c6..7a744fa 100644 --- a/src/modbus-data.c +++ b/src/modbus-data.c @@ -119,9 +119,18 @@ float modbus_get_float_abcd(const uint16_t *src) { float f; uint32_t i; + uint8_t a, b, c, d; - i = ntohl(((uint32_t)src[0] << 16) + src[1]); - memcpy(&f, &i, sizeof(float)); + a = (src[0] >> 8) & 0xFF; + b = (src[0] >> 0) & 0xFF; + c = (src[1] >> 8) & 0xFF; + d = (src[1] >> 0) & 0xFF; + + i = (a << 24) | + (b << 16) | + (c << 8) | + (d << 0); + memcpy(&f, &i, 4); return f; } @@ -131,9 +140,18 @@ float modbus_get_float_dcba(const uint16_t *src) { float f; uint32_t i; + uint8_t a, b, c, d; - i = ntohl(bswap_32((((uint32_t)src[0]) << 16) + src[1])); - memcpy(&f, &i, sizeof(float)); + a = (src[0] >> 8) & 0xFF; + b = (src[0] >> 0) & 0xFF; + c = (src[1] >> 8) & 0xFF; + d = (src[1] >> 0) & 0xFF; + + i = (d << 24) | + (c << 16) | + (b << 8) | + (a << 0); + memcpy(&f, &i, 4); return f; } @@ -143,9 +161,18 @@ float modbus_get_float_badc(const uint16_t *src) { float f; uint32_t i; + uint8_t a, b, c, d; - i = ntohl((uint32_t)(bswap_16(src[0]) << 16) + bswap_16(src[1])); - memcpy(&f, &i, sizeof(float)); + a = (src[0] >> 8) & 0xFF; + b = (src[0] >> 0) & 0xFF; + c = (src[1] >> 8) & 0xFF; + d = (src[1] >> 0) & 0xFF; + + i = (b << 24) | + (a << 16) | + (d << 8) | + (c << 0); + memcpy(&f, &i, 4); return f; } @@ -155,9 +182,18 @@ float modbus_get_float_cdab(const uint16_t *src) { float f; uint32_t i; + uint8_t a, b, c, d; - i = ntohl((((uint32_t)src[1]) << 16) + src[0]); - memcpy(&f, &i, sizeof(float)); + a = (src[0] >> 8) & 0xFF; + b = (src[0] >> 0) & 0xFF; + c = (src[1] >> 8) & 0xFF; + d = (src[1] >> 0) & 0xFF; + + i = (c << 24) | + (d << 16) | + (a << 8) | + (b << 0); + memcpy(&f, &i, 4); return f; } @@ -172,50 +208,84 @@ float modbus_get_float(const uint16_t *src) memcpy(&f, &i, sizeof(float)); return f; + } /* Set a float to 4 bytes for Modbus w/o any conversion (ABCD) */ void modbus_set_float_abcd(float f, uint16_t *dest) { uint32_t i; + uint8_t *out = (uint8_t*) dest; + uint8_t a, b, c, d; memcpy(&i, &f, sizeof(uint32_t)); - i = htonl(i); - dest[0] = (uint16_t)(i >> 16); - dest[1] = (uint16_t)i; + a = (i >> 24) & 0xFF; + b = (i >> 16) & 0xFF; + c = (i >> 8) & 0xFF; + d = (i >> 0) & 0xFF; + + out[0] = a; + out[1] = b; + out[2] = c; + out[3] = d; } /* Set a float to 4 bytes for Modbus with byte and word swap conversion (DCBA) */ void modbus_set_float_dcba(float f, uint16_t *dest) { uint32_t i; + uint8_t *out = (uint8_t*) dest; + uint8_t a, b, c, d; memcpy(&i, &f, sizeof(uint32_t)); - i = bswap_32(htonl(i)); - dest[0] = (uint16_t)(i >> 16); - dest[1] = (uint16_t)i; + a = (i >> 24) & 0xFF; + b = (i >> 16) & 0xFF; + c = (i >> 8) & 0xFF; + d = (i >> 0) & 0xFF; + + out[0] = d; + out[1] = c; + out[2] = b; + out[3] = a; + } /* Set a float to 4 bytes for Modbus with byte swap conversion (BADC) */ void modbus_set_float_badc(float f, uint16_t *dest) { uint32_t i; + uint8_t *out = (uint8_t*) dest; + uint8_t a, b, c, d; memcpy(&i, &f, sizeof(uint32_t)); - i = htonl(i); - dest[0] = (uint16_t)bswap_16(i >> 16); - dest[1] = (uint16_t)bswap_16(i & 0xFFFF); + a = (i >> 24) & 0xFF; + b = (i >> 16) & 0xFF; + c = (i >> 8) & 0xFF; + d = (i >> 0) & 0xFF; + + out[0] = b; + out[1] = a; + out[2] = d; + out[3] = c; } /* Set a float to 4 bytes for Modbus with word swap conversion (CDAB) */ void modbus_set_float_cdab(float f, uint16_t *dest) { uint32_t i; + uint8_t *out = (uint8_t*) dest; + uint8_t a, b, c, d; memcpy(&i, &f, sizeof(uint32_t)); - i = htonl(i); - dest[0] = (uint16_t)i; - dest[1] = (uint16_t)(i >> 16); + a = (i >> 24) & 0xFF; + b = (i >> 16) & 0xFF; + c = (i >> 8) & 0xFF; + d = (i >> 0) & 0xFF; + + out[0] = c; + out[1] = d; + out[2] = a; + out[3] = b; } /* DEPRECATED - Set a float to 4 bytes in a sort of Modbus format! */ diff --git a/tests/unit-test-client.c b/tests/unit-test-client.c index 3e315f4..3fccf3e 100644 --- a/tests/unit-test-client.c +++ b/tests/unit-test-client.c @@ -27,6 +27,7 @@ int send_crafted_request(modbus_t *ctx, int function, uint16_t max_value, uint16_t bytes, int backend_length, int backend_offset); int equal_dword(uint16_t *tab_reg, const uint32_t value); +int is_memory_equal(const void *s1, const void *s2, size_t size); #define BUG_REPORT(_cond, _format, _args ...) \ printf("\nLine %d: assertion error for '%s': " _format "\n", __LINE__, # _cond, ## _args) @@ -40,6 +41,11 @@ int equal_dword(uint16_t *tab_reg, const uint32_t value); } \ }; +int is_memory_equal(const void *s1, const void *s2, size_t size) +{ + return (memcmp(s1, s2, size) == 0); +} + int equal_dword(uint16_t *tab_reg, const uint32_t value) { return ((tab_reg[0] == (value >> 16)) && (tab_reg[1] == (value & 0xFFFF))); } @@ -286,26 +292,26 @@ int main(int argc, char *argv[]) /** FLOAT **/ printf("1/4 Set/get float ABCD: "); modbus_set_float_abcd(UT_REAL, tab_rp_registers); - ASSERT_TRUE(equal_dword(tab_rp_registers, UT_IREAL_ABCD), "FAILED Set float ABCD"); - real = modbus_get_float_abcd(tab_rp_registers); + ASSERT_TRUE(is_memory_equal(tab_rp_registers, UT_IREAL_ABCD_SET, 4), "FAILED Set float ABCD"); + real = modbus_get_float_abcd(UT_IREAL_ABCD_GET); ASSERT_TRUE(real == UT_REAL, "FAILED (%f != %f)\n", real, UT_REAL); printf("2/4 Set/get float DCBA: "); modbus_set_float_dcba(UT_REAL, tab_rp_registers); - ASSERT_TRUE(equal_dword(tab_rp_registers, UT_IREAL_DCBA), "FAILED Set float DCBA"); - real = modbus_get_float_dcba(tab_rp_registers); + ASSERT_TRUE(is_memory_equal(tab_rp_registers, UT_IREAL_DCBA_SET, 4), "FAILED Set float DCBA"); + real = modbus_get_float_dcba(UT_IREAL_DCBA_GET); ASSERT_TRUE(real == UT_REAL, "FAILED (%f != %f)\n", real, UT_REAL); printf("3/4 Set/get float BADC: "); modbus_set_float_badc(UT_REAL, tab_rp_registers); - ASSERT_TRUE(equal_dword(tab_rp_registers, UT_IREAL_BADC), "FAILED Set float BADC"); - real = modbus_get_float_badc(tab_rp_registers); + ASSERT_TRUE(is_memory_equal(tab_rp_registers, UT_IREAL_BADC_SET, 4), "FAILED Set float BADC"); + real = modbus_get_float_badc(UT_IREAL_BADC_GET); ASSERT_TRUE(real == UT_REAL, "FAILED (%f != %f)\n", real, UT_REAL); printf("4/4 Set/get float CDAB: "); modbus_set_float_cdab(UT_REAL, tab_rp_registers); - ASSERT_TRUE(equal_dword(tab_rp_registers, UT_IREAL_CDAB), "FAILED Set float CDAB"); - real = modbus_get_float_cdab(tab_rp_registers); + ASSERT_TRUE(is_memory_equal(tab_rp_registers, UT_IREAL_CDAB_SET, 4), "FAILED Set float CDAB"); + real = modbus_get_float_cdab(UT_IREAL_CDAB_GET); ASSERT_TRUE(real == UT_REAL, "FAILED (%f != %f)\n", real, UT_REAL); printf("\nAt this point, error messages doesn't mean the test has failed\n"); diff --git a/tests/unit-test.h.in b/tests/unit-test.h.in index dca826f..4ffa254 100644 --- a/tests/unit-test.h.in +++ b/tests/unit-test.h.in @@ -56,12 +56,45 @@ const uint16_t UT_INPUT_REGISTERS_ADDRESS = 0x108; const uint16_t UT_INPUT_REGISTERS_NB = 0x1; const uint16_t UT_INPUT_REGISTERS_TAB[] = { 0x000A }; +/* + * This float value is 0x47F12000 (in big-endian format). + * In Little-endian(intel) format, it will be stored in memory as follows: + * 0x00 0x20 0xF1 0x47 + * + * You can check this with the following code: + + float fl = UT_REAL; + uint8_t *inmem = (uint8_t*)&fl; + int x; + for(x = 0; x < 4; x++){ + printf("0x%02X ", inmem[ x ]); + } + printf("\n"); + */ const float UT_REAL = 123456.00; -const uint32_t UT_IREAL_ABCD = 0x0020F147; -const uint32_t UT_IREAL_DCBA = 0x47F12000; -const uint32_t UT_IREAL_BADC = 0x200047F1; -const uint32_t UT_IREAL_CDAB = 0xF1470020; +/* + * The following arrays assume that 'A' is the MSB, + * and 'D' is the LSB. + * Thus, the following is the case: + * A = 0x47 + * B = 0xF1 + * C = 0x20 + * D = 0x00 + * + * There are two sets of arrays: one to test that the setting is correct, + * the other to test that the getting is correct. + * Note that the 'get' values must be constants in processor-endianness, + * as libmodbus will convert all words to processor-endianness as they come in. + */ +const uint8_t UT_IREAL_ABCD_SET[] = {0x47, 0xF1, 0x20, 0x00}; +const uint16_t UT_IREAL_ABCD_GET[] = {0x47F1, 0x2000}; +const uint8_t UT_IREAL_DCBA_SET[] = {0x00, 0x20, 0xF1, 0x47}; +const uint16_t UT_IREAL_DCBA_GET[] = {0x0020, 0xF147}; +const uint8_t UT_IREAL_BADC_SET[] = {0xF1, 0x47, 0x00, 0x20}; +const uint16_t UT_IREAL_BADC_GET[] = {0xF147, 0x0020}; +const uint8_t UT_IREAL_CDAB_SET[] = {0x20, 0x00, 0x47, 0xF1}; +const uint16_t UT_IREAL_CDAB_GET[] = {0x2000, 0x47F1}; /* const uint32_t UT_IREAL_ABCD = 0x47F12000); const uint32_t UT_IREAL_DCBA = 0x0020F147;