cncjs / cncjs

A web-based interface for CNC milling controller running Grbl, Marlin, Smoothieware, or TinyG.
https://cnc.js.org
MIT License
2.28k stars 565 forks source link

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

Open telleropnul opened 5 years ago

telleropnul commented 5 years ago

CNC.js v1.9.20 Marlin bugfix 2.0

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

A request for enhancement that will positively affect CNC.js is being looked at: https://github.com/MarlinFirmware/Marlin/issues/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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

[ irrelevant information was deleted from this post ]

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

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

Small steps...

MitchBradley commented 5 years ago

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 commented 4 years ago

"In the case of Marlin, there is no easy way to coerce it to automatically issue timely position reports."

Marlin untested fix now offers timely position reports: cncjs/cncjs#549

Now that we have some position reporting in Marlin I am hoping the developer of the fix is willing to work together with CNC.js developers to address two concerns:

  1. Marlin does not offer automatic position reports containing mpos: and wpos: tags.

  2. During long moves at low feedrate (arcs), Marlin does not output any intermediate positional data until the move is comlete. This results in DROs not updating during the long slow move.

Many thanks for all your hard work. Happy CNC.js user here. 10/10 would use again.