Skip to content

task: run from line -- nothing a skipped line produces leaves the queue (#246) - #4671

Open
tzuohann wants to merge 4 commits into
LinuxCNC:masterfrom
tzuohann:rfl-skip-fix-master
Open

tzuohann wants to merge 4 commits into
LinuxCNC:masterfrom
tzuohann:rfl-skip-fix-master

Conversation

@tzuohann

@tzuohann tzuohann commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Part of #246. This stops skipped code from leaking out; what state the start line needs, and who sets it up, stays open on the issue.

When you run from a line, you expect every line before it to be skipped: nothing moves. Instead, if a skipped line calls a subroutine or remap that contains a wait (M66, G38, M61, an M6 remap, ...), movement can occur. Specifically, everything queued before the wait runs on the machine and everything after it is thrown away. In other words, the machine actually runs some of the "skipped" code buried in subroutines.

Run this example from line 7, you expect X and Y to stay where they are. Instead X moves to 10, and Y stays at 0.

1  o100 sub
2  G0 X10      (queue)
3  M66 P0 L0   (wait, queue sent to machine)
4  G0 Y10      (queued, then thrown away: the only move actually skipped)
5  o100 endsub
6  o100 call
7  M2

This happens because the skip and the queue disagree in emctaskmain.cc. The skip throws the queue away only after a main-program line has finished (lines 624-641). A wait first sends the queue to the machine (lines 601-609). Lines 2-4 belong to line 6, so the M66 on line 3 sends X before line 6 finishes.

The fix: while skipping, throw the queue away after every interpreter call at any depth, and let a wait neither send nor wait. The fix is not perfect: the correct tool must still be in the spindle before you run from a line, as before. Would love to dig more, but for now, this improves the line resume functionality while still requiring the correct tool in spindle before resuming, consistent with the documentation ("it does not start the spindle or confirm the proper tool"). It does prevent disasters like portions of a tool change sequence executing. Other things that leak today: a skipped G38 probe moves, M61 changes the tool number, and an M64 output turns on and stays on.

More details below.


Details

More examples: the same 7-line program with other contents inside o100, run from its last line (M2). All results were predicted before the runs and measured in simulation.

inside o100 without the fix with the fix
G0 X10, G0 Y10 (no wait: the control) nothing nothing
G0 X10, M66 P0 L0, G0 Y10, M66 P0 L0 X 10, Y 10 nothing
G0 X10, G38.3 Z-5, G0 Y10 X 10, probe runs nothing
G0 X10, M61 Q14, G0 Y10 X 10, tool 12 -> 14 nothing
M64 P0, M66 P1 L0, M65 P0 output 0 stays on nothing

The change (emctaskmain.cc, interp_namedparams.cc):

  • A flag, _task_skipping, is set by run from line. It ends just before the first main-program line at or after the start line, and is cleared at program end and on abort.
  • While skipping, queued commands are thrown away after every interpreter call, at any call depth. A wait neither sends them nor waits.
  • #<_task_skipping> (read-only, documented in gcode/overview and remap) is only a "no side effects while skipping" switch for remaps whose work is not a queued command: a probe (its result would be stale), an operator wait, Python that tracks a changer's state. It does not say what state the start line needs.
  • tool_probe_m6 (both stdglue copies) and qt_auto_probe_tool.ngc (the M6 remap woodpecker/qtdragon wire) return after the tool bookkeeping when skipped; before, their skipped G38.2 left #5070 == 0 in a fresh session and the run aborted.

Behavior changes:

  • Run from line 1 now behaves as a normal run. Before, line 1 itself was thrown away.
  • Skipped probes and tool changes behave like skipped main-program lines always have: nothing reaches the machine.

Follow-up, not in this PR: an opt-in [RS274NGC]ON_RUN_FROM_LINE hook at the end of the skip, before the synch, could snapshot wanted-versus-actual state (tool, spindle, coolant, feed, offsets, last endpoint) into read-only parameters and call a user sub that decides per machine what to restore. That is where the open questions on #246 (re-probe or not, tool check, restored outputs) lead.

Tests: tests/motion-logger/run-from-line-wait (the example above: no skipped move may reach motion) and run-from-line-first (from line 1 everything runs). mountaindew and basic still pass.

…ue (LinuxCNC#246)

A wait (M66, G38, M6 epilog, M61, user M-codes) inside a subroutine or
remap on a skipped line sent the queued commands to motion before the
skip discarded them, so skipped code half-executed. While skipping,
discard at every call level and let a wait neither send nor wait. The
skip ends before the first main-program line at or after the start line
(this also fixes run from line 1). #<_task_skipping> lets remaps skip
their own work.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@tzuohann
tzuohann force-pushed the rfl-skip-fix-master branch from aee4dec to 36de389 Compare October 12, 2026 00:13
tzuohann and others added 2 commits October 11, 2026 21:45
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…dew does

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@grandixximo

Copy link
Copy Markdown
Contributor

Thanks for digging into this one. The core change looks right to me: discarding after every interpreter call at any depth, and not letting a wait flush the queue while skipping, is exactly the gap zultron described in #246. Nothing from skipped code should reach motion, and with this PR it doesn't.

A few points.

Please don't close #246 with this PR. I would change "Fixes #246" to "Part of #246". This PR stops skipped code from leaking out, which is necessary, but the issue discussion (andypugh's comments in particular) is about the larger question of what state the start line needs and who sets it up. Your three reviewer questions are that same question, and it stays open after this merges.

Skipped probes now return stale results instead of re-probing. Inside a sub or remap, a skipped G38 used to physically run, which was the bug. Now it is discarded, and on resume set_probe_data() loads #5061-#5070 from motion's single last probe record. Main-program probes already behaved this way before the PR, so this is not new in kind, but it now reaches remaps. The visible case is stdglue's tool_probe_m6 (configs/sim/woodpecker/python/stdglue.py and nc_files/remap_lib/python-stdglue/stdglue.py): after a skipped G38.2 it checks self.params[5070] == 0, so in a fresh session where nothing has probed yet, a skipped M6 aborts the run from line. Could both stdglue copies return early on #<_task_skipping>, so a skipped M6 just does the bookkeeping and leaves the physical work alone?

On #<_task_skipping>: I think it is useful, but only as a narrow "no side effects while skipping" switch for remaps (probes, operator waits, Python remaps that track a changer's state). It is not a general run-from-line tool: unlike #<_task>, whether something should run depends on the start line and on what state that line needs, and a remap cannot know either from a single flag. I would document it with that limited scope.

On your three questions, I would not decide them in core. At skip_end(), before emcTaskPlanSynch(), the interpreter still holds what the program intended: the tool after the skipped tool changes, spindle direction and speed, flood and mist, feed, modal state, offsets, and the endpoint of the last move. synch() then overwrites all of that with the machine state and the information is gone. An opt-in [RS274NGC]ON_RUN_FROM_LINE=O<rfl> call, in the spirit of ON_ABORT_COMMAND, could snapshot that state into read-only parameters (wanted versus actual), call the user sub, and synch afterwards. The user sub then decides per machine: tool change if the tool differs, M3, M8, a safe approach move, re-probe or not. Digital and analog outputs are not tracked by the interpreter, so those would need extra work. That is clearly a follow-up and not something this PR should grow into, but it is where I think questions 1-3 lead, and it is a reason to keep #246 open.

…document #<_task_skipping>

A remapped M6 that probes tool length ran its G38 in the interpreter while
task skipped the line; the probe is discarded, #5070 stays 0 in a fresh
session, and the remap aborted the run. tool_probe_m6 (both stdglue copies)
and qt_auto_probe_tool.ngc now return after the tool bookkeeping when
#<_task_skipping> is set, before any unit switch, G49 or motion.

#<_task_skipping> is documented as a "no side effects while skipping"
switch for remaps only; it does not say what state the start line needs.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@tzuohann

Copy link
Copy Markdown
Contributor Author

Thanks for the review.

  • Changed to "Part of Run from line does not work with a remapped G-code toolchange routine #246".
  • tool_probe_m6 in both stdglue copies now stops after the tool bookkeeping when #<_task_skipping> is set. The same guard is in nc_files/remap-subroutines/qt_auto_probe_tool.ngc (the M6 remap woodpecker and qtdragon actually wire; the config copies are symlinks): its two #5070 EQ 0 checks aborted a skipped M6 the same way, so it now returns 3 before any motion and change_epilog does the bookkeeping.
  • #<_task_skipping> is documented with that limited scope in docs/src/gcode/overview.adoc and docs/src/remap/remap.adoc.
  • Agreed on ON_RUN_FROM_LINE as a follow-up; the description now points there instead of asking the three questions.
  • Three other shipped M6 remaps still abort (not half-run) when skipped in a fresh session, because they probe or wait: configs/sim/gmoccapy/macros/change.ngc (G38.2, #5070), manual-toolchange-with-tool-length-switch/nc_subroutines/manual_change.ngc (M66 and G38.3), VMC_toolchange/toolchange.ngc (M66 waits). Happy to give them the same guard here or in a follow-up, whichever you prefer.

@grandixximo

Copy link
Copy Markdown
Contributor

Thanks, the changes look good, and keeping the guard text scoped like that in the docs is what I had in mind.

On the other three: here is fine, but I ran them on this branch first (motion-logger, fresh session, T2 M6 on a skipped line) and only manual_change.ngc actually aborts (return -2), so that's the only one I'd guard. gmoccapy change.ngc calls the builtin M6 inside the remap, so change_epilog takes the builtin_used branch and ignores the negative return; it doesn't abort (nor on a real probe failure, but that's a separate bug). VMC toolchange.ngc doesn't abort either: with input_timeout forced to 0, #5399 reads the pin instead of -1. The same builtin_used path applies to qt_auto_probe_tool.ngc, so its guard isn't strictly needed, though I'd keep it since it skips the stale-probe math.

One small thing in the stdglue comment: after a skipped M6 the interpreter's current tool is the one actually in the spindle (the tool table reload undoes the bookkeeping), not the program's, so "the interpreter state the start line sees is the program's own" isn't quite true for the tool.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants