MarlinFirmware / Marlin

Marlin is an optimized firmware for RepRap 3D printers based on the Arduino platform. Many commercial 3D printers come with Marlin installed. Check with your vendor if you need source code for your specific machine.
https://marlinfw.org
GNU General Public License v3.0
16.26k stars 19.23k forks source link

[BUG] Markforged_XY option has problem to make small circles round #24086

Closed Marek-zl closed 2 years ago

Marek-zl commented 2 years ago

Did you test the latest bugfix-2.0.x code?

Yes, and the problem still exists.

Bug Description

1) Markforged_xy there is problem with motor direction, I can not find the right setting. Motor is going all the time in wrong dirrection. With all settings like true and false direction or change pins on motor. No right possition. 2) the circles are not circles are on left and right side smaller, sides of the circle are more like cube. With size of the circle, it is more less dissapier.

Bug Timeline

new

Expected behavior

No response

Actual behavior

No response

Steps to Reproduce

No response

Version of Marlin Firmware

02010000

Printer model

No response

Electronics

BTT octopus pro v1

Add-ons

No response

Bed Leveling

No Bed Leveling

Your Slicer

Prusa Slicer

Host Software

Other (explain below)

Additional information & file uploads

40x40mm detail 40x40mm

rondlh commented 2 years ago

That's odd, which slicer do you use? Could you show us the slicer preview?

Marek-zl commented 2 years ago

Prusa slicer, for this model. I tried also G2 instruction for circle and has the same result. slicer_prusa

rondlh commented 2 years ago

Thanks, the preview looks fine. Markforged_XY has some racking issues that are most obvious when changing X direction. You could try moving X left and right 10mm and see if it affects the Y position. A lose Y belt could be the cause.

Marek-zl commented 2 years ago

I tried almost everithing. First of all I was testing mechanical parts, belts too ofcourse. There is not issue in mechanic.

The X is moving smooth and holding possition. If I move 10 mm x or Y it is 10 mm so the callibration of the axis is good.

The problem with "no circle" is groving when the circle is smaller. There has to be I think some calculation problem in planner.

There is also problem with direction of the axis, in planner.

rondlh commented 2 years ago

If you can post your gcode, I will have a go at it...

Marek-zl commented 2 years ago

G2 I20 J20 or G2 I10 J10 for fast test it is just to draw a circle I use pen instead nozzle. attached is also gcode for this callibration test print. kalibrace4040pr10_43m_0,20mm_205C_PLA.zip 20220427_120225

rondlh commented 2 years ago

Note that in the gcode, there is no G2 command...

rondlh commented 2 years ago

I printed the gcode on an i3 style printer, the result is a nice small circle, so the gcode is fine. Circle

Then drew some circles on a Markforged system: G2 I5 J5 (D=14mm), G2 I10 J10 (D=28mm), here the results with a 25mm coin. This shows the problem quite clearly, the left and right side are flat. Circle+Coin

About what you said "Markforged_xy there is problem with motor direction". I don't have that problem. First make sure your Y-direction is correct, then adjust the X-direction if needed.

Marek-zl commented 2 years ago

yes, I printed the G-code on ender with no problem. But on Markforged_xy style printer is this issue with flat circle.

The direction of the axis: Y is ok, no problem. but X is oposit, if I change direction in FW or wiring on motor than motor is in good direction but not is not working in Markforged_XY possitionning.

I tried to change some +- in planner, I am not programmer I did it on blind. but I after this is was working.

Now I use bugfix 2 of FW version without any changes in planner. First would like to fix the circle problem, after this the problem with dirrection.

plannner2 plannner

descipher commented 2 years ago

I don't think it would be a planner movement issue since the X/Y lengths are equal in the print. G2 is and arc command so the arc reference point must be off center.

      if (r) {
        const xy_pos_t p1 = current_position, p2 = destination;
        if (p1 != p2) {
          const xy_pos_t d2 = (p2 - p1) * 0.5f;          // XY vector to midpoint of move from current
          const float e = clockwise ^ (r < 0) ? -1 : 1,  // clockwise -1/1, counterclockwise 1/-1
                      len = d2.magnitude(),              // Distance to mid-point of move from current
                      h2 = (r - len) * (r + len),        // factored to reduce rounding error
                      h = (h2 >= 0) ? SQRT(h2) : 0.0f;   // Distance to the arc pivot-point from midpoint
          const xy_pos_t s = { -d2.y, d2.x };            // Perpendicular bisector. (Divide by len for unit vector.)
          arc_offset = d2 + s / len * e * h;             // The calculated offset (mid-point if |r| <= len)
descipher commented 2 years ago

It appears to be the plan_arc call, it looks like it cannot do arcs on CORE_XY. What are you using to slice?

rondlh commented 2 years ago

I've been digging in the code a bit, generating some debug output. I can see that the G2 function creates a nice circle, and the circle is converted to steps and directions correctly and stored in the motion buffer. If there is any issue, then this happens afterwards.

descipher commented 2 years ago

Can't be arcs, there was no G2 in the gcode test sample provided.

Marek-zl commented 2 years ago

the problem is there also in normal G1 "arc" segments. in Markforged_xy where the position depends on two steppers. For this reason I thought that problem is in planner. seems that movement of the x asix is not complet, cos the flat segments of the circle.

descipher commented 2 years ago

@rondlh Can you do a 10mm box and then a circle in the center with a 5mm radius so we can see what the errored movements look like from a reference point? I don't have a a corexy to test on.

rondlh commented 2 years ago

@descipher Something like this?

G0 X0 Y0
G1 X0 Y10
G1 X10 Y10
G1 X10 Y0
G1 X0 Y0
G2 I5 J5

Please note that Markforged is not the same as Corexy. This plotted on a Markforged_xy: Circle3

descipher commented 2 years ago

Perfect! Thanks. Now we can measure and analyze the coordinate results. Immediately we can see X and Y are not equal. Y is greater than X

descipher commented 2 years ago

Question: Is BACKLASH_COMPENSATION enabled by chance?

Marek-zl commented 2 years ago

not in my case, will test it.

descipher commented 2 years ago

Based on the box being 10mm x 10mm Y should actually be Y - 1mm and X should be X + 1mm. Very odd.

descipher commented 2 years ago

This looks wrong. planner.cpp:2097

    steps_dist_mm.a      = (da - db) * mm_per_step[A_AXIS];

If the steps da and db are equal then it will calculate 0 mm per step.

rondlh commented 2 years ago

Question: Is BACKLASH_COMPENSATION enabled by chance?

Nope, it's not compatible with Markforged

descipher commented 2 years ago

Ok, that will need to be sanity checked.

rondlh commented 2 years ago

Ok, that will need to be sanity checked.

It's there already, I played around with it a bit, and got an error.

descipher commented 2 years ago

@rondlh Ok, sorry missed it.

rondlh commented 2 years ago

This looks wrong. planner.cpp:2097 steps_dist_mm.a = (da - db) * mm_per_step[A_AXIS]; If the steps da and db are equal then it will calculate 0 mm per step.

da = X-steps, db=Y-steps. A move in Y will also move the position of X (but not the other way around, Markforged_XY) So if by chance the Y movement causes the required movement in X then there are no more steps needed for X. Markforged

rondlh commented 2 years ago

@rondlh Ok, sorry missed it.

I checked it again, and it seems that backlash compensation is actually possible. I previously got a error saying that it was only possible on the Z-axis. But if I see the condition for this error than it looks like that you can use it on X or Y, but not on both.

#if ENABLED(BACKLASH_COMPENSATION)
  #ifndef BACKLASH_DISTANCE_MM
    #error "BACKLASH_COMPENSATION requires BACKLASH_DISTANCE_MM."
  #elif !defined(BACKLASH_CORRECTION)
    #error "BACKLASH_COMPENSATION requires BACKLASH_CORRECTION."
  #elif EITHER(MARKFORGED_XY, MARKFORGED_YX)
    constexpr float backlash_arr[] = BACKLASH_DISTANCE_MM;
    static_assert(!backlash_arr[CORE_AXIS_1] && !backlash_arr[CORE_AXIS_2],
                  "BACKLASH_COMPENSATION can only apply to " STRINGIFY(NORMAL_AXIS) " on a MarkForged system.");
  #elif IS_CORE
    constexpr float backlash_arr[] = BACKLASH_DISTANCE_MM;
    #ifndef CORE_BACKLASH
      static_assert(!backlash_arr[CORE_AXIS_1] && !backlash_arr[CORE_AXIS_2],
                  "BACKLASH_COMPENSATION can only apply to " STRINGIFY(NORMAL_AXIS) " with your CORE system.");
    #endif
  #endif
#endif
descipher commented 2 years ago

Yes only one makes sense.

descipher commented 2 years ago

'target - target position in steps units'

   if (da < 0) SBI(dm, X_HEAD);                  // Save the toolhead's true direction in X
   if (db < 0) SBI(dm, Y_HEAD);                  // ...and Y

da = X db =Y

_populate_block(block_t * const block, bool split_move,  const abce_long_t &target ...

If target is {100,100,0} and position is {0,0,0}

    da = target.a - position.a,
    db = target.b - position.b,

steps are now set da=100-0 db=100-0

block updated to

block->steps.set(NUM_AXIS_LIST(ABS(100 + 100), ABS(100), ABS(0) ...

da=100-0 db=100-0

  #elif ENABLED(MARKFORGED_XY)
    steps_dist_mm.a      = (100 - 100) * mm_per_step[A_AXIS];
    steps_dist_mm.b      = 100 * mm_per_step[B_AXIS]; 

Looks wrong. I am missing something?

descipher commented 2 years ago

Based on the formula:

image

I would think this is better:

steps_dist_mm.a = (da + (da - db)) mm_per_step[A_AXIS]; steps_dist_mm.a = 100 mm_per_step[A_AXIS];

rondlh commented 2 years ago

steps_dist_mm.a = (da + (da - db)) * mm_per_step[A_AXIS];

This is the same as: steps_dist_mm.a = (2 da - db) mm_per_step[A_AXIS]; This is not according to the give kinematics.

I feel the issue might be coming from a rounding error or perhaps the minimum amount of steps that is handled in a segment. Even a small circle results in a minimum of 72 segments (MIN_CIRCLE_SEGMENTS in configuration_adv.h). Below a picture of the positions calculated for: G2 I5 J5 circle. This looks fine, and also the resulting calculation in steps and directions are correct. The problem is introduced afterwards.

Circle circle2

rondlh commented 2 years ago

I've have a closer look at the steppers and found them to be running smoothly and constantly. So I don't believe the problem is on the software side, rather it's on the mechanical side. Racking is an issue for Markforged kinematics. If I look at the above picture I get the impression that the X-axis is sticking a bit. I have a closer look and the initial resistance against movement in the X-direction is a bit on the high side. This would explain the results, which shows that the carriage is not responding to small X-movements, until the force is enough and the friction is reduced (static friction > dynamic friction).

descipher commented 2 years ago

That's possible but less likely with two systems having exactly the same result. Also wouldn't you see faults in a more distributed way? If racking is the cause would you see it more by printing strait X lines repetitively.

rondlh commented 2 years ago

I think the main problem is sticking of the X-axis, and perhaps racking could also be a factor. I'm willing to do some tests if you have any suggestions...

descipher commented 2 years ago

If it's loosing steps due to racking then try printing X_AXIS only with 10 x 100mm lines and measure them. Then do Y individually. If the lines all measure exactly 100mm then it almost rules out a step loss due to racking alignment.

descipher commented 2 years ago

I did log the output step values on the simulator, based on the actual output results the axes movement formulas are correct.

descipher commented 2 years ago

The part that got me was the move formula distances are signed and based on direction.

descipher commented 2 years ago

One thought that came up for me is what if the X/Y axes moves were not in perfect sync. Y moves need X to move in perfect sync. One step out of sync and its not going to be pretty ... lol

descipher commented 2 years ago

If you suspect really small movement you could do 3 mm lines as an alternative.

rondlh commented 2 years ago

It's definitely not losing steps, that would show up very fast, and destroy any print. I think its a bit of flexibility in the belts, frame, and gantry that causes the carriage to stay behind about 0.5mm in the X-direction because of the carriage X-friction. In the pictures you can see that it catches up once it starts moving (dynamic friction < static friction), but for some reason it stays behind after it stopped moving for a bit. If I disconnect the carriage from the belt and push the carriage manually I can clearly feel this. I also was thinking of out of sync pulses, but that would create nice ellipses, there would be no straight sections. 1 step out of sync is not going to show up. I use 200 steps per mm, typical is 80 steps per mm for X and Y, so 1 step would only be 12.5um. I've printed huge parts on this printer, print area is 40x40x40cm. I not 100% clear about the test you propose... a bunch of 3mm lines along the x-axis?

descipher commented 2 years ago

Interleaved with y e.g. x y x y x y x y, if it's racking the flex effect may show up on the print.

Marek-zl commented 2 years ago

My last test Correction for X stepper direction

elif ENABLED(MARKFORGED_XY)

if (da - db < 0) SBI(dm, A_AXIS);              // Motor A direction  (- instead +)
if (db < 0) SBI(dm, B_AXIS);                   // Motor B direction

elif ENABLED(MARKFORGED_YX)

elif ENABLED(MARKFORGED_XY)

  ABS(da - db), ABS(db), ABS(dc) // - instead +
#elif ENABLED(MARKFORGED_YX)

elif ENABLED(MARKFORGED_XY)

steps_dist_mm.a      = (da + db) * mm_per_step[A_AXIS]; // + instead - 
steps_dist_mm.b      = db * mm_per_step[B_AXIS];

elif ENABLED(MARKFORGED_YX)

This help me to correct X stepper

Backlash

define BACKLASH_COMPENSATION

if ENABLED(BACKLASH_COMPENSATION)

// Define values for backlash distance and correction. // If BACKLASH_GCODE is enabled these values are the defaults.

define BACKLASH_DISTANCE_MM { 0, 0, 0 } // (linear=mm, rotational=°) One value for each linear axis

define BACKLASH_CORRECTION 1.0 // 0.0 = no correction; 1.0 = full correction

If I put some different value than 0 in distance is going to error

Last circle in 5 mm diam is on picture.

220430091950442

descipher commented 2 years ago

https://github.com/MarlinFirmware/Marlin/blob/3bcbd3259f8d1ff3fc262b2ede83efd873863d5f/Marlin/src/module/planner.cpp#L2063-L2065

This calculation is correct, it is not currently used anywhere else in the code. I did verify its value output using logging.

descipher commented 2 years ago

We would need line numbers to know where the discussion references are from.

descipher commented 2 years ago

Any chance you could run a 10mm or more diameter, 5mm may not show the fault as well.

descipher commented 2 years ago

@Marek-zl I think you are the right track, I think the issue is the direction calculation failing at the Y 0 crossing points, just can't tell if its correct yet. Kind of looks like the circle you printed has the fault on the lower side now.

Marek-zl commented 2 years ago

10mm circle. Result is horrible. the problem is significant on left and right side. If circle is smaller is going to be more square

220430175223254

descipher commented 2 years ago

Inverse side from what @rondlh experienced. Looks like only negative X and negative Y directions are impacted in the last print. There are odd and even sizes within infill cross hatching which could be direction related.

descipher commented 2 years ago

https://github.com/MarlinFirmware/Marlin/blob/3bcbd3259f8d1ff3fc262b2ede83efd873863d5f/Marlin/src/module/planner.cpp#L1962-L1967

 #elif ENABLED(MARKFORGED_XY) 
   if (da < 0) SBI(dm, A_AXIS);                   // Motor A direction 
   if (db < 0) SBI(dm, B_AXIS);                   // Motor B direction 
 #elif ENABLED(MARKFORGED_YX) 
   if (da < 0) SBI(dm, B_AXIS);                   // Motor B direction 
   if (db < 0) SBI(dm, A_AXIS);                   // Motor A direction 

@Marek-zl Could you please test this.

descipher commented 2 years ago

I may have the da db switched around though, hard to tell what MARKFORGE_XY means in respect to the belts A or B. We will fix that up once this is sorted out.