Michael Dressel - 2017-01-05

Hi,

since almost a year we see sporadic errors complained by a modbus master on some systems. We found that these occurs when changing on the slave from debian wheezy to jessie.

We tried to find what component actually does it. So on the slave I downgraded rxtx, used a different jdk but without clear result.

Switching on debugging on the slave showed an error message CRC should be ...

Now I saw in the method ModbusRTUTransport:getRequest for the function WRITE_MULTIPLE_COILS this problematic peace of code:

        case Modbus.WRITE_MULTIPLE_COILS:
        case Modbus.WRITE_MULTIPLE_REGISTERS:
            byteCount = m_InputStream.read(inpBuf, 0, 4);
            if (byteCount > 0)
                out.write(inpBuf, 0, byteCount);

            byteCount = m_InputStream.read();
            out.write(byteCount);
            readRequestData(byteCount, inpBuf, out);
            break;

The problem is that read may return with less than 4 bytes (-1, 1,2,3,4) but the code does not treat byteCount = 1,2,3 correctly.

For testing I replaced the above code with the following:

        case Modbus.WRITE_MULTIPLE_COILS:
        case Modbus.WRITE_MULTIPLE_REGISTERS:
            int remaining = 4;
            while(remaining>0) {
                byteCount = m_InputStream.read(inpBuf, 0, remaining);
                if (byteCount > 0) {
                    out.write(inpBuf, 0, byteCount);
                    remaining-=byteCount;
                } else {
                    System.err.println("Error: reading, remaining = " + remaining);
                    break;
                }
            }
            byteCount = m_InputStream.read();
            if (byteCount<0)
                System.err.println("Error: data length");

            out.write(byteCount);
            readRequestData(byteCount, inpBuf, out);
            break;

This test-code is also problematic because it does not clean things up
in the remaining error cases.

But at least for our long-lasting problem this fixes the error.

Searching for other places where read(buf, offsetm len) is used I found that at least for the slave the usage should be ok. But looking at the method getResponse shows many places where read is used wrongly.

Cheers,
Michael