[FR] Add a switch to M593 to disable input shaping

[tombrazier]

Is your feature request related to a problem? Please describe.

@InsanityAutomation has suggested an extension to M593 to disable input shaping. Whilst input shaping is effectively has no effect when damping ratio is equal to 1, all the logic is still being called and the generated pulse train is ever so slightly different. I think being able to disable the feature completely is worthwhile.

Are you looking for hardware support?

No response

Describe the feature you want

Here is what I think is needed for this feature:

• A boolean in class Stepper.
• A new flag for M593.
• An if statement around

  Marlin/Marlin/src/module/planner.cpp

  Lines 2487 to 2491 in 6b4d7b9

  	const float top_freq = _MIN(float(0x7FFFFFFFL)
  	OPTARG(HAS_SHAPING_X, stepper.get_shaping_frequency(X_AXIS))
  	OPTARG(HAS_SHAPING_Y, stepper.get_shaping_frequency(Y_AXIS))),
  	max_factor = (top_freq * float(shaping_dividends - 3) * 2.0f) / block->nominal_rate;
  	NOMORE(speed_factor, max_factor);

• An if statement around

  Marlin/Marlin/src/module/stepper.cpp

  Lines 1639 to 1643 in 6b4d7b9

  	#if ENABLED(INPUT_SHAPING)
  	shaping_dividend_queue.purge();
  	shaping_queue.purge();
  	TERN_(HAS_SHAPING_X, delta_error.x = 0);
  	TERN_(HAS_SHAPING_Y, delta_error.y = 0);

• An if statement around

  Marlin/Marlin/src/module/stepper.cpp

  Line 1822 in 6b4d7b9

  TERN_(INPUT_SHAPING, shaping_queue.enqueue());

• An if statement around

  Marlin/Marlin/src/module/stepper.cpp

  Lines 2497 to 2530 in 6b4d7b9

  	TERN_(HAS_SHAPING_X, advance_dividend.x = (uint64_t(current_block->steps.x) << 29) / step_event_count);
  	TERN_(HAS_SHAPING_X, if (TEST(current_block->direction_bits, X_AXIS)) advance_dividend.x *= -1);
  	TERN_(HAS_SHAPING_X, if (!shaping_queue.empty_x()) SET_BIT_TO(current_block->direction_bits, X_AXIS, TEST(last_direction_bits, X_AXIS)));
  	TERN_(HAS_SHAPING_Y, advance_dividend.y = (uint64_t(current_block->steps.y) << 29) / step_event_count);
  	TERN_(HAS_SHAPING_Y, if (TEST(current_block->direction_bits, Y_AXIS)) advance_dividend.y *= -1);
  	TERN_(HAS_SHAPING_Y, if (!shaping_queue.empty_y()) SET_BIT_TO(current_block->direction_bits, Y_AXIS, TEST(last_direction_bits, Y_AXIS)));
  	// The scaling operation above introduces rounding errors which must now be removed.
  	// For this segment, there will be step_event_count calls to the Bresenham logic and the same number of echoes.
  	// For each pair of calls to the Bresenham logic, delta_error will increase by advance_dividend modulo 0x20000000
  	// so (e.g. for x) delta_error.x will end up changing by (advance_dividend.x * step_event_count) % 0x20000000.
  	// For a divisor which is a power of 2, modulo is the same as as a bitmask, i.e.
  	// (advance_dividend.x * step_event_count) & 0x1FFFFFFF.
  	// This segment's final change in delta_error should actually be zero so we need to increase delta_error by
  	// 0 - ((advance_dividend.x * step_event_count) & 0x1FFFFFFF)
  	// And this needs to be adjusted to the range -0x10000000 to 0x10000000.
  	// Adding and subtracting 0x10000000 inside the outside the modulo achieves this.
  	TERN_(HAS_SHAPING_X, delta_error.x = old_delta_error_x + 0x10000000L - ((0x10000000L + advance_dividend.x * step_event_count) & 0x1FFFFFFFUL));
  	TERN_(HAS_SHAPING_Y, delta_error.y = old_delta_error_y + 0x10000000L - ((0x10000000L + advance_dividend.y * step_event_count) & 0x1FFFFFFFUL));
  	// when there is damping, the signal and its echo have different amplitudes
  	#if ENABLED(HAS_SHAPING_X)
  	const int32_t echo_x = shaping_x.factor * (advance_dividend.x >> 7);
  	#endif
  	#if ENABLED(HAS_SHAPING_Y)
  	const int32_t echo_y = shaping_y.factor * (advance_dividend.y >> 7);
  	#endif
  	// plan the change of values for advance_dividend for the input shaping echoes
  	TERN_(INPUT_SHAPING, shaping_dividend_queue.enqueue(TERN0(HAS_SHAPING_X, echo_x), TERN0(HAS_SHAPING_Y, echo_y)));
  	// apply the adjustment to the primary signal
  	TERN_(HAS_SHAPING_X, advance_dividend.x -= echo_x);
  	TERN_(HAS_SHAPING_Y, advance_dividend.y -= echo_y);

Additionally a few CPU cycles could be saved with if statements around other code that is conditional on INPUT_SHAPING or HAS_SHAPING_X or HAS_SHAPING_Y.

Additional context

No response

[The-EG]

Instead of adding a new parameter to M593, disabling an axis could be accomplished by specifying a frequency of 0. This is the existing behavior for RRF.
It may help to be consistent, I think, or would that be confusing for some reason?

[InsanityAutomation]

I'd agree if 0 should be effectively off, instead of comparing the bool we could compare for value over 0.

[tombrazier]

That would save the space of a bool. It would also mean axes can be switched off independently. The code changes would be a bit different to what I said above. Actually, that would be simple enough that I can throw a PR together really quickly.

[tombrazier]

New branch https://github.com/tombrazier/Marlin/tree/is_improvements has freq = 0 to switch off input shaping. I'll add bug fixes for TMC2208 here too hopefully.

[gudvinr]

This probably should be closed since PR was merged

[github-actions]

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.