classicrocker883 / MRiscoCProUI

This is optimized firmware for Voxelab Aquila & Ender3 V2/S1 3D printers.
https://classicrocker883.github.io/
Other
89 stars 17 forks source link

On the March branch enabling Input shaping makes steppers noisy #27

Closed oloendithas closed 9 months ago

oloendithas commented 1 year ago

On the March branch enabling Input shaping makes steppers noisy

Steps to reproduce the behavior:

  1. Copy Configuration Pro.h and Configuration_adv Pro.h from /configurations/Voxelab Aquila/UBL to /Marlin and rename properly
  2. Uncomment Input Shaping X/Y options
  3. Build/Flash (had to disable GCode preview and toolbar options to fit 227kB)
  4. Move X/Y axis on higher feedrate (e.g. G0 X70 F16000)
  5. Hear the noise, as if pulley teethes are skipping the belt.

Version (please complete the following information): Running Aquila S2 N32 (3D Touch)

Probe works fine. February branch build with the same options doesn't suffer from this issue.

commit.patch

oloendithas commented 1 year ago

Oh, I will gladly test this next week and report here, how it goes.

tombrazier commented 1 year ago

would this be the comment you are referring to?

Oh, yes, that will be it. So I take it your Marlin issue request is in response to this issue request.

The behaviour will be very dependent on mainboard and will also only happen at certain speeds. I look forward to seeing what @oloendithas finds.

thinkyhead commented 1 year ago

The behaviour will be very dependent on mainboard and will also only happen at certain speeds. I look forward to seeing what @oloendithas finds.

I notice that in both the old and new adaptive code we always use doubling and halving. I wonder if we could get more fluid behavior by incrementing and decrementing instead. This would add more complexity, and with 1x and 2x it would still amount to doubling and halving, but as the step rate starts to increase to 3x, 4x, and up it could start to make a difference.

There's also a hanging question about the way the regular stepper pulse phase is separated from the ZV shaper phase, such that it can generate up to double the amount of pulse delay time applied in a single call to the ISR. This tends to have more of an effect on time allocated to the main loop, but it might also be leading to irregular timing of ISR invocations. If we could combine both the regular and the shaping pulse phases in a single block that might help to optimize the amount of time spent in any single ISR invocation, and it should also lead to more accurate timing of shaping pulses. That would mean moving the shaping_isr code inside of the do loop in pulse_phase_isr and interleaving shaping steps with every multi-step.

Outside of that more disruptive change, one minor optimization we could make would be to track the last pulse start time for each axis in a global structure, and then we could avoid adding unnecessary delays to any stepper in all procedures that use stepping. A straightforward way to do that would be to make a stateful AxisStepper class to represent all the stepper motors on a single axis, then refactor all code to use AxisStepper objects instead of going directly to the STEP/DIR/ENA pins. We could then produce an optimal model for each type of stepper driver and only apply the minimum delay required for each individual stepping event. If the added complexity is paid for with less time spent in the ISR it might be worth the extra calculations. … Something to contemplate for the next revision.

classicrocker883 commented 1 year ago

I just came across in Configuration_adv.h

// Microstep settings (Requires a board with pins named X_MS1, X_MS2, etc.)
#define MICROSTEP_MODES { 16, 16, 16, 16, 16, 16 } // [1,2,4,8,16]

is this something that needs to be defined? I havent noticed the pins for Creality_V4 boards mention X_MS# so is this parameter necessary?

oloendithas commented 1 year ago

It would be easy enough to verify whether this is the cause. Change the code at the start of function Stepper::block_phase_isr() so that reducing multistepping is less responsive. Do this by playing with the condition time_spent_out_isr >= time_spent_in_isr + time_spent. e.g. something like:

So I've played with it. Tried:

if (steps_per_isr > 1 && time_spent_out_isr >= (time_spent_in_isr + time_spent) * **2**) {
if (steps_per_isr > 1 && time_spent_out_isr >= (time_spent_in_isr + time_spent) * **4**) {
if (steps_per_isr > 1 && time_spent_out_isr >= (time_spent_in_isr + time_spent) * **8**) {

That didn't changed anything.

Then I tried to play with this part:

    const hal_timer_t time_spent = HAL_timer_get_count(MF_TIMER_STEP);
    time_spent_in_isr += time_spent;

    if (next_isr_ticks < min_ticks) {
      next_isr_ticks = min_ticks;

      // When forced out of the ISR, increase multi-stepping
      #if MULTISTEPPING_LIMIT > 1
        if (steps_per_isr < MULTISTEPPING_LIMIT) {
          steps_per_isr <<= 1;
          // ticks_nominal will need to be recalculated if we are in cruise phase
          ticks_nominal = 0;
        }
      #endif
    }
    else {
      // Track the time spent voluntarily outside the ISR
      time_spent_out_isr += next_isr_ticks;
      time_spent_out_isr -= time_spent;
    }

Tried to set if (next_isr_ticks < min_ticks / 1.5) and this does change something: head moved slow with micro stuttering. I have no idea, what conclusions can I make out of this. Hope this all means some thing to you.

tombrazier commented 1 year ago

@oloendithas I have another idea about this. I observed last week that just multistepping by iteself can make motors sound really noisy. May be the issue is not that the printner has been flipping between multistepping levels. Maybe it is just that it is going to a level that is noisy for your printer.

Could you try disabling the logic that reduces multistepping, i.e. comment out steps_per_isr >>= 1; in

  #if DISABLED(OLD_ADAPTIVE_MULTISTEPPING)
    // If the ISR uses < 50% of MPU time, halve multi-stepping
    const hal_timer_t time_spent = HAL_timer_get_count(MF_TIMER_STEP);
    #if MULTISTEPPING_LIMIT > 1
      if (steps_per_isr > 1 && time_spent_out_isr >= 2 * (time_spent_in_isr + time_spent)) {
        steps_per_isr >>= 1;
        // ticks_nominal will need to be recalculated if we are in cruise phase
        ticks_nominal = 0;
      }
    #endif
    time_spent_in_isr = -time_spent;    // unsigned but guaranteed to be +ve when needed
    time_spent_out_isr = 0;
  #endif

And then try different values for MULTISTEPPING_LIMIT to see whether there is some value that causes the noise.

GenXEngineer commented 1 year ago

Had noticed the increase in motor noise as well - it was nearly like going back to pre silent driver days. Even the probe on my CR touch was rattling just making the move to probe at 100mm/s. Tried all the fixes above but none made anything better. Made sure belts weren't too tight. They can certainly cause nasty vibrations when too tight. The vibrations were actually strong enough to show on the outer skin of prints. Bummer.

I'm using SKR mini e3v3 on upgraded Creality enders. I noticed that in the TMC driver submenu that motor driver step mode and max current can be adjusted. Played around with those. Made sure stealth chop was enabled. Creality motors max amp ratings are shown in the clip below. The nightly bug fix BTT config go by has all the motors set to 580ma. I played around with those numbers for each axis and found that (naturally) they have a dramatic effect on motor noise - especially when set too low. Anyhoo in the end I chose 780ma for all motors as that was FAR and away the quietest. Now everything is running silent again.

Please let me know if I did anything stupid! :) Motors are all running cool. Oh and on edit - I did set Multistep to 8 (tried all the way down to 1) but couldn't really tell a difference.

creal

@tombrazier

classicrocker883 commented 1 year ago

I think that is the right idea. Increasing current is like (with Stock Aquila/Creality boards) changing the Voltage Reference. the downside with too little voltage/current is skipped steps - and from the sound of it - more motor noise.
the downside from too much voltage/current is running too hot, burn out the motor.

I really think the 42-34 stepper motors are much too undersized even for these printers. maybe not for the Z (with dual Z is fine), but especially for the Y axis... because this is the axis that moves the most Mass around. so naturally that calls for a larger motor.

I maybe said this before but I highly suggest anyone with these printers to upgrade their motors. stepperonline has a great deal - 3 motors for ~$20 bucks, Model: 3-17HS15-1504S-X1 42-40 motors like that of the Extruder. (technically they are 42-39).

anyway, say you want to upgrade to a direct drive... having a bigger motor will help the hotend having more weight, so you can use the same extruder motor and not worry with added weight. plus the shafts are a bit longer so you can add those nema 17 vibration dampers

github-actions[bot] commented 10 months ago

This issue has had no activity in the last 60 days. Please add a reply if you want to keep this issue active, otherwise it will be automatically closed within 10 days.

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