From dcc660580942fac4872e85ce80b143e086662d36 Mon Sep 17 00:00:00 2001 From: "D.R.racer" Date: Mon, 11 Jan 2021 11:13:40 +0100 Subject: [PATCH 01/16] Workaround for skipping large comment blocks If there are large blocks of comments in the G-code, the printer may get shot down by its own watchdog. Watchdog is generally set to 4s and updated only in manage_heaters (and some other spots in some specific cases). So far, the code reading the file and feeding it into Marlin cycles indefinitely until it finds valid G-code lines and fills up the command queue. If the block is large enough, the printer cannot read it completely within those 4s. A simple workaround - bail out after some consecutive empty/comment lines to enable other parts of code do their job (especially manage_heaters). Tested on MK404, previous FW fails with 600KB of comment lines at the beginning, this patch survives. The printer even draws some update on its status screen before starting a real print. --- Firmware/cmdqueue.cpp | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/Firmware/cmdqueue.cpp b/Firmware/cmdqueue.cpp index afdddfba2..f0ac1b395 100755 --- a/Firmware/cmdqueue.cpp +++ b/Firmware/cmdqueue.cpp @@ -573,6 +573,7 @@ void get_command() // this character _can_ occur in serial com, due to checksums. however, no checksums are used in SD printing static bool stop_buffering=false; + static uint8_t consecutiveEmptyLines = 0; if(buflen==0) stop_buffering=false; union { struct { @@ -586,11 +587,12 @@ void get_command() while( !card.eof() && !stop_buffering) { int16_t n=card.get(); char serial_char = (char)n; - if(serial_char == '\n' || - serial_char == '\r' || - ((serial_char == '#' || serial_char == ':') && comment_mode == false) || - serial_count >= (MAX_CMD_SIZE - 1) || n==-1) - { + if( serial_char == '\n' + || serial_char == '\r' + || ((serial_char == '#' || serial_char == ':') && comment_mode == false) + || serial_count >= (MAX_CMD_SIZE - 1) + || n==-1 + ){ if(serial_char=='#') stop_buffering=true; @@ -602,6 +604,10 @@ void get_command() // so that the lenght of the already read empty lines and comments will be added // to the following non-empty line. comment_mode = false; + if( ++consecutiveEmptyLines > 250 ){ + consecutiveEmptyLines = 0; + return; // prevent cycling indefinitely - let manage_heaters do their job + } continue; //if empty line } // The new command buffer could be updated non-atomically, because it is not yet considered @@ -638,9 +644,10 @@ void get_command() comment_mode = false; //for new command serial_count = 0; //clear buffer + consecutiveEmptyLines = 0; // reached a non-empty line which shall be enqueued if(card.eof()) break; - + // The following line will reserve buffer space if available. if (! cmdqueue_could_enqueue_back(MAX_CMD_SIZE-1, true)) return; From c3758d350efa9f677eb44e0ab5a98fabc190e5bf Mon Sep 17 00:00:00 2001 From: "D.R.racer" Date: Thu, 14 Jan 2021 13:21:58 +0100 Subject: [PATCH 02/16] Fast skipping of large comment blocks This is an extension/optimization of PR #2956. It uses the cached 512B block buffer to avoid heavy-weight read() in SdBaseFile. Even though this principle allowed the AVR to skip ~600KB of data within ~5 seconds, the impact on code base is huge, especially into well proven and long-term stable parts like reading a file from the SD card. The sole purpose of this PR is to show/verify the possibility of the AVR CPU in relation to adding thumbnails into MK3 G-codes. Moreover, this PR shall not be merged unless the missing/commented features are restored - especially file seeking and M84 search. PFW-1175 --- Firmware/Marlin_main.cpp | 6 +- Firmware/SdBaseFile.cpp | 126 +++++++++++++++++++++++++++++++++++++-- Firmware/SdBaseFile.h | 29 ++++++++- Firmware/SdVolume.h | 5 +- Firmware/cardreader.cpp | 65 ++++++++++++++++++++ Firmware/cardreader.h | 6 +- Firmware/cmdqueue.cpp | 24 ++++---- Firmware/ultralcd.cpp | 4 +- 8 files changed, 243 insertions(+), 22 deletions(-) diff --git a/Firmware/Marlin_main.cpp b/Firmware/Marlin_main.cpp index 4739dc4a8..a5bd4e04a 100755 --- a/Firmware/Marlin_main.cpp +++ b/Firmware/Marlin_main.cpp @@ -3987,7 +3987,9 @@ void process_commands() #endif }else if (code_seen_P("fv")) { // PRUSA fv // get file version - #ifdef SDSUPPORT + #if 0 + //@@TODO + def SDSUPPORT card.openFile(strchr_pointer + 3,true); while (true) { uint16_t readByte = card.get(); @@ -5767,7 +5769,7 @@ if(eSoundMode!=e_SOUND_MODE_SILENT) starpos = (strchr(strchr_pointer + 4,'*')); if(starpos!=NULL) *(starpos)='\0'; - card.openFile(strchr_pointer + 4,true); + card.openFileFilteredGcode(strchr_pointer + 4); break; /*! diff --git a/Firmware/SdBaseFile.cpp b/Firmware/SdBaseFile.cpp index b9e881ef2..4dead898c 100644 --- a/Firmware/SdBaseFile.cpp +++ b/Firmware/SdBaseFile.cpp @@ -530,9 +530,21 @@ bool SdBaseFile::mkdir(SdBaseFile* parent, const uint8_t dname[11]) { * \return The value one, true, is returned for success and * the value zero, false, is returned for failure. */ - bool SdBaseFile::open(const char* path, uint8_t oflag) { - return open(cwd_, path, oflag); - } +bool SdBaseFile::open(const char* path, uint8_t oflag) { + return open(cwd_, path, oflag); +} + +bool SdBaseFile::openFilteredGcode(SdBaseFile* dirFile, const char* path){ + if( open(dirFile, path, O_READ) ){ + gf.reset(0,0); + // compute the block to start with + if( ! computeNextFileBlock(&gf.block, &gf.offset) ) + return false; + return true; + } else { + return false; + } +} //------------------------------------------------------------------------------ /** Open a file or directory by name. * @@ -1030,6 +1042,112 @@ int16_t SdBaseFile::read() { uint8_t b; return read(&b, 1) == 1 ? b : -1; } + +int16_t SdBaseFile::readFilteredGcode() { + // avoid calling the default heavy-weight read() for just one byte + return gf.read_byte(); +} + +void GCodeInputFilter::reset(uint32_t blk, uint16_t ofs){ + // @@TODO clean up + block = blk; + offset = ofs; + cachePBegin = sd->vol_->cache()->data; + // reset cache read ptr to its begin + cacheP = cachePBegin; +} + +int16_t GCodeInputFilter::read_byte(){ + EnsureBlock(); // this is unfortunate :( ... other calls are using the cache and we can loose the data block of our gcode file + + // assume, we have the 512B block cache filled and terminated with a '\n' +// SERIAL_PROTOCOLPGM("read_byte enter:"); +// for(uint8_t i = 0; i < 16; ++i){ +// SERIAL_PROTOCOL( cacheP[i] ); +// } + + const uint8_t *start = cacheP; + uint8_t consecutiveCommentLines = 0; + while( *cacheP == ';' ){ + for(;;){ + while( *(++cacheP) != '\n' ); // skip until a newline is found + // found a newline, prepare the next block if block cache end reached + if( cacheP - cachePBegin >= 512 ){ + // at the end of block cache, fill new data in + sd->curPosition_ += cacheP - start; + if( ! sd->computeNextFileBlock(&block, &offset) )goto fail; + EnsureBlock(); // fetch it into RAM + cacheP = start = cachePBegin; + } else { + if(++consecutiveCommentLines == 255){ + // SERIAL_PROTOCOLLN(sd->curPosition_); + goto forceExit; + } + // peek the next byte - we are inside the block at least at 511th index - still safe + if( *(cacheP+1) == ';' ){ + // consecutive comment + ++cacheP; + ++consecutiveCommentLines; + } + break; // found the real end of the line even across many blocks + } + } + } +forceExit: + sd->curPosition_ += cacheP - start + 1; + { + int16_t rv = *cacheP++; + + // prepare next block if needed + if( cacheP - cachePBegin >= 512 ){ +// SERIAL_PROTOCOLLN(sd->curPosition_); + if( ! sd->computeNextFileBlock(&block, &offset) )goto fail; + // don't need to force fetch the block here, it will get loaded on the next call + cacheP = cachePBegin; + } + return rv; + } +fail: +// SERIAL_PROTOCOLLNPGM("CacheFAIL"); + return -1; +} + +bool GCodeInputFilter::EnsureBlock(){ + if ( sd->vol_->cacheRawBlock(block, SdVolume::CACHE_FOR_READ)){ + // terminate with a '\n' + const uint16_t terminateOfs = (sd->fileSize_ - offset) < 512 ? (sd->fileSize_ - offset) : 512; + sd->vol_->cache()->data[ terminateOfs ] = '\n'; + return true; + } else { + return false; + } +} + +bool SdBaseFile::computeNextFileBlock(uint32_t *block, uint16_t *offset) { + // error if not open or write only + if (!isOpen() || !(flags_ & O_READ)) return false; + + *offset = curPosition_ & 0X1FF; // offset in block + if (type_ == FAT_FILE_TYPE_ROOT_FIXED) { + *block = vol_->rootDirStart() + (curPosition_ >> 9); + } else { + uint8_t blockOfCluster = vol_->blockOfCluster(curPosition_); + if (*offset == 0 && blockOfCluster == 0) { + // start of new cluster + if (curPosition_ == 0) { + // use first cluster in file + curCluster_ = firstCluster_; + } else { + // get next cluster from FAT + if (!vol_->fatGet(curCluster_, &curCluster_)) return false; + } + } + *block = vol_->clusterStartBlock(curCluster_) + blockOfCluster; + } + return true; +} + + //------------------------------------------------------------------------------ /** Read data from a file starting at the current position. * @@ -1443,7 +1561,7 @@ bool SdBaseFile::rmRfStar() { * \param[in] oflag Values for \a oflag are constructed by a bitwise-inclusive * OR of open flags. see SdBaseFile::open(SdBaseFile*, const char*, uint8_t). */ -SdBaseFile::SdBaseFile(const char* path, uint8_t oflag) { +SdBaseFile::SdBaseFile(const char* path, uint8_t oflag):gf(this) { type_ = FAT_FILE_TYPE_CLOSED; writeError = false; open(path, oflag); diff --git a/Firmware/SdBaseFile.h b/Firmware/SdBaseFile.h index 923a391dd..4b3395875 100644 --- a/Firmware/SdBaseFile.h +++ b/Firmware/SdBaseFile.h @@ -174,15 +174,34 @@ static inline uint8_t FAT_SECOND(uint16_t fatTime) { uint16_t const FAT_DEFAULT_DATE = ((2000 - 1980) << 9) | (1 << 5) | 1; /** Default time for file timestamp is 1 am */ uint16_t const FAT_DEFAULT_TIME = (1 << 11); + + +class SdBaseFile; +class GCodeInputFilter { + SdBaseFile *sd; //@@TODO subject to removal - merge with some derived SdFileXX + const uint8_t *cachePBegin; + const uint8_t *cacheP; + bool EnsureBlock(); +public: + uint32_t block; // remember the current file block to be kept in cache - due to reuse of the memory, the block may fall out a must be read back + uint16_t offset; + +public: + inline GCodeInputFilter(SdBaseFile *sd):sd(sd){} + void reset(uint32_t blk, uint16_t ofs); + int16_t read_byte(); +}; + //------------------------------------------------------------------------------ /** * \class SdBaseFile * \brief Base class for SdFile with Print and C++ streams. */ class SdBaseFile { + GCodeInputFilter gf; public: /** Create an instance. */ - SdBaseFile() : writeError(false), type_(FAT_FILE_TYPE_CLOSED) {} + SdBaseFile() : gf(this), writeError(false), type_(FAT_FILE_TYPE_CLOSED) {} SdBaseFile(const char* path, uint8_t oflag); ~SdBaseFile() {if(isOpen()) close();} /** @@ -275,14 +294,21 @@ class SdBaseFile { bool open(SdBaseFile* dirFile, uint16_t index, uint8_t oflag); bool open(SdBaseFile* dirFile, const char* path, uint8_t oflag); bool open(const char* path, uint8_t oflag = O_READ); + bool openFilteredGcode(SdBaseFile* dirFile, const char* path); bool openNext(SdBaseFile* dirFile, uint8_t oflag); bool openRoot(SdVolume* vol); int peek(); static void printFatDate(uint16_t fatDate); static void printFatTime( uint16_t fatTime); bool printName(); + + int16_t readFilteredGcode(); + bool computeNextFileBlock(uint32_t *block, uint16_t *offset); + +private: int16_t read(); int16_t read(void* buf, uint16_t nbyte); +public: int8_t readDir(dir_t* dir, char* longFilename); static bool remove(SdBaseFile* dirFile, const char* path); bool remove(); @@ -322,6 +348,7 @@ class SdBaseFile { int16_t write(const void* buf, uint16_t nbyte); //------------------------------------------------------------------------------ private: + friend class GCodeInputFilter; // allow SdFat to set cwd_ friend class SdFat; // global pointer to cwd dir diff --git a/Firmware/SdVolume.h b/Firmware/SdVolume.h index 2ff2b6eb9..7c4fce959 100644 --- a/Firmware/SdVolume.h +++ b/Firmware/SdVolume.h @@ -36,7 +36,7 @@ */ union cache_t { /** Used to access cached file data blocks. */ - uint8_t data[512]; + uint8_t data[512 + 1]; // abuse the last byte for saving '\n' - ugly optimization of read_filtered's inner skipping loop /** Used to access cached FAT16 entries. */ uint16_t fat16[256]; /** Used to access cached FAT32 entries. */ @@ -119,6 +119,7 @@ class SdVolume { bool dbgFat(uint32_t n, uint32_t* v) {return fatGet(n, v);} //------------------------------------------------------------------------------ private: + friend class GCodeInputFilter; // Allow SdBaseFile access to SdVolume private data. friend class SdBaseFile; @@ -211,4 +212,4 @@ class SdVolume { #endif // ALLOW_DEPRECATED_FUNCTIONS }; #endif // SdVolume -#endif \ No newline at end of file +#endif diff --git a/Firmware/cardreader.cpp b/Firmware/cardreader.cpp index e228c5236..abeec6a8d 100644 --- a/Firmware/cardreader.cpp +++ b/Firmware/cardreader.cpp @@ -375,6 +375,71 @@ void CardReader::diveSubfolder (const char *fileName, SdFile& dir) } } +// @@TODO merge with openFile, too much duplicated code and texts +void CardReader::openFileFilteredGcode(const char* name, bool replace_current/* = false*/){ + if(!cardOK) + return; + + if(file.isOpen()){ //replacing current file by new file, or subfile call + if(!replace_current){ + if((int)file_subcall_ctr>(int)SD_PROCEDURE_DEPTH-1){ + // SERIAL_ERROR_START; + // SERIAL_ERRORPGM("trying to call sub-gcode files with too many levels. MAX level is:"); + // SERIAL_ERRORLN(SD_PROCEDURE_DEPTH); + kill(_n("trying to call sub-gcode files with too many levels."), 1); + return; + } + + SERIAL_ECHO_START; + SERIAL_ECHOPGM("SUBROUTINE CALL target:\""); + SERIAL_ECHO(name); + SERIAL_ECHOPGM("\" parent:\""); + + //store current filename and position + getAbsFilename(filenames[file_subcall_ctr]); + + SERIAL_ECHO(filenames[file_subcall_ctr]); + SERIAL_ECHOPGM("\" pos"); + SERIAL_ECHOLN(sdpos); + filespos[file_subcall_ctr]=sdpos; + file_subcall_ctr++; + } else { + SERIAL_ECHO_START; + SERIAL_ECHOPGM("Now doing file: "); + SERIAL_ECHOLN(name); + } + file.close(); + } else { //opening fresh file + file_subcall_ctr=0; //resetting procedure depth in case user cancels print while in procedure + SERIAL_ECHO_START; + SERIAL_ECHOPGM("Now fresh file: "); + SERIAL_ECHOLN(name); + } + sdprinting = false; + + SdFile myDir; + const char *fname=name; + diveSubfolder(fname,myDir); + + if (file.openFilteredGcode(curDir, fname)) { + filesize = file.fileSize(); + SERIAL_PROTOCOLRPGM(_N("File opened: "));////MSG_SD_FILE_OPENED + SERIAL_PROTOCOL(fname); + SERIAL_PROTOCOLRPGM(_n(" Size: "));////MSG_SD_SIZE + SERIAL_PROTOCOLLN(filesize); + sdpos = 0; + + SERIAL_PROTOCOLLNRPGM(_N("File selected"));////MSG_SD_FILE_SELECTED + getfilename(0, fname); + lcd_setstatus(longFilename[0] ? longFilename : fname); + lcd_setstatus("SD-PRINTING "); + } else { + SERIAL_PROTOCOLRPGM(MSG_SD_OPEN_FILE_FAIL); + SERIAL_PROTOCOL(fname); + SERIAL_PROTOCOLLN('.'); + } +} + void CardReader::openFile(const char* name,bool read, bool replace_current/*=true*/) { if(!cardOK) diff --git a/Firmware/cardreader.h b/Firmware/cardreader.h index 25e97882e..1c630b667 100644 --- a/Firmware/cardreader.h +++ b/Firmware/cardreader.h @@ -20,6 +20,7 @@ public: void checkautostart(bool x); void openFile(const char* name,bool read,bool replace_current=true); + void openFileFilteredGcode(const char* name, bool replace_current = false); void openLogFile(const char* name); void removeFile(const char* name); void closefile(bool store_location=false); @@ -59,7 +60,10 @@ public: FORCE_INLINE bool isFileOpen() { return file.isOpen(); } FORCE_INLINE bool eof() { return sdpos>=filesize ;}; - FORCE_INLINE int16_t get() { sdpos = file.curPosition();return (int16_t)file.read();}; +// FORCE_INLINE int16_t getX() { sdpos = file.curPosition();return (int16_t)file.read();}; + //@@TODO 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 void setIndex(long index) {sdpos = index;file.seekSet(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;}; diff --git a/Firmware/cmdqueue.cpp b/Firmware/cmdqueue.cpp index f0ac1b395..c89032750 100755 --- a/Firmware/cmdqueue.cpp +++ b/Firmware/cmdqueue.cpp @@ -573,7 +573,7 @@ void get_command() // this character _can_ occur in serial com, due to checksums. however, no checksums are used in SD printing static bool stop_buffering=false; - static uint8_t consecutiveEmptyLines = 0; +// static uint8_t consecutiveEmptyLines = 0; if(buflen==0) stop_buffering=false; union { struct { @@ -585,11 +585,11 @@ void get_command() sd_count.value = 0; // Reads whole lines from the SD card. Never leaves a half-filled line in the cmdbuffer. while( !card.eof() && !stop_buffering) { - int16_t n=card.get(); + int16_t n=card.getFilteredGcodeChar(); char serial_char = (char)n; if( serial_char == '\n' || serial_char == '\r' - || ((serial_char == '#' || serial_char == ':') && comment_mode == false) + || ((serial_char == '#' || serial_char == ':') /*&& comment_mode == false*/) || serial_count >= (MAX_CMD_SIZE - 1) || n==-1 ){ @@ -603,12 +603,12 @@ void get_command() // read from the sdcard into sd_count, // so that the lenght of the already read empty lines and comments will be added // to the following non-empty line. - comment_mode = false; - if( ++consecutiveEmptyLines > 250 ){ - consecutiveEmptyLines = 0; +// comment_mode = false; +// if( ++consecutiveEmptyLines > 10 ){ +// consecutiveEmptyLines = 0; return; // prevent cycling indefinitely - let manage_heaters do their job - } - continue; //if empty line +// } +// continue; //if empty line } // The new command buffer could be updated non-atomically, because it is not yet considered // to be inside the active queue. @@ -644,7 +644,7 @@ void get_command() comment_mode = false; //for new command serial_count = 0; //clear buffer - consecutiveEmptyLines = 0; // reached a non-empty line which shall be enqueued +// consecutiveEmptyLines = 0; // reached a non-empty line which shall be enqueued if(card.eof()) break; @@ -654,8 +654,10 @@ void get_command() } else { - if(serial_char == ';') comment_mode = true; - else if(!comment_mode) cmdbuffer[bufindw+CMDHDRSIZE+serial_count++] = serial_char; + /*if(serial_char == ';') comment_mode = true; + else if(!comment_mode)*/ + // there are no comments coming from the filtered file + cmdbuffer[bufindw+CMDHDRSIZE+serial_count++] = serial_char; } } if(card.eof()) diff --git a/Firmware/ultralcd.cpp b/Firmware/ultralcd.cpp index d7976d277..3fc5f5a66 100755 --- a/Firmware/ultralcd.cpp +++ b/Firmware/ultralcd.cpp @@ -8473,8 +8473,10 @@ static void lcd_selftest_screen_step(int _row, int _col, int _state, const char /** Menu action functions **/ static bool check_file(const char* filename) { + return true; // @@TODO + if (farm_mode) return true; - card.openFile((char*)filename, true); + card.openFileFilteredGcode((char*)filename, true); //@@TODO bool result = false; const uint32_t filesize = card.getFileSize(); uint32_t startPos = 0; From d275fe0e83e7d29397413e4804069dc0ef139670 Mon Sep 17 00:00:00 2001 From: "D.R.racer" Date: Wed, 27 Jan 2021 09:33:28 +0100 Subject: [PATCH 03/16] Extract gcode filter from SdBaseFile into SdFile + optimization - Start saving instructions as the whole PR was >1KB long. - It turned out the compiler was unable to understand the core skipping cycle and an ASM version had to be used. - Add seekSet aware of the G-code filter --- Firmware/SdBaseFile.cpp | 118 +--------------------------- Firmware/SdBaseFile.h | 29 +------ Firmware/SdFile.cpp | 170 ++++++++++++++++++++++++++++++++++++++++ Firmware/SdFile.h | 16 +++- Firmware/SdVolume.h | 2 +- Firmware/cardreader.h | 4 +- Firmware/ultralcd.cpp | 4 +- 7 files changed, 193 insertions(+), 150 deletions(-) diff --git a/Firmware/SdBaseFile.cpp b/Firmware/SdBaseFile.cpp index 4dead898c..4b19ceae6 100644 --- a/Firmware/SdBaseFile.cpp +++ b/Firmware/SdBaseFile.cpp @@ -534,17 +534,6 @@ bool SdBaseFile::open(const char* path, uint8_t oflag) { return open(cwd_, path, oflag); } -bool SdBaseFile::openFilteredGcode(SdBaseFile* dirFile, const char* path){ - if( open(dirFile, path, O_READ) ){ - gf.reset(0,0); - // compute the block to start with - if( ! computeNextFileBlock(&gf.block, &gf.offset) ) - return false; - return true; - } else { - return false; - } -} //------------------------------------------------------------------------------ /** Open a file or directory by name. * @@ -1043,111 +1032,6 @@ int16_t SdBaseFile::read() { return read(&b, 1) == 1 ? b : -1; } -int16_t SdBaseFile::readFilteredGcode() { - // avoid calling the default heavy-weight read() for just one byte - return gf.read_byte(); -} - -void GCodeInputFilter::reset(uint32_t blk, uint16_t ofs){ - // @@TODO clean up - block = blk; - offset = ofs; - cachePBegin = sd->vol_->cache()->data; - // reset cache read ptr to its begin - cacheP = cachePBegin; -} - -int16_t GCodeInputFilter::read_byte(){ - EnsureBlock(); // this is unfortunate :( ... other calls are using the cache and we can loose the data block of our gcode file - - // assume, we have the 512B block cache filled and terminated with a '\n' -// SERIAL_PROTOCOLPGM("read_byte enter:"); -// for(uint8_t i = 0; i < 16; ++i){ -// SERIAL_PROTOCOL( cacheP[i] ); -// } - - const uint8_t *start = cacheP; - uint8_t consecutiveCommentLines = 0; - while( *cacheP == ';' ){ - for(;;){ - while( *(++cacheP) != '\n' ); // skip until a newline is found - // found a newline, prepare the next block if block cache end reached - if( cacheP - cachePBegin >= 512 ){ - // at the end of block cache, fill new data in - sd->curPosition_ += cacheP - start; - if( ! sd->computeNextFileBlock(&block, &offset) )goto fail; - EnsureBlock(); // fetch it into RAM - cacheP = start = cachePBegin; - } else { - if(++consecutiveCommentLines == 255){ - // SERIAL_PROTOCOLLN(sd->curPosition_); - goto forceExit; - } - // peek the next byte - we are inside the block at least at 511th index - still safe - if( *(cacheP+1) == ';' ){ - // consecutive comment - ++cacheP; - ++consecutiveCommentLines; - } - break; // found the real end of the line even across many blocks - } - } - } -forceExit: - sd->curPosition_ += cacheP - start + 1; - { - int16_t rv = *cacheP++; - - // prepare next block if needed - if( cacheP - cachePBegin >= 512 ){ -// SERIAL_PROTOCOLLN(sd->curPosition_); - if( ! sd->computeNextFileBlock(&block, &offset) )goto fail; - // don't need to force fetch the block here, it will get loaded on the next call - cacheP = cachePBegin; - } - return rv; - } -fail: -// SERIAL_PROTOCOLLNPGM("CacheFAIL"); - return -1; -} - -bool GCodeInputFilter::EnsureBlock(){ - if ( sd->vol_->cacheRawBlock(block, SdVolume::CACHE_FOR_READ)){ - // terminate with a '\n' - const uint16_t terminateOfs = (sd->fileSize_ - offset) < 512 ? (sd->fileSize_ - offset) : 512; - sd->vol_->cache()->data[ terminateOfs ] = '\n'; - return true; - } else { - return false; - } -} - -bool SdBaseFile::computeNextFileBlock(uint32_t *block, uint16_t *offset) { - // error if not open or write only - if (!isOpen() || !(flags_ & O_READ)) return false; - - *offset = curPosition_ & 0X1FF; // offset in block - if (type_ == FAT_FILE_TYPE_ROOT_FIXED) { - *block = vol_->rootDirStart() + (curPosition_ >> 9); - } else { - uint8_t blockOfCluster = vol_->blockOfCluster(curPosition_); - if (*offset == 0 && blockOfCluster == 0) { - // start of new cluster - if (curPosition_ == 0) { - // use first cluster in file - curCluster_ = firstCluster_; - } else { - // get next cluster from FAT - if (!vol_->fatGet(curCluster_, &curCluster_)) return false; - } - } - *block = vol_->clusterStartBlock(curCluster_) + blockOfCluster; - } - return true; -} - - //------------------------------------------------------------------------------ /** Read data from a file starting at the current position. * @@ -1561,7 +1445,7 @@ bool SdBaseFile::rmRfStar() { * \param[in] oflag Values for \a oflag are constructed by a bitwise-inclusive * OR of open flags. see SdBaseFile::open(SdBaseFile*, const char*, uint8_t). */ -SdBaseFile::SdBaseFile(const char* path, uint8_t oflag):gf(this) { +SdBaseFile::SdBaseFile(const char* path, uint8_t oflag) { type_ = FAT_FILE_TYPE_CLOSED; writeError = false; open(path, oflag); diff --git a/Firmware/SdBaseFile.h b/Firmware/SdBaseFile.h index 4b3395875..ac39338da 100644 --- a/Firmware/SdBaseFile.h +++ b/Firmware/SdBaseFile.h @@ -176,32 +176,15 @@ uint16_t const FAT_DEFAULT_DATE = ((2000 - 1980) << 9) | (1 << 5) | 1; uint16_t const FAT_DEFAULT_TIME = (1 << 11); -class SdBaseFile; -class GCodeInputFilter { - SdBaseFile *sd; //@@TODO subject to removal - merge with some derived SdFileXX - const uint8_t *cachePBegin; - const uint8_t *cacheP; - bool EnsureBlock(); -public: - uint32_t block; // remember the current file block to be kept in cache - due to reuse of the memory, the block may fall out a must be read back - uint16_t offset; - -public: - inline GCodeInputFilter(SdBaseFile *sd):sd(sd){} - void reset(uint32_t blk, uint16_t ofs); - int16_t read_byte(); -}; - //------------------------------------------------------------------------------ /** * \class SdBaseFile * \brief Base class for SdFile with Print and C++ streams. */ class SdBaseFile { - GCodeInputFilter gf; public: /** Create an instance. */ - SdBaseFile() : gf(this), writeError(false), type_(FAT_FILE_TYPE_CLOSED) {} + SdBaseFile() : writeError(false), type_(FAT_FILE_TYPE_CLOSED) {} SdBaseFile(const char* path, uint8_t oflag); ~SdBaseFile() {if(isOpen()) close();} /** @@ -294,18 +277,13 @@ class SdBaseFile { bool open(SdBaseFile* dirFile, uint16_t index, uint8_t oflag); bool open(SdBaseFile* dirFile, const char* path, uint8_t oflag); bool open(const char* path, uint8_t oflag = O_READ); - bool openFilteredGcode(SdBaseFile* dirFile, const char* path); bool openNext(SdBaseFile* dirFile, uint8_t oflag); bool openRoot(SdVolume* vol); int peek(); static void printFatDate(uint16_t fatDate); static void printFatTime( uint16_t fatTime); bool printName(); - - int16_t readFilteredGcode(); - bool computeNextFileBlock(uint32_t *block, uint16_t *offset); - -private: +protected: int16_t read(); int16_t read(void* buf, uint16_t nbyte); public: @@ -347,8 +325,7 @@ public: SdVolume* volume() const {return vol_;} int16_t write(const void* buf, uint16_t nbyte); //------------------------------------------------------------------------------ - private: - friend class GCodeInputFilter; + protected: // allow SdFat to set cwd_ friend class SdFat; // global pointer to cwd dir diff --git a/Firmware/SdFile.cpp b/Firmware/SdFile.cpp index 2fb4d5943..4787a0930 100644 --- a/Firmware/SdFile.cpp +++ b/Firmware/SdFile.cpp @@ -30,6 +30,176 @@ */ SdFile::SdFile(const char* path, uint8_t oflag) : SdBaseFile(path, oflag) { } + +//size=100B +bool SdFile::openFilteredGcode(SdBaseFile* dirFile, const char* path){ + if( open(dirFile, path, O_READ) ){ + gfReset(0,0); + // compute the block to start with + if( ! gfComputeNextFileBlock() ) + return false; + return true; + } else { + return false; + } +} + +//size=90B +bool SdFile::seekSetFilteredGcode(uint32_t pos){ + bool rv = seekSet(pos); + gfComputeNextFileBlock(); + return rv; +} + +//size=50B +void SdFile::gfReset(uint32_t blk, uint16_t ofs){ + // @@TODO clean up + gfBlock = blk; + gfOffset = ofs; + gfCachePBegin = vol_->cache()->data; + // reset cache read ptr to its begin + gfCacheP = gfCachePBegin; +} + +//FORCE_INLINE const uint8_t * find_endl(const uint8_t *p){ +// while( *(++p) != '\n' ); // skip until a newline is found +// return p; +//} + +// think twice before allowing this to inline - manipulating 4B longs is costly +// moreover - this function has its parameters in registers only, so no heavy stack usage besides the call/ret +void __attribute__((noinline)) SdFile::gfUpdateCurrentPosition(uint16_t inc){ + curPosition_ += inc; +} + +#define find_endl(resultP, startP) \ +__asm__ __volatile__ ( \ +"cycle: \n" \ +"ld r22, Z+ \n" \ +"cpi r22, 0x0A \n" \ +"brne cycle \n" \ +: "=z" (resultP) /* result of the ASM code - in our case the Z register (R30:R31) */ \ +: "z" (startP) /* input of the ASM code - in our case the Z register as well (R30:R31) */ \ +: "r22" /* modifying register R22 - so that the compiler knows */ \ +) + +//size=400B +// avoid calling the default heavy-weight read() for just one byte +int16_t SdFile::readFilteredGcode(){ + gfEnsureBlock(); // this is unfortunate :( ... other calls are using the cache and we can loose the data block of our gcode file + + // assume, we have the 512B block cache filled and terminated with a '\n' +// SERIAL_PROTOCOLPGM("read_byte enter:"); +// for(uint8_t i = 0; i < 16; ++i){ +// SERIAL_PROTOCOL( cacheP[i] ); +// } + + const uint8_t *start = gfCacheP; + uint8_t consecutiveCommentLines = 0; + while( *gfCacheP == ';' ){ + for(;;){ + + //while( *(++gfCacheP) != '\n' ); // skip until a newline is found - suboptimal code! + // Wondering, why this "nice while cycle" is done in such a weird way using a separate find_endl() function? + // Have a look at the ASM code GCC produced! + + // At first - a separate find_endl() makes the compiler understand, + // that I don't need to store gfCacheP every time, I'm only interested in the final address where the '\n' was found + // - the cycle can run on CPU registers only without touching memory besides reading the character being compared. + // Not only makes the code run considerably faster, but is also 40B shorter! + // This was the generated code: + //FORCE_INLINE const uint8_t * find_endl(const uint8_t *p){ + // while( *(++p) != '\n' ); // skip until a newline is found + // return p; } + // 11c5e: movw r30, r18 + // 11c60: subi r18, 0xFF ; 255 + // 11c62: sbci r19, 0xFF ; 255 + // 11c64: ld r22, Z + // 11c66: cpi r22, 0x0A ; 10 + // 11c68: brne .-12 ; 0x11c5e + + // Still, even that was suboptimal as the compiler seems not to understand the usage of ld r22, Z+ (the plus is important) + // aka automatic increment of the Z register (R30:R31 pair) + // There is no other way than pure ASM! + find_endl(gfCacheP, gfCacheP); + + // found a newline, prepare the next block if block cache end reached + if( gfCacheP - gfCachePBegin >= 512 ){ + // at the end of block cache, fill new data in + gfUpdateCurrentPosition( gfCacheP - start ); + if( ! gfComputeNextFileBlock() )goto fail; + gfEnsureBlock(); // fetch it into RAM + gfCacheP = start = gfCachePBegin; + } else { + if(++consecutiveCommentLines == 255){ + // SERIAL_PROTOCOLLN(sd->curPosition_); + goto forceExit; + } + // peek the next byte - we are inside the block at least at 511th index - still safe + if( *(gfCacheP+1) == ';' ){ + // consecutive comment + ++gfCacheP; + ++consecutiveCommentLines; + } + break; // found the real end of the line even across many blocks + } + } + } +forceExit: + { + gfUpdateCurrentPosition( gfCacheP - start + 1 ); + int16_t rv = *gfCacheP++; + + // prepare next block if needed + if( gfCacheP - gfCachePBegin >= 512 ){ + if( ! gfComputeNextFileBlock() )goto fail; + // don't need to force fetch the block here, it will get loaded on the next call + gfCacheP = gfCachePBegin; + } + return rv; + } +fail: +// SERIAL_PROTOCOLLNPGM("CacheFAIL"); + return -1; +} + +//size=100B +bool SdFile::gfEnsureBlock(){ + if ( vol_->cacheRawBlock(gfBlock, SdVolume::CACHE_FOR_READ)){ + // terminate with a '\n' + const uint16_t terminateOfs = (fileSize_ - gfOffset) < 512 ? (fileSize_ - gfOffset) : 512U; + vol_->cache()->data[ terminateOfs ] = '\n'; + return true; + } else { + return false; + } +} + +//size=350B +bool SdFile::gfComputeNextFileBlock() { + // error if not open or write only + if (!isOpen() || !(flags_ & O_READ)) return false; + + gfOffset = curPosition_ & 0X1FF; // offset in block + if (type_ == FAT_FILE_TYPE_ROOT_FIXED) { + gfBlock = vol_->rootDirStart() + (curPosition_ >> 9); + } else { + uint8_t blockOfCluster = vol_->blockOfCluster(curPosition_); + if (gfOffset == 0 && blockOfCluster == 0) { + // start of new cluster + if (curPosition_ == 0) { + // use first cluster in file + curCluster_ = firstCluster_; + } else { + // get next cluster from FAT + if (!vol_->fatGet(curCluster_, &curCluster_)) return false; + } + } + gfBlock = vol_->clusterStartBlock(curCluster_) + blockOfCluster; + } + return true; +} + //------------------------------------------------------------------------------ /** Write data to an open file. * diff --git a/Firmware/SdFile.h b/Firmware/SdFile.h index 60e2f5deb..b4c91a7e8 100644 --- a/Firmware/SdFile.h +++ b/Firmware/SdFile.h @@ -34,7 +34,16 @@ * \brief SdBaseFile with Print. */ class SdFile : public SdBaseFile/*, public Print*/ { - public: + // GCode filtering vars and methods - due to optimization reasons not wrapped in a separate class + const uint8_t *gfCachePBegin; + const uint8_t *gfCacheP; + uint32_t gfBlock; // remember the current file block to be kept in cache - due to reuse of the memory, the block may fall out a must be read back + uint16_t gfOffset; + void gfReset(uint32_t blk, uint16_t ofs); + bool gfEnsureBlock(); + bool gfComputeNextFileBlock(); + void gfUpdateCurrentPosition(uint16_t inc); +public: SdFile() {} SdFile(const char* name, uint8_t oflag); #if ARDUINO >= 100 @@ -43,6 +52,9 @@ class SdFile : public SdBaseFile/*, public Print*/ { void write(uint8_t b); #endif + bool openFilteredGcode(SdBaseFile* dirFile, const char* path); + int16_t readFilteredGcode(); + bool seekSetFilteredGcode(uint32_t pos); int16_t write(const void* buf, uint16_t nbyte); void write(const char* str); void write_P(PGM_P str); @@ -51,4 +63,4 @@ class SdFile : public SdBaseFile/*, public Print*/ { #endif // SdFile_h -#endif \ No newline at end of file +#endif diff --git a/Firmware/SdVolume.h b/Firmware/SdVolume.h index 7c4fce959..17699190e 100644 --- a/Firmware/SdVolume.h +++ b/Firmware/SdVolume.h @@ -119,7 +119,7 @@ class SdVolume { bool dbgFat(uint32_t n, uint32_t* v) {return fatGet(n, v);} //------------------------------------------------------------------------------ private: - friend class GCodeInputFilter; + friend class SdFile; // Allow SdBaseFile access to SdVolume private data. friend class SdBaseFile; diff --git a/Firmware/cardreader.h b/Firmware/cardreader.h index 1c630b667..8dfb68c98 100644 --- a/Firmware/cardreader.h +++ b/Firmware/cardreader.h @@ -1,6 +1,8 @@ #ifndef CARDREADER_H #define CARDREADER_H +#define SDSUPPORT + #ifdef SDSUPPORT #define MAX_DIR_DEPTH 10 @@ -64,7 +66,7 @@ public: //@@TODO 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 void setIndex(long index) {sdpos = index;file.seekSet(index);}; + /*FORCE_INLINE*/ 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;}; FORCE_INLINE uint32_t get_sdpos() { if (!isFileOpen()) return 0; else return(sdpos); }; diff --git a/Firmware/ultralcd.cpp b/Firmware/ultralcd.cpp index 3fc5f5a66..78dddf2be 100755 --- a/Firmware/ultralcd.cpp +++ b/Firmware/ultralcd.cpp @@ -8473,10 +8473,8 @@ static void lcd_selftest_screen_step(int _row, int _col, int _state, const char /** Menu action functions **/ static bool check_file(const char* filename) { - return true; // @@TODO - if (farm_mode) return true; - card.openFileFilteredGcode((char*)filename, true); //@@TODO + card.openFileFilteredGcode((char*)filename, true); bool result = false; const uint32_t filesize = card.getFileSize(); uint32_t startPos = 0; From b6c59e08ac4ada0d78bf87b68a029d2e2c88a2f4 Mon Sep 17 00:00:00 2001 From: "D.R.racer" Date: Wed, 27 Jan 2021 09:52:20 +0100 Subject: [PATCH 04/16] Workaround ++gfCacheP into postincrement ld r22, Z+ TODO: ideally improve the automaton to work with postincrement only, at least in this case. --- Firmware/SdFile.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Firmware/SdFile.cpp b/Firmware/SdFile.cpp index 4787a0930..0d30618e2 100644 --- a/Firmware/SdFile.cpp +++ b/Firmware/SdFile.cpp @@ -74,10 +74,12 @@ void __attribute__((noinline)) SdFile::gfUpdateCurrentPosition(uint16_t inc){ #define find_endl(resultP, startP) \ __asm__ __volatile__ ( \ +"adiw r30, 1 \n" /* workaround the ++gfCacheP into post increment Z+ */ \ "cycle: \n" \ "ld r22, Z+ \n" \ "cpi r22, 0x0A \n" \ "brne cycle \n" \ +"sbiw r30, 1 \n" /* workaround the ++gfCacheP into post increment Z+ */ \ : "=z" (resultP) /* result of the ASM code - in our case the Z register (R30:R31) */ \ : "z" (startP) /* input of the ASM code - in our case the Z register as well (R30:R31) */ \ : "r22" /* modifying register R22 - so that the compiler knows */ \ From b2cf5b7b6c3818b783d63972d8507e24a84a6f24 Mon Sep 17 00:00:00 2001 From: "D.R.racer" Date: Wed, 27 Jan 2021 13:01:25 +0100 Subject: [PATCH 05/16] Fix seekSetFilteredGcode() +some more debug code which will vanish after all is done and verified --- Firmware/SdFile.cpp | 50 +++++++++++++++++++++++++++++++++++++-------- 1 file changed, 41 insertions(+), 9 deletions(-) diff --git a/Firmware/SdFile.cpp b/Firmware/SdFile.cpp index 0d30618e2..b3dfefe9a 100644 --- a/Firmware/SdFile.cpp +++ b/Firmware/SdFile.cpp @@ -46,9 +46,14 @@ bool SdFile::openFilteredGcode(SdBaseFile* dirFile, const char* path){ //size=90B bool SdFile::seekSetFilteredGcode(uint32_t pos){ - bool rv = seekSet(pos); - gfComputeNextFileBlock(); - return rv; +// SERIAL_PROTOCOLPGM("Seek:"); +// SERIAL_PROTOCOLLN(pos); + if(! seekSet(pos) )return false; + if(! gfComputeNextFileBlock() )return false; + gfCachePBegin = vol_->cache()->data; + // reset cache read ptr to its begin + gfCacheP = gfCachePBegin + gfOffset; + return true; } //size=50B @@ -91,10 +96,13 @@ int16_t SdFile::readFilteredGcode(){ gfEnsureBlock(); // this is unfortunate :( ... other calls are using the cache and we can loose the data block of our gcode file // assume, we have the 512B block cache filled and terminated with a '\n' -// SERIAL_PROTOCOLPGM("read_byte enter:"); +// SERIAL_PROTOCOLPGM("Read:"); +// SERIAL_PROTOCOL(curPosition_); +// SERIAL_PROTOCOL(':'); // for(uint8_t i = 0; i < 16; ++i){ -// SERIAL_PROTOCOL( cacheP[i] ); +// SERIAL_PROTOCOL( gfCacheP[i] ); // } +// SERIAL_PROTOCOLLN(); const uint8_t *start = gfCacheP; uint8_t consecutiveCommentLines = 0; @@ -154,6 +162,10 @@ forceExit: // prepare next block if needed if( gfCacheP - gfCachePBegin >= 512 ){ +// speed checking - now at roughly 170KB/s which is much closer to raw read speed of SD card blocks at ~250KB/s +// SERIAL_PROTOCOL(millis2()); +// SERIAL_PROTOCOL(':'); +// SERIAL_PROTOCOLLN(curPosition_); if( ! gfComputeNextFileBlock() )goto fail; // don't need to force fetch the block here, it will get loaded on the next call gfCacheP = gfCachePBegin; @@ -165,18 +177,35 @@ fail: return -1; } -//size=100B +//size=70B bool SdFile::gfEnsureBlock(){ +// SERIAL_PROTOCOLPGM("EB:"); +// SERIAL_PROTOCOLLN(gfBlock); if ( vol_->cacheRawBlock(gfBlock, SdVolume::CACHE_FOR_READ)){ // terminate with a '\n' - const uint16_t terminateOfs = (fileSize_ - gfOffset) < 512 ? (fileSize_ - gfOffset) : 512U; - vol_->cache()->data[ terminateOfs ] = '\n'; + const uint16_t terminateOfs = fileSize_ - gfOffset; + vol_->cache()->data[ terminateOfs < 512 ? terminateOfs : 512 ] = '\n'; return true; } else { return false; } } + +//#define shr9(resultCurPos, curPos) \ +//__asm__ __volatile__ ( \ +//"asr r23 \n" \ +//"asr r22 \n" \ +//"asr r21 \n" \ +//"asr r20 \n" \ +//"ldi r20, r21 \n" \ +//"ldi r21, r22 \n" \ +//"ldi r22, r23 \n" \ +//"ldi r23, 0 \n" \ +//: "=a" (resultCurPos) \ +//: "a" (curPos) \ +//) + //size=350B bool SdFile::gfComputeNextFileBlock() { // error if not open or write only @@ -184,7 +213,10 @@ bool SdFile::gfComputeNextFileBlock() { gfOffset = curPosition_ & 0X1FF; // offset in block if (type_ == FAT_FILE_TYPE_ROOT_FIXED) { - gfBlock = vol_->rootDirStart() + (curPosition_ >> 9); + // SHR by 9 means skip the last byte and shift just 3 bytes by 1 + // -> should be 8 instructions... and not the horrible loop shifting 4 bytes at once + // still need to get some work on this + gfBlock = vol_->rootDirStart() + (curPosition_ >> 9); } else { uint8_t blockOfCluster = vol_->blockOfCluster(curPosition_); if (gfOffset == 0 && blockOfCluster == 0) { From d1fd5a555f73889d3869761f826dfa4ce3c766c6 Mon Sep 17 00:00:00 2001 From: "D.R.racer" Date: Wed, 27 Jan 2021 14:12:11 +0100 Subject: [PATCH 06/16] Clean up gfReset() --- Firmware/SdFile.cpp | 13 ++++--------- Firmware/SdFile.h | 2 +- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/Firmware/SdFile.cpp b/Firmware/SdFile.cpp index b3dfefe9a..5739f45ff 100644 --- a/Firmware/SdFile.cpp +++ b/Firmware/SdFile.cpp @@ -34,10 +34,10 @@ SdFile::SdFile(const char* path, uint8_t oflag) : SdBaseFile(path, oflag) { //size=100B bool SdFile::openFilteredGcode(SdBaseFile* dirFile, const char* path){ if( open(dirFile, path, O_READ) ){ - gfReset(0,0); // compute the block to start with if( ! gfComputeNextFileBlock() ) return false; + gfReset(); return true; } else { return false; @@ -50,20 +50,15 @@ bool SdFile::seekSetFilteredGcode(uint32_t pos){ // SERIAL_PROTOCOLLN(pos); if(! seekSet(pos) )return false; if(! gfComputeNextFileBlock() )return false; - gfCachePBegin = vol_->cache()->data; - // reset cache read ptr to its begin - gfCacheP = gfCachePBegin + gfOffset; + gfReset(); return true; } //size=50B -void SdFile::gfReset(uint32_t blk, uint16_t ofs){ - // @@TODO clean up - gfBlock = blk; - gfOffset = ofs; +void SdFile::gfReset(){ gfCachePBegin = vol_->cache()->data; // reset cache read ptr to its begin - gfCacheP = gfCachePBegin; + gfCacheP = gfCachePBegin + gfOffset; } //FORCE_INLINE const uint8_t * find_endl(const uint8_t *p){ diff --git a/Firmware/SdFile.h b/Firmware/SdFile.h index b4c91a7e8..a801a2282 100644 --- a/Firmware/SdFile.h +++ b/Firmware/SdFile.h @@ -39,7 +39,7 @@ class SdFile : public SdBaseFile/*, public Print*/ { const uint8_t *gfCacheP; uint32_t gfBlock; // remember the current file block to be kept in cache - due to reuse of the memory, the block may fall out a must be read back uint16_t gfOffset; - void gfReset(uint32_t blk, uint16_t ofs); + void gfReset(); bool gfEnsureBlock(); bool gfComputeNextFileBlock(); void gfUpdateCurrentPosition(uint16_t inc); From c05b625b1cfcda507c31e84367cefbb5a32abe9b Mon Sep 17 00:00:00 2001 From: "D.R.racer" Date: Thu, 28 Jan 2021 08:13:16 +0100 Subject: [PATCH 07/16] Fix occasionally skipped valid G-code lines which also allowed for removal of the pre-increment -> post-increment workaround --- Firmware/SdFile.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/Firmware/SdFile.cpp b/Firmware/SdFile.cpp index 5739f45ff..9d898fbcf 100644 --- a/Firmware/SdFile.cpp +++ b/Firmware/SdFile.cpp @@ -74,12 +74,10 @@ void __attribute__((noinline)) SdFile::gfUpdateCurrentPosition(uint16_t inc){ #define find_endl(resultP, startP) \ __asm__ __volatile__ ( \ -"adiw r30, 1 \n" /* workaround the ++gfCacheP into post increment Z+ */ \ "cycle: \n" \ "ld r22, Z+ \n" \ "cpi r22, 0x0A \n" \ "brne cycle \n" \ -"sbiw r30, 1 \n" /* workaround the ++gfCacheP into post increment Z+ */ \ : "=z" (resultP) /* result of the ASM code - in our case the Z register (R30:R31) */ \ : "z" (startP) /* input of the ASM code - in our case the Z register as well (R30:R31) */ \ : "r22" /* modifying register R22 - so that the compiler knows */ \ @@ -129,22 +127,25 @@ int16_t SdFile::readFilteredGcode(){ find_endl(gfCacheP, gfCacheP); // found a newline, prepare the next block if block cache end reached - if( gfCacheP - gfCachePBegin >= 512 ){ + if( gfCacheP - gfCachePBegin > 512 ){ // at the end of block cache, fill new data in - gfUpdateCurrentPosition( gfCacheP - start ); + gfUpdateCurrentPosition( gfCacheP - start - 1 ); if( ! gfComputeNextFileBlock() )goto fail; gfEnsureBlock(); // fetch it into RAM gfCacheP = start = gfCachePBegin; } else { if(++consecutiveCommentLines == 255){ // SERIAL_PROTOCOLLN(sd->curPosition_); + --gfCacheP; // unget the already consumed newline goto forceExit; } // peek the next byte - we are inside the block at least at 511th index - still safe - if( *(gfCacheP+1) == ';' ){ + if( *gfCacheP == ';' ){ // consecutive comment - ++gfCacheP; ++consecutiveCommentLines; + } else { + --gfCacheP; // unget the already consumed newline + goto forceExit; } break; // found the real end of the line even across many blocks } From 7279de740382a6f1bbbfa860978ec0706128e6a4 Mon Sep 17 00:00:00 2001 From: "D.R.racer" Date: Thu, 28 Jan 2021 09:37:58 +0100 Subject: [PATCH 08/16] Separate reading G-code files and writing to a file - extract common strings - cleanup openFileWrite and openFileReadFilteredGcode formatting a bit Alltogether - code size 400B down --- Firmware/Marlin_main.cpp | 8 +- Firmware/cardreader.cpp | 174 ++++++++++++++++----------------------- Firmware/cardreader.h | 4 +- Firmware/ultralcd.cpp | 2 +- 4 files changed, 80 insertions(+), 108 deletions(-) diff --git a/Firmware/Marlin_main.cpp b/Firmware/Marlin_main.cpp index a5bd4e04a..73dd5df98 100755 --- a/Firmware/Marlin_main.cpp +++ b/Firmware/Marlin_main.cpp @@ -4005,7 +4005,7 @@ void process_commands() } else if (code_seen_P(PSTR("M28"))) { // PRUSA M28 trace(); prusa_sd_card_upload = true; - card.openFile(strchr_pointer+4,false); + card.openFileWrite(strchr_pointer+4); } else if (code_seen_P(PSTR("SN"))) { // PRUSA SN gcode_PRUSA_SN(); @@ -5769,7 +5769,7 @@ if(eSoundMode!=e_SOUND_MODE_SILENT) starpos = (strchr(strchr_pointer + 4,'*')); if(starpos!=NULL) *(starpos)='\0'; - card.openFileFilteredGcode(strchr_pointer + 4); + card.openFileReadFilteredGcode(strchr_pointer + 4); break; /*! @@ -5833,7 +5833,7 @@ if(eSoundMode!=e_SOUND_MODE_SILENT) strchr_pointer = strchr(npos,' ') + 1; *(starpos) = '\0'; } - card.openFile(strchr_pointer+4,false); + card.openFileWrite(strchr_pointer+4); break; /*! ### M29 - Stop SD write M29: Stop writing to SD card @@ -5894,7 +5894,7 @@ if(eSoundMode!=e_SOUND_MODE_SILENT) if( card.cardOK ) { - card.openFile(namestartpos,true,!call_procedure); + card.openFileReadFilteredGcode(namestartpos,!call_procedure); if(code_seen('S')) if(strchr_pointer(int)SD_PROCEDURE_DEPTH-1) - { - // SERIAL_ERROR_START; - // SERIAL_ERRORPGM("trying to call sub-gcode files with too many levels. MAX level is:"); - // SERIAL_ERRORLN(SD_PROCEDURE_DEPTH); - kill(_n("trying to call sub-gcode files with too many levels."), 1); - return; - } - - SERIAL_ECHO_START; - SERIAL_ECHOPGM("SUBROUTINE CALL target:\""); - SERIAL_ECHO(name); - SERIAL_ECHOPGM("\" parent:\""); - - //store current filename and position - getAbsFilename(filenames[file_subcall_ctr]); - - SERIAL_ECHO(filenames[file_subcall_ctr]); - SERIAL_ECHOPGM("\" pos"); - SERIAL_ECHOLN(sdpos); - filespos[file_subcall_ctr]=sdpos; - file_subcall_ctr++; - } - else - { - SERIAL_ECHO_START; - SERIAL_ECHOPGM("Now doing file: "); - SERIAL_ECHOLN(name); - } - file.close(); - } - else //opening fresh file - { - file_subcall_ctr=0; //resetting procedure depth in case user cancels print while in procedure - SERIAL_ECHO_START; - SERIAL_ECHOPGM("Now fresh file: "); - SERIAL_ECHOLN(name); - } - sdprinting = false; + if(!cardOK) + return; + if(file.isOpen()){ //replacing current file by new file, or subfile call - SdFile myDir; - const char *fname=name; - diveSubfolder(fname,myDir); + // @@TODO I doubt this is necessary for file saving: - if(read) - { - if (file.open(curDir, fname, O_READ)) - { - filesize = file.fileSize(); - SERIAL_PROTOCOLRPGM(_N("File opened: "));////MSG_SD_FILE_OPENED - SERIAL_PROTOCOL(fname); - SERIAL_PROTOCOLRPGM(_n(" Size: "));////MSG_SD_SIZE - SERIAL_PROTOCOLLN(filesize); - sdpos = 0; - - SERIAL_PROTOCOLLNRPGM(_N("File selected"));////MSG_SD_FILE_SELECTED - getfilename(0, fname); - lcd_setstatus(longFilename[0] ? longFilename : fname); - lcd_setstatuspgm(PSTR("SD-PRINTING")); + if((int)file_subcall_ctr>(int)SD_PROCEDURE_DEPTH-1){ + // SERIAL_ERROR_START; + // SERIAL_ERRORPGM("trying to call sub-gcode files with too many levels. MAX level is:"); + // SERIAL_ERRORLN(SD_PROCEDURE_DEPTH); + kill(ofKill, 1); + return; + } + + SERIAL_ECHO_START; + SERIAL_ECHORPGM(ofSubroutineCallTgt); + SERIAL_ECHO(name); + SERIAL_ECHORPGM(ofParent); + + //store current filename and position + getAbsFilename(filenames[file_subcall_ctr]); + + SERIAL_ECHO(filenames[file_subcall_ctr]); + SERIAL_ECHORPGM(ofPos); + SERIAL_ECHOLN(sdpos); + filespos[file_subcall_ctr]=sdpos; + file_subcall_ctr++; + file.close(); + } else { //opening fresh file + file_subcall_ctr=0; //resetting procedure depth in case user cancels print while in procedure + SERIAL_ECHO_START; + SERIAL_ECHORPGM(ofNowFreshFile); + SERIAL_ECHOLN(name); } - else - { - SERIAL_PROTOCOLRPGM(MSG_SD_OPEN_FILE_FAIL); - SERIAL_PROTOCOL(fname); - SERIAL_PROTOCOLLN('.'); + sdprinting = false; + + SdFile myDir; + const char *fname=name; + diveSubfolder(fname,myDir); + + //write + if (!file.open(curDir, fname, O_CREAT | O_APPEND | O_WRITE | O_TRUNC)){ + SERIAL_PROTOCOLRPGM(MSG_SD_OPEN_FILE_FAIL); + SERIAL_PROTOCOL(fname); + SERIAL_PROTOCOLLN('.'); + } else { + saving = true; + SERIAL_PROTOCOLRPGM(ofWritingToFile);////MSG_SD_WRITE_TO_FILE + SERIAL_PROTOCOLLN(name); + lcd_setstatus(fname); } - } - else - { //write - if (!file.open(curDir, fname, O_CREAT | O_APPEND | O_WRITE | O_TRUNC)) - { - SERIAL_PROTOCOLRPGM(MSG_SD_OPEN_FILE_FAIL); - SERIAL_PROTOCOL(fname); - SERIAL_PROTOCOLLN('.'); - } - else - { - saving = true; - SERIAL_PROTOCOLRPGM(_N("Writing to file: "));////MSG_SD_WRITE_TO_FILE - SERIAL_PROTOCOLLN(name); - lcd_setstatus(fname); - } - } - } void CardReader::removeFile(const char* name) @@ -1069,7 +1041,7 @@ void CardReader::printingHasFinished() { file.close(); file_subcall_ctr--; - openFile(filenames[file_subcall_ctr],true,true); + openFileReadFilteredGcode(filenames[file_subcall_ctr],true); setIndex(filespos[file_subcall_ctr]); startFileprint(); } diff --git a/Firmware/cardreader.h b/Firmware/cardreader.h index 8dfb68c98..241d5e8b8 100644 --- a/Firmware/cardreader.h +++ b/Firmware/cardreader.h @@ -21,8 +21,8 @@ public: //this is to delay autostart and hence the initialisaiton of the sd card to some seconds after the normal init, so the device is available quick after a reset void checkautostart(bool x); - void openFile(const char* name,bool read,bool replace_current=true); - void openFileFilteredGcode(const char* name, bool replace_current = false); + void openFileWrite(const char* name); + void openFileReadFilteredGcode(const char* name, bool replace_current = false); void openLogFile(const char* name); void removeFile(const char* name); void closefile(bool store_location=false); diff --git a/Firmware/ultralcd.cpp b/Firmware/ultralcd.cpp index 78dddf2be..61f022c40 100755 --- a/Firmware/ultralcd.cpp +++ b/Firmware/ultralcd.cpp @@ -8474,7 +8474,7 @@ static void lcd_selftest_screen_step(int _row, int _col, int _state, const char static bool check_file(const char* filename) { if (farm_mode) return true; - card.openFileFilteredGcode((char*)filename, true); + card.openFileReadFilteredGcode(filename, true); bool result = false; const uint32_t filesize = card.getFileSize(); uint32_t startPos = 0; From 71d825d0f2957578e27bae701e34064b2ac749ba Mon Sep 17 00:00:00 2001 From: "D.R.racer" Date: Thu, 28 Jan 2021 09:41:30 +0100 Subject: [PATCH 09/16] Return SdBaseFile into previous state no changes necessary afterall --- Firmware/SdBaseFile.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/Firmware/SdBaseFile.cpp b/Firmware/SdBaseFile.cpp index 4b19ceae6..e3b1c18c8 100644 --- a/Firmware/SdBaseFile.cpp +++ b/Firmware/SdBaseFile.cpp @@ -533,7 +533,6 @@ bool SdBaseFile::mkdir(SdBaseFile* parent, const uint8_t dname[11]) { bool SdBaseFile::open(const char* path, uint8_t oflag) { return open(cwd_, path, oflag); } - //------------------------------------------------------------------------------ /** Open a file or directory by name. * @@ -1031,7 +1030,6 @@ int16_t SdBaseFile::read() { uint8_t b; return read(&b, 1) == 1 ? b : -1; } - //------------------------------------------------------------------------------ /** Read data from a file starting at the current position. * From 6c9c1423c6ba173461bc42ab2acc1f594f140124 Mon Sep 17 00:00:00 2001 From: "D.R.racer" Date: Thu, 28 Jan 2021 09:42:50 +0100 Subject: [PATCH 10/16] Remove temporary changes from SdBaseFile.h --- Firmware/SdBaseFile.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/Firmware/SdBaseFile.h b/Firmware/SdBaseFile.h index ac39338da..a6bd311fe 100644 --- a/Firmware/SdBaseFile.h +++ b/Firmware/SdBaseFile.h @@ -174,8 +174,6 @@ static inline uint8_t FAT_SECOND(uint16_t fatTime) { uint16_t const FAT_DEFAULT_DATE = ((2000 - 1980) << 9) | (1 << 5) | 1; /** Default time for file timestamp is 1 am */ uint16_t const FAT_DEFAULT_TIME = (1 << 11); - - //------------------------------------------------------------------------------ /** * \class SdBaseFile From caf58b16b6c78986cd1510e2730960e5d18fa924 Mon Sep 17 00:00:00 2001 From: "D.R.racer" Date: Fri, 29 Jan 2021 08:29:51 +0100 Subject: [PATCH 11/16] Fix handling EOF + save ~160B by using local variables + rename some of the vars to more descriptive names + remove consecutiveEmptyLines handling from cmdqueue --- Firmware/SdFile.cpp | 102 +++++++++++++++++++++++++++--------------- Firmware/SdFile.h | 12 ++++- Firmware/cmdqueue.cpp | 6 +-- 3 files changed, 78 insertions(+), 42 deletions(-) diff --git a/Firmware/SdFile.cpp b/Firmware/SdFile.cpp index 9d898fbcf..19e0fad50 100644 --- a/Firmware/SdFile.cpp +++ b/Firmware/SdFile.cpp @@ -54,11 +54,13 @@ bool SdFile::seekSetFilteredGcode(uint32_t pos){ return true; } -//size=50B +const uint8_t *SdFile::gfBlockBuffBegin() const { + return vol_->cache()->data; // this is constant for the whole time, so it should be fast and sleek +} + void SdFile::gfReset(){ - gfCachePBegin = vol_->cache()->data; // reset cache read ptr to its begin - gfCacheP = gfCachePBegin + gfOffset; + gfReadPtr = gfBlockBuffBegin() + gfOffset; } //FORCE_INLINE const uint8_t * find_endl(const uint8_t *p){ @@ -86,28 +88,43 @@ __asm__ __volatile__ ( \ //size=400B // avoid calling the default heavy-weight read() for just one byte int16_t SdFile::readFilteredGcode(){ - gfEnsureBlock(); // this is unfortunate :( ... other calls are using the cache and we can loose the data block of our gcode file - + if( ! gfEnsureBlock() ){ + goto eof_or_fail; // this is unfortunate :( ... other calls are using the cache and we can loose the data block of our gcode file + } // assume, we have the 512B block cache filled and terminated with a '\n' // SERIAL_PROTOCOLPGM("Read:"); // SERIAL_PROTOCOL(curPosition_); // SERIAL_PROTOCOL(':'); // for(uint8_t i = 0; i < 16; ++i){ -// SERIAL_PROTOCOL( gfCacheP[i] ); +// SERIAL_PROTOCOL( gfReadPtr[i] ); // } // SERIAL_PROTOCOLLN(); +// SERIAL_PROTOCOLLN(curPosition_); + { + const uint8_t *start = gfReadPtr; + + // It may seem unreasonable to copy the variable into a local one and copy it back at the end of this method, + // but there is an important point of view: the compiler is unsure whether it can optimize the reads/writes + // to gfReadPtr within this method, because it is a class member variable. + // The compiler cannot see, if omitting read/write won't have any incorrect side-effects to the rest of the whole FW. + // So this trick explicitly states, that rdPtr is a local variable limited to the scope of this method, + // therefore the compiler can omit read/write to it (keep it in registers!) as it sees fit. + // And it does! Codesize dropped by 68B! + const uint8_t *rdPtr = gfReadPtr; + + // the same applies to gfXBegin, codesize dropped another 100B! + const uint8_t *blockBuffBegin = gfBlockBuffBegin(); - const uint8_t *start = gfCacheP; uint8_t consecutiveCommentLines = 0; - while( *gfCacheP == ';' ){ + while( *rdPtr == ';' ){ for(;;){ - //while( *(++gfCacheP) != '\n' ); // skip until a newline is found - suboptimal code! + //while( *(++gfReadPtr) != '\n' ); // skip until a newline is found - suboptimal code! // Wondering, why this "nice while cycle" is done in such a weird way using a separate find_endl() function? // Have a look at the ASM code GCC produced! // At first - a separate find_endl() makes the compiler understand, - // that I don't need to store gfCacheP every time, I'm only interested in the final address where the '\n' was found + // that I don't need to store gfReadPtr every time, I'm only interested in the final address where the '\n' was found // - the cycle can run on CPU registers only without touching memory besides reading the character being compared. // Not only makes the code run considerably faster, but is also 40B shorter! // This was the generated code: @@ -124,52 +141,67 @@ int16_t SdFile::readFilteredGcode(){ // Still, even that was suboptimal as the compiler seems not to understand the usage of ld r22, Z+ (the plus is important) // aka automatic increment of the Z register (R30:R31 pair) // There is no other way than pure ASM! - find_endl(gfCacheP, gfCacheP); + find_endl(rdPtr, rdPtr); // found a newline, prepare the next block if block cache end reached - if( gfCacheP - gfCachePBegin > 512 ){ + if( rdPtr - blockBuffBegin > 512 ){ // at the end of block cache, fill new data in - gfUpdateCurrentPosition( gfCacheP - start - 1 ); - if( ! gfComputeNextFileBlock() )goto fail; - gfEnsureBlock(); // fetch it into RAM - gfCacheP = start = gfCachePBegin; + gfUpdateCurrentPosition( rdPtr - start - 1 ); + if( ! gfComputeNextFileBlock() )goto eof_or_fail; + if( ! gfEnsureBlock() )goto eof_or_fail; // fetch it into RAM + rdPtr = start = blockBuffBegin; } else { if(++consecutiveCommentLines == 255){ // SERIAL_PROTOCOLLN(sd->curPosition_); - --gfCacheP; // unget the already consumed newline - goto forceExit; + --rdPtr; // unget the already consumed newline + goto emit_char; } // peek the next byte - we are inside the block at least at 511th index - still safe - if( *gfCacheP == ';' ){ + if( *rdPtr == ';' ){ // consecutive comment ++consecutiveCommentLines; } else { - --gfCacheP; // unget the already consumed newline - goto forceExit; + --rdPtr; // unget the already consumed newline + goto emit_char; } break; // found the real end of the line even across many blocks } } } -forceExit: +emit_char: { - gfUpdateCurrentPosition( gfCacheP - start + 1 ); - int16_t rv = *gfCacheP++; + gfUpdateCurrentPosition( rdPtr - start + 1 ); + int16_t rv = *rdPtr++; - // prepare next block if needed - if( gfCacheP - gfCachePBegin >= 512 ){ -// speed checking - now at roughly 170KB/s which is much closer to raw read speed of SD card blocks at ~250KB/s -// SERIAL_PROTOCOL(millis2()); -// SERIAL_PROTOCOL(':'); -// SERIAL_PROTOCOLLN(curPosition_); - if( ! gfComputeNextFileBlock() )goto fail; + if( curPosition_ >= fileSize_ ){ + // past the end of file + goto eof_or_fail; + } else if( rdPtr - blockBuffBegin >= 512 ){ + // past the end of current bufferred block - prepare the next one... + if( ! gfComputeNextFileBlock() )goto eof_or_fail; // don't need to force fetch the block here, it will get loaded on the next call - gfCacheP = gfCachePBegin; - } + rdPtr = blockBuffBegin; + } + +// SERIAL_PROTOCOLPGM("c="); +// SERIAL_ECHO((char)rv); +// SERIAL_ECHO('|'); +// SERIAL_ECHO((int)rv); +// SERIAL_PROTOCOL('|'); +// SERIAL_PROTOCOLLN(curPosition_); + + // save the current read ptr for the next run + gfReadPtr = rdPtr; return rv; } -fail: -// SERIAL_PROTOCOLLNPGM("CacheFAIL"); + +} + +eof_or_fail: +// SERIAL_PROTOCOLPGM("CacheFAIL:"); + + // make the rdptr point to a safe location - end of file + gfReadPtr = gfBlockBuffBegin() + 512; return -1; } diff --git a/Firmware/SdFile.h b/Firmware/SdFile.h index a801a2282..30a4da5d3 100644 --- a/Firmware/SdFile.h +++ b/Firmware/SdFile.h @@ -35,11 +35,19 @@ */ class SdFile : public SdBaseFile/*, public Print*/ { // GCode filtering vars and methods - due to optimization reasons not wrapped in a separate class - const uint8_t *gfCachePBegin; - const uint8_t *gfCacheP; + + // beware - this read ptr is manipulated inside just 2 methods - readFilteredGcode and gfReset + // If you even want to call gfReset from readFilteredGcode, you must make sure + // to update gfCacheP inside readFilteredGcode from a local copy (see explanation of this trick in readFilteredGcode) + const uint8_t *gfReadPtr; + uint32_t gfBlock; // remember the current file block to be kept in cache - due to reuse of the memory, the block may fall out a must be read back uint16_t gfOffset; + + const uint8_t *gfBlockBuffBegin()const; + void gfReset(); + bool gfEnsureBlock(); bool gfComputeNextFileBlock(); void gfUpdateCurrentPosition(uint16_t inc); diff --git a/Firmware/cmdqueue.cpp b/Firmware/cmdqueue.cpp index c89032750..d7a760c77 100755 --- a/Firmware/cmdqueue.cpp +++ b/Firmware/cmdqueue.cpp @@ -573,7 +573,6 @@ void get_command() // this character _can_ occur in serial com, due to checksums. however, no checksums are used in SD printing static bool stop_buffering=false; -// static uint8_t consecutiveEmptyLines = 0; if(buflen==0) stop_buffering=false; union { struct { @@ -604,10 +603,7 @@ void get_command() // so that the lenght of the already read empty lines and comments will be added // to the following non-empty line. // comment_mode = false; -// if( ++consecutiveEmptyLines > 10 ){ -// consecutiveEmptyLines = 0; - return; // prevent cycling indefinitely - let manage_heaters do their job -// } + return; // prevent cycling indefinitely - let manage_heaters do their job // continue; //if empty line } // The new command buffer could be updated non-atomically, because it is not yet considered From 15d76a75018e4af0af04d5060d29c5e5fb79c28a Mon Sep 17 00:00:00 2001 From: "D.R.racer" Date: Tue, 2 Feb 2021 07:57:06 +0100 Subject: [PATCH 12/16] Remove duplicit incrementation of consecutive comment lines It was left in the code in one of the refactoring/optimization passes. It really didn't do any harm, but was limiting the performance of the skipping algorithm. + some verification code added - will be removed after successful tests --- Firmware/SdFile.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Firmware/SdFile.cpp b/Firmware/SdFile.cpp index 19e0fad50..ee91bdfc9 100644 --- a/Firmware/SdFile.cpp +++ b/Firmware/SdFile.cpp @@ -151,7 +151,9 @@ int16_t SdFile::readFilteredGcode(){ if( ! gfEnsureBlock() )goto eof_or_fail; // fetch it into RAM rdPtr = start = blockBuffBegin; } else { - if(++consecutiveCommentLines == 255){ + if(consecutiveCommentLines >= 250){ +// SERIAL_ECHO("ccl="); +// SERIAL_ECHOLN((int)consecutiveCommentLines); // SERIAL_PROTOCOLLN(sd->curPosition_); --rdPtr; // unget the already consumed newline goto emit_char; From c1ead75a733f2a05690c1919b7d95277efef2e60 Mon Sep 17 00:00:00 2001 From: "D.R.racer" Date: Wed, 10 Feb 2021 11:18:59 +0100 Subject: [PATCH 13/16] Remove commented debug code the whole PR is ready for review after successfull tests --- Firmware/Marlin_main.cpp | 8 +++---- Firmware/SdFile.cpp | 49 ---------------------------------------- Firmware/SdFile.h | 2 +- Firmware/cardreader.cpp | 8 +++++-- Firmware/cardreader.h | 7 +++--- Firmware/cmdqueue.cpp | 15 ++++-------- 6 files changed, 18 insertions(+), 71 deletions(-) diff --git a/Firmware/Marlin_main.cpp b/Firmware/Marlin_main.cpp index 73dd5df98..62734d420 100755 --- a/Firmware/Marlin_main.cpp +++ b/Firmware/Marlin_main.cpp @@ -3987,12 +3987,10 @@ void process_commands() #endif }else if (code_seen_P("fv")) { // PRUSA fv // get file version - #if 0 - //@@TODO - def SDSUPPORT - card.openFile(strchr_pointer + 3,true); + #ifdef SDSUPPORT + card.openFileReadFilteredGcode(strchr_pointer + 3,true); while (true) { - uint16_t readByte = card.get(); + uint16_t readByte = card.getFilteredGcodeChar(); MYSERIAL.write(readByte); if (readByte=='\n') { break; diff --git a/Firmware/SdFile.cpp b/Firmware/SdFile.cpp index ee91bdfc9..1bad4319f 100644 --- a/Firmware/SdFile.cpp +++ b/Firmware/SdFile.cpp @@ -31,7 +31,6 @@ SdFile::SdFile(const char* path, uint8_t oflag) : SdBaseFile(path, oflag) { } -//size=100B bool SdFile::openFilteredGcode(SdBaseFile* dirFile, const char* path){ if( open(dirFile, path, O_READ) ){ // compute the block to start with @@ -44,10 +43,7 @@ bool SdFile::openFilteredGcode(SdBaseFile* dirFile, const char* path){ } } -//size=90B bool SdFile::seekSetFilteredGcode(uint32_t pos){ -// SERIAL_PROTOCOLPGM("Seek:"); -// SERIAL_PROTOCOLLN(pos); if(! seekSet(pos) )return false; if(! gfComputeNextFileBlock() )return false; gfReset(); @@ -63,11 +59,6 @@ void SdFile::gfReset(){ gfReadPtr = gfBlockBuffBegin() + gfOffset; } -//FORCE_INLINE const uint8_t * find_endl(const uint8_t *p){ -// while( *(++p) != '\n' ); // skip until a newline is found -// return p; -//} - // think twice before allowing this to inline - manipulating 4B longs is costly // moreover - this function has its parameters in registers only, so no heavy stack usage besides the call/ret void __attribute__((noinline)) SdFile::gfUpdateCurrentPosition(uint16_t inc){ @@ -85,21 +76,12 @@ __asm__ __volatile__ ( \ : "r22" /* modifying register R22 - so that the compiler knows */ \ ) -//size=400B // avoid calling the default heavy-weight read() for just one byte int16_t SdFile::readFilteredGcode(){ if( ! gfEnsureBlock() ){ goto eof_or_fail; // this is unfortunate :( ... other calls are using the cache and we can loose the data block of our gcode file } // assume, we have the 512B block cache filled and terminated with a '\n' -// SERIAL_PROTOCOLPGM("Read:"); -// SERIAL_PROTOCOL(curPosition_); -// SERIAL_PROTOCOL(':'); -// for(uint8_t i = 0; i < 16; ++i){ -// SERIAL_PROTOCOL( gfReadPtr[i] ); -// } -// SERIAL_PROTOCOLLN(); -// SERIAL_PROTOCOLLN(curPosition_); { const uint8_t *start = gfReadPtr; @@ -152,9 +134,6 @@ int16_t SdFile::readFilteredGcode(){ rdPtr = start = blockBuffBegin; } else { if(consecutiveCommentLines >= 250){ -// SERIAL_ECHO("ccl="); -// SERIAL_ECHOLN((int)consecutiveCommentLines); - // SERIAL_PROTOCOLLN(sd->curPosition_); --rdPtr; // unget the already consumed newline goto emit_char; } @@ -185,13 +164,6 @@ emit_char: rdPtr = blockBuffBegin; } -// SERIAL_PROTOCOLPGM("c="); -// SERIAL_ECHO((char)rv); -// SERIAL_ECHO('|'); -// SERIAL_ECHO((int)rv); -// SERIAL_PROTOCOL('|'); -// SERIAL_PROTOCOLLN(curPosition_); - // save the current read ptr for the next run gfReadPtr = rdPtr; return rv; @@ -200,17 +172,12 @@ emit_char: } eof_or_fail: -// SERIAL_PROTOCOLPGM("CacheFAIL:"); - // make the rdptr point to a safe location - end of file gfReadPtr = gfBlockBuffBegin() + 512; return -1; } -//size=70B bool SdFile::gfEnsureBlock(){ -// SERIAL_PROTOCOLPGM("EB:"); -// SERIAL_PROTOCOLLN(gfBlock); if ( vol_->cacheRawBlock(gfBlock, SdVolume::CACHE_FOR_READ)){ // terminate with a '\n' const uint16_t terminateOfs = fileSize_ - gfOffset; @@ -221,22 +188,6 @@ bool SdFile::gfEnsureBlock(){ } } - -//#define shr9(resultCurPos, curPos) \ -//__asm__ __volatile__ ( \ -//"asr r23 \n" \ -//"asr r22 \n" \ -//"asr r21 \n" \ -//"asr r20 \n" \ -//"ldi r20, r21 \n" \ -//"ldi r21, r22 \n" \ -//"ldi r22, r23 \n" \ -//"ldi r23, 0 \n" \ -//: "=a" (resultCurPos) \ -//: "a" (curPos) \ -//) - -//size=350B bool SdFile::gfComputeNextFileBlock() { // error if not open or write only if (!isOpen() || !(flags_ & O_READ)) return false; diff --git a/Firmware/SdFile.h b/Firmware/SdFile.h index 30a4da5d3..465224623 100644 --- a/Firmware/SdFile.h +++ b/Firmware/SdFile.h @@ -38,7 +38,7 @@ class SdFile : public SdBaseFile/*, public Print*/ { // beware - this read ptr is manipulated inside just 2 methods - readFilteredGcode and gfReset // If you even want to call gfReset from readFilteredGcode, you must make sure - // to update gfCacheP inside readFilteredGcode from a local copy (see explanation of this trick in readFilteredGcode) + // to update gfReadPtr inside readFilteredGcode from a local copy (see explanation of this trick in readFilteredGcode) const uint8_t *gfReadPtr; uint32_t gfBlock; // remember the current file block to be kept in cache - due to reuse of the memory, the block may fall out a must be read back diff --git a/Firmware/cardreader.cpp b/Firmware/cardreader.cpp index 7d709475e..54f3b593a 100644 --- a/Firmware/cardreader.cpp +++ b/Firmware/cardreader.cpp @@ -456,8 +456,9 @@ void CardReader::openFileWrite(const char* name) if(!cardOK) return; if(file.isOpen()){ //replacing current file by new file, or subfile call - - // @@TODO I doubt this is necessary for file saving: +#if 0 + // I doubt chained files support is necessary for file saving: + // Intentionally disabled because it takes a lot of code size while being not used if((int)file_subcall_ctr>(int)SD_PROCEDURE_DEPTH-1){ // SERIAL_ERROR_START; @@ -481,6 +482,9 @@ void CardReader::openFileWrite(const char* name) filespos[file_subcall_ctr]=sdpos; file_subcall_ctr++; file.close(); +#else + SERIAL_ECHOLNPGM("File already opened"); +#endif } else { //opening fresh file file_subcall_ctr=0; //resetting procedure depth in case user cancels print while in procedure SERIAL_ECHO_START; diff --git a/Firmware/cardreader.h b/Firmware/cardreader.h index 241d5e8b8..f8d6e628e 100644 --- a/Firmware/cardreader.h +++ b/Firmware/cardreader.h @@ -61,12 +61,11 @@ public: #endif FORCE_INLINE bool isFileOpen() { return file.isOpen(); } - FORCE_INLINE bool eof() { return sdpos>=filesize ;}; -// FORCE_INLINE int16_t getX() { sdpos = file.curPosition();return (int16_t)file.read();}; - //@@TODO potential performance problem - when the comment reading fails, sdpos points to the last correctly read character. + 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*/ void setIndex(long index) {sdpos = index;file.seekSetFilteredGcode(index);}; + 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;}; FORCE_INLINE uint32_t get_sdpos() { if (!isFileOpen()) return 0; else return(sdpos); }; diff --git a/Firmware/cmdqueue.cpp b/Firmware/cmdqueue.cpp index d7a760c77..9c822dab5 100755 --- a/Firmware/cmdqueue.cpp +++ b/Firmware/cmdqueue.cpp @@ -588,7 +588,7 @@ void get_command() char serial_char = (char)n; if( serial_char == '\n' || serial_char == '\r' - || ((serial_char == '#' || serial_char == ':') /*&& comment_mode == false*/) + || ((serial_char == '#' || serial_char == ':') ) || serial_count >= (MAX_CMD_SIZE - 1) || n==-1 ){ @@ -602,9 +602,7 @@ void get_command() // read from the sdcard into sd_count, // so that the lenght of the already read empty lines and comments will be added // to the following non-empty line. -// comment_mode = false; return; // prevent cycling indefinitely - let manage_heaters do their job -// continue; //if empty line } // The new command buffer could be updated non-atomically, because it is not yet considered // to be inside the active queue. @@ -620,10 +618,10 @@ void get_command() // MYSERIAL.print(sd_count.value, DEC); // SERIAL_ECHOPGM(") "); // SERIAL_ECHOLN(cmdbuffer+bufindw+CMDHDRSIZE); -// SERIAL_ECHOPGM("cmdbuffer:"); -// MYSERIAL.print(cmdbuffer); -// SERIAL_ECHOPGM("buflen:"); -// MYSERIAL.print(buflen+1); +// SERIAL_ECHOPGM("cmdbuffer:"); +// MYSERIAL.print(cmdbuffer); +// SERIAL_ECHOPGM("buflen:"); +// MYSERIAL.print(buflen+1); sd_count.value = 0; cli(); @@ -640,7 +638,6 @@ void get_command() comment_mode = false; //for new command serial_count = 0; //clear buffer -// consecutiveEmptyLines = 0; // reached a non-empty line which shall be enqueued if(card.eof()) break; @@ -650,8 +647,6 @@ void get_command() } else { - /*if(serial_char == ';') comment_mode = true; - else if(!comment_mode)*/ // there are no comments coming from the filtered file cmdbuffer[bufindw+CMDHDRSIZE+serial_count++] = serial_char; } From 8d39880abf860c630ddaa514f617478a4e5b03a3 Mon Sep 17 00:00:00 2001 From: "D.R.racer" Date: Wed, 10 Feb 2021 12:23:02 +0100 Subject: [PATCH 14/16] Fix compilation against latest MK3 branch --- Firmware/cardreader.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Firmware/cardreader.cpp b/Firmware/cardreader.cpp index 57113124a..afcbad736 100644 --- a/Firmware/cardreader.cpp +++ b/Firmware/cardreader.cpp @@ -506,9 +506,9 @@ void CardReader::openFileWrite(const char* name) } sdprinting = false; - SdFile myDir; const char *fname=name; - diveSubfolder(fname,myDir); + if (!diveSubfolder(fname)) + return; //write if (!file.open(curDir, fname, O_CREAT | O_APPEND | O_WRITE | O_TRUNC)){ From 7ad922e87b39485132d33bb06aee9e4b52c32ab5 Mon Sep 17 00:00:00 2001 From: "D.R.racer" Date: Wed, 10 Feb 2021 12:38:04 +0100 Subject: [PATCH 15/16] Report fname instead of name looks like being omitted in MK3 upstream fixes --- Firmware/cardreader.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Firmware/cardreader.cpp b/Firmware/cardreader.cpp index afcbad736..6fe890095 100644 --- a/Firmware/cardreader.cpp +++ b/Firmware/cardreader.cpp @@ -518,7 +518,7 @@ void CardReader::openFileWrite(const char* name) } else { saving = true; SERIAL_PROTOCOLRPGM(ofWritingToFile);////MSG_SD_WRITE_TO_FILE - SERIAL_PROTOCOLLN(name); + SERIAL_PROTOCOLLN(fname); lcd_setstatus(fname); } } From e010ca8ceb87e875d9353b55b8b69e97dca91cc5 Mon Sep 17 00:00:00 2001 From: Yuri D'Elia Date: Thu, 4 Feb 2021 20:36:06 +0100 Subject: [PATCH 16/16] Fix conflicting extern/inline declarations The functions find_bed_induction_sensor_point_* have conflicting extern and inline declarations. These are used outside of the compilation unit only, and thus there's no point in defining them inline. This causes a compilation failure at O1 and above, which is strangely avoided at Os. --- Firmware/mesh_bed_calibration.cpp | 8 ++++---- Firmware/mesh_bed_calibration.h | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Firmware/mesh_bed_calibration.cpp b/Firmware/mesh_bed_calibration.cpp index a0efc3aae..fb79022ff 100644 --- a/Firmware/mesh_bed_calibration.cpp +++ b/Firmware/mesh_bed_calibration.cpp @@ -944,7 +944,7 @@ static inline void update_current_position_z() // At the current position, find the Z stop. -inline bool find_bed_induction_sensor_point_z(float minimum_z, uint8_t n_iter, int +bool find_bed_induction_sensor_point_z(float minimum_z, uint8_t n_iter, int #ifdef SUPPORT_VERBOSITY verbosity_level #endif //SUPPORT_VERBOSITY @@ -1065,7 +1065,7 @@ error: } #ifdef NEW_XYZCAL -extern bool xyzcal_find_bed_induction_sensor_point_xy(); +bool xyzcal_find_bed_induction_sensor_point_xy(); #endif //NEW_XYZCAL // Search around the current_position[X,Y], // look for the induction sensor response. @@ -1081,7 +1081,7 @@ extern bool xyzcal_find_bed_induction_sensor_point_xy(); #endif //HEATBED_V2 #ifdef HEATBED_V2 -inline bool find_bed_induction_sensor_point_xy(int +bool find_bed_induction_sensor_point_xy(int #if !defined (NEW_XYZCAL) && defined (SUPPORT_VERBOSITY) verbosity_level #endif @@ -1335,7 +1335,7 @@ inline bool find_bed_induction_sensor_point_xy(int #endif //NEW_XYZCAL } #else //HEATBED_V2 -inline bool find_bed_induction_sensor_point_xy(int verbosity_level) +bool find_bed_induction_sensor_point_xy(int verbosity_level) { #ifdef NEW_XYZCAL return xyzcal_find_bed_induction_sensor_point_xy(); diff --git a/Firmware/mesh_bed_calibration.h b/Firmware/mesh_bed_calibration.h index 8adc1c119..7ca93c953 100644 --- a/Firmware/mesh_bed_calibration.h +++ b/Firmware/mesh_bed_calibration.h @@ -146,9 +146,9 @@ inline bool world2machine_clamp(float &x, float &y) return clamped; } -extern bool find_bed_induction_sensor_point_z(float minimum_z = -10.f, uint8_t n_iter = 3, int verbosity_level = 0); -extern bool find_bed_induction_sensor_point_xy(int verbosity_level = 0); -extern void go_home_with_z_lift(); +bool find_bed_induction_sensor_point_z(float minimum_z = -10.f, uint8_t n_iter = 3, int verbosity_level = 0); +bool find_bed_induction_sensor_point_xy(int verbosity_level = 0); +void go_home_with_z_lift(); /** * @brief Bed skew and offest detection result