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.27k stars 19.23k forks source link

Prints with short segments vibrate #2525

Closed shampine1 closed 8 years ago

shampine1 commented 9 years ago

When I print using the development branch (1938, 2474, and 2513), I find that all three of my printers vibrate like crazy. Marlin seems to either be stopping completely at the end of each segment, or some other non-smooth action. When I cut the acceleration and jerk settings way down it got significantly better, but in the release branch this sort of stuff was smooth. I'm suspicion that there is a problem with the look-ahead algorithm, but I don't know the underlying code...

#define DEFAULT_MAX_ACCELERATION      {750,750,100,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.

Here's the pertinent settings I'm using:

#define DEFAULT_ACCELERATION          750    // X, Y, Z and E acceleration in mm/s^2 for printing moves
#define DEFAULT_RETRACT_ACCELERATION  3000    // E acceleration in mm/s^2 for retracts
#define DEFAULT_TRAVEL_ACCELERATION   1000    // 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                10.0    // (mm/sec)
#define DEFAULT_ZJERK                 0.4     // (mm/sec)
#define DEFAULT_EJERK                 5.0    // (mm/sec)

These setting worked well on the released version:

#define DEFAULT_MAX_ACCELERATION      {1200,1200,100,10000}    // 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. {SD Patch}

#define DEFAULT_ACCELERATION          1000    // X, Y, Z and E max acceleration in mm/s^2 for printing moves {SD Patch}
#define DEFAULT_RETRACT_ACCELERATION  1000   // X, Y, Z and E max acceleration in mm/s^2 for retracts {SD Patch}
// The speed change that does not require acceleration (i.e. the software might assume it can be done instantaneously)
#define DEFAULT_XYJERK                20.0    // (mm/sec)
#define DEFAULT_ZJERK                 0.4     // (mm/sec)
#define DEFAULT_EJERK                 5.0    // (mm/sec)
fjsdc commented 9 years ago

@Wurstnase: i have read the issue you send me, and it appears to be the same problem. when i receive the ramps board i will try your solution of adding the code line OCR1A = (OCR1A < (TCNT1 +16)) ? (TCNT1 + 16) : OCR1A; and post the result.

thinkyhead commented 9 years ago

Any progress on this issue? Timing- and interrupt-related problems can be very tricky to suss out!

fjsdc commented 9 years ago

For my case, added the line "OCR1A = (OCR1A < (TCNT1 +16)) ? (TCNT1 + 16) : OCR1A;" was requested by Wurstnase and case solved... :)

thanks Wurstnase.

paulusjacobus commented 9 years ago

@Wurstnase can you explain to me what this code exactly does? It seems to be a step divider routine but I haven't digged into the stepper code yet so I am just very curious. Thx!

Wurstnase commented 9 years ago

I can't say why this happens, but I can say what happens.

TCNT1 is the counter of the ISR. The problem is that OCR1A is set in the end of the interrupt routine. So TCNT1 is counting upwards. Maybe allready at 5000. Just for this example. Now OCR1A is set to 4900. What will happen? The ISR won't trigger and run up to 65k.

What we do now? We look if TCNT1 is allready behind OCR1A and set OCR1A just some ticks behind it, so we can be sure that the ISR will trigger. And there we need some ticks to control this. Not time.

thinkyhead commented 9 years ago

I've submitted the patch as #138 so should this issue be closed?

jbrazio commented 8 years ago

Thank you for your interest making Marlin better and reporting this issue but this topic has been open for a long period of time without any further development. Marlin has been under heavy development for the past couple of months and moving to it's last mile to finish the RC cycle and release Marlin v1.1.0. We suggest you to try out the latest RCBugfix branch and reopening this issue if required.

lesaux commented 6 years ago

I see this vibration issue has appeared for me while upgrading from 1.1.0-rc5 to 1.1.8. I tried to keep settings as consistent as possible between firmwares. I will test other versions to see when this difference was implemented.

thinkyhead commented 6 years ago

@lesaux Try cutting your jerk values in half.

lesaux commented 6 years ago

@thinkyhead Ok thanks, I will try this.

lesaux commented 6 years ago

@thinkyhead unfortunately dividing jerk by two doesn't make a noticeable difference. I will try various versions in the upcoming days and report my findings.

thinkyhead commented 6 years ago

I will try various versions in the upcoming days and report my findings.

Great. I look forward to your results!

MiddleMan5 commented 6 years ago

Adding the following to stepper.h (1.1.9) in the aforementioned location seems to have fixed the stuttering issue that has plagued my PrintrBoard Rev F4

#ifdef STUTTER_FIX
    OCR1A = (OCR1A < (TCNT1 +16)) ? (TCNT1 + 16) : OCR1A;
#endif

What is the status of this issue?

thinkyhead commented 6 years ago

@ejtagle — Any comment on the proposed solution? I can see why it could work.

ejtagle commented 6 years ago

Basically, the proposed fix is incorrect. Line 1224 in Stepper.cpp does exactly that: Leaving some margin to avoid the described problem. Instead of adding the proposed fix, try to increase the last value (the '8' value)

min_ticks = HAL_timer_get_count(STEP_TIMER_NUM) + hal_timer_t((STEPPER_TIMER_TICKS_PER_US) * 8);

to idk, maybe 10...

min_ticks = HAL_timer_get_count(STEP_TIMER_NUM) + hal_timer_t((STEPPER_TIMER_TICKS_PER_US) * 10);

But the 8 value was the correct one, unless the compiler has changed and the ISR epilogue is longer ...

ejtagle commented 6 years ago

Stepper.cpp , lines 1216 to 1244 ensure exactly this same condition, but in a safer way, What i assume here is that the Stepper ISR is overloading the AVR. Maybe the maximum step rate for AVR should be decreased.

Now that i think, with short segments, we are probably running out of processing power. Adding that line forces to reduce the step rate, giving extra processing time to the planner- But that is NOT the proper way to do it -- That WILL ruin the acceleration profile, as it creates a sudden change of acceleration, as it is capping the maximum speed!

The proper fix is to decrease the maximum step rate, so it will allow the planner to create proper acceleration/deceleration profiles.

Yes, this seems (again) as AVR struggling to keep the planner running, and running out of processing power...

MiddleMan5 commented 6 years ago

I wasn't suggesting it was an actual solution, the opposite in fact. I was just interested in learning why it had an effect in the first place, when it seemed like the code was already handling the issue. It was one of those late-night "screw it" moments and it improved the behaviour of the printer.

I'll run some tests on min_ticks and see if I can narrow in on anything

thinkyhead commented 6 years ago

As a side note, I see we have a HAL function called HAL_timer_restrain that we never use. Is that now obsolete?

thinkyhead commented 6 years ago

Yes, this seems (again) as AVR struggling to keep the planner running, and running out of processing power...

In that case, simply reducing the feedrate slightly should eliminate the stutters. Try turning the knob to lower the feedrate @MiddleMan5 and see if it does in fact prevent stuttering.

ejtagle commented 6 years ago

Yes @thinkyhead : HAL_timer_restrain is obsolete.

Yes, reducing the feedrate should remove the stutters

AnHardt commented 6 years ago

It's not the printspeed what matters - it's the steprate. So for example a lower microstepping would have the same effect.

In general the answer is already given in the question (titel). Print larger segments. In small segments the overhead for planning a segments gets larger than its runtime - the buffer runs dry.

github-actions[bot] commented 4 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.

github-actions[bot] commented 4 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.

AnHardt commented 4 years ago

How many lock-messages will we get until a tread is really locked?

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.