From ac368f27887a4ab423a0103dc85d9268b3b7e00c Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Thu, 11 Jan 2018 20:59:16 -0600 Subject: [PATCH 1/4] Add STEPPER_ISR_ENABLED() to HALs Some also get a `HAL_timer_interrupt_enabled` function. --- Marlin/src/HAL/HAL_AVR/HAL_AVR.h | 1 + Marlin/src/HAL/HAL_DUE/HAL_timers_Due.cpp | 13 +++++++++---- Marlin/src/HAL/HAL_DUE/HAL_timers_Due.h | 12 +++++++----- Marlin/src/HAL/HAL_LPC1768/HAL_timers.cpp | 8 ++++++++ Marlin/src/HAL/HAL_LPC1768/HAL_timers.h | 3 +++ Marlin/src/HAL/HAL_STM32F1/HAL_timers_Stm32f1.cpp | 14 +++++++++++--- Marlin/src/HAL/HAL_STM32F1/HAL_timers_Stm32f1.h | 10 +++++----- .../src/HAL/HAL_TEENSY35_36/HAL_timers_Teensy.cpp | 8 ++++++++ Marlin/src/HAL/HAL_TEENSY35_36/HAL_timers_Teensy.h | 4 +++- 9 files changed, 55 insertions(+), 18 deletions(-) diff --git a/Marlin/src/HAL/HAL_AVR/HAL_AVR.h b/Marlin/src/HAL/HAL_AVR/HAL_AVR.h index 8ea43c815..c3b7403e3 100644 --- a/Marlin/src/HAL/HAL_AVR/HAL_AVR.h +++ b/Marlin/src/HAL/HAL_AVR/HAL_AVR.h @@ -139,6 +139,7 @@ extern "C" { #define ENABLE_STEPPER_DRIVER_INTERRUPT() SBI(TIMSK1, OCIE1A) #define DISABLE_STEPPER_DRIVER_INTERRUPT() CBI(TIMSK1, OCIE1A) +#define STEPPER_ISR_ENABLED() TEST(TIMSK1, OCIE1A) #define ENABLE_TEMPERATURE_INTERRUPT() SBI(TIMSK0, OCIE0B) #define DISABLE_TEMPERATURE_INTERRUPT() CBI(TIMSK0, OCIE0B) diff --git a/Marlin/src/HAL/HAL_DUE/HAL_timers_Due.cpp b/Marlin/src/HAL/HAL_DUE/HAL_timers_Due.cpp index dbeb28541..a604f6806 100644 --- a/Marlin/src/HAL/HAL_DUE/HAL_timers_Due.cpp +++ b/Marlin/src/HAL/HAL_DUE/HAL_timers_Due.cpp @@ -112,23 +112,28 @@ void HAL_timer_start(const uint8_t timer_num, const uint32_t frequency) { } void HAL_timer_enable_interrupt(const uint8_t timer_num) { - const tTimerConfig *pConfig = &TimerConfig[timer_num]; + const tTimerConfig * const pConfig = &TimerConfig[timer_num]; pConfig->pTimerRegs->TC_CHANNEL[pConfig->channel].TC_IER = TC_IER_CPCS; } void HAL_timer_disable_interrupt(const uint8_t timer_num) { - const tTimerConfig *pConfig = &TimerConfig[timer_num]; + const tTimerConfig * const pConfig = &TimerConfig[timer_num]; pConfig->pTimerRegs->TC_CHANNEL[pConfig->channel].TC_IDR = TC_IDR_CPCS; } +void HAL_timer_interrupt_enabled(const uint8_t timer_num) { + const tTimerConfig * const pConfig = &TimerConfig[timer_num]; + return pConfig->pTimerRegs->TC_CHANNEL[pConfig->channel].TC_IER == TC_IER_CPCS; +} + #if 0 void HAL_timer_set_count(const uint8_t timer_num, const uint32_t count) { - const tTimerConfig *pConfig = &TimerConfig[timer_num]; + const tTimerConfig * const pConfig = &TimerConfig[timer_num]; TC_SetRC(pConfig->pTimerRegs, pConfig->channel, count); } void HAL_timer_isr_prologue(const uint8_t timer_num) { - const tTimerConfig *pConfig = &TimerConfig[timer_num]; + const tTimerConfig * const pConfig = &TimerConfig[timer_num]; TC_GetStatus(pConfig->pTimerRegs, pConfig->channel); } #endif diff --git a/Marlin/src/HAL/HAL_DUE/HAL_timers_Due.h b/Marlin/src/HAL/HAL_DUE/HAL_timers_Due.h index b93e80b7d..d4d4a6e51 100644 --- a/Marlin/src/HAL/HAL_DUE/HAL_timers_Due.h +++ b/Marlin/src/HAL/HAL_DUE/HAL_timers_Due.h @@ -55,6 +55,7 @@ typedef uint32_t hal_timer_t; #define ENABLE_STEPPER_DRIVER_INTERRUPT() HAL_timer_enable_interrupt(STEP_TIMER_NUM) #define DISABLE_STEPPER_DRIVER_INTERRUPT() HAL_timer_disable_interrupt(STEP_TIMER_NUM) +#define STEPPER_ISR_ENABLED() HAL_timer_interrupt_enabled(STEP_TIMER_NUM) #define ENABLE_TEMPERATURE_INTERRUPT() HAL_timer_enable_interrupt(TEMP_TIMER_NUM) #define DISABLE_TEMPERATURE_INTERRUPT() HAL_timer_disable_interrupt(TEMP_TIMER_NUM) @@ -91,32 +92,33 @@ extern const tTimerConfig TimerConfig[]; void HAL_timer_start(const uint8_t timer_num, const uint32_t frequency); FORCE_INLINE static void HAL_timer_set_count(const uint8_t timer_num, const hal_timer_t count) { - const tTimerConfig *pConfig = &TimerConfig[timer_num]; + const tTimerConfig * const pConfig = &TimerConfig[timer_num]; pConfig->pTimerRegs->TC_CHANNEL[pConfig->channel].TC_RC = count; } FORCE_INLINE static hal_timer_t HAL_timer_get_count(const uint8_t timer_num) { - const tTimerConfig *pConfig = &TimerConfig[timer_num]; + const tTimerConfig * const pConfig = &TimerConfig[timer_num]; return pConfig->pTimerRegs->TC_CHANNEL[pConfig->channel].TC_RC; } FORCE_INLINE static void HAL_timer_set_current_count(const uint8_t timer_num, const hal_timer_t count) { - const tTimerConfig *pConfig = &TimerConfig[timer_num]; + const tTimerConfig * const pConfig = &TimerConfig[timer_num]; pConfig->pTimerRegs->TC_CHANNEL[pConfig->channel].TC_CV = count; } FORCE_INLINE static hal_timer_t HAL_timer_get_current_count(const uint8_t timer_num) { - const tTimerConfig *pConfig = &TimerConfig[timer_num]; + const tTimerConfig * const pConfig = &TimerConfig[timer_num]; return pConfig->pTimerRegs->TC_CHANNEL[pConfig->channel].TC_CV; } void HAL_timer_enable_interrupt(const uint8_t timer_num); void HAL_timer_disable_interrupt(const uint8_t timer_num); +bool HAL_timer_interrupt_enabled(const uint8_t timer_num); //void HAL_timer_isr_prologue(const uint8_t timer_num); FORCE_INLINE static void HAL_timer_isr_prologue(const uint8_t timer_num) { - const tTimerConfig *pConfig = &TimerConfig[timer_num]; + const tTimerConfig * const pConfig = &TimerConfig[timer_num]; // Reading the status register clears the interrupt flag pConfig->pTimerRegs->TC_CHANNEL[pConfig->channel].TC_SR; } diff --git a/Marlin/src/HAL/HAL_LPC1768/HAL_timers.cpp b/Marlin/src/HAL/HAL_LPC1768/HAL_timers.cpp index c9e25693e..3f7507aa5 100644 --- a/Marlin/src/HAL/HAL_LPC1768/HAL_timers.cpp +++ b/Marlin/src/HAL/HAL_LPC1768/HAL_timers.cpp @@ -75,6 +75,14 @@ void HAL_timer_disable_interrupt(const uint8_t timer_num) { } } +bool HAL_timer_interrupt_enabled(const uint8_t timer_num) { + switch (timer_num) { + case 0: return NVIC_GetActive(TIMER0_IRQn); + case 1: return NVIC_GetActive(TIMER1_IRQn); + } + return false; +} + void HAL_timer_isr_prologue(const uint8_t timer_num) { switch (timer_num) { case 0: SBI(LPC_TIM0->IR, 0); break; // Clear the Interrupt diff --git a/Marlin/src/HAL/HAL_LPC1768/HAL_timers.h b/Marlin/src/HAL/HAL_LPC1768/HAL_timers.h index 34b3b24b9..8b4c222af 100644 --- a/Marlin/src/HAL/HAL_LPC1768/HAL_timers.h +++ b/Marlin/src/HAL/HAL_LPC1768/HAL_timers.h @@ -58,6 +58,8 @@ typedef uint32_t hal_timer_t; #define ENABLE_STEPPER_DRIVER_INTERRUPT() HAL_timer_enable_interrupt(STEP_TIMER_NUM) #define DISABLE_STEPPER_DRIVER_INTERRUPT() HAL_timer_disable_interrupt(STEP_TIMER_NUM) +#define STEPPER_ISR_ENABLED() HAL_timer_interrupt_enabled(STEP_TIMER_NUM) + #define ENABLE_TEMPERATURE_INTERRUPT() HAL_timer_enable_interrupt(TEMP_TIMER_NUM) #define DISABLE_TEMPERATURE_INTERRUPT() HAL_timer_disable_interrupt(TEMP_TIMER_NUM) @@ -125,6 +127,7 @@ FORCE_INLINE static hal_timer_t HAL_timer_get_current_count(const uint8_t timer_ void HAL_timer_enable_interrupt(const uint8_t timer_num); void HAL_timer_disable_interrupt(const uint8_t timer_num); +bool HAL_timer_interrupt_enabled(const uint8_t timer_num); void HAL_timer_isr_prologue(const uint8_t timer_num); #endif // _HAL_TIMERS_DUE_H diff --git a/Marlin/src/HAL/HAL_STM32F1/HAL_timers_Stm32f1.cpp b/Marlin/src/HAL/HAL_STM32F1/HAL_timers_Stm32f1.cpp index 45ed20df1..0d64e86f5 100644 --- a/Marlin/src/HAL/HAL_STM32F1/HAL_timers_Stm32f1.cpp +++ b/Marlin/src/HAL/HAL_STM32F1/HAL_timers_Stm32f1.cpp @@ -93,7 +93,7 @@ const tTimerConfig TimerConfig [NUM_HARDWARE_TIMERS] = { * TODO: Calculate Timer prescale value, so we get the 32bit to adjust */ -void HAL_timer_start(uint8_t timer_num, uint32_t frequency) { +void HAL_timer_start(const uint8_t timer_num, const uint32_t frequency) { nvic_irq_num irq_num; switch (timer_num) { case 1: irq_num = NVIC_TIMER1_CC; break; @@ -135,7 +135,7 @@ void HAL_timer_start(uint8_t timer_num, uint32_t frequency) { } } -void HAL_timer_enable_interrupt(uint8_t timer_num) { +void HAL_timer_enable_interrupt(const uint8_t timer_num) { switch (timer_num) { case STEP_TIMER_NUM: timer_enable_irq(STEP_TIMER_DEV, STEP_TIMER_CHAN); @@ -148,7 +148,7 @@ void HAL_timer_enable_interrupt(uint8_t timer_num) { } } -void HAL_timer_disable_interrupt(uint8_t timer_num) { +void HAL_timer_disable_interrupt(const uint8_t timer_num) { switch (timer_num) { case STEP_TIMER_NUM: timer_disable_irq(STEP_TIMER_DEV, STEP_TIMER_CHAN); @@ -161,4 +161,12 @@ void HAL_timer_disable_interrupt(uint8_t timer_num) { } } +bool HAL_timer_interrupt_enabled(const uint8_t timer_num) { + switch (timer_num) { + case STEP_TIMER_NUM: return bool(TIM_DIER(STEP_TIMER_DEV) & STEP_TIMER_CHAN); + case TEMP_TIMER_NUM: return bool(TIM_DIER(TEMP_TIMER_DEV) & TEMP_TIMER_CHAN); + } + return false; +} + #endif // __STM32F1__ diff --git a/Marlin/src/HAL/HAL_STM32F1/HAL_timers_Stm32f1.h b/Marlin/src/HAL/HAL_STM32F1/HAL_timers_Stm32f1.h index bdc083e2a..79c9408ca 100644 --- a/Marlin/src/HAL/HAL_STM32F1/HAL_timers_Stm32f1.h +++ b/Marlin/src/HAL/HAL_STM32F1/HAL_timers_Stm32f1.h @@ -62,8 +62,6 @@ typedef uint16_t hal_timer_t; #define STEP_TIMER_DEV TIMER_DEV(STEP_TIMER_NUM) #define TEMP_TIMER_DEV TIMER_DEV(TEMP_TIMER_NUM) - - //STM32_HAVE_TIMER(n); #define HAL_TIMER_RATE (F_CPU) // frequency of timers peripherals @@ -79,6 +77,7 @@ typedef uint16_t hal_timer_t; #define ENABLE_STEPPER_DRIVER_INTERRUPT() timer_enable_irq(STEP_TIMER_DEV, STEP_TIMER_CHAN) #define DISABLE_STEPPER_DRIVER_INTERRUPT() timer_disable_irq(STEP_TIMER_DEV, STEP_TIMER_CHAN) +#define STEPPER_ISR_ENABLED() HAL_timer_interrupt_enabled(STEP_TIMER_NUM) #define ENABLE_TEMPERATURE_INTERRUPT() timer_enable_irq(TEMP_TIMER_DEV, TEMP_TIMER_CHAN) #define DISABLE_TEMPERATURE_INTERRUPT() timer_disable_irq(TEMP_TIMER_DEV, TEMP_TIMER_CHAN) @@ -113,9 +112,10 @@ static HardwareTimer TempTimer(TEMP_TIMER_NUM); // Public functions // -------------------------------------------------------------------------- -void HAL_timer_start(uint8_t timer_num, uint32_t frequency); -void HAL_timer_enable_interrupt(uint8_t timer_num); -void HAL_timer_disable_interrupt(uint8_t timer_num); +void HAL_timer_start(const uint8_t timer_num, const uint32_t frequency); +void HAL_timer_enable_interrupt(const uint8_t timer_num); +void HAL_timer_disable_interrupt(const uint8_t timer_num); +bool HAL_timer_interrupt_enabled(const uint8_t timer_num); /** * NOTE: By default libmaple sets ARPE = 1, which means the Auto reload register is preloaded (will only update with an update event) diff --git a/Marlin/src/HAL/HAL_TEENSY35_36/HAL_timers_Teensy.cpp b/Marlin/src/HAL/HAL_TEENSY35_36/HAL_timers_Teensy.cpp index 6a2d39234..03bf9dadd 100644 --- a/Marlin/src/HAL/HAL_TEENSY35_36/HAL_timers_Teensy.cpp +++ b/Marlin/src/HAL/HAL_TEENSY35_36/HAL_timers_Teensy.cpp @@ -67,6 +67,14 @@ void HAL_timer_disable_interrupt(const uint8_t timer_num) { } } +bool HAL_timer_interrupt_enabled(const uint8_t timer_num) { + switch (timer_num) { + case 0: return NVIC_IS_ENABLED(IRQ_FTM0); + case 1: return NVIC_IS_ENABLED(IRQ_FTM1); + } + return false; +} + void HAL_timer_isr_prologue(const uint8_t timer_num) { switch(timer_num) { case 0: diff --git a/Marlin/src/HAL/HAL_TEENSY35_36/HAL_timers_Teensy.h b/Marlin/src/HAL/HAL_TEENSY35_36/HAL_timers_Teensy.h index 3675ea60b..9482bee5b 100644 --- a/Marlin/src/HAL/HAL_TEENSY35_36/HAL_timers_Teensy.h +++ b/Marlin/src/HAL/HAL_TEENSY35_36/HAL_timers_Teensy.h @@ -68,6 +68,8 @@ typedef uint32_t hal_timer_t; #define ENABLE_STEPPER_DRIVER_INTERRUPT() HAL_timer_enable_interrupt(STEP_TIMER_NUM) #define DISABLE_STEPPER_DRIVER_INTERRUPT() HAL_timer_disable_interrupt(STEP_TIMER_NUM) +#define STEPPER_ISR_ENABLED() HAL_timer_interrupt_enabled(STEP_TIMER_NUM) + #define ENABLE_TEMPERATURE_INTERRUPT() HAL_timer_enable_interrupt(TEMP_TIMER_NUM) #define DISABLE_TEMPERATURE_INTERRUPT() HAL_timer_disable_interrupt(TEMP_TIMER_NUM) @@ -110,8 +112,8 @@ FORCE_INLINE static hal_timer_t HAL_timer_get_current_count(const uint8_t timer_ void HAL_timer_enable_interrupt(const uint8_t timer_num); void HAL_timer_disable_interrupt(const uint8_t timer_num); +bool HAL_timer_interrupt_enabled(const uint8_t timer_num); void HAL_timer_isr_prologue(const uint8_t timer_num); #endif // _HAL_TIMERS_TEENSY_H - From 4f5e087ff49a57790b3e02ca895b15795e595713 Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Thu, 11 Jan 2018 21:02:13 -0600 Subject: [PATCH 2/4] Planner anti-stutter by Sebastian Popp --- Marlin/src/module/planner.cpp | 11 +++++------ Marlin/src/module/planner.h | 10 ++++++++++ 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/Marlin/src/module/planner.cpp b/Marlin/src/module/planner.cpp index 701fe0e36..2d0e4ce38 100644 --- a/Marlin/src/module/planner.cpp +++ b/Marlin/src/module/planner.cpp @@ -1352,7 +1352,9 @@ void Planner::_buffer_steps(const int32_t (&target)[XYZE], float fr_mm_s, const // Initialize block entry speed. Compute based on deceleration to user-defined MINIMUM_PLANNER_SPEED. const float v_allowable = max_allowable_speed(-block->acceleration, MINIMUM_PLANNER_SPEED, block->millimeters); - block->entry_speed = min(vmax_junction, v_allowable); + // If stepper ISR is disabled, this indicates buffer_segment wants to add a split block. + // In this case start with the max. allowed speed to avoid an interrupted first move. + block->entry_speed = STEPPER_ISR_ENABLED() ? MINIMUM_PLANNER_SPEED : min(vmax_junction, v_allowable); // Initialize planner efficiency flags // Set flag if block will always reach maximum junction speed regardless of entry/exit speeds. @@ -1362,7 +1364,7 @@ void Planner::_buffer_steps(const int32_t (&target)[XYZE], float fr_mm_s, const // block nominal speed limits both the current and next maximum junction speeds. Hence, in both // the reverse and forward planners, the corresponding block junction speed will always be at the // the maximum junction speed and may always be ignored for any speed reduction checks. - block->flag |= BLOCK_FLAG_RECALCULATE | (block->nominal_speed <= v_allowable ? BLOCK_FLAG_NOMINAL_LENGTH : 0); + block->flag |= block->nominal_speed <= v_allowable ? BLOCK_FLAG_RECALCULATE | BLOCK_FLAG_NOMINAL_LENGTH : BLOCK_FLAG_RECALCULATE; // Update previous path unit_vector and nominal speed COPY(previous_speed, current_speed); @@ -1382,7 +1384,7 @@ void Planner::_buffer_steps(const int32_t (&target)[XYZE], float fr_mm_s, const * In that case, the retract and move will be executed together. * This leads to too many advance steps due to a huge e_acceleration. * The math is good, but we must avoid retract moves with advance! - * lin_dist_e > 0 : Extruder is running forward (e.g., for "Wipe while retracting" (Slic3r) or "Combing" (Cura) moves) + * lin_dist_e > 0 : Extruder is running forward (e.g., for "Wipe while retracting" (Slic3r) or "Combing" (Cura) moves) */ block->use_advance_lead = esteps && (block->steps[X_AXIS] || block->steps[Y_AXIS]) && extruder_advance_k @@ -1398,9 +1400,6 @@ void Planner::_buffer_steps(const int32_t (&target)[XYZE], float fr_mm_s, const #endif // LIN_ADVANCE - const float bnsr = 1.0 / block->nominal_speed; - calculate_trapezoid_for_block(block, block->entry_speed * bnsr, safe_speed * bnsr); - // Move buffer head block_buffer_head = next_buffer_head; diff --git a/Marlin/src/module/planner.h b/Marlin/src/module/planner.h index efdd4c7be..a904caacb 100644 --- a/Marlin/src/module/planner.h +++ b/Marlin/src/module/planner.h @@ -534,6 +534,16 @@ class Planner { static block_t* get_current_block() { if (blocks_queued()) { block_t * const block = &block_buffer[block_buffer_tail]; + + // If the block has no trapezoid calculated, it's unsafe to execute. + if (movesplanned() > 1) { + const block_t * const next = &block_buffer[next_block_index(block_buffer_tail)]; + if (TEST(block->flag, BLOCK_BIT_RECALCULATE) || TEST(next->flag, BLOCK_BIT_RECALCULATE)) + return NULL; + } + else if (TEST(block->flag, BLOCK_BIT_RECALCULATE)) + return NULL; + #if ENABLED(ULTRA_LCD) block_buffer_runtime_us -= block->segment_time_us; // We can't be sure how long an active block will take, so don't count it. #endif From c37d38886cb4ffda1f97251069c66bff49458a88 Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Thu, 11 Jan 2018 21:30:18 -0600 Subject: [PATCH 3/4] Fix serial.h avr block Followup to #8148 --- Marlin/src/core/serial.h | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/Marlin/src/core/serial.h b/Marlin/src/core/serial.h index 5f90cd0e7..232b69232 100644 --- a/Marlin/src/core/serial.h +++ b/Marlin/src/core/serial.h @@ -61,21 +61,7 @@ enum DebugFlags { }; #endif -//todo: HAL: breaks encapsulation -// For AVR only, define a serial interface based on configuration -#ifdef __AVR__ - #ifdef USBCON - #include - #if ENABLED(BLUETOOTH) - #define MYSERIAL0 bluetoothSerial - #else - #define MYSERIAL0 Serial - #endif // BLUETOOTH - #else - #include "../HAL/HAL_AVR/MarlinSerial.h" - #define MYSERIAL0 customizedSerial - #endif -#elif defined(ARDUINO_ARCH_SAM) +#ifdef ARDUINO_ARCH_SAM // To pull the Serial port definitions and overrides #include "../HAL/HAL_DUE/MarlinSerial_Due.h" #endif From 094e9b1dab4cd815b0d88033ed52dfa9568e798b Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Thu, 11 Jan 2018 21:46:01 -0600 Subject: [PATCH 4/4] Remove ARDUINO_ARCH_SAM from serial.h too? --- Marlin/src/core/serial.h | 5 ----- 1 file changed, 5 deletions(-) diff --git a/Marlin/src/core/serial.h b/Marlin/src/core/serial.h index 232b69232..5606f4772 100644 --- a/Marlin/src/core/serial.h +++ b/Marlin/src/core/serial.h @@ -61,11 +61,6 @@ enum DebugFlags { }; #endif -#ifdef ARDUINO_ARCH_SAM - // To pull the Serial port definitions and overrides - #include "../HAL/HAL_DUE/MarlinSerial_Due.h" -#endif - extern uint8_t marlin_debug_flags; #define DEBUGGING(F) (marlin_debug_flags & (DEBUG_## F))