Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix EEPROM checksum to not be lastchar or lastchar+1 #266

Open
wants to merge 1 commit into
base: edge
Choose a base branch
from

Conversation

drf5n
Copy link

@drf5n drf5n commented Apr 11, 2022

Per gnea#158 the logical OR in the code below discards most of the checksum information, resulting in the final chekcsum being either the last character, or the last character +1.

grbl-Mega-5X/grbl/eeprom.c

Lines 130 to 149 in 7a293a9

void memcpy_to_eeprom_with_checksum(unsigned int destination, char *source, unsigned int size) {
unsigned char checksum = 0;
for(; size > 0; size--) {
checksum = (checksum << 1) || (checksum >> 7);
checksum += *source;
eeprom_put_char(destination++, *(source++));
}
eeprom_put_char(destination, checksum);
}
int memcpy_from_eeprom_with_checksum(char *destination, unsigned int source, unsigned int size) {
unsigned char data, checksum = 0;
for(; size > 0; size--) {
data = eeprom_get_char(source++);
checksum = (checksum << 1) || (checksum >> 7);
checksum += data;
*(destination++) = data;
}
return(checksum == eeprom_get_char(source));
}

This fix brings the checksum in-line with grblHAL's implementation: https://github.com/grblHAL/core/blob/3a84b58d301f04279268b4ef1045fd6bc0961be5/nuts_bolts.c#L267-L278

See also the discussion at grbl#1249

Per gnea#158 the logical OR discards most of the checksum information, resulting in the final chekcsum being either the last character, or the last character +1.

This fix brings the checksum in-line with grblHAL's implementation:  https://github.com/grblHAL/core/blob/3a84b58d301f04279268b4ef1045fd6bc0961be5/nuts_bolts.c#L267-L278
@drf5n drf5n changed the title FIx EEPROM checksum to not be lastchar or lastchar+1 Fix EEPROM checksum to not be lastchar or lastchar+1 Apr 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant