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.28k stars 19.24k forks source link

[1.1.x bugfix] Problem with the extrusion on mixing extruder #11310

Closed yitongzh closed 5 years ago

yitongzh commented 6 years ago

We have a problem with the pulse input to the motor for E axis(the extrusion) on our mixng extruder. The E motor will run with the XYZ motor even if we didn't give the G-code for E axis after we set the G-code for the E motor once. BTW, this issues appers only in Marlin-bugfix-1.1.x version. Everything is ok on 1.1.8 version.

Procedures to reproduce. 1.Set a coordinate to only XYZ motor. At this stage, the E motor will not move with the other three motors. 2.Set the coordinates with alteration on both XYZ axes and E axis. The E motor will run with an abnormal fast speed. 3.The problem comes at the followings. Whatever the G-code we set, the E motor will run with the XYZ motor even we did not want it to move with an abnormal speed. Only reconnect the printer can reset this phenomena.

And here are our configurtion files. configuration.zip

thinkyhead commented 6 years ago

An improvement with the latest bugfix code? See also #11484.

AnHardt commented 6 years ago

@roguecode uses a DETA. Subdivision of moves could be the problem. When printing slow, the segments become the size of MIN_STEPS_PER_SEGMENT. If there is only one e-step in, mixing becomes impossible.

roguecode commented 6 years ago

The problem is that "slow" isn't actually slow. I'd be happy with printing a little faster. Unfortunately the problem is bad enough that to make it work I need to print at 0.3mm layer height and 1.5x extrusion.

Is it worth simply dropping MIN_STEPS_PER_SEGMENT and trying?

AnHardt commented 6 years ago

@Llarezzamo s machine is also a DELTA (please don't rename .rar files to zip)

@roguecode No! That would give infinitely small segments. Try to Increase to 10, 20, 40 until it works - just to prove the theory. The fix would look different. If i'm right, currently the mix is reset and calculated per segment. Somehow we have to save the error over the segments borders. (One dimensional Floyd–Steinberg dithering?)

AnHardt commented 6 years ago

Alternatively you could decrease DELTA_SEGMENTS_PER_SECOND to enlarge the segments. Or change to a higher microstepping for the extruder to get more e-steps into a segment. Find the right compromise.

e-Mole commented 6 years ago

@AnHardt : Setting DELTA_SEGMENTS_PER_SECOND to 80 (from 120) don't resolve the extruder "speed" problem on my delta printer. The extruder motors are still too fast :-(

AnHardt commented 6 years ago

Idea for a steady colour mix:

reset_color_bresenham( ) {
  //called at the end of setup_color_bresenham(), and from step_colour_bresenham().
  //reset the line ready to draw the first point 
}

setup_color_bresenham( colourmix[EXTRUDERS] ) {
  //Called when the color is changed. Other virtual tool, or set colour, or changed E parameters.
}

uint8_t step_colour_bresenham() {
  Called from get_extruder().
  Make a step. return a bitmask for the steppers who stepped.
  If at the end of the line reset_colour_bresenham()
}

uint8_t get_extruder(void) {
  //Called whenever an E-step is needed - forwards and backwards.
  //Returns the index of a stepper.
  static uint8_t stack = 0;
  static uint8:t b = 0;
  do {
    while (stack) {
        if (stack & 1) {
         stack= stack>>1;
         return b++;  // Steps could be pulsed out here. But then during the pulse nothing meaningful can be done
       } else {
         stack = stack>>1;
         b++;
       }
    }
    b=0;
    stack = step_colour_bresenham();
    // may produce steps for more than one extruder at once, but only one per extruder
  } while (true);
}
roguecode commented 6 years ago

Try to Increase to 10, 20, 40 until it works - just to prove the theory. @AnHardt If I set this to 30 then I'm able to print at a slow speed and 0.15mm layer height, thank you! Haven't tested much yet, but is there an obvious downside for me to just keep using it set to this for now?

AnHardt commented 6 years ago

@roguecode Incremental positioning does suffer. When adjusting z the smallest difference distance is now 30 steps. But you can reach any step exact position from coming farer away. Sending G1 Z31 steps followed by G1 Z-30 steps will still move you one step forward. You can also test if decreasing DELTA_SEGMENTS_PER_SECOND has a positive effect. Maybe you can then lower MIN_STEPS_PER_SEGMENT again. Search for your best compromise.

AnHardt commented 6 years ago

@e-Mole What makes you think altering DELTA_SEGMENTS_PER_SECOND would help with your (too much extrusion) problem? I had an idea for the reason of the monochrome printing problem - and i was right. I have an other idea where the (too much extrusion) problem could come from but need to dive deeply into the stepper code to prove it. Maybe than i can come up with some tests.

Does it make a difference in extruded amount, if you try to print 'black' E0, 'white' E1 or 'gray' E1+E2 [50%,50%], 'light gray' [25%,75%], 'dark gray' [75%,25%]? If yes guess, how much more. Does 'gray' extrude 1.5 or 2 times more than 'black' or 'white'? Try to describe the relations.

AnHardt commented 6 years ago

@e-Mole Please try #11856 or #11855. hopefully you'll see at least some improvement. I'm not confident that's all.

AnHardt commented 6 years ago

When mixing MIXING_STEPPERS_LOOP can cause up to MIXING_STEPPERS steps at once. One for each stepper. In other cases zero steps are made. Depends on the relation of the colors. I see worst effects when all colors are 1/MIXING_STEPPERS and E is not the leading axis. Can't be good for the pressure.

    #else // !LIN_ADVANCE - use linear interpolation for E also
      #if ENABLED(MIXING_EXTRUDER)

        // Tick the E axis
        delta_error[E_AXIS] += advance_dividend[E_AXIS];
        if (delta_error[E_AXIS] >= 0) {
          count_position[E_AXIS] += count_direction[E_AXIS];
          delta_error[E_AXIS] -= advance_divisor;
        }

        // Tick the counters used for this mix in proper proportion
        MIXING_STEPPERS_LOOP(j) {
          // Step mixing steppers (proportionally)
          delta_error_m[j] += advance_dividend_m[j];
          // Step when the counter goes over zero
          if (delta_error_m[j] >= 0) E_STEP_WRITE(j, !INVERT_E_STEP_PIN);
        }

      #else // !MIXING_EXTRUDER
        PULSE_START(E);
      #endif
    #endif // !LIN_ADVANCE
...
    #if DISABLED(LIN_ADVANCE)
      #if ENABLED(MIXING_EXTRUDER)
        MIXING_STEPPERS_LOOP(j) {
          if (delta_error_m[j] >= 0) {
            delta_error_m[j] -= advance_divisor_m;
            E_STEP_WRITE(j, INVERT_E_STEP_PIN);
          }
        }
      #else // !MIXING_EXTRUDER
        PULSE_STOP(E);
      #endif
    #endif // !LIN_ADVANCE

Solution could be a more steady e-step stream like suggested in https://github.com/MarlinFirmware/Marlin/issues/11310#issuecomment-422035637

Ping @thinkyhead

thinkyhead commented 6 years ago

I don't quite understand the delta_error_m addition. Perhaps @ejtagle can shed some light on what this is meant to be doing.

e-Mole commented 6 years ago

@AnHardt : Thanks! Patch # 11855 does not affect the behavior of my extruders. (I'll make the exact measurements of the extruded filament length tomorrow.) And other problem, this code will return message: "Warning: Mix factors must add up to 1.0. Scaling." Is this gcode correct (or am I wrong)? M163 S0 P0.5 M163 S1 P0.5 M164 S2

AnHardt commented 6 years ago
M163 S0 P2
M163 S1 P2
M164 S2

should work without the message. Mad isn't it. However, the skaling does work now. Maybe i have an idea for a bit of additional code.

if (value > 1.0) value = 1/value;

at the moment always the reciproce values are stored in the array. Your 0.5 values are converted to 2. No wonder they don't sum up to 1.

AnHardt commented 6 years ago

M163 Without additional code only one of the examples can be correct.

M163 S0 P0.6
M163 S1 P0.4
M164 S5

currently converts to 1/0.6 = 1.66, 1/0.4 = 2.5, 1.66+2.5 = 4.16, 1/4.16 = 0.24, 1.66*0.24 = 0.4, 2.5*0.24 = 0.6. That's the opposite of the expected relation.

M163 S0 P3
M163 S1 P5
M164 S4

will be as expected. 1/3 + 1/5 = 0.533, 1/0.533 = 1.875, 1/3*1.875 = 0.625, 1/5*1,875 = 0,375, 0,613+0,375 = 1 . Looks correct.


0.6 * 0.4 = 1.0 correct!

3 + 5 = 8, 1/8 = 0.125, 3*1/8 = 3/8, 5*1/8 = 5/8, 3/8 + 5/8 = 1 correct!

Ups. Sorry

thinkyhead commented 6 years ago

Oh wait, you are correct! I forgot that the normalization doesn't happen until M164.

thinkyhead commented 6 years ago

I should note that M164 is absolutely required after M163. If it's left out then the mix will never get normalized.

AnHardt commented 6 years ago

Values below 1 are already reziproke an additional unconditional 1/x messes up the relation.

roguecode commented 6 years ago
M163 S0 P2
M163 S1 P2
M164 S2

should work without the message. Mad isn't it.

I've been using

M163 S0 P0.5
M163 S1 P0.5
M164 S2

and it works without that message. If they don't add up to 1 then I get the message. Is that not intended?

Side question, does the M164 save all pending M163's to the tool specified? And does that also mean I can throw gcode like the above 3 code lines into my gcode at various intervals and have it change the active tool (assuming I do S2 and the printer is currently set to T2)?

thinkyhead commented 6 years ago

Values below 1 are already [reciprocated]. An additional 1/x messes them up

Normalization adds up all the values, then divides each value by that sum. Unless they all sum to zero, this works to make sure they all add up to 1.0. M164 makes this happen.

So normalize_mix() is now correct for the updated planner.

thinkyhead commented 6 years ago

does the M164 save all pending M163's to the tool specified?

That's what it does. But it normalizes them first so they sum to 1.0, as described in the comment above.

thinkyhead commented 6 years ago

It may be worthwhile to modify M164 so it can normalize the mix factors without storing to a tool index:

void GcodeSuite::M164() {
  normalize_mix();
  const int tool_index = parser.intval('S', -1);
  if (WITHIN(tool_index, 0, MIXING_VIRTUAL_TOOLS - 1)) {
    for (uint8_t i = 0; i < MIXING_STEPPERS; i++)
      mixing_virtual_tool_mix[tool_index][i] = mixing_factor[i];
  }
}
AnHardt commented 6 years ago

Could we please, for the moment, stay at the reversed relation when input values are below 1 in M163 as well as in DIRECT_MIXING_IN_G1.

thinkyhead commented 6 years ago

If they add up to 1.0 then it makes no difference. It's only when they add up to something other than 1.0 that normalization even takes place.

AnHardt commented 6 years ago
void GcodeSuite::M163() {
  const int mix_index = parser.intval('S');
  if (mix_index < MIXING_STEPPERS) {
    float mix_value = parser.floatval('P');
    NOLESS(mix_value, 0.0);
    mixing_factor[mix_index] = RECIPROCAL(mix_value); // !!!!!!!! this is unconditional !!!!
  }
}

A 0.1 becomes a 10. the arrays sum will never be 1.

thinkyhead commented 6 years ago

That appears to be the old errant code, not the current fixed code.

The fix should have already been made as part of your prior PRs. I guess it was missed.

AnHardt commented 6 years ago

https://github.com/MarlinFirmware/Marlin/blob/1.1.x/Marlin/Marlin_main.cpp#L12041 https://github.com/MarlinFirmware/Marlin/blob/bugfix-1.1.x/Marlin/Marlin_main.cpp#L12041

thinkyhead commented 6 years ago

A 0.1 becomes a 10. the arrays sum will never be 1.

As previously explained, the mix factors used to be used to increase the Bresenham ceiling, so RECIPROCAL used to be correct. Now it's not.

https://github.com/MarlinFirmware/Marlin/blob/1.1.x/Marlin/Marlin_main.cpp#L12041

Yep, I corrected my earlier comment already…

"The fix should have already been made as part of your prior PRs. I guess it was missed."

AnHardt commented 6 years ago

You still have not got it. Please be so kind to return to https://github.com/MarlinFirmware/Marlin/issues/11310#issuecomment-422202018 and follow my calculations. Than please recalculate both of the examples without the first 1/x.

thinkyhead commented 6 years ago

will be as expected. 1/3 + 1/5 = 0.533, 1/0.533 = 1.875, 1/31.875 = 0.625, 1/51,875 = 0,375, 0,613+0,375 = 1 . Looks correct.

Here is where you're confused by the lingering —should have been removedRECIPROCAL in M163. That RECIPROCAL should simply not be there.


Copying from my comment on #11861…

I hope now it is more clear that the reason for RECIPROCAL in M163 was to provide the mix multipliers that the old (1.1.8) Stepper / Planner code needed to scale up the Bresenham ceiling for the mixing stepper counters. The RECIPROCAL there should have simply been removed when the refactor was done, because the mix factors are now used to scale down the E addends for the mixing steppers instead.

The examples on the website are 100% valid. M164 normalizes the mix.

thinkyhead commented 6 years ago

So, to clarify the meaning of this example:

M163 S0 P3
M163 S1 P5
M164 S4

Mix factor 0 and 1 (with the new code) should be initially set to 3.0 and 5.0. Then when M164 is called, they are converted to 3/8 and 5/8, or 0.375 and 0.625.

The old (1.1.8) code would have stored the reciprocals instead, giving 2.666 and 1.6, and used those to scale up the Bresenham ceiling/limit. The refactor undid my earlier work, and so the normalized (non-reciprocals) are now supposed to be stored and used to scale down the E addends.

While the planner/stepper code was updated in the refactor, the needed updates to normalize_mix, M163-M165 were neglected. @ejtagle and I both dropped the ball on those changes.

Hope that clears up all the confusion! All should now be corrected by #11861.

AnHardt commented 6 years ago

Sorry for the mess! https://github.com/MarlinFirmware/Marlin/blob/bugfix-1.1.x/Marlin/Marlin_main.cpp#L3304 https://github.com/MarlinFirmware/Marlin/blob/bugfix-2.0.x/Marlin/src/feature/mixing.cpp#L80 remain.

thinkyhead commented 6 years ago

Changes from my two PRs are not yet merged.

e-Mole commented 6 years ago

Is there something new with "too fast" mixing extruder problem in 1.1.9? Can I help with testing? Thanks!

AnHardt commented 6 years ago

@e-Mole Still waiting upon the results of

Does it make a difference in extruded amount, if you try to print 'black' E0, 'white' E1 or 'gray' E1+E2 [50%,50%], 'light gray' [25%,75%], 'dark gray' [75%,25%]? If yes guess, how much more. Does 'gray' extrude 1.5 or 2 times more than 'black' or 'white'? Try to describe the relations.

You could try https://github.com/MarlinFirmware/Marlin/pull/11861#issuecomment-423543971. But it's very very alpha.

e-Mole commented 6 years ago
G92 E0
G92 E1
//no xy movement
T0 (E0 100% E1 0%)
G1 E50 F2000 (extrude 50 mm from E0 - o.k.)

T1 (E0 0% E1 100%)
G1 E50 F2000 (extrude 50 mm from E1 - o.k.)

T2 (E0 50% E1 50%)
G1 E50 F2000 (extrude 25 mm from E0 and 25 mm from E1 - o.k.)

//xy movement from 0,0 to 0, -100 
T0 (E0 100% E1 0%)
G1 X0 Y-100 E50 F2000 (extrude 66 mm from E0 - strange!)

T1 (E0 0% E1 100%)
G1 X0 Y-100 E50 F2000 (extrude 66 mm from E1 - strange!)

T2 (E0 50% E1 50%)
G1 X0 Y-100 E50 F2000 (extrude 33 mm from E0 and 33 mm from E1 - strange!)

XY movement is reflected on the axis E...

AnHardt commented 6 years ago

Thanks. So the relation of the colors does not make a difference. But when a xyz-distance is involved there are ~+1/3 e-steps. We'll see.

e-Mole commented 6 years ago

I have now noticed that when moving in xyz axes, E axis moves even if gcode for E is not contained! But only if preceded by a command with an E-axis:

//xy movement from 0,0 to 0, -100; no E?
T0 (E0 100% E1 0%)
G1 X0 Y-100 E100 F2000 (extrude 106 mm from E0 - strange!)
G1 X0 Y0 E0 F2000 (retract 106 mm from E0 - strange!)
G1 X0 Y-100 F2000 (extrude 66 mm from E0 - strange!)
G1 X0 Y0 F2000 (extrude nothing)
AnHardt commented 6 years ago

After changingblock->step_event_count with esteps:

//xy movement from 0,0 to 0, -100
T0 (E0 100% E1 0%)
G1 X0 Y-100 E100 F2000 (extrude 106 mm from E0 - strange!)
G1 X0 Y0 E0 F2000 (retract 106 mm from E0 - strange!)
G1 X0 Y-100 F2000 (extrude nothing) - here the behavior has changed!
G1 X0 Y0 F2000 (extrude nothing)

So we are at the right track. +6% could be the rounding error for small segments. Try decreasing DELTA_SEGMENTS_PER_SECOND to ~half of the value you had before. Does the error% shrink?

e-Mole commented 6 years ago

DELTA_SEGMENTS_PER_SECOND 120, 100, 50 - the extrusion is the same - 106 mm (The length of the extrusion is now the same with or without movements in the xyz axes.)

AnHardt commented 6 years ago

Hmmm. Rounding should be slightly improved by:

    for (uint8_t i = 0; i < MIXING_STEPPERS; i++)
      block->mix_steps[i] = mixing_factor[i] * ABS(esteps_float) + 0.5f;

in the block we altered before. The differentiation (#if ENABLED(LIN_ADVANCE)) does not make sense. The calculation was in float anyway. If that's still to much, try removing the + 0.5f

AnHardt commented 6 years ago

If the extrusion is now consistently (always) 106% this can be calibrated away with steps/mm.

AnHardt commented 6 years ago

I'll stop here investigating the old code and concentrate on my new one, hopefully solving some of the problems.

e-Mole commented 6 years ago

Removing the + 0.5f - I do not notice any change.

e-Mole commented 6 years ago

@AnHardt After re-calibration of the E-axis, color mixing is working now. In the case of a single extruder (and separate dual extruder too), the value "e steps per unit" must be bigger (149 for mixing, 158 for single/dual - 6 % difference). I'll try some test print ...

e-Mole commented 6 years ago

After further tests: Mixing is still a problem. At 1: 1 ratio, the result is good. At the moment when one extruder feeds significantly more material (eg 1: 5) there is a significant underextrusion. But only in some "regions" - it seems as negative effect of curvature (number of segments?).

AnHardt commented 6 years ago

Print longer segments. When setting up the bresnham lines there is rounding. Often there is one step omitted. With a normal extruder that one step usually does not really matter. With Mixing extruders the problem is MIXING_STEPPERS times larger.

HattonLe commented 5 years ago

As per e-Mole Sept 21 comment... I'm just upgrading my printer from 1 hotend to a diamond hotend with 3 extruder drives. I'm using Marlin-bugfix-1.1.x version, 1.1.9 code. All I changed in the configuration.h file was to uncomment "#define MIXING EXTRUDER". Something wasn't quite right during a test print with the filament getting chewed up in extruder 1. I eventually worked out that Extruder drive 1 was moving even when just doing a home at the end of the print (hot end has to be heated up to disable cold extrusion, thus enabling the E steppers to rotate for this problem to become apparent). During a Home, no extruder should ever move. ... Actually this exact issue is listed under #11891

AnHardt commented 5 years ago

In recent 1.1.x bugfix this should be fixed. MIXING EXTRUDER is completely reworked in bugfix-2.0.x.