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.17k stars 19.21k forks source link

new Advance Extrusion algorithm (LIN_ADVANCE) extruder motor not running #4549

Closed Macgyver001 closed 7 years ago

Macgyver001 commented 8 years ago
The Marlin version RC7 - 31 Jul 2016
Machine Self build coreXY
Electronics board ramps 1.4 with arduino 2560
Machine components (direct drive e3d lite, with termistors (my stepper drivers are m542T and I run  at 64 micro step also the extruder)
Host software (none
Slicer (if relevant)-
Printing method (Pronterface or Octoprint)

Configuration_and_adv.zip

Problem: When I enable Lin_advance my extruder motor does not turn/extrude. What Have I tried to do.

Turn off core XY, but still no extruding K factor put to 0 no effect Tried to lower the extrusion speed no effect Extruding in reverse is also not working When disable watchdog does not help

When I disable Lin_advance it extrudes well.

thinkyhead commented 7 years ago

is #4852 already landed/merged…?

image

thinkyhead commented 7 years ago

In case of LIN_ADVANCE, When I order g1 e100 f600 (single-stepping), pulse timing fall into disorder, subsequently, Marlin freeze completely. Green LED flash repeatedly, watchdog and reset button on RAMPS doesn't work. It needs complete power off. But when I order g1 e100 f630 (double-stepping), it looks like stable. I'm still researching now.

@esenapaj Did this resolve itself?

Macgyver001 commented 7 years ago

Sorry Thinkyhead for the question I am quite new with this.

I tried the PR4852 fix, but I have more or less the same as Esenapaj. I tried it with Lin_advance = 0 to start with. Then I god loads of this message: Resend: 12307 Error:No Checksum with line number, Last Line: 12346 [ERROR] Error:No Checksum with line number, Last Line: 12346

The printer is working but slow and seems sometimes a bit off. (but the print finished, and the result is slightly worse than Lin_advance disabled. (1 inch cube and 5mm high)

I tried with Lin_advance = 75 Started the same print, but I stopped it after 1 minute, print result was NOK bad lines, but worse my heater controller didn't respond as normal with same intervals, but did unreliable on / off. Totally different than normal.

I have the feeling that the MINIMUM_STEPPER_PULSE =1 did a better job than the volatile long Stepper::e_steps[E_STEPPERS]; I think it's taking to long? While with MINIMUM_STEPPER_PULSE and CYCLES_EATEN_BY_E_CODE 12 (in my case) you can tune the minimum delay time and therefore minimize the impact to the code.

I will try the MINIMUM_STEPPER_PULSE =1 again with Lin_advance = 75, will update you next time.

thinkyhead commented 7 years ago

@Macgyver001 I've made some tweaks to the way MINIMUM_STEPPER_PULSE is applied in #4887. Once that's merged, it should prevent the regular stepper ISR from gaining extra pulse-checking code when only the advance ISR needs it. You'll have to use a MINIMUM_STEPPER_PULSE of 4 or more, and/or adjust CYCLES_EATEN_BY_E.

Macgyver001 commented 7 years ago

I tried the latest RCBugFix branch 29-9-16, put Minimum_stepper_pulse to 4 and tried to get it working with Lin_advance = 75, still I have problems with my temperature controller. I tried Minimum_stepper_pulse to 0 and still problems with the temperature controller, but yes the E stepper was extruding? Then I tried Lin_advance = 0 & Minimum_stepper_pulse to 0 that was working, but got some Resend: 12307 Error:No Checksum with line number, Last Line: 12346 [ERROR] Error:No Checksum with line number, Last Line: 12346

After Lin_advance = 10 & Minimum_stepper_pulse to 0 the temperature controller got wrong again. The timing has changed, because the E stepper is extruding, but my system can't cope with I think the difference in timing of the temperature controller that expects the same time interval between samples?

Is is possible that the coreXY is using more processor power than the non-coreXY systems? Is there any idea how many people use Lin_advane?

Sebastianv650 commented 7 years ago

We had the discussion about coreXY and the power it's using once before. It's quite possible that coreXY and LIN_ADVANCE together may overload the CPU. I don't know if somebody ever got Advance to work on a coreXY up to now..

Regarding how many people are using it: I know about one or two Lulzbot TAZ users are using it with success and myself of course. There might be others?

mosh1 commented 7 years ago

I've definitely been using it with good success / E3D Bigbox (Cartesian gantry).

ghost commented 7 years ago

@thinkyhead

Did this resolve itself?

With newest RCBugFix, it respond erratic result. Pulse timing became stable after the PR #4868 (Cleanups for the Autumn release), But when e steps/mm is 953.1 (Bondtech QR at 32 micro-step) and ordered a g1 e100 f600(9531steps/sec), results are ・Not freeze, but Repetier-Host spout a lot of "Error:checksum mismatch" and "Error:No Checksum with line number" and "Resend". or ・Freeze silently and watchdog doesn't work, but reset button of RAMPS and Repetier-Host works. or ・Freeze with green LED on RAMPS flashing, watchdog and reset button on RAMPS and Repetier-Host doesn't works.

Marlin has the threshold of between single-stepping and double-stepping that it's set 10000(steps/sec or Hz). But in case of ADVANCED and LIN_ADVANCED, It looks like that it's too heavy for 16MHz AVR.

Macgyver001 commented 7 years ago

@thinkyhead & @esenapaj

With RCBugFix branch 29-9-16 and Lin_advance = 10 & Minimum_stepper_pulse to 0 I disabled the Watchdog, when I do this the temperature controller is just NOT making it, but way better than when I Enable Watchdog. So I also think that this Lin_advanced is using the 16MHz AVR to the limit.

I went back to the RC7 31 july 16, disabled the watchdog and enabled Lin_advance = 10 I adjusted the stepper.ccp code

void Stepper::advance_isr() {

old_OCR0A += eISR_Rate;
OCR0A = old_OCR0A;

#define STEP_E_ONCE(INDEX) \
  if (e_steps[INDEX] != 0) { \
    asm("nop\nnop\nnop\nnop\nnop\nnop\nnop\nnop\nnop\nnop\nnop\nnop\nnop\nnop\nnop\nnop\n"); \
    E## INDEX ##_STEP_WRITE(!INVERT_E_STEP_PIN); \
    if (e_steps[INDEX] < 0) { \
      asm("nop\nnop\nnop\nnop\nnop\nnop\nnop\nnop\nnop\nnop\nnop\nnop\nnop\nnop\nnop\nnop\n"); \
      E## INDEX ##_DIR_WRITE(!INVERT_E## INDEX ##_DIR); \
      e_steps[INDEX]++; \
     } \
    else { \
      asm("nop\nnop\nnop\nnop\nnop\nnop\nnop\nnop\nnop\nnop\nnop\nnop\nnop\nnop\nnop\nnop\n"); \
      E## INDEX ##_DIR_WRITE(INVERT_E## INDEX ##_DIR); \
      e_steps[INDEX]--; \
    } \
    E## INDEX ##_STEP_WRITE(INVERT_E_STEP_PIN); \
  }

I don't know if this is the wright way, it did not succeed yet, I will try to add more nop's? (E stepper is not moving yet) temperature control seems ok To be continued

thinkyhead commented 7 years ago

But in case of ADVANCED and LIN_ADVANCED, It looks like that it's too heavy for 16MHz AVR.

I'd be interested to know where we are hitting the limits, and compare this to older versions of Marlin to see where we stand in terms of comparative performance. I'm sure there is some added overhead in various places.

Mainly we should time the core operations…

Other performance-related questions:

thinkyhead commented 7 years ago

I don't know if this is the wright way, it did not succeed yet, I will try to add more nop's?

@Macgyver001 You should simply add your delay in the same place where MINIMUM_STEPPER_PULSE inserts the delay — after all the pulses (in this case, your single E pulse) has been started. That will ensure that it keeps working as more E steppers are added.

Elsewhere I suggested the somewhat kooky-looking:

#if F_CPU == 20000000UL
  #define P_NOPS asm("nop\nnop\nnop\nnop\nnop\nnop\nnop\nnop\nnop\nnop")
#else
  #define P_NOPS asm("nop\nnop\nnop\nnop\nnop\nnop\nnop\nnop")
#endif
switch ((uint32_t)(STEP_PULSE_CYCLES - CYCLES_EATEN_BY_CODE) / (F_CPU/1000000UL)) {
  case 9: P_NOPS; P_NOPS;
  case 8: P_NOPS; P_NOPS;
  case 7: P_NOPS; P_NOPS;
  case 6: P_NOPS; P_NOPS;
  case 5: P_NOPS; P_NOPS;
  case 4: P_NOPS; P_NOPS;
  case 3: P_NOPS; P_NOPS;
  case 2: P_NOPS; P_NOPS;
  case 1: P_NOPS; P_NOPS;
  case 0:
}

The compiler will condense the invariant switch statement down to the appropriate number of nop commands.

Macgyver001 commented 7 years ago

Thanks @thinkyhead, others..

Let me summarize what I have learned by adjusting the following. Put Minimum_stepper_pulse = 1. Enabled LIN_advance K = 10 to start off with. I commented as many defines I could think off, for example I don't use LCD. Put in Macros.h

#if F_CPU == 20000000UL
  #define P_NOPS asm("nop\nnop\nnop\nnop\nnop\nnop\nnop\nnop\nnop\nnop")
#else
  #define P_NOPS asm("nop\nnop\nnop\nnop\nnop\nnop\nnop\nnop")
#endif

I brought down the stepper.ccp code to a minimum, by commented every thing that I believe is not relevant, see below.

  void Stepper::advance_isr() {
    old_OCR0A += eISR_Rate;
    OCR0A = old_OCR0A;
    #define START_E_PULSE(INDEX) \
      if (e_steps[INDEX]) E## INDEX ##_STEP_WRITE(!INVERT_E_STEP_PIN)
    #define STOP_E_PULSE(INDEX) \
      if (e_steps[INDEX]) { \
        e_steps[INDEX] <= 0 ? ++e_steps[INDEX] : --e_steps[INDEX]; \
        E## INDEX ##_STEP_WRITE(INVERT_E_STEP_PIN); \
      }
    //#define CYCLES_EATEN_BY_E 60
    // Step all E steppers that have steps
    for (uint8_t i = 0; i < step_loops; i++) {
      //#if STEP_PULSE_CYCLES > CYCLES_EATEN_BY_E
      //  static uint32_t pulse_start;
      //  pulse_start = TCNT0;
      //#endif

      START_E_PULSE(0);
      #if E_STEPPERS > 1
        START_E_PULSE(1);
        #if E_STEPPERS > 2
          START_E_PULSE(2);
          #if E_STEPPERS > 3
            START_E_PULSE(3);
          #endif
        #endif
      #endif
      // For a minimum pulse time wait before stopping pulses
      //#if STEP_PULSE_CYCLES > CYCLES_EATEN_BY_E
      #if    MINIMUM_STEPPER_PULSE>0
        switch (MINIMUM_STEPPER_PULSE) {
        case 9: P_NOPS; P_NOPS;
        case 8: P_NOPS; P_NOPS;
        case 7: P_NOPS; P_NOPS;
        case 6: P_NOPS; P_NOPS;
        case 5: P_NOPS; P_NOPS;
        case 4: P_NOPS; P_NOPS;
        case 3: P_NOPS; P_NOPS;
        case 2: P_NOPS; P_NOPS;
        case 1: P_NOPS; P_NOPS;
        case 0:;
        }
      #endif

That did not solve my temperature control issue, and very bad prints.

To reduce delay timing I went down too see if printer would react fine:

case 1: P_NOPS;
#define P_NOPS asm("nop\nnop")

At that point my E_stepper is not moving anymore, the temperature controller improved but still not as normal.

That was as far I could go, but without good result.

I remember that in the beginning we changed another parameter from int to long:

#if ENABLED(LIN_ADVANCE)
-   volatile int Stepper::e_steps[E_STEPPERS];
+   volatile long Stepper::e_steps[E_STEPPERS];

For long straight lines. I changed this back

 volatile int Stepper::e_steps[E_STEPPERS];

I also changed the e_steps in stepper.h

I was down to core level -> with the delay (as small that the e-stepper didn't move. Than my temperature controller got back to normal!!! ( I have read in another issue that somebody stated that after some pr#.... LIN_advance didn't work properly before it did, could be related to this change int --> long??

After that I increased the number off nop's again, to the level, off:

case 1: P_NOPS; P_NOPS;
#define P_NOPS asm("nop\nnop\nnop\nnop\nnop\nnop\nnop\nnop")

The print was not fine yet, so I need to increase the number of nop's even more. During this process the temperature controller seems unaffected?

To increase processing power: Is it difficult to turn up at-mega from 16 to 20 MHz. Is the marlin firmware also usable for arduino Duo?? does anyone have experience.

I will update you later if I can print with more nops!

thinkyhead commented 7 years ago

Working on some patches to LIN_ADVANCE that may also fix the amount of extrusion, and the running speed. No guarantee it will have any effect for this issue. #4980

boelle commented 7 years ago

@thinkyhead still going to work on this one?

thinkyhead commented 7 years ago

@Macgyver001 How does the original issue stand? LIN_ADVANCE and planner acceleration have both been patched a lot lately.

Macgyver001 commented 7 years ago

Guys for running stable I have the feeling that I need quite a lot of speed improvement, not a few % or so but like 3x and up. I want even higher resolution (more micro steps). Therefore, I am upgrading my printer with a due and Radds board and can not run Marlin anymore. (that feels like pain in my hart because I really believe in Marlin)

I have enjoyed the good support from you guys and I think this issue brought some change in the way programming is done and where the firmware can be sped up. nice job to you all.

boelle commented 7 years ago

@thinkyhead do you think this might have been fixed? maybe close and let people throw a new issue if the problem is still there

boelle commented 7 years ago

@Macgyver001 remember that microstepping does not give better resolution. and reduces the steppers' holding torque. But 99% of the 3D printing world does not agree and don't do proper research.

thinkyhead commented 7 years ago

We'll continue to optimize as we go. Of course, faster boards are needed for the full compendium of features now available.

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.