From b540a8eec8b4d34891d509d56bbb8ae137a2a1b4 Mon Sep 17 00:00:00 2001
From: Mitch Bradley <wmb@firmworks.com>
Date: Thu, 24 Jun 2021 19:07:13 -1000
Subject: [PATCH] Fixed stepper ISR

---
 Grbl_Esp32/src/Report.cpp  |  5 +++++
 Grbl_Esp32/src/Stepper.cpp | 34 ++++++++++++++++++++++++++++------
 Grbl_Esp32/src/Stepper.h   |  6 +-----
 3 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/Grbl_Esp32/src/Report.cpp b/Grbl_Esp32/src/Report.cpp
index 29a20b5c..473fffab 100644
--- a/Grbl_Esp32/src/Report.cpp
+++ b/Grbl_Esp32/src/Report.cpp
@@ -54,6 +54,7 @@
 #include "SettingsDefinitions.h"
 #include "Limits.h"                      // limits_get_state
 #include "Planner.h"                     // plan_get_block_buffer_available
+#include "Stepper.h"                     // step_count
 #include "WebUI/NotificationsService.h"  // WebUI::notificationsservice
 #include "WebUI/WifiConfig.h"            // wifi_config
 #include "WebUI/TelnetServer.h"          // WebUI::telnet_server
@@ -760,6 +761,10 @@ void report_realtime_status(uint8_t client) {
         config->_sdCard->get_current_filename(temp);
         strcat(status, temp);
     }
+#ifdef DEBUG_STEPPER_ISR
+    sprintf(temp, "|cnt:%d", step_count);
+    strcat(status, temp);
+#endif
 #ifdef DEBUG_REPORT_HEAP
     sprintf(temp, "|Heap:%d", esp.getHeapSize());
     strcat(status, temp);
diff --git a/Grbl_Esp32/src/Stepper.cpp b/Grbl_Esp32/src/Stepper.cpp
index 3f29cdc6..8de4da53 100644
--- a/Grbl_Esp32/src/Stepper.cpp
+++ b/Grbl_Esp32/src/Stepper.cpp
@@ -190,6 +190,12 @@ EnumItem stepTypes[] = {
 
 */
 
+// Forward references to functions that are only used herein
+static void Stepper_Timer_WritePeriod(uint16_t timerTicks);
+static void Stepper_Timer_Init();
+static void Stepper_Timer_Start();
+static void Stepper_Timer_Stop();
+
 // Stepper timer configuration
 const int   stepTimerNumber = 0;
 hw_timer_t* stepTimer;  // Handle
@@ -199,6 +205,11 @@ const bool autoReload = true;
 
 static void stepper_pulse_func();
 
+// Counts stepper ISR invocations.  This variable can be inspected
+// from the mainline code to determine if the stepper ISR is running,
+// since printing from the ISR is not a good idea.
+uint32_t step_count = 0;
+
 // TODO: Replace direct updating of the int32 position counters in the ISR somehow. Perhaps use smaller
 // int8 variables and update position counters only when a segment completes. This can get complicated
 // with probing and homing cycles that require true real-time positions.
@@ -209,13 +220,24 @@ void IRAM_ATTR onStepperDriverTimer() {
 
     bool expected = false;
     if (busy.compare_exchange_strong(expected, true)) {
-        stepper_pulse_func();
+        ++step_count;
 
+        // Using autoReload results is less timing jitter so it is
+        // probably best to have it on.  We keep the variable for
+        // convenience in debugging.
         if (!autoReload) {
             timerWrite(stepTimer, 0ULL);
-            timerAlarmEnable(stepTimer);
         }
 
+        // It is tempting to defer this until after stepper_pulse_func(),
+        // but if stepper_pulse_func() determines that no more stepping
+        // is required and disables the timer, then that will be undone
+        // if the re-enable happens afterwards.
+
+        timerAlarmEnable(stepTimer);
+
+        stepper_pulse_func();
+
         busy.store(false);
     }
 }
@@ -906,7 +928,7 @@ float st_get_realtime_rate() {
 }
 
 // The argument is in units of ticks of the timer that generates ISRs
-void IRAM_ATTR Stepper_Timer_WritePeriod(uint16_t timerTicks) {
+static void IRAM_ATTR Stepper_Timer_WritePeriod(uint16_t timerTicks) {
     if (config->_stepType == ST_I2S_STREAM) {
         // 1 tick = fTimers / fStepperTimer
         // Pulse ISR is called for each tick of alarm_val.
@@ -917,14 +939,14 @@ void IRAM_ATTR Stepper_Timer_WritePeriod(uint16_t timerTicks) {
     }
 }
 
-void IRAM_ATTR Stepper_Timer_Init() {
+static void IRAM_ATTR Stepper_Timer_Init() {
     const bool isEdge  = false;
     const bool countUp = true;
     stepTimer          = timerBegin(stepTimerNumber, fTimers / fStepperTimer, countUp);
     timerAttachInterrupt(stepTimer, onStepperDriverTimer, isEdge);
 }
 
-void IRAM_ATTR Stepper_Timer_Start() {
+static void IRAM_ATTR Stepper_Timer_Start() {
     if (config->_stepType == ST_I2S_STREAM) {
         i2s_out_set_stepping();
     } else {
@@ -933,7 +955,7 @@ void IRAM_ATTR Stepper_Timer_Start() {
     }
 }
 
-void IRAM_ATTR Stepper_Timer_Stop() {
+static void IRAM_ATTR Stepper_Timer_Stop() {
     if (config->_stepType == ST_I2S_STREAM) {
         i2s_out_set_passthrough();
     } else {
diff --git a/Grbl_Esp32/src/Stepper.h b/Grbl_Esp32/src/Stepper.h
index 06881676..b5d24cad 100644
--- a/Grbl_Esp32/src/Stepper.h
+++ b/Grbl_Esp32/src/Stepper.h
@@ -72,9 +72,5 @@ bool get_stepper_disable();  // returns the state of the pin
 void set_stepper_pins_on(uint8_t onMask);
 void set_direction_pins_on(uint8_t onMask);
 
-void Stepper_Timer_WritePeriod(uint16_t timerTicks);
-void Stepper_Timer_Init();
-void Stepper_Timer_Start();
-void Stepper_Timer_Stop();
-
 extern EnumItem stepTypes[];
+extern uint32_t step_count;