Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Marlin 1.1.x ARC Support based on the improvements of Marlin 2.0.6 #61

Open
gustavomassa opened this issue Mar 19, 2021 · 6 comments

Comments

@gustavomassa
Copy link

gustavomassa commented Mar 19, 2021

Hello, thanks for this amazing lib.

I'm planning to get the Marlin 2.0.6 feature into Marlin 1.1.9 since I use my board is 8bits and I don't want to update to 2.x.
The idea is to compare the differences between the 2x algorithm with the 1.1x algorithm.
image
What you mean by greatly enhanced? What is missing on 1.1x?
Marlin 1.1.x ARC implementation: https://github.com/MarlinFirmware/Marlin/blob/1314b31d97bba8cd74c6625c47176d4692f57790/Marlin/Marlin_main.cpp#L14451
Marlin 2.x ARC implementation: https://github.com/MarlinFirmware/Marlin/blob/b9065195f1babf00ee453c39410206af90a525ae/Marlin/src/gcode/motion/G2_G3.cpp#L25

Some differences that I found:

  #ifdef MIN_ARC_SEGMENTS
    uint16_t min_segments = CEIL((MIN_ARC_SEGMENTS) * (angular_travel / RADIANS(360)));
    NOLESS(min_segments, 1U);
  #else
    constexpr uint16_t min_segments = 1;
  #endif  
  
      #ifdef MIN_ARC_SEGMENTS
      min_segments = MIN_ARC_SEGMENTS;
    #endif
    
      const feedRate_t scaled_fr_mm_s = MMS_SCALED(feedrate_mm_s);

  // Start with a nominal segment length
  float seg_length = (
    #ifdef ARC_SEGMENTS_PER_R
      constrain(MM_PER_ARC_SEGMENT * radius, MM_PER_ARC_SEGMENT, ARC_SEGMENTS_PER_R)
    #elif ARC_SEGMENTS_PER_SEC
      _MAX(scaled_fr_mm_s * RECIPROCAL(ARC_SEGMENTS_PER_SEC), MM_PER_ARC_SEGMENT)
    #else
      MM_PER_ARC_SEGMENT
    #endif
  );
  // Divide total travel by nominal segment length
  uint16_t segments = FLOOR(mm_of_travel / seg_length);
  NOLESS(segments, min_segments);         // At least some segments
  seg_length = mm_of_travel / segments;    

#18 is also related to it, right?

What do you think about the idea?: I'm a newb about ARC Welder, how could I test the modifications? Do you have an STL file that can be used as a test? I'm using Slic3r

Another part that I'm not sure if it is the same logic or improved:
Marlin 1.1.x

      float arc_offset[2] = { 0, 0 };
      if (parser.seenval('R')) {
        const float r = parser.value_linear_units(),
                    p1 = current_position[X_AXIS], q1 = current_position[Y_AXIS],
                    p2 = destination[X_AXIS], q2 = destination[Y_AXIS];
        if (r && (p2 != p1 || q2 != q1)) {
          const float e = clockwise ^ (r < 0) ? -1 : 1,             // clockwise -1/1, counterclockwise 1/-1
                      dx = p2 - p1, dy = q2 - q1,                   // X and Y differences
                      d = HYPOT(dx, dy),                            // Linear distance between the points
                      h2 = (r - 0.5f * d) * (r + 0.5f * d),         // factor to reduce rounding error
                      h = (h2 >= 0) ? SQRT(h2) : 0.0f,              // Distance to the arc pivot-point
                      mx = (p1 + p2) * 0.5f, my = (q1 + q2) * 0.5f, // Point between the two points
                      sx = -dy / d, sy = dx / d,                    // Slope of the perpendicular bisector
                      cx = mx + e * h * sx, cy = my + e * h * sy;   // Pivot-point of the arc
          arc_offset[0] = cx - p1;
          arc_offset[1] = cy - q1;
        }
      }

Marlin 2.x

    ab_float_t arc_offset = { 0, 0 };
    if (parser.seenval('R')) {
      const float r = parser.value_linear_units();
      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)
        }
      }
    }
@FormerLurker
Copy link
Owner

Yes, firmware compensation is basically accounting for the missing min_arc_segments define, which allows accurate geometry for small arcs. When using only mm_per_arc_segment, small arcs will end up looking like polygons. Compensation just prevents arcs from being generated if they violate arcwelders min_arc_segments + mm_per_arc_segment setting.

Adding min_arc_segments is the thing you should focus on, though there are other changes in marlin 2.0.6+ that are beneficial, like an enhanced junction deviation algorithm. You may be able to compensate for that by adding a min_segment_length define to prevent insanely shot segments from being generated.

Why don't you look at my marlin fork of prusa's fork of marlin 1. I have basically already done most of this there. It shouldn't be too hard to adapt. Check my repo list for this.

@gustavomassa
Copy link
Author

gustavomassa commented Mar 20, 2021

@FormerLurker thanks for the info, I will look up your repo as an example

I did the first commit, adding the min_arc_segments, 24 is a good value?
gustavomassa/Marlin@4f664e8

What about the ARC_SEGMENTS_PER_R and ARC_SEGMENTS_PER_SEC?
I still have to make changes to apply the scaled_fr_mm_s value, this will be complicated, the "scaled_fr_mm_s" depends on the "ARC_SEGMENTS_PER_SEC"

Also, do you recommend any small stl file to print as a test? like the calibration cube, but focused on the ARCS

@FormerLurker
Copy link
Owner

Those settings are good defaults. Though the feedrate based settings are cool, they require a bit of testing to properly configure. Maybe consider implementing those later.

As for a test, the first layer of benchy is pretty good, as it has curvy text. Slice it with archimedean bottom infill for a proper test.

@gustavomassa
Copy link
Author

gustavomassa commented Mar 20, 2021

@FormerLurker How much infill is needed for the test? "Archimedean Chords" for the bottom only? I cut the benchy file, so it will print the first 2 layers only.

image
image
image
image
image

Not sure if I should use the G90/G91 Influence Extruder with my firmware modifications
This is the result with these 20% infill archimedean settings:
result

==============================================================================

This is another test printing more layers and using a different interior infill:
image
image
48c75933-01cd-4ea7-abd1-95bfd6d22f38
a8b4d6e5-e0b0-49ce-9cbd-5b08233ea681

@FormerLurker
Copy link
Owner

Wow, that's some line width :)

How much infill is needed for the test? "Archimedean Chords" for the bottom only? I cut the benchy file, so it will print the first 2 layers only.

The infill amount won't batter, because top and bottom infill is solid. Since you are only printing the first two layers, you won't have sparse infill at all.

Not sure if I should use the G90/G91 Influence Extruder with my firmware modifications

You should not.

Looks good! drop a link to your mods. I'd be curious to see what you have.

@FormerLurker
Copy link
Owner

Regarding the link, I pretty easily found your repo, so not necessary. Nice job!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants