Skip to content

Commit 7009976

Browse files
greatEndianclaude
andcommitted
fix(g28): don't restore a coordinated mode a partially-homed machine can't hold
Addresses the first of grandixximo's findings on PR LinuxCNC#4172, and the failure Sigma1912 hit on real hardware (Mesa 7I95T gantry): "g28.3 p0" reported "all joints must be homed before going into coordinated mode", greyed out the GUI's mode controls, and left the machine needing F2 to recover. The G28.2/G28.3 sequencing dips motion into FREE (do_homing() only advances there) and restores the previous trajectory mode when the command finishes. That restore was gated only on the command having succeeded for its *target* joint. But a per-joint G28.2 Pn / G28.3 Pn can succeed for its own joint while leaving the machine as a whole unreferenced, and motion refuses to (re-)enter TELEOP or COORD in that state on non-identity kinematics -- switch_to_teleop_mode() (motion.c) and the EMCMOT_COORD case (command.c) both gate on "kinType != KINEMATICS_IDENTITY && !get_allhomed()". So the restore was silently rejected while task still reported DONE: the program advanced to the next line, motion had no valid frame for it, and the machine sat stranded in FREE. Sigma's third test is the same bug cascading -- the g28.3 p2 breaks the mode state, then the following g28 and g0 fail with "need to be enabled, in coord mode". Mirror motion's own condition before restoring, and fail the command cleanly (staying in FREE, with an operator error) instead of reporting success and stranding the operator. Identity kinematics are unaffected: motion permits the restore there, so the behaviour is unchanged for trivkins. Note this only reaches the buggy path with NO_FORCE_HOMING=1. With the default 0, the pre-existing NO_FORCE_HOMING re-check catches a partial unhome first -- which, together with the existing tests using trivkins, is why neither the sim tests nor Sigma's first test caught it. The kinematics type is read from emcStatus->motion.traj.kinematics_type rather than this file's static emcmotConfig: that copy is filled in once just before the main loop and never refreshed, so it goes stale as soon as switchkins changes kinematics at runtime (G43.4/G43.5). taskintf.cc re-reads the motion config whenever config_num changes and republishes it in status. Tests: adds gcode-homing/nonidentity-restore, which needs both knobs the existing coverage lacks -- corexykins (KINEMATICS_BOTH) and NO_FORCE_HOMING=1. The program is "G28.3 P0 / M64 P0 / M2"; the digital output is the witness, since it needs no coordinated motion and so would still run with the machine stuck in FREE. Verified the test actually catches the regression by temporarily reverting the fix and confirming it fails -- the interpreter never returns to idle, with dout0=1 proving the program had carried on past the G28.3 -- then restored the fix and confirmed it passes. Verified: tests/interp/gcode-homing (6/6), and the full tests/interp + tests/motion-logger suite (89/89, 1 pre-existing skip). One flush-order failure seen in an earlier sweep did not reproduce (89/89 on re-run, 8/8 in isolation including under load); it uses trivkins, where this change is a no-op by construction. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent a2bade8 commit 7009976

7 files changed

Lines changed: 326 additions & 0 deletions

File tree

src/emc/task/emctaskmain.cc

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2927,6 +2927,43 @@ static int emcTaskExecute(void)
29272927

29282928
homingWaiting = false;
29292929
emcTaskEager = 1;
2930+
2931+
// Is it legal to hand the machine back to the coordinated mode it
2932+
// was in before the FREE dip? Motion refuses to (re-)enter TELEOP
2933+
// or COORD on non-identity kinematics unless *every* joint is
2934+
// homed -- switch_to_teleop_mode() (motion.c) and the EMCMOT_COORD
2935+
// case (command.c) both gate on
2936+
// "kinType != KINEMATICS_IDENTITY && !get_allhomed()".
2937+
//
2938+
// A per-joint G28.2 Pn / G28.3 Pn can succeed for its own joint
2939+
// while leaving the machine as a whole unreferenced, so restoring
2940+
// unconditionally means motion rejects the request, task still
2941+
// reports DONE, and the machine is stranded in FREE with the GUI's
2942+
// mode controls greyed out -- recoverable only by cycling the
2943+
// controller (PR #4172: Sigma1912's "g28.3 p0" on a gantry gave
2944+
// "all joints must be homed before going into coordinated mode",
2945+
// then needed F2). Mirror motion's own condition here and abort
2946+
// cleanly instead of wedging.
2947+
//
2948+
// Mirroring the condition rather than issuing the restore and
2949+
// checking whether it took is deliberate: the COORD/TELEOP
2950+
// transition is deferred to the controller cycle (see
2951+
// "defer transition to controller cycle" in command.c), so an
2952+
// immediate read-back would race exactly the way the old
2953+
// .homing-based completion test did.
2954+
//
2955+
// Read the kinematics type from status, not from this file's
2956+
// static emcmotConfig: that copy is filled in once just before
2957+
// the main loop and never refreshed, so it goes stale the moment
2958+
// switchkins changes kinematics at runtime (G43.4/G43.5).
2959+
// taskintf.cc re-reads the motion config whenever config_num
2960+
// changes and republishes it as traj.kinematics_type, so the
2961+
// status field is the one that tracks a switchkins change.
2962+
const bool restore_ok = (homingPriorMode == EMC_TRAJ_MODE::FREE)
2963+
|| (emcStatus->motion.traj.kinematics_type
2964+
== KINEMATICS_IDENTITY)
2965+
|| all_homed();
2966+
29302967
if (success && homingIsUnhome && !all_homed() && !no_force_homing) {
29312968
// Close the hole a per-move check would be expensive to plug:
29322969
// [TRAJ]NO_FORCE_HOMING=0 (the default) already refuses to
@@ -2943,6 +2980,17 @@ static int emcTaskExecute(void)
29432980
// Same reasoning as the failure path below: leave it in FREE,
29442981
// don't snap back to a mode an unhomed machine can't legally
29452982
// run coordinated motion in.
2983+
} else if (success && !restore_ok) {
2984+
// The command itself did what was asked, but it left the
2985+
// machine partially referenced and motion will not take
2986+
// TELEOP/COORD back in that state. Stay in FREE and fail the
2987+
// program rather than report DONE and strand the operator.
2988+
emcOperatorError(_("%s succeeded but left the machine not fully "
2989+
"homed -- staying in joint mode, as non-identity "
2990+
"kinematics cannot re-enter coordinated motion "
2991+
"until every joint is homed"),
2992+
homingIsUnhome ? "G28.3 unhome" : "G28.2 home");
2993+
emcStatus->task.execState = EMC_TASK_EXEC::ERROR;
29462994
} else if (success) {
29472995
emcStatus->task.execState = EMC_TASK_EXEC::DONE;
29482996
if (homingPriorMode != EMC_TRAJ_MODE::FREE) {
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
#!/bin/sh
2+
exit 0 # test failure is indicated by test.sh exit value

tests/interp/gcode-homing/nonidentity-restore/sim.tbl

Whitespace-only changes.
Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
#!/usr/bin/env python3
2+
3+
"""
4+
Regression test: a partial G28.3 Pn on NON-IDENTITY kinematics must abort
5+
cleanly instead of stranding the machine.
6+
7+
Motion refuses to (re-)enter TELEOP or COORD while any joint is unhomed, but
8+
only when the kinematics are not identity -- switch_to_teleop_mode() (motion.c)
9+
and the EMCMOT_COORD case (command.c) both gate on
10+
"kinType != KINEMATICS_IDENTITY && !get_allhomed()".
11+
12+
The G28.2/G28.3 sequencing dips motion into FREE (do_homing() only advances
13+
there) and restores the previous trajectory mode afterwards. Restoring that
14+
mode unconditionally is wrong: a per-joint G28.3 Pn succeeds for its own joint
15+
while leaving the machine as a whole unreferenced, so motion rejects the
16+
restore, task still reports DONE, and the machine is stranded in FREE with the
17+
GUI's mode controls greyed out -- recoverable only by cycling the controller.
18+
Reported on real hardware (Mesa 7I95T gantry) in PR #4172, where it surfaced as
19+
"all joints must be homed before going into coordinated mode" followed by a
20+
dead UI needing F2.
21+
22+
Note this needs NO_FORCE_HOMING=1. With the default 0, an earlier branch
23+
catches the partial-unhome case first and this path is never reached -- which
24+
is exactly why the trivkins `sequencing` test (NO_FORCE_HOMING unset, identity
25+
kins) passes both before and after the fix.
26+
27+
The witness is deliberately M64 P0 rather than a move: a digital output needs
28+
no coordinated motion, so it still executes with the machine stuck in FREE.
29+
If it ends up set, the program continued past the G28.3 -- the bug.
30+
"""
31+
32+
import linuxcnc
33+
import hal
34+
35+
import os
36+
import sys
37+
import time
38+
39+
h = hal.component("python-ui")
40+
h.ready()
41+
42+
c = linuxcnc.command()
43+
s = linuxcnc.stat()
44+
e = linuxcnc.error_channel()
45+
46+
47+
def poll():
48+
s.poll()
49+
50+
51+
def fail(msg):
52+
print("FAIL: " + msg)
53+
sys.exit(1)
54+
55+
56+
def near(a, b, tol=0.001):
57+
return abs(a - b) < tol
58+
59+
60+
def wait_idle(timeout=10.0):
61+
t0 = time.time()
62+
while time.time() - t0 < timeout:
63+
poll()
64+
if s.interp_state == linuxcnc.INTERP_IDLE:
65+
return True
66+
time.sleep(0.01)
67+
return False
68+
69+
70+
def wait_homed(expected, timeout=10.0):
71+
t0 = time.time()
72+
while time.time() - t0 < timeout:
73+
poll()
74+
if list(s.homed[:3]) == expected:
75+
return True
76+
time.sleep(0.01)
77+
return False
78+
79+
80+
def drain_errors():
81+
msgs = []
82+
while True:
83+
err = e.poll()
84+
if not err:
85+
return msgs
86+
msgs.append(err[1])
87+
88+
89+
c.state(linuxcnc.STATE_ESTOP_RESET)
90+
c.state(linuxcnc.STATE_ON)
91+
c.home(0)
92+
c.home(1)
93+
c.home(2)
94+
if not wait_homed([1, 1, 1]):
95+
fail("initial home-all did not home all joints: {}".format(list(s.homed[:3])))
96+
97+
# Confirm the machine really is in a coordinated mode and can move, so that the
98+
# restore this test is about has something meaningful to restore to.
99+
c.mode(linuxcnc.MODE_MDI)
100+
c.mdi("G0 X1")
101+
if not wait_idle():
102+
fail("setup MDI move did not settle")
103+
poll()
104+
if not near(s.position[0], 1.0):
105+
fail("setup MDI move did not run (X at {}) -- machine not in a usable coordinated mode".format(s.position[0]))
106+
drain_errors()
107+
108+
# Run the program: G28.3 P0 (partial unhome) then M64 P0 (the witness).
109+
c.mode(linuxcnc.MODE_AUTO)
110+
c.program_open(os.path.join(os.path.dirname(os.path.abspath(__file__)), "test.ngc"))
111+
c.auto(linuxcnc.AUTO_RUN, 0)
112+
if not wait_idle():
113+
# This is the unfixed behaviour: task reported DONE for the G28.3, motion
114+
# silently refused to take TELEOP/COORD back, and the interpreter now sits
115+
# forever on a line it can never run. A timeout here IS the wedge.
116+
poll()
117+
fail(
118+
"interpreter never returned to idle after the G28.3 -- the machine is "
119+
"wedged: task accepted the partial unhome and tried to restore a "
120+
"coordinated mode that motion cannot grant while unreferenced "
121+
"(motion_mode={}, homed={}, dout0={})".format(
122+
s.motion_mode, list(s.homed[:3]), s.dout[0])
123+
)
124+
time.sleep(0.3)
125+
poll()
126+
127+
if list(s.homed[:3]) == [1, 1, 1]:
128+
fail("G28.3 P0 did not actually unhome joint 0: {}".format(list(s.homed[:3])))
129+
130+
if s.dout[0]:
131+
fail(
132+
"program continued past a G28.3 that left the machine partially homed "
133+
"(digital-out 0 got set): the mode restore was attempted and silently "
134+
"refused by motion, leaving the machine stranded in FREE while task "
135+
"reported DONE"
136+
)
137+
print("PASS: a partial G28.3 Pn aborts the program on non-identity kinematics")
138+
139+
if s.motion_mode != linuxcnc.TRAJ_MODE_FREE:
140+
fail("expected to be left in FREE after the aborted G28.3, got motion_mode={}".format(s.motion_mode))
141+
print("PASS: machine is left in FREE, the only mode it may legally sit in unreferenced")
142+
143+
msgs = drain_errors()
144+
if not msgs:
145+
fail("the aborted G28.3 reported no error at all -- the operator gets no explanation")
146+
print("PASS: an operator error was reported ({!r})".format(msgs[0][:70]))
147+
148+
# The whole point of failing cleanly rather than wedging: ordinary recovery
149+
# must work, with no controller restart.
150+
c.mode(linuxcnc.MODE_MANUAL)
151+
c.home(0)
152+
if not wait_homed([1, 1, 1]):
153+
fail("could not re-home joint 0 after the aborted G28.3 -- machine is wedged")
154+
155+
c.mode(linuxcnc.MODE_MDI)
156+
c.mdi("G0 X2")
157+
if not wait_idle():
158+
fail("post-recovery MDI move did not settle")
159+
poll()
160+
if not near(s.position[0], 2.0):
161+
fail("coordinated motion did not come back after re-homing (X stayed at {})".format(s.position[0]))
162+
print("PASS: re-homing recovers coordinated motion without cycling the controller")
163+
164+
print("done! it all worked")
165+
sys.exit(0)
Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
[EMC]
2+
DEBUG = 0
3+
VERSION = 1.1
4+
5+
[DISPLAY]
6+
DISPLAY = ./test-ui.py
7+
8+
[TASK]
9+
TASK = milltask
10+
CYCLE_TIME = 0.001
11+
12+
[RS274NGC]
13+
PARAMETER_FILE = sim.var
14+
15+
[EMCMOT]
16+
EMCMOT = motmod
17+
COMM_TIMEOUT = 4.0
18+
BASE_PERIOD = 0
19+
SERVO_PERIOD = 1000000
20+
21+
[HAL]
22+
HALUI = halui
23+
HALFILE = LIB:core_sim.hal
24+
25+
[TRAJ]
26+
AXES = 3
27+
COORDINATES = X Y Z
28+
HOME = 0 0 0
29+
LINEAR_UNITS = inch
30+
ANGULAR_UNITS = degree
31+
DEFAULT_LINEAR_VELOCITY = 1.2
32+
NO_FORCE_HOMING = 1
33+
MAX_LINEAR_VELOCITY = 4
34+
35+
[KINS]
36+
JOINTS = 3
37+
KINEMATICS = corexykins
38+
39+
[AXIS_X]
40+
MAX_VELOCITY = 4
41+
MAX_ACCELERATION = 1000.0
42+
MIN_LIMIT = -40.0
43+
MAX_LIMIT = 40.0
44+
45+
[AXIS_Y]
46+
MAX_VELOCITY = 4
47+
MAX_ACCELERATION = 1000.0
48+
MIN_LIMIT = -40.0
49+
MAX_LIMIT = 40.0
50+
51+
[AXIS_Z]
52+
MAX_VELOCITY = 4
53+
MAX_ACCELERATION = 1000.0
54+
MIN_LIMIT = -4.0
55+
MAX_LIMIT = 4.0
56+
57+
[JOINT_0]
58+
TYPE = LINEAR
59+
HOME = 0.000
60+
MAX_VELOCITY = 4
61+
MAX_ACCELERATION = 1000.0
62+
BACKLASH = 0.000
63+
INPUT_SCALE = 4000
64+
OUTPUT_SCALE = 1.000
65+
MIN_LIMIT = -40.0
66+
MAX_LIMIT = 40.0
67+
FERROR = 0.050
68+
MIN_FERROR = 0.010
69+
70+
[JOINT_1]
71+
TYPE = LINEAR
72+
HOME = 0.000
73+
MAX_VELOCITY = 4
74+
MAX_ACCELERATION = 1000.0
75+
BACKLASH = 0.000
76+
INPUT_SCALE = 4000
77+
OUTPUT_SCALE = 1.000
78+
MIN_LIMIT = -40.0
79+
MAX_LIMIT = 40.0
80+
FERROR = 0.050
81+
MIN_FERROR = 0.010
82+
83+
[JOINT_2]
84+
TYPE = LINEAR
85+
HOME = 0.0
86+
MAX_VELOCITY = 4
87+
MAX_ACCELERATION = 1000.0
88+
BACKLASH = 0.000
89+
INPUT_SCALE = 4000
90+
OUTPUT_SCALE = 1.000
91+
MIN_LIMIT = -4.0
92+
MAX_LIMIT = 4.0
93+
FERROR = 0.050
94+
MIN_FERROR = 0.010
95+
96+
[EMCIO]
97+
TOOL_CHANGE_QUILL_UP = 1
98+
RANDOM_TOOLCHANGER = 0
99+
TOOL_TABLE = sim.tbl
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
; A G28.3 Pn that leaves the machine only partially referenced must ABORT the
2+
; program on non-identity kinematics, because motion will not hand TELEOP/COORD
3+
; back until every joint is homed.
4+
;
5+
; M64 P0 is the witness: it needs no coordinated motion, so it would run happily
6+
; even with the machine stranded in FREE. If digital-out 0 ends up set, the
7+
; program carried on past the G28.3 -- which is the bug this test guards.
8+
G28.3 P0
9+
M64 P0
10+
M2
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
#!/bin/bash
2+
exec linuxcnc -r test.ini

0 commit comments

Comments
 (0)