Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modbus specification violation #40

Open
vsaljooghi opened this issue Oct 19, 2023 · 0 comments
Open

Modbus specification violation #40

vsaljooghi opened this issue Oct 19, 2023 · 0 comments

Comments

@vsaljooghi
Copy link

vsaljooghi commented Oct 19, 2023

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
."

grafik

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant