stephane / libmodbus

A Modbus library for Linux, Mac OS, FreeBSD and Windows
http://libmodbus.org
GNU Lesser General Public License v2.1
3.48k stars 1.76k forks source link

Confusing about RS485 support #331

Closed AndreySV closed 7 years ago

AndreySV commented 8 years ago

Hi, Stephane, I'm facing couple of issues in RS485 support.

If I understand correctly, libmodbus has two ways to control DE signal for RS485.

  1. All signal handling is done inside device driver (in Linux kernel) and turned on by modbus_rtu_set_serial_mode(ctx, MODBUS_RTU_RS485).
  2. Signal handling done in libmodbus and turned on by calling modbus_rtu_set_rts(ctx, MODBUS_RTU_RTS_UP).

Both ways are independent of each other. Therefore it's confusing. What method should user use? I think that better would be to change API slightly:

modbus_rtu_set_rts_delay - set the Request To Send delay period (as it's done now). modbus_rtu_set_rts - set the Request To Send mode to communicate on a RS485 serial bus (as it's done now);

These two functions just set proper values for corresponding settings.

modbus_rtu_set_serial_mode will set the way how rts level and rts delay should be implemented. I expect modbus_rtu_set_serial mode to take these values as mode: MODBUS_RTU_RS232 - ignores rts value and delays, MODBUS_RTU_RS485_DRV - rts handling is done in the kernel, MODBUS_RTU_RS485_LIB - rts handling is done in libmodbus.

What do you think?

Now about the issues. modbus_rtu_set_serial_mode doesn't really work for RS485, because libmodbus doesn't initialize bit fields SER_RS485_RTS_ON_SEND, SER_RS485_RTS_AFTER_SEND. Therefore if RS485 is enabled, then RTS is low always! Additionally struct serial_rs485 has fields for handling delays before and after send. Would be nice to make use of it too. Here is detailed description how to use serial_rs485.

karlp commented 8 years ago

Just a point on clarify, libmodbus should not set SER_RS485_RTS_ON_SEND because proper RS485 mode does not use the RTS signal. Using RTS to control an external rs485 tranceiver is a workaround that largely works, but it is not what a working functional rs485 kernel driver will be using. There will be an actual pin for controlling the driver enable. Often, surprise surprise, labelled "driver enable" RTS is an RS232 signal that can be poked to end up with something like rs485 support.

Generally, I'm not sure how the _DRV and _LIB versions would actually help with user clarity. At some point, you really simply need to understand what hardware you have, and what you're attempting to achieve. The lack of widespread implementations of RS485 mode in the kernel doesn't help much of course. This is the direct cause of your claim that "rs485 doesn't do anything, because RTS stays low" What that actually means is that rs485 doesn't work for you who are expecting to use the RTS signal as rs485 DE signal. If you were using hardware that supported RS485 mode in the kernel, you would be getting the rs485 support with the existing call.

TL;DR: Does rs485 pretty much suck in linux? yes. But it's really not libmodbus's fault and IMO, the changes you suggest don't really improve clarity of the situation, they just make it differently confusing.

AndreySV commented 8 years ago

Hi Karl,

Thank you for your comments. I know originals of RTS signal. But using RTS to control RS485 transceiver's DE line is almost de facto standard. Even if DE line is handled by simple GPIO, signal named RTS for convenience. See documentation for rs485 devicetree bindings, for example. Let us not discuss further whether RTS should be used to control DE line. Let us assume it does, as it's in most cases.

I agree with you. My proposal doesn't solve the problem, that the user'll need to know what does his hardware/driver support. But it makes API more consistent. If driver doesn't handle RS485 by itself, by changing one line of code, for the user it would be easy to switch to libmodbus RS485 handling.

You told _libmodbus should not set SER_RS485_RTS_ONSEND because proper RS485 mode does not use the RTS signal. Would you please point me to the proper driver in the Linux kernel. If you read carefully documentation for TIOCSRS485, you'll see that bits SER_RS485_RTS_ON_SEND, SER_RS485_RTS_AFTER_SEND are not optional. If driver doesn't handle SER_RS485_RTS_ON_SEND, SER_RS485_RTS_AFTER_SEND it just clears and ignores they. But by not setting these bits properly libmodbus breaks all drivers that fully implement TIOCSR485 ioctl!

I've looked closer to support of RS485 in Linux kernel. 11 drivers are supporting SER_RS485_ENABLED now. Here is overview of what driver support what flags of struct serial_rs485 and whether the RS485 will work with driver, if libmodbus doesn't fill properly struct serial_rs485.

column header description
rts_on Is SER_RS485_RTS_ON_SEND supported?
rts_after Is SER_RS485_RTS_AFTER_SEND supported?
delay_before Is delay_rts_before_send supported?
delay_after Is delay_rts_after_send supported?
libmodbus Will rs485 work with libmodbus now?
after fix Will rs485 work if libmodbus'd fill the fields?
driver rts_on rts_after delay_before delay_after libmodbus after fix
omap-serial yes yes yes yes no yes
max310x no no yes yes yes yes
imx yes yes yes yes no yes
mcf no no no no yes yes
sc16is7xx no yes no no yes yes
8250 yes yes yes yes no yes
8250_omap yes yes yes yes no yes
8250_pci yes no no no yes yes
8250_lpc8xx yes no no yes yes yes
8250_fintek yes no yes yes no yes
crisv10 yes yes yes no no yes
atmel_serial no no no yes yes yes

As you see in the last column, that if libmodbus'd properly set SER_RS485_RTS_ON_SEND, SER_RS485_RTS_AFTER_SEND bits, then rs485 mode will work with all drivers in the kernel!

So far so good. But then you are going to fill properly serial_rs485 structure, libmodbus needs to know polarity of RTS signal to set the bits correct. Where this information will come from? It could be another new libmodbus function or just already existed modbus_rtu_set_rts() call.

iweindesmedt commented 7 years ago

+1 for properly filling struct serial_rs485

stephane commented 7 years ago

Could you provide a patch (and test it)? I don't have RS485 devices...

iweindesmedt commented 7 years ago

Basically, the Linux kernel does about the same with struct serial_rs485, as what libmodbus is doing manually with _modbus_rtu_ioctl_rts and the delays in _modbus_rtu_send, I think. So the user should have full control over struct serial_rs485, just as they now have over the HAVE_DECL_TIOCM_RTS logic.

I'm not on a Linux kernel myself with libmodbus, so can't test it. But something like this could be a possible solution:

See http://lxr.free-electrons.com/source/include/uapi/linux/serial.h#L116 for the struct serial_rs485 members

jcwren commented 7 years ago

I believe that modbus_rtu_set_serial_mode() should not be forcing the SER_RS485_ENABLED flag, but should AND or OR it after fetching the current state. While the SER_RS485_RTS_ON_SEND and SER_RS485_RTS_AFTER_SEND flags are workarounds for using RTS to control an external RS485 transceiver, a handful of kernel drivers preset these values (ambi_pl011.c, crisv10.c, fsl_lpuart.c, imx.c, mac310.c, omap_serial.c, sc16is7xx.c). The current ioctl() set call is wiping out any flags set by the kernel.

The following code adds fetching the existing RS485 flags before changing writing them. This was tested on a Raspberry Pi 3 with a 4.14 kernel, and with amba-pl011.c changes that are not yet in the kernel source tree to natively support RS485 on the Pi. It works nicely :) (sorry it's not a git pull request, I'm not very good at git).

int modbus_rtu_set_serial_mode(modbus_t *ctx, int mode)
{
    if (ctx == NULL) {
        errno = EINVAL;
        return -1;
    }

    if (ctx->backend->backend_type == _MODBUS_BACKEND_TYPE_RTU) {
#if HAVE_DECL_TIOCSRS485
        modbus_rtu_t *ctx_rtu = ctx->backend_data;
        struct serial_rs485 rs485conf;
        /* this memset() should be safe to remove */
        memset(&rs485conf, 0x0, sizeof(struct serial_rs485));

        if (mode == MODBUS_RTU_RS485) {
            if (ioctl(ctx->s, TIOCGRS485, &rs485conf) < 0) {
                return -1;
            }
            rs485conf.flags |= SER_RS485_ENABLED;
            if (ioctl(ctx->s, TIOCSRS485, &rs485conf) < 0) {
                return -1;
            }
            ctx_rtu->serial_mode = MODBUS_RTU_RS485;

            return 0;
        } else if (mode == MODBUS_RTU_RS232) {
            /* Turn off RS485 mode only if required */
            if (ctx_rtu->serial_mode == MODBUS_RTU_RS485) {
                /* The ioctl call is avoided because it can fail on some RS232 ports */
                if (ioctl(ctx->s, TIOCGRS485, &rs485conf) < 0) {
                    return -1;
                }
                rs485conf.flags &= ~SER_RS485_ENABLED;
                if (ioctl(ctx->s, TIOCSRS485, &rs485conf) < 0) {
                    return -1;
                }
            }
            ctx_rtu->serial_mode = MODBUS_RTU_RS232;
            return 0;
        }
#else
        if (ctx->debug) {
            fprintf(stderr, "This function isn't supported on your platform\n");
        }
        errno = ENOTSUP;
        return -1;
#endif
    }

    /* Wrong backend and invalid mode specified */
    errno = EINVAL;
    return -1;
}
AndreySV commented 7 years ago

Solution suggested by @jcwren is good and pretty straight forward. Code looks good to me.

jcwren commented 7 years ago

@stephane, ping?

stephane commented 7 years ago

@jcwren 👍 Is it correct 1c5d969f46c ? Does it cover all issues of this thread?

jcwren commented 7 years ago

I added a comment to the commit.

rs485conf.flags = SER_RS485_ENABLED;

should be

rs485conf.flags |= SER_RS485_ENABLED;
stephane commented 7 years ago

🤦‍♂️ fixed by https://github.com/stephane/libmodbus/commit/91a1d74f76c64e7b35bfb10114e1a4a6ff351656

Thank you.