Issues calculating CRC16 MCRF4XX for more than 1 byte

Question

I've been trying to perform CRC16 MCRF4XX in my code however I've managed to do it correctly for only 1 byte.

I've followed this guide, for the specific method: http://www.piclist.com/techref/method/error/quickcrc16.htm and i've tested the same byte in https://crccalc.com/

code is as follows:

register uint32_t i;
    uint16_t Crc = 0;


    for ( i = 0; i < Len; i++ )
        Crc = Utils_CRC16_MCRF4XX(Crc,pData[i]);

    return ( Crc );

the function "Utils_CRC16_MCRF4XX":

    uint8_t     i;
uint16_t    TempByte, CurrentCRC = 0xFFFF;
//make byte 16 bit format
TempByte = (uint16_t)Byte;

for ( i = 0; i < 8; i++ )
{
    if ( (CurrentCRC & 0x0001) == (TempByte & 0x0001) )
    {
        //right shift crc
        CurrentCRC >>= 1;
        //right shift data
        TempByte >>= 1;   
    }
    else
    {
        CurrentCRC >>= 1;
        TempByte >>= 1;
        CurrentCRC = CurrentCRC ^ 0x8408; /* 1000 0100 0000 1000 = x^16 + x^12 + x^5 + 1 */
    }
}

return ( Crc ^ CurrentCRC);

the output for byte 0x54 would be 0x1B26. I've tried XORing the output with the inserted Crc, but it doesn't add up right.

now my issue starts when I'm trying to feed the function more than 1 byte.

if let's say i would send it : 0x54 0xFF. it would give me a totally different calculation than the calculator gives.

I'm assuming my error is where i add up the bytes together, after performing the action on each byte.

appreciate the help!

Answer 1

Your function Utils_CRC16_MCRF4XX should update the Crc, but keeps its own CurrentCRC variable that bares no relationship to the current CRC value and is reinitialised to 0xFFFF on each call. The Crc parameter passed in is teh current CRC and that should be updated.

Adapting your function with minimal changes:

uint16_t Utils_CRC16_MCRF4XX( uint16_t Crc, uint8_t Byte )
{
    //make byte 16 bit format
    uint16_t TempByte = (uint16_t)Byte;

    for( uint8_t i = 0; i < 8; i++ )
    {
        if( (Crc & 0x0001) == (TempByte & 0x0001) )
        {
            //right shift crc
            Crc >>= 1;
            //right shift data
            TempByte >>= 1;
        }
        else
        {
            Crc >>= 1;
            TempByte >>= 1;
            Crc = Crc ^ 0x8408;
        }
    }

    return Crc ;
}

In the code that calls this, the Crc must be initialised to 0xFFFF, not zero:

uint16_t crc( uint8_t* pData, uint32_t Len )
{
    uint16_t Crc = 0xffffu ;

    for( uint32_t i = 0; i < Len; i++ )
    {
        Crc = Utils_CRC16_MCRF4XX( Crc, pData[i] );
    }
    return (Crc);
}

The following test code, produces the result 0x6F91 which concurs with https://crccalc.com/:

int main()
{
    uint8_t test[] = "123456789" ;
    uint16_t c = crc( test, sizeof(test) - 1 ) ;
    printf( "%X", (int)c ) ;

    return 0 ;
}

The implicit conversion that occurs when applying the & operator make TempByte redundant so further simplification is possible:

uint16_t Utils_CRC16_MCRF4XX( uint16_t Crc, uint8_t Byte )
{
    for( uint8_t i = 0; i < 8; i++ )
    {
        if( (Crc & 0x0001) == (Byte & 0x0001) )
        {
            Crc >>= 1;
            Byte >>= 1;
        }
        else
        {
            Crc >>= 1;
            Byte >>= 1;
            Crc = Crc ^ 0x8408;
        }
    }

    return Crc ;
}

Adapting the solution at https://gist.github.com/aurelj/270bb8af82f65fa645c1 yields the somewhat more succinct solution:

uint16_t Utils_CRC16_MCRF4XX( uint16_t Crc, uint8_t Byte )
{
    Crc ^= Byte ;

    for( uint8_t i = 0; i < 8; i++ )
    {
        Crc = (Crc & 0x0001) != 0 ? (Crc >> 1) ^ 0x8408 : 
                                    Crc >> 1 ;
    }

    return Crc ;
}

Answer 2

Completed code, including main() driver.

---

#include <stdint.h>
#include <stdio.h>

uint16_t Utils_CRC16_MCRF4XX(uint16_t crc, uint16_t byte);

int main(int argc, char **argv) {

uint32_t i;
    uint16_t crc ;
    uint8_t data[200] =  { 0 };
    uint32_t len ;

    for(len = 0; len+1 < argc; len++ ) {
        sscanf(argv[len+1], "%hhx", &data[len] );
        }

    crc = 0xffff;
    for ( i = 0; i < len; i++ ) {
        crc = Utils_CRC16_MCRF4XX(crc, data[i] );
        fprintf(stderr, "[%u] %2hhx CrC=%04x\n", (unsigned) i, data[i], (unsigned) crc);
    }

    fprintf(stderr, "CrC=%04x\n", (unsigned) crc);
    return 0 ;
}

uint16_t Utils_CRC16_MCRF4XX(uint16_t crc, uint16_t byte)
{
uint8_t i;

for ( i = 0; i < 8; i++ ) {
    register int samelow;
    samelow =  (crc & 1) == (byte & 1) ?1 : 0 ;

    crc >>= 1;
    byte >>= 1;
    if (!samelow) crc ^= 0x8408; /* 1000 0100 0000 1000 = x^16 + x^12 + x^5 + 1 */
    }

  return crc;
}