-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Request] Warn users when using paren-comments in GCode (Doesn't properly support paren-comments in GCode) #3
Comments
Rather than it being the Usually, the * is followed by a checksum, to ensure proper communication with the printer. So when there is a If you remove the stars (but leave the other style of comments) are they properly ignored by the printer? |
You know, I didn't even think about the Since the file worked as-is when manually copied to the printer's SD card, I suppose the printer could ignore the I'll have to try that it this evening and see, because if that works, it could be arguably a Marlin bug (or gcode parser quirk) instead. |
Nope it wasn't the In my earlier testing, from which this issue originated, I had switched that first paren-comment-only line to a Tonight I looked at it closer -- first replacing the I then did a search-and-replace for all I did some quick math with the gcode command lines containing paren-comments in the serial log from OctoPrint, and it seems that OctoPrint is including the comment text itself in the checksum. But, if I'm reading the C++ source in Marlin/src/gcode/queue.cpp correctly, it is stripping both I didn't take time to manually open the serial port and bang-out a gcode command by hand with a paren-comment to confirm that you can transfer the comment as long as it's not part of the checksum -- that would be another interesting test to see if OctoPrint can transfer those comments if it doesn't checksum them. In any case, it seems I need to rename this issue to be |
nope, definitely not that. the comm layer definitely only seems to strip |
would it make sense to convert the strip_comment function to utilize a regular expression replacement here now with the complexity related to embedded parenthesis comments? Would be happy to submit a PR to fix. |
Yes, I do think that this function https://github.com/OctoPrint/OctoPrint/blob/53b9b6185781c07e8c4744a6e28462e96448f249/src/octoprint/util/comm.py#L6060 should behave like https://github.com/MarlinFirmware/Marlin/blob/fb76ce841b8e6108207b2d33eb217191fdc80427/Marlin/src/gcode/queue.cpp#L325 for stripping paren-comments too. The difficulty with such a regexp, that I can see, would be the nesting dilemma, https://3dprinting.stackexchange.com/questions/6410/are-parentheses-allowed-within-a-g-code-comment though most sources I can find say that nesting is simply not allowed, even if some firmware happens to support it. But yes, if you have a regexp change for OctoPrint to remove the paren-comments, that would be a welcomed PR. Though I'm thinking the best solution for me is to simply remove all paren-comments from my files. It was an artifact from about six years ago when I was using a machine that supported both CNC Milling and 3D printing (which, BTW, is not a good idea, should anyone reading this think that it might be --- any machine beefy enough to be a decent CNC Mill will be too slow to be a decent 3D printer and vice versa). |
Parenthesis style comments were so far never generated by 3d printer related tooling. As already mentioned they come from the CNC world, and the fully NIST standardized GCODE encountered there, contrary to intuition, has almost nothing to do with 3d printing, which abused the GCODE file format as a protocol, slapped on stuff like one sided transfer checksums and went with a lot of incompatible decisions. Long story short, OctoPrint currently only supports I'm kinda worried though about people using them in |
Agreed -- and the After this discussion, I'm leaning toward just leaving it as-is and say paren-comments aren't used in the 3D printer world, with the only possible OctoPrint change being it detecting them in a GCode file and aborting and/or warning the user about illegal file content rather than going down in flames. |
good point about the M117 command and all the other additional complexities of embedded comments. maybe a compromise that would prevent the issue at hand, non-embedded paren comments?
...but the escaping comes into play too...ugh. |
That could be a feature submitted to the (bundled, but developed separately) OctoPrint FileCheck plugin. Depending of course on if there is a better solution or not - just pointing out that it would be a possible solution that we already have the framework for. |
I'm leaning towards this as well now after learning that the file in question was not a recently generated one but rather something targeting a years old printer/cnc combo. |
I was just about to implement this as I have to push out a bugfix for this bundled plugin anyhow, but after pretty much finishing a warning about this I decided against it after all. Apart from this ticket I've never seen anyone run into a problem due to a slicer creating parenthesis based comments in the GCODE they then tried to run through OctoPrint. A warning would be easy to implement, however, I think it's probably more likely that there are more people out there using OctoPrint with something that is not a 3D printer and which supports that comment style than there are people who could run into this specific problem with a printer. So, adding a warning increases the likelihood of false positives and thus problems for some workflows more than solving a regular problem. And thus, this won't get implemented after all. Should things change and this actually becomes a regular problem, this decision can get changed. |
The problem
I was attempting to do a SD Card Upload of a GCode file. It fails with a repeating:
In looking at the OctoPrint log file and terminal dump, the problem seems to be in https://github.com/OctoPrint/OctoPrint/blob/master/src/octoprint/util/comm.py and how it processes the gcode lines that it is sending.
The source code seems to strip comments that use the ';' symbol, but not CNC-style paren-comments, which can be enabled in the printer's firmware and is used in this gcode file. If the paren-comments are on lines with actual gcode comments, there is no issue, as it sends the comment to the printer right along with the rest of the gcode command line. But, if a paren-comment appears on a line by itself, it seems that OctoPrint doesn't properly send the line number and checksum as it should, which causes the printer to rerequest the line and the two sides get out of sync.
The first few lines of the gcode file are:
The first line gets tossed completely by OctoPrint as a comment. However, the second line gets sent as-is, but without the proper line-number and checksum, since it contains no actual gcode command. That is,
gcode_command_for_cmd
returns nothing for it.This is what is seen in the OctoPrint log file:
That continues to repeat for a bit until it realizes it's stuck and tries to abort:
Then, for whatever reason, the printer and OctoPrint can't seem to get resynchronized until I reboot the printer board and reconnect with OctoPrint...
I think the problem is right here:
Where OctoPrint sends the
(**** start.gcode for The OctoForge ****)
text without a line number or checksum and without stripping it as a pure comment line.Now arguably, paren-comment-only lines are a bit outside of gcode standard, even for CNC format, but the printer itself, if you copy this file directly to the SD card, interprets it just fine. So, there's a definite bug in OctoPrint's reading and sending the file.
My workaround, of course, is to just change the start.gcode segment to use the ';' symbol for comment-only lines and let OctoPrint just strip them. But I wanted to open this issue so that it can eventually get fixed in OctoPrint.
Did the issue persist even in safe mode?
Yes, it did persist
If you could not test in safe mode, please state why
No response
Version of OctoPrint
1.8.1
Operating system running OctoPrint
octopi_version: 0.18.0, octopiuptodate_build: 0.18.0-1.7.3-20220323100241
Printer model & used firmware incl. version
Custom rebuild of FFCP using BTT Octopus (i.e. an "OctoForge"). Firmware: "Marlin bugfix-2.0.x (Jun 5 2022 20:21:01)"
Browser and version of browser, operating system running browser
Firefox 100.0.2 on Xubuntu (but not related to problem)
Checklist of files to include below
Additional information & file uploads
Systeminfo Bundle:
octoprint-systeminfo-20220609124913.zip
GCode file in question:
FilamentSensorPlug_octoforge.gcode.zip
Full Log file from OctoPrint:
octoprint.log.2022-06-06.zip
The text was updated successfully, but these errors were encountered: