Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix float32 encoding/decoding (#665, #694, #757) #770

Closed
wants to merge 1 commit into from
Closed

Fix float32 encoding/decoding (#665, #694, #757) #770

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
155 changes: 52 additions & 103 deletions src/modbus-data.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -128,11 +84,14 @@ float modbus_get_float_abcd(const uint16_t *src)
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;
// Mind: src contains 16-bit numbers in processor-endianness, hence
// we use shift operations and do not access memory directly
a = (src[0] >> 8) & 0xFF; // high byte of first word
b = (src[0] >> 0) & 0xFF; // low byte of first word
c = (src[1] >> 8) & 0xFF; // high byte of second word
d = (src[1] >> 0) & 0xFF; // low byte of second word

// we assemble 32bit integer always in abcd order via shift operations
i = (a << 24) | (b << 16) | (c << 8) | (d << 0);
memcpy(&f, &i, 4);

Expand All @@ -146,12 +105,14 @@ float modbus_get_float_dcba(const uint16_t *src)
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;
// byte order is defined when reading from src: dcba
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);
// we assemble 32bit integer always in abcd order via shift operations
i = (a << 24) | (b << 16) | (c << 8) | (d << 0);
memcpy(&f, &i, 4);

return f;
Expand All @@ -164,12 +125,14 @@ float modbus_get_float_badc(const uint16_t *src)
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;
// byte order is defined when reading from src: badc
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);
// we assemble 32bit integer always in abcd order via shift operations
i = (a << 24) | (b << 16) | (c << 8) | (d << 0);
memcpy(&f, &i, 4);

return f;
Expand All @@ -182,12 +145,14 @@ float modbus_get_float_cdab(const uint16_t *src)
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;
// byte order is defined when reading from src: cdab
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);
// we assemble 32bit integer always in abcd order via shift operations
i = (a << 24) | (b << 16) | (c << 8) | (d << 0);
memcpy(&f, &i, 4);

return f;
Expand All @@ -196,97 +161,81 @@ float modbus_get_float_cdab(const uint16_t *src)
/* 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;
// The straight-forward type conversion won't work because of type-punned pointer aliasing warning
// uint32_t i = *(uint32_t*)(&f);
float * fptr = &f;
uint32_t * iptr = (uint32_t *)fptr;
uint32_t i = *iptr;
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;
float * fptr = &f;
uint32_t * iptr = (uint32_t *)fptr;
uint32_t i = *iptr;
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;
float * fptr = &f;
uint32_t * iptr = (uint32_t *)fptr;
uint32_t i = *iptr;
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;
float * fptr = &f;
uint32_t * iptr = (uint32_t *)fptr;
uint32_t i = *iptr;
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);
modbus_set_float_cdab(f, dest);
}
4 changes: 4 additions & 0 deletions src/modbus.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,10 @@ extern const unsigned int libmodbus_version_micro;

typedef struct _modbus modbus_t;

/*! Memory layout in tab_xxx arrays is processor-endianness.
When receiving modbus data, it is converted to processor-endianness,
see read_registers().
*/
typedef struct _modbus_mapping_t {
int nb_bits;
int start_bits;
Expand Down
16 changes: 8 additions & 8 deletions tests/unit-test-client.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,30 +323,30 @@ 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(is_memory_equal(tab_rp_registers, UT_IREAL_ABCD_SET, 4),
ASSERT_TRUE(is_memory_equal(tab_rp_registers, UT_IREAL_ABCD, 4),
"FAILED Set float ABCD");
real = modbus_get_float_abcd(UT_IREAL_ABCD_GET);
real = modbus_get_float_abcd(UT_IREAL_ABCD);
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(is_memory_equal(tab_rp_registers, UT_IREAL_DCBA_SET, 4),
ASSERT_TRUE(is_memory_equal(tab_rp_registers, UT_IREAL_DCBA, 4),
"FAILED Set float DCBA");
real = modbus_get_float_dcba(UT_IREAL_DCBA_GET);
real = modbus_get_float_dcba(UT_IREAL_DCBA);
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(is_memory_equal(tab_rp_registers, UT_IREAL_BADC_SET, 4),
ASSERT_TRUE(is_memory_equal(tab_rp_registers, UT_IREAL_BADC, 4),
"FAILED Set float BADC");
real = modbus_get_float_badc(UT_IREAL_BADC_GET);
real = modbus_get_float_badc(UT_IREAL_BADC);
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(is_memory_equal(tab_rp_registers, UT_IREAL_CDAB_SET, 4),
ASSERT_TRUE(is_memory_equal(tab_rp_registers, UT_IREAL_CDAB, 4),
"FAILED Set float CDAB");
real = modbus_get_float_cdab(UT_IREAL_CDAB_GET);
real = modbus_get_float_cdab(UT_IREAL_CDAB);
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");
Expand Down
34 changes: 9 additions & 25 deletions tests/unit-test.h.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,31 +76,15 @@ const uint16_t UT_INPUT_REGISTERS_TAB[] = { 0x000A };
const float UT_REAL = 123456.00;

/*
* 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.
* libmodbus will convert all words to processor-endianness as they come in and also converts them
* back to big endian as they are sent out.
* The memory layout in modbus_mapping_t.tab_registers is thus processor-endianess.
* Hence we define the comparison constants to check against the content of modbus_mapping_t.tab_registers
* in terms of 16 bit integer values, that are likewise stored in memory in processor-endianess.
*/
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;
const uint32_t UT_IREAL_BADC = 0xF1470020;
const uint32_t UT_IREAL_CDAB = 0x200047F1;*/
const uint16_t UT_IREAL_ABCD[] = {0x47F1, 0x2000};
const uint16_t UT_IREAL_DCBA[] = {0x0020, 0xF147};
const uint16_t UT_IREAL_BADC[] = {0xF147, 0x0020};
const uint16_t UT_IREAL_CDAB[] = {0x2000, 0x47F1};

#endif /* _UNIT_TEST_H_ */
Loading