From 91d438353d8bb6aff23e26ba1ca1bf6b3bdd7f9c Mon Sep 17 00:00:00 2001 From: Adrian Scripca Date: Wed, 10 May 2023 16:14:57 +0300 Subject: [PATCH] Solved inter-thread communication issues by feeding commands over a queue. --- docs/todo.txt | 5 +- grbl_machine.cpp | 164 +++++++++++++++++++++++++------ grbl_machine.h | 115 +++++++++++++++++++--- main.cpp | 246 +++++++++++++++++++++++------------------------ 4 files changed, 356 insertions(+), 174 deletions(-) diff --git a/docs/todo.txt b/docs/todo.txt index 9f64cea..3f3b606 100644 --- a/docs/todo.txt +++ b/docs/todo.txt @@ -33,8 +33,11 @@ DONE - store Z offset and place it in the heightmap - if line contains a Z coordinate, update the Z according to the X and Y - if line does not contain a Z coordinate, add a Z coordinate according to the X and Y -Solve bug in which the probing data does not get rendered properly as it gets probed. +DONE -Let's get rid of the pesky inter-thread communication issue which is causing a lot of confusion. +DONE - Solve bug in which the probing data does not get rendered properly as it gets probed. + Bug: query coordinate systems after first z probe. +Refactor: see if we can unify machine state line handling and remove duplication Edge finding - prerequisites diff --git a/grbl_machine.cpp b/grbl_machine.cpp index fe6d974..87db1a4 100644 --- a/grbl_machine.cpp +++ b/grbl_machine.cpp @@ -560,10 +560,6 @@ void grbl::machine::cancel_jog() const { pipe->send_single_char_command(0x85); } -void grbl::machine::set_listener(grbl::machine_listener *l) { - listener = l; -} - void grbl::machine::request_unlock() { pipe->send("$X"); } @@ -615,7 +611,8 @@ void grbl::machine::zero_offset(int which) { pipe->send("$#"); awaiting_responses++; while (awaiting_responses > 0); - listener->on_parameters_reloaded(); + + push_event(std::make_shared()); } void grbl::machine::zero_offset_axis(int offset_index, int axis) { @@ -627,7 +624,8 @@ void grbl::machine::zero_offset_axis(int offset_index, int axis) { pipe->send("$#"); awaiting_responses++; while (awaiting_responses > 0); - listener->on_parameters_reloaded(); + + push_event(std::make_shared()); } void grbl::machine::go_to_zero(bool x, bool y, bool z) { @@ -695,6 +693,23 @@ void grbl::machine::probe_heightmap(grbl::heightmap& grid) { switch_to_state(grbl_machine_state::heightmap_probing); } +void grbl::machine::push_event(std::shared_ptr event) { + std::scoped_lock lock(event_mutex); + events.push_back(event); +} + +std::shared_ptr grbl::machine::pop_event() { + // TODO: make this more efficient + std::scoped_lock lock(event_mutex); + if (events.empty()) + return nullptr; + else { + auto result = events.front(); + events.pop_front(); + return result; + } +} + bool grbl::jog_state::no_jogging() const { return !(up_pressed || down_pressed || left_pressed || right_pressed || z_up_pressed || z_down_pressed); } @@ -719,7 +734,7 @@ void grbl::machine_state_connect::on_exit(grbl::machine *m) { void grbl::machine_state_connect::on_line_received(std::string line) { // std::cerr << "Should not get content while connecting!" << std::endl; if (starts_with(line, "Grbl")) { - cnc->listener->on_banner(line); + cnc->push_event(std::make_shared(line)); cnc->switch_to_state(grbl_machine_state::init); } } @@ -732,7 +747,7 @@ void grbl::machine_state_init::on_enter(grbl::machine *m) { } void grbl::machine_state_init::on_exit(grbl::machine *m) { - cnc->listener->on_init_completed(); + cnc->push_event(std::make_shared()); } void grbl::machine_state_init::on_line_received(std::string line) { @@ -808,15 +823,17 @@ void grbl::machine_state_idle::on_line_received(std::string line) { cnc->awaiting_responses--; } } else if (starts_with(line, "Grbl")) { - cnc->listener->on_banner(line); + cnc->push_event(std::make_shared(line)); cnc->reset_machine_state(); } else if (starts_with(line, "<")) { cnc->last_report = parse_status_report(line, cnc->last_report); - cnc->listener->on_realtime_status_report(cnc->last_report); + cnc->push_event(std::make_shared(cnc->last_report)); } else if (starts_with(line, "[MSG:")) { - cnc->listener->on_message(line.substr(5, line.size() - 6)); + auto message = line.substr(5, line.size() - 6); + cnc->push_event(std::make_shared(message)); } else if (starts_with(line, "ALARM:")) { - cnc->listener->on_alarm(std::stoi(line.substr(6))); + auto alarm = std::stoi(line.substr(6)); + cnc->push_event(std::make_shared(alarm)); } else if (starts_with(line, "$")) { auto pieces = split_string(line, "="); cnc->settings[pieces[0]] = pieces[1]; @@ -835,7 +852,7 @@ void grbl::machine_state_idle::on_line_received(std::string line) { probe_coords[i] = std::stof(coords_as_string[i]); } bool probe_touched = pieces[2] == "1"; - cnc->listener->on_probe_result(probe_touched, probe_coords); + cnc->push_event(std::make_shared(probe_touched, probe_coords)); } } } @@ -884,9 +901,15 @@ void grbl::machine_state_check_program::move_to_next_check_stage() { break; case check_stage::disable_check_mode: if (check_failed) { - cnc->listener->on_check_completed(false, cnc->executed_instructions - 1, check_error); + bool success = false; + size_t failed_idx = cnc->executed_instructions - 1; + size_t error = check_error; + cnc->push_event(std::make_shared(success, failed_idx, error)); } else { - cnc->listener->on_check_completed(true, 0, 0); + bool success = true; + size_t failed_idx = 0; + size_t error = 0; + cnc->push_event(std::make_shared(success, failed_idx, error)); } cnc->switch_to_state(grbl_machine_state::idle); break; @@ -908,15 +931,17 @@ void grbl::machine_state_check_program::on_line_received(std::string line) { move_to_next_check_stage(); } else if (starts_with(line, "Grbl")) { - cnc->listener->on_banner(line); + cnc->push_event(std::make_shared(line)); cnc->reset_machine_state(); } else if (starts_with(line, "<")) { cnc->last_report = parse_status_report(line, cnc->last_report); - cnc->listener->on_realtime_status_report(cnc->last_report); + cnc->push_event(std::make_shared(cnc->last_report)); } else if (starts_with(line, "[MSG:")) { - cnc->listener->on_message(line.substr(5, line.size() - 6)); + auto message = line.substr(5, line.size() - 6); + cnc->push_event(std::make_shared(message)); } else if (starts_with(line, "ALARM:")) { - cnc->listener->on_alarm(std::stoi(line.substr(6))); + auto alarm = std::stoi(line.substr(6)); + cnc->push_event(std::make_shared(alarm)); } } @@ -970,9 +995,15 @@ void grbl::machine_state_run_program::move_to_next_run_stage() { case run_stage::run_program: if (run_failed || !continue_program()) { if (run_failed) { - cnc->listener->on_run_completed(false, cnc->executed_instructions - 1, run_error); + bool success = false; + size_t failed_index = cnc->executed_instructions - 1; + size_t error = run_error; + cnc->push_event(std::make_shared(success, failed_index, error)); } else { - cnc->listener->on_run_completed(true, 0, 0); + bool success = true; + size_t failed_index = 0; + size_t error = 0; + cnc->push_event(std::make_shared(success, failed_index, error)); } cnc->switch_to_state(grbl_machine_state::idle); } @@ -1013,20 +1044,21 @@ void grbl::machine_state_run_program::on_line_received(std::string line) { move_to_next_run_stage(); } else if (starts_with(line, "Grbl")) { - cnc->listener->on_banner(line); + cnc->push_event(std::make_shared(line)); cnc->reset_machine_state(); } else if (starts_with(line, "<")) { cnc->last_report = parse_status_report(line, cnc->last_report); - cnc->listener->on_realtime_status_report(cnc->last_report); + cnc->push_event(std::make_shared(cnc->last_report)); } else if (starts_with(line, "[MSG:")) { - cnc->listener->on_message(line.substr(5, line.size() - 6)); + auto message = line.substr(5, line.size() - 6); + cnc->push_event(std::make_shared(message)); } else if (starts_with(line, "ALARM:")) { - cnc->listener->on_alarm(std::stoi(line.substr(6))); + auto alarm = std::stoi(line.substr(6)); + cnc->push_event(std::make_shared(alarm)); } } // heightmap probing - void grbl::machine_state_heightmap_probing::on_connected(grbl::machine *m) { } @@ -1062,15 +1094,17 @@ void grbl::machine_state_heightmap_probing::on_line_received(std::string line) { error = std::stoi(line.substr(6)); move_to_next_stage(); } else if (starts_with(line, "Grbl")) { - cnc->listener->on_banner(line); + cnc->push_event(std::make_shared(line)); cnc->reset_machine_state(); } else if (starts_with(line, "<")) { cnc->last_report = parse_status_report(line, cnc->last_report); - cnc->listener->on_realtime_status_report(cnc->last_report); + cnc->push_event(std::make_shared(cnc->last_report)); } else if (starts_with(line, "[MSG:")) { - cnc->listener->on_message(line.substr(5, line.size() - 6)); + auto message = line.substr(5, line.size() - 6); + cnc->push_event(std::make_shared(message)); } else if (starts_with(line, "ALARM:")) { - cnc->listener->on_alarm(std::stoi(line.substr(6))); + auto alarm = std::stoi(line.substr(6)); + cnc->push_event(std::make_shared(alarm)); } else if (starts_with(line, "[PRB")) { std::cout << "received PRB" << std::endl; line = line.substr(1, line.size() - 2); @@ -1132,7 +1166,8 @@ void grbl::machine_state_heightmap_probing::move_to_next_stage() { auto delta_z = current_z - z_zero_in_mpos; std::cout << "Z[" << probed_locations << "] = " << delta_z << std::endl; grid->vertices[probed_locations].z = delta_z; - cnc->listener->on_heightmap_probe_acquired(grid); + + cnc->push_event(std::make_shared(grid, probed_locations)); } } @@ -1167,3 +1202,70 @@ void grbl::machine_state_heightmap_probing::move_to_next_stage() { break; } } + + +grbl::machine_event_connect::machine_event_connect() { + machine_event::type = machine_event_type::connected; +} + +grbl::machine_event_disconnect::machine_event_disconnect() { + machine_event::type = machine_event_type::disconnected; +} + +grbl::machine_event_report_received::machine_event_report_received(const grbl::realtime_status_report& r) + : report(r) { + machine_event::type = machine_event_type::report_received; +} + +grbl::machine_event_banner::machine_event_banner(const std::string& b) + : banner{b} { + machine_event::type = machine_event_type::banner; +} + +grbl::machine_event_message::machine_event_message(const std::string& m) + : message{m} { + machine_event::type = machine_event_type::message; +} + +grbl::machine_event_alarm::machine_event_alarm(int code) + : alarm(code) { + machine_event::type = machine_event_type::alarm; +} + +grbl::machine_event_init_completed::machine_event_init_completed() { + machine_event::type = machine_event_type::init_completed; +} + +grbl::machine_event_run_completed::machine_event_run_completed(bool success, size_t failed_index, size_t error) + : success(success), + failed_index(failed_index), + error(error) { + machine_event::type = machine_event_type::run_completed; +} + +grbl::machine_event_check_completed::machine_event_check_completed(bool success, size_t failed_idx, size_t error) + : success(success), + failed_index(failed_idx), + error(error) { + machine_event::type = machine_event_type::check_completed; +} + +grbl::machine_event_settings_reloaded::machine_event_settings_reloaded() { + machine_event::type = machine_event_type::settings_reloaded; +} + +grbl::machine_event_parameters_reloaded::machine_event_parameters_reloaded() { + machine_event::type = machine_event_type::parameters_reloaded; +} + +grbl::machine_event_probe_result::machine_event_probe_result(bool touched, const float *coords) + : probe_touched(touched), + probe_coords{coords[0], coords[1], coords[2]} { + machine_event::type = machine_event_type::probe_result; +} + +grbl::machine_event_heightmap_probe_acquired::machine_event_heightmap_probe_acquired(grbl::heightmap *g, size_t location) + : grid(g), + probed_location(location) { + machine_event::type = machine_event_type::heightmap_probe_acquired; +} diff --git a/grbl_machine.h b/grbl_machine.h index d04bad0..795addc 100644 --- a/grbl_machine.h +++ b/grbl_machine.h @@ -2,6 +2,8 @@ #include #include +#include +#include #include "grbl_communication.h" #include "grbl.h" #include "heightmap.h" @@ -116,20 +118,92 @@ static bool operator!=(const jog_state& a, const jog_state& b) { return !(a == b); } -struct machine_listener { - virtual void on_connected() = 0; - virtual void on_disconnected() = 0; - virtual void on_realtime_status_report(realtime_status_report) = 0; - virtual void on_banner(std::string line) = 0; - virtual void on_message(std::string message) = 0; - virtual void on_alarm(int alarm) = 0; - virtual void on_init_completed() = 0; - virtual void on_run_completed(bool success, size_t failed_index, size_t error) = 0; - virtual void on_check_completed(bool success, size_t failed_index, size_t error) = 0; - virtual void on_settings_reloaded() = 0; - virtual void on_parameters_reloaded() = 0; - virtual void on_probe_result(bool probe_touched, float probe_coords[3]) = 0; - virtual void on_heightmap_probe_acquired(heightmap* grid) = 0; +enum class machine_event_type { + connected, + disconnected, + report_received, + banner, + message, + alarm, + init_completed, + run_completed, + check_completed, + settings_reloaded, + parameters_reloaded, + probe_result, + heightmap_probe_acquired, +}; + +struct machine_event { + machine_event_type type; + // TODO: ewwww + virtual void omg() {} +}; + +struct machine_event_connect : public machine_event { + machine_event_connect(); +}; + +struct machine_event_disconnect : public machine_event { + machine_event_disconnect(); +}; + +struct machine_event_report_received : public machine_event { + explicit machine_event_report_received(const realtime_status_report& report); + realtime_status_report report; +}; + +struct machine_event_banner : public machine_event { + explicit machine_event_banner(const std::string& banner); + std::string banner; +}; + +struct machine_event_message : public machine_event { + explicit machine_event_message(const std::string& message); + std::string message; +}; + +struct machine_event_alarm : public machine_event { + explicit machine_event_alarm(int code); + int alarm; +}; + +struct machine_event_init_completed : public machine_event { + machine_event_init_completed(); +}; + +struct machine_event_run_completed : public machine_event { + machine_event_run_completed(bool success, size_t failed_index, size_t error); + bool success; + size_t failed_index; + size_t error; +}; + +struct machine_event_check_completed : public machine_event { + machine_event_check_completed(bool success, size_t failed_idx, size_t error); + bool success; + size_t failed_index; + size_t error; +}; + +struct machine_event_settings_reloaded : public machine_event { + machine_event_settings_reloaded(); +}; + +struct machine_event_parameters_reloaded : public machine_event { + machine_event_parameters_reloaded(); +}; + +struct machine_event_probe_result : public machine_event { + machine_event_probe_result(bool touched, const float *coords); + bool probe_touched; + float probe_coords[3]; +}; + +struct machine_event_heightmap_probe_acquired : public machine_event { + machine_event_heightmap_probe_acquired(heightmap *g, size_t location); + grbl::heightmap *grid; + size_t probed_location; }; @@ -278,7 +352,6 @@ struct machine : public transport_callbacks { machine(); ~machine(); - void set_listener(grbl::machine_listener *listener); void connect(); void set_work_offset(std::string work_offset); @@ -311,7 +384,18 @@ struct machine : public transport_callbacks { void start_z_probe(float min_z, float feed_rate); void probe_heightmap(grbl::heightmap& grid); + + void push_event(std::shared_ptr event); + + // pops nullptr if queue is empty + std::shared_ptr pop_event(); + + protected: + + std::mutex event_mutex; + std::deque> events; + void on_connected(transport *transport) override; void on_disconnected(transport *transport) override; void on_line_received(std::string line, transport *transport) override; @@ -337,7 +421,6 @@ protected: volatile size_t awaiting_responses = 0; realtime_status_report last_report{}; - machine_listener *listener = nullptr; transport *pipe = nullptr; grbl_machine_state state = grbl_machine_state::disconnected; program running_program; diff --git a/main.cpp b/main.cpp index 62132a3..7cc423c 100644 --- a/main.cpp +++ b/main.cpp @@ -49,15 +49,15 @@ using namespace nanogui; grbl::machine cnc{}; -class SenderApp : public Screen, public grbl::machine_listener { +class SenderApp : public Screen { public: Window *window; grbl::jog_state jog; TextBox *txt_status; - TextBox *txtMessage; + TextBox *txt_message; nanogui::Color color_red = nanogui::Color(255, 0, 0, 255); - nanogui::Color colGreen = nanogui::Color(0, 255, 0, 255); + nanogui::Color color_green = nanogui::Color(0, 255, 0, 255); int last_alarm = 0; grbl::program pgm; Button *btn_load_program, *btn_check_program, *btn_run_program; @@ -242,8 +242,8 @@ public: // status_text_holder->add