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

configs/sim/woodpecker/python/stdglue.py

@@ -528,6 +528,25 @@ def tool_probe_m6(self, **words):
     if not self.task:
         yield INTERP_OK
 
+    # run from line: this M6 sits on a line before the start line. Task
+    # discards everything a skipped line queues (issue #246), so the probe
+    # below would hand back a stale #5070 and abort the run. Do the tool
+    # bookkeeping and nothing else: no unit switch, no G49, no motion, so
+    # the interpreter state the start line sees is the program's own.
+    if self.params["_task_skipping"]:
+        try:
+            emccanon.CHANGE_TOOL()
+            self.current_pocket = self.selected_pocket
+            self.selected_pocket = -1
+            self.selected_tool = -1
+            # cause a sync()
+            self.set_tool_parameters()
+            self.toolchange_flag = True
+        except InterpreterException as e:
+            self.set_errormsg("tool_probe_m6 remap error: %s" % (e))
+            yield INTERP_ERROR
+        yield INTERP_OK
+
     IMPERIAL_BASED = not(bool(self.params['_metric_machine']))
 
     try:

docs/src/gcode/overview.adoc

@@ -617,6 +617,14 @@ can be added easily without changes to the source code.
   to retain proper preview, for instance when testing the success of
   a probe (G38.n) by inspecting #5070, which will always fail in the
   preview interpreter (e.g. Axis).
+* '#<_task_skipping>' - 1.0 while task is skipping the lines before a
+  run-from-line start line, 0.0 otherwise (always 0.0 in a preview
+  interpreter). Nothing a skipped line queues reaches the machine. This
+  is only a "no side effects while skipping" switch for remaps whose work
+  is not a queued command: a probe whose result would then be stale, an
+  operator wait, or Python that tracks a tool changer's state. It does
+  not say what state the start line needs; that depends on the start
+  line, which a remap cannot know from this flag.
 * '#<_call_level>' - current nesting level of O-code procedures. For debugging.
 * '#<_remap_level>' - current level of the remap stack. Each remap in a block adds one
   to the remap level. For debugging.

docs/src/remap/remap.adoc

@@ -1777,6 +1777,16 @@ See 'configs/sim/axis/remap/manual-toolchange-with-tool-length-switch/nc_subrout
 Within Embedded Python, the `task` instance can be checked for by testing 'self.task' -
 this will be 1 in the milltask instance, and 0 in the preview instance(s).
 
+A related case is run from line: task reads the lines before the start line to
+rebuild the modal state and discards what they queue, so a remapped code on
+such a line is executed by the interpreter but nothing it queues reaches the
+machine. A remap whose work is not a queued command - a probe (its result would
+be stale), an operator wait, Python that tracks a changer's state - can test
+`#<_task_skipping>` (`self.params["_task_skipping"]` in Python), which is 1
+while task is skipping, and return early. It is only that: it does not tell a
+remap what state the start line needs. See 'nc_files/remap-subroutines/qt_auto_probe_tool.ngc'
+and `tool_probe_m6` in 'nc_files/remap_lib/python-stdglue/stdglue.py'.
+
 [[remap:remappable-codes]]
 == Remappable Codes
 

nc_files/remap-subroutines/qt_auto_probe_tool.ngc

@@ -68,6 +68,14 @@ o100 if [#<_task> EQ 0]
 o100     return [999]
 o100 endif
 
+; run from line: this M6 is on a line before the start line. Task discards
+; everything a skipped line queues (issue #246), so the probe below would
+; read a stale #5070 and abort the run; a positive return lets the epilog
+; do the tool bookkeeping and nothing moves.
+o101 if [#<_task_skipping> EQ 1]
+o101     return [3]
+o101 endif
+
 ; check we are in right mode
 o110 if [#<_metric_machine>]
     o115 if [#<_imperial>]

nc_files/remap_lib/python-stdglue/stdglue.py

@@ -528,6 +528,25 @@ def tool_probe_m6(self, **words):
     if not self.task:
         yield INTERP_OK
 
+    # run from line: this M6 sits on a line before the start line. Task
+    # discards everything a skipped line queues (issue #246), so the probe
+    # below would hand back a stale #5070 and abort the run. Do the tool
+    # bookkeeping and nothing else: no unit switch, no G49, no motion, so
+    # the interpreter state the start line sees is the program's own.
+    if self.params["_task_skipping"]:
+        try:
+            emccanon.CHANGE_TOOL()
+            self.current_pocket = self.selected_pocket
+            self.selected_pocket = -1
+            self.selected_tool = -1
+            # cause a sync()
+            self.set_tool_parameters()
+            self.toolchange_flag = True
+        except InterpreterException as e:
+            self.set_errormsg("tool_probe_m6 remap error: %s" % (e))
+            yield INTERP_ERROR
+        yield INTERP_OK
+
     IMPERIAL_BASED = not(bool(self.params['_metric_machine']))
 
     try:

src/emc/rs274ngc/interp_namedparams.cc

@@ -55,6 +55,10 @@ namespace bp = boost::python;
 
 using namespace linuxcnc;
 
+// 1 while task skips lines before a run-from-line start line (issue #246).
+// Only milltask sets it; the preview and standalone rs274 leave it 0.
+int _task_skipping = 0;
+
 enum predefined_named_parameters {
     NP_LINE,
     NP_MOTION_MODE,
@@ -114,6 +118,7 @@ enum predefined_named_parameters {
     NP_SELECTED_TOOL,
     NP_VALUE_RETURNED,
     NP_TASK,
+    NP_TASK_SKIPPING,
 };
 
 /****************************************************************************/
@@ -859,6 +864,11 @@ int Interp::lookup_named_param(const char *nameBuf,
 	*value = _task;
 	break;
 
+    case NP_TASK_SKIPPING:
+	// 1 while task skips lines before a run-from-line start line (issue #246)
+	*value = _task_skipping;
+	break;
+
     default:
 	ERS(_("BUG: lookup_named_param(%s): unhandled index=%fn"),
 	      nameBuf,index);
@@ -1031,6 +1041,7 @@ int Interp::init_named_parameters()
 
   // predicate: 1 in milltask instance, 0 in UI - control preview behaviour
   init_readonly_param("_task", NP_TASK, PA_USE_LOOKUP);
+  init_readonly_param("_task_skipping", NP_TASK_SKIPPING, PA_USE_LOOKUP);
 
   // debugging aids
   init_readonly_param("_call_level", NP_CALL_LEVEL, PA_USE_LOOKUP);

src/emc/task/emctaskmain.cc

@@ -474,6 +474,43 @@ extern void emcTaskQueueTaskPlanSynchCmd();
 
 static EMC_TASK_INTERP interpResumeState = EMC_TASK_INTERP::IDLE;
 static int programStartLine = 0;	// which line to run program from
+// RUN FROM LINE (issue #246): 1 from an AUTO_RUN with a start line until the
+// skip ends. While it is set, nothing a skipped line produces -- at any call
+// level -- may leave interp_list: a queue-busting wait inside a remap or
+// subroutine used to send the queued commands to the machine before the skip
+// discarded them, so skipped code half-executed. Read by the interpreter as
+// #<_task_skipping> so a remap can skip its own side effects.
+extern int _task_skipping;	// defined in the interpreter library
+
+static void skip_end(void)
+{
+    if (!_task_skipping) return;
+    _task_skipping = 0;
+    programStartLine = 0;
+}
+
+static void skip_reset(void)
+{
+    _task_skipping = 0;
+}
+
+// Discard what a skipped line queued: report operator messages (as the skip
+// always did), then clear.
+static void skip_discard(void)
+{
+    while (interp_list.len() > 0) {
+        auto cmd = interp_list.get();
+        if (!cmd) break;
+        switch (cmd->_type) {
+        case EMC_OPERATOR_ERROR_TYPE:
+            emcOperatorError("%s", static_cast<EMC_OPERATOR_ERROR *>(cmd.get())->error);
+            break;
+        default:
+            break;
+        }
+    }
+    interp_list.clear();
+}
 // how long the interp list can be
 
 int stepping = 0;
@@ -495,48 +532,13 @@ static NML_INTERP_LIST mdi_input_queue;
 #define  MAX_MDI_QUEUE 10
 static int max_mdi_queued_commands = MAX_MDI_QUEUE;
 
-/*
-  checkInterpList(NML_INTERP_LIST *il, EMC_STAT *stat) takes a pointer
-  to an interpreter list and a pointer to the EMC status, pops each NML
-  message off the list, and checks it against limits, resource availability,
-  etc. in the status.
-
-  It returns 0 if all messages check out, -1 if any of them fail. If one
-  fails, the rest of the list is not checked.
- */
-static int checkInterpList(NML_INTERP_LIST * il, EMC_STAT * /*stat*/)
-{
-    while (il->len() > 0) {
-	auto cmd = il->get();
-
-	switch (cmd->_type) {
-
-	case EMC_OPERATOR_ERROR_TYPE: {
-	    auto error_msg = static_cast<EMC_OPERATOR_ERROR*>(cmd.get());
-    	    emcOperatorError("%s", error_msg->error);
-    	    break;
-	}
-
-	//FIXME: there was limit checking tests below, see if they were needed
-	case EMC_TRAJ_LINEAR_MOVE_TYPE:
-	    break;
-
-	case EMC_TRAJ_CIRCULAR_MOVE_TYPE:
-	    break;
-
-	default:
-	    break;
-	}
-    }
-
-    return 0;
-}
 extern int emcTaskMopup();
 
 void readahead_reading(void)
 {
     int readRetval;
     int execRetval;
+    int skipYield = 0;		// a wait was met while skipping: stop reading this cycle
 
 		if (interp_list.len() <= emc_task_interp_max_len) {
                     int count = 0;
@@ -582,6 +584,25 @@ void readahead_reading(void)
 
 			    emcTaskPlanCommand((char *) &emcStatus->task.
 					       command);
+			    // RUN FROM LINE: the skip ends before the first
+			    // main-program line at or after the start line runs --
+			    // also when the line before it never ran at level 0
+			    // (start line 1, a %-headed file).
+			    if (_task_skipping && programStartLine > 0
+				    && emcTaskPlanLevel() == 0
+				    && emcStatus->task.readLine >= programStartLine) {
+				CANON_UPDATE_END_POINT(emcStatus->motion.traj.actualPosition.tran.x,
+						       emcStatus->motion.traj.actualPosition.tran.y,
+						       emcStatus->motion.traj.actualPosition.tran.z,
+						       emcStatus->motion.traj.actualPosition.a,
+						       emcStatus->motion.traj.actualPosition.b,
+						       emcStatus->motion.traj.actualPosition.c,
+						       emcStatus->motion.traj.actualPosition.u,
+						       emcStatus->motion.traj.actualPosition.v,
+						       emcStatus->motion.traj.actualPosition.w);
+				emcTaskPlanSynch();
+				skip_end();
+			    }
 			    // and execute it
 			    execRetval = emcTaskPlanExecute(NULL);
 			    // line number may need update after
@@ -598,6 +619,12 @@ void readahead_reading(void)
 				    || execRetval == INTERP_EXIT ) {
 				emcStatus->task.interpState =
 				    EMC_TASK_INTERP::WAITING;
+			    } else if (execRetval == INTERP_EXECUTE_FINISH && _task_skipping) {
+				// RUN FROM LINE: a wait inside skipped code must not send
+				// the skipped commands to the machine. Discard below, do
+				// not wait, re-read nothing; one wait per task cycle.
+				emcStatus->task.input_timeout = 0;
+				skipYield = 1;
 			    } else if (execRetval == INTERP_EXECUTE_FINISH) {
 				// INTERP_EXECUTE_FINISH signifies
 				// that no more reading should be done until
@@ -621,24 +648,13 @@ void readahead_reading(void)
 			    // throw the results away if we're supposed to
 			    // read
 			    // through it
-			    if ( programStartLine != 0 &&
-				 emcTaskPlanLevel() == 0 &&
-				 ( programStartLine < 0 ||
-				   emcTaskPlanLine() <= programStartLine )) {
-				// we're stepping over lines, so check them
-				// for
-				// limits, etc. and clear then out
-				if (0 != checkInterpList(&interp_list,
-							 emcStatus)) {
-				    // problem with actions, so do same as we
-				    // did
-				    // for a bad read from emcTaskPlanRead()
-				    // above
-				    emcStatus->task.interpState =
-					EMC_TASK_INTERP::WAITING;
-				}
-				// and clear it regardless
-				interp_list.clear();
+			    if (_task_skipping) {
+				// RUN FROM LINE: every block executed while skipping,
+				// at any call level, is discarded before anything can
+				// send it (formerly: only main-program lines, after the
+				// whole line -- remap/subroutine bodies leaked out at
+				// their waits). Operator messages are still shown.
+				skip_discard();
 			    }
 
 			    if (emcStatus->task.readLine < programStartLine &&
@@ -663,10 +679,12 @@ void readahead_reading(void)
                                     // reset programStartLine so we don't fall into our stepping routines
                                     // if we happen to execute lines before the current point later (due to subroutines).
                                     programStartLine = 0;
+                                    skip_end();
                                 }
 			    }
 
                             if (count++ < emc_task_interp_max_len
+                                    && !skipYield
                                     && emcStatus->task.interpState == EMC_TASK_INTERP::READING
                                     && interp_list.len() <= emc_task_interp_max_len * 2/3) {
                                 goto interpret_again;
@@ -777,6 +795,7 @@ void readahead_waiting(void)
 		emcStatus->task.interpState = EMC_TASK_INTERP::IDLE;
 	    }
 	    emcStatus->task.readLine = 0;
+	    skip_reset();	// run over: never carry "skipping" into MDI or the next run
 	} else {
 	    // still executing
         }
@@ -2188,6 +2207,7 @@ static int emcTaskIssueCommand(NMLmsg * cmd)
 
     case EMC_TASK_ABORT_TYPE:
 	// abort everything
+	skip_reset();
 	emcTaskAbort();
 	// KLUDGE call motion abort before state restore to make absolutely sure no
 	// stray restore commands make it down to motion
@@ -2440,6 +2460,8 @@ static int emcTaskIssueCommand(NMLmsg * cmd)
 	}
 	run_msg = reinterpret_cast<EMC_TASK_PLAN_RUN *>(cmd);
 	programStartLine = run_msg->line;
+	skip_reset();
+	_task_skipping = (programStartLine != 0);
 	emcStatus->task.interpState = EMC_TASK_INTERP::READING;
 	emcStatus->task.task_paused = 0;
 	retval = 0;

tests/motion-logger/run-from-line-first/.gitignore

@@ -0,0 +1,3 @@
+out.motion-logger
+rs274ngc.var
+rs274ngc.var.bak

tests/motion-logger/run-from-line-first/checkresult

@@ -0,0 +1,8 @@
+#!/bin/sh
+# Run from line 1: nothing is skipped, the M442 call after it runs in full.
+cd "$(dirname "$1")" || exit 1
+fail=0
+grep -q "z=1," out.motion-logger || { echo "FAIL: G0 Z1 did not run"; fail=1; }
+grep -q "SET_LINE x=7," out.motion-logger || { echo "FAIL: X7 of the called subroutine did not run"; fail=1; }
+grep -q "y=7," out.motion-logger || { echo "FAIL: Y7 of the called subroutine did not run"; fail=1; }
+exit $fail

tests/motion-logger/run-from-line-first/mock-motion.hal

@@ -0,0 +1,3 @@
+loadusr -W motion-logger out.motion-logger
+setp iocontrol.0.emc-enable-in 1
+

tests/motion-logger/run-from-line-first/subs/m442.ngc

@@ -0,0 +1,7 @@
+o<m442> sub
+(a skipped call: one move before a wait, one after)
+G0 X7
+M66 P0 L0
+G0 Y7
+o<m442> endsub
+m2

tests/motion-logger/run-from-line-first/test-ui.py

@@ -0,0 +1,17 @@
+#!/usr/bin/env python3
+# Run-from-line with a remapped code whose subroutine waits (M66): issue #246.
+import linuxcnc, sys, time
+c = linuxcnc.command(); s = linuxcnc.stat(); e = linuxcnc.error_channel()
+c.state(linuxcnc.STATE_ESTOP_RESET)
+c.state(linuxcnc.STATE_ON)
+c.mode(linuxcnc.MODE_AUTO)
+c.program_open('test.ngc')
+c.auto(linuxcnc.AUTO_RUN, 1)
+c.wait_complete()
+start = time.time()
+while time.time() - start < 10:
+    s.poll()
+    if s.interp_state == linuxcnc.INTERP_IDLE and s.exec_state == linuxcnc.EXEC_DONE:
+        break
+    time.sleep(0.1)
+sys.exit(0)