diff --git a/src/slic3r/GUI/DeviceManager.cpp b/src/slic3r/GUI/DeviceManager.cpp index 279ac316d1..1354909afd 100644 --- a/src/slic3r/GUI/DeviceManager.cpp +++ b/src/slic3r/GUI/DeviceManager.cpp @@ -2631,7 +2631,12 @@ void MachineObject::reset() vt_slot.erase(vt_slot.begin() + 1); } } - subtask_ = nullptr; + // why: reset reuses MachineObject, so release its lazy subtask + // before dropping the pointer to prevent reconnect leaks. + if (subtask_) { + delete subtask_; + subtask_ = nullptr; + } has_extra_flow_type = false; m_partskip_ids.clear(); } @@ -2641,6 +2646,20 @@ void MachineObject::set_print_state(std::string status) print_status = status; } +// why: printer agents can report progress without BBL cloud task identity. +void MachineObject::update_print_progress(const json& value) +{ + if (value.is_string()) + mc_print_percent = stoi(value.get()); + else if (value.is_number_integer()) + mc_print_percent = value.get(); + else + return; + + if (BBLSubTask* curr_task = get_subtask()) + curr_task->task_progress = mc_print_percent; +} + int MachineObject::connect(bool use_openssl) { if (get_dev_ip().empty()) return -1; @@ -3319,10 +3338,7 @@ int MachineObject::parse_json(std::string tunnel, std::string payload, bool key_ print_type = jj["print_type"].get(); } if (jj.contains("mc_percent")) { - if (jj["mc_percent"].is_string()) - mc_print_percent = stoi(j["print"]["mc_percent"].get()); - else if (jj["mc_percent"].is_number_integer()) - mc_print_percent = j["print"]["mc_percent"].get(); + update_print_progress(jj["mc_percent"]); } if (jj.contains("mc_print_sub_stage")) { if (jj["mc_print_sub_stage"].is_number_integer()) @@ -3540,7 +3556,6 @@ int MachineObject::parse_json(std::string tunnel, std::string payload, bool key_ update_slice_info(jj["project_id"].get(), jj["profile_id"].get(), jj["subtask_id"].get(), plate_index); BBLSubTask* curr_task = get_subtask(); if (curr_task) { - curr_task->task_progress = mc_print_percent; curr_task->printing_status = print_status; curr_task->task_id = jj["subtask_id"].get(); } diff --git a/src/slic3r/GUI/DeviceManager.hpp b/src/slic3r/GUI/DeviceManager.hpp index 18f8599381..42f53864c2 100644 --- a/src/slic3r/GUI/DeviceManager.hpp +++ b/src/slic3r/GUI/DeviceManager.hpp @@ -894,6 +894,7 @@ public: static bool is_in_printing_status(std::string status); void set_print_state(std::string status); + void update_print_progress(const json& value); bool is_connected(); bool is_connecting(); diff --git a/tests/slic3rutils/CMakeLists.txt b/tests/slic3rutils/CMakeLists.txt index d04be0273a..16c19910ee 100644 --- a/tests/slic3rutils/CMakeLists.txt +++ b/tests/slic3rutils/CMakeLists.txt @@ -2,6 +2,7 @@ get_filename_component(_TEST_NAME ${CMAKE_CURRENT_LIST_DIR} NAME) add_executable(${_TEST_NAME}_tests ${_TEST_NAME}_tests_main.cpp test_dev_mapping.cpp + test_device_progress.cpp test_network_versions.cpp test_action_source.cpp test_plugin_host_api.cpp diff --git a/tests/slic3rutils/test_device_progress.cpp b/tests/slic3rutils/test_device_progress.cpp new file mode 100644 index 0000000000..be58dc6030 --- /dev/null +++ b/tests/slic3rutils/test_device_progress.cpp @@ -0,0 +1,141 @@ +// why: match the GUI include order to avoid rpcndr.h byte/std::byte +// ambiguity in the Windows COM headers. +// why: wx/timer.h must precede DeviceManager.hpp because +// DeviceErrorDialog.hpp uses wxTimerEvent. +#ifdef WIN32 + #ifndef WIN32_LEAN_AND_MEAN + #define WIN32_LEAN_AND_MEAN + #endif + #ifndef NOMINMAX + #define NOMINMAX + #endif + #include +#endif + +#include + +#include + +#include + +#include "slic3r/GUI/DeviceManager.hpp" + +#include + +using json = nlohmann::json; +using namespace Slic3r; + +TEST_CASE("Integer progress reaches the shared subtask", "[DeviceManager][Progress]") +{ + MachineObject machine(nullptr, nullptr, "test", "test-device", "127.0.0.1"); + + machine.update_print_progress(json(37)); + + REQUIRE(machine.mc_print_percent == 37); + BBLSubTask* subtask = machine.get_subtask(); + REQUIRE(subtask != nullptr); + CHECK(subtask->task_progress == 37); +} + +TEST_CASE("String progress reaches the shared subtask", "[DeviceManager][Progress]") +{ + MachineObject machine(nullptr, nullptr, "test", "test-device", "127.0.0.1"); + + machine.update_print_progress(json("41")); + + REQUIRE(machine.mc_print_percent == 41); + BBLSubTask* subtask = machine.get_subtask(); + REQUIRE(subtask != nullptr); + CHECK(subtask->task_progress == 41); +} + +TEST_CASE("Floating-point progress preserves the previous shared value", "[DeviceManager][Progress]") +{ + MachineObject machine(nullptr, nullptr, "test", "test-device", "127.0.0.1"); + + machine.update_print_progress(json(29)); + REQUIRE(machine.mc_print_percent == 29); + BBLSubTask* subtask = machine.get_subtask(); + REQUIRE(subtask != nullptr); + REQUIRE(subtask->task_progress == 29); + + machine.update_print_progress(json(29.5)); + BBLSubTask* current_subtask = machine.get_subtask(); + REQUIRE(current_subtask != nullptr); + REQUIRE(current_subtask == subtask); + CHECK(machine.mc_print_percent == 29); + CHECK(current_subtask->task_progress == 29); +} + +TEST_CASE("Unsupported progress values leave a fresh machine unchanged", "[DeviceManager][Progress]") +{ + SECTION("boolean") { + MachineObject machine(nullptr, nullptr, "test", "test-device", "127.0.0.1"); + REQUIRE(machine.subtask_ == nullptr); + + machine.update_print_progress(json(true)); + + CHECK(machine.mc_print_percent == 0); + CHECK(machine.subtask_ == nullptr); + } + + SECTION("null") { + MachineObject machine(nullptr, nullptr, "test", "test-device", "127.0.0.1"); + REQUIRE(machine.subtask_ == nullptr); + + machine.update_print_progress(json(nullptr)); + + CHECK(machine.mc_print_percent == 0); + CHECK(machine.subtask_ == nullptr); + } + + SECTION("object") { + MachineObject machine(nullptr, nullptr, "test", "test-device", "127.0.0.1"); + REQUIRE(machine.subtask_ == nullptr); + + machine.update_print_progress(json::object()); + + CHECK(machine.mc_print_percent == 0); + CHECK(machine.subtask_ == nullptr); + } + + SECTION("array") { + MachineObject machine(nullptr, nullptr, "test", "test-device", "127.0.0.1"); + REQUIRE(machine.subtask_ == nullptr); + + machine.update_print_progress(json::array()); + + CHECK(machine.mc_print_percent == 0); + CHECK(machine.subtask_ == nullptr); + } +} + +TEST_CASE("Malformed string progress leaves a fresh machine unchanged", "[DeviceManager][Progress]") +{ + MachineObject machine(nullptr, nullptr, "test", "test-device", "127.0.0.1"); + REQUIRE(machine.subtask_ == nullptr); + + CHECK_THROWS_AS(machine.update_print_progress(json("not-a-percent")), std::invalid_argument); + CHECK(machine.mc_print_percent == 0); + CHECK(machine.subtask_ == nullptr); +} + +TEST_CASE("Zero progress replaces active shared progress", "[DeviceManager][Progress]") +{ + MachineObject machine(nullptr, nullptr, "test", "test-device", "127.0.0.1"); + + machine.update_print_progress(json(63)); + BBLSubTask* subtask = machine.get_subtask(); + REQUIRE(subtask != nullptr); + REQUIRE(machine.mc_print_percent == 63); + REQUIRE(subtask->task_progress == 63); + + machine.set_print_state("FAILED"); + machine.update_print_progress(json(0)); + + BBLSubTask* current_subtask = machine.get_subtask(); + REQUIRE(current_subtask != nullptr); + REQUIRE(current_subtask == subtask); + REQUIRE(machine.mc_print_percent == 0); + CHECK(current_subtask->task_progress == 0); +}