#13420 GCodeWriter Correct Z speed too high and add unit tests

#13402 test_gcodewriter.cpp add travel_speed_z tests

These tests fail with the legacy implementation of GCodeWriter::travel_to_xyz_force() because it does not decompose the speed into it's components according to the movement unit vector.

#13420 Correct Z speed too high
This commit is contained in:
jalapenopuzzle 2025-02-03 10:41:20 +11:00 committed by Lukas Matena
parent 462758153b
commit 419d857a4b
3 changed files with 332 additions and 18 deletions

View File

@ -358,20 +358,21 @@ std::string GCodeWriter::travel_to_xyz_force(const Vec3d &to, const std::string_
GCodeG1Formatter w;
w.emit_xyz(to);
double speed_z = this->config.travel_speed_z.value;
if (speed_z == 0.)
speed_z = this->config.travel_speed.value;
double speed = this->config.travel_speed.value;
const double speed_z = this->config.travel_speed_z.value;
const double distance_xy{(to.head<2>() - m_pos.head<2>()).norm()};
const double distnace_z{std::abs(to.z() - m_pos.z())};
const double time_z = distnace_z / speed_z;
const double time_xy = distance_xy / this->config.travel_speed.value;
const double factor = time_z > 0 ? time_xy / time_z : 1;
if (factor < 1) {
w.emit_f(std::floor((this->config.travel_speed.value * factor + (1 - factor) * speed_z) * 60.0));
} else {
w.emit_f(this->config.travel_speed.value * 60.0);
if (speed_z) {
const Vec3d move{to - m_pos};
const double distance{move.norm()};
const double abs_unit_vector_z{std::abs(move.z())/distance};
// De-compose speed into z vector component according to the movement unit vector.
const double speed_vector_z{abs_unit_vector_z * speed};
if (speed_vector_z > speed_z) {
// Re-compute speed so that the z component is exactly speed_z.
speed = speed_z / abs_unit_vector_z;
}
}
w.emit_f(speed * 60.0);
w.emit_comment(this->config.gcode_comments, comment);
m_pos = to;

View File

@ -5,6 +5,7 @@
#include <memory>
#include "libslic3r/GCode/GCodeWriter.hpp"
#include "libslic3r/GCodeReader.hpp"
using namespace Slic3r;
@ -35,6 +36,265 @@ SCENARIO("set_speed emits values with fixed-point output.", "[GCodeWriter]") {
}
}
void check_gcode_feedrate(const std::string& gcode, const GCodeConfig& config, double expected_speed) {
GCodeReader parser;
parser.parse_buffer(gcode, [&] (Slic3r::GCodeReader &self, const Slic3r::GCodeReader::GCodeLine &line) {
const double travel_speed = config.opt_float("travel_speed");
const double feedrate = line.has_f() ? line.f() : self.f();
CHECK(feedrate == Approx(expected_speed * 60).epsilon(GCodeFormatter::XYZ_EPSILON));
if (line.dist_Z(self) != 0) {
// lift move or lift + change layer
const double travel_speed_z = config.opt_float("travel_speed_z");
if (travel_speed_z) {
Vec3d move{line.dist_X(self), line.dist_Y(self), line.dist_Z(self)};
double move_u_z = move.z() / move.norm();
double travel_speed_ = std::abs(travel_speed_z / move_u_z);
INFO("move Z feedrate Z component is less than or equal to travel_speed_z");
CHECK(feedrate * std::abs(move_u_z) <= Approx(travel_speed_z * 60).epsilon(GCodeFormatter::XYZ_EPSILON));
if (travel_speed_ < travel_speed) {
INFO("move Z at travel speed Z");
CHECK(feedrate == Approx(travel_speed_ * 60).epsilon(GCodeFormatter::XYZ_EPSILON));
INFO("move Z feedrate Z component is equal to travel_speed_z");
CHECK(feedrate * std::abs(move_u_z) == Approx(travel_speed_z * 60).epsilon(GCodeFormatter::XYZ_EPSILON));
} else {
INFO("move Z at travel speed");
CHECK(feedrate == Approx(travel_speed * 60).epsilon(GCodeFormatter::XYZ_EPSILON));
}
} else {
INFO("move Z at travel speed");
CHECK(feedrate == Approx(travel_speed * 60).epsilon(GCodeFormatter::XYZ_EPSILON));
}
} else if (not line.extruding(self)) {
// normal move
INFO("move XY at travel speed");
CHECK(feedrate == Approx(travel_speed * 60));
}
});
}
SCENARIO("travel_speed_z is zero should use travel_speed.", "[GCodeWriter]") {
GIVEN("GCodeWriter instance") {
GCodeWriter writer;
WHEN("travel_speed_z is set to 0") {
writer.config.travel_speed.value = 1000;
writer.config.travel_speed_z.value = 0;
THEN("XYZ move feed rate should be equal to travel_speed") {
const Vec3d move{10, 10, 10};
const double speed = writer.config.travel_speed.value;
const Vec3d p1 = writer.get_position();
const Vec3d p2 = p1 + move;
const std::string result = writer.travel_to_xyz(p2);
check_gcode_feedrate(result, writer.config, speed);
}
}
}
}
SCENARIO("travel_speed_z is respected in Z speed component.", "[GCodeWriter]") {
GIVEN("GCodeWriter instance") {
GCodeWriter writer;
WHEN("travel_speed_z is set to 10") {
writer.config.travel_speed.value = 1000;
writer.config.travel_speed_z.value = 10;
THEN("Z move feed rate should be equal to travel_speed_z") {
const Vec3d move{0, 0, 10};
const double speed = writer.config.travel_speed_z.value;
const Vec3d p1 = writer.get_position();
const Vec3d p2 = p1 + move;
const std::string result = writer.travel_to_xyz(p2);
check_gcode_feedrate(result, writer.config, speed);
}
THEN("-Z move feed rate should be equal to travel_speed_z") {
const Vec3d move{0, 0, -10};
const double speed = writer.config.travel_speed_z.value;
const Vec3d p1 = writer.get_position();
const Vec3d p2 = p1 + move;
const std::string result = writer.travel_to_xyz(p2);
check_gcode_feedrate(result, writer.config, speed);
}
THEN("XY move feed rate should be equal to travel_speed") {
const Vec3d move{10, 10, 0};
const double speed = writer.config.travel_speed.value;
const Vec3d p1 = writer.get_position();
const Vec3d p2 = p1 + move;
const std::string result = writer.travel_to_xyz(p2);
check_gcode_feedrate(result, writer.config, speed);
}
THEN("-XY move feed rate should be equal to travel_speed") {
const Vec3d move{-10, 10, 0};
const double speed = writer.config.travel_speed.value;
const Vec3d p1 = writer.get_position();
const Vec3d p2 = p1 + move;
const std::string result = writer.travel_to_xyz(p2);
check_gcode_feedrate(result, writer.config, speed);
}
THEN("X-Y move feed rate should be equal to travel_speed") {
const Vec3d move{10, -10, 0};
const double speed = writer.config.travel_speed.value;
const Vec3d p1 = writer.get_position();
const Vec3d p2 = p1 + move;
const std::string result = writer.travel_to_xyz(p2);
check_gcode_feedrate(result, writer.config, speed);
}
THEN("-X-Y move feed rate should be equal to travel_speed") {
const Vec3d move{-10, -10, 0};
const double speed = writer.config.travel_speed.value;
const Vec3d p1 = writer.get_position();
const Vec3d p2 = p1 + move;
const std::string result = writer.travel_to_xyz(p2);
check_gcode_feedrate(result, writer.config, speed);
}
THEN("XZ move feed rate Z component should be equal to travel_speed_z") {
const Vec3d move{10, 0, 10};
const Vec3d move_u = move / move.norm();
const double speed = std::abs(writer.config.travel_speed_z.value / move_u.z());
Vec3d p1 = writer.get_position();
Vec3d p2 = p1 + move;
std::string result = writer.travel_to_xyz(p2);
check_gcode_feedrate(result, writer.config, speed);
}
THEN("-XZ move feed rate Z component should be equal to travel_speed_z") {
const Vec3d move{-10, 0, 10};
const Vec3d move_u = move / move.norm();
const double speed = std::abs(writer.config.travel_speed_z.value / move_u.z());
const Vec3d p1 = writer.get_position();
const Vec3d p2 = p1 + move;
const std::string result = writer.travel_to_xyz(p2);
check_gcode_feedrate(result, writer.config, speed);
}
THEN("X-Z move feed rate Z component should be equal to travel_speed_z") {
const Vec3d move{10, 0, -10};
const Vec3d move_u = move / move.norm();
const double speed = std::abs(writer.config.travel_speed_z.value / move_u.z());
const Vec3d p1 = writer.get_position();
const Vec3d p2 = p1 + move;
const std::string result = writer.travel_to_xyz(p2);
check_gcode_feedrate(result, writer.config, speed);
}
THEN("-X-Z move feed rate Z component should be equal to travel_speed_z") {
const Vec3d move{-10, 0, -10};
const Vec3d move_u = move / move.norm();
const double speed = std::abs(writer.config.travel_speed_z.value / move_u.z());
const Vec3d p1 = writer.get_position();
const Vec3d p2 = p1 + move;
const std::string result = writer.travel_to_xyz(p2);
check_gcode_feedrate(result, writer.config, speed);
}
THEN("YZ move feed rate Z component should be equal to travel_speed_z") {
const Vec3d move{0, 10, 10};
const Vec3d move_u = move / move.norm();
const double speed = std::abs(writer.config.travel_speed_z.value / move_u.z());
const Vec3d p1 = writer.get_position();
const Vec3d p2 = p1 + move;
const std::string result = writer.travel_to_xyz(p2);
check_gcode_feedrate(result, writer.config, speed);
}
THEN("-YZ move feed rate Z component should be equal to travel_speed_z") {
const Vec3d move{0, -10, 10};
const Vec3d move_u = move / move.norm();
const double speed = std::abs(writer.config.travel_speed_z.value / move_u.z());
const Vec3d p1 = writer.get_position();
const Vec3d p2 = p1 + move;
const std::string result = writer.travel_to_xyz(p2);
check_gcode_feedrate(result, writer.config, speed);
}
THEN("Y-Z move feed rate Z component should be equal to travel_speed_z") {
const Vec3d move{0, 10, -10};
const Vec3d move_u = move / move.norm();
const double speed = std::abs(writer.config.travel_speed_z.value / move_u.z());
const Vec3d p1 = writer.get_position();
const Vec3d p2 = p1 + move;
const std::string result = writer.travel_to_xyz(p2);
check_gcode_feedrate(result, writer.config, speed);
}
THEN("-Y-Z move feed rate Z component should be equal to travel_speed_z") {
const Vec3d move{0, -10, -10};
const Vec3d move_u = move / move.norm();
const double speed = std::abs(writer.config.travel_speed_z.value / move_u.z());
const Vec3d p1 = writer.get_position();
const Vec3d p2 = p1 + move;
const std::string result = writer.travel_to_xyz(p2);
check_gcode_feedrate(result, writer.config, speed);
}
THEN("XYZ move feed rate Z component should be equal to travel_speed_z") {
const Vec3d move{10, 10, 10};
const Vec3d move_u = move / move.norm();
const double speed = std::abs(writer.config.travel_speed_z.value / move_u.z());
const Vec3d p1 = writer.get_position();
const Vec3d p2 = p1 + move;
const std::string result = writer.travel_to_xyz(p2);
check_gcode_feedrate(result, writer.config, speed);
}
THEN("-XYZ move feed rate Z component should be equal to travel_speed_z") {
const Vec3d move{-10, 10, 10};
const Vec3d move_u = move / move.norm();
const double speed = std::abs(writer.config.travel_speed_z.value / move_u.z());
const Vec3d p1 = writer.get_position();
const Vec3d p2 = p1 + move;
const std::string result = writer.travel_to_xyz(p2);
check_gcode_feedrate(result, writer.config, speed);
}
THEN("X-YZ move feed rate Z component should be equal to travel_speed_z") {
const Vec3d move{10, -10, 10};
const Vec3d move_u = move / move.norm();
const double speed = std::abs(writer.config.travel_speed_z.value / move_u.z());
const Vec3d p1 = writer.get_position();
const Vec3d p2 = p1 + move;
const std::string result = writer.travel_to_xyz(p2);
check_gcode_feedrate(result, writer.config, speed);
}
THEN("-X-YZ move feed rate Z component should be equal to travel_speed_z") {
const Vec3d move{-10, -10, 10};
const Vec3d move_u = move / move.norm();
const double speed = std::abs(writer.config.travel_speed_z.value / move_u.z());
const Vec3d p1 = writer.get_position();
const Vec3d p2 = p1 + move;
const std::string result = writer.travel_to_xyz(p2);
check_gcode_feedrate(result, writer.config, speed);
}
THEN("XY-Z move feed rate Z component should be equal to travel_speed_z") {
const Vec3d move{10, 10, -10};
const Vec3d move_u = move / move.norm();
const double speed = std::abs(writer.config.travel_speed_z.value / move_u.z());
const Vec3d p1 = writer.get_position();
const Vec3d p2 = p1 + move;
const std::string result = writer.travel_to_xyz(p2);
check_gcode_feedrate(result, writer.config, speed);
}
THEN("-XY-Z move feed rate Z component should be equal to travel_speed_z") {
const Vec3d move{-10, 10, -10};
const Vec3d move_u = move / move.norm();
const double speed = std::abs(writer.config.travel_speed_z.value / move_u.z());
const Vec3d p1 = writer.get_position();
const Vec3d p2 = p1 + move;
const std::string result = writer.travel_to_xyz(p2);
check_gcode_feedrate(result, writer.config, speed);
}
THEN("X-Y-Z move feed rate Z component should be equal to travel_speed_z") {
const Vec3d move{10, -10, -10};
const Vec3d move_u = move / move.norm();
const double speed = std::abs(writer.config.travel_speed_z.value / move_u.z());
const Vec3d p1 = writer.get_position();
const Vec3d p2 = p1 + move;
const std::string result = writer.travel_to_xyz(p2);
check_gcode_feedrate(result, writer.config, speed);
}
THEN("-X-Y-Z move feed rate Z component should be equal to travel_speed_z") {
const Vec3d move{-10, -10, -10};
const Vec3d move_u = move / move.norm();
const double speed = std::abs(writer.config.travel_speed_z.value / move_u.z());
const Vec3d p1 = writer.get_position();
const Vec3d p2 = p1 + move;
const std::string result = writer.travel_to_xyz(p2);
check_gcode_feedrate(result, writer.config, speed);
}
}
}
}
TEST_CASE("GCodeWriter emits G1 code correctly according to XYZF_EXPORT_DIGITS", "[GCodeWriter]") {
GCodeWriter writer;

View File

@ -5,8 +5,9 @@
#include <catch2/catch_test_macros.hpp>
#include <catch2/catch_approx.hpp>
#include <libslic3r/GCodeReader.hpp>
#include <libslic3r/Config.hpp>
#include "libslic3r/GCodeReader.hpp"
#include "libslic3r/GCode/GCodeWriter.hpp"
#include "libslic3r/Config.hpp"
#include "test_data.hpp"
#include <regex>
@ -61,6 +62,10 @@ void check_gcode(std::initializer_list<TestMesh> meshes, const DynamicPrintConfi
const double retract_restart_extra = config.option<ConfigOptionFloats>("retract_restart_extra")->get_at(tool);
const double retract_restart_extra_toolchange = config.option<ConfigOptionFloats>("retract_restart_extra_toolchange")->get_at(tool);
const double travel_speed = config.opt_float("travel_speed");
const double feedrate = line.has_f() ? line.f() : self.f();
if (line.dist_Z(self) != 0) {
// lift move or lift + change layer
const double retract_lift = config.option<ConfigOptionFloats>("retract_lift")->get_at(tool);
@ -89,9 +94,26 @@ void check_gcode(std::initializer_list<TestMesh> meshes, const DynamicPrintConfi
lift_dist = 0;
lifted = false;
}
const double feedrate = line.has_f() ? line.f() : self.f();
INFO("move Z at travel speed");
CHECK(feedrate == Approx(config.opt_float("travel_speed") * 60));
const double travel_speed_z = config.opt_float("travel_speed_z");
if (travel_speed_z) {
Vec3d move{line.dist_X(self), line.dist_Y(self), line.dist_Z(self)};
const double move_u_z = move.z() / move.norm();
const double travel_speed_ = std::abs(travel_speed_z / move_u_z);
INFO("move Z feedrate Z component is less than or equal to travel_speed_z");
CHECK(feedrate * std::abs(move_u_z) <= Approx(travel_speed_z * 60).epsilon(GCodeFormatter::XYZ_EPSILON));
if (travel_speed_ < travel_speed) {
INFO("move Z at travel speed Z");
CHECK(feedrate == Approx(travel_speed_ * 60).epsilon(GCodeFormatter::XYZ_EPSILON));
INFO("move Z feedrate Z component is equal to travel_speed_z");
CHECK(feedrate * std::abs(move_u_z) == Approx(travel_speed_z * 60).epsilon(GCodeFormatter::XYZ_EPSILON));
} else {
INFO("move Z at travel speed");
CHECK(feedrate == Approx(travel_speed * 60).epsilon(GCodeFormatter::XYZ_EPSILON));
}
} else {
INFO("move Z at travel speed");
CHECK(feedrate == Approx(travel_speed * 60).epsilon(GCodeFormatter::XYZ_EPSILON));
}
}
if (line.retracting(self)) {
retracted[tool] = true;
@ -144,7 +166,7 @@ void test_slicing(std::initializer_list<TestMesh> meshes, DynamicPrintConfig& co
}
TEST_CASE("Slicing with retraction and lifing", "[retraction]") {
TEST_CASE("Slicing with retraction and lifting", "[retraction]") {
DynamicPrintConfig config = Slic3r::DynamicPrintConfig::full_print_config();
config.set_deserialize_strict({
{ "nozzle_diameter", "0.6,0.6,0.6,0.6" },
@ -173,6 +195,37 @@ TEST_CASE("Slicing with retraction and lifing", "[retraction]") {
}
}
TEST_CASE("Slicing with retraction and lifting with travel_speed_z=10", "[retraction]") {
DynamicPrintConfig config = Slic3r::DynamicPrintConfig::full_print_config();
config.set_deserialize_strict({
{ "nozzle_diameter", "0.6,0.6,0.6,0.6" },
{ "first_layer_height", config.opt_float("layer_height") },
{ "first_layer_speed", "100%" },
{ "start_gcode", "" }, // To avoid dealing with the nozzle lift in start G-code
{ "retract_length", "1.5" },
{ "retract_before_travel", "3" },
{ "retract_layer_change", "1" },
{ "only_retract_when_crossing_perimeters", 0 },
{ "travel_speed", "600" },
{ "travel_speed_z", "10" },
});
SECTION("Standard run") {
test_slicing({TestMesh::cube_20x20x20}, config);
}
SECTION("With duplicate cube") {
test_slicing({TestMesh::cube_20x20x20}, config, 2);
}
SECTION("Dual extruder with multiple skirt layers") {
config.set_deserialize_strict({
{"infill_extruder", 2},
{"skirts", 4},
{"skirt_height", 3},
});
test_slicing({TestMesh::cube_20x20x20}, config);
}
}
TEST_CASE("Z moves", "[retraction]") {
DynamicPrintConfig config = Slic3r::DynamicPrintConfig::full_print_config();