Review of RTS flow control code

- avoid export of _modbus_rtu_set_rts and change its name to avoid confusion - namespace RTU functions and constants - use_rts is renamed rts - add missing '_' in function name - remove useless debug message in usual write - wrong message about 'not supported' - add check for TIOCM_RTS support - block RTS flow control when not available

Review of RTS flow control code
- avoid export of _modbus_rtu_set_rts and change its name to avoid confusion - namespace RTU functions and constants - use_rts is renamed rts - add missing '_' in function name - remove useless debug message in usual write - wrong message about 'not supported' - add check for TIOCM_RTS support - block RTS flow control when not available
Stéphane Raimbault
1 parent 88b2eaf3
Showing 4 changed files with 51 additions and 53 deletions
configure.ac
src/modbus-rtu-private.h
src/modbus-rtu.c
src/modbus-rtu.h
@@ -129,6 +129,8 @@ fi
 # Check for RS485 support (Linux kernel version 2.6.28+)
 AC_CHECK_DECLS([TIOCSRS485], [], [], [[#include <sys/ioctl.h>]])
+# Check for RTS flags
+AC_CHECK_DECLS([TIOCM_RTS], [], [], [[#include <sys/ioctl.h>]])
 AC_CONFIG_FILES([
         Makefile
@@ -37,6 +37,10 @@
 #define _MODBUS_RTU_CHECKSUM_LENGTH    2
+/* Time waited beetween the RTS switch before transmit data or after transmit
+   data before to read */
+#define _MODBUS_RTU_TIME_BETWEEN_RTS_SWITCH 10000
+
 #if defined(_WIN32)
 #define ENOTSUP WSAEOPNOTSUPP
@@ -80,12 +84,8 @@ typedef struct _modbus_rtu {
 #endif
 #if HAVE_DECL_TIOCSRS485
     int serial_mode;
-    int use_rts;
+    int rts;
 #endif
 } modbus_rtu_t;
-/* Time waited beetween the RTS switch before transmit data or after transmit data before to read */
-#define TIME_BETWEEN_RTS_SWITCH  10000
-void _modbus_rtu_setrts(int fd, int on);
-
 #endif /* _MODBUS_RTU_PRIVATE_H_ */
@@ -257,6 +257,21 @@ static int win32_ser_read(struct win32_ser *ws, uint8_t *p_msg,
 }
 #endif
+void _modbus_rtu_ioctl_rts(int fd, int on)
+{
+#if HAVE_DECL_TIOCM_RTS
+    int flags;
+
+    ioctl(fd, TIOCMGET, &flags);
+    if (on) {
+        flags |= TIOCM_RTS;
+    } else {
+        flags &= ~TIOCM_RTS;
+    }
+    ioctl(fd, TIOCMSET, &flags);
+#endif
+}
+
 ssize_t _modbus_rtu_send(modbus_t *ctx, const uint8_t *req, int req_length)
 {
 #if defined(_WIN32)
@@ -265,25 +280,23 @@ ssize_t _modbus_rtu_send(modbus_t *ctx, const uint8_t *req, int req_length)
     return (WriteFile(ctx_rtu->w_ser.fd, req, req_length, &n_bytes, NULL)) ? n_bytes : -1;
 #else
     modbus_rtu_t *ctx_rtu = ctx->backend_data;
-    if (ctx_rtu->use_rts != MODBUS_RTS_NONE) {
+    if (ctx_rtu->rts != MODBUS_RTU_RTS_NONE) {
+        ssize_t size;
+
         if (ctx->debug) {
-            fprintf(stderr, "sending request using RTS signal\n");
+            fprintf(stderr, "Sending request using RTS signal\n");
         }
-        ssize_t size;
-
-        _modbus_rtu_setrts(ctx->s, (ctx_rtu->use_rts == MODBUS_RTS_UP ? 1 : 0));
-        usleep(TIME_BETWEEN_RTS_SWITCH);
+        _modbus_rtu_ioctl_rts(ctx->s, ctx_rtu->rts == MODBUS_RTU_RTS_UP);
+        usleep(_MODBUS_RTU_TIME_BETWEEN_RTS_SWITCH);
         size = write(ctx->s, req, req_length);
-        usleep(TIME_BETWEEN_RTS_SWITCH);
-        _modbus_rtu_setrts(ctx->s, (ctx_rtu->use_rts == MODBUS_RTS_UP ? 0 : 1));
+
+        usleep(_MODBUS_RTU_TIME_BETWEEN_RTS_SWITCH);
+        _modbus_rtu_ioctl_rts(ctx->s, ctx_rtu->rts != MODBUS_RTU_RTS_UP);
         return size;
     } else {
-        if (ctx->debug) {
-            fprintf(stderr, "sending request without RTS signal\n");
-        }
         return write(ctx->s, req, req_length);
     }
 #endif
@@ -726,7 +739,7 @@ static int _modbus_rtu_connect(modbus_t *ctx)
     ctx_rtu->serial_mode = MODBUS_RTU_RS232;
     /* The RTS use has been set by default */
-    ctx_rtu->use_rts = MODBUS_RTS_NONE;
+    ctx_rtu->rts = MODBUS_RTU_RTS_NONE;
 #endif
@@ -789,35 +802,32 @@ int modbus_rtu_get_serial_mode(modbus_t *ctx) {
     }
 }
-int modbus_rtu_set_use_rts(modbus_t *ctx, int mode)
+int modbus_rtu_set_rts(modbus_t *ctx, int mode)
 {
+#if HAVE_DECL_TIOCM_RTS
     if (ctx->backend->backend_type == _MODBUS_BACKEND_TYPE_RTU) {
         modbus_rtu_t *ctx_rtu = ctx->backend_data;
-        
-        if (mode == MODBUS_RTS_NONE || mode == MODBUS_RTS_UP || mode == MODBUS_RTS_DOWN) {
-            ctx_rtu->use_rts = mode;
-            // Set the RTS bit in order to not reserve the RS485 bus
-            _modbus_rtu_setrts(ctx->s, (ctx_rtu->use_rts == MODBUS_RTS_UP ? 0 : 1));
-            return 0;
-        }
+        if (mode == MODBUS_RTU_RTS_NONE || mode == MODBUS_RTU_RTS_UP ||
+            mode == MODBUS_RTU_RTS_DOWN) {
+            ctx_rtu->rts = mode;
-        if (ctx->debug) {
-            fprintf(stderr, "This function isn't supported on your platform\n");
+            /* Set the RTS bit in order to not reserve the RS485 bus */
+            _modbus_rtu_ioctl_rts(ctx->s, ctx_rtu->rts != MODBUS_RTU_RTS_UP);
+
+            return 0;
         }
-        errno = ENOTSUP;
-        return -1;
     }
-
-    /* Wrong backend and invalid mode specified */
+#endif
+    /* Wrong backend or invalid mode specified */
     errno = EINVAL;
     return -1;
 }
-int modbus_rtu_get_use_rts(modbus_t *ctx) {
+int modbus_rtu_get_rts(modbus_t *ctx) {
     if (ctx->backend->backend_type == _MODBUS_BACKEND_TYPE_RTU) {
         modbus_rtu_t *ctx_rtu = ctx->backend_data;
-        return ctx_rtu->use_rts;
+        return ctx_rtu->rts;
     } else {
         errno = EINVAL;
         return -1;
@@ -975,17 +985,3 @@ modbus_t* modbus_new_rtu(const char *device,
     return ctx;
 }
-
-void _modbus_rtu_setrts(int fd, int on)
-{
-    int controlbits;
-
-    ioctl(fd, TIOCMGET, &controlbits);
-    if (on) {
-        controlbits |= TIOCM_RTS;
-    } else {
-      controlbits &= ~TIOCM_RTS;
-    }
-    ioctl(fd, TIOCMSET, &controlbits);
-}
-
@@ -32,14 +32,14 @@ modbus_t* modbus_new_rtu(const char *device, int baud, char parity,
 #define MODBUS_RTU_RS232 0
 #define MODBUS_RTU_RS485 1
-#define MODBUS_RTS_NONE   0
-#define MODBUS_RTS_UP     1
-#define MODBUS_RTS_DOWN   2
-
 int modbus_rtu_set_serial_mode(modbus_t *ctx, int mode);
 int modbus_rtu_get_serial_mode(modbus_t *ctx);
-int modbus_rtu_set_use_rts(modbus_t *ctx, int mode);
-int modbus_rtu_get_use_rts(modbus_t *ctx);
+#define MODBUS_RTU_RTS_NONE   0
+#define MODBUS_RTU_RTS_UP     1
+#define MODBUS_RTU_RTS_DOWN   2
+
+int modbus_rtu_set_rts(modbus_t *ctx, int mode);
+int modbus_rtu_get_rts(modbus_t *ctx);
 #endif /* _MODBUS_RTU_H_ */