Add ARC_SEGMENTS_PER_SEC for finer G2/G3 arcs

[vector76]

Description

The MM_PER_ARC_SEGMENT approach has a weakness where high values lose resolution (most noticeable on small-radius arcs) and low values risk flooding the system with huge numbers of small segments if a high feedrate is used.

Instead of fixing the length of the arc segment, this update adds the ability to derive the segment length from the feed rate. Faster curves use coarser physical resolution and slower curves use finer resolution.

Benefits

CNC firmware can use a wide range of feed rates and there can be wide differences in expectations as far as what resolution is considered reasonable. A plotter may expect to draw very fast, while a helical plunge to drill a small hole may expect a circle and not a hexagon. With feedrate-derived arc segment lengths, both of these can be supported with common firmware.

Related Issues

[thinkyhead]

Thanks for this enhancement. I was contemplating how to decide the best segment length for any given arc and how to deal with small circles losing detail. A segments-per-second limit is a good approach that can be tailored for what the machine can handle.

[thinkyhead]

Since we usually use the segments-per-second value for settings the option is now called… ARC_SEGMENTS_PER_SEC.

[AnHardt]

There is a logic or linguistic error.
#define MM_PER_ARC_SEGMENT 1 // (mm) Length (or minimum length) of each arc segment -> No segment can be smaller than 1 mm!
#define MIN_ARC_SEGMENTS 24 // Minimum number of segments in a complete circle -> When the circumference of the circle is smaller than 24 mm print a circle with not smaller than that - because the segments cant be smaller then 1mm.

One of this defines is unlikely to be a minimum. Else arcs with small diameters are impossible.

[vector76]

MIN_ARC_SEGMENTS overrides MM_PER_ARC_SEGMENT to make possibly shorter segments. Personally I don't like MIN_ARC_SEGMENTS because it is trying to enforce smoothness irrespective of radius or feed rate, and it can generate excessive resolution that causes stalls.

[AnHardt]

So MM_PER_ARC_SEGMENT isn't a minimum but a maximum segment length.
And is that is right now, there is a conflict with:
//#define ARC_SEGMENTS_PER_SEC 50 // Use feedrate to choose segment length (with MM_PER_ARC_SEGMENT as the minimum)
If it's a maximum as well as a minimum there is no other possible value.

[AnHardt]

With MIN_STEPS_PER_SEGMENT we already have a lower limit for the length of one segment.

[vector76]

MM_PER_ARC_SEGMENT, when not using ARC_SEGMENTS_PER_SEC, is the 'default' segment length and it can be shorter if MIN_ARC_SEGMENTS implies more smoothness than MM_PER_ARC_SEGMENT provides.

When using ARC_SEGMENTS_PER_SEC, the 'usual' behavior is for segment length to be determined by the feed rate but for very low feed rates the segments can become extremely short. If the feed rate is low enough that the segments are shorter than MM_PER_ARC_SEGMENT then it uses MM_PER_ARC_SEGMENT instead, so in that sense it is a minimum on the speed-derived segment length. But the MIN_ARC_SEGMENTS is still in effect, so a very fast small circle might be a hexagon according to ARC_SEGMENTS_PER_SEC, and MIN_ARC_SEGMENTS of 7 or more would force a smoother curve and the segments would be made shorter (and the segment rate would be higher than ARC_SEGMENTS_PER_SEC).

Not all combinations of these settings make sense, for example a large MM_PER_ARC_SEGMENT and a high rate of segments per second would mean the MM_PER_ARC_SEGMENT would almost always take precedence and the segments per second has no effect. A large value for MIN_ARC_SEGMENTS would mean that small circles will ignore feed rate or (otherwise) minimum resolution and proceed with possibly extremely fine resolution that has no benefit.

Yes MIN_STEPS_PER_SEGMENT causes small segments to collapse, so there is no real benefit resolution-wise for MM_PER_ARC_SEGMENT to be much less than that.

I think a combination of settings that makes sense is for ARC_SEGMENTS_PER_SEC to be a value that the CPU can keep up with comfortably, and for MM_PER_ARC_SEGMENT to be a small value similar to the length that corresponds to MIN_STEPS_PER_SEGMENT. And MIN_ARC_SEGMENTS should not be used because ARC_SEGMENTS_PER_SEC will already provide the best smoothness that's reasonable at a given feed rate.

[LinFor]

Usage of private-styled macro _RECIP from another module.

I have compilation fail here:
Compiling .pio\build\LPC1768\src\src\gcode\motion\G2_G3.cpp.o Marlin\src\gcode\motion\G2_G3.cpp: In function 'void plan_arc(const xyze_pos_t&, const ab_float_t&, uint8_t)': Marlin\src\gcode\motion\G2_G3.cpp:109:41: error: '_RECIP' was not declared in this scope float seg_length = scaled_fr_mm_s * _RECIP(ARC_SEGMENTS_PER_SEC); ^~~~~~ *** [.pio\build\LPC1768\src\src\gcode\motion\G2_G3.cpp.o] Error 1

[LinFor]

Is it possible to use RECIPROCAL() macro instead?