Skip to content

Commit

Permalink
Only load each wide integer once in MODBUS_SET_INT*_TO_INT*() macros
Browse files Browse the repository at this point in the history
If you have a union like this as part of the read data:
	union {
		uint8_t serial[6];
		uint16_t serial_raw[3];
	};
and do the obvious
	for(size_t i = 0; i < sizeof(rdng.serial_raw) / sizeof(*rdng.serial_raw); ++i) {
		 MODBUS_SET_INT16_TO_INT8(rdng.serial, i*2, rdng.serial_raw[i]);
	}
then because serial_raw[i] is updated through serial[i] at first, then
loaded again, you end up with rdng.serial[i*2]=rdng.serial[i*2+1] for
all i, which is both confusing and wrong; instead, you must do
	for(size_t i = 0; i < sizeof(rdng.serial_raw) / sizeof(*rdng.serial_raw); ++i) {
		 uint16_t r = rdng.serial_raw[i];
		 MODBUS_SET_INT16_TO_INT8(rdng.serial, i*2, rdng.serial_raw[i]);
	}
but there's really no reason to require this,
and this patch lets you use them directly
  • Loading branch information
nabijaczleweli committed Feb 11, 2023
1 parent b25629b commit bce2a89
Showing 1 changed file with 17 additions and 14 deletions.
31 changes: 17 additions & 14 deletions src/modbus.h
Original file line number Diff line number Diff line change
Expand Up @@ -283,22 +283,25 @@ MODBUS_API int modbus_disable_quirks(modbus_t *ctx, unsigned int quirks_mask);
(((int32_t) tab_int16[(index)] << 16) | (int32_t) tab_int16[(index) + 1])
#define MODBUS_GET_INT16_FROM_INT8(tab_int8, index) \
(((int16_t) tab_int8[(index)] << 8) | (int16_t) tab_int8[(index) + 1])
#define MODBUS_SET_INT16_TO_INT8(tab_int8, index, value) \
do { \
((int8_t *) (tab_int8))[(index)] = (int8_t) ((value) >> 8); \
((int8_t *) (tab_int8))[(index) + 1] = (int8_t) (value); \
#define MODBUS_SET_INT16_TO_INT8(tab_int8, index, value) \
do { \
uint16_t _val = (value); \
((int8_t *) (tab_int8))[(index)] = (int8_t) _val; \
((int8_t *) (tab_int8))[(index) + 1] = (int8_t) _val; \
} while (0)
#define MODBUS_SET_INT32_TO_INT16(tab_int16, index, value) \
do { \
((int16_t *) (tab_int16))[(index)] = (int16_t) ((value) >> 16); \
((int16_t *) (tab_int16))[(index) + 1] = (int16_t) (value); \
#define MODBUS_SET_INT32_TO_INT16(tab_int16, index, value) \
do { \
uint32_t _val = (value); \
((int16_t *) (tab_int16))[(index)] = (int16_t) _val; \
((int16_t *) (tab_int16))[(index) + 1] = (int16_t) _val; \
} while (0)
#define MODBUS_SET_INT64_TO_INT16(tab_int16, index, value) \
do { \
((int16_t *) (tab_int16))[(index)] = (int16_t) ((value) >> 48); \
((int16_t *) (tab_int16))[(index) + 1] = (int16_t) ((value) >> 32); \
((int16_t *) (tab_int16))[(index) + 2] = (int16_t) ((value) >> 16); \
((int16_t *) (tab_int16))[(index) + 3] = (int16_t) (value); \
#define MODBUS_SET_INT64_TO_INT16(tab_int16, index, value) \
do { \
uint64_t _val = (value); \
((int16_t *) (tab_int16))[(index)] = (int16_t) (_val >> 48); \
((int16_t *) (tab_int16))[(index) + 1] = (int16_t) (_val >> 32); \
((int16_t *) (tab_int16))[(index) + 2] = (int16_t) (_val >> 16); \
((int16_t *) (tab_int16))[(index) + 3] = (int16_t) _val; \
} while (0)

MODBUS_API void modbus_set_bits_from_byte(uint8_t *dest, int idx, const uint8_t value);
Expand Down

0 comments on commit bce2a89

Please sign in to comment.