Permalink
Browse files

Read/write were swapped in _FC_READ_AND_WRITE_REGISTERS. Closes GH-2.

Page 38 in the document Modbus_Application_Protocol_V1_1b.pdf:
6.17 23 (0x17) Read/Write Multiple registers

This function code performs a combination of one read operation
and one write operation in a single MODBUS transaction. The write
operation is performed before the read.

The unit test has been updated.
  • Loading branch information...
1 parent 7337853 commit 49d6f4a71f686de64e5c28d6dd8acd2c98c5038d @stephane stephane committed Jan 6, 2011
Showing with 21 additions and 32 deletions.
  1. +1 −0 NEWS
  2. +8 −6 src/modbus.c
  3. +11 −25 tests/unit-test-client.c
  4. +1 −1 tests/unit-test.h
View
1 NEWS
@@ -1,6 +1,7 @@
libmodbus 2.9.3 (2011-01-XX)
============================
+- Fix GH-2. Read/write were swapped in _FC_READ_AND_WRITE_REGISTERS
- Install an ignore handler for SIGPIPE on *BSD
Original patch by Jason Oster.
- Fix closing of Win32 socket.
View
@@ -794,16 +794,18 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req,
rsp_length = ctx->backend->build_response_basis(&sft, rsp);
rsp[rsp_length++] = nb << 1;
- for (i = address; i < address + nb; i++) {
- rsp[rsp_length++] = mb_mapping->tab_registers[i] >> 8;
- rsp[rsp_length++] = mb_mapping->tab_registers[i] & 0xFF;
- }
-
- /* 10 and 11 = first value */
+ /* Write first.
+ 10 and 11 are the offset of the first values to write */
for (i = address_write, j = 10; i < address_write + nb_write; i++, j += 2) {
mb_mapping->tab_registers[i] =
(req[offset + j] << 8) + req[offset + j + 1];
}
+
+ /* and read the data for the response */
+ for (i = address; i < address + nb; i++) {
+ rsp[rsp_length++] = mb_mapping->tab_registers[i] >> 8;
+ rsp[rsp_length++] = mb_mapping->tab_registers[i] & 0xFF;
+ }
}
}
break;
View
@@ -269,45 +269,31 @@ int main(int argc, char *argv[])
UT_REGISTERS_NB_POINTS : UT_INPUT_REGISTERS_NB_POINTS;
memset(tab_rp_registers, 0, nb_points * sizeof(uint16_t));
- /* Write registers to zero from tab_rp_registers and read registers to
- tab_rp_registers. They should be same as UT_REGISTERS_TAB. */
+ /* Write registers to zero from tab_rp_registers and store read registers
+ into tab_rp_registers. So the read registers must set to 0, except the
+ first one because there is an offset of 1 register on write. */
rc = modbus_read_and_write_registers(ctx,
UT_REGISTERS_ADDRESS,
UT_REGISTERS_NB_POINTS,
tab_rp_registers,
- UT_REGISTERS_ADDRESS,
- UT_REGISTERS_NB_POINTS,
+ UT_REGISTERS_ADDRESS + 1,
+ UT_REGISTERS_NB_POINTS - 1,
tab_rp_registers);
- printf("4/5 modbus_read_and_write_registers, read part: ");
+ printf("4/5 modbus_read_and_write_registers: ");
if (rc != UT_REGISTERS_NB_POINTS) {
printf("FAILED (nb points %d != %d)\n", rc, UT_REGISTERS_NB_POINTS);
goto close;
}
- for (i=0; i < UT_REGISTERS_NB_POINTS; i++) {
- if (tab_rp_registers[i] != UT_REGISTERS_TAB[i]) {
- printf("FAILED (%0X != %0X)\n",
- tab_rp_registers[i],
- UT_REGISTERS_TAB[i]);
- goto close;
- }
- }
- printf("OK\n");
-
- rc = modbus_read_registers(ctx, UT_REGISTERS_ADDRESS,
- UT_REGISTERS_NB_POINTS,
- tab_rp_registers);
- printf("5/5 modbus_read_and_write_registers, write part: ");
- if (rc != UT_REGISTERS_NB_POINTS) {
- printf("FAILED (nb points %d != %d)\n", rc, UT_REGISTERS_NB_POINTS);
- goto close;
+ if (tab_rp_registers[0] != UT_REGISTERS_TAB[0]) {
+ printf("FAILED (%0X != %0X)\n",
+ tab_rp_registers[0], UT_REGISTERS_TAB[0]);
}
- for (i=0; i < UT_REGISTERS_NB_POINTS; i++) {
+ for (i=1; i < UT_REGISTERS_NB_POINTS; i++) {
if (tab_rp_registers[i] != 0) {
printf("FAILED (%0X != %0X)\n",
- tab_rp_registers[i],
- UT_REGISTERS_TAB[i]);
+ tab_rp_registers[i], 0);
goto close;
}
}
View
@@ -41,7 +41,7 @@ const uint16_t UT_REGISTERS_ADDRESS = 0x6B;
/* Raise a manual exception when this adress is used for the first byte */
const uint16_t UT_REGISTERS_ADDRESS_SPECIAL = 0x6C;
const uint16_t UT_REGISTERS_NB_POINTS = 0x3;
-const uint16_t UT_REGISTERS_TAB[] = { 0x022B, 0x0000, 0x0064 };
+const uint16_t UT_REGISTERS_TAB[] = { 0x022B, 0x0001, 0x0064 };
/* If the following value is used, a bad response is sent.
It's better to test with a lower value than
UT_REGISTERS_NB_POINTS to try to raise a segfault. */

0 comments on commit 49d6f4a

Please sign in to comment.