motion: wait for spindle at-speed after M5 spindle stop

[grandixximo]

Wait for spindle at-speed after M5 (fixes #4158)

Problem

When a program stops the spindle (M5) and then makes a feed or probe move, motion starts immediately without waiting for the spindle to actually reach its commanded state. As reported in #4158, this is a safety hazard: a G38.n probe can begin while the tool is still spinning down, so a still-rotating tool can contact the probe.

The at-speed barrier was only ever armed by M3/M4 (spindle start). M5 went through STOP_SPINDLE_TURNING and never set the atspeed_next_feed flag, so the next feed move was never gated on spindle.N.at-speed.

Change

M5 now arms the same at-speed barrier that M3/M4 use. The next feed-type move (G1/G2/G3, and probing G38.n) waits until spindle.N.at-speed reflects the stopped state before it starts. Rapids (G0), jogs and tool-change traverses are unaffected, matching existing M3/M4 behavior.

wait_for_spindle_at_speed is plumbed through the existing path: EMC_SPINDLE_OFF carries the flag, STOP_SPINDLE_TURNING sets it, emcSpindleOff forwards it to motion, and EMCMOT_SPINDLE_OFF sets atspeed_next_feed.

Safety of the default

This is on by default, and that is safe because of how the at-speed pin is initialized. spindle.N.at-speed defaults to 1 (motion.c): when the pin is left unwired, its value stays 1, so the barrier is satisfied instantly and motion is never blocked. Only configs that wire spindle.N.at-speed to reflect actual spindle state get the new wait, which is exactly the behavior the issue asks for.

Guarding against a stuck wait

The one remaining risk is a config that wires at-speed so it never goes true while the spindle is stopped. There, the post-M5 barrier would hold motion and the operator would see an idle machine with no obvious cause. To make that self-explaining:

On abort, if motion was being held waiting for at-speed after a spindle stop, I emit a loud operator error naming the spindle and pointing at spindle.N.at-speed, and I clear the pending barrier so it cannot strand a later move. So if a misconfigured at-speed signal ever holds the machine, the abort the operator naturally reaches for is the thing that tells them why.

Scope and limitation

The barrier only holds feed-type and probing moves. A G0 move toward a probe after M5 is still not gated, since rapids are never inhibited by at-speed. This matches existing M3/M4 semantics and is called out here so it is not mistaken for full protection.

Testing

Built clean (make in src/, uspace). Verified the at-speed pin default and the abort path by inspection; no functional change for configs without at-speed wired.

Note

@BsAtHome the indentation in command.c is a nightmare to work with (mixed tabs and spaces throughout). I tried my best to match the surrounding tab style in the abort block; let me know if it looks right on your end.

[hansu]

Can this be made optional?
Normally you would turn off the spindle when your part is finished and then move the spindle away from the part so that you can safely remove the part from the vice/table.
With this change you cannot move away if the spindle is still spinning. So it's is a bit ineffective.

[grandixximo]

Move away in what way exactly?

Are you saying you have a gcode that is something like

G0 Z0
M3S1000
G1 Z-100 F100
G1 X100
M5
G1Z-80
G0Z0

and the G1Z-80 will be held up? You stop the spindle in between G1 moves, and you are expecting the move to happen as the spindle slows down?

[c-morley]

You could fix that with a smarter comparison module. By checking motion type. If Probe then set at speed false, if not then set true.

Or make this behavior only work with Probe commands.

[grandixximo]

I don't believe the kind of gcode I posted is valid, a move in G1 after M5, really? M5 truncates the TP so you have a full stop at that point anyhow, that is not the right way to write a G1 retraction, or is that on purpose? Just trying to understand if that's a real use case, you guys are leveraging the fact that it would let you move in G1 before the spindle would actually stop?

[c-morley]

True, I was thinking G0 but it gnores the spindle at speed command.

[grandixximo]

Correct

Rapids (G0), jogs and tool-change traverses are unaffected, matching existing M3/M4 behavior.

This is only about "the spindle should be at the speed I set, when I do a feed move", we already have clear separation feed move vs rapid moves for inhibit, I think doing another split for probing only for spindle-at-speed only when slowing down is just heck confusing.

The behavior I'm proposing is explained in a sentence, and is IMO the expected behavior, and what it should have been from day one.

[hansu]

More like this, bit if it does not affect G0 then it's fine

G0 X0 Y0 Z10
M3 S1000
G1 Z-10 F100
G1 X100
G1 Z0
M5
G0 Z10
G0 X-100 Y-100 Z100

[wucke13]

@grandixximo do you plan to backport this to 2.9.8 too? The patch does unfortunately not cleanly apply to 2.9.8 AFAICT

[wucke13]

I did the backport myself, at least a flaky version of it. See https://github.com/LinuxCNC/linuxcnc/pull/4161/changes for the details.

Using that (#4161), I now get the following behavior:

• waits for spindle to be stopped before the G1

  M5
  G91
  G1 Z-0.0001
  G38.2 Z-5
  G90

• does not wait for spindle to be stopped before the G1

  M5
  G91
  G1 Z0
  G38.2 Z-5
  G90

• does not wait for spindle to be stopped before the G38.2

  M5
  G91
  G38.2 Z-5
  G90

This is IMHO better, but still not satisfactory; I think that G38.n should behave exactly like G1 with regard to the waiting for spindle.at-speed. However, maybe also my poor backport did break G38.n waiting?

[grandixximo]

@wucke13 good catch, and your backport wasn't the problem. The G38.n behavior you saw is the original code in both 2.9 and master.

Probe moves don't go through the same path as G1. They use EMCMOT_PROBE, which passed a hardcoded 0 for the at-speed flag to tpAddLine, so a probe never waited for spindle.N.at-speed regardless of the spindle state. I had assumed it would wait if you spun the spindle up and it wasn't yet at speed, but that was never actually the case; the probe path ignored at-speed entirely.

I pushed a second commit that makes EMCMOT_PROBE honor the barrier like G1 does, so G38.n now waits for spin-up after M3/M4 and for stop after M5. Should work now. Could you re-test on the updated PR? Same one-spot change applies to 2.9.8 if you want it there too.

[grandixximo]

Was checking if a documentation change was needed here, and actually the new behavior matches the current docs specified behavior

---

spindle.M.at-speed IN BIT

Motion will pause until this pin is TRUE, under the following conditions: before the first feed move after each spindle start or speed change; before the start of every chain of spindle-synchronized moves; and if in CSS mode, at every rapid->feed transition.

---

It's perfectly reasonable to assume M5 causes a spindle speed change, and G38 is a feed move, therefore nothing to change in the docs, these are just edge cases that were not caught during the initial implementation.

[BsAtHome]

The default here is wait_for_atspeed=1. The default in emc.hh:426 is extern int emcSpindleOff(int spindle, int wait_for_atspeed = 0);

How does this relate?

[grandixximo]

Two layers, defaults match their use:

• STOP_SPINDLE_TURNING (canon, default 1) is called by the interpreter for a programmed stop (M5, program end, tool change). 1 arms the barrier: that's the feature.
• emcSpindleOff (task, default 0) is only called bare by emcSpindleAbort() and the estop/abort cleanup. Those must not arm a barrier, so 0.

The M5 path never uses either default: the flag is passed explicitly all the way. Interp's STOP_SPINDLE_TURNING(s) sets EMC_SPINDLE_OFF.wait_for_spindle_at_speed = 1, and emctaskmain forwards it into emcSpindleOff(spindle, msg->wait_for_spindle_at_speed). So 1 governs programmed M5, 0 governs only abort/estop.

[BsAtHome]

the indentation in command.c is a nightmare to work with

Yes, monsters are lurking (under the bed scaring us when we're awake)... There will be a time when we want to fix it without muddying the blame view.

[wucke13]

Yes, monsters are lurking (under the bed scaring us when we're awake)... There will be a time when we want to fix it without muddying the blame view.

Might I suggest that the LinuxCNC project adopts https://github.com/numtide/treefmt? That would provide to have a single tool to be called, that then in turn would call the actual formats with exactly the same arguments/configuration on every devs machine. That is what we use in our bigger projects to make sure everything is canonically formatted.

I would consider to format all the c/cpp/h/python source code. Of course the adoc, the XML derivates, mostly everything automatically. Of course the change should be made incrementally.

[wucke13]

Should work now. Could you re-test on the updated PR? Same one-spot change applies to 2.9.8 if you want it there too.

I applied 344395e over in #4161, and it does work as intended with

M5
G38.3