What input should I give to crc16 Method

Question

This my Mod bus crc_16 embedded code. I have run this code in code block many Times. There is no error but I am not getting actual crc value. I should get crc 05C8 and I am getting 8512 right now.

I think I giving wrong input while calling CRC method. I am passing string and its length. So please help me.

    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    #define POLY 0x8005
    unsigned int ModRTU_CRC(unsigned char * mod_data,unsigned int length)
    {
    unsigned int CRC16 = 0xFFFF;

    unsigned int pos=0,i=0;

    for(pos=0;pos<length;pos++)
    {
    CRC16 ^= (unsigned int) mod_data[pos];

    for(i=0;i<8;i++)
    {
        if((CRC16 & 0x0001)!=0)
        {
            CRC16 >>=1;
            CRC16^=0xA001;
        }
        else
        {
            CRC16 >>=1;
        }
    }

    }

   return CRC16;

    }

   int main()
   {

    //char *frame = "010600081388";
   //  char *frame = "010300080001";

   char frame[7];
        frame[0]=0x01;
       frame[1]=0x03;
       frame[2]=0x00;
       frame[3]=0x08;
       frame[4]=0x00;
       frame[5]=0x01;
       frame[6]='\0';


     printf("%x\n",frame);
     int len = strlen(frame);
     unsigned int  crcv = ModRTU_CRC(frame,len);
    printf("%x\n",crcv);
   return 0;
       }

`frame[6]='\0'`: ouch: buffer overrun! your buffer is too short. may be overwritten just when calling `strlen` and get a different length as a result... — Jean-François Fabre, Jul 05 '17 at 11:02
How many elements are there in your array `frame`? What is the last valid index of that array? Note that you don't *need* to terminate it since it's not really a string, but an array of arbitrary binary data. Use `sizeof frame` to get the size of the array. — Some programmer dude, Jul 05 '17 at 11:02
Also, none of the "characters" in the array are printable, so printing it will not work. Furthermore, if you want to print the `crcv` variable as hex then use the `"%x"` format. — Some programmer dude, Jul 05 '17 at 11:02
@ Jean-François Fabre even i if i remove that or increase array size i am geting wrong crc value — Shrikant, Jul 05 '17 at 11:04
@Shrikant agreed. But fix it anyway. it's wrong. Are you sure you're using the proper algorithm? the proper init value (-1 vs 0) ? finding the proper CRC is always difficult. — Jean-François Fabre, Jul 05 '17 at 11:04
Lastly I recommend you take some time to read [How to debug small programs](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/) by Eric Lippert. — Some programmer dude, Jul 05 '17 at 11:06
You don't pass a string to this function, you pass raw bytes and number of them in the buffer. As for your test - doing `frame[i]=0x00;` is equivalent to `frame[i]='\0';`. Also, `frame[6]='\0';` does a write outside specified buffer. Also, test data "010300080001" if treated as bytes (0x01, 0x03 etc.) results in Modbus CRC equal to 0xC805. If treated as ASCII it's 0x49D2. Neither of those values is 0x05C8, nor 0x8512. — J_S, Jul 05 '17 at 11:07
@ Jacek Ślimok ok ,then i should convert my string to bytes first and then pass to method? — Shrikant, Jul 05 '17 at 11:12
@Shrikant I don't know what you mean by "should" or "should not". This function accepts a buffer and its length and calculates CRC of it, in binary form. If you calculate CRC of something you read (data transmission received, file read etc.) then you take the data as-is without any additional conversions. Put it to the buffer, pass it to the function. However, if all you have is `010300080001;` then it is quite unclear how the data should be treated - as binary or as string. I would assume as binary, given the contents but again, it is just an assumption. — J_S, Jul 05 '17 at 11:30
You aren't using the CRC16-IBM polynomial 0x8005 anywhere in your code, which makes it fishy. Sure 0xA001 is 0x8005 inverted but then why `#define` the polynomial? If I remember correctly, I believe that this algorithm should also invert the data buffer after the calculation. — Lundin, Jul 05 '17 at 11:42

score 3 · Answer 1 · answered Jul 05 '17 at 12:38

The bug is that in your original code you used frame[2]=0x00; followed by strlen(), which will interpret this as the null terminator and give the wrong size 2 instead of 6. You cannot use strlen() on binary data.

In the "fixed" version you hard-code the size to 6 so the bug was removed by accident. It has nothing to do with uint8_t versus char (although it is always better to use uint8_t for binary data).

score 0 · Answer 2 · answered Jul 05 '17 at 11:40

0

#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#define POLY 0x8005
unsigned int ModRTU_CRC(unsigned char * mod_data,unsigned int length)
{
unsigned int CRC16 = 0xFFFF;

unsigned int pos=0,i=0;

for(pos=0;pos<length;pos++)
{
    CRC16 ^= (unsigned int) mod_data[pos];

    for(i=0;i<8;i++)
    {
        if((CRC16 & 0x0001)!=0)
        {
            CRC16 >>=1;
            CRC16^=0xA001;
        }
        else
        {
            CRC16 >>=1;
        }
    }

  }

  return CRC16;

 }

 int main()
 {



        uint8_t message[80] = { // 6-byte test vector
  0x01, 0x03, 0x00, 0x08, 0x00, 0x01
};
int message_length = 6;
 printf("%x\n",message);
// int len = strlen(message);
 uint16_t the_CRC = ModRTU_CRC(message,message_length);
 printf("%x\n",the_CRC);
        return 0;
   }


 `

answered Jul 05 '17 at 11:40

Shrikant

1
1

1

Why did you post this as an answer? Is it a solution? What's different from the question? If you wish to add details to the question please edit it. – Lundin Jul 05 '17 at 11:43
@ Lundin yes this is correct answer. I have change data type from `char` to `uint8_t ` . – Shrikant Jul 05 '17 at 12:27
No, that shouldn't matter the slightest, as long as you don't use numbers larger than 0x7F. – Lundin Jul 05 '17 at 12:36
1

Now I just spotted the actual bug. I'll post an answer. Had you put the code through a debugger you would have found this bug after approximately 1 minute. – Lundin Jul 05 '17 at 12:38

What input should I give to crc16 Method

2 Answers2