G90 mid line is ignored, likely G91 is too
Opened this issue · 6 comments
Some GCode senders like to get crafty and will embed a G90/G91 within a movement command, like so:
G0 G90 X5 Y5
G1 G91 X5 Y5
This currently confuses your viewer because it is only looking for them at the beginning of a line (assumedly).
Something like this should make it much better:
if "G90" in line.upper():
# do absolute
if "G91" in line.upper():
# do relative
Have been using this plugin a bit lately and am really digging it.
Which program generates that?
I am thinking this is a special case of a wider g-code definition that allows multiple commands per line and commands that persist across lines.
G1
X5 Y5
X10 Y5
X10 Y10
Or in your case, the G1 command persists across another command. At some point these become the responsibility of the CAM post-processor, or the parser handles the more general syntax.
The more general syntax costs only labor, but I'm thinking it's not worth it because one, it's a pretty specialized use, and two, post-processors can also solve the problem.
The question more specific to this issue is whether it's worth implementing this specific case of G90 or G91 within a G0 or G1 command. The implementation is straightforward enough but I am worried about the cognitive complexity in knowing what it does. G90 and G91 can be added as parameter keywords to the G0 and G1 commands, and it would work to say G0 G90 X5 Y5 but it won't work to say G90 G0 X5 Y5, and other commands like G2 G90 X5 Y5 I3 J3 won't work either. Also a case like G0 X5 Y5 G91 should probably set relative mode after the X5 Y5 movement is completed, as if it were two consecutive commands on one line, but line-oriented processing with G90/G91 parameter keywords would do the wrong thing.
So even though the implementation is straightforward enough, I feel that the complexity that it brings is too much. When diagnosing an unexpected behavior, it's important that the human can predict what the result should be, and one command per line makes it easy for the human to spot problem areas. I would hope that a post-processor could provide a strong alternative solution by translating the crafty code into an appropriate sequence of two commands.
your example above is merely a form of shorthand. The case I mentioned is completely valid and supported.
This should help for handling shorthand:
# save our G command for shorthand post processors
if line.upper().lstrip().startswith("G"):
lastGCommand = line.lstrip()[:3] if line.lstrip()[2:3].isnumeric() else line.lstrip()[:2]
# use our saved G command if our line starts with a coordinate
if line.upper().lstrip().startswith(("X", "Y", "Z")):
command = lastGCommand.upper() + " " + line.upper().strip()
else:
command = line.upper().strip()
Welcome to the wonderful world of variability when it comes to CNC gcode. What I shared is fully supported and others will come across it as well. You can either choose to support it or provide a broken product. Your choice.
Marlin (the primary use-case for OctoPrint as far as I am aware) does not support G1 G91 (it ignores the G91), so in that sense the rendering correctly predicts Marlin's behavior when given G1 G91. To "fix" it would then create wrong predictions for how Marlin would behave.
I couldn't find a reference that says it's legal in LinuxCNC, Grbl, Smoothieware, RepRap, or NIST RS274NGC.
I don't know what CAM you're using or what controller, but regardless, post-processors exist to fix these discrepancies. Using a post-processor is normal and proper.
I pushed (untested) code to a branch https://github.com/vector76/cnc_gcodeviewer/tree/G1-G91-support which you can try, but I'm not intending for this to be supported by default. No tool supports all flavors of gcode and it's foolish to make that a goal. Welcome to the wonderful world of variability when it comes to CNC gcode.
You are incorrect. There are many flavors of gcode and Octoprint supports all of them. Long before Marlin we were printing with Repetier, for example.
I understand you're still fairly new to all of this and perhaps you forgot the whole premise of me sharing data with you was to promote GRBL compatibility. I think I'm going to just stop waisting my time with you. Perhaps you just don't like bug reports? Best of luck, regardless.
Ok. I think I see my mistake.
Marlin has zero responsibility to mimic the idiosyncrasies of Grbl and vice versa. The responsibility for handling the differences between the possible consumers is 100% the responsibility of the producers: the CAM software or postprocessor.
I was considering the gcode viewer as a consumer and taking this position with respect to allocating the responsibility for variations to the client, but this is a major error because the gcode viewer is not the target of the gcode. The controller is the target and the gcode viewer is only an observer. The controller can dictate the syntax but the observer cannot. Supporting alternative flavors is not at all a goal for the controller, but for the observer it absolutely should be.
This should have been obvious and I'm not sure how I made this mistake. I'm sorry.
This shift in perspective reverses basically everything that I've said above. Even shorthand is on the table, whereas I had previously dismissed it as an example of a non-goal.
Restricting to a "clean" subset of the language with no weirdness might be of value to a controller that dictates its interface, but for an observer that will see multiple flavors, having "nice" syntax is not a value. It just needs to work.
I'll check if the update in the branch works as intended, and if so I'll merge it back into the main repository.
Now we're talking!
Marlin is a nice general purpose protocol and has taken over the 3d printer world. We just can't assume everyone uses Marlin for CNC. Marlin is actually the edge case / outlier with the majority of (consumer) CNC's being based on GRBL (especially now with ESP32), clipper, smoothie, etc.
Interesting read: https://all3dp.com/2/best-cnc-software-for-arduino/
Thanks for keeping an open mind.