Update motion_control.c

[kimstik]

bug: cubic spline step is up to 2*BEZIER_MAX_STEP

[terjeio]

AFAIKT your fix increases it to 4*BEZIER_MAX_STEP. Have you checked running code?

[kimstik]

AFAIKT your fix increases it to 4*BEZIER_MAX_STEP. Have you checked running code?

I got this result by testing:

#define BEZIER_MIN_STEP 0.002f
#define BEZIER_MAX_STEP 0.1f
#define BEZIER_SIGMA 0.1f

mc_cubic_b_spline() with args:
  position = (0,0,0)
  first    = (0,1,0)
  second   = (1,0,0)
  target   = (1,1,1)

gives:
t 0.000000 step=0.200  line to (0.104000,0.392000,0.000000)
t 0.200000 step=0.200  line to (0.352000,0.496000,0.000000)
t 0.400000 step=0.200  line to (0.648000,0.504000,0.000000)
t 0.600000 step=0.200  line to (0.896000,0.608000,0.000000)
t 0.800000 step=0.200  line to (1.000000,1.000000,0.000000)
multiplications:281 additions:224

with BEZIER_MAX_STEP set to 0.1, 10 interpolation points are expected over t=0.0...1.0, but only 5 were generated.

relevant link is : https://github.com/MarlinFirmware/Marlin/blob/bugfix-2.1.x/Marlin/src/module/planner_bezier.cpp#L159

[kimstik]

where you see 4*BEZIER_MAX_STEP ? Do you got it by running code?

[kimstik]

BTW I like "interp(position.z, target.z, t)" in the Marlin. Not ideal but it is much better then no Z interp.

[terjeio]

with BEZIER_MAX_STEP set to 0.1, 10 interpolation points are expected over t=0.0...1.0, but only 5 were generated.

According to the description how many steps to expect depends on the curve straightness.

When I run the code with double precision math I get four segments, not five. And with your change I get five vs. ten. And to add to float impreciseness dist1() returns an approximation. So IMO the algorithm is kind of "fuzzy".

[kimstik]

According to the description

description: "MAX_STEP is taken at the first iteration."
The first iteration with my correction uses MAX_STEP; otherwise, it uses 2*MAX_STEP. This behavior has been fixed in Marlin (for float).
The cause was an incorrect entry into the "enlarge step" logic because, on the first iteration,
step = BEZIER_MAX_STEP
and the condition (new_t - t <= BEZIER_MAX_STEP) may be triggered.
The condition (new_t - t < BEZIER_MAX_STEP) would be more appropriate here (as per Marlin).

4 steps sounds buggy as per "2*MAX_STEP" rule from description. In this case 5 steps is minimum, not 4.

I haven’t checked the double version yet..
I’ll need to look at it carefully

[terjeio]

The condition (new_t - t < BEZIER_MAX_STEP) would be more appropriate here (as per Marlin).

My while loop produces the same steps as the Marlin for loop, so the linked Marlin code has the same bug? Or is Marlin using a different float library that has produces slightly different results that affects the loop exit condition?