Fix #591142 - Slave id check should be disabled in TCP connection

A new API will be committed to remove the slave in TCP communication.

NEWS

@@ -10,6 +10,8 @@ libmodbus 2.1.1 (2010-XX-XX)
   Reported by David Olivari.
 - Fix #463299 - New functions to define the timeouts of begin and end of trame
   Original patch by Sisyph (eric-paul).
+- Fix #591142 - Slave id check should be disabled in TCP connection
+  Reported by aladdinwu.
 
 libmodbus 2.1.0 (2010-03-24)
 ============================

src/modbus.c

@@ -363,7 +363,7 @@ static int build_response_basis_tcp(sft_t *sft, uint8_t *response)
 
         /* Length will be set later by modbus_send (4 and 5) */
 
-        response[6] = sft->slave;
+        response[6] = 0xFF;
         response[7] = sft->function;
 
         return PRESET_RESPONSE_LENGTH_TCP;
@@ -887,8 +887,10 @@ int modbus_slave_manage(modbus_param_t *mb_param, const uint8_t *query,
         int resp_length = 0;
         sft_t sft;
 
-        if (slave != mb_param->slave && slave != MODBUS_BROADCAST_ADDRESS) {
-                // Ignores the query (not for me)
+        /* Filter on the Modbus unit identifier (slave) in RTU mode */
+        if (mb_param->type_com == RTU &&
+            slave != mb_param->slave && slave != MODBUS_BROADCAST_ADDRESS) {
+            /* Ignores the query (not for me) */
                 if (mb_param->debug) {
                         printf("Request for slave %d ignored (not %d)\n",
                                slave, mb_param->slave);
@@ -1483,7 +1485,7 @@ void init_common(modbus_param_t *mb_param)
    - stop_bits: 1, 2
    - slave: slave number
 */
-void modbus_init_rtu(modbus_param_t *mb_param, const char *device,
+int modbus_init_rtu(modbus_param_t *mb_param, const char *device,
                      int baud, const char *parity, int data_bit,
                      int stop_bit, int slave)
 {
@@ -1496,9 +1498,10 @@ void modbus_init_rtu(modbus_param_t *mb_param, const char *device,
         mb_param->stop_bit = stop_bit;
         mb_param->type_com = RTU;
         mb_param->error_recovery = FALSE;
-        mb_param->slave = slave;
 
         init_common(mb_param);
+
+        return modbus_set_slave(mb_param, slave);
 }
 
 /* Initializes the modbus_param_t structure for TCP.
@@ -1510,35 +1513,48 @@ void modbus_init_rtu(modbus_param_t *mb_param, const char *device,
    to 1024 because it's not necessary to be root to use this port
    number.
 */
-void modbus_init_tcp(modbus_param_t *mb_param, const char *ip, int port, int slave)
+int modbus_init_tcp(modbus_param_t *mb_param, const char *ip, int port)
 {
         memset(mb_param, 0, sizeof(modbus_param_t));
         strncpy(mb_param->ip, ip, sizeof(char)*16);
         mb_param->port = port;
         mb_param->type_com = TCP;
         mb_param->error_recovery = FALSE;
-        mb_param->slave = slave;
+        /* Can be changed after to reach remote serial Modbus device */
+        mb_param->slave = 0xFF;
 
         init_common(mb_param);
+
+        return 0;
 }
 
-/* Define the slave number */
-void modbus_set_slave(modbus_param_t *mb_param, int slave)
+/* Define the slave number, the special value MODBUS_TCP_SLAVE (0xFF) can be
+ * used in TCP mode to restore the default value. */
+int modbus_set_slave(modbus_param_t *mb_param, int slave)
 {
-        mb_param->slave = slave;
+        if (slave >= 1 && slave <= 247) {
+                mb_param->slave = slave;
+        } else if (mb_param->type_com == TCP && slave == MODBUS_TCP_SLAVE) {
+                mb_param->slave = slave;
+        } else {
+                errno = EINVAL;
+                return -1;
+        }
+
+        return 0;
 }
 
 /*
-   When disabled (default), it is expected that the application will check for error
-   returns and deal with them as necessary.
+   When disabled (default), it is expected that the application will check for
+   error returns and deal with them as necessary.
 
    It's not recommanded to enable error recovery for slave/server.
 
    When enabled, the library will attempt an immediate reconnection which may
    hang for several seconds if the network to the remote target unit is down.
    The write will try a infinite close/connect loop until to be successful and
-   the select/read calls will just try to retablish the connection one time
-   then will return an error (if the connecton was down, the values to read are
+   the select/read calls will just try to retablish the connection one time then
+   will return an error (if the connecton was down, the values to read are
    certainly not available anymore after reconnection, except for slave/server).
 */
 int modbus_set_error_recovery(modbus_param_t *mb_param, int enabled)

src/modbus.h.in

@@ -39,6 +39,7 @@ extern "C" {
 
 #define MODBUS_TCP_DEFAULT_PORT   502
 #define MODBUS_BROADCAST_ADDRESS    0
+#define MODBUS_TCP_SLAVE         0xFF
 
 /* Slave index */
 #define HEADER_LENGTH_RTU           1
@@ -249,12 +250,11 @@ typedef struct {
         uint16_t *tab_holding_registers;
 } modbus_mapping_t;
 
-void modbus_init_rtu(modbus_param_t *mb_param, const char *device,
-                     int baud, const char *parity, int data_bit,
-                     int stop_bit, int slave);
-void modbus_init_tcp(modbus_param_t *mb_param, const char *ip_address, int port,
-                     int slave);
-void modbus_set_slave(modbus_param_t *mb_param, int slave);
+int modbus_init_rtu(modbus_param_t *mb_param, const char *device,
+                    int baud, const char *parity, int data_bit,
+                    int stop_bit, int slave);
+int modbus_init_tcp(modbus_param_t *mb_param, const char *ip_address, int port);
+int modbus_set_slave(modbus_param_t *mb_param, int slave);
 int modbus_set_error_recovery(modbus_param_t *mb_param, int enabled);
 
 void modbus_get_timeout_begin(modbus_param_t *mb_param, struct timeval *timeout);

tests/bandwidth-master.c

@@ -26,7 +26,6 @@
 #include <modbus.h>
 
 /* Tests based on PI-MBUS-300 documentation */
-#define MY_ID          1
 #define SERVER_ID     17
 #define NB_LOOPS  100000
 
@@ -55,7 +54,7 @@ int main(void)
         int rc;
 
         /* TCP */
-        modbus_init_tcp(&mb_param, "127.0.0.1", 1502, MY_ID);
+        modbus_init_tcp(&mb_param, "127.0.0.1", 1502);
         rc = modbus_connect(&mb_param);
         if (rc == -1) {
                 fprintf(stderr, "Connexion failed: %s\n",

tests/bandwidth-slave-many-up.c

@@ -48,7 +48,7 @@ int main(void)
         /* Maximum file descriptor number */
         int fdmax;
 
-        modbus_init_tcp(&mb_param, "127.0.0.1", 1502, 17);
+        modbus_init_tcp(&mb_param, "127.0.0.1", 1502);
 
         rc = modbus_mapping_new(&mb_mapping, MAX_STATUS, 0, MAX_REGISTERS, 0);
         if (rc == -1) {

tests/bandwidth-slave-one.c

@@ -30,7 +30,7 @@ int main(void)
         modbus_mapping_t mb_mapping;
         int rc;
 
-        modbus_init_tcp(&mb_param, "127.0.0.1", 1502, 17);
+        modbus_init_tcp(&mb_param, "127.0.0.1", 1502);
 
         rc = modbus_mapping_new(&mb_mapping,  MAX_STATUS, 0, MAX_REGISTERS, 0);
         if (rc == -1) {

tests/random-test-master.c

@@ -64,7 +64,7 @@ int main(void)
         */
 
         /* TCP */
-        modbus_init_tcp(&mb_param, "127.0.0.1", 1502, MY_ID);
+        modbus_init_tcp(&mb_param, "127.0.0.1", 1502);
         modbus_set_debug(&mb_param, TRUE);
         if (modbus_connect(&mb_param) == -1) {
                 fprintf(stderr, "Connection failed: %s\n",

tests/random-test-slave.c

@@ -29,7 +29,7 @@ int main(void)
         modbus_mapping_t mb_mapping;
         int rc;
 
-        modbus_init_tcp(&mb_param, "127.0.0.1", 1502, 17);
+        modbus_init_tcp(&mb_param, "127.0.0.1", 1502);
         /* modbus_set_debug(&mb_param, TRUE); */
 
         rc = modbus_mapping_new(&mb_mapping, 500, 500, 500, 500);

tests/unit-test-master.c

@@ -46,7 +46,7 @@ int main(void)
         */
 
         /* TCP */
-        modbus_init_tcp(&mb_param, "127.0.0.1", 1502, CLIENT_ID);
+        modbus_init_tcp(&mb_param, "127.0.0.1", 1502);
         modbus_set_debug(&mb_param, TRUE);
 
         if (modbus_connect(&mb_param) == -1) {
@@ -450,16 +450,28 @@ int main(void)
 
         /** SLAVE REPLY **/
         printf("\nTEST SLAVE REPLY:\n");
+
         rc = read_holding_registers(&mb_param, 18,
                                     UT_HOLDING_REGISTERS_ADDRESS,
                                     UT_HOLDING_REGISTERS_NB_POINTS,
                                     tab_rp_registers);
-        printf("1/3 No reply from slave %d: ", 18);
-        if (rc == -1 && errno == ETIMEDOUT) {
-                printf("OK\n");
+        printf("1/3 No or response from slave %d: ", 18);
+        if (mb_param.type_com == RTU) {
+                /* No response in RTU mode */
+                if (rc == -1 && errno == ETIMEDOUT) {
+                        printf("OK\n");
+                } else {
+                        printf("FAILED\n");
+                        goto close;
+                }
         } else {
-                printf("FAILED\n");
-                goto close;
+                /* Response in TCP mode */
+                if (rc == UT_HOLDING_REGISTERS_NB_POINTS) {
+                        printf("OK\n");
+                } else {
+                        printf("FAILED\n");
+                        goto close;
+                }
         }
 
         rc = read_holding_registers(&mb_param, MODBUS_BROADCAST_ADDRESS,

tests/unit-test-slave.c

@@ -32,7 +32,7 @@ int main(void)
         int rc;
         int i;
 
-        modbus_init_tcp(&mb_param, "127.0.0.1", 1502, SERVER_ID);
+        modbus_init_tcp(&mb_param, "127.0.0.1", 1502);
         modbus_set_debug(&mb_param, TRUE);
         modbus_set_error_recovery(&mb_param, TRUE);