Parsing gcode from SD Cards - and end/abort

[illuminarti]

gcode from SD cards is parsed differently, depending on whether it was created on windows or mac.

Windows uses \r\n to delimit lines. So, in Marlin_main.cpp at line 637, the \r gets treated as the end of the command, and it is buffered. The code then loops, and reads the \n next. This activates the 'empty line' code at line 658, and the parser 'returns', going on to process the gcode in the line it just buffered, monitor timers, etc, before returning to read and buffer the next line in the next pass through the main loop.

On the other hand, on a Mac, only \n is used as a delimiter. Therefore, the 'empty line' code is almost never triggered, and after reading one command, the code loops and reads and buffers the next, and the next until the buffer is full. Only then does it go and parse and act on the gcode in the first line that was read.

As a result, gcode from a Mac will typically keep the buffer full, while gcode from a pc will keep just 1 or 0 lines in the buffer.

I think that the intent was probably to put a 'continue' in the empty line case, rather than a return.

This looks to be worked around in the latest source by flushing the command buffer, but I think that has some issues too...

On an abort, it's ok to flush the command buffer first. And it should probably flush the planner buffer too, so that printing stops immediately.

But there is also some code in there to clean up when EOF is reached. However, that code gets triggered when the EOF gets detected while reading into the command buffer. When in fact, there may still be buffered commands waiting to be parsed. That then interacts with the LCD UI code for 'check print finished', and it all just seems very tangled.

Since the 'get commands' code is only called when there is at least 1 slot in the buffer, I think a better approach for 'end of file' would be to add a single new gcode command to the buffer which later gets parsed to mean 'we're done, sync up the planner buffer, and then run the end gcode'. (which would only be added in the case that we're using ultigcode, and expecting the source file to not take care of it). This would have the advantage of keeping the buffering and parsing logic separate, rather than entwining them quite as much.

Similarly, for 'Abort', just adding a single command to the command buffer, and then allowing the process_commands logic to handle it might be a better option. Again, decoupling the UI and operational logic as much as possible.

Another option that might be considered is adding a second 'system use' command buffer that only the system can write to. That could have a flag associated with it, that indicates how to treat the buffer: 'run the system buffer when the user buffer empties' or 'ignore and empty the user buffer, and run the system buffer immediately' or 'run the system buffer commands first, but then go carry on with whatever is in the user buffer' so that the system buffer can play different roles as needed. Again that would avoid having the buffering, parsing and UI code needing to tromp all over one another quite as much.

The SD file utils seem to have some problems, btw. One subtle issue seems to be that it's a bit vague whether sdpos is the last byte read, or the next byte to be read. The lower level cur_position is the value of the next byte to be read. But for instance, in card.get() sdpos is set before the character is then read and returned - making sdpos the last character read. But then in other routines like setIndex, sdpos is set to the offset of the next character to be read. So, for instance in the error reset code in get_commands, it actually jumps back to the last character of the previous line rather than the first character of the next line. Since that's generally a newline or carriage return, it probably doesn't make much material difference, but it's wrong, as best I can tell.