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

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

Closed
Macgyver001 opened this issue Aug 6, 2016 · 70 comments
Closed
Labels
Bug: Potential ? Needs: Testing Testing is needed for this change
Milestone

Comments

@Macgyver001
Copy link

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
Copy link
Member

Sounds like a possible bug! Between myself and @Sebastianv650 we should be able to figure it out…

@Sebastianv650
Copy link
Contributor

Sebastianv650 commented Aug 7, 2016

If the problem is also there if K=0 then it should be something with the extruder ISR. This is the only big change when LIN_ADVANCE is enabled.
I had a short look on your config, the values look quite strange to me:

#define DEFAULT_AXIS_STEPS_PER_UNIT   {320,320,800,386}  // default steps per unit for Ultimaker
#define DEFAULT_MAX_FEEDRATE          {500, 500, 2, 50}   // (mm/sec)
#define DEFAULT_MAX_ACCELERATION      {60,60,200,500}    // X, Y, Z, E maximum start speed for accelerated moves. E default values are good for Skeinforge 40+, for older versions raise them a lot. was 75

#define DEFAULT_ACCELERATION          60    // X, Y, Z and E acceleration in mm/s^2 for printing moves
#define DEFAULT_RETRACT_ACCELERATION  300    // E acceleration in mm/s^2 for retracts
#define DEFAULT_TRAVEL_ACCELERATION   80    // X, Y, Z acceleration in mm/s^2 for travel (non printing) moves

// The speed change that does not require acceleration (i.e. the software might assume it can be done instantaneously)
#define DEFAULT_XYJERK                500.0    // (mm/sec) was 20 was 700
#define DEFAULT_ZJERK                 0.4     // (mm/sec)
#define DEFAULT_EJERK                50.0

Why do you use so incredible high jerk values combined with ultra-low acceleration values? I can't imagine this can work..! Do you ever printed good things with theese values?

@Macgyver001
Copy link
Author

Macgyver001 commented Aug 8, 2016

Hi thanks for the quick response.

The jerk settings are not aligned very carefully so thank you for your comment.
I have lowered a lot of values one by one, but without any result, no extruding when Lin_advance is enable, final settings are listed below:

#define DEFAULT_AXIS_STEPS_PER_UNIT   {320,320,800,100}  // default steps per unit for Ultimaker 160*2,160*2,800,((200*64)*1.0/(10.56*3.14159))=386
#define DEFAULT_MAX_FEEDRATE          {200, 200, 2, 10}   // (mm/sec)
#define DEFAULT_MAX_ACCELERATION      {60,60,100,10}    // X, Y, Z, E maximum start speed for accelerated moves. E default values are good for Skeinforge 40+, for older versions raise them a lot. was 75 e=500

#define DEFAULT_ACCELERATION          10    // X, Y, Z and E acceleration in mm/s^2 for printing moves
#define DEFAULT_RETRACT_ACCELERATION  10    // E acceleration in mm/s^2 for retracts
#define DEFAULT_TRAVEL_ACCELERATION   80    // X, Y, Z acceleration in mm/s^2 for travel (non printing) moves

// The speed change that does not require acceleration (i.e. the software might assume it can be done instantaneously)
#define DEFAULT_XYJERK               5.0    // (mm/sec) was 20 was 500
#define DEFAULT_ZJERK                 0.4     // (mm/sec)
#define DEFAULT_EJERK                 2.0    // (mm/sec) 5.0 was 2000

Is there something changed in the extruder stepper motor signals when using Lin_advance? I have connected my ramps 1.4 via self made wiring towards my stepper drivers m542T (but as I mentioned I do not change connections Lin_advance disabled or enabled and it works when I disable Lin_advance)

@thinkyhead
Copy link
Member

I recommend you keep testing daily with RCBugFix as we keep fixing things by mistake. This will also allow us to insert some debugging code to help track the issue – something we would generally only do with the working branch.

@Sebastianv650
Copy link
Contributor

Sebastianv650 commented Aug 9, 2016

No, LIN_ADVANCE isn't changing anything in the way the stepper is electrically driven. The only difference is the timing, when enabled the extruder has its own ISR.

I would start with common configuration values and see what happens. For example, maybe a value can overflow with an acceleration of only 10mm/s². Therefore, I would set all acceleration values to 500 and test again.

If a simple extruder-only move (for example G1 E3 F50) isn't working with enabled advance, I would check the wiring or other values in the basic config. There is no reason I can think of why such a command should fail due to LIN_ADVANCE.

@thinkyhead
Copy link
Member

thinkyhead commented Aug 10, 2016

@Sebastianv650 Well, you will want to look through the LIN_ADVANCE code and make sure nothing got broken since it was first integrated. So far it looks okay, but my eye is not likely to be as keen.

@ghost
Copy link

ghost commented Aug 10, 2016

Possibly this?
2/4 phase Nema 23 Stepper Motor Driver 24-50VDC 1.5A-4.5A 256 Microstep M542T|M542T|Stepper Motor Drivers
http://www.omc-stepperonline.com/24-phase-nema-23-stepper-motor-driver-2450vdc-15a45a-256-microstep-m542t-p-293.html

For reliable response, pulse width should be longer than 1.5μs.

Extruder working fine with A4988s, not with DRV8825, repetier works, marlin not · Issue #975 · MarlinFirmware/Marlin
#975 (comment)

From Pololu's site:

With the DRV8825, the high and low STEP pulses must each be at least 1.9 us; they can be as short as 1 us when using the A4988.

@ghost
Copy link

ghost commented Aug 10, 2016

@to see the Repetier-Firmware, Additional delay is added in here and there (STEPPER_HIGH_DELAY and DOUBLE_STEP_DELAY) for stepper driver that it needs long pulse.

@Macgyver001
Copy link
Author

Hello all,

Today I checked the enable signal on my stepper driver, that seems to work fine and is not the issue
I also tried the acceleration = 500 and jerk 50 but that also did not help.
It could be related to pulse width? Is there an option to change that in the code.
I will check if I can find anything related to pulse with in the data sheet of the m542 driver?

@jbrazio jbrazio modified the milestone: 1.1.0 Aug 11, 2016
@Sebastianv650
Copy link
Contributor

Sebastianv650 commented Aug 11, 2016

With a jerk of 50, you may end up with such high speed changes that the extruder can't keep up with the pressure adjustment moves. Extruder only moves shouldn't be affected by LIN_ADVANCE, but I will have a look at the latest Marlin code after my holidays in about 2 weeks.

@Macgyver001
Copy link
Author

Ok, but last time I tested with jerk of 2!
I checked the timing of the step pulse and its indeed 1.5 us up and down and 5 us after a direction switch, I don't know what the current timing is, and if its shorter than that?

Thanks for checking, I will also go on holiday, so lets catch up after..

timing diagram m542t controller

@thinkyhead
Copy link
Member

@esenapaj Do the Marlin settings CONFIG_STEPPERS_TOSHIBA and MAX_STEP_FREQUENCY seem to be equivalent to the repetier delay options?

@ghost
Copy link

ghost commented Aug 12, 2016

No, it isn't.
It looks like that CONFIG_STEPPERS_TOSHIBA of Marlin suppress the MAX_STEP_FREQUENCY simply, and disable double / quad stepping mode automatically.

In case of STEPPER_HIGH_DELAY and DOUBLE_STEP_DELAY of Repetier-Firmware,
those add additional delay to section of stepper.
For example, they add additional delay to A, B, C.
Maybe those lines of Marlin are A, B, C.

And, Marlin decide threshold value between single and double stepping automatically.
In case of Repetier-Firmware, threshold value between single and double stepping is configurable (STEP_DOUBLER_FREQUENCY).

@boelle
Copy link
Contributor

boelle commented Aug 14, 2016

@thinkyhead does this make a bell ring ? :-D

@thinkyhead
Copy link
Member

I'm too shy to start messing with the stepper code at this junction.

@Macgyver001
Copy link
Author

Hi all,

I have tried to reduce MAX_STEP_FREQUENCY to 10000, but without any result.
Is there a dirty way to add a delay in only the Extruder stepper, for testing purposes? If this is the correct direction.
In the older version of marlin from April 2016 I enabled the 128 u stepping, this gives me a resolution of 640 steps/mm and Extruder 772 steps/mm, at 20mm/s the processor is still capable of running this setup.

@Roxy-3D
Copy link
Member

Roxy-3D commented Sep 1, 2016

Is there a dirty way to add a delay in only the Extruder stepper, for testing purposes? If this is the correct direction.

Can you explain in a little more detail what you are thinking and why this would be beneficial? I need to understand the problem a little better.

But the reason I'm asking for more detail is this: Right now I'm messing with the mesh_buffer_line() routine to make it non-recursive (to lighten the CPU load). On any given Planner request, it would not be difficult to stagger when an axis moves relative to the other axis.

@Macgyver001
Copy link
Author

Hi Roxy,

The problem is that when I enable Lin_advance my extruder stepper does not work.
In the documentation of my external stepper controller it is stated that the steps (digital signals) should not be shorter than 1.5 u seconds. Therefore I would like to enlarge the timing of a high / low signal of only the extruder (or all stepper signals if necessary) (nothing else), to investigate if this is the problem. When I disable LIn_advance everything is working correctly. The XYZ axis are working fine in all occasions.

@Blue-Marlin
Copy link
Contributor

@thinkyhead
Copy link
Member

thinkyhead commented Sep 2, 2016

@Blue-Marlin Is the overhead of invoking interrupts very large, I wonder? I was thinking that the advance ISR could start the pulse(s) for E, and instead of sitting and waiting, it could instead set up an interrupt to stop the pulse(s) (soon) and exit right away. Perhaps the same approach is possible with the main stepper ISR.

This would allow them both to exit sooner and give time back to the main thread. But if the overhead of interrupts is significant, then this might actually lead to more cycles being used up, or if other interrupts might block the pulse-stopping interrupt, then pulses might not be stopped soon enough.

There's also the notion of not using any timer-based interrupts at all, but instead just always set up the next single-fire interrupt that's needed, and use a persistent shared state so that each of these tasks can be much smaller slices of the whole stepping procedure.

@Blue-Marlin
Copy link
Contributor

I might have seen something like that in ether grbl or RepetierFirmware. On the other hand i had a look into Prusa-Firmware today, where they not even have the interleave

        counter_x += current_block->steps_x;
        if (counter_x > 0) {
          WRITE(X_STEP_PIN, !INVERT_X_STEP_PIN);
          counter_x -= current_block->step_event_count;
          count_position[X_AXIS]+=count_direction[X_AXIS];   
          WRITE(X_STEP_PIN, INVERT_X_STEP_PIN);
        }

But with supporting only 2-3 defined boards they pretty well know what timing to expect. (And they kicked Advance at all.)

not be shorter than 1.5 u seconds.
That is ~24 ticks. With only one extruder in the loop this could be a bordercase, especially with the old STEP_E_ONCE(INDEX). (Have not really counted cycles.)

@Macgyver001 asked for a delay - there it is. But in reality i do not believe there is one needed.
Let' see if it helps.

@thinkyhead
Copy link
Member

thinkyhead commented Sep 2, 2016

had a look into Prusa-Firmware today, where they not even have the interleave

Hmm! I've wondered if possibly some of these stepper drivers are smartly leaving the pulse on for the duration needed, even though it's been turned off very quickly. I suppose we can query that with an oscilloscope.

Meanwhile, apparently Prusa Research has employed fixed-point maths where performance matters. I've been thinking about that a lot also.

@Blue-Marlin
Copy link
Contributor

I still try to find out what they have really done. The forks are pretty far apart from each other.
For now i was mostly interested what they have done with the new bed and the inductive sensor. Really interesting.

@Blue-Marlin
Copy link
Contributor

Blue-Marlin commented Sep 2, 2016

Comments like

//FIXME This routine is called 15x every time a new line is added to the planner,
// therefore it is a bottle neck and it shall be rewritten into a Fixed Point arithmetics,
// if the CPU is found lacking computational power.

do look more like a plan than like a done.

@thinkyhead
Copy link
Member

thinkyhead commented Sep 2, 2016

Searching the interwebs turns up a lot of discussion about pulse widths, decay time, etc. Lately I'm extra-interested in the topic because I'm not sure I'm a fan of DRV8825 for the current application. The SCARA machine I'm writing code for is currently set up to use 100:1 harmonic drives on the A/B axes, which means if using 32x micro-stepping, we need 1778 pulses per degree to move the steppers (which results in ~7mm per degree in the most extreme case, but only half of that per linkage).

Or in other words, total overkill in terms of movement accuracy. Even at 16x microstepping it amounts to 889 pulses per degree. When we start having this many pulses the bugs and errors start to get a bit weird. Imagine also, this is all on a Mega2560 platform, so again, really pushing the limits unnecessarily. I'm inclined to drop down even as low as 8x microstepping, but wondering how much extra sound that will produce.

In the end, I may have no choice but to either go with far lower microstepping or insist that we switch to 25:1 harmonic drives, just so the Mega can keep up.

@Macgyver001
Copy link
Author

Hey guys,

@Macgyver001 asked for a delay - there it is. But in reality i do not believe there is one needed.
Let' see if it helps.

I am sorry, but I am quite new in C, so Blue Marlin I don't know what I have to change where?
I downloaded the latest RC bugfix but I don't know where to find STEP_E_ONCE(INDEX). Unfortunately I don't have a oscilloscope else I would have checked it.
Advance and Lin_advance are both not extruding when one off them is enabled. including the use of CONFIG_STEPPERS_TOSHIBA to limit the frequency.

Thinkyhead, I have changed my drivers for m542T because they run smoother and I had some resonance problems. My CoreXY is quite stiff (ridgid) and I have better looking prints with higher mirco-stepping. If you like I can show the difference between lets say 16 and 128 (mirco stepping), I need some time for that? My latest Treefrog is really nice except some small artifacts I hope to resolve with Lin_advance, and the Lin_advance concept is way smarter than the normal implementation, that's why I like to enable Lin_advance.

@thinkyhead thinkyhead removed the Needs: Work More work is needed label Sep 20, 2016
@thinkyhead
Copy link
Member

thinkyhead commented Sep 20, 2016

#4852 is merged. Please confirm that it fixes the original issue.

To see if that's working more stable ( without the : MINIMUM_STEPPER_PULSE = 0 )

I finally tried MINIMUM_STEPPER_PULSE recently (in general) and it made the steppers sluggish. But in my case the machine has 100:1 harmonic gears on some axes, so probably it just can't afford any extra delay.

@thinkyhead
Copy link
Member

thinkyhead commented Sep 20, 2016

This is benchmark program

That is some very useful info. Perhaps it can be timed with all interrupts (temporarily) disabled, using TCNT0 (or another hardware counter) instead of millis to get completely accurate values.

@Sebastianv650
Copy link
Contributor

Great research!! I'm realy sad I still have no time to contribute in this topic (even my printer wasn't running the last months) but I definitly must so some tests with the merged PR :)

Thanks to everyone here 👍

@Macgyver001
Copy link
Author

Macgyver001 commented Sep 21, 2016

@thinkyhead is #4852 already landed/merged in the RCBugFix branch? I can't find it back in the stepper.cpp file?

@ghost
Copy link

ghost commented Sep 22, 2016

PR #4852 (Fix for advance extrusion algorithms) was merged to RCBugFix branch.

@thinkyhead
Copy link
Member

thinkyhead commented Sep 22, 2016

is #4852 already landed/merged…?

image

@thinkyhead
Copy link
Member

thinkyhead commented Sep 22, 2016

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
Copy link
Author

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
Copy link
Member

thinkyhead commented Sep 24, 2016

@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
Copy link
Author

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
Copy link
Contributor

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
Copy link
Contributor

mosh1 commented Sep 30, 2016

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

@ghost
Copy link

ghost commented Oct 1, 2016

@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
Copy link
Author

@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
Copy link
Member

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…

  • Stepper::isr(), stepping CPU requirements in typical printing situations.
  • Planner buffer_line to see how long it takes to make an average block
    • Can the planner fill in a block in advance of waiting for a free block?
    • (Apparently it can pre-process some before that wait)
  • LCD event handling: lcd_update() called from idle().
  • idle() and the things it calls: host_keepalive(), manage_inactivity(), Temperature::manage_heater(), buzzer.tick(), print_job_timer.tick().

Other performance-related questions:

  • Should the watchdog get reset more often, not just when a temperature reading is ready? Does the watchdog reset belong in a more generic place, such as the main idle() function?
  • Should the idle() function manage the time-slicing of the things it calls, instead of calling them only to get back a "not yet" most of the time?
  • Can we use "throttling" to free up CPU –for example reducing the frequency of the Temperature::isr and the LCD update– whenever time becomes constrained?
  • Display sleep? It can free up a lot of cycles to "sleep" the display.
  • Low Power LCD mode? Drawing between 1 - 3 lines of information…
    • 195°_Z:5.40_SD25%___
    • _70°_1d_01:11:19____
    • F_95%_Short_Message.

@thinkyhead
Copy link
Member

thinkyhead commented Oct 2, 2016

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
Copy link
Author

Macgyver001 commented Oct 6, 2016

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
Copy link
Member

thinkyhead commented Oct 9, 2016

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
Copy link
Contributor

boelle commented Nov 2, 2016

@thinkyhead still going to work on this one?

@thinkyhead
Copy link
Member

thinkyhead commented Nov 2, 2016

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

@Macgyver001
Copy link
Author

Macgyver001 commented Nov 7, 2016

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
Copy link
Contributor

boelle commented Nov 8, 2016

@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
Copy link
Contributor

boelle commented Nov 11, 2016

@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
Copy link
Member

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

@github-actions
Copy link

github-actions bot commented Apr 1, 2022

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.

@github-actions github-actions bot locked and limited conversation to collaborators Apr 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug: Potential ? Needs: Testing Testing is needed for this change
Projects
None yet
Development

No branches or pull requests

8 participants