Move RTU filtering in CRC check to avoid useless call to modbus_reply

Warning, modbus_receive returns 0 when a query is ignored. - function removed from TCP and RTU code and from backend structure - updated documentation - updated examples

Move RTU filtering in CRC check to avoid useless call to modbus_reply
Warning, modbus_receive returns 0 when a query is ignored. - function removed from TCP and RTU code and from backend structure - updated documentation - updated examples
Stéphane Raimbault
1 parent a6e63d7d
Showing 10 changed files with 38 additions and 53 deletions
doc/modbus_receive.txt
doc/modbus_receive_confirmation.txt
src/modbus-private.h
src/modbus-rtu.c
src/modbus-tcp.c
src/modbus.c
tests/bandwidth-server-many-up.c
tests/bandwidth-server-one.c
tests/random-test-server.c
tests/unit-test-server.c
@@ -25,8 +25,9 @@ context &#39;ctx&#39;, see the function linkmb:modbus_set_socket[3].
 RETURN VALUE
 ------------
 The _modbus_receive()_ function shall store the indication request in 'req' and
-return the request length if sucessful. Otherwise it shall return -1 and set
-errno.
+return the request length if sucessful. The returned request length can be zero
+if the indication request is ignored (eg. a query for another slave in RTU
+mode). Otherwise it shall return -1 and set errno.
 SEE ALSO
@@ -23,8 +23,9 @@ function can be used to receive request not handled by the library.
 RETURN VALUE
 ------------
 The _modbus_receive_confirmation()_ function shall store the confirmation
-request in 'rsp' and return the response length if sucessful. Otherwise it shall
-return -1 and set errno.
+request in 'rsp' and return the response length if sucessful. The returned
+request length can be zero if the indication request is ignored (eg. a query for
+another slave in RTU mode). Otherwise it shall return -1 and set errno.
 SEE ALSO
@@ -97,7 +97,6 @@ typedef struct _modbus_backend {
     void (*close) (modbus_t *ctx);
     int (*flush) (modbus_t *ctx);
     int (*select) (modbus_t *ctx, fd_set *rfds, struct timeval *tv, int msg_length);
-    int (*filter_request) (modbus_t *ctx, int slave);
 } modbus_backend_t;
 struct _modbus {
@@ -328,7 +328,7 @@ int _modbus_rtu_pre_check_confirmation(modbus_t *ctx, const uint8_t *req,
     if (req[0] != 0 && req[0] != rsp[0]) {
         if (ctx->debug) {
             fprintf(stderr,
-                    "The responding slave %d it not the requested slave %d",
+                    "The responding slave %d isn't the requested slave %d",
                     rsp[0], req[0]);
         }
         errno = EMBBADSLAVE;
@@ -338,13 +338,24 @@ int _modbus_rtu_pre_check_confirmation(modbus_t *ctx, const uint8_t *req,
     }
 }
-/* The check_crc16 function shall return the message length if the CRC is
-   valid. Otherwise it shall return -1 and set errno to EMBADCRC. */
+/* The check_crc16 function shall return 0 is the message is ignored and the
+   message length if the CRC is valid. Otherwise it shall return -1 and set
+   errno to EMBADCRC. */
 int _modbus_rtu_check_integrity(modbus_t *ctx, uint8_t *msg,
                                 const int msg_length)
 {
     uint16_t crc_calculated;
     uint16_t crc_received;
+    int slave = msg[0];
+
+    /* Filter on the Modbus unit identifier (slave) in RTU mode */
+    if (slave != ctx->slave && slave != MODBUS_BROADCAST_ADDRESS) {
+        /* Ignores the request (not for me) */
+        if (ctx->debug) {
+            printf("Request for slave %d ignored (not %d)\n", slave, ctx->slave);
+        }
+        return 0;
+    }
     crc_calculated = crc16(msg, msg_length - 2);
     crc_received = (msg[msg_length - 2] << 8) | msg[msg_length - 1];
@@ -948,21 +959,6 @@ int _modbus_rtu_select(modbus_t *ctx, fd_set *rfds,
     return s_rc;
 }
-int _modbus_rtu_filter_request(modbus_t *ctx, int slave)
-{
-    /* Filter on the Modbus unit identifier (slave) in RTU mode */
-    if (slave != ctx->slave && slave != MODBUS_BROADCAST_ADDRESS) {
-        /* Ignores the request (not for me) */
-        if (ctx->debug) {
-            printf("Request for slave %d ignored (not %d)\n",
-                   slave, ctx->slave);
-        }
-        return 1;
-    } else {
-        return 0;
-    }
-}
-
 const modbus_backend_t _modbus_rtu_backend = {
     _MODBUS_BACKEND_TYPE_RTU,
     _MODBUS_RTU_HEADER_LENGTH,
@@ -980,8 +976,7 @@ const modbus_backend_t _modbus_rtu_backend = {
     _modbus_rtu_connect,
     _modbus_rtu_close,
     _modbus_rtu_flush,
-    _modbus_rtu_select,
-    _modbus_rtu_filter_request
+    _modbus_rtu_select
 };
 modbus_t* modbus_new_rtu(const char *device,
@@ -586,11 +586,6 @@ int _modbus_tcp_select(modbus_t *ctx, fd_set *rfds, struct timeval *tv, int leng
     return s_rc;
 }
-int _modbus_tcp_filter_request(modbus_t *ctx, int slave)
-{
-    return 0;
-}
-
 const modbus_backend_t _modbus_tcp_backend = {
     _MODBUS_BACKEND_TYPE_TCP,
     _MODBUS_TCP_HEADER_LENGTH,
@@ -608,8 +603,7 @@ const modbus_backend_t _modbus_tcp_backend = {
     _modbus_tcp_connect,
     _modbus_tcp_close,
     _modbus_tcp_flush,
-    _modbus_tcp_select,
-    _modbus_tcp_filter_request
+    _modbus_tcp_select
 };
@@ -630,8 +624,7 @@ const modbus_backend_t _modbus_tcp_pi_backend = {
     _modbus_tcp_pi_connect,
     _modbus_tcp_close,
     _modbus_tcp_flush,
-    _modbus_tcp_select,
-    _modbus_tcp_filter_request
+    _modbus_tcp_select
 };
 modbus_t* modbus_new_tcp(const char *ip, int port)
@@ -661,11 +661,6 @@ int modbus_reply(modbus_t *ctx, const uint8_t *req,
     int rsp_length = 0;
     sft_t sft;
-    if (ctx->backend->filter_request(ctx, slave) == 1) {
-        /* Filtered */
-        return 0;
-    }
-
     sft.slave = slave;
     sft.function = function;
     sft.t_id = ctx->backend->prepare_response_tid(req, &req_length);
@@ -935,11 +930,6 @@ int modbus_reply_exception(modbus_t *ctx, const uint8_t *req,
     int dummy_length = 99;
     sft_t sft;
-    if (ctx->backend->filter_request(ctx, slave) == 1) {
-        /* Filtered */
-        return 0;
-    }
-
     sft.slave = slave;
     sft.function = function + 0x80;;
     sft.t_id = ctx->backend->prepare_response_tid(req, &dummy_length);
@@ -120,10 +120,11 @@ int main(void)
                     modbus_set_socket(ctx, master_socket);
                     rc = modbus_receive(ctx, query);
-                    if (rc != -1) {
+                    if (rc > 0) {
                         modbus_reply(ctx, query, rc, mb_mapping);
-                    } else {
-                        /* Connection closed by the client, end of server */
+                    } else if (rc == -1) {
+                        /* This example server in ended on connection closing or
+                         * any errors. */
                         printf("Connection closed on socket %d\n", master_socket);
                         close(master_socket);
@@ -75,10 +75,10 @@ int main(int argc, char *argv[])
         uint8_t query[MODBUS_TCP_MAX_ADU_LENGTH];
         rc = modbus_receive(ctx, query);
-        if (rc >= 0) {
+        if (rc > 0) {
             modbus_reply(ctx, query, rc, mb_mapping);
-        } else {
-            /* Connection closed by the client or server */
+        } else if (rc  == -1) {
+            /* Connection closed by the client or error */
             break;
         }
     }
@@ -47,10 +47,10 @@ int main(void)
         int rc;
         rc = modbus_receive(ctx, query);
-        if (rc != -1) {
+        if (rc > 0) {
             /* rc is the query size */
             modbus_reply(ctx, query, rc, mb_mapping);
-        } else {
+        } else if (rc == -1) {
             /* Connection closed by the client or error */
             break;
         }
@@ -114,12 +114,17 @@ int main(int argc, char*argv[])
     }
     for (;;) {
-        rc = modbus_receive(ctx, query);
+        do {
+            rc = modbus_receive(ctx, query);
+            /* Filtered queries return 0 */
+        } while (rc == 0);
+
         if (rc == -1) {
             /* Connection closed by the client or error */
             break;
         }
+
         /* Read holding registers */
         if (query[header_length] == 0x03) {
             if (MODBUS_GET_INT16_FROM_INT8(query, header_length + 3)