fix deadlock/race condition

diff --git a/src/libs/StepTicker.cpp b/src/libs/StepTicker.cpp
index 30c4cfc..61bec02 100644 (file)
--- a/src/libs/StepTicker.cpp
+++ b/src/libs/StepTicker.cpp
@@ -45,15 +45,16 @@ StepTicker::StepTicker(){
 
     // Default start values
     this->a_move_finished = false;
-    this->pending_sv = false;
+    this->do_move_finished = false;
     this->reset_step_pins = false;
-    this->set_frequency(0.001);
+    this->set_frequency(100000);
     this->set_reset_delay(100);
     this->set_acceleration_ticks_per_second(1000);
     this->last_duration = 0;
     this->num_motors= 0;
     this->active_motor.reset();
     this->acceleration_tick_cnt= 0;
+    this->do_acceleration_tick= false;
 
     NVIC_EnableIRQ(TIMER0_IRQn);     // Enable interrupt handler
     NVIC_EnableIRQ(TIMER1_IRQn);     // Enable interrupt handler
@@ -101,7 +102,6 @@ void StepTicker::signal_a_move_finished(){
                 // }
         }
     }
-    this->a_move_finished = false;
 
     _isr_context = false;
 }
@@ -134,16 +134,11 @@ extern "C" void PendSV_Handler(void) {
 
 // slightly lower priority than TIMER0, the whole end of block/start of block is done here allowing the timer to continue ticking
 // also handles the acceleration tick
-void StepTicker::PendSV_IRQHandler (void){
-    if(this->do_acceleration_tick) {
-        this->do_acceleration_tick= false;
-        // call registered acceleration handlers
-        for (size_t i = 0; i < acceleration_tick_handlers.size(); ++i) {
-            acceleration_tick_handlers[i]();
-        }
-    }
+void StepTicker::PendSV_IRQHandler (void) {
+
+    if(this->do_move_finished) {
+        this->do_move_finished= false;
 
-    if(this->a_move_finished) {
         #ifdef STEPTICKER_DEBUG_PIN
         stepticker_debug_pin= 1;
         #endif
@@ -155,7 +150,13 @@ void StepTicker::PendSV_IRQHandler (void){
         #endif
     }
 
-    this->pending_sv= false;
+    if(this->do_acceleration_tick) {
+        this->do_acceleration_tick= false;
+        // call registered acceleration handlers
+        for (size_t i = 0; i < acceleration_tick_handlers.size(); ++i) {
+            acceleration_tick_handlers[i]();
+        }
+    }
 }
 
 void StepTicker::TIMER0_IRQHandler (void){
@@ -183,9 +184,13 @@ void StepTicker::TIMER0_IRQHandler (void){
         this->do_acceleration_tick= true;
     }
 
+    if(this->a_move_finished) {
+        this->a_move_finished = false;
+        this->do_move_finished= 1;
+    }
+
     // If a move finished in this tick, we have to tell the actuator to act accordingly
-    if(!this->pending_sv && (this->a_move_finished || this->do_acceleration_tick)){
-        this->pending_sv= true; // don't multiple trigger pendsv
+    if(this->do_move_finished || this->do_acceleration_tick){
         // we delegate the slow stuff to the pendsv handler which will run as soon as this interrupt exits
         //NVIC_SetPendingIRQ(PendSV_IRQn); this doesn't work
         SCB->ICSR = 0x10000000; // SCB_ICSR_PENDSVSET_Msk;

diff --git a/src/libs/StepTicker.h b/src/libs/StepTicker.h
index 6e2a086..8efe2f7 100644 (file)
--- a/src/libs/StepTicker.h
+++ b/src/libs/StepTicker.h
@@ -53,7 +53,7 @@ class StepTicker{
         struct {
             uint8_t num_motors:5;
             volatile bool a_move_finished:1;
-            volatile bool pending_sv:1;
+            volatile bool do_move_finished:1;
             volatile bool do_acceleration_tick:1;
             bool reset_step_pins:1;
         };

diff --git a/src/main.cpp b/src/main.cpp
index e165be4..793ba71 100644 (file)
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -84,10 +84,6 @@ GPIO leds[5] = {
 };
 
 // debug pins, only used if defined in src/makefile
-#ifdef BLOCK_DEBUG_PIN
-GPIO block_debug_pin(BLOCK_DEBUG_PIN);
-#endif
-
 #ifdef STEPTICKER_DEBUG_PIN
 GPIO stepticker_debug_pin(STEPTICKER_DEBUG_PIN);
 #endif
@@ -100,10 +96,6 @@ void init() {
         leds[i]= 0;
     }
 
-#ifdef BLOCK_DEBUG_PIN
-    block_debug_pin.output();
-    block_debug_pin= 0;
-#endif
 #ifdef STEPTICKER_DEBUG_PIN
     stepticker_debug_pin.output();
     stepticker_debug_pin= 0;

diff --git a/src/makefile b/src/makefile
index bb5f9e4..c838bf6 100644 (file)
--- a/src/makefile
+++ b/src/makefile
@@ -53,11 +53,10 @@ endif
 # use c++11 features for the checksums and set default baud rate for serial uart
 DEFINES += -DCHECKSUM_USE_CPP -DDEFAULT_SERIAL_BAUD_RATE=$(DEFAULT_SERIAL_BAUD_RATE)
 
-# Set a Pin here that toggles high on block begin and low on block end for debugging with logic analyzer
-#DEFINES += -DBLOCK_DEBUG_PIN=P4_29
-
-# Set a Pin here that toggles on Stepticker overrun for debugging with logic analyzer
-DEFINES += -DSTEPTICKER_DEBUG_PIN=P4_29
+ifneq "$(STEPTICKER_DEBUG_PIN)" ""
+# Set a Pin here that toggles on end of move
+DEFINES += -DSTEPTICKER_DEBUG_PIN=$(STEPTICKER_DEBUG_PIN)
+endif
 
 # add any modules that you do not want included in the build
 export EXCLUDED_MODULES = tools/touchprobe

diff --git a/src/modules/robot/Block.cpp b/src/modules/robot/Block.cpp
index bca5c51..e0d798f 100644 (file)
--- a/src/modules/robot/Block.cpp
+++ b/src/modules/robot/Block.cpp
@@ -22,11 +22,6 @@
 using std::string;
 #include <vector>
 
-#ifdef BLOCK_DEBUG_PIN
-#include "gpio.h"
-extern GPIO block_debug_pin;
-#endif
-
 // A block represents a movement, it's length for each stepper motor, and the corresponding acceleration curves.
 // It's stacked on a queue, and that queue is then executed in order, to move the motors.
 // Most of the accel math is also done in this class
@@ -252,10 +247,6 @@ void Block::append_gcode(Gcode* gcode)
 
 void Block::begin()
 {
-#ifdef BLOCK_DEBUG_PIN
-    block_debug_pin= 1;
-#endif
-
     recalculate_flag = false;
 
     if (!is_ready)
@@ -291,20 +282,14 @@ void Block::take()
 // Mark the block as no longer taken by one module, go to next block if this free's it
 void Block::release()
 {
-    if (--this->times_taken <= 0)
-    {
+    if (--this->times_taken <= 0) {
         times_taken = 0;
-        if (is_ready)
-        {
-#ifdef BLOCK_DEBUG_PIN
-            block_debug_pin= 0;
-#endif
+        if (is_ready) {
             is_ready = false;
             THEKERNEL->call_event(ON_BLOCK_END, this);
 
             // ensure conveyor gets called last
             THEKERNEL->conveyor->on_block_end(this);
-
         }
     }
 }