CRC16_Modbus_RTU

Projects Programming
1

Hello,

I'm doing a modbus RTU communication protocol...
the function for calculating the CRC I came across a strange effect.

The function returns an incorrect value and I don't know why.
The same function returns a correct value if I change the data (see BYTE nr. 9).

char test1[14]= {0x00,0x00,0x32,0x00,0x00,0x00,0x00,0x00,0x80,0x02,0x00,0x00,0x00,0x00};
char test2[14]= {0x00,0x00,0x32,0x00,0x00,0x00,0x00,0x00,0x7F,0x02,0x00,0x00,0x00,0x00};

void loop(){

Serial.print(crc_calc(test1,14), HEX);
Serial.print(crc_calc(test2,14), HEX);

}

uint16_t crc_calc(char *pcode , uint16_t len) {
uint16_t crc=0xFFFF; //Inizializzazione del registro CRC con tutti '1'
int z;
int y;
for(z=0;z < len; z++) {
crc = crc ^ (uint16_t)pcode[z];
for(y=0; y < 8; y++) {
if((crc & 0x0001) != 0) crc = (crc >> 1) ^ 0xA001;
else crc = (crc >> 1);
}
}
return crc;
}

test1 return 0x9260 and is not correct ....
the correct value for CRC-16 modbus is 0x8674 .

test2 return 0x8960 and is correct, look at the calculator at the following link:

Copy and paste one at a time next sequence of byte.
Set HEX and try.

test1) --> 00 00 32 00 00 00 00 00 80 02 00 00 00 00
test2) --> 00 00 32 00 00 00 00 00 7F 02 00 00 00 00

someone found the error?

2

Hello everyone, I have been a long time in front of this problem without unfortunately understand where is the bug. I hope someone can help me, thanks.

3

Because programming is done on linux and arduino uno, I thought I'd change the IDE controller, and also to give it a try. So I tested all about IDE Windows and Arduino Mega 2560 .... same thing, same error :frowning:

char test1[14]= {0x00,0x00,0x32,0x00,0x00,0x00,0x00,0x00,0x80,0x02,0x00,0x00,0x00,0x00};
char test2[14]= {0x00,0x00,0x32,0x00,0x00,0x00,0x00,0x00,0x7F,0x02,0x00,0x00,0x00,0x00};
unsigned long time;
unsigned long start;
bool app1;


void setup() {
  Serial.begin(115200);
  start = millis();
}

void loop() {
  time = millis()-start;
  if ( (time > 1000) ) {
    Serial.println(crc_calc(test1,14), HEX);
    Serial.println(crc_calc(test2,14), HEX);
    start = millis();  
  }
    
}

uint16_t crc_calc(char *pcode , uint16_t len) {
uint16_t crc=0xFFFF; 
int z;
int y;
for(z=0;z < len; z++) {
crc = crc ^ (uint16_t)pcode[z];
for(y=0; y < 8; y++) {
if((crc & 0x0001) != 0) crc = (crc >> 1) ^ 0xA001;
else crc = (crc >> 1);
}
}
return crc;
}

Serial Monitor:
9260
8960
9260
8960
9260
8960
9260
8960
9260
8960
9260
8960
9260
8960
9260
8960
9260
8960
9260
8960
........
4

I tried to change the function, by copying from another modbus library, but always the same ...
Still if you try another calculator https://www.lammertbies.nl/comm/info/crc-calculation.html function in arduino remains always wrong

unsigned int crc_calc(char *pcode , unsigned int len) {
unsigned int flag;
unsigned int crc=0xFFFF; 
unsigned char z;
unsigned char y;
for(z=0;z < len; z++) {
crc = crc ^ pcode[z];
for(y=1; y <= 8; y++) {
  flag = crc & 0x0001;
  crc >>= 1;
  if(flag) crc ^= 0xA001;
}
}
return crc;
}
5

The problem is in the crc function. This is the primary offending line of code.

crc = crc ^ (uint16_t)pcode[z];

Since char is signed and 'pcode' is an array of char 'pcode[z]' gets sign extended with ones if the most significant bit is a 1.

You can either mask out the top 8 bits or, as I prefer, to convert to use uint8_t instead of char throughout since then the code will be pretty much work on any OS with almost any C compiler.

I prefer the uint8_t approach as in -

uint8_t test1[14] = {0x00, 0x00, 0x32, 0x00, 0x00, 0x00, 0x00, 0x00, 0x80, 0x02, 0x00, 0x00, 0x00, 0x00};
uint8_t test2[14] = {0x00, 0x00, 0x32, 0x00, 0x00, 0x00, 0x00, 0x00, 0x7F, 0x02, 0x00, 0x00, 0x00, 0x00};

void setup()
{
  Serial.begin(115200);
  
  Serial.println(crc_calc(test1, 14), HEX);
  Serial.println(crc_calc(test2, 14), HEX);

}
void loop() {

}

uint16_t crc_calc(uint8_t *pcode , size_t len) {
  uint16_t crc = 0xFFFF; //Inizializzazione del registro CRC con tutti '1'
  int z;
  int y;
  for (z = 0; z < len; z++) {
    crc = crc ^ (uint16_t)pcode[z];
    for (y = 0; y < 8; y++) {
      if ((crc & 0x0001) != 0) crc = (crc >> 1) ^ 0xA001;
      else crc = (crc >> 1);
    }
  }
  return crc;
}

P.S. Please make sure you post code that can be run by the members of this forum. Your code was missing the setup() method and uses print() but should use println() .

6

Hi stowite, Congrats you nailed it :slight_smile:
Although I do not understand why char should have the sign,
I think this is a big deal for those of you who want to implement modbus in arduino.
I'm surprised that those who wrote the modbus library does not have noticed this problem.
If you go into a library like this simplemodbusng/SimpleModbusMaster.cpp at master · angeloc/simplemodbusng · GitHub
you will find that the function has the same mistake of my error...
thanks anyway you relieved by this that cost me many many hours of work .

7

elsabz:
I'm surprised that those who wrote the modbus library does not have noticed this problem.
If you go into a library like this simplemodbusng/SimpleModbusMaster/SimpleModbusMaster.cpp at master · angeloc/simplemodbusng · GitHub
you will find that the function has the same mistake of my error...

As far as I can see that code specifies 'unsigned char' and not just 'char' so it will not have the problem!

8

Yes you're right, I had it under my nose and I didn't notice ... Thanks again