Modbus specification violation

[vsaljooghi]

Dear Simon,

There is a subtle issue in modbus library that doesn't conform with modbus specification (section 2.4.1): https://www.modbus.org/docs/Modbus_over_serial_line_V1_02.pdf

The specification mentions that in case of receiving a modbus reply/response from a unit/slave/device which doesn't contain a matching/expected ID with the request, the modbus library should ignore/discard the received response and continues waiting until a response from requested ID arrives (or if not, it shall finally time out).

"When a reply is received, the Master checks the reply before starting the data processing. The checking may result in an error, for example a reply from an unexpected slave, or an error in the received frame. In case of a reply received from an unexpected slave, the Response time-out is kept running."

The current behavior of modbus library is to produce "bad unit id" error and returns immediately (without waiting for the response from the requested unit).

This is an important issue which needs to be fixed because there may be some devices on the bus that misbehave by replying to requests which are not intended for them. When modbus library returns immediately with "bad unit id" error, it will result in a domino/cascade of failures/errors for coming requests.

I looked at the code a little bit but I don't have deep knowledge to modify and test it. Maybe you can tell me if below change will solve the issue (probably there is need for a fix for tcp as well):

Note: To implement state loop of state machine, I recursively called readRTUFrame().

rtu_transport.go

// Waits for, reads and decodes a frame from the rtu link.
func (rt *rtuTransport) readRTUFrame(req *pdu) (res *pdu, err error) {
    var rxbuf   []byte
    var byteCount   int
    var bytesNeeded int
    var crc     crc

    rxbuf       = make([]byte, maxRTUFrameLength)

    // read the serial ADU header: unit id (1 byte), function code (1 byte) and
    // PDU length/exception code (1 byte)
    byteCount, err  = io.ReadFull(rt.link, rxbuf[0:3])
    if (byteCount > 0 || err == nil) && byteCount != 3 {
        err = ErrShortFrame
        return
    }
    if err != nil && err != io.ErrUnexpectedEOF {
        return
    }

    // figure out how many further bytes to read
    bytesNeeded, err = expectedResponseLenth(uint8(rxbuf[1]), uint8(rxbuf[2]))
    if err != nil {
        return
    }

    // we need to read 2 additional bytes of CRC after the payload
    bytesNeeded += 2

    // never read more than the max allowed frame length
    if byteCount + bytesNeeded > maxRTUFrameLength {
        err = ErrProtocolError
        return
    }

    byteCount, err  = io.ReadFull(rt.link, rxbuf[3:3 + bytesNeeded])
    if err != nil && err != io.ErrUnexpectedEOF {
        return
    }

    // make sure the resp unit id matches that of the req, otherwise ignore the response and wait for the real response from requested unit id
    if rxbuf[0]  != req.unitId {
        rt.logger.Warningf("ErrBadUnitId")
        res, err = rt.readRTUFrame(req)
        return
    }

    if byteCount != bytesNeeded {
        rt.logger.Warningf("expected %v bytes, received %v", bytesNeeded, byteCount)
        err = ErrShortFrame
        return
    }

    // compute the CRC on the entire frame, excluding the CRC
    crc.init()
    crc.add(rxbuf[0:3 + bytesNeeded - 2])

    // compare CRC values
    if !crc.isEqual(rxbuf[3 + bytesNeeded - 2], rxbuf[3 + bytesNeeded - 1]) {
        err = ErrBadCRC
        return
    }

    res = &pdu{
        unitId:     rxbuf[0],
        functionCode:   rxbuf[1],
        // pass the byte count + trailing data as payload, withtout the CRC
        payload:    rxbuf[2:3 + bytesNeeded  - 2],
    }

    return
}

MfG, Vahid Saljooghi