Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix race with sys_rt_exec_state by breaking out bools - v2 #740

Open
wants to merge 1 commit into
base: Devt
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions Grbl_Esp32/Custom/CoreXY.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,9 @@ bool user_defined_homing(uint8_t cycle_mask) {
}
st_prep_buffer(); // Check and prep segment buffer. NOTE: Should take no longer than 200us.
// Exit routines: No time to run protocol_execute_realtime() in this loop.
if (sys_rt_exec_state.bit.safetyDoor || sys_rt_exec_state.bit.reset || cycle_stop) {
if (sys_safetyDoor || sys_reset || sys_cycleStop) {
ExecState rt_exec_state;
rt_exec_state.value = sys_rt_exec_state.value;
rt_exec_state.value = sys_get_rt_exec_state().value;
// Homing failure condition: Reset issued during cycle.
if (rt_exec_state.bit.reset) {
sys_rt_exec_alarm = ExecAlarm::HomingFailReset;
Expand All @@ -163,7 +163,7 @@ bool user_defined_homing(uint8_t cycle_mask) {
sys_rt_exec_alarm = ExecAlarm::HomingFailPulloff;
}
// Homing failure condition: Limit switch not found during approach.
if (approach && cycle_stop) {
if (approach && rt_exec_state.bit.cycleStop) {
sys_rt_exec_alarm = ExecAlarm::HomingFailApproach;
}

Expand All @@ -174,7 +174,7 @@ bool user_defined_homing(uint8_t cycle_mask) {
return true;
} else {
// Pull-off motion complete. Disable CYCLE_STOP from executing.
cycle_stop = false;
sys_cycleStop = false;
break;
}
}
Expand Down
2 changes: 1 addition & 1 deletion Grbl_Esp32/src/Exec.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
struct ExecStateBits {
uint8_t statusReport : 1;
uint8_t cycleStart : 1;
uint8_t cycleStop : 1; // Unused, per cycle_stop variable
uint8_t cycleStop : 1;
uint8_t feedHold : 1;
uint8_t reset : 1;
uint8_t safetyDoor : 1;
Expand Down
2 changes: 1 addition & 1 deletion Grbl_Esp32/src/GCode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1563,7 +1563,7 @@ Error gc_execute_line(char* line, uint8_t client) {
case ProgramFlow::Paused:
protocol_buffer_synchronize(); // Sync and finish all remaining buffered motions before moving on.
if (sys.state != State::CheckMode) {
sys_rt_exec_state.bit.feedHold = true; // Use feed hold for program pause.
sys_feedHold = true; // Use feed hold for program pause.
protocol_execute_realtime(); // Execute suspend.
}
break;
Expand Down
12 changes: 10 additions & 2 deletions Grbl_Esp32/src/Grbl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,18 @@ static void reset_variables() {
memset(sys_probe_position, 0, sizeof(sys_probe_position)); // Clear probe position.

sys_probe_state = Probe::Off;
sys_rt_exec_state.value = 0;

sys_statusReport = false;
sys_cycleStart = false;
sys_cycleStop = false;
sys_feedHold = false;
sys_reset = false;
sys_safetyDoor = false;
sys_motionCancel = false;
sys_sleep = false;

sys_rt_exec_accessory_override.value = 0;
sys_rt_exec_alarm = ExecAlarm::None;
cycle_stop = false;
sys_rt_f_override = FeedOverride::Default;
sys_rt_r_override = RapidOverride::Default;
sys_rt_s_override = SpindleSpeedOverride::Default;
Expand Down
10 changes: 5 additions & 5 deletions Grbl_Esp32/src/Limits.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -173,9 +173,9 @@ void limits_go_home(uint8_t cycle_mask) {
}
st_prep_buffer(); // Check and prep segment buffer. NOTE: Should take no longer than 200us.
// Exit routines: No time to run protocol_execute_realtime() in this loop.
if (sys_rt_exec_state.bit.safetyDoor || sys_rt_exec_state.bit.reset || cycle_stop) {
if (sys_safetyDoor || sys_reset || sys_cycleStop) {
ExecState rt_exec_state;
rt_exec_state.value = sys_rt_exec_state.value;
rt_exec_state.value = sys_get_rt_exec_state().value;
// Homing failure condition: Reset issued during cycle.
if (rt_exec_state.bit.reset) {
sys_rt_exec_alarm = ExecAlarm::HomingFailReset;
Expand All @@ -189,7 +189,7 @@ void limits_go_home(uint8_t cycle_mask) {
sys_rt_exec_alarm = ExecAlarm::HomingFailPulloff;
}
// Homing failure condition: Limit switch not found during approach.
if (approach && cycle_stop) {
if (approach && rt_exec_state.bit.cycleStop) {
sys_rt_exec_alarm = ExecAlarm::HomingFailApproach;
}

Expand All @@ -200,7 +200,7 @@ void limits_go_home(uint8_t cycle_mask) {
return;
} else {
// Pull-off motion complete. Disable CYCLE_STOP from executing.
cycle_stop = false;
sys_cycleStop = false;
break;
}
}
Expand Down Expand Up @@ -343,7 +343,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_rt_exec_state.bit.feedHold = true;
sys_feedHold = true;
do {
protocol_execute_realtime();
if (sys.abort) {
Expand Down
6 changes: 3 additions & 3 deletions Grbl_Esp32/src/MotionControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ GCUpdatePos mc_probe_cycle(float* target, plan_line_data_t* pl_data, uint8_t par
// Activate the probing state monitor in the stepper module.
sys_probe_state = Probe::Active;
// Perform probing cycle. Wait here until probe is triggered or motion completes.
sys_rt_exec_state.bit.cycleStart = true;
sys_cycleStart = true;
do {
protocol_execute_realtime();
if (sys.abort) {
Expand Down Expand Up @@ -498,8 +498,8 @@ void mc_override_ctrl_update(uint8_t override_state) {
// realtime abort command and hard limits. So, keep to a minimum.
void mc_reset() {
// Only this function can set the system reset. Helps prevent multiple kill calls.
if (!sys_rt_exec_state.bit.reset) {
sys_rt_exec_state.bit.reset = true;
if (!sys_reset) {
sys_reset = true;
// Kill spindle and coolant.
spindle->stop();
coolant_stop();
Expand Down
2 changes: 1 addition & 1 deletion Grbl_Esp32/src/Probe.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,6 @@ void probe_state_monitor() {
if (probe_get_state() ^ is_probe_away) {
sys_probe_state = Probe::Off;
memcpy(sys_probe_position, sys_position, sizeof(sys_position));
sys_rt_exec_state.bit.motionCancel = true;
sys_motionCancel = true;
}
}
2 changes: 1 addition & 1 deletion Grbl_Esp32/src/ProcessSettings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ Error home_c(const char* value, WebUI::AuthenticationLevel auth_level, WebUI::ES
return home(bit(C_AXIS));
}
Error sleep_grbl(const char* value, WebUI::AuthenticationLevel auth_level, WebUI::ESPResponseStream* out) {
sys_rt_exec_state.bit.sleep = true;
sys_sleep = true;
return Error::Ok;
}
Error get_report_build_info(const char* value, WebUI::AuthenticationLevel auth_level, WebUI::ESPResponseStream* out) {
Expand Down
38 changes: 19 additions & 19 deletions Grbl_Esp32/src/Protocol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ void protocol_main_loop() {
// Check if the safety door is open.
sys.state = State::Idle;
if (system_check_safety_door_ajar()) {
sys_rt_exec_state.bit.safetyDoor = true;
sys_safetyDoor = true;
protocol_execute_realtime(); // Enter safety door mode. Should return as IDLE state.
}
// All systems go!
Expand Down Expand Up @@ -223,7 +223,7 @@ void protocol_buffer_synchronize() {
// execute calls a buffer sync, or the planner buffer is full and ready to go.
void protocol_auto_cycle_start() {
if (plan_get_current_block() != NULL) { // Check if there are any blocks in the buffer.
sys_rt_exec_state.bit.cycleStart = true; // If so, execute them!
sys_cycleStart = true; // If so, execute them!
}
}

Expand All @@ -236,8 +236,8 @@ void protocol_auto_cycle_start() {
// handles them, removing the need to define more computationally-expensive volatile variables. This
// also provides a controlled way to execute certain tasks without having two or more instances of
// the same task, such as the planner recalculating the buffer upon a feedhold or overrides.
// NOTE: The sys_rt_exec_state.bit variable flags are set by any process, step or serial interrupts, pinouts,
// limit switches, or the main program.
// NOTE: The sys_* (reset, feedHold, safetyDoor, reset, motionCancel, cycleStart, cycleStop) variable
// flags are set by any process, step or serial interrupts, pinouts, limit switches, or the main program.
void protocol_execute_realtime() {
protocol_exec_rt_system();
if (sys.suspend.value) {
Expand All @@ -259,20 +259,20 @@ void protocol_exec_rt_system() {
// Halt everything upon a critical event flag. Currently hard and soft limits flag this.
if ((alarm == ExecAlarm::HardLimit) || (alarm == ExecAlarm::SoftLimit)) {
report_feedback_message(Message::CriticalEvent);
sys_rt_exec_state.bit.reset = false; // Disable any existing reset
sys_reset = false; // Disable any existing reset
do {
// Block everything, except reset and status reports, until user issues reset or power
// cycles. Hard limits typically occur while unattended or not paying attention. Gives
// the user and a GUI time to do what is needed before resetting, like killing the
// incoming stream. The same could be said about soft limits. While the position is not
// lost, continued streaming could cause a serious crash if by chance it gets executed.
} while (!sys_rt_exec_state.bit.reset);
} while (!sys_reset);
}
sys_rt_exec_alarm = ExecAlarm::None;
}
ExecState rt_exec_state;
rt_exec_state.value = sys_rt_exec_state.value; // Copy volatile sys_rt_exec_state.
if (rt_exec_state.value != 0 || cycle_stop) { // Test if any bits are on
rt_exec_state.value = sys_get_rt_exec_state().value; // Copy volatile sys_rt_exec_state.
if (rt_exec_state.value != 0) { // Test if any bits are on
// Execute system abort.
if (rt_exec_state.bit.reset) {
sys.abort = true; // Only place this is set true.
Expand All @@ -281,7 +281,7 @@ void protocol_exec_rt_system() {
// Execute and serial print status
if (rt_exec_state.bit.statusReport) {
report_realtime_status(CLIENT_ALL);
sys_rt_exec_state.bit.statusReport = false;
sys_statusReport = false;
}
// NOTE: Once hold is initiated, the system immediately enters a suspend state to block all
// main program processes until either reset or resumed. This ensures a hold completes safely.
Expand Down Expand Up @@ -315,15 +315,15 @@ void protocol_exec_rt_system() {
if (sys.state != State::Jog) {
sys.suspend.bit.motionCancel = true; // NOTE: State is State::Cycle.
}
sys_rt_exec_state.bit.motionCancel = false;
sys_motionCancel = false;
}
// Execute a feed hold with deceleration, if required. Then, suspend system.
if (rt_exec_state.bit.feedHold) {
// Block SAFETY_DOOR, JOG, and SLEEP states from changing to HOLD state.
if (!(sys.state == State::SafetyDoor || sys.state == State::Jog || sys.state == State::Sleep)) {
sys.state = State::Hold;
}
sys_rt_exec_state.bit.feedHold = false;
sys_feedHold = false;
}
// Execute a safety door stop with a feed hold and disable spindle/coolant.
// NOTE: Safety door differs from feed holds by stopping everything no matter state, disables powered
Expand Down Expand Up @@ -355,7 +355,7 @@ void protocol_exec_rt_system() {
if (sys.state != State::Sleep) {
sys.state = State::SafetyDoor;
}
sys_rt_exec_state.bit.safetyDoor = false;
sys_safetyDoor = false;
}
// NOTE: This flag doesn't change when the door closes, unlike sys.state. Ensures any parking motions
// are executed if the door switch closes and the state returns to HOLD.
Expand All @@ -368,7 +368,7 @@ void protocol_exec_rt_system() {
sys.suspend.bit.holdComplete = true;
}
sys.state = State::Sleep;
sys_rt_exec_state.bit.sleep = false;
sys_sleep = false;
}
}
// Execute a cycle start by starting the stepper interrupt to begin executing the blocks in queue.
Expand Down Expand Up @@ -408,14 +408,14 @@ void protocol_exec_rt_system() {
}
}
}
sys_rt_exec_state.bit.cycleStart = false;
sys_cycleStart = false;
}
if (cycle_stop) {
if (rt_exec_state.bit.cycleStop) {
// Reinitializes the cycle plan and stepper system after a feed hold for a resume. Called by
// realtime command execution in the main program, ensuring that the planner re-plans safely.
// NOTE: Bresenham algorithm variables are still maintained through both the planner and stepper
// cycle reinitializations. The stepper path should continue exactly as if nothing has happened.
// NOTE: cycle_stop is set by the stepper subsystem when a cycle or feed hold completes.
// NOTE: cycleStop is set by the stepper subsystem when a cycle or feed hold completes.
if ((sys.state == State::Hold || sys.state == State::SafetyDoor || sys.state == State::Sleep) && !(sys.soft_limit) &&
!(sys.suspend.bit.jogCancel)) {
// Hold complete. Set to indicate ready to resume. Remain in HOLD or DOOR states until user
Expand Down Expand Up @@ -445,7 +445,7 @@ void protocol_exec_rt_system() {
sys.state = State::Idle;
}
}
cycle_stop = false;
sys_cycleStop = false;
}
}
// Execute overrides.
Expand Down Expand Up @@ -696,7 +696,7 @@ static void protocol_exec_rt_suspend() {
#endif
if (!sys.suspend.bit.restartRetract) {
sys.suspend.bit.restoreComplete = true;
sys_rt_exec_state.bit.cycleStart = true; // Set to resume program.
sys_cycleStart = true; // Set to resume program.
}
}
}
Expand Down Expand Up @@ -725,7 +725,7 @@ static void protocol_exec_rt_suspend() {
}
}
if (sys.spindle_stop_ovr.bit.restoreCycle) {
sys_rt_exec_state.bit.cycleStart = true; // Set to resume program.
sys_cycleStart = true; // Set to resume program.
}
sys.spindle_stop_ovr.value = 0; // Clear stop override state
}
Expand Down
8 changes: 4 additions & 4 deletions Grbl_Esp32/src/Serial.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -251,17 +251,17 @@ void execute_realtime_command(Cmd command, uint8_t client) {
report_realtime_status(client); // direct call instead of setting flag
break;
case Cmd::CycleStart:
sys_rt_exec_state.bit.cycleStart = true;
sys_cycleStart = true;
break;
case Cmd::FeedHold:
sys_rt_exec_state.bit.feedHold = true;
sys_feedHold = true;
break;
case Cmd::SafetyDoor:
sys_rt_exec_state.bit.safetyDoor = true;
sys_safetyDoor = true;
break;
case Cmd::JogCancel:
if (sys.state == State::Jog) { // Block all other states from invoking motion cancel.
sys_rt_exec_state.bit.motionCancel = true;
sys_motionCancel = true;
}
break;
case Cmd::DebugReport:
Expand Down
2 changes: 1 addition & 1 deletion Grbl_Esp32/src/Stepper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ static void stepper_pulse_func() {
spindle->set_rpm(0);
}
}
cycle_stop = true;
sys_cycleStop = true;
return; // Nothing to do but exit.
}
}
Expand Down
33 changes: 28 additions & 5 deletions Grbl_Esp32/src/System.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,18 @@ system_t sys;
int32_t sys_position[MAX_N_AXIS]; // Real-time machine (aka home) position vector in steps.
int32_t sys_probe_position[MAX_N_AXIS]; // Last probe position in machine coordinates and steps.
volatile Probe sys_probe_state; // Probing state value. Used to coordinate the probing cycle with stepper ISR.
volatile ExecState sys_rt_exec_state; // Global realtime executor bitflag variable for state management. See EXEC bitmasks.
volatile ExecAlarm sys_rt_exec_alarm; // Global realtime executor bitflag variable for setting various alarms.
volatile ExecAccessory sys_rt_exec_accessory_override; // Global realtime executor bitflag variable for spindle/coolant overrides.
volatile bool cycle_stop; // For state transitions, instead of bitflag

volatile bool sys_statusReport; // For state transitions, instead of bitflag
volatile bool sys_cycleStart;
volatile bool sys_cycleStop;
volatile bool sys_feedHold;
volatile bool sys_reset;
volatile bool sys_safetyDoor;
volatile bool sys_motionCancel;
volatile bool sys_sleep;

#ifdef DEBUG
volatile bool sys_rt_exec_debug;
#endif
Expand Down Expand Up @@ -251,11 +259,11 @@ void system_exec_control_pin(ControlPins pins) {
grbl_msg_sendf(CLIENT_SERIAL, MsgLevel::Info, "Reset via control pin");
mc_reset();
} else if (pins.bit.cycleStart) {
sys_rt_exec_state.bit.cycleStart = true;
sys_cycleStart = true;
} else if (pins.bit.feedHold) {
sys_rt_exec_state.bit.feedHold = true;
sys_feedHold = true;
} else if (pins.bit.safetyDoor) {
sys_rt_exec_state.bit.safetyDoor = true;
sys_safetyDoor = true;
} else if (pins.bit.macro0) {
user_defined_macro(0); // function must be implemented by user
} else if (pins.bit.macro1) {
Expand Down Expand Up @@ -327,6 +335,21 @@ uint8_t sys_calc_pwm_precision(uint32_t freq) {

return precision - 1;
}

ExecState sys_get_rt_exec_state() {
ExecState result;
result.value = 0;
result.bit.statusReport = sys_statusReport;
result.bit.cycleStart = sys_cycleStart;
result.bit.cycleStop = sys_cycleStop;
result.bit.feedHold = sys_feedHold;
result.bit.reset = sys_reset;
result.bit.safetyDoor = sys_safetyDoor;
result.bit.motionCancel = sys_motionCancel;
result.bit.sleep = sys_sleep;
return result;
}

void __attribute__((weak)) user_defined_macro(uint8_t index) {
// must be in Idle
if (sys.state != State::Idle) {
Expand Down
Loading