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

CRC byte Order in Modbus RTU #397

Closed
IRQ85 opened this issue Sep 7, 2017 · 10 comments
Closed

CRC byte Order in Modbus RTU #397

IRQ85 opened this issue Sep 7, 2017 · 10 comments
Labels

Comments

@IRQ85
Copy link

IRQ85 commented Sep 7, 2017

Dear Developers,

according to the MODBUS over serial line specification and
implementation guide V1.02 p. 14, the low order byte of the CRC comes
first in the RTU message:

"The CRC field is appended to the message as the last field in the
message. When this is done, the low–order byte of the field is
appended first, followed by the high–order byte. The CRC high–order
byte is the last byte to be sent in the message."

This is unusual because with other PDU data (address, quantity...), the
high byte comes first.

If you now look in modbus-rtu.c, the byte order of the CRC is swapped
(lines 156f.):

req[req_length++] = crc >> 8;
req[req_length++] = crc & 0x00FF;

This causes no trouble as the Function crc16(...) in the same file
handles the high byte as the low byte vice versa.

If it has no deeper meaning, you might consider fixing this to clarify the code. The cited
document contains code for proper calculation of the CRC which is in
fact very similar but not completely equal. Never mind if the double swapping is intended.

With kind regards

Oliver


libmodbus version

The version is libmodbus-3.1.4.

OS and/or distribution

Environment

Description

See above.

Expected behaviour

CRC low byte comes first in message.

Actual behaviour

CRC low byte comes first in message but only because it is swapped twice.

Steps to reproduce the behavior (commands or source code)

See above

libmodbus output with debug mode enabled

@stephane
Copy link
Owner

If the CRC is swapped in req pointer that means it will be swapped too on the serial line, won't it?
In this case, why libmodbus is able to communicate with other Modbus devices!?
Does it mean that nobody respects the specs?

@OliverBehr
Copy link

Good question but I didn't say the outcome is wrong.

The CRC byte order is swapped twice so in the end it is right again. Its like making two sign errors in a calculation ;-)

What I suggest is:

  1. Swap lines 156 and 157 to match the specification:
req[req_length++] = crc & 0x00FF;
req[req_length++] = crc >> 8;
  1. Modify the function CRC16 to treat high and low byte right. Starting from line 138:
/* pass through message buffer */
while (buffer_length--) {
        i = crc_lo ^ *buffer++; /* calculate the CRC  */
        crc_lo = crc_hi ^ table_crc_hi[i];
        crc_hi = table_crc_lo[i];
}

It matches the code sample from MODBUS over Serial Line Specification and implementation Guide V1.02 then:

while (usDataLen--)  /* pass through message buffer   */ 
{ 
uIndex = uchCRCLo ^ *puchMsg++ ;   /* calculate the CRC   */ 
uchCRCLo = uchCRCHi ^ auchCRCHi[uIndex] ; 
uchCRCHi = auchCRCLo[uIndex] ; 
} 
return (uchCRCHi << 8 | uchCRCLo) ; 

@stephane
Copy link
Owner

OK I see but in this case, you need to swap CRC in handling of response too.
Could you compile and test my associated branch?
#405

@stephane stephane added valid and removed needinfo labels Oct 26, 2017
@OliverBehr
Copy link

Checked out the branch and compiled successful. Do you have a suggestion for how to test?

@stephane
Copy link
Owner

To test the changes I changed the tty device names in unit-test-server.c and unit-test-client.c and I launched them with the rtu argument to pass the test suite, but you can also test it by running the random-test-client (with a few changes on range of addresses) on a remote Modbus device.

@karlp
Copy link
Contributor

karlp commented Oct 27, 2017

please make sure this is tested against someone else's device, not just a test server and test client running from the same codebase :)

@pboettch
Copy link
Contributor

I will certainly try this change in the coming weeks on 1 slave with two different masters using RS485. I'll report back.

@stephane
Copy link
Owner

stephane commented Nov 14, 2017 via email

@pboettch
Copy link
Contributor

Not yet. It should happen during the next weeks though. Hopefully before the end of the year.

@pboettch
Copy link
Contributor

I'm using this change in my production environment (with the my server-code, not merged here) to do multi-client (with TCP) and multi-slave (with RTU) proxies. No problems so far.

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

No branches or pull requests

5 participants