SIM: negative spindle speed with simulated encoder

[rmu75]

User "DauntlessA" (who it seems didn't file an issue) reported on QtPyVcp channel:

I came across this issue again in spindle_sim.hal, where the spindle speed becomes negative at or above 12500rpm, is this something that can be fixed in the base install?
It causes the spindle-at-speed signal to stay low, so when trying to run a program the simulated machine will dwell forever waiting for the spindle to reach the requested speed.

# assume 120 ppr = 480 counts/rev for the spindle

setp sim_encoder_0.ppr 2
# ^^^ used to be 12 which works up to 12499rpm, 2 works up to 74999rpm (with a 100000ns base-thread)

# iocontrol output is in rpm, but sim-encoder speed is rps
setp sim_encoder_0.scale 60
# scale encoder output to read in revolutions
#(that way thread pitches can be straightforward,
#  a 20 tpi thread would multiply the encoder output
#  by 1/20, etc)

setp encoder_0.position-scale 8
# ^^^ Used to be 48, 8 works up to just 74999rpm (with a 100000ns base-thread)

As the sim_encoder would only be used in a simulated machine config, it there a reason such a high sampling frequency needs to be used?

If sim_encoder_0.ppr and encoder_0.position-scale are both halved from their original values of 12 and 48 respectively, the issue occurs at double the spindle speed compared to before (at or above 25000rpm) and so on for additional values.

I've personally set the values for sim_encoder_0.ppr and encoder_0.position-scale to 2 and 8 respectively, so the issue doesn't occur until at or above 75000rpm, which will likely cover almost all machines.

(This is all for a base thread of 100000ns, the trigger values are all proportional to the base frequency)

and

It seems that this issue is also present in gmoccapy...

Because their hal logic is different, the negative spindle velocity and position when at or exceeding 12500rpm isn't displayed. But the spindle-at-speed issue is still present.

Found a post here:
https://www.forum.linuxcnc.org/38-general-linuxcnc-questions/58764-can-t-sim-programs-with-spindle-speed-over-12500rpm

The BASE_PERIOD in the sim config is too low for that spindle speed so the sim_encoder component cannot calculate the speed correctly.

I would definitely call this a bug, the fact that the default sampling frequency causes an issue with the default base_period if the spindle speed ≥12500rpm (and lots of people including me have programs to run on high speed spindles which exceed that)

I'm just trying to think of a justification for such a high sampling frequency in a sim, all I can think of is that this code in gmoccapy predates high speed spindles so it wasn't seen as an issue at the time?

If it is worth keeping unchanged then the other options could be a note in spindle_sim.hal or logic to detect the issue and display a warning message?

forum topic:

https://www.forum.linuxcnc.org/38-general-linuxcnc-questions/58764-can-t-sim-programs-with-spindle-speed-over-12500rpm#349793

[DauntlessAq]

Summary:

spindle_sim.hal does not function correctly when the commanded spindle speed exceeds a specific frequency which is proportional to the base period.
For the default configs, this is at or above 12500rpm for a base period of 100000ns.
(Gmoccapy and QtPyVCP/Probe_Basic simulated configurations both have a base period of 100000ns)
If the sampling frequency exceeds the base frequency, the spindle speed is not read correctly, so the spindle-at-speed never goes high, among other issues (which may be GUI-dependent).

Hal explanation:

This is spindle_sim.hal in Gmoccapy:
https://github.com/LinuxCNC/linuxcnc/blob/master/configs/sim/gmoccapy/spindle_sim.hal
I've pasted the important sections, everything else is fairly self-explanatory:

# counting the spindle encoder in software
loadrt encoder names=encoder_0
# simulate the encoder
loadrt sim_encoder names=sim_encoder_0

...

# assume 120 ppr = 480 counts/rev for the spindle
setp sim_encoder_0.ppr 12
# iocontrol output is in rpm, but sim-encoder speed is rps
setp sim_encoder_0.scale 60
# scale encoder output to read in revolutions
# (that way thread pitches can be straightforward,
#  a 20 tpi thread would multiply the encoder output
#  by 1/20, etc)
setp encoder_0.position-scale 48

...

addf sim-encoder.make-pulses    base-thread
addf encoder.update-counters    base-thread

And these are the important realtime hal components:
https://www.linuxcnc.org/docs/html/man/man9/sim_encoder.9.html
https://www.linuxcnc.org/docs/html/man/man9/encoder.9.html
http://linuxcnc.org/docs/html/man/man9/motion.9.html#SPINDLE%20PINS

The spindle speed is set by spindle.0.speed-out
(Desired spindle speed in rotations per minute.)

This float is then passed to a limit_speed component as the signal spindle-speed-cmd. The limit_speed component which uses limit_speed.maxv to simulate the mass of the spindle by limiting the frequency change of the spindle as the signal spindle-speed-limited. This works correctly.

The sim_encoder then takes the spindle-speed-limited, and creates A and B quaderature pulses, and Z index pulses. As sim_encoder_0.scale is set to 60, the sim_encoder component handles the rpm value directly.
The sim_encoder parameters are set as 12 pulses per revolution. However, there are 4 counts per pulse, so this is 48 counts per revolution. Therefore, the receiving encoder component uses a position-scale of 48, so it reports position as a (float) number of spindle rotations.

In the halfile, we run sim-encoder.make-pulses and encoder.update-counters on the base thread, which is set at a default 100000ns base period in the simulated configs for Gmoccapy and QtPyVCP/Probe_Basic. This is a 10000Hz base frequency.

Example of when this logic works correctly, for 6000rpm:
6000rpm = 100rps (100Hz)
As above, We sample this 100Hz signal 48 times per rotation, giving a sampling frequency of 4800Hz.
This works correctly as 4800Hz is below the 10000Hz base frequency. The encoder component therefore calculates both the spindle velocity and position correctly.

This works up until a spindle speed of 12500rpm, where:
12500rpm = 208.333rps (208.333Hz)
Sampling this signal 48 times per rotation gives us a sampling frequency of exactly 10000Hz, so we are sampling exactly once per rotation.
This therefore causes the encoder component to read the spindle at rotating in a negative direction, so both position and velocity are reported with an incorrect sign.
Additionally, since we cannot sample at a higher frequency than this, any higher spindle speed inputs will give the same encoder outputs.

Effect on GUIs/other processes

There are a number of issues caused by this, however they may be specific to the logic (hal or otherwise) which follows spindle_sim.hal, so differs slightly between GUIs.

Also, I haven't looked further into how this affects threading/rigid tapping since it's not an operation I'm familar with, although I'm assuming that since it is performed at low spindle speeds, then this logic will never be an issue. In fact, I'm guessing that because this logic was intended for threading/rigid tapping, that might be why this issue was missed when the logic was initially written?

In Gmoccapy, the encoder output velocity (in rps) is put through a lowpass component and then scaled to give rpm. These components do not change the sign of the velocity.

A NEAR component is then used to compare this encoder output velocity to the commanded spindle.0.speed-out. If the commanded spindle speed is exactly 12500rpm, then the NEAR component returns false as the sign is different.
When higher spindle speeds are commanded, then not only is the sign different but also the encoder spindle speed is capped at 12500rpm due to the sampling frequency cap being exceeded.
This also means that this issue occurs when the spindle speed is set using both M3 and M4, because even when the sign is calculated correctly at high speeds using M4, the spindle speed is still not calculated correctly.

Either way, because the result of the NEAR component is used as an input to both the MOTION spindle at speed signal and the Gmoccapy spindle_at_speed_led, before cutting the machine will endlessly dwell while waiting to reach the commanded speed.

The user will be aware via the Gmoccapy GUI LED that the spindle is not at speed, but it is not apparent why, as Gmoccapy's spindle_feedback_bar only displays the absolute value of the result of the limit_speed component at the start of the process.
This issue might have also been more appparent if Gmoccapy's sim config reported the output of the encoder, and not just the commanded speed.

In QtPyVCP/Probe_Basic, the issues are essentially the same as gmoccapy. One difference, however, is that a signal (spindle-rpm-filtered) connects scale_to_rpm.out to qtpyvcp.spindle-encoder-rpm.in, causing the actual encoder output to be displayed (and giving a clue as to the issue without looking at the hal logic).

Possible Fixes

First of all, I think this is definitely a bug.
12500rpm is a very common spindle speed to exceed today, now that high speed spindles are easily available. If the simulated config is broken above that speed, and it isn't obvious why, it definitely needs fixing.

The obvious options while keeping the current structure are to reduce the base period or loosen the timings for sim_encoder_0.ppr and hence encoder_0.position-scale.
I think the former seems quite drastic to fix a single issue (and will make the sim configs more difficult to run), so I personally did the latter. Making the timings 6 times looser allows 6 times the spindle speed before this issue occurs, with the issue occurring at 75000rpm and above.

setp sim_encoder_0.ppr 12
# ^^^ used to be 12 which works up to 12499rpm, 2 works up to 74999rpm (with a 100000ns base-thread)

# iocontrol output is in rpm, but sim-encoder speed is rps
setp sim_encoder_0.scale 60
# scale encoder output to read in revolutions
#(that way thread pitches can be straightforward,
#  a 20 tpi thread would multiply the encoder output
#  by 1/20, etc)

setp encoder_0.position-scale 48
# ^^^ Used to be 48, 8 works up to just 74999rpm (with a 100000ns base-thread)

But of course for both these cases the issue is still there, just harder to encounter.
I'm also not sure how much of an issue this would have on simulating threading correctly (I'm assuming at low spindle speeds the spindle speed will be measured less accurately, but since this is only a simulation and not real threading, is this really a problem?)

Another option I can think of could be to remove the simulated encoder for the general simulated configuration, and move it to a "threading_sim" config, so this issue affects far fewer users, but I get why people might not want that, because it it removing functionality from the main sim config.

And another option could be to read the value of the base period from the ini, and use it to configure encoder timings which are always loose enough to work for a high spindle speed, but if the base period is tightened, the timings can be tightened as well. But there probably isn't much point doing this...
e.g:

# Create a constant pin for the base period
loadrt constant names=constant_base_period_ns
addf constant_base_period_ns servo-thread
setp constant_base_period_ns.value [EMCMOT]BASE_PERIOD
net base_period_ns constant_base_period_ns.out

[rmu75]

There is something off with the numbers. Setting sim_encoder ppr to 12 means 12 pulses per revolution (according to sim_encoder man page), 48 counts. At 100rps that amounts to 4800Hz, which would be reasonably safe with a 10kHz (100µs/100000ns) base thread and jitter.

At 12500rpm you are exactly at 10kHz with the pulse frequency and it's clear that that can't work with a 10kHz base thread.

Additionally to changing configs to at least support simulation of somewhat-common 24000rpm spindle and correcting wrong numbers, it could make sense to add some kind of "error" output to indicate that sim_encoder can't follow the requested speed.

[grandixximo]

Opened PR #4583 with a fix.

The sign flip at 12500 rpm is a bug in sim_encoder itself, not just a config limit: at the cap the frequency-generator add value reaches exactly +2^31, and make_pulses() uses bit 31 of that value as the direction bit, so maximum positive speed is misread as negative rotation. This is also why M4 kept the correct sign at high speed, as @DauntlessAq observed: -2^31 has bit 31 set, which correctly means negative.

The PR clamps the add value below 2^31 so capped speeds saturate in the correct direction, and lowers the sim encoder count rates: mills get ppr 2 / scale 8 (cap 75000 rpm), lathes get ppr 6 / scale 24 (cap 25000 rpm while keeping 24 counts/rev for threading simulation). Per @rmu75's suggestion, sim_encoder also gains a saturated output pin that is true while the cap is engaged, plus a one-shot warning naming the maximum supported speed.

On the threading accuracy question: the encoder component computes velocity from the time between edge timestamps, so lower ppr does not cost velocity precision (measured slightly better, in fact). The only trade-off is coarser position steps on spindle.0.revs (1/8 rev for mills, 1/24 rev for lathes), which only makes sim threading backplots lurch a bit more; pitch stays exact.