Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dynamic perimeter fan speeds are being applied to infill #11648

Closed
2 tasks done
jschuh opened this issue Nov 12, 2023 · 5 comments
Closed
2 tasks done

Dynamic perimeter fan speeds are being applied to infill #11648

jschuh opened this issue Nov 12, 2023 · 5 comments

Comments

@jschuh
Copy link
Contributor

jschuh commented Nov 12, 2023

Description of the bug

It looks like the fan speed is not getting reset when an external perimeter completes, so the last dynamic fan speed ends up getting applied to all of the infill. In the picture below the infill should be dark blue (no fan), but you can see that it's instead using the fan speed from the perimeter seam.

image

This showed up in the first 2.7 alpha, and continues to be present in the most recent beta.

Project file & How to reproduce

This is a simple test file that I used to generate the image above: external_perimeter_fan_test.zip

It just uses an extreme set of of fan speeds to make it easier to visualize what's happening.

Checklist of files included above

  • Project file
  • Screenshot

Version of PrusaSlicer

Version 2.7.0-beta1

Operating system

Windows 11 (but it's an OS independent issue)

Printer model

Prusa MK3S (but it's a printer independent issue)

@jschuh
Copy link
Contributor Author

jschuh commented Nov 12, 2023

I nerd-sniped myself into taking a look. The problem is here:

if (m_enable_cooling_markers)
gcode += path_attr.role.is_bridge() ? ";_BRIDGE_FAN_END" : ";_EXTRUDE_END";
if (dynamic_speed_and_fan_speed.second >= 0)
gcode += ";_RESET_FAN_SPEED";

Since a fan reset always happens at the end of an extrusion, the resulting string looks like ";_EXTRUDE_END;_RESET_FAN_SPEED". So in CoolingBuffer::parse_layer_gcode it always hits this path:

} else if (boost::starts_with(sline, ";_EXTRUDE_END")) {

And thus this later conditional is just unreachable code:
} else if (boost::contains(sline, ";_RESET_FAN_SPEED")) {

Given that the cooling buffer code already uses bit flags to manage this state, it sure seems like the set/reset flags need only be conditioned on each other, and not the rest of the flag state. In my testing that change produces correct output, so I'll post a PR.

@jschuh
Copy link
Contributor Author

jschuh commented Nov 12, 2023

I just posted PR #11650. This is what it looks like with the PR applied, which is also how the same file looks in version 2.6.

image

@jschuh
Copy link
Contributor Author

jschuh commented Nov 13, 2023

I poked around a bit more and found that the regression was introduced in fa0986c. Previously the ";_RESET_FAN_SPEED" was always wrapped in newlines. So, one potential fix is to just add the newlines back, but that seems more fragile to me than updating the flag setting in CoolingBuffer.cpp. Although, I tested and verified that either approach works.

@hejllukas
Copy link
Collaborator

I merged your PR (with the modification that I mentioned there) into our private repository, so the commit will appear with the next release.
Thank you for detailed investigation of the issue and proposed ways to fix it.

@lukasmatena
Copy link
Collaborator

Fixed in 2.7.0-rc1. Closing.
Thanks to @jschuh for the fix provided in #11650).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants