New API for set/get response and byte timeouts

- update documentation
- 5 new tests
- updated NEWS file
- avoid include of time.h

NEWS

@@ -1,3 +1,23 @@
+libmodbus 3.1.2 (2013-XX-XX)
+============================
+
+This release introduces API changes on modbus_get_byte_timeout,
+modbus_get_response_timeout, modbus_set_byte_timeout,
+modbus_set_response_timeout to ease writing of language bindings.
+
+- New API to set/get response and byte timeouts.
+  New unit tests and updated documentation.
+- Export Modbus function codes supported by libmodbus
+- Fix bandwidth-server-one (closes #152)
+- Check debug flag in RTU code
+- Sync packaging with official from Debian (closes #134)
+- Remove warnings caused by shadowed 'index' variable.
+  Thanks to Åke Forslund.
+- Use accept4 in TCP PI if available
+- Add documentation for tcp[_pi]_accept (closes #31)
+- Fix mistake in modbus_tcp_listen documentation
+- Add documentation for modbus_tcp_pi_listen
+
 libmodbus 3.1.1 (2013-10-06)
 ============================
 

doc/modbus_get_byte_timeout.txt

@@ -9,28 +9,31 @@ modbus_get_byte_timeout - get timeout between bytes
 
 SYNOPSIS
 --------
-*int modbus_get_byte_timeout(modbus_t *'ctx', struct timeval *'timeout');*
+*int modbus_get_byte_timeout(modbus_t *'ctx', long *'to_sec', long *'to_usec');*
 
 
 DESCRIPTION
 -----------
 The _modbus_get_byte_timeout()_ function shall store the timeout interval
-between two consecutive bytes of the same message in the 'timeout' argument.
+between two consecutive bytes of the same message in the _to_sec_ and _to_usec_
+arguments.
 
 
 RETURN VALUE
 ------------
-The function shall return 0 if successful. Otherwise it shall return -1 and set errno.
+The function shall return 0 if successful. Otherwise it shall return -1 and set
+errno.
 
 
 EXAMPLE
 -------
 [source,c]
 -------------------
-struct timeval byte_timeout;
+long to_sec;
+long to_usec;
 
 /* Save original timeout */
-modbus_get_byte_timeout(ctx, &byte_timeout);
+modbus_get_byte_timeout(ctx, &to_sec, &to_usec);
 -------------------
 
 

doc/modbus_get_response_timeout.txt

@@ -9,34 +9,33 @@ modbus_get_response_timeout - get timeout for response
 
 SYNOPSIS
 --------
-*int modbus_get_response_timeout(modbus_t *'ctx', struct timeval *'timeout');*
+*int modbus_get_response_timeout(modbus_t *'ctx', long *'to_sec', long *'to_usec');*
 
 
 DESCRIPTION
 -----------
-The _modbus_get_response_timeout()_ function shall store the timeout interval
-used to wait for a response in the 'timeout' argument.
+The _modbus_get_response_timeout()_ function shall return the timeout interval
+used to wait for a response in the _to_sec_ and _to_usec_ arguments.
 
 
 RETURN VALUE
 ------------
-The function shall return 0 if successful. Otherwise it shall return -1 and set errno.
+The function shall return 0 if successful. Otherwise it shall return -1 and set
+errno.
 
 
 EXAMPLE
 -------
 [source,c]
 -------------------
-struct timeval old_response_timeout;
-struct timeval response_timeout;
+long old_response_to_sec;
+long old_response_to_usec;
 
 /* Save original timeout */
-modbus_get_response_timeout(ctx, &old_response_timeout);
+modbus_get_response_timeout(ctx, &old_response_to_sec, &old_response_to_usec);
 
 /* Define a new and too short timeout! */
-response_timeout.tv_sec = 0;
-response_timeout.tv_usec = 0;
-modbus_set_response_timeout(ctx, &response_timeout);
+modbus_set_response_timeout(ctx, 0, 0);
 -------------------
 
 

doc/modbus_set_byte_timeout.txt

@@ -9,23 +9,35 @@ modbus_set_byte_timeout - set timeout between bytes
 
 SYNOPSIS
 --------
-*void modbus_set_byte_timeout(modbus_t *'ctx', struct timeval *'timeout');*
+*void modbus_set_byte_timeout(modbus_t *'ctx', long 'to_sec', long 'to_usec');*
 
 
 DESCRIPTION
 -----------
 The _modbus_set_byte_timeout()_ function shall set the timeout interval between
-two consecutive bytes of the same message. If the delay between is longer than
-the given timeout, an error will be raised.
+two consecutive bytes of the same message. If the delay between bytes is longer
+than the given timeout, the 'ETIMEDOUT' error will be raised by the the function
+waiting for a response.
 
-If the timeout value has a tv_sec of -1 then this timeout will not be used at
-all. This results in modbus_set_response_timeout governing the entire timeout
-duration of an operation.
+The value of _to_usec_ argument must be in the range 0 to 999999.
+
+If _to_sec_ is set to -1 then this timeout will not be used at all. In this
+case, _modbus_set_response_timeout()_ governs the entire handling of the
+response, the full confirmation response must be received before expiration of
+the response timeout. When a byte timeout is set, the response timeout is only
+used to wait for the first byte of the respone.
 
 
 RETURN VALUE
 ------------
-The function shall return 0 if successful. Otherwise it shall return -1 and set errno.
+The function shall return 0 if successful. Otherwise it shall return -1 and set
+errno.
+
+
+ERRORS
+------
+*EINVAL*::
+The argument _ctx_ is NULL or _to_usec_ is not smaller than 1000000.
 
 
 SEE ALSO

doc/modbus_set_response_timeout.txt

@@ -9,35 +9,47 @@ modbus_set_response_timeout - set timeout for response
 
 SYNOPSIS
 --------
-*int modbus_set_response_timeout(modbus_t *'ctx', struct timeval *'timeout');*
+*int modbus_set_response_timeout(modbus_t *'ctx', long 'to_sec', long 'to_usec');*
 
 
 DESCRIPTION
 -----------
+
 The _modbus_set_response_timeout()_ function shall set the timeout interval used
-to wait for a response. If the waiting before receiving the response is longer than
-the given timeout, an error will be raised.
+to wait for a response. When a byte timeout is set, if the waiting before
+receiving the first byte of response is longer than the given timeout, the
+'ETIMEDOUT' error will be raised by the function waiting for a response. When
+byte timeout is disabled, the full confirmation response must be received before
+expiration of the response timeout.
+
+The value of to_usec argument must be in the range 0 to 999999.
 
 
 RETURN VALUE
 ------------
-The function shall return 0 if successful. Otherwise it shall return -1 and set errno.
+The function shall return 0 if successful. Otherwise it shall return -1 and set
+errno.
+
+
+ERRORS
+------
+*EINVAL*::
+The argument _ctx_ is NULL or _to_sec_/_to_usec_ aren't equal or greater than 0 or
+_to_usec_ is not smaller than 1000000.
 
 
 EXAMPLE
 -------
 [source,c]
 -------------------
-struct timeval old_response_timeout;
-struct timeval response_timeout;
+long old_response_to_sec;
+long old_response_to_usec;
 
 /* Save original timeout */
-modbus_get_response_timeout(ctx, &old_response_timeout);
+modbus_get_response_timeout(ctx, &old_response_to_sec, &old_response_to_usec);
 
 /* Define a new and too short timeout! */
-response_timeout.tv_sec = 0;
-response_timeout.tv_usec = 0;
-modbus_set_response_timeout(ctx, &response_timeout);
+modbus_set_response_timeout(ctx, 0, 0);
 -------------------
 
 

src/modbus.c

@@ -104,6 +104,7 @@ void _error_print(modbus_t *ctx, const char *context)
 
 static void _sleep_response_timeout(modbus_t *ctx)
 {
+    /* Response timeout is always positive */
 #ifdef _WIN32
     /* usleep doesn't exist on Windows */
     Sleep((ctx->response_timeout.tv_sec * 1000) +
@@ -112,10 +113,10 @@ static void _sleep_response_timeout(modbus_t *ctx)
     /* usleep source code */
     struct timespec request, remaining;
     request.tv_sec = ctx->response_timeout.tv_sec;
-    request.tv_nsec = ((long int)ctx->response_timeout.tv_usec % 1000000)
-        * 1000;
-    while (nanosleep(&request, &remaining) == -1 && errno == EINTR)
+    request.tv_nsec = ((long int)ctx->response_timeout.tv_usec) * 1000;
+    while (nanosleep(&request, &remaining) == -1 && errno == EINTR) {
         request = remaining;
+    }
 #endif
 }
 
@@ -459,7 +460,7 @@ int _modbus_receive_msg(modbus_t *ctx, uint8_t *msg, msg_type_t msg_type)
             }
         }
 
-        if (length_to_read > 0 && ctx->byte_timeout.tv_sec != -1) {
+        if (length_to_read > 0 && ctx->byte_timeout.tv_sec >= 0 && ctx->byte_timeout.tv_usec >= 0) {
             /* If there is no character in the buffer, the allowed timeout
                interval between two consecutive bytes is defined by
                byte_timeout */
@@ -467,6 +468,8 @@ int _modbus_receive_msg(modbus_t *ctx, uint8_t *msg, msg_type_t msg_type)
             tv.tv_usec = ctx->byte_timeout.tv_usec;
             p_tv = &tv;
         }
+        /* else timeout isn't set again, the full response must be read before
+           expiration of response timeout (for CONFIRMATION only) */
     }
 
     if (ctx->debug)
@@ -1639,48 +1642,54 @@ int modbus_get_socket(modbus_t *ctx)
 }
 
 /* Get the timeout interval used to wait for a response */
-int modbus_get_response_timeout(modbus_t *ctx, struct timeval *timeout)
+int modbus_get_response_timeout(modbus_t *ctx, long *to_sec, long *to_usec)
 {
     if (ctx == NULL) {
         errno = EINVAL;
         return -1;
     }
 
-    *timeout = ctx->response_timeout;
+    *to_sec = ctx->response_timeout.tv_sec;
+    *to_usec = ctx->response_timeout.tv_usec;
     return 0;
 }
 
-int modbus_set_response_timeout(modbus_t *ctx, const struct timeval *timeout)
+int modbus_set_response_timeout(modbus_t *ctx, long to_sec, long to_usec)
 {
-    if (ctx == NULL) {
+    if (ctx == NULL ||
+        to_sec < 0 || to_usec < 0 || to_usec > 999999) {
         errno = EINVAL;
         return -1;
     }
 
-    ctx->response_timeout = *timeout;
+    ctx->response_timeout.tv_sec = to_sec;
+    ctx->response_timeout.tv_usec = to_usec;
     return 0;
 }
 
 /* Get the timeout interval between two consecutive bytes of a message */
-int modbus_get_byte_timeout(modbus_t *ctx, struct timeval *timeout)
+int modbus_get_byte_timeout(modbus_t *ctx, long *to_sec, long *to_usec)
 {
     if (ctx == NULL) {
         errno = EINVAL;
         return -1;
     }
 
-    *timeout = ctx->byte_timeout;
+    *to_sec = ctx->byte_timeout.tv_sec;
+    *to_usec = ctx->byte_timeout.tv_usec;
     return 0;
 }
 
-int modbus_set_byte_timeout(modbus_t *ctx, const struct timeval *timeout)
+int modbus_set_byte_timeout(modbus_t *ctx, long to_sec, long to_usec)
 {
-    if (ctx == NULL) {
+    /* Byte timeout can be disabled with negative values */
+    if (ctx == NULL || to_usec > 999999) {
         errno = EINVAL;
         return -1;
     }
 
-    ctx->byte_timeout = *timeout;
+    ctx->byte_timeout.tv_sec = to_sec;
+    ctx->byte_timeout.tv_usec = to_usec;
     return 0;
 }
 

src/modbus.h

@@ -26,10 +26,8 @@
 
 #ifndef _MSC_VER
 #include <stdint.h>
-#include <sys/time.h>
 #else
 #include "stdint.h"
-#include <time.h>
 #endif
 
 #include "modbus-version.h"
@@ -174,11 +172,11 @@ MODBUS_API int modbus_set_error_recovery(modbus_t *ctx, modbus_error_recovery_mo
 MODBUS_API int modbus_set_socket(modbus_t *ctx, int s);
 MODBUS_API int modbus_get_socket(modbus_t *ctx);
 
-MODBUS_API int modbus_get_response_timeout(modbus_t *ctx, struct timeval *timeout);
-MODBUS_API int modbus_set_response_timeout(modbus_t *ctx, const struct timeval *timeout);
+MODBUS_API int modbus_get_response_timeout(modbus_t *ctx, long *to_sec, long *to_usec);
+MODBUS_API int modbus_set_response_timeout(modbus_t *ctx, long to_sec, long to_usec);
 
-MODBUS_API int modbus_get_byte_timeout(modbus_t *ctx, struct timeval *timeout);
-MODBUS_API int modbus_set_byte_timeout(modbus_t *ctx, const struct timeval *timeout);
+MODBUS_API int modbus_get_byte_timeout(modbus_t *ctx, long *to_sec, long *to_usec);
+MODBUS_API int modbus_set_byte_timeout(modbus_t *ctx, long to_sec, long to_usec);
 
 MODBUS_API int modbus_get_header_length(modbus_t *ctx);
 
@@ -202,8 +200,8 @@ MODBUS_API int modbus_write_bits(modbus_t *ctx, int addr, int nb, const uint8_t
 MODBUS_API int modbus_write_registers(modbus_t *ctx, int addr, int nb, const uint16_t *data);
 MODBUS_API int modbus_mask_write_register(modbus_t *ctx, int addr, uint16_t and_mask, uint16_t or_mask);
 MODBUS_API int modbus_write_and_read_registers(modbus_t *ctx, int write_addr, int write_nb,
-                                           const uint16_t *src, int read_addr, int read_nb,
-                                           uint16_t *dest);
+                                               const uint16_t *src, int read_addr, int read_nb,
+                                               uint16_t *dest);
 MODBUS_API int modbus_report_slave_id(modbus_t *ctx, uint8_t *dest);
 
 MODBUS_API modbus_mapping_t* modbus_mapping_new(int nb_bits, int nb_input_bits,
@@ -218,9 +216,9 @@ MODBUS_API int modbus_receive_from(modbus_t *ctx, int sockfd, uint8_t *req);
 MODBUS_API int modbus_receive_confirmation(modbus_t *ctx, uint8_t *rsp);
 
 MODBUS_API int modbus_reply(modbus_t *ctx, const uint8_t *req,
-                        int req_length, modbus_mapping_t *mb_mapping);
+                            int req_length, modbus_mapping_t *mb_mapping);
 MODBUS_API int modbus_reply_exception(modbus_t *ctx, const uint8_t *req,
-                                  unsigned int exception_code);
+                                      unsigned int exception_code);
 
 /**
  * UTILS FUNCTIONS

tests/unit-test-client.c

@@ -44,8 +44,8 @@ int main(int argc, char *argv[])
     int rc;
     float real;
     uint32_t ireal;
-    struct timeval old_response_timeout;
-    struct timeval response_timeout;
+    long old_response_timeout_sec;
+    long old_response_timeout_usec;
     int use_backend;
 
     if (argc > 1) {
@@ -647,27 +647,82 @@ int main(int argc, char *argv[])
     }
 
     /* Save original timeout */
-    modbus_get_response_timeout(ctx, &old_response_timeout);
+    modbus_get_response_timeout(ctx, &old_response_timeout_sec, &old_response_timeout_usec);
 
-    /* Define a new and too short timeout */
-    response_timeout.tv_sec = 0;
-    response_timeout.tv_usec = 0;
-    modbus_set_response_timeout(ctx, &response_timeout);
+    rc = modbus_set_response_timeout(ctx, -1, 0);
+    printf("1/6 Invalid response timeout (negative): ");
+    if (rc == -1 && errno == EINVAL) {
+        printf("OK\n");
+    } else {
+        printf("FAILED\n");
+        goto close;
+    }
+
+    rc = modbus_set_response_timeout(ctx, 0, 1000000);
+    printf("2/6 Invalid response timeout (too large): ");
+    if (rc == -1 && errno == EINVAL) {
+        printf("OK\n");
+    } else {
+        printf("FAILED\n");
+        goto close;
+    }
 
+    rc = modbus_set_byte_timeout(ctx, 0, 1000000);
+    printf("3/6 Invalid byte timeout (too large): ");
+    if (rc == -1 && errno == EINVAL) {
+        printf("OK\n");
+    } else {
+        printf("FAILED\n");
+        goto close;
+    }
+
+    modbus_set_response_timeout(ctx, 0, 0);
     rc = modbus_read_registers(ctx, UT_REGISTERS_ADDRESS,
                                UT_REGISTERS_NB, tab_rp_registers);
-    printf("4/4 Too short timeout: ");
+    printf("4/6 Zero response timeout: ");
     if (rc == -1 && errno == ETIMEDOUT) {
         printf("OK\n");
     } else {
         printf("FAILED (can fail on slow systems or Windows)\n");
     }
 
-    /* Restore original timeout */
-    modbus_set_response_timeout(ctx, &old_response_timeout);
-
     /* A wait and flush operation is done by the error recovery code of
-     * libmodbus */
+     * libmodbus but after a sleep of current response timeout
+     * so 0 can't be too short!
+     */
+    usleep(old_response_timeout_sec * 1000000 + old_response_timeout_usec);
+    modbus_flush(ctx);
+
+    /* Trigger a special behaviour on server to wait for 0.5 second before
+     * replying whereas allowed timeout is 0.2 second */
+    modbus_set_response_timeout(ctx, 0, 200000);
+    rc = modbus_read_registers(ctx, UT_REGISTERS_ADDRESS_SLEEP_500_MS,
+                               1, tab_rp_registers);
+    printf("5/6 Too short response timeout (0.2s < 0.5s): ");
+    if (rc == -1 && errno == ETIMEDOUT) {
+        printf("OK\n");
+    } else {
+        printf("FAILED\n");
+        goto close;
+    }
+
+    /* Wait for reply (0.2 + 0.4 > 0.5 s) and flush before continue */
+    usleep(400000);
+    modbus_flush(ctx);
+
+    modbus_set_response_timeout(ctx, 0, 600000);
+    rc = modbus_read_registers(ctx, UT_REGISTERS_ADDRESS_SLEEP_500_MS,
+                               1, tab_rp_registers);
+    printf("6/6 Adequate response timeout (0.6s > 0.5s): ");
+    if (rc == 1) {
+        printf("OK\n");
+    } else {
+        printf("FAILED\n");
+        goto close;
+    }
+
+    /* Restore original timeout */
+    modbus_set_response_timeout(ctx, old_response_timeout_sec, old_response_timeout_usec);
 
     /** BAD RESPONSE **/
     printf("\nTEST BAD RESPONSE ERROR:\n");
@@ -675,6 +730,7 @@ int main(int argc, char *argv[])
     /* Allocate only the required space */
     tab_rp_registers_bad = (uint16_t *) malloc(
         UT_REGISTERS_NB_SPECIAL * sizeof(uint16_t));
+
     rc = modbus_read_registers(ctx, UT_REGISTERS_ADDRESS,
                                UT_REGISTERS_NB_SPECIAL, tab_rp_registers_bad);
     printf("* modbus_read_registers: ");
@@ -684,14 +740,13 @@ int main(int argc, char *argv[])
         printf("FAILED\n");
         goto close;
     }
-
     free(tab_rp_registers_bad);
 
     /** MANUAL EXCEPTION **/
     printf("\nTEST MANUAL EXCEPTION:\n");
-
     rc = modbus_read_registers(ctx, UT_REGISTERS_ADDRESS_SPECIAL,
                                UT_REGISTERS_NB, tab_rp_registers);
+
     printf("* modbus_read_registers at special address: ");
     if (rc == -1 && errno == EMBXSBUSY) {
         printf("OK\n");

tests/unit-test-server.c

@@ -175,6 +175,10 @@ int main(int argc, char*argv[])
                 printf("Reply with an invalid TID or slave\n");
                 modbus_send_raw_request(ctx, raw_req, RAW_REQ_LENGTH * sizeof(uint8_t));
                 continue;
+            } else if (MODBUS_GET_INT16_FROM_INT8(query, header_length + 1)
+                       == UT_REGISTERS_ADDRESS_SLEEP_500_MS) {
+                printf("Sleep 0.5 s before replying\n");
+                usleep(500000);
             }
         }
 

tests/unit-test.h.in

@@ -50,6 +50,8 @@ const uint16_t UT_REGISTERS_ADDRESS = 0x16B;
 const uint16_t UT_REGISTERS_ADDRESS_SPECIAL = 0x6C;
 /* The response of the server will contains an invalid TID or slave */
 const uint16_t UT_REGISTERS_ADDRESS_INVALID_TID_OR_SLAVE = 0x6D;
+/* The server will wait for 1 second before replying to test timeout */
+const uint16_t UT_REGISTERS_ADDRESS_SLEEP_500_MS = 0x6E;
 
 const uint16_t UT_REGISTERS_NB = 0x3;
 const uint16_t UT_REGISTERS_TAB[] = { 0x022B, 0x0001, 0x0064 };