search

gnea / grbl-Mega

An open source, embedded, high performance g-code-parser and CNC milling controller written in optimized C that will run on an Arduino Mega2560
https://github.com/gnea/grbl/wiki
Other
493 stars 229 forks source link

EEPROM Checksum is either last char or last char+1 #158

Open drf5n opened 2 years ago

drf5n commented 2 years ago

grbl/eeprom.c: In function 'memcpy_to_eeprom_with_checksum': grbl/eeprom.c:133:26: warning: '<<' in boolean context, did you mean '' ? [-Wint-in-bool-context] 133 | checksum = (checksum << 1) || (checksum >> 7); | ~~^~~ grbl/eeprom.c: In function 'memcpy_from_eeprom_with_checksum': grbl/eeprom.c:144:26: warning: '<<' in boolean context, did you mean '' ? [-Wint-in-bool-context] 144 | checksum = (checksum << 1) || (checksum >> 7); | ~~^~~

That looks like it was a typo -- it squashes the checksum down to a single bit of information, when it looks like it intended to roll it right one bit, so the final checksum ends up either zero or one larger than the last character in the data:

https://github.com/gnea/grbl-Mega/blob/df87b36f48b90930714accf03b411b60d5d948e5/grbl/eeprom.c#L130-L149

Maybe it was supposed to be 

checksum = (checksum << 1) | (checksum >> 7);

Originally posted by @drf5n in https://github.com/gnea/grbl-Mega/issues/157#issuecomment-1095029184

drf5n commented 2 years ago

See grblHAL's checksum implementation in https://github.com/grblHAL/core/blob/3a84b58d301f04279268b4ef1045fd6bc0961be5/nuts_bolts.c#L267-L278

drf5n commented 1 year ago

See also GRBL issue # https://github.com/grbl/grbl/issues/1249