From: Jim Morris Date: Sat, 29 Nov 2014 22:18:08 +0000 (-0800) Subject: fix deadlock/race condition X-Git-Url: http://git.hcoop.net/clinton/Smoothieware.git/commitdiff_plain/3b1acdaa8ca4c7e5fcb2b8671003e2c3ec9e9f65 fix deadlock/race condition --- diff --git a/src/libs/StepTicker.cpp b/src/libs/StepTicker.cpp index 30c4cfc7..61bec02e 100644 --- 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 6e2a086b..8efe2f72 100644 --- 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 e165be45..793ba71d 100644 --- 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 bb5f9e46..c838bf69 100644 --- 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 bca5c51a..e0d798f2 100644 --- a/src/modules/robot/Block.cpp +++ b/src/modules/robot/Block.cpp @@ -22,11 +22,6 @@ using std::string; #include -#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); - } } }