Brad Bishop | 0f291cc | 2019-09-01 15:16:57 -0400 | [diff] [blame^] | 1 | From: =?utf-8?b?IlNaIExpbiAo5p6X5LiK5pm6KSI=?= <szlin@debian.org> |
| 2 | Date: Wed, 19 Dec 2018 10:24:47 +0800 |
| 3 | Subject: Fix float endianness issue on big endian arch |
| 4 | |
| 5 | It converts float values depending on what order they come in. |
| 6 | |
| 7 | This patch was modified from rm5248 [1] |
| 8 | |
| 9 | [1] https://github.com/synexxus/libmodbus/commit/a511768e7fe7ec52d7bae1d9ae04e33f87a59627 |
| 10 | |
| 11 | --- |
| 12 | src/modbus-data.c | 110 ++++++++++++++++++++++++++++++++++++++--------- |
| 13 | tests/unit-test-client.c | 22 ++++++---- |
| 14 | tests/unit-test.h.in | 41 ++++++++++++++++-- |
| 15 | 3 files changed, 141 insertions(+), 32 deletions(-) |
| 16 | |
| 17 | diff --git a/src/modbus-data.c b/src/modbus-data.c |
| 18 | index 902b8c6..7a744fa 100644 |
| 19 | --- a/src/modbus-data.c |
| 20 | +++ b/src/modbus-data.c |
| 21 | @@ -119,9 +119,18 @@ float modbus_get_float_abcd(const uint16_t *src) |
| 22 | { |
| 23 | float f; |
| 24 | uint32_t i; |
| 25 | + uint8_t a, b, c, d; |
| 26 | |
| 27 | - i = ntohl(((uint32_t)src[0] << 16) + src[1]); |
| 28 | - memcpy(&f, &i, sizeof(float)); |
| 29 | + a = (src[0] >> 8) & 0xFF; |
| 30 | + b = (src[0] >> 0) & 0xFF; |
| 31 | + c = (src[1] >> 8) & 0xFF; |
| 32 | + d = (src[1] >> 0) & 0xFF; |
| 33 | + |
| 34 | + i = (a << 24) | |
| 35 | + (b << 16) | |
| 36 | + (c << 8) | |
| 37 | + (d << 0); |
| 38 | + memcpy(&f, &i, 4); |
| 39 | |
| 40 | return f; |
| 41 | } |
| 42 | @@ -131,9 +140,18 @@ float modbus_get_float_dcba(const uint16_t *src) |
| 43 | { |
| 44 | float f; |
| 45 | uint32_t i; |
| 46 | + uint8_t a, b, c, d; |
| 47 | |
| 48 | - i = ntohl(bswap_32((((uint32_t)src[0]) << 16) + src[1])); |
| 49 | - memcpy(&f, &i, sizeof(float)); |
| 50 | + a = (src[0] >> 8) & 0xFF; |
| 51 | + b = (src[0] >> 0) & 0xFF; |
| 52 | + c = (src[1] >> 8) & 0xFF; |
| 53 | + d = (src[1] >> 0) & 0xFF; |
| 54 | + |
| 55 | + i = (d << 24) | |
| 56 | + (c << 16) | |
| 57 | + (b << 8) | |
| 58 | + (a << 0); |
| 59 | + memcpy(&f, &i, 4); |
| 60 | |
| 61 | return f; |
| 62 | } |
| 63 | @@ -143,9 +161,18 @@ float modbus_get_float_badc(const uint16_t *src) |
| 64 | { |
| 65 | float f; |
| 66 | uint32_t i; |
| 67 | + uint8_t a, b, c, d; |
| 68 | |
| 69 | - i = ntohl((uint32_t)(bswap_16(src[0]) << 16) + bswap_16(src[1])); |
| 70 | - memcpy(&f, &i, sizeof(float)); |
| 71 | + a = (src[0] >> 8) & 0xFF; |
| 72 | + b = (src[0] >> 0) & 0xFF; |
| 73 | + c = (src[1] >> 8) & 0xFF; |
| 74 | + d = (src[1] >> 0) & 0xFF; |
| 75 | + |
| 76 | + i = (b << 24) | |
| 77 | + (a << 16) | |
| 78 | + (d << 8) | |
| 79 | + (c << 0); |
| 80 | + memcpy(&f, &i, 4); |
| 81 | |
| 82 | return f; |
| 83 | } |
| 84 | @@ -155,9 +182,18 @@ float modbus_get_float_cdab(const uint16_t *src) |
| 85 | { |
| 86 | float f; |
| 87 | uint32_t i; |
| 88 | + uint8_t a, b, c, d; |
| 89 | |
| 90 | - i = ntohl((((uint32_t)src[1]) << 16) + src[0]); |
| 91 | - memcpy(&f, &i, sizeof(float)); |
| 92 | + a = (src[0] >> 8) & 0xFF; |
| 93 | + b = (src[0] >> 0) & 0xFF; |
| 94 | + c = (src[1] >> 8) & 0xFF; |
| 95 | + d = (src[1] >> 0) & 0xFF; |
| 96 | + |
| 97 | + i = (c << 24) | |
| 98 | + (d << 16) | |
| 99 | + (a << 8) | |
| 100 | + (b << 0); |
| 101 | + memcpy(&f, &i, 4); |
| 102 | |
| 103 | return f; |
| 104 | } |
| 105 | @@ -172,50 +208,84 @@ float modbus_get_float(const uint16_t *src) |
| 106 | memcpy(&f, &i, sizeof(float)); |
| 107 | |
| 108 | return f; |
| 109 | + |
| 110 | } |
| 111 | |
| 112 | /* Set a float to 4 bytes for Modbus w/o any conversion (ABCD) */ |
| 113 | void modbus_set_float_abcd(float f, uint16_t *dest) |
| 114 | { |
| 115 | uint32_t i; |
| 116 | + uint8_t *out = (uint8_t*) dest; |
| 117 | + uint8_t a, b, c, d; |
| 118 | |
| 119 | memcpy(&i, &f, sizeof(uint32_t)); |
| 120 | - i = htonl(i); |
| 121 | - dest[0] = (uint16_t)(i >> 16); |
| 122 | - dest[1] = (uint16_t)i; |
| 123 | + a = (i >> 24) & 0xFF; |
| 124 | + b = (i >> 16) & 0xFF; |
| 125 | + c = (i >> 8) & 0xFF; |
| 126 | + d = (i >> 0) & 0xFF; |
| 127 | + |
| 128 | + out[0] = a; |
| 129 | + out[1] = b; |
| 130 | + out[2] = c; |
| 131 | + out[3] = d; |
| 132 | } |
| 133 | |
| 134 | /* Set a float to 4 bytes for Modbus with byte and word swap conversion (DCBA) */ |
| 135 | void modbus_set_float_dcba(float f, uint16_t *dest) |
| 136 | { |
| 137 | uint32_t i; |
| 138 | + uint8_t *out = (uint8_t*) dest; |
| 139 | + uint8_t a, b, c, d; |
| 140 | |
| 141 | memcpy(&i, &f, sizeof(uint32_t)); |
| 142 | - i = bswap_32(htonl(i)); |
| 143 | - dest[0] = (uint16_t)(i >> 16); |
| 144 | - dest[1] = (uint16_t)i; |
| 145 | + a = (i >> 24) & 0xFF; |
| 146 | + b = (i >> 16) & 0xFF; |
| 147 | + c = (i >> 8) & 0xFF; |
| 148 | + d = (i >> 0) & 0xFF; |
| 149 | + |
| 150 | + out[0] = d; |
| 151 | + out[1] = c; |
| 152 | + out[2] = b; |
| 153 | + out[3] = a; |
| 154 | + |
| 155 | } |
| 156 | |
| 157 | /* Set a float to 4 bytes for Modbus with byte swap conversion (BADC) */ |
| 158 | void modbus_set_float_badc(float f, uint16_t *dest) |
| 159 | { |
| 160 | uint32_t i; |
| 161 | + uint8_t *out = (uint8_t*) dest; |
| 162 | + uint8_t a, b, c, d; |
| 163 | |
| 164 | memcpy(&i, &f, sizeof(uint32_t)); |
| 165 | - i = htonl(i); |
| 166 | - dest[0] = (uint16_t)bswap_16(i >> 16); |
| 167 | - dest[1] = (uint16_t)bswap_16(i & 0xFFFF); |
| 168 | + a = (i >> 24) & 0xFF; |
| 169 | + b = (i >> 16) & 0xFF; |
| 170 | + c = (i >> 8) & 0xFF; |
| 171 | + d = (i >> 0) & 0xFF; |
| 172 | + |
| 173 | + out[0] = b; |
| 174 | + out[1] = a; |
| 175 | + out[2] = d; |
| 176 | + out[3] = c; |
| 177 | } |
| 178 | |
| 179 | /* Set a float to 4 bytes for Modbus with word swap conversion (CDAB) */ |
| 180 | void modbus_set_float_cdab(float f, uint16_t *dest) |
| 181 | { |
| 182 | uint32_t i; |
| 183 | + uint8_t *out = (uint8_t*) dest; |
| 184 | + uint8_t a, b, c, d; |
| 185 | |
| 186 | memcpy(&i, &f, sizeof(uint32_t)); |
| 187 | - i = htonl(i); |
| 188 | - dest[0] = (uint16_t)i; |
| 189 | - dest[1] = (uint16_t)(i >> 16); |
| 190 | + a = (i >> 24) & 0xFF; |
| 191 | + b = (i >> 16) & 0xFF; |
| 192 | + c = (i >> 8) & 0xFF; |
| 193 | + d = (i >> 0) & 0xFF; |
| 194 | + |
| 195 | + out[0] = c; |
| 196 | + out[1] = d; |
| 197 | + out[2] = a; |
| 198 | + out[3] = b; |
| 199 | } |
| 200 | |
| 201 | /* DEPRECATED - Set a float to 4 bytes in a sort of Modbus format! */ |
| 202 | diff --git a/tests/unit-test-client.c b/tests/unit-test-client.c |
| 203 | index 3e315f4..3fccf3e 100644 |
| 204 | --- a/tests/unit-test-client.c |
| 205 | +++ b/tests/unit-test-client.c |
| 206 | @@ -27,6 +27,7 @@ int send_crafted_request(modbus_t *ctx, int function, |
| 207 | uint16_t max_value, uint16_t bytes, |
| 208 | int backend_length, int backend_offset); |
| 209 | int equal_dword(uint16_t *tab_reg, const uint32_t value); |
| 210 | +int is_memory_equal(const void *s1, const void *s2, size_t size); |
| 211 | |
| 212 | #define BUG_REPORT(_cond, _format, _args ...) \ |
| 213 | printf("\nLine %d: assertion error for '%s': " _format "\n", __LINE__, # _cond, ## _args) |
| 214 | @@ -40,6 +41,11 @@ int equal_dword(uint16_t *tab_reg, const uint32_t value); |
| 215 | } \ |
| 216 | }; |
| 217 | |
| 218 | +int is_memory_equal(const void *s1, const void *s2, size_t size) |
| 219 | +{ |
| 220 | + return (memcmp(s1, s2, size) == 0); |
| 221 | +} |
| 222 | + |
| 223 | int equal_dword(uint16_t *tab_reg, const uint32_t value) { |
| 224 | return ((tab_reg[0] == (value >> 16)) && (tab_reg[1] == (value & 0xFFFF))); |
| 225 | } |
| 226 | @@ -286,26 +292,26 @@ int main(int argc, char *argv[]) |
| 227 | /** FLOAT **/ |
| 228 | printf("1/4 Set/get float ABCD: "); |
| 229 | modbus_set_float_abcd(UT_REAL, tab_rp_registers); |
| 230 | - ASSERT_TRUE(equal_dword(tab_rp_registers, UT_IREAL_ABCD), "FAILED Set float ABCD"); |
| 231 | - real = modbus_get_float_abcd(tab_rp_registers); |
| 232 | + ASSERT_TRUE(is_memory_equal(tab_rp_registers, UT_IREAL_ABCD_SET, 4), "FAILED Set float ABCD"); |
| 233 | + real = modbus_get_float_abcd(UT_IREAL_ABCD_GET); |
| 234 | ASSERT_TRUE(real == UT_REAL, "FAILED (%f != %f)\n", real, UT_REAL); |
| 235 | |
| 236 | printf("2/4 Set/get float DCBA: "); |
| 237 | modbus_set_float_dcba(UT_REAL, tab_rp_registers); |
| 238 | - ASSERT_TRUE(equal_dword(tab_rp_registers, UT_IREAL_DCBA), "FAILED Set float DCBA"); |
| 239 | - real = modbus_get_float_dcba(tab_rp_registers); |
| 240 | + ASSERT_TRUE(is_memory_equal(tab_rp_registers, UT_IREAL_DCBA_SET, 4), "FAILED Set float DCBA"); |
| 241 | + real = modbus_get_float_dcba(UT_IREAL_DCBA_GET); |
| 242 | ASSERT_TRUE(real == UT_REAL, "FAILED (%f != %f)\n", real, UT_REAL); |
| 243 | |
| 244 | printf("3/4 Set/get float BADC: "); |
| 245 | modbus_set_float_badc(UT_REAL, tab_rp_registers); |
| 246 | - ASSERT_TRUE(equal_dword(tab_rp_registers, UT_IREAL_BADC), "FAILED Set float BADC"); |
| 247 | - real = modbus_get_float_badc(tab_rp_registers); |
| 248 | + ASSERT_TRUE(is_memory_equal(tab_rp_registers, UT_IREAL_BADC_SET, 4), "FAILED Set float BADC"); |
| 249 | + real = modbus_get_float_badc(UT_IREAL_BADC_GET); |
| 250 | ASSERT_TRUE(real == UT_REAL, "FAILED (%f != %f)\n", real, UT_REAL); |
| 251 | |
| 252 | printf("4/4 Set/get float CDAB: "); |
| 253 | modbus_set_float_cdab(UT_REAL, tab_rp_registers); |
| 254 | - ASSERT_TRUE(equal_dword(tab_rp_registers, UT_IREAL_CDAB), "FAILED Set float CDAB"); |
| 255 | - real = modbus_get_float_cdab(tab_rp_registers); |
| 256 | + ASSERT_TRUE(is_memory_equal(tab_rp_registers, UT_IREAL_CDAB_SET, 4), "FAILED Set float CDAB"); |
| 257 | + real = modbus_get_float_cdab(UT_IREAL_CDAB_GET); |
| 258 | ASSERT_TRUE(real == UT_REAL, "FAILED (%f != %f)\n", real, UT_REAL); |
| 259 | |
| 260 | printf("\nAt this point, error messages doesn't mean the test has failed\n"); |
| 261 | diff --git a/tests/unit-test.h.in b/tests/unit-test.h.in |
| 262 | index dca826f..4ffa254 100644 |
| 263 | --- a/tests/unit-test.h.in |
| 264 | +++ b/tests/unit-test.h.in |
| 265 | @@ -56,12 +56,45 @@ const uint16_t UT_INPUT_REGISTERS_ADDRESS = 0x108; |
| 266 | const uint16_t UT_INPUT_REGISTERS_NB = 0x1; |
| 267 | const uint16_t UT_INPUT_REGISTERS_TAB[] = { 0x000A }; |
| 268 | |
| 269 | +/* |
| 270 | + * This float value is 0x47F12000 (in big-endian format). |
| 271 | + * In Little-endian(intel) format, it will be stored in memory as follows: |
| 272 | + * 0x00 0x20 0xF1 0x47 |
| 273 | + * |
| 274 | + * You can check this with the following code: |
| 275 | + |
| 276 | + float fl = UT_REAL; |
| 277 | + uint8_t *inmem = (uint8_t*)&fl; |
| 278 | + int x; |
| 279 | + for(x = 0; x < 4; x++){ |
| 280 | + printf("0x%02X ", inmem[ x ]); |
| 281 | + } |
| 282 | + printf("\n"); |
| 283 | + */ |
| 284 | const float UT_REAL = 123456.00; |
| 285 | |
| 286 | -const uint32_t UT_IREAL_ABCD = 0x0020F147; |
| 287 | -const uint32_t UT_IREAL_DCBA = 0x47F12000; |
| 288 | -const uint32_t UT_IREAL_BADC = 0x200047F1; |
| 289 | -const uint32_t UT_IREAL_CDAB = 0xF1470020; |
| 290 | +/* |
| 291 | + * The following arrays assume that 'A' is the MSB, |
| 292 | + * and 'D' is the LSB. |
| 293 | + * Thus, the following is the case: |
| 294 | + * A = 0x47 |
| 295 | + * B = 0xF1 |
| 296 | + * C = 0x20 |
| 297 | + * D = 0x00 |
| 298 | + * |
| 299 | + * There are two sets of arrays: one to test that the setting is correct, |
| 300 | + * the other to test that the getting is correct. |
| 301 | + * Note that the 'get' values must be constants in processor-endianness, |
| 302 | + * as libmodbus will convert all words to processor-endianness as they come in. |
| 303 | + */ |
| 304 | +const uint8_t UT_IREAL_ABCD_SET[] = {0x47, 0xF1, 0x20, 0x00}; |
| 305 | +const uint16_t UT_IREAL_ABCD_GET[] = {0x47F1, 0x2000}; |
| 306 | +const uint8_t UT_IREAL_DCBA_SET[] = {0x00, 0x20, 0xF1, 0x47}; |
| 307 | +const uint16_t UT_IREAL_DCBA_GET[] = {0x0020, 0xF147}; |
| 308 | +const uint8_t UT_IREAL_BADC_SET[] = {0xF1, 0x47, 0x00, 0x20}; |
| 309 | +const uint16_t UT_IREAL_BADC_GET[] = {0xF147, 0x0020}; |
| 310 | +const uint8_t UT_IREAL_CDAB_SET[] = {0x20, 0x00, 0x47, 0xF1}; |
| 311 | +const uint16_t UT_IREAL_CDAB_GET[] = {0x2000, 0x47F1}; |
| 312 | |
| 313 | /* const uint32_t UT_IREAL_ABCD = 0x47F12000); |
| 314 | const uint32_t UT_IREAL_DCBA = 0x0020F147; |