Computer Interfacing
Discussions about interfacing and electronics
 

CRC16 and CRC16 Modbus issues on ATMEL (atmega164)

Problem with calculating CRC using the lib that given by the site.


 

       Computer Interfacing Forum Index -> Error detection and correction
Author Message
TheAgent1982
New User



Joined: 06 Dec 2012
Posts: 4


Dec 06, 2012 11:32 am

Hello all,
I am using ATMEL atmega164 Micro Controller.
Programming it with MikroC.

In my project I trying to calculate CRC16 and CRC 16 Modbus, after receiving the data string from RS-232.

The code I have written:

Code:

#include <stdlib.h>
#include <string.h>

#include "lib_crc.h"

char *output;
char *ptr;
unsigned short crc_16, crc_16_modbus;

void main() {

  UART1_Init(4800);               // Initialize UART module at 9600 bps
  Delay_ms(100);                  // Wait for UART module to stabilize

  DDRD = 0xFF;                    // Set direction to be output

  while (1) {                     // Endless loop
   if (UART1_Data_Ready() == 1) {          // if data is received
    UART1_Read_Text(output, "OK", 255);    // reads text until 'OK' is found
    UART1_Write_Text(output);              // sends back text
   }

 // CRC Checking
   ptr    = output;
   crc_16 = 0;
   crc_16_modbus = 0xffff;
   while ( *ptr ) {
          crc_16         = update_crc_16(crc_16, *ptr);
          crc_16_modbus  = update_crc_16(crc_16_modbus, *ptr);

          ptr++;
   }
   // CRC Compare and LED
      if (strcmp(crc_16,"30C0") == 0) {
           PORTD = 0x40;        // If CRC16 OK for A, Turn ON Green diode on PORTD
     }


   if (strcmp(output,"A") == 0) {
           PORTD = 0x20;        // Turn ON RED diode on PORTD
     }
     else if (strcmp(output,"B") == 0) {
           PORTD = 0x40;        // Turn ON GREEN diode on PORTD
     }
     else if (strcmp(output,"C") == 0) {
           PORTD = 0x80;        // Turn ON BLUE diode on PORTD
     }
     else if (strcmp(output,"D") == 0) {
           PORTD = 0x10;        // Turn ON YELLOW diode on PORTD
     }
     else {
           PORTD = 0x00;        // Turn OFF diodes on PORTD
     }

  }
}


I am trying to to receive the output data from RS-232, than, to calculate the CRC.
If the CRC is ok,when A (red) is selected, the green Led should light.
But it did not light Sad

(The value for compare, was calculated by the site calculator and also verified by the example tst_crc.exe)

I do not understand why, what am I doing wrong?

I have all the includes, the program compile without any problems. the lib_crc.h and lib_crc.c in project also.

Any help will be welcomed
Thanks in Advance!
Gammatester
Guest







Dec 06, 2012 2:35 pm

Without knowing the exact reason of your failure I assume that your compare is not correct: In this snippet
Quote:
Code:
// CRC Compare and LED
   if (strcmp(crc_16,"30C0") == 0) {
     PORTD = 0x40;    // If CRC16 OK for A, Turn ON Green diode on PORTD
   }
you compare an unsigned short with a string. I think you should use something like this
Code:
   if (crc_16 == 0x30C0) {...}
Are you sure that you always must compare against 0x30C0?
TheAgent1982
New User



Joined: 06 Dec 2012
Posts: 4


Dec 06, 2012 2:59 pm

Thank you for your replay!
I have been checking it for test propose, if "correct" selection is made.
(On advanced, I will check it against CRC table or something like that ...)

I applied you fix, But with no luck Sad

Any idea about the CRC calculation I have implanted in my program? Any error I made there?
(I have suspicious about wrong calculation in the implanted code).

Thanks in Advance!
Gammatester
Guest







Dec 06, 2012 3:11 pm

I do not see any CRC calculation.

Normally it is good practice to divide the tasks: First write and test your CRC routines with known data. If this runs reliably, then as a second step integrate the hardware part.

If something fails at this stage you know that the hardware/protocoll is the culprit.
TheAgent1982
New User



Joined: 06 Dec 2012
Posts: 4


Dec 06, 2012 3:18 pm

Code:

// CRC Checking
   ptr    = output;
   crc_16 = 0;
   crc_16_modbus = 0xffff;
   while ( *ptr ) {
          crc_16         = update_crc_16(crc_16, *ptr);
          crc_16_modbus  = update_crc_16(crc_16_modbus, *ptr);

          ptr++;
   }

the function update_crc_16 is calling to calculation function provided by the author of the site.


Code:
  /*******************************************************************\
    *                                                                   *
    *   unsigned short update_crc_16( unsigned short crc, char c );     *
    *                                                                   *
    *   The function update_crc_16 calculates a  new  CRC-16  value     *
    *   based  on  the  previous value of the CRC and the next byte     *
    *   of the data to be checked.                                      *
    *                                                                   *
    \*******************************************************************/

unsigned short update_crc_16( unsigned short crc, char c ) {

    unsigned short tmp, short_c;

    short_c = 0x00ff & (unsigned short) c;

    if ( ! crc_tab16_init ) init_crc16_tab();

    tmp =  crc       ^ short_c;
    crc = (crc >> 8) ^ crc_tab16[ tmp & 0xff ];

    return crc;

}  /* update_crc_16 */


and init_crc_tab16 here:

Code:

   /*******************************************************************\
    *                                                                   *
    *   static void init_crc16_tab( void );                             *
    *                                                                   *
    *   The function init_crc16_tab() is used  to  fill  the  array     *
    *   for calculation of the CRC-16 with values.                      *
    *                                                                   *
    \*******************************************************************/

static void init_crc16_tab( void ) {

    int i, j;
    unsigned short crc, c;

    for (i=0; i<256; i++) {

        crc = 0;
        c   = (unsigned short) i;

        for (j=0; j<8; j++) {

            if ( (crc ^ c) & 0x0001 ) crc = ( crc >> 1 ) ^ P_16;
            else                      crc =   crc >> 1;

            c = c >> 1;
        }

        crc_tab16[i] = crc;
    }

    crc_tab16_init = TRUE;

}  /* init_crc16_tab */

So as I check it, it goes well by the functions...
Do I miss something?
Is it for calculation or only for comparing to table of values?

Thank you for your help!
Gammatester
Guest







Dec 06, 2012 3:30 pm

Ah, I see.

Another possible issue: Your code cannot process zero bytes (Hex 00). Are you sure, that your string does not contain them?

You read until you find "OK"? Is this "OK" part of the data to be CRC-checked? Who terminates the output with 0x00? I do not see any *output++ = 0x00;

Where is the memory allocation for output?
TheAgent1982
New User



Joined: 06 Dec 2012
Posts: 4


Dec 06, 2012 3:47 pm

The string output contain the ASCII char without the "OK".
It "OK" sign to terminate reading communication.

example: I press AOK.
the output recieve A.
The CRC calculated for A without the OK.

please tell me more about the 00 issue, how did you realize it ?
As you realize, I am new to that ...

For now, I have A, B, C, D (as ASCII) option for output string.
Thanks for your time!
Gammtester
Guest







Dec 07, 2012 7:36 am

Quote:
please tell me more about the 00 issue, how did you realize it? As you realize, I am new to that ...
Your calculation loop is controlled by
Code:
while ( *ptr ) {}
and it runs as long as the byte or char pointed by ptr is non zero. Therefore the input must be terminated by appending a trailing zero byte, otherwise the loop runs normally until reaches a garbage zero (or your program is killed by an access violation).

       Computer Interfacing Forum Index -> Error detection and correction
Page 1 of 1



Running on php BB © 2001, 2009 php BB Group
   Lammert Bies     Interfacing     Sitemap     Forum