sim_encoder: fix negative velocity at the thread rate cap, raise sim spindle speed limits - #4583
grandixximo wants to merge 2 commits into
Conversation
The frequency generator add value is freq * 2^31/maxf, and make_pulses() uses bit 31 of addval as the direction bit. The old freq clamp at +/-maxf allowed addval to reach exactly +2^31, setting bit 31, so a commanded speed at or above the cap was misread as negative rotation: the simulated counts ran backwards and encoder.N.velocity went negative (e.g. at >= 12500 rpm with ppr 12 and a 100000 ns base thread). With a spindle-at-speed interlock this makes a program dwell forever. Clamp the add value magnitude to less than 2^31 instead, so the outputs now keep turning in the commanded direction at the maximum rate. Also add a 'saturated' output pin that is true while the cap is engaged, and print a one-shot warning naming the maximum supported speed.
The software encoder is sampled in the base thread, so the quadrature count rate (rpm/60 * ppr * 4) must stay below the base thread frequency or the count saturates and spindle-at-speed never asserts. With the stock 100000 ns base period the old settings capped at 12500 rpm (ppr 12), well below common spindle speeds. Mill configs (gmoccapy, scara): ppr 12 -> 2, position-scale 48 -> 8, cap at 75000 rpm. Lathe configs (gmoccapy lathe, lathe_multispindle): ppr -> 6, position-scale -> 24, cap at 25000 rpm, keeping 24 counts/rev for threading simulation. Fixes LinuxCNC#4582 together with the sim_encoder direction-bit fix.
|
didn't try it, but LGTM. |
|
@DauntlessAq could you give it a test? |
|
On hold until 64-bit HAL merge. May need to change again. |
|
I think we should park this until after the 64-bit change. |
|
remove and replace with stepgen in velocity mode? |
|
Parked, agreed. I looked into sim_encoder against stepgen in velocity mode before answering. stepgen can't replace it: its quadrature output has A and B but no index, and these sims need the index for The replacement that fits is So after the 64-bit work I'd rework this PR: switch the six sims that use sim_encoder (gmoccapy mill, lathe and scara, |
Fixes #4582.
Root cause
sim_encoderruns its pulse generator in the base thread, so the quadrature count rate (rpm/60 × ppr × 4) cannot exceed the base thread frequency.update_speed()clamped the requested frequency at that limit (maxf), which makes the frequency-generator add value exactly +2^31.make_pulses()uses bit 31 ofaddvalas the direction bit, so any positive speed at or above the cap was misread as negative rotation: the simulated counts ran backwards andencoder.N.velocitywent negative (12500 rpm with the stock ppr 12 and a 100000 ns base period).spindle.0.at-speedthen never asserts and a program dwells forever.Verified with a halrun harness: 12000 rpm => +200 rps, 12500+ rpm => -208 rps before the fix.
Changes
saturatedoutput pin that is true while the cap is engaged (suggested by @rmu75); print a one-shot warning naming the maximum supported speed; document the pin and the cap formula in the man page.Velocity precision is unaffected (actually slightly better at low ppr): the
encodercomponent computes velocity from the time between edge timestamps, so fewer edges means a longer measurement window. The only real trade-off is coarser threading position quantization (1/8 vs 1/48 rev steps onspindle.0.revs) in the mill sims.Testing
saturatedtoggles correctly in both directions, warning fires once.tests/module-loading/sim_encoder: 7/7 pass.