mac-zhou / midea-msmart

This is a library to allow communicating to a Midea AC via the Local area network.
MIT License
147 stars 40 forks source link

Potential issue in crc calculation? #39

Closed dzek69 closed 3 years ago

dzek69 commented 3 years ago

Hey, I was looking through the code and I found this:

https://github.com/mac-zhou/midea-msmart/blob/696b8f106ac12fb723de8dc4a5b1ef5cf1eedd89/msmart/crc8.py#L44

I think this is a mistake and it should be >= 256 (or just > 255) because in some rare cases if k stays at exactly 256 then there will be no value to take from the table.

Please verify and if current solution is really correct then maybe leave a comment in the code why, because it looks really confusing.

thanks!

mac-zhou commented 3 years ago

i think 256 will never appear, max value is 255 this is written just to mimic the java code of nethome app.

  public static int getCRC(byte[] paramArrayOfbyte, int paramInt) {
    int i = 0;
    if (paramArrayOfbyte == null || paramInt == 0)
      return 0; 
    byte b = 0;
    while (true) {
      if (b < paramInt) {
        int j = i ^ paramArrayOfbyte[b];
        i = j;
        if (j > 256)
          i = j - 256; 
        j = i;
        if (i < 0)
          j = i + 256; 
        i = crc8_854_table[j];
        b++;
        continue;
      } 
      return i;
    } 
  }
dzek69 commented 3 years ago

i think this is possible, could be even bug in nethome, but it's that's a copy of logic then ok, better leave it that way :)