From 017e17801cdd69ddc29265acd0aa18c95963499f Mon Sep 17 00:00:00 2001 From: Markus Hitter Date: Thu, 9 Jul 2015 14:02:47 +0200 Subject: [PATCH] SD card: move parsing closer to the metal. This means a custom function in pff_diskio.c, disk_parsep(), which replaces disk_readp(). Now it no longer reads a whole sector for every single byte, but only once per line of G-code. Performance is now much better than with buffered reading, the 1 MB file with 16'468 lines of comments, 64 bytes per line, read and parses now in just 46 seconds, which means 358 lines or 22'795 bytes per second. Unfortunately all the binary size loss is gone compared to the previous version, we're 68 bytes bigger than before buffer-less parsing. Reason is the new function disk_parsep() in pff_diskio.h. RAM usage is still low. ATmega... '168 '328(P) '644(P) '1280 FLASH: 22276 bytes 155.39% 72.51% 35.09% 17.27% RAM: 1331 bytes 129.98% 64.99% 32.50% 16.25% EEPROM: 32 bytes 3.12% 1.56% 1.56% 0.78% --- pff.c | 15 +++-------- pff_diskio.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++++ pff_diskio.h | 5 +++- 3 files changed, 82 insertions(+), 12 deletions(-) diff --git a/pff.c b/pff.c index 714bf27..dfc69dc 100644 --- a/pff.c +++ b/pff.c @@ -959,7 +959,6 @@ FRESULT pf_parse_line ( should return 1 on EOL, else zero. */ ) { - BYTE one_byte_buffer; DRESULT dr; CLUST clst; DWORD sect; @@ -987,17 +986,11 @@ FRESULT pf_parse_line ( if (!sect) ABORT(FR_DISK_ERR); fs->dsect = sect + cs; } - rcnt = 1; - // TODO: this is very inefficient, it reads a full sector for every - // single byte. Instead the parser function should be passed to - // disk_readp(), so it can read and parse the remaining sector - // in search for an EOL and pass back the number of bytes read. - // It should also pass back wether the line was completed or not. - // If yes, we're done; if no, we call again with the next sector. - dr = disk_readp(&one_byte_buffer, fs->dsect, (UINT)fs->fptr % 512, 1); - if (dr) ABORT(FR_DISK_ERR); + dr = disk_parsep(fs->dsect, (UINT)fs->fptr % 512, &rcnt, parser); + if (dr != RES_OK && dr != RES_EOL_FOUND) + ABORT(FR_DISK_ERR); fs->fptr += rcnt; /* Update pointers and counters */ - if (parser(one_byte_buffer)) + if (dr == RES_EOL_FOUND) return FR_OK; } diff --git a/pff_diskio.c b/pff_diskio.c index baeada1..d3dfa2b 100644 --- a/pff_diskio.c +++ b/pff_diskio.c @@ -209,6 +209,80 @@ DRESULT disk_readp(BYTE* buffer, DWORD sector, UINT offset, UINT count) { return result; } + +/** Read and parse a line. + + \param sector Sector number (LBA). + \param offset Offset into the sector. + \param count Number of bytes read. + \param parser Pointer to the parser function, which should return 1 on EOL, + else zero. + + \return RES_OK on success, else RES_ERROR. + + This starts reading a sector at offset and sends each character to the + parser. The parser reports back wether an end of line (EOL) was reached, + which ends this function. If end of the sector is reached without finding + an EOL, this function should be called again with the next sector of the + file. + + Reading lines of code this way should be more efficient than reading all the + bytes into a small, not line-aligned buffer, just to read this buffer(s) + right again byte by byte for parsing. It makes buffering entirely obsolete. +*/ +DRESULT disk_parsep(DWORD sector, UINT offset, UINT* count, + uint8_t (*parser)(uint8_t)) { + DRESULT result; + BYTE token = 0xFF; + uint16_t timeout; + UINT trailing = 514 - offset, read = 0; /* 514 = block size + 2 bytes CRC */ + + /* Convert to byte address on non-block cards. */ + if ( ! (card_type & CT_BLOCK)) + sector *= 512; + + /* Read one sector, copy only as many bytes as required. */ + result = RES_ERROR; + spi_speed_max(); + if (send_cmd(CMD17, sector) == 0) { /* Read single block. */ + /* Wait for data packet. */ + for (timeout = 40000; timeout && (token == 0xFF); timeout--) + token = spi_rw(0xFF); + + if (token == 0xFE) { /* Valid data arrived. */ + /* Skip leading bytes. */ + while (offset--) + spi_rw(0xFF); + + /* Receive and parse the sector up to EOL or end of the sector. */ + do { + /** + Note that this isn't optimised for performance. See + http://www.matuschek.net/atmega-spi/. + */ + result = parser(spi_rw(0xFF)); + read++; + trailing--; + } while (trailing > 2 && ! result); + + *count = read; + + /* Skip trailing bytes and CRC. */ + while (trailing--) + spi_rw(0xFF); + + if (result) + result = RES_EOL_FOUND; + else + result = RES_OK; + } + } + + spi_deselect_sd(); /* Every send_cmd() selects. */ + spi_rw(0xFF); + + return result; +} #endif /* _USE_READ */ /** Write partial pector. diff --git a/pff_diskio.h b/pff_diskio.h index 5546a3b..692693f 100644 --- a/pff_diskio.h +++ b/pff_diskio.h @@ -26,7 +26,8 @@ typedef enum { RES_OK = 0, /* 0: Function succeeded */ RES_ERROR, /* 1: Disk error */ RES_NOTRDY, /* 2: Not ready */ - RES_PARERR /* 3: Invalid parameter */ + RES_PARERR, /* 3: Invalid parameter */ + RES_EOL_FOUND, /* 4: Function succeeded and also found an EOL. */ } DRESULT; @@ -36,6 +37,8 @@ typedef enum { DSTATUS disk_initialize (void); #if _USE_READ DRESULT disk_readp (BYTE* buffer, DWORD sector, UINT offset, UINT count); + DRESULT disk_parsep (DWORD sector, UINT offset, UINT* count, + uint8_t (*parser)(uint8_t)); #endif #if _USE_WRITE DRESULT disk_writep (const BYTE* buffer, DWORD sc);