Step timing cleanup and rounding fix (#16258)
This commit is contained in:
parent
f8065da48f
commit
d5bc5547ee
|
@ -338,6 +338,13 @@ xyze_int8_t Stepper::count_direction{0};
|
|||
#if DISABLED(MIXING_EXTRUDER)
|
||||
#define E_APPLY_STEP(v,Q) E_STEP_WRITE(stepper_extruder, v)
|
||||
#endif
|
||||
|
||||
#define CYCLES_TO_NS(CYC) (1000UL * (CYC) / ((F_CPU) / 1000000))
|
||||
constexpr uint32_t NS_PER_PULSE_TIMER_TICK = 1000000000UL / (STEPPER_TIMER_RATE);
|
||||
|
||||
// Round up when converting from ns to timer ticks
|
||||
constexpr uint32_t NS_TO_PULSE_TIMER_TICKS(uint32_t NS) { return (NS + (NS_PER_PULSE_TIMER_TICK) / 2) / (NS_PER_PULSE_TIMER_TICK); }
|
||||
|
||||
#define TIMER_SETUP_NS (CYCLES_TO_NS(TIMER_READ_ADD_AND_STORE_CYCLES))
|
||||
|
||||
#define PULSE_HIGH_TICK_COUNT hal_timer_t(NS_TO_PULSE_TIMER_TICKS(_MIN_PULSE_HIGH_NS - _MIN(_MIN_PULSE_HIGH_NS, TIMER_SETUP_NS)))
|
||||
|
@ -1430,7 +1437,7 @@ void Stepper::stepper_pulse_phase_isr() {
|
|||
// Take multiple steps per interrupt (For high speed moves)
|
||||
bool firstStep = true;
|
||||
xyze_bool_t step_needed{0};
|
||||
hal_timer_t end_tick_count;
|
||||
hal_timer_t end_tick_count = 0;
|
||||
|
||||
do {
|
||||
#define _APPLY_STEP(AXIS) AXIS ##_APPLY_STEP
|
||||
|
@ -1941,7 +1948,7 @@ uint32_t Stepper::stepper_block_phase_isr() {
|
|||
|
||||
// Step E stepper if we have steps
|
||||
bool firstStep = true;
|
||||
hal_timer_t end_tick_count;
|
||||
hal_timer_t end_tick_count = 0;
|
||||
|
||||
while (LA_steps) {
|
||||
#if (MINIMUM_STEPPER_PULSE || MAXIMUM_STEPPER_RATE) && DISABLED(I2S_STEPPER_STREAM)
|
||||
|
|
|
@ -56,7 +56,23 @@
|
|||
// Estimate the amount of time the Stepper ISR will take to execute
|
||||
//
|
||||
|
||||
/**
|
||||
* The method of calculating these cycle-constants is unclear.
|
||||
* Most of them are no longer used directly for pulse timing, and exist
|
||||
* only to estimate a maximum step rate based on the user's configuration.
|
||||
* As 32-bit processors continue to diverge, maintaining cycle counts
|
||||
* will become increasingly difficult and error-prone.
|
||||
*/
|
||||
|
||||
#ifdef CPU_32_BIT
|
||||
/**
|
||||
* Duration of START_TIMED_PULSE
|
||||
*
|
||||
* ...as measured on an LPC1768 with a scope and converted to cycles.
|
||||
* Not applicable to other 32-bit processors, but as long as others
|
||||
* take longer, pulses will be longer. For example the SKR Pro
|
||||
* (stm32f407zgt6) requires ~60 cyles.
|
||||
*/
|
||||
#define TIMER_READ_ADD_AND_STORE_CYCLES 34UL
|
||||
|
||||
// The base ISR takes 792 cycles
|
||||
|
@ -86,6 +102,7 @@
|
|||
#define ISR_STEPPER_CYCLES 16UL
|
||||
|
||||
#else
|
||||
// Cycles to perform actions in START_TIMED_PULSE
|
||||
#define TIMER_READ_ADD_AND_STORE_CYCLES 13UL
|
||||
|
||||
// The base ISR takes 752 cycles
|
||||
|
@ -157,17 +174,13 @@
|
|||
#define MIN_STEPPER_PULSE_CYCLES _MIN_STEPPER_PULSE_CYCLES(1UL)
|
||||
#endif
|
||||
|
||||
// Calculate the minimum ticks of the PULSE timer that must elapse with the step pulse enabled
|
||||
// adding the "start stepper pulse" code section execution cycles to account for that not all
|
||||
// pulses start at the beginning of the loop, so an extra time must be added to compensate so
|
||||
// the last generated pulse (usually the extruder stepper) has the right length
|
||||
// Calculate the minimum pulse times (high and low)
|
||||
#if MINIMUM_STEPPER_PULSE && MAXIMUM_STEPPER_RATE
|
||||
constexpr uint32_t _MIN_STEP_PERIOD_NS = 1000000000UL / MAXIMUM_STEPPER_RATE;
|
||||
constexpr uint32_t _MIN_PULSE_HIGH_NS = 1000UL * MINIMUM_STEPPER_PULSE;
|
||||
constexpr uint32_t _MIN_PULSE_LOW_NS = _MAX((_MIN_STEP_PERIOD_NS - _MIN(_MIN_STEP_PERIOD_NS, _MIN_PULSE_HIGH_NS)), _MIN_PULSE_HIGH_NS);
|
||||
#elif MINIMUM_STEPPER_PULSE
|
||||
// Assume 50% duty cycle
|
||||
constexpr uint32_t _MIN_STEP_PERIOD_NS = 1000000000UL / MAXIMUM_STEPPER_RATE;
|
||||
constexpr uint32_t _MIN_PULSE_HIGH_NS = 1000UL * MINIMUM_STEPPER_PULSE;
|
||||
constexpr uint32_t _MIN_PULSE_LOW_NS = _MIN_PULSE_HIGH_NS;
|
||||
#elif MAXIMUM_STEPPER_RATE
|
||||
|
@ -178,11 +191,6 @@
|
|||
#error "Expected at least one of MINIMUM_STEPPER_PULSE or MAXIMUM_STEPPER_RATE to be defined"
|
||||
#endif
|
||||
|
||||
// TODO: NS_TO_PULSE_TIMER_TICKS has some rounding issues:
|
||||
// 1. PULSE_TIMER_TICKS_PER_US rounds to an integer, which loses 20% of the count for a 2.5 MHz pulse tick (such as for LPC1768)
|
||||
// 2. The math currently rounds down to the closes tick. Perhaps should round up.
|
||||
constexpr uint32_t NS_TO_PULSE_TIMER_TICKS(uint32_t NS) { return PULSE_TIMER_TICKS_PER_US * (NS) / 1000UL; }
|
||||
#define CYCLES_TO_NS(CYC) (1000UL * (CYC) / ((F_CPU) / 1000000))
|
||||
|
||||
// But the user could be enforcing a minimum time, so the loop time is
|
||||
#define ISR_LOOP_CYCLES (ISR_LOOP_BASE_CYCLES + _MAX(MIN_STEPPER_PULSE_CYCLES, MIN_ISR_LOOP_CYCLES))
|
||||
|
|
Loading…
Reference in a new issue