Browse Source

Fix Crash/PP recovery position on instructions with comments

PR #2967 altered the way ``sdpos_atomic`` was set, causing issues in the
crashdetect/powerpanic recovery offset if the instruction being
recovered happens to contain a comment.

Previously ``sdpos`` was assumed to be a single byte prior to the last
read character. sdpos+1 would thus position the index to the next
instruction. With gcode-filtering in place, sdpos is left just before
the comment, while the actual read position is at the newline. This
causes to parser to resume in the middle of the comment.

Change the value returned by cardreader::get_sdpos() to always return
the last read position, as everybody expects (!!).

This avoids the +1, and correctly sets the resume position to the next
valid instruction without overhead.
Yuri D'Elia 3 years ago
parent
commit
23c75da727
2 changed files with 8 additions and 5 deletions
  1. 6 3
      Firmware/cardreader.h
  2. 2 2
      Firmware/cmdqueue.cpp

+ 6 - 3
Firmware/cardreader.h

@@ -59,9 +59,12 @@ public:
 
   FORCE_INLINE bool isFileOpen() { return file.isOpen(); }
   bool eof() { return sdpos>=filesize; }
-  // There may be a potential performance problem - when the comment reading fails, sdpos points to the last correctly read character.
-  // However, repeated reading (e.g. after power panic) the comment will be read again - it should survive correctly, it will just take a few moments to skip
-  FORCE_INLINE int16_t getFilteredGcodeChar() {  sdpos = file.curPosition();return (int16_t)file.readFilteredGcode();};
+  FORCE_INLINE int16_t getFilteredGcodeChar()
+  {
+      int16_t c = (int16_t)file.readFilteredGcode();
+      sdpos = file.curPosition();
+      return c;
+  };
   void setIndex(long index) {sdpos = index;file.seekSetFilteredGcode(index);};
   FORCE_INLINE uint8_t percentDone(){if(!isFileOpen()) return 0; if(filesize) return sdpos/((filesize+99)/100); else return 0;};
   FORCE_INLINE char* getWorkDirName(){workDir.getFilename(filename);return filename;};

+ 2 - 2
Firmware/cmdqueue.cpp

@@ -600,7 +600,7 @@ void get_command()
       }
       // The new command buffer could be updated non-atomically, because it is not yet considered
       // to be inside the active queue.
-      sd_count.value = (card.get_sdpos()+1) - sdpos_atomic;
+      sd_count.value = card.get_sdpos() - sdpos_atomic;
       cmdbuffer[bufindw] = CMDBUFFER_CURRENT_TYPE_SDCARD;
       cmdbuffer[bufindw+1] = sd_count.lohi.lo;
       cmdbuffer[bufindw+2] = sd_count.lohi.hi;
@@ -625,7 +625,7 @@ void get_command()
       // or a 115200 Bd serial line receive interrupt, which will not trigger faster than 12kHz.
       ++ buflen;
       bufindw += len;
-      sdpos_atomic = card.get_sdpos()+1;
+      sdpos_atomic = card.get_sdpos();
       if (bufindw == sizeof(cmdbuffer))
           bufindw = 0;
       sei();