tp: keep the G33 sync origin on the spindle index

[grandixximo]

Fixes #3455.

When the axis reaches synchronized velocity, tpSyncPositionMode moved the sync origin to wherever the axis had got to:

tp->spindle.offset = tp->spindle.revs - tc->progress / tc->uu_per_rev;

That folds the lead-in ramp lag into the thread's phase. The lag is roughly omega^2 * K^2 / (2a), so it grows with the square of the spindle speed and the same thread cut at a different RPM lands at a different Z. No amount of lead-in fixes it, because the lag is absorbed rather than recovered.

This leaves the origin on the index so the target stays revs * pitch, and hands the lag to the tracking loop that already runs one line below. tpCheckSyncLeadIn warns when the segment is too short for the loop to close the error, which is the case the old code was silently hiding.

Siemens documents the same rule for G33 on the 840D: at the end of the acceleration ramp the coupling is established so that the axis position relative to the spindle zero mark is as if the axis had accelerated abruptly at the thread start position, with alarm 22280 "Programmed run-in path too short" when the run-in cannot be traversed. Fanuc does no such compensation and instead relies on the operator programming a lead-in of roughly feed x RPM / 500, which is why threading under G97 at one constant RPM is the standing advice there.

The G33 "Technical Info" block in the docs described a start point worked out from the spindle speed and the acceleration limits. The planner never did that. #3455 quotes that paragraph as the spec the code did not meet, so it is rewritten here to describe what the planner does and what lead-in the move needs.

Measured on a sim lathe, K0.1 in/rev, three G33 passes at 240/960/240 rpm in one program, Z offset from the ideal thread:

pass	before	after
240 rpm	+0.0394 mm	+0.0051 mm
960 rpm	+0.4050 mm	+0.0201 mm
240 rpm	+0.0394 mm	+0.0050 mm

The shift between the two speeds drops from 0.366 mm to 0.015 mm. What is left is half a servo cycle of travel and is linear in speed, not quadratic.

Rigid tap depth and the servo-rate sync jitter of #164 are unchanged. runtests tests passes.

Note that #581 does not fix this. It rewrites tpSyncPositionMode but keeps the same re-anchor, renamed to tp->spindle.origin.position = spindle_pos; tc->progress_at_sync = tc->progress;.

[grandixximo]

@spumco1234 want to give it a try?

[grandixximo]

@Sigma1912 probably up your alley as well :-)

[Sigma1912]

So, is the ban on spindle override for G33 and G76 still necessary with this fix?

[grandixximo]

Yes, but for different reasons than what I stated in the docs, I will send an additional PR once both fixes land to revise what having both of these working mean for the docs.

[Sigma1912]

Results look good.

Setup:
Spindle is a servo motor with drive setup to output 36kcounts/rev going into the mesa 7i95T

Method:
Halscope capture 'spindle.0.revs' (trigger on the index resetting) and encoder.0.position (linear glasscale on one of the Z gantry joints)

Results

Spindle 1000 rpm, pitch 3
rev / pos
3.006 / 41.13
14.995 / 5.16

Spindle 600 rpm, pitch 3
rev / pos
2.994 / 41.12
15.001 / 5.10

Same test with 1000/600 rpm on master showed pos (rev=3) at 42.86 and 41.78 respectively.

So I'd say your fix works just wonderfully.

[grandixximo]

That is a better result than I expected on real hardware, thanks.

Worth pointing out what your master numbers show on their own. Taking 41.12 as the true anchor, the 1000 rpm lag of 1.73 mm implies an axis acceleration of about 720 mm/s². That same figure predicts 0.62 mm at 600 rpm, and you measured 0.66. Two independent speeds agreeing within 6% is the v²/2a signature, so the mechanism in the commit message is confirmed rather than just plausible.

Could you post [AXIS_Z]MAX_ACCELERATION from the config? If it is near 720 that closes it.

One thing I want to be honest about: normalizing your two readings to the same revolution count, the residual between speeds is about 0.04 mm rather than the 0.01 the raw table suggests. I predicted half a servo cycle of travel, which for these speeds is nearer 0.01. So either there is a bit more left than I claimed, or it is cursor reading precision. Not something I can tell from here.

[Sigma1912]

Could you post [AXIS_Z]MAX_ACCELERATION from the config? If it is near 720 that closes it.

MAX_ACCELERATION = 800

The readings on the two scope traces are at the same time from trigger (eg I would get f( 0.37800) at revs as close to 3 as possible and then read the value on the encoder position by running the cursor along to f = 0.37800 ). Not sure if there is a better way than that. Also mind that those are glass scales from aliexpress. :)

I can easily rerun if I can improve on the measurements.

[grandixximo]

800 fits. My 722 assumed the whole lag was quadratic. Fitting both your speeds to lag = v²/(2a) + c instead:

1.73 = 2500/(2a) + c
0.66 =  900/(2a) + c
=>  a = 748 mm/s²,  c = 0.058 mm

748 against a configured 800 is 6.5% low, in the expected direction since it is a gantry and joint or traj limits can bind below the axis figure. And c is the residual I flagged, now with a shape: a speed-independent offset, which is not what #3455 is about since it does not vary with RPM.

On measuring it, there is a better way than matching cursors across two traces. Compute the error in HAL and scope that directly:

loadrt mult2 count=1
loadrt sum2 count=1
net spindle-revs  spindle.0.revs      mult2.0.in0
setp mult2.0.in1  3                 # pitch
net expected      mult2.0.out         sum2.0.in0
net z-actual      <your-scale-pos>    sum2.0.in1
setp sum2.0.gain0  1
setp sum2.0.gain1 -1

sum2.0.out is then the tracking error at full servo resolution, and the two speeds are two traces of the same signal you can overlay. That would settle whether c is real or a reading artifact in one run. No need to redo anything otherwise, the result stands as it is.