-1

I'm trying to implement a CRC check on my Arduino and Thanks to you guys I think I've got this working:

unsigned int CRC(byte input[], int array_size)
{
unsigned int crc16 = 0xFFFF;
int pos = 0;
int i = 0;

/*Main Code*/
  for (pos = 0; pos < sizeof(input); pos++) {
    crc16 ^= input[pos];          // XOR byte into least sig. byte of crc

    for (i = 8; i != 0; i--) {    // Loop over each bit
      if ((crc16 & 0x0001) != 0) {      // If the LSB is set
        crc16 >>= 1;                    // Shift right and XOR 0xA001
        crc16 ^= 0xA001;
      }
      else                            // Else LSB is not set
        crc16 >>= 1;                    // Just shift right
    }
  }
  /*Note, this number has low and high bytes swapped,
  so swap bytes*/
  return crc16;
}  
  • What did the compiler tell you? – Mikael Patel May 16 '17 at 16:50
  • 3
    Note that byte crc[] stack-allocates crc, so it goes away when the function returns; ie, officially is no good as a returned result. Also, it doesn't match the byte type of the function; crc is a byte[] or byte*. Just compute and return an unsigned int instead of an array – James Waldby - jwpat7 May 16 '17 at 16:56
  • The compiler tells me that it works, but doing what I want it to do and working are a bit different XD I'm looking to return an array as I'm going to concatenate it into a larger byte array. Should I do this in the parent function? If I change the output to byte* will it work as intended? – ATE-ENGE May 16 '17 at 17:06
  • 3
    With sizeof(input), you get the sizeof a byte pointer. You need to pass the length of the array as a parameter, – Johnny Mopp May 16 '17 at 17:30
  • 2
    Your edits to the post have made it useless to future readers. You should edit the question back in - that's the format of the site, question and answer(s). – Mark Smith May 16 '17 at 22:04
1

As I mentioned in the comments, once you pass an array as a parameter to a function, it "decays" into a pointer and the sizeof operator no longer works the way you want. A typical solution is to pass a second argument for the number of elements in the array:

byte* CRC(byte input[], int array_size)
{
    //....
    for (pos = 0; pos < array_size; pos++) {

In the example you linked to, the array is global, so it stays an array. It would be similar to this:

byte input[1234]; // global
byte* CRC()
{
    //....
    for (pos = 0; pos < sizeof(input); pos++) {

Also mentioned in the comments is that you are returning the address of a local variable. That's not allowed (unless it's a static variable). I would just return crc16 and let the calling function sort it out;

unsigned int crc = CRC(input, 1234);
byte crc1 = crc >> 8;
byte crc2 = crc & 0xFF

And change the function to:

unsigned int CRC(byte input[], int array_size)
{
    unsigned int crc16 = 0xFFFF;
    // ........
    return crc16;
}

One last note: the sizeof operator only works on a byte array because a byte is 1 byte. The proper way to get the sizeof an array is:

int the_array[32];
size_t array_length = sizeof(the_array) / sizeof(int);

Even better:

size_t array_length = sizeof(the_array) / sizeof(the_array[0]);
  • Ok! I understand now I don't mean to be difficult, I'm new here XD – ATE-ENGE May 16 '17 at 20:21
  • 1
    No problem. We are here to help each other learn and improve. – Johnny Mopp May 16 '17 at 20:24

Not the answer you're looking for? Browse other questions tagged or ask your own question.