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.

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.

src/modbus.c

@@ -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;

tests/unit-test-client.c

@@ -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;
         }
     }

tests/unit-test.h

@@ -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. */