-
-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
controller_fan: add ability to watch temperature_sensors too #6479
base: master
Are you sure you want to change the base?
controller_fan: add ability to watch temperature_sensors too #6479
Conversation
Hey @slaesh |
Thank you for submitting a PR, pleas refer to point 3 in "What to expect in a review" in https://github.com/Klipper3d/klipper/blob/master/docs/CONTRIBUTING.md and provide a signed off by line. Thanks |
Hey James, I am happy to do that. But I am not sure exactly what to do 🙈 Where do I need to upload that "signed" file? :) BR Sascha ✌️ |
do I need to add another commit with |
oh I see, the link was not working on my github app this morning. so.. we add this sign-off thingy at the squashed commit or how do I add this kind of stuff for the already done commits? Oo
|
@JamesH1978 done. Just saw another PR which had it in the description text. Hope this does the job ;) |
Just a thought, but would the combined temperature sensor and a regular |
Thank you for your contribution to Klipper. Unfortunately, a reviewer has not assigned themselves to this GitHub Pull Request. All Pull Requests are reviewed before merging, and a reviewer will need to volunteer. Further information is available at: https://www.klipper3d.org/CONTRIBUTING.html There are some steps that you can take now:
Unfortunately, if a reviewer does not assign themselves to this GitHub Pull Request then it will be automatically closed. If this happens, then it is a good idea to move further discussion to the Klipper Discourse server. Reviewers can reach out on that forum to let you know if they are interested and when they are available. Best regards, PS: I'm just an automated script, not a human being. |
Is this not what's already covered with temperature_fan? You can set a temperature_fan to check an mcu temp. Example: [temperature_fan SB2040_Fan] Ignore my PID settings, they're horrible. I was messing around. |
Seems to me the reason is that you cant have a single fan be both a controller_fan and a temperature_fan. |
Hi @slaesh ! Love your Zigbee sticks BTW ) So your solution does not resolve the primary intention to reduce the fan noice: either it will be all the time Full On because of CPU temp, or Full On because of stepper / heater activity. I would suggest much simpler solution as a compromise: allow the "min_speed" or "default_speed" option for [controller_fan] so it will run at a predefined minimal speed when not triggered On by steppers / heaters, just to cool down the controllers and CPU. Otherwise we have to extend the [controller_fan] to a very complex thing with PID controls which can be difficult as we can't get the TMC2209 drivers actual temperature for example. EDIT: Just discovered there were already some similar "min_speed" PRs rejected in 2022, with the following comment from Kevin:
Though I've never heard of fan_templates being introduced since that.... @KevinOConnor, |
Actually we can, with the [duplicate_pin_override] Klipper option. But the problem is - which of them will take precedence over this single fan at specific moment? :) It's like placing a hot iron into the fridge guessing "who will win" :) I have tried this workaround but can't imagine how to check the actual fan speed... |
I searched a way to have a fan controller that just watches one or more temperature sensors.. like host-temperature. since the enclosure fans of my voron are pretty loud.. and I noticed that most of the stuff is not getting hot at all.. so.. having this fan-controller work without the heater/extruder.. and just watch actual temperatures would be awesome! for my ears.. for the fans.. and hopefully for other guys out there ;)
I found this pretty old PR from @mnigbur, which did not make it back then..
just pinging some of the guys who where active in the past in this thread.. @Ramalama2, @theopensourcerer, @meteyou, @edddeduck, @KevinOConnor
question: any chance to test it without running an actual machine? (in general, not for this one =))
To-Do:
Signed-off-by: Sascha Ehlers [email protected]