Merge pull request #4980 from thinkyhead/rc_lin_update

LIN_ADVANCE bug fix and optimization

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();

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],
@@ -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;
     }
   }
@@ -391,7 +403,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 +413,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 +428,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 +532,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 +604,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 +654,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 +664,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
 
@@ -904,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
@@ -911,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();

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]

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;

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); \
     }