Skip to content
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

[Marlin] CNC.js Axis widget instantly stops displaying machine coordinates when I switch to G54 #514

Open
telleropnul opened this issue Aug 24, 2019 · 5 comments

Comments

@telleropnul
Copy link

commented Aug 24, 2019

CNC.js v1.9.20
Marlin bugfix 2.0

A Marlin bug was fixed a few days ago that will positively affect CNC.js:
MarlinFirmware/Marlin#14743

A request for enhancement that will positively affect CNC.js is being looked at:
MarlinFirmware/Marlin#14734

There is a last issue I could use some help with:

When I switch from machine coordinates (G53) to work offset coordinates (G54) in Marlin by simply entering "G54", the Axis widget shows work coordinates of X,Y,Z 0,0,0 which is correct. However, the machine coordinates instantly change to X,Y,Z 0,0,0 as well. This behavior is incorrect - the machine coordinate system digital read-out (DRO) should ALWAYS display machine coordinates. When I enter "G53" to switch back to machine coordinates, the machine coordinates are instantly displayed correctly. It appears Marlin does not provide data for both machine coordinates and work coordinates to CNC.js simultaneously, or perhaps CNC.js does not interpret the data correctly, not sure.

Any suggestions would be appreciated.

@MitchBradley

This comment has been minimized.

Copy link
Contributor

commented Aug 24, 2019

You surmise correctly that Marlin only reports one set of coordinates. Historically, Marlin only had machine coordinates, so in order to retain UI consistency across different controllers, CNCjs just displays whatever Marlin reports in both the machine and work DROs. Now that Marlin supports coordinate systems that may need to be rethought. It will not be easy since, unless Marlin changes its reports to make both sets available, CNCjs would have to snoop on the GCode and infer the coordinate offset. That has its own set of problems; suppose, for example, that Marlin rejects a coordinate-offset change command for whatever reason. That would greatly complicate the logic for maintaining the correct offset. It is one of those kinds of problems that makes programmers grind their teeth - very difficult to get it right in all cases, and users are guaranteed to trip over all the corner cases.

That is why I am somewhat skeptical of your claim that the proposed change will "positively" affect CNCjs. To my way of thinking, it will make it even harder to make Marlin support work well. Marlin's serial protocol feels like an afterthought. Marlin works really well in the 3d printing use case especially when driven from a controller-attached LCD. Driving it from a serial line via CNCjs is fraught with difficulty mostly arising from timing issues around status reports and interference between status reports and GCode transmission. I think it is unfortunate that MPCNC chose Marlin. I understand why they did so - coming from the 3D printing world, the designers were presumably very familiar with Marlin and RAMPS boards, so that was the path of least resistance. But as I see it, Marlin is just not that good for milling. It might get better over time, but the road will be rocky and will cause a great deal of work for CNCjs developers to track the developments.

The other problem is that the coordinate support is optional, so many or most Marlin users will not enable it. That compounds the support problem, since you have to cope with both possibilities in order to handle a capability that, for a long time, will only have a very few users. And with few people using it, bugs will take longer to surface and be fixed.

@MitchBradley

This comment has been minimized.

Copy link
Contributor

commented Aug 24, 2019

I would also point out that Marlin's handling of G53 is idiosyncratic. On other machines, G53 is non-modal, affecting only the current line. The next line is supposed to automatically revert to the previous WCS (G54..G59 ). That further complicates Marlin support because you would need a GCode recognizer that understands Marlin quirks (of which there are many).

@telleropnul

This comment has been minimized.

Copy link
Author

commented Aug 24, 2019

Thank you very much for your detailed response - most insightful.

That is why I am somewhat skeptical of your claim that the proposed change will "positively" affect CNCjs.

My apologies for causing alarm. You have provided very useful insights that are new to me. After having carefully read your response I hope I can ease at least one of the concerns:

"Positively affect CNC.js"
If I could ask you to have a closer look at the linked Marlin bugfix #14743 article, I hope you will agree this bugfix at the very least does not negatively impact CNC.js. The bug report describes how entering G92 X0 Y0 (= set X and Y current position as new absolute zero; basically set current position as new origin) in G54 work offset coordinates would inadvertently also reset the machine coordinate system's origin (G53) at the same time. This behavior is incorrect and somewhat dangerous; the machine coordinate system (G53) should always display absolete machine coordinates when in operation after endstop calibration and not have its origin reset. It was also inconsistent as it was unique to G54; G55-G59 were not affected and showed correct behavior. I think it is a good thing this inconsistency was addressed. I am now able to enter "G54 + G92 X0 Y0" and use absolute negative X and Y coordinates in machining jobs where the origin is centered on an object, something not possible when using machine coordinates and Xmin + Ymin endstops (= only absolute positive X and Y values allowed). The code change does not negatively affect CNC.js:

Example

G21 ; millimeters
G90 ; absolute coords
G53 ; machine coordinate system
M114 ; get position
    X:0.00 Y:0.00 Z:0.00 E:0.00 Count X:0 Y:0 Z:0
**CNC.js Axis widget shows the same coordinates in both DROs.**
G54 ; switch to (1st) work coordinate system
G0 X10 Y10 ; move
    X:10.00 Y:10.00 Z:0.00 E:0.00 Count X:2000 Y:2000 Z:0
**CNC.js Axis widget shows the same coordinates in both DROs.**
G92 X0 Y0 ; set current position as new origin (similar to G10 L20 P1 X0 Y0)
M114 ; get position
    X:0.00 Y:0.00 Z:0.00 E:0.00 Count X:2000 Y:2000 Z:0
**CNC.js Axis widget shows the same coordinates in both DROs.**
G53 ; switch back to machine coordinates
M114 ; get position
    X:10.00 Y:10.00 Z:0.00 E:0.00 Count X:2000 Y:2000 Z:0
**The bugfix ensures machine coordinates remain correct and are not set to zero**
**CNC.js Axis widget shows the same coordinates in both DROs.**
**CNC.js correctly follows a switch between displaying machine coordinates and work offset coordinates.

CNC.js shows the same coordinates in both DROs at all times. This is because Marlin only outputs one set of coordinates as you state.

I feel your concerns are mainly centered around Marlin quirks i.e. not outputting 2 sets of coordinates and not so much Marlin bugfix #14743. I understand your concerns and would have to agree not outputting 2 sets of coordinates -whilst acceptable for 3D printer use- makes it somewhat challenging for CNC machine use.

Wild idea
Rather than forcing Marlin to output 2 sets of coordinates (which is the correct but difficult solution to the problem) perhaps solely in the case of Marlin we could have a temporary workaround where the Axis widget simply displays a single set of coordinates at all times and change the DRO label to "machine coordinates" when in G53 and to change the DRO label to "work offset coordinates" when in G54 (or G54-G59). This could be relatively easy implemented by simply hiding the second DRO readout. This workaround would improve alignment between Marlin and CNC.js and a fairer representation of the underlying data stream without the need for snooping or interpreting data. I understand a 'one solution fits all' clean codebase is preferred over having to maintain separate code routines for different logic boards, so this suggestion may be flawed and not desirable. Any exceptions in the CNC.js codebase would add to the complexity of things i.e. add additional test case scenarios when beta testing future CNC.js code releases. So just an idea, please ignore if of no use.

Marlin's serial protocol feels like
I am not familiar with the protocol from a programmer's perspective. Your statement suggests CNC protocols are ideally parallel rather than serial. If you could elaborate on this that would be much appreciated.

G53 non-modal
Interesting. I am not sure which behavior is more common or industry standard; G53 only affecting a single line (non-modal) or G53 acting as a WCS switch (modal). Marlin treats a G53 command without arguments as modal i.e. a WCS switch and a G53 command followed by G... X...Y... as non-modal i.e. only affecting the current line:
http://marlinfw.org/docs/gcode/G053.html

Thoughts
I will give it a lot of thought whether or not I should request further changes to Marlin codebase. Your opinion on this would be much appreciated. My thoughts in a nutshell: The 3D printer community does not need work offset coordinates much. I don't know if outputting two sets of coordinates would be destructive to how 3D printers parse gcode. I don't even know how non-Marlin CNC controller logic boards i.e. Smoothieboard, GRBL, etc. output two sets of coordinate data to correctly feed the DROs. Any technical information you have on what is expected from Marlin to aid CNC.js and feed the two DROs correctly would be most appreciated.

I would also point out
I understand. I am trying to close the gap between Marlin and CNC.js but am not a developer. "Marlin quirks (of which there are many)" is somewhat discouraging. It is possible to switch to MACH3, but I love CNC.js and Marlin. There is something to be said for going with whatever hardware has a strong following and active forum community. If you have a list of quirks you would like to see addressed, please share.

G10 L20 P1 X0 Y0
For now I am focussing on a request for enhancement that would implement "G10 L20...." in Marlin. Small steps...

Ticket closed
This ticket can be considered closed. I realize that the root cause lies with Marlin and not CNC.js: Marlin would need to output two sets of coordinate data (machine + work) to feed the DROs. The current situation is actually not all that bad as CNC.js shows the same coordinates in both DROs and correctly switches between G53 and G54. Other than slightly confusing at first, there appear to be no issues with this at all (no guarantees, not peer reviewed, not confirmed). The somewhat unrelated Marlin bugfix #14743 has now made G54 +G92 X0 Y0 behave properly and not affect G53 origin.

Thank you
Thank you for your detailed response - much appreciated. Thank you for your hard work and a most elegant CNC web based intuitive gcode parser.

@MitchBradley

This comment has been minimized.

Copy link
Contributor

commented Aug 25, 2019

The negative effect is that the expectation that CNCjs should support this new Marlin feature that does not yet work very well creates extra work for the already-overworked main developer. The fact that the Marlin serial protocol has design problems makes it hard to even estimate the difficulty of supporting it correctly.
You are welcome to fork the CNCjs source code and modify it to display only one DRO. I did exactly that in the cncjs-shopfloor-tablet UI which I maintain.
Other controllers have a soft-configurable (does not require recompilation) option to include both machine and work coordinates in their position reports. For example, g2core tags machine coordinates with "mpos:" and work coordinates with "wpos:" in the JSON-formatted reports. The report format has little or nothing to do with the parsing of GCode. In the case of Marlin, there is no easy way to coerce it to automatically issue timely position reports. You have to send a command asking it for a report, and the periodic sending of those commands sometimes causes motion stuttering because the "send a report" command has to be interspersed with the GCode from the file. The lack of automatic position reports can be troublesome for milling, where it is common for a single GCode move to take several seconds - for example a long arc at a low feedrate. With Marlin, the DROs don't update during that move, and might not update until the end of several such moves, depending on how often CNCjs has inserted "send position" commands.

@telleropnul

This comment has been minimized.

Copy link
Author

commented Aug 27, 2019

It appears you are correct.

What logic board can you recommend for optimum compatibility with CNC.js ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.