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.24k stars 19.22k forks source link

Hysteresis #3664

Closed MoonshineSG closed 8 years ago

MoonshineSG commented 8 years ago

I used to have a XY hysteresis correction to compensate for backlash which called for

extern Hysteresis hysteresis;
extern long position[4]; // defined in planner.cpp

in Hysteresis.h

but after the "rc_singletons" update in RCBugFix position has been made private.

Any idea how to solve this ?

thinkyhead commented 8 years ago

You can use planner.adjusted_position() or you could use the current_position. Is there some reason you need to use the planner's internal position in particular?

MoonshineSG commented 8 years ago

It's not really my code. Found it on another repo (can't remember which one) and "adopted" it.. I'll try to change to either suggestion and do some tests.

MoonshineSG commented 8 years ago

@thinkyhead do the position in planner and current_position in marlin_main represent the same thing ? one is const long* , the other one float*

thinkyhead commented 8 years ago

They represent the same thing in different forms. Planner maintains its current position in steps, while the current_position is represented in mm. Use axis_steps_per_mm[axis] to convert between them.

The current_position may be ahead of (or different from) the Planner's position values, which are updated after queuing a move. The Stepper positions will be the most up-to-date.

Note that the Planner step values may be "very different" from the Stepper step values (on a DELTA or COREXY, for example). The stepper.get_axis_position_mm(axis) method properly converts a CORE position to axis units. (This method is not used for DELTA. We don't have the forward kinematics formulae in Marlin.)

MoonshineSG commented 8 years ago

@thinkyhead If I understand correctly how the Hysteresis code works, the planner.position is the one that needs to be modified. That is because the correction is done in the buffer_line.

#if ENABLED(AUTO_BED_LEVELING_FEATURE) || ENABLED(MESH_BED_LEVELING)
  void Planner::buffer_line(float x, float y, float z, const float& e, float feed_rate, const uint8_t extruder)
#else
  void Planner::buffer_line(const float& x, const float& y, const float& z, const float& e, float feed_rate, const uint8_t extruder)
#endif  // AUTO_BED_LEVELING_FEATURE
{
  hysteresis.InsertCorrection(x,y,z,e);
  // Calculate the buffer head after we push this byte
  int next_buffer_head = next_block_index(block_buffer_head);

  // If the buffer is full: good! That means we are well ahead of the robot.
  // Rest here until there is room in the buffer.
[...]

So for my own use, I made planner.position public ... Not ideal, but I can't think of anything else right now...

thinkyhead commented 8 years ago

@MoonshineSG That's fine, since you're already using a modified Marlin. Another option is to use friend class Hysteresis in the definition of class Planner (or make class Hysteresis an extension of class Planner).

MoonshineSG commented 8 years ago

@thinkyhead Thanks for the suggestions. I decided to go with friend as this looks more elegant than changing the access level for position

Phaelz commented 7 years ago

Sorry to post on an old topic.

But why doesn't Marlin have backlash compensation?

Lawsy from soolidoodle forums uploaded this to their fw.

I think Neil Martin was the guy who wrote those lines in 2013

marcio-ao commented 6 years ago

We are currently developing printers that are using gear motors on Z and we are finding hysteresis/backlash issues too. I would be interested to know the status of this feature in Marlin. It sounds like this is not planned to be integrated into the codebase?

Phaelz commented 6 years ago

I tried to merge the code to the most recent version of marlin and I got 1209347985723 errors. My programming skills were not enough.

This could improve the printing quality tremendously.

marcio-ao commented 6 years ago

Yeah. The code has changed a lot since this was first suggested. I'm a bit curious as to why this change was never accepted into Marlin.

fiveangle commented 6 years ago

I'm a bit curious as to why this change was never accepted into Marlin.

You'll probably have to ask Erik: https://github.com/ErikZalm/Marlin/issues/247

This could improve the printing quality tremendously.

and monkeys could also fly out of my butt ! ;)

I'd imagine this being a much larger problme for CNC type systems where there are lots of gearing between the drive and the toolpiece, but Marcio's point about Z is not without merit. It would certainly be more of a requirement for printing in a weightless enviroment [grin]

marcio-ao commented 6 years ago

and monkeys could also fly out of my butt ! ;)

The running joke around here is that all mechanical issues can be fixed by a firmware tweak. Print volume not big enough? Just upgrade to the jumbo firmware...

Phaelz commented 6 years ago

@fiveangle maybe my standards of quality are so high we are not even talking about the same thing.

There certainly is hysteresis on 3d printers using gt2 belt systems.

AnHardt commented 6 years ago

Traditional backlash compensation can be done by a preprocessor. But there are more interesting effects, like dynamic overshot. The head is overshooting the target when braked hard. The magnitude of the failure is different on the both ends of the axis and depends on the direction. Spring-constant of the breaking part of the belt differs with the length to the stepper. When that is handled resonances of the frame and the table below can be fighted. Interesting here is the change of the x-resonance frequence with z - longer pendulum.

For almost all of us (the hobby programmers) the development of a nice algorithm for that would be fun. But evaluating the positive effect and finding the correct set of parameters for our always modificated machines is too time consuming or measurement equipment too expensive. This could make sense for Stratasys, selling several thousand machines from one type, but not for us. On the other hand, when only selling a few thousand items it's likely cheaper to invest in better mechanics.

For the average user alone setting up the heater sanity checks is too complex. Slightly more complex systems like bed levelling needed several iterations to make them usable and we still have to see if UBL is teachable.

Phaelz commented 6 years ago

I think of something like the extrusion linear advance algorithm. Whenever the carriage reverse direction, this algorithm would compensate advancing some steps, according to the intensity of this reversing.

I should investigate more. I notice the tiny visual effects of hysteresis on big prints, with lots of artifacts (a big linear path in one direction, then a sudden change in direction with a travel path, then an island of extrusion somewhere else and then back to that linear path). and travel paths on the same layer.

I don't know if I'm making myself clear enough... but I feel that, for example, if a print is done in vase mode, there are never signs of hysteresis. It's just like the change in direction is too subtle to cause any visual deffect. But if there is a a discontinuity in the extrusion followed by a hard and fast change in direction, hysteresis effects are always there.

It is not step skipping, it occurs only in some cases, on both axes but one at a time... also it's not resonance. I've seen it on several printers, ranging from ultimaker to repraps...

It's kinda easy to spot. Check for the external perimeter on some prints that there is a certain misalignment somewhere but in the same layer the other side is perfectly aligned. You can see that that happened after a travel path or discontinuity.

Phaelz commented 6 years ago

Check this: https://github.com/MarlinFirmware/Marlin/issues/7579

marcio-ao commented 6 years ago

Traditional backlash compensation can be done by a preprocessor.

It can, but then things like positioning the head using the LCD will be broken. It's pretty sloppy to do it outside of the FW, in my opinion.

The change necessary to "planner.cpp" to make this work seems small. The general algorithm is simple:

The code which was originally presented managed to do it with a few lines of change to "planner.cpp" and cleverly put all the extra logic in a separate .h and .cpp , so it was not obtrusive at all. The challenge now is that the planner has been modified quite a bit since then and it would take some effort to update the code.

I found our prototype printers have two Z motors (with differing backlash) and we only have one Z stepper driver, so sadly software compensation is no longer looking like a fix for our Z backlash. Because of this, I likely would not be able to spend time working on this; it's probably a better job for someone with a good understanding of how the planner code works anyhow.

fiveangle commented 6 years ago

Check this: #7579

Yes, you referred to it earlier. "Code is ready" is a bit clickbaity I'd say ;)

marcio-ao commented 6 years ago

I'd say code is never ready; it merely attains a tolerable level of unreadiness :)

Anyhow, I've been doing some measurements on our printers and we are seeing about 0.25 mm of backlash on the X axis. I doubt correcting for this will lead to visibly better prints, but it could possibly throw off the tolerance of tightly interlocking prints. I'll take a stab of updating that compensation code for Marlin 2.0 and see if it makes any difference.

marcio-ao commented 6 years ago

If anyone wants to experiment with hysteresis compensation, here is a quick-and-dirty modification to "planner.cpp" that has been tested under Marlin 1.1.5 branch:

void Planner::_buffer_line(const float &a, const float &b, const float &c, const float &e, float fr_mm_s, const uint8_t extruder) {

  ...

  if (de < 0) SBI(dm, E_AXIS);

  #if defined(LULZBOT_AXIS_BACKLASH)
  {
    static uint8_t last_direction_bits;
    float backlash_compensation[NUM_AXIS] = LULZBOT_AXIS_BACKLASH;
    long saved_position[NUM_AXIS] = { 0 };
    uint8_t changed_directions = last_direction_bits ^ dm;
    if(changed_directions & (_BV(X_AXIS) | _BV(Y_AXIS) | _BV(Z_AXIS))) {
      last_direction_bits = dm;
      COPY(position, saved_position);
      _buffer_line(
        float(position[X_AXIS])/axis_steps_per_mm[X_AXIS] + (TEST(changed_directions, X_AXIS) ? backlash_compensation[X_AXIS] : 0) * ((a < 0) ? -1 : 1),
        float(position[Y_AXIS])/axis_steps_per_mm[Y_AXIS] + (TEST(changed_directions, Y_AXIS) ? backlash_compensation[Y_AXIS] : 0) * ((b < 0) ? -1 : 1),
        float(position[Z_AXIS])/axis_steps_per_mm[Z_AXIS] + (TEST(changed_directions, Z_AXIS) ? backlash_compensation[Z_AXIS] : 0) * ((c < 0) ? -1 : 1),
        e, fr_mm_s, extruder
      );
      COPY(saved_position, position);
    }
  }
  #endif

  const float esteps_float = de * volumetric_multiplier[extruder] * flow_percentage[extruder] * 0.01;
  ...
}

The only change is inside the #if/#endif block. I've included the function declaration and lines immediately before and after for reference of where the code needs to be inserted.

This is a quick-and-dirty implementation based very loosely on the work referenced above. It does not add additional GCODEs; instead the backlash compensation done at compile-time by adding the following line to "Configuration.h":

#define LULZBOT_AXIS_BACKLASH {0.27, 0.25, 0, 0}

This is a rough implementation with much room for improvement. As it is written, it will only work on the X, Y and Z motors. It also will only work for regular cartesian printers (not CORE_ styles). There are several ways this code could be improved, but it would require splitting Planner::_buffer_line into two functions, one which would take raw steps as arguments and inserted a new planner block without altering the position or doing any conversions. Having the first function available would allow the backlash compensation code to add a new block without having to back-convert steps to mm, as well as eliminating the need to save/restore the position. It would also make it possible to do backlash compensation on CORE printers, as the compensation would be applied to each stepper, not the axis, which involves a coordinated move of different steppers on CORE machines.

If you guys would be willing to review and accept a more complete implementation, I could work on it.

-- Marcio

marcio-ao commented 6 years ago

Here is an update on the backlash compensation code:

#define AXIS_BACKLASH {0.00, 0.00, 0.35, 0}

#if defined(AXIS_BACKLASH)
    #define SIGN(v) ((v < 0) ? -1.0 : 1.0)
    #define AXIS_BACKLASH_CORRECTION \
        { \
            static const float backlash[NUM_AXIS] = AXIS_BACKLASH; \
            static uint8_t last_direction_bits; \
            static bool is_correction = false; \
            if(!is_correction) { \
                uint8_t changed_dir = last_direction_bits ^ dm; \
                /* Ignore direction change if no steps are taken in that direction */ \
                if(da == 0) CBI(changed_dir, X_AXIS); \
                if(db == 0) CBI(changed_dir, Y_AXIS); \
                if(dc == 0) CBI(changed_dir, Z_AXIS); \
                if(de == 0) CBI(changed_dir, E_AXIS); \
                last_direction_bits ^= changed_dir; \
                /* When there is motion in an opposing direction, apply the backlash correction */ \
                if(changed_dir) { \
                    long saved_position[NUM_AXIS] = { 0 }; \
                    COPY(saved_position, position); \
                    const long x_backlash = TEST(changed_dir, X_AXIS) ? backlash[X_AXIS] * axis_steps_per_mm[X_AXIS] * SIGN(da) : 0; \
                    const long y_backlash = TEST(changed_dir, Y_AXIS) ? backlash[Y_AXIS] * axis_steps_per_mm[Y_AXIS] * SIGN(db) : 0; \
                    const long z_backlash = TEST(changed_dir, Z_AXIS) ? backlash[Z_AXIS] * axis_steps_per_mm[Z_AXIS] * SIGN(dc) : 0; \
                    const long e_backlash = TEST(changed_dir, E_AXIS) ? backlash[E_AXIS] * axis_steps_per_mm[E_AXIS] * SIGN(de) : 0; \
                    is_correction = true; /* Avoid infinite recursion */ \
                    _buffer_line( \
                        (position[X_AXIS] + x_backlash)/axis_steps_per_mm[X_AXIS], \
                        (position[Y_AXIS] + y_backlash)/axis_steps_per_mm[Y_AXIS], \
                        (position[Z_AXIS] + z_backlash)/axis_steps_per_mm[Z_AXIS], \
                        (position[E_AXIS] + e_backlash)/axis_steps_per_mm[E_AXIS_N], \
                        fr_mm_s, extruder \
                    ); \
                    is_correction = false; \
                    COPY(position, saved_position); \
                } \
            } \
        }
#else
    #define AXIS_BACKLASH_CORRECTION
#endif

Then in "planner.cpp", insert as such:

void Planner::_buffer_line(const float &a, const float &b, const float &c, const float &e, float fr_mm_s, const uint8_t extruder) {
  ...
  if (de < 0) SBI(dm, E_AXIS);

  AXIS_BACKLASH_CORRECTION

  const float esteps_float = de * volumetric_multiplier[extruder] * flow_percentage[extruder] * 0.01;
  ...
}

This code has successfully eliminated backlash on our Z gear motors, but since tramming on X and Y causes Z to switch directions very often, it causes the Z motor to bounce back and forth. It's rather noisy and unnerving. Jury is still out on whether this is a beneficial feature.

fiveangle commented 6 years ago

You mention 0.25mm backlash on X/Y... I'd expect if you print a 90-deg zig-zag pattern single-thickness wall:

/\/\/\/\/\/\/\/\/\/\/\/\/\

with and without backlash compensation (identical gcode otherwise), you should be able to measure with an accurate set of calipers accross the wall thickness at many points, averaging them: if the average wall thickness is thinner, then the compensation is correctly adjusting for the X/Y backlash to lay the beads more accurately on top of each other. If the average wall thickness is larger than without, it's making it worse. Does the theory seem sound ?

marcio-ao commented 6 years ago

@fiveangle: My initial test with XY compensation showed significant shifted layers, leading me to believe that the extra acceleration is causing skipped steps. What I believe happens is that Marlin accelerates the motor while taking up the backlash, but then when the slack is taken up, the toolhead is jerked into motion and steps are skipped.

I've had better results with backlash compensation on Z, where the motors we are testing out have 1:27 gearing and significant backlash. I have found that without the correction, the bed leveling correction falls inside the backlash region and so the first layer is printed as if auto-leveling was turned off. Enabling backlash compensation fixes this and makes a significant difference on the first layer quality. However, the downside is that Z backlash compensation seems to degrade the print quality over the rest of the print. Taking up backlash introduces a delay causes slight imperfections on continuous moves. This is visible on seams where a direction change takes place, such as the bow of a Benchy.

Next I am planning to consider a hybrid approach where backlash is only taken up during the initial layers. I may implement this either on the backlash code, or combine it with the ENABLE_LEVELING_FADE_HEIGHT option. BTW, where is the default fading height set? I do not see a parameter in the Configuration files.

Phaelz commented 6 years ago

It seems to me that the approach should be universal. The move with the corrected amount of steps should be as natural as possible. It should correct the steps before any bed level algorithm, before speed calculation.

AnHardt commented 6 years ago

It should correct the steps before any bed level algorithm

No. Before applying that you don't know the direction of a z-move.

fiveangle commented 6 years ago

Z fade is initted to zero in Conditionals_store I think. Not sure why this isn’t in Configuration.h. It really should be.

github-actions[bot] commented 2 years ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.