Repository navigation
task: do not take a motion status from before a command as current (#3650) - #4666
Open
grandixximo wants to merge 5 commits into
Open
grandixximo wants to merge 5 commits into
grandixximo wants to merge 5 commits into
Conversation
The test sent the home commands right after the machine-on command, all three at once. Wait until the machine is on, and home one joint at a time, since a home command is refused while another joint homes.
grandixximo
changed the title
grandixximo
force-pushed
the
probe-queue-test
branch
from
October 12, 2026 10:59
c19ae21 to
d879668
Compare
BsAtHome
reviewed
Oct 12, 2026
Comment on lines
+590
to
+592
| uint64_t commandHeartbeat; /* heartbeat when the command was echoed: | ||
| the controller has acted on the command | ||
| once heartbeat has moved past it */ |
Contributor
There was a problem hiding this comment.
Isn't motion.h used in RTAPI code (RT)? That means you cannot use uint64_t and need to use rtapi_u64.
Contributor
Author
There was a problem hiding this comment.
Good morning, you were missed on Sunday's meeting, hope I didn't mess things up too much ;-)
I will change this and heartbeat as well, so we compare with apples with apples.
Contributor
There was a problem hiding this comment.
Good morning, you were missed on Sunday's meeting
Life happens once in a while... And, since nobody is paying me for the work, well, I do not have any obligation to attend 8-)
…command Motion echoes a command in motion-command-handler, but motion-controller acts on it: it queues the move, enables the machine, switches mode, and then publishes the queue depth and state. A status read between the two shows the command taken and motion idle, so task takes motion as done. After a G38 move the SYNCH then finishes at once and the next line reads the result of the probe before, or fails with "Queue is not empty after probing" (LinuxCNC#3650), and wait_complete() returns before the machine is on or in auto mode. Record the heartbeat at which each command is echoed, and report motion busy until the heartbeat has moved past it, that is until the controller has run once after taking the command. motion-logger has no controller and counts each command as acted on. Both heartbeat fields are rtapi_u64, the RTAPI type for headers RT code includes.
emcMotionUpdate() returns early when the status or config read fails, which happens while motion-controller runs, and leaves the motion status as it was. Right after a command that copy predates it, and with a probe just queued it says motion is done. Report motion busy until a read that has seen the command succeeds.
Probe at alternating heights, and check that the machine is on and in auto mode once wait_complete() returns. late-controller runs the controller a 50 ms servo period after the command handler, slow-controller uses kinematics that make the controller take most of the servo period, so that status reads fail. Both fail on master, and each needs one of the two changes before.
grandixximo
force-pushed
the
probe-queue-test
branch
from
October 12, 2026 11:45
d879668 to
7ad67f6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the root cause of #3650; #3971 stays as a backstop.
Motion echoes a command in
motion-command-handler, butmotion-controlleracts on it later: it queues the move, enables the machine, switches mode, then publishes queue depth and state. Task can take a status from before the command as current in two ways:Task then finishes the command too soon. After a G38 move the SYNCH completes at once, and the next line either fails with "Queue is not empty after probing" (what #3971 catches) or silently reads the previous probe's result when the stale window outlasts the synch.
wait_complete()also returns before the machine is on or in auto mode.Fix:
Tests:
probe-queue-not-empty: 100 G38.2 probes in AUTO withservo_delay, a test-only comp that busy waits between handler and controller. Fails on master before task: tighten AUTO read-ahead wait-clear to match MDI (#3650) #3971, 8 of 8.stale-motion-status/late-controller: controller runs before the handler at a 50 ms period, so a whole period separates them.stale-motion-status/slow-controller: kinematics that busy wait most of the 1 ms period, so task's status reads often fail.stale-motion-statustests probe at alternating heights and fail if a probe reads the height of the one before, and check machine on and auto mode after eachwait_complete().Each change is needed: without the heartbeat check late-controller fails, without the guard slow-controller fails (2 of 3 runs).
The tests that compile a comp skip on package builds (
SYSTEM_BUILD).runtests -upasses locally exceptui-smoke/touchy-fit, which fails the same way on the base commit (#4656, fixed by #4658).