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

Multi Part Cooling Fans With M106 & M107 #25808

Merged
merged 5 commits into from
May 13, 2023

Conversation

Vertabreak
Copy link
Contributor

Expand M106 & M107 with the ability to use up to 3 pwm fans for part cooling and add defines to switch on/off.
I have encountered a need for this feature a number of times and feel its time it become available to everyone.

pwm fans for part cooling and add defines to switch on/off
@ellensp
Copy link
Contributor

ellensp commented May 11, 2023

we already have 1 additional fan

/**
 * Use one of the PWM fans as a redundant part-cooling fan
 */
//#define REDUNDANT_PART_COOLING_FAN 2  // Index of the fan to sync with FAN 0.

@Vertabreak
Copy link
Contributor Author

the comment for this define was so vague i had no idea that is was it was for, will see if i can expand this feature instead.

@ellensp
Copy link
Contributor

ellensp commented May 11, 2023

I agree the name is not good... it is not what i called it in when i added it ... (DUAL_PART_COOLING_FANS) back n #21888

@Vertabreak
Copy link
Contributor Author

i started re-coding using the noted feature and seem to have found the feature to be kinda broken due to the way fan numbering handled. you should be able to have dual part cooler with fan0 and fan1 headers but it was written so that fan2 is required for dual. i had to comment out part of the sanity check due to the invalid numbering and adjust a few bits to correct the numbering. once i look over the changes again ill update this PR. additional testing will be needed for sure.

@thinkyhead
Copy link
Member

If this PR is just meant to augment REDUNDANT_PART_COOLING_FAN then perhaps that option should simply be renamed to REDUNDANT_PART_COOLING_FANS and changed into an array of indexes, indicating the fan pins as defined by pins files.

Alternatively, we could add a bit more control over M106 by having a single option such as…

#define PRINT_COOLING_FANS { { 0, 2 }, { 3, 5 } }

…that specifies which fan indexes (as defined by pins files) are controlled and synchronized together via M106 P. The default, if not defined, would simply allocate one fan per carriage, as in…

#ifndef PRINT_COOLING_FANS
  #define PRINT_COOLING_FANS { { 0 }, { 1 } ... }
#endif

The option would be error-checked to make sure there are no collisions with fan indexes that have been assigned for other purposes.

@thinkyhead thinkyhead force-pushed the PR branch 4 times, most recently from fbc2cd2 to 7376090 Compare May 11, 2023 16:44
@thinkyhead
Copy link
Member

thinkyhead commented May 11, 2023

I fixed up some things:

  • Define NUM_REDUNDANT_FANS to better fit our naming paradigm.
  • Use #ifdef REDUNDANT_PART_COOLING_FAN because ENABLED doesn't work that way.
  • Remove code setting redundant speeds from M106/M107 because Temperature::set_fan_speed already handles REDUNDANT_PART_COOLING_FAN.
  • Added a fallback to define NUM_REDUNDANT_FANS as 1 when not defined.
  • Fixed the sanity checks.
  • Added macro FAN_IS_M106ABLE for use by M106/M107 and MarlinUI menu items to skip redundant fans that synchronize with Fan "0".

@thinkyhead
Copy link
Member

Currently, the REDUNDANT_PART_COOLING_FAN feature is tailored only to a setup with a single extruder, so in the long run it needs to be reworked for DUAL_X_CARRIAGE ("IDEX") setups that need extra fans. The suggestion above to define PART_COOLING_FANS as a nested array of fan indexes could work for that refactor. Before undertaking that refactor I'd like to wait until after 2.1.3 and the completion of #24092, which still needs some work to get fan kickstart solved and keep the build size small.

thinkyhead added a commit to MarlinFirmware/Configurations that referenced this pull request May 13, 2023
@thinkyhead thinkyhead merged commit 5859ff0 into MarlinFirmware:bugfix-2.1.x May 13, 2023
oponyx pushed a commit to oponyx/Marlin that referenced this pull request May 14, 2023
Co-authored-by: Scott Lahteine <thinkyhead@users.noreply.github.com>
oponyx pushed a commit to oponyx/Marlin that referenced this pull request May 14, 2023
Co-authored-by: Scott Lahteine <thinkyhead@users.noreply.github.com>
EvilGremlin pushed a commit to EvilGremlin/Marlin that referenced this pull request May 17, 2023
Co-authored-by: Scott Lahteine <thinkyhead@users.noreply.github.com>
@Vertabreak Vertabreak deleted the PR branch May 17, 2023 20:23
tspiva pushed a commit to tspiva/Marlin that referenced this pull request May 25, 2023
Co-authored-by: Scott Lahteine <thinkyhead@users.noreply.github.com>
Andy-Big pushed a commit to Andy-Big/Marlin_FB_Reborn that referenced this pull request Jul 15, 2023
Co-authored-by: Scott Lahteine <thinkyhead@users.noreply.github.com>
Andy-Big pushed a commit to Andy-Big/Marlin_FB_Reborn that referenced this pull request Jul 18, 2023
Co-authored-by: Scott Lahteine <thinkyhead@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants