LinuxCNC / linuxcnc

LinuxCNC controls CNC machines. It can drive milling machines, lathes, 3d printers, laser cutters, plasma cutters, robot arms, hexapods, and more.
http://linuxcnc.org/
GNU General Public License v2.0
1.81k stars 1.16k forks source link

False Limit violation warnings in AxisGUI #2646

Closed Sigma1912 closed 1 year ago

Sigma1912 commented 1 year ago

Proposal: Remove the function 'run_warn()' and the two function calls 'if run_warn(): return' from the 'axis.py' source code.

AxisGUI produces false warnings about limit violations when the user executes gcode with active tool length compensation. Basically it will calculate limit violations using the currently active tool length offset regardless of toolchanges in the gcode.

This came up recently on the forum but has been around for years: https://forum.linuxcnc.org/20-g-code/50095-tool-offsets-issue-cannot-start-program-video-question#280947 and also here: https://forum.linuxcnc.org/38-general-linuxcnc-questions/50135-error-program-exceeds-machine-maximum#281280

The check for limit violations 'run_warn()' which is called on 'run' and 'step' events:

    def task_run(*event):
        if run_warn(): return

        global program_start_line, program_start_line_last
        program_start_line_last = program_start_line;
        ensure_mode(linuxcnc.MODE_AUTO)
        c.auto(linuxcnc.AUTO_RUN, program_start_line)
        program_start_line = 0
        t.tag_remove("ignored", "0.0", "end")
        o.set_highlight_line(None)

    def task_step(*event):
        if s.task_mode != linuxcnc.MODE_AUTO or s.interp_state != linuxcnc.INTERP_IDLE:
            o.set_highlight_line(None)
            if run_warn(): return
        ensure_mode(linuxcnc.MODE_AUTO)
        c.auto(linuxcnc.AUTO_STEP)

The run_warn() function looks like this:

def run_warn():
    warnings = []
    if o.canon:
        machine_limit_min, machine_limit_max = soft_limits()
        for i in range(3): # Does not enforce angle limits
            if not(s.axis_mask & (1<<i)): continue
            if o.canon.min_extents_notool[i] + to_internal_linear_unit(o.last_tool_offset[i]) < machine_limit_min[i]:
                warnings.append(_("Program exceeds machine minimum on axis %s")
                    % "XYZABCUVW"[i])
            if o.canon.max_extents_notool[i] + to_internal_linear_unit(o.last_tool_offset[i]) > machine_limit_max[i]:
                warnings.append(_("Program exceeds machine maximum on axis %s")
                    % "XYZABCUVW"[i])
    if warnings:
        text = "\n".join(warnings)
        return int(root_window.tk.call("nf_dialog", ".error",
            _("Program exceeds machine limits"),
            text,
            "warning",
            1, _("Run Anyway"), _("Cancel")))
    return 0

Checking limit violations is done by the motion planner ( or is it the interpreter? ) and the Axis GUI is clearly not doing a very good job at it.

I would propose to remove the function 'run_warn()' and the two function calls 'if run_warn(): return' from the 'axis.py' source code.

andypugh commented 1 year ago

Doing some archeaology:

v2.7 of LinuxCNC has:

def run_warn():
    warnings = []
    if o.canon:
        machine_limit_min, machine_limit_max = soft_limits()
        for i in range(3): # Does not enforce angle limits
            if not(s.axis_mask & (1<<i)): continue
            if o.canon.min_extents_notool[i] < machine_limit_min[i]:
                warnings.append(_("Program exceeds machine minimum on axis %s")
                    % "XYZABCUVW"[i])
            if o.canon.max_extents_notool[i] > machine_limit_max[i]:
                warnings.append(_("Program exceeds machine maximum on axis %s")
                    % "XYZABCUVW"[i])
    if warnings:
        text = "\n".join(warnings)
        return int(root_window.tk.call("nf_dialog", ".error",
            _("Program exceeds machine limits"),
            text,
            "warning",
            1, _("Run Anyway"), _("Cancel")))
    return 0

Whereas in 2.9 we have

def run_warn():
    warnings = []
    if o.canon:
        machine_limit_min, machine_limit_max = soft_limits()
        for i in range(3): # Does not enforce angle limits
            if not(s.axis_mask & (1<<i)): continue
            if o.canon.min_extents_notool[i] + to_internal_linear_unit(o.last_tool_offset[i]) < machine_limit_min[i]:
                warnings.append(_("Program exceeds machine minimum on axis %s")
                    % "XYZABCUVW"[i])
            if o.canon.max_extents_notool[i] + to_internal_linear_unit(o.last_tool_offset[i]) > machine_limit_max[i]:
                warnings.append(_("Program exceeds machine maximum on axis %s")
                    % "XYZABCUVW"[i])
    if warnings:
        text = "\n".join(warnings)
        return int(root_window.tk.call("nf_dialog", ".error",
            _("Program exceeds machine limits"),
            text,
            "warning",
            1, _("Run Anyway"), _("Cancel")))
    return 0

It feels like what should be used is canon.min_extents and max_extents and it certainly looks like the tool data is meant to be in there: https://github.com/LinuxCNC/linuxcnc/blob/2.9/src/emc/rs274ngc/gcodemodule.cc#L841 I am interpreting xs to be x start, xe to be x end and zt to be the tool x offset.

andypugh commented 1 year ago

And I just found that I have been in here before: https://github.com/LinuxCNC/linuxcnc/commit/275c79b8cc11b6d0f243590a69b5bce9e31a0ae1 It looks like I fixed this 12 years ago...

Sigma1912 commented 1 year ago

Thanks for looking into this. I have just tested on axis_mm sim config on master (built in march). Gcode 1 (this gives a false limit violation message): `G17 G21 G54 G80 G90
T10 M6 G43 (TLO = 10mm) G0 Z25 G1 Z-10 F300 G0 Z0

M2`

Calculates the limits as:

canon.max_extents_notool, machine_limit_max, last tool offset: 2 , -0.19685039370078744 , 0.0 , 0.7874015748031497 canon.min_extents_notool, machine_limit_min, last tool offset: 2 , -1.5748031496062993 , -12.598425196850394 , 0.7874015748031497 canon.max_extents, machine_limit_max, last tool offset: 2 , -0.5905511811023623 , 0.0 , 0.7874015748031497 canon.min_extents, machine_limit_min, last tool offset: 2 , -1.968503937007874 , -12.598425196850394 , 0.7874015748031497 Program exceeds machine maximum on axis Z

GCode2 (this gives correct limit violation message):

`G17 G21 G54 G80 G90
;T10 M6 G43 (TLO = 10mm) G0 Z25 G1 Z-10 F300 G0 Z0

M2`

Calculates the same limits as Gcode1:

canon.max_extents_notool, machine_limit_max, last tool offset: 2 , 0.19685039370078738 , 0.0 , 0.7874015748031497 canon.min_extents_notool, machine_limit_min, last tool offset: 2 , -1.1811023622047245 , -12.598425196850394 , 0.7874015748031497 canon.max_extents, machine_limit_max, last tool offset: 2 , -0.5905511811023623 , 0.0 , 0.7874015748031497 canon.min_extents, machine_limit_min, last tool offset: 2 , -1.968503937007874 , -12.598425196850394 , 0.7874015748031497 Program exceeds machine maximum on axis Z USRMOT: ERROR: invalid command Linear move on line 3 would exceed Z's positive limit Linear move on line 3 would exceed joint 2's positive limit invalid params in linear command

So it ignores the tool change in the gcode and I a wondering if these calculations are done in the preview interpreter and if that is maybe why tool changes are ignored.

andypugh commented 1 year ago

It might depend on where you look. I added this line to axis.py (which only executes once you press "run") and I am still trying to work out what the results mean.

in the run_warn sub

 for i in range(3): # Does not enforce angle limits
 +            print("Axis.py min %f max %f min-t %f max-t %f" % (o.canon.min_extents_notool[i] , o.canon.max_extents_notool[i] , o.canon.min_extents[i] , o.canon.max_extents[i]) , file=sys.stdout)
            if not(s.axis_mask & (1<<i)): continue

I am starting to think that the *no_tool values are the correct ones to check for limits, showing axis positions and that the others show the tool tip position.

Sigma1912 commented 1 year ago

It might depend on where you look

In any case it seems that I wasn't looking at the printout I posted because there is actually a difference:

'canon.min_extents_notool' and 'canon.max_extents_notool' differ by 10mm (0.3937 in) which is correct.

Sigma1912 commented 1 year ago

So I've reverted to the historic version (with some debug print statement):

`
if o.canon.min_extents_notool[i] < machine_limitmin[i]: print("Program exceeds machine minimum on axis %s" % "XYZABCUVW"[i]) warnings.append(("Program exceeds machine minimum on axis %s") % "XYZABCUVW"[i]) if o.canon.max_extents_notool[i] > machine_limitmax[i]: print("Program exceeds machine maximum on axis %s" % "XYZABCUVW"[i]) warnings.append(("Program exceeds machine maximum on axis %s") % "XYZABCUVW"[i])

`

This seems to work sometimes but not always.

[edit] I can't figure out how to properly insert code in this editor

Sigma1912 commented 1 year ago

I might have found a pattern for an issue without toolchange in the gcode.

Gcode is this (ie no toolchange here) with a 20mm TLO this should create a max limit error:

G17 G21 G54 G80 G90
;T10 M6 G43 (TLO = 10mm) G0 Z25 G1 Z-10 F300 G0 Z0

M2

Sequence 1: Start Config -> home -> change tool (offset 20mm) -> Load the Gcode -> run : Correct message limit violation

Sequence 2: Start Config -> home -> Load the Gcode -> change tool (offset 20mm) -> run : No message limit violation

Sigma1912 commented 1 year ago

Update:

Using this Gcode (ie with toolchange in the gcode):

G17 G21 G54 G80 G90
T10 M6 G43 (TLO = 10mm) G0 Z25 G1 Z-10 F300 G0 Z0

M2

Both sequences in the post above work correctly.

Which leads me to the conclusion that the historic code works with toolchanges in gcode but has an issue if the tool is changed in MDI after the Gcode is loaded.

andypugh commented 1 year ago

[edit] I can't figure out how to properly insert code in this editor

Four backticks (ie ```` ) above and below the code block

lke this
andypugh commented 1 year ago

The program extents are calculated when the G-code is loaded, or reloaded. The warning is displayed when you press the run button.

So, the sequence you see makes sense.

Sigma1912 commented 1 year ago

The program extents are calculated when the G-code is loaded, or reloaded. The warning is displayed when you press the run button.

So for this to ever work correctly the extents would need to be recalculated when the user presses the run button?

andypugh commented 1 year ago

So for this to ever work correctly the extents would need to be recalculated when the user presses the run button?

To work 100%, yes. But there is a cost to that, as running the preview with a large G-code file can take significant time.

It will only ever not work (in theory at least) when there is an MDI change to tool-length after the file is loaded and before the run button is pressed.

One option might be to add a "reload and re-check" button to the dialog box,

Or simply document that you should reload the G-code if making an MDI tool length change.

Sigma1912 commented 1 year ago

One option might be to add a "reload and re-check" button to the dialog box

Given that it is not uncommon to load a gcode file and then use a probe to touch off I would think a clear explanation in the pop up is probably the least, otherwise we will just keep having reports about this on the forum.

Personally I still don't see the point of having AXIS redundantly checking limits particularly if it can't do it properly but that is clearly not my call.

andypugh commented 1 year ago

The idea is to know beforehand wherther the job will finish. Getting 8 hours into a job then hitting a soft-limit isn't great either.

AFAIK only Axis runs this test, but I think it is a good one.

Probing a tool before running the code isn't something that happens on proper machines with fixed position tooling and known tool lengths. I _think_ that Axis recomputes everything when you do a workpiece touch-off inside the GUI. Quite possibly with a tool touch off too. The issue is with MDI and remap probes where the GUI doesn't know that it has happened. It would be fairly easy to re-run the sim if the MDI line contained a G43 or M10, but less easy to catch a subroutine call or user-m-code.
Sigma1912 commented 1 year ago

The idea is to know beforehand wherther the job will finish. Getting 8 hours into a job then hitting a soft-limit isn't great either.

Since I always got axis and joint errors from the interpreter when I clicked 'Run anyway' in the pop up window I was under the impression that the interpreter checks the limits properly when the run button is pressed?

andypugh commented 1 year ago

Since I always got axis and joint errors from the interpreter when I clicked 'Run anyway' in the pop up window I was under the impression that the interpreter checks the limits properly when the run button is pressed?

Only if there are no queue-busters in the G-code. (like probes, toolchanges(?) or wait-on-input)

Sigma1912 commented 1 year ago

Only if there are no queue-busters in the G-code. (like probes, toolchanges(?) or wait-on-input)

So isn't that the same for the limit calculations in AXIS?

andypugh commented 1 year ago

No, the axis sim runs the whole code in to create the graphical preview. There is a full (limited) interpreter in gcodemodule.cc

Sigma1912 commented 1 year ago

No, the axis sim runs the whole code in to create the graphical preview. There is a full (limited) interpreter in gcodemodule.cc

I see, then I agree that this is a rather useful feature.

Probing a tool before running the code isn't something that happens on proper machines with fixed position tooling and known tool lengths.

What I meant was to use a probe in the spindle and touch off the workpiece (eg g54) like the user in the forum. This is quite a standard procedure.

t would be fairly easy to re-run the sim if the MDI line contained a G43 or M10,

If you see a possibility to add this for G43 then I think it would be well worth it (not sure what M10 does).

andypugh commented 1 year ago

Sorry, I mean G10. G10 is the code that sets the tool table and coordinate offsets. It's basically what Axis uses if you press the touch-off buttons. With Touchy this is very obvious, as the buttons just pre-populate the MDI with a G10. The fact that you have pressed the button lets Axis re-calculate the extents. So, if you do a touch-off with the button in Axis you will see the box move, but if you manually use G10 in MDI then I don't think you will. http://linuxcnc.org/docs/stable/html/gcode/g-code.html#gcode:g10-l1

andypugh commented 1 year ago

See https://github.com/LinuxCNC/linuxcnc/commit/5c9a79d186ea5e8b64772c71694debaddb0005bb

I think that this is a fix. It's not perfect but it will be wrong a lot less often, and for explicable reasons.

Can you pull the latest 2.9, compile and test?

1) Start sim-axis_mm 2) Run. Runs => good 3) Stop 4) MDI G43.1 Z1000 5) Run. => Errors on limit, because the check was based on stale data. => Not good but I see no easy answer. 6) Reload 7) Run. Now it errors because of extent recalc => Good 8) MDI G43.1 Z0 9) Reload 10) Run. Runs because of extent recalc => good 11) G43.1 Z1000 12) Reload 13) Run. Errors => Good, that's correct 14) G43.1 Z0 15) Run. Errors => Wrong. But press re-check-> runs => Good

Please feel free to try more combinations, bearing in mind that the limits are calculated when the G-code is loaded and they are checked when run is pressed.

Without MDI tool length changes the behaviour is now correct (I believe) and with them, there is a clearly offered recovery option in some cases.

This swaps false-positive for false-negative. But I don't see a way round that without force-reloading and we have enough issues about that with very large files.

Sigma1912 commented 1 year ago

Thank you, I'll report back.

Sigma1912 commented 1 year ago

This swaps false-positive for false-negative. But I don't see a way round that without force-reloading and we have enough issues about that with very large files.

I've tested the changes and agree. Not perfect but certainly an improvement. Thank you for your explanations and your work.