From 60a8f46ea6ce34bbdbf68ec3d4c753afd5ea0e9a Mon Sep 17 00:00:00 2001 From: Erwan MATHIEU Date: Tue, 2 Jul 2024 15:21:37 +0200 Subject: [PATCH 1/5] Fix fan speed interpolation with wrong setting CURA-6410 --- src/LayerPlan.cpp | 29 +++++++---------------------- 1 file changed, 7 insertions(+), 22 deletions(-) diff --git a/src/LayerPlan.cpp b/src/LayerPlan.cpp index 365d7c2ef1..191fa15442 100644 --- a/src/LayerPlan.cpp +++ b/src/LayerPlan.cpp @@ -1903,7 +1903,8 @@ TimeMaterialEstimates ExtruderPlan::computeNaiveTimeEstimates(Point2LL starting_ void ExtruderPlan::processFanSpeedForMinimalLayerTime(Duration minTime, double time_other_extr_plans) { - /* + /* interpolate fan speed + min layer time : : min layer time fan speed min @@ -1914,28 +1915,12 @@ void ExtruderPlan::processFanSpeedForMinimalLayerTime(Duration minTime, double t speed min..|... \:___________ |________________ layer time > - - */ - // interpolate fan speed (for cool_fan_full_layer and for cool_min_layer_time_fan_speed_max) - double totalLayerTime = estimates_.getTotalTime() + time_other_extr_plans; - if (totalLayerTime < minTime) - { - fan_speed = fan_speed_layer_time_settings_.cool_fan_speed_max; - } - else if (minTime >= fan_speed_layer_time_settings_.cool_min_layer_time_fan_speed_max) - { - // ignore gradual increase of fan speed - return; - } - else if (totalLayerTime < fan_speed_layer_time_settings_.cool_min_layer_time_fan_speed_max) - { - // when forceMinimalLayerTime didn't change the extrusionSpeedFactor, we adjust the fan speed - double fan_speed_diff = fan_speed_layer_time_settings_.cool_fan_speed_max - fan_speed; - double layer_time_diff = fan_speed_layer_time_settings_.cool_min_layer_time_fan_speed_max - minTime; - double fraction_of_slope = (totalLayerTime - minTime) / layer_time_diff; - fan_speed = fan_speed_layer_time_settings_.cool_fan_speed_max - fan_speed_diff * fraction_of_slope; - } + + const double total_layer_time = estimates_.getTotalTime() + time_other_extr_plans; + const double layer_time_diff = fan_speed_layer_time_settings_.cool_min_layer_time_fan_speed_max - minTime; + const double fraction_of_slope = std::clamp((total_layer_time - minTime) / layer_time_diff, 0.0, 1.0); + fan_speed = std::lerp(fan_speed_layer_time_settings_.cool_fan_speed_max, fan_speed, fraction_of_slope); } void ExtruderPlan::processFanSpeedForFirstLayers() From 024ca614db28a3c3cc8c018c7b50351bb6e22de2 Mon Sep 17 00:00:00 2001 From: Erwan MATHIEU Date: Tue, 2 Jul 2024 15:57:47 +0200 Subject: [PATCH 2/5] Write the fan speed value only when really different Previously we were basing the writing or non-writing of the fan speed command based on the raw value comparison. This could result in some commands not being written if the new value was too much similar to the previous one. Now we check the actual output value and base the writing on this. CURA-6410 --- src/gcodeExport.cpp | 77 +++++++++++++++++++++++++++++++-------------- 1 file changed, 53 insertions(+), 24 deletions(-) diff --git a/src/gcodeExport.cpp b/src/gcodeExport.cpp index 43fdc8ba3b..ae83b047b4 100644 --- a/src/gcodeExport.cpp +++ b/src/gcodeExport.cpp @@ -1434,44 +1434,73 @@ void GCodeExport::writeFanCommand(double speed, std::optional extruder) void GCodeExport::writeSpecificFanCommand(double speed, size_t fan_number) { - auto iterator = current_fans_speeds_.find(fan_number); - - if (iterator != current_fans_speeds_.end() && std::abs(iterator->second - speed) < 0.1) - { - return; - } + const auto iterator = current_fans_speeds_.find(fan_number); + const std::optional current_fan_speed = (iterator != current_fans_speeds_.end()) ? std::optional(iterator->second) : std::nullopt; if (flavor_ == EGCodeFlavor::MAKERBOT) { - if (speed >= 50) + // Makerbot cannot PWM the fan speed, only turn it on or off + + bool write_value = true; + const bool new_on = speed >= 50; + if (current_fan_speed.has_value()) { - *output_stream_ << "M126 T0" << new_line_; // Makerbot cannot PWM the fan speed... + const bool old_on = current_fan_speed.value() >= 50; + write_value = new_on != old_on; } - else + + if (write_value) { - *output_stream_ << "M127 T0" << new_line_; + if (new_on) + { + *output_stream_ << "M126 T0" << new_line_; + } + else + { + *output_stream_ << "M127 T0" << new_line_; + } } } - else if (speed > 0) + else { const bool should_scale_zero_to_one = Application::getInstance().current_slice_->scene.settings.get("machine_scale_fan_speed_zero_to_one"); - *output_stream_ << "M106 S" - << PrecisionedDouble{ (should_scale_zero_to_one ? static_cast(2) : static_cast(1)), - (should_scale_zero_to_one ? speed : speed * 255) / 100 }; - if (fan_number) + bool write_value = true; + std::ostringstream new_value; + new_value << PrecisionedDouble{ (should_scale_zero_to_one ? static_cast(2) : static_cast(1)), (should_scale_zero_to_one ? speed : speed * 255) / 100 }; + const std::string new_value_str = new_value.str(); + if (current_fan_speed.has_value()) { - *output_stream_ << " P" << fan_number; + std::ostringstream old_value; + old_value << PrecisionedDouble{ (should_scale_zero_to_one ? static_cast(2) : static_cast(1)), + (should_scale_zero_to_one ? current_fan_speed.value() : current_fan_speed.value() * 255) / 100 }; + + write_value = new_value_str != old_value.str(); } - *output_stream_ << new_line_; - } - else - { - *output_stream_ << "M107"; - if (fan_number) + + if (write_value) { - *output_stream_ << " P" << fan_number; + // Check if the value to be written is only made with zeroes, in which case it is actually a turn off + std::string new_value_str_reduced = new_value_str; + std::erase(new_value_str_reduced, '0'); + std::erase(new_value_str_reduced, '.'); + const bool turn_off = new_value_str_reduced.empty(); + + if (turn_off) + { + *output_stream_ << "M107"; + } + else + { + *output_stream_ << "M106 S" << new_value_str; + } + + if (fan_number) + { + *output_stream_ << " P" << fan_number; + } + + *output_stream_ << new_line_; } - *output_stream_ << new_line_; } current_fans_speeds_[fan_number] = speed; From 3435b6d952d30bd53cc8700c2b79c2a44583905d Mon Sep 17 00:00:00 2001 From: Remco Burema Date: Thu, 4 Jul 2024 17:31:48 +0200 Subject: [PATCH 3/5] Apply DRY to fan speed 'should scale zero to one' functionality. Some printers use 0.00-1.00 others use 0.0-256.0 as scale for their fans. This was (nearly) duplicated earlier in the PR. done as part of CURA-6410 --- src/gcodeExport.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/gcodeExport.cpp b/src/gcodeExport.cpp index ae83b047b4..d395b5037d 100644 --- a/src/gcodeExport.cpp +++ b/src/gcodeExport.cpp @@ -1434,6 +1434,8 @@ void GCodeExport::writeFanCommand(double speed, std::optional extruder) void GCodeExport::writeSpecificFanCommand(double speed, size_t fan_number) { + + const auto iterator = current_fans_speeds_.find(fan_number); const std::optional current_fan_speed = (iterator != current_fans_speeds_.end()) ? std::optional(iterator->second) : std::nullopt; @@ -1464,16 +1466,17 @@ void GCodeExport::writeSpecificFanCommand(double speed, size_t fan_number) else { const bool should_scale_zero_to_one = Application::getInstance().current_slice_->scene.settings.get("machine_scale_fan_speed_zero_to_one"); + const auto scale_zero_to_one_optional = [should_scale_zero_to_one](double value) -> PrecisionedDouble { + return { (should_scale_zero_to_one ? static_cast(2) : static_cast(1)), (should_scale_zero_to_one ? value : value * 255.0) / 100.0 }; + }; bool write_value = true; std::ostringstream new_value; - new_value << PrecisionedDouble{ (should_scale_zero_to_one ? static_cast(2) : static_cast(1)), (should_scale_zero_to_one ? speed : speed * 255) / 100 }; + new_value << scale_zero_to_one_optional(speed); const std::string new_value_str = new_value.str(); if (current_fan_speed.has_value()) { std::ostringstream old_value; - old_value << PrecisionedDouble{ (should_scale_zero_to_one ? static_cast(2) : static_cast(1)), - (should_scale_zero_to_one ? current_fan_speed.value() : current_fan_speed.value() * 255) / 100 }; - + old_value << scale_zero_to_one_optional(current_fan_speed.value()); write_value = new_value_str != old_value.str(); } From c750ec6755ba0347806f33ada167a389cb24f34e Mon Sep 17 00:00:00 2001 From: Remco Burema Date: Thu, 4 Jul 2024 17:53:02 +0200 Subject: [PATCH 4/5] Replace string manipulation with numeric calculation. done as part of CURA-6410 --- include/utils/string.h | 6 ++++++ src/gcodeExport.cpp | 12 ++++-------- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/include/utils/string.h b/include/utils/string.h index 90ddc318fd..7468077279 100644 --- a/include/utils/string.h +++ b/include/utils/string.h @@ -4,6 +4,7 @@ #ifndef UTILS_STRING_H #define UTILS_STRING_H +#include #include // sprintf #include #include // ostringstream @@ -169,6 +170,11 @@ struct PrecisionedDouble uint8_t precision; //!< Number of digits after the decimal mark with which to convert to string double value; //!< The double value + bool wouldWriteZero() const + { + return (std::abs(value) * std::pow(10.0, precision)) < 1.0; + } + friend inline std::ostream& operator<<(std::ostream& out, const PrecisionedDouble precision_and_input) { writeDoubleToStream(precision_and_input.precision, precision_and_input.value, out); diff --git a/src/gcodeExport.cpp b/src/gcodeExport.cpp index d395b5037d..329f5e7129 100644 --- a/src/gcodeExport.cpp +++ b/src/gcodeExport.cpp @@ -1471,7 +1471,8 @@ void GCodeExport::writeSpecificFanCommand(double speed, size_t fan_number) }; bool write_value = true; std::ostringstream new_value; - new_value << scale_zero_to_one_optional(speed); + const auto num_new_val = scale_zero_to_one_optional(speed); + new_value << num_new_val; const std::string new_value_str = new_value.str(); if (current_fan_speed.has_value()) { @@ -1482,14 +1483,9 @@ void GCodeExport::writeSpecificFanCommand(double speed, size_t fan_number) if (write_value) { - // Check if the value to be written is only made with zeroes, in which case it is actually a turn off - std::string new_value_str_reduced = new_value_str; - std::erase(new_value_str_reduced, '0'); - std::erase(new_value_str_reduced, '.'); - const bool turn_off = new_value_str_reduced.empty(); - - if (turn_off) + if (num_new_val.wouldWriteZero()) { + // Turn off when the fan value is zero. *output_stream_ << "M107"; } else From 9040b6a7262a2768df709c712005084877c8c452 Mon Sep 17 00:00:00 2001 From: rburema Date: Thu, 4 Jul 2024 15:57:06 +0000 Subject: [PATCH 5/5] Applied clang-format. --- src/gcodeExport.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/gcodeExport.cpp b/src/gcodeExport.cpp index 329f5e7129..1f783bf106 100644 --- a/src/gcodeExport.cpp +++ b/src/gcodeExport.cpp @@ -1434,8 +1434,6 @@ void GCodeExport::writeFanCommand(double speed, std::optional extruder) void GCodeExport::writeSpecificFanCommand(double speed, size_t fan_number) { - - const auto iterator = current_fans_speeds_.find(fan_number); const std::optional current_fan_speed = (iterator != current_fans_speeds_.end()) ? std::optional(iterator->second) : std::nullopt; @@ -1466,9 +1464,10 @@ void GCodeExport::writeSpecificFanCommand(double speed, size_t fan_number) else { const bool should_scale_zero_to_one = Application::getInstance().current_slice_->scene.settings.get("machine_scale_fan_speed_zero_to_one"); - const auto scale_zero_to_one_optional = [should_scale_zero_to_one](double value) -> PrecisionedDouble { + const auto scale_zero_to_one_optional = [should_scale_zero_to_one](double value) -> PrecisionedDouble + { return { (should_scale_zero_to_one ? static_cast(2) : static_cast(1)), (should_scale_zero_to_one ? value : value * 255.0) / 100.0 }; - }; + }; bool write_value = true; std::ostringstream new_value; const auto num_new_val = scale_zero_to_one_optional(speed);