Fixes float encoding/decoding for both big and little endian (fixes #665, #694)

* Removes useless memcpy calls and no longer used swap32 and swap16 macros.
* Updated float encoding functions to consider endianess of 16-bit registers (fixes #665)
* Deprecated modbus_get_float() and modbus_set_float() are implemented using cdab variants.

Author: ghorwin

src/modbus-data.c

@@ -26,50 +26,6 @@
 
 #include "modbus.h"
 
-#if defined(HAVE_BYTESWAP_H)
-#  include <byteswap.h>
-#endif
-
-#if defined(__APPLE__)
-#  include <libkern/OSByteOrder.h>
-#  define bswap_16 OSSwapInt16
-#  define bswap_32 OSSwapInt32
-#  define bswap_64 OSSwapInt64
-#endif
-
-#if defined(__GNUC__)
-#  define GCC_VERSION (__GNUC__ * 100 + __GNUC_MINOR__ * 10)
-#  if GCC_VERSION >= 430
-// Since GCC >= 4.30, GCC provides __builtin_bswapXX() alternatives so we switch to them
-#    undef bswap_32
-#    define bswap_32 __builtin_bswap32
-#  endif
-#  if GCC_VERSION >= 480
-#    undef bswap_16
-#    define bswap_16 __builtin_bswap16
-#  endif
-#endif
-
-#if defined(_MSC_VER) && (_MSC_VER >= 1400)
-#  define bswap_32 _byteswap_ulong
-#  define bswap_16 _byteswap_ushort
-#endif
-
-#if !defined(bswap_16)
-#  warning "Fallback on C functions for bswap_16"
-static inline uint16_t bswap_16(uint16_t x)
-{
-    return (x >> 8) | (x << 8);
-}
-#endif
-
-#if !defined(bswap_32)
-#  warning "Fallback on C functions for bswap_32"
-static inline uint32_t bswap_32(uint32_t x)
-{
-    return (bswap_16(x & 0xffff) << 16) | (bswap_16(x >> 16));
-}
-#endif
 // clang-format on
 
 /* Sets many bits from a single byte value (all 8 bits of the byte value are
@@ -124,17 +80,29 @@ uint8_t modbus_get_byte_from_bits(const uint8_t *src, int idx, unsigned int nb_b
 /* Get a float from 4 bytes (Modbus) without any conversion (ABCD) */
 float modbus_get_float_abcd(const uint16_t *src)
 {
+    /* Suppose the modbus byte stream contained the 32-bit float 0x10203040  - abcd.
+       On big endian systems, the memory starting at src contains two 16-bit registers 0x1020  and 0x3040
+       On little endian system, the 16-bit registers memory holds 0x2010 and 0x4030
+
+       To convert the data to float32 on big-endian we only need to cast the pointer and we are done.
+       On little endian systems, we need to swap the bytes in each word again and then assemble
+       an integer via shift operations and finally cast to float32.
+
+       A portable way is to retrieve low and high bytes of both words using shift operations, then assemble
+       the 32-bit integer.
+    */
+
     float f;
-    uint32_t i;
     uint8_t a, b, c, d;
 
-    a = (src[0] >> 8) & 0xFF;
-    b = (src[0] >> 0) & 0xFF;
-    c = (src[1] >> 8) & 0xFF;
-    d = (src[1] >> 0) & 0xFF;
+    a = (src[0] >> 8) & 0xFF; // high byte if first word
+    b = (src[0] >> 0) & 0xFF; // low byte if first word
+    c = (src[1] >> 8) & 0xFF; // high byte if second word
+    d = (src[1] >> 0) & 0xFF; // low byte if second word
 
-    i = (a << 24) | (b << 16) | (c << 8) | (d << 0);
-    memcpy(&f, &i, 4);
+    // assemble in memory location of float
+    // from right to left: get address of float, interpret as address to uint32, dereference and write uint32
+    *(uint32_t *)&f = (a << 24) | (b << 16) | (c << 8) | (d << 0);
 
     return f;
 }
@@ -143,16 +111,16 @@ float modbus_get_float_abcd(const uint16_t *src)
 float modbus_get_float_dcba(const uint16_t *src)
 {
     float f;
-    uint32_t i;
     uint8_t a, b, c, d;
 
-    a = (src[0] >> 8) & 0xFF;
-    b = (src[0] >> 0) & 0xFF;
-    c = (src[1] >> 8) & 0xFF;
-    d = (src[1] >> 0) & 0xFF;
+    d = (src[0] >> 8) & 0xFF;
+    c = (src[0] >> 0) & 0xFF;
+    b = (src[1] >> 8) & 0xFF;
+    a = (src[1] >> 0) & 0xFF;
 
-    i = (d << 24) | (c << 16) | (b << 8) | (a << 0);
-    memcpy(&f, &i, 4);
+    // assemble in memory location of float
+    // from right to left: get address of float, interpret as address to uint32, dereference and write uint32
+    *(uint32_t *)&f = (a << 24) | (b << 16) | (c << 8) | (d << 0);
 
     return f;
 }
@@ -161,16 +129,16 @@ float modbus_get_float_dcba(const uint16_t *src)
 float modbus_get_float_badc(const uint16_t *src)
 {
     float f;
-    uint32_t i;
     uint8_t a, b, c, d;
 
-    a = (src[0] >> 8) & 0xFF;
-    b = (src[0] >> 0) & 0xFF;
-    c = (src[1] >> 8) & 0xFF;
-    d = (src[1] >> 0) & 0xFF;
+    b = (src[0] >> 8) & 0xFF;
+    a = (src[0] >> 0) & 0xFF;
+    d = (src[1] >> 8) & 0xFF;
+    c = (src[1] >> 0) & 0xFF;
 
-    i = (b << 24) | (a << 16) | (d << 8) | (c << 0);
-    memcpy(&f, &i, 4);
+    // assemble in memory location of float
+    // from right to left: get address of float, interpret as address to uint32, dereference and write uint32
+    *(uint32_t *)&f = (a << 24) | (b << 16) | (c << 8) | (d << 0);
 
     return f;
 }
@@ -179,114 +147,88 @@ float modbus_get_float_badc(const uint16_t *src)
 float modbus_get_float_cdab(const uint16_t *src)
 {
     float f;
-    uint32_t i;
     uint8_t a, b, c, d;
 
-    a = (src[0] >> 8) & 0xFF;
-    b = (src[0] >> 0) & 0xFF;
-    c = (src[1] >> 8) & 0xFF;
-    d = (src[1] >> 0) & 0xFF;
+    c = (src[0] >> 8) & 0xFF;
+    d = (src[0] >> 0) & 0xFF;
+    a = (src[1] >> 8) & 0xFF;
+    b = (src[1] >> 0) & 0xFF;
 
-    i = (c << 24) | (d << 16) | (a << 8) | (b << 0);
-    memcpy(&f, &i, 4);
+    // assemble in memory location of float
+    // from right to left: get address of float, interpret as address to uint32, dereference and write uint32
+    *(uint32_t *)&f = (a << 24) | (b << 16) | (c << 8) | (d << 0);
 
     return f;
 }
 
 /* DEPRECATED - Get a float from 4 bytes in sort of Modbus format */
 float modbus_get_float(const uint16_t *src)
 {
-    float f;
-    uint32_t i;
-
-    i = (((uint32_t) src[1]) << 16) + src[0];
-    memcpy(&f, &i, sizeof(float));
-
-    return f;
+    return modbus_get_float_cdab(src);
 }
 
 /* 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;
+    uint32_t i = *(uint32_t*)(&f);
     uint8_t a, b, c, d;
 
-    memcpy(&i, &f, sizeof(uint32_t));
     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;
+    dest[0] = (a << 8) | b;
+    dest[1] = (c << 8) | 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;
+    uint32_t i = *(uint32_t*)(&f);
     uint8_t a, b, c, d;
 
-    memcpy(&i, &f, sizeof(uint32_t));
     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;
+    dest[0] = (d << 8) | c;
+    dest[1] = (b << 8) | 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;
+    uint32_t i = *(uint32_t*)(&f);
     uint8_t a, b, c, d;
 
-    memcpy(&i, &f, sizeof(uint32_t));
     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;
+    dest[0] = (b << 8) | a;
+    dest[1] = (d << 8) | 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;
+    uint32_t i = *(uint32_t*)(&f);
     uint8_t a, b, c, d;
 
-    memcpy(&i, &f, sizeof(uint32_t));
     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;
+    dest[0] = (c << 8) | d;
+    dest[1] = (a << 8) | b;
 }
 
 /* DEPRECATED - Set a float to 4 bytes in a sort of Modbus format! */
 void modbus_set_float(float f, uint16_t *dest)
 {
-    uint32_t i;
-
-    memcpy(&i, &f, sizeof(uint32_t));
-    dest[0] = (uint16_t) i;
-    dest[1] = (uint16_t) (i >> 16);
+    return modbus_set_float_cdab(f, dest);
 }