gcode: Update M20 to fit common marlin behaviour #3765
Conversation
|
EXTENDED_M20 capability needs to be reported, too https://reprap.org/wiki/Firmware_Capabilities_Protocol#Cap:EXTENDED_M20 |
|
Implementation has been updated. |
bkerler
force-pushed
the
m20_upgrade
branch
3 times, most recently
from
8568873 to
a419933
Compare
There was a problem hiding this comment.
Have no way to test this on a printer but tested time_to_m20() implementation on Linux and it correctly translates dates/times that fit into 32bit (tests from OctoPrint test suite, beside 64bit one).
I would move printing LFN inside stat() success path because other implementations parse fat dir directly and always know the size (thus always return it). OctoPrint parser was made with that in mind and won't be able to parse long file name from just
shortfilename "longfilename"
output. It needs at least size in 3 element lines (so "shortfilename size timestamp" or "shortfilename size longfilename)
For reference:
https://github.com/OctoPrint/OctoPrint/blob/1.10.0rc2/src/octoprint/util/comm.py#L6451-L6472
Agree. I've moved it into the stat() success path. |
|
|
||
| static void print_file_info(const char *const path, const char *const long_path, const struct TimeFlags tf) { | ||
| // &path[5] and &long_path[5], because we skip the /usb/ prefix on print as it is only used internally | ||
| SERIAL_ECHO(&path[5]); |
There was a problem hiding this comment.
There's a strlen_constexpr() in str_utils.hpp, we should be able to use that. Not that I am expecting the internal path to change, it just looks more readable/maintainable to me.
| SERIAL_ECHOLN(entry->d_name); | ||
| char path[5 + sizeof(entry->d_name) * 2]; | ||
| snprintf(path, sizeof(path), "%s/%s", prefix, entry->d_name); | ||
| char long_path[5 + sizeof(entry->lfn) * 2]; |
There was a problem hiding this comment.
I still don't understand these buffer sizes. Why *2 ? We are only supporting root directory, right? I would imagine we need "/usb/" + respective filename (sfn or lfn) + possibly a null terminator. What am I missing?
There was a problem hiding this comment.
The idea was that sizeof(entry->d_name) has the same size as prefix. But yeah, I've changed that completely because it wasn't clean and safe. Should be fine now.
|
Closing in favor of #3976, which has additional support |
This addresses the issue #3054.
It improves the gcode M20 to: