MPCNC/Firmware
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

[1.1.x] Fix stepper/planner race condition, Stepper pulse timer (#11084)

Browse Source 
* Fix planner/stepper race condition

Co-Authored-By: ejtagle <ejtagle@hotmail.com>
Co-Authored-By: AnHardt <github@kitelab.de>

* Fix stepper pulse timing

Co-Authored-By: ejtagle <ejtagle@hotmail.com>
This commit is contained in:
Scott Lahteine 2018-06-22 10:53:46 -04:00 committed by GitHub
parent 53745446f9
commit dd9c65d0be
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 88 additions and 70 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

40
Marlin/planner.cpp
View File

@ -740,7 +740,11 @@ void Planner::calculate_trapezoid_for_block(block_t* const block, const float &e
const bool was_enabled = STEPPER_ISR_ENABLED(); const bool was_enabled = STEPPER_ISR_ENABLED();
if (was_enabled) DISABLE_STEPPER_DRIVER_INTERRUPT(); if (was_enabled) DISABLE_STEPPER_DRIVER_INTERRUPT();
// Don't update variables if block is busy: It is being interpreted by the planner // Don't update variables if block is busy; it is being interpreted by the planner.
// If this happens, there's a problem... The block speed is inconsistent. Some values
// have already been updated, but the Stepper ISR is already using the block. Fortunately,
// the values being used by the Stepper ISR weren't touched, so just stop here...
// TODO: There may be a way to update a running block, depending on the stepper ISR position.
if (!TEST(block->flag, BLOCK_BIT_BUSY)) { if (!TEST(block->flag, BLOCK_BIT_BUSY)) {
block->accelerate_until = accelerate_steps; block->accelerate_until = accelerate_steps;
block->decelerate_after = accelerate_steps + plateau_steps; block->decelerate_after = accelerate_steps + plateau_steps;
@ -844,10 +848,13 @@ void Planner::reverse_pass_kernel(block_t* const current, const block_t * const
? max_entry_speed_sqr ? max_entry_speed_sqr
: MIN(max_entry_speed_sqr, max_allowable_speed_sqr(-current->acceleration, next ? next->entry_speed_sqr : sq(MINIMUM_PLANNER_SPEED), current->millimeters)); : MIN(max_entry_speed_sqr, max_allowable_speed_sqr(-current->acceleration, next ? next->entry_speed_sqr : sq(MINIMUM_PLANNER_SPEED), current->millimeters));
if (current->entry_speed_sqr != new_entry_speed_sqr) { if (current->entry_speed_sqr != new_entry_speed_sqr) {
current->entry_speed_sqr = new_entry_speed_sqr;
// Need to recalculate the block speed // Need to recalculate the block speed - Mark it now, so the stepper
// ISR does not consume the block before being recalculated
SBI(current->flag, BLOCK_BIT_RECALCULATE); SBI(current->flag, BLOCK_BIT_RECALCULATE);
// Set the new entry speed
current->entry_speed_sqr = new_entry_speed_sqr;
} }
} }
} }
@ -907,14 +914,15 @@ void Planner::forward_pass_kernel(const block_t* const previous, block_t* const
// If true, current block is full-acceleration and we can move the planned pointer forward. // If true, current block is full-acceleration and we can move the planned pointer forward.
if (new_entry_speed_sqr < current->entry_speed_sqr) { if (new_entry_speed_sqr < current->entry_speed_sqr) {
// Mark we need to recompute the trapezoidal shape, and do it now,
// so the stepper ISR does not consume the block before being recalculated
SBI(current->flag, BLOCK_BIT_RECALCULATE);
// Always <= max_entry_speed_sqr. Backward pass sets this. // Always <= max_entry_speed_sqr. Backward pass sets this.
current->entry_speed_sqr = new_entry_speed_sqr; // Always <= max_entry_speed_sqr. Backward pass sets this. current->entry_speed_sqr = new_entry_speed_sqr; // Always <= max_entry_speed_sqr. Backward pass sets this.
// Set optimal plan pointer. // Set optimal plan pointer.
block_buffer_planned = block_index; block_buffer_planned = block_index;
// And mark we need to recompute the trapezoidal shape
SBI(current->flag, BLOCK_BIT_RECALCULATE);
} }
} }
@ -1001,6 +1009,12 @@ void Planner::recalculate_trapezoids() {
if (current) { if (current) {
// Recalculate if current block entry or exit junction speed has changed. // Recalculate if current block entry or exit junction speed has changed.
if (TEST(current->flag, BLOCK_BIT_RECALCULATE) || TEST(next->flag, BLOCK_BIT_RECALCULATE)) { if (TEST(current->flag, BLOCK_BIT_RECALCULATE) || TEST(next->flag, BLOCK_BIT_RECALCULATE)) {
// Mark the current block as RECALCULATE, to protect it from the Stepper ISR running it.
// Note that due to the above condition, there's a chance the current block isn't marked as
// RECALCULATE yet, but the next one is. That's the reason for the following line.
SBI(current->flag, BLOCK_BIT_RECALCULATE);
// NOTE: Entry and exit factors always > 0 by all previous logic operations. // NOTE: Entry and exit factors always > 0 by all previous logic operations.
const float current_nominal_speed = SQRT(current->nominal_speed_sqr), const float current_nominal_speed = SQRT(current->nominal_speed_sqr),
nomr = 1.0 / current_nominal_speed; nomr = 1.0 / current_nominal_speed;
@ -1012,7 +1026,10 @@ void Planner::recalculate_trapezoids() {
current->final_adv_steps = next_entry_speed * comp; current->final_adv_steps = next_entry_speed * comp;
} }
#endif #endif
CBI(current->flag, BLOCK_BIT_RECALCULATE); // Reset current only to ensure next trapezoid is computed
// Reset current only to ensure next trapezoid is computed - The
// stepper is free to use the block from now on.
CBI(current->flag, BLOCK_BIT_RECALCULATE);
} }
} }
@ -1025,6 +1042,12 @@ void Planner::recalculate_trapezoids() {
// Last/newest block in buffer. Exit speed is set with MINIMUM_PLANNER_SPEED. Always recalculated. // Last/newest block in buffer. Exit speed is set with MINIMUM_PLANNER_SPEED. Always recalculated.
if (next) { if (next) {
// Mark the next(last) block as RECALCULATE, to prevent the Stepper ISR running it.
// As the last block is always recalculated here, there is a chance the block isn't
// marked as RECALCULATE yet. That's the reason for the following line.
SBI(next->flag, BLOCK_BIT_RECALCULATE);
const float next_nominal_speed = SQRT(next->nominal_speed_sqr), const float next_nominal_speed = SQRT(next->nominal_speed_sqr),
nomr = 1.0 / next_nominal_speed; nomr = 1.0 / next_nominal_speed;
calculate_trapezoid_for_block(next, next_entry_speed * nomr, (MINIMUM_PLANNER_SPEED) * nomr); calculate_trapezoid_for_block(next, next_entry_speed * nomr, (MINIMUM_PLANNER_SPEED) * nomr);
@ -1035,6 +1058,9 @@ void Planner::recalculate_trapezoids() {
next->final_adv_steps = (MINIMUM_PLANNER_SPEED) * comp; next->final_adv_steps = (MINIMUM_PLANNER_SPEED) * comp;
} }
#endif #endif
// Reset next only to ensure its trapezoid is computed - The stepper is free to use
// the block from now on.
CBI(next->flag, BLOCK_BIT_RECALCULATE); CBI(next->flag, BLOCK_BIT_RECALCULATE);
} }
} }

68
Marlin/stepper.h
View File

@ -53,7 +53,7 @@
// //
#ifndef MINIMUM_STEPPER_PULSE #ifndef MINIMUM_STEPPER_PULSE
#define MINIMUM_STEPPER_PULSE 0 #define MINIMUM_STEPPER_PULSE 0UL
#endif #endif
#ifndef MAXIMUM_STEPPER_RATE #ifndef MAXIMUM_STEPPER_RATE
@ -64,33 +64,6 @@
#endif #endif
#endif #endif
#ifdef CPU_32_BIT
// The base ISR takes 792 cycles
#define ISR_BASE_CYCLES 792UL
// Linear advance base time is 64 cycles
#if ENABLED(LIN_ADVANCE)
#define ISR_LA_BASE_CYCLES 64UL
#else
#define ISR_LA_BASE_CYCLES 0UL
#endif
// S curve interpolation adds 40 cycles
#if ENABLED(S_CURVE_ACCELERATION)
#define ISR_S_CURVE_CYCLES 40UL
#else
#define ISR_S_CURVE_CYCLES 0UL
#endif
// Stepper Loop base cycles
#define ISR_LOOP_BASE_CYCLES 4UL
// And each stepper takes 16 cycles
#define ISR_STEPPER_CYCLES 16UL
#else
// The base ISR takes 752 cycles // The base ISR takes 752 cycles
#define ISR_BASE_CYCLES 752UL #define ISR_BASE_CYCLES 752UL
@ -111,54 +84,73 @@
// Stepper Loop base cycles // Stepper Loop base cycles
#define ISR_LOOP_BASE_CYCLES 32UL #define ISR_LOOP_BASE_CYCLES 32UL
// And each stepper takes 88 cycles // To start the step pulse, in the worst case takes
#define ISR_STEPPER_CYCLES 88UL #define ISR_START_STEPPER_CYCLES 57UL
#endif // And each stepper (start + stop pulse) takes in worst case
#define ISR_STEPPER_CYCLES 88UL
// Add time for each stepper // Add time for each stepper
#ifdef HAS_X_STEP #ifdef HAS_X_STEP
#define ISR_START_X_STEPPER_CYCLES ISR_START_STEPPER_CYCLES
#define ISR_X_STEPPER_CYCLES ISR_STEPPER_CYCLES #define ISR_X_STEPPER_CYCLES ISR_STEPPER_CYCLES
#else #else
#define ISR_START_X_STEPPER_CYCLES 0UL
#define ISR_X_STEPPER_CYCLES 0UL #define ISR_X_STEPPER_CYCLES 0UL
#endif #endif
#ifdef HAS_Y_STEP #ifdef HAS_Y_STEP
#define ISR_START_Y_STEPPER_CYCLES ISR_START_STEPPER_CYCLES
#define ISR_Y_STEPPER_CYCLES ISR_STEPPER_CYCLES #define ISR_Y_STEPPER_CYCLES ISR_STEPPER_CYCLES
#else #else
#define ISR_START_Y_STEPPER_CYCLES 0UL
#define ISR_Y_STEPPER_CYCLES 0UL #define ISR_Y_STEPPER_CYCLES 0UL
#endif #endif
#ifdef HAS_Z_STEP #ifdef HAS_Z_STEP
#define ISR_START_Z_STEPPER_CYCLES ISR_START_STEPPER_CYCLES
#define ISR_Z_STEPPER_CYCLES ISR_STEPPER_CYCLES #define ISR_Z_STEPPER_CYCLES ISR_STEPPER_CYCLES
#else #else
#define ISR_START_Z_STEPPER_CYCLES 0UL
#define ISR_Z_STEPPER_CYCLES 0UL #define ISR_Z_STEPPER_CYCLES 0UL
#endif #endif
// E is always interpolated, even for mixing extruders // E is always interpolated, even for mixing extruders
#define ISR_START_E_STEPPER_CYCLES ISR_START_STEPPER_CYCLES
#define ISR_E_STEPPER_CYCLES ISR_STEPPER_CYCLES #define ISR_E_STEPPER_CYCLES ISR_STEPPER_CYCLES
// If linear advance is disabled, then the loop also handles them // If linear advance is disabled, then the loop also handles them
#if DISABLED(LIN_ADVANCE) && ENABLED(MIXING_EXTRUDER) #if DISABLED(LIN_ADVANCE) && ENABLED(MIXING_EXTRUDER)
#define ISR_START_MIXING_STEPPER_CYCLES ((MIXING_STEPPERS) * (ISR_START_STEPPER_CYCLES))
#define ISR_MIXING_STEPPER_CYCLES ((MIXING_STEPPERS) * (ISR_STEPPER_CYCLES)) #define ISR_MIXING_STEPPER_CYCLES ((MIXING_STEPPERS) * (ISR_STEPPER_CYCLES))
#else #else
#define ISR_START_MIXING_STEPPER_CYCLES 0UL
#define ISR_MIXING_STEPPER_CYCLES 0UL #define ISR_MIXING_STEPPER_CYCLES 0UL
#endif #endif
// Calculate the minimum time to start all stepper pulses in the ISR loop
#define MIN_ISR_START_LOOP_CYCLES (ISR_START_X_STEPPER_CYCLES + ISR_START_Y_STEPPER_CYCLES + ISR_START_Z_STEPPER_CYCLES + ISR_START_E_STEPPER_CYCLES + ISR_START_MIXING_STEPPER_CYCLES)
// And the total minimum loop time, not including the base // And the total minimum loop time, not including the base
#define MIN_ISR_LOOP_CYCLES (ISR_X_STEPPER_CYCLES + ISR_Y_STEPPER_CYCLES + ISR_Z_STEPPER_CYCLES + ISR_E_STEPPER_CYCLES + ISR_MIXING_STEPPER_CYCLES) #define MIN_ISR_LOOP_CYCLES (ISR_X_STEPPER_CYCLES + ISR_Y_STEPPER_CYCLES + ISR_Z_STEPPER_CYCLES + ISR_E_STEPPER_CYCLES + ISR_MIXING_STEPPER_CYCLES)
// Calculate the minimum MPU cycles needed per pulse to enforce, limited to the max stepper rate // Calculate the minimum MPU cycles needed per pulse to enforce, limited to the max stepper rate
#define _MIN_STEPPER_PULSE_CYCLES(N) max((F_CPU) / (MAXIMUM_STEPPER_RATE), ((F_CPU) / 500000UL) * (N)) #define _MIN_STEPPER_PULSE_CYCLES(N) MAX((F_CPU) / (MAXIMUM_STEPPER_RATE), ((F_CPU) / 500000UL) * (N))
#if MINIMUM_STEPPER_PULSE #if MINIMUM_STEPPER_PULSE
#define MIN_STEPPER_PULSE_CYCLES _MIN_STEPPER_PULSE_CYCLES(MINIMUM_STEPPER_PULSE) #define MIN_STEPPER_PULSE_CYCLES _MIN_STEPPER_PULSE_CYCLES((MINIMUM_STEPPER_PULSE))
#else #else
#define MIN_STEPPER_PULSE_CYCLES _MIN_STEPPER_PULSE_CYCLES(1) #define MIN_STEPPER_PULSE_CYCLES _MIN_STEPPER_PULSE_CYCLES(1UL)
#endif #endif
#define MIN_PULSE_TICKS ((PULSE_TIMER_TICKS_PER_US) * (MINIMUM_STEPPER_PULSE)) // Calculate the minimum ticks of the PULSE timer that must elapse with the step pulse enabled
#define ADDED_STEP_TICKS ((MIN_STEPPER_PULSE_CYCLES) / (PULSE_TIMER_PRESCALE) - MIN_PULSE_TICKS) // 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
#define MIN_PULSE_TICKS (((PULSE_TIMER_TICKS_PER_US) * (MINIMUM_STEPPER_PULSE)) + ((MIN_ISR_START_LOOP_CYCLES) / (PULSE_TIMER_PRESCALE)))
// Calculate the extra ticks of the PULSE timer between step pulses
#define ADDED_STEP_TICKS (((MIN_STEPPER_PULSE_CYCLES) / (PULSE_TIMER_PRESCALE)) - (MIN_PULSE_TICKS))
// But the user could be enforcing a minimum time, so the loop time is // 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)) #define ISR_LOOP_CYCLES (ISR_LOOP_BASE_CYCLES + MAX(MIN_STEPPER_PULSE_CYCLES, MIN_ISR_LOOP_CYCLES))
// If linear advance is enabled, then it is handled separately // If linear advance is enabled, then it is handled separately
#if ENABLED(LIN_ADVANCE) #if ENABLED(LIN_ADVANCE)
@ -171,7 +163,7 @@
#endif #endif
// And the real loop time // And the real loop time
#define ISR_LA_LOOP_CYCLES max(MIN_STEPPER_PULSE_CYCLES, MIN_ISR_LA_LOOP_CYCLES) #define ISR_LA_LOOP_CYCLES MAX(MIN_STEPPER_PULSE_CYCLES, MIN_ISR_LA_LOOP_CYCLES)
#else #else
#define ISR_LA_LOOP_CYCLES 0UL #define ISR_LA_LOOP_CYCLES 0UL