user2952987
user2952987

Reputation: 31

Convert crc8 function in C to Java

I received a CRC function written in C from a hardware partner. Messages send by his devices are signed using this code. Can anyone help me to translate it to Java?

int8 crc8(int8*buf, int8 data_byte)
{
    int8 crc=0x00;
    int8 data_bit=0x80;
    while(data_byte>0)
    {
        if(((crc&0x01)!=0)!=((buf[data_byte]&data_bit)!=0))
        {
            crc>>=1;
            crc^=0xCD;
        }
        else 
            crc>>=1;
        data_bit>>=1;
        if(!data_bit)
        {
            data_bit=0x80;
            data_byte--;
        }
    }
    return(crc);
}

I tried to convert this to Java, but the result is not I expect.

public static byte crc8(byte [] buf, byte data_byte)
{
  byte crc = 0x00;
  byte data_bit = (byte)0x80;
  while(data_byte>0)
  {
    if(((crc&0x01)!=0)!=((buf[data_byte]&data_bit)!=0))
    {
      crc>>=1;
      crc^=0xCD;
    }
    else
    {
      crc>>=1;
    }
    data_bit>>=1;
    if(data_bit == 0)
    {
      data_bit= (byte)0x80;
      data_byte--;
    }
  }
  return crc;
}

I suppose that this is the error: if(data_bit != 0)

EDIT:

I changed the code to byte in my conversion method. I receive my data from a socket and convert this then to a String where I get a byteArray out from.

An input example is 16, 0, 1, -15, 43, 6, 1, 6, 8, 0, 111, 0, 0 ,49 where the last field (49) should be the checksum

I also tried Durandals version, but my result is still not valid.

This is how I read the data

BufferedReader bufferedReader = new BufferedReader(new InputStreamReader(socket.getInputStream()));
char[] buffer = new char[14];
int count= bufferedReader.read(buffer, 0, 14); 
String msg = new String(buffer, 0, count);
byte[] content = msg.getBytes();

Upvotes: 3

Views: 2375

Answers (2)

Durandal
Durandal

Reputation: 20069

Translate the code 1:1, paying extra attention to all operations done on bytes to account for java's implicit cast to int (e.g. (byte >>> 1) is absolutely worthless because the byte is first extendet to int, shifted and then cast back, making it effectively a signed shift no matter what).

Therefore local variables are best declared as int and when loaded from a bytearray masked to yield unsigned extension: int x = byte[i] & 0xFF; Since in the only place that is done data is already masked down to a single bit (in the if) there is nothing special to be done.

Applying to the C code yields:

int crc8(byte[] buf, int dataCount) {
    int crc = 0;
    int data_bit = 0x80;
    while(dataCount > 0) {
        if ( ((crc & 0x01)!=0) != ((buf[dataCount] & data_bit)!=0)) {
            crc >>= 1;
            crc ^= 0xCD;
        } else {
            crc >>= 1;
        }
        data_bit >>= 1;
        if (data_bit == 0) {
            data_bit = 0x80;
            dataCount--;
        }
    }
    return crc;
}

That said, the code isn't very efficient (it processes input bit by bit, there are faster implementations processing entire bytes, using a table for each possible byte added, but you probably don't care for this use case).

Also, beware when you compare the crc from this method to a byte, you must mask the byte properly with 0xFF, otherwise comparison will fail for values >=0x80:

(int) crc == (byte) crc & 0xFF

EDIT:

What worries my even about the original code, that data_byte is clearly intended to specify a length, first it calculates in reverse order and also, it will access an additional byte after the specfied number (data_byte is not decremented before the loop). I suspect the original is (already) broken code, or the calls to it are very messy.

Upvotes: 0

Charlie Burns
Charlie Burns

Reputation: 7056

if(!data_bit)

translates to

if(data_bit == 0)

You really need to use bytes and not shorts. To get around the problem you had using bytes, use this

byte data_bit = (byte)0x80;

Also, as Mark says, you need to use >>> instead of >>.

Upvotes: 2

Related Questions