Skip to content

Commit

Permalink
Fix for a wrong checksum truncation for certain commands
Browse files Browse the repository at this point in the history 
Positioning of string terminator to truncate checksum from the commands
M23, M28, M30, M32, M928 and M117 was off by one, causing the last
letter of the actual command to be truncated instead of just the
checksum.

In case of the SD commands this caused checksummed commands targeting
existing files to fail since the last letter of the filename was
truncated.

In case of M117 this caused the last given letter not to be displayed.

This patch fixes the off-by-one error and sets the null terminator
on the exact position of the * starting the checksum instead of the
character before that.
  • Loading branch information
@foosel
foosel committed Jul 24, 2014
1 parent ea9bffd commit 2d22902
Showing 1 changed file with 6 additions and 6 deletions.
12 changes: 6 additions & 6 deletions Marlin/Marlin_main.cpp
View file
Expand Up @@ -1693,7 +1693,7 @@ void process_commands()
case 23: //M23 - Select file
starpos = (strchr(strchr_pointer + 4,'*'));
if(starpos!=NULL)
*(starpos-1)='\0';
*(starpos)='\0';
card.openFile(strchr_pointer + 4,true);
break;
case 24: //M24 - Start SD print
Expand All @@ -1716,7 +1716,7 @@ void process_commands()
if(starpos != NULL){
char* npos = strchr(cmdbuffer[bufindr], 'N');
strchr_pointer = strchr(npos,' ') + 1;
*(starpos-1) = '\0';
*(starpos) = '\0';
}
card.openFile(strchr_pointer+4,false);
break;
Expand All @@ -1731,7 +1731,7 @@ void process_commands()
if(starpos != NULL){
char* npos = strchr(cmdbuffer[bufindr], 'N');
strchr_pointer = strchr(npos,' ') + 1;
*(starpos-1) = '\0';
*(starpos) = '\0';
}
card.removeFile(strchr_pointer + 4);
}
Expand All @@ -1753,7 +1753,7 @@ void process_commands()
namestartpos++; //to skip the '!'

if(starpos!=NULL)
*(starpos-1)='\0';
*(starpos)='\0';

bool call_procedure=(code_seen('P'));

Expand All @@ -1776,7 +1776,7 @@ void process_commands()
if(starpos != NULL){
char* npos = strchr(cmdbuffer[bufindr], 'N');
strchr_pointer = strchr(npos,' ') + 1;
*(starpos-1) = '\0';
*(starpos) = '\0';
}
card.openLogFile(strchr_pointer+5);
break;
Expand Down Expand Up @@ -2193,7 +2193,7 @@ void process_commands()
case 117: // M117 display message
starpos = (strchr(strchr_pointer + 5,'*'));
if(starpos!=NULL)
*(starpos-1)='\0';
*(starpos)='\0';
lcd_setstatus(strchr_pointer + 5);
break;
case 114: // M114
Expand Down

1 comment on commit 2d22902

@foosel
Copy link
Contributor Author

@foosel foosel commented on 2d22902 Oct 12, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is the right change considering the reality of how stuff works. Yes, the gcode page says fields are supposed to be separated by white space or line breaks, no, this is not how it's implemented across all slicers and other host software or as it's stated in the nist gcode standard. So the firmware has to cope with that reality. Which it does just fine for defined named fields (X, Y, Z, S, etc) but before this patch not for textual parameters which according to the wiki page don't even exist.

And keeping the patch as minimally invasive as possible keeps it hopefully better mergable across the millions of forks out there.

Please sign in to comment.