From 015d5fa19105f90694ca46986fa264893bc7155a Mon Sep 17 00:00:00 2001 From: Krzysztof Foltman Date: Thu, 26 Jun 2014 23:06:26 +0100 Subject: [PATCH] Fixed atomic access to flags in sys.execute. This seems to fix the bug that caused Grbl to hang during some operations, especially jogging. --- limits.c | 10 +++++----- motion_control.c | 10 +++++----- nuts_bolts.h | 6 +++--- probe.c | 2 +- protocol.c | 2 +- serial.c | 6 +++--- system.c | 4 ++-- 7 files changed, 20 insertions(+), 20 deletions(-) diff --git a/limits.c b/limits.c index c31988a..f6e5f01 100644 --- a/limits.c +++ b/limits.c @@ -86,7 +86,7 @@ ISR(LIMIT_INT_vect) // DEFAULT: Limit pin change interrupt process. if (sys.state != STATE_ALARM) { if (bit_isfalse(sys.execute,EXEC_ALARM)) { mc_reset(); // Initiate system kill. - sys.execute |= (EXEC_ALARM | EXEC_CRIT_EVENT); // Indicate hard limit critical event + bit_true(sys.execute, (EXEC_ALARM | EXEC_CRIT_EVENT)); // Indicate hard limit critical event } } } @@ -103,7 +103,7 @@ ISR(WDT_vect) // Watchdog timer ISR if (bit_istrue(settings.flags,BITFLAG_INVERT_LIMIT_PINS)) { bits ^= LIMIT_MASK; } if (bits & LIMIT_MASK) { mc_reset(); // Initiate system kill. - sys.execute |= (EXEC_ALARM | EXEC_CRIT_EVENT); // Indicate hard limit critical event + bit_true(sys.execute, (EXEC_ALARM | EXEC_CRIT_EVENT)); // Indicate hard limit critical event } } } @@ -238,7 +238,7 @@ void limits_go_home(uint8_t cycle_mask) // Initiate pull-off using main motion control routines. // TODO : Clean up state routines so that this motion still shows homing state. sys.state = STATE_QUEUED; - sys.execute |= EXEC_CYCLE_START; + bit_true(sys.execute, EXEC_CYCLE_START); protocol_execute_runtime(); protocol_buffer_synchronize(); // Complete pull-off motion. @@ -259,7 +259,7 @@ void limits_soft_check(float *target) // workspace volume so just come to a controlled stop so position is not lost. When complete // enter alarm mode. if (sys.state == STATE_CYCLE) { - sys.execute |= EXEC_FEED_HOLD; + bit_true(sys.execute, EXEC_FEED_HOLD); do { protocol_execute_runtime(); if (sys.abort) { return; } @@ -267,7 +267,7 @@ void limits_soft_check(float *target) } mc_reset(); // Issue system reset and ensure spindle and coolant are shutdown. - sys.execute |= (EXEC_ALARM | EXEC_CRIT_EVENT); // Indicate soft limit critical event + bit_true(sys.execute, (EXEC_ALARM | EXEC_CRIT_EVENT)); // Indicate soft limit critical event protocol_execute_runtime(); // Execute to enter critical event loop and system abort return; diff --git a/motion_control.c b/motion_control.c index adc4a73..c57dfea 100644 --- a/motion_control.c +++ b/motion_control.c @@ -288,13 +288,13 @@ void mc_probe_cycle(float *target, float feed_rate, uint8_t invert_feed_rate) // NOTE: Parser error-checking ensures the probe isn't already closed/triggered. sys.probe_state = PROBE_ACTIVE; - sys.execute |= EXEC_CYCLE_START; + bit_true(sys.execute, EXEC_CYCLE_START); do { protocol_execute_runtime(); if (sys.abort) { return; } // Check for system abort } while ((sys.state != STATE_IDLE) && (sys.state != STATE_QUEUED)); - if (sys.probe_state == PROBE_ACTIVE) { sys.execute |= EXEC_CRIT_EVENT; } + if (sys.probe_state == PROBE_ACTIVE) { bit_true(sys.execute, EXEC_CRIT_EVENT); } protocol_execute_runtime(); // Check and execute run-time commands if (sys.abort) { return; } // Check for system abort @@ -316,7 +316,7 @@ void mc_probe_cycle(float *target, float feed_rate, uint8_t invert_feed_rate) mc_line(target, feed_rate, invert_feed_rate); // Bypass mc_line(). Directly plan homing motion. #endif - sys.execute |= EXEC_CYCLE_START; + bit_true(sys.execute, EXEC_CYCLE_START); protocol_buffer_synchronize(); // Complete pull-off motion. if (sys.abort) { return; } // Did not complete. Alarm state set by mc_alarm. @@ -337,7 +337,7 @@ void mc_reset() { // Only this function can set the system reset. Helps prevent multiple kill calls. if (bit_isfalse(sys.execute, EXEC_RESET)) { - sys.execute |= EXEC_RESET; + bit_true(sys.execute, EXEC_RESET); // Kill spindle and coolant. spindle_stop(); @@ -348,7 +348,7 @@ void mc_reset() // the steppers enabled by avoiding the go_idle call altogether, unless the motion state is // violated, by which, all bets are off. if (sys.state & (STATE_CYCLE | STATE_HOLD | STATE_HOMING)) { - sys.execute |= EXEC_ALARM; // Flag main program to execute alarm state. + bit_true(sys.execute, EXEC_ALARM); // Flag main program to execute alarm state. st_go_idle(); // Force kill steppers. Position has likely been lost. } } diff --git a/nuts_bolts.h b/nuts_bolts.h index d68af30..5cab6f4 100644 --- a/nuts_bolts.h +++ b/nuts_bolts.h @@ -44,9 +44,9 @@ // Bit field and masking macros #define bit(n) (1 << n) -#define bit_true(x,mask) (x |= mask) -#define bit_false(x,mask) (x &= ~mask) -#define bit_toggle(x,mask) (x ^= mask) +#define bit_true(x,mask) {uint8_t sreg = SREG; cli(); (x) |= (mask); SREG = sreg; } +#define bit_false(x,mask) {uint8_t sreg = SREG; cli(); (x) &= ~(mask); SREG = sreg; } +#define bit_toggle(x,mask) {uint8_t sreg = SREG; cli(); (x) ^= (mask); SREG = sreg; } #define bit_istrue(x,mask) ((x & mask) != 0) #define bit_isfalse(x,mask) ((x & mask) == 0) diff --git a/probe.c b/probe.c index cad0449..84ec01d 100644 --- a/probe.c +++ b/probe.c @@ -45,7 +45,7 @@ void probe_state_monitor() if (probe_get_state()) { sys.probe_state = PROBE_OFF; memcpy(sys.probe_position, sys.position, sizeof(float)*N_AXIS); - sys.execute |= EXEC_FEED_HOLD; + bit_true(sys.execute, EXEC_FEED_HOLD); } } } diff --git a/protocol.c b/protocol.c index c97829b..3f22489 100644 --- a/protocol.c +++ b/protocol.c @@ -288,4 +288,4 @@ void protocol_buffer_synchronize() // NOTE: This function is called from the main loop and mc_line() only and executes when one of // two conditions exist respectively: There are no more blocks sent (i.e. streaming is finished, // single commands), or the planner buffer is full and ready to go. -void protocol_auto_cycle_start() { if (sys.auto_start) { sys.execute |= EXEC_CYCLE_START; } } +void protocol_auto_cycle_start() { if (sys.auto_start) { bit_true(sys.execute, EXEC_CYCLE_START); } } diff --git a/serial.c b/serial.c index 29e361c..3b1a8d5 100644 --- a/serial.c +++ b/serial.c @@ -157,9 +157,9 @@ ISR(SERIAL_RX) // Pick off runtime command characters directly from the serial stream. These characters are // not passed into the buffer, but these set system state flag bits for runtime execution. switch (data) { - case CMD_STATUS_REPORT: sys.execute |= EXEC_STATUS_REPORT; break; // Set as true - case CMD_CYCLE_START: sys.execute |= EXEC_CYCLE_START; break; // Set as true - case CMD_FEED_HOLD: sys.execute |= EXEC_FEED_HOLD; break; // Set as true + case CMD_STATUS_REPORT: bit_true(sys.execute, EXEC_STATUS_REPORT); break; // Set as true + case CMD_CYCLE_START: bit_true(sys.execute, EXEC_CYCLE_START); break; // Set as true + case CMD_FEED_HOLD: bit_true(sys.execute, EXEC_FEED_HOLD); break; // Set as true case CMD_RESET: mc_reset(); break; // Call motion control reset routine. default: // Write character to buffer next_head = rx_buffer_head + 1; diff --git a/system.c b/system.c index 181a07b..83177d2 100644 --- a/system.c +++ b/system.c @@ -46,9 +46,9 @@ ISR(PINOUT_INT_vect) if (bit_isfalse(PINOUT_PIN,bit(PIN_RESET))) { mc_reset(); } else if (bit_isfalse(PINOUT_PIN,bit(PIN_FEED_HOLD))) { - sys.execute |= EXEC_FEED_HOLD; + bit_true(sys.execute, EXEC_FEED_HOLD); } else if (bit_isfalse(PINOUT_PIN,bit(PIN_CYCLE_START))) { - sys.execute |= EXEC_CYCLE_START; + bit_true(sys.execute, EXEC_CYCLE_START); } } }