From 07565d9016162fb829f0484d462c6ae87bd41615 Mon Sep 17 00:00:00 2001 From: David Buezas Date: Wed, 17 Dec 2025 03:05:55 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=9A=B8=20Safer=20FTMotion=20parameter=20e?= =?UTF-8?q?diting=20(#28191)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Scott Lahteine --- Marlin/src/HAL/GD32_MFL/timers.cpp | 4 - Marlin/src/HAL/GD32_MFL/timers.h | 9 ++- Marlin/src/HAL/STM32/timers.cpp | 4 - Marlin/src/HAL/STM32/timers.h | 17 +++-- Marlin/src/gcode/feature/ft_motion/M493.cpp | 85 ++++++--------------- Marlin/src/inc/Conditionals-5-post.h | 10 +-- Marlin/src/inc/SanityCheck.h | 2 +- Marlin/src/lcd/menu/menu_motion.cpp | 3 +- Marlin/src/module/ft_motion.cpp | 1 + Marlin/src/module/ft_motion.h | 74 +++++++++++++++++- Marlin/src/module/ft_motion/stepping.h | 12 +-- 11 files changed, 128 insertions(+), 93 deletions(-) diff --git a/Marlin/src/HAL/GD32_MFL/timers.cpp b/Marlin/src/HAL/GD32_MFL/timers.cpp index 13a5c166ef..3bcfdd464b 100644 --- a/Marlin/src/HAL/GD32_MFL/timers.cpp +++ b/Marlin/src/HAL/GD32_MFL/timers.cpp @@ -69,10 +69,6 @@ #endif #endif -#ifndef HAL_TIMER_RATE - #define HAL_TIMER_RATE GetStepperTimerClkFreq() -#endif - #ifndef STEP_TIMER #define STEP_TIMER MF_TIMER_STEP #endif diff --git a/Marlin/src/HAL/GD32_MFL/timers.h b/Marlin/src/HAL/GD32_MFL/timers.h index 1c69c0b468..8aff77aa96 100644 --- a/Marlin/src/HAL/GD32_MFL/timers.h +++ b/Marlin/src/HAL/GD32_MFL/timers.h @@ -37,15 +37,18 @@ typedef uint32_t hal_timer_t; #define HAL_TIMER_TYPE_MAX hal_timer_t(UINT16_MAX) -extern uint32_t GetStepperTimerClkFreq(); +#ifndef HAL_TIMER_RATE + extern uint32_t GetStepperTimerClkFreq(); + #define HAL_TIMER_RATE GetStepperTimerClkFreq() +#endif // Timer configuration constants #define STEPPER_TIMER_RATE 2000000 #define TEMP_TIMER_FREQUENCY 1000 // Timer prescaler calculations -#define STEPPER_TIMER_PRESCALE (GetStepperTimerClkFreq() / STEPPER_TIMER_RATE) // Prescaler = 30 -#define STEPPER_TIMER_TICKS_PER_US ((STEPPER_TIMER_RATE) / 1000000UL) // (MHz) Stepper Timer ticks per µs +#define STEPPER_TIMER_PRESCALE ((HAL_TIMER_RATE) / (STEPPER_TIMER_RATE)) // Prescaler = 30 +#define STEPPER_TIMER_TICKS_PER_US ((STEPPER_TIMER_RATE) / 1000000UL) // (MHz) Stepper Timer ticks per µs // Pulse Timer (counter) calculations #define PULSE_TIMER_RATE STEPPER_TIMER_RATE // (Hz) Frequency of Pulse Timer diff --git a/Marlin/src/HAL/STM32/timers.cpp b/Marlin/src/HAL/STM32/timers.cpp index b4271d3a58..5b58a915f0 100644 --- a/Marlin/src/HAL/STM32/timers.cpp +++ b/Marlin/src/HAL/STM32/timers.cpp @@ -81,10 +81,6 @@ #define MCU_TEMP_TIMER 14 // TIM7 is consumed by Software Serial if used. #endif -#ifndef HAL_TIMER_RATE - #define HAL_TIMER_RATE GetStepperTimerClkFreq() -#endif - #ifndef STEP_TIMER #define STEP_TIMER MCU_STEP_TIMER #endif diff --git a/Marlin/src/HAL/STM32/timers.h b/Marlin/src/HAL/STM32/timers.h index 7b55f8a52d..a6f09fd24e 100644 --- a/Marlin/src/HAL/STM32/timers.h +++ b/Marlin/src/HAL/STM32/timers.h @@ -48,13 +48,18 @@ #define TIMER_INDEX_(T) TIMER##T##_INDEX // TIMER#_INDEX enums (timer_index_t) depend on TIM#_BASE defines. #define TIMER_INDEX(T) TIMER_INDEX_(T) // Convert Timer ID to HardwareTimer_Handle index. -#define TEMP_TIMER_FREQUENCY 1000 // Temperature::isr() is expected to be called at around 1kHz +#ifndef HAL_TIMER_RATE + extern uint32_t GetStepperTimerClkFreq(); + #define HAL_TIMER_RATE GetStepperTimerClkFreq() +#endif -// TODO: get rid of manual rate/prescale/ticks/cycles taken for procedures in stepper.cpp -#define STEPPER_TIMER_RATE 2'000'000 // 2 Mhz -extern uint32_t GetStepperTimerClkFreq(); -#define STEPPER_TIMER_PRESCALE (GetStepperTimerClkFreq() / (STEPPER_TIMER_RATE)) -#define STEPPER_TIMER_TICKS_PER_US ((STEPPER_TIMER_RATE) / 1000000UL) // (MHz) Stepper Timer ticks per µs +// Timer configuration constants +#define STEPPER_TIMER_RATE 2000000 +#define TEMP_TIMER_FREQUENCY 1000 // Temperature::isr() should run at ~1kHz + +// Timer prescaler calculations +#define STEPPER_TIMER_PRESCALE ((HAL_TIMER_RATE) / (STEPPER_TIMER_RATE)) +#define STEPPER_TIMER_TICKS_PER_US ((STEPPER_TIMER_RATE) / 1000000UL) // (MHz) Stepper Timer ticks per µs // Pulse Timer (counter) calculations #define PULSE_TIMER_RATE STEPPER_TIMER_RATE // (Hz) Frequency of Pulse Timer diff --git a/Marlin/src/gcode/feature/ft_motion/M493.cpp b/Marlin/src/gcode/feature/ft_motion/M493.cpp index f38bbf9ca1..25ab2460f3 100644 --- a/Marlin/src/gcode/feature/ft_motion/M493.cpp +++ b/Marlin/src/gcode/feature/ft_motion/M493.cpp @@ -236,10 +236,8 @@ void GcodeSuite::M493() { return; } auto set_shaper = [&](const AxisEnum axis, ftMotionShaper_t newsh) { - if (newsh != c.shaper[axis]) { - c.shaper[axis] = newsh; + if (c.setShaper(axis, newsh)) flag.update = flag.report = true; - } }; if (seenC) { #define _SET_SHAPER(A) set_shaper(_AXIS(A), shaperVal); @@ -248,14 +246,9 @@ void GcodeSuite::M493() { #endif // NUM_AXES_SHAPED > 0 - // Parse 'H' Axis Synchronization parameter. - if (parser.seenval('H')) { - const bool enabled = parser.value_bool(); - if (enabled != c.axis_sync_enabled) { - c.axis_sync_enabled = enabled; - flag.report = true; - } - } + // Parse bool 'H' Axis Synchronization parameter. + if (parser.seen('H') && c.setAxisSync(parser.value_bool())) + flag.report = true; #if HAS_DYNAMIC_FREQ @@ -317,10 +310,8 @@ void GcodeSuite::M493() { if (seenA) { if (AXIS_IS_SHAPING(X)) { // TODO: Frequency minimum is dependent on the shaper used; the above check isn't always correct. - if (goodBaseFreq) { - c.baseFreq.x = baseFreqVal; + if (goodBaseFreq && c.setBaseFreq(X_AXIS, baseFreqVal)) flag.update = flag.report = true; - } } else // Mode doesn't use frequency. SERIAL_ECHOLNPGM("?Wrong mode for ", C(STEPPER_A_NAME), " (A) frequency."); @@ -328,19 +319,15 @@ void GcodeSuite::M493() { #if HAS_DYNAMIC_FREQ // Parse X frequency scaling parameter - if (seenF && modeUsesDynFreq) { - c.dynFreqK.x = baseDynFreqVal; + if (seenF && c.setDynFreqK(X_AXIS, baseDynFreqVal)) flag.report = true; - } #endif // Parse X zeta parameter if (seenI) { if (AXIS_IS_SHAPING(X)) { - if (goodZeta) { - c.zeta.x = zetaVal; + if (goodZeta && c.setZeta(X_AXIS, zetaVal)) flag.update = true; - } } else SERIAL_ECHOLNPGM("?Wrong mode for ", C(STEPPER_A_NAME), " (I) zeta parameter."); @@ -350,10 +337,8 @@ void GcodeSuite::M493() { // Parse X vtol parameter if (seenQ) { if (AXIS_IS_EISHAPING(X)) { - if (goodVtol) { - c.vtol.x = vtolVal; + if (goodVtol && c.setVtol(X_AXIS, vtolVal)) flag.update = true; - } } else SERIAL_ECHOLNPGM("?Wrong mode for ", C(STEPPER_A_NAME), " (Q) vtol parameter."); @@ -370,10 +355,8 @@ void GcodeSuite::M493() { // Parse Y frequency parameter if (seenA) { if (AXIS_IS_SHAPING(Y)) { - if (goodBaseFreq) { - c.baseFreq.y = baseFreqVal; + if (goodBaseFreq && c.setBaseFreq(Y_AXIS, baseFreqVal)) flag.update = flag.report = true; - } } else // Mode doesn't use frequency. SERIAL_ECHOLNPGM("?Wrong mode for ", C(STEPPER_B_NAME), " (A) frequency."); @@ -381,32 +364,26 @@ void GcodeSuite::M493() { #if HAS_DYNAMIC_FREQ // Parse Y frequency scaling parameter - if (seenF && modeUsesDynFreq) { - c.dynFreqK.y = baseDynFreqVal; + if (seenF && c.setDynFreqK(Y_AXIS, baseDynFreqVal)) flag.report = true; - } #endif // Parse Y zeta parameter if (seenI) { if (AXIS_IS_SHAPING(Y)) { - if (goodZeta) { - c.zeta.y = zetaVal; + if (goodZeta && c.setZeta(Y_AXIS, zetaVal)) flag.update = true; - } } else SERIAL_ECHOLNPGM("?Wrong mode for ", C(STEPPER_B_NAME), " (I) zeta parameter."); } - // Parse Y vtol parameter #if HAS_FTM_EI_SHAPING + // Parse Y vtol parameter if (seenQ) { if (AXIS_IS_EISHAPING(Y)) { - if (goodVtol) { - c.vtol.y = vtolVal; + if (goodVtol && c.setVtol(Y_AXIS, vtolVal)) flag.update = true; - } } else SERIAL_ECHOLNPGM("?Wrong mode for ", C(STEPPER_B_NAME), " (Q) vtol parameter."); @@ -423,10 +400,8 @@ void GcodeSuite::M493() { // Parse Z frequency parameter if (seenA) { if (AXIS_IS_SHAPING(Z)) { - if (goodBaseFreq) { - c.baseFreq.z = baseFreqVal; + if (goodBaseFreq && c.setBaseFreq(Z_AXIS, baseFreqVal)) flag.update = flag.report = true; - } } else // Mode doesn't use frequency. SERIAL_ECHOLNPGM("?Wrong mode for ", C(STEPPER_C_NAME), " (A) frequency."); @@ -434,32 +409,26 @@ void GcodeSuite::M493() { #if HAS_DYNAMIC_FREQ // Parse Z frequency scaling parameter - if (seenF && modeUsesDynFreq) { - c.dynFreqK.z = baseDynFreqVal; + if (seenF && c.setDynFreqK(Z_AXIS, baseDynFreqVal)) flag.report = true; - } #endif // Parse Z zeta parameter if (seenI) { if (AXIS_IS_SHAPING(Z)) { - if (goodZeta) { - c.zeta.z = zetaVal; + if (goodZeta && c.setZeta(Z_AXIS, zetaVal)) flag.update = true; - } } else SERIAL_ECHOLNPGM("?Wrong mode for ", C(STEPPER_C_NAME), " (I) zeta parameter."); } - // Parse Z vtol parameter #if HAS_FTM_EI_SHAPING + // Parse Z vtol parameter if (seenQ) { if (AXIS_IS_EISHAPING(Z)) { - if (goodVtol) { - c.vtol.z = vtolVal; + if (goodVtol && c.setVtol(Z_AXIS, vtolVal)) flag.update = true; - } } else SERIAL_ECHOLNPGM("?Wrong mode for ", C(STEPPER_C_NAME), " (Q) vtol parameter."); @@ -476,10 +445,8 @@ void GcodeSuite::M493() { // Parse E frequency parameter if (seenA) { if (AXIS_IS_SHAPING(E)) { - if (goodBaseFreq) { - c.baseFreq.e = baseFreqVal; + if (goodBaseFreq && c.setBaseFreq(E_AXIS, baseFreqVal)) flag.update = flag.report = true; - } } else // Mode doesn't use frequency. SERIAL_ECHOLNPGM("?Wrong mode for ", C('E'), " (A) frequency."); @@ -487,32 +454,26 @@ void GcodeSuite::M493() { #if HAS_DYNAMIC_FREQ // Parse E frequency scaling parameter - if (seenF && modeUsesDynFreq) { - c.dynFreqK.e = baseDynFreqVal; + if (seenF && c.setDynFreqK(E_AXIS, baseDynFreqVal)) flag.report = true; - } #endif // Parse E zeta parameter if (seenI) { if (AXIS_IS_SHAPING(E)) { - if (goodZeta) { - c.zeta.e = zetaVal; + if (goodZeta && c.setZeta(E_AXIS, zetaVal)) flag.update = true; - } } else SERIAL_ECHOLNPGM("?Wrong mode for ", C('E'), " (I) zeta parameter."); } - // Parse E vtol parameter #if HAS_FTM_EI_SHAPING + // Parse E vtol parameter if (seenQ) { if (AXIS_IS_EISHAPING(E)) { - if (goodVtol) { - c.vtol.e = vtolVal; + if (goodVtol && c.setVtol(E_AXIS, vtolVal)) flag.update = true; - } } else SERIAL_ECHOLNPGM("?Wrong mode for ", C('E'), " (Q) vtol parameter."); diff --git a/Marlin/src/inc/Conditionals-5-post.h b/Marlin/src/inc/Conditionals-5-post.h index fb2a6865f2..024eb42f51 100644 --- a/Marlin/src/inc/Conditionals-5-post.h +++ b/Marlin/src/inc/Conditionals-5-post.h @@ -3700,11 +3700,9 @@ #ifndef FTM_BUFFER_SIZE #define FTM_BUFFER_SIZE 128 #endif - #define FTM_BUFFER_MASK (FTM_BUFFER_SIZE - 1u) - #if ANY(BIQU_MICROPROBE_V1, BIQU_MICROPROBE_V2) - #ifndef PROBE_WAKEUP_TIME_MS - #define PROBE_WAKEUP_TIME_MS 30 - #define PROBE_WAKEUP_TIME_WARNING 1 - #endif + + #if ANY(BIQU_MICROPROBE_V1, BIQU_MICROPROBE_V2) && !defined(PROBE_WAKEUP_TIME_MS) + #define PROBE_WAKEUP_TIME_MS 30 + #define PROBE_WAKEUP_TIME_WARNING 1 #endif #endif diff --git a/Marlin/src/inc/SanityCheck.h b/Marlin/src/inc/SanityCheck.h index 618abadae1..5856723c77 100644 --- a/Marlin/src/inc/SanityCheck.h +++ b/Marlin/src/inc/SanityCheck.h @@ -4472,7 +4472,7 @@ static_assert(_PLUS_TEST(3), "DEFAULT_MAX_ACCELERATION values must be positive." * Fixed-Time Motion limitations */ #if ENABLED(FT_MOTION) - static_assert(FTM_BUFFER_SIZE >= 4 && (FTM_BUFFER_SIZE & FTM_BUFFER_MASK) == 0, "FTM_BUFFER_SIZE must be a power of two (128, 256, 512, ...)."); + static_assert(FTM_BUFFER_SIZE >= 4 && (FTM_BUFFER_SIZE & (FTM_BUFFER_SIZE - 1u)) == 0, "FTM_BUFFER_SIZE must be a power of two (128, 256, 512, ...)."); #if ENABLED(MIXING_EXTRUDER) #error "FT_MOTION does not currently support MIXING_EXTRUDER." #endif diff --git a/Marlin/src/lcd/menu/menu_motion.cpp b/Marlin/src/lcd/menu/menu_motion.cpp index 4c7dea00f0..0c21fdd6aa 100644 --- a/Marlin/src/lcd/menu/menu_motion.cpp +++ b/Marlin/src/lcd/menu/menu_motion.cpp @@ -517,7 +517,8 @@ void menu_move() { CARTES_MAP(_FTM_AXIS_SUBMENU); - EDIT_ITEM(bool, MSG_FTM_AXIS_SYNC, &c.axis_sync_enabled); + editable.state = c.axis_sync_enabled; + EDIT_ITEM(bool, MSG_FTM_AXIS_SYNC, &editable.state, []{ c.setAxisSync(editable.state); }); #if ENABLED(FTM_RESONANCE_TEST) SUBMENU(MSG_FTM_RESONANCE_TEST, menu_ftm_resonance_test); diff --git a/Marlin/src/module/ft_motion.cpp b/Marlin/src/module/ft_motion.cpp index 2c58119b97..c8db197655 100644 --- a/Marlin/src/module/ft_motion.cpp +++ b/Marlin/src/module/ft_motion.cpp @@ -214,6 +214,7 @@ void FTMotion::loop() { bool FTMotion::set_smoothing_time(const AxisEnum axis, const float s_time) { if (!WITHIN(s_time, 0.0f, FTM_MAX_SMOOTHING_TIME)) return false; + planner.synchronize(); cfg.smoothingTime[axis] = s_time; update_smoothing_params(); return true; diff --git a/Marlin/src/module/ft_motion.h b/Marlin/src/module/ft_motion.h index ad1417fd44..88a9a49e4d 100644 --- a/Marlin/src/module/ft_motion.h +++ b/Marlin/src/module/ft_motion.h @@ -102,15 +102,51 @@ typedef struct FTConfig { bool setActive(const bool a) { if (a == active) return false; stepper.ftMotion_syncPosition(); + planner.synchronize(); active = a; return true; } #endif + bool setAxisSync(const bool ena) { + if (ena == axis_sync_enabled) return false; + planner.synchronize(); + axis_sync_enabled = ena; + return true; + } + #if HAS_FTM_SHAPING + bool setShaper(const AxisEnum a, const ftMotionShaper_t s) { + if (s == shaper[a]) return false; + planner.synchronize(); + shaper[a] = s; + return true; + } + constexpr bool goodZeta(const float z) { return WITHIN(z, 0.01f, ftm_max_dampening); } - constexpr bool goodVtol(const float v) { return WITHIN(v, 0.00f, 1.0f); } + + bool setZeta(const AxisEnum a, const float z) { + if (z == zeta[a]) return false; + if (!goodZeta(z)) return false; + planner.synchronize(); + zeta[a] = z; + return true; + } + + #if HAS_FTM_EI_SHAPING + + constexpr bool goodVtol(const float v) { return WITHIN(v, 0.00f, 1.0f); } + + bool setVtol(const AxisEnum a, const float v) { + if (v == vtol[a]) return false; + if (!goodVtol(v)) return false; + planner.synchronize(); + vtol[a] = v; + return true; + } + + #endif #if HAS_DYNAMIC_FREQ @@ -133,12 +169,28 @@ typedef struct FTConfig { || TERN0(HAS_DYNAMIC_FREQ_G, dynFreqMode == dynFreqMode_MASS_BASED)); } + bool setDynFreqK(const AxisEnum a, const float k) { + if (!modeUsesDynFreq()) return false; + if (k == dynFreqK[a]) return false; + planner.synchronize(); + dynFreqK[a] = k; + return true; + } + #endif // HAS_DYNAMIC_FREQ #endif // HAS_FTM_SHAPING constexpr bool goodBaseFreq(const float f) { return WITHIN(f, FTM_MIN_SHAPE_FREQ, (FTM_FS) / 2); } + bool setBaseFreq(const AxisEnum a, const float f) { + if (f == baseFreq[a]) return false; + if (!goodBaseFreq(a)) return false; + planner.synchronize(); + baseFreq[a] = f; + return true; + } + void set_defaults() { #if HAS_STANDARD_MOTION active = ENABLED(FTM_IS_DEFAULT_MOTION); @@ -234,6 +286,26 @@ class FTMotion { } #endif + // Setters for baseFreq, zeta, vtol + static bool setBaseFreq(const AxisEnum a, const float f) { + if (!cfg.setBaseFreq(a, f)) return false; + update_shaping_params(); + return true; + } + static bool setZeta(const AxisEnum a, const float z) { + if (!cfg.setZeta(a, z)) return false; + update_shaping_params(); + return true; + } + + #if HAS_FTM_EI_SHAPING + static bool setVtol(const AxisEnum a, const float v) { + if (!cfg.setVtol(a, v)) return false; + update_shaping_params(); + return true; + } + #endif + // Trajectory generator selection #if ENABLED(FTM_POLYS) static void setTrajectoryType(const TrajectoryType type); diff --git a/Marlin/src/module/ft_motion/stepping.h b/Marlin/src/module/ft_motion/stepping.h index ca814a81be..ff72d90be1 100644 --- a/Marlin/src/module/ft_motion/stepping.h +++ b/Marlin/src/module/ft_motion/stepping.h @@ -41,11 +41,11 @@ constexpr uint32_t FTM_Q_INT = 32u - TICKS_BITS; // Bits remaining constexpr uint32_t FTM_Q = 16u - FTM_Q_INT; // uint16 interval fractional bits. // Intervals buffer has fixed point numbers with the point on this position -static_assert(FRAME_TICKS < FTM_NEVER, "(STEPPER_TIMER_RATE / FTM_FS) must be < " STRINGIFY(FTM_NEVER) " to fit 16-bit fixed-point numbers."); -static_assert(FRAME_TICKS != 2000 || FTM_Q_INT == 11, "FTM_Q_INT should be 11"); -static_assert(FRAME_TICKS != 2000 || FTM_Q == 5, "FTM_Q should be 5"); -static_assert(FRAME_TICKS != 25000 || FTM_Q_INT == 15, "FTM_Q_INT should be 15"); -static_assert(FRAME_TICKS != 25000 || FTM_Q == 1, "FTM_Q should be 1"); +static_assert(FRAME_TICKS < FTM_NEVER, "(STEPPER_TIMER_RATE / FTM_FS) (" STRINGIFY(STEPPER_TIMER_RATE) " / " STRINGIFY(FTM_FS) ") must be < " STRINGIFY(FTM_NEVER) " to fit 16-bit fixed-point numbers."); + +// Sanity check +static_assert(FRAME_TICKS != 2000 || FTM_Q == 5, "FTM_Q should be 5"); +static_assert(FRAME_TICKS != 25000 || FTM_Q == 1, "FTM_Q should be 1"); // The _FP and _fp suffixes mean the number is in fixed point format with the point at the FTM_Q position. // See: https://en.wikipedia.org/wiki/Fixed-point_arithmetic @@ -154,6 +154,8 @@ typedef struct Stepping { // Buffering part // + #define FTM_BUFFER_MASK (FTM_BUFFER_SIZE - 1u) + stepper_plan_t stepper_plan_buff[FTM_BUFFER_SIZE]; uint32_t stepper_plan_tail = 0, stepper_plan_head = 0; XYZEval curr_steps_q48_16{0};