From a448cedbc57c9ea68c3f264115a8e83d694b0c2c Mon Sep 17 00:00:00 2001 From: Sebastianv650 Date: Sat, 8 Oct 2016 18:13:04 +0200 Subject: [PATCH 1/4] LIN_ADVANCE bug fix and optimization .) long to int: Extruder stalls at 10kHz / 20kHz step limits with long. .) Take the delta_adv_steps calculation out of the step_loops loop. Wasted calculation performance if done inside. .) >> 2 replaced by 3: Is divide by 8. Reason: Timer 0 runs at 16/8=2MHz, Timer 1 at 16/64=0.25MHz. ==> 2/0.25=8. --- Marlin/stepper.cpp | 43 +++++++++++++++++++++---------------------- Marlin/stepper.h | 2 +- 2 files changed, 22 insertions(+), 23 deletions(-) diff --git a/Marlin/stepper.cpp b/Marlin/stepper.cpp index f2800ebe5..7889b8a88 100644 --- a/Marlin/stepper.cpp +++ b/Marlin/stepper.cpp @@ -95,7 +95,7 @@ volatile uint32_t Stepper::step_events_completed = 0; // The number of step even volatile unsigned char Stepper::eISR_Rate = 200; // Keep the ISR at a low rate until needed #if ENABLED(LIN_ADVANCE) - volatile long Stepper::e_steps[E_STEPPERS]; + volatile int Stepper::e_steps[E_STEPPERS]; int Stepper::extruder_advance_k = LIN_ADVANCE_K, Stepper::final_estep_rate, Stepper::current_estep_rate[E_STEPPERS], @@ -391,7 +391,7 @@ void Stepper::isr() { #if ENABLED(MIXING_EXTRUDER) // Step mixing steppers proportionally - bool dir = motor_direction(E_AXIS); + const bool dir = motor_direction(E_AXIS); MIXING_STEPPERS_LOOP(j) { counter_m[j] += current_block->steps[E_AXIS]; if (counter_m[j] > 0) { @@ -401,22 +401,6 @@ void Stepper::isr() { } #endif - if (current_block->use_advance_lead) { - int delta_adv_steps = (((long)extruder_advance_k * current_estep_rate[TOOL_E_INDEX]) >> 9) - current_adv_steps[TOOL_E_INDEX]; - #if ENABLED(MIXING_EXTRUDER) - // Mixing extruders apply advance lead proportionally - MIXING_STEPPERS_LOOP(j) { - int steps = delta_adv_steps * current_block->step_event_count / current_block->mix_event_count[j]; - e_steps[j] += steps; - current_adv_steps[j] += steps; - } - #else - // For most extruders, advance the single E stepper - e_steps[TOOL_E_INDEX] += delta_adv_steps; - current_adv_steps[TOOL_E_INDEX] += delta_adv_steps; - #endif - } - #elif ENABLED(ADVANCE) // Always count the unified E axis @@ -432,7 +416,7 @@ void Stepper::isr() { #if ENABLED(MIXING_EXTRUDER) // Step mixing steppers proportionally - bool dir = motor_direction(E_AXIS); + const bool dir = motor_direction(E_AXIS); MIXING_STEPPERS_LOOP(j) { counter_m[j] += current_block->steps[E_AXIS]; if (counter_m[j] > 0) { @@ -536,6 +520,21 @@ void Stepper::isr() { } } + #if ENABLED(LIN_ADVANCE) + if (current_block->use_advance_lead) { + int delta_adv_steps = (((long)extruder_advance_k * current_estep_rate[TOOL_E_INDEX]) >> 9) - current_adv_steps[TOOL_E_INDEX]; + current_adv_steps[TOOL_E_INDEX] += delta_adv_steps; + #if ENABLED(MIXING_EXTRUDER) + // Mixing extruders apply advance lead proportionally + MIXING_STEPPERS_LOOP(j) + e_steps[j] += delta_adv_steps * current_block->step_event_count / current_block->mix_event_count[j]; + #else + // For most extruders, advance the single E stepper + e_steps[TOOL_E_INDEX] += delta_adv_steps; + #endif + } + #endif + #if ENABLED(ADVANCE) || ENABLED(LIN_ADVANCE) // If we have esteps to execute, fire the next advance_isr "now" if (e_steps[TOOL_E_INDEX]) OCR0A = TCNT0 + 2; @@ -593,7 +592,7 @@ void Stepper::isr() { #endif // ADVANCE or LIN_ADVANCE #if ENABLED(ADVANCE) || ENABLED(LIN_ADVANCE) - eISR_Rate = (timer >> 2) * step_loops / abs(e_steps[TOOL_E_INDEX]); + eISR_Rate = (timer >> 3) * step_loops / abs(e_steps[TOOL_E_INDEX]); //>> 3 is divide by 8. Reason: Timer 0 runs at 16/8=2MHz, Timer 1 at 16/64=0.25MHz. ==> 2/0.25=8. #endif } else if (step_events_completed > (uint32_t)current_block->decelerate_after) { @@ -643,7 +642,7 @@ void Stepper::isr() { #endif // ADVANCE or LIN_ADVANCE #if ENABLED(ADVANCE) || ENABLED(LIN_ADVANCE) - eISR_Rate = (timer >> 2) * step_loops / abs(e_steps[TOOL_E_INDEX]); + eISR_Rate = (timer >> 3) * step_loops / abs(e_steps[TOOL_E_INDEX]); #endif } else { @@ -653,7 +652,7 @@ void Stepper::isr() { if (current_block->use_advance_lead) current_estep_rate[TOOL_E_INDEX] = final_estep_rate; - eISR_Rate = (OCR1A_nominal >> 2) * step_loops_nominal / abs(e_steps[TOOL_E_INDEX]); + eISR_Rate = (OCR1A_nominal >> 3) * step_loops_nominal / abs(e_steps[TOOL_E_INDEX]); #endif diff --git a/Marlin/stepper.h b/Marlin/stepper.h index 3e31f82d1..5b6b144f0 100644 --- a/Marlin/stepper.h +++ b/Marlin/stepper.h @@ -108,7 +108,7 @@ class Stepper { static unsigned char old_OCR0A; static volatile unsigned char eISR_Rate; #if ENABLED(LIN_ADVANCE) - static volatile long e_steps[E_STEPPERS]; + static volatile int e_steps[E_STEPPERS]; static int extruder_advance_k; static int final_estep_rate; static int current_estep_rate[E_STEPPERS]; // Actual extruder speed [steps/s] From 3752d9aca8f72ac397623669405ee739bfb7c53c Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Sun, 9 Oct 2016 10:06:31 -0500 Subject: [PATCH 2/4] Fix timer comments --- Marlin/stepper.cpp | 24 +++++++++++++++++++----- Marlin/temperature.cpp | 25 ++++++++++++++++++++----- 2 files changed, 39 insertions(+), 10 deletions(-) diff --git a/Marlin/stepper.cpp b/Marlin/stepper.cpp index 7889b8a88..ebe55b9ad 100644 --- a/Marlin/stepper.cpp +++ b/Marlin/stepper.cpp @@ -311,8 +311,20 @@ void Stepper::set_directions() { #endif // !ADVANCE && !LIN_ADVANCE } -// "The Stepper Driver Interrupt" - This timer interrupt is the workhorse. -// It pops blocks from the block_buffer and executes them by pulsing the stepper pins appropriately. +/** + * Stepper Driver Interrupt + * + * Directly pulses the stepper motors at high frequency. + * Timer 1 runs at a base frequency of 2MHz, with this ISR using OCR1A compare mode. + * + * OCR1A Frequency + * 1 2 MHz + * 50 40 KHz + * 100 20 KHz - capped max rate + * 200 10 KHz - nominal max rate + * 2000 1 KHz - sleep rate + * 4000 500 Hz - init rate + */ ISR(TIMER1_COMPA_vect) { Stepper::isr(); } void Stepper::isr() { @@ -323,7 +335,7 @@ void Stepper::isr() { if ((cleaning_buffer_counter == 1) && (SD_FINISHED_STEPPERRELEASE)) enqueue_and_echo_commands_P(PSTR(SD_FINISHED_RELEASECOMMAND)); #endif cleaning_buffer_counter--; - OCR1A = 200; + OCR1A = 200; // Run at max speed - 10 KHz return; } @@ -348,7 +360,7 @@ void Stepper::isr() { #if ENABLED(Z_LATE_ENABLE) if (current_block->steps[Z_AXIS] > 0) { enable_z(); - OCR1A = 2000; //1ms wait + OCR1A = 2000; // Run at slow speed - 1 KHz return; } #endif @@ -358,7 +370,7 @@ void Stepper::isr() { // #endif } else { - OCR1A = 2000; // 1kHz. + OCR1A = 2000; // Run at slow speed - 1 KHz return; } } @@ -903,6 +915,7 @@ void Stepper::init() { // output mode = 00 (disconnected) TCCR1A &= ~(3 << COM1A0); TCCR1A &= ~(3 << COM1B0); + // Set the timer pre-scaler // Generally we use a divider of 8, resulting in a 2MHz timer // frequency on a 16MHz MCU. If you are going to change this, be @@ -910,6 +923,7 @@ void Stepper::init() { // create_speed_lookuptable.py TCCR1B = (TCCR1B & ~(0x07 << CS10)) | (2 << CS10); + // Init Stepper ISR to 122 Hz for quick starting OCR1A = 0x4000; TCNT1 = 0; ENABLE_STEPPER_DRIVER_INTERRUPT(); diff --git a/Marlin/temperature.cpp b/Marlin/temperature.cpp index 42f68f438..285871a87 100644 --- a/Marlin/temperature.cpp +++ b/Marlin/temperature.cpp @@ -1371,7 +1371,7 @@ void Temperature::set_current_temp_raw() { * Timer 0 is shared with millies so don't change the prescaler. * * This ISR uses the compare method so it runs at the base - * frequency (16 MHz / 256 = 62500 Hz), but at the TCNT0 set + * frequency (16 MHz / 64 / 256 = 976.5625 Hz), but at the TCNT0 set * in OCR0B above (128 or halfway between OVFs). * * - Manage PWM to all the heaters and fan @@ -1485,9 +1485,16 @@ void Temperature::isr() { #endif #endif - // 488.28 Hz (or 1:976.56, 2:1953.12, 3:3906.25, 4:7812.5, 5:7812.5 6:15625, 6:15625 7:31250) + // SOFT_PWM_SCALE to frequency: + // + // 0: 16000000/64/256/128 = 7.6294 Hz + // 1: / 64 = 15.2588 Hz + // 2: / 32 = 30.5176 Hz + // 3: / 16 = 61.0352 Hz + // 4: / 8 = 122.0703 Hz + // 5: / 4 = 244.1406 Hz pwm_count += _BV(SOFT_PWM_SCALE); - pwm_count &= 0x7f; + pwm_count &= 0x7F; #else // SLOW_PWM_HEATERS @@ -1586,10 +1593,18 @@ void Temperature::isr() { #endif #endif //FAN_SOFT_PWM + // SOFT_PWM_SCALE to frequency: + // + // 0: 16000000/64/256/128 = 7.6294 Hz + // 1: / 64 = 15.2588 Hz + // 2: / 32 = 30.5176 Hz + // 3: / 16 = 61.0352 Hz + // 4: / 8 = 122.0703 Hz + // 5: / 4 = 244.1406 Hz pwm_count += _BV(SOFT_PWM_SCALE); - pwm_count &= 0x7f; + pwm_count &= 0x7F; - // increment slow_pwm_count only every 64 pwm_count circa 65.5ms + // increment slow_pwm_count only every 64 pwm_count (e.g., every 8s) if ((pwm_count % 64) == 0) { slow_pwm_count++; slow_pwm_count &= 0x7f; From e4d2662d819052ec417d332eba411aa948575414 Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Sun, 9 Oct 2016 12:21:05 -0500 Subject: [PATCH 3/4] Use some macros in M48 --- Marlin/Marlin_main.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Marlin/Marlin_main.cpp b/Marlin/Marlin_main.cpp index 5a4338af7..5d0e0e3c8 100755 --- a/Marlin/Marlin_main.cpp +++ b/Marlin/Marlin_main.cpp @@ -4899,8 +4899,8 @@ inline void gcode_M42() { for (uint8_t j = 0; j <= n; j++) sum += sample_set[j]; mean = sum / (n + 1); - if(sample_set[n] < min) min = sample_set[n]; - if(sample_set[n] > max) max = sample_set[n]; + NOMORE(min, sample_set[n]); + NOLESS(max, sample_set[n]); /** * Now, use that mean to calculate the standard deviation for the @@ -4956,7 +4956,6 @@ inline void gcode_M42() { SERIAL_PROTOCOLPGM("Standard Deviation: "); SERIAL_PROTOCOL_F(sigma, 6); SERIAL_EOL; - SERIAL_EOL; clean_up_after_endstop_or_probe_move(); From ff6b23cb0fc370e89d948e78e50e2f16e54d6b0f Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Sun, 9 Oct 2016 13:00:00 -0500 Subject: [PATCH 4/4] Fix an issue with shifted LCD lines --- Marlin/ultralcd.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Marlin/ultralcd.cpp b/Marlin/ultralcd.cpp index 2cf83a609..bcedace90 100755 --- a/Marlin/ultralcd.cpp +++ b/Marlin/ultralcd.cpp @@ -223,7 +223,7 @@ uint8_t lcdDrawUpdate = LCDVIEW_CLEAR_CALL_REDRAW; // Set when the LCD needs to static int8_t _countedItems = 0; \ int8_t encoderLine = encoderPosition / ENCODER_STEPS_PER_MENU_ITEM; \ if (_countedItems > 0 && encoderLine >= _countedItems - LIMIT) { \ - encoderLine = _countedItems - LIMIT; \ + encoderLine = max(0, _countedItems - LIMIT); \ encoderPosition = encoderLine * (ENCODER_STEPS_PER_MENU_ITEM); \ }