From 1014558c913148c25eedf0572703ad0186384731 Mon Sep 17 00:00:00 2001 From: Andrew <159703254+andrewsoonqn@users.noreply.github.com> Date: Fri, 31 Jul 2026 14:06:26 +0800 Subject: [PATCH] Keep printer-agent progress in sync Keep the shared task progress aligned with agent reports that lack Bambu cloud task identity. Release the lazily allocated task during reset to avoid leaks when machine objects reconnect. --- src/slic3r/GUI/DeviceManager.cpp | 27 +++- src/slic3r/GUI/DeviceManager.hpp | 1 + tests/slic3rutils/CMakeLists.txt | 1 + tests/slic3rutils/test_device_progress.cpp | 141 +++++++++++++++++++++ 4 files changed, 164 insertions(+), 6 deletions(-) create mode 100644 tests/slic3rutils/test_device_progress.cpp 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); +}