M20 T support (timestamps for files on sd card) #4610
Conversation
arekm
commented
Aug 4, 2022
arekm
commented
github-actions
bot
added
docs
arekm
force-pushed
the
arekm/devel/m20t
branch
6 times, most recently
from
59d8602
to
54f242f
Compare
|
Printers don't have real time clock and on M28 upload they set ancient date (like 2000-01-01 01:00:00 - prusa mk3s does that) which then shows up in octoprint as "Uploaded: 23 years ago". Printer tells us such timestamp in M20 T, so it is being shown but maybe it would be better to ignore such dates and show nothing(?) Virtual printer emulates this behaviour, too. If you want updated timestamp just touch file in ~/.octoprint/virtualSd/. timestamps on vfat filesystems (used on most sd cards) are considered to be in "local time" (this is behaviour of windows and macos). Linux unfortunately considers these UTC, so if someone mounts sd card on Linux (with default options), puts gcode files there and then put sd card into printer there will be offset shown, but we can't do nothing about this. If someone does the same on windows or macos then printer and octoprint will show correct upload time. |
I've always followed the approach that what the firmware reports gets used. Otherwise we'll get "bug reports" about missing timestamps. This way of course we'll risk "bug reports" about invalid timestamps, but at least we can then just point people to this PR and explain that we just display what the firmware reports and that it is up to the firmware to make a decision here (e.g. NOT return a timestamp for files created directly by the firmware). |
|
Please make sure to request a new review once you've implemented the requested changes. |
|
I sadly only just now noticed that you opened this PR against |
|
Should be now against maintenance branch (and I saw docs but wasn't sure what printers do if T is unrecognized. Fortunately they just ignore it). |
arekm
force-pushed
the
arekm/devel/m20t
branch
3 times, most recently
from
ad7141f
to
3346f50
Compare
Change the way line is built, so it will be easier to add new things without duplicating conditions.
T as documented in https://reprap.org/wiki/G-code#M20:_List_SD_card and implemented by, for example, Prusa Firmware.
arekm
force-pushed
the
arekm/devel/m20t
branch
from
3346f50
to
d38c11b
Compare
Use "M20 L T" (if CAPABILITY_EXTENDED_M20 is available) which gives us long file names but also file timestamps. Wire up that timestamp into "date" field which gives us "Uploaded: ... ago" for files on SD card. Sorting by upload date also works. Implementing this also fixes parsing of response to (for example manually sent) "M20 T" command. Octoprint assumed that third entry in response is always long file name while with M20 T it is timestamp. Add conversion routines between M20 T timestamp and unix timestamp. Wire up that conversion in virtual printer.
arekm
force-pushed
the
arekm/devel/m20t
branch
from
d38c11b
to
9b9c609
Compare
Move file list parsing into separate parse_file_list_line function and add tests for it. Add better timestamp validation.
arekm
force-pushed
the
arekm/devel/m20t
branch
from
9b9c609
to
2a40cfe
Compare
|
Moved parsing into separate function, added tests. |
Make m20_timestamp_to_unix_timestamp() directly accept hex timestamp string. Use this function for timestamp validation. unix_timestamp_to_m20_timestamp() returns hex string now.
arekm
force-pushed
the
arekm/devel/m20t
branch
from
7c234e2
to
02caf59
Compare
|
Please make sure to request a new review once you're done working on this and it is ready for another review. |
|
I see "foosel was requested for review" and I don't see any new way to request review again in github UI. At this moment I have no pending changes/improvements. |
|
I just kept seeing new and new pushes, so I wanted to make sure you really are done here before I give this another review. It's a bit tricky to hit a moving target ;) |
foosel
added
targets maintenance
|
Looks good now, merging, thank you :) |
