Error in checksum calculation in flash.cpp

[gilcrouse]

There's a compile warning in grbl-LPC about the checksum calculation routine in flash.cpp. There's a logical OR in the code which should be a bitwise OR. Existing code shouldn't generate spurious errors, but I don't think it will really catch any errors either. It basically returns the last character in the memory block potentially with 1 added.

I've included a patch below.

Thanks, Gil

diff --git a/grbl-lpc/flash.cpp b/grbl-lpc/flash.cpp
index a42b843..aabc422 100644
--- a/grbl-lpc/flash.cpp
+++ b/grbl-lpc/flash.cpp
@@ -77,7 +77,7 @@ static unsigned char memcpy_with_checksum(char *dest, char *src, unsigned size)
     unsigned char checksum = 0;
     while (size--)
     {
-        checksum = ((checksum << 1) || (checksum >> 7)) + *src;
+        checksum = ((checksum << 1) | (checksum >> 7)) + *src;
         *dest++ = *src++;
     }
     return checksum;

[tbfleming]

I also suspect it should be |, but I made use the same formula as memcpy_to_eeprom_with_checksum and memcpy_from_eeprom_with_checksum in https://github.com/gnea/grbl/blob/5c8dcefcbdc72e953fb07c5f8bb8ef4925ac6b5b/grbl/eeprom.c

@chamnit ?

[chamnit]

That piece of code has a bug and hasnt worked since 2010-2011 when it was first installed by the original author of Grbl. I discovered it a couple years ago but decided to leave it alone. If I fix it, everyone would get their EEPROM wiped automatically since their checksums would all be wrong. This problem is being fixed in the next major version with a better Fletcher checksum.

@tbfleming It’s up to you if you want to fix it. Like I said before, it’ll wipe everyone’s settings unless you write and test some countermeasures.

[tbfleming]

I'll leave it be for now. I'll keep this issue open.